SIGSEGV when independent DSA keys are concurrently disposed #71738
Labels
area-System.Security
os-mac-os-x
macOS aka OSX
tracking-external-issue
The issue is caused by external problem (e.g. OS) - nothing we can do to fix it directly
Milestone
Comments
|
Tagging subscribers to this area: @dotnet/area-system-security, @vcsjones Issue DetailsOn macOS Monterey, I would expect this program to run forever. using System.Security.Cryptography;
DSA dsa;
void Work()
{
while (true)
{
try
{
dsa = DSA.Create();
dsa.ImportParameters(new DSAParameters
{
P = Convert.FromBase64String(@"
nEx7rLmUg+FLq23XB/8rVFU3Txktd4NYVppGrJMdRKi0FktEj39g7vM33rA0g8Xf
BurQu9HkcblSR25E5beYrMbU8pJD1ZqmrltbnnlB+PHX5Pgbu91BCr2d5UjAIfiA
qIlnySMuV0XSqbb1A3qyWGIx3ATXBaXN9mm+paF2itE="),
Q = Convert.FromBase64String("vV0TbUwrTOkOoiyTJDxsaKWqWjE="),
G = Convert.FromBase64String(@"
XZESzrsgUFaS697sgeQEnFKrhh3S6C+gfVG2wL9JBv636QsEq2uxpOMl/1VQxjqx
Cys3x9YFOkdY1xYdk4ayhco6LYVr81X/lRUtx0YZxpaTt10XgcnlLwx772pYCcOH
UlyyGxq3GYCA1cglXtS80gPHIYieOqmUhvBHXMYBCAg="),
Y = Convert.FromBase64String(@"
e7NMNCxX/44GS2gUH+JyReWzdCUXcp6ax0PcF/XvIZ1mak74P8o8yqWseGa/10hR
CT92or4YBROsGtKqD/wqN0yJvVMkpPHHsWU9zs1Zt4CsQaZgUTw+vyjkw674OuyN
933pL+qQNvPuJcb/HK9ME2vSN/3Ki1lAqqKWuzcvggY="),
X = Convert.FromBase64String(@"DQrQZHBuIxyLlLqtNqOULp/tlH0="),
});
dsa.Dispose();
}
catch (Exception)
{
// Managed exceptions are okay, looking for crashes.
}
}
}
Thread t1 = new Thread(Work);
Thread t2 = new Thread(Work);
t1.Start();
t2.Start();
t1.Join();
t2.Join();It will quickly exit with status code 139, a segmentation fault. I cannot reproduce this for RSA or ECDsa, only DSA. The native stack trace is below. Unfortunately I've been struggling quite a bit to get dotnet-sos and dotnet-symbol to work on Apple Silicon, so no managed stack trace. frame #1: 0x00000001b2067ca8 Security`CssmManager::getModule(Security::Guid const&) + 28
frame #2: 0x00000001b20676a0 Security`CSSM_ModuleAttach + 104
frame #3: 0x00000001b2058c04 Security`Security::CssmClient::AttachmentImpl::activate() + 332
frame #4: 0x00000001b2089f50 Security`Security::CssmClient::KeyImpl::deactivate() + 180
frame #5: 0x00000001b21dd940 Security`Security::CssmClient::KeyImpl::~KeyImpl() + 84
frame #6: 0x00000001b2089e84 Security`Security::CssmClient::KeyImpl::~KeyImpl() + 16
frame #7: 0x00000001b21b6dd0 Security`Security::RefPointer::release_internal() + 112
frame #8: 0x00000001b2058498 Security`Security::RefPointer::release() + 56
frame #9: 0x00000001b21b6d34 Security`Security::RefPointer::~RefPointer() + 24
frame #10: 0x00000001b2290304 Security`Security::KeychainCore::KeyItem::~KeyItem() + 140
frame #11: 0x00000001b2089e5c Security`Security::KeychainCore::KeyItem::~KeyItem() + 16
frame #12: 0x00000001b22bed74 Security`SecCDSAKeyDestroy(__SecKey*) + 328
frame #13: 0x00000001b00fad78 CoreFoundation`_CFRelease + 232
frame #14: 0x0000000280f56944
frame #15: 0x00000002804ed554
frame #16: 0x00000002804ed25c
frame #17: 0x0000000280f56a28
frame #18: 0x00000002804ed094
frame #19: 0x00000001033af5c8 libcoreclr.dylib`CallDescrWorkerInternal + 132
frame #20: 0x000000010321fc2c libcoreclr.dylib`DispatchCallSimple(unsigned long*, unsigned int, unsigned long, unsigned int) + 284
frame #21: 0x00000001031a8cb4 libcoreclr.dylib`MethodTable::CallFinalizer(Object*) + 400
frame #22: 0x0000000103260068 libcoreclr.dylib`FinalizerThread::FinalizeAllObjects() + 364
frame #23: 0x0000000103260270 libcoreclr.dylib`FinalizerThread::FinalizerThreadWorker(void*) + 184
frame #24: 0x00000001031eaaa0 libcoreclr.dylib`ManagedThreadBase_DispatchOuter(ManagedThreadCallState*) + 260
frame #25: 0x00000001031eb074 libcoreclr.dylib`ManagedThreadBase::FinalizerBase(void (*)(void*)) + 36
frame #26: 0x00000001032603ec libcoreclr.dylib`FinalizerThread::FinalizerThreadStart(void*) + 88
frame #27: 0x00000001030fd380 libcoreclr.dylib`CorUnix::CPalThread::ThreadEntry(void*) + 380
frame #28: 0x00000001aff5426c libsystem_pthread.dylib`_pthread_start + 148
|
ghost
added
the
|
Maybe it's time to just mark @GrabYourPitchforks, thoughts? |
|
ImportFromPem_Pkcs8_UnrelatedPrecedingPem ( ) doesn't have concurrency in it... |
|
Oh, hm. Not sure how I misread that. Regardless, we can open a separate issue for that test failure then, if we want. But if we deprecate DSA on MacOS then it would have the same result. |
|
DSA is the only algorithm that uses the old CSSM keys (aside from X509Certificate impl. calling into it). Given the fact that it was always limited to one size of key and key creation didn't work I would be fine with deprecating it. That said, it would be nice to understand the problem so the X509 classes don't run into it. |
|
Native stack trace: Managed stack trace: The fact that this comes from finalization worries me. All the handles should have been disposed properly. |
|
Ah, I see now that the managed |
|
For completeness where |
|
Okay, multithreading and concurrency is not related. Just letting Simpler repro (takes longer to crash, but still happens pretty quickly) using System.Security.Cryptography;
while (true)
{
DSA dsa = DSA.Create();
dsa.ImportParameters(new DSAParameters
{
P = Convert.FromBase64String(@"
nEx7rLmUg+FLq23XB/8rVFU3Txktd4NYVppGrJMdRKi0FktEj39g7vM33rA0g8Xf
BurQu9HkcblSR25E5beYrMbU8pJD1ZqmrltbnnlB+PHX5Pgbu91BCr2d5UjAIfiA
qIlnySMuV0XSqbb1A3qyWGIx3ATXBaXN9mm+paF2itE="),
Q = Convert.FromBase64String("vV0TbUwrTOkOoiyTJDxsaKWqWjE="),
G = Convert.FromBase64String(@"
XZESzrsgUFaS697sgeQEnFKrhh3S6C+gfVG2wL9JBv636QsEq2uxpOMl/1VQxjqx
Cys3x9YFOkdY1xYdk4ayhco6LYVr81X/lRUtx0YZxpaTt10XgcnlLwx772pYCcOH
UlyyGxq3GYCA1cglXtS80gPHIYieOqmUhvBHXMYBCAg="),
Y = Convert.FromBase64String(@"
e7NMNCxX/44GS2gUH+JyReWzdCUXcp6ax0PcF/XvIZ1mak74P8o8yqWseGa/10hR
CT92or4YBROsGtKqD/wqN0yJvVMkpPHHsWU9zs1Zt4CsQaZgUTw+vyjkw674OuyN
933pL+qQNvPuJcb/HK9ME2vSN/3Ki1lAqqKWuzcvggY="),
X = Convert.FromBase64String(@"DQrQZHBuIxyLlLqtNqOULp/tlH0="),
});
GC.Collect();
} |
vcsjones
changed the title
If both threads hit the line |
"longer" is an understatement. Running that on .NET 7 didn't crash for 2 hours now 😅 |
Hm. Crashed for me in about 30 seconds. Will get a dump when I am back at the computer. But I was running it under .NET 6. |
|
So, with the "simple" repro, I can get it to crash pretty easily but only on Apple Silicon for .NET 6 and .NET 7. I cannot get it to crash on x86_64. @filipnavara were you using x86_64? Native backtrace on AMD64: * thread #7, stop reason = ESR_EC_DABORT_EL0 (fault address: 0xb2)
* frame #0: 0x00000001b8da512c Security`std::__1::__tree_iterator, std::__1::__tree_node, void*>*, long> std::__1::__tree, std::__1::__map_value_compare, std::__1::less, true>, std::__1::allocator > >::find(Security::Guid const&) + 16
frame #1: 0x00000001b8b9fca8 Security`CssmManager::getModule(Security::Guid const&) + 28
frame #2: 0x00000001b8b9f6a0 Security`CSSM_ModuleAttach + 104
frame #3: 0x00000001b8b90c04 Security`Security::CssmClient::AttachmentImpl::activate() + 332
frame #4: 0x00000001b8bc1f50 Security`Security::CssmClient::KeyImpl::deactivate() + 180
frame #5: 0x00000001b8d15940 Security`Security::CssmClient::KeyImpl::~KeyImpl() + 84
frame #6: 0x00000001b8bc1e84 Security`Security::CssmClient::KeyImpl::~KeyImpl() + 16
frame #7: 0x00000001b8ceedd0 Security`Security::RefPointer::release_internal() + 112
frame #8: 0x00000001b8b90498 Security`Security::RefPointer::release() + 56
frame #9: 0x00000001b8ceed34 Security`Security::RefPointer::~RefPointer() + 24
frame #10: 0x00000001b8dc8304 Security`Security::KeychainCore::KeyItem::~KeyItem() + 140
frame #11: 0x00000001b8bc1e5c Security`Security::KeychainCore::KeyItem::~KeyItem() + 16
frame #12: 0x00000001b8df6d74 Security`SecCDSAKeyDestroy(__SecKey*) + 328
frame #13: 0x00000001b6c32d78 CoreFoundation`_CFRelease + 232
frame #14: 0x0000000280d8e308
frame #15: 0x0000000280d8d910
frame #16: 0x0000000280d8e13c
frame #17: 0x0000000102e9f5c8 libcoreclr.dylib`CallDescrWorkerInternal + 132
frame #18: 0x0000000102d0fc2c libcoreclr.dylib`DispatchCallSimple(unsigned long*, unsigned int, unsigned long, unsigned int) + 284
frame #19: 0x0000000102c98cb4 libcoreclr.dylib`MethodTable::CallFinalizer(Object*) + 400
frame #20: 0x0000000102d50068 libcoreclr.dylib`FinalizerThread::FinalizeAllObjects() + 364
frame #21: 0x0000000102d50270 libcoreclr.dylib`FinalizerThread::FinalizerThreadWorker(void*) + 184
frame #22: 0x0000000102cdaaa0 libcoreclr.dylib`ManagedThreadBase_DispatchOuter(ManagedThreadCallState*) + 260
frame #23: 0x0000000102cdb074 libcoreclr.dylib`ManagedThreadBase::FinalizerBase(void (*)(void*)) + 36
frame #24: 0x0000000102d503ec libcoreclr.dylib`FinalizerThread::FinalizerThreadStart(void*) + 88
frame #25: 0x0000000102bed380 libcoreclr.dylib`CorUnix::CPalThread::ThreadEntry(void*) + 380
frame #26: 0x00000001b6a8c26c libsystem_pthread.dylib`_pthread_start + 148
|
ARM64 on Apple M1. Still running, still no crash... The native stack trace is identical to the one I saw earlier. |
Bit of a tangent but where did you get a Or did you build it yourself? |
A little birdie taught me a trick... :) The |
It... would have been nice if it defaulted to ARM64 on an ARM64 machine. Nonetheless, I have a working SOS plugin now. Sparkles and thank you! |
It totally would but I already complained about it one too many times here - dotnet/sdk#26417 - and it looks like there will be some guidance on how to solve this properly in future. |
|
I suspect that this is a bug in the Apple code. Most of |
|
Reported to Apple Feedback as FB10581430. |
|
Interesting. This may have been the cause of some other exceptions or crashes we've seen, such as #42568. |
|
Here's a fairly trivial way to trigger it: #include <pthread.h>
#include <Security/Security.h>
static const char *public_key = "-----BEGIN DSA PUBLIC KEY-----\n"
"MIHxMIGoBgcqhkjOOAQBMIGcAkEAvPM8vp7lHRrWFhpso2I/Wrq1qV8TSl7/YITH\n"
"7cHsINCP/xrZZpTlx14pKNkKwEEf3t3bdkKY97NQKRJ+cIRyawIVAMDJQP8l7EVy\n"
"fcqtVnJjJupPIccxAkBhLjwIRUNerlWb0kW357ABc4+65XB90lQIdcwVLGqRsx9A\n"
"wKoeeMUEyVdQhjJMnclvYJU+xqnl2AP9224QOGGLA0QAAkEAkFQyL1jGMfEjer1O\n"
"QjBq7knMY8zHEUVNRbPXBNS5QenFg07rgMUFL/Bj6/876pWvubwpDAcXkiK+SR3A\n"
"FRF/VA==\n"
"-----END DSA PUBLIC KEY-----\n";
static void *thread_start(void *arg)
{
while (1)
{
CFDataRef publicKeyData = CFDataCreateWithBytesNoCopy(NULL, (const UInt8 *)public_key, strlen(public_key), kCFAllocatorNull);
SecItemImportExportKeyParameters params = {};
SecExternalItemType keyType = kSecItemTypePublicKey;
SecExternalFormat keyFormat = kSecFormatPEMSequence;
CFArrayRef importArray = NULL;
SecItemImport((CFDataRef)publicKeyData,
NULL,
&keyFormat,
&keyType,
0,
¶ms,
NULL,
&importArray);
CFRelease(publicKeyData);
CFRelease(importArray);
}
}
int main(int argc, char *argv[])
{
pthread_attr_t attr;
pthread_t t1, t2;
pthread_attr_init(&attr);
pthread_create(&t1, &attr, &thread_start, NULL);
pthread_create(&t2, &attr, &thread_start, NULL);
while (1) { }
return 0;
}Compile with |
Can this be hit by X509Certificate2.GetRSAPrivateKey() for RSA keys? |
Cursory look says that it can... but let's try. |
|
The test above crashes for RSA keys imported through |
|
How much of the shim would we need to mutex if we feel we need to work around it? Seems like it's not just our CFRelease call; but if it's just that we can't CFRelease a SecKeyRef during a SecItemImport then maybe we can mutex/semaphore those operations. |
There's no easy answer to that I am afraid. At minimum pal_dsa.c, pal_x509_macos.c and anything that gets If you think it's worth investigating I can look at the Apple code in more detail. |
|
OK, so you think it's not as easy as internal partial class Interop
{
internal partial class AppleCrypto
{
private static readonly object s_cssmLock = new object();
partial class SafeSecKeyRefHandle
{
public bool ReleaseHandle()
{
lock (s_cssmLock)
{
Interop.CoreFoundation.CFRelease(handle);
}
SetHandle(IntPtr.Zero);
return true;
}
}
internal static SafeSecKeyRefHandle ImportEphemeralKey(ReadOnlySpan<byte> keyBlob, bool hasPrivateKey)
{
Debug.Assert(keyBlob != null);
SafeSecKeyRefHandle keyHandle;
int osStatus;
int ret;
lock (s_cssmLock)
{
ret = AppleCryptoNative_SecKeyImportEphemeral(
keyBlob,
hasPrivateKey ? 1 : 0,
out keyHandle,
out osStatus);
}
if (ret == 1 && !keyHandle.IsInvalid)
{
return keyHandle;
}
if (ret == 0)
{
throw CreateExceptionForOSStatus(osStatus);
}
Debug.Fail($"SecKeyImportEphemeral returned {ret}");
throw new CryptographicException();
}
// maybe one or two other things
}
}but is possibly "lock everything using SafeKeyRef" and maybe also SecCertRef and SecIdentityRef? |
|
At one or two methods I'd say we can just lock it. But at more than that it feels like we might want to wait for Apple's answer 😄 |
|
I hope the answer is "no" but I am going to ask anyway because I am on an iPad for most of the week... this CSSM module map isn't shared between processes, is it? Or, more specifically, if those threads turned in to separate processes, it would not reproduce? Looking at my backtrace more in my "simpler" reproduction, it's still a multi threading issue - the finalizer is the other thread. So while there is no explicit threading going on, the finalizer and the main thread are racing this CSSM issue. |
Yep. The issue can happen with any place where the CSSM module is loaded/unloaded. I had different workaround in mind - loading all CSSM modules as part of initialization and keeping them alive forever. That way the lack of the lock would not be observable because the hash map would no longer be modified. I abandoned this idea when I realized that there's more than 2 CSSM modules. The list of actually usable CSSM modules is likely finite (the public API is deprecated since macOS 10.7) and limited but it's not obvious how limited.
No, thankfully not.
It would not.
Correct. It is multi-threading issue. |
|
For what it's worth, it looks like Chromium has noticed this issue:
If you look at usages of |
|
Thanks for opening this can of worms:
|
|
Playing on @filipnavara's earlier supposition about the last reference being the problem, then I wonder if we can do something like hold a DSA public+private keypair in a static from DSASecurityTransforms (maybe even going so far as to marking them as For @vcsjones' test, that'd be just hold one imported DSA key outside the loop, and then the crash may go away. We might end up with one for DSA, one for ECDH, one for ECDSA, and one for RSA, but that's better than locking? |
|
That's basically the same idea as holding references to all CSSM modules, expressed in high-level API terminology. (You may not need one key per type, perhaps even one would be enough but it's not immediately obvious which CSSM module does what) |
This still crashes almost instantly using System.Security.Cryptography;
internal class Program
{
private static readonly DSAParameters _dsaParameters = new()
{
P = Convert.FromBase64String(@"
nEx7rLmUg+FLq23XB/8rVFU3Txktd4NYVppGrJMdRKi0FktEj39g7vM33rA0g8Xf
BurQu9HkcblSR25E5beYrMbU8pJD1ZqmrltbnnlB+PHX5Pgbu91BCr2d5UjAIfiA
qIlnySMuV0XSqbb1A3qyWGIx3ATXBaXN9mm+paF2itE="),
Q = Convert.FromBase64String("vV0TbUwrTOkOoiyTJDxsaKWqWjE="),
G = Convert.FromBase64String(@"
XZESzrsgUFaS697sgeQEnFKrhh3S6C+gfVG2wL9JBv636QsEq2uxpOMl/1VQxjqx
Cys3x9YFOkdY1xYdk4ayhco6LYVr81X/lRUtx0YZxpaTt10XgcnlLwx772pYCcOH
UlyyGxq3GYCA1cglXtS80gPHIYieOqmUhvBHXMYBCAg="),
Y = Convert.FromBase64String(@"
e7NMNCxX/44GS2gUH+JyReWzdCUXcp6ax0PcF/XvIZ1mak74P8o8yqWseGa/10hR
CT92or4YBROsGtKqD/wqN0yJvVMkpPHHsWU9zs1Zt4CsQaZgUTw+vyjkw674OuyN
933pL+qQNvPuJcb/HK9ME2vSN/3Ki1lAqqKWuzcvggY="),
X = Convert.FromBase64String(@"DQrQZHBuIxyLlLqtNqOULp/tlH0="),
};
private static readonly DSA _cssmDsaKeepAlive;
static Program()
{
_cssmDsaKeepAlive = DSA.Create();
_cssmDsaKeepAlive.ImportParameters(_dsaParameters);
_cssmDsaKeepAlive.CreateSignature(new byte[20]);
}
private static void Main(string[] args)
{
void Work()
{
while (true)
{
DSA dsa = DSA.Create();
dsa.ImportParameters(_dsaParameters);
dsa.Dispose();
}
}
Thread t1 = new Thread(Work);
Thread t2 = new Thread(Work);
t1.Start();
t2.Start();
t1.Join();
t2.Join();
GC.KeepAlive(_cssmDsaKeepAlive);
}
}Maybe the Even with my example using just the finalizer as the concurrent thread still crashes. |
vcsjones
changed the title
jeffhandley
added
the
tracking-external-issue
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
On macOS Monterey, I would expect this program to run forever.
It will quickly exit with status code 139, a segmentation fault.
I cannot reproduce this for RSA or ECDsa, only DSA.
The native stack trace is below. Unfortunately I've been struggling quite a bit to get dotnet-sos and dotnet-symbol to work on Apple Silicon, so no managed stack trace.
frame #1: 0x00000001b2067ca8 Security`CssmManager::getModule(Security::Guid const&) + 28 frame #2: 0x00000001b20676a0 Security`CSSM_ModuleAttach + 104 frame #3: 0x00000001b2058c04 Security`Security::CssmClient::AttachmentImpl::activate() + 332 frame #4: 0x00000001b2089f50 Security`Security::CssmClient::KeyImpl::deactivate() + 180 frame #5: 0x00000001b21dd940 Security`Security::CssmClient::KeyImpl::~KeyImpl() + 84 frame #6: 0x00000001b2089e84 Security`Security::CssmClient::KeyImpl::~KeyImpl() + 16 frame #7: 0x00000001b21b6dd0 Security`Security::RefPointer::release_internal() + 112 frame #8: 0x00000001b2058498 Security`Security::RefPointer::release() + 56 frame #9: 0x00000001b21b6d34 Security`Security::RefPointer::~RefPointer() + 24 frame #10: 0x00000001b2290304 Security`Security::KeychainCore::KeyItem::~KeyItem() + 140 frame #11: 0x00000001b2089e5c Security`Security::KeychainCore::KeyItem::~KeyItem() + 16 frame #12: 0x00000001b22bed74 Security`SecCDSAKeyDestroy(__SecKey*) + 328 frame #13: 0x00000001b00fad78 CoreFoundation`_CFRelease + 232 frame #14: 0x0000000280f56944 frame #15: 0x00000002804ed554 frame #16: 0x00000002804ed25c frame #17: 0x0000000280f56a28 frame #18: 0x00000002804ed094 frame #19: 0x00000001033af5c8 libcoreclr.dylib`CallDescrWorkerInternal + 132 frame #20: 0x000000010321fc2c libcoreclr.dylib`DispatchCallSimple(unsigned long*, unsigned int, unsigned long, unsigned int) + 284 frame #21: 0x00000001031a8cb4 libcoreclr.dylib`MethodTable::CallFinalizer(Object*) + 400 frame #22: 0x0000000103260068 libcoreclr.dylib`FinalizerThread::FinalizeAllObjects() + 364 frame #23: 0x0000000103260270 libcoreclr.dylib`FinalizerThread::FinalizerThreadWorker(void*) + 184 frame #24: 0x00000001031eaaa0 libcoreclr.dylib`ManagedThreadBase_DispatchOuter(ManagedThreadCallState*) + 260 frame #25: 0x00000001031eb074 libcoreclr.dylib`ManagedThreadBase::FinalizerBase(void (*)(void*)) + 36 frame #26: 0x00000001032603ec libcoreclr.dylib`FinalizerThread::FinalizerThreadStart(void*) + 88 frame #27: 0x00000001030fd380 libcoreclr.dylib`CorUnix::CPalThread::ThreadEntry(void*) + 380 frame #28: 0x00000001aff5426c libsystem_pthread.dylib`_pthread_start + 148The text was updated successfully, but these errors were encountered: