Skip to content

Commit 1a43cb6

Browse files
dmcgowandmcgowan
authored
Merge commit from fork
[release 2.0] validate uid/gid
2 parents bf82ae2 + 07a0b54 commit 1a43cb6

2 files changed

Lines changed: 112 additions & 4 deletions

File tree

‎pkg/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"
@@ -593,6 +594,20 @@ func WithUser(userstr string) SpecOpts {
593594
defer ensureAdditionalGids(s)
594595
setProcess(s)
595596
s.Process.User.AdditionalGids = nil
597+
// While the Linux kernel allows the max UID to be MaxUint32 - 2,
598+
// and the OCI Runtime Spec has no definition about the max UID,
599+
// the runc implementation is known to require the UID to be <= MaxInt32.
600+
//
601+
// containerd follows runc's limitation here.
602+
//
603+
// In future we may relax this limitation to allow MaxUint32 - 2,
604+
// or, amend the OCI Runtime Spec to codify the implementation limitation.
605+
const (
606+
minUserID = 0
607+
maxUserID = math.MaxInt32
608+
minGroupID = 0
609+
maxGroupID = math.MaxInt32
610+
)
596611

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

‎pkg/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)