From 2cc0370169ea1fcf45429f9586e0ffd4ab32ed26 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stanislav=20L=C3=A1zni=C4=8Dka?= Date: Thu, 16 Feb 2023 14:01:53 +0100 Subject: [PATCH] client-go: add the UID to the auth-proxy roundtripper --- .../pkg/util/peerproxy/peerproxy_handler.go | 2 +- .../k8s.io/client-go/transport/round_trippers.go | 13 ++++++++++--- .../client-go/transport/round_trippers_test.go | 15 ++++++++++++++- .../pkg/apiserver/handler_proxy.go | 4 ++-- .../pkg/apiserver/handler_proxy_test.go | 15 ++++++++++++++- .../status/remote/remote_available_controller.go | 2 +- 6 files changed, 42 insertions(+), 9 deletions(-) diff --git a/staging/src/k8s.io/apiserver/pkg/util/peerproxy/peerproxy_handler.go b/staging/src/k8s.io/apiserver/pkg/util/peerproxy/peerproxy_handler.go index bc342165b21..ac5a05f1fae 100644 --- a/staging/src/k8s.io/apiserver/pkg/util/peerproxy/peerproxy_handler.go +++ b/staging/src/k8s.io/apiserver/pkg/util/peerproxy/peerproxy_handler.go @@ -251,7 +251,7 @@ func (h *peerProxyHandler) proxyRequestToDestinationAPIServer(req *http.Request, newReq.Header.Add(PeerProxiedHeader, "true") defer cancelFn() - proxyRoundTripper := transport.NewAuthProxyRoundTripper(user.GetName(), user.GetGroups(), user.GetExtra(), h.proxyTransport) + proxyRoundTripper := transport.NewAuthProxyRoundTripper(user.GetName(), user.GetUID(), user.GetGroups(), user.GetExtra(), h.proxyTransport) delegate := &epmetrics.ResponseWriterDelegator{ResponseWriter: rw} w := responsewriter.WrapForHTTP1Or2(delegate) diff --git a/staging/src/k8s.io/client-go/transport/round_trippers.go b/staging/src/k8s.io/client-go/transport/round_trippers.go index e2d1dcc9a9c..52fefb53163 100644 --- a/staging/src/k8s.io/client-go/transport/round_trippers.go +++ b/staging/src/k8s.io/client-go/transport/round_trippers.go @@ -86,6 +86,7 @@ func DebugWrappers(rt http.RoundTripper) http.RoundTripper { type authProxyRoundTripper struct { username string + uid string groups []string extra map[string][]string @@ -98,15 +99,17 @@ var _ utilnet.RoundTripperWrapper = &authProxyRoundTripper{} // authentication terminating proxy cases // assuming you pull the user from the context: // username is the user.Info.GetName() of the user +// uid is the user.Info.GetUID() of the user // groups is the user.Info.GetGroups() of the user // extra is the user.Info.GetExtra() of the user // extra can contain any additional information that the authenticator // thought was interesting, for example authorization scopes. // In order to faithfully round-trip through an impersonation flow, these keys // MUST be lowercase. -func NewAuthProxyRoundTripper(username string, groups []string, extra map[string][]string, rt http.RoundTripper) http.RoundTripper { +func NewAuthProxyRoundTripper(username, uid string, groups []string, extra map[string][]string, rt http.RoundTripper) http.RoundTripper { return &authProxyRoundTripper{ username: username, + uid: uid, groups: groups, extra: extra, rt: rt, @@ -115,14 +118,15 @@ func NewAuthProxyRoundTripper(username string, groups []string, extra map[string func (rt *authProxyRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) { req = utilnet.CloneRequest(req) - SetAuthProxyHeaders(req, rt.username, rt.groups, rt.extra) + SetAuthProxyHeaders(req, rt.username, rt.uid, rt.groups, rt.extra) return rt.rt.RoundTrip(req) } // SetAuthProxyHeaders stomps the auth proxy header fields. It mutates its argument. -func SetAuthProxyHeaders(req *http.Request, username string, groups []string, extra map[string][]string) { +func SetAuthProxyHeaders(req *http.Request, username, uid string, groups []string, extra map[string][]string) { req.Header.Del("X-Remote-User") + req.Header.Del("X-Remote-Uid") req.Header.Del("X-Remote-Group") for key := range req.Header { if strings.HasPrefix(strings.ToLower(key), strings.ToLower("X-Remote-Extra-")) { @@ -131,6 +135,9 @@ func SetAuthProxyHeaders(req *http.Request, username string, groups []string, ex } req.Header.Set("X-Remote-User", username) + if len(uid) > 0 { + req.Header.Set("X-Remote-Uid", uid) + } for _, group := range groups { req.Header.Add("X-Remote-Group", group) } diff --git a/staging/src/k8s.io/client-go/transport/round_trippers_test.go b/staging/src/k8s.io/client-go/transport/round_trippers_test.go index d0410452f8f..1e20f7094cf 100644 --- a/staging/src/k8s.io/client-go/transport/round_trippers_test.go +++ b/staging/src/k8s.io/client-go/transport/round_trippers_test.go @@ -306,12 +306,14 @@ func TestImpersonationRoundTripper(t *testing.T) { func TestAuthProxyRoundTripper(t *testing.T) { for n, tc := range map[string]struct { username string + uid string groups []string extra map[string][]string expectedExtra map[string][]string }{ "allfields": { username: "user", + uid: "7db46926-e803-4337-9a29-f9c1fab7d34a", groups: []string{"groupA", "groupB"}, extra: map[string][]string{ "one": {"alpha", "bravo"}, @@ -324,6 +326,7 @@ func TestAuthProxyRoundTripper(t *testing.T) { }, "escaped extra": { username: "user", + uid: "7db46926-e803-4337-9a29-f9c1fab7d34a", groups: []string{"groupA", "groupB"}, extra: map[string][]string{ "one": {"alpha", "bravo"}, @@ -336,6 +339,7 @@ func TestAuthProxyRoundTripper(t *testing.T) { }, "double escaped extra": { username: "user", + uid: "7db46926-e803-4337-9a29-f9c1fab7d34a", groups: []string{"groupA", "groupB"}, extra: map[string][]string{ "one": {"alpha", "bravo"}, @@ -349,7 +353,7 @@ func TestAuthProxyRoundTripper(t *testing.T) { } { rt := &testRoundTripper{} req := &http.Request{} - NewAuthProxyRoundTripper(tc.username, tc.groups, tc.extra, rt).RoundTrip(req) + _, _ = NewAuthProxyRoundTripper(tc.username, tc.uid, tc.groups, tc.extra, rt).RoundTrip(req) if rt.Request == nil { t.Errorf("%s: unexpected nil request: %v", n, rt) continue @@ -368,6 +372,15 @@ func TestAuthProxyRoundTripper(t *testing.T) { t.Errorf("%s expected %v, got %v", n, e, a) continue } + actualUID, ok := rt.Request.Header["X-Remote-Uid"] + if !ok { + t.Errorf("%s missing value", n) + continue + } + if e, a := []string{tc.uid}, actualUID; !reflect.DeepEqual(e, a) { + t.Errorf("%s expected %v, got %v", n, e, a) + continue + } actualGroups, ok := rt.Request.Header["X-Remote-Group"] if !ok { t.Errorf("%s missing value", n) diff --git a/staging/src/k8s.io/kube-aggregator/pkg/apiserver/handler_proxy.go b/staging/src/k8s.io/kube-aggregator/pkg/apiserver/handler_proxy.go index a59974f3005..5292ec86489 100644 --- a/staging/src/k8s.io/kube-aggregator/pkg/apiserver/handler_proxy.go +++ b/staging/src/k8s.io/kube-aggregator/pkg/apiserver/handler_proxy.go @@ -159,7 +159,7 @@ func (r *proxyHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) { proxyRoundTripper := handlingInfo.proxyRoundTripper upgrade := httpstream.IsUpgradeRequest(req) - proxyRoundTripper = transport.NewAuthProxyRoundTripper(user.GetName(), user.GetGroups(), user.GetExtra(), proxyRoundTripper) + proxyRoundTripper = transport.NewAuthProxyRoundTripper(user.GetName(), user.GetUID(), user.GetGroups(), user.GetExtra(), proxyRoundTripper) if utilfeature.DefaultFeatureGate.Enabled(genericfeatures.APIServerTracing) && !upgrade { tracingWrapper := tracing.WrapperFor(r.tracerProvider) @@ -170,7 +170,7 @@ func (r *proxyHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) { // NOT use the proxyRoundTripper. It's a direct dial that bypasses the proxyRoundTripper. This means that we have to // attach the "correct" user headers to the request ahead of time. if upgrade { - transport.SetAuthProxyHeaders(newReq, user.GetName(), user.GetGroups(), user.GetExtra()) + transport.SetAuthProxyHeaders(newReq, user.GetName(), user.GetUID(), user.GetGroups(), user.GetExtra()) } handler := proxy.NewUpgradeAwareHandler(location, proxyRoundTripper, true, upgrade, &responder{w: w}) diff --git a/staging/src/k8s.io/kube-aggregator/pkg/apiserver/handler_proxy_test.go b/staging/src/k8s.io/kube-aggregator/pkg/apiserver/handler_proxy_test.go index ab2ccea11f2..632727018a5 100644 --- a/staging/src/k8s.io/kube-aggregator/pkg/apiserver/handler_proxy_test.go +++ b/staging/src/k8s.io/kube-aggregator/pkg/apiserver/handler_proxy_test.go @@ -154,6 +154,7 @@ func TestProxyHandler(t *testing.T) { "proxy with user, insecure": { user: &user.DefaultInfo{ Name: "username", + UID: "6b60d791-1af9-4513-92e5-e4252a1e0a78", Groups: []string{"one", "two"}, }, path: "/request/path", @@ -178,6 +179,7 @@ func TestProxyHandler(t *testing.T) { "X-Forwarded-Uri": {"/request/path"}, "X-Forwarded-For": {"127.0.0.1"}, "X-Remote-User": {"username"}, + "X-Remote-Uid": {"6b60d791-1af9-4513-92e5-e4252a1e0a78"}, "User-Agent": {"Go-http-client/1.1"}, "Accept-Encoding": {"gzip"}, "X-Remote-Group": {"one", "two"}, @@ -186,6 +188,7 @@ func TestProxyHandler(t *testing.T) { "proxy with user, cabundle": { user: &user.DefaultInfo{ Name: "username", + UID: "6b60d791-1af9-4513-92e5-e4252a1e0a78", Groups: []string{"one", "two"}, }, path: "/request/path", @@ -210,6 +213,7 @@ func TestProxyHandler(t *testing.T) { "X-Forwarded-Uri": {"/request/path"}, "X-Forwarded-For": {"127.0.0.1"}, "X-Remote-User": {"username"}, + "X-Remote-Uid": {"6b60d791-1af9-4513-92e5-e4252a1e0a78"}, "User-Agent": {"Go-http-client/1.1"}, "Accept-Encoding": {"gzip"}, "X-Remote-Group": {"one", "two"}, @@ -218,6 +222,7 @@ func TestProxyHandler(t *testing.T) { "service unavailable": { user: &user.DefaultInfo{ Name: "username", + UID: "6b60d791-1af9-4513-92e5-e4252a1e0a78", Groups: []string{"one", "two"}, }, path: "/request/path", @@ -240,6 +245,7 @@ func TestProxyHandler(t *testing.T) { "service unresolveable": { user: &user.DefaultInfo{ Name: "username", + UID: "6b60d791-1af9-4513-92e5-e4252a1e0a78", Groups: []string{"one", "two"}, }, path: "/request/path", @@ -263,6 +269,7 @@ func TestProxyHandler(t *testing.T) { "fail on bad serving cert": { user: &user.DefaultInfo{ Name: "username", + UID: "6b60d791-1af9-4513-92e5-e4252a1e0a78", Groups: []string{"one", "two"}, }, path: "/request/path", @@ -284,6 +291,7 @@ func TestProxyHandler(t *testing.T) { "fail on bad serving cert w/o SAN and increase SAN error counter metrics": { user: &user.DefaultInfo{ Name: "username", + UID: "6b60d791-1af9-4513-92e5-e4252a1e0a78", Groups: []string{"one", "two"}, }, path: "/request/path", @@ -425,6 +433,7 @@ func newBrokenDialerAndSelector() (*mockEgressDialer, *egressselector.EgressSele func TestProxyUpgrade(t *testing.T) { upgradeUser := "upgradeUser" + upgradeUID := "upgradeUser-UID" testcases := map[string]struct { APIService *apiregistration.APIService NewEgressSelector func() (*mockEgressDialer, *egressselector.EgressSelector) @@ -534,6 +543,10 @@ func TestProxyUpgrade(t *testing.T) { if user != upgradeUser { t.Errorf("expected user %q, got %q", upgradeUser, user) } + uid := req.Header.Get("X-Remote-Uid") + if uid != upgradeUID { + t.Errorf("expected UID %q, got %q", upgradeUID, uid) + } body := make([]byte, 5) ws.Read(body) ws.Write([]byte("hello " + string(body))) @@ -576,7 +589,7 @@ func TestProxyUpgrade(t *testing.T) { } proxyHandler.updateAPIService(tc.APIService) - aggregator := httptest.NewServer(contextHandler(proxyHandler, &user.DefaultInfo{Name: upgradeUser})) + aggregator := httptest.NewServer(contextHandler(proxyHandler, &user.DefaultInfo{Name: upgradeUser, UID: upgradeUID})) defer aggregator.Close() ws, err := websocket.Dial("ws://"+aggregator.Listener.Addr().String()+path, "", "http://127.0.0.1/") diff --git a/staging/src/k8s.io/kube-aggregator/pkg/controllers/status/remote/remote_available_controller.go b/staging/src/k8s.io/kube-aggregator/pkg/controllers/status/remote/remote_available_controller.go index ade744708cd..a94e254cd8f 100644 --- a/staging/src/k8s.io/kube-aggregator/pkg/controllers/status/remote/remote_available_controller.go +++ b/staging/src/k8s.io/kube-aggregator/pkg/controllers/status/remote/remote_available_controller.go @@ -305,7 +305,7 @@ func (c *AvailableConditionController) sync(key string) error { } // setting the system-masters identity ensures that we will always have access rights - transport.SetAuthProxyHeaders(newReq, "system:kube-aggregator", []string{"system:masters"}, nil) + transport.SetAuthProxyHeaders(newReq, "system:kube-aggregator", "", []string{"system:masters"}, nil) resp, err := discoveryClient.Do(newReq) if resp != nil { resp.Body.Close()