Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

kubeadm: update embedded CA in kubeconfig files on renewal #88052

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions cmd/kubeadm/app/phases/certs/renewal/BUILD
Expand Up @@ -41,6 +41,7 @@ go_test(
embed = [":go_default_library"],
deps = [
"//cmd/kubeadm/app/apis/kubeadm:go_default_library",
"//cmd/kubeadm/app/constants:go_default_library",
"//cmd/kubeadm/app/util/certs:go_default_library",
"//cmd/kubeadm/app/util/kubeconfig:go_default_library",
"//cmd/kubeadm/app/util/pkiutil:go_default_library",
Expand Down
3 changes: 2 additions & 1 deletion cmd/kubeadm/app/phases/certs/renewal/manager.go
Expand Up @@ -154,7 +154,8 @@ func NewManager(cfg *kubeadmapi.ClusterConfiguration, kubernetesDir string) (*Ma
// create a CertificateRenewHandler for each kubeConfig file
for _, kubeConfig := range kubeConfigs {
// create a ReadWriter for certificates embedded in kubeConfig files
kubeConfigReadWriter := newKubeconfigReadWriter(kubernetesDir, kubeConfig.fileName)
kubeConfigReadWriter := newKubeconfigReadWriter(kubernetesDir, kubeConfig.fileName,
rm.cfg.CertificatesDir, kubeadmconstants.CACertAndKeyBaseName)

// adds the certificateRenewHandler.
// Certificates embedded kubeConfig files in are indexed by fileName, that is a well know constant defined
Expand Down
30 changes: 26 additions & 4 deletions cmd/kubeadm/app/phases/certs/renewal/readwriter.go
Expand Up @@ -28,6 +28,7 @@ import (
clientcmdapi "k8s.io/client-go/tools/clientcmd/api"
certutil "k8s.io/client-go/util/cert"
"k8s.io/client-go/util/keyutil"
certsphase "k8s.io/kubernetes/cmd/kubeadm/app/phases/certs"
pkiutil "k8s.io/kubernetes/cmd/kubeadm/app/util/pkiutil"
)

Expand Down Expand Up @@ -104,14 +105,19 @@ type kubeConfigReadWriter struct {
kubeConfigFileName string
kubeConfigFilePath string
kubeConfig *clientcmdapi.Config
baseName string
certificateDir string
caCert *x509.Certificate
}

// newKubeconfigReadWriter return a new kubeConfigReadWriter
func newKubeconfigReadWriter(kubernetesDir string, kubeConfigFileName string) *kubeConfigReadWriter {
func newKubeconfigReadWriter(kubernetesDir string, kubeConfigFileName string, certificateDir, baseName string) *kubeConfigReadWriter {
return &kubeConfigReadWriter{
kubernetesDir: kubernetesDir,
kubeConfigFileName: kubeConfigFileName,
kubeConfigFilePath: filepath.Join(kubernetesDir, kubeConfigFileName),
certificateDir: certificateDir,
baseName: baseName,
}
}

Expand All @@ -130,6 +136,16 @@ func (rw *kubeConfigReadWriter) Read() (*x509.Certificate, error) {
return nil, errors.Wrapf(err, "failed to load kubeConfig file %s", rw.kubeConfigFilePath)
}

// The CA cert is required for updating kubeconfig files.
// For local CA renewal, the local CA on disk could have changed, thus a reload is needed.
// For CSR renewal we assume the same CA on disk is mounted for usage with KCM's
// '--cluster-signing-cert-file' flag.
caCert, _, err := certsphase.LoadCertificateAuthority(rw.certificateDir, rw.baseName)
if err != nil {
return nil, err
}
rw.caCert = caCert

// get current context
if _, ok := kubeConfig.Contexts[kubeConfig.CurrentContext]; !ok {
return nil, errors.Errorf("invalid kubeConfig file %s: missing context %s", rw.kubeConfigFilePath, kubeConfig.CurrentContext)
Expand All @@ -143,7 +159,7 @@ func (rw *kubeConfigReadWriter) Read() (*x509.Certificate, error) {

cluster := kubeConfig.Clusters[clusterName]
if len(cluster.CertificateAuthorityData) == 0 {
return nil, errors.Errorf("kubeConfig file %s does not have and embedded server certificate", rw.kubeConfigFilePath)
return nil, errors.Errorf("kubeConfig file %s does not have an embedded server certificate", rw.kubeConfigFilePath)
}

// get auth info for current context and ensure a client certificate is embedded in it
Expand All @@ -154,7 +170,7 @@ func (rw *kubeConfigReadWriter) Read() (*x509.Certificate, error) {

authInfo := kubeConfig.AuthInfos[authInfoName]
if len(authInfo.ClientCertificateData) == 0 {
return nil, errors.Errorf("kubeConfig file %s does not have and embedded client certificate", rw.kubeConfigFilePath)
return nil, errors.Errorf("kubeConfig file %s does not have an embedded client certificate", rw.kubeConfigFilePath)
}

// parse the client certificate, retrive the cert config and then renew it
Expand All @@ -174,7 +190,7 @@ func (rw *kubeConfigReadWriter) Read() (*x509.Certificate, error) {
func (rw *kubeConfigReadWriter) Write(newCert *x509.Certificate, newKey crypto.Signer) error {
// check if Read was called before Write
if rw.kubeConfig == nil {
return errors.Errorf("failed to Write kubeConfig file with renewd certs. It is necessary to call Read before Write")
return errors.Errorf("failed to Write kubeConfig file with renewed certs. It is necessary to call Read before Write")
}

// encodes the new key
Expand All @@ -183,6 +199,12 @@ func (rw *kubeConfigReadWriter) Write(newCert *x509.Certificate, newKey crypto.S
return errors.Wrapf(err, "failed to marshal private key to PEM")
}

// Update the embedded CA in the kubeconfig file.
// This assumes that the user has kept the current context to the desired one.
clusterName := rw.kubeConfig.Contexts[rw.kubeConfig.CurrentContext].Cluster
cluster := rw.kubeConfig.Clusters[clusterName]
cluster.CertificateAuthorityData = pkiutil.EncodeCertPEM(rw.caCert)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks a bit scary. Perhaps log the overwrite somewhere if the old and new bytes differ?
Other than that, it looks good to me.

/lgtm
/hold
for @fabriziopandini to TAL.

Copy link
Member Author

@neolit123 neolit123 Feb 14, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well, we are using the same CA file for the renewal of certificates.
using a comparison first is fine, but it will not gain security benefits, only a performance benefit, only a minor performance degradation.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, I guess I am over paranoid here. It's fine like it is.


// get auth info for current context and ensure a client certificate is embedded in it
authInfoName := rw.kubeConfig.Contexts[rw.kubeConfig.CurrentContext].AuthInfo

Expand Down
23 changes: 18 additions & 5 deletions cmd/kubeadm/app/phases/certs/renewal/readwriter_test.go
Expand Up @@ -27,6 +27,7 @@ import (
"k8s.io/client-go/tools/clientcmd"
certutil "k8s.io/client-go/util/cert"
"k8s.io/client-go/util/keyutil"
kubeadmconstants "k8s.io/kubernetes/cmd/kubeadm/app/constants"
kubeconfigutil "k8s.io/kubernetes/cmd/kubeadm/app/util/kubeconfig"
pkiutil "k8s.io/kubernetes/cmd/kubeadm/app/util/pkiutil"
testutil "k8s.io/kubernetes/cmd/kubeadm/test"
Expand Down Expand Up @@ -79,15 +80,27 @@ func TestPKICertificateReadWriter(t *testing.T) {
}

func TestKubeconfigReadWriter(t *testing.T) {
// creates a tmp folder
dir := testutil.SetupTempDir(t)
defer os.RemoveAll(dir)
// creates tmp folders
dirKubernetes := testutil.SetupTempDir(t)
defer os.RemoveAll(dirKubernetes)
dirPKI := testutil.SetupTempDir(t)
defer os.RemoveAll(dirPKI)

// write the CA cert and key to the temporary PKI dir
caName := kubeadmconstants.CACertAndKeyBaseName
if err := pkiutil.WriteCertAndKey(
dirPKI,
caName,
testCACert,
testCAKey); err != nil {
t.Fatalf("couldn't write out certificate %s to %s", caName, dirPKI)
}

// creates a certificate and then embeds it into a kubeconfig file
cert := writeTestKubeconfig(t, dir, "test", testCACert, testCAKey)
cert := writeTestKubeconfig(t, dirKubernetes, "test", testCACert, testCAKey)

// Creates a KubeconfigReadWriter
kubeconfigReadWriter := newKubeconfigReadWriter(dir, "test")
kubeconfigReadWriter := newKubeconfigReadWriter(dirKubernetes, "test", dirPKI, caName)

// Reads the certificate embedded in a kubeconfig
readCert, err := kubeconfigReadWriter.Read()
Expand Down