cli: Don't check for SAN in root and intermediate certs (linkerd#5237)

As discussed in linkerd#5228, it is not correct for root and intermediate certs to have SAN. This PR updates the check to not verify the intermediate issuer cert with the identity dns name (which checks with SAN and not CN as the the `verify` func is used to verify leaf certs and not root and intermediate certs). This PR also avoids setting a SAN field when generating certs in the `install` command. Fixes linkerd#5228
GMarkfjard · Dec 2, 2020 · 2acdf69 · 2acdf69
1 parent d2e1b0d
commit 2acdf69
Show file tree

Hide file tree

Showing 8 changed files with 11 additions and 25 deletions.
diff --git a/cli/cmd/install_test.go b/cli/cmd/install_test.go
@@ -462,7 +462,6 @@ func TestValidate(t *testing.T) {
 			{"valid", ""},
 			{"expired", "failed to validate issuer credentials: not valid anymore. Expired on 1990-01-01T01:01:11Z"},
 			{"not-valid-yet", "failed to validate issuer credentials: not valid before: 2100-01-01T01:00:51Z"},
-			{"wrong-domain", "failed to validate issuer credentials: x509: certificate is valid for wrong.linkerd.cluster.local, not identity.linkerd.cluster.local"},
 			{"wrong-algo", "failed to validate issuer credentials: must use P-256 curve for public key, instead P-521 was used"},
 		}
 		for _, tc := range testCases {

diff --git a/cli/cmd/options.go b/cli/cmd/options.go
@@ -571,7 +571,7 @@ func validateValues(ctx context.Context, k *k8s.KubernetesAPI, values *l5dcharts
 		if err != nil {
 			return err
 		}
-		_, err = externalIssuerData.VerifyAndBuildCreds(issuerName(values.Global.IdentityTrustDomain))
+		_, err = externalIssuerData.VerifyAndBuildCreds()
 		if err != nil {
 			return fmt.Errorf("failed to validate issuer credentials: %s", err)
 		}
@@ -583,7 +583,7 @@ func validateValues(ctx context.Context, k *k8s.KubernetesAPI, values *l5dcharts
 			IssuerKey:    values.Identity.Issuer.TLS.KeyPEM,
 			TrustAnchors: values.Global.IdentityTrustAnchorsPEM,
 		}
-		_, err := issuerData.VerifyAndBuildCreds(issuerName(values.Global.IdentityTrustDomain))
+		_, err := issuerData.VerifyAndBuildCreds()
 		if err != nil {
 			return fmt.Errorf("failed to validate issuer credentials: %s", err)
 		}

diff --git a/pkg/healthcheck/healthcheck.go b/pkg/healthcheck/healthcheck.go
@@ -1022,7 +1022,7 @@ func (hc *HealthChecker) allCategories() []category {
 					description: "issuer cert is issued by the trust anchor",
 					hintAnchor:  "l5d-identity-issuer-cert-issued-by-trust-anchor",
 					check: func(ctx context.Context) error {
-						return hc.issuerCert.Verify(tls.CertificatesToPool(hc.trustAnchors), hc.issuerIdentity(), time.Time{})
+						return hc.issuerCert.Verify(tls.CertificatesToPool(hc.trustAnchors), "", time.Time{})
 					},
 				},
 			},
@@ -1446,10 +1446,6 @@ func (hc *HealthChecker) checkMinReplicasAvailable(ctx context.Context) error {
 	return nil
 }
 
-func (hc *HealthChecker) issuerIdentity() string {
-	return fmt.Sprintf("identity.%s.%s", hc.ControlPlaneNamespace, hc.linkerdConfig.Global.IdentityTrustDomain)
-}
-
 // Add adds an arbitrary checker. This should only be used for testing. For
 // production code, pass in the desired set of checks when calling
 // NewHealthChecker.

diff --git a/pkg/healthcheck/healthcheck_test.go b/pkg/healthcheck/healthcheck_test.go
@@ -3077,15 +3077,6 @@ func TestLinkerdIdentityCheckCertValidity(t *testing.T) {
 	}
 }
 
-func TestLinkerdIdentityCheckWrongDns(t *testing.T) {
-	expectedOutput := []string{"linkerd-identity-test-cat issuer cert is issued by the trust anchor: x509: certificate is valid for wrong.linkerd.cluster.local, not identity.linkerd.cluster.local"}
-	issuerData := createIssuerData("wrong.linkerd.cluster.local", time.Now().AddDate(-1, 0, 0), time.Now().AddDate(1, 0, 0))
-	fakeConfigMap := getFakeConfigMap(k8s.IdentityIssuerSchemeLinkerd, issuerData)
-	fakeSecret := getFakeSecret(k8s.IdentityIssuerSchemeLinkerd, issuerData)
-	runIdentityCheckTestCase(context.Background(), t, 0, "fails when cert dns is wrong", "issuer cert is issued by the trust anchor", fakeConfigMap, fakeSecret, expectedOutput)
-
-}
-
 type fakeCniResourcesOpts struct {
 	hasConfigMap          bool
 	hasPodSecurityPolicy  bool

diff --git a/pkg/identity/service.go b/pkg/identity/service.go
@@ -114,7 +114,8 @@ func (svc *Service) loadCredentials() (tls.Issuer, error) {
 		return nil, fmt.Errorf("failed to read CA from disk: %s", err)
 	}
 
-	if err := creds.Crt.Verify(svc.trustAnchors, svc.expectedName, time.Time{}); err != nil {
+	// Don't verify with dns name as this is not a leaf certificate
+	if err := creds.Crt.Verify(svc.trustAnchors, "", time.Time{}); err != nil {
 		return nil, fmt.Errorf("failed to verify issuer credentials for '%s' with trust anchors: %s", svc.expectedName, err)
 	}
 
@@ -149,7 +150,8 @@ func (svc *Service) ensureIssuerStillValid() error {
 	issuer := *svc.issuer
 	switch is := issuer.(type) {
 	case *tls.CA:
-		return is.Cred.Verify(svc.trustAnchors, svc.expectedName, time.Time{})
+		// Don't verify with dns name as this is not a leaf certificate
+		return is.Cred.Verify(svc.trustAnchors, "", time.Time{})
 	default:
 		return fmt.Errorf("unsupported issuer type. Expected *tls.CA, got %v", is)
 	}

diff --git a/pkg/issuercerts/issuercerts.go b/pkg/issuercerts/issuercerts.go
@@ -155,7 +155,7 @@ func CheckCertAlgoRequirements(cert *x509.Certificate) error {
 }
 
 // VerifyAndBuildCreds builds and validates the creds out of the data in IssuerCertData
-func (ic *IssuerCertData) VerifyAndBuildCreds(dnsName string) (*tls.Cred, error) {
+func (ic *IssuerCertData) VerifyAndBuildCreds() (*tls.Cred, error) {
 	creds, err := tls.ValidateAndCreateCreds(ic.IssuerCrt, ic.IssuerKey)
 	if err != nil {
 		return nil, fmt.Errorf("failed to read CA: %s", err)
@@ -180,7 +180,7 @@ func (ic *IssuerCertData) VerifyAndBuildCreds(dnsName string) (*tls.Cred, error)
 		return nil, err
 	}
 
-	if err := creds.Verify(anchors, dnsName, time.Time{}); err != nil {
+	if err := creds.Verify(anchors, "", time.Time{}); err != nil {
 		return nil, err
 	}
 

diff --git a/pkg/tls/ca.go b/pkg/tls/ca.go
@@ -118,7 +118,6 @@ func CreateRootCA(
 	// Configure the root certificate.
 	t := createTemplate(1, &key.PublicKey, validity)
 	t.Subject = pkix.Name{CommonName: name}
-	t.DNSNames = []string{name}
 	t.IsCA = true
 	t.MaxPathLen = -1
 	t.BasicConstraintsValid = true
@@ -167,7 +166,6 @@ func (ca *CA) GenerateCA(name string, maxPathLen int) (*CA, error) {
 
 	t := ca.createTemplate(&key.PublicKey)
 	t.Subject = pkix.Name{CommonName: name}
-	t.DNSNames = []string{name}
 	t.IsCA = true
 	t.MaxPathLen = maxPathLen
 	t.MaxPathLenZero = true // 0-values are actually 0

diff --git a/pkg/tls/cred_test.go b/pkg/tls/cred_test.go
@@ -28,7 +28,7 @@ func TestCrtRoundtrip(t *testing.T) {
 		t.Fatalf("Failed to decode PEM Crt: %s", err)
 	}
 
-	if err := crt.Verify(rootTrust, "endentity.test", time.Time{}); err != nil {
+	if err := crt.Verify(rootTrust, "", time.Time{}); err != nil {
 		t.Fatal("Failed to verify round-tripped certificate")
 	}
 }
@@ -103,7 +103,7 @@ func TestCrtExpiry(t *testing.T) {
 			crt.Certificate.NotBefore = tc.notBefore
 			crt.Certificate.NotAfter = tc.notAfter
 
-			err := crt.Verify(rootTrust, "expired.test", tc.currentTime)
+			err := crt.Verify(rootTrust, "", tc.currentTime)
 			if tc.valid && err != nil {
 				t.Fatalf("expected certificate to be valid but was invalid: %s", err.Error())
 			}