Skip to content

Commit

Permalink
Ensure cert is removed from consul if an invalid one is presented
Browse files Browse the repository at this point in the history
  • Loading branch information
jm96441n committed Jun 29, 2023
1 parent 4b2ebcd commit 47c77a4
Show file tree
Hide file tree
Showing 4 changed files with 68 additions and 46 deletions.
4 changes: 2 additions & 2 deletions control-plane/api-gateway/binding/binder_test.go
Expand Up @@ -2331,11 +2331,11 @@ func controlledBinder(config BinderConfig) BinderConfig {
}

func generateInvalidTestCertificate(t *testing.T, namespace, name string) (*api.InlineCertificateConfigEntry, corev1.Secret) {
return generateTestCertificateByKeyLen(t, namespace, name, MinKeyLength-1)
return generateTestCertificateByKeyLen(t, namespace, name, common.MinKeyLength-1)
}

func generateTestCertificate(t *testing.T, namespace, name string) (*api.InlineCertificateConfigEntry, corev1.Secret) {
return generateTestCertificateByKeyLen(t, namespace, name, MinKeyLength)
return generateTestCertificateByKeyLen(t, namespace, name, common.MinKeyLength)
}

func generateTestCertificateByKeyLen(t *testing.T, namespace, name string, keyLen int) (*api.InlineCertificateConfigEntry, corev1.Secret) {
Expand Down
43 changes: 4 additions & 39 deletions control-plane/api-gateway/binding/validation.go
Expand Up @@ -4,8 +4,6 @@
package binding

import (
"crypto/x509"
"encoding/pem"
"strings"

corev1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -208,57 +206,24 @@ func validateTLS(gateway gwv1beta1.Gateway, tls *gwv1beta1.GatewayTLSConfig, res
return nil, err
}

// 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 validateCertificateData(secret corev1.Secret) error {
_, privateKey, err := common.ParseCertificateData(secret)
if err != nil {
return errListenerInvalidCertificateRef_InvalidData
}

return validateKeyLength(privateKey)
}

func validateKeyLength(privateKey string) error {
// we can assume this is non-nil as it would've caused an error from common.ParseCertificate if it was
privateKeyBlock, _ := pem.Decode([]byte(privateKey))

if privateKeyBlock.Type != "RSA PRIVATE KEY" {
return nil
}

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

keyBitLen := key.N.BitLen()

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

return nonFipsLenCheck(keyBitLen)
}
if version.IsFIPS() {
return errListenerInvalidCertificateRef_FIPSRSAKeyLen
}

func nonFipsLenCheck(keyLen int) error {
// ensure private key is of the correct length
if keyLen < MinKeyLength {
return errListenerInvalidCertificateRef_NonFIPSRSAKeyLen
}

return nil
}

func fipsLenCheck(keyLen int) error {
if keyLen != 2048 && keyLen != 3072 && keyLen != 4096 {
return errListenerInvalidCertificateRef_FIPSRSAKeyLen
}
return nil
}

// validateListeners validates the given listeners both internally and with respect to each
// other for purposes of setting "Conflicted" status conditions.
func validateListeners(gateway gwv1beta1.Gateway, listeners []gwv1beta1.Listener, resources *common.ResourceMap) listenerValidationResults {
Expand Down
53 changes: 52 additions & 1 deletion control-plane/api-gateway/common/secrets.go
Expand Up @@ -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)
Expand Down Expand Up @@ -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
}
14 changes: 10 additions & 4 deletions control-plane/api-gateway/common/translation.go
Expand Up @@ -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.
Expand Down Expand Up @@ -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{
Expand Down

0 comments on commit 47c77a4

Please sign in to comment.