(trafficserver) 12/22: origin_server_auth: clarify x-amz- stripping scope in docs and comments

[bn...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

bneradt pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficserver.git

commit 9c04c80ca79bfc369d63cf4593087ee8fd8bc81a
Author: Chris McFarlen <ch...@mcfarlen.us>
AuthorDate: Mon Aug 24 13:30:01 2026 -0500

    origin_server_auth: clarify x-amz- stripping scope in docs and comments
---
 doc/admin-guide/plugins/origin_server_auth.en.rst | 18 +++++++++++++-----
 plugins/origin_server_auth/origin_server_auth.cc  | 18 +++++++++++-------
 2 files changed, 24 insertions(+), 12 deletions(-)

diff --git a/doc/admin-guide/plugins/origin_server_auth.en.rst b/doc/admin-guide/plugins/origin_server_auth.en.rst
index 0946674cc5..08ed030b17 100644
--- a/doc/admin-guide/plugins/origin_server_auth.en.rst
+++ b/doc/admin-guide/plugins/origin_server_auth.en.rst
@@ -144,7 +144,9 @@ If ``--v4-region-map`` is not specified the plugin defaults to the mapping defin
 
 According to `Transferring Payload in a Single Chunk <http://docs.aws.amazon.com/AmazonS3/latest/API/sig-v4-header-based-auth.html>`_ specification
 the ``CanonicalHeaders`` list *must* include the ``Host`` header,  the ``Content-Type`` header if present in the request and all the ``x-amz-*`` headers
-so ``--v4-include-headers`` and ``--v4-exclude-headers`` do not impact those headers and they are *always* signed.
+so ``--v4-include-headers`` and ``--v4-exclude-headers`` do not change whether those headers are signed: any of them
+still present when the signature is calculated is *always* signed. ``--v4-include-headers`` does, however, govern which
+``x-amz-*`` headers are still present at that point, as described below.
 
 The ``Via`` and ``X-Forwarded-For`` headers are *always* excluded from the signature since they are meant to be changed by the proxies and signing them could lead to invalidation of the signature.
 
@@ -152,10 +154,16 @@ If ``--v4-include-headers`` is not specified all headers except those specified
 
 If ``--v4-include-headers`` is specified only the headers specified will be signed except those specified in ``--v4-exclude-headers``
 
-Since all ``x-amz-*`` headers are always signed, an ``x-amz-*`` header sent by a client would otherwise reach the
-origin with a valid signature covering whatever value the client chose. To prevent this, any ``x-amz-*`` header
-present on the client request is stripped before signing unless it is explicitly named in
-``--v4-include-headers``.
+Since every ``x-amz-*`` header present at signing time is signed, an ``x-amz-*`` header supplied by a client
+would otherwise reach the origin carrying a valid signature over whatever value the client chose. To prevent
+this, the plugin removes ``x-amz-*`` headers from the request before signing unless they are explicitly named
+in ``--v4-include-headers``.
+
+Signing happens at ``TS_HTTP_SEND_REQUEST_HDR_HOOK``, at which point the plugin cannot distinguish an
+``x-amz-*`` header sent by the client from one added by another plugin earlier in the chain, so the removal
+applies to both. If a deployment relies on another plugin supplying an ``x-amz-*`` header (for example
+``x-amz-server-side-encryption`` set through :ref:`admin-plugins-header-rewrite`), name that header in
+``--v4-include-headers`` so that it is preserved and signed.
 
 
 AWS Authentication version 2
diff --git a/plugins/origin_server_auth/origin_server_auth.cc b/plugins/origin_server_auth/origin_server_auth.cc
index 22dae2876e..bfd128983d 100644
--- a/plugins/origin_server_auth/origin_server_auth.cc
+++ b/plugins/origin_server_auth/origin_server_auth.cc
@@ -868,12 +868,16 @@ hasXAmzPrefix(const char *name, int name_len)
 }
 
 ///////////////////////////////////////////////////////////////////////////
-// Strip any x-amz- request header the client sent that isn't explicitly
-// allow-listed via v4-include-headers. AWS SigV4 always signs whatever
-// x-amz- headers are present at signing time, so an unsolicited client
-// x-amz- header (e.g. x-amz-acl) would otherwise reach S3 with a valid
-// signature, which can trigger bucket-policy denials that get negatively
-// cached.
+// Strip any x-amz- request header that isn't explicitly allow-listed via
+// v4-include-headers. AWS SigV4 always signs whatever x-amz- headers are
+// present at signing time, so an unsolicited x-amz- header (e.g. x-amz-acl)
+// would otherwise reach S3 with a valid signature, which can trigger
+// bucket-policy denials that get negatively cached.
+//
+// This runs at SEND_REQUEST_HDR, where a client-sent header is
+// indistinguishable from one added by another plugin earlier in the chain, so
+// both are subject to removal. A header another plugin is expected to supply
+// has to be named in v4-include-headers.
 void
 S3Request::stripUnauthorizedAmzHeaders(const StringSet &allowedHeaders)
 {
@@ -899,7 +903,7 @@ S3Request::stripUnauthorizedAmzHeaders(const StringSet &allowedHeaders)
       }
 
       if (strip) {
-        Dbg(dbg_ctl, "stripping unauthorized client header %.*s", name_len, name);
+        Dbg(dbg_ctl, "stripping unauthorized x-amz- header %.*s", name_len, name);
         TSMimeHdrFieldDestroy(_bufp, _hdr_loc, field_loc);
       }
     }