fix proxyless gRPC mTLS (#36033)

* don't set sans for serverside lds Change-Id: Ib9efd027ff730d49bcd10c3766e725a5e984fcbb * fix golden Change-Id: I839476909a44dff065626bdb878a2ba3e6c6459c
istio · Nov 11, 2021 · 50fcf17 · 50fcf17
1 parent e761f98
commit 50fcf17
Show file tree

Hide file tree

Showing 5 changed files with 54 additions and 10 deletions.
diff --git a/pilot/pkg/networking/grpcgen/grpcecho_test.go b/pilot/pkg/networking/grpcgen/grpcecho_test.go
@@ -233,7 +233,6 @@ spec:
 }
 
 func TestMtls(t *testing.T) {
-	t.Skip("https://github.com/istio/istio/issues/35843")
 	tt := newConfigGenTest(t, xds.FakeOptions{
 		KubernetesObjectString: `
 apiVersion: v1

diff --git a/pilot/pkg/networking/grpcgen/grpcgen.go b/pilot/pkg/networking/grpcgen/grpcgen.go
@@ -16,6 +16,7 @@ package grpcgen
 
 import (
 	tls "github.com/envoyproxy/go-control-plane/envoy/extensions/transport_sockets/tls/v3"
+	matcher "github.com/envoyproxy/go-control-plane/envoy/type/matcher/v3"
 
 	"istio.io/istio/pilot/pkg/model"
 	"istio.io/istio/pilot/pkg/networking/util"
@@ -69,6 +70,10 @@ func (g *GrpcConfigGenerator) Generate(proxy *model.Proxy, push *model.PushConte
 // buildCommonTLSContext creates a TLS context that assumes 'default' name, and credentials/tls/certprovider/pemfile
 // (see grpc/xds/internal/client/xds.go securityConfigFromCluster).
 func buildCommonTLSContext(sans []string) *tls.CommonTlsContext {
+	var sanMatch []*matcher.StringMatcher
+	if len(sans) > 0 {
+		sanMatch = util.StringToExactMatch(sans)
+	}
 	return &tls.CommonTlsContext{
 		TlsCertificateCertificateProviderInstance: &tls.CommonTlsContext_CertificateProviderInstance{
 			InstanceName:    "default",
@@ -81,7 +86,7 @@ func buildCommonTLSContext(sans []string) *tls.CommonTlsContext {
 					CertificateName: "ROOTCA",
 				},
 				DefaultValidationContext: &tls.CertificateValidationContext{
-					MatchSubjectAltNames: util.StringToExactMatch(sans),
+					MatchSubjectAltNames: sanMatch,
 				},
 			},
 		},

diff --git a/pilot/pkg/networking/grpcgen/lds.go b/pilot/pkg/networking/grpcgen/lds.go
@@ -29,7 +29,6 @@ import (
 	wrappers "google.golang.org/protobuf/types/known/wrapperspb"
 
 	"istio.io/istio/pilot/pkg/model"
-	authnplugin "istio.io/istio/pilot/pkg/networking/plugin/authn"
 	"istio.io/istio/pilot/pkg/networking/util"
 	"istio.io/istio/pilot/pkg/security/authn"
 	"istio.io/istio/pilot/pkg/security/authn/factory"
@@ -98,7 +97,7 @@ func buildInboundListeners(node *model.Proxy, push *model.PushContext, names []s
 					},
 				},
 			}},
-			FilterChains: buildFilterChains(node, push, si, policyApplier),
+			FilterChains: buildInboundFilterChains(node, push, si, policyApplier),
 			// the following must not be set or the client will NACK
 			ListenerFilters: nil,
 			UseOriginalDst:  nil,
@@ -111,13 +110,16 @@ func buildInboundListeners(node *model.Proxy, push *model.PushContext, names []s
 	return out
 }
 
-func buildFilterChains(node *model.Proxy, push *model.PushContext, si *model.ServiceInstance, applier authn.PolicyApplier) []*listener.FilterChain {
+// nolint: unparam
+func buildInboundFilterChains(node *model.Proxy, push *model.PushContext, si *model.ServiceInstance, applier authn.PolicyApplier) []*listener.FilterChain {
 	mode := applier.GetMutualTLSModeForPort(si.Endpoint.EndpointPort)
 
 	var tlsContext *tls.DownstreamTlsContext
 	if mode != model.MTLSDisable && mode != model.MTLSUnknown {
 		tlsContext = &tls.DownstreamTlsContext{
-			CommonTlsContext: buildCommonTLSContext(authnplugin.TrustDomainsForValidation(push.Mesh)),
+			CommonTlsContext: buildCommonTLSContext(nil),
+			// TODO match_subject_alt_names field in validation context is not supported on the server
+			// CommonTlsContext: buildCommonTLSContext(authnplugin.TrustDomainsForValidation(push.Mesh)),
 			// TODO plain TLS support
 			RequireClientCertificate: &wrappers.BoolValue{Value: true},
 		}
@@ -137,16 +139,16 @@ func buildFilterChains(node *model.Proxy, push *model.PushContext, si *model.Ser
 	var out []*listener.FilterChain
 	switch mode {
 	case model.MTLSDisable:
-		out = append(out, buildFilterChain("plaintext", nil))
+		out = append(out, buildInboundFilterChain("plaintext", nil))
 	case model.MTLSStrict:
-		out = append(out, buildFilterChain("mtls", tlsContext))
+		out = append(out, buildInboundFilterChain("mtls", tlsContext))
 		// TODO permissive builts both plaintext and mtls; when tlsContext is present add a match for protocol
 	}
 
 	return out
 }
 
-func buildFilterChain(nameSuffix string, tlsContext *tls.DownstreamTlsContext) *listener.FilterChain {
+func buildInboundFilterChain(nameSuffix string, tlsContext *tls.DownstreamTlsContext) *listener.FilterChain {
 	out := &listener.FilterChain{
 		Name:             "inbound-" + nameSuffix,
 		FilterChainMatch: nil,

diff --git a/pkg/istio-agent/grpcxds/grpc_bootstrap.go b/pkg/istio-agent/grpcxds/grpc_bootstrap.go
@@ -28,6 +28,7 @@ import (
 	"istio.io/istio/pilot/pkg/model"
 	"istio.io/istio/pkg/file"
 	"istio.io/istio/pkg/util/protomarshal"
+	"istio.io/pkg/log"
 )
 
 const (
@@ -65,6 +66,43 @@ type CertificateProvider struct {
 	Config     interface{} `json:"config,omitempty"`
 }
 
+func (cp *CertificateProvider) UnmarshalJSON(data []byte) error {
+	var dat map[string]*json.RawMessage
+	if err := json.Unmarshal(data, &dat); err != nil {
+		return err
+	}
+	*cp = CertificateProvider{}
+
+	if pluginNameVal, ok := dat["plugin_name"]; ok {
+		if err := json.Unmarshal(*pluginNameVal, &cp.PluginName); err != nil {
+			log.Warnf("failed parsing plugin_name in certificate_provider: %v", err)
+		}
+	} else {
+		log.Warnf("did not find plugin_name in certificate_provider")
+	}
+
+	if configVal, ok := dat["config"]; ok {
+		var err error
+		switch cp.PluginName {
+		case FileWatcherCertProviderName:
+			config := FileWatcherCertProviderConfig{}
+			err = json.Unmarshal(*configVal, &config)
+			cp.Config = config
+		default:
+			config := FileWatcherCertProviderConfig{}
+			err = json.Unmarshal(*configVal, &config)
+			cp.Config = config
+		}
+		if err != nil {
+			log.Warnf("failed parsing config in certificate_provider: %v", err)
+		}
+	} else {
+		log.Warnf("did not find config in certificate_provider")
+	}
+
+	return nil
+}
+
 const FileWatcherCertProviderName = "file_watcher"
 
 type FileWatcherCertProviderConfig struct {

diff --git a/pkg/istio-agent/testdata/grpc-bootstrap.json b/pkg/istio-agent/testdata/grpc-bootstrap.json
@@ -27,9 +27,9 @@
     "default": {
       "plugin_name": "file_watcher",
       "config": {
-        "ca_certificate_file": "/cert/path/root-cert.pem",
         "certificate_file": "/cert/path/cert-chain.pem",
         "private_key_file": "/cert/path/key.pem",
+        "ca_certificate_file": "/cert/path/root-cert.pem",
         "refresh_interval": "900s"
       }
     }