Skip to content

Commit

Permalink
Ensure RSA keys are at least 2048 bits in length (#17911)
Browse files Browse the repository at this point in the history
* Ensure RSA keys are at least 2048 bits in length

* Add changelog

* update key length check for FIPS compliance

* Fix no new variables error and failing to return when error exists from
validating

* clean up code for better readability

* actually return value
  • Loading branch information
jm96441n authored Jun 28, 2023
1 parent 7dbba6c commit 67a239a
Show file tree
Hide file tree
Showing 3 changed files with 78 additions and 3 deletions.
4 changes: 4 additions & 0 deletions .changelog/17911.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
```release-note:bug
gateway: Fixes a bug where envoy would silently reject RSA keys that are smaller than 2048 bits,
we now reject those earlier in the process when we validate the certificate.
```
53 changes: 50 additions & 3 deletions agent/structs/config_entry_inline_certificate.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,10 @@ import (
"errors"
"fmt"

"github.com/hashicorp/consul/acl"
"github.com/miekg/dns"

"github.com/hashicorp/consul/acl"
"github.com/hashicorp/consul/version"
)

// InlineCertificateConfigEntry manages the configuration for an inline certificate
Expand Down Expand Up @@ -42,8 +44,13 @@ func (e *InlineCertificateConfigEntry) GetEnterpriseMeta() *acl.EnterpriseMeta {
}
func (e *InlineCertificateConfigEntry) GetRaftIndex() *RaftIndex { return &e.RaftIndex }

// Envoy will silently reject any RSA keys that are less than 2048 bytes long
// https://github.com/envoyproxy/envoy/blob/main/source/extensions/transport_sockets/tls/context_impl.cc#L238
const MinKeyLength = 2048

func (e *InlineCertificateConfigEntry) Validate() error {
if err := validateConfigEntryMeta(e.Meta); err != nil {
err := validateConfigEntryMeta(e.Meta)
if err != nil {
return err
}

Expand All @@ -52,13 +59,18 @@ func (e *InlineCertificateConfigEntry) Validate() error {
return errors.New("failed to parse private key PEM")
}

err = validateKeyLength(privateKeyBlock)
if err != nil {
return err
}

certificateBlock, _ := pem.Decode([]byte(e.Certificate))
if certificateBlock == nil {
return errors.New("failed to parse certificate PEM")
}

// make sure we have a valid x509 certificate
_, err := x509.ParseCertificate(certificateBlock.Bytes)
_, err = x509.ParseCertificate(certificateBlock.Bytes)
if err != nil {
return fmt.Errorf("failed to parse certificate: %w", err)
}
Expand All @@ -84,6 +96,41 @@ func (e *InlineCertificateConfigEntry) Validate() error {
return nil
}

func validateKeyLength(privateKeyBlock *pem.Block) error {
if privateKeyBlock.Type != "RSA PRIVATE KEY" {
return nil
}

key, err := x509.ParsePKCS1PrivateKey(privateKeyBlock.Bytes)
if err != nil {
return err
}

keyBitLen := key.N.BitLen()

if version.IsFIPS() {
return fipsLenCheck(keyBitLen)
}

return nonFipsLenCheck(keyBitLen)
}

func nonFipsLenCheck(keyLen int) error {
// ensure private key is of the correct length
if keyLen < MinKeyLength {
return errors.New("key length must be at least 2048 bits")
}

return nil
}

func fipsLenCheck(keyLen int) error {
if keyLen != 2048 && keyLen != 3072 && keyLen != 4096 {
return errors.New("key length invalid: only RSA lengths of 2048, 3072, and 4096 are allowed in FIPS mode")
}
return nil
}

func (e *InlineCertificateConfigEntry) Hosts() ([]string, error) {
certificateBlock, _ := pem.Decode([]byte(e.Certificate))
if certificateBlock == nil {
Expand Down
24 changes: 24 additions & 0 deletions agent/structs/config_entry_inline_certificate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,21 @@ NtyHRuD+KYRmjXtyX1yHNqfGN3vOQmwavHq2R8wHYuBSc6LAHHV9vG+j0VsgMELO
qwxn8SmLkSKbf2+MsQVzLCXXN5u+D8Yv+4py+oKP4EQ5aFZuDEx+r/G/31rTthww
AAJAMaoXmoYVdgXV+CPuBb2M4XCpuzLu3bcA2PXm5ipSyIgntMKwXV7r
-----END CERTIFICATE-----`
tooShortPrivateKey = `-----BEGIN RSA PRIVATE KEY-----
MIICXAIBAAKBgQCtmK1VjmXJ7vm4CZkkOSjc+kjGNMlyce5rXxwlDRz9LcGGc3Tg
kwUJesyBpDtxLLVHXQIPr5mWYbX/W/ezQ9sntxrATbDek8pBgoOlARebwkD2ivVW
BWfVhlryVihWlXApKiJ2n3i0m+OVtdrceC9Bv2hEMhYVOwzxtb3O0YFkbwIDAQAB
AoGAIxgnipFUEKPIRiVimUkY8ruCdNd9Fi7kNT6wEOl6v9A9PHIg4bm3Hfh+WYMb
JUEVkMzDuuoUEavFQE+WXt5L8oE1lEBmN2++FQsvllN+MRBTRg2sfw4mUWDI6S4r
h8+XNTzTIg2sUd2J3o2qNmQoOheYb+iuYDj76IFoEdwwZ0kCQQDYKKs5HAbnrLj1
UrOp8TyHdFf0YNw5tGdbNTbffq4rlBD6SW70+Sj624i2UqdnYwRiWzdXv3zN08aI
Vfoh2cGlAkEAzZe5B6BhiX/PcIYutMtuT3K+mysFNlowrutXWoQOpR7gGAkgEt6e
oCDgx1QJRjsp6NFQxKc6l034Hzs17gqJgwJAcu9U873aUg9+HTuHOoKB28haCCAE
mU46cr3d2oKCW7uUN3EaZXmid5iJneBfENMOfrnfuHGiC9NiShXlNWCS3QJAO5Ne
w83+1ahaxUGs4SkeExmuECrcPM7P0rBRxOIFmGWlDHIAgFdQYhiE6l34vghA8b1O
CV5oRRYL84jl7M/S3wJBALDfL5YXcc8P6scLJJ1biqhLYppvGN5CUwbsJsluvHCW
XCTVIbPOaS42A0xUfpoiTcdbNSFRvdCzPR5nsGy8Y7g=
-----END RSA PRIVATE KEY-----`
)

func TestInlineCertificate(t *testing.T) {
Expand All @@ -140,6 +155,15 @@ func TestInlineCertificate(t *testing.T) {
},
validateErr: "failed to parse certificate PEM",
},
"invalid private key length": {
entry: &InlineCertificateConfigEntry{
Kind: InlineCertificate,
Name: "cert-two",
PrivateKey: tooShortPrivateKey,
Certificate: "foo",
},
validateErr: "key length must be at least 2048 bits",
},
"mismatched certificate": {
entry: &InlineCertificateConfigEntry{
Kind: InlineCertificate,
Expand Down

0 comments on commit 67a239a

Please sign in to comment.