-
Notifications
You must be signed in to change notification settings - Fork 66
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
Pinniped is hard to automate because of the way the clientSecret is requested. #1489
Comments
Thanks for creating the issue @simonfelding. I agree with you about the awkwardness of automating this one part of Pinniped setup, and I agree with you that the secret is actually not that secret (in the sense that the web app needs to know the plaintext secret). I had actually argued to design this feature differently to better support the type of automation that you are describing, but unfortunately I lost that battle and we introduced this bit of awkwardness to the Supervisor. The general flow for automating this today is:
While I agree it is not ideal for automation, it isn't actually that hard to automate thanks to kubectl and other similar clients being able to make the API call required. kubectl makes it pretty easy to automate it from a shell script, and Pinniped Go packages make it pretty easy to automate it from a golang program (see pinniped/test/integration/supervisor_oidcclientsecret_test.go Lines 929 to 930 in a041295
By the way, at the end of the day, the Supervisor stores this bcrypted secret in a Kubernetes Secret, so there is technically probably nothing stopping you from reverse engineering the name/location/contents of that Secret and just writing it yourself without using the official APIs. :) |
Does the above help with automation at all @simonfelding? We'd love to hear your suggestions for how to improve the documentation too! The current maintainers are all coders, not professional writers. We try our best to provide usable docs, but we are also aware of our own limitations as technical writers. :) Contributions and suggestions are very welcome. |
Thanks so much for the explanation! That's exactly the flow I read from the documentation and the tests I found in my attempt to find a secret way to do what I want. I appreciate your openness to feedback very much. The problem is obviously step 3 - it assumes that secrets are managed manually which I believe is rarely the case in any modern production cluster. Where I work we don't let developers anywhere near production environments, and many secrets would and should require a pretty high level security clearance to even access, meaning that in case of some sort of emergency we would have to call up special people to change things manually - and these kinds of things somehow always happen at night at the most inconvenient moment possible. And now imagine there are maybe 6 clusters with 300 microservices each, and all the microservices use OIDC to authenticate... That's a lot of manual reconfiguration to do at 3:40am on a saturday night. And now imagine someone had used pinniped to manage user authentication - what steps are you supposed to take gain access to the cluster if something bad happens to the clientSecrets? It probably ends up with needing backdoor access to the clusters, requiring calls very important people who do not like calls at 3:45am on a saturday night in order to restore a backup to production clusters. tl;dr:I think the problem is supervisorconfigv1alpha1.OIDCClientSpec. It should allow for a clientSecret to be passed so the state can be (re)configured through automation without anyone actually having to know what the secrets should be, since those are simply generated through a loop in Terraform or whatever automation people like. This would not lower security - it would in fact improve it because it would make it possible to configure frequent automatic secrets rotation through external tools. These tools can of course make sure that the secret isn't configured before the pinniped Supervisor is ready to receive them - assuming the health checks don't lie :) And just in case the health checks are wrong for some reason, I think it would also be very nice if the secret configuration could be created in a manner where the plain text secret is 1: created, 2: consumed by the Supervisor whenever it is ready, 3: deleted by the supervisor, since it is now stored in a hashed state. I also wouldn't recommend storing important secrets outside of a database - databases are easier to backup and recover and to keep in high availability (because they have less strict security policies than pinniped mandates!), but that's an issue for another time I suppose. Regarding documentation - I would love to contribute some easy to read technical writing, but only if I can automate my secrets! ;) |
even shorter tl;dr: The current method promotes very poor security because it almost requires that the generated clientSecret is stored in plaintext somewhere else instead of being generated automatically every time an application is deployed. For that reason, it should be possible for a deployment script to reliably generate and provide a clientSecret to prevent unauthorized access. |
Thanks for sharing your thoughts here @simonfelding.
I'm not sure if I understood this point. The current API only assumes that client secrets are requested by a client of the Kubernetes API. That client can be kubectl or any other client. This means that the client can, for example, be CI/CD automation scripts which generate/rotate client secrets and store them into the appropriate secret storage tooling used by your preferred web app deployment system (e.g. HashiCorp Vault, AWS Secret Manager, GCP Secret Manager, Kubernetes Secrets, or anything else). Keep in mind that the Supervisor does not assume that you web app is deployed on Kubernetes, since it can be any web app. Also keep in mind that client secrets must be submitted from the web app to the Supervisor during an authorization flow, so the web app must be aware of the plaintext client secret regardless of the API design of the Supervisor. I agree that it can be inconvenient that this API is effectively not declarative (can't declare client secret values up front), as every almost other Pinniped configuration API is, which means that the configuration of client secrets cannot be decided before Pinniped is even installed. I don't love this API for this reason. However, that doesn't make it less secure or less possible to automate, although it may require writing come custom scripts to automate.
I'd like to understand your use case better, because from your description it sounds like you work at a place that is appropriately careful about security. I think you are saying that an operations team is the only team which is allowed to touch production systems. That's a common setup for enterprises. Couldn't that operation team use CI/CD scripts to create client secrets using the Supervisor API and distribute those client secrets via your preferred secret manager, and to automatically reconfigure web apps when their client secrets have changed? Or use CI/CD scripts to automatically rotate the client secret whenever the app is redeployed? Or whatever automation is preferred? |
Closing this for now but please let is know if you have any further thoughts. |
Is your feature request related to a problem? Please describe.
It's really great that developers can request a clientSecret, but it makes infrastructure automation horrible (a general problem with open source OIDC providers).
For example, if I want to register a bunch of core applications such as ArgoCD and some other stuff, I need to make a request, copy the clientSecret and add it to a secret ArgoCD can read. This takes a long time and is a mess to automate.
Describe the solution you'd like
Let me specify the clientSecret for an application through a CR or whatever.
Describe alternatives you've considered
I'm sure you can imagine the string parsing mess the current method requires.
Are you considering submitting a PR for this feature?
Yes.
Tests are already implemented
It allows operators to automate things. The secret is not THAT secret, it's stored somewhere an application can access it too.
It's fully backwards compatible.
By writing a few lines on how it works in the documentation.
Additional context
I'd love to provide a guide on how to automate a pinniped deployment if you can implement this. The current documentation is not very good despite how easy pinniped actually is to use (as seen in for example #1464 and #1392). It takes a very long time just to parse how to set up a client - a tl;dr would be nice. I have one pretty much ready, but it's not ready to be contributed because I can't register my client automatically.
The text was updated successfully, but these errors were encountered: