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

breaking change in 7.82.0: mandatory login token in .netrc #8653

Closed
standsed opened this issue Mar 30, 2022 · 10 comments
Closed

breaking change in 7.82.0: mandatory login token in .netrc #8653

standsed opened this issue Mar 30, 2022 · 10 comments

Comments

@standsed
Copy link

standsed commented Mar 30, 2022

I did this

Create/append ~/.netrc file with record without login token, e.g.,
machine curl.com password S3cr3t

run
curl https://host.com --netrc --verbose

I expected the following

Header to be sent
Authorization: Basic Onp4Y3Y=

curl/libcurl version

Does not work:
docker image: curlimages/curl:7.82.0
curl 7.82.0-DEV (x86_64-pc-linux-musl) libcurl/7.82.0-DEV OpenSSL/1.1.1n zlib/1.2.11 brotli/1.0.9 libssh2/1.10.0 nghttp2/1.46.0
Release-Date: [unreleased]
Protocols: dict file ftp ftps gopher gophers http https imap imaps mqtt pop3 pop3s rtsp scp sftp smb smbs smtp smtps telnet tftp
Features: alt-svc AsynchDNS brotli HSTS HTTP2 HTTPS-proxy IPv6 Largefile libz NTLM NTLM_WB SSL TLS-SRP UnixSockets

Works:
docker image: curlimages/curl:7.81.0
curl 7.81.0-DEV (x86_64-pc-linux-musl) libcurl/7.81.0-DEV OpenSSL/1.1.1l zlib/1.2.11 brotli/1.0.9 libssh2/1.10.0 nghttp2/1.46.0
Release-Date: [unreleased]
Protocols: dict file ftp ftps gopher gophers http https imap imaps mqtt pop3 pop3s rtsp scp sftp smb smbs smtp smtps telnet tftp
Features: alt-svc AsynchDNS brotli HSTS HTTP2 HTTPS-proxy IPv6 Largefile libz NTLM NTLM_WB SSL TLS-SRP UnixSockets

curlimages/curl:7.77.0 also ok

@jay
Copy link
Member

jay commented Mar 30, 2022

Bisected to 7d600ad (#8451) which removed the user_passwd flag in favor of checking user pointer. AFAICT that means now that a username must always be set if a password is set. There is code in parseurlandfillconn which sets an empty username if no username was found in the URL but a password was found:

curl/lib/url.c

Lines 2066 to 2069 in 64db5c5

else if(data->state.aptr.passwd) {
/* no user was set but a password, set a blank user */
result = Curl_setstropt(&data->state.aptr.user, "");
}

netrc user/pass comes in later in override_login, but it doesn't have the same logic to create a blank username if there's a password but no username. I'm not sure the reasons for that but here is some similar logic that works:

diff --git a/lib/url.c b/lib/url.c
index a56e4b0..9f29593 100644
--- a/lib/url.c
+++ b/lib/url.c
@@ -2971,6 +2971,12 @@ static CURLcode override_login(struct Curl_easy *data,
       /* don't update the user name below */
       userp = NULL;
     }
+    /* no user was set but a password, set a blank user */
+    if(userp && !*userp && *passwdp) {
+      *userp = strdup("");
+      if(!*userp)
+        return CURLE_OUT_OF_MEMORY;
+    }
   }
 #endif
 

curlimages/curl:7.77.0 also ok

Though 7.81.0 may work, 7.77.0 and some other older versions have a bug with blank usernames where they actually send "(nil)" for the username. So if you must work around this immediately use 7.81.0 not older versions.

@bagder
Copy link
Member

bagder commented Apr 3, 2022

@jay, if this is a proposal shouldn't it be made as a PR?

@jay
Copy link
Member

jay commented Apr 4, 2022

I was more at the concept phase. Does it make sense to check in override_login?

@bagder
Copy link
Member

bagder commented May 6, 2022

If all other existing tests work and you add a new one that reproduces the problem without the patch and works with the patch, then I think it sounds l like a good fix!

@domenkozar
Copy link

domenkozar commented Jun 8, 2022

This broke https://cachix.org and a lot of users are having problems when they upgrade to latest curl.

How can I help to expedite this?

@domenkozar
Copy link

domenkozar commented Jun 9, 2022

I can confirm the patch fixes the issue.

github-actions bot pushed a commit to NixOS/nixpkgs that referenced this issue Jun 9, 2022
gador pushed a commit to gador/nixpkgs that referenced this issue Jun 10, 2022
@jay
Copy link
Member

jay commented Jun 24, 2022

I can confirm the patch fixes the issue.

Thanks. I will look at turning the patch into a PR.

jay added a commit to jay/curl that referenced this issue Jun 29, 2022
- If, after parsing netrc, there is a password with no username then
  set a blank username.

This used to be the case prior to 7d600ad (precedes 7.82). Note
parseurlandfillconn already does the same thing for URLs.

Reported-by: Raivis <standsed@users.noreply.github.com>
Testing-by: Domen Kožar

Fixes curl#8653
Closes #xxxx
orgads added a commit to orgads/MINGW-packages that referenced this issue Aug 3, 2022
qtprojectorg pushed a commit to qt-creator/qt-creator that referenced this issue Aug 3, 2022
It is set in .netrc anyway, and there is a regression in curl that causes
it to fail when passing the user name.

See curl/curl#8653

Change-Id: Ic7aa2d874884db71f71d162486acf4e054eab7e9
Reviewed-by: hjk <hjk@qt.io>
@orgads
Copy link
Contributor

orgads commented Aug 3, 2022

I've noticed another regression. I do have username in netrc, but if I provide a username in the URL, none of them is used, and no Authorization header is sent (even if it is the same user).

Example:
.netrc:

machine server login user password 5up3r53cr31
$ curl -n http://user@server/
Unauthorized

This is not covered by #9066.

@orgads
Copy link
Contributor

orgads commented Aug 3, 2022

My issue is caused by d1237ac.

@orgads
Copy link
Contributor

orgads commented Aug 3, 2022

orgads pushed a commit to orgads/curl that referenced this issue Aug 18, 2022
- If, after parsing netrc, there is a password with no username then
  set a blank username.

This used to be the case prior to 7d600ad (precedes 7.82). Note
parseurlandfillconn already does the same thing for URLs.

Reported-by: Raivis <standsed@users.noreply.github.com>
Testing-by: Domen Kožar

Fixes curl#8653
Closes #xxxx
@bagder bagder closed this as completed in 8bd0351 Aug 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants