Merge pull request #97266 from robbiemcmichael/kubeadm-validate-cert-chains

kubeadm: support certificate chain validation

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

@@ -263,7 +263,7 @@ func runCertPhase(cert *certsphase.KubeadmCert, caCert *certsphase.KubeadmCert)
 			return nil
 		}
 
-		if certData, _, err := pkiutil.TryLoadCertAndKeyFromDisk(data.CertificateDir(), cert.BaseName); err == nil {
+		if certData, intermediates, err := pkiutil.TryLoadCertChainFromDisk(data.CertificateDir(), cert.BaseName); err == nil {
 			certsphase.CheckCertificatePeriodValidity(cert.BaseName, certData)
 
 			caCertData, err := pkiutil.TryLoadCertFromDisk(data.CertificateDir(), caCert.BaseName)
@@ -273,7 +273,7 @@ func runCertPhase(cert *certsphase.KubeadmCert, caCert *certsphase.KubeadmCert)
 
 			certsphase.CheckCertificatePeriodValidity(caCert.BaseName, caCertData)
 
-			if err := certData.CheckSignatureFrom(caCertData); err != nil {
+			if err := pkiutil.VerifyCertChain(certData, intermediates, caCertData); err != nil {
 				return errors.Wrapf(err, "[certs] certificate %s not signed by CA certificate %s", cert.BaseName, caCert.BaseName)
 			}
 

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

@@ -229,8 +229,14 @@ func writeCertificateFilesIfNotExist(pkiDir string, baseName string, signingCert
 
 	// Checks if the signed certificate exists in the PKI directory
 	if pkiutil.CertOrKeyExist(pkiDir, baseName) {
-		// Try to load signed certificate .crt and .key from the PKI directory
-		signedCert, _, err := pkiutil.TryLoadCertAndKeyFromDisk(pkiDir, baseName)
+		// Try to load key from the PKI directory
+		_, err := pkiutil.TryLoadKeyFromDisk(pkiDir, baseName)
+		if err != nil {
+			return errors.Wrapf(err, "failure loading %s key", baseName)
+		}
+
+		// Try to load certificate from the PKI directory
+		signedCert, intermediates, err := pkiutil.TryLoadCertChainFromDisk(pkiDir, baseName)
 		if err != nil {
 			return errors.Wrapf(err, "failure loading %s certificate", baseName)
 		}
@@ -238,7 +244,7 @@ func writeCertificateFilesIfNotExist(pkiDir string, baseName string, signingCert
 		CheckCertificatePeriodValidity(baseName, signedCert)
 
 		// Check if the existing cert is signed by the given CA
-		if err := signedCert.CheckSignatureFrom(signingCert); err != nil {
+		if err := pkiutil.VerifyCertChain(signedCert, intermediates, signingCert); err != nil {
 			return errors.Errorf("certificate %s is not signed by corresponding CA", baseName)
 		}
 
@@ -416,10 +422,17 @@ func validateSignedCert(l certKeyLocation) error {
 	return validateSignedCertWithCA(l, caCert)
 }
 
-// validateSignedCertWithCA tries to load a certificate and validate it with the given caCert
+// validateSignedCertWithCA tries to load a certificate and private key and
+// validates that the cert is signed by the given caCert
 func validateSignedCertWithCA(l certKeyLocation, caCert *x509.Certificate) error {
-	// Try to load key and signed certificate
-	signedCert, _, err := pkiutil.TryLoadCertAndKeyFromDisk(l.pkiDir, l.baseName)
+	// Try to load key from the PKI directory
+	_, err := pkiutil.TryLoadKeyFromDisk(l.pkiDir, l.baseName)
+	if err != nil {
+		return errors.Wrapf(err, "failure loading key for %s", l.baseName)
+	}
+
+	// Try to load certificate from the PKI directory
+	signedCert, intermediates, err := pkiutil.TryLoadCertChainFromDisk(l.pkiDir, l.baseName)
 	if err != nil {
 		return errors.Wrapf(err, "failure loading certificate for %s", l.uxName)
 	}
@@ -427,7 +440,7 @@ func validateSignedCertWithCA(l certKeyLocation, caCert *x509.Certificate) error
 	CheckCertificatePeriodValidity(l.uxName, signedCert)
 
 	// Check if the cert is signed by the CA
-	if err := signedCert.CheckSignatureFrom(caCert); err != nil {
+	if err := pkiutil.VerifyCertChain(signedCert, intermediates, caCert); err != nil {
 		return errors.Wrapf(err, "certificate %s is not signed by corresponding CA", l.uxName)
 	}
 	return nil

cmd/kubeadm/app/util/pkiutil/pki_helpers.go

@@ -17,6 +17,7 @@ limitations under the License.
 package pkiutil
 
 import (
+	"bytes"
 	"crypto"
 	"crypto/ecdsa"
 	"crypto/elliptic"
@@ -79,14 +80,33 @@ func NewCertificateAuthority(config *CertConfig) (*x509.Certificate, crypto.Sign
 	return cert, key, nil
 }
 
+// NewIntermediateCertificateAuthority creates new certificate and private key for an intermediate certificate authority
+func NewIntermediateCertificateAuthority(parentCert *x509.Certificate, parentKey crypto.Signer, config *CertConfig) (*x509.Certificate, crypto.Signer, error) {
+	key, err := NewPrivateKey(config.PublicKeyAlgorithm)
+	if err != nil {
+		return nil, nil, errors.Wrap(err, "unable to create private key while generating intermediate CA certificate")
+	}
+
+	cert, err := NewSignedCert(config, key, parentCert, parentKey, true)
+	if err != nil {
+		return nil, nil, errors.Wrap(err, "unable to sign intermediate CA certificate")
+	}
+
+	return cert, key, nil
+}
+
 // NewCertAndKey creates new certificate and key by passing the certificate authority certificate and key
 func NewCertAndKey(caCert *x509.Certificate, caKey crypto.Signer, config *CertConfig) (*x509.Certificate, crypto.Signer, error) {
+	if len(config.Usages) == 0 {
+		return nil, nil, errors.New("must specify at least one ExtKeyUsage")
+	}
+
 	key, err := NewPrivateKey(config.PublicKeyAlgorithm)
 	if err != nil {
 		return nil, nil, errors.Wrap(err, "unable to create private key")
 	}
 
-	cert, err := NewSignedCert(config, key, caCert, caKey)
+	cert, err := NewSignedCert(config, key, caCert, caKey, false)
 	if err != nil {
 		return nil, nil, errors.Wrap(err, "unable to sign certificate")
 	}
@@ -142,6 +162,26 @@ func WriteCert(pkiPath, name string, cert *x509.Certificate) error {
 	return nil
 }
 
+// WriteCertBundle stores the given certificate bundle at the given location
+func WriteCertBundle(pkiPath, name string, certs []*x509.Certificate) error {
+	for i, cert := range certs {
+		if cert == nil {
+			return errors.Errorf("found nil certificate at position %d when writing bundle to file", i)
+		}
+	}
+
+	certificatePath := pathForCert(pkiPath, name)
+	encoded, err := EncodeCertBundlePEM(certs)
+	if err != nil {
+		return errors.Wrapf(err, "unable to marshal certificate bundle to PEM")
+	}
+	if err := certutil.WriteCert(certificatePath, encoded); err != nil {
+		return errors.Wrapf(err, "unable to write certificate bundle to file %s", certificatePath)
+	}
+
+	return nil
+}
+
 // WriteKey stores the given key at the given location
 func WriteKey(pkiPath, name string, key crypto.Signer) error {
 	if key == nil {
@@ -256,6 +296,21 @@ func TryLoadCertFromDisk(pkiPath, name string) (*x509.Certificate, error) {
 	return cert, nil
 }
 
+// TryLoadCertChainFromDisk tries to load the cert chain from the disk
+func TryLoadCertChainFromDisk(pkiPath, name string) (*x509.Certificate, []*x509.Certificate, error) {
+	certificatePath := pathForCert(pkiPath, name)
+
+	certs, err := certutil.CertsFromFile(certificatePath)
+	if err != nil {
+		return nil, nil, errors.Wrapf(err, "couldn't load the certificate file %s", certificatePath)
+	}
+
+	cert := certs[0]
+	intermediates := certs[1:]
+
+	return cert, intermediates, nil
+}
+
 // TryLoadKeyFromDisk tries to load the key from the disk and validates that it is valid
 func TryLoadKeyFromDisk(pkiPath, name string) (crypto.Signer, error) {
 	privateKeyPath := pathForKey(pkiPath, name)
@@ -533,6 +588,24 @@ func EncodeCertPEM(cert *x509.Certificate) []byte {
 	return pem.EncodeToMemory(&block)
 }
 
+// EncodeCertBundlePEM returns PEM-endcoded certificate bundle
+func EncodeCertBundlePEM(certs []*x509.Certificate) ([]byte, error) {
+	buf := bytes.Buffer{}
+
+	block := pem.Block{
+		Type: CertificateBlockType,
+	}
+
+	for _, cert := range certs {
+		block.Bytes = cert.Raw
+		if err := pem.Encode(&buf, &block); err != nil {
+			return nil, err
+		}
+	}
+
+	return buf.Bytes(), nil
+}
+
 // EncodePublicKeyPEM returns PEM-encoded public data
 func EncodePublicKeyPEM(key crypto.PublicKey) ([]byte, error) {
 	der, err := x509.MarshalPKIXPublicKey(key)
@@ -556,7 +629,7 @@ func NewPrivateKey(keyType x509.PublicKeyAlgorithm) (crypto.Signer, error) {
 }
 
 // NewSignedCert creates a signed certificate using the given CA certificate and key
-func NewSignedCert(cfg *CertConfig, key crypto.Signer, caCert *x509.Certificate, caKey crypto.Signer) (*x509.Certificate, error) {
+func NewSignedCert(cfg *CertConfig, key crypto.Signer, caCert *x509.Certificate, caKey crypto.Signer, isCA bool) (*x509.Certificate, error) {
 	serial, err := cryptorand.Int(cryptorand.Reader, new(big.Int).SetInt64(math.MaxInt64))
 	if err != nil {
 		return nil, err
@@ -564,8 +637,10 @@ func NewSignedCert(cfg *CertConfig, key crypto.Signer, caCert *x509.Certificate,
 	if len(cfg.CommonName) == 0 {
 		return nil, errors.New("must specify a CommonName")
 	}
-	if len(cfg.Usages) == 0 {
-		return nil, errors.New("must specify at least one ExtKeyUsage")
+
+	keyUsage := x509.KeyUsageKeyEncipherment | x509.KeyUsageDigitalSignature
+	if isCA {
+		keyUsage |= x509.KeyUsageCertSign
 	}
 
 	RemoveDuplicateAltNames(&cfg.AltNames)
@@ -580,8 +655,10 @@ func NewSignedCert(cfg *CertConfig, key crypto.Signer, caCert *x509.Certificate,
 		SerialNumber:          serial,
 		NotBefore:             caCert.NotBefore,
 		NotAfter:              time.Now().Add(kubeadmconstants.CertificateValidity).UTC(),
-		KeyUsage:     x509.KeyUsageKeyEncipherment | x509.KeyUsageDigitalSignature,
+		KeyUsage:              keyUsage,
 		ExtKeyUsage:           cfg.Usages,
+		BasicConstraintsValid: true,
+		IsCA:                  isCA,
 	}
 	certDERBytes, err := x509.CreateCertificate(cryptorand.Reader, &certTmpl, caCert, key.Public(), caKey)
 	if err != nil {
@@ -624,3 +701,27 @@ func ValidateCertPeriod(cert *x509.Certificate, offset time.Duration) error {
 	}
 	return nil
 }
+
+// VerifyCertChain verifies that a certificate has a valid chain of
+// intermediate CAs back to the root CA
+func VerifyCertChain(cert *x509.Certificate, intermediates []*x509.Certificate, root *x509.Certificate) error {
+	rootPool := x509.NewCertPool()
+	rootPool.AddCert(root)
+
+	intermediatePool := x509.NewCertPool()
+	for _, c := range intermediates {
+		intermediatePool.AddCert(c)
+	}
+
+	verifyOptions := x509.VerifyOptions{
+		Roots:         rootPool,
+		Intermediates: intermediatePool,
+		KeyUsages:     []x509.ExtKeyUsage{x509.ExtKeyUsageAny},
+	}
+
+	if _, err := cert.Verify(verifyOptions); err != nil {
+		return err
+	}
+
+	return nil
+}

cmd/kubeadm/app/util/pkiutil/pki_helpers_test.go

@@ -199,12 +199,27 @@ func TestWriteCert(t *testing.T) {
 	actual := WriteCert(tmpdir, "foo", caCert)
 	if actual != nil {
 		t.Errorf(
-			"failed WriteCertAndKey with an error: %v",
+			"failed WriteCert with an error: %v",
 			actual,
 		)
 	}
 }
 
+func TestWriteCertBundle(t *testing.T) {
+	tmpdir, err := ioutil.TempDir("", "")
+	if err != nil {
+		t.Fatalf("Couldn't create tmpdir")
+	}
+	defer os.RemoveAll(tmpdir)
+
+	certs := []*x509.Certificate{{}, {}}
+
+	actual := WriteCertBundle(tmpdir, "foo", certs)
+	if actual != nil {
+		t.Errorf("failed WriteCertBundle with an error: %v", actual)
+	}
+}
+
 func TestWriteKey(t *testing.T) {
 	tmpdir, err := ioutil.TempDir("", "")
 	if err != nil {
@@ -309,14 +324,14 @@ func TestTryLoadCertAndKeyFromDisk(t *testing.T) {
 		Config: certutil.Config{CommonName: "kubernetes"},
 	})
 	if err != nil {
-		t.Errorf(
+		t.Fatalf(
 			"failed to create cert and key with an error: %v",
 			err,
 		)
 	}
 	err = WriteCertAndKey(tmpdir, "foo", caCert, caKey)
 	if err != nil {
-		t.Errorf(
+		t.Fatalf(
 			"failed to write cert and key with an error: %v",
 			err,
 		)
@@ -366,14 +381,14 @@ func TestTryLoadCertFromDisk(t *testing.T) {
 		Config: certutil.Config{CommonName: "kubernetes"},
 	})
 	if err != nil {
-		t.Errorf(
+		t.Fatalf(
 			"failed to create cert and key with an error: %v",
 			err,
 		)
 	}
 	err = WriteCert(tmpdir, "foo", caCert)
 	if err != nil {
-		t.Errorf(
+		t.Fatalf(
 			"failed to write cert and key with an error: %v",
 			err,
 		)
@@ -412,6 +427,91 @@ func TestTryLoadCertFromDisk(t *testing.T) {
 	}
 }
 
+func TestTryLoadCertChainFromDisk(t *testing.T) {
+	tmpdir, err := ioutil.TempDir("", "")
+	if err != nil {
+		t.Fatalf("Couldn't create tmpdir")
+	}
+	defer os.RemoveAll(tmpdir)
+
+	caCert, caKey, err := NewCertificateAuthority(&CertConfig{
+		Config: certutil.Config{CommonName: "Intermediate CA"},
+	})
+	if err != nil {
+		t.Fatalf("failed to create intermediate CA cert and key with an error: %v", err)
+	}
+
+	cert, _, err := NewCertAndKey(caCert, caKey, &CertConfig{
+		Config: certutil.Config{
+			CommonName: "kubernetes",
+			Usages:     []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth},
+		},
+	})
+	if err != nil {
+		t.Fatalf("failed to create leaf cert and key with an error: %v", err)
+	}
+
+	err = WriteCert(tmpdir, "leaf", cert)
+	if err != nil {
+		t.Fatalf("failed to write cert: %v", err)
+	}
+
+	bundle := []*x509.Certificate{cert, caCert}
+	err = WriteCertBundle(tmpdir, "bundle", bundle)
+	if err != nil {
+		t.Fatalf("failed to write cert bundle: %v", err)
+	}
+
+	var tests = []struct {
+		desc          string
+		path          string
+		name          string
+		expected      bool
+		intermediates int
+	}{
+		{
+			desc:          "empty path and name",
+			path:          "",
+			name:          "",
+			expected:      false,
+			intermediates: 0,
+		},
+		{
+			desc:          "leaf certificate",
+			path:          tmpdir,
+			name:          "leaf",
+			expected:      true,
+			intermediates: 0,
+		},
+		{
+			desc:          "certificate bundle",
+			path:          tmpdir,
+			name:          "bundle",
+			expected:      true,
+			intermediates: 1,
+		},
+	}
+	for _, rt := range tests {
+		t.Run(rt.desc, func(t *testing.T) {
+			_, intermediates, actual := TryLoadCertChainFromDisk(rt.path, rt.name)
+			if (actual == nil) != rt.expected {
+				t.Errorf(
+					"failed TryLoadCertChainFromDisk:\n\texpected: %t\n\t  actual: %t",
+					rt.expected,
+					(actual == nil),
+				)
+			}
+			if len(intermediates) != rt.intermediates {
+				t.Errorf(
+					"TryLoadCertChainFromDisk returned the wrong number of intermediate certificates:\n\texpected: %d\n\t  actual: %d",
+					rt.intermediates,
+					len(intermediates),
+				)
+			}
+		})
+	}
+}
+
 func TestTryLoadKeyFromDisk(t *testing.T) {
 
 	var tests = []struct {
@@ -804,3 +904,97 @@ func TestRemoveDuplicateAltNames(t *testing.T) {
 		}
 	}
 }
+
+func TestVerifyCertChain(t *testing.T) {
+	tmpdir, err := ioutil.TempDir("", "")
+	if err != nil {
+		t.Fatalf("Couldn't create tmpdir")
+	}
+	defer os.RemoveAll(tmpdir)
+
+	rootCert1, rootKey1, err := NewCertificateAuthority(&CertConfig{
+		Config: certutil.Config{CommonName: "Root CA 1"},
+	})
+	if err != nil {
+		t.Errorf("failed to create root CA cert and key with an error: %v", err)
+	}
+
+	leafCert1, _, err := NewCertAndKey(rootCert1, rootKey1, &CertConfig{
+		Config: certutil.Config{
+			CommonName: "Leaf Certificate 1",
+			Usages:     []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth},
+		},
+	})
+	if err != nil {
+		t.Errorf("failed to create leaf cert and key with an error: %v", err)
+	}
+
+	rootCert2, rootKey2, err := NewCertificateAuthority(&CertConfig{
+		Config: certutil.Config{CommonName: "Root CA 2"},
+	})
+	if err != nil {
+		t.Errorf("failed to create root CA cert and key with an error: %v", err)
+	}
+
+	intCert2, intKey2, err := NewIntermediateCertificateAuthority(rootCert2, rootKey2, &CertConfig{
+		Config: certutil.Config{
+			CommonName: "Intermediate CA 2",
+			Usages:     []x509.ExtKeyUsage{},
+		},
+	})
+	if err != nil {
+		t.Errorf("failed to create intermediate CA cert and key with an error: %v", err)
+	}
+
+	leafCert2, _, err := NewCertAndKey(intCert2, intKey2, &CertConfig{
+		Config: certutil.Config{
+			CommonName: "Leaf Certificate 2",
+			Usages:     []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth},
+		},
+	})
+	if err != nil {
+		t.Errorf("failed to create leaf cert and key with an error: %v", err)
+	}
+
+	var tests = []struct {
+		desc          string
+		leaf          *x509.Certificate
+		intermediates []*x509.Certificate
+		root          *x509.Certificate
+		expected      bool
+	}{
+		{
+			desc:          "without any intermediate CAs",
+			leaf:          leafCert1,
+			intermediates: []*x509.Certificate{},
+			root:          rootCert1,
+			expected:      true,
+		},
+		{
+			desc:          "missing intermediate CA",
+			leaf:          leafCert2,
+			intermediates: []*x509.Certificate{},
+			root:          rootCert2,
+			expected:      false,
+		},
+		{
+			desc:          "with one intermediate CA",
+			leaf:          leafCert2,
+			intermediates: []*x509.Certificate{intCert2},
+			root:          rootCert2,
+			expected:      true,
+		},
+	}
+	for _, rt := range tests {
+		t.Run(rt.desc, func(t *testing.T) {
+			actual := VerifyCertChain(rt.leaf, rt.intermediates, rt.root)
+			if (actual == nil) != rt.expected {
+				t.Errorf(
+					"failed VerifyCertChain:\n\texpected: %t\n\t  actual: %t",
+					rt.expected,
+					(actual == nil),
+				)
+			}
+		})
+	}
+}