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

IP addresses mismatch in --noproxy and related env variable (in 7.86.0) #9813

Closed
henning-schild opened this issue Oct 27, 2022 · 16 comments
Closed

Comments

@henning-schild
Copy link

I did this

cd /tmp
touch testfile
php -S 127.0.0.1:8000 &
# anything where there is no proxy will do
export http_proxy=http://127.0.0.5:3128
curl -vv -o /dev/null http://127.0.0.1:8000/testfile --noproxy "127.0.0.1"
curl -vv -o /dev/null http://127.0.0.1:8000/testfile --noproxy "127.0.0.1,localhost"

I expected the following

both curls downloading the file and not reaching out to the proxy, but if "127.0.0.1" is not the last part of the string ... it will not work

curl/libcurl version

[curl -V output]

[root@6a5006b5a040 ~]# curl -V
curl 7.86.0 (x86_64-pc-linux-gnu) libcurl/7.86.0 OpenSSL/1.1.1q zlib/1.2.13 brotli/1.0.9 zstd/1.5.2 libidn2/2.3.3 libpsl/0.21.1 (+libidn2/2.3.0) libssh2/1.10.0 nghttp2/1.50.0
Release-Date: 2022-10-26
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 GSS-API HSTS HTTP2 HTTPS-proxy IDN IPv6 Kerberos Largefile libz NTLM NTLM_WB PSL SPNEGO SSL threadsafe TLS-SRP UnixSockets zstd

This came with 7.86.0 and showed up in a CI pipline testing multiple "modern distros". It came with "alpine:edge" and "archlinux:latest" at the same time. Both switched to that new curl yesterday.

operating system

docker containers of "alpine:edge" and "archlinux:latest" ... or Linux with the latest curl

@bagder
Copy link
Member

bagder commented Oct 27, 2022

Introduced in #9775 no doubt

bagder added a commit that referenced this issue Oct 27, 2022
If the host name is an IP address and the noproxy string contained that
IP address with a following comma, it would erroneously not match.

Extended test 1614 to verify this combo as well.

Reported-by: Henning Schild

Fixes #9813
@DhruvaSambrani
Copy link

Same issue, other applications using curl libs also affected. Is there a temporary workaround? Need to no-proxy on .ts.net, what should I be using instead?

@bagder
Copy link
Member

bagder commented Oct 27, 2022

Work-around: if you append /32 on the right side of the 127.0.0.1 in the noproxy string it should work. Like:

127.0.0.1/32,localhost

@DhruvaSambrani
Copy link

Work-around works. I assume this would need to be done on all ip addresses?

@bagder
Copy link
Member

bagder commented Oct 27, 2022

Yes, and you can now use proper CIDR notation so you can match a larger network in a single string, like 127.0.0.1/8.

@bagder bagder changed the title latest release is problems in handling --noproxy and related env variables 7.86.0 problems in handling --noproxy and related env variable Oct 27, 2022
@henning-schild
Copy link
Author

I would say that is a regression which might even call for a new release rather soon. Nice to see there is a workaround. But if you ever had the honors to having to work with proxies and related env variables, you will know how horrible all of that is.

The variables are used by many tools and they all have their own implementation on dealing with them. While curl is an important player in that game it is not the only one. Just think multiple versions of curl all hoping to use the same NO_PROXY.

In my example i pointed it out via cmdline --noproxy but the env variables are the real deal, that is where users have to find values that work for all their tools and have the same effect in all of them. That is in fact impossible ... but any tool should try its best.

@bagder
Copy link
Member

bagder commented Oct 27, 2022

a regression which might even call for a new release rather soon

We will certainly take this into consideration.

@bagder bagder closed this as completed in efc286b Oct 27, 2022
@henning-schild
Copy link
Author

Thanks for that quick fix but there is more to it. I tried to make that workaround get a whole complex pipeline green where proxies are needed for several things. The IP just being one of several essential parts of no_proxy.

curl 7.86.1-DEV (x86_64-pc-linux-gnu) libcurl/7.86.1-DEV OpenSSL/1.1.1n libidn2/2.3.0
Release-Date: [unreleased]
Protocols: dict file ftp ftps gopher gophers http https imap imaps mqtt pop3 pop3s rtsp smb smbs smtp smtps telnet tftp 
Features: alt-svc AsynchDNS HSTS HTTPS-proxy IDN IPv6 Largefile NTLM NTLM_WB SSL threadsafe TLS-SRP UnixSockets
commit d4fed2a13a81d23e73f1fb491c335a1b1d91e3fb
export https_proxy=http://127.0.0.5:3128
export http_proxy=http://127.0.0.5:3128
export no_proxy=localhost,.google.com,.google.de
curl -vv -o /dev/null http://www.google.com/testfile

And again it tries to go via that proxy where it should not.

@bagder bagder reopened this Oct 28, 2022
@bagder
Copy link
Member

bagder commented Oct 28, 2022

Not the same problem as first reported though.

@bagder
Copy link
Member

bagder commented Oct 28, 2022

filed #9821 for the new one

@bagder bagder closed this as completed Oct 28, 2022
@bagder bagder changed the title 7.86.0 problems in handling --noproxy and related env variable IP addresses mismatch in --noproxy and related env variable (in 7.86.0) Oct 28, 2022
@henning-schild
Copy link
Author

Not the same problem as first reported though.

Maybe yes, one could argue but it does not matter. Thanks for taking this into a separate issue.

@henning-schild
Copy link
Author

henning-schild commented Nov 14, 2022

arch was not notfied so let my try with a ping @eworm-de

alpine is also affected @ncopa

opensuse tumbleweed is now also affected by both issues, also trying a ping @pmgdeb

debian bookworm is affected ... ping @samueloph

fedora and gentoo have backported the patches for this one and #9821

https://github.com/gentoo/gentoo/blob/master/net-misc/curl/curl-7.86.0-r1.ebuild#L98

https://src.fedoraproject.org/rpms/curl/c/394bdcb95657341453d63fe508d549572fee9083

Sorry for the direct maintainer SPAM, but i think this one is important and easy to miss. So i did dig out some names of distros and people to notify all at once, not following individual workflows. Please do not ask me to open a bug, i might do that for debian but not the rest.

@eworm-de
Copy link
Contributor

Should be fixed in curl 7.86.0-4... I hope I found all relevant commits.

@samueloph
Copy link
Contributor

Pushed the fixes to the Debian package at 7.86.0-2.

The following commits were picked:
b830f9b
efc286b
b1953c1

Thank you for the heads up @henning-schild :)

@henning-schild
Copy link
Author

The following commits were picked:
(b1953c1)

#9842

Nice catch, i did not know about that third one yet. Will try to get that into gentoo as well. But it seems a little less severe so maybe people can wait for the next release.

@fbezdeka
Copy link

@kdudka : Seems Fedora rawhide is missing b830f9b and b1953c1 to fully solve this issue.
I also opened https://bugzilla.redhat.com/show_bug.cgi?id=2149224 to track this issue.

henning-schild referenced this issue Jan 6, 2023
When there are filters addded for both socket and SSL, the code
previously checked the SSL sockets during connect when it *should* first
check the socket layer until that has connected.

Fixes #10157
Fixes #10146
Closes #10160

Reviewed-by: Stefan Eissing
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

6 participants