nginx-zauth-module: Support AWS4-HMAC-SHA256 auth scheme #4593
Conversation
zebot
added
the
ok-to-test
akshaymankar
force-pushed
the
aws-auth-with-zauth
branch
from
9e1733b to
9fdc180
Compare
akshaymankar
force-pushed
the
aws-auth-with-zauth
branch
from
a3f66d3 to
ba517b3
Compare
…g them C compiler calculates it compile time.
fisx
left a comment
fisx
left a comment
There was a problem hiding this comment.
L(almost)GTM. i've only found one (trivial) bug, approving on condition of that getting fixed.
…' into aws-auth-with-zauth
| char const * bearer = "Bearer "; | ||
| size_t bearer_len = 7; // strlen(bearer) is not safe, says sonar cloud | ||
| char const * aws4_hmac_sha256 = "AWS4-HMAC-SHA256 "; | ||
| size_t aws4_hmac_sha256_len = 17; |
There was a problem hiding this comment.
i think i've solved the mystery of why the test does not fail when i say aws4_hmac_sha256_len = 7: token_from_aws_hmac_header skips all tokens up to Credentials=. auth_header_len there is calculated here given the shorter aws4_hmac_sha256_len, so still reaches to the actual end of the buffer.
so the bug that we had here was actually never able to materialize in behavior due to defensive programming.
| static ZauthResult token_from_header (ngx_str_t const * hdr, ZauthToken ** t) { | ||
| if (strncmp((char const *) hdr->data, "Bearer ", 7) == 0) { | ||
| return zauth_token_parse(&hdr->data[7], hdr->len - 7, t); | ||
| char const * bearer = "Bearer "; |
There was a problem hiding this comment.
If you define your strings as char[], then you can use sizeof instead of hardcoding the length.
There was a problem hiding this comment.
i tried, also the -1 trick, but somehow got it wrong, and the code didn't really seem easier to read or much safer to me. also, this is how it's done in this code base.
https://wearezeta.atlassian.net/browse/WPB-17733
Checklist
changelog.d