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

fix(proxy): do not pass unused credentials upstream #839

Merged

Conversation

andrewazores
Copy link
Member

@andrewazores andrewazores commented Jun 4, 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"

Fixes: #838

Description of the change:

Explicitly sets some flags on the OpenShift OAuth Proxy to prevent it from passing headers to upstream servers (Cryostat, Grafana dashboard, storage) containing Basic credentials or auth tokens which the upstreams do not use and should not have access to if they don't have a particular need.

According to https://oauth2-proxy.github.io/oauth2-proxy/configuration/alpha-config#removed-options , the oauth2-proxy's equivalent flags are not accessible when using the alpha configuration. We use alpha configuration because it is the only way to turn on the proxyRawPath setting (https://oauth2-proxy.github.io/oauth2-proxy/configuration/alpha-config#upstreamconfig), which is the switch for this proxy to avoid the path encoding redirect bug. When using alpha configuration it seems that none of these flags are enabled by default, they must each be configured explicitly using the new https://oauth2-proxy.github.io/oauth2-proxy/configuration/alpha-config#header options.

Motivation for the change:

This enhances security, but also fixes a bug where Grafana 9 seems to attempt to validate credentials provided to it even if it is configured for anonymous access. Since we only configure it for anonymous access, passing along unexpected headers to it causes it to fail to evaluate these credentials and block the request, despite the anonymous access configuration.

How to manually test:

  1. Insert steps here...
  2. ...

@andrewazores
Copy link
Member Author

/build_test

Copy link

github-actions bot commented Jun 4, 2024

/build_test completed successfully ✅.
View Actions Run.

@andrewazores andrewazores merged commit 0832d98 into cryostatio:cryostat3 Jun 4, 2024
23 checks passed
@andrewazores andrewazores deleted the cryostat3-proxy-headers branch June 4, 2024 18:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants