cirruslabs / cirrus-ci-docs

Documentation for Cirrus CI 📚
https://cirrus-ci.org
MIT License
347 stars 108 forks source link

Masking of additional secrets #1052

Closed malena-ebert-sonarsource closed 1 year ago

malena-ebert-sonarsource commented 1 year ago

Description

I like to configure custom masks to hide secrets at runtime for values that are not passed via an ENCRYPTED[...] variable to the task.

Context

The generic case is that secrets are fetched from an external service and written to $CIRRUS_ENV at the beginning of a task. But adding a mask should not be restricted to the beginning, to allow fetching a secret with a short lease just before the usage.

fkorotkov commented 1 year ago

One though I had on this matter is to mask variables written to $CIRRUS_ENV that have suffixes like *_TOKEN, *_PASSWORD and *_CREDENTIALS. Make the Cirrus Agent a bit smarter and make it mask values of such environment variables automatically. Will that work for your use case?

malena-ebert-sonarsource commented 1 year ago

A fixed list of patterns has a high chance to leak sensitive data at some point.

Would it be possible to introduce a second env file, which is available via $CIRRUS_ENV_MASKED? All variables that are written there will be masked automatically.

fkorotkov commented 1 year ago

Second env file looks like an unnecessary complexity. In case of GHA they have so called "workflow commands" which allows to print out instructions to logs like masking values. But IMO it's a very questionable thing. There was a least one CVE around that area.

I think the easiest option to memorize will be that environment variables ending with *_SECRET from the $CIRRUS_ENV file will be masked. Just one option which is very explicit.

malena-ebert-sonarsource commented 1 year ago

My experience showed me that enforcing naming patterns is not working well in this context because it is hard to validate. In most cases, a misconfiguration is detected at a stage where sensitive information was leaked already.

fkorotkov commented 1 year ago

What do you mean by "in this context because it is hard to validate"? I had really hard time to find any similar functionality in other CIs. Most of them doesn't seem to support adding such variables dynamically. The only close thing I found is how Buildkite is doing it and by default they mask *_PASSWORD, *_SECRET, *_TOKEN, *_ACCESS_KEY, *_SECRET_KEY.

malena-ebert-sonarsource commented 1 year ago

In contexts where autonomous users are able to configure new secrets, but forget to use a naming pattern.

I rethought the problem we try to solve because I have the feeling it is getting too complex. Instead of masking a subset of environment variables, every value of $CIRRUS_ENV should be masked by default. If this is a breaking change for you, an opt-in flag on the organization level would be okay.

fkorotkov commented 1 year ago

I though about masking all the variables from $CIRRUS_ENV but people might just override $PATH there.... Yeah, let's put this logic under CIRRUS_ENV_SENSITIVE=true which will indicate that values are sensitive and should be masked. But let's also mask *_PASSWORD, *_SECRET, *_TOKEN, *_ACCESS_KEY, *_SECRET_KEY variables by default since it looks like a good thing to have.