Include extra attributes in SubjectAccessReview

Kubernetes authorization plugins can rely on extra attributes on a user, and these are provided via `X-Remote-Extra-` headers. Currently the Linkerd Viz `tap` API doesn't include these attributes when making the `SubjectAccessReview` request which means the Tap API cannot be used by end-users who's clusters use such authz plugins.

This change updates the `tap` controller to parse the `X-Remote-Extra-` headers and include them in the SubjectAccessReview request.
Fixed linkerd#13169

Signed-off-by: David Symons <[email protected]>

Author: multimac

pkg/k8s/authz.go

@@ -48,11 +48,13 @@ 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,

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,17 @@ func (h *handler) handleTap(w http.ResponseWriter, req *http.Request, p httprout
 		namespace, resource, name, h.usernameHeader, h.groupHeader,
 	)
 
+	extra := make(map[string]authV1.ExtraValue)
+	for key, values := range req.Header {
+		if strings.HasPrefix(key, h.extraHeaderPrefix) {
+			key, err := url.QueryUnescape(strings.TrimPrefix(key, h.extraHeaderPrefix))
+
+			if err != nil {
+				extra[key] = values
+			}
+		}
+	}
+
 	// 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 +151,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)

viz/tap/api/server.go

@@ -50,7 +50,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 +80,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 +165,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 +195,23 @@ 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-group-headers"])
+	if err != nil {
+		return "", nil, "", "", "", err
+	}
+	extraHeaderPrefix := ""
+	if len(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

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,12 @@ 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,
 		},
 	}
 
@@ -62,7 +64,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 +88,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)
+			}
 		})
 	}
 }