8000 Merge commit from fork · containerd/containerd@05044ec · GitHub
[go: up one dir, main page]

Skip to content

Commit 05044ec

Browse files
authored
Merge commit from fork
[release 1.7] validate uid/gid
2 parents 0b7f2a5 + 11504c3 commit 05044ec

File tree

2 files changed

+112
-4
lines changed

2 files changed

+112
-4
lines changed

oci/spec_opts.go

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222
"encoding/json"
2323
"errors"
2424
"fmt"
25+
"math"
2526
"os"
2627
"path/filepath"
2728
"runtime"
@@ -594,6 +595,20 @@ func WithUser(userstr string) SpecOpts {
594595
defer ensureAdditionalGids(s)
595596
setProcess(s)
596597
s.Process.User.AdditionalGids = nil
598+
// While the Linux kernel allows the max UID to be MaxUint32 - 2,
599+
// and the OCI Runtime Spec has no definition about the max UID,
600+
// the runc implementation is known to require the UID to be <= MaxInt32.
601+
//
602+
// containerd follows runc's limitation here.
603+
//
604+
// In future we may relax this limitation to allow MaxUint32 - 2,
605+
// or, amend the OCI Runtime Spec to codify the implementation limitation.
606+
const (
607+
minUserID = 0
608+
maxUserID = math.MaxInt32
609+
minGroupID = 0
610+
maxGroupID = math.MaxInt32
611+
)
597612

598613
// For LCOW it's a bit harder to confirm that the user actually exists on the host as a rootfs isn't
599614
// mounted on the host and shared into the guest, but rather the rootfs is constructed entirely in the
@@ -612,8 +627,8 @@ func WithUser(userstr string) SpecOpts {
612627
switch len(parts) {
613628
case 1:
614629
v, err := strconv.Atoi(parts[0])
615-
if err != nil {
616-
// if we cannot parse as a uint they try to see if it is a username
630+
if err != nil || v < minUserID || v > maxUserID {
631+
// if we cannot parse as an int32 then try to see if it is a username
617632
return WithUsername(userstr)(ctx, client, c, s)
618633
}
619634
return WithUserID(uint32(v))(ctx, client, c, s)
@@ -624,12 +639,13 @@ func WithUser(userstr string) SpecOpts {
624639
)
625640
var uid, gid uint32
626641
v, err := strconv.Atoi(parts[0])
627-
if err != nil {
642+
if err != nil || v < minUserID || v > maxUserID {
628643
username = parts[0]
629644
} else {
630645
uid = uint32(v)
631646
}
632-
if v, err = strconv.Atoi(parts[1]); err != nil {
647+
v, err = strconv.Atoi(parts[1])
648+
if err != nil || v < minGroupID || v > maxGroupID {
633649
groupname = parts[1]
634650
} else {
635651
gid = uint32(v)

oci/spec_opts_linux_test.go

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,98 @@ import (
3333
"golang.org/x/sys/unix"
3434
)
3535

36+
//nolint:gosec
37+
func TestWithUser(t *testing.T) {
38+
t.Parallel()
39+
40+
expectedPasswd := `root:x:0:0:root:/root:/bin/ash
41+
guest:x:405:100:guest:/dev/null:/sbin/nologin
42+
`
43+
expectedGroup := `root:x:0:root
44+
bin:x:1:root,bin,daemon
45+
daemon:x:2:root,bin,daemon
46+
sys:x:3:root,bin,adm
47+
guest:x:100:guest
48+
`
49+
td := t.TempDir()
50+
apply := fstest.Apply(
51+
fstest.CreateDir("/etc", 0777),
52+
fstest.CreateFile("/etc/passwd", []byte(expectedPasswd), 0777),
53+
fstest.CreateFile("/etc/group", []byte(expectedGroup), 0777),
54+
)
55+
if err := apply.Apply(td); err != nil {
56+
t.Fatalf("failed to apply: %v", err)
57+
}
58+
c := containers.Container{ID: t.Name()}
59+
testCases := []struct {
60+
user string
61+
expectedUID uint32
62+
expectedGID uint32
63+
err string
64+
}{
65+
{
66+
user: "0",
67+
expectedUID: 0,
68+
expectedGID: 0,
69+
},
70+
{
71+
user: "root:root",
72+
expectedUID: 0,
73+
expectedGID: 0,
74+
},
75+
{
76+
user: "guest",
77+
expectedUID: 405,
78+
expectedGID: 100,
79+
},
80+
{
81+
user: "guest:guest",
82+
expectedUID: 405,
83+
expectedGID: 100,
84+
},
85+
{
86+
user: "guest:nobody",
87+
err: "no groups found",
88+
},
89+
{
90+
user: "405:100",
91+
expectedUID: 405,
92+
expectedGID: 100,
93+
},
94+
{
95+
user: "405:2147483648",
96+
err: "no groups found",
97+
},
98+
{
99+
user: "-1000",
100+
err: "no users found",
101+
},
102+
{
103+
user: "2147483648",
104+
err: "no users found",
105+
},
106+
}
107+
for _, testCase := range testCases {
108+
testCase := testCase
109+
t.Run(testCase.user, func(t *testing.T) {
110+
t.Parallel()
111+
s := Spec{
112+
Version: specs.Version,
113+
Root: &specs.Root{
114+
Path: td,
115+
},
116+
Linux: &specs.Linux{},
117+
}
118+
err := WithUser(testCase.user)(context.Background(), nil, &c, &s)
119+
if err != nil {
120+
assert.EqualError(t, err, testCase.err)
121+
}
122+
assert.Equal(t, testCase.expectedUID, s.Process.User.UID)
123+
assert.Equal(t, testCase.expectedGID, s.Process.User.GID)
124+
})
125+
}
126+
}
127+
36128
//nolint:gosec
37129
func TestWithUserID(t *testing.T) {
38130
t.Parallel()

0 commit comments

Comments
 (0)
0