From 26902de531620d2df5ce1bb572d2ea6965a7b7e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stanislav=20L=C3=A1zni=C4=8Dka?= Date: Mon, 20 May 2024 13:43:22 +0200 Subject: [PATCH] delegate authn: don't default the ReqHeaders UID header --- cmd/kube-apiserver/app/testing/testserver.go | 18 ++++++++++++++++ .../app/options/options_test.go | 2 +- .../pkg/server/options/authentication.go | 9 ++++++-- .../cloud-provider/options/options_test.go | 4 ++-- test/integration/examples/apiserver_test.go | 21 ++++++++++++------- 5 files changed, 42 insertions(+), 12 deletions(-) diff --git a/cmd/kube-apiserver/app/testing/testserver.go b/cmd/kube-apiserver/app/testing/testserver.go index 3c3b9e55ecd..9b97da28169 100644 --- a/cmd/kube-apiserver/app/testing/testserver.go +++ b/cmd/kube-apiserver/app/testing/testserver.go @@ -214,6 +214,7 @@ func StartTestServer(t ktesting.TB, instanceOptions *TestServerInstanceOptions, } s.SecureServing.ServerCert.CertDirectory = result.TmpDir + reqHeaderFromFlags := s.Authentication.RequestHeader if instanceOptions.EnableCertAuth { // set up default headers for request header auth reqHeaders := serveroptions.NewDelegatingAuthenticationOptions() @@ -347,6 +348,23 @@ func StartTestServer(t ktesting.TB, instanceOptions *TestServerInstanceOptions, return result, err } + // the RequestHeader options pointer gets replaced in the case of EnableCertAuth override + // and so flags are connected to a struct that no longer appears in the ServerOptions struct + // we're using. + // We still want to make it possible to configure the headers config for the RequestHeader authenticator. + if usernameHeaders := reqHeaderFromFlags.UsernameHeaders; len(usernameHeaders) > 0 { + s.Authentication.RequestHeader.UsernameHeaders = usernameHeaders + } + if uidHeaders := reqHeaderFromFlags.UIDHeaders; len(uidHeaders) > 0 { + s.Authentication.RequestHeader.UIDHeaders = uidHeaders + } + if groupHeaders := reqHeaderFromFlags.GroupHeaders; len(groupHeaders) > 0 { + s.Authentication.RequestHeader.GroupHeaders = groupHeaders + } + if extraHeaders := reqHeaderFromFlags.ExtraHeaderPrefixes; len(extraHeaders) > 0 { + s.Authentication.RequestHeader.ExtraHeaderPrefixes = extraHeaders + } + if err := utilversion.DefaultComponentGlobalsRegistry.Set(); err != nil { return result, err } diff --git a/cmd/kube-controller-manager/app/options/options_test.go b/cmd/kube-controller-manager/app/options/options_test.go index 4dbcda55eac..874a4292fe9 100644 --- a/cmd/kube-controller-manager/app/options/options_test.go +++ b/cmd/kube-controller-manager/app/options/options_test.go @@ -430,7 +430,7 @@ func TestAddFlags(t *testing.T) { ClientCert: apiserveroptions.ClientCertAuthenticationOptions{}, RequestHeader: apiserveroptions.RequestHeaderAuthenticationOptions{ UsernameHeaders: []string{"x-remote-user"}, - UIDHeaders: []string{"x-remote-uid"}, + UIDHeaders: nil, GroupHeaders: []string{"x-remote-group"}, ExtraHeaderPrefixes: []string{"x-remote-extra-"}, }, diff --git a/staging/src/k8s.io/apiserver/pkg/server/options/authentication.go b/staging/src/k8s.io/apiserver/pkg/server/options/authentication.go index b994bd87733..6b53908355c 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/options/authentication.go +++ b/staging/src/k8s.io/apiserver/pkg/server/options/authentication.go @@ -245,8 +245,13 @@ func NewDelegatingAuthenticationOptions() *DelegatingAuthenticationOptions { CacheTTL: 10 * time.Second, ClientCert: ClientCertAuthenticationOptions{}, RequestHeader: RequestHeaderAuthenticationOptions{ - UsernameHeaders: []string{"x-remote-user"}, - UIDHeaders: []string{"x-remote-uid"}, + UsernameHeaders: []string{"x-remote-user"}, + // we specifically don't default UID headers as these were introduced + // later (kube 1.32) and we don't want 3rd parties to be trusting the default headers + // before we can safely say that all KAS instances know they should + // remove them from an incoming request in its WithAuthentication handler. + // The defaulting will be enabled in a future (1.33+) version. + UIDHeaders: nil, GroupHeaders: []string{"x-remote-group"}, ExtraHeaderPrefixes: []string{"x-remote-extra-"}, }, diff --git a/staging/src/k8s.io/cloud-provider/options/options_test.go b/staging/src/k8s.io/cloud-provider/options/options_test.go index b7f536e43cf..94bb30b4320 100644 --- a/staging/src/k8s.io/cloud-provider/options/options_test.go +++ b/staging/src/k8s.io/cloud-provider/options/options_test.go @@ -133,7 +133,7 @@ func TestDefaultFlags(t *testing.T) { ClientCert: apiserveroptions.ClientCertAuthenticationOptions{}, RequestHeader: apiserveroptions.RequestHeaderAuthenticationOptions{ UsernameHeaders: []string{"x-remote-user"}, - UIDHeaders: []string{"x-remote-uid"}, + UIDHeaders: nil, GroupHeaders: []string{"x-remote-group"}, ExtraHeaderPrefixes: []string{"x-remote-extra-"}, }, @@ -294,7 +294,7 @@ func TestAddFlags(t *testing.T) { ClientCert: apiserveroptions.ClientCertAuthenticationOptions{}, RequestHeader: apiserveroptions.RequestHeaderAuthenticationOptions{ UsernameHeaders: []string{"x-remote-user"}, - UIDHeaders: []string{"x-remote-uid"}, + UIDHeaders: nil, GroupHeaders: []string{"x-remote-group"}, ExtraHeaderPrefixes: []string{"x-remote-extra-"}, }, diff --git a/test/integration/examples/apiserver_test.go b/test/integration/examples/apiserver_test.go index 840f37edf09..a2e652221a7 100644 --- a/test/integration/examples/apiserver_test.go +++ b/test/integration/examples/apiserver_test.go @@ -268,7 +268,7 @@ func TestFrontProxyConfig(t *testing.T) { kubeBinaryVersion := sampleserver.WardleVersionToKubeVersion(version.MustParse(wardleBinaryVersion)).String() // start up the KAS and prepare the options for the wardle API server - testKAS, wardleOptions, wardlePort := prepareAggregatedWardleAPIServer(ctx, t, testNamespace, kubeBinaryVersion, wardleBinaryVersion) + testKAS, wardleOptions, wardlePort := prepareAggregatedWardleAPIServer(ctx, t, testNamespace, kubeBinaryVersion, wardleBinaryVersion, []string{"--requestheader-uid-headers=x-remote-uid"}) kubeConfig := getKubeConfig(testKAS) // create the SA that we will use to query the aggregated API @@ -281,8 +281,6 @@ func TestFrontProxyConfig(t *testing.T) { if err != nil { t.Fatal(err) } - expectedSAUserInfo := serviceaccount.UserInfo(expectedSA.Namespace, expectedSA.Name, string(expectedSA.UID)) - expectedRealSAGroups := append(expectedSAUserInfo.GetGroups(), user.AllAuthenticated) saTokenReq, err := kubeClient.CoreV1().ServiceAccounts(testNamespace).CreateToken(ctx, "wardle-client-sa", &v1.TokenRequest{}, metav1.CreateOptions{}) if err != nil { @@ -301,6 +299,9 @@ func TestFrontProxyConfig(t *testing.T) { if err != nil { t.Fatalf("failed to retrieve details about the SA: %v", err) } + + expectedSAUserInfo := serviceaccount.UserInfo(expectedSA.Namespace, expectedSA.Name, string(expectedSA.UID)) + expectedRealSAGroups := append(expectedSAUserInfo.GetGroups(), user.AllAuthenticated) expectedExtra := expectedSAUserInfo.GetExtra() if expectedExtra == nil { expectedExtra = map[string][]string{} @@ -315,7 +316,7 @@ func TestFrontProxyConfig(t *testing.T) { transport.WrapperFunc(func(rt http.RoundTripper) http.RoundTripper { return roundTripperFunc(func(req *http.Request) (*http.Response, error) { gotUser, ok := genericapirequest.UserFrom(req.Context()) - if !ok { + if !ok || gotUser.GetName() == "system:anonymous" { return nil, fmt.Errorf("got an unauthenticated request") } @@ -382,7 +383,7 @@ func testAggregatedAPIServer(t *testing.T, setWardleFeatureGate, banFlunder bool // each wardle binary is bundled with a specific kube binary. kubeBinaryVersion := sampleserver.WardleVersionToKubeVersion(version.MustParse(wardleBinaryVersion)).String() - testKAS, wardleOptions, wardlePort := prepareAggregatedWardleAPIServer(ctx, t, testNamespace, kubeBinaryVersion, wardleBinaryVersion) + testKAS, wardleOptions, wardlePort := prepareAggregatedWardleAPIServer(ctx, t, testNamespace, kubeBinaryVersion, wardleBinaryVersion, nil) kubeClientConfig := getKubeConfig(testKAS) wardleCertDir, _ := os.MkdirTemp("", "test-integration-wardle-server") @@ -662,7 +663,7 @@ func TestAggregatedAPIServerRejectRedirectResponse(t *testing.T) { } } -func prepareAggregatedWardleAPIServer(ctx context.Context, t *testing.T, namespace, kubebinaryVersion, wardleBinaryVersion string) (*kastesting.TestServer, *sampleserver.WardleServerOptions, int) { +func prepareAggregatedWardleAPIServer(ctx context.Context, t *testing.T, namespace, kubebinaryVersion, wardleBinaryVersion string, kubeAPIServerFlags []string) (*kastesting.TestServer, *sampleserver.WardleServerOptions, int) { // makes the kube-apiserver very responsive. it's normally a minute dynamiccertificates.FileRefreshDuration = 1 * time.Second @@ -674,7 +675,13 @@ func prepareAggregatedWardleAPIServer(ctx context.Context, t *testing.T, namespa // endpoints cannot have loopback IPs so we need to override the resolver itself t.Cleanup(app.SetServiceResolverForTests(staticURLServiceResolver(fmt.Sprintf("https://127.0.0.1:%d", wardlePort)))) - testServer := kastesting.StartTestServerOrDie(t, &kastesting.TestServerInstanceOptions{EnableCertAuth: true, BinaryVersion: kubebinaryVersion}, nil, framework.SharedEtcd()) + testServer := kastesting.StartTestServerOrDie(t, + &kastesting.TestServerInstanceOptions{ + EnableCertAuth: true, + BinaryVersion: kubebinaryVersion, + }, + kubeAPIServerFlags, + framework.SharedEtcd()) t.Cleanup(func() { testServer.TearDownFn() }) _, _ = utilversion.DefaultComponentGlobalsRegistry.ComponentGlobalsOrRegister(