From d10efcae34d4568105ee257b34d1bd654eb67f57 Mon Sep 17 00:00:00 2001 From: Jakob Beckmann Date: Sun, 5 Feb 2023 14:50:35 +0100 Subject: [PATCH] impr: finish implementation for selector definition in roles Implements: #155 --- Makefile | 2 + backend.go | 9 +- go.mod | 1 + go.sum | 1 + integrationtest/integration_test.go | 25 +++++ .../vault/namespaceControllerBinding.yaml | 26 +++++ namespace_validator.go | 97 ++++++++++++++++++- path_login.go | 40 ++++++-- path_login_test.go | 87 +++++++++++++---- path_role.go | 9 +- path_role_test.go | 12 +++ 11 files changed, 273 insertions(+), 36 deletions(-) create mode 100644 integrationtest/vault/namespaceControllerBinding.yaml diff --git a/Makefile b/Makefile index c1b30b75..ec9c531d 100644 --- a/Makefile +++ b/Makefile @@ -51,6 +51,7 @@ vault-image: setup-integration-test: teardown-integration-test vault-image kind --name $(KIND_CLUSTER_NAME) load docker-image hashicorp/vault:dev kubectl --context="kind-$(KIND_CLUSTER_NAME)" create namespace test + kubectl --context="kind-$(KIND_CLUSTER_NAME)" label namespaces test target=integration-test other=label helm upgrade --install vault vault --repo https://helm.releases.hashicorp.com --version=0.25.0 \ --kube-context="kind-$(KIND_CLUSTER_NAME)" \ --wait --timeout=5m \ @@ -63,6 +64,7 @@ setup-integration-test: teardown-integration-test vault-image --set server.extraArgs="-dev-plugin-dir=/vault/plugin_directory" kubectl --context="kind-$(KIND_CLUSTER_NAME)" apply --namespace=test -f integrationtest/vault/tokenReviewerServiceAccount.yaml kubectl --context="kind-$(KIND_CLUSTER_NAME)" apply -f integrationtest/vault/tokenReviewerBinding.yaml + kubectl --context="kind-$(KIND_CLUSTER_NAME)" apply -f integrationtest/vault/namespaceControllerBinding.yaml kubectl --context="kind-$(KIND_CLUSTER_NAME)" wait --namespace=test --for=condition=Ready --timeout=5m pod -l app.kubernetes.io/name=vault .PHONY: teardown-integration-test diff --git a/backend.go b/backend.go index 1e94c45e..acd0c484 100644 --- a/backend.go +++ b/backend.go @@ -80,6 +80,12 @@ type kubeAuthBackend struct { // review. Mocks should only be used in tests. reviewFactory tokenReviewFactory + // nsValidatorFactory is used to configure the strategy for validating + // namespace properties (currently labels). Currently, the only options + // are using the kubernetes API or mocking the validation. Mocks should + // only be used in tests. + nsValidatorFactory namespaceValidatorFactory + // localSATokenReader caches the service account token in memory. // It periodically reloads the token to support token rotation/renewal. // Local token is used when running in a pod with following configuration @@ -133,7 +139,8 @@ func Backend() *kubeAuthBackend { // Set the default TLSConfig tlsConfig: getDefaultTLSConfig(), // Set the review factory to default to calling into the kubernetes API. - reviewFactory: tokenReviewAPIFactory, + reviewFactory: tokenReviewAPIFactory, + nsValidatorFactory: namespaceValidatorAPIFactory, } b.Backend = &framework.Backend{ diff --git a/go.mod b/go.mod index ff3d9d47..34320bea 100644 --- a/go.mod +++ b/go.mod @@ -75,4 +75,5 @@ require ( k8s.io/utils v0.0.0-20230406110748-d93618cff8a2 // indirect sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect sigs.k8s.io/structured-merge-diff/v4 v4.2.3 // indirect + sigs.k8s.io/yaml v1.3.0 // indirect ) diff --git a/go.sum b/go.sum index 73bd44e2..6d0e88af 100644 --- a/go.sum +++ b/go.sum @@ -332,3 +332,4 @@ sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd/go.mod h1:B8JuhiUyNFVKdsE8h6 sigs.k8s.io/structured-merge-diff/v4 v4.2.3 h1:PRbqxJClWWYMNV1dhaG4NsibJbArud9kFxnAMREiWFE= sigs.k8s.io/structured-merge-diff/v4 v4.2.3/go.mod h1:qjx8mGObPmV2aSZepjQjbmb2ihdVs8cGKBraizNC69E= sigs.k8s.io/yaml v1.3.0 h1:a2VclLzOGrwOHDiV8EfBGhvjHvP46CtW5j6POvhYGGo= +sigs.k8s.io/yaml v1.3.0/go.mod h1:GeOyir5tyXNByN85N/dRIT9es5UQNerPYEKK56eTBm8= diff --git a/integrationtest/integration_test.go b/integrationtest/integration_test.go index d02185ee..d08c644d 100644 --- a/integrationtest/integration_test.go +++ b/integrationtest/integration_test.go @@ -16,6 +16,14 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) +const ( + matchLabelsKeyValue = `{ + "matchLabels": { + "target": "integration-test" + } +}` +) + // Set the environment variable INTEGRATION_TESTS to any non-empty value to run // the tests in this package. The test assumes it has available: // - A Kubernetes cluster in which: @@ -149,6 +157,23 @@ func TestSuccessWithTokenReviewerJwt(t *testing.T) { } } +func TestSuccessWithNamespaceLabels(t *testing.T) { + roleConfigOverride := map[string]interface{}{ + "bound_service_account_names": "vault", + "bound_service_account_namespace_selector": matchLabelsKeyValue, + } + client, cleanup := setupKubernetesAuth(t, "vault", nil, roleConfigOverride) + defer cleanup() + + _, err := client.Logical().Write("auth/kubernetes/login", map[string]interface{}{ + "role": "test-role", + "jwt": os.Getenv("KUBERNETES_JWT"), + }) + if err != nil { + t.Fatalf("Expected successful login but got: %v", err) + } +} + func TestFailWithBadTokenReviewerJwt(t *testing.T) { client, cleanup := setupKubernetesAuth(t, "vault", map[string]interface{}{ "kubernetes_host": "https://kubernetes.default.svc.cluster.local", diff --git a/integrationtest/vault/namespaceControllerBinding.yaml b/integrationtest/vault/namespaceControllerBinding.yaml new file mode 100644 index 00000000..eafcac49 --- /dev/null +++ b/integrationtest/vault/namespaceControllerBinding.yaml @@ -0,0 +1,26 @@ +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRoleBinding +metadata: + name: test-namespacelister-account-binding +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: ClusterRole + name: system:controller:namespace-controller +subjects: +- kind: ServiceAccount + name: test-token-reviewer-account + namespace: test +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRoleBinding +metadata: + name: test-namespacelister-account-binding-vault +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: ClusterRole + name: system:controller:namespace-controller +subjects: +- kind: ServiceAccount + name: vault + namespace: test + diff --git a/namespace_validator.go b/namespace_validator.go index d35bec1f..032434ea 100644 --- a/namespace_validator.go +++ b/namespace_validator.go @@ -2,12 +2,22 @@ package kubeauth import ( "context" + "encoding/json" + "errors" + "fmt" + "io/ioutil" "net/http" + "strings" + + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" + k8s_yaml "k8s.io/apimachinery/pkg/util/yaml" ) // This exists so we can use a mock namespace validation when running tests type namespaceValidator interface { - ValidateLabels(context.Context, *http.Client, string, map[string]string) (bool, error) + ValidateLabels(context.Context, *http.Client, string, metav1.LabelSelector) (bool, error) } type namespaceValidatorFactory func(*kubeConfig) namespaceValidator @@ -23,6 +33,87 @@ func namespaceValidatorAPIFactory(config *kubeConfig) namespaceValidator { } } -func (t *namespaceValidatorAPI) ValidateLabels(ctx context.Context, client *http.Client, name string, labels map[string]string) (bool, error) { - return true, nil +func (v *namespaceValidatorAPI) ValidateLabels(ctx context.Context, client *http.Client, namespace string, selector metav1.LabelSelector) (bool, error) { + nsLabels, err := v.getNamespaceLabels(ctx, client, namespace) + if err != nil { + return false, err + } + + labelSelector, err := metav1.LabelSelectorAsSelector(&selector) + if err != nil { + return false, err + } + return labelSelector.Matches(labels.Set(nsLabels)), nil +} + +func makeLabelSelector(selector string) (metav1.LabelSelector, error) { + labelSelector := metav1.LabelSelector{} + decoder := k8s_yaml.NewYAMLOrJSONDecoder(strings.NewReader(selector), len(selector)) + err := decoder.Decode(&labelSelector) + if err != nil { + return labelSelector, err + } + return labelSelector, nil +} + +func (v *namespaceValidatorAPI) getNamespaceLabels(ctx context.Context, client *http.Client, namespace string) (map[string]string, error) { + url := fmt.Sprintf("%s/api/v1/namespaces/%s", strings.TrimSuffix(v.config.Host, "/"), namespace) + req, err := http.NewRequestWithContext(ctx, "GET", url, nil) + if err != nil { + return nil, err + } + + // If we have a configured TokenReviewer JWT use it as the bearer, otherwise + // try to use the passed in JWT. + if v.config.TokenReviewerJWT == "" { + return nil, errors.New("namespace lookup failed: TokenReviewer JWT needs to be configured to use namespace selectors") + } + bearer := fmt.Sprintf("Bearer %s", v.config.TokenReviewerJWT) + bearer = strings.TrimSpace(bearer) + + // Set the JWT as the Bearer token + req.Header.Set("Authorization", bearer) + + // Set the MIME type headers + req.Header.Set("Content-Type", "application/json") + req.Header.Set("Accept", "application/json") + + resp, err := client.Do(req) + if err != nil { + return nil, err + } + body, err := ioutil.ReadAll(resp.Body) + if err != nil { + return nil, err + } + if resp.StatusCode != 200 { + return nil, fmt.Errorf("failed to get namespace (code %d): %s", resp.StatusCode, body) + } + ns := v1.Namespace{} + + err = json.Unmarshal(body, &ns) + if err != nil { + return nil, err + } + return ns.Labels, nil +} + +type mockNamespaceValidator struct { + labels map[string]string +} + +func mockNamespaceValidatorFactory(labels map[string]string) namespaceValidatorFactory { + return func(config *kubeConfig) namespaceValidator { + return &mockNamespaceValidator{ + labels: labels, + } + } +} + +func (v *mockNamespaceValidator) ValidateLabels(ctx context.Context, client *http.Client, namespace string, selector metav1.LabelSelector) (bool, error) { + labelSelector, err := metav1.LabelSelectorAsSelector(&selector) + if err != nil { + return false, err + } + return labelSelector.Matches(labels.Set(v.labels)), nil } diff --git a/path_login.go b/path_login.go index 69f4e6ee..b258043d 100644 --- a/path_login.go +++ b/path_login.go @@ -124,7 +124,13 @@ func (b *kubeAuthBackend) pathLogin(ctx context.Context, req *logical.Request, d return nil, errors.New("could not load backend configuration") } - serviceAccount, err := b.parseAndValidateJWT(jwtStr, role, config) + client, err := b.getHTTPClient() + if err != nil { + b.Logger().Error(`Failed to get the HTTP client`, "err", err) + return nil, logical.ErrUnrecoverable + } + + serviceAccount, err := b.parseAndValidateJWT(ctx, client, jwtStr, role, config) if err != nil { if err == jose.ErrCryptoFailure || strings.Contains(err.Error(), "verifying token signature") { b.Logger().Debug(`login unauthorized`, "err", err) @@ -138,12 +144,6 @@ func (b *kubeAuthBackend) pathLogin(ctx context.Context, req *logical.Request, d return nil, err } - client, err := b.getHTTPClient() - if err != nil { - b.Logger().Error(`Failed to get the HTTP client`, "err", err) - return nil, logical.ErrUnrecoverable - } - // look up the JWT token in the kubernetes API err = serviceAccount.lookup(ctx, client, jwtStr, role.Audience, b.reviewFactory(config)) @@ -247,7 +247,13 @@ func (b *kubeAuthBackend) aliasLookahead(ctx context.Context, req *logical.Reque } // validation of the JWT against the provided role ensures alias look ahead requests // are authentic. - sa, err := b.parseAndValidateJWT(jwtStr, role, config) + client, err := b.getHTTPClient() + if err != nil { + b.Logger().Error(`Failed to get the HTTP client`, "err", err) + return nil, logical.ErrUnrecoverable + } + + sa, err := b.parseAndValidateJWT(ctx, client, jwtStr, role, config) if err != nil { return nil, err } @@ -282,7 +288,7 @@ func (keySet DontVerifySignature) VerifySignature(_ context.Context, token strin } // parseAndValidateJWT is used to parse, validate and lookup the JWT token. -func (b *kubeAuthBackend) parseAndValidateJWT(jwtStr string, role *roleStorageEntry, config *kubeConfig) (*serviceAccount, error) { +func (b *kubeAuthBackend) parseAndValidateJWT(ctx context.Context, client *http.Client, jwtStr string, role *roleStorageEntry, config *kubeConfig) (*serviceAccount, error) { expected := capjwt.Expected{ SigningAlgorithms: supportedJwtAlgs, } @@ -333,7 +339,21 @@ func (b *kubeAuthBackend) parseAndValidateJWT(jwtStr string, role *roleStorageEn } // verify the namespace is allowed - if len(role.ServiceAccountNamespaces) > 1 || role.ServiceAccountNamespaces[0] != "*" { + valid := false + if role.ServiceAccountNamespaceSelector != "" { + labelSelector, err := makeLabelSelector(role.ServiceAccountNamespaceSelector) + if err != nil { + return nil, err + } + valid, err = b.nsValidatorFactory(config).ValidateLabels(ctx, client, sa.namespace(), labelSelector) + if err != nil { + return nil, err + } + if !valid && len(role.ServiceAccountNamespaces) == 0 { + return nil, logical.CodedError(http.StatusForbidden, "namespace not authorized") + } + } + if !valid && (len(role.ServiceAccountNamespaces) > 1 || role.ServiceAccountNamespaces[0] != "*") { if !strutil.StrListContainsGlob(role.ServiceAccountNamespaces, sa.namespace()) { return nil, logical.CodedError(http.StatusForbidden, "namespace not authorized") } diff --git a/path_login_test.go b/path_login_test.go index e20240f2..15566b08 100644 --- a/path_login_test.go +++ b/path_login_test.go @@ -30,20 +30,29 @@ import ( "k8s.io/apimachinery/pkg/types" ) +const ( + matchLabelsKeyValue = `{ + "matchLabels": { + "key": "value" + } +}` +) + var ( - testNamespace = "default" - testName = "vault-auth" - testUID = "d77f89bc-9055-11e7-a068-0800276d99bf" - testMockFactory = mockTokenReviewFactory(testName, testNamespace, testUID) + testNamespace = "default" + testName = "vault-auth" + testUID = "d77f89bc-9055-11e7-a068-0800276d99bf" + testMockTokenReviewFactory = mockTokenReviewFactory(testName, testNamespace, testUID) + testMockNamespaceValidatorFactory = mockNamespaceValidatorFactory(map[string]string{"key": "value", "other": "label"}) testGlobbedNamespace = "def*" testGlobbedName = "vault-*" // Projected ServiceAccount tokens have name "default", and require a // different mock token reviewer - testProjectedName = "default" - testProjectedUID = "77c81ad7-1bea-4d94-9ca5-f5d7f3632331" - testProjectedMockFactory = mockTokenReviewFactory(testProjectedName, testNamespace, testProjectedUID) + testProjectedName = "default" + testProjectedUID = "77c81ad7-1bea-4d94-9ca5-f5d7f3632331" + testProjectedMockTokenReviewFactory = mockTokenReviewFactory(testProjectedName, testNamespace, testProjectedUID) testDefaultPEMs []string ecdsaPrivateKey *ecdsa.PrivateKey @@ -210,18 +219,20 @@ func jwtSign(header string, payload string, privateKey *ecdsa.PrivateKey) string } type testBackendConfig struct { - pems []string - saName string - saNamespace string - aliasNameSource string + pems []string + saName string + saNamespace string + saNamespaceSelector string + aliasNameSource string } func defaultTestBackendConfig() *testBackendConfig { return &testBackendConfig{ - pems: testDefaultPEMs, - saName: testName, - saNamespace: testNamespace, - aliasNameSource: aliasNameSourceDefault, + pems: testDefaultPEMs, + saName: testName, + saNamespace: testNamespace, + saNamespaceSelector: "", + aliasNameSource: aliasNameSourceDefault, } } @@ -258,6 +269,10 @@ func setupBackend(t *testing.T, config *testBackendConfig) (logical.Backend, log "alias_name_source": config.aliasNameSource, } + if config.saNamespaceSelector != "" { + data["bound_service_account_namespace_selector"] = config.saNamespaceSelector + } + req = &logical.Request{ Operation: logical.CreateOperation, Path: "role/plugin-test", @@ -270,7 +285,8 @@ func setupBackend(t *testing.T, config *testBackendConfig) (logical.Backend, log t.Fatalf("err:%s resp:%#v\n", err, resp) } - b.(*kubeAuthBackend).reviewFactory = testMockFactory + b.(*kubeAuthBackend).reviewFactory = testMockTokenReviewFactory + b.(*kubeAuthBackend).nsValidatorFactory = testMockNamespaceValidatorFactory return b, storage } @@ -758,6 +774,35 @@ func TestLoginSvcAcctAndNamespaceSplats(t *testing.T) { } } +func TestLoginSvcAcctNamespaceSelector(t *testing.T) { + config := defaultTestBackendConfig() + config.saName = "*" + config.saNamespace = "non-default" + config.saNamespaceSelector = matchLabelsKeyValue + b, storage := setupBackend(t, config) + + // test successful login + data := map[string]interface{}{ + "role": "plugin-test", + "jwt": jwtGoodDataToken, + } + + req := &logical.Request{ + Operation: logical.UpdateOperation, + Path: "login", + Storage: storage, + Data: data, + Connection: &logical.Connection{ + RemoteAddr: "127.0.0.1", + }, + } + + resp, err := b.HandleRequest(context.Background(), req) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("err:%s resp:%#v\n", err, resp) + } +} + func TestAliasLookAhead(t *testing.T) { testCases := map[string]struct { role string @@ -1079,29 +1124,29 @@ func TestLoginProjectedToken(t *testing.T) { "normal": { role: "plugin-test", jwt: jwtGoodDataToken, - tokenReview: testMockFactory, + tokenReview: testMockTokenReviewFactory, }, "fail": { role: "plugin-test-x", jwt: jwtGoodDataToken, - tokenReview: testMockFactory, + tokenReview: testMockTokenReviewFactory, e: roleNameError, }, "projected-token": { role: "plugin-test", jwt: jwtProjectedData, - tokenReview: testProjectedMockFactory, + tokenReview: testProjectedMockTokenReviewFactory, }, "projected-token-expired": { role: "plugin-test", jwt: jwtProjectedDataExpired, - tokenReview: testProjectedMockFactory, + tokenReview: testProjectedMockTokenReviewFactory, e: errors.New("invalid expiration time (exp) claim: token is expired"), }, "projected-token-invalid-role": { role: "plugin-test-x", jwt: jwtProjectedData, - tokenReview: testProjectedMockFactory, + tokenReview: testProjectedMockTokenReviewFactory, e: roleNameError, }, } diff --git a/path_role.go b/path_role.go index 2cdbd531..364d4d4c 100644 --- a/path_role.go +++ b/path_role.go @@ -54,7 +54,7 @@ are allowed.`, Type: framework.TypeString, Description: `A label selector for Kubernetes namspaces which are allowed to access this role. Accepts either a JSON or YAML object. If set with bound_service_account_namespaces, -the conditions are conjuncted.`, +the conditions are disjuncted.`, }, "audience": { Type: framework.TypeString, @@ -327,6 +327,13 @@ func (b *kubeAuthBackend) pathRoleCreateUpdate(ctx context.Context, req *logical if len(role.ServiceAccountNamespaces) == 0 && role.ServiceAccountNamespaceSelector == "" { return logical.ErrorResponse("%q can not be empty if %q is not set", "bound_service_account_namespaces", "bound_service_account_namespace_selector"), nil } + // Verify namespace selector is correct + if role.ServiceAccountNamespaceSelector != "" { + if _, err := makeLabelSelector(role.ServiceAccountNamespaceSelector); err != nil { + return logical.ErrorResponse("failed to parse %q as k8s.io/api/meta/v1/LabelSelector object", "bound_service_account_namespace_selector"), nil + } + } + // Verify * was not set with other data if len(role.ServiceAccountNamespaces) > 1 && strutil.StrListContains(role.ServiceAccountNamespaces, "*") { return logical.ErrorResponse("can not mix %q with values", "*"), nil diff --git a/path_role_test.go b/path_role_test.go index 4860f66e..0e99a35c 100644 --- a/path_role_test.go +++ b/path_role_test.go @@ -214,6 +214,18 @@ func TestPath_Create(t *testing.T) { }, wantErr: errInvalidAliasNameSource, }, + "invalid_namespace_label_selector": { + data: map[string]interface{}{ + "bound_service_account_names": "name", + "bound_service_account_namespace_selector": badYAMLSelector, + "policies": "test", + "period": "3s", + "ttl": "1s", + "num_uses": 12, + "max_ttl": "5s", + }, + wantErr: errors.New(`failed to parse "bound_service_account_namespace_selector" as k8s.io/api/meta/v1/LabelSelector object`), + }, "no_service_account_names": { data: map[string]interface{}{ "policies": "test",