Merge pull request kubernetes#76500 from rojkov/issue-1399

kubeadm: check all available CA certs against pinned certs

Author: k8s-ci-robot

cmd/kubeadm/app/discovery/token/token.go

@@ -119,14 +119,14 @@ func RetrieveValidatedConfigInfo(cfg *kubeadmapi.JoinConfiguration) (*clientcmda
 		for _, cluster := range insecureConfig.Clusters {
 			clusterCABytes = cluster.CertificateAuthorityData
 		}
-		clusterCA, err := parsePEMCert(clusterCABytes)
+		clusterCAs, err := parsePEMCerts(clusterCABytes)
 		if err != nil {
 			return nil, errors.Wrapf(err, "failed to parse cluster CA from the %s configmap", bootstrapapi.ConfigMapClusterInfo)
 
 		}
 
 		// Validate the cluster CA public key against the pinned set
-		err = pubKeyPins.Check(clusterCA)
+		err = pubKeyPins.CheckAny(clusterCAs)
 		if err != nil {
 			return nil, errors.Wrapf(err, "cluster CA found in %s configmap is invalid", bootstrapapi.ConfigMapClusterInfo)
 		}
@@ -226,14 +226,27 @@ func fetchKubeConfigWithTimeout(apiEndpoint string, discoveryTimeout time.Durati
 	}
 }
 
-// parsePEMCert decodes a PEM-formatted certificate and returns it as an x509.Certificate
-func parsePEMCert(certData []byte) (*x509.Certificate, error) {
-	pemBlock, trailingData := pem.Decode(certData)
-	if pemBlock == nil {
-		return nil, errors.New("invalid PEM data")
-	}
-	if len(trailingData) != 0 {
-		return nil, errors.New("trailing data after first PEM block")
+// parsePEMCerts decodes PEM-formatted certificates into a slice of x509.Certificates
+func parsePEMCerts(certData []byte) ([]*x509.Certificate, error) {
+	var certificates []*x509.Certificate
+	var pemBlock *pem.Block
+
+	for {
+		pemBlock, certData = pem.Decode(certData)
+		if pemBlock == nil {
+			return nil, errors.New("invalid PEM data")
+		}
+
+		cert, err := x509.ParseCertificate(pemBlock.Bytes)
+		if err != nil {
+			return nil, errors.Wrap(err, "unable to parse certificate")
+		}
+		certificates = append(certificates, cert)
+
+		if len(certData) == 0 {
+			break
+		}
 	}
-	return x509.ParseCertificate(pemBlock.Bytes)
+
+	return certificates, nil
 }

cmd/kubeadm/app/discovery/token/token_test.go

@@ -103,23 +103,24 @@ func TestParsePEMCert(t *testing.T) {
 	}{
 		{"invalid certificate data", []byte{0}, false},
 		{"certificate with junk appended", []byte(testCertPEM + "\nABC"), false},
-		{"multiple certificates", []byte(testCertPEM + "\n" + testCertPEM), false},
+		{"multiple certificates", []byte(testCertPEM + "\n" + testCertPEM), true},
 		{"valid", []byte(testCertPEM), true},
+		{"empty input", []byte{}, false},
 	} {
-		cert, err := parsePEMCert(testCase.input)
+		certs, err := parsePEMCerts(testCase.input)
 		if testCase.expectValid {
 			if err != nil {
 				t.Errorf("failed TestParsePEMCert(%s): unexpected error %v", testCase.name, err)
 			}
-			if cert == nil {
+			if certs == nil {
 				t.Errorf("failed TestParsePEMCert(%s): returned nil", testCase.name)
 			}
 		} else {
 			if err == nil {
 				t.Errorf("failed TestParsePEMCert(%s): expected an error", testCase.name)
 			}
-			if cert != nil {
-				t.Errorf("failed TestParsePEMCert(%s): expected not to get a certificate back, but got one", testCase.name)
+			if certs != nil {
+				t.Errorf("failed TestParsePEMCert(%s): expected not to get a certificate back, but got some", testCase.name)
 			}
 		}
 	}

cmd/kubeadm/app/util/pubkeypin/pubkeypin.go

@@ -61,12 +61,18 @@ func (s *Set) Allow(pubKeyHashes ...string) error {
 	return nil
 }
 
-// Check if a certificate matches one of the public keys in the set
-func (s *Set) Check(certificate *x509.Certificate) error {
-	if s.checkSHA256(certificate) {
-		return nil
+// CheckAny checks if at least one certificate matches one of the public keys in the set
+func (s *Set) CheckAny(certificates []*x509.Certificate) error {
+	var hashes []string
+
+	for _, certificate := range certificates {
+		if s.checkSHA256(certificate) {
+			return nil
+		}
+
+		hashes = append(hashes, Hash(certificate))
 	}
-	return errors.Errorf("public key %s not pinned", Hash(certificate))
+	return errors.Errorf("none of the public keys %q are pinned", strings.Join(hashes, ":"))
 }
 
 // Empty returns true if the Set contains no pinned public keys.

cmd/kubeadm/app/util/pubkeypin/pubkeypin_test.go

@@ -121,7 +121,7 @@ func TestSet(t *testing.T) {
 		return
 	}
 
-	err = s.Check(testCert(t, testCertPEM))
+	err = s.CheckAny([]*x509.Certificate{testCert(t, testCertPEM)})
 	if err == nil {
 		t.Error("expected test cert to not be allowed (yet)")
 		return
@@ -133,13 +133,13 @@ func TestSet(t *testing.T) {
 		return
 	}
 
-	err = s.Check(testCert(t, testCertPEM))
+	err = s.CheckAny([]*x509.Certificate{testCert(t, testCertPEM)})
 	if err != nil {
 		t.Errorf("expected test cert to be allowed, but got back: %v", err)
 		return
 	}
 
-	err = s.Check(testCert(t, testCert2PEM))
+	err = s.CheckAny([]*x509.Certificate{testCert(t, testCert2PEM)})
 	if err == nil {
 		t.Error("expected the second test cert to be disallowed")
 		return