diff --git a/pkg/k8s/authz.go b/pkg/k8s/authz.go index 602cffe45f5b6..065954fab0b5f 100644 --- a/pkg/k8s/authz.go +++ b/pkg/k8s/authz.go @@ -48,11 +48,15 @@ func ResourceAuthz( func ResourceAuthzForUser( ctx context.Context, client kubernetes.Interface, - namespace, verb, group, version, resource, subresource, name, user string, userGroups []string) error { + namespace, verb, group, version, resource, subresource, name, user string, + userGroups []string, + extra map[string]authV1.ExtraValue, +) error { sar := &authV1.SubjectAccessReview{ Spec: authV1.SubjectAccessReviewSpec{ User: user, Groups: userGroups, + Extra: extra, ResourceAttributes: &authV1.ResourceAttributes{ Namespace: namespace, Verb: verb, diff --git a/viz/tap/api/handlers.go b/viz/tap/api/handlers.go index 5869eb7bb1b30..e256019653958 100644 --- a/viz/tap/api/handlers.go +++ b/viz/tap/api/handlers.go @@ -5,6 +5,7 @@ import ( "encoding/json" "fmt" "net/http" + "net/url" "strings" "github.com/go-openapi/spec" @@ -17,17 +18,19 @@ import ( "github.com/prometheus/client_golang/prometheus/promhttp" "github.com/sirupsen/logrus" "google.golang.org/grpc/metadata" + authV1 "k8s.io/api/authorization/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/version" ) type handler struct { - k8sAPI *k8s.API - usernameHeader string - groupHeader string - grpcTapServer pb.TapServer - log *logrus.Entry + k8sAPI *k8s.API + usernameHeader string + groupHeader string + extraHeaderPrefix string + grpcTapServer pb.TapServer + log *logrus.Entry } // TODO: share with api_handlers.go @@ -123,6 +126,8 @@ func (h *handler) handleTap(w http.ResponseWriter, req *http.Request, p httprout namespace, resource, name, h.usernameHeader, h.groupHeader, ) + extra := extractExtraHeaders(req.Header, h.extraHeaderPrefix) + // TODO: it's possible this SubjectAccessReview is redundant, consider // removing, more info at https://github.com/linkerd/linkerd2/issues/3182 err := pkgK8s.ResourceAuthzForUser( @@ -137,6 +142,7 @@ func (h *handler) handleTap(w http.ResponseWriter, req *http.Request, p httprout name, req.Header.Get(h.usernameHeader), req.Header.Values(h.groupHeader), + extra, ) if err != nil { err = fmt.Errorf("tap authorization failed (%w), visit %s for more information", err, pkg.TapRbacURL) @@ -180,6 +186,29 @@ func (h *handler) handleTap(w http.ResponseWriter, req *http.Request, p httprout } } +func extractExtraHeaders(header http.Header, extraHeaderPrefix string) map[string]authV1.ExtraValue { + if extraHeaderPrefix == "" { + return nil + } + + extraHeaderPrefixLower := strings.ToLower(extraHeaderPrefix) + extraHeaderPrefixLen := len(extraHeaderPrefix) + extra := make(map[string]authV1.ExtraValue) + + for key, values := range header { + if !strings.HasPrefix(strings.ToLower(key), extraHeaderPrefixLower) { + continue + } + + extraKey, err := url.QueryUnescape(key[extraHeaderPrefixLen:]) + if err == nil { + extra[extraKey] = authV1.ExtraValue(values) + } + } + + return extra +} + // GET (not found) func handleNotFound() http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { diff --git a/viz/tap/api/handlers_test.go b/viz/tap/api/handlers_test.go index 48bc15e64829f..cc2b3b55789f3 100644 --- a/viz/tap/api/handlers_test.go +++ b/viz/tap/api/handlers_test.go @@ -11,6 +11,9 @@ import ( "github.com/julienschmidt/httprouter" "github.com/linkerd/linkerd2/controller/k8s" "github.com/sirupsen/logrus" + authV1 "k8s.io/api/authorization/v1" + k8sFake "k8s.io/client-go/kubernetes/fake" + k8sTesting "k8s.io/client-go/testing" ) func TestHandleTap(t *testing.T) { @@ -71,3 +74,115 @@ func TestHandleTap(t *testing.T) { }) } } + +func TestHandleTap_ExtraHeaders(t *testing.T) { + k8sAPI, err := k8s.NewFakeAPI() + if err != nil { + t.Fatalf("NewFakeAPI returned an error: %s", err) + } + + h := &handler{ + k8sAPI: k8sAPI, + log: logrus.WithField("test", t.Name()), + extraHeaderPrefix: "X-Remote-Extra-", + } + + recorder := httptest.NewRecorder() + req := httptest.NewRequest("POST", "/apis/tap.linkerd.io/v1alpha1/watch/namespaces/foo/tap", nil) + req.Header.Set("X-Remote-Extra-Foo", "bar") + req.Header.Set("X-Remote-Extra-Baz", "qux") + + params := httprouter.Params{ + {Key: "namespace", Value: "foo"}, + } + + h.handleTap(recorder, req, params) + + sar := getSubjectAccessReview(t, k8sAPI) + + if len(sar.Spec.Extra) != 2 { + t.Errorf("Expected 2 extra headers, got %d", len(sar.Spec.Extra)) + } + + if v, ok := sar.Spec.Extra["Foo"]; !ok || v[0] != "bar" { + t.Errorf("Expected Extra['Foo'] to be ['bar'], got %v", v) + } + if v, ok := sar.Spec.Extra["Baz"]; !ok || v[0] != "qux" { + t.Errorf("Expected Extra['Baz'] to be ['qux'], got %v", v) + } +} + +func TestHandleTap_ExtraHeadersPrefixCaseInsensitive(t *testing.T) { + k8sAPI, err := k8s.NewFakeAPI() + if err != nil { + t.Fatalf("NewFakeAPI returned an error: %s", err) + } + + h := &handler{ + k8sAPI: k8sAPI, + log: logrus.WithField("test", t.Name()), + extraHeaderPrefix: "x-remote-extra-", + } + + recorder := httptest.NewRecorder() + req := httptest.NewRequest("POST", "/apis/tap.linkerd.io/v1alpha1/watch/namespaces/foo/tap", nil) + req.Header.Set("X-Remote-Extra-Foo", "bar") + + params := httprouter.Params{ + {Key: "namespace", Value: "foo"}, + } + + h.handleTap(recorder, req, params) + sar := getSubjectAccessReview(t, k8sAPI) + + if v, ok := sar.Spec.Extra["Foo"]; !ok || v[0] != "bar" { + t.Errorf("Expected Extra['Foo'] to be ['bar'], got %v", v) + } +} + +func TestHandleTap_ExtraHeadersEmptyPrefix(t *testing.T) { + k8sAPI, err := k8s.NewFakeAPI() + if err != nil { + t.Fatalf("NewFakeAPI returned an error: %s", err) + } + + h := &handler{ + k8sAPI: k8sAPI, + log: logrus.WithField("test", t.Name()), + } + + recorder := httptest.NewRecorder() + req := httptest.NewRequest("POST", "/apis/tap.linkerd.io/v1alpha1/watch/namespaces/foo/tap", nil) + req.Header.Set("X-Remote-Extra-Foo", "bar") + req.Header.Set("X-Remote-User", "alice") + req.Header.Set("Content-Type", "application/json") + + params := httprouter.Params{ + {Key: "namespace", Value: "foo"}, + } + + h.handleTap(recorder, req, params) + sar := getSubjectAccessReview(t, k8sAPI) + + if len(sar.Spec.Extra) != 0 { + t.Errorf("Expected 0 extra headers, got %d", len(sar.Spec.Extra)) + } +} + +func getSubjectAccessReview(t *testing.T, k8sAPI *k8s.API) *authV1.SubjectAccessReview { + t.Helper() + + client := k8sAPI.Client.(*k8sFake.Clientset) + actions := client.Actions() + + for _, action := range actions { + if action.GetVerb() == "create" && action.GetResource().Resource == "subjectaccessreviews" { + createAction := action.(k8sTesting.CreateAction) + obj := createAction.GetObject() + return obj.(*authV1.SubjectAccessReview) + } + } + + t.Fatal("Expected SubjectAccessReview to be created") + return nil +} diff --git a/viz/tap/api/server.go b/viz/tap/api/server.go index 0a04b42d5f1e0..743dd56701ad2 100644 --- a/viz/tap/api/server.go +++ b/viz/tap/api/server.go @@ -22,6 +22,8 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) +const defaultExtraHeaderPrefix = "X-Remote-Extra-" + // Server holds the underlying http server and its config type Server struct { *http.Server @@ -50,7 +52,7 @@ func NewServer( } }() - clientCAPem, allowedNames, usernameHeader, groupHeader, err := serverAuth(ctx, k8sAPI) + clientCAPem, allowedNames, usernameHeader, groupHeader, extraHeaderPrefix, err := serverAuth(ctx, k8sAPI) if err != nil { return nil, err } @@ -80,11 +82,12 @@ func NewServer( var emptyCert atomic.Value h := &handler{ - k8sAPI: k8sAPI, - usernameHeader: usernameHeader, - groupHeader: groupHeader, - grpcTapServer: grpcTapServer, - log: log, + k8sAPI: k8sAPI, + usernameHeader: usernameHeader, + groupHeader: groupHeader, + extraHeaderPrefix: extraHeaderPrefix, + grpcTapServer: grpcTapServer, + log: log, } lis, err := net.Listen("tcp", addr) @@ -164,28 +167,28 @@ func (a *Server) validate(req *http.Request) error { // authentication. // kubectl -n kube-system get cm/extension-apiserver-authentication // accessible via the extension-apiserver-authentication-reader role -func serverAuth(ctx context.Context, k8sAPI *k8s.API) (string, []string, string, string, error) { +func serverAuth(ctx context.Context, k8sAPI *k8s.API) (string, []string, string, string, string, error) { cm, err := k8sAPI.Client.CoreV1(). ConfigMaps(metav1.NamespaceSystem). Get(ctx, pkgk8s.ExtensionAPIServerAuthenticationConfigMapName, metav1.GetOptions{}) if err != nil { - return "", nil, "", "", fmt.Errorf("failed to load [%s] config: %w", pkgk8s.ExtensionAPIServerAuthenticationConfigMapName, err) + return "", nil, "", "", "", fmt.Errorf("failed to load [%s] config: %w", pkgk8s.ExtensionAPIServerAuthenticationConfigMapName, err) } clientCAPem, ok := cm.Data[pkgk8s.ExtensionAPIServerAuthenticationRequestHeaderClientCAFileKey] if !ok { - return "", nil, "", "", fmt.Errorf("no client CA cert available for apiextension-server") + return "", nil, "", "", "", fmt.Errorf("no client CA cert available for apiextension-server") } allowedNames, err := deserializeStrings(cm.Data["requestheader-allowed-names"]) if err != nil { - return "", nil, "", "", err + return "", nil, "", "", "", err } usernameHeaders, err := deserializeStrings(cm.Data["requestheader-username-headers"]) if err != nil { - return "", nil, "", "", err + return "", nil, "", "", "", err } usernameHeader := "" if len(usernameHeaders) > 0 { @@ -194,14 +197,28 @@ func serverAuth(ctx context.Context, k8sAPI *k8s.API) (string, []string, string, groupHeaders, err := deserializeStrings(cm.Data["requestheader-group-headers"]) if err != nil { - return "", nil, "", "", err + return "", nil, "", "", "", err } groupHeader := "" if len(groupHeaders) > 0 { groupHeader = groupHeaders[0] } - return clientCAPem, allowedNames, usernameHeader, groupHeader, nil + extraHeaderPrefixes, err := deserializeStrings(cm.Data["requestheader-extra-headers-prefix"]) + if err != nil { + return "", nil, "", "", "", err + } + // The extra headers prefix is used to identify headers that contain additional + // user attributes for authorization (e.g., "X-Remote-Extra-"). These headers are + // forwarded by the Kubernetes API server when acting as an aggregating proxy. + // The prefix is configurable via the --requestheader-extra-headers-prefix flag + // on the API server (defaults to "X-Remote-Extra-"). + extraHeaderPrefix := defaultExtraHeaderPrefix + if len(extraHeaderPrefixes) > 0 && extraHeaderPrefixes[0] != "" { + extraHeaderPrefix = extraHeaderPrefixes[0] + } + + return clientCAPem, allowedNames, usernameHeader, groupHeader, extraHeaderPrefix, nil } // copied from https://github.com/kubernetes/apiserver/blob/781c3cd1b3dc5b6f79c68ab0d16fe544600421ef/pkg/server/options/authentication.go#L360 diff --git a/viz/tap/api/server_test.go b/viz/tap/api/server_test.go index 4b2d4825d0228..8b35d808ea23d 100644 --- a/viz/tap/api/server_test.go +++ b/viz/tap/api/server_test.go @@ -18,12 +18,13 @@ import ( func TestAPIServerAuth(t *testing.T) { expectations := []struct { - k8sRes []string - clientCAPem string - allowedNames []string - usernameHeader string - groupHeader string - err error + k8sRes []string + clientCAPem string + allowedNames []string + usernameHeader string + groupHeader string + extraHeaderPrefix string + err error }{ { err: fmt.Errorf("failed to load [%s] config: configmaps %q not found", k8sutils.ExtensionAPIServerAuthenticationConfigMapName, k8sutils.ExtensionAPIServerAuthenticationConfigMapName), @@ -44,11 +45,29 @@ data: requestheader-username-headers: '["X-Remote-User"]' `, }, - clientCAPem: "requestheader-client-ca-file", - allowedNames: []string{"name1", "name2"}, - usernameHeader: "X-Remote-User", - groupHeader: "X-Remote-Group", - err: nil, + clientCAPem: "requestheader-client-ca-file", + allowedNames: []string{"name1", "name2"}, + usernameHeader: "X-Remote-User", + groupHeader: "X-Remote-Group", + extraHeaderPrefix: "X-Remote-Extra-", + err: nil, + }, + { + k8sRes: []string{` +apiVersion: v1 +kind: ConfigMap +metadata: + name: extension-apiserver-authentication + namespace: kube-system +data: + requestheader-client-ca-file: 'requestheader-client-ca-file' +`}, + clientCAPem: "requestheader-client-ca-file", + allowedNames: nil, + usernameHeader: "", + groupHeader: "", + extraHeaderPrefix: defaultExtraHeaderPrefix, + err: nil, }, } @@ -62,7 +81,7 @@ data: t.Fatalf("NewFakeAPI returned an error: %s", err) } - clientCAPem, allowedNames, usernameHeader, groupHeader, err := serverAuth(ctx, k8sAPI) + clientCAPem, allowedNames, usernameHeader, groupHeader, extraHeaderPrefix, err := serverAuth(ctx, k8sAPI) if err != nil && exp.err != nil { if err.Error() != exp.err.Error() { @@ -86,6 +105,9 @@ data: if groupHeader != exp.groupHeader { t.Errorf("apiServerAuth returned unexpected groupHeader: %q, expected: %q", groupHeader, exp.groupHeader) } + if extraHeaderPrefix != exp.extraHeaderPrefix { + t.Errorf("apiServerAuth returned unexpected extraHeaderPrefix: %q, expected: %q", extraHeaderPrefix, exp.extraHeaderPrefix) + } }) } }