APIGW: Validate length of RSA Keys

.changelog/2478.txt

@@ -0,0 +1,5 @@
+```release-note:bug
+api-gateway: fixes bug where envoy will silently reject RSA keys less than 2048 bits in length when not in FIPS mode, and
+will reject keys that are not 2048, 3072, or 4096 bits in length in FIPS mode. We now validate
+and reject invalid certs earlier.
+```

control-plane/api-gateway/binding/binder_test.go

@@ -15,7 +15,6 @@ import (
 
 	logrtest "github.com/go-logr/logr/testing"
 	"github.com/google/go-cmp/cmp"
-	"github.com/hashicorp/consul/api"
 	"github.com/stretchr/testify/require"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -26,6 +25,7 @@ import (
 
 	"github.com/hashicorp/consul-k8s/control-plane/api-gateway/common"
 	"github.com/hashicorp/consul-k8s/control-plane/api/v1alpha1"
+	"github.com/hashicorp/consul/api"
 )
 
 func init() {
@@ -2330,8 +2330,16 @@ func controlledBinder(config BinderConfig) BinderConfig {
 	return config
 }
 
+func generateInvalidTestCertificate(t *testing.T, namespace, name string) (*api.InlineCertificateConfigEntry, corev1.Secret) {
+	return generateTestCertificateByKeyLen(t, namespace, name, common.MinKeyLength-1)
+}
+
 func generateTestCertificate(t *testing.T, namespace, name string) (*api.InlineCertificateConfigEntry, corev1.Secret) {
-	privateKey, err := rsa.GenerateKey(rand.Reader, 1024)
+	return generateTestCertificateByKeyLen(t, namespace, name, common.MinKeyLength)
+}
+
+func generateTestCertificateByKeyLen(t *testing.T, namespace, name string, keyLen int) (*api.InlineCertificateConfigEntry, corev1.Secret) {
+	privateKey, err := rsa.GenerateKey(rand.Reader, keyLen)
 	require.NoError(t, err)
 
 	usage := x509.KeyUsageCertSign

control-plane/api-gateway/binding/result.go

@@ -218,15 +218,17 @@ var (
 	// to the ListenerConditionReason given in the spec. If a reason is overloaded and can
 	// be used with two different types of things (i.e. something is not found or it's not supported)
 	// then we distinguish those two usages with errListener*_Usage.
-	errListenerUnsupportedProtocol                = errors.New("listener protocol is unsupported")
-	errListenerPortUnavailable                    = errors.New("listener port is unavailable")
-	errListenerHostnameConflict                   = errors.New("listener hostname conflicts with another listener")
-	errListenerProtocolConflict                   = errors.New("listener protocol conflicts with another listener")
-	errListenerInvalidCertificateRef_NotFound     = errors.New("certificate not found")
-	errListenerInvalidCertificateRef_NotSupported = errors.New("certificate type is not supported")
-	errListenerInvalidCertificateRef_InvalidData  = errors.New("certificate is invalid or does not contain a supported server name")
-	errListenerInvalidRouteKinds                  = errors.New("allowed route kind is invalid")
-	errListenerProgrammed_Invalid                 = errors.New("listener cannot be programmed because it is invalid")
+	errListenerUnsupportedProtocol                    = errors.New("listener protocol is unsupported")
+	errListenerPortUnavailable                        = errors.New("listener port is unavailable")
+	errListenerHostnameConflict                       = errors.New("listener hostname conflicts with another listener")
+	errListenerProtocolConflict                       = errors.New("listener protocol conflicts with another listener")
+	errListenerInvalidCertificateRef_NotFound         = errors.New("certificate not found")
+	errListenerInvalidCertificateRef_NotSupported     = errors.New("certificate type is not supported")
+	errListenerInvalidCertificateRef_InvalidData      = errors.New("certificate is invalid or does not contain a supported server name")
+	errListenerInvalidCertificateRef_NonFIPSRSAKeyLen = errors.New("certificate has an invalid length: RSA Keys must be at least 2048-bit")
+	errListenerInvalidCertificateRef_FIPSRSAKeyLen    = errors.New("certificate has an invalid length: RSA keys must be either 2048-bit, 3072-bit, or 4096-bit in FIPS mode")
+	errListenerInvalidRouteKinds                      = errors.New("allowed route kind is invalid")
+	errListenerProgrammed_Invalid                     = errors.New("listener cannot be programmed because it is invalid")
 
 	// Below is where any custom generic listener validation errors should go.
 	// We map anything under here to a custom ListenerConditionReason of Invalid on
@@ -372,7 +374,7 @@ func (l listenerValidationResult) resolvedRefsCondition(generation int64) metav1
 	}
 
 	switch l.refErr {
-	case errListenerInvalidCertificateRef_NotFound, errListenerInvalidCertificateRef_NotSupported, errListenerInvalidCertificateRef_InvalidData:
+	case errListenerInvalidCertificateRef_NotFound, errListenerInvalidCertificateRef_NotSupported, errListenerInvalidCertificateRef_InvalidData, errListenerInvalidCertificateRef_NonFIPSRSAKeyLen, errListenerInvalidCertificateRef_FIPSRSAKeyLen:
 		return metav1.Condition{
 			Type:               "ResolvedRefs",
 			Status:             metav1.ConditionFalse,

control-plane/api-gateway/binding/validation.go

@@ -6,9 +6,6 @@ package binding
 import (
 	"strings"
 
-	"github.com/hashicorp/consul-k8s/control-plane/api-gateway/common"
-	"github.com/hashicorp/consul-k8s/control-plane/api/v1alpha1"
-	"github.com/hashicorp/consul/api"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	klabels "k8s.io/apimachinery/pkg/labels"
@@ -17,6 +14,11 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	gwv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2"
 	gwv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1"
+
+	"github.com/hashicorp/consul-k8s/control-plane/api-gateway/common"
+	"github.com/hashicorp/consul-k8s/control-plane/api/v1alpha1"
+	"github.com/hashicorp/consul-k8s/control-plane/version"
+	"github.com/hashicorp/consul/api"
 )
 
 var (
@@ -205,10 +207,20 @@ func validateTLS(gateway gwv1beta1.Gateway, tls *gwv1beta1.GatewayTLSConfig, res
 }
 
 func validateCertificateData(secret corev1.Secret) error {
-	_, _, err := common.ParseCertificateData(secret)
+	_, privateKey, err := common.ParseCertificateData(secret)
 	if err != nil {
 		return errListenerInvalidCertificateRef_InvalidData
 	}
+
+	err = common.ValidateKeyLength(privateKey)
+	if err != nil {
+		if version.IsFIPS() {
+			return errListenerInvalidCertificateRef_FIPSRSAKeyLen
+		}
+
+		return errListenerInvalidCertificateRef_NonFIPSRSAKeyLen
+	}
+
 	return nil
 }
 
@@ -235,7 +247,7 @@ func validateListeners(gateway gwv1beta1.Gateway, listeners []gwv1beta1.Listener
 			_, supported := supportedKindsForProtocol[listener.Protocol]
 			if !supported {
 				result.acceptedErr = errListenerUnsupportedProtocol
-			} else if listener.Port == 20000 { //admin port
+			} else if listener.Port == 20000 { // admin port
 				result.acceptedErr = errListenerPortUnavailable
 			}
 

control-plane/api-gateway/common/secrets.go

@@ -12,15 +12,19 @@ import (
 
 	"github.com/miekg/dns"
 	corev1 "k8s.io/api/core/v1"
+
+	"github.com/hashicorp/consul-k8s/control-plane/version"
 )
 
+var errFailedToParsePrivateKeyPem = errors.New("failed to parse private key PEM")
+
 func ParseCertificateData(secret corev1.Secret) (cert string, privateKey string, err error) {
 	decodedPrivateKey := secret.Data[corev1.TLSPrivateKeyKey]
 	decodedCertificate := secret.Data[corev1.TLSCertKey]
 
 	privateKeyBlock, _ := pem.Decode(decodedPrivateKey)
 	if privateKeyBlock == nil {
-		return "", "", errors.New("failed to parse private key PEM")
+		return "", "", errFailedToParsePrivateKeyPem
 	}
 
 	certificateBlock, _ := pem.Decode(decodedCertificate)
@@ -66,3 +70,50 @@ func validateCertificateHosts(certificate *x509.Certificate) error {
 
 	return nil
 }
+
+// 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
+
+// ValidateKeyLength ensures that the key length for a certificate is of a valid length
+// for envoy dependent on if consul is running in FIPS mode or not
+func ValidateKeyLength(privateKey string) error {
+	privateKeyBlock, _ := pem.Decode([]byte(privateKey))
+
+	if privateKeyBlock == nil {
+		return errFailedToParsePrivateKeyPem
+	}
+
+	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("RSA key length must be at least 2048-bit")
+	}
+
+	return nil
+}
+
+func fipsLenCheck(keyLen int) error {
+	if keyLen != 2048 && keyLen != 3072 && keyLen != 4096 {
+		return errors.New("RSA key length must be at either 2048-bit, 3072-bit, or 4096-bit in FIPS mode")
+	}
+	return nil
+}

control-plane/api-gateway/common/translation.go

@@ -6,14 +6,15 @@ package common
 import (
 	"strings"
 
-	"github.com/hashicorp/consul-k8s/control-plane/api/v1alpha1"
-	"github.com/hashicorp/consul-k8s/control-plane/connect-inject/constants"
-	"github.com/hashicorp/consul-k8s/control-plane/namespaces"
-	"github.com/hashicorp/consul/api"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/types"
 	gwv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2"
 	gwv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1"
+
+	"github.com/hashicorp/consul-k8s/control-plane/api/v1alpha1"
+	"github.com/hashicorp/consul-k8s/control-plane/connect-inject/constants"
+	"github.com/hashicorp/consul-k8s/control-plane/namespaces"
+	"github.com/hashicorp/consul/api"
 )
 
 // ResourceTranslator handles translating K8s resources into Consul config entries.
@@ -340,6 +341,11 @@ func (t ResourceTranslator) ToInlineCertificate(secret corev1.Secret) (*api.Inli
 		return nil, err
 	}
 
+	err = ValidateKeyLength(privateKey)
+	if err != nil {
+		return nil, err
+	}
+
 	namespace := t.Namespace(secret.Namespace)
 
 	return &api.InlineCertificateConfigEntry{