Certificate Revocation List support (istio#45104)

Signed-off-by: Faseela K <faseela.k@est.tech>

pilot/pkg/credentials/kube/multicluster.go

@@ -132,7 +132,7 @@ func (a *AggregateController) GetCertInfo(name, namespace string) (certInfo *cre
 	return nil, firstError
 }
 
-func (a *AggregateController) GetCaCert(name, namespace string) (cert []byte, err error) {
+func (a *AggregateController) GetCaCert(name, namespace string) (certInfo *credentials.CertInfo, err error) {
 	// Search through all clusters, find first non-empty result
 	var firstError error
 	for _, c := range a.controllers {

pilot/pkg/credentials/kube/secrets.go

@@ -44,6 +44,8 @@ const (
 	GenericScrtKey = "key"
 	// The ID/name for the CA certificate in kubernetes generic secret.
 	GenericScrtCaCert = "cacert"
+	// The ID/name for the CRL in kubernetes generic secret.
+	GenericScrtCRL = "crl"
 
 	// The ID/name for the certificate chain in kubernetes tls secret.
 	TLSSecretCert = "tls.crt"
@@ -53,6 +55,8 @@ const (
 	TLSSecretOcspStaple = "tls.ocsp-staple"
 	// The ID/name for the CA certificate in kubernetes tls secret
 	TLSSecretCaCert = "ca.crt"
+	// The ID/name for the CRL in kubernetes tls secret.
+	TLSSecretCrl = "ca.crl"
 )
 
 type CredentialsController struct {
@@ -175,7 +179,7 @@ func (s *CredentialsController) GetCertInfo(name, namespace string) (certInfo *c
 	return ExtractCertInfo(k8sSecret)
 }
 
-func (s *CredentialsController) GetCaCert(name, namespace string) (cert []byte, err error) {
+func (s *CredentialsController) GetCaCert(name, namespace string) (certInfo *credentials.CertInfo, err error) {
 	k8sSecret := s.secrets.Get(name, namespace)
 	if k8sSecret == nil {
 		strippedName := strings.TrimSuffix(name, securitymodel.SdsCaSuffix)
@@ -229,14 +233,14 @@ func ExtractCertInfo(scrt *v1.Secret) (certInfo *credentials.CertInfo, err error
 	if hasValue(scrt.Data, GenericScrtCert, GenericScrtKey) {
 		ret.Cert = scrt.Data[GenericScrtCert]
 		ret.Key = scrt.Data[GenericScrtKey]
+		ret.CRL = scrt.Data[GenericScrtCRL]
 		return ret, nil
 	}
 	if hasValue(scrt.Data, TLSSecretCert, TLSSecretKey) {
 		ret.Cert = scrt.Data[TLSSecretCert]
 		ret.Key = scrt.Data[TLSSecretKey]
-		if hasValue(scrt.Data, TLSSecretOcspStaple) {
-			ret.Staple = scrt.Data[TLSSecretOcspStaple]
-		}
+		ret.Staple = scrt.Data[TLSSecretOcspStaple]
+		ret.CRL = scrt.Data[TLSSecretCrl]
 		return ret, nil
 	}
 	// No cert found. Try to generate a helpful error messsage
@@ -264,12 +268,17 @@ func truncatedKeysMessage(data map[string][]byte) string {
 }
 
 // extractRoot extracts the root certificate
-func extractRoot(scrt *v1.Secret) (cert []byte, err error) {
+func extractRoot(scrt *v1.Secret) (certInfo *credentials.CertInfo, err error) {
+	ret := &credentials.CertInfo{}
 	if hasValue(scrt.Data, GenericScrtCaCert) {
-		return scrt.Data[GenericScrtCaCert], nil
+		ret.Cert = scrt.Data[GenericScrtCaCert]
+		ret.CRL = scrt.Data[GenericScrtCRL]
+		return ret, nil
 	}
 	if hasValue(scrt.Data, TLSSecretCaCert) {
-		return scrt.Data[TLSSecretCaCert], nil
+		ret.Cert = scrt.Data[TLSSecretCaCert]
+		ret.CRL = scrt.Data[TLSSecretCrl]
+		return ret, nil
 	}
 	// No cert found. Try to generate a helpful error messsage
 	if hasKeys(scrt.Data, GenericScrtCaCert) {

pilot/pkg/credentials/kube/secrets_test.go

@@ -72,12 +72,21 @@ var (
 	tlsMtlsCert = makeSecret("tls-mtls", map[string]string{
 		TLSSecretCert: "tls-mtls-cert", TLSSecretKey: "tls-mtls-key", TLSSecretCaCert: "tls-mtls-ca",
 	}, corev1.SecretTypeTLS)
+	tlsMtlsCertWithCrl = makeSecret("tls-mtls-crl", map[string]string{
+		TLSSecretCert: "tls-mtls-cert", TLSSecretKey: "tls-mtls-key", TLSSecretCaCert: "tls-mtls-ca", TLSSecretCrl: "tls-mtls-crl",
+	}, corev1.SecretTypeTLS)
 	tlsMtlsCertSplit = makeSecret("tls-mtls-split", map[string]string{
 		TLSSecretCert: "tls-mtls-split-cert", TLSSecretKey: "tls-mtls-split-key",
 	}, corev1.SecretTypeTLS)
 	tlsMtlsCertSplitCa = makeSecret("tls-mtls-split-cacert", map[string]string{
 		TLSSecretCaCert: "tls-mtls-split-ca",
 	}, corev1.SecretTypeTLS)
+	tlsMtlsCertSplitCaWithCrl = makeSecret("tls-mtls-split-crl-cacert", map[string]string{
+		TLSSecretCaCert: "tls-mtls-split-ca", TLSSecretCrl: "tls-mtls-split-crl",
+	}, corev1.SecretTypeTLS)
+	tlsMtlsCertSplitWithCrl = makeSecret("tls-mtls-split-crl", map[string]string{
+		TLSSecretCert: "tls-mtls-split-crl-cert", TLSSecretKey: "tls-mtls-split-crl-key",
+	}, corev1.SecretTypeTLS)
 	emptyCert = makeSecret("empty-cert", map[string]string{
 		TLSSecretCert: "", TLSSecretKey: "tls-key",
 	}, corev1.SecretTypeTLS)
@@ -102,8 +111,11 @@ func TestSecretsController(t *testing.T) {
 		overlappingCa,
 		tlsCert,
 		tlsMtlsCert,
+		tlsMtlsCertWithCrl,
 		tlsMtlsCertSplit,
 		tlsMtlsCertSplitCa,
+		tlsMtlsCertSplitCaWithCrl,
+		tlsMtlsCertSplitWithCrl,
 		emptyCert,
 		wrongKeys,
 	}
@@ -117,6 +129,8 @@ func TestSecretsController(t *testing.T) {
 		key             string
 		staple          string
 		caCert          string
+		caCrl           string
+		crl             string
 		expectedError   string
 		expectedCAError string
 	}{
@@ -169,6 +183,15 @@ func TestSecretsController(t *testing.T) {
 			key:       "tls-mtls-key",
 			caCert:    "tls-mtls-ca",
 		},
+		{
+			name:      "tls-mtls-crl",
+			namespace: "default",
+			cert:      "tls-mtls-cert",
+			key:       "tls-mtls-key",
+			caCert:    "tls-mtls-ca",
+			crl:       "tls-mtls-crl",
+			caCrl:     "tls-mtls-crl",
+		},
 		{
 			name:            "tls-mtls-split",
 			namespace:       "default",
@@ -182,6 +205,20 @@ func TestSecretsController(t *testing.T) {
 			caCert:        "tls-mtls-split-ca",
 			expectedError: "found secret, but didn't have expected keys (cert and key) or (tls.crt and tls.key); found: ca.crt",
 		},
+		{
+			name:          "tls-mtls-split-crl-cacert",
+			namespace:     "default",
+			caCert:        "tls-mtls-split-ca",
+			expectedError: "found secret, but didn't have expected keys (cert and key) or (tls.crt and tls.key); found: ca.crl, ca.crt",
+			caCrl:         "tls-mtls-split-crl",
+		},
+		{
+			name:            "tls-mtls-split-crl",
+			namespace:       "default",
+			cert:            "tls-mtls-split-crl-cert",
+			key:             "tls-mtls-split-crl-key",
+			expectedCAError: "found secret, but didn't have expected keys cacert or ca.crt; found: tls.crt, tls.key",
+		},
 		{
 			name:            "generic",
 			namespace:       "wrong-namespace",
@@ -207,10 +244,12 @@ func TestSecretsController(t *testing.T) {
 			var actualKey []byte
 			var actualCert []byte
 			var actualStaple []byte
+			var actualCrl []byte
 			if certInfo != nil {
 				actualKey = certInfo.Key
 				actualCert = certInfo.Cert
 				actualStaple = certInfo.Staple
+				actualCrl = certInfo.CRL
 			}
 			if tt.key != string(actualKey) {
 				t.Errorf("got key %q, wanted %q", string(actualKey), tt.key)
@@ -221,12 +260,18 @@ func TestSecretsController(t *testing.T) {
 			if tt.staple != string(actualStaple) {
 				t.Errorf("got staple %q, wanted %q", string(actualStaple), tt.staple)
 			}
+			if tt.crl != string(actualCrl) {
+				t.Errorf("got crl %q, wanted %q", string(actualCrl), tt.crl)
+			}
 			if tt.expectedError != errString(err) {
 				t.Errorf("got err %q, wanted %q", errString(err), tt.expectedError)
 			}
-			caCert, err := sc.GetCaCert(tt.name, tt.namespace)
-			if tt.caCert != string(caCert) {
-				t.Errorf("got caCert %q, wanted %q", string(caCert), tt.caCert)
+			caCertInfo, err := sc.GetCaCert(tt.name, tt.namespace)
+			if caCertInfo != nil && tt.caCert != string(caCertInfo.Cert) {
+				t.Errorf("got caCert %q, wanted %q", string(caCertInfo.Cert), tt.caCert)
+			}
+			if caCertInfo != nil && tt.caCrl != string(caCertInfo.CRL) {
+				t.Errorf("got caCrl %q, wanted %q", string(caCertInfo.CRL), tt.crl)
 			}
 			if tt.expectedCAError != errString(err) {
 				t.Errorf("got ca err %q, wanted %q", errString(err), tt.expectedCAError)
@@ -474,9 +519,9 @@ func TestSecretsControllerMulticluster(t *testing.T) {
 			if tt.cert != string(actualCert) {
 				t.Errorf("got cert %q, wanted %q", string(actualCert), tt.cert)
 			}
-			caCert, err := con.GetCaCert(tt.name, tt.namespace)
-			if tt.caCert != string(caCert) {
-				t.Errorf("got caCert %q, wanted %q with err %v", string(caCert), tt.caCert, err)
+			caCertInfo, err := con.GetCaCert(tt.name, tt.namespace)
+			if caCertInfo != nil && tt.caCert != string(caCertInfo.Cert) {
+				t.Errorf("got caCert %q, wanted %q with err %v", string(caCertInfo.Cert), tt.caCert, err)
 			}
 		})
 	}

pilot/pkg/credentials/model.go

@@ -26,11 +26,13 @@ type CertInfo struct {
 	Key []byte
 	// The oscp staple
 	Staple []byte
+	// Certificate Revocation List information
+	CRL []byte
 }
 
 type Controller interface {
 	GetCertInfo(name, namespace string) (certInfo *CertInfo, err error)
-	GetCaCert(name, namespace string) (cert []byte, err error)
+	GetCaCert(name, namespace string) (certInfo *CertInfo, err error)
 	GetDockerCredential(name, namespace string) (cred []byte, err error)
 	Authorize(serviceAccount, namespace string) error
 	AddEventHandler(func(name, namespace string))

pilot/pkg/xds/sds.go

@@ -182,22 +182,21 @@ func (s *SecretGen) generate(sr SecretResource, configClusterSecrets, proxyClust
 
 	isCAOnlySecret := strings.HasSuffix(sr.Name, securitymodel.SdsCaSuffix)
 	if isCAOnlySecret {
-		caCert, err := secretController.GetCaCert(sr.Name, sr.Namespace)
+		caCertInfo, err := secretController.GetCaCert(sr.Name, sr.Namespace)
 		if err != nil {
 			pilotSDSCertificateErrors.Increment()
 			log.Warnf("failed to fetch ca certificate for %s: %v", sr.ResourceName, err)
 			return nil
 		}
 		if features.VerifySDSCertificate {
-			if err := ValidateCertificate(caCert); err != nil {
+			if err := ValidateCertificate(caCertInfo.Cert); err != nil {
 				recordInvalidCertificate(sr.ResourceName, err)
 				return nil
 			}
 		}
-		res := toEnvoyCaSecret(sr.ResourceName, caCert)
+		res := toEnvoyCaSecret(sr.ResourceName, caCertInfo)
 		return res
 	}
-
 	certInfo, err := secretController.GetCertInfo(sr.Name, sr.Namespace)
 	if err != nil {
 		pilotSDSCertificateErrors.Increment()
@@ -320,14 +319,19 @@ func atMostNJoin(data []string, limit int) string {
 	return strings.Join(data[:limit-1], ", ") + fmt.Sprintf(", and %d others", len(data)-limit+1)
 }
 
-func toEnvoyCaSecret(name string, cert []byte) *discovery.Resource {
+func toEnvoyCaSecret(name string, certInfo *credscontroller.CertInfo) *discovery.Resource {
 	res := protoconv.MessageToAny(&envoytls.Secret{
 		Name: name,
 		Type: &envoytls.Secret_ValidationContext{
 			ValidationContext: &envoytls.CertificateValidationContext{
 				TrustedCa: &core.DataSource{
 					Specifier: &core.DataSource_InlineBytes{
-						InlineBytes: cert,
+						InlineBytes: certInfo.Cert,
+					},
+				},
+				Crl: &core.DataSource{
+					Specifier: &core.DataSource_InlineBytes{
+						InlineBytes: certInfo.CRL,
 					},
 				},
 			},

pilot/pkg/xds/sds_test.go

@@ -68,6 +68,12 @@ var (
 		credentials.GenericScrtKey:    readFile(filepath.Join(certDir, "dns/key.pem")),
 		credentials.GenericScrtCaCert: readFile(filepath.Join(certDir, "dns/root-cert.pem")),
 	})
+	genericMtlsCertCrl = makeSecret("generic-mtls-crl", map[string]string{
+		credentials.GenericScrtCert:   readFile(filepath.Join(certDir, "dns/cert-chain.pem")),
+		credentials.GenericScrtKey:    readFile(filepath.Join(certDir, "dns/key.pem")),
+		credentials.GenericScrtCaCert: readFile(filepath.Join(certDir, "dns/root-cert.pem")),
+		credentials.GenericScrtCRL:    readFile(filepath.Join(certDir, "dns/cert-chain.pem")),
+	})
 	genericMtlsCertSplit = makeSecret("generic-mtls-split", map[string]string{
 		credentials.GenericScrtCert: readFile(filepath.Join(certDir, "mountedcerts-client/cert-chain.pem")),
 		credentials.GenericScrtKey:  readFile(filepath.Join(certDir, "mountedcerts-client/key.pem")),
@@ -87,10 +93,12 @@ func TestGenerate(t *testing.T) {
 		Key    string
 		Cert   string
 		CaCert string
+		CaCrl  string
 	}
 	allResources := []string{
 		"kubernetes://generic", "kubernetes://generic-mtls", "kubernetes://generic-mtls-cacert",
-		"kubernetes://generic-mtls-split", "kubernetes://generic-mtls-split-cacert",
+		"kubernetes://generic-mtls-split", "kubernetes://generic-mtls-split-cacert", "kubernetes://generic-mtls-crl",
+		"kubernetes://generic-mtls-crl-cacert",
 	}
 	cases := []struct {
 		name                 string
@@ -155,6 +163,14 @@ func TestGenerate(t *testing.T) {
 				"kubernetes://generic-mtls-split-cacert": {
 					CaCert: string(genericMtlsCertSplitCa.Data[credentials.GenericScrtCaCert]),
 				},
+				"kubernetes://generic-mtls-crl": {
+					Key:  string(genericMtlsCertCrl.Data[credentials.GenericScrtKey]),
+					Cert: string(genericMtlsCertCrl.Data[credentials.GenericScrtCert]),
+				},
+				"kubernetes://generic-mtls-crl-cacert": {
+					CaCert: string(genericMtlsCertCrl.Data[credentials.GenericScrtCaCert]),
+					CaCrl:  string(genericMtlsCertCrl.Data[credentials.GenericScrtCRL]),
+				},
 			},
 		},
 		{
@@ -210,6 +226,25 @@ func TestGenerate(t *testing.T) {
 				},
 			},
 		},
+		{
+			name:      "incremental push with updates - mtls with crl",
+			proxy:     &model.Proxy{VerifiedIdentity: &spiffe.Identity{Namespace: "istio-system"}, Type: model.Router},
+			resources: allResources,
+			request: &model.PushRequest{
+				Full:           false,
+				ConfigsUpdated: sets.New(model.ConfigKey{Kind: kind.Secret, Name: "generic-mtls-crl", Namespace: "istio-system"}),
+			},
+			expect: map[string]Expected{
+				"kubernetes://generic-mtls-crl": {
+					Key:  string(genericMtlsCertCrl.Data[credentials.GenericScrtKey]),
+					Cert: string(genericMtlsCertCrl.Data[credentials.GenericScrtCert]),
+				},
+				"kubernetes://generic-mtls-crl-cacert": {
+					CaCert: string(genericMtlsCertCrl.Data[credentials.GenericScrtCaCert]),
+					CaCrl:  string(genericMtlsCertCrl.Data[credentials.GenericScrtCRL]),
+				},
+			},
+		},
 		{
 			name:      "incremental push with updates - mtls split",
 			proxy:     &model.Proxy{VerifiedIdentity: &spiffe.Identity{Namespace: "istio-system"}, Type: model.Router},
@@ -279,7 +314,7 @@ func TestGenerate(t *testing.T) {
 			}
 			tt.proxy.Metadata.ClusterID = "Kubernetes"
 			s := NewFakeDiscoveryServer(t, FakeOptions{
-				KubernetesObjects: []runtime.Object{genericCert, genericMtlsCert, genericMtlsCertSplit, genericMtlsCertSplitCa},
+				KubernetesObjects: []runtime.Object{genericCert, genericMtlsCert, genericMtlsCertCrl, genericMtlsCertSplit, genericMtlsCertSplitCa},
 			})
 			cc := s.KubeClient().Kube().(*fake.Clientset)
 
@@ -302,6 +337,7 @@ func TestGenerate(t *testing.T) {
 					Key:    string(scrt.GetTlsCertificate().GetPrivateKey().GetInlineBytes()),
 					Cert:   string(scrt.GetTlsCertificate().GetCertificateChain().GetInlineBytes()),
 					CaCert: string(scrt.GetValidationContext().GetTrustedCa().GetInlineBytes()),
+					CaCrl:  string(scrt.GetValidationContext().GetCrl().GetInlineBytes()),
 				}
 			}
 			if diff := cmp.Diff(got, tt.expect); diff != "" {

releasenotes/notes/crl_support.yaml

@@ -0,0 +1,8 @@
+apiVersion: release-notes/v2
+kind: feature
+area: security
+docs:
+  - https://docs.google.com/document/d/13LNbJnLHe_prlOg7sPr77PjLiIuGj452/edit?usp=sharing&ouid=105743966233121608166&rtpof=true&sd=true
+releaseNotes:
+  - |
+    **Added** Certificate Revocation List(CRL) support for peer certificate validation.