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

Regression in signatures with the new Java HTTP implementation #263

Open
JJ-Atkinson opened this issue Jan 24, 2025 · 8 comments
Open

Regression in signatures with the new Java HTTP implementation #263

JJ-Atkinson opened this issue Jan 24, 2025 · 8 comments
Assignees

Comments

@JJ-Atkinson
Copy link

Hi,

I've recently upgraded from a prior version where jetty was the default http client to the latest 0.8.723, and I'm now using the default java builtin http client. It appears that there's a regression in signatures with this new version. If I'm not very much mistaken, the port number is being attached to the host header (implicitly controlled by java http - not much aws-api can do about that) but that wasn't expected behavior before. I'm using wireshark to capture packets, and I see the following http request:

GET /lotus-eaters HTTP/1.1
Connection: Upgrade, HTTP2-Settings
Content-Length: 0
Host: garage-ct.hippo-castor.ts.net:3900
HTTP2-Settings: AAEAAEAAAAIAAAAAAAMAAAAAAAQBAAAAAAUAAEAAAAYABgAA
Upgrade: h2c
User-Agent: Java-http-client/21.0.5
authorization: AWS4-HMAC-SHA256 Credential=[...]/20250124/us-west-1/s3/aws4_request, SignedHeaders=host;x-amz-content-sha256;x-amz-date, Signature=[...]
x-amz-content-sha256: e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855
x-amz-date: 20250124T035739Z

Note the host header in particular. With some careful repl manipulation, I was able to get the aws.signers/canonical-request for this particular request:

GET
/lotus-eaters

host:garage-ct.hippo-castor.ts.net
x-amz-content-sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855
x-amz-date:20250124T032636Z

host;x-amz-content-sha256;x-amz-date
e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855

Note the lack of a port number on the host.

When using the standard aws cli v2 in debug mode, I can see the canonical string also contains a port number:

2025-01-23 21:26:36,220 - MainThread - botocore.auth - DEBUG - CanonicalRequest:
GET
/lotus-eaters
encoding-type=url&list-type=2
host:garage-ct.hippo-castor.ts.net:3900
x-amz-content-sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855
x-amz-date:20250124T032636Z

host;x-amz-content-sha256;x-amz-date
e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855

The port number is also visible in the wireguard capture of the aws cli v2 requests.

I'm not familiar enough with the intention behind the signing code to provide a good suggestion about where to place a modified host header, but I'm quite sure that's what's wrong with my requests.

@JJ-Atkinson
Copy link
Author

I just ran an experiment with the following code:

(defmethod a.signers/sign-http-request "s3"
  [service endpoint credentials http-request]
  (a.signers/v4-sign-http-request service
                                  endpoint
                                  credentials
                                  (-> http-request
                                      (assoc-in [:headers "host"] (str (get-in http-request [:headers "host"]) ":3900")))
                                  :content-sha256-header?
                                  true))

This fixes my authentication issues for s3. I'll be using this until a more permanent solution is identified.

@MichaelDrogalis
Copy link

I ran into this issue as well. Here's @JJ-Atkinson's code, generalizing the port:

(require '[[cognitect.aws.signers :as signers])

(defmethod signers/sign-http-request "s3"
  [service endpoint credentials http-request]
  (signers/v4-sign-http-request service
                                endpoint
                                credentials
                                (-> http-request
                                    (assoc-in [:headers "host"] (str (get-in http-request [:headers "host"]) ":" (:server-port http-request))))
                                :content-sha256-header?
                                true))

@p-himik
Copy link

p-himik commented Feb 10, 2025

Just noticed that I'm getting HTTP 403 with :cognitect.aws.error/code "SignatureDoesNotMatch" and :body "The request signature we calculated does not match the signature you provided. Check your key and signing method.".

I assume it's the same error as reported, given that the hotfix also worked for me. Mentioning just to help others find this issue via GitHub search.

@marcobiscaro2112
Copy link
Collaborator

Thanks for reporting. Could you confirm:

  • this only happens with endpoints using a non-default port?
  • what is the version that you used previously in which the signature works correctly?

@marcobiscaro2112
Copy link
Collaborator

Also: how do you create your client? I assume you are using :endpoint-override, can you share it here?

@MichaelDrogalis
Copy link

MichaelDrogalis commented Feb 11, 2025

Thanks for reporting. Could you confirm:

* this only happens with endpoints using a non-default port?

* what is the version that you used previously in which the signature works correctly?

(1) Yes, I only see this when connecting to MinIO, which is on a different port than S3.
(2) It worked as of aws-api 0.8.686 for me.

Also: how do you create your client? I assume you are using :endpoint-override, can you share it here?

(defn unwrap-endpoint [endpoint]
  (let [url ^URL (io/as-url endpoint)
        port (.getPort url)
        path (.getPath url)]
    (cond-> {:protocol (.getProtocol url)
             :hostname (.getHost url)}
      (not= port -1) (assoc :port port)
      (not (s/blank? path)) (assoc :path path))))

(defn make-s3-client [{:strs [endpoint] :as connection-configs}]
  (let [opts (cond-> {:api :s3}
               endpoint (assoc :endpoint-override (unwrap-endpoint endpoint)))]
    (aws/client opts)))

@p-himik
Copy link

p-himik commented Feb 12, 2025

I just encountered the error again, but this time with AWS S3 and after implementing the workaround. (Hmm, maybe the workaround is only suitable for MinIO that I was also using when I first saw the issue?) I guess I'll downgrade the library for now.

The client was created in the simplest way, by just passing this map to aws/client:

{:api                  :s3
 :region               region
 :credentials-provider (credentials/basic-credentials-provider
                         {:access-key-id     access-key-id
                          :secret-access-key secret-access-key})}

@p-himik
Copy link

p-himik commented Feb 12, 2025

Ended up not downgrading but using this version of the workaround instead, that uses it only if the hostname is localhost:

(let [orig-sign-fn (get-method signers/sign-http-request "s3")]
  (defmethod signers/sign-http-request "s3"
    [service endpoint credentials http-request]
    (if (= (:hostname endpoint) "localhost")
      (signers/v4-sign-http-request service
                                    endpoint
                                    credentials
                                    (-> http-request
                                        (assoc-in [:headers "host"] (str (get-in http-request [:headers "host"]) ":" (:server-port http-request))))
                                    :content-sha256-header?
                                    true)
      (orig-sign-fn service endpoint credentials http-request))))

totakke added a commit to totakke/cavia that referenced this issue Feb 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants