-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
schannel: use CERT_CONTEXT's pbCertEncoded to determine chain order #11632
Conversation
b16426f
to
40555a3
Compare
cebc66b
to
b56b6d2
Compare
The xml test file needs to be in LF format, except for the request responses (data,datacheck,protocol etc) which are CRLF. It seems I can't do this for you probably because your PR comes from an organization. The line endings should look like this for example (screenshot from test3101): ci job for example this one: |
b56b6d2
to
b1db7e5
Compare
4c17fda
to
006344f
Compare
It looks like the test is not running in Windows CI jobs but it's running in Linux jobs. I don't see anything obvious that could be causing that. |
Which Windows CI jobs are you looking at? The ones I am seeing from AppVeyor only have a short build log. The only thing I can think of is maybe it doesn't like this: # SSL with libraries supporting CURLOPT_CERTINFO
<features>
SSL
!bearssl
!mbedtls
!rustls
!wolfssl
</features> Perhaps I need to specifically include |
It looks as though tests that use SSL are not running in Windows CI jobs because of an stunnel bug. mback2k/curl-docker-winbuildenv#2. I still do not know why some of the test results do not show a skipped test message for this test, which is what should happen if the test is skipped. It may be because of the way stunnel fails. The installation is valid and the file is executable but its version information can't be parsed. There's logic that lists SSL as a supported test type but then none of those tests actually run because stunnel technically isn't considered valid if the version info parsing fails. |
Thanks for diving in and looking at it. I reviewed those two PRs, and I can rebase after they are merged in. |
006344f
to
452f9fe
Compare
Rebased |
This reverts commit f540a39.
CERT_CONTEXT from SECPKG_ATTR_REMOTE_CERT_CONTEXT contains end-entity/server certificate in pbCertEncoded. We can use this pointer to determine the order of certificates when enumerating hCertStore using CertEnumCertificatesInStore.
Uses lib3102 tool. Test data is based on test560.
452f9fe
to
243ea04
Compare
Rebased again. |
Thanks |
- Use CERT_CONTEXT's pbCertEncoded to determine chain order. CERT_CONTEXT from SECPKG_ATTR_REMOTE_CERT_CONTEXT contains end-entity/server certificate in pbCertEncoded. We can use this pointer to determine the order of certificates when enumerating hCertStore using CertEnumCertificatesInStore. This change is to help ensure that the ordering of the certificate chain requested by the user via CURLINFO_CERTINFO has the same ordering on all versions of Windows. Prior to this change Schannel certificate order was reversed in 8986df8 but that was later reverted in f540a39 when it was discovered that Windows 11 22H2 does the reversal on its own. Ref: curl#9706 Closes curl#11632
CERT_CONTEXT
fromSECPKG_ATTR_REMOTE_CERT_CONTEXT
contains end-entity/server certificate inpbCertEncoded
. We can use this pointer to determine the order of certificates when enumeratinghCertStore
usingCertEnumCertificatesInStore
.I tested this method of determining enumeration order on Windows 10 and Windows 11.
For background info see PR #9706 and this comment.