From 2ba1b66b57b77c712534dd1537cfaadaf19813c5 Mon Sep 17 00:00:00 2001 From: Kai Wohlfahrt Date: Fri, 5 Sep 2025 21:55:39 +0200 Subject: [PATCH] Fix kubelet certificate reload when connecting by IP Currently, we set TLSConfig.Config.GetCertificate, but then also pass certificate and key paths to http.Server.ListenAndServeTLS. ListenAndServeTLS uses these paths to populate the TLS config Certificate property. Then, when accepting connections, a non-nil Certificate is preferred over GetCertificate if the ServerName is not set in ClientHelloInfo. Finally, the Go TLS client doesn't set ServerName when connecting by IP. As a result, when connecting to the kubelet by IP (e.g. to fetch pod logs), stale certificates are served. This patch passes empty certFile and keyFile arguments, to force the TLS server to use the GetCertificate function. This is done by clearing key/cert file config when setting GetCertificate as suggested in PR review. This way, all downstream users of kubeDeps.TLSConfig will do the right thing automatically. --- pkg/kubelet/kubelet.go | 7 ++ pkg/kubelet/server/server.go | 3 - test/e2e_node/kubelet_tls_test.go | 110 ++++++++++++++++++++++++++++++ 3 files changed, 117 insertions(+), 3 deletions(-) create mode 100644 test/e2e_node/kubelet_tls_test.go diff --git a/pkg/kubelet/kubelet.go b/pkg/kubelet/kubelet.go index b8b7cfd84e9..9ba453897de 100644 --- a/pkg/kubelet/kubelet.go +++ b/pkg/kubelet/kubelet.go @@ -920,6 +920,13 @@ func NewMainKubelet(ctx context.Context, } return cert, nil } + + // GetCertificate is only preferred over the certificate files by + // Golang TLS if the ClientHelloInfo.ServerName is set, which it is + // not when connecting to a host by IP address. Clear the files to + // force the use of GetCertificate. + kubeDeps.TLSOptions.CertFile = "" + kubeDeps.TLSOptions.KeyFile = "" } } diff --git a/pkg/kubelet/server/server.go b/pkg/kubelet/server/server.go index 31ecfe65672..191482784c6 100644 --- a/pkg/kubelet/server/server.go +++ b/pkg/kubelet/server/server.go @@ -194,9 +194,6 @@ func ListenAndServeKubeletServer( if tlsOptions != nil { s.TLSConfig = tlsOptions.Config - // Passing empty strings as the cert and key files means no - // cert/keys are specified and GetCertificate in the TLSConfig - // should be called instead. if err := s.ListenAndServeTLS(tlsOptions.CertFile, tlsOptions.KeyFile); err != nil { logger.Error(err, "Failed to listen and serve") os.Exit(1) diff --git a/test/e2e_node/kubelet_tls_test.go b/test/e2e_node/kubelet_tls_test.go new file mode 100644 index 00000000000..f2f3b52e4ac --- /dev/null +++ b/test/e2e_node/kubelet_tls_test.go @@ -0,0 +1,110 @@ +/* +Copyright 2025 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package e2enode + +import ( + "context" + "crypto/tls" + "crypto/x509" + "fmt" + "net" + "os" + "path/filepath" + + "github.com/onsi/ginkgo/v2" + "github.com/onsi/gomega" + "k8s.io/client-go/util/cert" + "k8s.io/kubernetes/pkg/cluster/ports" + kubeletconfig "k8s.io/kubernetes/pkg/kubelet/apis/config" + "k8s.io/kubernetes/test/e2e/framework" +) + +func createCertAndKeyFiles(certDir string) (string, string, error) { + cert, key, err := cert.GenerateSelfSignedCertKey( + "localhost", + []net.IP{{127, 0, 0, 1}}, + []string{"localhost"}, + ) + if err != nil { + return "", "", nil + } + + certPath := filepath.Join(certDir, "kubelet.cert") + keyPath := filepath.Join(certDir, "kubelet.key") + if err = os.WriteFile(certPath, cert, os.FileMode(0644)); err != nil { + return "", "", err + } + + if err = os.WriteFile(keyPath, key, os.FileMode(0600)); err != nil { + return "", "", err + } + + return certPath, keyPath, nil +} + +func getCert(addr string) (*x509.Certificate, error) { + conn, err := tls.Dial("tcp", addr, &tls.Config{InsecureSkipVerify: true}) + if err != nil { + return nil, err + } + defer func() { _ = conn.Close() }() + + return conn.ConnectionState().PeerCertificates[0], nil +} + +var _ = SIGDescribe("KubletTLS", framework.WithSerial(), framework.WithNodeConformance(), func() { + f := framework.NewDefaultFramework("kubelet-tls-test") + + ginkgo.Context("certificate reload", func() { + var tempDir string + + t := ginkgo.GinkgoT() + ginkgo.BeforeEach(func(ctx context.Context) { + tempDir = t.TempDir() + cfg, err := getCurrentKubeletConfig(ctx) + framework.ExpectNoError(err) + ginkgo.DeferCleanup(func(ctx context.Context, cfg *kubeletconfig.KubeletConfiguration) { + updateKubeletConfig(ctx, f, cfg, true) + }, cfg.DeepCopy()) + + certPath, keyPath, err := createCertAndKeyFiles(tempDir) + framework.ExpectNoError(err) + + cfg.TLSCertFile = certPath + cfg.TLSPrivateKeyFile = keyPath + updateKubeletConfig(ctx, f, cfg, true) + }) + + ginkgo.It("should reload certificates from disk", func(ctx context.Context) { + addr := fmt.Sprintf("127.0.0.1:%d", ports.KubeletPort) + oldCert, err := getCert(addr) + framework.ExpectNoError(err) + + _, _, err = createCertAndKeyFiles(tempDir) + framework.ExpectNoError(err) + + gomega.Eventually(ctx, func(ctx context.Context) (bool, error) { + newCert, err := getCert(addr) + if err != nil { + return true, err + } + + return newCert.Equal(oldCert), nil + }).Should(gomega.BeFalseBecause("new certificate should be loaded")) + }) + }) +})