Skip to content

Commit 727b254

Browse files
everpeaceeverpeace
committed
fix userstr for dditionalGids on Linux
It should fallback to imageConfig.User when no securityContext.RunAsUser/RunAsUsername Signed-off-by: Shingo Omura <[email protected]>
1 parent 887395a commit 727b254

4 files changed

Lines changed: 178 additions & 6 deletions

File tree

‎pkg/cri/sbserver/container_create_linux.go‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -130,12 +130,14 @@ func (c *criService) containerSpecOpts(config *runtime.ContainerConfig, imageCon
130130
specOpts = append(specOpts, oci.WithUser(userstr))
131131
}
132132

133+
userstr = "0" // runtime default
133134
if securityContext.GetRunAsUsername() != "" {
134135
userstr = securityContext.GetRunAsUsername()
135-
} else {
136-
// Even if RunAsUser is not set, we still call `GetValue` to get uid 0.
137-
// Because it is still useful to get additional gids for uid 0.
136+
} else if securityContext.GetRunAsUser() != nil {
138137
userstr = strconv.FormatInt(securityContext.GetRunAsUser().GetValue(), 10)
138+
} else if imageConfig.User != "" {
139+
parts := strings.Split(imageConfig.User, ":")
140+
userstr = parts[0]
139141
}
140142
specOpts = append(specOpts, customopts.WithAdditionalGIDs(userstr),
141143
customopts.WithSupplementalGroups(securityContext.GetSupplementalGroups()))

‎pkg/cri/sbserver/container_create_linux_test.go‎

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1506,6 +1506,90 @@ func TestGenerateUserString(t *testing.T) {
15061506
}
15071507
}
15081508

1509+
func TestProcessUser(t *testing.T) {
1510+
testID := "test-id"
1511+
testSandboxID := "sandbox-id"
1512+
testContainerName := "container-name"
1513+
testPid := uint32(1234)
1514+
ociRuntime := config.Runtime{}
1515+
c := newTestCRIService()
1516+
testContainer := &containers.Container{ID: "64ddfe361f0099f8d59075398feeb3dcb3863b6851df7b946744755066c03e9d"}
1517+
ctx := context.Background()
1518+
1519+
etcPasswd := `
1520+
root:x:0:0:root:/root:/bin/sh
1521+
alice:x:1000:1000:alice:/home/alice:/bin/sh
1522+
` // #nosec G101
1523+
etcGroup := `
1524+
root:x:0
1525+
alice:x:1000:
1526+
additional-group-for-alice:x:11111:alice
1527+
additional-group-for-root:x:22222:root
1528+
`
1529+
tempRootDir, err := os.MkdirTemp("", "TestContainerUser-")
1530+
require.NoError(t, err)
1531+
if tempRootDir != "" {
1532+
defer os.RemoveAll(tempRootDir)
1533+
}
1534+
require.NoError(t,
1535+
os.MkdirAll(filepath.Join(tempRootDir, "etc"), 0755),
1536+
)
1537+
require.NoError(t,
1538+
os.WriteFile(filepath.Join(tempRootDir, "etc", "passwd"), []byte(etcPasswd), 0644),
1539+
)
1540+
require.NoError(t,
1541+
os.WriteFile(filepath.Join(tempRootDir, "etc", "group"), []byte(etcGroup), 0644),
1542+
)
1543+
1544+
for desc, test := range map[string]struct {
1545+
imageConfigUser string
1546+
securityContext *runtime.LinuxContainerSecurityContext
1547+
expected runtimespec.User
1548+
}{
1549+
"Only SecurityContext was set, SecurityContext defines User": {
1550+
securityContext: &runtime.LinuxContainerSecurityContext{
1551+
RunAsUser: &runtime.Int64Value{Value: 1000},
1552+
RunAsGroup: &runtime.Int64Value{Value: 2000},
1553+
SupplementalGroups: []int64{3333},
1554+
},
1555+
expected: runtimespec.User{UID: 1000, GID: 2000, AdditionalGids: []uint32{2000, 3333, 11111}},
1556+
},
1557+
"Only imageConfig.User was set, imageConfig.User defines User": {
1558+
imageConfigUser: "1000",
1559+
securityContext: nil,
1560+
expected: runtimespec.User{UID: 1000, GID: 1000, AdditionalGids: []uint32{1000, 11111}},
1561+
},
1562+
"Both SecurityContext and ImageConfig.User was set, SecurityContext defines User": {
1563+
imageConfigUser: "0",
1564+
securityContext: &runtime.LinuxContainerSecurityContext{
1565+
RunAsUser: &runtime.Int64Value{Value: 1000},
1566+
RunAsGroup: &runtime.Int64Value{Value: 2000},
1567+
SupplementalGroups: []int64{3333},
1568+
},
1569+
expected: runtimespec.User{UID: 1000, GID: 2000, AdditionalGids: []uint32{2000, 3333, 11111}},
1570+
},
1571+
"No SecurityContext nor ImageConfig.User were set, runtime default defines User": {
1572+
expected: runtimespec.User{UID: 0, GID: 0, AdditionalGids: []uint32{0, 22222}},
1573+
},
1574+
} {
1575+
t.Run(desc, func(t *testing.T) {
1576+
containerConfig, sandboxConfig, imageConfig, _ := getCreateContainerTestData()
1577+
containerConfig.Linux.SecurityContext = test.securityContext
1578+
imageConfig.User = test.imageConfigUser
1579+
1580+
spec, err := c.buildContainerSpec(currentPlatform, testID, testSandboxID, testPid, "", testContainerName, testImageName, containerConfig, sandboxConfig, imageConfig, nil, ociRuntime)
1581+
require.NoError(t, err)
1582+
1583+
spec.Root.Path = tempRootDir // simulating /etc/{passwd, group}
1584+
opts, err := c.containerSpecOpts(containerConfig, imageConfig)
1585+
require.NoError(t, err)
1586+
oci.ApplyOpts(ctx, nil, testContainer, spec, opts...)
1587+
1588+
require.Equal(t, test.expected, spec.Process.User)
1589+
})
1590+
}
1591+
}
1592+
15091593
func TestNonRootUserAndDevices(t *testing.T) {
15101594
testPid := uint32(1234)
15111595
c := newTestCRIService()

‎pkg/cri/server/container_create_linux.go‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -366,12 +366,14 @@ func (c *criService) containerSpecOpts(config *runtime.ContainerConfig, imageCon
366366
specOpts = append(specOpts, oci.WithUser(userstr))
367367
}
368368

369+
userstr = "0" // runtime default
369370
if securityContext.GetRunAsUsername() != "" {
370371
userstr = securityContext.GetRunAsUsername()
371-
} else {
372-
// Even if RunAsUser is not set, we still call `GetValue` to get uid 0.
373-
// Because it is still useful to get additional gids for uid 0.
372+
} else if securityContext.GetRunAsUser() != nil {
374373
userstr = strconv.FormatInt(securityContext.GetRunAsUser().GetValue(), 10)
374+
} else if imageConfig.User != "" {
375+
parts := strings.Split(imageConfig.User, ":")
376+
userstr = parts[0]
375377
}
376378
specOpts = append(specOpts, customopts.WithAdditionalGIDs(userstr),
377379
customopts.WithSupplementalGroups(securityContext.GetSupplementalGroups()))

‎pkg/cri/server/container_create_linux_test.go‎

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1506,6 +1506,90 @@ func TestGenerateUserString(t *testing.T) {
15061506
}
15071507
}
15081508

1509+
func TestProcessUser(t *testing.T) {
1510+
testID := "test-id"
1511+
testSandboxID := "sandbox-id"
1512+
testContainerName := "container-name"
1513+
testPid := uint32(1234)
1514+
ociRuntime := config.Runtime{}
1515+
c := newTestCRIService()
1516+
testContainer := &containers.Container{ID: "64ddfe361f0099f8d59075398feeb3dcb3863b6851df7b946744755066c03e9d"}
1517+
ctx := context.Background()
1518+
1519+
etcPasswd := `
1520+
root:x:0:0:root:/root:/bin/sh
1521+
alice:x:1000:1000:alice:/home/alice:/bin/sh
1522+
` // #nosec G101
1523+
etcGroup := `
1524+
root:x:0
1525+
alice:x:1000:
1526+
additional-group-for-alice:x:11111:alice
1527+
additional-group-for-root:x:22222:root
1528+
`
1529+
tempRootDir, err := os.MkdirTemp("", "TestContainerUser-")
1530+
require.NoError(t, err)
1531+
if tempRootDir != "" {
1532+
defer os.RemoveAll(tempRootDir)
1533+
}
1534+
require.NoError(t,
1535+
os.MkdirAll(filepath.Join(tempRootDir, "etc"), 0755),
1536+
)
1537+
require.NoError(t,
1538+
os.WriteFile(filepath.Join(tempRootDir, "etc", "passwd"), []byte(etcPasswd), 0644),
1539+
)
1540+
require.NoError(t,
1541+
os.WriteFile(filepath.Join(tempRootDir, "etc", "group"), []byte(etcGroup), 0644),
1542+
)
1543+
1544+
for desc, test := range map[string]struct {
1545+
imageConfigUser string
1546+
securityContext *runtime.LinuxContainerSecurityContext
1547+
expected runtimespec.User
1548+
}{
1549+
"Only SecurityContext was set, SecurityContext defines User": {
1550+
securityContext: &runtime.LinuxContainerSecurityContext{
1551+
RunAsUser: &runtime.Int64Value{Value: 1000},
1552+
RunAsGroup: &runtime.Int64Value{Value: 2000},
1553+
SupplementalGroups: []int64{3333},
1554+
},
1555+
expected: runtimespec.User{UID: 1000, GID: 2000, AdditionalGids: []uint32{2000, 3333, 11111}},
1556+
},
1557+
"Only imageConfig.User was set, imageConfig.User defines User": {
1558+
imageConfigUser: "1000",
1559+
securityContext: nil,
1560+
expected: runtimespec.User{UID: 1000, GID: 1000, AdditionalGids: []uint32{1000, 11111}},
1561+
},
1562+
"Both SecurityContext and ImageConfig.User was set, SecurityContext defines User": {
1563+
imageConfigUser: "0",
1564+
securityContext: &runtime.LinuxContainerSecurityContext{
1565+
RunAsUser: &runtime.Int64Value{Value: 1000},
1566+
RunAsGroup: &runtime.Int64Value{Value: 2000},
1567+
SupplementalGroups: []int64{3333},
1568+
},
1569+
expected: runtimespec.User{UID: 1000, GID: 2000, AdditionalGids: []uint32{2000, 3333, 11111}},
1570+
},
1571+
"No SecurityContext nor ImageConfig.User were set, runtime default defines User": {
1572+
expected: runtimespec.User{UID: 0, GID: 0, AdditionalGids: []uint32{0, 22222}},
1573+
},
1574+
} {
1575+
t.Run(desc, func(t *testing.T) {
1576+
containerConfig, sandboxConfig, imageConfig, _ := getCreateContainerTestData()
1577+
containerConfig.Linux.SecurityContext = test.securityContext
1578+
imageConfig.User = test.imageConfigUser
1579+
1580+
spec, err := c.containerSpec(testID, testSandboxID, testPid, "", testContainerName, testImageName, containerConfig, sandboxConfig, imageConfig, nil, ociRuntime)
1581+
require.NoError(t, err)
1582+
1583+
spec.Root.Path = tempRootDir // simulating /etc/{passwd, group}
1584+
opts, err := c.containerSpecOpts(containerConfig, imageConfig)
1585+
require.NoError(t, err)
1586+
oci.ApplyOpts(ctx, nil, testContainer, spec, opts...)
1587+
1588+
require.Equal(t, test.expected, spec.Process.User)
1589+
})
1590+
}
1591+
}
1592+
15091593
func TestNonRootUserAndDevices(t *testing.T) {
15101594
testPid := uint32(1234)
15111595
c := newTestCRIService()

0 commit comments

Comments
 (0)