From 93ccfe4c1195ba0ab2d12443f25d9cf29e9e4f0c Mon Sep 17 00:00:00 2001 From: jm96441n Date: Tue, 27 Jun 2023 16:03:04 -0400 Subject: [PATCH 1/6] Ensure RSA keys are at least 2048 bits in length --- .../config_entry_inline_certificate.go | 24 ++++++++++++++++--- .../config_entry_inline_certificate_test.go | 24 +++++++++++++++++++ 2 files changed, 45 insertions(+), 3 deletions(-) diff --git a/agent/structs/config_entry_inline_certificate.go b/agent/structs/config_entry_inline_certificate.go index 7f2dc285e5ab..90482e36269d 100644 --- a/agent/structs/config_entry_inline_certificate.go +++ b/agent/structs/config_entry_inline_certificate.go @@ -10,8 +10,9 @@ import ( "errors" "fmt" - "github.com/hashicorp/consul/acl" "github.com/miekg/dns" + + "github.com/hashicorp/consul/acl" ) // InlineCertificateConfigEntry manages the configuration for an inline certificate @@ -42,8 +43,13 @@ func (e *InlineCertificateConfigEntry) GetEnterpriseMeta() *acl.EnterpriseMeta { } func (e *InlineCertificateConfigEntry) GetRaftIndex() *RaftIndex { return &e.RaftIndex } +// Envoy will silently reject any 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 } @@ -52,13 +58,25 @@ func (e *InlineCertificateConfigEntry) Validate() error { return errors.New("failed to parse private key PEM") } + if privateKeyBlock.Type == "RSA PRIVATE KEY" { + key, err := x509.ParsePKCS1PrivateKey(privateKeyBlock.Bytes) + if err != nil { + return err + } + + // ensure private key is of the correct length + if key.N.BitLen() < MinKeyLength { + return errors.New("key length must be at least 2048 bits") + } + } + 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) } diff --git a/agent/structs/config_entry_inline_certificate_test.go b/agent/structs/config_entry_inline_certificate_test.go index 407fc102997b..8c7754013106 100644 --- a/agent/structs/config_entry_inline_certificate_test.go +++ b/agent/structs/config_entry_inline_certificate_test.go @@ -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) { @@ -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, From f03248a0425494fcf028a8c90d34a6bd753ebac4 Mon Sep 17 00:00:00 2001 From: jm96441n Date: Tue, 27 Jun 2023 16:14:18 -0400 Subject: [PATCH 2/6] Add changelog --- .changelog/17911.txt | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 .changelog/17911.txt diff --git a/.changelog/17911.txt b/.changelog/17911.txt new file mode 100644 index 000000000000..a17cd6d2bdf2 --- /dev/null +++ b/.changelog/17911.txt @@ -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. +``` From 8ba9e20e07ccd91afa4ea09ec093513afd6e8697 Mon Sep 17 00:00:00 2001 From: jm96441n Date: Tue, 27 Jun 2023 17:11:54 -0400 Subject: [PATCH 3/6] update key length check for FIPS compliance --- .../config_entry_inline_certificate.go | 49 ++++++++++++++----- 1 file changed, 37 insertions(+), 12 deletions(-) diff --git a/agent/structs/config_entry_inline_certificate.go b/agent/structs/config_entry_inline_certificate.go index 90482e36269d..4900c3874a18 100644 --- a/agent/structs/config_entry_inline_certificate.go +++ b/agent/structs/config_entry_inline_certificate.go @@ -13,6 +13,7 @@ import ( "github.com/miekg/dns" "github.com/hashicorp/consul/acl" + "github.com/hashicorp/consul/version" ) // InlineCertificateConfigEntry manages the configuration for an inline certificate @@ -43,7 +44,7 @@ func (e *InlineCertificateConfigEntry) GetEnterpriseMeta() *acl.EnterpriseMeta { } func (e *InlineCertificateConfigEntry) GetRaftIndex() *RaftIndex { return &e.RaftIndex } -// Envoy will silently reject any keys that are less than 2048 bytes long +// 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 @@ -58,17 +59,7 @@ func (e *InlineCertificateConfigEntry) Validate() error { return errors.New("failed to parse private key PEM") } - if privateKeyBlock.Type == "RSA PRIVATE KEY" { - key, err := x509.ParsePKCS1PrivateKey(privateKeyBlock.Bytes) - if err != nil { - return err - } - - // ensure private key is of the correct length - if key.N.BitLen() < MinKeyLength { - return errors.New("key length must be at least 2048 bits") - } - } + err := validateKeyLength(privateKeyBlock) certificateBlock, _ := pem.Decode([]byte(e.Certificate)) if certificateBlock == nil { @@ -102,6 +93,40 @@ func (e *InlineCertificateConfigEntry) Validate() error { return nil } +func validateKeyLength(privateKeyBlock *pem.Block) error { + if privateKeyBlock.Type != "RSA PRIVATE KEY" { + return nil + } + + lenCheckFn := nonFipsLenCheck + + if version.IsFIPS() { + lenCheckFn = fipsLenCheck + } + key, err := x509.ParsePKCS1PrivateKey(privateKeyBlock.Bytes) + if err != nil { + return err + } + + return lenCheckFn(key.N.BitLen()) +} + +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 { From 31c65cb38c45f0bb54fd0a3836ac742b7e570809 Mon Sep 17 00:00:00 2001 From: jm96441n Date: Tue, 27 Jun 2023 17:29:48 -0400 Subject: [PATCH 4/6] Fix no new variables error and failing to return when error exists from validating --- agent/structs/config_entry_inline_certificate.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/agent/structs/config_entry_inline_certificate.go b/agent/structs/config_entry_inline_certificate.go index 4900c3874a18..f701e4497407 100644 --- a/agent/structs/config_entry_inline_certificate.go +++ b/agent/structs/config_entry_inline_certificate.go @@ -59,7 +59,10 @@ func (e *InlineCertificateConfigEntry) Validate() error { return errors.New("failed to parse private key PEM") } - err := validateKeyLength(privateKeyBlock) + err = validateKeyLength(privateKeyBlock) + if err != nil { + return err + } certificateBlock, _ := pem.Decode([]byte(e.Certificate)) if certificateBlock == nil { From 1854949799a0aae7a54f6433b1890dd10699c071 Mon Sep 17 00:00:00 2001 From: jm96441n Date: Wed, 28 Jun 2023 10:48:14 -0400 Subject: [PATCH 5/6] clean up code for better readability --- agent/structs/config_entry_inline_certificate.go | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/agent/structs/config_entry_inline_certificate.go b/agent/structs/config_entry_inline_certificate.go index f701e4497407..5cace9965b9f 100644 --- a/agent/structs/config_entry_inline_certificate.go +++ b/agent/structs/config_entry_inline_certificate.go @@ -101,17 +101,18 @@ func validateKeyLength(privateKeyBlock *pem.Block) error { return nil } - lenCheckFn := nonFipsLenCheck - - if version.IsFIPS() { - lenCheckFn = fipsLenCheck - } key, err := x509.ParsePKCS1PrivateKey(privateKeyBlock.Bytes) if err != nil { return err } - return lenCheckFn(key.N.BitLen()) + keyBitLen := key.N.BitLen() + + if version.IsFIPS() { + fipsLenCheck(keyBitLen) + } + + return nonFipsLenCheck(keyBitLen) } func nonFipsLenCheck(keyLen int) error { From c9599af084474d0d04bada8dbffeded4065d5e9a Mon Sep 17 00:00:00 2001 From: jm96441n Date: Wed, 28 Jun 2023 10:57:50 -0400 Subject: [PATCH 6/6] actually return value --- agent/structs/config_entry_inline_certificate.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/agent/structs/config_entry_inline_certificate.go b/agent/structs/config_entry_inline_certificate.go index 5cace9965b9f..de11f2c95045 100644 --- a/agent/structs/config_entry_inline_certificate.go +++ b/agent/structs/config_entry_inline_certificate.go @@ -109,7 +109,7 @@ func validateKeyLength(privateKeyBlock *pem.Block) error { keyBitLen := key.N.BitLen() if version.IsFIPS() { - fipsLenCheck(keyBitLen) + return fipsLenCheck(keyBitLen) } return nonFipsLenCheck(keyBitLen)