dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.25k stars 4.73k forks source link

A delay is necessary when ECDSA-signing data with smart cards #65680

Open EatonZ opened 2 years ago

EatonZ commented 2 years ago

Description

I have a YubiKey 5C NFC (FW: 5.2.7). It is connected to my desktop over USB-C. With a YubiKey it's possible to import a signing key/certificate into one of its PIV slots. Once imported, it's possible to load the key using CngKey.Open("GUID_HERE", CngProvider.MicrosoftSmartCardKeyStorageProvider);. You can then create an ECDsaCng instance using that key and sign data.

That process above works, but there is a major problem with the signing data step. During a call to ECDsaCng's SignData method, you are prompted by Windows to enter your smart card pin. When the correct pin is entered, SignData returns a valid signature. However, if you try to sign more data via subsequent calls to SignData, you run into a WindowsCryptographicException: Access was denied because of a security violation.

Reproduction Steps

To properly reproduce, you will need a smart card/security key that can load in custom certificates/keys. In other words, PIV-enabled. I am using a YubiKey and the steps below are tailored for reproducing on YubiKey.

  1. Install the YubiKey Smart Card Minidriver if you do not have it already.
  2. Download this sample PFX
  3. Download this sample .NET 6 console application project
  4. Download the latest yubico-piv-tool and run this command from the folder you extracted the PFX to. In this command, you need to fill in the management key (replace "MGM-KEY" with your own) and also provide a key slot value for --slot (any free slot you have). yubico-piv-tool --key="MGM-KEY" --action="import-key" --action="import-certificate" --slot="" --input="Package.pfx" --key-format="PKCS12" --password="PASSWORD" --pin-policy="always" --touch-policy="never"
  5. Unplug your YubiKey and plug it back in.
  6. You need to find your key container GUID. To do this, run this command. You will be prompted for your pin at least once. After it's done, find the certificate and copy its Key Container value into the sample project. certutil -store -user "My"
  7. Finally, build the sample project and run it to reproduce. You will be prompted for your pin once. Enter it and you should see printed errors shortly after.

Expected behavior

A WindowsCryptographicException should not occur on subsequent ECDsaCng SignData calls, and a delay should not be necessary.

Actual behavior

WindowsCryptographicException: Access was denied because of a security violation. can occur on subsequent ECDsaCng SignData calls.

Stack trace of the exception:

   at Internal.Cryptography.CngCommon.SignHash(SafeNCryptKeyHandle keyHandle, ReadOnlySpan`1 hash, AsymmetricPaddingMode paddingMode, Void* pPaddingInfo, Int32 estimatedSize)
   at System.Security.Cryptography.ECDsaCng.SignHash(Byte[] hash)
   at System.Security.Cryptography.ECDsa.SignData(Byte[] data, Int32 offset, Int32 count, HashAlgorithmName hashAlgorithm)
   at System.Security.Cryptography.ECDsa.SignData(Byte[] data, HashAlgorithmName hashAlgorithm)
   at System.Security.Cryptography.ECDsaCng.SignData(Byte[] data)
   at Program.<<Main>$>d__0.MoveNext() in C:\Users\Eaton\Source\Repos\YubiDelaySample\YubiDelaySample\Program.cs:line 14

Regression?

The problem also occurs on Windows 10 and .NET 5. It was my hope that a clean Windows 11 install may solve the problem, but it did not. Re-importing the key to the YubiKey did not help either.

Known Workarounds

A 3-second delay works around the problem, but it slows down the entire process and is not ideal. I don't know if there is an exact amount of time that needs to pass before signing data again, but I've found 3 seconds to work most of the time.

Configuration

.NET 6.0.1 Windows 11 x64

Other information

I asked Yubico support about the issue and they had this to say (start reading from the bottom message):

Click to view image

ghost commented 2 years ago

Tagging subscribers to this area: @dotnet/area-system-security, @vcsjones See info in area-owners.md if you want to be subscribed.

Issue Details
### Description I have a [YubiKey 5C NFC](https://www.yubico.com/product/yubikey-5c-nfc/) (FW: 5.2.7). It is connected to my desktop over USB-C. With a YubiKey it's possible to import a signing key/certificate into one of its PIV slots. Once imported, it's possible to load the key using `CngKey.Open("GUID_HERE", CngProvider.MicrosoftSmartCardKeyStorageProvider);`. You can then create an `ECDsaCng` instance using that key and sign data. That process above works, but there is a major problem with the signing data step. During a call to `ECDsaCng`'s `SignData` method, you are prompted by Windows to enter your smart card pin. When the correct pin is entered, `SignData` returns a valid signature. However, if you try to sign _more_ data via subsequent calls to `SignData`, you run into a `WindowsCryptographicException`: _Access was denied because of a security violation._ ### Reproduction Steps To properly reproduce, you will need a smart card/security key that can load in custom certificates/keys. I am using a YubiKey and the steps below are tailored for reproducing on YubiKey. 1. Install the [YubiKey Smart Card Minidriver](https://www.yubico.com/support/download/smart-card-drivers-tools/) if you do not have it already. 2. [Download this sample PFX](https://github.com/dotnet/runtime/files/8112090/Package.zip) 3. [Download this sample .NET 6 console application project](https://github.com/dotnet/runtime/files/8112147/YubiDelaySample.zip) 3. [Download the latest yubico-piv-tool](https://developers.yubico.com/yubico-piv-tool/Releases/) and run this command from the folder you extracted the PFX to. In this command, you need to fill in the management key (replace "MGM-KEY" with your own) and also provide a key slot value for --slot (any free slot you have). `yubico-piv-tool --key="MGM-KEY" --action="import-key" --action="import-certificate" --slot="" --input="Package.pfx" --key-format="PKCS12" --password="PASSWORD" --pin-policy="always" --touch-policy="never"` 4. Unplug your YubiKey and plug it back in. 5. You need to find your key container GUID. To do this, run this command. You will be prompted for your pin at least once. After it's done, find the certificate and copy its Key Container value into the sample project. `certutil -store -user "My"` 6. Finally, build the sample project and run it to reproduce. You will be prompted for your pin once. Enter it and you should see printed errors shortly after. ### Expected behavior A `WindowsCryptographicException` should not occur on subsequent `ECDsaCng` `SignData` calls, and a delay should not be necessary. ### Actual behavior `WindowsCryptographicException`: _Access was denied because of a security violation._ can occur on subsequent `ECDsaCng` `SignData` calls. Stack trace of the exception: ``` at Internal.Cryptography.CngCommon.SignHash(SafeNCryptKeyHandle keyHandle, ReadOnlySpan`1 hash, AsymmetricPaddingMode paddingMode, Void* pPaddingInfo, Int32 estimatedSize) at System.Security.Cryptography.ECDsaCng.SignHash(Byte[] hash) at System.Security.Cryptography.ECDsa.SignData(Byte[] data, Int32 offset, Int32 count, HashAlgorithmName hashAlgorithm) at System.Security.Cryptography.ECDsa.SignData(Byte[] data, HashAlgorithmName hashAlgorithm) at System.Security.Cryptography.ECDsaCng.SignData(Byte[] data) at Program.<
$>d__0.MoveNext() in C:\Users\Eaton\Source\Repos\YubiDelaySample\YubiDelaySample\Program.cs:line 14 ``` ### Regression? The problem also occurs on Windows 10 and .NET 5. It was my hope that a clean Windows 11 install may solve the problem, but it did not. Re-importing the key to the YubiKey did not help either. ### Known Workarounds A 3-second delay works around the problem, but it slows down the entire process and is not ideal. I don't know if there is an exact amount of time that needs to pass before signing data again, but I've found 3 seconds to work most of the time. ### Configuration .NET 6.0.1 Windows 11 x64 ### Other information I asked Yubico support about the issue and they had this to say (start reading from the bottom message): [Click to view image](https://user-images.githubusercontent.com/3020941/155018419-18e6ba5b-3fa7-41e5-b6cb-5bef1cadd428.png)
Author: EatonZ
Assignees: -
Labels: `area-System.Security`, `untriaged`
Milestone: -
vcsjones commented 2 years ago

Well, after some fiddling with my Yubikey I was able to reproduce it.

Pass 1 signed
Pass 2 fail: Access was denied because of a security violation.
Pass 2 signed
Pass 3 fail: Access was denied because of a security violation.
Pass 3 signed
Pass 4 fail: Access was denied because of a security violation.

..etc...

On the other hand, I can reproduce it in C++ with direct calls to CNG (forgive the lack of cleanup and proper error handling, but that is not pertinent to reproducing this).

#include <iostream>
#include <windows.h>
#include <stdio.h>
#include <ncrypt.h>

int main()
{
    NCRYPT_PROV_HANDLE provider = NULL;
    NCRYPT_KEY_HANDLE key = NULL;

    if (FAILED(NCryptOpenStorageProvider(&provider, MS_SMART_CARD_KEY_STORAGE_PROVIDER, 0)))
    {
        std::cout << "Couldn't open storage provider.\n";
        return 1;
    }

    if (FAILED(NCryptOpenKey(provider, &key, L"956ac088-eafa-f370-1b98-104bc75fc10a", 0, 0)))
    {
        std::cout << "Couldn't find key.\n";
        return 1;
    }

    BYTE data[32] = { 0 }; // 32-bytes for a SHA256 hash. We don't need "real" data here.
    BYTE sig[256] = { 0 }; // Big enough for any ECDSA signature.
    DWORD written = 0;
    SECURITY_STATUS status;

    while (true)
    {
        if (FAILED(status = NCryptSignHash(key, NULL, (PBYTE)&data, 32, (PBYTE)&sig, 256, &written, 0)))
        {
            std::cout << "Failed to create signature: 0x";
            std::cout << std::hex << status;
            std::cout << "\n";
            Sleep(3000);
        }
        else
        {
            std::cout << "Signed successfully.\n";
        }
    }

    return 0;

}

Gives me the output:

Signed successfully.
Failed to create signature: 0x8010006a
Signed successfully.
Failed to create signature: 0x8010006a
Signed successfully.
Failed to create signature: 0x8010006a

0x8010006A is SCARD_W_SECURITY_VIOLATION.

Yubico has a support article on this here: https://support.yubico.com/hc/en-us/articles/360013790519-Troubleshooting-SCARD-W-SECURITY-VIOLATION, though I don't see anything in there that addresses the problem. I reset my Yubikey to have the default management key and can still reproduce the issue.

Since the issue reproduces without .NET, I'm inclined to believe the problem lies somewhere with CNG or their Minidriver.

EatonZ commented 2 years ago

@vcsjones Thanks for checking!

I too believe that the issue lies deeper in the Windows CNG. This seemed like the best place to report it though. mainly because if a delay must be added, perhaps .NET could implement one on top of the CNG calls to the OS, so that the problem won't occur.

bartonjs commented 2 years ago

I'm not quite sure what a workaround would look like for us. We certainly don't want to sleep all calls to NCryptSignHash (50ms calls becoming 3050ms seems... unfriendly). Additionally, "I've found 3 seconds to work most of the time" makes it hard to decide something like "OK, if the call fails with SCARD_W_SECURITY_VIOLATION wait 3 seconds and try again", because if it still doesn't work now we have to decide "another 3?" or "well, should've just thrown 3 seconds ago..."

I've passed @vcsjones' CNG-directly version onto the CNG team, but I agree with his supposition that it's really a problem in the Yubikey minidriver.

Since there's not something I think we can do as a general approach, I feel like this has to be left as an application-level workaround

EatonZ commented 2 years ago

@bartonjs My workaround is terrible and it slows things down so much, and feels like a hack. If you believe it's a minidriver problem, can you share some details so I can pass it on to Yubico to see what they can do about it?

vcsjones commented 2 years ago

I fiddled with this a bit, and found some debugging opportunities. The registry key

[HKEY_LOCAL_MACHINE\SOFTWARE\Yubico\ykmd]
"DebugOn"=dword:00000001

The logs go to C:\Logs. Note, the logs are very verbose and get large quick. Don't forget to turn it off.

Doesn't reveal a ton, but some interesting observations:

2022-02-25 16:58:54 cardcrypto.cpp:29 IN pInfo->pbData[32]:
000000524990f3f8 / 32
    00000000  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................                                                                                                                                                                                                                                               
    00000010  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................                                                                                                                                                                                                                                               
2022-02-25 16:58:54 cardcrypto.cpp:38 IN pInfo->dwPaddingType = 0
2022-02-25 16:58:54 ykcontext.cpp:51 -> YKContext::Restore
2022-02-25 16:58:54 ykcontext.cpp:54 <- YKContext::Restore returns 00000000 (The operation completed successfully. )
2022-02-25 16:58:54 ykcontext.key.cpp:628 -> YKContext::GetKeyBlockSize
2022-02-25 16:58:54 ykcontext.key.cpp:642 <- YKContext::GetKeyBlockSize returns 00000000 (The operation completed successfully. )
2022-02-25 16:58:54 ykcontext.key.cpp:588 -> YKContext::GetKeySpec
2022-02-25 16:58:54 ykcontext.key.cpp:602 <- YKContext::GetKeySpec returns 00000000 (The operation completed successfully. )
2022-02-25 16:58:54 ykcontext.cpp:96 -> YKContext::Sign
2022-02-25 16:58:54 ykcontext.cpp:165 ykpiv_sign_data failed: YKPIV_AUTHENTICATION_ERROR (-5)
2022-02-25 16:58:54 ykcontext.cpp:221 <- YKContext::Sign returns 8010006a (Access was denied because of a security violation. )
2022-02-25 16:58:54 cardcrypto.cpp:102 <- CardSignData returns 8010006a (Access was denied because of a security violation. )

During the sleep, the card deauthenticates:

2022-02-25 16:58:52 cardpin.cpp:446 -> CardDeauthenticateEx

After the deauthentication, it starts working again. If I remove the Sleep and don't give the card a chance to deauthenticate, it never works and I never see a deauthentication in the log.

At this point the best thing I could recommend is forward the sample program (or link them to GitHub) and include my or your own logs. Given the YKPIV_AUTHENTICATION_ERROR coming from ykcontext.cpp, it seems at the very least their minidriver is playing a role. Unfortunately, I could not find the full source for it.

EatonZ commented 2 years ago

@vcsjones I will re-open the support ticket with a link to this issue. Thanks again for taking a deeper look - please post anything else you may discover.

EatonZ commented 2 years ago

@vcsjones Is there a way to force the deauthentication?

vcsjones commented 2 years ago

@EatonZ best of my knowledge, no, I don't see a way.

EatonZ commented 2 years ago

I received this reply from Yubico. I tried what they recommended with using --pin-policy="once" instead of --pin-policy="always". That appears to have solved the problem.

It seems they do not consider this to be a bug. Do you agree? If so, I can close this, but maybe we should wait until the CNG team comes back with any thoughts?

EatonZ commented 2 years ago

Ok, well, it's mostly fixed with --pin-policy="once" but I am seeing this new error randomly now. Thoughts?

WindowsCryptographicException: An unexpected card error has occurred. HRESULT: 0x8010001F (SCARD_E_UNEXPECTED)

   at Internal.Cryptography.CngCommon.SignHash(SafeNCryptKeyHandle keyHandle, ReadOnlySpan`1 hash, AsymmetricPaddingMode paddingMode, Void* pPaddingInfo, Int32 estimatedSize)
   at System.Security.Cryptography.ECDsaCng.SignHash(Byte[] hash)
   at System.Security.Cryptography.ECDsa.SignData(Stream data, HashAlgorithmName hashAlgorithm)
   at Program.<<Main>$>d__0.MoveNext() in C:\Project\Program.cs:line 163
vcsjones commented 2 years ago

@EatonZ does enabling logging reveal anything interesting?

EatonZ commented 2 years ago

@vcsjones:

2022-03-03 15:57:15 cardcrypto.cpp:38 IN pInfo->dwPaddingType = 0
2022-03-03 15:57:15 ykcontext.cpp:51 -> YKContext::Restore
2022-03-03 15:57:15 ykcontext.cpp:54 <- YKContext::Restore returns 00000000 (The operation completed successfully. )
2022-03-03 15:57:15 ykcontext.key.cpp:628 -> YKContext::GetKeyBlockSize
2022-03-03 15:57:15 ykcontext.key.cpp:642 <- YKContext::GetKeyBlockSize returns 00000000 (The operation completed successfully. )
2022-03-03 15:57:15 ykcontext.key.cpp:588 -> YKContext::GetKeySpec
2022-03-03 15:57:15 ykcontext.key.cpp:602 <- YKContext::GetKeySpec returns 00000000 (The operation completed successfully. )
2022-03-03 15:57:15 ykcontext.cpp:96 -> YKContext::Sign
2022-03-03 15:57:15 ykcontext.cpp:221 <- YKContext::Sign returns 8010001f (An unexpected card error has occurred. )
2022-03-03 15:57:15 cardcrypto.cpp:102 <- CardSignData returns 8010001f (An unexpected card error has occurred. )
2022-03-03 15:57:16 cardpin.cpp:446 -> CardDeauthenticateEx
2022-03-03 15:57:16 cardpin.cpp:447 IN PinID: 2
2022-03-03 15:57:16 cardpin.cpp:448 IN dwFlags: 0
2022-03-03 15:57:16 ykcontext.cpp:51 -> YKContext::Restore
2022-03-03 15:57:16 ykcontext.cpp:54 <- YKContext::Restore returns 00000000 (The operation completed successfully. )
2022-03-03 15:57:16 ykcontext.auth.cpp:384 -> YKContext::Deauthenticate
2022-03-03 15:57:16 ykcontext.auth.cpp:411 <- YKContext::Deauthenticate returns 00000000 (The operation completed successfully. )
2022-03-03 15:57:16 cardpin.cpp:460 <- CardDeauthenticateEx returns 00000000 (The operation completed successfully. )
vcsjones commented 2 years ago

@EatonZ I think then at this point my advice remains the same, just with a different error. It seems the error is coming from their PIV / minidriver and it's just bubbling up through CNG.

EatonZ commented 2 years ago

I am working with Yubico support on that new error. Once I have new information I will post here to potentially help others, and probably will close this issue, unless there is any input from the CNG team we are waiting on.

EatonZ commented 2 years ago

I received the following from Yubico support today. Looks like we will indeed need some help from the CNG team to get the SCARD_E_UNEXPECTED issue figured out. @bartonjs Can you pass on the below to them and let me know what our options are?

Hello Eaton,

Our engineering team has reviewed the ADPU trace you have provided. However they have come to the conclusion the error is likely within the CSP/CNG. Unfortunately, this brings us to the inability to troubleshoot further as it will require gathering an ETL trace of the CSP/CNG provider which can only be debugged with the help of Microsoft as the debug symbols are not publicly available. I'm sorry we cannot assist any further and I know this is not the best news to hear as well, unless you are able to open a case with Microsoft and gather a ETL trace and provide us with a decrypted trace.

bartonjs commented 2 years ago

The CNG/OS team recommends opening a support case, such as via https://docs.microsoft.com/en-us/services-hub/unified/support/open-support-requests#:~:text=To%20open%20a%20support%20request,and%20level%20of%20support%20required (or from the Azure portal, or wherever you can find one).

Hi Jeremy

The recommendation would be to request Eaton to open a CSS case. The same engineer can likely pick up the case from GitHub that Yubico was pursuing to close it out.

When the issue is with Windows, the recommendation is to send a collaboration task in the Windows CSS case management system to CSS Windows support – the queue name is

msaas windows directory services premier

This is where you will be better supported in a timely manner and the more traditional/long standing support path

(Partly the recommendation was to transfer our support case, but our GitHub-based issues and their customer support cases aren't transferrable)

EatonZ commented 2 years ago

@bartonjs The first link wants a contract or subscription and doesn't provide a way to purchase.

Tried Azure and it wants a paid support plan. That's ok I guess and I can upgrade, but what is the best service/problem types to file it under because they are all Azure-specific?

If there is a "free" web form or email I can submit the information to, that would also be nice.🙂

bartonjs commented 2 years ago

I.. honestly have no idea. @richlander I don't suppose you know where the customer side/experience is for opening OS support cases? :smile:

richlander commented 2 years ago

I don't. I'd suggest you talk with your friends in Windows. They are the best source of that info.

bartonjs commented 2 years ago

https://support.microsoft.com/contactus looks promising (either via the app or the browser based one)

EatonZ commented 2 years ago

@bartonjs I did try that. It only provides chat and phone support and not a form where I can provide code samples, attachments, links, etc.

EatonZ commented 2 years ago

FYI if there is a private form or email address I can use to open a support request, feel free to email it to me (email in profile).

This is getting a little off-topic from .NET so I'll be happy to close this issue once I can get the support request opened up.

vcsjones commented 2 years ago

@bartonjs do you think using the Windows Insider Feedback Hub would work, to at least get some kind of internal ticket working?

bartonjs commented 2 years ago

That... sounds.... reasonable? :smile:

EatonZ commented 2 years ago

If you think that would suffice and you could point the right people at it, I can try going that route.

EatonZ commented 1 year ago

Hi - Happy New Year.🙂 Is there any interest in getting an internal ticket going? This is still happening. Not a huge problem, but still seems worth investigating the root cause.

drew-512 commented 5 months ago

Hey y'all, I seem to be getting this too, but worse in that there is no workaround since this is through signtool.exe:

2024-05-24 21:33:50 ykcontext.key.cpp:597 -> YKContext::GetKeySpec
2024-05-24 21:33:50 ykcontext.key.cpp:611 <- YKContext::GetKeySpec returns 00000000 (The operation completed successfully. )
2024-05-24 21:33:50 ykcontext.cpp:97 -> YKContext::Sign
2024-05-24 21:33:50 ykcontext.cpp:167 ykpiv_sign_data failed: YKPIV_ARGUMENT_ERROR (-14)
2024-05-24 21:33:50 ykcontext.cpp:223 <- YKContext::Sign returns 80100014 (An internal error has been detected, but the source is unknown. )
2024-05-24 21:33:50 cardcrypto.cpp:102 <- CardSignData returns 80100014 (An internal error has been detected, but the source is unknown. )
2024-05-24 21:33:50 cardpin.cpp:456 -> CardDeauthenticateEx
2024-05-24 21:33:50 cardpin.cpp:457 IN PinID: 2

Since this is signtool.exe I can't specify --pin-policy="once" yikes.

I'm an indie dev that just dropped $500 on this cert racket so I can sign my software, so I'm scared now that my YubiKey faith will be a costly mistake (not even to mention the time burned). Still waiting to hear back from Yubikey support.

Workarounds would be deeply appreciated!

EatonZ commented 5 months ago

@drew-512 Unfortunately no one here got back to me on how to open a real MS support ticket to get this looked at. It still happens to me and it's definitely annoying.

In your case, I would retry signtool in a loop until it succeeds. You could create a batch file or something to do it, then call it from the build step.

drew-512 commented 5 months ago

Appreciate you @EatonZ . Unfortunately, I've yet to get a successful sign. Worse, even if it did, I missed where ya had set the pin policy (on import) other than always, which means I'd looking forward to having to enter the PIN dozens of times to run our build script. Brutal outcome.

Although this is MS's issue this effectively kills YubiKey's ability to offer a useful product for signtool. What I can't figure out is that everything is vanilla here, so how are there not floods of people with this issue using signtool.

richlander commented 5 months ago

This seems like something that the signtool team should drive and I'd recommend we move this issue to that repo. I think it is entirely reasonable for the signtool team to open the issue with Windows. The Yubikey and Windows teams should primary be driving this and sharing their progress.

@clairernovotny @dtivel

drew-512 commented 5 months ago

Hi friends, my unusual flavor of crash has been resolved and wanted to put it in the record.

Sectigo sent me the signed cert in ECCP384 rather than ECCP256. What's unimpressive is that Yubikey just merrily imported a cert without doing any verification:

$ ./ykman piv info
PIV version:              5.4.2
PIN tries remaining:      3/3
PUK tries remaining:      1/3
Management key algorithm: TDES
Management key is stored on the YubiKey, protected by PIN.

Slot 9A (AUTHENTICATION):
  Private key type: ECCP256
  Public key type:  ECCP384
  ...

So this crash was caused by the cert sig not matching the private key. Not clear who's issue it is, but whomever it is, a hard crash rather than an error message is also unimpressive. That may be the way to file this since the repro is 100%. Happy to support that, lmk if I can help.

Sectigo let me key a new cert and the newly issued ECCP384 and imported cert is working.

Appreciate y'all, gents!

EatonZ commented 4 months ago

@richlander While @drew-512's specific issue is resolved, my original issue remains. Is it still possible to move this forward somehow?

richlander commented 4 months ago

I'm asking folks.