Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 7 commits into from
Aug 9, 2023
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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")
Comment on lines +245 to +248
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The snake_case values here refer to the annotation keys that you'll find in common/constants.go below


// 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 these two if Envoy ever sets TLS 1.3 as default minimum
nathancoleman marked this conversation as resolved.
Show resolved Hide resolved
"": {},
"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": {},
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are basically copy-pasted from the legacy implementation with some simplification of the structure

)

// 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 {
Copy link
Member Author

@nathancoleman nathancoleman Aug 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The diff here is easiest to view in split mode IMO. It amounts to splitting the existing mega-function out into some more granular validation functions and adding a new function to validate the TLS options

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 {
cipherSuitsVal := string(listener.TLS.Options[TLSCipherSuitesAnnotationKey])
nathancoleman marked this conversation as resolved.
Show resolved Hide resolved
if cipherSuitsVal != "" {
cipherSuites = strings.Split(cipherSuitsVal, ",")
}
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