Skip to content

AWS sigv4: refactor and URL encode username#21923

Closed
bagder wants to merge 9 commits into
masterfrom
bagder/sigv4-user
Closed

AWS sigv4: refactor and URL encode username#21923
bagder wants to merge 9 commits into
masterfrom
bagder/sigv4-user

Conversation

@bagder

@bagder bagder commented Jun 9, 2026

Copy link
Copy Markdown
Member

No description provided.

@github-actions github-actions Bot added the tests label Jun 9, 2026
@bagder bagder changed the title AWS sigv4: refactorand URL encode username AWS sigv4: refactor and URL encode username Jun 9, 2026
@bagder bagder marked this pull request as ready for review June 9, 2026 10:03
@bagder bagder requested a review from Copilot June 9, 2026 10:08

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors the AWS SigV4 HTTP auth implementation and addresses header-injection risk by URL-encoding the SigV4 username used in the Credential= field.

Changes:

  • Refactor Curl_output_aws_sigv4() into smaller helper functions for parameter parsing, payload hashing, timestamp generation, canonical request creation, and signing.
  • URL-escape the SigV4 username before inserting it into the Authorization: header.
  • Add a new regression test covering CRLF in the username and wire it into the test list.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
lib/http_aws_sigv4.c Refactors SigV4 generation flow and URL-encodes the username used in the Authorization header.
tests/data/test3222 Adds a regression test ensuring CRLF in username is encoded in SigV4 Credential=.
tests/data/Makefile.am Registers the new test case.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread lib/http_aws_sigv4.c Outdated
Comment thread lib/http_aws_sigv4.c Outdated
@bagder

bagder commented Jun 9, 2026

Copy link
Copy Markdown
Member Author

augment review

@augmentcode

augmentcode Bot commented Jun 9, 2026

Copy link
Copy Markdown
🤖 Augment PR Summary

Summary: Refactors the HTTP AWS SigV4 authentication implementation and hardens how the access key (username) is emitted in the generated Authorization header.

Changes:

  • Split Curl_output_aws_sigv4() into focused helpers for parameter parsing, payload hashing, timestamp generation, canonical request construction, string-to-sign creation, and header signing/output.
  • Percent-encode the SigV4 credential “username” when composing the Authorization: header to avoid embedding raw control characters.
  • Preserve existing behavior for deriving service/region from the hostname when not explicitly provided.
  • Keep S3-specific handling for x-*-content-sha256 and UNSIGNED-PAYLOAD behavior, while centralizing payload-hash selection.
  • Update test metadata keywords to include aws-sigv4 for existing SigV4-related tests.
  • Add new test test3222 covering CRLF in the username and verifying the resulting Authorization header contains an encoded credential.
  • Register the new test in tests/data/Makefile.am.

Technical Notes: The change is primarily a refactor plus a safety hardening: encoding the credential field prevents header injection if a username contains CR/LF, and the new test asserts the encoded output.

🤖 Was this summary useful? React with 👍 or 👎

@augmentcode augmentcode Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Review completed. No suggestions at this time.

Comment augment review to trigger a new review at any time.

@bagder bagder closed this in c7cba2f Jun 9, 2026
@bagder bagder deleted the bagder/sigv4-user branch June 9, 2026 14:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

2 participants