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

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

Comments

@johnhany97
Copy link

@johnhany97 johnhany97 commented Feb 14, 2022

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

@bagder
Copy link
Member

@bagder 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

@johnhany97 johnhany97 commented Feb 14, 2022

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

@johnhany97 johnhany97 commented Feb 14, 2022

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

@johnhany97 johnhany97 commented Feb 14, 2022

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 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
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
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
The authentication status should be told by the transfer and not the
connection.

Fixes #8449
Reported-by: John H. Ayad
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

2 participants