feat(hmac-auth): default signed_headers to ["date"]#13388
Open
shreemaan-abhishek wants to merge 3 commits into
Open
feat(hmac-auth): default signed_headers to ["date"]#13388shreemaan-abhishek wants to merge 3 commits into
shreemaan-abhishek wants to merge 3 commits into
Conversation
Previously signed_headers had no default. With no operator configuration, the signing string degenerated to just keyId and the Date header was not bound into the HMAC. Defaulting to ["date"] ensures the request's Date value is part of the signing string by default, so a refreshed Date on an existing Authorization causes HMAC verification to fail. Existing routes that explicitly set signed_headers keep their configuration unchanged.
Add a regression case that locks in the documented compatibility fallback: when an operator sets signed_headers to an empty array, a request whose Authorization declares only @request-target is accepted.
When a route's signed_headers requires a specific header (e.g. the new ["date"] default) but the client's Authorization carries no headers="..." clause at all, the plugin previously returned the opaque "headers missing" diagnostic. Treat the absent clause as an empty declared-headers list and walk conf.signed_headers so the emitted error names the specific header the client must add. Update TEST 36 to use ngx.print so the asserted body does not pick up an extra trailing newline.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR defaults the
hmac-authplugin'ssigned_headersto["date"].Previously,
signed_headershad no default. When a route operator did not configure it, the plugin'sgenerate_signaturereduced the signing string to justkeyId. TheDateheader was therefore not bound into the HMAC, even thoughclock_skewis enabled by default and the documentation describes it as protection against stale requests. Defaultingsigned_headersto["date"]ensures the request'sDatevalue is always part of the signing string unless the operator explicitly opts out.Documentation in
docs/en/latest/plugins/hmac-auth.mdanddocs/zh/latest/plugins/hmac-auth.mdhas been updated to reflect the new default and to clarify howclock_skewandsigned_headersinteract.Which issue(s) this PR fixes:
Fixes # N/A
Breaking change
Clients whose
Authorization: Signature ...value does not currently listdatein itsheaders="..."clause will start receiving401responses from routes whosehmac-authplugin does not explicitly overridesigned_headers. The documented Python example indocs/en/latest/plugins/hmac-auth.mdalready bindsdate, so clients following the docs are unaffected. Operators who need the prior behavior can setsigned_headers: []explicitly.Checklist