pki: When a role sets key_type to any ignore key_bits value when signing a csr (#16246)

* pki: When a role sets key_type to any ignore key_bits value when signing

 - Bypass the validation for the role's key_bits value when signing CSRs
   if the key_type is set to any. We still validate the key is at least
   2048 for RSA backed CSRs as we did in 1.9.x and lower.

builtin/logical/pki/backend_test.go

@@ -2486,6 +2486,40 @@ func TestBackend_SignIntermediate_AllowedPastCA(t *testing.T) {
 	}
 }
 
+func TestBackend_ConsulSignLeafWithLegacyRole(t *testing.T) {
+	// create the backend
+	b, s := createBackendWithStorage(t)
+
+	// generate root
+	data, err := CBWrite(b, s, "root/generate/internal", map[string]interface{}{
+		"ttl":         "40h",
+		"common_name": "myvault.com",
+	})
+	require.NoError(t, err, "failed generating internal root cert")
+	rootCaPem := data.Data["certificate"].(string)
+
+	// Create a signing role like Consul did with the default args prior to Vault 1.10
+	_, err = CBWrite(b, s, "roles/test", map[string]interface{}{
+		"allow_any_name":         true,
+		"allowed_serial_numbers": []string{"MySerialNumber"},
+		"key_type":               "any",
+		"key_bits":               "2048",
+		"signature_bits":         "256",
+	})
+	require.NoError(t, err, "failed creating legacy role")
+
+	_, csrPem := generateTestCsr(t, certutil.ECPrivateKey, 256)
+	data, err = CBWrite(b, s, "sign/test", map[string]interface{}{
+		"csr": csrPem,
+	})
+	require.NoError(t, err, "failed signing csr")
+	certAsPem := data.Data["certificate"].(string)
+
+	signedCert := parseCert(t, certAsPem)
+	rootCert := parseCert(t, rootCaPem)
+	requireSignedBy(t, signedCert, rootCert.PublicKey)
+}
+
 func TestBackend_SignSelfIssued(t *testing.T) {
 	// create the backend
 	b, storage := createBackendWithStorage(t)

builtin/logical/pki/cert_util.go

@@ -836,22 +836,24 @@ func signCert(b *backend,
 		// We update the value of KeyBits and SignatureBits here (from the
 		// role), using the specified key type. This allows us to convert
 		// the default value (0) for SignatureBits and KeyBits to a
-		// meaningful value. In the event KeyBits takes a zero value, we also
+		// meaningful value.
-		// update that to a new value.
 		//
-		// This is mandatory because on some roles, with KeyType any, we'll
+		// We ignore the role's original KeyBits value if the KeyType is any
-		// set a default SignatureBits to 0, but this will need to be updated
+		// as legacy (pre-1.10) roles had default values that made sense only
-		// in order to behave correctly during signing.
+		// for RSA keys (key_bits=2048) and the older code paths ignored the role value
-		roleBitsWasZero := data.role.KeyBits == 0
+		// set for KeyBits when KeyType was set to any. This also enforces the
-		if data.role.KeyBits, data.role.SignatureBits, err = certutil.ValidateDefaultOrValueKeyTypeSignatureLength(actualKeyType, data.role.KeyBits, data.role.SignatureBits); err != nil {
+		// docs saying when key_type=any, we only enforce our specified minimums
+		// for signing operations
+		if data.role.KeyBits, data.role.SignatureBits, err = certutil.ValidateDefaultOrValueKeyTypeSignatureLength(
+			actualKeyType, 0, data.role.SignatureBits); err != nil {
 			return nil, errutil.InternalError{Err: fmt.Sprintf("unknown internal error updating default values: %v", err)}
 		}
 
-		// We're using the KeyBits field as a minimum value, and P-224 is safe
+		// We're using the KeyBits field as a minimum value below, and P-224 is safe
 		// and a previously allowed value. However, the above call defaults
-		// to P-256 as that's a saner default than P-224 (w.r.t. generation).
+		// to P-256 as that's a saner default than P-224 (w.r.t. generation), so
-		// So, override our fake Role value if it was previously zero.
+		// override it here to allow 224 as the smallest size we permit.
-		if actualKeyType == "ec" && roleBitsWasZero {
+		if actualKeyType == "ec" {
 			data.role.KeyBits = 224
 		}
 	}

builtin/logical/pki/path_roles.go

@@ -587,7 +587,7 @@ func (b *backend) getRole(ctx context.Context, s logical.Storage, n string) (*ro
 		modified = true
 	}
 
-	// Ensure the role is valida fter updating.
+	// Ensure the role is valid after updating.
 	_, err = validateRole(b, &result, ctx, s)
 	if err != nil {
 		return nil, err

changelog/16246.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+secret/pki: Do not fail validation with a legacy key_bits default value and key_type=any when signing CSRs
+```

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

@@ -2338,7 +2338,7 @@ request is denied.
   generated private keys and the type of key expected for submitted CSRs.
   Currently, `rsa`, `ec`, and `ed25519` are supported, or when signing
   existing CSRs, `any` can be specified to allow keys of either type
-  and with any bit size (subject to >1024 bits for RSA keys).
+  and with any bit size (subject to >=2048 bits for RSA keys or >= 224 for EC keys).
 
 ~> **Note**: In FIPS 140-2 mode, the following algorithms are not certified
    and thus should not be used: `ed25519`.
@@ -2347,7 +2347,8 @@ request is denied.
   generated keys. Allowed values are 0 (universal default); with
   `key_type=rsa`, allowed values are: 2048 (default), 3072, or
   4096; with `key_type=ec`, allowed values are: 224, 256 (default),
-  384, or 521; ignored with `key_type=ed25519`.
+  384, or 521; ignored with `key_type=ed25519` or in signing operations
+  when `key_type=any`.
 
 - `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,