Skip to content

Commit

Permalink
userns: fix off-by-one userns max size detection
Browse files Browse the repository at this point in the history
fix the detection for the maximum userns size from an image.

If the maximum ID used in an image is X, we need to use a user
namespace with size X+1 to include UID=X.

Closes: #2104

Signed-off-by: Giuseppe Scrivano <[email protected]>
  • Loading branch information
giuseppe authored and mheon committed Oct 14, 2024
1 parent d46cc6a commit 8038220
Show file tree
Hide file tree
Showing 2 changed files with 89 additions and 3 deletions.
6 changes: 3 additions & 3 deletions userns.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,10 +103,10 @@ func parseMountedFiles(containerMount, passwdFile, groupFile string) uint32 {
continue
}
if u.Uid > size && u.Uid != nobodyUser {
size = u.Uid
size = u.Uid + 1
}
if u.Gid > size && u.Gid != nobodyUser {
size = u.Gid
size = u.Gid + 1
}
}
}
Expand All @@ -118,7 +118,7 @@ func parseMountedFiles(containerMount, passwdFile, groupFile string) uint32 {
continue
}
if g.Gid > size && g.Gid != nobodyUser {
size = g.Gid
size = g.Gid + 1
}
}
}
Expand Down
86 changes: 86 additions & 0 deletions userns_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package storage

import (
"os"
"path/filepath"
"reflect"
"testing"

Expand Down Expand Up @@ -170,3 +172,87 @@ func TestGetAutoUserNSMapping(t *testing.T) {
})
}
}

func TestParseMountedFiles(t *testing.T) {
tests := []struct {
name string
passwdContent string
groupContent string
expectedMax uint32
}{
{
name: "basic case",
passwdContent: `
root:x:0:0:root:/root:/bin/bash
user1:x:1000:1000::/home/user1:/bin/bash
nobody:x:65534:65534:nobody:/nonexistent:/usr/sbin/nologin`,
groupContent: `
root:x:0:
user1:x:1000:
nogroup:x:65534:`,
expectedMax: 1001,
},
{
name: "only passwd",
passwdContent: `
root:x:0:0:root:/root:/bin/bash
user1:x:4001:4001::/home/user1:/bin/bash
nobody:x:65534:65534:nobody:/nonexistent:/usr/sbin/nologin`,
groupContent: "",
expectedMax: 4002,
},
{
name: "only groups",
passwdContent: "",
groupContent: `
root:x:0:
admin:x:3000:
nobody:x:65534:`,
expectedMax: 3001,
},
{
name: "empty files",
passwdContent: "",
groupContent: "",
expectedMax: 0,
},
{
name: "invalid passwd file",
passwdContent: "FOOBAR",
groupContent: "",
expectedMax: 0,
},
{
name: "invalid groups file",
passwdContent: "",
groupContent: "FOOBAR",
expectedMax: 0,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
tmpDir, err := os.MkdirTemp("", "containermount")
if err != nil {
t.Fatalf("Failed to create temp dir: %v", err)
}
defer os.RemoveAll(tmpDir)

passwdFile := filepath.Join(tmpDir, "passwd")
if err := os.WriteFile(passwdFile, []byte(tt.passwdContent), 0o644); err != nil {
t.Fatalf("Failed to write passwd file: %v", err)
}

groupFile := filepath.Join(tmpDir, "group")
if err := os.WriteFile(groupFile, []byte(tt.groupContent), 0o644); err != nil {
t.Fatalf("Failed to write group file: %v", err)
}

result := parseMountedFiles(tmpDir, passwdFile, groupFile)

if result != tt.expectedMax {
t.Errorf("Expected max %d, but got %d", tt.expectedMax, result)
}
})
}
}

0 comments on commit 8038220

Please sign in to comment.