Skip to content

Commit

Permalink
Backport of NET-4413 Implement translation + validation of TLS option…
Browse files Browse the repository at this point in the history
…s into release/1.2.x (#2747)

NET-4413 Implement translation + validation of TLS options (#2711)

* Implement validation of TLS options

* Use constants for annotation keys

* Add changelog entry

* Implement TLS options translation

* Update changelog entry

* Add unit test coverage for TLS option validation

* Code review feedback

Co-authored-by: Nathan Coleman <nathan.coleman@hashicorp.com>
  • Loading branch information
1 parent 86cd307 commit 8e311bd
Show file tree
Hide file tree
Showing 7 changed files with 194 additions and 19 deletions.
3 changes: 3 additions & 0 deletions .changelog/2711.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:feature
api-gateway: translate and validate TLS configuration options, including min/max version and cipher suites, setting Gateway status appropriately
```
6 changes: 5 additions & 1 deletion control-plane/api-gateway/binding/result.go
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,11 @@ var (
// Below is where any custom generic listener validation errors should go.
// We map anything under here to a custom ListenerConditionReason of Invalid on
// an Accepted status type.
errListenerNoTLSPassthrough = errors.New("TLS passthrough is not supported")
errListenerNoTLSPassthrough = errors.New("TLS passthrough is not supported")
errListenerTLSCipherSuiteNotConfigurable = errors.New("tls_min_version does not allow tls_cipher_suites configuration")
errListenerUnsupportedTLSCipherSuite = errors.New("unsupported cipher suite in tls_cipher_suites")
errListenerUnsupportedTLSMaxVersion = errors.New("unsupported tls_max_version")
errListenerUnsupportedTLSMinVersion = errors.New("unsupported tls_min_version")

// This custom listener validation error is used to differentiate between an errListenerPortUnavailable because of
// direct port conflicts defined by the user (two listeners on the same port) vs a port conflict because we map
Expand Down
124 changes: 106 additions & 18 deletions control-plane/api-gateway/binding/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,45 @@ var (
gwv1beta1.Kind("HTTPRoute"): {},
gwv1beta1.Kind("TCPRoute"): {},
}

allSupportedTLSVersions = map[string]struct{}{
"TLS_AUTO": {},
"TLSv1_0": {},
"TLSv1_1": {},
"TLSv1_2": {},
"TLSv1_3": {},
}

allTLSVersionsWithConfigurableCipherSuites = map[string]struct{}{
// Remove "" and "TLS_AUTO" if Envoy ever sets TLS 1.3 as default minimum
"": {},
"TLS_AUTO": {},
"TLSv1_0": {},
"TLSv1_1": {},
"TLSv1_2": {},
}

allSupportedTLSCipherSuites = map[string]struct{}{
"TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256": {},
"TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256": {},
"TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256": {},
"TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256": {},
"TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384": {},
"TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384": {},

// NOTE: the following cipher suites are currently supported by Envoy
// but have been identified as insecure and are pending removal
// https://github.com/envoyproxy/envoy/issues/5399
"TLS_RSA_WITH_AES_128_GCM_SHA256": {},
"TLS_RSA_WITH_AES_128_CBC_SHA": {},
"TLS_RSA_WITH_AES_256_GCM_SHA384": {},
"TLS_RSA_WITH_AES_256_CBC_SHA": {},
// https://github.com/envoyproxy/envoy/issues/5400
"TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA": {},
"TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA": {},
"TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA": {},
"TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA": {},
}
)

// validateRefs validates backend references for a route, determining whether or
Expand Down Expand Up @@ -167,43 +206,92 @@ func (m mergedListeners) validateHostname(index int, listener gwv1beta1.Listener
// validateTLS validates that the TLS configuration for a given listener is valid and that
// the certificates that it references exist.
func validateTLS(gateway gwv1beta1.Gateway, tls *gwv1beta1.GatewayTLSConfig, resources *common.ResourceMap) (error, error) {
namespace := gateway.Namespace

// If there's no TLS, there's nothing to validate
if tls == nil {
return nil, nil
}

var err error
// Validate the certificate references and then return any error
// alongside any TLS configuration error that we find below.
refsErr := validateCertificateRefs(gateway, tls.CertificateRefs, resources)

if tls.Mode != nil && *tls.Mode == gwv1beta1.TLSModePassthrough {
return errListenerNoTLSPassthrough, refsErr
}

if err := validateTLSOptions(tls.Options); err != nil {
return err, refsErr
}

return nil, refsErr
}

for _, cert := range tls.CertificateRefs {
// break on the first error
func validateCertificateRefs(gateway gwv1beta1.Gateway, refs []gwv1beta1.SecretObjectReference, resources *common.ResourceMap) error {
for _, cert := range refs {
// Verify that the reference has a group and kind that we support
if !common.NilOrEqual(cert.Group, "") || !common.NilOrEqual(cert.Kind, common.KindSecret) {
err = errListenerInvalidCertificateRef_NotSupported
break
return errListenerInvalidCertificateRef_NotSupported
}

// Verify that the reference is within the namespace or,
// if cross-namespace, that it's allowed by a ReferenceGrant
if !resources.GatewayCanReferenceSecret(gateway, cert) {
err = errRefNotPermitted
break
return errRefNotPermitted
}

key := common.IndexedNamespacedNameWithDefault(cert.Name, cert.Namespace, namespace)
// Verify that the referenced resource actually exists
key := common.IndexedNamespacedNameWithDefault(cert.Name, cert.Namespace, gateway.Namespace)
secret := resources.Certificate(key)

if secret == nil {
err = errListenerInvalidCertificateRef_NotFound
break
return errListenerInvalidCertificateRef_NotFound
}

err = validateCertificateData(*secret)
// Verify that the referenced resource contains the data shape that we expect
if err := validateCertificateData(*secret); err != nil {
return err
}
}

if tls.Mode != nil && *tls.Mode == gwv1beta1.TLSModePassthrough {
return errListenerNoTLSPassthrough, err
return nil
}

func validateTLSOptions(options map[gwv1beta1.AnnotationKey]gwv1beta1.AnnotationValue) error {
if options == nil {
return nil
}

tlsMinVersionValue := string(options[common.TLSMinVersionAnnotationKey])
if tlsMinVersionValue != "" {
if _, supported := allSupportedTLSVersions[tlsMinVersionValue]; !supported {
return errListenerUnsupportedTLSMinVersion
}
}

tlsMaxVersionValue := string(options[common.TLSMaxVersionAnnotationKey])
if tlsMaxVersionValue != "" {
if _, supported := allSupportedTLSVersions[tlsMaxVersionValue]; !supported {
return errListenerUnsupportedTLSMaxVersion
}
}

// TODO: validate tls options
return nil, err
tlsCipherSuitesValue := string(options[common.TLSCipherSuitesAnnotationKey])
if tlsCipherSuitesValue != "" {
// If a minimum TLS version is configured, verify that it supports configuring cipher suites
if tlsMinVersionValue != "" {
if _, supported := allTLSVersionsWithConfigurableCipherSuites[tlsMinVersionValue]; !supported {
return errListenerTLSCipherSuiteNotConfigurable
}
}

for _, tlsCipherSuiteValue := range strings.Split(tlsCipherSuitesValue, ",") {
tlsCipherSuite := strings.TrimSpace(tlsCipherSuiteValue)
if _, supported := allSupportedTLSCipherSuites[tlsCipherSuite]; !supported {
return errListenerUnsupportedTLSCipherSuite
}
}
}

return nil
}

func validateCertificateData(secret corev1.Secret) error {
Expand Down
41 changes: 41 additions & 0 deletions control-plane/api-gateway/binding/validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -510,6 +510,47 @@ func TestValidateTLS(t *testing.T) {
expectedResolvedRefsErr: nil,
expectedAcceptedErr: nil,
},
"invalid cipher suite": {
gateway: gatewayWithFinalizer(gwv1beta1.GatewaySpec{}),
tls: &gwv1beta1.GatewayTLSConfig{
Options: map[gwv1beta1.AnnotationKey]gwv1beta1.AnnotationValue{
common.TLSCipherSuitesAnnotationKey: "invalid",
},
},
certificates: nil,
expectedAcceptedErr: errListenerUnsupportedTLSCipherSuite,
},
"cipher suite not configurable": {
gateway: gatewayWithFinalizer(gwv1beta1.GatewaySpec{}),
tls: &gwv1beta1.GatewayTLSConfig{
Options: map[gwv1beta1.AnnotationKey]gwv1beta1.AnnotationValue{
common.TLSMinVersionAnnotationKey: "TLSv1_3",
common.TLSCipherSuitesAnnotationKey: "TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256",
},
},
certificates: nil,
expectedAcceptedErr: errListenerTLSCipherSuiteNotConfigurable,
},
"invalid max version": {
gateway: gatewayWithFinalizer(gwv1beta1.GatewaySpec{}),
tls: &gwv1beta1.GatewayTLSConfig{
Options: map[gwv1beta1.AnnotationKey]gwv1beta1.AnnotationValue{
common.TLSMaxVersionAnnotationKey: "invalid",
},
},
certificates: nil,
expectedAcceptedErr: errListenerUnsupportedTLSMaxVersion,
},
"invalid min version": {
gateway: gatewayWithFinalizer(gwv1beta1.GatewaySpec{}),
tls: &gwv1beta1.GatewayTLSConfig{
Options: map[gwv1beta1.AnnotationKey]gwv1beta1.AnnotationValue{
common.TLSMinVersionAnnotationKey: "invalid",
},
},
certificates: nil,
expectedAcceptedErr: errListenerUnsupportedTLSMinVersion,
},
} {
t.Run(name, func(t *testing.T) {
resources := common.NewResourceMap(common.ResourceTranslator{}, NewReferenceValidator(tt.grants), logrtest.NewTestLogger(t))
Expand Down
5 changes: 5 additions & 0 deletions control-plane/api-gateway/common/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,9 @@ const (
GatewayClassControllerName = "consul.hashicorp.com/gateway-controller"

AnnotationGatewayClassConfig = "consul.hashicorp.com/gateway-class-config"

// The following annotation keys are used in the v1beta1.GatewayTLSConfig's Options on a v1beta1.Listener.
TLSCipherSuitesAnnotationKey = "api-gateway.consul.hashicorp.com/tls_cipher_suites"
TLSMaxVersionAnnotationKey = "api-gateway.consul.hashicorp.com/tls_max_version"
TLSMinVersionAnnotationKey = "api-gateway.consul.hashicorp.com/tls_min_version"
)
12 changes: 12 additions & 0 deletions control-plane/api-gateway/common/translation.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,17 @@ func (t ResourceTranslator) toAPIGatewayListener(gateway gwv1beta1.Gateway, list
namespace := gateway.Namespace

var certificates []api.ResourceReference
var cipherSuites []string
var maxVersion, minVersion string

if listener.TLS != nil {
cipherSuitesVal := string(listener.TLS.Options[TLSCipherSuitesAnnotationKey])
if cipherSuitesVal != "" {
cipherSuites = strings.Split(cipherSuitesVal, ",")
}
maxVersion = string(listener.TLS.Options[TLSMaxVersionAnnotationKey])
minVersion = string(listener.TLS.Options[TLSMinVersionAnnotationKey])

for _, ref := range listener.TLS.CertificateRefs {
if !resources.GatewayCanReferenceSecret(gateway, ref) {
return api.APIGatewayListener{}, false
Expand Down Expand Up @@ -116,6 +125,9 @@ func (t ResourceTranslator) toAPIGatewayListener(gateway gwv1beta1.Gateway, list
Protocol: listenerProtocolMap[strings.ToLower(string(listener.Protocol))],
TLS: api.APIGatewayTLSConfiguration{
Certificates: certificates,
CipherSuites: cipherSuites,
MaxVersion: maxVersion,
MinVersion: minVersion,
},
}, true
}
Expand Down
22 changes: 22 additions & 0 deletions control-plane/api-gateway/common/translation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"encoding/pem"
"fmt"
"math/big"
"strings"
"testing"
"time"

Expand Down Expand Up @@ -134,6 +135,9 @@ func TestTranslator_ToAPIGateway(t *testing.T) {
listenerOneCertK8sNamespace := "one-cert-ns"
listenerOneCertConsulNamespace := "one-cert-ns"
listenerOneCert := generateTestCertificate(t, "one-cert-ns", "one-cert")
listenerOneMaxVersion := "TLSv1_2"
listenerOneMinVersion := "TLSv1_3"
listenerOneCipherSuites := []string{"TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256", "TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256"}

// listener one status
listenerOneLastTransmissionTime := time.Now()
Expand All @@ -157,6 +161,7 @@ func TestTranslator_ToAPIGateway(t *testing.T) {
annotations map[string]string
expectedGWName string
listenerOneK8sCertRefs []gwv1beta1.SecretObjectReference
listenerOneTLSOptions map[gwv1beta1.AnnotationKey]gwv1beta1.AnnotationValue
}{
"gw name": {
annotations: make(map[string]string),
Expand All @@ -167,6 +172,11 @@ func TestTranslator_ToAPIGateway(t *testing.T) {
Namespace: PointerTo(gwv1beta1.Namespace(listenerOneCertK8sNamespace)),
},
},
listenerOneTLSOptions: map[gwv1beta1.AnnotationKey]gwv1beta1.AnnotationValue{
TLSMaxVersionAnnotationKey: gwv1beta1.AnnotationValue(listenerOneMaxVersion),
TLSMinVersionAnnotationKey: gwv1beta1.AnnotationValue(listenerOneMinVersion),
TLSCipherSuitesAnnotationKey: gwv1beta1.AnnotationValue(strings.Join(listenerOneCipherSuites, ",")),
},
},
"when k8s has certs that are not referenced in consul": {
annotations: make(map[string]string),
Expand All @@ -181,6 +191,11 @@ func TestTranslator_ToAPIGateway(t *testing.T) {
Namespace: PointerTo(gwv1beta1.Namespace(listenerOneCertK8sNamespace)),
},
},
listenerOneTLSOptions: map[gwv1beta1.AnnotationKey]gwv1beta1.AnnotationValue{
TLSMaxVersionAnnotationKey: gwv1beta1.AnnotationValue(listenerOneMaxVersion),
TLSMinVersionAnnotationKey: gwv1beta1.AnnotationValue(listenerOneMinVersion),
TLSCipherSuitesAnnotationKey: gwv1beta1.AnnotationValue(strings.Join(listenerOneCipherSuites, ",")),
},
},
}

Expand All @@ -207,6 +222,7 @@ func TestTranslator_ToAPIGateway(t *testing.T) {
Protocol: gwv1beta1.ProtocolType(listenerOneProtocol),
TLS: &gwv1beta1.GatewayTLSConfig{
CertificateRefs: tc.listenerOneK8sCertRefs,
Options: tc.listenerOneTLSOptions,
},
},
{
Expand Down Expand Up @@ -288,6 +304,9 @@ func TestTranslator_ToAPIGateway(t *testing.T) {
Namespace: listenerOneCertConsulNamespace,
},
},
CipherSuites: listenerOneCipherSuites,
MaxVersion: listenerOneMaxVersion,
MinVersion: listenerOneMinVersion,
},
},
{
Expand All @@ -303,6 +322,9 @@ func TestTranslator_ToAPIGateway(t *testing.T) {
Namespace: listenerTwoCertConsulNamespace,
},
},
CipherSuites: nil,
MaxVersion: "",
MinVersion: "",
},
},
},
Expand Down

0 comments on commit 8e311bd

Please sign in to comment.