Skip to content

Commit eace671

Browse files
everpeacethaJeztah
everpeace
authored and
thaJeztah
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]> (cherry picked from commit 727b254) Signed-off-by: Sebastiaan van Stijn <[email protected]>
1 parent fd0566c commit eace671

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
@@ -73,12 +73,14 @@ func (c *criService) containerSpecOpts(config *runtime.ContainerConfig, imageCon
7373
specOpts = append(specOpts, oci.WithUser(userstr))
7474
}
7575

76+
userstr = "0" // runtime default
7677
if securityContext.GetRunAsUsername() != "" {
7778
userstr = securityContext.GetRunAsUsername()
78-
} else {
79-
// Even if RunAsUser is not set, we still call `GetValue` to get uid 0.
80-
// Because it is still useful to get additional gids for uid 0.
79+
} else if securityContext.GetRunAsUser() != nil {
8180
userstr = strconv.FormatInt(securityContext.GetRunAsUser().GetValue(), 10)
81+
} else if imageConfig.User != "" {
82+
parts := strings.Split(imageConfig.User, ":")
83+
userstr = parts[0]
8284
}
8385
specOpts = append(specOpts, customopts.WithAdditionalGIDs(userstr),
8486
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
@@ -1326,6 +1326,90 @@ func TestGenerateUserString(t *testing.T) {
13261326
}
13271327
}
13281328

1329+
func TestProcessUser(t *testing.T) {
1330+
testID := "test-id"
1331+
testSandboxID := "sandbox-id"
1332+
testContainerName := "container-name"
1333+
testPid := uint32(1234)
1334+
ociRuntime := config.Runtime{}
1335+
c := newTestCRIService()
1336+
testContainer := &containers.Container{ID: "64ddfe361f0099f8d59075398feeb3dcb3863b6851df7b946744755066c03e9d"}
1337+
ctx := context.Background()
1338+
1339+
etcPasswd := `
1340+
root:x:0:0:root:/root:/bin/sh
1341+
alice:x:1000:1000:alice:/home/alice:/bin/sh
1342+
` // #nosec G101
1343+
etcGroup := `
1344+
root:x:0
1345+
alice:x:1000:
1346+
additional-group-for-alice:x:11111:alice
1347+
additional-group-for-root:x:22222:root
1348+
`
1349+
tempRootDir, err := os.MkdirTemp("", "TestContainerUser-")
1350+
require.NoError(t, err)
1351+
if tempRootDir != "" {
1352+
defer os.RemoveAll(tempRootDir)
1353+
}
1354+
require.NoError(t,
1355+
os.MkdirAll(filepath.Join(tempRootDir, "etc"), 0755),
1356+
)
1357+
require.NoError(t,
1358+
os.WriteFile(filepath.Join(tempRootDir, "etc", "passwd"), []byte(etcPasswd), 0644),
1359+
)
1360+
require.NoError(t,
1361+
os.WriteFile(filepath.Join(tempRootDir, "etc", "group"), []byte(etcGroup), 0644),
1362+
)
1363+
1364+
for desc, test := range map[string]struct {
1365+
imageConfigUser string
1366+
securityContext *runtime.LinuxContainerSecurityContext
1367+
expected runtimespec.User
1368+
}{
1369+
"Only SecurityContext was set, SecurityContext defines User": {
1370+
securityContext: &runtime.LinuxContainerSecurityContext{
1371+
RunAsUser: &runtime.Int64Value{Value: 1000},
1372+
RunAsGroup: &runtime.Int64Value{Value: 2000},
1373+
SupplementalGroups: []int64{3333},
1374+
},
1375+
expected: runtimespec.User{UID: 1000, GID: 2000, AdditionalGids: []uint32{2000, 3333, 11111}},
1376+
},
1377+
"Only imageConfig.User was set, imageConfig.User defines User": {
1378+
imageConfigUser: "1000",
1379+
securityContext: nil,
1380+
expected: runtimespec.User{UID: 1000, GID: 1000, AdditionalGids: []uint32{1000, 11111}},
1381+
},
1382+
"Both SecurityContext and ImageConfig.User was set, SecurityContext defines User": {
1383+
imageConfigUser: "0",
1384+
securityContext: &runtime.LinuxContainerSecurityContext{
1385+
RunAsUser: &runtime.Int64Value{Value: 1000},
1386+
RunAsGroup: &runtime.Int64Value{Value: 2000},
1387+
SupplementalGroups: []int64{3333},
1388+
},
1389+
expected: runtimespec.User{UID: 1000, GID: 2000, AdditionalGids: []uint32{2000, 3333, 11111}},
1390+
},
1391+
"No SecurityContext nor ImageConfig.User were set, runtime default defines User": {
1392+
expected: runtimespec.User{UID: 0, GID: 0, AdditionalGids: []uint32{0, 22222}},
1393+
},
1394+
} {
1395+
t.Run(desc, func(t *testing.T) {
1396+
containerConfig, sandboxConfig, imageConfig, _ := getCreateContainerTestData()
1397+
containerConfig.Linux.SecurityContext = test.securityContext
1398+
imageConfig.User = test.imageConfigUser
1399+
1400+
spec, err := c.buildContainerSpec(currentPlatform, testID, testSandboxID, testPid, "", testContainerName, testImageName, containerConfig, sandboxConfig, imageConfig, nil, ociRuntime)
1401+
require.NoError(t, err)
1402+
1403+
spec.Root.Path = tempRootDir // simulating /etc/{passwd, group}
1404+
opts, err := c.containerSpecOpts(containerConfig, imageConfig)
1405+
require.NoError(t, err)
1406+
oci.ApplyOpts(ctx, nil, testContainer, spec, opts...)
1407+
1408+
require.Equal(t, test.expected, spec.Process.User)
1409+
})
1410+
}
1411+
}
1412+
13291413
func TestNonRootUserAndDevices(t *testing.T) {
13301414
testPid := uint32(1234)
13311415
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)