Skip to content

Commit

Permalink
Backport of Ensure RSA keys are at least 2048 bits in length into rel…
Browse files Browse the repository at this point in the history
…ease/1.16.x (#17935)

* backport of commit 93ccfe4

* Ensure RSA keys are at least 2048 bits in length (#17911)

* 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

---------

Co-authored-by: jm96441n <john.maguire@hashicorp.com>
  • Loading branch information
hc-github-team-consul-core and jm96441n authored Jun 28, 2023
1 parent 7b53c99 commit 61c5d9e
Show file tree
Hide file tree
Showing 3 changed files with 77 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.
```
52 changes: 49 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,23 +44,32 @@ 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
}

privateKeyBlock, _ := pem.Decode([]byte(e.PrivateKey))
if privateKeyBlock == nil {
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 +95,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 61c5d9e

Please sign in to comment.