From d125f3bddceff4cc394b1a4384658bd58711cc42 Mon Sep 17 00:00:00 2001 From: Dmitry Rozhkov Date: Wed, 10 Apr 2019 11:10:06 +0300 Subject: [PATCH] kubeadm: add support for ECDSA keys kubeadm still generates RSA keys when deploying a node, but also accepts ECDSA keys if they already exist pregenerated in the directory specified in --cert-dir. --- cmd/kubeadm/app/cmd/alpha/certs_test.go | 10 +- cmd/kubeadm/app/phases/certs/BUILD | 1 + cmd/kubeadm/app/phases/certs/certlist.go | 8 +- cmd/kubeadm/app/phases/certs/certs.go | 18 +-- cmd/kubeadm/app/phases/certs/certs_test.go | 28 +++-- .../app/phases/certs/renewal/certsapi.go | 4 +- .../app/phases/certs/renewal/filerenewal.go | 8 +- .../app/phases/certs/renewal/interface.go | 4 +- .../app/phases/certs/renewal/renewal_test.go | 4 +- .../app/phases/kubeconfig/kubeconfig.go | 4 +- .../app/phases/kubeconfig/kubeconfig_test.go | 4 +- cmd/kubeadm/app/util/certs/util.go | 7 +- cmd/kubeadm/app/util/pkiutil/pki_helpers.go | 31 ++--- .../app/util/pkiutil/pki_helpers_test.go | 115 +++++++++++------- 14 files changed, 148 insertions(+), 98 deletions(-) diff --git a/cmd/kubeadm/app/cmd/alpha/certs_test.go b/cmd/kubeadm/app/cmd/alpha/certs_test.go index 9e184a3509..d337308b8e 100644 --- a/cmd/kubeadm/app/cmd/alpha/certs_test.go +++ b/cmd/kubeadm/app/cmd/alpha/certs_test.go @@ -209,11 +209,13 @@ func TestRunRenewCommands(t *testing.T) { t.Errorf("couldn't verify renewed cert: %v", err) } - pubKey, ok := newCert.PublicKey.(*rsa.PublicKey) - if !ok { + switch pubKey := newCert.PublicKey.(type) { + case *rsa.PublicKey: + if pubKey.N.Cmp(newKey.(*rsa.PrivateKey).N) != 0 { + t.Error("private key does not match public key") + } + default: t.Errorf("unknown public key type %T", newCert.PublicKey) - } else if pubKey.N.Cmp(newKey.N) != 0 { - t.Error("private key does not match public key") } } diff --git a/cmd/kubeadm/app/phases/certs/BUILD b/cmd/kubeadm/app/phases/certs/BUILD index c8a637aa15..0d3333e08b 100644 --- a/cmd/kubeadm/app/phases/certs/BUILD +++ b/cmd/kubeadm/app/phases/certs/BUILD @@ -20,6 +20,7 @@ go_test( "//cmd/kubeadm/app/util/pkiutil:go_default_library", "//cmd/kubeadm/test:go_default_library", "//staging/src/k8s.io/client-go/util/cert:go_default_library", + "//staging/src/k8s.io/client-go/util/keyutil:go_default_library", "//vendor/github.com/pkg/errors:go_default_library", "//vendor/github.com/stretchr/testify/assert:go_default_library", ], diff --git a/cmd/kubeadm/app/phases/certs/certlist.go b/cmd/kubeadm/app/phases/certs/certlist.go index 08d42046cf..e58822f541 100644 --- a/cmd/kubeadm/app/phases/certs/certlist.go +++ b/cmd/kubeadm/app/phases/certs/certlist.go @@ -17,7 +17,7 @@ limitations under the License. package certs import ( - "crypto/rsa" + "crypto" "crypto/x509" "github.com/pkg/errors" @@ -54,7 +54,7 @@ func (k *KubeadmCert) GetConfig(ic *kubeadmapi.InitConfiguration) (*certutil.Con } // CreateFromCA makes and writes a certificate using the given CA cert and key. -func (k *KubeadmCert) CreateFromCA(ic *kubeadmapi.InitConfiguration, caCert *x509.Certificate, caKey *rsa.PrivateKey) error { +func (k *KubeadmCert) CreateFromCA(ic *kubeadmapi.InitConfiguration, caCert *x509.Certificate, caKey crypto.Signer) error { cfg, err := k.GetConfig(ic) if err != nil { return errors.Wrapf(err, "couldn't create %q certificate", k.Name) @@ -80,7 +80,7 @@ func (k *KubeadmCert) CreateFromCA(ic *kubeadmapi.InitConfiguration, caCert *x50 } // CreateAsCA creates a certificate authority, writing the files to disk and also returning the created CA so it can be used to sign child certs. -func (k *KubeadmCert) CreateAsCA(ic *kubeadmapi.InitConfiguration) (*x509.Certificate, *rsa.PrivateKey, error) { +func (k *KubeadmCert) CreateAsCA(ic *kubeadmapi.InitConfiguration) (*x509.Certificate, crypto.Signer, error) { cfg, err := k.GetConfig(ic) if err != nil { return nil, nil, errors.Wrapf(err, "couldn't get configuration for %q CA certificate", k.Name) @@ -114,7 +114,7 @@ func (t CertificateTree) CreateTree(ic *kubeadmapi.InitConfiguration) error { return err } - var caKey *rsa.PrivateKey + var caKey crypto.Signer caCert, err := pkiutil.TryLoadCertFromDisk(ic.CertificatesDir, ca.BaseName) if err == nil { diff --git a/cmd/kubeadm/app/phases/certs/certs.go b/cmd/kubeadm/app/phases/certs/certs.go index 6d3da11d63..e03c719ae4 100644 --- a/cmd/kubeadm/app/phases/certs/certs.go +++ b/cmd/kubeadm/app/phases/certs/certs.go @@ -17,7 +17,7 @@ limitations under the License. package certs import ( - "crypto/rsa" + "crypto" "crypto/x509" "fmt" "os" @@ -80,7 +80,7 @@ func CreateServiceAccountKeyAndPublicKeyFiles(certsDir string) error { } // NewServiceAccountSigningKey generate public/private key pairs for signing service account tokens. -func NewServiceAccountSigningKey() (*rsa.PrivateKey, error) { +func NewServiceAccountSigningKey() (crypto.Signer, error) { // The key does NOT exist, let's generate it now saSigningKey, err := pkiutil.NewPrivateKey() if err != nil { @@ -117,7 +117,7 @@ func CreateCACertAndKeyFiles(certSpec *KubeadmCert, cfg *kubeadmapi.InitConfigur } // NewCSR will generate a new CSR and accompanying key -func NewCSR(certSpec *KubeadmCert, cfg *kubeadmapi.InitConfiguration) (*x509.CertificateRequest, *rsa.PrivateKey, error) { +func NewCSR(certSpec *KubeadmCert, cfg *kubeadmapi.InitConfiguration) (*x509.CertificateRequest, crypto.Signer, error) { certConfig, err := certSpec.GetConfig(cfg) if err != nil { return nil, nil, errors.Wrap(err, "failed to retrieve cert configuration") @@ -151,7 +151,7 @@ func CreateCertAndKeyFilesWithCA(certSpec *KubeadmCert, caCertSpec *KubeadmCert, } // LoadCertificateAuthority tries to load a CA in the given directory with the given name. -func LoadCertificateAuthority(pkiDir string, baseName string) (*x509.Certificate, *rsa.PrivateKey, error) { +func LoadCertificateAuthority(pkiDir string, baseName string) (*x509.Certificate, crypto.Signer, error) { // Checks if certificate authority exists in the PKI directory if !pkiutil.CertOrKeyExist(pkiDir, baseName) { return nil, nil, errors.Errorf("couldn't load %s certificate authority from %s", baseName, pkiDir) @@ -175,7 +175,7 @@ func LoadCertificateAuthority(pkiDir string, baseName string) (*x509.Certificate // 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 writeCertificateAuthorithyFilesIfNotExist(pkiDir string, baseName string, caCert *x509.Certificate, caKey *rsa.PrivateKey) error { +func writeCertificateAuthorithyFilesIfNotExist(pkiDir string, baseName string, caCert *x509.Certificate, caKey crypto.Signer) error { // If cert or key exists, we should try to load them if pkiutil.CertOrKeyExist(pkiDir, baseName) { @@ -210,7 +210,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, cfg *certutil.Config) error { +func writeCertificateFilesIfNotExist(pkiDir string, baseName string, signingCert *x509.Certificate, cert *x509.Certificate, key crypto.Signer, cfg *certutil.Config) error { // Checks if the signed certificate exists in the PKI directory if pkiutil.CertOrKeyExist(pkiDir, baseName) { @@ -250,7 +250,7 @@ func writeCertificateFilesIfNotExist(pkiDir string, baseName string, signingCert // If there already is a key file at the given path; kubeadm tries to load it and check if the values in the // existing and the expected key equals. If they do; kubeadm will just skip writing the file as it's up-to-date, // otherwise this function returns an error. -func writeKeyFilesIfNotExist(pkiDir string, baseName string, key *rsa.PrivateKey) error { +func writeKeyFilesIfNotExist(pkiDir string, baseName string, key crypto.Signer) error { // Checks if the key exists in the PKI directory if pkiutil.CertOrKeyExist(pkiDir, baseName) { @@ -274,7 +274,7 @@ func writeKeyFilesIfNotExist(pkiDir string, baseName string, key *rsa.PrivateKey return errors.Wrapf(err, "failure while saving %s key", baseName) } - if err := pkiutil.WritePublicKey(pkiDir, baseName, &key.PublicKey); err != nil { + if err := pkiutil.WritePublicKey(pkiDir, baseName, key.Public()); err != nil { return errors.Wrapf(err, "failure while saving %s public key", baseName) } } @@ -285,7 +285,7 @@ func writeKeyFilesIfNotExist(pkiDir string, baseName string, key *rsa.PrivateKey // writeCSRFilesIfNotExist writes a new CSR to the given path. // If there already is a CSR file at the given path; kubeadm tries to load it and check if it's a valid certificate. // otherwise this function returns an error. -func writeCSRFilesIfNotExist(csrDir string, baseName string, csr *x509.CertificateRequest, key *rsa.PrivateKey) error { +func writeCSRFilesIfNotExist(csrDir string, baseName string, csr *x509.CertificateRequest, key crypto.Signer) error { if pkiutil.CSROrKeyExist(csrDir, baseName) { _, _, err := pkiutil.TryLoadCSRAndKeyFromDisk(csrDir, baseName) if err != nil { diff --git a/cmd/kubeadm/app/phases/certs/certs_test.go b/cmd/kubeadm/app/phases/certs/certs_test.go index 8924c72bd4..c0dfcb46ea 100644 --- a/cmd/kubeadm/app/phases/certs/certs_test.go +++ b/cmd/kubeadm/app/phases/certs/certs_test.go @@ -17,7 +17,8 @@ limitations under the License. package certs import ( - "crypto/rsa" + "bytes" + "crypto" "crypto/sha256" "crypto/x509" "io/ioutil" @@ -31,6 +32,7 @@ import ( "github.com/stretchr/testify/assert" certutil "k8s.io/client-go/util/cert" + "k8s.io/client-go/util/keyutil" kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm" kubeadmconstants "k8s.io/kubernetes/cmd/kubeadm/app/constants" certstestutil "k8s.io/kubernetes/cmd/kubeadm/app/util/certs" @@ -38,7 +40,7 @@ import ( testutil "k8s.io/kubernetes/cmd/kubeadm/test" ) -func createTestCSR(t *testing.T) (*x509.CertificateRequest, *rsa.PrivateKey) { +func createTestCSR(t *testing.T) (*x509.CertificateRequest, crypto.Signer) { csr, key, err := pkiutil.NewCSRAndKey( &certutil.Config{ CommonName: "testCert", @@ -307,7 +309,7 @@ func TestWriteKeyFilesIfNotExist(t *testing.T) { var tests = []struct { setupFunc func(pkiDir string) error expectedError bool - expectedKey *rsa.PrivateKey + expectedKey crypto.Signer }{ { // key does not exists > key written expectedKey: key, @@ -349,7 +351,7 @@ func TestWriteKeyFilesIfNotExist(t *testing.T) { } else if test.expectedError && err == nil { t.Error("error writeKeyFilesIfNotExist didn't failed when expected") continue - } else if test.expectedError { + } else if test.expectedError || test.expectedKey == nil { continue } @@ -363,8 +365,18 @@ func TestWriteKeyFilesIfNotExist(t *testing.T) { continue } - //TODO: check if there is a better method to compare keys - if resultingKey.D == test.expectedKey.D { + resultingKeyPEM, err := keyutil.MarshalPrivateKeyToPEM(resultingKey) + if err != nil { + t.Errorf("failure marshaling created key: %v", err) + continue + } + + expectedKeyPEM, err := keyutil.MarshalPrivateKeyToPEM(test.expectedKey) + if err != nil { + t.Fatalf("Failed to marshal expected private key: %v", err) + } + + if bytes.Compare(resultingKeyPEM, expectedKeyPEM) != 0 { t.Error("created key does not match expected key") } } @@ -373,7 +385,7 @@ func TestWriteKeyFilesIfNotExist(t *testing.T) { func TestSharedCertificateExists(t *testing.T) { caCert, caKey := certstestutil.CreateCACert(t) _, key, _ := certstestutil.CreateTestCert(t, caCert, caKey, certutil.AltNames{}) - publicKey := &key.PublicKey + publicKey := key.Public() var tests = []struct { name string @@ -664,7 +676,7 @@ func TestValidateMethods(t *testing.T) { { name: "validatePrivatePublicKey", files: certstestutil.PKIFiles{ - "sa.pub": &key.PublicKey, + "sa.pub": key.Public(), "sa.key": key, }, validateFunc: validatePrivatePublicKey, diff --git a/cmd/kubeadm/app/phases/certs/renewal/certsapi.go b/cmd/kubeadm/app/phases/certs/renewal/certsapi.go index d2c5e89643..bb61cdd642 100644 --- a/cmd/kubeadm/app/phases/certs/renewal/certsapi.go +++ b/cmd/kubeadm/app/phases/certs/renewal/certsapi.go @@ -17,7 +17,7 @@ limitations under the License. package renewal import ( - "crypto/rsa" + "crypto" "crypto/x509" "crypto/x509/pkix" "fmt" @@ -51,7 +51,7 @@ func NewCertsAPIRenawal(client kubernetes.Interface) Interface { } // Renew takes a certificate using the cert and key. -func (r *CertsAPIRenewal) Renew(cfg *certutil.Config) (*x509.Certificate, *rsa.PrivateKey, error) { +func (r *CertsAPIRenewal) Renew(cfg *certutil.Config) (*x509.Certificate, crypto.Signer, error) { reqTmp := &x509.CertificateRequest{ Subject: pkix.Name{ CommonName: cfg.CommonName, diff --git a/cmd/kubeadm/app/phases/certs/renewal/filerenewal.go b/cmd/kubeadm/app/phases/certs/renewal/filerenewal.go index df7c5f7f22..66fd9b3a3c 100644 --- a/cmd/kubeadm/app/phases/certs/renewal/filerenewal.go +++ b/cmd/kubeadm/app/phases/certs/renewal/filerenewal.go @@ -17,7 +17,7 @@ limitations under the License. package renewal import ( - "crypto/rsa" + "crypto" "crypto/x509" certutil "k8s.io/client-go/util/cert" @@ -27,11 +27,11 @@ import ( // FileRenewal renews a certificate using local certs type FileRenewal struct { caCert *x509.Certificate - caKey *rsa.PrivateKey + caKey crypto.Signer } // NewFileRenewal takes a certificate pair to construct the Interface. -func NewFileRenewal(caCert *x509.Certificate, caKey *rsa.PrivateKey) Interface { +func NewFileRenewal(caCert *x509.Certificate, caKey crypto.Signer) Interface { return &FileRenewal{ caCert: caCert, caKey: caKey, @@ -39,6 +39,6 @@ func NewFileRenewal(caCert *x509.Certificate, caKey *rsa.PrivateKey) Interface { } // Renew takes a certificate using the cert and key -func (r *FileRenewal) Renew(cfg *certutil.Config) (*x509.Certificate, *rsa.PrivateKey, error) { +func (r *FileRenewal) Renew(cfg *certutil.Config) (*x509.Certificate, crypto.Signer, error) { return pkiutil.NewCertAndKey(r.caCert, r.caKey, cfg) } diff --git a/cmd/kubeadm/app/phases/certs/renewal/interface.go b/cmd/kubeadm/app/phases/certs/renewal/interface.go index fd11e68004..3da747d40c 100644 --- a/cmd/kubeadm/app/phases/certs/renewal/interface.go +++ b/cmd/kubeadm/app/phases/certs/renewal/interface.go @@ -17,7 +17,7 @@ limitations under the License. package renewal import ( - "crypto/rsa" + "crypto" "crypto/x509" certutil "k8s.io/client-go/util/cert" @@ -25,5 +25,5 @@ import ( // Interface represents a standard way to renew a certificate. type Interface interface { - Renew(*certutil.Config) (*x509.Certificate, *rsa.PrivateKey, error) + Renew(*certutil.Config) (*x509.Certificate, crypto.Signer, error) } diff --git a/cmd/kubeadm/app/phases/certs/renewal/renewal_test.go b/cmd/kubeadm/app/phases/certs/renewal/renewal_test.go index 0d9c6df54a..61b4dcb623 100644 --- a/cmd/kubeadm/app/phases/certs/renewal/renewal_test.go +++ b/cmd/kubeadm/app/phases/certs/renewal/renewal_test.go @@ -17,7 +17,7 @@ limitations under the License. package renewal import ( - "crypto/rsa" + "crypto" "crypto/x509" "crypto/x509/pkix" "net" @@ -113,7 +113,7 @@ func defaultReactionFunc(obj runtime.Object) k8stesting.ReactionFunc { } } -func getCertReq(t *testing.T, caCert *x509.Certificate, caKey *rsa.PrivateKey) *certsapi.CertificateSigningRequest { +func getCertReq(t *testing.T, caCert *x509.Certificate, caKey crypto.Signer) *certsapi.CertificateSigningRequest { cert, _, err := pkiutil.NewCertAndKey(caCert, caKey, &certutil.Config{ CommonName: "testcert", AltNames: certutil.AltNames{ diff --git a/cmd/kubeadm/app/phases/kubeconfig/kubeconfig.go b/cmd/kubeadm/app/phases/kubeconfig/kubeconfig.go index 666b39da47..fcadee4927 100644 --- a/cmd/kubeadm/app/phases/kubeconfig/kubeconfig.go +++ b/cmd/kubeadm/app/phases/kubeconfig/kubeconfig.go @@ -18,7 +18,7 @@ package kubeconfig import ( "bytes" - "crypto/rsa" + "crypto" "crypto/x509" "fmt" "io" @@ -41,7 +41,7 @@ import ( // clientCertAuth struct holds info required to build a client certificate to provide authentication info in a kubeconfig object type clientCertAuth struct { - CAKey *rsa.PrivateKey + CAKey crypto.Signer Organizations []string } diff --git a/cmd/kubeadm/app/phases/kubeconfig/kubeconfig_test.go b/cmd/kubeadm/app/phases/kubeconfig/kubeconfig_test.go index bf52c353cf..3ab7c8f0ab 100644 --- a/cmd/kubeadm/app/phases/kubeconfig/kubeconfig_test.go +++ b/cmd/kubeadm/app/phases/kubeconfig/kubeconfig_test.go @@ -18,7 +18,7 @@ package kubeconfig import ( "bytes" - "crypto/rsa" + "crypto" "crypto/x509" "fmt" "io" @@ -621,7 +621,7 @@ func TestValidateKubeconfigsForExternalCA(t *testing.T) { } // setupdKubeConfigWithClientAuth is a test utility function that wraps buildKubeConfigFromSpec for building a KubeConfig object With ClientAuth -func setupdKubeConfigWithClientAuth(t *testing.T, caCert *x509.Certificate, caKey *rsa.PrivateKey, APIServer, clientName, clustername string, organizations ...string) *clientcmdapi.Config { +func setupdKubeConfigWithClientAuth(t *testing.T, caCert *x509.Certificate, caKey crypto.Signer, APIServer, clientName, clustername string, organizations ...string) *clientcmdapi.Config { spec := &kubeConfigSpec{ CACert: caCert, APIServer: APIServer, diff --git a/cmd/kubeadm/app/util/certs/util.go b/cmd/kubeadm/app/util/certs/util.go index cc0731f87e..e7881b8a3b 100644 --- a/cmd/kubeadm/app/util/certs/util.go +++ b/cmd/kubeadm/app/util/certs/util.go @@ -17,6 +17,7 @@ limitations under the License. package certs import ( + "crypto" "crypto/rsa" "crypto/x509" "net" @@ -30,7 +31,7 @@ import ( // SetupCertificateAuthorithy is a utility function for kubeadm testing that creates a // CertificateAuthorithy cert/key pair -func SetupCertificateAuthorithy(t *testing.T) (*x509.Certificate, *rsa.PrivateKey) { +func SetupCertificateAuthorithy(t *testing.T) (*x509.Certificate, crypto.Signer) { caCert, caKey, err := pkiutil.NewCertificateAuthority(&certutil.Config{CommonName: "kubernetes"}) if err != nil { t.Fatalf("failure while generating CA certificate and key: %v", err) @@ -130,7 +131,7 @@ func AssertCertificateHasIPAddresses(t *testing.T, cert *x509.Certificate, IPAdd } // CreateCACert creates a generic CA cert. -func CreateCACert(t *testing.T) (*x509.Certificate, *rsa.PrivateKey) { +func CreateCACert(t *testing.T) (*x509.Certificate, crypto.Signer) { certCfg := &certutil.Config{CommonName: "kubernetes"} cert, key, err := pkiutil.NewCertificateAuthority(certCfg) if err != nil { @@ -140,7 +141,7 @@ func CreateCACert(t *testing.T) (*x509.Certificate, *rsa.PrivateKey) { } // 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) { +func CreateTestCert(t *testing.T, caCert *x509.Certificate, caKey crypto.Signer, altNames certutil.AltNames) (*x509.Certificate, crypto.Signer, *certutil.Config) { config := &certutil.Config{ CommonName: "testCert", Usages: []x509.ExtKeyUsage{x509.ExtKeyUsageAny}, diff --git a/cmd/kubeadm/app/util/pkiutil/pki_helpers.go b/cmd/kubeadm/app/util/pkiutil/pki_helpers.go index 6f68a01cbc..ba91fe3a8c 100644 --- a/cmd/kubeadm/app/util/pkiutil/pki_helpers.go +++ b/cmd/kubeadm/app/util/pkiutil/pki_helpers.go @@ -18,6 +18,7 @@ package pkiutil import ( "crypto" + "crypto/ecdsa" "crypto/rand" cryptorand "crypto/rand" "crypto/rsa" @@ -58,7 +59,7 @@ const ( ) // NewCertificateAuthority creates new certificate and private key for the certificate authority -func NewCertificateAuthority(config *certutil.Config) (*x509.Certificate, *rsa.PrivateKey, error) { +func NewCertificateAuthority(config *certutil.Config) (*x509.Certificate, crypto.Signer, error) { key, err := NewPrivateKey() if err != nil { return nil, nil, errors.Wrap(err, "unable to create private key while generating CA certificate") @@ -73,7 +74,7 @@ func NewCertificateAuthority(config *certutil.Config) (*x509.Certificate, *rsa.P } // NewCertAndKey creates new certificate and key by passing the certificate authority certificate and key -func NewCertAndKey(caCert *x509.Certificate, caKey *rsa.PrivateKey, config *certutil.Config) (*x509.Certificate, *rsa.PrivateKey, error) { +func NewCertAndKey(caCert *x509.Certificate, caKey crypto.Signer, config *certutil.Config) (*x509.Certificate, crypto.Signer, error) { key, err := NewPrivateKey() if err != nil { return nil, nil, errors.Wrap(err, "unable to create private key") @@ -88,7 +89,7 @@ func NewCertAndKey(caCert *x509.Certificate, caKey *rsa.PrivateKey, config *cert } // NewCSRAndKey generates a new key and CSR and that could be signed to create the given certificate -func NewCSRAndKey(config *certutil.Config) (*x509.CertificateRequest, *rsa.PrivateKey, error) { +func NewCSRAndKey(config *certutil.Config) (*x509.CertificateRequest, crypto.Signer, error) { key, err := NewPrivateKey() if err != nil { return nil, nil, errors.Wrap(err, "unable to create private key") @@ -113,7 +114,7 @@ func HasServerAuth(cert *x509.Certificate) bool { } // WriteCertAndKey stores certificate and key at the specified location -func WriteCertAndKey(pkiPath string, name string, cert *x509.Certificate, key *rsa.PrivateKey) error { +func WriteCertAndKey(pkiPath string, name string, cert *x509.Certificate, key crypto.Signer) error { if err := WriteKey(pkiPath, name, key); err != nil { return errors.Wrap(err, "couldn't write key") } @@ -136,7 +137,7 @@ func WriteCert(pkiPath, name string, cert *x509.Certificate) error { } // WriteKey stores the given key at the given location -func WriteKey(pkiPath, name string, key *rsa.PrivateKey) error { +func WriteKey(pkiPath, name string, key crypto.Signer) error { if key == nil { return errors.New("private key cannot be nil when writing to file") } @@ -175,7 +176,7 @@ func WriteCSR(csrDir, name string, csr *x509.CertificateRequest) error { } // WritePublicKey stores the given public key at the given location -func WritePublicKey(pkiPath, name string, key *rsa.PublicKey) error { +func WritePublicKey(pkiPath, name string, key crypto.PublicKey) error { if key == nil { return errors.New("public key cannot be nil when writing to file") } @@ -219,7 +220,7 @@ func CSROrKeyExist(csrDir, name string) bool { } // TryLoadCertAndKeyFromDisk tries to load a cert and a key from the disk and validates that they are valid -func TryLoadCertAndKeyFromDisk(pkiPath, name string) (*x509.Certificate, *rsa.PrivateKey, error) { +func TryLoadCertAndKeyFromDisk(pkiPath, name string) (*x509.Certificate, crypto.Signer, error) { cert, err := TryLoadCertFromDisk(pkiPath, name) if err != nil { return nil, nil, errors.Wrap(err, "failed to load certificate") @@ -259,7 +260,7 @@ func TryLoadCertFromDisk(pkiPath, name string) (*x509.Certificate, error) { } // TryLoadKeyFromDisk tries to load the key from the disk and validates that it is valid -func TryLoadKeyFromDisk(pkiPath, name string) (*rsa.PrivateKey, error) { +func TryLoadKeyFromDisk(pkiPath, name string) (crypto.Signer, error) { privateKeyPath := pathForKey(pkiPath, name) // Parse the private key from a file @@ -268,20 +269,22 @@ func TryLoadKeyFromDisk(pkiPath, name string) (*rsa.PrivateKey, error) { return nil, errors.Wrapf(err, "couldn't load the private key file %s", privateKeyPath) } - // Allow RSA format only - var key *rsa.PrivateKey + // Allow RSA and ECDSA formats only + var key crypto.Signer switch k := privKey.(type) { case *rsa.PrivateKey: key = k + case *ecdsa.PrivateKey: + key = k default: - return nil, errors.Errorf("the private key file %s isn't in RSA format", privateKeyPath) + return nil, errors.Errorf("the private key file %s is neither in RSA nor ECDSA format", privateKeyPath) } return key, nil } // TryLoadCSRAndKeyFromDisk tries to load the CSR and key from the disk -func TryLoadCSRAndKeyFromDisk(pkiPath, name string) (*x509.CertificateRequest, *rsa.PrivateKey, error) { +func TryLoadCSRAndKeyFromDisk(pkiPath, name string) (*x509.CertificateRequest, crypto.Signer, error) { csrPath := pathForCSR(pkiPath, name) csr, err := CertificateRequestFromFile(csrPath) @@ -528,7 +531,7 @@ func EncodeCertPEM(cert *x509.Certificate) []byte { } // EncodePublicKeyPEM returns PEM-encoded public data -func EncodePublicKeyPEM(key *rsa.PublicKey) ([]byte, error) { +func EncodePublicKeyPEM(key crypto.PublicKey) ([]byte, error) { der, err := x509.MarshalPKIXPublicKey(key) if err != nil { return []byte{}, err @@ -541,7 +544,7 @@ func EncodePublicKeyPEM(key *rsa.PublicKey) ([]byte, error) { } // NewPrivateKey creates an RSA private key -func NewPrivateKey() (*rsa.PrivateKey, error) { +func NewPrivateKey() (crypto.Signer, error) { return rsa.GenerateKey(cryptorand.Reader, rsaKeySize) } diff --git a/cmd/kubeadm/app/util/pkiutil/pki_helpers_test.go b/cmd/kubeadm/app/util/pkiutil/pki_helpers_test.go index b4542a2ecc..704b47472e 100644 --- a/cmd/kubeadm/app/util/pkiutil/pki_helpers_test.go +++ b/cmd/kubeadm/app/util/pkiutil/pki_helpers_test.go @@ -17,6 +17,9 @@ limitations under the License. package pkiutil import ( + "crypto" + "crypto/ecdsa" + "crypto/elliptic" "crypto/rand" "crypto/rsa" "crypto/x509" @@ -49,27 +52,38 @@ func TestNewCertificateAuthority(t *testing.T) { func TestNewCertAndKey(t *testing.T) { var tests = []struct { - name string - caKeySize int - expected bool + name string + keyGenFunc func() (crypto.Signer, error) + expected bool }{ { - name: "RSA key too small", - caKeySize: 128, - expected: false, + name: "RSA key too small", + keyGenFunc: func() (crypto.Signer, error) { + return rsa.GenerateKey(rand.Reader, 128) + }, + expected: false, }, { - name: "Should succeed", - caKeySize: 2048, - expected: true, + name: "RSA should succeed", + keyGenFunc: func() (crypto.Signer, error) { + return rsa.GenerateKey(rand.Reader, 2048) + }, + expected: true, + }, + { + name: "ECDSA should succeed", + keyGenFunc: func() (crypto.Signer, error) { + return ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + }, + expected: true, }, } for _, rt := range tests { t.Run(rt.name, func(t *testing.T) { - caKey, err := rsa.GenerateKey(rand.Reader, rt.caKeySize) + caKey, err := rt.keyGenFunc() if err != nil { - t.Fatalf("Couldn't create rsa Private Key") + t.Fatalf("Couldn't create Private Key") } caCert := &x509.Certificate{} config := &certutil.Config{ @@ -375,49 +389,66 @@ func TestTryLoadCertFromDisk(t *testing.T) { } func TestTryLoadKeyFromDisk(t *testing.T) { - tmpdir, err := ioutil.TempDir("", "") - if err != nil { - t.Fatalf("Couldn't create tmpdir") - } - defer os.RemoveAll(tmpdir) - - _, caKey, err := NewCertificateAuthority(&certutil.Config{CommonName: "kubernetes"}) - if err != nil { - t.Errorf( - "failed to create cert and key with an error: %v", - err, - ) - } - err = WriteKey(tmpdir, "foo", caKey) - if err != nil { - t.Errorf( - "failed to write cert and key with an error: %v", - err, - ) - } var tests = []struct { - desc string - path string - name string - expected bool + desc string + pathSuffix string + name string + keyGenFunc func() (crypto.Signer, error) + expected bool }{ { - desc: "empty path and name", - path: "", - name: "", + desc: "empty path and name", + pathSuffix: "somegarbage", + name: "", + keyGenFunc: func() (crypto.Signer, error) { + return rsa.GenerateKey(rand.Reader, 2048) + }, expected: false, }, { - desc: "valid path and name", - path: tmpdir, - name: "foo", + desc: "RSA valid path and name", + pathSuffix: "", + name: "foo", + keyGenFunc: func() (crypto.Signer, error) { + return rsa.GenerateKey(rand.Reader, 2048) + }, + expected: true, + }, + { + desc: "ECDSA valid path and name", + pathSuffix: "", + name: "foo", + keyGenFunc: func() (crypto.Signer, error) { + return ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + }, expected: true, }, } for _, rt := range tests { t.Run(rt.desc, func(t *testing.T) { - _, actual := TryLoadKeyFromDisk(rt.path, rt.name) + tmpdir, err := ioutil.TempDir("", "") + if err != nil { + t.Fatalf("Couldn't create tmpdir") + } + defer os.RemoveAll(tmpdir) + + caKey, err := rt.keyGenFunc() + if err != nil { + t.Errorf( + "failed to create key with an error: %v", + err, + ) + } + + err = WriteKey(tmpdir, "foo", caKey) + if err != nil { + t.Errorf( + "failed to write key with an error: %v", + err, + ) + } + _, actual := TryLoadKeyFromDisk(tmpdir+rt.pathSuffix, rt.name) if (actual == nil) != rt.expected { t.Errorf( "failed TryLoadCertAndKeyFromDisk:\n\texpected: %t\n\t actual: %t",