Skip to content

Commit 7084bd2

Browse files
authored
rego policy enforcer should use the same user parsing logic as GCS (#2405)
This PR fixes a discrepancy between user info handling between GCS and rego policy enforcer. For example, GCS doesn't require the user/group to exist in container's /etc/passwd and /etc/group and has a fallback to UID and GID 0, when the user is absent. Rego enforcer's `GetUserInfo`, however, always tries to lookup user/group in /etc/passwd and /etc/group and returns an error when the UID doesn't exist. This behavior is inconsistent with non confidential LCOW workloads and fixed in this PR. To avoid circular imports, the spec.go and spec_devices.go under `internal/guest/runtime/hcsv2` have been moved under `internal/guest/spec` and the dependent code updated accordingly. As a result a bunch of methods are now exported, but still under `internal`, so this shouldn't cause problems. User parsing has been updated and split into `ParseUserStr`, which returns UID and GID for a given `username` string and `SetUserStr`, which just sets the UID and GID for the OCI process. Rego enforcer's `GetUserInfo` now prioritizes the result of `ParseUserStr` and fallbacks to the previous behavior of UID/GID lookup in container's filesystem. Signed-off-by: Maksim An <maksiman@microsoft.com>
1 parent a5c5b4c commit 7084bd2

File tree

16 files changed

+413
-402
lines changed

16 files changed

+413
-402
lines changed

go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ require (
5757
github.com/cpuguy83/go-md2man/v2 v2.0.4 // indirect
5858
github.com/decred/dcrd/dcrec/secp256k1/v4 v4.2.0 // indirect
5959
github.com/docker/cli v24.0.0+incompatible // indirect
60-
github.com/docker/distribution v2.8.2+incompatible // indirect
60+
github.com/docker/distribution v2.8.3+incompatible // indirect
6161
github.com/docker/docker v27.3.1+incompatible // indirect
6262
github.com/docker/docker-credential-helpers v0.7.0 // indirect
6363
github.com/docker/go-events v0.0.0-20190806004212-e31b211e4f1c // indirect

go.sum

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,8 +79,8 @@ github.com/dgryski/trifles v0.0.0-20230903005119-f50d829f2e54 h1:SG7nF6SRlWhcT7c
7979
github.com/dgryski/trifles v0.0.0-20230903005119-f50d829f2e54/go.mod h1:if7Fbed8SFyPtHLHbg49SI7NAdJiC5WIA09pe59rfAA=
8080
github.com/docker/cli v24.0.0+incompatible h1:0+1VshNwBQzQAx9lOl+OYCTCEAD8fKs/qeXMx3O0wqM=
8181
github.com/docker/cli v24.0.0+incompatible/go.mod h1:JLrzqnKDaYBop7H2jaqPtU4hHvMKP+vjCwu2uszcLI8=
82-
github.com/docker/distribution v2.8.2+incompatible h1:T3de5rq0dB1j30rp0sA2rER+m322EBzniBPB6ZIzuh8=
83-
github.com/docker/distribution v2.8.2+incompatible/go.mod h1:J2gT2udsDAN96Uj4KfcMRqY0/ypR+oyYUYmja8H+y+w=
82+
github.com/docker/distribution v2.8.3+incompatible h1:AtKxIZ36LoNK51+Z6RpzLpddBirtxJnzDrHLEKxTAYk=
83+
github.com/docker/distribution v2.8.3+incompatible/go.mod h1:J2gT2udsDAN96Uj4KfcMRqY0/ypR+oyYUYmja8H+y+w=
8484
github.com/docker/docker v27.3.1+incompatible h1:KttF0XoteNTicmUtBO0L2tP+J7FGRFTjaEF4k6WdhfI=
8585
github.com/docker/docker v27.3.1+incompatible/go.mod h1:eEKB0N0r5NX/I1kEveEz05bcu8tLC/8azJZsviup8Sk=
8686
github.com/docker/docker-credential-helpers v0.7.0 h1:xtCHsjxogADNZcdv1pKUHXryefjlVRqWqIhk/uXJp0A=

internal/guest/policy/default.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,15 +6,15 @@ package policy
66
import (
77
oci "github.com/opencontainers/runtime-spec/specs-go"
88

9-
internalSpec "github.com/Microsoft/hcsshim/internal/guest/spec"
9+
specGuest "github.com/Microsoft/hcsshim/internal/guest/spec"
1010
"github.com/Microsoft/hcsshim/pkg/securitypolicy"
1111
)
1212

1313
func ExtendPolicyWithNetworkingMounts(sandboxID string, enforcer securitypolicy.SecurityPolicyEnforcer, spec *oci.Spec) error {
1414
roSpec := &oci.Spec{
1515
Root: spec.Root,
1616
}
17-
networkingMounts := internalSpec.GenerateWorkloadContainerNetworkMounts(sandboxID, roSpec)
17+
networkingMounts := specGuest.GenerateWorkloadContainerNetworkMounts(sandboxID, roSpec)
1818
if err := enforcer.ExtendDefaultMounts(networkingMounts); err != nil {
1919
return err
2020
}

internal/guest/runtime/hcsv2/container.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ import (
2121
"github.com/Microsoft/hcsshim/internal/guest/gcserr"
2222
"github.com/Microsoft/hcsshim/internal/guest/prot"
2323
"github.com/Microsoft/hcsshim/internal/guest/runtime"
24-
specInternal "github.com/Microsoft/hcsshim/internal/guest/spec"
24+
specGuest "github.com/Microsoft/hcsshim/internal/guest/spec"
2525
"github.com/Microsoft/hcsshim/internal/guest/stdio"
2626
"github.com/Microsoft/hcsshim/internal/guest/storage"
2727
"github.com/Microsoft/hcsshim/internal/guest/transport"
@@ -115,7 +115,7 @@ func (c *Container) ExecProcess(ctx context.Context, process *oci.Process, conSe
115115
// assign the uid:gid from the container.
116116
if process.User.Username != "" {
117117
// The exec provided a user string of it's own. Grab the uid:gid pairing for the string (if one exists).
118-
if err := setUserStr(&oci.Spec{Root: c.spec.Root, Process: process}, process.User.Username); err != nil {
118+
if err := specGuest.SetUserStr(&oci.Spec{Root: c.spec.Root, Process: process}, process.User.Username); err != nil {
119119
return -1, err
120120
}
121121
// Runc doesn't care about this, and just to be safe clear it.
@@ -194,12 +194,12 @@ func (c *Container) Delete(ctx context.Context) error {
194194
entity.Info("opengcs::Container::Delete")
195195
if c.isSandbox {
196196
// remove user mounts in sandbox container
197-
if err := storage.UnmountAllInPath(ctx, specInternal.SandboxMountsDir(c.id), true); err != nil {
197+
if err := storage.UnmountAllInPath(ctx, specGuest.SandboxMountsDir(c.id), true); err != nil {
198198
entity.WithError(err).Error("failed to unmount sandbox mounts")
199199
}
200200

201201
// remove hugepages mounts in sandbox container
202-
if err := storage.UnmountAllInPath(ctx, specInternal.HugePagesMountsDir(c.id), true); err != nil {
202+
if err := storage.UnmountAllInPath(ctx, specGuest.HugePagesMountsDir(c.id), true); err != nil {
203203
entity.WithError(err).Error("failed to unmount hugepages mounts")
204204
}
205205
}

internal/guest/runtime/hcsv2/sandbox_container.go

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,21 +14,21 @@ import (
1414
"go.opencensus.io/trace"
1515

1616
"github.com/Microsoft/hcsshim/internal/guest/network"
17-
specInternal "github.com/Microsoft/hcsshim/internal/guest/spec"
17+
specGuest "github.com/Microsoft/hcsshim/internal/guest/spec"
1818
"github.com/Microsoft/hcsshim/internal/oc"
1919
"github.com/Microsoft/hcsshim/pkg/annotations"
2020
)
2121

2222
func getSandboxHostnamePath(id string) string {
23-
return filepath.Join(specInternal.SandboxRootDir(id), "hostname")
23+
return filepath.Join(specGuest.SandboxRootDir(id), "hostname")
2424
}
2525

2626
func getSandboxHostsPath(id string) string {
27-
return filepath.Join(specInternal.SandboxRootDir(id), "hosts")
27+
return filepath.Join(specGuest.SandboxRootDir(id), "hosts")
2828
}
2929

3030
func getSandboxResolvPath(id string) string {
31-
return filepath.Join(specInternal.SandboxRootDir(id), "resolv.conf")
31+
return filepath.Join(specGuest.SandboxRootDir(id), "resolv.conf")
3232
}
3333

3434
func setupSandboxContainerSpec(ctx context.Context, id string, spec *oci.Spec) (err error) {
@@ -38,7 +38,7 @@ func setupSandboxContainerSpec(ctx context.Context, id string, spec *oci.Spec) (
3838
span.AddAttributes(trace.StringAttribute("cid", id))
3939

4040
// Generate the sandbox root dir
41-
rootDir := specInternal.SandboxRootDir(id)
41+
rootDir := specGuest.SandboxRootDir(id)
4242
if err := os.MkdirAll(rootDir, 0755); err != nil {
4343
return errors.Wrapf(err, "failed to create sandbox root directory %q", rootDir)
4444
}
@@ -71,7 +71,7 @@ func setupSandboxContainerSpec(ctx context.Context, id string, spec *oci.Spec) (
7171
}
7272

7373
// Write resolv.conf
74-
ns, err := getNetworkNamespace(getNetworkNamespaceID(spec))
74+
ns, err := getNetworkNamespace(specGuest.GetNetworkNamespaceID(spec))
7575
if err != nil {
7676
return err
7777
}
@@ -98,13 +98,13 @@ func setupSandboxContainerSpec(ctx context.Context, id string, spec *oci.Spec) (
9898
// guest. The username field is used as a temporary holding place until we can perform this work here when
9999
// we actually have the rootfs to inspect.
100100
if spec.Process.User.Username != "" {
101-
if err := setUserStr(spec, spec.Process.User.Username); err != nil {
101+
if err := specGuest.SetUserStr(spec, spec.Process.User.Username); err != nil {
102102
return err
103103
}
104104
}
105105

106106
if rlimCore := spec.Annotations[annotations.RLimitCore]; rlimCore != "" {
107-
if err := setCoreRLimit(spec, rlimCore); err != nil {
107+
if err := specGuest.SetCoreRLimit(spec, rlimCore); err != nil {
108108
return err
109109
}
110110
}

0 commit comments

Comments
 (0)