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

crypto/rsa: verification error while signing with RSA key #1528

Open
melkikh opened this issue Nov 24, 2023 · 0 comments
Open

crypto/rsa: verification error while signing with RSA key #1528

melkikh opened this issue Nov 24, 2023 · 0 comments
Labels
bug Something isn't working

Comments

@melkikh
Copy link

melkikh commented Nov 24, 2023

Description

Hi!
I use the sigstore Azure KMS package (github.com/sigstore/sigstore/pkg/signature/kms/azure), to sign X.509 certificates with non-exportable private keys stored in Azure KeyVault.
So, I decided to use the RSA2048 key with SHA265 hash function for signing Certificate Signing Requests, and use the crypto.Signer implementation from the sigstore package.
Here is an example of creating a key in Azure Key Vault:
az keyvault key create --exportable false --vault-name 'test-sigstore-kv' --name 'test-root-ca-key-rsa2048-1' --kty RSA --size 2048

Here is an example of code for creating a self-signed Root CA certificate:

azPKI, err := azure.LoadSignerVerifier(ctx, referenceStr)
publicKey, err := az.PublicKey()
caCertificateTemplate := &x509.Certificate{
	PublicKeyAlgorithm: x509.RSA,
	PublicKey:          publicKey,

	SignatureAlgorithm: x509.SHA256WithRSA,

	SerialNumber: big.NewInt(1),
	Issuer: pkix.Name{
		CommonName: "Company Name",
	},
	Subject: pkix.Name{
		CommonName: "Company Name",
	},

	NotBefore: time.Now(),
	NotAfter:  time.Now().Add(time.Hour * 24 * 180),
	IsCA:      true,
	KeyUsage:  x509.KeyUsageDigitalSignature | x509.KeyUsageCertSign | x509.KeyUsageKeyEncipherment,
	BasicConstraintsValid: true,
}
errorFunc := func(err error) {
	if err != nil {
		logger.Fatalln("failed to sign using KeyVault: %w", err)
	}
}
signer, _, err := az.CryptoSigner(ctx, errorFunc)
crt, err := x509.CreateCertificate(rand.Reader, caCertificateTemplate, caCertificateTemplate, publicKey, signer)
if err != nil {
	return fmt.Errorf("failed to create certificate using CSR: %w", err)
}

This code triggers an error:
failed to create certificate using CSR: x509: signature over certificate returned by signer is invalid: crypto/rsa: verification error
which is generating by built-in signature check in x509.

Version

github.com/sigstore/sigstore/pkg/signature/kms/azure v1.7.5
go version go1.19.4 darwin/arm64

Root cause
I found a similar question on Stack Overflow, which led me to check the hashing implementation in sigstore azure package.

The sigstore package uses the hashing scheme implemented in ECDSA:

// Convert the concatenated r||s byte string to an ASN.1 sequence
// This logic is borrowed from https://cs.opensource.google/go/go/+/refs/tags/go1.17.3:src/crypto/ecdsa/ecdsa.go;l=121
var b cryptobyte.Builder
b.AddASN1(asn1.SEQUENCE, func(b *cryptobyte.Builder) {
b.AddASN1BigInt(r)
b.AddASN1BigInt(s)
})

But for RSASSA-PKCS1-V1_5-SIGN the scheme is different.

Solution
I forked a package and patched this behavior:

--- /Users/melkikh/go/pkg/mod/github.com/sigstore/sigstore/pkg/signature/kms/azure@v1.7.5/signer.go     2023-11-20 23:55:48
+++ signer.go   2023-11-23 22:58:55
@@ -78,6 +78,17 @@
        return a, nil
 }

+var hashPrefixes = map[crypto.Hash][]byte{
+       crypto.MD5:       {0x30, 0x20, 0x30, 0x0c, 0x06, 0x08, 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x02, 0x05, 0x05, 0x00, 0x04, 0x10},
+       crypto.SHA1:      {0x30, 0x21, 0x30, 0x09, 0x06, 0x05, 0x2b, 0x0e, 0x03, 0x02, 0x1a, 0x05, 0x00, 0x04, 0x14},
+       crypto.SHA224:    {0x30, 0x2d, 0x30, 0x0d, 0x06, 0x09, 0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x04, 0x05, 0x00, 0x04, 0x1c},
+       crypto.SHA256:    {0x30, 0x31, 0x30, 0x0d, 0x06, 0x09, 0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x01, 0x05, 0x00, 0x04, 0x20},
+       crypto.SHA384:    {0x30, 0x41, 0x30, 0x0d, 0x06, 0x09, 0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x02, 0x05, 0x00, 0x04, 0x30},
+       crypto.SHA512:    {0x30, 0x51, 0x30, 0x0d, 0x06, 0x09, 0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x03, 0x05, 0x00, 0x04, 0x40},
+       crypto.MD5SHA1:   {}, // A special TLS case which doesn't use an ASN1 prefix.
+       crypto.RIPEMD160: {0x30, 0x20, 0x30, 0x08, 0x06, 0x06, 0x28, 0xcf, 0x06, 0x03, 0x00, 0x31, 0x04, 0x14},
+}
+
 // SignMessage signs the provided message using Azure Key Vault. If the message is provided,
 // this method will compute the digest according to the hash function specified
 // when the Signer was created.
@@ -97,13 +108,20 @@

        for _, opt := range opts {
                opt.ApplyDigest(&digest)
+               opt.ApplyContext(&ctx)
        }

-       hashFunc, _, err := a.client.getKeyVaultHashFunc(ctx)
+       var signerOpts crypto.SignerOpts
+       signerOpts, _, err := a.client.getKeyVaultHashFunc(ctx)
        if err != nil {
-               return nil, err
+               return nil, fmt.Errorf("getting fetching default hash function: %w", err)
        }

+       for _, opt := range opts {
+               opt.ApplyCryptoSignerOpts(&signerOpts)
+       }
+
+       hashFunc := signerOpts.HashFunc()
        digest, _, err = signature.ComputeDigestForSigning(message, hashFunc, azureSupportedHashFuncs, opts...)
        if err != nil {
                return nil, err
@@ -114,22 +132,61 @@
                return nil, err
        }

-       l := len(rawSig)
-       r, s := &big.Int{}, &big.Int{}
-       r.SetBytes(rawSig[0 : l/2])
-       s.SetBytes(rawSig[l/2:])
+       // l := len(rawSig)
+       // r, s := &big.Int{}, &big.Int{}
+       // r.SetBytes(rawSig[0 : l/2])
+       // s.SetBytes(rawSig[l/2:])

-       // Convert the concatenated r||s byte string to an ASN.1 sequence
-       // This logic is borrowed from https://cs.opensource.google/go/go/+/refs/tags/go1.17.3:src/crypto/ecdsa/ecdsa.go;l=121
-       var b cryptobyte.Builder
-       b.AddASN1(asn1.SEQUENCE, func(b *cryptobyte.Builder) {
-               b.AddASN1BigInt(r)
-               b.AddASN1BigInt(s)
-       })
+       // // Convert the concatenated r||s byte string to an ASN.1 sequence
+       // // This logic is borrowed from https://cs.opensource.google/go/go/+/refs/tags/go1.17.3:src/crypto/ecdsa/ecdsa.go;l=121
+       // var b cryptobyte.Builder
+       // b.AddASN1(asn1.SEQUENCE, func(b *cryptobyte.Builder) {
+       //      b.AddASN1BigInt(r)
+       //      b.AddASN1BigInt(s)
+       // })

-       return b.Bytes()
+       hashLen, prefix, err := pkcs1v15HashInfo(hashFunc, len(digest))
+       if err != nil {
+               return nil, err
+       }
+
+       tLen := len(prefix) + hashLen
+       k := 256
+       if k < tLen+11 {
+               return nil, fmt.Errorf("too long")
+       }
+
+       // EM = 0x00 || 0x01 || PS || 0x00 || T
+       em := make([]byte, k)
+       em[1] = 1
+       for i := 2; i < k-tLen-1; i++ {
+               em[i] = 0xff
+       }
+       copy(em[k-tLen:k-hashLen], prefix)
+       copy(em[k-hashLen:k], digest)
+
+       m := new(big.Int).SetBytes(rawSig).FillBytes(em)
+       return m, nil
 }

+func pkcs1v15HashInfo(hash crypto.Hash, inLen int) (hashLen int, prefix []byte, err error) {
+       // Special case: crypto.Hash(0) is used to indicate that the data is
+       // signed directly.
+       if hash == 0 {
+               return inLen, nil, nil
+       }
+
+       hashLen = hash.Size()
+       if inLen != hashLen {
+               return 0, nil, errors.New("crypto/rsa: input must be hashed message")
+       }
+       prefix, ok := hashPrefixes[hash]
+       if !ok {
+               return 0, nil, errors.New("crypto/rsa: unsupported hash function")
+       }
+       return
+}
+
 // VerifySignature verifies the signature for the given message. Unless provided
 // in an option, the digest of the message will be computed using the hash function specified
 // when the SignerVerifier was created.

This patch works, but a the moment I decided to use an ECDSA key, while the sigstore package doesn't support RSA keys for X.509 signature.

If interested, I could rework this patch a bit and submit a pull request.

@melkikh melkikh added the bug Something isn't working label Nov 24, 2023
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

1 participant