Fix PKI Weak Cryptographic Key Lenghths Warning (#12886)

* Modernize SSH key lengths

No default change was made in this commit; note that the code already
enforced a default of 2048 bits. ssh-keygen and Go's RSA key generation
allows for key sizes including 3072, 4096, 8192; update the values of
SSH key generation to match PKI's allowed RSA key sizes (from
certutil.ValidateKeyTypeLength(...)). We still allow the legacy SSH key
size of 1024; in the near future we should likely remove it.

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

* Ensure minimum of 2048-bit PKI RSA keys

While the stated path is a false-positive, verifying all paths is
non-trivial. We largely validate API call lengths using
certutil.ValidateKeyTypeLength(...), but ensuring no other path calls
certutil.generatePrivateKey(...) --- directly or indirectly --- is
non-trivial. Thus enforcing a minimum in this method sounds like a sane
compromise.

Resolves: https://github.com/hashicorp/vault/security/code-scanning/55

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
This commit is contained in:
Alexander Scheel 2021-10-26 09:30:09 -04:00 committed by GitHub
parent 14101f8664
commit 8833875b10
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 16 additions and 6 deletions

View file

@ -461,16 +461,15 @@ func (b *backend) pathRoleWrite(ctx context.Context, req *logical.Request, d *fr
return logical.ErrorResponse("missing admin username"), nil
}
// This defaults to 1024 and it can also be 2048 and 4096.
// This defaults to 2048, but it can also be 1024, 3072, 4096, or 8192.
// In the near future, we should disallow 1024-bit SSH keys.
keyBits := d.Get("key_bits").(int)
if keyBits != 0 && keyBits != 1024 && keyBits != 2048 && keyBits != 4096 {
return logical.ErrorResponse("invalid key_bits field"), nil
}
// If user has not set this field, default it to 2048
if keyBits == 0 {
keyBits = 2048
}
if keyBits != 1024 && keyBits != 2048 && keyBits != 3072 && keyBits != 4096 && keyBits != 8192 {
return logical.ErrorResponse("invalid key_bits field"), nil
}
// Store all the fields required by dynamic key type
roleEntry = sshRole{

View file

@ -222,6 +222,17 @@ func generatePrivateKey(keyType string, keyBits int, container ParsedPrivateKeyC
switch keyType {
case "rsa":
// XXX: there is a false-positive CodeQL path here around keyBits;
// because of a default zero value in the TypeDurationSecond and
// TypeSignedDurationSecond cases of schema.DefaultOrZero(), it
// thinks it is possible to end up with < 2048 bit RSA Key here.
// While this is true for SSH keys, it isn't true for PKI keys
// due to ValidateKeyTypeLength(...) below. While we could close
// the report as a false-positive, enforcing a minimum keyBits size
// here of 2048 would ensure no other paths exist.
if keyBits < 2048 {
return errutil.InternalError{Err: fmt.Sprintf("insecure bit length for RSA private key: %d", keyBits)}
}
privateKeyType = RSAPrivateKey
privateKey, err = rsa.GenerateKey(randReader, keyBits)
if err != nil {