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

System.Security.Cryptography.X509Certificates.Tests: Assertion `0 && "Huge length X509_NAME"' failed. #49732

Closed
jkotas opened this issue Mar 17, 2021 · 10 comments · Fixed by #56277
Assignees
Milestone

Comments

@jkotas
Copy link
Member

jkotas commented Mar 17, 2021

  Discovering: System.Security.Cryptography.X509Certificates.Tests (method display = ClassAndMethod, method display options = None)
  Discovered:  System.Security.Cryptography.X509Certificates.Tests (found 596 of 690 test cases)
  Starting:    System.Security.Cryptography.X509Certificates.Tests (parallel test collections = on, max threads = 2)
dotnet: /__w/1/s/src/libraries/Native/Unix/System.Security.Cryptography.Native/openssl.c:402:
int32_t CryptoNative_GetX509NameRawBytes(X509_NAME *, uint8_t *, int32_t):
Assertion `0 && "Huge length X509_NAME"' failed.
./RunTests.sh: line 162: 31560 Aborted

Failed in #49721 on net6.0-Linux-Debug-x64-CoreCLR_release-Ubuntu.1604.Amd64.Open

@dotnet-issue-labeler dotnet-issue-labeler bot added area-System.Security untriaged New issue has not been triaged by the area owner labels Mar 17, 2021
@ghost
Copy link

ghost commented Mar 17, 2021

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

Issue Details
  Discovering: System.Security.Cryptography.X509Certificates.Tests (method display = ClassAndMethod, method display options = None)
  Discovered:  System.Security.Cryptography.X509Certificates.Tests (found 596 of 690 test cases)
  Starting:    System.Security.Cryptography.X509Certificates.Tests (parallel test collections = on, max threads = 2)
dotnet: /__w/1/s/src/libraries/Native/Unix/System.Security.Cryptography.Native/openssl.c:402:
int32_t CryptoNative_GetX509NameRawBytes(X509_NAME *, uint8_t *, int32_t):
Assertion `0 && "Huge length X509_NAME"' failed.
./RunTests.sh: line 162: 31560 Aborted

Failed in #49721

Author: jkotas
Assignees: -
Labels:

area-System.Security, untriaged

Milestone: -

@jkotas
Copy link
Member Author

jkotas commented Mar 17, 2021

Stack trace from the dump attached to #49721 :

00007F4262FFA200 00007f429ade50a4 [InlinedCallFrame: 00007f4262ffa200] Interop+Crypto.GetX509NameRawBytes(IntPtr, Byte[], Int32)
00007F4262FFA1F0 00007F429ADE50A4 ILStubClass.IL_STUB_PInvoke(IntPtr, Byte[], Int32)
00007F4262FFA2E0 00007F429ADE4F6D Interop+Crypto+<>c.<LoadX500Name>b__213_0(IntPtr, Byte[], Int32)
00007F4262FFA310 00007F429ADE4E4A Interop+Crypto.GetDynamicBuffer[[System.IntPtr, System.Private.CoreLib]](NegativeSizeReadMethod`1<IntPtr>, IntPtr)
00007F4262FFA390 00007F429ADE4C3E Interop+Crypto.LoadX500Name(IntPtr)
00007F4262FFA3F0 00007F429ADE4935 Internal.Cryptography.Pal.OpenSslX509CertificateReader.get_Subject()
00007F4262FFA440 00007F429ADE4788 System.Security.Cryptography.X509Certificates.X509Certificate.get_Subject()
00007F4262FFA490 00007F429B598C37 System.Security.Cryptography.X509Certificates.Tests.Common.CertificateAuthority.get_SubjectName()
00007F4262FFA4B0 00007F429B58DB33 System.Security.Cryptography.X509Certificates.Tests.Common.RevocationResponder.HandleRequest(System.Net.HttpListenerContext, Boolean ByRef)
00007F4262FFA850 00007F429B58D2C8 System.Security.Cryptography.X509Certificates.Tests.Common.RevocationResponder.HandleRequest(System.Net.HttpListenerContext)
00007F4262FFA990 00007F429B58D193 System.Security.Cryptography.X509Certificates.Tests.Common.RevocationResponder.<HandleRequest>b__24_0(System.Net.HttpListenerContext)

Where is the code that is supposed to validate that the certificate is well-formatted?

@vcsjones
Copy link
Member

Where is the code that is supposed to validate that the certificate is well-formatted?

That happens during .ctor of the X509Certificate{2}. It looks like this failed on an already instantiated certificate.

Since this looks like a cert we generate on-the-fly in the tests I would not expect this to have an especially large subject name. I'll see if I can glean anything from the core dump.

@stephentoub stephentoub added the bug label Jul 1, 2021
@stephentoub stephentoub added this to the 6.0.0 milestone Jul 1, 2021
@jeffhandley jeffhandley removed the untriaged New issue has not been triaged by the area owner label Jul 13, 2021
@bartonjs
Copy link
Member

A recent failure was in #55820, job 98790a12-c5ae-496f-82c6-ed509deeb6be.

Something does seem to have gone... wrong.

(lldb) p *x509Name;
(X509_NAME) $5 = {
  entries = 0x00007fca800a5f90
  modified = 0
  bytes = 0x00007fca80070800
  canon_enc = 0x00007fca800ab100 "\xffffff90(\v\xffffff80\xffffffca\x7f"
  canon_enclen = 189
}
(lldb) p *(x509Name->bytes);
(BUF_MEM) $6 = (length = 140507708246688, data = "\xffffffe0\xffffffb7\v\xffffff80\xffffffca\x7f", max = 260)
(lldb) memory read -c 189 -s 1 0x00007fca800ab100
0x7fca800ab100: 90 28 0b 80 ca 7f 00 00 20 fa 0b 80 ca 7f 00 00  .(...... .......
0x7fca800ab110: 76 6f 63 61 74 69 6f 6e 2c 20 69 73 73 75 65 72  vocation, issuer
0x7fca800ab120: 61 75 74 68 6f 72 69 74 79 68 61 73 64 65 73 69  authorityhasdesi
0x7fca800ab130: 67 6e 61 74 65 64 6f 63 73 70 72 65 73 70 6f 6e  gnatedocsprespon
0x7fca800ab140: 64 65 72 2c 20 72 6f 6f 74 61 75 74 68 6f 72 69  der, rootauthori
0x7fca800ab150: 74 79 68 61 73 64 65 73 69 67 6e 61 74 65 64 6f  tyhasdesignatedo
0x7fca800ab160: 63 73 70 72 65 73 70 6f 6e 64 65 72 31 2e 30 2c  cspresponder1.0,
0x7fca800ab170: 06 03 55 04 0a 0c 25 72 65 76 6f 6b 65 65 6e 64  ..U...%revokeend
0x7fca800ab180: 65 6e 74 69 74 79 5f 72 6f 6f 74 72 65 76 6f 63  entity_rootrevoc
0x7fca800ab190: 61 74 69 6f 6e 6f 66 66 6c 69 6e 65 31 1f 30 1d  ationoffline1.0.
0x7fca800ab1a0: 06 03 55 04 03 0c 16 61 20 72 65 76 6f 63 61 74  ..U....a revocat
0x7fca800ab1b0: 69 6f 6e 20 74 65 73 74 20 63 61 20 30           ion test ca 0

The structure we see claims the name is 140507708246688 bytes long, and the "canonical encoding" of it is gibberish.

Unfortunately, clrstack is not happy:

(lldb) clrstack
OS Thread Id: 0x0 (1)
Failed to start stack walk: 8000ffff

so I don't know a good way to track this back to the X509* that produced it, to see if it's also in a bad state.

@vcsjones
Copy link
Member

If I had to guess based on the original exception, I wonder if this is a use-after-dispose. The stub OSCP/AIA responder are "multi threaded" now, I wonder if a request comes in to it while it's in the middle of disposing.

@bartonjs
Copy link
Member

I was going to say "can't be, the SafeHandles protect us"... but they don't. internal static extern IntPtr X509GetSubjectName(SafeX509Handle x); We turn the SafeHandle into an interior pointer.

Coming up with a threading test to show that this hits periodically seems good. And then it can show that doing DangerousAddRef/DangerousRelease in

_subject = Interop.Crypto.LoadX500Name(Interop.Crypto.X509GetSubjectName(_cert)).Name;
solves it.

@vcsjones
Copy link
Member

A number of properties in certificate reader behave the same way. I'll take a stab at reproducing this over the weekend.

@vcsjones
Copy link
Member

vcsjones commented Jul 24, 2021

That appears to be it. I was able to reproduce it. I raced the Dispose and Subject call from two different threads.

It’s a little difficult to hit. Even locally it took a few million tries to get it to reproduce.

B07BA895-8B6A-42EC-B7FF-CD7647D99D88

@vcsjones
Copy link
Member

vcsjones commented Jul 25, 2021

@bartonjs I'll attempt to get a PR in over the next few days assuming you have no objections, seeing as this is currently assigned to you.

@vcsjones
Copy link
Member

Another assertion I was able to hit using GetPublicKey.

The active test run was aborted. Reason: Test host process crashed : dotnet: /code/personal/dotnet/runtime/src/libraries/Native/Unix/System.Security.Cryptography.Native/openssl.c:354: int32_t CryptoNative_GetAsn1StringBytes(ASN1_STRING *, uint8_t *, int32_t): Assertion `length >= 0' failed.

@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Jul 25, 2021
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Jul 26, 2021
@bartonjs bartonjs removed their assignment Jul 26, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Aug 25, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants