Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue with AWS API Gateway signed requests in k6 tests #96

Closed
rgrygorovych opened this issue Mar 26, 2024 · 7 comments · Fixed by #97
Closed

Issue with AWS API Gateway signed requests in k6 tests #96

rgrygorovych opened this issue Mar 26, 2024 · 7 comments · Fixed by #97
Assignees
Labels

Comments

@rgrygorovych
Copy link
Contributor

Dear Team,

We are currently working on integrating k6 tests with our private AWS API Gateway. While doing so, we encountered an issue with performing signed requests using k6 internal libraries.

After some investigation, we found a workaround by modifying the signature implementation in the signature.ts file. Specifically, we changed the request header to accommodate a different hostname value compared to the host value in the header.

The original line in signature.ts:

request.headers[constants.HOST_HEADER] = request.endpoint.hostname

was replaced with:

request.headers[constants.HOST_HEADER] = request.headers.host

This change allowed us to successfully perform requests from the open-source k6 tool to our API. Currently, we have forked the k6-jslib-aws repository. However, we would like to seek clarification on whether this modification indicates an issue in the implementation. Should the change be made on your end, or is there something we may be doing incorrectly on our side?

Your guidance on this matter would be greatly appreciated.

@joanlopez
Copy link
Contributor

joanlopez commented Mar 26, 2024

Hey @rgrygorovych,

First of all, thanks so much for bringing this topic! 🙏🏻

The original line in signature.ts:

request.headers[constants.HOST_HEADER] = request.endpoint.hostname

was replaced with:

request.headers[constants.HOST_HEADER] = request.headers.host

Looking at the line of code you pointed out, and comparing it to the one you provided, I suspect the problem, in your case, is that the original value of the Host header is being overwritten by request.endpoint.hostname, because I think the line you suggest does nothing: aren't request.headers[constants.HOST_HEADER] and request.headers.host the same thing?

If so, can you confirm that the header value (request.headers[constants.HOST_HEADER]) and the endpoint's hostname (request.endpoint.hostname) are different in your case? Is that expected for some reason? Can you bring further details?

In such case, I'm wondering whether would make sense to slightly modify the current code to only set the value of request.headers[constants.HOST_HEADER] when empty (not set), otherwise just leave the existing value.

So, something like:

if (!request.headers[constants.HOST_HEADER]) {
    request.headers[constants.HOST_HEADER] = request.endpoint.hostname;
}

Do you think that would make sense? Could you verify that would work in your case?
cc/ @oleiade, as you may have more context.

Thanks! 🙇🏻

@oleiade
Copy link
Member

oleiade commented Mar 26, 2024

Out of the blue with my "cache" cold, I think the solution you proposed @joanlopez makes sense indeed. I'd be interested in learning wether that addresses your problem indeed @rgrygorovych.

For context the sign and presign methods were heavily inspired by the implementation the aws-sdk-js v3 had at the time. Unfortunately, it looks like AWS switched to code generation in the meantime, and I can't find that reference code anymore.

Furthermore, we adjusted it to use the request.endpoint instead, in order to cater to users with an "alternative" backend (minIO, digital ocean spaces, etc).

I think we should be open to making changes to it indeed, but we should provide tests ensuring it passes the signature test suite (also ported from the initial AWS implementation), because the SignatureV4 class is key to all our client classes 👍🏻

@rgrygorovych
Copy link
Contributor Author

rgrygorovych commented Mar 26, 2024

Hello @oleiade and @joanlopez here is more details related to this, hope you find it helpful.

Context: The team is working on accessing API endpoints that require a signature through a VPC endpoint. The following log outputs illustrate the issue and the suggested change:

Code Before Override:


        console.log("BEFORE")
        console.log("request.headers[constants.HOST_HEADER] = ", request.headers[constants.HOST_HEADER]);
        console.log("request.endpoint.hostname = ", request.endpoint.hostname);
        console.log("request.headers.host = ", request.headers.host);
        
        request.headers[constants.HOST_HEADER] = request.endpoint.hostname

        console.log("AFTER")
        console.log("request.headers[constants.HOST_HEADER] = ", request.headers[constants.HOST_HEADER]);
        console.log("request.endpoint.hostname = ", request.endpoint.hostname);
        console.log("request.headers.host = ", request.headers.host);

...

Result with failing POST Requests:

time="2024-03-26T19:09:47Z" level=info msg=BEFORE source=console
time="2024-03-26T19:09:47Z" level=info msg="request.headers[constants.HOST_HEADER] =  xxxxxxxxxx.execute-api.us-east-1.amazonaws.com" source=console
time="2024-03-26T19:09:47Z" level=info msg="request.endpoint.hostname =  vpce-yyyyyyyyy-zzzzzzz.execute-api.us-east-1.vpce.amazonaws.com" source=console
time="2024-03-26T19:09:47Z" level=info msg="request.headers.host =  xxxxxxxxxx.execute-api.us-east-1.amazonaws.com" source=console
time="2024-03-26T19:09:47Z" level=info msg=AFTER source=console
time="2024-03-26T19:09:47Z" level=info msg="request.headers[constants.HOST_HEADER] =  vpce-yyyyyyyyy-zzzzzzz.execute-api.us-east-1.vpce.amazonaws.com" source=console
time="2024-03-26T19:09:47Z" level=info msg="request.endpoint.hostname =  vpce-yyyyyyyyy-zzzzzzz.execute-api.us-east-1.vpce.amazonaws.com" source=console
time="2024-03-26T19:09:47Z" level=info msg="request.headers.host =  vpce-yyyyyyyyy-zzzzzzz.execute-api.us-east-1.vpce.amazonaws.com" source=console

Another execution with the proposed change:

Code After Override:

        console.log("BEFORE")
        console.log("request.headers[constants.HOST_HEADER] = ", request.headers[constants.HOST_HEADER]);
        console.log("request.endpoint.hostname = ", request.endpoint.hostname);
        console.log("request.headers.host = ", request.headers.host);
        
        // Change to accommodate for hostname to be different from host value in header 
        if (!request.headers[constants.HOST_HEADER]) {
            request.headers[constants.HOST_HEADER] = request.endpoint.hostname;
        }
        
        console.log("AFTER")
        console.log("request.headers[constants.HOST_HEADER] = ", request.headers[constants.HOST_HEADER]);
        console.log("request.endpoint.hostname = ", request.endpoint.hostname);
        console.log("request.headers.host = ", request.headers.host);
...

Result with passing POST Requests:

time="2024-03-26T19:05:21Z" level=info msg=BEFORE source=console
time="2024-03-26T19:05:21Z" level=info msg="request.headers[constants.HOST_HEADER] =  xxxxxxxxxx.execute-api.us-east-1.amazonaws.com" source=console
time="2024-03-26T19:05:21Z" level=info msg="request.endpoint.hostname =  vpce-yyyyyyyyy-zzzzzzz.execute-api.us-east-1.vpce.amazonaws.com" source=console
time="2024-03-26T19:05:21Z" level=info msg="request.headers.host =  xxxxxxxxxx.execute-api.us-east-1.amazonaws.com" source=console
time="2024-03-26T19:05:21Z" level=info msg=AFTER source=console
time="2024-03-26T19:05:21Z" level=info msg="request.headers[constants.HOST_HEADER] =  xxxxxxxxxx.execute-api.us-east-1.amazonaws.com" source=console
time="2024-03-26T19:05:21Z" level=info msg="request.endpoint.hostname =  vpce-yyyyyyyyy-zzzzzzz.execute-api.us-east-1.vpce.amazonaws.com" source=console
time="2024-03-26T19:05:21Z" level=info msg="request.headers.host =  xxxxxxxxxx.execute-api.us-east-1.amazonaws.com" source=console

Summary: Requests were failing before the change but succeeded after the change was applied.

We were inspired by the following reference materials:

@joanlopez
Copy link
Contributor

Great! Thanks for such a detailed summary @rgrygorovych!

Do you want to open the pull request with the change agreed? I think you did the hard work, so you definitely deserve the credits for that contribution! 🙇🏻

@rgrygorovych
Copy link
Contributor Author

rgrygorovych commented Mar 27, 2024

Great! Thanks for such a detailed summary @rgrygorovych!

Do you want to open the pull request with the change agreed? I think you did the hard work, so you definitely deserve the credits for that contribution! 🙇🏻

sure, just need permissions :).

@joanlopez
Copy link
Contributor

sure, just need permissions :).

Can you fork it and create the PR from your own fork, please? 🙏🏻

rgrygorovych added a commit to EpisourceLLC/k6-jslib-aws that referenced this issue Mar 27, 2024
@rgrygorovych
Copy link
Contributor Author

sure, just need permissions :).

Can you fork it and create the PR from your own fork, please? 🙏🏻

Here is link #97

rgrygorovych added a commit to EpisourceLLC/k6-jslib-aws that referenced this issue Mar 27, 2024
joanlopez pushed a commit that referenced this issue Apr 2, 2024
* resolves #96

* added tests to reproduce issue #96

* executed webpack
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants