diff --git a/cmd/kubeadm/app/constants/constants.go b/cmd/kubeadm/app/constants/constants.go index b1ce9609428..05e6754bae5 100644 --- a/cmd/kubeadm/app/constants/constants.go +++ b/cmd/kubeadm/app/constants/constants.go @@ -31,6 +31,10 @@ const ( APIServerKubeletClientCertName = "apiserver-kubelet-client.crt" APIServerKubeletClientKeyName = "apiserver-kubelet-client.key" + ServiceAccountKeyBaseName = "sa" + ServiceAccountPublicKeyName = "sa.pub" + ServiceAccountPrivateKeyName = "sa.key" + // TODO: These constants should actually come from pkg/kubeapiserver/authorizer, but we can't vendor that package in now // because of all the other sub-packages that would get vendored. To fix this, a pkg/kubeapiserver/authorizer/modes package // or similar should exist that only has these constants; then we can vendor it. diff --git a/cmd/kubeadm/app/master/manifests.go b/cmd/kubeadm/app/master/manifests.go index 421c2250a8e..7632eabc900 100644 --- a/cmd/kubeadm/app/master/manifests.go +++ b/cmd/kubeadm/app/master/manifests.go @@ -309,7 +309,7 @@ func getAPIServerCommand(cfg *kubeadmapi.MasterConfiguration, selfHosted bool) [ "--insecure-bind-address=127.0.0.1", "--admission-control=NamespaceLifecycle,LimitRanger,ServiceAccount,PersistentVolumeLabel,DefaultStorageClass,ResourceQuota", "--service-cluster-ip-range="+cfg.Networking.ServiceSubnet, - "--service-account-key-file="+getCertFilePath(kubeadmconstants.APIServerKeyName), + "--service-account-key-file="+getCertFilePath(kubeadmconstants.ServiceAccountPublicKeyName), "--client-ca-file="+getCertFilePath(kubeadmconstants.CACertName), "--tls-cert-file="+getCertFilePath(kubeadmconstants.APIServerCertName), "--tls-private-key-file="+getCertFilePath(kubeadmconstants.APIServerKeyName), @@ -384,7 +384,7 @@ func getControllerManagerCommand(cfg *kubeadmapi.MasterConfiguration, selfHosted "--master=127.0.0.1:8080", "--cluster-name="+DefaultClusterName, "--root-ca-file="+getCertFilePath(kubeadmconstants.CACertName), - "--service-account-private-key-file="+getCertFilePath(kubeadmconstants.APIServerKeyName), + "--service-account-private-key-file="+getCertFilePath(kubeadmconstants.ServiceAccountPrivateKeyName), "--cluster-signing-cert-file="+getCertFilePath(kubeadmconstants.CACertName), "--cluster-signing-key-file="+getCertFilePath(kubeadmconstants.CAKeyName), "--insecure-experimental-approve-all-kubelet-csrs-for-group="+KubeletBootstrapGroup, diff --git a/cmd/kubeadm/app/master/manifests_test.go b/cmd/kubeadm/app/master/manifests_test.go index df04770f42b..c8bf75932c4 100644 --- a/cmd/kubeadm/app/master/manifests_test.go +++ b/cmd/kubeadm/app/master/manifests_test.go @@ -372,7 +372,7 @@ func TestGetAPIServerCommand(t *testing.T) { "--insecure-bind-address=127.0.0.1", "--admission-control=NamespaceLifecycle,LimitRanger,ServiceAccount,PersistentVolumeLabel,DefaultStorageClass,ResourceQuota", "--service-cluster-ip-range=bar", - "--service-account-key-file=" + kubeadmapi.GlobalEnvParams.HostPKIPath + "/apiserver.key", + "--service-account-key-file=" + kubeadmapi.GlobalEnvParams.HostPKIPath + "/sa.pub", "--client-ca-file=" + kubeadmapi.GlobalEnvParams.HostPKIPath + "/ca.crt", "--tls-cert-file=" + kubeadmapi.GlobalEnvParams.HostPKIPath + "/apiserver.crt", "--tls-private-key-file=" + kubeadmapi.GlobalEnvParams.HostPKIPath + "/apiserver.key", @@ -396,7 +396,7 @@ func TestGetAPIServerCommand(t *testing.T) { "--insecure-bind-address=127.0.0.1", "--admission-control=NamespaceLifecycle,LimitRanger,ServiceAccount,PersistentVolumeLabel,DefaultStorageClass,ResourceQuota", "--service-cluster-ip-range=bar", - "--service-account-key-file=" + kubeadmapi.GlobalEnvParams.HostPKIPath + "/apiserver.key", + "--service-account-key-file=" + kubeadmapi.GlobalEnvParams.HostPKIPath + "/sa.pub", "--client-ca-file=" + kubeadmapi.GlobalEnvParams.HostPKIPath + "/ca.crt", "--tls-cert-file=" + kubeadmapi.GlobalEnvParams.HostPKIPath + "/apiserver.crt", "--tls-private-key-file=" + kubeadmapi.GlobalEnvParams.HostPKIPath + "/apiserver.key", @@ -422,7 +422,7 @@ func TestGetAPIServerCommand(t *testing.T) { "--insecure-bind-address=127.0.0.1", "--admission-control=NamespaceLifecycle,LimitRanger,ServiceAccount,PersistentVolumeLabel,DefaultStorageClass,ResourceQuota", "--service-cluster-ip-range=bar", - "--service-account-key-file=" + kubeadmapi.GlobalEnvParams.HostPKIPath + "/apiserver.key", + "--service-account-key-file=" + kubeadmapi.GlobalEnvParams.HostPKIPath + "/sa.pub", "--client-ca-file=" + kubeadmapi.GlobalEnvParams.HostPKIPath + "/ca.crt", "--tls-cert-file=" + kubeadmapi.GlobalEnvParams.HostPKIPath + "/apiserver.crt", "--tls-private-key-file=" + kubeadmapi.GlobalEnvParams.HostPKIPath + "/apiserver.key", @@ -468,7 +468,7 @@ func TestGetControllerManagerCommand(t *testing.T) { "--master=127.0.0.1:8080", "--cluster-name=" + DefaultClusterName, "--root-ca-file=" + kubeadmapi.GlobalEnvParams.HostPKIPath + "/ca.crt", - "--service-account-private-key-file=" + kubeadmapi.GlobalEnvParams.HostPKIPath + "/apiserver.key", + "--service-account-private-key-file=" + kubeadmapi.GlobalEnvParams.HostPKIPath + "/sa.key", "--cluster-signing-cert-file=" + kubeadmapi.GlobalEnvParams.HostPKIPath + "/ca.crt", "--cluster-signing-key-file=" + kubeadmapi.GlobalEnvParams.HostPKIPath + "/ca.key", "--insecure-experimental-approve-all-kubelet-csrs-for-group=kubeadm:kubelet-bootstrap", @@ -483,7 +483,7 @@ func TestGetControllerManagerCommand(t *testing.T) { "--master=127.0.0.1:8080", "--cluster-name=" + DefaultClusterName, "--root-ca-file=" + kubeadmapi.GlobalEnvParams.HostPKIPath + "/ca.crt", - "--service-account-private-key-file=" + kubeadmapi.GlobalEnvParams.HostPKIPath + "/apiserver.key", + "--service-account-private-key-file=" + kubeadmapi.GlobalEnvParams.HostPKIPath + "/sa.key", "--cluster-signing-cert-file=" + kubeadmapi.GlobalEnvParams.HostPKIPath + "/ca.crt", "--cluster-signing-key-file=" + kubeadmapi.GlobalEnvParams.HostPKIPath + "/ca.key", "--insecure-experimental-approve-all-kubelet-csrs-for-group=kubeadm:kubelet-bootstrap", @@ -499,7 +499,7 @@ func TestGetControllerManagerCommand(t *testing.T) { "--master=127.0.0.1:8080", "--cluster-name=" + DefaultClusterName, "--root-ca-file=" + kubeadmapi.GlobalEnvParams.HostPKIPath + "/ca.crt", - "--service-account-private-key-file=" + kubeadmapi.GlobalEnvParams.HostPKIPath + "/apiserver.key", + "--service-account-private-key-file=" + kubeadmapi.GlobalEnvParams.HostPKIPath + "/sa.key", "--cluster-signing-cert-file=" + kubeadmapi.GlobalEnvParams.HostPKIPath + "/ca.crt", "--cluster-signing-key-file=" + kubeadmapi.GlobalEnvParams.HostPKIPath + "/ca.key", "--insecure-experimental-approve-all-kubelet-csrs-for-group=kubeadm:kubelet-bootstrap", diff --git a/cmd/kubeadm/app/phases/certs/certs.go b/cmd/kubeadm/app/phases/certs/certs.go index 4db869d15a4..c5271b6999d 100644 --- a/cmd/kubeadm/app/phases/certs/certs.go +++ b/cmd/kubeadm/app/phases/certs/certs.go @@ -165,6 +165,33 @@ func CreatePKIAssets(cfg *kubeadmapi.MasterConfiguration, pkiDir string) error { fmt.Println("[certificates] Generated API server kubelet client certificate and key.") } + // If the key exists, we should try to load it + if pkiutil.CertOrKeyExist(pkiDir, kubeadmconstants.ServiceAccountPrivateKeyName) { + // Try to load sa.key from the PKI directory + _, err := pkiutil.TryLoadKeyFromDisk(pkiDir, kubeadmconstants.ServiceAccountPrivateKeyName) + if err != nil { + return fmt.Errorf("certificate and/or key existed but they could not be loaded properly [%v]", err) + } + + fmt.Println("[certificates] Using the existing service account token signing key.") + } else { + // The key does NOT exist, let's generate it now + saTokenSigningKey, err := certutil.NewPrivateKey() + if err != nil { + return fmt.Errorf("failure while creating service account token signing key [%v]", err) + } + + if err = pkiutil.WriteKey(pkiDir, kubeadmconstants.ServiceAccountKeyBaseName, saTokenSigningKey); err != nil { + return fmt.Errorf("failure while saving service account token signing key [%v]", err) + } + fmt.Println("[certificates] Generated service account token signing key.") + + if err = pkiutil.WritePublicKey(pkiDir, kubeadmconstants.ServiceAccountKeyBaseName, &saTokenSigningKey.PublicKey); err != nil { + return fmt.Errorf("failure while saving service account token signing public key [%v]", err) + } + fmt.Println("[certificates] Generated service account token signing public key.") + } + fmt.Printf("[certificates] Valid certificates and keys now exist in %q\n", pkiDir) return nil diff --git a/cmd/kubeadm/app/phases/certs/pkiutil/pki_helpers.go b/cmd/kubeadm/app/phases/certs/pkiutil/pki_helpers.go index 5a95160db80..94ddf733788 100644 --- a/cmd/kubeadm/app/phases/certs/pkiutil/pki_helpers.go +++ b/cmd/kubeadm/app/phases/certs/pkiutil/pki_helpers.go @@ -17,7 +17,6 @@ limitations under the License. package pkiutil import ( - "crypto/ecdsa" "crypto/rsa" "crypto/x509" "fmt" @@ -63,21 +62,55 @@ func NewCertAndKey(caCert *x509.Certificate, caKey *rsa.PrivateKey, config certu } func WriteCertAndKey(pkiPath string, name string, cert *x509.Certificate, key *rsa.PrivateKey) error { - certificatePath, privateKeyPath := pathsForCertAndKey(pkiPath, name) + if err := WriteKey(pkiPath, name, key); err != nil { + return err + } + if err := WriteCert(pkiPath, name, cert); err != nil { + return err + } + + return nil +} + +func WriteCert(pkiPath, name string, cert *x509.Certificate) error { if cert == nil { return fmt.Errorf("certificate cannot be nil when writing to file") } - if cert == nil { + + certificatePath := pathForCert(pkiPath, name) + if err := certutil.WriteCert(certificatePath, certutil.EncodeCertPEM(cert)); err != nil { + return fmt.Errorf("unable to write certificate to file %q: [%v]", certificatePath, err) + } + + return nil +} + +func WriteKey(pkiPath, name string, key *rsa.PrivateKey) error { + if key == nil { return fmt.Errorf("private key cannot be nil when writing to file") } + privateKeyPath := pathForKey(pkiPath, name) if err := certutil.WriteKey(privateKeyPath, certutil.EncodePrivateKeyPEM(key)); err != nil { return fmt.Errorf("unable to write private key to file %q: [%v]", privateKeyPath, err) } - if err := certutil.WriteCert(certificatePath, certutil.EncodeCertPEM(cert)); err != nil { - return fmt.Errorf("unable to write certificate to file %q: [%v]", certificatePath, err) + return nil +} + +func WritePublicKey(pkiPath, name string, key *rsa.PublicKey) error { + if key == nil { + return fmt.Errorf("public key cannot be nil when writing to file") + } + + publicKeyBytes, err := certutil.EncodePublicKeyPEM(key) + if err != nil { + return err + } + publicKeyPath := pathForPublicKey(pkiPath, name) + if err := certutil.WriteKey(publicKeyPath, publicKeyBytes); err != nil { + return fmt.Errorf("unable to write public key to file %q: [%v]", publicKeyPath, err) } return nil @@ -100,46 +133,78 @@ func CertOrKeyExist(pkiPath, 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) { - certificatePath, privateKeyPath := pathsForCertAndKey(pkiPath, name) + cert, err := TryLoadCertFromDisk(pkiPath, name) + if err != nil { + return nil, nil, err + } + + key, err := TryLoadKeyFromDisk(pkiPath, name) + if err != nil { + return nil, nil, err + } + + return cert, key, nil +} + +// TryLoadCertFromDisk tries to load the cert from the disk and validates that it is valid +func TryLoadCertFromDisk(pkiPath, name string) (*x509.Certificate, error) { + certificatePath := pathForCert(pkiPath, name) certs, err := certutil.CertsFromFile(certificatePath) if err != nil { - return nil, nil, fmt.Errorf("couldn't load the certificate file %s: %v", certificatePath, err) + return nil, fmt.Errorf("couldn't load the certificate file %s: %v", certificatePath, err) } // We are only putting one certificate in the certificate pem file, so it's safe to just pick the first one // TODO: Support multiple certs here in order to be able to rotate certs cert := certs[0] + // Check so that the certificate is valid now + now := time.Now() + if now.Before(cert.NotBefore) { + return nil, fmt.Errorf("the certificate is not valid yet") + } + if now.After(cert.NotAfter) { + return nil, fmt.Errorf("the certificate has expired") + } + + return cert, nil +} + +// TryLoadKeyFromDisk tries to load the key from the disk and validates that it is valid +func TryLoadKeyFromDisk(pkiPath, name string) (*rsa.PrivateKey, error) { + privateKeyPath := pathForKey(pkiPath, name) + // Parse the private key from a file privKey, err := certutil.PrivateKeyFromFile(privateKeyPath) if err != nil { - return nil, nil, fmt.Errorf("couldn't load the private key file %s: %v", privateKeyPath, err) + return nil, fmt.Errorf("couldn't load the private key file %s: %v", privateKeyPath, err) } + + // Allow RSA format only var key *rsa.PrivateKey switch k := privKey.(type) { case *rsa.PrivateKey: key = k - case *ecdsa.PrivateKey: - // TODO: Abstract rsa.PrivateKey away and make certutil.NewSignedCert accept a ecdsa.PrivateKey as well - // After that, we can support generating kubeconfig files from ecdsa private keys as well - return nil, nil, fmt.Errorf("the private key file %s isn't in RSA format", privateKeyPath) default: - return nil, nil, fmt.Errorf("the private key file %s isn't in RSA format", privateKeyPath) + return nil, fmt.Errorf("the private key file %s isn't in RSA format", privateKeyPath) } - // Check so that the certificate is valid now - now := time.Now() - if now.Before(cert.NotBefore) { - return nil, nil, fmt.Errorf("the certificate is not valid yet") - } - if now.After(cert.NotAfter) { - return nil, nil, fmt.Errorf("the certificate is has expired") - } - - return cert, key, nil + return key, nil } func pathsForCertAndKey(pkiPath, name string) (string, string) { - return path.Join(pkiPath, fmt.Sprintf("%s.crt", name)), path.Join(pkiPath, fmt.Sprintf("%s.key", name)) + return pathForCert(pkiPath, name), pathForKey(pkiPath, name) +} + +func pathForCert(pkiPath, name string) string { + return path.Join(pkiPath, fmt.Sprintf("%s.crt", name)) +} + +func pathForKey(pkiPath, name string) string { + return path.Join(pkiPath, fmt.Sprintf("%s.key", name)) +} + +func pathForPublicKey(pkiPath, name string) string { + return path.Join(pkiPath, fmt.Sprintf("%s.pub", name)) } diff --git a/cmd/kubeadm/app/phases/certs/pkiutil/pki_helpers_test.go b/cmd/kubeadm/app/phases/certs/pkiutil/pki_helpers_test.go index 08658bf77c0..75004cc65f3 100644 --- a/cmd/kubeadm/app/phases/certs/pkiutil/pki_helpers_test.go +++ b/cmd/kubeadm/app/phases/certs/pkiutil/pki_helpers_test.go @@ -108,6 +108,63 @@ func TestWriteCertAndKey(t *testing.T) { } } +func TestWriteCert(t *testing.T) { + tmpdir, err := ioutil.TempDir("", "") + if err != nil { + t.Fatalf("Couldn't create tmpdir") + } + defer os.RemoveAll(tmpdir) + + caCert := &x509.Certificate{} + actual := WriteCert(tmpdir, "foo", caCert) + if actual != nil { + t.Errorf( + "failed WriteCertAndKey with an error: %v", + actual, + ) + } +} + +func TestWriteKey(t *testing.T) { + tmpdir, err := ioutil.TempDir("", "") + if err != nil { + t.Fatalf("Couldn't create tmpdir") + } + defer os.RemoveAll(tmpdir) + + caKey, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + t.Fatalf("Couldn't create rsa Private Key") + } + actual := WriteKey(tmpdir, "foo", caKey) + if actual != nil { + t.Errorf( + "failed WriteCertAndKey with an error: %v", + actual, + ) + } +} + +func TestWritePublicKey(t *testing.T) { + tmpdir, err := ioutil.TempDir("", "") + if err != nil { + t.Fatalf("Couldn't create tmpdir") + } + defer os.RemoveAll(tmpdir) + + caKey, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + t.Fatalf("Couldn't create rsa Private Key") + } + actual := WritePublicKey(tmpdir, "foo", &caKey.PublicKey) + if actual != nil { + t.Errorf( + "failed WriteCertAndKey with an error: %v", + actual, + ) + } +} + func TestCertOrKeyExist(t *testing.T) { tmpdir, err := ioutil.TempDir("", "") if err != nil { @@ -205,3 +262,134 @@ func TestTryLoadCertAndKeyFromDisk(t *testing.T) { } } } + +func TestTryLoadCertFromDisk(t *testing.T) { + tmpdir, err := ioutil.TempDir("", "") + if err != nil { + t.Fatalf("Couldn't create tmpdir") + } + defer os.RemoveAll(tmpdir) + + caCert, _, err := NewCertificateAuthority() + if err != nil { + t.Errorf( + "failed to create cert and key with an error: %v", + err, + ) + } + err = WriteCert(tmpdir, "foo", caCert) + if err != nil { + t.Errorf( + "failed to write cert and key with an error: %v", + err, + ) + } + + var tests = []struct { + path string + name string + expected bool + }{ + { + path: "", + name: "", + expected: false, + }, + { + path: tmpdir, + name: "foo", + expected: true, + }, + } + for _, rt := range tests { + _, actual := TryLoadCertFromDisk(rt.path, rt.name) + if (actual == nil) != rt.expected { + t.Errorf( + "failed TryLoadCertAndKeyFromDisk:\n\texpected: %t\n\t actual: %t", + rt.expected, + (actual == nil), + ) + } + } +} + +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() + 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 { + path string + name string + expected bool + }{ + { + path: "", + name: "", + expected: false, + }, + { + path: tmpdir, + name: "foo", + expected: true, + }, + } + for _, rt := range tests { + _, actual := TryLoadKeyFromDisk(rt.path, rt.name) + if (actual == nil) != rt.expected { + t.Errorf( + "failed TryLoadCertAndKeyFromDisk:\n\texpected: %t\n\t actual: %t", + rt.expected, + (actual == nil), + ) + } + } +} + +func TestPathsForCertAndKey(t *testing.T) { + crtPath, keyPath := pathsForCertAndKey("/foo", "bar") + if crtPath != "/foo/bar.crt" { + t.Errorf("unexpected certificate path: %s", crtPath) + } + if keyPath != "/foo/bar.key" { + t.Errorf("unexpected key path: %s", keyPath) + } +} + +func TestPathForCert(t *testing.T) { + crtPath := pathForCert("/foo", "bar") + if crtPath != "/foo/bar.crt" { + t.Errorf("unexpected certificate path: %s", crtPath) + } +} + +func TestPathForKey(t *testing.T) { + keyPath := pathForKey("/foo", "bar") + if keyPath != "/foo/bar.key" { + t.Errorf("unexpected certificate path: %s", keyPath) + } +} + +func TestPathForPublicKey(t *testing.T) { + pubPath := pathForPublicKey("/foo", "bar") + if pubPath != "/foo/bar.pub" { + t.Errorf("unexpected certificate path: %s", pubPath) + } +}