Share one filterCiipherSuites (istio#41364)

WillardHu · Oct 11, 2022 · 1f11ac0 · 1f11ac0
1 parent ca49211
commit 1f11ac0
Show file tree

Hide file tree

Showing 4 changed files with 25 additions and 42 deletions.
diff --git a/pilot/pkg/networking/core/v1alpha3/gateway.go b/pilot/pkg/networking/core/v1alpha3/gateway.go
@@ -46,7 +46,6 @@ import (
 	"istio.io/istio/pkg/config/security"
 	"istio.io/istio/pkg/proto"
 	"istio.io/istio/pkg/util/istiomultierror"
-	"istio.io/istio/pkg/util/sets"
 	"istio.io/pkg/log"
 )
 
@@ -686,7 +685,7 @@ func buildGatewayListenerTLSContext(
 		return nil // We don't need to setup TLS context for passthrough mode
 	}
 
-	server.Tls.CipherSuites = filteredGatewayCipherSuites(server)
+	server.Tls.CipherSuites = security.FilterCipherSuites(server.Tls.CipherSuites)
 	return BuildListenerTLSContext(server.Tls, proxy, transportProtocol, gateway.IsTCPServerWithTLSTermination(server))
 }
 
@@ -1034,22 +1033,3 @@ func buildGatewayVirtualHostDomains(node *model.Proxy, hostname string, port int
 	}
 	return domains
 }
-
-// Invalid cipher suites lead Envoy to NACKing. This filters the list down to just the supported set.
-func filteredGatewayCipherSuites(server *networking.Server) []string {
-	suites := server.Tls.CipherSuites
-	ret := make([]string, 0, len(suites))
-	validCiphers := sets.New[string]()
-	for _, s := range suites {
-		if security.IsValidCipherSuite(s) {
-			if !validCiphers.InsertContains(s) {
-				ret = append(ret, s)
-			} else if log.DebugEnabled() {
-				log.Debugf("ignoring duplicated cipherSuite: %q for server %s", s, server.String())
-			}
-		} else if log.DebugEnabled() {
-			log.Debugf("ignoring unsupported cipherSuite: %q for server %s", s, server.String())
-		}
-	}
-	return ret
-}
diff --git a/pilot/pkg/networking/core/v1alpha3/listener.go b/pilot/pkg/networking/core/v1alpha3/listener.go
@@ -43,10 +43,8 @@ import (
 	"istio.io/istio/pkg/config/constants"
 	"istio.io/istio/pkg/config/host"
 	"istio.io/istio/pkg/config/protocol"
-	"istio.io/istio/pkg/config/security"
 	"istio.io/istio/pkg/proto"
 	secconst "istio.io/istio/pkg/security"
-	"istio.io/istio/pkg/util/sets"
 	"istio.io/pkg/log"
 	"istio.io/pkg/monitoring"
 )
@@ -200,24 +198,6 @@ func BuildListenerTLSContext(serverTLSSettings *networking.ServerTLSSettings,
 	return ctx
 }
 
-// Invalid cipher suites lead Envoy to NACKing. This filters the list down to just the supported set.
-func filteredSidecarCipherSuites(suites []string) []string {
-	ret := make([]string, 0, len(suites))
-	validCiphers := sets.New[string]()
-	for _, s := range suites {
-		if security.IsValidCipherSuite(s) {
-			if !validCiphers.InsertContains(s) {
-				ret = append(ret, s)
-			} else if log.DebugEnabled() {
-				log.Debugf("ignoring duplicated cipherSuite: %q", s)
-			}
-		} else if log.DebugEnabled() {
-			log.Debugf("ignoring unsupported cipherSuite: %q", s)
-		}
-	}
-	return ret
-}
-
 // buildSidecarListeners produces a list of listeners for sidecar proxies
 func (configgen *ConfigGeneratorImpl) buildSidecarListeners(builder *ListenerBuilder) *ListenerBuilder {
 	if builder.push.Mesh.ProxyListenPort > 0 {

diff --git a/pilot/pkg/networking/core/v1alpha3/listener_inbound.go b/pilot/pkg/networking/core/v1alpha3/listener_inbound.go
@@ -42,6 +42,7 @@ import (
 	xdsfilters "istio.io/istio/pilot/pkg/xds/filters"
 	"istio.io/istio/pkg/config/host"
 	"istio.io/istio/pkg/config/protocol"
+	"istio.io/istio/pkg/config/security"
 	"istio.io/istio/pkg/proto"
 	"istio.io/pkg/log"
 )
@@ -367,7 +368,7 @@ func (lb *ListenerBuilder) buildInboundChainConfigs() []inboundChainConfig {
 			if i.Tls != nil && features.EnableTLSOnSidecarIngress {
 				// User provided custom TLS settings
 				cc.tlsSettings = i.Tls.DeepCopy()
-				cc.tlsSettings.CipherSuites = filteredSidecarCipherSuites(cc.tlsSettings.CipherSuites)
+				cc.tlsSettings.CipherSuites = security.FilterCipherSuites(cc.tlsSettings.CipherSuites)
 				cc.port.Protocol = cc.port.Protocol.AfterTLSTermination()
 			}
 

diff --git a/pkg/config/security/security.go b/pkg/config/security/security.go
@@ -26,6 +26,7 @@ import (
 
 	"istio.io/istio/pkg/config/host"
 	"istio.io/istio/pkg/util/sets"
+	"istio.io/pkg/log"
 )
 
 // JwksInfo provides values resulting from parsing a jwks URI.
@@ -215,3 +216,24 @@ func IsValidCipherSuite(cs string) bool {
 	}
 	return ValidCipherSuites.Contains(cs)
 }
+
+// FilterCipherSuites filters out invalid cipher suites which would lead Envoy to NACKing.
+func FilterCipherSuites(suites []string) []string {
+	if len(suites) == 0 {
+		return nil
+	}
+	ret := make([]string, 0, len(suites))
+	validCiphers := sets.New[string]()
+	for _, s := range suites {
+		if IsValidCipherSuite(s) {
+			if !validCiphers.InsertContains(s) {
+				ret = append(ret, s)
+			} else if log.DebugEnabled() {
+				log.Debugf("ignoring duplicated cipherSuite: %q", s)
+			}
+		} else if log.DebugEnabled() {
+			log.Debugf("ignoring unsupported cipherSuite: %q", s)
+		}
+	}
+	return ret
+}