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

Unable to use signtool /ds option with the CNG provider #20

Open
obones opened this issue Sep 27, 2023 · 7 comments
Open

Unable to use signtool /ds option with the CNG provider #20

obones opened this issue Sep 27, 2023 · 7 comments
Labels
bug Something isn't working

Comments

@obones
Copy link

obones commented Sep 27, 2023

Hello,

Because of the absence of an x86 version of the provider (See #18), I'm trying a workaround by using the following steps:

  1. get the file digest by using the /dg option

     signtool sign /q /f "%CertificatePublicKeyFilename%" /du %SignatureUrl%  /fd sha256 /dg . "%FileToSign%"
    
  2. Sign the generated digest with the CNG provider and the /ds option

     signtool sign /q /f "%CertificatePublicKeyFilename%"  /csp "%GoogleCSPName%" /kc "%GoogleCloudKeyVersionFullPath%" /ds "%DigestFile%"
    
  3. Integrate the signed digest with the /di option

     signtool sign /q /f "%CertificatePublicKeyFilename%" /di . "%FileToSign%"
    
  4. Timestamp the generated signature

     signtool timestamp /q /tr %SignatureTimeStampURL% /td sha256 "%FileToSign%"
    

When using a certificate whose private key is stored on a physical usb token (Yubikey), this works fine.
But when I use the CNG provider, I get the following error:

I0927 12:56:16.817818   31368 logging.cc:81] returning 0x80090027 from SignHashFn due to status INVALID_ARGUMENT: at bridge.cc:421: unsupported pPaddingInfo [type.googleapis.com/kmscng.StatusDetails='SECURITY_STATUS=0x80090027']

Looking at bridge.cc:421 the error seems to be created on purpose but I'm not sure what to make of this.
I would simply remove the test as it seems pPaddingInfo is not used at all in that function, but this must be a very naive approach.

Any help would be most welcome.

@tdbhacks
Copy link
Member

Hello,

I am not familiar with this flow, but per the CNG spec, the pPaddingInfo parameter should really only be used when passing in structs of type BCRYPT_PKCS1_PADDING_INFO or BCRYPT_PSS_PADDING_INFO.

I guess the parameter description leaves some room for interpretation because it says "This parameter is only used with asymmetric keys and must be NULL otherwise", and ECDSA is an asymmetric algorithm, but we are expecting it to be NULL in our provider because it doesn't really make sense to specify it for ECDSA algorithms (unless my reading of the spec is wrong).

Just to confirm, are you trying to use this with an ECDSA key or an RSA-PKCS1 / RSA-PSS key? See the limitations section. If you are indeed using an ECDSA key, I'll tweak the provider logging when I have some time and try to go through the same flow to understand what it's trying to set it to. Thanks for bringing this up!

@obones
Copy link
Author

obones commented Oct 18, 2023

To me this is an ECDSA key:
image

And the CNG provider works if I don't use the /ds option but rather have signtool do all actions at once.

@tdbhacks
Copy link
Member

tdbhacks commented Nov 6, 2023

Indeed, I found some time to reproduce this with my own EC-P256 key and I also ran into the same error.

Played with it for a bit and tweaked our logging to check what is being passed to the provider: it looks like SignTool is trying to use the BCRYPT_PAD_PKCS1 flag in combination with a BCRYPT_PKCS1_PADDING_INFO struct with pszAlgId == SHA256, so at this point I'm pretty confused because this whole flow should only apply to RSA-PKCS1 keys...

We should investigate further, unclear if this is our bug or a weird behavior of SignTool, but will keep this issue open as a bug for now.

@tdbhacks tdbhacks added the bug Something isn't working label Nov 6, 2023
@ysichrisdag
Copy link

Is there any plan to address this bug? Really need either this one addressed or #18

@tdbhacks
Copy link
Member

@obones @ysichrisdag as I was going through the open issues of the repo, I figured I'd try out this flow again now that RSA4096-PKCS1 support has been merged (#29). I haven't tried ECDSA, I assume the bug is still present and I don't think we can do much about that, but I believe I was able to sign something successfully using RSA_SIGN_PKCS1_4096_SHA256!

Is ECDSA a requirement? If not, it would be great if you could test this out with RSA

@obones
Copy link
Author

obones commented May 13, 2024

I haven't tried ECDSA, I assume the bug is still present and I don't think we can do much about that

Well, this issue is here just because I tried a workaround issue #18 but if that other issue was to be addressed, then this one could go way down in the priority list.

Is ECDSA a requirement?

Yes, it is because it comes from my signing key provider as I require a signing key backed by the PKI recognized by the Windows operating system.

@obones
Copy link
Author

obones commented Aug 19, 2024

FWIW, I tried with signtool from SDK 10.0.26100.0 but this did not change the situation

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants