diff --git a/salt/_auth/kubernetes_rbac.py b/salt/_auth/kubernetes_rbac.py index fa38800b63..7c72e40f9f 100644 --- a/salt/_auth/kubernetes_rbac.py +++ b/salt/_auth/kubernetes_rbac.py @@ -43,23 +43,6 @@ def wrapped(*args, **kwargs): return wrapped -def _check_k8s_creds(kubeconfig, token): - """Check the provided credentials against /version.""" - # Using the '/version/' endpoint which is unauthenticated by default but, - # when presented authentication data, will process this information and fail - # accordingly. - url = '{}/version/'.format(kubeconfig.host) - verify = kubeconfig.ssl_ca_cert if kubeconfig.verify_ssl else False - try: - response = requests.get( - url, headers={'Authorization': token}, verify=verify - ) - return 200 <= response.status_code < 300 - except: - log.exception('Error during request') - raise - - def _check_auth_args(on_fail=lambda: False): def wrapper(f): @wraps(f) @@ -168,7 +151,34 @@ def _groups_basic(kubeconfig, username, password): @_log_exceptions def _auth_bearer(kubeconfig, username, token): - return _check_k8s_creds(kubeconfig, 'Bearer {}'.format(token)) + """Check the provided bearer token using the TokenReview API.""" + client = kubernetes.client.ApiClient(configuration=kubeconfig) + authn_api = kubernetes.client.AuthenticationV1Api(api_client=client) + + token_review = authn_api.create_token_review( + body=kubernetes.client.V1TokenReview( + spec=kubernetes.client.V1TokenReviewSpec(token=token) + ) + ) + + if token_review.status.error: + log.error('Failed to create TokenReview: %s', + token_review.status.error) + return False + + if token_review.status.authenticated: + if token_review.status.user.username != username: + log.error( + "Provided token belongs to '%s', does not match '%s'", + token_review.status.user.username, + username, + ) + return False + else: + return True + else: + log.error('Provided token failed to authenticate') + return False AUTH_HANDLERS['bearer'] = { 'auth': _auth_bearer, diff --git a/salt/metalk8s/salt/master/files/master-99-metalk8s.conf.j2 b/salt/metalk8s/salt/master/files/master-99-metalk8s.conf.j2 index 49865335f4..35dc2ccd48 100644 --- a/salt/metalk8s/salt/master/files/master-99-metalk8s.conf.j2 +++ b/salt/metalk8s/salt/master/files/master-99-metalk8s.conf.j2 @@ -42,7 +42,7 @@ external_auth: - '@wheel' - '@runner' - '@jobs' - storage-operator: + 'system:serviceaccount:kube-system:storage-operator': - '*': - 'disk.dump' - 'state.sls' diff --git a/storage-operator/pkg/controller/volume/volume_controller.go b/storage-operator/pkg/controller/volume/volume_controller.go index 606942c1ef..f9b151af57 100644 --- a/storage-operator/pkg/controller/volume/volume_controller.go +++ b/storage-operator/pkg/controller/volume/volume_controller.go @@ -2,7 +2,6 @@ package volume import ( "context" - b64 "encoding/base64" "fmt" "io/ioutil" "time" @@ -975,7 +974,10 @@ func getAuthCredential(config *rest.Config) *salt.Credential { if config.BearerToken != "" { log.Info("using ServiceAccount bearer token") return salt.NewCredential( - "storage-operator", config.BearerToken, salt.Bearer, + // FIXME: this should depend on the actual SA used + "system:serviceaccount:kube-system:storage-operator", + config.BearerToken, + salt.Bearer, ) } else if config.Username != "" && config.Password != "" { log.Info("using Basic HTTP authentication") diff --git a/storage-operator/pkg/controller/volume/volume_controller_test.go b/storage-operator/pkg/controller/volume/volume_controller_test.go index f4a296b61b..9063fe2730 100644 --- a/storage-operator/pkg/controller/volume/volume_controller_test.go +++ b/storage-operator/pkg/controller/volume/volume_controller_test.go @@ -18,7 +18,9 @@ func TestGetAuthCredential(t *testing.T) { "ServiceAccount": { token: "foo", username: "", password: "", expected: salt.NewCredential( - "storage-operator", "foo", salt.Bearer, + "system:serviceaccount:kube-system:storage-operator", + "foo", + salt.Bearer, ), }, "BasicAuth": { diff --git a/tests/post/features/salt_api.feature b/tests/post/features/salt_api.feature index e5972addfc..e6f12ebd63 100644 --- a/tests/post/features/salt_api.feature +++ b/tests/post/features/salt_api.feature @@ -9,12 +9,22 @@ Feature: SaltAPI And we have '@runner' perms And we have '@jobs' perms - Scenario: Login to SaltAPI using a ServiceAccount + Scenario: Login to SaltAPI using the storage-operator ServiceAccount Given the Kubernetes API is available - When we login to SaltAPI with the ServiceAccount 'storage-operator' + When we login to SaltAPI with the ServiceAccount 'kube-system/storage-operator' Then we can invoke '["disk.dump", "state.sls"]' on '*' And we have '@jobs' perms + Scenario: Login to SaltAPI using any ServiceAccount + Given the Kubernetes API is available + When we login to SaltAPI with the ServiceAccount 'kube-system/default' + Then we have no permissions + + Scenario: SaltAPI impersonation using a ServiceAccount + Given the Kubernetes API is available + When we impersonate user 'system:serviceaccount:kube-system:storage-operator' against SaltAPI using the ServiceAccount 'kube-system/default' + Then authentication fails + Scenario: Login to SaltAPI using an incorrect password Given the Kubernetes API is available When we login to SaltAPI as 'admin' using password 'notadmin' diff --git a/tests/post/steps/test_salt_api.py b/tests/post/steps/test_salt_api.py index 52bd3dda41..27cf893a61 100644 --- a/tests/post/steps/test_salt_api.py +++ b/tests/post/steps/test_salt_api.py @@ -16,8 +16,19 @@ def test_login_basic_auth_to_salt_api(host): pass @scenario('../features/salt_api.feature', - 'Login to SaltAPI using a ServiceAccount') -def test_login_bearer_auth_to_salt_api(host): + 'Login to SaltAPI using the storage-operator ServiceAccount') +def test_login_salt_api_storage_operator(host): + pass + + +@scenario('../features/salt_api.feature', + 'Login to SaltAPI using any ServiceAccount') +def test_login_salt_api_service_account(host): + pass + +@scenario('../features/salt_api.feature', + 'SaltAPI impersonation using a ServiceAccount') +def test_salt_api_impersonation_with_bearer_auth(host): pass @scenario('../features/salt_api.feature', 'Login to SaltAPI using an incorrect password') @@ -49,21 +60,43 @@ def login_salt_api_basic(host, username, password, version, context): @when(parsers.parse( - "we login to SaltAPI with the ServiceAccount '{account_name}'")) -def login_salt_api_token(host, k8s_client, account_name, version, context): + "we login to SaltAPI with the ServiceAccount '{namespace}/{account_name}'")) +def login_salt_api_token( + host, k8s_client, namespace, account_name, version, context +): address = _get_salt_api_address(host, version) service_account = k8s_client.read_namespaced_service_account( - name=account_name, namespace='kube-system' + name=account_name, namespace=namespace ) + username = 'system:serviceaccount:{}:{}'.format(namespace, account_name) secret = k8s_client.read_namespaced_secret( - name=service_account.secrets[0].name, namespace='kube-system' + name=service_account.secrets[0].name, namespace=namespace ) token = base64.decodebytes(secret.data['token'].encode('utf-8')) context['salt-api'] = _salt_api_login( - address, username=account_name, token=token + address, username=username, token=token ) +@when(parsers.parse( + "we impersonate user '{username}' against SaltAPI " + "using the ServiceAccount '{namespace}/{account_name}'" +)) +def login_salt_api_token_override_username( + host, k8s_client, namespace, account_name, username, version, context +): + address = _get_salt_api_address(host, version) + service_account = k8s_client.read_namespaced_service_account( + name=account_name, namespace=namespace + ) + secret = k8s_client.read_namespaced_secret( + name=service_account.secrets[0].name, namespace=namespace + ) + token = base64.decodebytes(secret.data['token'].encode('utf-8')) + context['salt-api'] = _salt_api_login( + address, username=username, token=token + ) + # }}} # Then {{{ @@ -102,6 +135,9 @@ def invoke_module_on_target(host, context, modules, targets): def have_perms(host, context, perms): assert perms in context['salt-api']['perms'] +@then(parsers.parse("we have no permissions")) +def have_no_perms(host, context): + assert context['salt-api']['perms'] == {} # }}} # Helpers {{{