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

curl verifies incorrect subject alternative name when contacting https proxy #12831

Closed
HsiehYuho opened this issue Jan 31, 2024 · 10 comments
Closed

Comments

@HsiehYuho
Copy link

I did this

I tried

curl -x https://[proxy:ipv6]:8082 https://destination.hostname

It gives me SAN match failure.

SSL:no alternative certificate subject name matches target hostname [proxy:ipv6]

However, the [proxy:ipv6] is in the proxy cert SAN field.
When switching to

curl -x https://[proxy:ipv6]:8082 https://[destination.ipv6]

the curl succeeds.

We (my colleagues and me) are suspecting checks around here https://github.com/curl/curl/blob/master/lib/vtls/openssl.c#L2152 lead to that curl only checks the proxy SAN type that is same as the destination type

I expected the following

curl -x https://[proxy:ipv6]:8082 https://destination.hostname 
curl -x https://[proxy:ipv6]:8082 https://[destination.ipv6]

should at least give me the same result

curl/libcurl version

curl 7.61.1 (x86_64-redhat-linux-gnu)

operating system

Linux, but our company (meta) dev our own linux ..

@HsiehYuho
Copy link
Author

I didn't spend time working on a repro but we fairly sure that there is at least inconsistent output where there shouldn't be.

@dfandrich
Copy link
Contributor

dfandrich commented Jan 31, 2024 via email

@HsiehYuho
Copy link
Author

Hi Dan,
Yes, it is reproducible using the latest curl

./src/.libs/curl --version
curl 8.6.0-DEV (x86_64-pc-linux-gnu) 

we are using openssl.

@icing
Copy link
Contributor

icing commented Feb 1, 2024

@HsiehYuho I added a test case using '127.0.0.1' for the proxy and gave it a certificate with a x590 IPAddress in the alt names. This test works with curl 8.6.0.

Could you recheck your findings? Is there anything specific in your ipv6 address spec that we may not account for?

bagder pushed a commit that referenced this issue Feb 6, 2024
- improve info logging when peer verification fails to indicate
  if DNS name or ip address has been tried to match
- add test case for contacting https proxy with ip address
- add pytest env check on loaded credentials and re-issue
  when they are no longer valid
- disable proxy ip address test for bearssl, since not supported there

Ref: #12831
Closes #12838
@knekritz
Copy link

knekritz commented Feb 9, 2024

@icing I think the issue is specific to IPv6 because of the conn->bits.ipv6_ip check at

if(conn->bits.ipv6_ip &&

icing added a commit to icing/curl that referenced this issue Feb 13, 2024
- refs curl#12831
- when verifying a proxy certificate for an ip address, use
  the correct ip family. That may be different from the
  "connection" ip family.
@icing
Copy link
Contributor

icing commented Feb 13, 2024

@knekritz, @HsiehYuho thanks, that is exactly the right spot. Would you be able to verify that #12931 fixes the issue for you as well?

@HsiehYuho
Copy link
Author

@icing i tried to verify it. I git clone your branch and follow https://github.com/curl/curl/blob/master/GIT-INFO to make the binary, then execute it through

[yhhsieh@mydev ~/curl (proxy-ipv6-cert-check)]$ ./src/.libs/curl telnet://DESTINATION_HOSTNAME:PORT -p -x  https://[PROXY:IPV6]:8082  --proxy-cert $PROXY_CERT --proxy-key $PRXOY_KEY -vvv 

but it seems not hit you fix. I suspect my build is wrong. the ./src/.libs/curl was built but the commit seems not applied. Did i do anything wrong ?

@jay
Copy link
Member

jay commented Feb 14, 2024

./src/.libs/curl

Use src/curl instead, which may be a libtool wrapper script that uses LD_LIBRARY_PATH to load the libcurl that's in lib/.libs

@icing
Copy link
Contributor

icing commented Feb 14, 2024

@HsiehYuho if you run ./src/curl with options -v --trace-config all and your other arguments, what is the output?

@HsiehYuho
Copy link
Author

@jay @icing thank you guys so much! Yes if i replace with ./src/curl then the fix kicks in and works! I think the fix is the right one!

@jay jay closed this as completed in e87751d Feb 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

6 participants