-
-
Notifications
You must be signed in to change notification settings - Fork 6.7k
schannel: Handle pkcs12 client certificates which contain CA certificates #16825
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
JDepooter
wants to merge
3
commits into
curl:master
from
JDepooter:schannel_support_p12_file_with_ca_certs
Closed
schannel: Handle pkcs12 client certificates which contain CA certificates #16825
JDepooter
wants to merge
3
commits into
curl:master
from
JDepooter:schannel_support_p12_file_with_ca_certs
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ates The SChannel code uses the CertFindCertificateInStore function to retrieve the client certificate from a pkcs12 certificate store. However, when called with the CERT_FIND_ANY flag, this function does not provide any guarantees on the order in which certificates are retrieved. If a pkcs12 file contains an entire certificate chain instead of a single client certificate, the CertFindCertificateInStore function may return the CA or an intermediate certificate instead of the desired client certificate. Since there is no associated private key for such a certificate, the TLS handshake fails. With this change, we now pass the CERT_FIND_HAS_PRIVATE_KEY flag. This ensures that the CertFindCertificateInStore function will return a certificate which has a corresponding private key. This will stop the CA and intermediate certificates from being selected. I don't think there would be much use in a client certificate which has no associated private key, so this should ensure the client certificate is selected. I suppose it may be possible for a pkcs12 file to contain multiple certificates with private keys and the new behaviour may not guarantee which is selected. However, this is no worse that the previous behaviour in which any certificate may been selected. The CERT_FIND_HAS_PRIVATE_KEY is only available in Windows 8 / Server 2012 (aka Windows NT6.2). For older versions, we will fall back to using the CERT_FIND_ANY flag.
293e114
to
b003bbc
Compare
vszakats
reviewed
Mar 25, 2025
vszakats
reviewed
Mar 25, 2025
This comment was marked as outdated.
This comment was marked as outdated.
LGTM from my end. |
bagder
approved these changes
Apr 15, 2025
Thanks! (also thanks for the reminder) |
nbaws
pushed a commit
to nbaws/curl
that referenced
this pull request
Apr 26, 2025
…ates The SChannel code uses the CertFindCertificateInStore function to retrieve the client certificate from a pkcs12 certificate store. However, when called with the CERT_FIND_ANY flag, this function does not provide any guarantees on the order in which certificates are retrieved. If a pkcs12 file contains an entire certificate chain instead of a single client certificate, the CertFindCertificateInStore function may return the CA or an intermediate certificate instead of the desired client certificate. Since there is no associated private key for such a certificate, the TLS handshake fails. With this change, we now pass the CERT_FIND_HAS_PRIVATE_KEY flag. This ensures that the CertFindCertificateInStore function will return a certificate which has a corresponding private key. This will stop the CA and intermediate certificates from being selected. I don't think there would be much use in a client certificate which has no associated private key, so this should ensure the client certificate is selected. I suppose it may be possible for a pkcs12 file to contain multiple certificates with private keys and the new behaviour may not guarantee which is selected. However, this is no worse that the previous behaviour in which any certificate may been selected. The CERT_FIND_HAS_PRIVATE_KEY is only available in Windows 8 / Server 2012 (aka Windows NT6.2). For older versions, we will fall back to using the CERT_FIND_ANY flag. Closes curl#16825
nbaws
pushed a commit
to nbaws/curl
that referenced
this pull request
Apr 26, 2025
…ates The SChannel code uses the CertFindCertificateInStore function to retrieve the client certificate from a pkcs12 certificate store. However, when called with the CERT_FIND_ANY flag, this function does not provide any guarantees on the order in which certificates are retrieved. If a pkcs12 file contains an entire certificate chain instead of a single client certificate, the CertFindCertificateInStore function may return the CA or an intermediate certificate instead of the desired client certificate. Since there is no associated private key for such a certificate, the TLS handshake fails. With this change, we now pass the CERT_FIND_HAS_PRIVATE_KEY flag. This ensures that the CertFindCertificateInStore function will return a certificate which has a corresponding private key. This will stop the CA and intermediate certificates from being selected. I don't think there would be much use in a client certificate which has no associated private key, so this should ensure the client certificate is selected. I suppose it may be possible for a pkcs12 file to contain multiple certificates with private keys and the new behaviour may not guarantee which is selected. However, this is no worse that the previous behaviour in which any certificate may been selected. The CERT_FIND_HAS_PRIVATE_KEY is only available in Windows 8 / Server 2012 (aka Windows NT6.2). For older versions, we will fall back to using the CERT_FIND_ANY flag. Closes curl#16825
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The SChannel code uses the
CertFindCertificateInStore
function to retrieve the client certificate from a pkcs12 certificate store. However, when called with theCERT_FIND_ANY
flag, this function does not provide any guarantees on the order in which certificates are retrieved. If a pkcs12 file contains an entire certificate chain instead of a single client certificate, theCertFindCertificateInStore
function may return the CA or an intermediate certificate instead of the desired client certificate. Since there is no associated private key for such a certificate, the TLS handshake fails.With this change, we now pass the
CERT_FIND_HAS_PRIVATE_KEY
flag. This ensures that theCertFindCertificateInStore
function will return a certificate which has a corresponding private key. This will stop the CA and intermediate certificates from being selected. I don't think there would be much use in a client certificate which has no associated private key, so this should ensure the client certificate is selected. I suppose it may be possible for a pkcs12 file to contain multiple certificates with private keys and the new behaviour may not guarantee which is selected. However, this is no worse that the previous behaviour in which any certificate may been selected.The
CERT_FIND_HAS_PRIVATE_KEY
flag is only available in Windows 8 / Server 2012 (aka Windows NT6.2). For older versions, we will fall back to using theCERT_FIND_ANY
flag.