diff --git a/pkg/kubelet/server/server.go b/pkg/kubelet/server/server.go index 73a36b5b56b..35f93f161e6 100644 --- a/pkg/kubelet/server/server.go +++ b/pkg/kubelet/server/server.go @@ -306,9 +306,10 @@ func NewServer( if auth != nil { server.InstallAuthFilter() } - server.InstallDefaultHandlers() + server.InstallAuthNotRequiredHandlers() if kubeCfg != nil && kubeCfg.EnableDebuggingHandlers { - server.InstallDebuggingHandlers() + klog.InfoS("Adding debug handlers to kubelet server") + server.InstallAuthRequiredHandlers() // To maintain backward compatibility serve logs and pprof only when enableDebuggingHandlers is also enabled // see https://github.com/kubernetes/kubernetes/pull/87273 server.InstallSystemLogHandler(kubeCfg.EnableSystemLogHandler, kubeCfg.EnableSystemLogQuery) @@ -402,9 +403,11 @@ func (s *Server) getMetricMethodBucket(method string) string { return "other" } -// InstallDefaultHandlers registers the default set of supported HTTP request -// patterns with the restful Container. -func (s *Server) InstallDefaultHandlers() { +// InstallAuthNotRequiredHandlers registers request handlers that do not require authorization, which are +// installed on both the unsecured and secured (TLS) servers. +// NOTE: This method is maintained for backwards compatibility, but no new endpoints should be added +// to this set. New handlers should be added under InstallAuthorizedHandlers. +func (s *Server) InstallAuthNotRequiredHandlers() { s.addMetricsBucketMatcher("healthz") checkers := []healthz.HealthChecker{ healthz.PingHealthz, @@ -480,12 +483,15 @@ func (s *Server) InstallDefaultHandlers() { s.restfulCont.Handle(proberMetricsPath, compbasemetrics.HandlerFor(p, compbasemetrics.HandlerOpts{ErrorHandling: compbasemetrics.ContinueOnError}), ) + + // DO NOT ADD NEW HANDLERS HERE! + // See note in method comment. } -// InstallDebuggingHandlers registers the HTTP request patterns that serve logs or run commands/containers -func (s *Server) InstallDebuggingHandlers() { - klog.InfoS("Adding debug handlers to kubelet server") - +// InstallAuthRequiredHandlers registers the HTTP handlers that should only be installed on servers +// with authorization enabled. +// NOTE: New endpoints must require authorization. +func (s *Server) InstallAuthRequiredHandlers() { s.addMetricsBucketMatcher("run") ws := new(restful.WebService) ws. diff --git a/pkg/kubelet/server/server_test.go b/pkg/kubelet/server/server_test.go index e3a9340962c..fe58760dd4e 100644 --- a/pkg/kubelet/server/server_test.go +++ b/pkg/kubelet/server/server_test.go @@ -44,6 +44,7 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/httpstream" "k8s.io/apimachinery/pkg/util/httpstream/spdy" + "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apiserver/pkg/authentication/authenticator" "k8s.io/apiserver/pkg/authentication/user" "k8s.io/apiserver/pkg/authorization/authorizer" @@ -575,6 +576,72 @@ func TestAuthzCoverage(t *testing.T) { } } +func TestInstallAuthNotRequiredHandlers(t *testing.T) { + fw := newServerTestWithDebug(false, nil) + defer fw.testHTTPServer.Close() + + // No new handlers should be added to this list. + allowedAuthNotRequiredHandlers := sets.NewString( + "/healthz", + "/healthz/log", + "/healthz/ping", + "/healthz/syncloop", + "/metrics", + "/metrics/slis", + "/metrics/cadvisor", + "/metrics/probes", + "/metrics/resource", + "/pods/", + "/stats/", + "/stats/summary", + ) + + // These handlers are explicitly disabled. + debuggingDisabledHandlers := sets.NewString( + "/run/", + "/exec/", + "/attach/", + "/portForward/", + "/containerLogs/", + "/runningpods/", + "/debug/pprof/", + "/logs/", + ) + allowedAuthNotRequiredHandlers.Insert(debuggingDisabledHandlers.UnsortedList()...) + + // Test all the non-web-service handlers + for _, path := range fw.serverUnderTest.restfulCont.RegisteredHandlePaths() { + if !allowedAuthNotRequiredHandlers.Has(path) { + t.Errorf("New handler %q must require auth", path) + } + } + + // Test all the generated web-service paths + for _, ws := range fw.serverUnderTest.restfulCont.RegisteredWebServices() { + for _, r := range ws.Routes() { + if !allowedAuthNotRequiredHandlers.Has(r.Path) { + t.Errorf("New handler %q must require auth", r.Path) + } + } + } + + // Ensure the disabled handlers are in fact disabled. + for path := range debuggingDisabledHandlers { + for _, method := range []string{"GET", "POST", "PUT", "PATCH", "DELETE"} { + t.Run(method+":"+path, func(t *testing.T) { + req, err := http.NewRequest(method, fw.testHTTPServer.URL+path, nil) + require.NoError(t, err) + + resp, err := http.DefaultClient.Do(req) + require.NoError(t, err) + defer resp.Body.Close() //nolint:errcheck + + assert.Equal(t, http.StatusMethodNotAllowed, resp.StatusCode) + }) + } + } +} + func TestAuthFilters(t *testing.T) { // Enable features.ContainerCheckpoint during test featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ContainerCheckpoint, true)