luxolus
/
open-vault
2
0
Fork 0
Code Issues 1 Pull Requests Releases Activity

Add universal default key_bits value for PKI endpoints (#13080) 

* Allow universal default for key_bits

This allows the key_bits field to take a universal default value, 0,
which, depending on key_type, gets adjusted appropriately into a
specific default value (rsa->2048, ec->256, ignored under ed25519).

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

* Handle universal default key size in certutil

Also move RSA < 2048 error message into certutil directly, instead of in
ca_util/path_roles.

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

* Add missing RSA key sizes to pki/backend_test.go

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

* Switch to returning updated values

When determining the default, don't pass in pointer types, but instead
return the newly updated value.

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

* Add changelog entry

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

* Re-add fix for ed25519 from #13254

Ed25519 internally specifies a hash length; by changing the default from
256 to 0, we fail validation in ValidateSignatureLength(...) unless we
specify the key algorithm.

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
This commit is contained in:
Alexander Scheel 2021-12-13 15:26:42 -05:00 committed by GitHub
parent 9674a75a4d
commit 31ff2be589
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 106 additions and 46 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
builtin/logical/pki/backend_test.go
View File

@ -1183,21 +1183,21 @@ func generateRoleSteps(t *testing.T, useCSRs bool) []logicaltest.TestStep {
}
getRandCsr := func(keyType string, errorOk bool, csrTemplate *x509.CertificateRequest) csrPlan {
rsaKeyBits := []int{2048, 4096}
rsaKeyBits := []int{2048, 3072, 4096}
ecKeyBits := []int{224, 256, 384, 521}
plan := csrPlan{errorOk: errorOk}
var testBitSize int
switch keyType {
case "rsa":
plan.roleKeyBits = rsaKeyBits[mathRand.Int()%2]
plan.roleKeyBits = rsaKeyBits[mathRand.Int()%len(rsaKeyBits)]
testBitSize = plan.roleKeyBits
// If we don't expect an error already, randomly choose a
// key size and expect an error if it's less than the role
// setting
if !keybitSizeRandOff && !errorOk {
testBitSize = rsaKeyBits[mathRand.Int()%2]
testBitSize = rsaKeyBits[mathRand.Int()%len(rsaKeyBits)]
}
if testBitSize < plan.roleKeyBits {
@ -1205,14 +1205,14 @@ func generateRoleSteps(t *testing.T, useCSRs bool) []logicaltest.TestStep {
}
case "ec":
plan.roleKeyBits = ecKeyBits[mathRand.Int()%4]
plan.roleKeyBits = ecKeyBits[mathRand.Int()%len(ecKeyBits)]
testBitSize = plan.roleKeyBits
// If we don't expect an error already, randomly choose a
// key size and expect an error if it's less than the role
// setting
if !keybitSizeRandOff && !errorOk {
testBitSize = ecKeyBits[mathRand.Int()%4]
testBitSize = ecKeyBits[mathRand.Int()%len(ecKeyBits)]
}
if testBitSize < plan.roleKeyBits {

8
builtin/logical/pki/ca_util.go
View File

@ -50,12 +50,8 @@ func (b *backend) getGenerationParams(
PostalCode: data.Get("postal_code").([]string),
}
if role.KeyType == "rsa" && role.KeyBits < 2048 {
errorResp = logical.ErrorResponse("RSA keys < 2048 bits are unsafe and not supported")
return
}
if err := certutil.ValidateKeyTypeSignatureLength(role.KeyType, role.KeyBits, &role.SignatureBits); err != nil {
var err error
if role.KeyBits, role.SignatureBits, err = certutil.ValidateDefaultOrValueKeyTypeSignatureLength(role.KeyType, role.KeyBits, role.SignatureBits); err != nil {
errorResp = logical.ErrorResponse(err.Error())
}

11
builtin/logical/pki/fields.go
View File

@ -250,12 +250,13 @@ the private key!`,
fields["key_bits"] = &framework.FieldSchema{
Type: framework.TypeInt,
Default: 2048,
Description: `The number of bits to use. You will almost
certainly want to change this if you adjust
the key_type.`,
Default: 0,
Description: `The number of bits to use. Allowed values are
0 (universal default); with rsa key_type: 2048 (default), 3072, or
4096; with ec key_type: 224, 256 (default), 384, or 521; ignored with
ed25519.`,
DisplayAttrs: &framework.DisplayAttributes{
Value: 2048,
Value: 0,
},
}

15
builtin/logical/pki/path_roles.go
View File

@ -199,10 +199,11 @@ protection use. Defaults to false.`,
"key_bits": {
Type: framework.TypeInt,
Default: 2048,
Description: `The number of bits to use. You will almost
certainly want to change this if you adjust
the key_type.`,
Default: 0,
Description: `The number of bits to use. Allowed values are
0 (universal default); with rsa key_type: 2048 (default), 3072, or
4096; with ec key_type: 224, 256 (default), 384, or 521; ignored with
ed25519.`,
},
"signature_bits": &framework.FieldSchema{
@ -615,17 +616,13 @@ func (b *backend) pathRoleCreate(ctx context.Context, req *logical.Request, data
*entry.GenerateLease = data.Get("generate_lease").(bool)
}
if entry.KeyType == "rsa" && entry.KeyBits < 2048 {
return logical.ErrorResponse("RSA keys < 2048 bits are unsafe and not supported"), nil
}
if entry.MaxTTL > 0 && entry.TTL > entry.MaxTTL {
return logical.ErrorResponse(
`"ttl" value must be less than "max_ttl" value`,
), nil
}
if err := certutil.ValidateKeyTypeSignatureLength(entry.KeyType, entry.KeyBits, &entry.SignatureBits); err != nil {
if entry.KeyBits, entry.SignatureBits, err = certutil.ValidateDefaultOrValueKeyTypeSignatureLength(entry.KeyType, entry.KeyBits, entry.SignatureBits); err != nil {
return logical.ErrorResponse(err.Error()), nil
}

3
changelog/13080.txt Normal file
View File

@ -0,0 +1,3 @@
```release-note:bug
secrets/pki: Default value for key_bits changed to 0, enabling key_type=ec key generation with default value
```

105
sdk/helper/certutil/helpers.go
View File

@ -33,6 +33,14 @@ import (
cbasn1 "golang.org/x/crypto/cryptobyte/asn1"
)
const rsaMinimumSecureKeySize = 2048
// Mapping of key types to default key lengths
var defaultAlgorithmKeyBits = map[string]int {
"rsa": 2048,
"ec": 256,
}
// Mapping of NIST P-Curve's key length to expected signature bits.
var expectedNISTPCurveHashBits = map[int]int{
224: 256,
@ -533,14 +541,27 @@ func StringToOid(in string) (asn1.ObjectIdentifier, error) {
return asn1.ObjectIdentifier(ret), nil
}
// 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
// Returns default key bits for the specified key type, or the present value
// if keyBits is non-zero.
func DefaultOrValueKeyBits(keyType string, keyBits int) (int, error) {
if keyBits == 0 {
newValue, present := defaultAlgorithmKeyBits[keyType]
if present {
keyBits = newValue
} /* else {
// We cannot return an error here as ed25519 (and potentially ed448
// in the future) aren't in defaultAlgorithmKeyBits -- the value of
// the keyBits parameter is ignored under that algorithm.
} */
}
return keyBits, nil
}
// Returns default signature hash bit length for the specified key type and
// bits, or the present value if hashBits is non-zero. Returns an error under
// certain internal circumstances.
func DefaultOrValueHashBits(keyType string, keyBits int, hashBits int) (int, error) {
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
@ -548,35 +569,72 @@ func ValidateKeyTypeSignatureLength(keyType string, keyBits int, hashBits *int)
// 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
if expectedHashBits != hashBits && hashBits != 0 {
return hashBits, 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" {
} else if keyType == "rsa" && hashBits == 0 {
// To match previous behavior (and ignoring NIST's recommendations for
// hash size to align with RSA key sizes), default to SHA-2-256.
hashBits = 256
} else if keyType == "ed25519" || keyType == "ed448" {
// 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.
return nil
// certificate signing and we must
return 0, nil
}
return hashBits, nil
}
// Validates that the combination of keyType, keyBits, and hashBits are
// valid together; replaces individual calls to ValidateSignatureLength and
// ValidateKeyTypeLength. Also updates the value of keyBits and hashBits on
// return.
func ValidateDefaultOrValueKeyTypeSignatureLength(keyType string, keyBits int, hashBits int) (int, int, error) {
var err error
if keyBits, err = DefaultOrValueKeyBits(keyType, keyBits); err != nil {
return keyBits, hashBits, err
}
if err = ValidateKeyTypeLength(keyType, keyBits); err != nil {
return keyBits, hashBits, err
}
if hashBits, err = DefaultOrValueHashBits(keyType, keyBits, hashBits); err != nil {
return keyBits, hashBits, err
}
// 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
if err = ValidateSignatureLength(keyType, hashBits); err != nil {
return keyBits, hashBits, err
}
return nil
return keyBits, hashBits, 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 {
func ValidateSignatureLength(keyType string, hashBits int) error {
if keyType == "ed25519" || keyType == "ed448" {
// ed25519 and ed448 include built-in hashing and is not externally
// configurable. There are three modes for each of these schemes:
//
// 1. Built-in hash (default, used in TLS, x509).
// 2. Double hash (notably used in some block-chain implementations,
// but largely regarded as a specialized use case with security
// concerns).
// 3. No hash (bring your own hash function, less commonly used).
//
// In all cases, we won't have a hash algorithm to validate here, so
// return nil.
return nil
}
switch hashBits {
case 256:
case 384:
@ -584,12 +642,17 @@ func ValidateSignatureLength(hashBits int) error {
default:
return fmt.Errorf("unsupported hash signature algorithm: %d", hashBits)
}
return nil
}
func ValidateKeyTypeLength(keyType string, keyBits int) error {
switch keyType {
case "rsa":
if keyBits < rsaMinimumSecureKeySize {
return fmt.Errorf("RSA keys < %d bits are unsafe and not supported: got %d", rsaMinimumSecureKeySize, keyBits)
}
switch keyBits {
case 2048:
case 3072: