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

Jump to bottom

Basic auth credentials passed solely through the url are no longer being reused #8449

Closed
johnhany97 opened this issue Feb 14, 2022 · 5 comments
Closed

Basic auth credentials passed solely through the url are no longer being reused #8449

johnhany97 opened this issue Feb 14, 2022 · 5 comments
Assignees
@bagder
Labels
authentication HTTP

Comments

@johnhany97
Copy link

I did this

We use libcurl through mamba. After mamba upgraded past libcurl 7.75.0, we started seeing issues with basic auth passed through the URL.

We give curl through curl_easy_setopt(handle, CURLOPT_URL, url.c_str()); a url that has basic auth as part of it.

An example is:

https://:test@localhost:8005/

However, we started noticing that reused connections would get a 401 back from the service due to the lack of an auth header.

I expected the following

I expected that reused connections would continue to have the basic auth credentials all throughout.

I opened mamba-org/mamba#1141 where I started iterating to getting a minimal repro and I was able to get that, but from there debugged my way into a potential workaround to this issue.

If I directly set curl_easy_setopt(handle, CURLOPT_USERPWD, ":test");, the issue would no longer come up. So I believe the issue might have something to do with how curl is handling the credentials from the url and they're not being set correctly such that future reuse of the same connection would benefit from the same credentials.

I think this might have something to do with #6545 which is very relevant to the release where this got introduced.

curl/libcurl version

Any libcurl higher than 7.75.0 (I tested against latest as well)

operating system
@johnhany97 johnhany97 mentioned this issue Feb 14, 2022
Closed
@bagder
Copy link
Member

bagder commented Feb 14, 2022

I built curl from master just now and I cannot seem to reproduce:

$ ./src/curl -v http://:secret@localhost -o /dev/null
...
* Server auth using Basic with user ''
> GET / HTTP/1.1
> Host: localhost
> Authorization: Basic OnNlY3JldA==
> User-Agent: curl/7.82.0-DEV
> Accept: */*
> 
* Mark bundle as not supporting multiuse
* HTTP 1.1 or later with persistent connection
< HTTP/1.1 200 OK
< Date: Mon, 14 Feb 2022 11:28:24 GMT
< Server: Apache/2.4.52 (Debian)
< Last-Modified: Fri, 19 Oct 2018 22:16:27 GMT
< ETag: "29cd-5789c412a5844"
< Accept-Ranges: bytes
< Content-Length: 10701
< Vary: Accept-Encoding
< Content-Type: text/html
...

$ echo OnNlY3JldA== | base64 -d
:secret

@johnhany97
Copy link
Author

The first connection in the transfer always has the auth header. But if you were to reuse the same connection, the credentials aren't passed. You can see the curl logs in https://github.com/mamba-org/mamba/runs/5174793912?check_suite_focus=true where all of a sudden, we started getting 401s because the auth header was not sent back.

@johnhany97
Copy link
Author

Something else to note here is that by default, mamba sets https://github.com/mamba-org/mamba/blob/b73db2d7ab0af14c891bcaa17482885661499f25/libmamba/src/core/fetch.cpp#L625-L626 to 5, hence why in my repro when I tell it about 6 urls, the failures start to come up.

@johnhany97
Copy link
Author

I was able to repro this from the CLI using the following:

curl  -I -v --parallel --parallel-max 2 "https://:test@localhost:8005/file1.json" "https://:test@localhost:8005/file2.json" "https://:test@localhost:8005/file3.json" "https://:test@localhost:8005/file4.json"

@bagder
Copy link
Member

bagder commented Feb 14, 2022

Thanks, using this I could see the header go missing as well!

@bagder bagder self-assigned this Feb 14, 2022
bagder added a commit that referenced this issue Feb 14, 2022
@bagder
url: keep bit.user_passwd untouched when re-using connection
900a0bf 
While connections for HTTP don't actually have credentials (unless using
NTLM horrors), we maintain that info in there for now and just make sure
to not reset it when a connection is reused.

Fixes #8449
Reported-by: John H. Ayad
bagder added a commit that referenced this issue Feb 14, 2022
@bagder
url: keep bit.user_passwd untouched when re-using connection
6e150e6 
While connections for HTTP don't actually have credentials (unless using
NTLM horrors), we maintain that info in there for now and just make sure
to not reset it when a connection is reused.

Fixes #8449
Reported-by: John H. Ayad
@bagder bagder mentioned this issue Feb 14, 2022
Closed
bagder added a commit that referenced this issue Feb 14, 2022
@bagder
urldata: remove conn->bits.user_passwd
21a47d5 
The authentication status should be told by the transfer and not the
connection.

Fixes #8449
Reported-by: John H. Ayad
@bagder bagder closed this as completed in 7d600ad Feb 16, 2022
@jay jay mentioned this issue Mar 30, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@bagder bagder

Labels
authentication HTTP
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

urldata: remove conn->bits.user_passwd curl/curl
2 participants
@bagder @johnhany97