From 67a239a8210e200c6bff5697d511830d606333b7 Mon Sep 17 00:00:00 2001
From: John Maguire <john.maguire@hashicorp.com>
Date: Wed, 28 Jun 2023 11:34:09 -0400
Subject: [PATCH] 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
---
 .changelog/17911.txt                          |  4 ++
 .../config_entry_inline_certificate.go        | 53 +++++++++++++++++--
 .../config_entry_inline_certificate_test.go   | 24 +++++++++
 3 files changed, 78 insertions(+), 3 deletions(-)
 create mode 100644 .changelog/17911.txt

diff --git a/.changelog/17911.txt b/.changelog/17911.txt
new file mode 100644
index 0000000000..a17cd6d2bd
--- /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.
+```
diff --git a/agent/structs/config_entry_inline_certificate.go b/agent/structs/config_entry_inline_certificate.go
index 7f2dc285e5..de11f2c950 100644
--- a/agent/structs/config_entry_inline_certificate.go
+++ b/agent/structs/config_entry_inline_certificate.go
@@ -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
@@ -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
 	}
 
@@ -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)
 	}
@@ -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 {
diff --git a/agent/structs/config_entry_inline_certificate_test.go b/agent/structs/config_entry_inline_certificate_test.go
index 407fc10299..8c77540131 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,