kubeadm: verify that present certificates contain at least the required SANs

This avoids ending in a wrong cluster state by assuming that the
present certificates will work. It is specially important when we
are growing etcd from 1 member to 2, in which case in case of failure
upon joining etcd will be unavailable.
pull/564/head
Rafael Fernández López 2019-01-18 22:14:07 +01:00
parent 3ed638b233
commit f22f594d34
No known key found for this signature in database
GPG Key ID: 8902294E78418CF9
4 changed files with 73 additions and 27 deletions

View File

@ -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

View File

@ -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
}

View File

@ -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 {
return writeCertificateFilesIfNotExist(pkiDir, "dummy", caCert, setupCert, setupKey)
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, 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

View File

@ -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{
// 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{
{