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

Add entitlements to bake #179

Open
tonistiigi opened this issue Oct 26, 2019 · 2 comments
Open

Add entitlements to bake #179

tonistiigi opened this issue Oct 26, 2019 · 2 comments
Assignees
Labels
area/bake help wanted Extra attention is needed kind/enhancement New feature or request
Milestone

Comments

@tonistiigi
Copy link
Member

tonistiigi commented Oct 26, 2019

Entitlements are not currently enabled in bake. You can do buildx build --allow security.insecure but you can't set the same in a file for bake.

Setting this should be allowed with a two-step process. A target can request that entitlement is required, but that doesn't automatically mean that any value should be directly sent to the daemon as validation of the authority of the request. The model of entitlements requires that the user explicitly allows a possibly insecure option, and running bake should still be considered somewhat secure.

So instead, if the user is building a bake target that has requests an insecure entitlement, they would need to pass the --allow flag with the bake command, or possibly if they don't, they get prompted.

For example:

Error: target "foo" requires entitlement "security.insecure"

Pass "--allow security.insecure" to grant the requested privileges.

Your full command with requested privileges:

docker buildx bake --allow security.insecure release

The benefit of defining the requirements in the files is that we can show nice message (or prompt) and we can error right away, not on failure for a specific build. We can also show all the errors together as a group.

Not only build entitlements should get protected. There are some other properties (some of which we already support) that should be protected as well.

These are:

  • File paths:
    Context directory, Dockerfile path, the output path for local/tar exporter, the output path for local cache exporter, secret and ssh key paths.

  • Images:
    Image reference (name), exported cache reference, push action

  • SSH socket

Most of these options have values that are allowed by default. For the file paths, all paths pointing inside the working directory should be allowed by default, but if you set a Dockerfile path to /etc/passwd/ in a bake file, it should not be allowed unless you pass --allow fs.read=/etc/.

The same goes for images. It is expected that you can create a new image by default, but it is possibly harmful to allow bake to push an image automatically. A tricky case is protecting certain image names. I think we should allow tagging images as docker.io/username/foo but not just foo because the user does not own the docker.io/library namespace. So a bake shouldn't be used to rewrite an image like "ubuntu" without the user noticing. This is somewhat controversial as it would probably break some compose files. Otoh, it would motivate users to avoid this harmful behavior of not properly naming images.

Proposed values for bake --allow:

network.host
security.insecure
fs.read=<path>
fs.write=<path>
fs=<path>
image.push=<pattern>
image.load=<pattern>
image.pull=<pattern> // requires buildkit change
image=<pattern>
ssh

Default would be to allow fs=. and (maybe) image.push!=docker.io/library/*. If we can invent a better way to detect valid repo names (eg. maybe using the basename of the working directory), we could deny more.

Setting the defaults should be configurable so users can choose how cautious they want to be. Eg. you could define allow: network-host and never be prompted about this. It should also be possible to do even tighten the security, eg. deny image.name=* allow image.name=mycompany/* if you never wish to allow building anything(by default) that isn't named in the company namespace. Possibly we could also add bake --deny, so you can do --deny fs=* if you never want to give build request access to read any of your local files.

Setting the default should also be possible per project, but the actual values need to go to the local config directory, not stored in the working directory where they could get committed to git.

@tonistiigi
Copy link
Member Author

Update:

  • The value for all the keys (eg. image.push) could be <pattern> and default for setting allow with only key and not with a value should be --allow key=*. So --allow fs would grant access to every path, --allow img.push=name would allow push to a specific name.

  • With a buildkit change, we could also have image.pull that would default to * but could be used to limit access to certain images.

  • We could change the datatype for requested entitlements inside files/targets from array to a map where the value would be a human-readable description of why entitlement is needed. Eg. security.insecure: "mounting overlay filesystems" and then the error message could show target "foo" requires entitlement "security.insecure" for mounting overlay filesystems and the user could decide better if it makes sense to grant the capabilities or not.

@tonistiigi tonistiigi added the kind/enhancement New feature or request label Oct 30, 2019
@admackin

This comment was marked as resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/bake help wanted Extra attention is needed kind/enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants