Skip to content
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

Add name of corrupted certificate to CryptographicException on Mac #54477

Merged
merged 6 commits into from
Jun 21, 2021

Conversation

odhanson
Copy link
Member

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.

fix #54336

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.

fix dotnet#54336
@ghost
Copy link

ghost commented Jun 21, 2021

Tagging subscribers to this area: @bartonjs, @vcsjones, @krwq, @GrabYourPitchforks
See info in area-owners.md if you want to be subscribed.

Issue Details

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.

fix #54336

Author: odhanson
Assignees: -
Labels:

area-System.Security

Milestone: -

@odhanson
Copy link
Member Author

Couldn't find a way to automate a test for this, but performed a manual test by following @vcsjones advice and creating a Test keychain, and importing the following corrupted key into it:

-----BEGIN CERTIFICATE-----
MIIDazCCAlOgAwIBAgIUD3IKm1vJUWEyang38XAtHV9WQCYwDQYJ////////////
BQAwRTELMAkGA1UEBhMCQVUxEzARBgNVBAgMClNvbWUtU3RhdGUxITAfBgNVBAoM
GEludGVybmV0IFdpZGdpdHMgUHR5IEx0ZDAeFw0yMTA2MTcxNzQ4MjBaFw0yMTA3
MTcxNzQ4MjBaMEUxCzAJBgNVBAYTAkFVMRMwEQYDVQQIDApTb21lLVN0YXRlMSEw
HwYDVQQKDBhJbnRlcm5ldCBXaWRnaXRzIFB0eSBMdGQwggEiMA0GCSqGSIb3DQEB
AQUAA4IBDwAwggEKAoIBAQDFa9HIa+9SMgU1RnhHrXiwTDB3yf5y3OFp7FzjM144
T10KVKrM3yAUrEeFq4CSy6SfkP4vxVMuRSa5V8LSqUqskx8OqNgGubHJFL50pERG
kxNYR1ixrAJwHhN/6qkHjVjKO9PIGMme09pRKUjNwheQpRJkB59VwxwiOlBe64sU
2fDTZQDQfxQ6Nhq6U/8aRDIoUCmkc9UsCX10+OpzylCn95L3IKYu3HLefTZvshzq
I9Wg9EGNbT2VHeanfmqil/YjMcOBPTuRghAYtUgY4QzJ3BPijl962qGg3ffjakS6
2S4ssMcziqoCIotSSbGCt1qZWMPJ/JIH95ZytYNCU6vZAgMBAAGjUzBRMB0GA1Ud
DgQWBBRdEFst6BHxQsvCpQflQfqMm7lyhjAfBgNVHSMEGDAWgBRdEFst6BHxQsvC
pQflQfqMm7lyhjAPBgNVHRMBAf8EBTADAQH/MA0GCSqGSIb3DQEBCwUAA4IBAQDA
KpvZc8vbCg4jOTcGqjTJnJR3BsLtyznxxURwTWOfEjA47NTnmwFol9UggLiGLm9V
khMHe8TZAeNckzzNQ9DgKjVuYm0p8vBw/UI3jCTFi//5hRuqLAnNG+mWl1p0c72b
KSnWE9XF39+EFnCb6swMIHhRE7ZhW5IkvulQpdlMlP+oO850UA9cWHboMrpQexew
wffeOJiqSboEqauCqVxtQeqxFhL2Rt37oBWLCrWyucQfk+47sDTOObPmvDKgGVuu
sXGzqCQwEWbCQUByKG/6ckx85H1syWrH+RRWDK+7cahktUAF3AK0C9e67dhZatwN
TblhkvRxk+XDhnZrxiSP
-----END CERTIFICATE-----

Then the following c# code:

X509Store store = new X509Store("Test");
store.Open(OpenFlags.ReadOnly);
try
{
  foreach (var cert in store.Certificates)
  {
      Console.WriteLine(cert.Subject);
   }
}
catch (Exception r)
{
    Console.WriteLine(r);
}

Will produce the following result:

ystem.Security.Cryptography.CryptographicException: Certificate 'Internet Widgits Pty Ltd' is corrupted.
 ---> System.Security.Cryptography.CryptographicException: Error in reading certificate:
-----BEGIN CERTIFICATE-----
MIIDazCCAlOgAwIBAgIUD3IKm1vJUWEyang38XAtHV9WQCYwDQYJ////////////BQAwRTELMAkG
A1UEBhMCQVUxEzARBgNVBAgMClNvbWUtU3RhdGUxITAfBgNVBAoMGEludGVybmV0IFdpZGdpdHMg
UHR5IEx0ZDAeFw0yMTA2MTcxNzQ4MjBaFw0yMTA3MTcxNzQ4MjBaMEUxCzAJBgNVBAYTAkFVMRMw
EQYDVQQIDApTb21lLVN0YXRlMSEwHwYDVQQKDBhJbnRlcm5ldCBXaWRnaXRzIFB0eSBMdGQwggEi
MA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQDFa9HIa+9SMgU1RnhHrXiwTDB3yf5y3OFp7Fzj
M144T10KVKrM3yAUrEeFq4CSy6SfkP4vxVMuRSa5V8LSqUqskx8OqNgGubHJFL50pERGkxNYR1ix
rAJwHhN/6qkHjVjKO9PIGMme09pRKUjNwheQpRJkB59VwxwiOlBe64sU2fDTZQDQfxQ6Nhq6U/8a
RDIoUCmkc9UsCX10+OpzylCn95L3IKYu3HLefTZvshzqI9Wg9EGNbT2VHeanfmqil/YjMcOBPTuR
ghAYtUgY4QzJ3BPijl962qGg3ffjakS62S4ssMcziqoCIotSSbGCt1qZWMPJ/JIH95ZytYNCU6vZ
AgMBAAGjUzBRMB0GA1UdDgQWBBRdEFst6BHxQsvCpQflQfqMm7lyhjAfBgNVHSMEGDAWgBRdEFst
6BHxQsvCpQflQfqMm7lyhjAPBgNVHRMBAf8EBTADAQH/MA0GCSqGSIb3DQEBCwUAA4IBAQDAKpvZ
c8vbCg4jOTcGqjTJnJR3BsLtyznxxURwTWOfEjA47NTnmwFol9UggLiGLm9VkhMHe8TZAeNckzzN
Q9DgKjVuYm0p8vBw/UI3jCTFi//5hRuqLAnNG+mWl1p0c72bKSnWE9XF39+EFnCb6swMIHhRE7Zh
W5IkvulQpdlMlP+oO850UA9cWHboMrpQexewwffeOJiqSboEqauCqVxtQeqxFhL2Rt37oBWLCrWy
ucQfk+47sDTOObPmvDKgGVuusXGzqCQwEWbCQUByKG/6ckx85H1syWrH+RRWDK+7cahktUAF3AK0
C9e67dhZatwNTblhkvRxk+XDhnZrxiSP
-----END CERTIFICATE-----

 ---> System.Security.Cryptography.CryptographicException: ASN1 corrupted data.
 ---> System.Formats.Asn1.AsnContentException: The ASN.1 value is invalid.
   at System.Formats.Asn1.AsnDecoder.ReadSubIdentifier(ReadOnlySpan`1 source, Int32& bytesRead, Nullable`1& smallValue, Nullable`1& largeValue) in System.Formats.Asn1.dll:token 0x60000ae+0x4f
   at System.Formats.Asn1.AsnDecoder.ReadObjectIdentifier(ReadOnlySpan`1 source, AsnEncodingRules ruleSet, Nullable`1 expectedTag, Int32& totalBytesRead) in System.Formats.Asn1.dll:token 0x60000af+0x44
   at System.Formats.Asn1.AsnDecoder.ReadObjectIdentifier(ReadOnlySpan`1 source, AsnEncodingRules ruleSet, Int32& bytesConsumed, Nullable`1 expectedTag) in System.Formats.Asn1.dll:token 0x60000ad+0x0
   at System.Formats.Asn1.AsnValueReader.ReadObjectIdentifier(Nullable`1 expectedTag) in System.Security.Cryptography.X509Certificates.dll:token 0x60006c5+0x1
   at System.Security.Cryptography.Asn1.AlgorithmIdentifierAsn.DecodeCore(AsnValueReader& reader, Asn1Tag expectedTag, ReadOnlyMemory`1 rebind, AlgorithmIdentifierAsn& decoded) in System.Security.Cryptography.X509Certificates.dll:token 0x6000620+0x1d
   at System.Security.Cryptography.Asn1.AlgorithmIdentifierAsn.Decode(AsnValueReader& reader, Asn1Tag expectedTag, ReadOnlyMemory`1 rebind, AlgorithmIdentifierAsn& decoded) in System.Security.Cryptography.X509Certificates.dll:token 0x600061f+0x2
   --- End of inner exception stack trace ---
   at System.Security.Cryptography.Asn1.AlgorithmIdentifierAsn.Decode(AsnValueReader& reader, Asn1Tag expectedTag, ReadOnlyMemory`1 rebind, AlgorithmIdentifierAsn& decoded) in System.Security.Cryptography.X509Certificates.dll:token 0x600061f+0x1c
   at System.Security.Cryptography.Asn1.AlgorithmIdentifierAsn.Decode(AsnValueReader& reader, ReadOnlyMemory`1 rebind, AlgorithmIdentifierAsn& decoded) in System.Security.Cryptography.X509Certificates.dll:token 0x600061e+0x1
   at System.Security.Cryptography.X509Certificates.Asn1.TbsCertificateAsn.DecodeCore(AsnValueReader& reader, Asn1Tag expectedTag, ReadOnlyMemory`1 rebind, TbsCertificateAsn& decoded) in System.Security.Cryptography.X509Certificates.dll:token 0x60005fe+0x118
   at System.Security.Cryptography.X509Certificates.Asn1.TbsCertificateAsn.Decode(AsnValueReader& reader, Asn1Tag expectedTag, ReadOnlyMemory`1 rebind, TbsCertificateAsn& decoded) in System.Security.Cryptography.X509Certificates.dll:token 0x60005fd+0x2
   at System.Security.Cryptography.X509Certificates.Asn1.TbsCertificateAsn.Decode(AsnValueReader& reader, ReadOnlyMemory`1 rebind, TbsCertificateAsn& decoded) in System.Security.Cryptography.X509Certificates.dll:token 0x60005fc+0x1
   at System.Security.Cryptography.X509Certificates.Asn1.CertificateAsn.DecodeCore(AsnValueReader& reader, Asn1Tag expectedTag, ReadOnlyMemory`1 rebind, CertificateAsn& decoded) in System.Security.Cryptography.X509Certificates.dll:token 0x60005f3+0x1d
   at System.Security.Cryptography.X509Certificates.Asn1.CertificateAsn.Decode(Asn1Tag expectedTag, ReadOnlyMemory`1 encoded, AsnEncodingRules ruleSet) in System.Security.Cryptography.X509Certificates.dll:token 0x60005f2+0x11
   at System.Security.Cryptography.X509Certificates.Asn1.CertificateAsn.Decode(ReadOnlyMemory`1 encoded, AsnEncodingRules ruleSet) in System.Security.Cryptography.X509Certificates.dll:token 0x60005f1+0x1
   at Internal.Cryptography.Pal.CertificateData..ctor(Byte[] rawData) in System.Security.Cryptography.X509Certificates.dll:token 0x600015c+0x10
   --- End of inner exception stack trace ---
   at Internal.Cryptography.Pal.CertificateData..ctor(Byte[] rawData) in System.Security.Cryptography.X509Certificates.dll:token 0x600015c+0x158
   at Internal.Cryptography.Pal.AppleCertificatePal.EnsureCertData() in System.Security.Cryptography.X509Certificates.dll:token 0x6000193+0x3a
   --- End of inner exception stack trace ---
   at Internal.Cryptography.Pal.AppleCertificatePal.EnsureCertData() in System.Security.Cryptography.X509Certificates.dll:token 0x6000193+0x5c
   at Internal.Cryptography.Pal.AppleCertificatePal.get_Thumbprint() in System.Security.Cryptography.X509Certificates.dll:token 0x6000189+0x1
   at System.Security.Cryptography.X509Certificates.X509Certificate.GetRawCertHash() in System.Security.Cryptography.X509Certificates.dll:token 0x60004b3+0x14
   at System.Security.Cryptography.X509Certificates.X509Certificate.GetHashCode() in System.Security.Cryptography.X509Certificates.dll:token 0x60004b9+0x12
   at System.Collections.Generic.HashSet`1.AddIfNotPresent(T value, Int32& location) in System.Private.CoreLib.dll:token 0x6006306+0x38
   at System.Collections.Generic.HashSet`1.Add(T item) in System.Private.CoreLib.dll:token 0x60062ef+0x0
   at Internal.Cryptography.Pal.StorePal.ReadCollection(SafeCFArrayHandle matches, HashSet`1 collection) in System.Security.Cryptography.X509Certificates.dll:token 0x60001f0+0x75
   at Internal.Cryptography.Pal.StorePal.AppleKeychainStore.CloneTo(X509Certificate2Collection collection) in System.Security.Cryptography.X509Certificates.dll:token 0x60001fa+0x37
   at System.Security.Cryptography.X509Certificates.X509Store.get_Certificates() in System.Security.Cryptography.X509Certificates.dll:token 0x60005d1+0x15
   at TestCertificates.Program.Main(String[] args) in /Users/odhanson/git/runtime/Test/Program.cs:line 17

Note that the extra error with the actual certificate data is generated only in Debug mode.

odhanson and others added 3 commits June 21, 2021 18:46
…c/Internal/Cryptography/Pal.OSX/AppleCertificatePal.cs

Co-authored-by: Jeremy Barton <[email protected]>
…c/Internal/Cryptography/Pal.OSX/AppleCertificatePal.cs

Co-authored-by: Jeremy Barton <[email protected]>
@vcsjones
Copy link
Member

In addition to the failing tests for trailing white space, I think AppleCryptoNative_X509GetSubjectSummary needs to be added to entrypoints.c around here:

DllImportEntry(AppleCryptoNative_X509ChainGetStatusAtIndex)

Though I am not clear why there wasn't a build failure for that. There is (or, was if there was not) a build check for this.

@odhanson
Copy link
Member Author

In addition to the failing tests for trailing white space, I think AppleCryptoNative_X509GetSubjectSummary needs to be added to entrypoints.c around here:

DllImportEntry(AppleCryptoNative_X509ChainGetStatusAtIndex)

Though I am not clear why there wasn't a build failure for that. There is (or, was if there was not) a build check for this.

This file must be relatively new. Because initially I made the change against the net5.0 tag and I searched for AppleCryptoNative_X509GetRawData everywhere. Also definitely weird that it built for me when I switched to 6.

Thanks, I will fix it

@bartonjs bartonjs merged commit fb2e61e into dotnet:main Jun 21, 2021
@bartonjs
Copy link
Member

/backport to release/5.0

@github-actions
Copy link
Contributor

Started backporting to release/5.0: https://github.com/dotnet/runtime/actions/runs/964761518

@github-actions
Copy link
Contributor

@bartonjs backporting to release/5.0 failed, the patch most likely resulted in conflicts:

$ git am --3way --ignore-whitespace --keep-non-patch changes.patch

Applying: Add name of corrupted certificate to CryptographicException on Mac
Using index info to reconstruct a base tree...
M	src/libraries/Common/src/Interop/OSX/System.Security.Cryptography.Native.Apple/Interop.X509.cs
M	src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/pal_x509.c
M	src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/pal_x509.h
M	src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.OSX/AppleCertificatePal.cs
M	src/libraries/System.Security.Cryptography.X509Certificates/src/Resources/Strings.resx
Falling back to patching base and 3-way merge...
Auto-merging src/libraries/System.Security.Cryptography.X509Certificates/src/Resources/Strings.resx
Auto-merging src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.OSX/AppleCertificatePal.cs
Auto-merging src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/pal_x509.h
CONFLICT (content): Merge conflict in src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/pal_x509.h
Auto-merging src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/pal_x509.c
CONFLICT (content): Merge conflict in src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/pal_x509.c
Auto-merging src/libraries/Common/src/Interop/OSX/System.Security.Cryptography.Native.Apple/Interop.X509.cs
CONFLICT (content): Merge conflict in src/libraries/Common/src/Interop/OSX/System.Security.Cryptography.Native.Apple/Interop.X509.cs
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 Add name of corrupted certificate to CryptographicException on Mac
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Error: The process '/usr/bin/git' failed with exit code 128

Please backport manually!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ASN1 corrupted data while calling X509Store.Certificates on Mac
3 participants