From 5c00024c70bee36d12a1a5293a1940f2bb7de95b Mon Sep 17 00:00:00 2001 From: "Lubomir I. Ivanov" Date: Tue, 29 Jun 2021 23:13:32 +0300 Subject: [PATCH] kubeadm: fix wrong check for keys/certs during "download-certs" During "join" of new control plane machines, kubeadm would download shared certificates and keys from the cluster stored in a Secret. Based on the contents of an entry in the Secret, it would use helper functions from client-go to either write it as public key, cert (mode 644) or as a private key (mode 600). The existing logic is always writing both keys and certs with mode 600. Allow detecting public readable data properly and writing some files with mode 644. First check the data with ParsePrivateKeyPEM(); if this passes there must be at least one private key and the file should be written with mode 600 as private. If that fails, validate if the data contains public keys with ParsePublicKeysPEM() and write the file as public (mode 644). As a result of this new logic, and given the current set of managed kubeadm files, .key files will end up with 600, while .crt and .pub files will end up with 644. --- cmd/kubeadm/app/phases/copycerts/copycerts.go | 4 ++-- cmd/kubeadm/app/phases/copycerts/copycerts_test.go | 5 ++--- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/cmd/kubeadm/app/phases/copycerts/copycerts.go b/cmd/kubeadm/app/phases/copycerts/copycerts.go index 199fc4845a5..cdc65918854 100644 --- a/cmd/kubeadm/app/phases/copycerts/copycerts.go +++ b/cmd/kubeadm/app/phases/copycerts/copycerts.go @@ -251,9 +251,9 @@ func DownloadCerts(client clientset.Interface, cfg *kubeadmapi.InitConfiguration } func writeCertOrKey(certOrKeyPath string, certOrKeyData []byte) error { - if _, err := keyutil.ParsePublicKeysPEM(certOrKeyData); err == nil { + if _, err := keyutil.ParsePrivateKeyPEM(certOrKeyData); err == nil { return keyutil.WriteKey(certOrKeyPath, certOrKeyData) - } else if _, err := certutil.ParseCertsPEM(certOrKeyData); err == nil { + } else if _, err := keyutil.ParsePublicKeysPEM(certOrKeyData); err == nil { return certutil.WriteCert(certOrKeyPath, certOrKeyData) } return errors.New("unknown data found in Secret entry") diff --git a/cmd/kubeadm/app/phases/copycerts/copycerts_test.go b/cmd/kubeadm/app/phases/copycerts/copycerts_test.go index be2032aab6a..76a91c2b254 100644 --- a/cmd/kubeadm/app/phases/copycerts/copycerts_test.go +++ b/cmd/kubeadm/app/phases/copycerts/copycerts_test.go @@ -29,7 +29,6 @@ import ( v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" fakeclient "k8s.io/client-go/kubernetes/fake" - certutil "k8s.io/client-go/util/cert" keyutil "k8s.io/client-go/util/keyutil" kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm" kubeadmconstants "k8s.io/kubernetes/cmd/kubeadm/app/constants" @@ -240,7 +239,7 @@ func TestDownloadCerts(t *testing.T) { } // Check that the written files are either certificates or keys, and that they have // the expected permissions - if _, err := keyutil.ParsePublicKeysPEM(diskCertData); err == nil { + if _, err := keyutil.ParsePrivateKeyPEM(diskCertData); err == nil { if stat, err := os.Stat(certPath); err == nil { if stat.Mode() != keyFileMode { t.Errorf("key %q should have mode %#o, has %#o", certName, keyFileMode, stat.Mode()) @@ -248,7 +247,7 @@ func TestDownloadCerts(t *testing.T) { } else { t.Errorf("could not stat key %q: %v", certName, err) } - } else if _, err := certutil.ParseCertsPEM(diskCertData); err == nil { + } else if _, err := keyutil.ParsePublicKeysPEM(diskCertData); err == nil { if stat, err := os.Stat(certPath); err == nil { if stat.Mode() != certFileMode { t.Errorf("cert %q should have mode %#o, has %#o", certName, certFileMode, stat.Mode())