aws / aws-sdk-cpp

AWS SDK for C++
Apache License 2.0
1.96k stars 1.06k forks source link

appears to mishandle failing HTTP2 upgrade / doesn't send Authorization header #1821

Open fogti opened 2 years ago

fogti commented 2 years ago

Confirm by changing [ ] to [x] below to ensure that it's a bug:

Describe the bug To be honest, I'm not completely sure where the bug is/who is at fault (original issue: https://github.com/NixOS/nix/issues/5707). I try to use Minio as an S3 backend for a nix cache, nix uses aws-sdk-cpp for uploading to S3 stores, minio currently doesn't fully support HTTP/2 and some reverse-proxies like lighttpd, which I also use, don't handle the upgrade implicitly. thus, the upgrade request to h2 is ignored, which then results in an authentication error from the backend, because (and that's what probably changed) aws-sdk-cpp stopped supplying the Authorization: header in the initial requests which wants to upgrade to h2. It then doesn't seem to detect that the upgrade failed and thus doesn't retry via plain HTTP/1.1 + Authorization.

SDK version number 1.8.121

Platform/OS/Hardware/Device NixOS (Linux)

To Reproduce (observed behavior) I don't have any reduced snippet yet, but the related code is here: https://github.com/NixOS/nix/blob/master/src/libstore/s3-binary-cache-store.cc, https://github.com/NixOS/nix/blob/master/src/libstore/s3.hh

Expected behavior When a connection/request can't be upgraded to h2 (or alternatively h2c), then IF the request failed (because of missing authentication, which doesn't always happen, e.g. if the IAM policy is "wide" enough to allow completely unauthenticated requests to the endpoint+bucket+method combination) the request should be re-sent via HTTP/1.1 (without Upgrade: h2(c) and with(!) Authorization: ... instead).

Logs/output See issue above. I haven't yet looked deep enough into the aws-sdk-cpp API to reproduce it in a standalone example, and nix doesn't offer enough interesting output per default. In the original issue, I posted some server-side HTTP traces, which hopefully illustrate some part of the problem.

To enable logging, set the following system properties:

REMEMBER TO SANITIZE YOUR PERSONAL INFO

options.loggingOptions.logLevel = Aws::Utils::Logging::LogLevel::Trace;
Aws::InitAPI(options)

(maybe I do that later, but it would take some time to recompile nix itself)

Additional context see above and linked issue.

harshavardhana commented 2 years ago

minio currently doesn't fully support HTTP/2 and some reverse-proxies like lighttpd

AWS S3 also doesn't support HTTP2.0 - AFAIK

jmklix commented 2 years ago

@harshavardhana is correct and this sdk doesn't fully support http2 as of right now. It is currently being added and I will change this to a feature request so you can know when it is released.

fogti commented 2 years ago

@jmklix My problem is not that I want HTTP/2 support, my problem is that the SDK doesn't send the Authorization: header, and I'm suspecting some weird interaction with HTTP/1.1->HTTP/2 upgrade stuff.

fogti commented 2 years ago

I was a bit busy, I will try to build nix with aws "tracing" tomorrow.

fogti commented 2 years ago

log file

fogti commented 2 years ago

relevant snip:

AWS: [DEBUG] 2021-12-03 07:49:46.278 CURL [140537079085696] (Text) Connected to 192.168.16.1 (192.168.16.1) port 9000 (#0)
AWS: [DEBUG] 2021-12-03 07:49:46.278 CURL [140537079085696] (HeaderOut) PUT /nix-cache/nar/1i6530iyipd83rad3lg0v3hsr871ln6nmzxmvpvvdm3c7yndfscv.nar.xz HTTP/1.1
Host: 192.168.16.1:9000
Accept: */*
Connection: Upgrade, HTTP2-Settings
Upgrade: h2c
HTTP2-Settings: AAMAAABkAAQCAAAAAAIAAAAA
amz-sdk-invocation-id: C10A8CA3-DBB9-4080-8DA5-A7D337C6FBC2
amz-sdk-request: attempt=1
content-length: 374512
content-type: application/x-nix-nar
user-agent: aws-sdk-cpp/1.8.121/S3/Linux/5.15.4 x86_64 GCC/10.3.0
Expect: 100-continue
AWS: [DEBUG] 2021-12-03 07:49:46.306 CURL [140537079085696] (Text) Mark bundle as not supporting multiuse
AWS: [DEBUG] 2021-12-03 07:49:46.306 CURL [140537079085696] (HeaderIn) HTTP/1.1 403 Forbidden

ok, why??

fogti commented 2 years ago

@jmklix you should afaik remove the feature-request tag, because this is not a feature request, I don't need HTTP/2 support, I just want working HTTP/1.1 authentication, which somehow is skipped (??) and then fails...

gstrauss commented 2 years ago

minio currently doesn't fully support HTTP/2 and some reverse-proxies like lighttpd, which I also use, don't handle the upgrade implicitly. thus, the upgrade request to h2 is ignored

lighttpd 1.4.56 and later supports HTTP/2, including HTTP/1.1 Upgrade: h2c. These are enabled by default in lighttpd 1.4.59 and later (unless disabled in lighttpd.conf)

github-actions[bot] commented 1 year ago

Greetings! Sorry to say but this is a very old issue that is probably not getting as much attention as it deservers. We encourage you to check if this is still an issue in the latest release and if you find that this is still a problem, please feel free to open a new one.

harshavardhana commented 1 year ago

This is still an issue

GuanLuo commented 1 year ago

Came across this issue and just want to check on the status. Can I conclude that, as of now (June 2023), this AWS SDK hasn't fully supported HTTP/2 and the SDK user should ensure that HTTP/1.1 should be used for communication?

jmklix commented 10 months ago

Can you reproduce this issue while uploading directly to s3? This sdk isn't guaranteed to work with minio so it might be an issue with that

harshavardhana commented 10 months ago

Connection: Upgrade, HTTP2-Settings Upgrade: h2c

Here is the upgrade attempted via the load balancer since the h2c upgrade is not working as advertised, forcing the LB to use http1.1, which is the correct solution.

AWS SDK is not adding these headers nor MinIO - it is being added incorrectly by another third-party load balancer.