From fb2e61e92572273dbbc356cb1c80e67f82ec143d Mon Sep 17 00:00:00 2001 From: Oded Hanson Date: Tue, 22 Jun 2021 01:14:16 +0300 Subject: [PATCH] Add name of corrupted certificate to CryptographicException on Mac * Add name of corrupted certificate to CryptographicException on Mac When trying to create a CertificateData out of raw X509 byte array it might throw if the data is corrupted. The existing exception message does not provide any information which might help the user identify the corrupted certificate and fix it. This change, makes a native call to SecCertificateCopySubjectSummary which will provide a human readable summary of the certificate, and will generate an exception message using this string. Co-authored-by: Jeremy Barton --- .../Interop.X509.cs | 30 +++++++++++++++++++ .../entrypoints.c | 1 + .../pal_x509.c | 12 ++++++++ .../pal_x509.h | 10 +++++++ .../Pal.OSX/AppleCertificatePal.cs | 21 ++++++++++++- .../src/Resources/Strings.resx | 3 ++ 6 files changed, 76 insertions(+), 1 deletion(-) diff --git a/src/libraries/Common/src/Interop/OSX/System.Security.Cryptography.Native.Apple/Interop.X509.cs b/src/libraries/Common/src/Interop/OSX/System.Security.Cryptography.Native.Apple/Interop.X509.cs index 7094e90dd8fe4..b7a8be1ba8a7c 100644 --- a/src/libraries/Common/src/Interop/OSX/System.Security.Cryptography.Native.Apple/Interop.X509.cs +++ b/src/libraries/Common/src/Interop/OSX/System.Security.Cryptography.Native.Apple/Interop.X509.cs @@ -20,6 +20,11 @@ internal static partial class AppleCrypto out SafeCFDataHandle cfDataOut, out int pOSStatus); + [DllImport(Libraries.AppleCryptoNative)] + private static extern int AppleCryptoNative_X509GetSubjectSummary( + SafeSecCertificateHandle cert, + out SafeCFStringHandle cfSubjectSummaryOut); + [DllImport(Libraries.AppleCryptoNative)] private static extern int AppleCryptoNative_X509GetPublicKey(SafeSecCertificateHandle cert, out SafeSecKeyRefHandle publicKey, out int pOSStatus); @@ -72,6 +77,31 @@ internal static byte[] X509GetRawData(SafeSecCertificateHandle cert) } } + internal static string? X509GetSubjectSummary(SafeSecCertificateHandle cert) + { + SafeCFStringHandle subjectSummary; + + int ret = AppleCryptoNative_X509GetSubjectSummary( + cert, + out subjectSummary); + + using (subjectSummary) + { + if (ret == 1) + { + return CoreFoundation.CFStringToString(subjectSummary); + } + } + + if (ret == 0) + { + return null; + } + + Debug.Fail($"Unexpected return value {ret}"); + throw new CryptographicException(); + } + internal static SafeSecKeyRefHandle X509GetPrivateKeyFromIdentity(SafeSecIdentityHandle identity) { SafeSecKeyRefHandle key; diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/entrypoints.c b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/entrypoints.c index ff4df6f41fcb2..1833d4a2161ac 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/entrypoints.c +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/entrypoints.c @@ -106,6 +106,7 @@ static const Entry s_cryptoAppleNative[] = DllImportEntry(AppleCryptoNative_GetOSStatusForChainStatus) DllImportEntry(AppleCryptoNative_X509ChainSetTrustAnchorCertificates) DllImportEntry(AppleCryptoNative_Pbkdf2) + DllImportEntry(AppleCryptoNative_X509GetSubjectSummary) }; EXTERN_C const void* CryptoAppleResolveDllImport(const char* name); diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/pal_x509.c b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/pal_x509.c index 7d3e61f4b5d57..0b6d1f889bc69 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/pal_x509.c +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/pal_x509.c @@ -230,3 +230,15 @@ int32_t AppleCryptoNative_X509GetRawData(SecCertificateRef cert, CFDataRef* ppDa *pOSStatus = *ppDataOut == NULL ? errSecParam : noErr; return (*pOSStatus == noErr); } + +int32_t AppleCryptoNative_X509GetSubjectSummary(SecCertificateRef cert, CFStringRef* ppSummaryOut) +{ + if (ppSummaryOut != NULL) + *ppSummaryOut = NULL; + + if (cert == NULL || ppSummaryOut == NULL) + return kErrorBadInput; + + *ppSummaryOut = SecCertificateCopySubjectSummary(cert); + return (*ppSummaryOut != NULL); +} diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/pal_x509.h b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/pal_x509.h index 28124de10c98d..a0bc58044c3eb 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/pal_x509.h +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/pal_x509.h @@ -72,3 +72,13 @@ ppDataOut: Receives a CFDataRef with the exported blob pOSStatus: Receives the result of SecItemExport */ PALEXPORT int32_t AppleCryptoNative_X509GetRawData(SecCertificateRef cert, CFDataRef* ppDataOut, int32_t* pOSStatus); + +/* +Extract a string that contains a human-readable summary of the contents of the certificate + +Returns 1 on success, 0 on failure, any other value indicates invalid state. + +Output: +ppSummaryOut: Receives a CFDataRef with the exported blob +*/ +PALEXPORT int32_t AppleCryptoNative_X509GetSubjectSummary(SecCertificateRef cert, CFStringRef* ppSummaryOut); diff --git a/src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.OSX/AppleCertificatePal.cs b/src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.OSX/AppleCertificatePal.cs index cb949b8a93f7a..07959c3e77b5a 100644 --- a/src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.OSX/AppleCertificatePal.cs +++ b/src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.OSX/AppleCertificatePal.cs @@ -399,7 +399,26 @@ private void EnsureCertData() return; Debug.Assert(!_certHandle.IsInvalid); - _certData = new CertificateData(Interop.AppleCrypto.X509GetRawData(_certHandle)); + string? subjectSummary = Interop.AppleCrypto.X509GetSubjectSummary(_certHandle); + + try + { + _certData = new CertificateData(Interop.AppleCrypto.X509GetRawData(_certHandle)); + } + catch (CryptographicException e) + { + if (subjectSummary is null) + { + throw; + } + + string message = SR.Format( + SR.Cryptography_X509_CertificateCorrupted, + subjectSummary); + + throw new CryptographicException(message, e); + } + _readCertData = true; } } diff --git a/src/libraries/System.Security.Cryptography.X509Certificates/src/Resources/Strings.resx b/src/libraries/System.Security.Cryptography.X509Certificates/src/Resources/Strings.resx index a5c33a43b40b8..322e2bab91d0c 100644 --- a/src/libraries/System.Security.Cryptography.X509Certificates/src/Resources/Strings.resx +++ b/src/libraries/System.Security.Cryptography.X509Certificates/src/Resources/Strings.resx @@ -331,6 +331,9 @@ The key contents do not contain a PEM, the content is malformed, or the key does not match the certificate. + + Certificate '{0}' is corrupted. + Enumeration has not started. Call MoveNext.