schannel: when importing PFX, disable key persistence #9460
Conversation
By default, the PFXImportCertStore API persists the key in the user's key store (as though the certificate was being imported for permanent, ongoing use.) The documentation specifies that keys that are not to be persisted should be imported with the flag `PKCS12_NO_PERSIST_KEY`. NOTE: this flag is only supported on versions of Windows newer than XP and Server 2003. Closes curl#9300
…ection Since we are no longer storing the key in permanent storage, we need to keep the handle to the cert store that we produced in client cert loading and close it only when we're done with all SSL transactions.
|
This was tested for use in |
|
The tests pass locally at the top of my branch (04c11a4), and it seems like these specific build runs (mingw64, WinIDN) are failing in a few other pull requests. I marked the pull request as draft until I could validate the tests, so I'm going to mark it as ready for review now. |
By default, the PFXImportCertStore API persists the key in the user's key store (as though the certificate was being imported for permanent, ongoing use.) The documentation specifies that keys that are not to be persisted should be imported with the flag `PKCS12_NO_PERSIST_KEY`. NOTE: this flag is only supported on versions of Windows newer than XP and Server 2003. --- This is take 2 of the original fix. It extends the lifetime of the client certificate store to that of the credential handle. The original fix which landed in 70d010d and was later reverted in aec8d30 failed to work properly because it did not do that. Fixes curl#9300 Closes curl#9460
|
Pushed an update. I've taken your updated commit message into the PR body for tracking purposes. Thanks for the review! |
wincrypt defines symbols we test for such as CryptStringToBinary, therefore it must always be included beforehand. prior to this change it was not included for mingw.
|
I've pushed a change to your branch to always include wincrypt, since now there is a test to see if CryptStringToBinary is defined, which is defined by wincrypt if it is defined. Assuming the CI has no problems I will review again but otherwise this looks ready. |
|
Thanks! That makes sense. Surprising number of OpenSSL and WolfSSL failures in CI. Are those already known/baselined? |
|
The 3 CI failures I see are unrelated. The BoringSSL failure might be an actual problem, though unrelated, and the other two look like random failures that occasionally happen due to resource constraints. |
Ah, makes sense. Is there anything I need to do to get this over the finish line for the coming release? 😄 Thanks! |
|
@jay any remaining nits or can we merge? |
|
Thanks! |
By default, the PFXImportCertStore API persists the key in the user's key store (as though the certificate was being imported for permanent, ongoing use.) The documentation specifies that keys that are not to be persisted should be imported with the flag PKCS12_NO_PERSIST_KEY. NOTE: this flag is only supported on versions of Windows newer than XP and Server 2003. -- This is take 2 of the original fix. It extends the lifetime of the client certificate store to that of the credential handle. The original fix which landed in 70d010d and was later reverted in aec8d30 failed to work properly because it did not do that. Minor changes were made to the schannel credential context to support closing the client certificate store handle at the end of an SSL session. -- Reported-by: ShadowZzj@users.noreply.github.com Fixes curl#9300 Supersedes curl#9363 Closes curl#9460
By default, the PFXImportCertStore API persists the key in the user's key store (as though the certificate was being imported for permanent, ongoing use.) The documentation specifies that keys that are not to be persisted should be imported with the flag PKCS12_NO_PERSIST_KEY. NOTE: this flag is only supported on versions of Windows newer than XP and Server 2003. -- This is take 2 of the original fix. It extends the lifetime of the client certificate store to that of the credential handle. The original fix which landed in 70d010d and was later reverted in aec8d30 failed to work properly because it did not do that. Minor changes were made to the schannel credential context to support closing the client certificate store handle at the end of an SSL session. -- Reported-by: ShadowZzj@users.noreply.github.com Fixes curl/curl#9300 Supersedes curl/curl#9363 Closes curl/curl#9460
By default, the PFXImportCertStore API persists the key in the user's
key store (as though the certificate was being imported for permanent,
ongoing use.)
The documentation specifies that keys that are not to be persisted
should be imported with the flag
PKCS12_NO_PERSIST_KEY.NOTE: this flag is only supported on versions of Windows newer than XP
and Server 2003.
--
This is take 2 of the original fix. It extends the lifetime of the
client certificate store to that of the credential handle. The original
fix which landed in 70d010d and was later reverted in aec8d30 failed to
work properly because it did not do that.
Minor changes were made to the schannel credential context to support
closing the client certificate store handle at the end of an SSL session.
Supersedes #9363
Closes #9300