PKI: Add support for signature_bits param to the intermediate/generat…

…e api (#17388)

* PKI: Add support for signature_bits param to the intermediate/generate api

 - Mainly to work properly with GCP backed managed keys, we need to
   issue signatures that would match the GCP key algorithm.
 - At this time due to golang/go#45990 we
   can't issue PSS signed CSRs, as the libraries in Go always request
   a PKCS1v15.
 - Add an extra check in intermediate/generate that validates the CSR's
   signature before providing it back to the client in case we generated
   a bad signature such as if an end-user used a GCP backed managed key
   with a RSA PSS algorithm.
   - GCP ignores the requested signature type and always signs with the
     key's algorithm which can lead to a CSR that says it is signed with
     a PKCS1v15 algorithm but is actually a RSA PSS signature

* Add cl

* PR feedback

builtin/logical/pki/backend_test.go

@@ -2539,7 +2539,7 @@ func TestBackend_ConsulSignLeafWithLegacyRole(t *testing.T) {
 
 	signedCert := parseCert(t, certAsPem)
 	rootCert := parseCert(t, rootCaPem)
-	requireSignedBy(t, signedCert, rootCert.PublicKey)
+	requireSignedBy(t, signedCert, rootCert)
 }
 
 func TestBackend_SignSelfIssued(t *testing.T) {
@@ -4097,7 +4097,7 @@ func runFullCAChainTest(t *testing.T, keyType string) {
 
 	rootCaCert := parseCert(t, rootCert)
 	intermediaryCaCert := parseCert(t, intermediateCert)
-	requireSignedBy(t, intermediaryCaCert, rootCaCert.PublicKey)
+	requireSignedBy(t, intermediaryCaCert, rootCaCert)
 	intermediateCaChain := intermediateSignedData["ca_chain"].([]string)
 
 	require.Equal(t, parseCert(t, intermediateCaChain[0]), intermediaryCaCert, "intermediate signed cert should have been part of ca_chain")
@@ -4191,7 +4191,7 @@ func runFullCAChainTest(t *testing.T, keyType string) {
 	issuedCrt := parseCert(t, issueCrtAsPem)
 
 	// Verify that the certificates are signed by the intermediary CA key...
-	requireSignedBy(t, issuedCrt, intermediaryCaCert.PublicKey)
+	requireSignedBy(t, issuedCrt, intermediaryCaCert)
 
 	// Test that we can request that the root ca certificate not appear in the ca_chain field
 	resp, err = CBWrite(b_ext, s_ext, "issue/example", map[string]interface{}{

builtin/logical/pki/integation_test.go

@@ -2,6 +2,13 @@ package pki
 
 import (
 	"context"
+	"crypto"
+	"crypto/ecdsa"
+	"crypto/ed25519"
+	"crypto/rsa"
+	"crypto/x509"
+	"encoding/pem"
+	"fmt"
 	"testing"
 
 	"github.com/hashicorp/vault/sdk/logical"
@@ -297,6 +304,59 @@ func TestIntegration_SetSignedWithBackwardsPemBundles(t *testing.T) {
 	require.False(t, resp.IsError(), "got an error issuing a leaf cert from int ca: %#v", resp)
 }
 
+func TestIntegration_CSRGeneration(t *testing.T) {
+	t.Parallel()
+	b, s := createBackendWithStorage(t)
+	testCases := []struct {
+		keyType               string
+		usePss                bool
+		keyBits               int
+		sigBits               int
+		expectedPublicKeyType crypto.PublicKey
+		expectedSignature     x509.SignatureAlgorithm
+	}{
+		{"rsa", false, 2048, 0, &rsa.PublicKey{}, x509.SHA256WithRSA},
+		{"rsa", false, 2048, 384, &rsa.PublicKey{}, x509.SHA384WithRSA},
+		// Add back once https://github.com/golang/go/issues/45990 is fixed.
+		// {"rsa", true, 2048, 0, &rsa.PublicKey{}, x509.SHA256WithRSAPSS},
+		// {"rsa", true, 2048, 512, &rsa.PublicKey{}, x509.SHA512WithRSAPSS},
+		{"ec", false, 224, 0, &ecdsa.PublicKey{}, x509.ECDSAWithSHA256},
+		{"ec", false, 256, 0, &ecdsa.PublicKey{}, x509.ECDSAWithSHA256},
+		{"ec", false, 384, 0, &ecdsa.PublicKey{}, x509.ECDSAWithSHA384},
+		{"ec", false, 521, 0, &ecdsa.PublicKey{}, x509.ECDSAWithSHA512},
+		{"ec", false, 521, 224, &ecdsa.PublicKey{}, x509.ECDSAWithSHA512}, // We ignore signature_bits for ec
+		{"ed25519", false, 0, 0, ed25519.PublicKey{}, x509.PureEd25519},   // We ignore both fields for ed25519
+	}
+	for _, tc := range testCases {
+		keyTypeName := tc.keyType
+		if tc.usePss {
+			keyTypeName = tc.keyType + "-pss"
+		}
+		testName := fmt.Sprintf("%s-%d-%d", keyTypeName, tc.keyBits, tc.sigBits)
+		t.Run(testName, func(t *testing.T) {
+			resp, err := CBWrite(b, s, "intermediate/generate/internal", map[string]interface{}{
+				"common_name":    "myint.com",
+				"key_type":       tc.keyType,
+				"key_bits":       tc.keyBits,
+				"signature_bits": tc.sigBits,
+				"use_pss":        tc.usePss,
+			})
+			requireSuccessNonNilResponse(t, resp, err)
+			requireFieldsSetInResp(t, resp, "csr")
+
+			csrString := resp.Data["csr"].(string)
+			pemBlock, _ := pem.Decode([]byte(csrString))
+			require.NotNil(t, pemBlock, "failed to parse returned csr pem block")
+			csr, err := x509.ParseCertificateRequest(pemBlock.Bytes)
+			require.NoError(t, err, "failed parsing certificate request")
+
+			require.Equal(t, tc.expectedSignature, csr.SignatureAlgorithm,
+				"Expected %s, got %s", tc.expectedSignature.String(), csr.SignatureAlgorithm.String())
+			require.IsType(t, tc.expectedPublicKeyType, csr.PublicKey)
+		})
+	}
+}
+
 func genTestRootCa(t *testing.T, b *backend, s logical.Storage) (issuerID, keyID) {
 	return genTestRootCaWithIssuerName(t, b, s, "")
 }

builtin/logical/pki/path_intermediate.go

@@ -63,16 +63,7 @@ func (b *backend) pathGenerateIntermediate(ctx context.Context, req *logical.Req
 		data.Raw["exported"] = "existing"
 	}
 
-	// Nasty hack part two. :-) For generation of CSRs, certutil presently doesn't
-	// support configuration of this. However, because we need generation parameters,
-	// which create a role and attempt to read this parameter, we need to provide
-	// a value (which will be ignored). Hence, we stub in the missing parameters here,
-	// including its schema, just enough for it to work..
-	data.Schema["signature_bits"] = &framework.FieldSchema{
-		Type:    framework.TypeInt,
-		Default: 0,
-	}
-	data.Raw["signature_bits"] = 0
+	// Remove this once https://github.com/golang/go/issues/45990 is fixed
 	data.Schema["use_pss"] = &framework.FieldSchema{
 		Type:    framework.TypeBool,
 		Default: false,
@@ -95,6 +86,7 @@ func (b *backend) pathGenerateIntermediate(ctx context.Context, req *logical.Req
 		req:     req,
 		apiData: data,
 	}
+
 	parsedBundle, warnings, err := generateIntermediateCSR(sc, input, b.Backend.GetRandomReader())
 	if err != nil {
 		switch err.(type) {

builtin/logical/pki/path_manage_issuers.go

@@ -80,11 +80,8 @@ workaround in some compatibility scenarios
 with Active Directory Certificate Services.`,
 	}
 
-	// Signature bits isn't respected on intermediate generation, as this
-	// only impacts the CSR's internal signature and doesn't impact the
-	// signed certificate's bits (that's on the /sign-intermediate
-	// endpoints). Remove it from the list of fields to avoid confusion.
-	delete(ret.Fields, "signature_bits")
+	// At this time Go does not support signing CSRs using PSS signatures, see
+	// https://github.com/golang/go/issues/45990
 	delete(ret.Fields, "use_pss")
 
 	return ret

builtin/logical/pki/test_helpers.go

@@ -3,17 +3,12 @@ package pki
 import (
 	"context"
 	"crypto"
-	"crypto/ecdsa"
-	"crypto/ed25519"
 	"crypto/rand"
 	"crypto/rsa"
-	"crypto/sha256"
-	"crypto/sha512"
 	"crypto/x509"
 	"crypto/x509/pkix"
 	"encoding/pem"
 	"fmt"
-	"hash"
 	"io"
 	"strings"
 	"testing"
@@ -52,66 +47,12 @@ func mountPKIEndpoint(t testing.TB, client *api.Client, path string) {
 }
 
 // Signing helpers
-func requireSignedBy(t *testing.T, cert *x509.Certificate, key crypto.PublicKey) {
-	switch typedKey := key.(type) {
-	case *rsa.PublicKey:
-		requireRSASignedBy(t, cert, typedKey)
-	case *ecdsa.PublicKey:
-		requireECDSASignedBy(t, cert, typedKey)
-	case ed25519.PublicKey:
-		requireED25519SignedBy(t, cert, typedKey)
-	default:
-		require.Fail(t, "unknown public key type %#v", key)
+func requireSignedBy(t *testing.T, cert *x509.Certificate, signingCert *x509.Certificate) {
+	if err := cert.CheckSignatureFrom(signingCert); err != nil {
+		t.Fatalf("signature verification failed: %v", err)
 	}
 }
 
-func requireRSASignedBy(t *testing.T, cert *x509.Certificate, key *rsa.PublicKey) {
-	require.Contains(t, []x509.SignatureAlgorithm{x509.SHA256WithRSA, x509.SHA512WithRSA},
-		cert.SignatureAlgorithm, "only sha256 signatures supported")
-
-	var hasher hash.Hash
-	var hashAlgo crypto.Hash
-
-	switch cert.SignatureAlgorithm {
-	case x509.SHA256WithRSA:
-		hasher = sha256.New()
-		hashAlgo = crypto.SHA256
-	case x509.SHA512WithRSA:
-		hasher = sha512.New()
-		hashAlgo = crypto.SHA512
-	}
-
-	hasher.Write(cert.RawTBSCertificate)
-	hashData := hasher.Sum(nil)
-
-	err := rsa.VerifyPKCS1v15(key, hashAlgo, hashData, cert.Signature)
-	require.NoError(t, err, "the certificate was not signed by the expected public rsa key.")
-}
-
-func requireECDSASignedBy(t *testing.T, cert *x509.Certificate, key *ecdsa.PublicKey) {
-	require.Contains(t, []x509.SignatureAlgorithm{x509.ECDSAWithSHA256, x509.ECDSAWithSHA512},
-		cert.SignatureAlgorithm, "only ecdsa signatures supported")
-
-	var hasher hash.Hash
-	switch cert.SignatureAlgorithm {
-	case x509.ECDSAWithSHA256:
-		hasher = sha256.New()
-	case x509.ECDSAWithSHA512:
-		hasher = sha512.New()
-	}
-
-	hasher.Write(cert.RawTBSCertificate)
-	hashData := hasher.Sum(nil)
-
-	verify := ecdsa.VerifyASN1(key, hashData, cert.Signature)
-	require.True(t, verify, "the certificate was not signed by the expected public ecdsa key.")
-}
-
-func requireED25519SignedBy(t *testing.T, cert *x509.Certificate, key ed25519.PublicKey) {
-	require.Equal(t, x509.PureEd25519, cert.SignatureAlgorithm)
-	ed25519.Verify(key, cert.RawTBSCertificate, cert.Signature)
-}
-
 // Certificate helper
 func parseCert(t *testing.T, pemCert string) *x509.Certificate {
 	block, _ := pem.Decode([]byte(pemCert))

changelog/17388.txt

@@ -0,0 +1,3 @@
+```release-note:improvement
+secrets/pki: Add support to specify signature bits when generating CSRs through intermediate/generate apis
+```

sdk/helper/certutil/helpers.go

@@ -789,7 +789,7 @@ func CreateCertificateWithKeyGenerator(data *CreationBundle, randReader io.Reade
 	return createCertificate(data, randReader, keyGenerator)
 }
 
-// Set correct correct RSA sig algo
+// Set correct RSA sig algo
 func certTemplateSetSigAlgo(certTemplate *x509.Certificate, data *CreationBundle) {
 	if data.Params.UsePSS {
 		switch data.Params.SignatureBits {
@@ -812,6 +812,35 @@ func certTemplateSetSigAlgo(certTemplate *x509.Certificate, data *CreationBundle
 	}
 }
 
+// selectSignatureAlgorithmForRSA returns the proper x509.SignatureAlgorithm based on various properties set in the
+// Creation Bundle parameter. This method will default to a SHA256 signature algorithm if the requested signature
+// bits is not set/unknown.
+func selectSignatureAlgorithmForRSA(data *CreationBundle) x509.SignatureAlgorithm {
+	if data.Params.UsePSS {
+		switch data.Params.SignatureBits {
+		case 256:
+			return x509.SHA256WithRSAPSS
+		case 384:
+			return x509.SHA384WithRSAPSS
+		case 512:
+			return x509.SHA512WithRSAPSS
+		default:
+			return x509.SHA256WithRSAPSS
+		}
+	}
+
+	switch data.Params.SignatureBits {
+	case 256:
+		return x509.SHA256WithRSA
+	case 384:
+		return x509.SHA384WithRSA
+	case 512:
+		return x509.SHA512WithRSA
+	default:
+		return x509.SHA256WithRSA
+	}
+}
+
 func createCertificate(data *CreationBundle, randReader io.Reader, privateKeyGenerator KeyGenerator) (*ParsedCertBundle, error) {
 	var err error
 	result := &ParsedCertBundle{}
@@ -878,7 +907,11 @@ func createCertificate(data *CreationBundle, randReader io.Reader, privateKeyGen
 
 	var certBytes []byte
 	if data.SigningBundle != nil {
-		switch data.SigningBundle.PrivateKeyType {
+		privateKeyType := data.SigningBundle.PrivateKeyType
+		if privateKeyType == ManagedPrivateKey {
+			privateKeyType = GetPrivateKeyTypeFromSigner(data.SigningBundle.PrivateKey)
+		}
+		switch privateKeyType {
 		case RSAPrivateKey:
 			certTemplateSetSigAlgo(certTemplate, data)
 		case Ed25519PrivateKey:
@@ -1049,9 +1082,10 @@ func createCSR(data *CreationBundle, addBasicConstraints bool, randReader io.Rea
 
 	switch data.Params.KeyType {
 	case "rsa":
-		csrTemplate.SignatureAlgorithm = x509.SHA256WithRSA
+		// use specified RSA algorithm defaulting to the appropriate SHA256 RSA signature type
+		csrTemplate.SignatureAlgorithm = selectSignatureAlgorithmForRSA(data)
 	case "ec":
-		csrTemplate.SignatureAlgorithm = x509.ECDSAWithSHA256
+		csrTemplate.SignatureAlgorithm = selectSignatureAlgorithmForECDSA(result.PrivateKey.Public(), data.Params.SignatureBits)
 	case "ed25519":
 		csrTemplate.SignatureAlgorithm = x509.PureEd25519
 	}
@@ -1067,6 +1101,10 @@ func createCSR(data *CreationBundle, addBasicConstraints bool, randReader io.Rea
 		return nil, errutil.InternalError{Err: fmt.Sprintf("unable to parse created certificate: %v", err)}
 	}
 
+	if err = result.CSR.CheckSignature(); err != nil {
+		return nil, errors.New("failed signature validation for CSR")
+	}
+
 	return result, nil
 }
 

website/content/api-docs/secret/pki.mdx

@@ -1764,6 +1764,16 @@ generated depending on the `type` request parameter.
   name, or by identifier) to use for generating this request. Only suitable
   for `type=existing` requests.
 
+- `signature_bits` `(int: 0)` - Specifies the number of bits to use in
+  the signature algorithm; accepts 256 for SHA-2-256, 384 for SHA-2-384,
+  and 512 for SHA-2-512. Defaults to 0 to automatically detect based
+  on key length (SHA-2-256 for RSA keys, and matching the curve size
+  for NIST P-Curves).
+
+~> **Note**: ECDSA and Ed25519 issuers do not follow configuration of the
+`signature_bits` value; only RSA issuers will change signature types
+based on this parameter.
+
 - `exclude_cn_from_sans` `(bool: false)` - If true, the given `common_name` will
   not be included in DNS or Email Subject Alternate Names (as appropriate).
   Useful if the CN is not a hostname or email address, but is instead some

website/content/docs/secrets/pki/considerations.mdx

@@ -591,6 +591,12 @@ Additionally, some implementations allow rsaPSS OID certificates to contain
 restrictions on signature parameters allowed by this certificate, but Go and
 Vault do not support adding such restrictions.
 
+At this time Go lacks support for CSRs with the PSS signature algorithm. If
+using a GCP managed key with a RSA PSS algorithm as a backing CA key,
+attempting to generate a CSR will fail signature verification. In this case
+the CSR will need to be generated outside of Vault and the signed version
+can be imported into the mount.
+
 ## Issuer Subjects and CRLs
 
 As noted on several [GitHub issues](https://github.com/hashicorp/vault/issues/10176),