Finish off the X509Certificates implementation on macOS. #16707
Conversation
* Enable add/remove from a keychain-backed store. * Improve X509KeyStorageFlags usage * PersistKeySet will save a PFX key (and certificate) into the default keychain * EphemeralKeySet will throw PNSE * Exportable is respected for PFX * Add support for reading the Disallowed trust. * This will only read "full deny" behaviors * Enable or permanent-assign all remaining ActiveIssue(-1) tests. * Add DSA support to cert.PrivateKey from the Apple PAL * And add a test for it
bartonjs
force-pushed
the
apple_x509_part3
branch
from
9d8fd9d to
49ddd36
Compare
| @@ -373,13 +414,22 @@ extern "C" int32_t AppleCryptoNative_X509ImportCertificate(uint8_t* pbData, | |||
| if (pOSStatus != nullptr) | |||
| *pOSStatus = noErr; | |||
|
|
|||
| if (pbData == nullptr || cbData < 0 || pCertOut == nullptr || pIdentityOut == nullptr || pOSStatus == nullptr) | |||
| if (pbData == nullptr || cbData < 0 || pCertOut == nullptr || pIdentityOut == nullptr || pOSStatus == nullptr || | |||
| exportable != !!exportable) | |||
There was a problem hiding this comment.
The !! is being used to normalize the Boolean value?
There was a problem hiding this comment.
Yeah, making 2 and beyond be invalid leaves us open to recycle that spot for a flags value later if needed.
| } | ||
| catch | ||
| { | ||
| tmpKeychain.Dispose(); | ||
| keychain.Dispose(); |
There was a problem hiding this comment.
I assume no problems will be caused by disposing of the singleton InvalidHandle?
There was a problem hiding this comment.
Correct. The Dispose method checks to see if it's the singleton InvalidInstance and if so doesn't call down to base.Dispose(). (The existing SafeHandleCache usage/pattern)
| // netstandard: DefaultKeySet | ||
| // netcoreapp-OSX: DefaultKeySet | ||
| // netcoreapp-other: EphemeralKeySet | ||
| internal static readonly X509KeyStorageFlags EphemeralIfPossible = | ||
| #if netcoreapp11 |
There was a problem hiding this comment.
Do we still need this "#if netcoreapp11"s in the code?
There was a problem hiding this comment.
The test library builds and runs for both netcore and netstandard. EphemeralKeySet is a new feature to netcoreapp.
| Assert.Throws<PlatformNotSupportedException>( | ||
| () => | ||
| { | ||
| using (Cert.Import(TestData.EmptyPfx, null, X509KeyStorageFlags.EphemeralKeySet)) |
There was a problem hiding this comment.
What's throwing the PNSE? Assuming it's the call to Import, there's no need for the using.
There was a problem hiding this comment.
Fair enough. I'm pretty sure I was guarding it so that if it didn't throw it wouldn't finalize... but I guess I wouldn't care about the finalization numbers when tests were failing, and there's prior art for asserting that the X509Certificate2 ctor throws without having a using.
Move the new Add/Remove certificate methods to a new Interop file, since they're more about acting on a cert store than the keychain.
…fx#16707) * Enable add/remove from a keychain-backed store. * Improve X509KeyStorageFlags usage * PersistKeySet will save a PFX key (and certificate) into the default keychain * EphemeralKeySet will throw PNSE * Exportable is respected for PFX * Add support for reading the Disallowed trust. * This will only read "full deny" behaviors * Enable or permanent-assign all remaining ActiveIssue(-1) tests. * Add DSA support to cert.PrivateKey from the Apple PAL * And add a test for it Commit migrated from dotnet/corefx@5694965
Enable add/remove from a keychain-backed store.
Improve X509KeyStorageFlags usage
Add support for reading the Disallowed trust.
Enable or permanent-assign all remaining ActiveIssue(-1) tests.
Add DSA support to cert.PrivateKey from the Apple PAL