Skip to content

Commit

Permalink
llbsolver: make sure interactive container API validates entitlements
Browse files Browse the repository at this point in the history
Ensure interactive calls validate same conditions that
the build requests do. Refactor of the build side is to ensure
we use the same validation function for both cases. There
was no validation issue with the LLB validation.

Signed-off-by: Tonis Tiigi <[email protected]>
(cherry picked from commit d1970522d7145be5f4a1f1a028b1910bb527126c)
(cherry picked from commit e1e30278d0a491dfd34bd80fa66b54106614cffa)
  • Loading branch information
tonistiigi committed Jan 31, 2024
1 parent 5026d95 commit 92cc595
Show file tree
Hide file tree
Showing 4 changed files with 97 additions and 16 deletions.
58 changes: 51 additions & 7 deletions client/build_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,8 @@ func TestClientGatewayIntegration(t *testing.T) {
), integration.WithMirroredImages(integration.OfficialImages("busybox:latest")))

integration.Run(t, integration.TestFuncs(
testClientGatewayContainerSecurityMode,
testClientGatewayContainerSecurityModeCaps,
testClientGatewayContainerSecurityModeValidation,
), integration.WithMirroredImages(integration.OfficialImages("busybox:latest")),
integration.WithMatrix("secmode", map[string]interface{}{
"sandbox": securitySandbox,
Expand All @@ -71,7 +72,8 @@ func TestClientGatewayIntegration(t *testing.T) {
)

integration.Run(t, integration.TestFuncs(
testClientGatewayContainerHostNetworking,
testClientGatewayContainerHostNetworkingAccess,
testClientGatewayContainerHostNetworkingValidation,
),
integration.WithMirroredImages(integration.OfficialImages("busybox:latest")),
integration.WithMatrix("netmode", map[string]interface{}{
Expand Down Expand Up @@ -1836,9 +1838,17 @@ func testClientGatewayExecFileActionError(t *testing.T, sb integration.Sandbox)
checkAllReleasable(t, c, sb, true)
}

// testClientGatewayContainerSecurityMode ensures that the correct security mode
// testClientGatewayContainerSecurityModeCaps ensures that the correct security mode
// is propagated to the gateway container
func testClientGatewayContainerSecurityMode(t *testing.T, sb integration.Sandbox) {
func testClientGatewayContainerSecurityModeCaps(t *testing.T, sb integration.Sandbox) {
testClientGatewayContainerSecurityMode(t, sb, false)
}

func testClientGatewayContainerSecurityModeValidation(t *testing.T, sb integration.Sandbox) {
testClientGatewayContainerSecurityMode(t, sb, true)
}

func testClientGatewayContainerSecurityMode(t *testing.T, sb integration.Sandbox, expectFail bool) {
integration.CheckFeatureCompat(t, sb, integration.FeatureSecurityMode)
requiresLinux(t)

Expand All @@ -1865,6 +1875,9 @@ func testClientGatewayContainerSecurityMode(t *testing.T, sb integration.Sandbox
require.EqualValues(t, 0xa80425fb, caps)
}
allowedEntitlements = []entitlements.Entitlement{}
if expectFail {
return
}
} else {
assertCaps = func(caps uint64) {
/*
Expand All @@ -1881,6 +1894,9 @@ func testClientGatewayContainerSecurityMode(t *testing.T, sb integration.Sandbox
}
mode = llb.SecurityModeInsecure
allowedEntitlements = []entitlements.Entitlement{entitlements.EntitlementSecurityInsecure}
if expectFail {
allowedEntitlements = []entitlements.Entitlement{}
}
}

b := func(ctx context.Context, c client.Client) (*client.Result, error) {
Expand Down Expand Up @@ -1930,6 +1946,12 @@ func testClientGatewayContainerSecurityMode(t *testing.T, sb integration.Sandbox
t.Logf("Stdout: %q", stdout.String())
t.Logf("Stderr: %q", stderr.String())

if expectFail {
require.Error(t, err)
require.Contains(t, err.Error(), "security.insecure is not allowed")
return nil, err
}

require.NoError(t, err)

capsValue, err := strconv.ParseUint(strings.TrimSpace(stdout.String()), 16, 64)
Expand All @@ -1944,7 +1966,13 @@ func testClientGatewayContainerSecurityMode(t *testing.T, sb integration.Sandbox
AllowedEntitlements: allowedEntitlements,
}
_, err = c.Build(ctx, solveOpts, product, b, nil)
require.NoError(t, err)

if expectFail {
require.Error(t, err)
require.Contains(t, err.Error(), "security.insecure is not allowed")
} else {
require.NoError(t, err)
}

checkAllReleasable(t, c, sb, true)
}
Expand Down Expand Up @@ -2020,7 +2048,15 @@ func testClientGatewayContainerExtraHosts(t *testing.T, sb integration.Sandbox)
checkAllReleasable(t, c, sb, true)
}

func testClientGatewayContainerHostNetworking(t *testing.T, sb integration.Sandbox) {
func testClientGatewayContainerHostNetworkingAccess(t *testing.T, sb integration.Sandbox) {
testClientGatewayContainerHostNetworking(t, sb, false)
}

func testClientGatewayContainerHostNetworkingValidation(t *testing.T, sb integration.Sandbox) {
testClientGatewayContainerHostNetworking(t, sb, true)
}

func testClientGatewayContainerHostNetworking(t *testing.T, sb integration.Sandbox, expectFail bool) {
if os.Getenv("BUILDKIT_RUN_NETWORK_INTEGRATION_TESTS") == "" {
t.SkipNow()
}
Expand All @@ -2041,6 +2077,9 @@ func testClientGatewayContainerHostNetworking(t *testing.T, sb integration.Sandb
if sb.Value("netmode") == hostNetwork {
netMode = pb.NetMode_HOST
allowedEntitlements = []entitlements.Entitlement{entitlements.EntitlementNetworkHost}
if expectFail {
allowedEntitlements = []entitlements.Entitlement{}
}
}
c, err := New(sb.Context(), sb.Address())
require.NoError(t, err)
Expand Down Expand Up @@ -2099,7 +2138,12 @@ func testClientGatewayContainerHostNetworking(t *testing.T, sb integration.Sandb
t.Logf("Stderr: %q", stderr.String())

if netMode == pb.NetMode_HOST {
require.NoError(t, err)
if expectFail {
require.Error(t, err)
require.Contains(t, err.Error(), "network.host is not allowed")
} else {
require.NoError(t, err)
}
} else {
require.Error(t, err)
}
Expand Down
21 changes: 21 additions & 0 deletions solver/llbsolver/bridge.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"github.com/moby/buildkit/sourcepolicy"
spb "github.com/moby/buildkit/sourcepolicy/pb"
"github.com/moby/buildkit/util/bklog"
"github.com/moby/buildkit/util/entitlements"
"github.com/moby/buildkit/util/flightcontrol"
"github.com/moby/buildkit/util/progress"
"github.com/moby/buildkit/worker"
Expand Down Expand Up @@ -165,14 +166,34 @@ func (b *llbBridge) loadResult(ctx context.Context, def *pb.Definition, cacheImp
return res, nil
}

func (b *llbBridge) validateEntitlements(p executor.ProcessInfo) error {
ent, err := loadEntitlements(b.builder)
if err != nil {
return err
}
v := entitlements.Values{
NetworkHost: p.Meta.NetMode == pb.NetMode_HOST,
SecurityInsecure: p.Meta.SecurityMode == pb.SecurityMode_INSECURE,
}
return ent.Check(v)
}

func (b *llbBridge) Run(ctx context.Context, id string, rootfs executor.Mount, mounts []executor.Mount, process executor.ProcessInfo, started chan<- struct{}) (resourcestypes.Recorder, error) {
if err := b.validateEntitlements(process); err != nil {
return nil, err
}

if err := b.loadExecutor(); err != nil {
return nil, err
}
return b.executor.Run(ctx, id, rootfs, mounts, process, started)
}

func (b *llbBridge) Exec(ctx context.Context, id string, process executor.ProcessInfo) error {
if err := b.validateEntitlements(process); err != nil {
return err
}

if err := b.loadExecutor(); err != nil {
return err
}
Expand Down
14 changes: 5 additions & 9 deletions solver/llbsolver/vertex.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,16 +101,12 @@ func ValidateEntitlements(ent entitlements.Set) LoadOpt {
return func(op *pb.Op, _ *pb.OpMetadata, opt *solver.VertexOptions) error {
switch op := op.Op.(type) {
case *pb.Op_Exec:
if op.Exec.Network == pb.NetMode_HOST {
if !ent.Allowed(entitlements.EntitlementNetworkHost) {
return errors.Errorf("%s is not allowed", entitlements.EntitlementNetworkHost)
}
v := entitlements.Values{
NetworkHost: op.Exec.Network == pb.NetMode_HOST,
SecurityInsecure: op.Exec.Security == pb.SecurityMode_INSECURE,
}

if op.Exec.Security == pb.SecurityMode_INSECURE {
if !ent.Allowed(entitlements.EntitlementSecurityInsecure) {
return errors.Errorf("%s is not allowed", entitlements.EntitlementSecurityInsecure)
}
if err := ent.Check(v); err != nil {
return err
}
}
return nil
Expand Down
20 changes: 20 additions & 0 deletions util/entitlements/entitlements.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,3 +58,23 @@ func (s Set) Allowed(e Entitlement) bool {
_, ok := s[e]
return ok
}

func (s Set) Check(v Values) error {
if v.NetworkHost {
if !s.Allowed(EntitlementNetworkHost) {
return errors.Errorf("%s is not allowed", EntitlementNetworkHost)
}
}

if v.SecurityInsecure {
if !s.Allowed(EntitlementSecurityInsecure) {
return errors.Errorf("%s is not allowed", EntitlementSecurityInsecure)
}
}
return nil
}

type Values struct {
NetworkHost bool
SecurityInsecure bool
}

0 comments on commit 92cc595

Please sign in to comment.