Provide secrets using environment variables

[skorhone]

Currently the only supported mechanism for passing secrets is to store them on filesystem. I'd like to see something like --secret id=secret,env=FOO that could be used to expose secret as environment variable.

The reason for this request is that most CI services pass secrets as environment variables. Creating a temporary file on persistent storage just doesn't feel right (or safe)

Build kit has this feature: https://pythonspeed.com/articles/docker-build-secrets/

This is extension to #1684

[rhatdan]

@ashley-cui PTAL

[skorhone]

Also, if possible, let user select whether secret from environment variable is exposed as file or environment variable. If target RUN command supports secret from environment, reading it from file with FOO=$(cat /secrets/foo) doesn't make much sense

[rhatdan]

@skorhone interested in opening a PR?

[skorhone]

@rhatdan I think resolving this properly is beyond my current abilities. If a solution that isn't 100% compatible or solution that isn't safe in all environments is fine, then I might be able to contribute.

In unsafe implementation secrets would get stored as files in a temporary layer on a persistent storage. This would inherit all the problems that current solution using secret files does. (I guess that the current implementation doesn't take in account that even for files secrets could very well be coming from non persistent storage, e.g. kubernetes secrets)

An ideal solution should use memory backed storage that gets wiped. Theoretically a tmpfs in a system that is configured without swap should work. Ramfs would be far safer, but as far as I know that's not supported for non root users (fuse).

Or if you don't mind that solution isn't buildkit/docker compatible, then I guess I could pass environment variables directly to builder rather than storing them on filesystem as files. This would solve problem for use cases, where secrets can be passed without using files. e.g. from security standpoint, it should be acceptable (while not ideal) to pass an encrypted RSA private key as a file through persistent storage, if encryption passphrase is passed using non-persistent storage such as environment variable.

Any thoughts? Security or compatibility?

[ashley-cui]

@skorhone I think we definitely have to create the file somewhere as you said, whether it be in a temporary layer, persistent storage, or tmpfs. Let me take a look at what docker does and report back

[rhatdan]

You can create it in /run or mount a tmpfs with the secrets in it, that goes away when the container exits.

[skorhone]

Assuming that current location used to store secret isn't necessarily located in tmpfs, work could be split to two parts. First part would be storing secrets from environment using the same method that is used for files.

The second part would be to move all secrets to a location that is guaranteed to be on tmpfs. Or if default is tmpfs then maybe it would be enough, if user was warned about possible security issue when secrets are used and default is overridden with persistent storage (non tmpfs / ramfs). Now that I think of, I kind of like the second option. It allows end-user to choose storage type.

Is run mount used during build process by default on tmpfs? If not, is there a tmpfs location created for each container run command that I can reuse without creating new location? If not, I guess I can create one that is not dedicated just for secrets.

This is what secrets are currently using. Documentation says that it's persistent storage, but is it really?

	// After this point we need to know the per-container persistent storage directory.
	cdir, err := b.store.ContainerDirectory(b.ContainerID)

In store there's option to get ContainerRunDirectory. Would it be safe enough? This location is bound to lifecycle of container, not the run command.

[rhatdan]

I talked to @ashley-cui about storing this temporary secret file on /dev/shm as a random tmp file and then remove it when we are finished. If the machine crashes then the file disappears.

[skorhone]

@ashley-cui @rhatdan Documentation should be updated to reflect this change.

[rhatdan]

Please open a PR to fix documentation.