Continue feedback on AWS signing

[alanbrent]

I wouldn't call this ready to go just yet, but this is the first setup that actually works (for Sagemaker calls, at least).

There's still some work to be done to iron out the API. Perhaps we need to take a full endpoint (scheme://fqdn/path?query) as well as an AWS service name (e.g. sagemaker). I'm not sure.

In the current iteration, we are dropping a lot more requests than in my fork that uses the AWS go library for signing instead.

Load test using this JS library for request signing

data_received..................: 7.6 GB  13 MB/s
data_sent......................: 4.7 GB  7.9 MB/s
dropped_iterations.............: 4570646 7615.553268/s
http_req_blocked...............: avg=82.84µs  min=1.74µs  med=3.73µs   max=1.16s    p(90)=4.83µs   p(95)=5.32µs  
http_req_connecting............: avg=38.91µs  min=0s      med=0s       max=1.09s    p(90)=0s       p(95)=0s      
http_req_duration..............: avg=129.21ms min=1.55ms  med=136.42ms max=2.39s    p(90)=240.06ms p(95)=267.66ms
  { expected_response:true }...: avg=175.84ms min=3.8ms   med=176.98ms max=2.39s    p(90)=255.52ms p(95)=282.34ms
http_req_failed................: 31.89%  ✓ 774314 ✗ 1653475
http_req_receiving.............: avg=253.13µs min=13.42µs med=46.76µs  max=774.37ms p(90)=137.94µs p(95)=227.05µs
http_req_sending...............: avg=3.57ms   min=11.84µs med=21.96µs  max=508.03ms p(90)=115.46µs p(95)=10.52ms 
http_req_tls_handshaking.......: avg=33.99µs  min=0s      med=0s       max=486.81ms p(90)=0s       p(95)=0s      
http_req_waiting...............: avg=125.38ms min=1.51ms  med=133.96ms max=2.39s    p(90)=233.99ms p(95)=258.39ms
http_reqs......................: 2427789 4045.151704/s
iteration_duration.............: avg=153.76ms min=2.22ms  med=161.21ms max=2.42s    p(90)=274.46ms p(95)=320.53ms
iterations.....................: 2427789 4045.151704/s
vus............................: 1000    min=404  max=1000 
vus_max........................: 1000    min=404  max=1000 

Load test using AWS Go SDK for request signing

data_received..............: 2.8 GB  4.7 MB/s
data_sent..................: 7.7 GB  13 MB/s
dropped_iterations.........: 358253  597.013883/s
http_req_blocked...........: avg=14.81µs min=764ns   med=2.63µs  max=1.09s    p(90)=3.76µs  p(95)=4.36µs  
http_req_connecting........: avg=5.18µs  min=0s      med=0s      max=1s       p(90)=0s      p(95)=0s      
http_req_duration..........: avg=6.59ms  min=1.4ms   med=1.79ms  max=1.76s    p(90)=19.86ms p(95)=34.17ms 
http_req_receiving.........: avg=52.85µs min=11.75µs med=29.85µs max=211.34ms p(90)=53µs    p(95)=70.28µs 
http_req_sending...........: avg=947.8µs min=5.33µs  med=17.4µs  max=200.27ms p(90)=80.4µs  p(95)=398.59µs
http_req_tls_handshaking...: avg=5.63µs  min=0s      med=0s      max=212.08ms p(90)=0s      p(95)=0s      
http_req_waiting...........: avg=5.59ms  min=1.36ms  med=1.72ms  max=1.76s    p(90)=16.41ms p(95)=29.77ms 
http_reqs..................: 6641491 11067.771472/s
iteration_duration.........: avg=7.6ms   min=1.49ms  med=1.94ms  max=2.05s    p(90)=22.54ms p(95)=39.51ms 
iterations.................: 6641491 11067.771472/s
vus........................: 1000    min=57 max=1000
vus_max....................: 1000    min=57 max=1000

@mstoykov

[alanbrent]

We need to move the sort() to occur after the toLowerCase() call in the block, as the case change modifies the sorting outcome.

[alanbrent]

Same story here, re: moving the sort()

[alanbrent]

These are both required at least for Sagemaker requests.

[alanbrent]

From https://docs.aws.amazon.com/general/latest/gr/sigv4-create-canonical-request.html

You can use temporary security credentials provided by the AWS Security Token Service (AWS STS) to sign a request. The process is the same as using long-term credentials, but when you add signing information to the query string you must add an additional query parameter for the security token. The parameter name is X-Amz-Security-Token, and the parameter's value is the URI-encoded session token (the string you received from AWS STS when you obtained temporary security credentials).

For some services, you must include the X-Amz-Security-Token query parameter in the canonical (signed) query string. For other services, you add the X-Amz-Security-Token parameter at the end, after you calculate the signature. For details, see the API reference documentation for that service.

[alanbrent]

This is an example of what I mean when I say the API still needs to be ironed out. Sagemaker URLs use runtime.sagemaker.${REGION}.amazonaws.com, but the service name for the purposes of request signing is simply sagemaker.

[mstoykov]

I haven't looked at every AWS service and how it handles it, but IMO this should just be one more parameter:

1. we have the service which is what svc is here
2. serviceSubdomain which if not set is just equal to service

And then for the services that have this be different the user will need to provide both or do a wrapper, that can then be added to the jslib ;)

[alanbrent]

From https://docs.aws.amazon.com/general/latest/gr/sigv4-add-signature-to-request.html#sigv4-add-signature-auth-header

There is no comma between the algorithm and Credential. However, the SignedHeaders and Signature are separated from the preceding values with a comma.

[mstoykov]

Is there a particular error that is being returned?

Looking at data_received seem like k6 has received a lot more data for a lot less requests?

Also, the http_req_duration for the jslib is considerably higher, but looking at your code there doesn't seem any reason for that to be true unless we are sending very different requests from both of them 🤔

[tom-miseur]

I now have a working Lambda example that uses a real endpoint with this library, after also having done so with a browserified aws4.js. Running the example script will require access to the endpoint, and I'm not sure about making it public, so not sure whether it would be appropriate to include as a test for Lambda. WDYT?

Incidentally, I also tested it with #11, not realizing that the code has progressed a bit since then.

This PR allowed me to skip passing in any headers to signWithHeaders:

const method = 'POST';
const service = 'lambda';
const region = 'us-west-2';

const path = '/2015-03-31/functions/hello-world/invocations';
const body = '{"key1":"value1","key2":"value2","key3":"value3"}';

// these used to be necessary
// const headers = {
//   'Content-Type': 'application/x-www-form-urlencoded; charset=utf-8',
//   'Content-Length': body.length.toString(),
//   'Host': `${service}.${region}.amazonaws.com`,
//   'X-Amz-Date': new Date().toISOString().replace(/[:\-]|\.\d{3}/g, '')
// }

const signed = signWithHeaders(
  method, 
  service, 
  region, 
  "", 
  path, 
  body, 
  "", 
  // headers
  {}
);

This also resulted in less headers being used for the actual request signing (SignedHeaders), which the endpoint didn't complain about (I believe Host and Date are the bare minimum). Whether or not this varies depending on service and security settings... I don't know. I think we should add tests for the most common ones we encounter, but like @mstoykov says, there's no test APIs to facilitate this.

The eventual http.post could be written as a rather neat:

let res = http.post(
  signed.url,
  body,
  {
    headers: signed.headers
  }
);

console.log(res.body);

Which is great!

A few unrelated comments:

• signWithHeaders, would it perhaps be more appropriate to call it signRequest or simply sign?
• I'm not much of a fan of passing in empty strings as arguments, but that's because I come from C# where there's optional ones. Would it perhaps be more elegant to pass in a single object as an argument?

[tom-miseur]

I found I needed to set region in the options object based on receiving it as a parameter, otherwise it generated an undefined later on in core.js:

Suggested change

	var options = { headers: headers }
	var options = {
	headers: headers,
	region: region
	}