Env Var allowing proxies to utilize OS CA Cert (#33472)

* Env Var allowing proxies to utilize OS CA Cert

* Introduce VERIFY_CERTIFICATE_AT_CLIENT as a bool env var
* Select an existing CA cert bundle from a set of known file locations
* If bool set and DR does not set CaCertificates, replace the proxies'
CA cert with an OS CA cert found in the file system

* Move OS cert replace logic to proxy

* Fix typo

* Move function in pilot code and check for OS cert once

* Move SdsCertificateConfigFromResourceName out of pilot authenticate
code and into utils for agent.
* Determine OS CA certificate in pilot-agent to pass down instead of
searching for the file more often

* Linter and change setting off by default

* Move OS CA Path find to option generation

* Add release notes

* Lock pointers when accessing or using them

* Remove mutex inside Options

* Fix secret resource choice

* Add unit tests

* Fix tests and linter errors

* Improve tests, releasenotes and logging

* Add back code that was needed

* Fix linter issues

* Use string constant in place of file-root:system

* Update to remove proxy from ClusterBuilder

* Fix auto import location

* Refactor code

* Refactor SdsCertificateConfig to common package

* Move security tests from util.go

* Fix releasenotes

* Add documentation to public methods

* Improve VERIFY_CERT_AT_CLIENT testing

* initialize CAFilePath in security instead of main

* Refactor and remove dead code

* Remove code that is no longer needed with CA file path being generated
on init
* Move CACertFilePath init outside of shared package so it only needs to
be made for sdsservice.go and secretcache.go

* Refactor to move cafile package

* Refactor toEnvoySecret funciton

* Revert changes that were needed for istiod to know the OS CA file path

* Fix linter problems

* Correct tests and introduce caRootPath to SecretManager

* SecretManager uses GenerateSecret and to avoid adding another
perameter to the function, SecretManager implementations need to have it
in the type.
* `file-root:` was missing from secretcache_test.go to pull the correct
certificate information

* Fix missing argument for mock initializaiton

* Reverse mock changes and replace string with const

* Fix SdsCertificateConfig to use file-root:system cert.

* Fix tests by removing file-root: prefix

* Fix empty OS CA cert causing an error

* Remove testing print code

* Rebase cluster_builder_test update struct

* Fix rebase changes made to DestiantionRule

pilot/cmd/pilot-agent/options/security.go

@@ -26,6 +26,7 @@ import (
 	"istio.io/istio/pkg/jwt"
 	"istio.io/istio/pkg/security"
 	"istio.io/istio/security/pkg/credentialfetcher"
+	"istio.io/istio/security/pkg/nodeagent/cafile"
 	"istio.io/istio/security/pkg/nodeagent/plugin/providers/google/stsclient"
 	"istio.io/istio/security/pkg/stsservice/tokenmanager"
 	"istio.io/pkg/log"
@@ -52,6 +53,7 @@ func NewSecurityOptions(proxyConfig *meshconfig.ProxyConfig, stsPort int, tokenM
 		SecretRotationGracePeriodRatio: secretRotationGracePeriodRatioEnv,
 		STSPort:                        stsPort,
 		CertSigner:                     certSigner.Get(),
+		CARootPath:                     cafile.CACertFilePath,
 	}
 
 	o, err := SetupSecurityOptions(proxyConfig, o, jwtPolicy.Get(),

pilot/pkg/features/pilot.go

@@ -536,6 +536,9 @@ var (
 	EnableQUICListeners = env.RegisterBoolVar("PILOT_ENABLE_QUIC_LISTENERS", false,
 		"If true, QUIC listeners will be generated wherever there are listeners terminating TLS on gateways "+
 			"if the gateway service exposes a UDP port with the same number (for example 443/TCP and 443/UDP)").Get()
+
+	VerifyCertAtClient = env.RegisterBoolVar("VERIFY_CERTIFICATE_AT_CLIENT", false,
+		"If enabled, certificates received by the proxy will be verified against the OS CA certificate bundle.").Get()
 )
 
 // UnsafeFeaturesEnabled returns true if any unsafe features are enabled.

pilot/pkg/model/authentication.go

@@ -270,58 +270,3 @@ func getConfigsForWorkload(configsByNamespace map[string][]config.Config,
 
 	return configs
 }
-
-// SdsCertificateConfig holds TLS certs needed to build SDS TLS context.
-type SdsCertificateConfig struct {
-	CertificatePath   string
-	PrivateKeyPath    string
-	CaCertificatePath string
-}
-
-// GetResourceName converts a SdsCertificateConfig to a string to be used as an SDS resource name
-func (s SdsCertificateConfig) GetResourceName() string {
-	if s.IsKeyCertificate() {
-		return "file-cert:" + s.CertificatePath + ResourceSeparator + s.PrivateKeyPath // Format: file-cert:%s~%s
-	}
-	return ""
-}
-
-// GetRootResourceName converts a SdsCertificateConfig to a string to be used as an SDS resource name for the root
-func (s SdsCertificateConfig) GetRootResourceName() string {
-	if s.IsRootCertificate() {
-		return "file-root:" + s.CaCertificatePath // Format: file-root:%s
-	}
-	return ""
-}
-
-// IsRootCertificate returns true if this config represents a root certificate config.
-func (s SdsCertificateConfig) IsRootCertificate() bool {
-	return s.CaCertificatePath != ""
-}
-
-// IsKeyCertificate returns true if this config represents key certificate config.
-func (s SdsCertificateConfig) IsKeyCertificate() bool {
-	return s.CertificatePath != "" && s.PrivateKeyPath != ""
-}
-
-// SdsCertificateConfigFromResourceName converts the provided resource name into a SdsCertificateConfig
-// If the resource name is not valid, false is returned.
-func SdsCertificateConfigFromResourceName(resource string) (SdsCertificateConfig, bool) {
-	if strings.HasPrefix(resource, "file-cert:") {
-		filesString := strings.TrimPrefix(resource, "file-cert:")
-		split := strings.Split(filesString, ResourceSeparator)
-		if len(split) != 2 {
-			return SdsCertificateConfig{}, false
-		}
-		return SdsCertificateConfig{split[0], split[1], ""}, true
-	} else if strings.HasPrefix(resource, "file-root:") {
-		filesString := strings.TrimPrefix(resource, "file-root:")
-		split := strings.Split(filesString, ResourceSeparator)
-		if len(split) != 1 {
-			return SdsCertificateConfig{}, false
-		}
-		return SdsCertificateConfig{"", "", split[0]}, true
-	} else {
-		return SdsCertificateConfig{}, false
-	}
-}

pilot/pkg/model/authentication_test.go

@@ -37,75 +37,6 @@ const (
 
 var baseTimestamp = time.Date(2020, 2, 2, 2, 2, 2, 0, time.UTC)
 
-func TestSdsCertificateConfigFromResourceName(t *testing.T) {
-	cases := []struct {
-		name             string
-		resource         string
-		root             bool
-		key              bool
-		output           SdsCertificateConfig
-		rootResourceName string
-		resourceName     string
-	}{
-		{
-			"cert",
-			"file-cert:cert~key",
-			false,
-			true,
-			SdsCertificateConfig{"cert", "key", ""},
-			"",
-			"file-cert:cert~key",
-		},
-		{
-			"root cert",
-			"file-root:root",
-			true,
-			false,
-			SdsCertificateConfig{"", "", "root"},
-			"file-root:root",
-			"",
-		},
-		{
-			"invalid prefix",
-			"file:root",
-			false,
-			false,
-			SdsCertificateConfig{"", "", ""},
-			"",
-			"",
-		},
-		{
-			"invalid contents",
-			"file-root:root~extra",
-			false,
-			false,
-			SdsCertificateConfig{"", "", ""},
-			"",
-			"",
-		},
-	}
-	for _, tt := range cases {
-		t.Run(tt.name, func(t *testing.T) {
-			got, _ := SdsCertificateConfigFromResourceName(tt.resource)
-			if got != tt.output {
-				t.Fatalf("got %v, expected %v", got, tt.output)
-			}
-			if root := got.IsRootCertificate(); root != tt.root {
-				t.Fatalf("unexpected isRootCertificate got %v, expected %v", root, tt.root)
-			}
-			if key := got.IsKeyCertificate(); key != tt.key {
-				t.Fatalf("unexpected IsKeyCertificate got %v, expected %v", key, tt.key)
-			}
-			if root := got.GetRootResourceName(); root != tt.rootResourceName {
-				t.Fatalf("unexpected GetRootResourceName got %v, expected %v", root, tt.rootResourceName)
-			}
-			if cert := got.GetResourceName(); cert != tt.resourceName {
-				t.Fatalf("unexpected GetRootResourceName got %v, expected %v", cert, tt.resourceName)
-			}
-		})
-	}
-}
-
 func TestGetPoliciesForWorkload(t *testing.T) {
 	policies := getTestAuthenticationPolicies(createTestConfigs(true /* with mesh peer authn */), t)
 

pilot/pkg/networking/core/v1alpha3/cluster.go

@@ -543,6 +543,11 @@ func selectTrafficPolicyComponents(policy *networking.TrafficPolicy) (
 	loadBalancer := policy.LoadBalancer
 	tls := policy.Tls
 
+	// Check if CA Certificate should be System CA Certificate
+	if features.VerifyCertAtClient && tls != nil && tls.CaCertificates == "" {
+		tls.CaCertificates = "system"
+	}
+
 	return connectionPool, outlierDetection, loadBalancer, tls
 }
 

pilot/pkg/networking/core/v1alpha3/cluster_builder.go

@@ -50,6 +50,7 @@ import (
 	"istio.io/istio/pkg/config/labels"
 	"istio.io/istio/pkg/config/schema/gvk"
 	"istio.io/istio/pkg/network"
+	"istio.io/istio/pkg/security"
 	"istio.io/istio/pkg/util/gogo"
 	"istio.io/pkg/log"
 )
@@ -1038,7 +1039,7 @@ func (cb *ClusterBuilder) buildUpstreamClusterTLSContext(opts *buildClusterOpts,
 			authn_model.ApplyCustomSDSToClientCommonTLSContext(tlsContext.CommonTlsContext, tls)
 		} else {
 			// If CredentialName is not set fallback to files specified in DR.
-			res := model.SdsCertificateConfig{
+			res := security.SdsCertificateConfig{
 				CaCertificatePath: tls.CaCertificates,
 			}
 			// If tls.CaCertificate or CaCertificate in Metadata isn't configured don't set up SdsSecretConfig
@@ -1082,7 +1083,7 @@ func (cb *ClusterBuilder) buildUpstreamClusterTLSContext(opts *buildClusterOpts,
 			// These are certs being mounted from within the pod and specified in Destination Rules.
 			// Rather than reading directly in Envoy, which does not support rotation, we will
 			// serve them over SDS by reading the files.
-			res := model.SdsCertificateConfig{
+			res := security.SdsCertificateConfig{
 				CertificatePath:   tls.ClientCertificate,
 				PrivateKeyPath:    tls.PrivateKey,
 				CaCertificatePath: tls.CaCertificates,