Skip to content

Win32/SCHANNEL align --cacert behaviour with OpenSSL and LibreSSL #17418

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

Closed
wants to merge 2 commits into from

Conversation

rodwiddowson
Copy link
Contributor

@rodwiddowson rodwiddowson commented May 22, 2025

I'm pushing this PR to act as a point of reference for others.

I would not be surprised to see it rejected since I appreciate that there are disagreements on how the spec should be implemented.

Also, I feel uncomfortable changing behaviour by default (particularly in a way that renders the code more lenient) and the fact that the behaviour is different depending on the host OS version is weird. But the alternative is to build an entire option stack to do the exact opposite of CURLSSLOPT_NO_PARTIALCHAIN and it doesn't seem to be worth that

As per the commit message:


Win8/Server2012 widened the PKIX chain traversal API to allow certificate
traversal to terminate at an intermediate.

This behaviour (terminate at the fist matching intermediate) is the default
for LibreSSL and OpenSSL (with OpenSSL allowing control via
CURLSSLOPT_NO_PARTIALCHAIN)

This change uses the new API if it is available, and also allows the behaviour
to revert legacy if CURLSSLOPT_NO_PARTIALCHAIN is present.

@testclutch
Copy link

Analysis of PR #17418 at 877b181e:

Test 3028 failed, which has NOT been flaky recently, so there could be a real issue in this PR.

Generated by Testclutch

@rodwiddowson rodwiddowson force-pushed the schannel branch 5 times, most recently from d2a52a8 to a078f67 Compare May 22, 2025 15:14
@rodwiddowson
Copy link
Contributor Author

rodwiddowson commented May 22, 2025

Apologies for the noise as I was getting the code style conformant across all architectures. Its been too long.

A bit surprised by

Test 3028 failed, which has NOT been flaky recently, so there could be a real issue in this PR.

AFAICS there are no failures. OTOH this is exactly something I would expect to see a regression test fail for

@dfandrich
Copy link
Contributor

The failed test was on your first commit 877b181, not the two subsequent commits which you force pushed afterwards. Test Clutch only looks at one commit in any given PR.

@rodwiddowson
Copy link
Contributor Author

thanks @dfandrich (and again sorry for the checkin noise)

@bagder
Copy link
Member

bagder commented May 23, 2025

It would be great if you could amend this by explaining what "align --cacert behaviour with OpenSSL and LibreSSL" actually means. Aligning how? I believe you refer to accepting intermediate certificates found in the CA store as trust-anchors ?

Win8/Server2012 widened the PKIX chain traversal API to allow certificate
traversal to terminate at an intermediate.

This behaviour (terminate at the fist matching intermediate) is the default
for LibreSSL and OpenSSL (with OpenSSL allowing control via
CURLSSLOPT_NO_PARTIALCHAIN)

This change uses the new API if it is available, and also allows the behaviour
to revert legacy if CURLSSLOPT_NO_PARTIALCHAIN is present.
@rodwiddowson
Copy link
Contributor Author

Updated the commit message slightly and force-pushed. Copied most of of the commit message into the PR description

@jay
Copy link
Member

jay commented May 24, 2025

Ref: #4655

@jay jay added the feature-window A merge of this requires an open feature window label May 24, 2025
@jay
Copy link
Member

jay commented May 24, 2025

I'll land this in the next feature window

- Clarify that CURLSSLOPT_NO_PARTIALCHAIN now affects Schannel.
@jay
Copy link
Member

jay commented May 24, 2025

I think this should also have something in the documentation for CURLSSLOPT_NO_PARTIALCHAIN so I pushed a suggestion "Works with Schannel if the user specified certificates to verify the peer. (Added in 8.15.0)"

@jay jay closed this in df1ff17 Jun 14, 2025
@jay
Copy link
Member

jay commented Jun 14, 2025

Thanks

@vszakats vszakats mentioned this pull request Jun 15, 2025
vszakats added a commit that referenced this pull request Jun 15, 2025
Follow-up to 2c27a67 #17590
Follow-up to df1ff17 #17418

Closes #17624
denandz pushed a commit to denandz/curl that referenced this pull request Jun 21, 2025
- Align --cacert behaviour with OpenSSL and LibreSSL.

This changes the default behavior of Schannel manual certificate
verification, which is used when the user provides their own CA
certificates for verification, to accept partial chains. In other words,
the user may provide an intermediate certificate without having to
provide the root CA.

Win8/Server2012 widened the PKIX chain traversal API to allow
certificate traversal to terminate at an intermediate.

This behaviour (terminate at the fist matching intermediate) is the
default for LibreSSL and OpenSSL (with OpenSSL allowing control via
CURLSSLOPT_NO_PARTIALCHAIN).

This change uses the new API if it is available, and also allows the
behaviour to revert legacy if CURLSSLOPT_NO_PARTIALCHAIN is present.

Closes curl#17418
denandz pushed a commit to denandz/curl that referenced this pull request Jun 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-window A merge of this requires an open feature window TLS Windows Windows-specific
Development

Successfully merging this pull request may close these issues.

5 participants