From b30d93fe2a0f524f973544769f182acbf36d478a Mon Sep 17 00:00:00 2001 From: Ricardo Herrera Date: Fri, 8 Nov 2024 15:33:24 +0000 Subject: [PATCH 1/5] add method to get tls config based on client hello * This method allows looking up custom config based on client hello * Expand certificate registry type to allow for both certs and config --- secrets/certregistry/certregistry.go | 31 +++++++++++++++++++++------- skipper.go | 1 + 2 files changed, 25 insertions(+), 7 deletions(-) diff --git a/secrets/certregistry/certregistry.go b/secrets/certregistry/certregistry.go index df6f8a16d5..012e7a148f 100644 --- a/secrets/certregistry/certregistry.go +++ b/secrets/certregistry/certregistry.go @@ -9,16 +9,22 @@ import ( log "github.com/sirupsen/logrus" ) +// CertRegistryEntry holds a certificate and tls configuration. +type CertRegistryEntry struct { + Certificate *tls.Certificate + Config *tls.Config +} + // CertRegistry object holds TLS certificates to be used to terminate TLS connections // ensuring synchronized access to them. type CertRegistry struct { mu sync.Mutex - lookup map[string]*tls.Certificate + lookup map[string]*CertRegistryEntry } // NewCertRegistry initializes the certificate registry. func NewCertRegistry() *CertRegistry { - l := make(map[string]*tls.Certificate) + l := make(map[string]*CertRegistryEntry) return &CertRegistry{ lookup: l, @@ -43,16 +49,16 @@ func (r *CertRegistry) ConfigureCertificate(host string, cert *tls.Certificate) curr, found := r.lookup[host] if found { - if cert.Leaf.NotBefore.After(curr.Leaf.NotBefore) { + if cert.Leaf.NotBefore.After(curr.Certificate.Leaf.NotBefore) { log.Infof("updating certificate in registry - %s", host) - r.lookup[host] = cert + r.lookup[host].Certificate = cert return nil } else { return nil } } else { log.Infof("adding certificate to registry - %s", host) - r.lookup[host] = cert + r.lookup[host].Certificate = cert return nil } } @@ -61,10 +67,21 @@ func (r *CertRegistry) ConfigureCertificate(host string, cert *tls.Certificate) // If no certificate is found for the host it will return nil. func (r *CertRegistry) GetCertFromHello(hello *tls.ClientHelloInfo) (*tls.Certificate, error) { r.mu.Lock() - cert, found := r.lookup[hello.ServerName] + entry, found := r.lookup[hello.ServerName] r.mu.Unlock() if found { - return cert, nil + return entry.Certificate, nil } return nil, nil } + +// GetConfigFromHello reads the SNI from a TLS client and returns the appropriate config. +func (r *CertRegistry) GetConfigFromHello(hello *tls.ClientHelloInfo) (*tls.Config, error) { + r.mu.Lock() + entry, found := r.lookup[hello.ServerName] + r.mu.Unlock() + if found { + return entry.Config, nil + } + return entry.Config, nil +} diff --git a/skipper.go b/skipper.go index 19e24f027f..abe75ba4c4 100644 --- a/skipper.go +++ b/skipper.go @@ -1211,6 +1211,7 @@ func (o *Options) tlsConfig(cr *certregistry.CertRegistry) (*tls.Config, error) if cr != nil { config.GetCertificate = cr.GetCertFromHello + config.GetConfigForClient = cr.GetConfigFromHello } if o.CertPathTLS == "" && o.KeyPathTLS == "" { From 35391e643957dfce112e85aaa5dfc9659dc20294 Mon Sep 17 00:00:00 2001 From: Ricardo Herrera Date: Fri, 8 Nov 2024 15:33:57 +0000 Subject: [PATCH 2/5] adjust cert registry tests for expanded registry entry type --- secrets/certregistry/certregistry_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/secrets/certregistry/certregistry_test.go b/secrets/certregistry/certregistry_test.go index 0734c7ca78..b684f5463f 100644 --- a/secrets/certregistry/certregistry_test.go +++ b/secrets/certregistry/certregistry_test.go @@ -156,11 +156,11 @@ func TestCertRegistry(t *testing.T) { t.Run("sync new certificate", func(t *testing.T) { cr := NewCertRegistry() cr.ConfigureCertificate(validHostname, validCert) - cert, found := cr.lookup[validHostname] + entry, found := cr.lookup[validHostname] if !found { t.Error("failed to read certificate") } - if cert.Leaf == nil { + if entry.Certificate.Leaf == nil { t.Error("synced cert should have a parsed leaf") } }) @@ -178,10 +178,10 @@ func TestCertRegistry(t *testing.T) { cr := NewCertRegistry() cr.ConfigureCertificate(validHostname, validCert) - cert1 := cr.lookup[validHostname] + entry1 := cr.lookup[validHostname] cr.ConfigureCertificate(validHostname, newValidCert) - cert2 := cr.lookup[validHostname] - if equalCert(cert1, cert2) { + entry2 := cr.lookup[validHostname] + if equalCert(entry1.Certificate, entry2.Certificate) { t.Error("host cert was not updated") } From 43dd917480d128bcf30a4a643f78694020200d51 Mon Sep 17 00:00:00 2001 From: Ricardo Herrera Date: Sat, 9 Nov 2024 17:11:54 +0000 Subject: [PATCH 3/5] add method to add tls config to cert registry --- secrets/certregistry/certregistry.go | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/secrets/certregistry/certregistry.go b/secrets/certregistry/certregistry.go index 012e7a148f..ad327eb4a4 100644 --- a/secrets/certregistry/certregistry.go +++ b/secrets/certregistry/certregistry.go @@ -63,6 +63,30 @@ func (r *CertRegistry) ConfigureCertificate(host string, cert *tls.Certificate) } } +// ConfigureTLSConfig configures a tls config for the host. +func (r *CertRegistry) ConfigureTLSConfig(host string, config *tls.Config) error { + if config == nil { + return fmt.Errorf("cannot configure nil tls config") + } + + r.mu.Lock() + defer r.mu.Unlock() + + curr, found := r.lookup[host] + + if found && curr.Config != nil { + log.Infof("updating tls config for host - %s", host) + r.lookup[host].Config = config + return nil + } else { + log.Infof("adding tls config for host - %s", host) + r.lookup[host] = &CertRegistryEntry{ + Config: config, + } + return nil + } +} + // GetCertFromHello reads the SNI from a TLS client and returns the appropriate certificate. // If no certificate is found for the host it will return nil. func (r *CertRegistry) GetCertFromHello(hello *tls.ClientHelloInfo) (*tls.Certificate, error) { From 4c74244bf810a52d31d5b4d43bf198f91859a879 Mon Sep 17 00:00:00 2001 From: Ricardo Herrera Date: Sat, 9 Nov 2024 17:15:36 +0000 Subject: [PATCH 4/5] allow client auth types to be set from ingress annotation * Create function to parse client auth from annotation * Set tls config when adding tls spec from ingress * functions to parse and add configs to registry --- dataclients/kubernetes/ingress.go | 18 ++++++++++++++++++ dataclients/kubernetes/ingressv1.go | 8 ++++++++ dataclients/kubernetes/kube.go | 29 +++++++++++++++++++++++++++++ 3 files changed, 55 insertions(+) diff --git a/dataclients/kubernetes/ingress.go b/dataclients/kubernetes/ingress.go index d44520f5c1..868c01b8f3 100644 --- a/dataclients/kubernetes/ingress.go +++ b/dataclients/kubernetes/ingress.go @@ -23,6 +23,7 @@ const ( skipperLoadBalancerAnnotationKey = "zalando.org/skipper-loadbalancer" skipperBackendProtocolAnnotationKey = "zalando.org/skipper-backend-protocol" pathModeAnnotationKey = "zalando.org/skipper-ingress-path-mode" + tlsClientAuthAnnotationKey = "zalando.org/skipper-client-auth" ingressOriginName = "ingress" tlsSecretType = "kubernetes.io/tls" tlsSecretDataCrt = "tls.crt" @@ -38,6 +39,7 @@ type ingressContext struct { extraRoutes []*eskip.Route backendWeights map[string]float64 pathMode PathMode + tlsClientAuth tls.ClientAuthType redirect *redirectInfo hostRoutes map[string][]*eskip.Route defaultFilters defaultFilters @@ -51,6 +53,7 @@ type ingress struct { allowedExternalNames []*regexp.Regexp kubernetesEastWestDomain string pathMode PathMode + clientAuth tls.ClientAuthType httpsRedirectCode int kubernetesEnableEastWest bool provideHTTPSRedirect bool @@ -291,6 +294,21 @@ func pathMode(m *definitions.Metadata, globalDefault PathMode, logger *logger) P return pathMode } +// parse tls client auth type from annotation or fallback to global default +func tlsClientAuth(m *definitions.Metadata, globalDefault tls.ClientAuthType, logger *logger) tls.ClientAuthType { + clientAuth := globalDefault + + if clientAuthString, ok := m.Annotations[tlsClientAuthAnnotationKey]; ok { + if c, err := ParseTLSClientAuth(clientAuthString); err != nil { + logger.Errorf("Failed to get tls client auth: %v", err) + } else { + logger.Debugf("Set tls client auth to %s", c) + clientAuth = c + } + } + return clientAuth +} + func (ing *ingress) addCatchAllRoutes(host string, r *eskip.Route, redirect *redirectInfo) []*eskip.Route { catchAll := &eskip.Route{ Id: routeID("", "catchall", host, "", ""), diff --git a/dataclients/kubernetes/ingressv1.go b/dataclients/kubernetes/ingressv1.go index ed2431ed5b..8b2464048d 100644 --- a/dataclients/kubernetes/ingressv1.go +++ b/dataclients/kubernetes/ingressv1.go @@ -1,6 +1,7 @@ package kubernetes import ( + "crypto/tls" "errors" "fmt" "regexp" @@ -318,6 +319,12 @@ func (ing *ingress) addSpecIngressTLSV1(ic *ingressContext, ingtls *definitions. return } addTLSCertToRegistry(ic.certificateRegistry, ic.logger, hostlist, secret) + + // Set tls config for all hosts defined in the ingress + tlsConfig := &tls.Config{ + ClientAuth: ic.tlsClientAuth, + } + addTLSConfigToRegistry(ic.certificateRegistry, ic.logger, hostlist, tlsConfig) } // converts the default backend if any @@ -435,6 +442,7 @@ func (ing *ingress) ingressV1Route( extraRoutes: extraRoutes(i.Metadata), backendWeights: backendWeights(i.Metadata, logger), pathMode: pathMode(i.Metadata, ing.pathMode, logger), + tlsClientAuth: tlsClientAuth(i.Metadata, ing.clientAuth, logger), redirect: redirect, hostRoutes: hostRoutes, defaultFilters: df, diff --git a/dataclients/kubernetes/kube.go b/dataclients/kubernetes/kube.go index 523b71fdd8..613c5bf0f6 100644 --- a/dataclients/kubernetes/kube.go +++ b/dataclients/kubernetes/kube.go @@ -1,6 +1,7 @@ package kubernetes import ( + "crypto/tls" "fmt" "net" "net/http" @@ -383,6 +384,25 @@ func ParsePathMode(s string) (PathMode, error) { } } +// ParseTLSClientAuth parses the string representations of different +// client auth types. +func ParseTLSClientAuth(s string) (tls.ClientAuthType, error) { + switch s { + case "noclientcert": + return tls.NoClientCert, nil + case "requestclientcert": + return tls.RequestClientCert, nil + case "requireanyclientcert": + return tls.RequireAnyClientCert, nil + case "verifyclientcertifgiven": + return tls.VerifyClientCertIfGiven, nil + case "requireandverifyclientcert": + return tls.RequireAndVerifyClientCert, nil + default: + return 0, fmt.Errorf("invalid client auth type string: %s", s) + } +} + func mapRoutes(routes []*eskip.Route) (map[string]*eskip.Route, []*eskip.Route) { var uniqueRoutes []*eskip.Route routesById := make(map[string]*eskip.Route) @@ -618,3 +638,12 @@ func addTLSCertToRegistry(cr *certregistry.CertRegistry, logger *logger, hosts [ } } } + +func addTLSConfigToRegistry(cr *certregistry.CertRegistry, logger *logger, hosts []string, tlsConfig *tls.Config) { + for _, host := range hosts { + err := cr.ConfigureTLSConfig(host, tlsConfig) + if err != nil { + logger.Errorf("Failed to configure TLS config: %v", err) + } + } +} From f8a24893211705363e572edd4d9373dac5329a3d Mon Sep 17 00:00:00 2001 From: Ricardo Herrera Date: Mon, 11 Nov 2024 21:08:24 +0000 Subject: [PATCH 5/5] feat: use tls GetConfigForClient on certregistry * Store tls.Config in registry to allow for certs and config to be stored * Refactor registry to handle setting configs with hash for comparison * Only ClientAuth and certs can be set with option for more tls configs --- dataclients/kubernetes/ingressv1.go | 6 +- dataclients/kubernetes/kube.go | 17 +--- dataclients/kubernetes/routegroup.go | 4 +- secrets/certregistry/certregistry.go | 130 +++++++++++++++------------ skipper.go | 2 +- 5 files changed, 81 insertions(+), 78 deletions(-) diff --git a/dataclients/kubernetes/ingressv1.go b/dataclients/kubernetes/ingressv1.go index 8b2464048d..9a7139d1c0 100644 --- a/dataclients/kubernetes/ingressv1.go +++ b/dataclients/kubernetes/ingressv1.go @@ -1,7 +1,6 @@ package kubernetes import ( - "crypto/tls" "errors" "fmt" "regexp" @@ -318,13 +317,12 @@ func (ing *ingress) addSpecIngressTLSV1(ic *ingressContext, ingtls *definitions. ic.logger.Errorf("Failed to find secret %s in namespace %s", secretID.Name, secretID.Namespace) return } - addTLSCertToRegistry(ic.certificateRegistry, ic.logger, hostlist, secret) // Set tls config for all hosts defined in the ingress - tlsConfig := &tls.Config{ + tlsConfig := &certregistry.Config{ ClientAuth: ic.tlsClientAuth, } - addTLSConfigToRegistry(ic.certificateRegistry, ic.logger, hostlist, tlsConfig) + addTLSConfigToRegistry(ic.certificateRegistry, ic.logger, hostlist, tlsConfig, secret) } // converts the default backend if any diff --git a/dataclients/kubernetes/kube.go b/dataclients/kubernetes/kube.go index 613c5bf0f6..afb4e8e6de 100644 --- a/dataclients/kubernetes/kube.go +++ b/dataclients/kubernetes/kube.go @@ -623,25 +623,16 @@ func compareStringList(a, b []string) []string { return c } -// addTLSCertToRegistry adds a TLS certificate to the certificate registry per host using the provided -// Kubernetes TLS secret -func addTLSCertToRegistry(cr *certregistry.CertRegistry, logger *logger, hosts []string, secret *secret) { +// addTLSConfigToRegistry adds a TLS Config to the registry per host using the provided config and secret. +func addTLSConfigToRegistry(cr *certregistry.CertRegistry, logger *logger, hosts []string, config *certregistry.Config, secret *secret) { cert, err := generateTLSCertFromSecret(secret) if err != nil { logger.Errorf("Failed to generate TLS certificate from secret: %v", err) return } for _, host := range hosts { - err := cr.ConfigureCertificate(host, cert) - if err != nil { - logger.Errorf("Failed to configure certificate: %v", err) - } - } -} - -func addTLSConfigToRegistry(cr *certregistry.CertRegistry, logger *logger, hosts []string, tlsConfig *tls.Config) { - for _, host := range hosts { - err := cr.ConfigureTLSConfig(host, tlsConfig) + config.Certificate = *cert + err := cr.SetTLSConfig(host, config) if err != nil { logger.Errorf("Failed to configure TLS config: %v", err) } diff --git a/dataclients/kubernetes/routegroup.go b/dataclients/kubernetes/routegroup.go index 4ec23928f9..9b64edb4ee 100644 --- a/dataclients/kubernetes/routegroup.go +++ b/dataclients/kubernetes/routegroup.go @@ -512,7 +512,9 @@ func (r *routeGroups) addRouteGroupTLS(ctx *routeGroupContext, tls *definitions. ctx.logger.Errorf("Failed to find secret %s in namespace %s", secretID.Name, secretID.Namespace) return } - addTLSCertToRegistry(ctx.certificateRegistry, ctx.logger, hostlist, secret) + + config := &certregistry.Config{} + addTLSConfigToRegistry(ctx.certificateRegistry, ctx.logger, hostlist, config, secret) } diff --git a/secrets/certregistry/certregistry.go b/secrets/certregistry/certregistry.go index ad327eb4a4..07b0f20d96 100644 --- a/secrets/certregistry/certregistry.go +++ b/secrets/certregistry/certregistry.go @@ -1,6 +1,7 @@ package certregistry import ( + "bytes" "crypto/tls" "crypto/x509" "fmt" @@ -9,94 +10,77 @@ import ( log "github.com/sirupsen/logrus" ) -// CertRegistryEntry holds a certificate and tls configuration. -type CertRegistryEntry struct { - Certificate *tls.Certificate - Config *tls.Config +// Config holds a certificate registry TLS configuration. +type Config struct { + ClientAuth tls.ClientAuthType + Certificate tls.Certificate } -// CertRegistry object holds TLS certificates to be used to terminate TLS connections +// CertRegistry object holds TLS Config to be used to terminate TLS connections // ensuring synchronized access to them. type CertRegistry struct { mu sync.Mutex - lookup map[string]*CertRegistryEntry + lookup map[string]*tlsConfigWrapper + + // defaultTLSConfig is TLS config to be used as a base config for all host configs. + defaultConfig *tls.Config +} + +// tlsConfigWrapper holds the tls.Config and a hash of a host configuration. +type tlsConfigWrapper struct { + config *tls.Config + hash []byte } // NewCertRegistry initializes the certificate registry. func NewCertRegistry() *CertRegistry { - l := make(map[string]*CertRegistryEntry) + l := make(map[string]*tlsConfigWrapper) return &CertRegistry{ - lookup: l, + lookup: l, + defaultConfig: &tls.Config{}, } } -// Configures certificate for the host if no configuration exists or -// if certificate is valid (`NotBefore` field) after previously configured certificate. -func (r *CertRegistry) ConfigureCertificate(host string, cert *tls.Certificate) error { - if cert == nil { - return fmt.Errorf("cannot configure nil certificate") +// Configures TLS for the host if no configuration exists or +// if config certificate is valid (`NotBefore` field) after previously configured certificate. +func (r *CertRegistry) SetTLSConfig(host string, config *Config) error { + if config == nil { + return fmt.Errorf("cannot configure nil tls config") } // loading parsed leaf certificate to certificate - leaf, err := x509.ParseCertificate(cert.Certificate[0]) + leaf, err := x509.ParseCertificate(config.Certificate.Certificate[0]) if err != nil { return fmt.Errorf("failed parsing leaf certificate: %w", err) } - cert.Leaf = leaf + config.Certificate.Leaf = leaf + + // Get tls.config and hash from the config + tlsConfig, configHash := r.configToTLSConfig(config) r.mu.Lock() defer r.mu.Unlock() + // Check if the config is already set curr, found := r.lookup[host] - if found { - if cert.Leaf.NotBefore.After(curr.Certificate.Leaf.NotBefore) { - log.Infof("updating certificate in registry - %s", host) - r.lookup[host].Certificate = cert - return nil - } else { - return nil - } - } else { - log.Infof("adding certificate to registry - %s", host) - r.lookup[host].Certificate = cert + if found && bytes.Equal(curr.hash, configHash) { return nil } -} -// ConfigureTLSConfig configures a tls config for the host. -func (r *CertRegistry) ConfigureTLSConfig(host string, config *tls.Config) error { - if config == nil { - return fmt.Errorf("cannot configure nil tls config") - } - - r.mu.Lock() - defer r.mu.Unlock() - - curr, found := r.lookup[host] - - if found && curr.Config != nil { - log.Infof("updating tls config for host - %s", host) - r.lookup[host].Config = config - return nil - } else { - log.Infof("adding tls config for host - %s", host) - r.lookup[host] = &CertRegistryEntry{ - Config: config, + if found && !bytes.Equal(curr.hash, configHash) { + if !config.Certificate.Leaf.NotBefore.After(curr.config.Certificates[0].Leaf.NotBefore) { + return nil } - return nil } -} -// GetCertFromHello reads the SNI from a TLS client and returns the appropriate certificate. -// If no certificate is found for the host it will return nil. -func (r *CertRegistry) GetCertFromHello(hello *tls.ClientHelloInfo) (*tls.Certificate, error) { - r.mu.Lock() - entry, found := r.lookup[hello.ServerName] - r.mu.Unlock() - if found { - return entry.Certificate, nil + log.Infof("setting tls config in registry - %s", host) + wrapper := &tlsConfigWrapper{ + config: tlsConfig, + hash: configHash, } - return nil, nil + r.lookup[host] = wrapper + + return nil } // GetConfigFromHello reads the SNI from a TLS client and returns the appropriate config. @@ -105,7 +89,35 @@ func (r *CertRegistry) GetConfigFromHello(hello *tls.ClientHelloInfo) (*tls.Conf entry, found := r.lookup[hello.ServerName] r.mu.Unlock() if found { - return entry.Config, nil + return entry.config, nil + } + return entry.config, nil +} + +// configToTLSConfig converts a Config to a tls.Config and returns the hash of the config. +func (r *CertRegistry) configToTLSConfig(config *Config) (*tls.Config, []byte) { + if config == nil { + return nil, nil + } + + var hash []byte + + tlsConfig := r.defaultConfig.Clone() + + // Add client auth settings + tlsConfig.ClientAuth = config.ClientAuth + hash = append(hash, byte(config.ClientAuth>>8), byte(config.ClientAuth)) + + // Add certificate + tlsConfig.Certificates = append(tlsConfig.Certificates, config.Certificate) + for _, certData := range config.Certificate.Certificate { + hash = append(hash, certData...) } - return entry.Config, nil + + return tlsConfig, hash +} + +// SetDefaultTLSConfig sets the default TLS config which should be used as a base for all host specific configs. +func (r *CertRegistry) SetDefaultTLSConfig(config *tls.Config) { + r.defaultConfig = config } diff --git a/skipper.go b/skipper.go index abe75ba4c4..ecf3a430d0 100644 --- a/skipper.go +++ b/skipper.go @@ -1210,7 +1210,7 @@ func (o *Options) tlsConfig(cr *certregistry.CertRegistry) (*tls.Config, error) } if cr != nil { - config.GetCertificate = cr.GetCertFromHello + cr.SetDefaultTLSConfig(config) config.GetConfigForClient = cr.GetConfigFromHello }