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

Creating a BitwardenSecret loads all secrets in project into the destination secret #60

Open
deefdragon opened this issue Sep 14, 2024 · 9 comments

Comments

@deefdragon
Copy link

I have created a BitwardenSecret with one item in the map, and the end result secret has literally every secret from my secret manager in it (the key being the secret's UUID), which is a non-trivial number of items.

I have certain tooling that expects exact key format for the secret, and having a whole bunch of UUID keys in the secret will be problimatic for that tooling, let alone the security implications of having all those extra secret values lying around.

This line is one problem line, adding ALL secrets tothe output secret. This would be fine on its own were it cleaned up, but ApplySecretMap run directly after it on the secret, only updates the k8s secret to have a key/value pair with a sane name instead of the UUID.

A more sane approach would be to pass the secrets fetched directly to ApplySecretMap instead of setting it on the k8s secret in UpdateSecretValues.

@deefdragon
Copy link
Author

After making the change mentioned, the tests failed. Fixing the tests, I have come to the conclusion that this is intended behavior? If so, I think this is not a good decision, and there should at minimum be an option to disable this functionality. in a manner of least privilage, Secrets should be exposed as little as possible, and this was exposing EVERY secret to a given namespace.

@evanjarrett
Copy link

Can confirm this is an issue, or rather, I believe this is unexpected behavior.
If I'm filtering values, I would expect only the bwSecretId I specify to be exposed to the secret.
Why would I want all secrets to be exposed to an application if it doesn't need them?

A quick way to fix this would be to move this delete call one line down outside of the enclosing if, so that when a map is specified, only the secrets with bwSecretId maps remain.

The only other work around would be to create a new machine account for each kubes secret you want to create, which I think is going to make this project not very scalable... I would already be past the free tier, and for even modest deployments I could see this going past the 20 account limit.

@M4RC02U1F4A4
Copy link

Is there any news about a fix for this issue?

@deefdragon
Copy link
Author

I've been running a custom variant of this that solves this problem and a few others, (and has proven stable), but it is brute force about the lot, so not ideal and I'm not ready to make a PR for it.

That and I need permission from my employer to contribute to open source projects due to certain contract shenanigans and I cant get that until I'm off leave.

@evanjarrett
Copy link

I put together my own fork with this feature too, it just deletes out everything else in the list.
you can see the diff here (has some GHA changes so that I'm able to build it)
evanjarrett@8481fd3

This does not seem like the original intent of this project, but I don't understand how you would want all your secrets stored in a single Secret object in Kubes.

@M4RC02U1F4A4
Copy link

M4RC02U1F4A4 commented Nov 5, 2024

I agree that having a single secret makes no sense, I also don't understand why this choice

If a user needs more than one secret, it seems that currently the only option is to create a machine account for each sync (then going to give permissions to individual values rather than to a project), which is not viable, since even with the Enterprise plan the limit is 50 machine accounts

@AndreaF17
Copy link

I agree as well, If we look at the security point of view....can any maintainer provide a feedback about it?

@Krikooo
Copy link

Krikooo commented Nov 27, 2024

Any new on this issue ?

@eldarmus
Copy link

eldarmus commented Dec 5, 2024

any news ?

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

No branches or pull requests

6 participants