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

Local Cert verify not working for Azure KMS #1384

Open
shubham2110 opened this issue Sep 5, 2023 · 19 comments
Open

Local Cert verify not working for Azure KMS #1384

shubham2110 opened this issue Sep 5, 2023 · 19 comments
Labels
bug Something isn't working

Comments

@shubham2110
Copy link

Description

Using cosign with Azure KMS results in different behavior for validation with KMS and local pub cert.

cosign  --insecure-ignore-tlog=true verify -key azurekms://keyvault-xyz.vault.azure.net/cosign registryXYZ.azurecr.io/example-func:1.0.0
WARNING: Skipping tlog verification is an insecure practice that lacks of transparency and auditability verification for the signature.

Verification for registryXYZ.azurecr.io/example-func:1.0.0 --
The following checks were performed on each of these signatures:
  - The cosign claims were validated
  - The signatures were verified against the specified public key

But when I use the created public certificate, the result is as follows:

cosign  --insecure-ignore-tlog=true verify -key cosign.pub registryXYZ.azurecr.io/example-func:1.0.0
WARNING: Skipping tlog verification is an insecure practice that lacks of transparency and auditability verification for the signature.
Error: no matching signatures: crypto/rsa: verification error
main.go:69: error during command execution: no matching signatures: crypto/rsa: verification error

I also exported the key again and tried to reproduce, with the same result:

cosign public-key -key azurekms://ckeyvaultXYZ.azure.net/cosign > cosign-new.pub
cosign verify -key cosign-new.pub registryXYZ.azurecr.io/example-func:1.0.0
WARNING: Skipping tlog verification is an insecure practice that lacks of transparency and auditability verification for the signature.
Error: no matching signatures: crypto/rsa: verification error
main.go:69: error during command execution: no matching signatures: crypto/rsa: verification error

Is there anything I did wrong or is there a bug in the verification?
-->

Version
cosign version
GitVersion: v2.2.0
GitCommit: 546f1c5b91ef58d6b034a402d0211d980184a0e5
GitTreeState: clean
BuildDate: 2023-08-31T18:52:52Z
GoVersion: go1.21.0
Compiler: gc
Platform: linux/amd64
-->

@shubham2110
Copy link
Author

Key type: RSA
RSA key size: 2048

@dylrich
Copy link

dylrich commented Oct 18, 2023

I am also experiencing this bug -- RSA key size 4096 with cosign 2.2

@haydentherapper
Copy link
Contributor

@malancas Any guesses? I don't have the environment to repro this.

@malancas
Copy link
Contributor

malancas commented Nov 21, 2023

@malancas Any guesses? I don't have the environment to repro this.

@haydentherapper I have some ideas, I'll take a look at replicating today.

@d7h
Copy link

d7h commented Dec 27, 2023

Any Updates here?

@d7h
Copy link

d7h commented Jan 18, 2024

@malancas Can you tell me your ideas? I could check them because we still have the error in our environment.

@malancas
Copy link
Contributor

malancas commented Jan 18, 2024

@d7h I'm taking a look into this today. Just to confirm, are you encountering the error after updating to the latest version of Cosign?

@d7h
Copy link

d7h commented Jan 19, 2024

@malancas Ok, thanks. Yes:

GitVersion: 2.2.2
GitCommit: bf6b57bc3edf8deb7e225e4dbd2d26c0d432979b
GitTreeState: "clean"
BuildDate: 2023-12-05T18:59:25Z
GoVersion: go1.21.4
Compiler: gc
Platform: darwin/arm64

@malancas
Copy link
Contributor

I've identified the cause of the bug and am currently working on a solution

@d7h
Copy link

d7h commented Feb 19, 2024

@malancas Hello, I wanted to inquire about the current status because our company has to decide on a signature product shortly. Our security team wants to have RSA encryption. Unfortunately, verification against the public key in Azure is not an option.

@malancas
Copy link
Contributor

@dh7 after some additional debugging and reading through the Azure Key Vault. documentation, I think the issue may have to do with the presence of an optional flag when calling the verify command. The SHA-512 hash function is used with Azure Key Vault RSA-4096 keys and SHA-384 is used with RSA-3072 keys. By default, the verify command will assume the SHA-256 hash function is used. Do you still get the verification error using a local key when the --signature-digest-algorithm set to either sha512 or sha384 (depending on your key size) is specified?

I am seeing a separate bug in the verify-blob command that is caused by hash function hardcoding, if you are also running into this issue with the verify-blob command.

@d7h
Copy link

d7h commented Feb 21, 2024

@malancas Unfortunately, I still encounter the same error even when specifying the -signature-digest-algorithm parameter. I have tried all three key sizes. We do not require the verify-blob command at this time. Please let me know if there is anything I can do to help identify the cause or a workaround.

@d7h
Copy link

d7h commented Feb 22, 2024

@malancas I have no idea about GO and only a little bit about cryptography, but as far as I can see, when verified locally, the error is thrown here: https://github.com/golang/go/blob/master/src/crypto/rsa/pkcs1v15.go#L351.
Because k = 512 and len(sig) = 524

When verifying against Azure directly, it never reaches this function in this class. I don't know why. I think the same key should end up in the same description algorithm; it doesn't matter if it's stored in a different location. I don't Know if this helps somehow.

@malancas
Copy link
Contributor

malancas commented Feb 23, 2024

@malancas I have no idea about GO and only a little bit about cryptography, but as far as I can see, when verified locally, the error is thrown here: https://github.com/golang/go/blob/master/src/crypto/rsa/pkcs1v15.go#L351. Because k = 512 and len(sig) = 524

When verifying against Azure directly, it never reaches this function in this class. I don't know why. I think the same key should end up in the same description algorithm; it doesn't matter if it's stored in a different location. I don't Know if this helps somehow.

Thanks for the information here, I will look into this code and debug.

@akashsinghal
Copy link

@malancas thanks for looking into this issue. I am running into the same issue. Is there an update on the fix?

@akashsinghal
Copy link

@malancas I have no idea about GO and only a little bit about cryptography, but as far as I can see, when verified locally, the error is thrown here: https://github.com/golang/go/blob/master/src/crypto/rsa/pkcs1v15.go#L351. Because k = 512 and len(sig) = 524

When verifying against Azure directly, it never reaches this function in this class. I don't know why. I think the same key should end up in the same description algorithm; it doesn't matter if it's stored in a different location. I don't Know if this helps somehow.

I took a look at this and I think it's related to the way the azure signer verifier is implemented. The signer ASN.1 encodes the sig payload returned from AKV as an ASN.1 format. I'm not an expert in this but from the code comments it seems that this is part of ECDSA format. This is likely the reason for why RSA verifier throws the length mismatch error?

// This logic is borrowed from https://cs.opensource.google/go/go/+/refs/tags/go1.17.3:src/crypto/ecdsa/ecdsa.go;l=121

Verifying without downloading the Public Key from AKV uses the azure signer verifiers verify method assumes the sig will be ASN.1 encoded, which is probably why it works.

@malancas
Copy link
Contributor

malancas commented Apr 5, 2024

@malancas I have no idea about GO and only a little bit about cryptography, but as far as I can see, when verified locally, the error is thrown here: https://github.com/golang/go/blob/master/src/crypto/rsa/pkcs1v15.go#L351. Because k = 512 and len(sig) = 524
When verifying against Azure directly, it never reaches this function in this class. I don't know why. I think the same key should end up in the same description algorithm; it doesn't matter if it's stored in a different location. I don't Know if this helps somehow.

I took a look at this and I think it's related to the way the azure signer verifier is implemented. The signer ASN.1 encodes the sig payload returned from AKV as an ASN.1 format. I'm not an expert in this but from the code comments it seems that this is part of ECDSA format. This is likely the reason for why RSA verifier throws the length mismatch error?

sigstore/pkg/signature/kms/azure/signer.go

Line 117 in 8b208f7

// This logic is borrowed from https://cs.opensource.google/go/go/+/refs/tags/go1.17.3:src/crypto/ecdsa/ecdsa.go;l=121
Verifying without downloading the Public Key from AKV uses the azure signer verifiers verify method assumes the sig will be ASN.1 encoded, which is probably why it works.

Thanks for adding your findings here. I'm going to take a look at this section of the code and debug.

@akashsinghal
Copy link

@malancas I decided to test the hypothesis that the ASN.1 encoding was causing the issue. I manually unwrapped the ASN.1 encoding from the sig before passing to the cosign RSA PKCS1v15 verifier and it worked. Looks like a fix would involve detecting the key type and if it's RSA, return the raw signature instead of ASN.1 encoding.

@malancas
Copy link
Contributor

@malancas I decided to test the hypothesis that the ASN.1 encoding was causing the issue. I manually unwrapped the ASN.1 encoding from the sig before passing to the cosign RSA PKCS1v15 verifier and it worked. Looks like a fix would involve detecting the key type and if it's RSA, return the raw signature instead of ASN.1 encoding.

Thanks for testing and adding this context, I can take a look at opening a PR to fix this.

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

6 participants