serviceaccount impersonation fails

[adabuleanu]

Bug description

While working with flux and the multitenancy setup, flux is not able to impersonate the service account which is tenant owner. This happened after upgrade from capsule-proxy v0.4.5 to v0.5.0.

How to reproduce

1. Install capsule and capsule proxy v0.5.0.
2. Install flux.
3. Create a tenant with the owner the sa gitops-reconciler and a tenant namespace
   More details on https://github.com/maxgio92/capsule-gitops-testing/

Expected behavior

Service account should be impersonated and flux reconciliation should not fail.

Logs

Flux reconciliation logs:

my-kustomization       my-namespace                            XXXXXX        False           False   timeout waiting for: [Namespace/my-namespace: 'Unknown': namespaces is forbidden: User "system:serviceaccount:my-namespace:gitops-reconciler" cannot list resource "namespaces" in API group "" at the cluster scope]      

Capsule-proxy error:

{"level":"error","ts":"2024-02-09T14:01:48.606Z","msg":"Reconciliation failed after 30.767523613s, next try in 1m0s","controller":"kustomization","controllerGroup":"kustomize.toolkit.fluxcd.io","controllerKind":"Kustomization","Kustomization":{"name":"my-kustomization","namespace":"my-namespace"},"namespace":"my-namespace","name":""my-kustomization","reconcileID":"XXXXXX","revision":"XXXXXXX","error":"timeout waiting for: [Namespace/my-tenant-namespace status: 'Unknown': namespaces is forbidden: User \"system:serviceaccount:my-namespace:gitops-reconciler\" cannot list resource \"namespaces\" in API group \"\" at the cluster scope]"}

Additional context

This was initially posted in #349 and the fix was proposed for v0.4.7 release, but unfortunately I still get the issue in the latest v0.5.0 release. The only workaround is to downgrade to v0.4.5. More details can be found in #349 as the issue is identical.

[oliverbaehler]

Could you share your capsuleConfiguration?

[mtheeren-asml]

Hi Oliver, me and Adrian are working on the same project. The configuration is:

apiVersion: capsule.clastix.io/v1beta2
kind: CapsuleConfiguration
metadata:
  labels:
    app.kubernetes.io/instance: capsule
    app.kubernetes.io/name: capsule
    app.kubernetes.io/version: 0.3.2
  name: default
spec:
  enableTLSReconciler: false
  forceTenantPrefix: true
  overrides:
    TLSSecretName: capsule-tls
    mutatingWebhookConfigurationName: capsule-mutating-webhook-configuration
    validatingWebhookConfigurationName: capsule-validating-webhook-configuration
  protectedNamespaceRegex: ""
  userGroups:
  - system:authenticated

[oliverbaehler]

@maxgio92 do you have to investigate here?

[maxgio92]

Hey @adabuleanu and @mtheeren-asml, thank you for reporting this.

I'd like to highlight that https://GitHub.com/maxgio92/capsule-gitops-testing Is not an official repository, so I recommend to start from the official docs at capsule.clastix.io.

Then, I'd like to ask you:

• would you mind providing the log of capsule-proxy at the time of the specific request forbidden?
• why are you setting all authenticated users (system:authenticated group) into the Capsule group, and not the SA group?

Finally, not to introduce too much noise, we've just released an addon for Flux: Https://GitHub.com/projectcapsule/capsule-addon-fluxcd. Take a look at it :) We're going soon to document it in the main official documentation.

[mtheeren-asml]

Hey @maxgio92 ,

We actually based ourselves on the official docs, but I guess @adabuleanu mentioned that repo as an example.

Regarding your questions:

• Sure, unfortunately in the logs there's nothing besides

2024-02-08T02:02:50Z	{"level":"Level(-4)","ts":"2024-02-08T02:02:50.161Z","logger":"proxy","msg":"impersonating for the current request","username":"system:serviceaccount:consumer:gitops-reconciler","groups":["system:serviceaccounts","system:serviceaccounts:consumer"],"uri":"/api/v1/namespaces/consumer-service-exposure-poc"}
2024-02-08T02:02:50Z	{"level":"Level(-4)","ts":"2024-02-08T02:02:50.189Z","logger":"proxy","msg":"impersonating for the current request","username":"system:serviceaccount:consumer:gitops-reconciler","groups":["system:serviceaccounts","system:serviceaccounts:consumer"],"uri":"/api/v1/namespaces"}

• Because we are having some troubles with setting up a correct IAM, we are currently using gcloud login but we're lacking groups so all users are coming from same group. Since this will be reworked later on we have for the time being set this to system:authenticated

I will take a look at the addon.

[oliverbaehler]

@mtheeren-asml as you can see from you logs, your serviceaccount only have the groups system:serviceaccounts and system:serviceaccounts:consumer but not system:authenticated. If I understand the docs correctly "Allows a user read-only access to basic information about themselves." this group is only given to User kinds, not ServiceAccounts. Since your ServiceAccounts are missing the system:authenticated group they are not considered tenant user and therefor the error message is correct. You would need to add system:serviceaccounts:consumer. The addon @maxgio92 offers a clean solution for this case, since you probably have a dedicated namespace/serviceaccount for each tenant and dont want to update the capsuleConfiguration always manually.

[adabuleanu]

@mtheeren-asml as you can see from you logs, your serviceaccount only have the groups system:serviceaccounts and system:serviceaccounts:consumer but not system:authenticated. If I understand the docs correctly "Allows a user read-only access to basic information about themselves." this group is only given to User kinds, not ServiceAccounts. Since your ServiceAccounts are missing the system:authenticated group they are not considered tenant user and therefor the error message is correct. You would need to add system:serviceaccounts:consumer. The addon @maxgio92 offers a clean solution for this case, since you probably have a dedicated namespace/serviceaccount for each tenant and dont want to update the capsuleConfiguration always manually.

I was in vacation, so sorry for the latest response. In v0.4.7 the fix for #349 introduced the groups for serviceAccounts, but it did not add the system:authenticated. Was this done on purpose? In the v0.4.5 capsule-proxy logs, we can see this appearing in the groups:

{"level":"Level(-4)","ts":"2024-02-28T15:07:04.076Z","logger":"proxy","msg":"impersonating for the current request","username":""system:serviceaccount:consumer:gitops-reconciler","groups":["system:serviceaccounts","system:serviceaccounts:consumer","system:authenticated"],"uri":"/api/v1/namespaces/consumer-service-exposure-poc"}

but not in the v0.5.0 logs:

2024-02-08T02:02:50Z	{"level":"Level(-4)","ts":"2024-02-08T02:02:50.161Z","logger":"proxy","msg":"impersonating for the current request","username":"system:serviceaccount:consumer:gitops-reconciler","groups":["system:serviceaccounts","system:serviceaccounts:consumer"],"uri":"/api/v1/namespaces/consumer-service-exposure-poc"}

What this intentional (related to CVE Authentication bypass using an empty token) or a miss when applying the fix for #349?

[rgruchalski-klarrio]

Hey @oliverbaehler, @maxgio92. I work with Adrian and Mathieu on the same project.

We wonder if commit d188f12 which fixes 1c829a4 is missing system:authenticated for any particular reason?

Is it possible that the commit d188f12 should also include:

diff --git a/internal/request/http.go b/internal/request/http.go
index 29370b7..8a30af6 100644
--- a/internal/request/http.go
+++ b/internal/request/http.go
@@ -11,6 +11,7 @@ import (
        authenticationv1 "k8s.io/api/authentication/v1"
        authorizationv1 "k8s.io/api/authorization/v1"
        "k8s.io/apiserver/pkg/authentication/serviceaccount"
+       "k8s.io/apiserver/pkg/authentication/user"
        "sigs.k8s.io/controller-runtime/pkg/client"
 )

@@ -106,6 +107,7 @@ func (h http) GetUserAndGroups() (username string, groups []string, err error) {
                        if namespace, _, err := serviceaccount.SplitUsername(username); err == nil {
                                groups = append(groups, serviceaccount.AllServiceAccountsGroup)
                                groups = append(groups, fmt.Sprintf("%s%s", serviceaccount.ServiceAccountGroupPrefix, namespace))
+                               groups = append(groups, user.AllAuthenticated)
                        }
                }()
        }

?

[maxgio92]

Hello @adabuleanu @rgruchalski-klarrio, good catch! No worries for the late reply, we're going to reopen the issue.

Would you like @rgruchalski-klarrio to open a PR with that patch?

We can discuss the possible fix there if it's ok for you.

/cc @oliverbaehler @prometherion

[prometherion]

LGTM, that's definitely a regression.

[rgruchalski-klarrio]

Will do shortly, thank you for acknowledging!

[rgruchalski-klarrio]

Hey @maxgio92, @prometherion I've created a PR: #405.

[maxgio92]

Hey @oliverbaehler would you mind re-opening this issue?