Improve parsing of iam_request_headers (AWS IAM authentication)

[restfulhead]

I'm using the AWS JavaScript SDK to generate the required STS headers for the AWS IAM authentication login request. It generates something like this:

{
  "User-Agent": "aws-sdk-nodejs/2.28.0 darwin/v6.10.0",
  "Content-Type": "application/x-www-form-urlencoded; charset=utf-8",
  "X-Vault-AWS-IAM-Server-ID": "Bla",
  "X-Amz-Content-Sha256": "ABCDE",
  "Content-Length": 43,
  "Host": "sts.amazonaws.com",
  "X-Amz-Date": "20170605T191548Z",
  "x-amz-security-token": "very long token here"
}

When sending this (Base64 encoded, etc.) to Vault login, I'm getting the following error:

 "failed to JSON decode iam_request_headers \"{\\\"User-Agent\\\":\\\"aws-sdk-nodejs/2.23.0 linux/v6.10.2 exec-env/AWS_Lambda_nodejs6.10\\\",\\\"Content-Type\\\":\\\"application/x-www-form-urlencoded; charset=utf-8\\\",\\\"X-Vault-AWS-IAM-Server-ID\\\":\\\"bla\\\",\\\"X-Amz-Content-Sha256\\\":\\\"1234\\\",\\\"Content-Length\\\":43,\\\..."}\": json: cannot unmarshal number into Go value of type []string"

Since Content-Length is the only number, I removed it for testing. However, then I got: json: cannot unmarshal string into Go value of type []string

I looked into the Vault source code and saw that the string is unmarshalled into http.Header, which defines an array of strings for each header value.

This means I can't directly use the STS request header values, instead I have to convert any numbers to strings and any string objects to string arrays. This is a bit tedious, also the above error messages aren't straight forward, so I think this could be improved from a usability point of view. I also just wanted to post this here, so that people encountering the same error message can search here and find some information.

[jefferai]

We use http.Header because it is legal to define more than one value for a given header. We could probably change this around and accept both string slices and strings, but that won't help content-length. That said, I think we're actually doing the right thing here -- HTTP headers are always opaque text, so the AWS SDK mixing and matching JSON types seems to be problematic. All values ought to be strings.

[restfulhead]

Right, I agree with everything you said, so feel free to close this issue. Just wanted to post, more as a user report from a JavaScript/Node.js perspective. Since the AWS SDK is unlikely to change this anytime soon, other people might run into this issue. I saw (a little too late) that the documentation is already mentioning that "JSON serialization assumes that each header key maps to an array of string values". RTM I know, but I was just following the examples at the top, where it's not mentioned. Maybe this could also be added to the error message?

[jefferai]

We may add an ability to not use the http.Header abstraction in the future. It's correct, but it'd be nice to support both methods.

AWS is unlikely to change their SDK for backwards compatibility reasons but I do encourage you to file a report about content-length being serialized as an int. As far as I can tell it's just plain wrong and maybe they will indeed change it if they realize it's not really in-spec.

[jefferai]

Adding this to our milestone tracker to suggest adding the ability for headers to be non-slices.

If you do file a report with AWS, please link here so we can know if they address it.

[joelthompson]

Hi @ruhkopf,

Agreed it's a bit annoying, I recently posted some python code to the mailing list that also deals with the same thing: https://gist.github.com/joelthompson/378cbe449d541debf771f5a6a171c5ed (the headers_to_go_style method). Annoying, but not the end of the world. If you have suggestions on how to improve the docs to make this more obvious to others, I'm sure the Vault team would welcome a PR :)

In fact, we discussed this a bit in the original feature and decided to use http.Header for the initial release because it's more idiomatic in go. But, as @jefferai says, it doesn't need to be this way and could be changed in the future.

[jefferai]

It's actually less that it's Go-idiomatic and more that it's technically correct. But we can add workarounds for non-conforming implementations over time...

[joelthompson]

FYI, I've started working on a PR to be able to accept headers in this format, if nobody else is working on it.