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

Don't include probeToken in responses if user doesn't have pt.view permission #2477

Closed
foot opened this issue Jan 23, 2019 · 14 comments
Closed

Comments

@foot
Copy link
Contributor

foot commented Jan 23, 2019

☂️ issue: #2125

@foot foot changed the title Do Don't include probeToken in responses if user doesn't have pt.view permission Jan 23, 2019
@foot foot self-assigned this Jan 23, 2019
@foot
Copy link
Contributor Author

foot commented Jan 23, 2019

Right, so we can easily filter this out of responses from users service, but scope exposes the token in command line args etc. =/

@foot
Copy link
Contributor Author

foot commented Jan 23, 2019

I guess permission on namespaces would solve this...

@bboreham
Copy link

We could set up the token as a Secret for the scope agent? (in launch-generator)

@foot
Copy link
Contributor Author

foot commented Jan 24, 2019

Good thought!

Atm the fluxd etc still receives the secret on the command line. Could the process talk directly to k8s to get the secret?

screenshot 2019-01-24 at 11 35 28

@foot
Copy link
Contributor Author

foot commented Jan 24, 2019

Other hacky solutions:

  • Have a (middleware?) layer that does a string search / replace on any encoded JSON response just before its sends back to the client. scope launch --forbidden-strings=abcafe123
  • Have a similar layer in authfe which does a string search/replace on the stream as it goes back to the browser.

@foot
Copy link
Contributor Author

foot commented Jan 24, 2019

Hacks continued

  • Expand out the authfe auth middleware so it starts understanding scope json structure and doing a more fine grained sanitisation pass. Which could allow us to explore doing namespace filtering etc in the future... (graphqqqqqql?)

@foot
Copy link
Contributor Author

foot commented Jan 24, 2019

Have a similar layer in authfe which does a string search/replace on the stream as it goes back to the browser.

If this was very general it would be pretty cool if you had permissions to exec/attach but not to view probe token. Then we'd also filter the WS terminal stream.

$ ps aux | grep fluxd
fluxd 100 344 ./flux --token=MISSINGPERMISSIONS

@foot
Copy link
Contributor Author

foot commented Jan 24, 2019

Then we'd also filter the WS terminal stream.

Which you could hack by getting the first half and then the 2nd half of the token somehow... we should have warnings about exec/attach permissions, that is essentially giving view-token and thus admin rights.

@foot
Copy link
Contributor Author

foot commented Jan 24, 2019

Ooops didn't mean to close this.

@foot foot reopened this Jan 24, 2019
@foot
Copy link
Contributor Author

foot commented Jan 25, 2019

In light of weaveworks/scope#2106 (scope can expose passwords and tokens for all your services) we should probably include a permission to use scope at all?

@rade
Copy link
Member

rade commented Jan 25, 2019

we should probably include a permission to use scope at all?

No. If we do anything here it should be a permission that controls whether a user can see env vars, command line args, etc. This is something that would be enforced in the scope app rather than probe, since we do still want probes to send all info as some users (e.g.) need to be able to see it.

@foot
Copy link
Contributor Author

foot commented Jan 28, 2019

Cool. This feels like a good way forward then.

  • Some header / query params appended by authfe hideProcessArgs=1&hideEnvVars=1 depending on permissions.
  • Scope does per request filtering on process.args && env.var.

@foot
Copy link
Contributor Author

foot commented Jan 28, 2019

Probe gives a good indication about where to filter w/ noEnvironmentVariables && noCommandLineArguments

@foot foot removed their assignment Jan 29, 2019
@fbarl fbarl self-assigned this Feb 14, 2019
@fbarl
Copy link

fbarl commented Feb 21, 2019

Scope report censoring PR weaveworks/scope#3571 implements the Scope part of #2477 (comment).

After that one's done and merged, we can add a small conditional wrapper in authfe to wrap up the Scope part of this issue as @foot suggested.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants