Restrict ECDSA/NIST P-Curve hash function sizes for cert signing (#12872

)

* Restrict ECDSA signatures with NIST P-Curve hashes

When using an ECDSA signature with a NIST P-Curve, we should follow
recommendations from BIS (Section 4.2) and Mozilla's root store policy
(section 5.1.2) to ensure that arbitrary selection of signature_bits
does not exceed what the curve is capable of signing.

Related: #11245

Signed-off-by: Alexander Scheel <[email protected]>

* Switch to certutil.ValidateKeyTypeSignatureLength(...)

Replaces previous calls to certutil.ValidateKeyTypeLength(...) and
certutil.ValidateSignatureLength(...) with a single call, allowing for
curve<->hash validation.

Signed-off-by: Alexander Scheel <[email protected]>

* Switch to autodetection of signature_bits

This enables detection of whether the caller manually specified a value
for signature_bits or not; when not manually specified, we can provision
a value that complies with new NIST P-Curve policy.

Signed-off-by: Alexander Scheel <[email protected]>

* Select hash function length automatically

Due to our change in behavior (to default to -1 as the value to
signature_bits to allow for automatic hash selection), switch
ValidateKeyTypeSignatureLength(...) to accept a pointer to hashBits and
provision it with valid default values.

Signed-off-by: Alexander Scheel <[email protected]>

* Prevent invalid Curve size lookups

Signed-off-by: Alexander Scheel <[email protected]>

* Switch from -1 to 0 as default SignatureBits

Signed-off-by: Alexander Scheel <[email protected]>

* Add changelog entry

Signed-off-by: Alexander Scheel <[email protected]>

builtin/logical/pki/backend_test.go

@@ -827,7 +827,6 @@ func generateRoleSteps(t *testing.T, useCSRs bool) []logicaltest.TestStep {
 		KeyType:       "rsa",
 		KeyBits:       2048,
 		RequireCN:     true,
-		SignatureBits: 256,
 	}
 	issueVals := certutil.IssueData{}
 	ret := []logicaltest.TestStep{}

builtin/logical/pki/ca_util.go

@@ -55,14 +55,9 @@ func (b *backend) getGenerationParams(
 		return
 	}
 
-	if err := certutil.ValidateKeyTypeLength(role.KeyType, role.KeyBits); err != nil {
+	if err := certutil.ValidateKeyTypeSignatureLength(role.KeyType, role.KeyBits, &role.SignatureBits); err != nil {
 		errorResp = logical.ErrorResponse(err.Error())
 	}
 
-	if err := certutil.ValidateSignatureLength(role.SignatureBits); err != nil {
-		errorResp = logical.ErrorResponse(err.Error())
-		return
-	}
-
 	return
 }

builtin/logical/pki/fields.go

@@ -261,13 +261,13 @@ the key_type.`,
 
 	fields["signature_bits"] = &framework.FieldSchema{
 		Type:    framework.TypeInt,
-		Default: 256,
+		Default: 0,
 		Description: `The number of bits to use in the signature
-algorithm. Defaults to 256 for SHA256.
-Set to 384 for SHA384 and 512 for SHA512.
-`,
+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).`,
 		DisplayAttrs: &framework.DisplayAttributes{
-			Value: 256,
+			Value: 0,
 		},
 	}
 

builtin/logical/pki/path_roles.go

@@ -207,10 +207,11 @@ the key_type.`,
 
 			"signature_bits": &framework.FieldSchema{
 				Type:    framework.TypeInt,
-				Default: 256,
+				Default: 0,
 				Description: `The number of bits to use in the signature
-algorithm. Defaults to 256 for SHA256.
-Set to 384 for SHA384 and 512 for SHA512.`,
+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).`,
 			},
 
 			"key_usage": &framework.FieldSchema{
@@ -624,11 +625,7 @@ func (b *backend) pathRoleCreate(ctx context.Context, req *logical.Request, data
 		), nil
 	}
 
-	if err := certutil.ValidateKeyTypeLength(entry.KeyType, entry.KeyBits); err != nil {
-		return logical.ErrorResponse(err.Error()), nil
-	}
-
-	if err := certutil.ValidateSignatureLength(entry.SignatureBits); err != nil {
+	if err := certutil.ValidateKeyTypeSignatureLength(entry.KeyType, entry.KeyBits, &entry.SignatureBits); err != nil {
 		return logical.ErrorResponse(err.Error()), nil
 	}
 

changelog/12872.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+secrets/pki: Fixes around NIST P-curve signature hash length, default value for signature_bits changed to 0.
+```

sdk/helper/certutil/helpers.go

@@ -33,6 +33,14 @@ import (
 	cbasn1 "golang.org/x/crypto/cryptobyte/asn1"
 )
 
+// Mapping of NIST P-Curve's key length to expected signature bits.
+var expectedNISTPCurveHashBits = map[int]int{
+	224: 256,
+	256: 256,
+	384: 384,
+	521: 512,
+}
+
 // GetHexFormatted returns the byte buffer formatted in hex with
 // the specified separator between bytes.
 func GetHexFormatted(buf []byte, sep string) string {
@@ -525,13 +533,55 @@ func StringToOid(in string) (asn1.ObjectIdentifier, error) {
 	return asn1.ObjectIdentifier(ret), nil
 }
 
-func ValidateSignatureLength(keyBits int) error {
-	switch keyBits {
+// Validates that the combination of keyType, keyBits, and hashBits are
+// valid together; replaces individual calls to ValidateSignatureLength and
+// ValidateKeyTypeLength.
+func ValidateKeyTypeSignatureLength(keyType string, keyBits int, hashBits *int) error {
+	if err := ValidateKeyTypeLength(keyType, keyBits); err != nil {
+		return err
+	}
+
+	if keyType == "ec" {
+		// To comply with BSI recommendations Section 4.2 and Mozilla root
+		// store policy section 5.1.2, enforce that NIST P-curves use a hash
+		// length corresponding to curve length. Note that ed25519 does not
+		// the "ec" key type.
+		expectedHashBits := expectedNISTPCurveHashBits[keyBits]
+
+		if expectedHashBits != *hashBits && *hashBits != 0 {
+			return fmt.Errorf("unsupported signature hash algorithm length (%d) for NIST P-%d", *hashBits, keyBits)
+		} else if *hashBits == 0 {
+			*hashBits = expectedHashBits
+		}
+	} else if keyType == "rsa" && *hashBits == 0 {
+		// To match previous behavior (and ignoring recommendations of hash
+		// size to match RSA key sizes), default to SHA-2-256.
+		*hashBits = 256
+	} /* else if keyType == "ed25519" {
+		// No-op; ed25519 and ed448 internally specify their own hash and
+		// we do not need to select one. Double hashing isn't supported in
+		// certificate signing.
+	} */
+
+	// Note that this check must come after we've selected a value for
+	// hashBits above, in the event it was left as the default, but we
+	// were allowed to update it.
+	if err := ValidateSignatureLength(*hashBits); err != nil || *hashBits == 0 {
+		return err
+	}
+
+	return nil
+}
+
+// Validates that the length of the hash (in bits) used in the signature
+// calculation is a known, approved value.
+func ValidateSignatureLength(hashBits int) error {
+	switch hashBits {
 	case 256:
 	case 384:
 	case 512:
 	default:
-		return fmt.Errorf("unsupported signature algorithm: %d", keyBits)
+		return fmt.Errorf("unsupported hash signature algorithm: %d", hashBits)
 	}
 	return nil
 }
@@ -548,12 +598,8 @@ func ValidateKeyTypeLength(keyType string, keyBits int) error {
 			return fmt.Errorf("unsupported bit length for RSA key: %d", keyBits)
 		}
 	case "ec":
-		switch keyBits {
-		case 224:
-		case 256:
-		case 384:
-		case 521:
-		default:
+		_, present := expectedNISTPCurveHashBits[keyBits]
+		if !present {
 			return fmt.Errorf("unsupported bit length for EC key: %d", keyBits)
 		}
 	case "any", "ed25519":