Merge pull request #73093 from ereslibre/verify-certificate-sans

cmd/kubeadm/app/phases/certs/certlist.go

@@ -69,10 +69,11 @@ func (k *KubeadmCert) CreateFromCA(ic *kubeadmapi.InitConfiguration, caCert *x50
 		caCert,
 		cert,
 		key,
+		cfg,
 	)
 
 	if err != nil {
-		return errors.Wrapf(err, "failed to write certificate %q", k.Name)
+		return errors.Wrapf(err, "failed to write or validate certificate %q", k.Name)
 	}
 
 	return nil

cmd/kubeadm/app/phases/certs/certs.go

@@ -220,7 +220,7 @@ func writeCertificateAuthorithyFilesIfNotExist(pkiDir string, baseName string, c
 // If there already is a certificate file at the given path; kubeadm tries to load it and check if the values in the
 // existing and the expected certificate equals. If they do; kubeadm will just skip writing the file as it's up-to-date,
 // otherwise this function returns an error.
-func writeCertificateFilesIfNotExist(pkiDir string, baseName string, signingCert *x509.Certificate, cert *x509.Certificate, key *rsa.PrivateKey) error {
+func writeCertificateFilesIfNotExist(pkiDir string, baseName string, signingCert *x509.Certificate, cert *x509.Certificate, key *rsa.PrivateKey, cfg *certutil.Config) error {
 
 	// Checks if the signed certificate exists in the PKI directory
 	if pkiutil.CertOrKeyExist(pkiDir, baseName) {
@@ -235,10 +235,11 @@ func writeCertificateFilesIfNotExist(pkiDir string, baseName string, signingCert
 			return errors.Errorf("certificate %s is not signed by corresponding CA", baseName)
 		}
 
-		// kubeadm doesn't validate the existing certificate more than this;
-		// Basically, if we find a certificate file with the same path; and it is signed by
-		// the expected certificate authority, kubeadm thinks those files are equal and
-		// doesn't bother writing a new file
+		// Check if the certificate has the correct attributes
+		if err := validateCertificateWithConfig(signedCert, baseName, cfg); err != nil {
+			return err
+		}
+
 		fmt.Printf("[certs] Using the existing %q certificate and key\n", baseName)
 	} else {
 		// Write .crt and .key files to disk
@@ -458,3 +459,19 @@ func validatePrivatePublicKey(l certKeyLocation) error {
 	}
 	return nil
 }
+
+// validateCertificateWithConfig makes sure that a given certificate is valid at
+// least for the SANs defined in the configuration.
+func validateCertificateWithConfig(cert *x509.Certificate, baseName string, cfg *certutil.Config) error {
+	for _, dnsName := range cfg.AltNames.DNSNames {
+		if err := cert.VerifyHostname(dnsName); err != nil {
+			return errors.Wrapf(err, "certificate %s is invalid", baseName)
+		}
+	}
+	for _, ipAddress := range cfg.AltNames.IPs {
+		if err := cert.VerifyHostname(ipAddress.String()); err != nil {
+			return errors.Wrapf(err, "certificate %s is invalid", baseName)
+		}
+	}
+	return nil
+}

cmd/kubeadm/app/phases/certs/certs_test.go

@@ -21,6 +21,7 @@ import (
 	"crypto/sha256"
 	"crypto/x509"
 	"io/ioutil"
+	"net"
 	"os"
 	"path"
 	"path/filepath"
@@ -76,8 +77,8 @@ func TestWriteCertificateAuthorithyFilesIfNotExist(t *testing.T) {
 		},
 		{ // cert exists, but it is not a ca > err
 			setupFunc: func(pkiDir string) error {
-				cert, key := certstestutil.CreateTestCert(t, setupCert, setupKey)
-				return writeCertificateFilesIfNotExist(pkiDir, "dummy", setupCert, cert, key)
+				cert, key, config := certstestutil.CreateTestCert(t, setupCert, setupKey, certutil.AltNames{})
+				return writeCertificateFilesIfNotExist(pkiDir, "dummy", setupCert, cert, key, config)
 			},
 			expectedError: true,
 		},
@@ -125,10 +126,16 @@ func TestWriteCertificateAuthorithyFilesIfNotExist(t *testing.T) {
 }
 
 func TestWriteCertificateFilesIfNotExist(t *testing.T) {
+	altNames := certutil.AltNames{
+		DNSNames: []string{"example.com"},
+		IPs: []net.IP{
+			net.IPv4(0, 0, 0, 0),
+		},
+	}
 
 	caCert, caKey := certstestutil.CreateCACert(t)
-	setupCert, setupKey := certstestutil.CreateTestCert(t, caCert, caKey)
-	cert, key := certstestutil.CreateTestCert(t, caCert, caKey)
+	setupCert, setupKey, _ := certstestutil.CreateTestCert(t, caCert, caKey, altNames)
+	cert, key, config := certstestutil.CreateTestCert(t, caCert, caKey, altNames)
 
 	var tests = []struct {
 		setupFunc     func(pkiDir string) error
@@ -138,9 +145,29 @@ func TestWriteCertificateFilesIfNotExist(t *testing.T) {
 		{ // cert does not exists > cert written
 			expectedCert: cert,
 		},
-		{ // cert exists, is signed by the same ca > existing cert used
+		{ // cert exists, is signed by the same ca, missing SANs (dns name) > err
+			setupFunc: func(pkiDir string) error {
+				setupCert, setupKey, setupConfig := certstestutil.CreateTestCert(t, caCert, caKey, certutil.AltNames{
+					IPs: []net.IP{
+						net.IPv4(0, 0, 0, 0),
+					},
+				})
+				return writeCertificateFilesIfNotExist(pkiDir, "dummy", caCert, setupCert, setupKey, setupConfig)
+			},
+			expectedError: true,
+		},
+		{ // cert exists, is signed by the same ca, missing SANs (IP address) > err
+			setupFunc: func(pkiDir string) error {
+				setupCert, setupKey, setupConfig := certstestutil.CreateTestCert(t, caCert, caKey, certutil.AltNames{
+					DNSNames: []string{"example.com"},
+				})
+				return writeCertificateFilesIfNotExist(pkiDir, "dummy", caCert, setupCert, setupKey, setupConfig)
+			},
+			expectedError: true,
+		},
+		{ // cert exists, is signed by the same ca, all SANs present > existing cert used
 			setupFunc: func(pkiDir string) error {
-				return writeCertificateFilesIfNotExist(pkiDir, "dummy", caCert, setupCert, setupKey)
+				return writeCertificateFilesIfNotExist(pkiDir, "dummy", caCert, setupCert, setupKey, config)
 			},
 			expectedCert: setupCert,
 		},
@@ -154,9 +181,9 @@ func TestWriteCertificateFilesIfNotExist(t *testing.T) {
 		{ // cert exists, is signed by another ca > err
 			setupFunc: func(pkiDir string) error {
 				anotherCaCert, anotherCaKey := certstestutil.CreateCACert(t)
-				anotherCert, anotherKey := certstestutil.CreateTestCert(t, anotherCaCert, anotherCaKey)
+				anotherCert, anotherKey, config := certstestutil.CreateTestCert(t, anotherCaCert, anotherCaKey, certutil.AltNames{})
 
-				return writeCertificateFilesIfNotExist(pkiDir, "dummy", anotherCaCert, anotherCert, anotherKey)
+				return writeCertificateFilesIfNotExist(pkiDir, "dummy", anotherCaCert, anotherCert, anotherKey, config)
 			},
 			expectedError: true,
 		},
@@ -176,13 +203,13 @@ func TestWriteCertificateFilesIfNotExist(t *testing.T) {
 		}
 
 		// executes create func
-		err := writeCertificateFilesIfNotExist(tmpdir, "dummy", caCert, cert, key)
+		err := writeCertificateFilesIfNotExist(tmpdir, "dummy", caCert, cert, key, config)
 
 		if !test.expectedError && err != nil {
 			t.Errorf("error writeCertificateFilesIfNotExist failed when not expected to fail: %v", err)
 			continue
 		} else if test.expectedError && err == nil {
-			t.Error("error writeCertificateFilesIfNotExist didn't failed when expected")
+			t.Error("error writeCertificateFilesIfNotExist didn't fail when expected")
 			continue
 		} else if test.expectedError {
 			continue
@@ -355,7 +382,7 @@ func TestNewCACertAndKey(t *testing.T) {
 
 func TestSharedCertificateExists(t *testing.T) {
 	caCert, caKey := certstestutil.CreateCACert(t)
-	_, key := certstestutil.CreateTestCert(t, caCert, caKey)
+	_, key, _ := certstestutil.CreateTestCert(t, caCert, caKey, certutil.AltNames{})
 	publicKey := &key.PublicKey
 
 	var tests = []struct {
@@ -536,7 +563,7 @@ func TestUsingExternalCA(t *testing.T) {
 func TestValidateMethods(t *testing.T) {
 
 	caCert, caKey := certstestutil.CreateCACert(t)
-	cert, key := certstestutil.CreateTestCert(t, caCert, caKey)
+	cert, key, _ := certstestutil.CreateTestCert(t, caCert, caKey, certutil.AltNames{})
 
 	tests := []struct {
 		name            string

cmd/kubeadm/app/util/certs/util.go

@@ -145,17 +145,18 @@ func CreateCACert(t *testing.T) (*x509.Certificate, *rsa.PrivateKey) {
 	return cert, key
 }
 
-// CreateTestCert makes a generic certficate with the given CA.
-func CreateTestCert(t *testing.T, caCert *x509.Certificate, caKey *rsa.PrivateKey) (*x509.Certificate, *rsa.PrivateKey) {
-	cert, key, err := pkiutil.NewCertAndKey(caCert, caKey,
-		&certutil.Config{
-			CommonName: "testCert",
-			Usages:     []x509.ExtKeyUsage{x509.ExtKeyUsageAny},
-		})
+// CreateTestCert makes a generic certificate with the given CA and alternative names.
+func CreateTestCert(t *testing.T, caCert *x509.Certificate, caKey *rsa.PrivateKey, altNames certutil.AltNames) (*x509.Certificate, *rsa.PrivateKey, *certutil.Config) {
+	config := &certutil.Config{
+		CommonName: "testCert",
+		Usages:     []x509.ExtKeyUsage{x509.ExtKeyUsageAny},
+		AltNames:   altNames,
+	}
+	cert, key, err := pkiutil.NewCertAndKey(caCert, caKey, config)
 	if err != nil {
 		t.Fatalf("couldn't create test cert: %v", err)
 	}
-	return cert, key
+	return cert, key, config
 }
 
 // CertTestCase is a configuration of certificates and whether it's expected to work.
@@ -172,7 +173,7 @@ func GetSparseCertTestCases(t *testing.T) []CertTestCase {
 	fpCACert, fpCAKey := CreateCACert(t)
 	etcdCACert, etcdCAKey := CreateCACert(t)
 
-	fpCert, fpKey := CreateTestCert(t, fpCACert, fpCAKey)
+	fpCert, fpKey, _ := CreateTestCert(t, fpCACert, fpCAKey, certutil.AltNames{})
 
 	return []CertTestCase{
 		{