Skip to content

Commit 41cf740

Browse files
committed
user: fix ToHost treating a non-root uid equal to the remapped-root base as the remapped root
ToHost translated a container uid to a host uid by skipping the remap when the container uid matched the host remapped-root value (RootPair). That comparison was wrong: the input is a container-namespace id, so it must be compared against the container root (0), not the host remapped-root base. When the subuid/subgid base is below 65536 (e.g. 'rootless:1000:65536', used to align dind-rootless container uids), a non-root container uid that equals the base (e.g. 1000) was incorrectly left unmapped and therefore appeared as root inside the container. Compare against the container root (0) instead, which preserves the original intent (the remapped root maps to RootPair) and correctly translates every other uid. Add a regression test covering standard, low-base, and empty mappings.
1 parent d7c3b9e commit 41cf740

2 files changed

Lines changed: 87 additions & 13 deletions

File tree

user/idtools.go

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -118,21 +118,24 @@ func (i IdentityMapping) RootPair() (int, int) {
118118
}
119119

120120
// ToHost returns the host UID and GID for the container uid, gid.
121-
// Remapping is only performed if the ids aren't already the remapped root ids
121+
//
122+
// Every container id is translated through the id map. The container root
123+
// (id 0) maps to the host remapped-root base (the ParentID of the map entry
124+
// covering id 0), because toHost(0) resolves to that base--which is exactly
125+
// what [IdentityMapping.RootPair] returns, so no special case is needed.
126+
// An empty (nil) mapping is treated as identity.
127+
//
128+
// Callers must pass container-namespace ids. ToHost does not treat an id as
129+
// "already remapped" based on the host remapped-root value: doing so would
130+
// incorrectly leave a non-root container uid that happens to equal the host
131+
// remapped-root base unmapped (and therefore owned by the remapped root inside
132+
// the container).
122133
func (i IdentityMapping) ToHost(uid, gid int) (int, int, error) {
123-
var err error
124-
ruid, rgid := i.RootPair()
125-
126-
if uid != ruid {
127-
ruid, err = toHost(uid, i.UIDMaps)
128-
if err != nil {
129-
return ruid, rgid, err
130-
}
131-
}
132-
133-
if gid != rgid {
134-
rgid, err = toHost(gid, i.GIDMaps)
134+
ruid, err := toHost(uid, i.UIDMaps)
135+
if err != nil {
136+
return ruid, 0, err
135137
}
138+
rgid, err := toHost(gid, i.GIDMaps)
136139
return ruid, rgid, err
137140
}
138141

user/idtools_unix_test.go

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -358,6 +358,77 @@ func TestGetRootUIDGID(t *testing.T) {
358358
}
359359
}
360360

361+
func TestToHost(t *testing.T) {
362+
for _, tc := range []struct {
363+
name string
364+
idMap []IDMap
365+
uid int
366+
gid int
367+
wantUID int
368+
wantGID int
369+
}{
370+
{
371+
name: "standard remap, container root",
372+
idMap: []IDMap{{ID: 0, ParentID: 100000, Count: 65536}},
373+
uid: 0,
374+
gid: 0,
375+
wantUID: 100000,
376+
wantGID: 100000,
377+
},
378+
{
379+
name: "standard remap, non-root user",
380+
idMap: []IDMap{{ID: 0, ParentID: 100000, Count: 65536}},
381+
uid: 1000,
382+
gid: 1000,
383+
wantUID: 101000,
384+
wantGID: 101000,
385+
},
386+
{
387+
// Regression test: when the remapped-root base is below the
388+
// container uid range, a non-root uid that equals the base must
389+
// still be translated, and must not be treated as the remapped
390+
// root (which would make the file owned by root inside the
391+
// container).
392+
name: "low base remap, non-root uid equal to base",
393+
idMap: []IDMap{{ID: 0, ParentID: 1000, Count: 65536}},
394+
uid: 1000,
395+
gid: 1000,
396+
wantUID: 2000,
397+
wantGID: 2000,
398+
},
399+
{
400+
name: "low base remap, container root",
401+
idMap: []IDMap{{ID: 0, ParentID: 1000, Count: 65536}},
402+
uid: 0,
403+
gid: 0,
404+
wantUID: 1000,
405+
wantGID: 1000,
406+
},
407+
{
408+
name: "no remap (empty mapping)",
409+
idMap: nil,
410+
uid: 1000,
411+
gid: 1000,
412+
wantUID: 1000,
413+
wantGID: 1000,
414+
},
415+
} {
416+
t.Run(tc.name, func(t *testing.T) {
417+
m := IdentityMapping{UIDMaps: tc.idMap, GIDMaps: tc.idMap}
418+
uid, gid, err := m.ToHost(tc.uid, tc.gid)
419+
if err != nil {
420+
t.Fatal(err)
421+
}
422+
if uid != tc.wantUID {
423+
t.Errorf("uid: got %d, want %d", uid, tc.wantUID)
424+
}
425+
if gid != tc.wantGID {
426+
t.Errorf("gid: got %d, want %d", gid, tc.wantGID)
427+
}
428+
})
429+
}
430+
}
431+
361432
func TestToContainer(t *testing.T) {
362433
uidMap := []IDMap{
363434
{

0 commit comments

Comments
 (0)