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

Respect extraHeaders* options in canonical headers #168

Conversation

mxxk
Copy link
Contributor

@mxxk mxxk commented Jul 25, 2024

The options extraHeadersToInclude and extraHeadersToIgnore (added in 180aebd) currently control whether headers are present in (or excluded from) signed headers,

aws4/aws4.js

Lines 310 to 321 in 7e2d1cb

RequestSigner.prototype.signedHeaders = function() {
var extraHeadersToInclude = this.extraHeadersToInclude,
extraHeadersToIgnore = this.extraHeadersToIgnore
return Object.keys(this.request.headers)
.map(function(key) { return key.toLowerCase() })
.filter(function(key) {
return extraHeadersToInclude[key] ||
(HEADERS_TO_IGNORE[key] == null && !extraHeadersToIgnore[key])
})
.sort()
.join(';')
}

but neither option has any effect on the canonical headers:

aws4/aws4.js

Lines 298 to 308 in 7e2d1cb

RequestSigner.prototype.canonicalHeaders = function() {
var headers = this.request.headers
function trimAll(header) {
return header.toString().trim().replace(/\s+/g, ' ')
}
return Object.keys(headers)
.filter(function(key) { return HEADERS_TO_IGNORE[key.toLowerCase()] == null })
.sort(function(a, b) { return a.toLowerCase() < b.toLowerCase() ? -1 : 1 })
.map(function(key) { return key.toLowerCase() + ':' + trimAll(headers[key]) })
.join('\n')
}

This causes problems with signature calculation: When AWS computes canonical headers, it uses the list of signed headers to determine what headers to include in the canonical headers. If the list of signed headers does not match the headers in the canonical headers, the signature verification fails (see new integration tests).


Fixes #167.
Fixes #157.
Supersedes #158.

.filter(function(key) { return HEADERS_TO_IGNORE[key.toLowerCase()] == null })
.sort(function(a, b) { return a.toLowerCase() < b.toLowerCase() ? -1 : 1 })
.map(function(key) { return key.toLowerCase() + ':' + trimAll(headers[key]) })
return Object.entries(headers)
Copy link
Contributor Author

@mxxk mxxk Jul 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does aws4 have a recommended ECMAScript language target? I wasn't sure which language features are okay to use. For example, Object.entries is a newer addition to the language than Object.keys, and I was careful to not rely on destructuring, which could make this logic more readable, for example:

-    .filter(function(entry) {
+    .filter(function([key, value]) {
-      var key = entry[0]
       return extraHeadersToInclude[key] ||
         (HEADERS_TO_IGNORE[key] == null && !extraHeadersToIgnore[key])
     })
-    .map(function(entry) { return entry[0] + ':' + trimAll(entry[1]) })
+    .map(function([key, value]) { return key + ':' + trimAll(value) })

Comment on lines +135 to +157
}, {
url: 'https://dynamodb.us-east-1.amazonaws.com/',
headers: {
'Content-Type': 'application/x-amz-json-1.0',
'X-Amz-Target': 'DynamoDB_20120810.ListTables',
'Accept-Encoding': 'gzip, deflate, br',
'User-Agent': 'node',
},
extraHeadersToInclude: {
'user-agent': true,
},
body: '{}',
}, {
url: 'https://dynamodb.us-east-1.amazonaws.com/',
headers: {
'Content-Type': 'application/x-amz-json-1.0',
'X-Amz-Target': 'DynamoDB_20120810.ListTables',
'Accept-Encoding': 'gzip, deflate, br',
},
extraHeadersToIgnore: {
'content-type': true,
},
body: '{}',
Copy link
Contributor Author

@mxxk mxxk Jul 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These new integration tests cover regressions for this bugfix, since they fail without the accompanying changes in aws4.js:

npm run integration
> aws4@1.13.0 integration
> node ./test/slow.js

Passed tests 0
Passed tests 1
Passed tests 2
Passed tests 3
Passed tests 4
Passed tests 5
Passed tests 6
Passed tests 7
Passed tests 8
Passed tests 9
Passed tests 10
Passed tests 11
Passed tests 12
Passed tests 13
Passed tests 14
Passed tests 15
Passed tests 16
Passed tests 17
Passed tests 18
Passed tests 19
Passed tests 20
Passed tests 21
Passed tests 22
Passed tests 23
Passed tests 24
Passed tests 25
Passed tests 26
Passed tests 27
Passed tests 28
Passed tests 29
Passed tests 30
Passed tests 31
Passed tests 32
Passed tests 33
Passed tests 34
Passed tests 35
Passed tests 36
Passed tests 37
Passed tests 38
Passed tests 39
Passed tests 40
Passed tests 41
Passed tests 42
Passed tests 43
Passed tests 44
Passed tests 45
Passed tests 46
Passed tests 47
Passed tests 48
Passed tests 49
Passed tests 50
Passed tests 51
Passed tests 52
Passed tests 53
Passed tests 54
Passed tests 55
Passed tests 56
Passed tests 57
Passed tests 58
Passed tests 59
Passed tests 60
Passed tests 61
Passed tests 62
Passed tests 63
Passed tests 64
Passed tests 65
Passed tests 66
Passed tests 67
Passed tests 68
Passed tests 69
Passed tests 70
Passed tests 71
Passed tests 72
Passed tests 73
Passed tests 74
Passed tests 75
Passed tests 76
Passed tests 77
Passed tests 78
Passed tests 79
Passed okTests 0
Passed okTests 1
Passed okTests 2
Passed okTests 3
Passed okTests 4
Passed okTests 5
Passed okTests 6
Passed okTests 7
Passed okTests 8
Passed okTests 9
Passed okTests 10
Passed okTests 11
Passed okTests 12
Passed okTests 13
Passed okTests 14
Passed okTests 15
Passed okTests 16
Passed okTests 17
Passed okTests 18
Passed okTests 19
Passed okTests 20
Passed okTests 21
Passed okTests 22
Passed okTests 23
Passed okTests 24
Passed okTests 25
Passed okTests 26
Passed okTests 29
Passed okTests 30
Passed okTests 31
Passed okTests 32
Passed okTests 33
Passed okTests 34
Passed okTests 35
Passed okTests 36
Passed okTests 37
Passed okTests 38
Passed okTests 39
Passed okTests 40
Passed okTests 41
Passed okTests 42
Passed okTests 43
Passed okTests 44
Passed okTests 45
Passed okTests 46
Passed okTests 47
Passed okTests 48
Passed okTests 49
Passed okTests 50
Passed okTests 51
Passed okTests 52
Passed okTests 53
Passed okTests 54
Passed okTests 55
Passed okTests 56
Passed okTests 57
Passed okTests 58
Passed okTests 59
Passed okTests 60
Passed okTests 61
Passed okTests 62
Passed okTests 63
Passed okTests 64
Passed okTests 65
Passed okTests 66
Passed okTests 67
Passed okTests 68
Passed okTests 69
Passed okTests 72
Passed okTests 73
Passed okTests 74
Passed okTests 75
Passed okTests 76
Passed okTests 77
Passed okTests 78
Passed okTests 79
Passed okTests 80
Passed okTests 81
Passed okTests 82
Passed okTests 83
Passed okTests 84
Passed okTests 85
Test 27
POST /
--------------
https://dynamodb.us-east-1.amazonaws.com/
--------------
POST
/

accept-encoding:gzip, deflate, br
content-length:2
content-type:application/x-amz-json-1.0
host:dynamodb.us-east-1.amazonaws.com
x-amz-date:20240725T210946Z
x-amz-security-token:REDACTED
x-amz-target:DynamoDB_20120810.ListTables

accept-encoding;content-length;content-type;host;user-agent;x-amz-date;x-amz-security-token;x-amz-target
44136fa355b3678a1146ad16f7e8649e94fb4fc21fe77e8310c060f61caaff8a
--------------
{"__type":"com.amazon.coral.service#InvalidSignatureException","message":"The request signature we calculated does not match the signature you provided. Check your AWS Secret Access Key and signing method. Consult the service documentation for details."}
==============
Test 28
POST /
--------------
https://dynamodb.us-east-1.amazonaws.com/
--------------
POST
/

accept-encoding:gzip, deflate, br
content-length:2
content-type:application/x-amz-json-1.0
host:dynamodb.us-east-1.amazonaws.com
x-amz-date:20240725T210946Z
x-amz-security-token:REDACTED
x-amz-target:DynamoDB_20120810.ListTables

accept-encoding;content-length;host;x-amz-date;x-amz-security-token;x-amz-target
44136fa355b3678a1146ad16f7e8649e94fb4fc21fe77e8310c060f61caaff8a
--------------
{"__type":"com.amazon.coral.service#InvalidSignatureException","message":"The request signature we calculated does not match the signature you provided. Check your AWS Secret Access Key and signing method. Consult the service documentation for details."}
==============
Test 70
POST /?X-Amz-Security-Token=REDACTED&X-Amz-Date=20240725T210946Z&X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=REDACTED%2F20240725%2Fus-east-1%2Fdynamodb%2Faws4_request&X-Amz-SignedHeaders=accept-encoding%3Bcontent-type%3Bhost%3Buser-agent%3Bx-amz-target&X-Amz-Signature=0c9dff366c23e760db5558e897358a8490de6576c7cc3dfd4a1c6a9b9349a978
--------------
https://dynamodb.us-east-1.amazonaws.com/
--------------
POST
/
X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=REDACTED%2F20240725%2Fus-east-1%2Fdynamodb%2Faws4_request&X-Amz-Date=20240725T210946Z&X-Amz-Security-Token=REDACTED&X-Amz-Signature=0c9dff366c23e760db5558e897358a8490de6576c7cc3dfd4a1c6a9b9349a978&X-Amz-SignedHeaders=accept-encoding%3Bcontent-type%3Bhost%3Buser-agent%3Bx-amz-target
accept-encoding:gzip, deflate, br
content-type:application/x-amz-json-1.0
host:dynamodb.us-east-1.amazonaws.com
x-amz-target:DynamoDB_20120810.ListTables

accept-encoding;content-type;host;user-agent;x-amz-target
44136fa355b3678a1146ad16f7e8649e94fb4fc21fe77e8310c060f61caaff8a
--------------
{"__type":"com.amazon.coral.service#InvalidSignatureException","message":"The request signature we calculated does not match the signature you provided. Check your AWS Secret Access Key and signing method. Consult the service documentation for details."}
==============
Test 71
POST /?X-Amz-Security-Token=REDACTED&X-Amz-Date=20240725T210946Z&X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=REDACTED%2F20240725%2Fus-east-1%2Fdynamodb%2Faws4_request&X-Amz-SignedHeaders=accept-encoding%3Bhost%3Bx-amz-target&X-Amz-Signature=83160a823ee73b5ffd070d821ccc01983e3c055e9256e7b13f40dd813cfcd2e9
--------------
https://dynamodb.us-east-1.amazonaws.com/
--------------
POST
/
X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=REDACTED%2F20240725%2Fus-east-1%2Fdynamodb%2Faws4_request&X-Amz-Date=20240725T210946Z&X-Amz-Security-Token=REDACTED&X-Amz-Signature=83160a823ee73b5ffd070d821ccc01983e3c055e9256e7b13f40dd813cfcd2e9&X-Amz-SignedHeaders=accept-encoding%3Bhost%3Bx-amz-target
accept-encoding:gzip, deflate, br
content-type:application/x-amz-json-1.0
host:dynamodb.us-east-1.amazonaws.com
x-amz-target:DynamoDB_20120810.ListTables

accept-encoding;host;x-amz-target
44136fa355b3678a1146ad16f7e8649e94fb4fc21fe77e8310c060f61caaff8a
--------------
{"__type":"com.amazon.coral.service#InvalidSignatureException","message":"The request signature we calculated does not match the signature you provided. Check your AWS Secret Access Key and signing method. Consult the service documentation for details."}
==============

Note

This assumes the npm run integration command is run with AWS credentials which grant sufficient access (for example, the ReadOnlyAccess AWS-managed policy).

Comment on lines -160 to -167
}, {
url: 'https://opsworks.us-east-1.amazonaws.com/',
headers: {
'Content-Type': 'application/x-amz-json-1.1',
'X-Amz-Target': 'OpsWorks_20130218.DescribeStacks',
'Accept-Encoding': 'gzip, deflate, br',
},
body: '{}',
Copy link
Contributor Author

@mxxk mxxk Jul 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AWS OpsWorks is EOL, so without this removal, the integration tests fail as follows:

npm run integration
> aws4@1.13.0 integration
> node ./test/slow.js

Passed tests 0
Passed tests 1
Passed tests 2
Passed tests 3
Passed tests 4
Passed tests 5
Passed tests 6
Passed tests 7
Passed tests 8
Passed tests 9
Passed tests 10
Passed tests 11
Passed tests 12
Passed tests 13
Passed tests 14
Passed tests 15
Passed tests 16
Passed tests 17
Passed tests 18
Passed tests 19
Passed tests 20
Passed tests 21
Passed tests 22
Passed tests 23
Passed tests 24
Passed tests 25
Passed tests 26
Passed tests 27
Passed tests 28
Passed tests 29
Passed tests 30
Passed tests 31
Passed tests 32
Passed tests 33
Passed tests 34
Passed tests 35
Passed tests 36
Passed tests 37
Passed tests 38
Passed tests 39
Passed tests 40
Passed tests 41
Passed tests 42
Passed tests 43
Passed tests 44
Passed tests 45
Passed tests 46
Passed tests 47
Passed tests 48
Passed tests 49
Passed tests 50
Passed tests 51
Passed tests 52
Passed tests 53
Passed tests 54
Passed tests 55
Passed tests 56
Passed tests 57
Passed tests 58
Passed tests 59
Passed tests 60
Passed tests 61
Passed tests 62
Passed tests 63
Passed tests 64
Passed tests 65
Passed tests 66
Passed tests 67
Passed tests 68
Passed tests 69
Passed tests 70
Passed tests 71
Passed tests 72
Passed tests 73
Passed tests 74
Passed tests 75
Passed tests 76
Passed tests 77
Passed tests 78
Passed tests 79
Passed okTests 0
Passed okTests 1
Passed okTests 2
Passed okTests 3
Passed okTests 4
Passed okTests 5
Passed okTests 6
Passed okTests 7
Passed okTests 8
Passed okTests 9
Passed okTests 10
Passed okTests 11
Passed okTests 12
Passed okTests 13
Passed okTests 14
Passed okTests 15
Passed okTests 16
Passed okTests 17
Passed okTests 18
Passed okTests 19
Passed okTests 20
Passed okTests 21
Passed okTests 22
Passed okTests 23
Passed okTests 24
Passed okTests 25
Passed okTests 26
Passed okTests 27
Passed okTests 28
Passed okTests 29
Passed okTests 30
Passed okTests 31
Passed okTests 33
Passed okTests 34
Passed okTests 35
Passed okTests 36
Passed okTests 37
Passed okTests 38
Passed okTests 39
Passed okTests 40
Passed okTests 41
Passed okTests 42
Passed okTests 43
Passed okTests 44
Passed okTests 45
Passed okTests 46
Passed okTests 47
Passed okTests 48
Passed okTests 49
Passed okTests 50
Passed okTests 51
Passed okTests 52
Passed okTests 53
Passed okTests 54
Passed okTests 55
Passed okTests 56
Passed okTests 57
Passed okTests 58
Passed okTests 59
Passed okTests 60
Passed okTests 61
Passed okTests 62
Passed okTests 63
Passed okTests 64
Passed okTests 65
Passed okTests 66
Passed okTests 67
Passed okTests 68
Passed okTests 69
Passed okTests 70
Passed okTests 71
Passed okTests 72
Passed okTests 73
Passed okTests 74
Passed okTests 75
Passed okTests 77
Passed okTests 78
Passed okTests 79
Passed okTests 80
Passed okTests 81
Passed okTests 82
Passed okTests 83
Passed okTests 84
Passed okTests 85
Passed okTests 86
Passed okTests 87
Test 32
POST /
--------------
https://opsworks.us-east-1.amazonaws.com/
--------------
POST
/

accept-encoding:gzip, deflate, br
content-length:2
content-type:application/x-amz-json-1.1
host:opsworks.us-east-1.amazonaws.com
x-amz-date:20240725T202725Z
x-amz-security-token:REDACTED
x-amz-target:OpsWorks_20130218.DescribeStacks

accept-encoding;content-length;content-type;host;x-amz-date;x-amz-security-token;x-amz-target
44136fa355b3678a1146ad16f7e8649e94fb4fc21fe77e8310c060f61caaff8a
--------------
{"__type":"AccessDeniedException","Message":"AWS OpsWorks Stacks is no longer accepting new customers"}
==============
Test 76
POST /?X-Amz-Security-Token=REDACTED&X-Amz-Date=20240725T202725Z&X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=REDACTEDK%2F20240725%2Fus-east-1%2Fopsworks%2Faws4_request&X-Amz-SignedHeaders=accept-encoding%3Bcontent-type%3Bhost%3Bx-amz-target&X-Amz-Signature=91403156ede5a0308abcd00899091234acd2d55057ca8acf9001f9906177d729
--------------
https://opsworks.us-east-1.amazonaws.com/
--------------
POST
/
X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=REDACTEDK%2F20240725%2Fus-east-1%2Fopsworks%2Faws4_request&X-Amz-Date=20240725T202725Z&X-Amz-Security-Token=REDACTED&X-Amz-Signature=91403156ede5a0308abcd00899091234acd2d55057ca8acf9001f9906177d729&X-Amz-SignedHeaders=accept-encoding%3Bcontent-type%3Bhost%3Bx-amz-target
accept-encoding:gzip, deflate, br
content-type:application/x-amz-json-1.1
host:opsworks.us-east-1.amazonaws.com
x-amz-target:OpsWorks_20130218.DescribeStacks

accept-encoding;content-type;host;x-amz-target
44136fa355b3678a1146ad16f7e8649e94fb4fc21fe77e8310c060f61caaff8a
--------------
{"__type":"AccessDeniedException","Message":"AWS OpsWorks Stacks is no longer accepting new customers"}
==============

@mxxk
Copy link
Contributor Author

mxxk commented Jul 25, 2024

@ryanblock since you reviewed #158, I wanted to ask for your 👀 on this PR! 🙂

mhart added a commit that referenced this pull request Aug 6, 2024
Fixes #157 #158 #168

We introduce an extra function here, filterHeaders, and a cached value filteredHeaders similar to parsedPath.

Code isn't exactly as I'd write it using modern JS, but this fits in with the current style and ensures we don't accidentally use any newer JS APIs.
@mhart
Copy link
Owner

mhart commented Aug 6, 2024

Awesome, thanks for this and apologies for taking so long – I included this in v1.13.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants