Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(auth): deploy openshift auth proxy #799

Merged
merged 22 commits into from
Apr 26, 2024

Conversation

andrewazores
Copy link
Member

@andrewazores andrewazores commented Apr 23, 2024

Welcome to Cryostat! 👋

Before contributing, make sure you have:

  • Read the contributing guidelines
  • Linked a relevant issue which this PR resolves
  • Linked any other relevant issues, PR's, or documentation, if any
  • Resolved all conflicts, if any
  • Rebased your branch PR on top of the latest upstream main branch
  • Attached at least one of the following labels to the PR: [chore, ci, docs, feat, fix, test]
  • Signed all commits: git commit -S -m "YOUR_COMMIT_MESSAGE"

Related to #710
Based on (merged in) #800

Description of the change:

Deploys openshift-oauth-proxy in the Pod and points the existing Service (and Route or Ingress) at it, rather than at Cryostat directly. The auth proxy is always configured to require one particular set of Roles/permissions for both interactive users as well as programmatic clients, and always delegates to OpenShift SSO. In this PR it cannot be disabled and it cannot be augmented with htpasswd Basic auth. It also does not handle TLS.

Motivation for the change:

In Cryostat 3.0 there should always be an auth proxy placed in front of the operand containers, since in the new architecture the authz responsibility is always delegated to such an auth proxy and never performed by the Cryostat container. The OpenShift OAuth Proxy with SubjectAccessReview and TokenReview configuration in this manner is very close to what Cryostat <= 2.4 did and generally allows users and clients to authenticate and access the application and API with the same permissions as before.

How to manually test:

  1. OPERATOR_IMG=quay.io/andrewazores/cryostat-operator:3.0.0-openshift-oauth-proxy-3 DISABLE_SERVICE_TLS=true DEPLOY_NAMESPACE=$(oc project -q) make install deploy
  2. Create a Cryostat CR with enableCertManager: false
  3. Wait for deployment to become ready, then open the route URL in your browser
  4. Try to log in with an unprivileged user, ex. developer. You should see a "permission denied" page. Click "Log in" again and this time log in with a privileged user, ex. kubeadmin. You should get in to the application and be able to perform some basic functionality ex view Security certs/credentials lists (many still broken due to HTTP 301 redirection responses mangle Location header openshift/oauth-proxy#272)
  5. Log out and verify that repeating step 4 behaves the same way again
  6. In a terminal, use http/curl/wget: http --follow cryostat-sample-cryostat3-helm.apps-crc.testing/api/v3/tls/certs produces a login page response, while http --follow --auth-type=bearer --auth=$(oc whoami -t) cryostat-sample-cryostat3-helm.apps-crc.testing/api/v3/tls/certs returns the list response (assuming you are logged in with oc as a privileged user). http --follow --auth-type=bearer --auth=FAKE$(oc whoami -t) cryostat-sample-cryostat3-helm.apps-crc.testing/api/v3/tls/certs (a bad/corrupted token) should result in the login page response.

@andrewazores andrewazores added feat New feature or request safe-to-test labels Apr 23, 2024
@andrewazores
Copy link
Member Author

Currently fails like this:

HTTP/1.1 400 Bad Request
Audit-Id: 70518848-d2f5-4428-9570-a8307416cc29
Cache-Control: no-cache, no-store, max-age=0, must-revalidate, no-cache, no-store, max-age=0, must-revalidate
Content-Length: 200
Content-Type: application/json
Date: Tue, 23 Apr 2024 20:08:11 GMT
Expires: 0, Fri, 01 Jan 1990 00:00:00 GMT
Pragma: no-cache, no-cache
Referrer-Policy: strict-origin-when-cross-origin
X-Content-Type-Options: nosniff
X-Dns-Prefetch-Control: off
X-Frame-Options: DENY
X-Xss-Protection: 1; mode=block

{
    "error": "server_error",
    "error_description": "The authorization server encountered an unexpected condition that prevented it from fulfilling the request.",
    "state": "0cbe0919c4dd9515042bac90e1d6dbd6:/"
}

@andrewazores
Copy link
Member Author

Screenshot_2024-04-23_21-51-44

Screenshot_2024-04-23_21-51-53

Works now in the most basic case: openshift-oauth-proxy is always deployed, never oauth2_proxy. Basic auth via htpasswd is not yet implemented. SubjectAccessReview and TokenReview (therefore Bearer auth) are not yet implemented. This is deploying the upstream proxy image which has the URL decoding/redirecting bug, so many features don't work and it's tough to test much but I suspect that report generation and "View in Grafana" probably aren't working yet, either.

Tests are failing so must currently be skipped during build.

@andrewazores
Copy link
Member Author

2024/04/24 17:37:38 provider.go:129: Defaulting client-id to system:serviceaccount:cryostat3:cryostat-sample
2024/04/24 17:37:38 provider.go:134: Defaulting client-secret to service account token /var/run/secrets/kubernetes.io/serviceaccount/token
2024/04/24 17:37:38 provider.go:358: Delegation of authentication and authorization to OpenShift is enabled for bearer tokens and client certificates.
2024/04/24 17:37:38 provider.go:400: unable to retrieve authorization information for users: subjectaccessreviews.authorization.k8s.io is forbidden: User "system:serviceaccount:cryostat3:cryostat-sample" cannot create resource "subjectaccessreviews" in API group "authorization.k8s.io" at the cluster scope
2024/04/24 17:37:40 provider.go:400: unable to retrieve authorization information for users: subjectaccessreviews.authorization.k8s.io is forbidden: User "system:serviceaccount:cryostat3:cryostat-sample" cannot create resource "subjectaccessreviews" in API group "authorization.k8s.io" at the cluster scope
2024/04/24 17:37:42 provider.go:400: unable to retrieve authorization information for users: subjectaccessreviews.authorization.k8s.io is forbidden: User "system:serviceaccount:cryostat3:cryostat-sample" cannot create resource "subjectaccessreviews" in API group "authorization.k8s.io" at the cluster scope
2024/04/24 17:37:44 provider.go:400: unable to retrieve authorization information for users: subjectaccessreviews.authorization.k8s.io is forbidden: User "system:serviceaccount:cryostat3:cryostat-sample" cannot create resource "subjectaccessreviews" in API group "authorization.k8s.io" at the cluster scope
2024/04/24 17:37:46 provider.go:400: unable to retrieve authorization information for users: subjectaccessreviews.authorization.k8s.io is forbidden: User "system:serviceaccount:cryostat3:cryostat-sample" cannot create resource "subjectaccessreviews" in API group "authorization.k8s.io" at the cluster scope
2024/04/24 17:37:48 provider.go:400: unable to retrieve authorization information for users: subjectaccessreviews.authorization.k8s.io is forbidden: User "system:serviceaccount:cryostat3:cryostat-sample" cannot create resource "subjectaccessreviews" in API group "authorization.k8s.io" at the cluster scope
2024/04/24 17:37:48 provider.go:400: unable to retrieve authorization information for users: subjectaccessreviews.authorization.k8s.io is forbidden: User "system:serviceaccount:cryostat3:cryostat-sample" cannot create resource "subjectaccessreviews" in API group "authorization.k8s.io" at the cluster scope
2024/04/24 17:37:48 oauthproxy.go:210: mapping path "/" => upstream "http://localhost:8181/"

I guess the (Cluster)Role applied to the Pod, or the authproxy container, needs to gain the permission to create SubjectAccessReviews.

@andrewazores
Copy link
Member Author

It looks to me like it should already have such permissions:

- subjectaccessreviews

@ebaron what am I missing here?

@andrewazores andrewazores marked this pull request as ready for review April 25, 2024 19:07
@andrewazores andrewazores requested a review from ebaron April 25, 2024 19:07
@andrewazores
Copy link
Member Author

@ebaron this is still quite rough and incomplete, but I think it's a reasonable starting point for the OpenShift OAuth Proxy. The TLS configuration stuff is a little beyond me right now, but after this I'll try configuring the Basic htpasswd auth on this proxy container as a follow-up PR. Once that's done I can try switching out between the OpenShift OAuth Proxy and OAuth2 Proxy depending on where the Operator is running.

@ebaron
Copy link
Member

ebaron commented Apr 25, 2024

It looks to me like it should already have such permissions:

- subjectaccessreviews

@ebaron what am I missing here?

Is it working after adding it to the cryostat_role.yaml as well?

@andrewazores
Copy link
Member Author

After this commit the auth proxy stopped complaining like that in its logs and the behaviour started doing what I expected, ie same as in cryostatio/cryostat-helm#132

Copy link
Member

@ebaron ebaron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed this line in the OAuth Proxy README:

  • Requires no additional permissions to be granted for the proxy service account

It seems like if don't use the delegate URLs option, then we somehow don't need SAR permissions on the service account. This would be great since we could likely remove the entire Cluster Role and Cluster Role Binding for Cryostat's service account. Perhaps we could accomplish the same thing by using the --bypass-auth-for option instead like this:

diff --git a/internal/controllers/common/resource_definitions/resource_definitions.go b/internal/controllers/common/resource_definitions/resource_definitions.go
index e05c691d..55e2f899 100644
--- a/internal/controllers/common/resource_definitions/resource_definitions.go
+++ b/internal/controllers/common/resource_definitions/resource_definitions.go
@@ -632,12 +632,8 @@ func NewOpenShiftAuthProxyContainer(cr *model.CryostatInstance, specs *ServiceSp
                        `--openshift-sar=[{"group":"","name":"","namespace":"%s","resource":"pods","subresource":"exec","verb":"create","version":""}]`,
                        cr.InstallNamespace,
                ),
-               fmt.Sprintf(
-                       `--openshift-delegate-urls={"/health":{},"/api":{"group":"","name":"","namespace":"%s","resource":"pods","subresource":"exec","verb":"create","version":""}, "/storage":{"group":"","name":"","namespace":"%s","resource":"pods","subresource":"exec","verb":"create","version":""}, "/grafana":{"group":"","name":"","namespace":"%s","resource":"pods","subresource":"exec","verb":"create","version":""} }`,
-                       cr.InstallNamespace,
-                       cr.InstallNamespace,
-                       cr.InstallNamespace,
-               ),
+
+               `--bypass-auth-for=^/health$`,
                "--proxy-prefix=/oauth2",
        }
        // if tls != nil {

I tried it in OpenShift and it seemed to work. Does it work for you too?

@andrewazores
Copy link
Member Author

I can test it, but from what I understand from the proxy's docs, that config flag is required for the proxy to handle Bearer authorization headers, which we need for our Agent.

@andrewazores
Copy link
Member Author

andrewazores commented Apr 26, 2024

Tested it and it does look like this flag is required for the Authorization: Bearer handling to work. When I remove that flag I am still able to log in interactively in my browser, due to the --openshift-sar flag, but using http as described in the PR body always fails and just returns the login page HTML response.

https://github.com/openshift/oauth-proxy/blob/30f8012482023689655252dc2af2f17fe6a09253/README.md?plain=1#L94

Copy link
Member

@ebaron ebaron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Understood on the agent use-case. It wasn't immediately clear to me what the delegate URLs option was for. I'm still not totally clear how they get away without the SAR permission in the normal case, but this seems like our only option.

We could use an entry here for RELATED_IMAGE_OPENSHIFT_OAUTH_PROXY: https://github.com/cryostatio/cryostat-operator/blob/main/hack/image_tag_patch.yaml.in
Then we'd need another make bundle.

@andrewazores
Copy link
Member Author

andrewazores commented Apr 26, 2024

I think in the normal interactive case the proxy doesn't need the SAR permission because it is only acting as a redirector to send the client to the OpenShift cluster's internal SSO provider. That's what performs the SAR, and if it passes, issues a new restricted token which the client stores as a cookie. Then it redirects the user back to the authproxy, which now receives the cookie, passes it back to the internal SSO provider which approves it, and then the authproxy begins forwarding the user's request to the upstream service (Cryostat container).

@andrewazores
Copy link
Member Author

As for why the TokenReview for Authorization: Bearer headers does require an elevated TokenReview role... I guess because in this case the proxy itself is handling the raw token passed by the client, rather than simply redirecting the client to the cluster's infrastructure SSO provider. I suppose the proxy must therefore perform the SAR/TR against the internal infrastructure. Even if it could simply pass it on to the cluster SSO, it still handled the client's raw token directly, so it should require elevated permissions. Much like how Cryostat 2.4 directly handles the user's auth tokens.

@andrewazores andrewazores merged commit a26ff9c into cryostatio:cryostat3 Apr 26, 2024
5 checks passed
@andrewazores andrewazores deleted the cryostat3-oauth2-proxy branch April 26, 2024 21:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat New feature or request safe-to-test
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants