Use authentication policy to service-to-service mTLS. (istio#4089)

* Use authentication policy to service-to-service mTLS.

* Lint

* Remove pre-mature changes. authn policy doen't need to be part of buildListenerOpts yet.

* Remove unused definitions.

* Add e2e test case.

pilot/pkg/proxy/envoy/v1/authentication.go

@@ -0,0 +1,93 @@
+// Copyright 2018 Istio Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+// AuthN filter configuration
+
+package v1
+
+import (
+	"fmt"
+
+	authn "istio.io/api/authentication/v1alpha1"
+	meshconfig "istio.io/api/mesh/v1alpha1"
+	"istio.io/istio/pilot/pkg/model"
+	"istio.io/istio/pkg/log"
+)
+
+// getConsolidateAuthenticationPolicy returns the authentication policy for
+// service specified by hostname and port, if defined.
+// If not, it generates and output a policy that is equivalent to the legacy flag
+// and/or service annotation. Once these legacy flags/config deprecated,
+// this function can be placed by a call to store.AuthenticationPolicyByDestination
+// directly.
+func getConsolidateAuthenticationPolicy(mesh *meshconfig.MeshConfig, store model.IstioConfigStore, hostname string, port *model.Port) *authn.Policy {
+	config := store.AuthenticationPolicyByDestination(hostname, port)
+	if config == nil {
+		legacyPolicy := consolidateAuthPolicy(mesh, port.AuthenticationPolicy)
+		log.Debugf("No authentication policy found for  %s:%d. Fallback to legacy authentication mode %v\n",
+			hostname, port.Port, legacyPolicy)
+		return legacyAuthenticationPolicyToPolicy(legacyPolicy)
+	}
+
+	return config.Spec.(*authn.Policy)
+}
+
+// consolidateAuthPolicy returns service auth policy, if it's not INHERIT. Else,
+// returns mesh policy.
+func consolidateAuthPolicy(mesh *meshconfig.MeshConfig,
+	serviceAuthPolicy meshconfig.AuthenticationPolicy) meshconfig.AuthenticationPolicy {
+	if serviceAuthPolicy != meshconfig.AuthenticationPolicy_INHERIT {
+		return serviceAuthPolicy
+	}
+	// TODO: use AuthenticationPolicy for mesh policy and remove this conversion
+	switch mesh.AuthPolicy {
+	case meshconfig.MeshConfig_MUTUAL_TLS:
+		return meshconfig.AuthenticationPolicy_MUTUAL_TLS
+	case meshconfig.MeshConfig_NONE:
+		return meshconfig.AuthenticationPolicy_NONE
+	default:
+		// Never get here, there are no other enum value for mesh.AuthPolicy.
+		panic(fmt.Sprintf("Unknown mesh auth policy: %v\n", mesh.AuthPolicy))
+	}
+}
+
+// If input legacy is AuthenticationPolicy_MUTUAL_TLS, return a authentication policy equivalent
+// to it. Else, returns nil (implies no authentication is used)
+func legacyAuthenticationPolicyToPolicy(legacy meshconfig.AuthenticationPolicy) *authn.Policy {
+	if legacy == meshconfig.AuthenticationPolicy_MUTUAL_TLS {
+		return &authn.Policy{
+			Peers: []*authn.PeerAuthenticationMethod{{
+				Params: &authn.PeerAuthenticationMethod_Mtls{}}},
+		}
+	}
+	return nil
+}
+
+// requireTLS returns true if the policy use mTLS for (peer) authentication.
+func requireTLS(policy *authn.Policy) bool {
+	if policy == nil {
+		return false
+	}
+	if len(policy.Peers) > 0 {
+		for _, method := range policy.Peers {
+			switch method.GetParams().(type) {
+			case *authn.PeerAuthenticationMethod_Mtls:
+				return true
+			default:
+				continue
+			}
+		}
+	}
+	return false
+}

pilot/pkg/proxy/envoy/v1/authentication_test.go

@@ -0,0 +1,86 @@
+// Copyright 2018 Istio Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package v1
+
+import (
+	"reflect"
+	"testing"
+
+	authn "istio.io/api/authentication/v1alpha1"
+	meshconfig "istio.io/api/mesh/v1alpha1"
+)
+
+func TestRequireTls(t *testing.T) {
+	cases := []struct {
+		in       authn.Policy
+		expected bool
+	}{
+		{
+			in:       authn.Policy{},
+			expected: false,
+		},
+		{
+			in: authn.Policy{
+				Peers: []*authn.PeerAuthenticationMethod{{
+					Params: &authn.PeerAuthenticationMethod_Mtls{},
+				}},
+			},
+			expected: true,
+		},
+		{
+			in: authn.Policy{
+				Peers: []*authn.PeerAuthenticationMethod{{
+					Params: &authn.PeerAuthenticationMethod_Jwt{},
+				},
+					{
+						Params: &authn.PeerAuthenticationMethod_Mtls{},
+					},
+				},
+			},
+			expected: true,
+		},
+	}
+	for _, c := range cases {
+		if got := requireTLS(&c.in); got != c.expected {
+			t.Errorf("requireTLS(%v): got(%v) != want(%v)\n", c.in, got, c.expected)
+		}
+	}
+}
+
+func TestLegacyAuthenticationPolicyToPolicy(t *testing.T) {
+	cases := []struct {
+		in       meshconfig.AuthenticationPolicy
+		expected *authn.Policy
+	}{
+		{
+			in: meshconfig.AuthenticationPolicy_MUTUAL_TLS,
+			expected: &authn.Policy{
+				Peers: []*authn.PeerAuthenticationMethod{{
+					Params: &authn.PeerAuthenticationMethod_Mtls{},
+				}},
+			},
+		},
+		{
+			in:       meshconfig.AuthenticationPolicy_NONE,
+			expected: nil,
+		},
+	}
+
+	for _, c := range cases {
+		if got := legacyAuthenticationPolicyToPolicy(c.in); !reflect.DeepEqual(got, c.expected) {
+			t.Errorf("legacyAuthenticationPolicyToPolicy(%v): got(%#v) != want(%#v)\n", c.in, got, c.expected)
+		}
+	}
+}

pilot/pkg/proxy/envoy/v1/config.go

@@ -28,6 +28,7 @@ import (
 
 	multierror "github.com/hashicorp/go-multierror"
 
+	authn "istio.io/api/authentication/v1alpha1"
 	meshconfig "istio.io/api/mesh/v1alpha1"
 	networking "istio.io/api/networking/v1alpha3"
 	routing "istio.io/api/routing/v1alpha1"
@@ -448,28 +449,9 @@ func buildHTTPListener(opts buildHTTPListenerOpts) *Listener {
 	}
 }
 
-// consolidateAuthPolicy returns service auth policy, if it's not INHERIT. Else,
-// returns mesh policy.
-func consolidateAuthPolicy(mesh *meshconfig.MeshConfig, serviceAuthPolicy meshconfig.AuthenticationPolicy) meshconfig.AuthenticationPolicy {
-	if serviceAuthPolicy != meshconfig.AuthenticationPolicy_INHERIT {
-		return serviceAuthPolicy
-	}
-	// TODO: use AuthenticationPolicy for mesh policy and remove this conversion
-	switch mesh.AuthPolicy {
-	case meshconfig.MeshConfig_MUTUAL_TLS:
-		return meshconfig.AuthenticationPolicy_MUTUAL_TLS
-	case meshconfig.MeshConfig_NONE:
-		return meshconfig.AuthenticationPolicy_NONE
-	default:
-		// Never get here, there are no other enum value for mesh.AuthPolicy.
-		panic(fmt.Sprintf("Unknown mesh auth policy: %v\n", mesh.AuthPolicy))
-	}
-}
-
-// mayApplyInboundAuth adds ssl_context to the listener if consolidateAuthPolicy.
-func mayApplyInboundAuth(listener *Listener, mesh *meshconfig.MeshConfig,
-	serviceAuthPolicy meshconfig.AuthenticationPolicy) {
-	if consolidateAuthPolicy(mesh, serviceAuthPolicy) == meshconfig.AuthenticationPolicy_MUTUAL_TLS {
+// mayApplyInboundAuth adds ssl_context to the listener if the given authN policy require TLS.
+func mayApplyInboundAuth(listener *Listener, authenticationPolicy *authn.Policy) {
+	if requireTLS(authenticationPolicy) {
 		listener.SSLContext = buildListenerSSLContext(model.AuthCertsPath)
 	}
 }
@@ -903,7 +885,9 @@ func buildInboundListeners(mesh *meshconfig.MeshConfig, node model.Proxy,
 		}
 
 		if listener != nil {
-			mayApplyInboundAuth(listener, mesh, endpoint.ServicePort.AuthenticationPolicy)
+			authenticationPolicy := getConsolidateAuthenticationPolicy(mesh,
+				config, instance.Service.Hostname, endpoint.ServicePort)
+			mayApplyInboundAuth(listener, authenticationPolicy)
 			listeners = append(listeners, listener)
 		}
 	}

pilot/pkg/proxy/envoy/v1/config_test.go

@@ -507,6 +507,26 @@ var (
 		meta: model.ConfigMeta{Type: model.EndUserAuthenticationPolicySpecBinding.Type, Name: "auth-spec-binding"},
 		file: "testdata/auth-spec-binding.yaml.golden",
 	}
+
+	authnPolicyNamespaceMTlsOff = fileConfig{
+		meta: model.ConfigMeta{Type: model.AuthenticationPolicy.Type, Name: "authn-namespace-mtls-off"},
+		file: "testdata/authn-namespace-mtls-off.yaml.golden",
+	}
+
+	authnPolicyNamespaceMTlsOn = fileConfig{
+		meta: model.ConfigMeta{Type: model.AuthenticationPolicy.Type, Name: "authn-namespace-mtls-on"},
+		file: "testdata/authn-namespace-mtls-on.yaml.golden",
+	}
+
+	authnPolicyHelloMTlsOff = fileConfig{
+		meta: model.ConfigMeta{Type: model.AuthenticationPolicy.Type, Name: "authn-hello-mtls-off"},
+		file: "testdata/authn-hello-mtls-off.yaml.golden",
+	}
+
+	authnPolicyWorldMTlsOff = fileConfig{
+		meta: model.ConfigMeta{Type: model.AuthenticationPolicy.Type, Name: "authn-world-mtls-on"},
+		file: "testdata/authn-world-mtls-off.yaml.golden",
+	}
 )
 
 func addConfig(r model.ConfigStore, config fileConfig, t *testing.T) {

pilot/pkg/proxy/envoy/v1/discovery_test.go

@@ -328,6 +328,36 @@ func TestClusterDiscoveryWithSecurityOn(t *testing.T) {
 	compareResponse(response, "testdata/cds-ssl-context.json", t)
 }
 
+func TestClusterDiscoveryWithSecurityOnByByAuthenticationPolicy(t *testing.T) {
+	// This test enable mesh mTLS using authN policy. Currently, the broadest scope
+	// for policy is namespace. However, this test setup has only one namespace, so
+	// the effect is the same as using mesh config flag (i.e
+	// TestClusterDiscoveryWithSecurityOn)
+	_, registry, ds := commonSetup(t)
+	addConfig(registry, egressRule, t) // original dst cluster should not have auth
+	addConfig(registry, authnPolicyNamespaceMTlsOn, t)
+
+	url := fmt.Sprintf("/v1/clusters/%s/%s", "istio-proxy", mock.HelloProxyV0.ServiceNode())
+	response := makeDiscoveryRequest(ds, "GET", url, t)
+	compareResponse(response, "testdata/cds-ssl-context.json", t)
+}
+
+func TestClusterDiscoveryWithSecurityOffByByAuthenticationPolicy(t *testing.T) {
+	// This test shows mesh config AuthPolicy will be overridden by policy. The
+	// test will enable mTLS via mesh flag, but then disable with authn policy. The
+	// end result should be equivalent to mesh's auth policy is disable in the
+	// first place (i.e TestClusterDiscovery)
+	mesh := makeMeshConfig()
+	mesh.AuthPolicy = meshconfig.MeshConfig_MUTUAL_TLS
+	registry := memory.Make(model.IstioConfigTypes)
+	addConfig(registry, authnPolicyNamespaceMTlsOff, t)
+
+	ds := makeDiscoveryService(t, registry, &mesh)
+	url := fmt.Sprintf("/v1/clusters/%s/%s", "istio-proxy", mock.HelloProxyV0.ServiceNode())
+	response := makeDiscoveryRequest(ds, "GET", url, t)
+	compareResponse(response, "testdata/cds.json", t)
+}
+
 func TestClusterDiscoveryWithAuthOptOut(t *testing.T) {
 	mesh := makeMeshConfig()
 	mesh.AuthPolicy = meshconfig.MeshConfig_MUTUAL_TLS
@@ -346,6 +376,23 @@ func TestClusterDiscoveryWithAuthOptOut(t *testing.T) {
 	mock.WorldService.Ports[0].AuthenticationPolicy = meshconfig.AuthenticationPolicy_INHERIT
 }
 
+func TestClusterDiscoveryWithOptOutByAuthenticationPolicy(t *testing.T) {
+	// This test using authentication policies (CRD) to enable mTLS for the whole
+	// namespace (we don't support global policy yet, but for this test setup, it's
+	// the same as global since we have only one namespace), and disable mTLS for
+	// 'world' service. In other words, this has the same effect as
+	// TestClusterDiscoveryWithAuthOptOut above.
+	_, registry, ds := commonSetup(t)
+	addConfig(registry, egressRule, t) // original dst cluster should not have auth
+
+	addConfig(registry, authnPolicyNamespaceMTlsOn, t)
+	addConfig(registry, authnPolicyWorldMTlsOff, t)
+
+	url := fmt.Sprintf("/v1/clusters/%s/%s", "istio-proxy", mock.HelloProxyV0.ServiceNode())
+	response := makeDiscoveryRequest(ds, "GET", url, t)
+	compareResponse(response, "testdata/cds-ssl-context-optout.json", t)
+}
+
 func TestClusterDiscoveryIngress(t *testing.T) {
 	_, registry, ds := commonSetup(t)
 	addIngressRoutes(registry, t)
@@ -880,6 +927,19 @@ func TestListenerDiscoverySidecarAuthOptOut(t *testing.T) {
 	mock.HelloService.Ports[0].AuthenticationPolicy = meshconfig.AuthenticationPolicy_INHERIT
 }
 
+func TestListenerDiscoverySidecarAuthOptOutByByAuthenticationPolicy(t *testing.T) {
+	// This test using authentication policies (CRD) to enable mTLS for the whole
+	// namespace and disable for Hello service. In other words, it has the same effect
+	// as TestListenerDiscoverySidecarAuthOptOut above.
+	_, registry, ds := commonSetup(t)
+	addConfig(registry, authnPolicyNamespaceMTlsOn, t)
+	addConfig(registry, authnPolicyHelloMTlsOff, t)
+
+	url := fmt.Sprintf("/v1/listeners/%s/%s", "istio-proxy", mock.HelloProxyV0.ServiceNode())
+	response := makeDiscoveryRequest(ds, "GET", url, t)
+	compareResponse(response, "testdata/lds-v0-none-auth-optout.json", t)
+}
+
 func TestRouteDiscoverySidecarError(t *testing.T) {
 	_, _, ds := commonSetup(t)
 	mockDiscovery.ServicesError = errors.New("mock Services() error")

pilot/pkg/proxy/envoy/v1/policy.go

@@ -54,7 +54,7 @@ func ApplyClusterPolicy(cluster *Cluster,
 	// where Istio auth does not apply.
 	if cluster.Type != ClusterTypeOriginalDST {
 		if !isDestinationExcludedForMTLS(cluster.ServiceName, mesh.MtlsExcludedServices) &&
-			consolidateAuthPolicy(mesh, cluster.Port.AuthenticationPolicy) == meshconfig.AuthenticationPolicy_MUTUAL_TLS {
+			requireTLS(getConsolidateAuthenticationPolicy(mesh, config, cluster.Hostname, cluster.Port)) {
 			// apply auth policies
 			ports := model.PortList{cluster.Port}.GetNames()
 			serviceAccounts := accounts.GetIstioServiceAccounts(cluster.Hostname, ports)

pilot/pkg/proxy/envoy/v1/testdata/authn-hello-mtls-off.yaml.golden

@@ -0,0 +1,6 @@
+destinations:
+- name: hello
+  port:
+    name: "http"
+peers:
+- none: null

pilot/pkg/proxy/envoy/v1/testdata/authn-namespace-mtls-off.yaml.golden

@@ -0,0 +1,2 @@
+peers:
+- none: null

pilot/pkg/proxy/envoy/v1/testdata/authn-namespace-mtls-on.yaml.golden

@@ -0,0 +1,2 @@
+peers:
+- mtls: null

pilot/pkg/proxy/envoy/v1/testdata/authn-world-mtls-off.yaml.golden

@@ -0,0 +1,6 @@
+destinations:
+- name: world
+  port:
+    name: "http"
+peers:
+- none: null