Implement frontend support for RUN --security=insecure by smira · Pull Request #1081 · moby/buildkit

smira · 2019-07-16T16:41:20Z

This follows syntax suggested in #950.

Example:

RUN --security=insecure cat /proc/self/status | grep CapEff
#84 0.093 CapEff:	0000003fffffffff

GordonTheTurtle · 2019-07-16T16:41:22Z

Please sign your commits following these rules:
https://github.com/moby/moby/blob/master/CONTRIBUTING.md#sign-your-work
The easiest way to do this is to amend the last commit:

$ git clone -b "frontend-ents" git@github.com:smira/buildkit.git somewhere
$ cd somewhere
$ git commit --amend -s --no-edit
$ git push -f

Amending updates the existing PR. You DO NOT need to open a new one.

util/testutil/integration/oci.go

smira · 2019-07-16T17:28:30Z

Tests are failing because I enabled the security.insecure entitlement for dockerfile tests to pass.

tonistiigi · 2019-07-16T18:42:45Z

I think we should leave the entitlements keyword only for granting capabilities, not for enabling features.

Maybe:

RUN --security=insecure

with possible other more fine-tuned values in the future.

And for network

RUN --net=host|none

Tests are failing because I enabled the security.insecure entitlement for dockerfile tests to pass.

Use the test matrix instead of setting entitlements globally.

buildkit/client/client_test.go

Lines 107 to 110 in aee28f4

    
           integration.WithMatrix("secmode", map[string]interface{}{ 
        
           	"sandbox":  securitySandbox, 
        
           	"insecure": securityInsecure, 
        
           }),

@AkihiroSuda @tiborvass @justincormack @kunalkushwaha

smira · 2019-07-16T19:51:35Z

Use the test matrix instead of setting entitlements globally.

thanks, I will get it updated.

Maybe:
RUN --security=insecure

sure, if this is the consensus, I'll use this syntax

frontend/dockerfile/docs/experimental.md

frontend/dockerfile/dockerfile_runsecurity_test.go

tonistiigi · 2019-07-17T18:17:49Z

frontend/dockerfile/dockerfile_runsecurity_test.go

+
+	dockerfile := []byte(`
+FROM busybox
+RUN --security=insecure [ "$(cat /proc/self/status | grep CapEff)" == "CapEff:	0000003fffffffff" ]


Make sure there is a test that checks that this is not set when --security=insecure is not specified(even if entitlement is allowed).

added another test for that, when entitlements are allowed, solver fails if insecure is not enabled server side still

nit: maybe CapBnd would be slightly better to check instead

thanks, @tonistiigi updated

tiborvass · 2019-07-17T19:47:56Z

+1 for RUN --security=insecure. @tonistiigi Should we support sandbox to allow RUN --security=$param ?

tonistiigi · 2019-07-17T20:11:11Z

@tiborvass yes

frontend/dockerfile/docs/experimental.md

smira · 2019-07-17T20:33:06Z

@tiborvass yes

I'll get it updated to support =sandbox

smira · 2019-07-17T20:46:16Z

@tonistiigi @tiborvass added security=sandbox

smira · 2019-07-17T21:48:14Z

I will squash commits, just keeping them separate to make it easier to review them.

smira · 2019-07-18T13:47:35Z

Updated CapEff -> CapBnd, squashed commits (no other changes)

This relies on two PRs to the buildkit, which aren't merged yet, so I had to do some overrides to apply them: * moby/buildkit#1081 * moby/buildkit#1085 Signed-off-by: Andrey Smirnov <smirnov.andrey@gmail.com>

tonistiigi · 2019-07-18T21:11:44Z

frontend/dockerfile/docs/experimental.md

+This runs the command without sandbox in insecure mode, which allows to run flows requiring elevated
+privileges (e.g. containerd). This is equivalent to running `docker run --privileged`.
+
+Default sandbox mode can be activated via `--security=sandbox`, but that is no-op.


add: In order to build Dockerfiles with --security=insecure user needs to enable security.insecure entitlement when starting the buildkit daemon and allow it for a specific build request.

tonistiigi · 2019-07-18T21:15:16Z

frontend/dockerfile/docs/experimental.md

 You can also specify a path to `*.pem` file on the host directly instead of `$SSH_AUTH_SOCK`.
 However, pem files with passphrases are not supported.

+### RUN --security=insecure


I'd prefer --security=sandbox|insecure for completeness. You can tweak the next sentence a bit so that it only applies if the value is set to "insecure".

Example: RUN --security=insecure cat /proc/self/status | grep CapEff moby#84 0.093 CapEff: 0000003fffffffff Signed-off-by: Andrey Smirnov <smirnov.andrey@gmail.com>

smira · 2019-07-18T22:02:25Z

squashed with docs updated

frontend/dockerfile/release/experimental/tags

This relies on two PRs to the buildkit: * moby/buildkit#1081 * moby/buildkit#1085 Sysfs fix was merged to upstream, so updated tag, while using `Dockerfile` slug I can switch to dockerfile2llb with support for `--security=insecure`. Signed-off-by: Andrey Smirnov <smirnov.andrey@gmail.com>

This allows to do `make test TESTPKGS=./internal/app/machined`. Also update Dockerfile slug as moby/buildkit#1081 was merged into master. Signed-off-by: Andrey Smirnov <smirnov.andrey@gmail.com>

GordonTheTurtle added the dco/no label Jul 16, 2019

smira force-pushed the frontend-ents branch from c4cea72 to 8cc1c06 Compare July 16, 2019 16:41

GordonTheTurtle removed the dco/no label Jul 16, 2019

smira commented Jul 16, 2019

View reviewed changes

util/testutil/integration/oci.go Outdated Show resolved Hide resolved

smira force-pushed the frontend-ents branch 2 times, most recently from ca56db5 to 4271975 Compare July 17, 2019 17:17

smira changed the title ~~Implement frontend support for RUN --ents=...~~ Implement frontend support for RUN --security=insecure Jul 17, 2019

tonistiigi reviewed Jul 17, 2019

View reviewed changes

GordonTheTurtle added the dco/no label Jul 17, 2019

tonistiigi reviewed Jul 17, 2019

View reviewed changes

frontend/dockerfile/docs/experimental.md Outdated Show resolved Hide resolved

smira force-pushed the frontend-ents branch from 699c7cd to ecc924c Compare July 18, 2019 13:46

GordonTheTurtle removed the dco/no label Jul 18, 2019

smira mentioned this pull request Jul 18, 2019

chore: run tests in the buildkit itself siderolabs/talos#888

Merged

tonistiigi reviewed Jul 18, 2019

View reviewed changes

tonistiigi mentioned this pull request Jul 18, 2019

build: add allowed entitlements docker/buildx#104

Merged

GordonTheTurtle added the dco/no label Jul 18, 2019

Implement frontend support for RUN --security=insecure

86e19b6

Example: RUN --security=insecure cat /proc/self/status | grep CapEff moby#84 0.093 CapEff: 0000003fffffffff Signed-off-by: Andrey Smirnov <smirnov.andrey@gmail.com>

smira force-pushed the frontend-ents branch from d975a90 to 86e19b6 Compare July 18, 2019 22:01

GordonTheTurtle removed the dco/no label Jul 18, 2019

AkihiroSuda reviewed Jul 19, 2019

View reviewed changes

frontend/dockerfile/release/experimental/tags Show resolved Hide resolved

AkihiroSuda approved these changes Jul 19, 2019

View reviewed changes

tonistiigi approved these changes Jul 22, 2019

View reviewed changes

tonistiigi merged commit 7e31f58 into moby:master Jul 22, 2019

smira mentioned this pull request Jul 23, 2019

chore: allow to run tests only for specified packages siderolabs/talos#902

Merged

bossmc mentioned this pull request Aug 17, 2019

Implement support for RUN --network=none|default|host #1141

Merged

thaJeztah mentioned this pull request Oct 22, 2019

[19.03 backport] builder entitlements configuration added. docker-archive/engine#412

Merged

thaJeztah mentioned this pull request Mar 26, 2024

Adding instructions in Dockerfile : CMD --privileged sh -c command1 && command2 ... moby/moby#47632

Open

Conversation

smira commented Jul 16, 2019 • edited by tiborvass Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

GordonTheTurtle commented Jul 16, 2019

Uh oh!

Uh oh!

smira commented Jul 16, 2019

Uh oh!

tonistiigi commented Jul 16, 2019

Uh oh!

smira commented Jul 16, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tonistiigi Jul 17, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

smira Jul 17, 2019

Choose a reason for hiding this comment

Uh oh!

tonistiigi Jul 17, 2019

Choose a reason for hiding this comment

Uh oh!

smira Jul 18, 2019

Choose a reason for hiding this comment

Uh oh!

tiborvass commented Jul 17, 2019

Uh oh!

tonistiigi commented Jul 17, 2019

Uh oh!

Uh oh!

smira commented Jul 17, 2019

Uh oh!

smira commented Jul 17, 2019

Uh oh!

smira commented Jul 17, 2019

Uh oh!

smira commented Jul 18, 2019

Uh oh!

tonistiigi Jul 18, 2019

Choose a reason for hiding this comment

Uh oh!

smira Jul 18, 2019

Choose a reason for hiding this comment

Uh oh!

tonistiigi Jul 18, 2019

Choose a reason for hiding this comment

Uh oh!

smira Jul 18, 2019

Choose a reason for hiding this comment

Uh oh!

smira commented Jul 18, 2019

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

smira commented Jul 16, 2019 •

edited by tiborvass

Loading

tonistiigi Jul 17, 2019 •

edited

Loading