|
| 1 | +From 9639b9625554183d0c4d8d072dccb84fedd2320f Mon Sep 17 00:00:00 2001 |
| 2 | +From: Craig Ingram <Cjingram@google.com> |
| 3 | +Date: Fri, 7 Mar 2025 13:27:58 +0000 |
| 4 | +Subject: [PATCH] validate uid/gid |
| 5 | + |
| 6 | +--- |
| 7 | + oci/spec_opts.go | 24 ++++++++-- |
| 8 | + oci/spec_opts_linux_test.go | 92 +++++++++++++++++++++++++++++++++++++ |
| 9 | + 2 files changed, 112 insertions(+), 4 deletions(-) |
| 10 | + |
| 11 | +diff --git a/oci/spec_opts.go b/oci/spec_opts.go |
| 12 | +index 8acb38f43b11..d2c08e0b012d 100644 |
| 13 | +--- a/oci/spec_opts.go |
| 14 | ++++ b/oci/spec_opts.go |
| 15 | +@@ -22,6 +22,7 @@ import ( |
| 16 | + "encoding/json" |
| 17 | + "errors" |
| 18 | + "fmt" |
| 19 | ++ "math" |
| 20 | + "os" |
| 21 | + "path/filepath" |
| 22 | + "runtime" |
| 23 | +@@ -576,6 +577,20 @@ func WithUser(userstr string) SpecOpts { |
| 24 | + defer ensureAdditionalGids(s) |
| 25 | + setProcess(s) |
| 26 | + s.Process.User.AdditionalGids = nil |
| 27 | ++ // While the Linux kernel allows the max UID to be MaxUint32 - 2, |
| 28 | ++ // and the OCI Runtime Spec has no definition about the max UID, |
| 29 | ++ // the runc implementation is known to require the UID to be <= MaxInt32. |
| 30 | ++ // |
| 31 | ++ // containerd follows runc's limitation here. |
| 32 | ++ // |
| 33 | ++ // In future we may relax this limitation to allow MaxUint32 - 2, |
| 34 | ++ // or, amend the OCI Runtime Spec to codify the implementation limitation. |
| 35 | ++ const ( |
| 36 | ++ minUserID = 0 |
| 37 | ++ maxUserID = math.MaxInt32 |
| 38 | ++ minGroupID = 0 |
| 39 | ++ maxGroupID = math.MaxInt32 |
| 40 | ++ ) |
| 41 | + |
| 42 | + // For LCOW it's a bit harder to confirm that the user actually exists on the host as a rootfs isn't |
| 43 | + // mounted on the host and shared into the guest, but rather the rootfs is constructed entirely in the |
| 44 | +@@ -592,8 +607,8 @@ func WithUser(userstr string) SpecOpts { |
| 45 | + switch len(parts) { |
| 46 | + case 1: |
| 47 | + v, err := strconv.Atoi(parts[0]) |
| 48 | +- if err != nil { |
| 49 | +- // if we cannot parse as a uint they try to see if it is a username |
| 50 | ++ if err != nil || v < minUserID || v > maxUserID { |
| 51 | ++ // if we cannot parse as an int32 then try to see if it is a username |
| 52 | + return WithUsername(userstr)(ctx, client, c, s) |
| 53 | + } |
| 54 | + return WithUserID(uint32(v))(ctx, client, c, s) |
| 55 | +@@ -604,12 +619,13 @@ func WithUser(userstr string) SpecOpts { |
| 56 | + ) |
| 57 | + var uid, gid uint32 |
| 58 | + v, err := strconv.Atoi(parts[0]) |
| 59 | +- if err != nil { |
| 60 | ++ if err != nil || v < minUserID || v > maxUserID { |
| 61 | + username = parts[0] |
| 62 | + } else { |
| 63 | + uid = uint32(v) |
| 64 | + } |
| 65 | +- if v, err = strconv.Atoi(parts[1]); err != nil { |
| 66 | ++ v, err = strconv.Atoi(parts[1]) |
| 67 | ++ if err != nil || v < minGroupID || v > maxGroupID { |
| 68 | + groupname = parts[1] |
| 69 | + } else { |
| 70 | + gid = uint32(v) |
| 71 | +diff --git a/oci/spec_opts_linux_test.go b/oci/spec_opts_linux_test.go |
| 72 | +index f4acf573ba46..c3f436e85f1a 100644 |
| 73 | +--- a/oci/spec_opts_linux_test.go |
| 74 | ++++ b/oci/spec_opts_linux_test.go |
| 75 | +@@ -32,6 +32,98 @@ import ( |
| 76 | + "golang.org/x/sys/unix" |
| 77 | + ) |
| 78 | + |
| 79 | ++//nolint:gosec |
| 80 | ++func TestWithUser(t *testing.T) { |
| 81 | ++ t.Parallel() |
| 82 | ++ |
| 83 | ++ expectedPasswd := `root:x:0:0:root:/root:/bin/ash |
| 84 | ++guest:x:405:100:guest:/dev/null:/sbin/nologin |
| 85 | ++` |
| 86 | ++ expectedGroup := `root:x:0:root |
| 87 | ++bin:x:1:root,bin,daemon |
| 88 | ++daemon:x:2:root,bin,daemon |
| 89 | ++sys:x:3:root,bin,adm |
| 90 | ++guest:x:100:guest |
| 91 | ++` |
| 92 | ++ td := t.TempDir() |
| 93 | ++ apply := fstest.Apply( |
| 94 | ++ fstest.CreateDir("/etc", 0777), |
| 95 | ++ fstest.CreateFile("/etc/passwd", []byte(expectedPasswd), 0777), |
| 96 | ++ fstest.CreateFile("/etc/group", []byte(expectedGroup), 0777), |
| 97 | ++ ) |
| 98 | ++ if err := apply.Apply(td); err != nil { |
| 99 | ++ t.Fatalf("failed to apply: %v", err) |
| 100 | ++ } |
| 101 | ++ c := containers.Container{ID: t.Name()} |
| 102 | ++ testCases := []struct { |
| 103 | ++ user string |
| 104 | ++ expectedUID uint32 |
| 105 | ++ expectedGID uint32 |
| 106 | ++ err string |
| 107 | ++ }{ |
| 108 | ++ { |
| 109 | ++ user: "0", |
| 110 | ++ expectedUID: 0, |
| 111 | ++ expectedGID: 0, |
| 112 | ++ }, |
| 113 | ++ { |
| 114 | ++ user: "root:root", |
| 115 | ++ expectedUID: 0, |
| 116 | ++ expectedGID: 0, |
| 117 | ++ }, |
| 118 | ++ { |
| 119 | ++ user: "guest", |
| 120 | ++ expectedUID: 405, |
| 121 | ++ expectedGID: 100, |
| 122 | ++ }, |
| 123 | ++ { |
| 124 | ++ user: "guest:guest", |
| 125 | ++ expectedUID: 405, |
| 126 | ++ expectedGID: 100, |
| 127 | ++ }, |
| 128 | ++ { |
| 129 | ++ user: "guest:nobody", |
| 130 | ++ err: "no groups found", |
| 131 | ++ }, |
| 132 | ++ { |
| 133 | ++ user: "405:100", |
| 134 | ++ expectedUID: 405, |
| 135 | ++ expectedGID: 100, |
| 136 | ++ }, |
| 137 | ++ { |
| 138 | ++ user: "405:2147483648", |
| 139 | ++ err: "no groups found", |
| 140 | ++ }, |
| 141 | ++ { |
| 142 | ++ user: "-1000", |
| 143 | ++ err: "no users found", |
| 144 | ++ }, |
| 145 | ++ { |
| 146 | ++ user: "2147483648", |
| 147 | ++ err: "no users found", |
| 148 | ++ }, |
| 149 | ++ } |
| 150 | ++ for _, testCase := range testCases { |
| 151 | ++ testCase := testCase |
| 152 | ++ t.Run(testCase.user, func(t *testing.T) { |
| 153 | ++ t.Parallel() |
| 154 | ++ s := Spec{ |
| 155 | ++ Version: specs.Version, |
| 156 | ++ Root: &specs.Root{ |
| 157 | ++ Path: td, |
| 158 | ++ }, |
| 159 | ++ Linux: &specs.Linux{}, |
| 160 | ++ } |
| 161 | ++ err := WithUser(testCase.user)(context.Background(), nil, &c, &s) |
| 162 | ++ if err != nil { |
| 163 | ++ assert.EqualError(t, err, testCase.err) |
| 164 | ++ } |
| 165 | ++ assert.Equal(t, testCase.expectedUID, s.Process.User.UID) |
| 166 | ++ assert.Equal(t, testCase.expectedGID, s.Process.User.GID) |
| 167 | ++ }) |
| 168 | ++ } |
| 169 | ++} |
| 170 | ++ |
| 171 | + //nolint:gosec |
| 172 | + func TestWithUserID(t *testing.T) { |
| 173 | + t.Parallel() |
0 commit comments