Skip to content

Commit

Permalink
Merge pull request #3079 from kolyshkin/1.0-backport-3062
Browse files Browse the repository at this point in the history 
[1.0] libct/user: fix parsing long /etc/group lines
  • Loading branch information
Mrunal Patel authored Jul 9, 2021
2 parents 8304d0c + 0a5d8ba commit 8e259d2
Show file tree
Hide file tree
Showing 2 changed files with 107 additions and 45 deletions.
92 changes: 63 additions & 29 deletions libcontainer/user/user.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package user

import (
"bufio"
"bytes"
"errors"
"fmt"
"io"
Expand Down Expand Up @@ -55,11 +56,11 @@ type IDMap struct {
Count int64
}

func parseLine(line string, v ...interface{}) {
parseParts(strings.Split(line, ":"), v...)
func parseLine(line []byte, v ...interface{}) {
parseParts(bytes.Split(line, []byte(":")), v...)
}

func parseParts(parts []string, v ...interface{}) {
func parseParts(parts [][]byte, v ...interface{}) {
if len(parts) == 0 {
return
}
Expand All @@ -75,16 +76,16 @@ func parseParts(parts []string, v ...interface{}) {
// This is legit.
switch e := v[i].(type) {
case *string:
*e = p
*e = string(p)
case *int:
// "numbers", with conversion errors ignored because of some misbehaving configuration files.
*e, _ = strconv.Atoi(p)
*e, _ = strconv.Atoi(string(p))
case *int64:
*e, _ = strconv.ParseInt(p, 10, 64)
*e, _ = strconv.ParseInt(string(p), 10, 64)
case *[]string:
// Comma-separated lists.
if p != "" {
*e = strings.Split(p, ",")
if len(p) != 0 {
*e = strings.Split(string(p), ",")
} else {
*e = []string{}
}
Expand Down Expand Up @@ -128,8 +129,8 @@ func ParsePasswdFilter(r io.Reader, filter func(User) bool) ([]User, error) {
)

for s.Scan() {
line := strings.TrimSpace(s.Text())
if line == "" {
line := bytes.TrimSpace(s.Bytes())
if len(line) == 0 {
continue
}

Expand Down Expand Up @@ -179,15 +180,53 @@ func ParseGroupFilter(r io.Reader, filter func(Group) bool) ([]Group, error) {
if r == nil {
return nil, fmt.Errorf("nil source for group-formatted data")
}
rd := bufio.NewReader(r)
out := []Group{}

var (
s = bufio.NewScanner(r)
out = []Group{}
)
// Read the file line-by-line.
for {
var (
isPrefix bool
wholeLine []byte
err error
)

for s.Scan() {
text := s.Text()
if text == "" {
// Read the next line. We do so in chunks (as much as reader's
// buffer is able to keep), check if we read enough columns
// already on each step and store final result in wholeLine.
for {
var line []byte
line, isPrefix, err = rd.ReadLine()

if err != nil {
// We should return no error if EOF is reached
// without a match.
if err == io.EOF { //nolint:errorlint // comparison with io.EOF is legit, https://github.com/polyfloyd/go-errorlint/pull/12
err = nil
}
return out, err
}

// Simple common case: line is short enough to fit in a
// single reader's buffer.
if !isPrefix && len(wholeLine) == 0 {
wholeLine = line
break
}

wholeLine = append(wholeLine, line...)

// Check if we read the whole line already.
if !isPrefix {
break
}
}

// There's no spec for /etc/passwd or /etc/group, but we try to follow
// the same rules as the glibc parser, which allows comments and blank
// space at the beginning of a line.
wholeLine = bytes.TrimSpace(wholeLine)
if len(wholeLine) == 0 || wholeLine[0] == '#' {
continue
}

Expand All @@ -197,17 +236,12 @@ func ParseGroupFilter(r io.Reader, filter func(Group) bool) ([]Group, error) {
// root:x:0:root
// adm:x:4:root,adm,daemon
p := Group{}
parseLine(text, &p.Name, &p.Pass, &p.Gid, &p.List)
parseLine(wholeLine, &p.Name, &p.Pass, &p.Gid, &p.List)

if filter == nil || filter(p) {
out = append(out, p)
}
}
if err := s.Err(); err != nil {
return nil, err
}

return out, nil
}

type ExecUser struct {
Expand Down Expand Up @@ -278,7 +312,7 @@ func GetExecUser(userSpec string, defaults *ExecUser, passwd, group io.Reader) (

// Allow for userArg to have either "user" syntax, or optionally "user:group" syntax
var userArg, groupArg string
parseLine(userSpec, &userArg, &groupArg)
parseLine([]byte(userSpec), &userArg, &groupArg)

// Convert userArg and groupArg to be numeric, so we don't have to execute
// Atoi *twice* for each iteration over lines.
Expand Down Expand Up @@ -496,8 +530,8 @@ func ParseSubIDFilter(r io.Reader, filter func(SubID) bool) ([]SubID, error) {
)

for s.Scan() {
line := strings.TrimSpace(s.Text())
if line == "" {
line := bytes.TrimSpace(s.Bytes())
if len(line) == 0 {
continue
}

Expand Down Expand Up @@ -549,14 +583,14 @@ func ParseIDMapFilter(r io.Reader, filter func(IDMap) bool) ([]IDMap, error) {
)

for s.Scan() {
line := strings.TrimSpace(s.Text())
if line == "" {
line := bytes.TrimSpace(s.Bytes())
if len(line) == 0 {
continue
}

// see: man 7 user_namespaces
p := IDMap{}
parseParts(strings.Fields(line), &p.ID, &p.ParentID, &p.Count)
parseParts(bytes.Fields(line), &p.ID, &p.ParentID, &p.Count)

if filter == nil || filter(p) {
out = append(out, p)
Expand Down
60 changes: 44 additions & 16 deletions libcontainer/user/user_test.go
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package user

import (
"fmt"
"io"
"reflect"
"sort"
Expand All @@ -16,42 +17,42 @@ func TestUserParseLine(t *testing.T) {
d int
)

parseLine("", &a, &b)
parseLine([]byte(""), &a, &b)
if a != "" || b != "" {
t.Fatalf("a and b should be empty ('%v', '%v')", a, b)
}

parseLine("a", &a, &b)
parseLine([]byte("a"), &a, &b)
if a != "a" || b != "" {
t.Fatalf("a should be 'a' and b should be empty ('%v', '%v')", a, b)
}

parseLine("bad boys:corny cows", &a, &b)
parseLine([]byte("bad boys:corny cows"), &a, &b)
if a != "bad boys" || b != "corny cows" {
t.Fatalf("a should be 'bad boys' and b should be 'corny cows' ('%v', '%v')", a, b)
}

parseLine("", &c)
parseLine([]byte(""), &c)
if len(c) != 0 {
t.Fatalf("c should be empty (%#v)", c)
}

parseLine("d,e,f:g:h:i,j,k", &c, &a, &b, &c)
parseLine([]byte("d,e,f:g:h:i,j,k"), &c, &a, &b, &c)
if a != "g" || b != "h" || len(c) != 3 || c[0] != "i" || c[1] != "j" || c[2] != "k" {
t.Fatalf("a should be 'g', b should be 'h', and c should be ['i','j','k'] ('%v', '%v', '%#v')", a, b, c)
}

parseLine("::::::::::", &a, &b, &c)
parseLine([]byte("::::::::::"), &a, &b, &c)
if a != "" || b != "" || len(c) != 0 {
t.Fatalf("a, b, and c should all be empty ('%v', '%v', '%#v')", a, b, c)
}

parseLine("not a number", &d)
parseLine([]byte("not a number"), &d)
if d != 0 {
t.Fatalf("d should be 0 (%v)", d)
}

parseLine("b:12:c", &a, &d, &b)
parseLine([]byte("b:12:c"), &a, &d, &b)
if a != "b" || b != "c" || d != 12 {
t.Fatalf("a should be 'b' and b should be 'c', and d should be 12 ('%v', '%v', %v)", a, b, d)
}
Expand Down Expand Up @@ -82,12 +83,12 @@ func TestUserParseGroup(t *testing.T) {
root:x:0:root
adm:x:4:root,adm,daemon
this is just some garbage data
`), nil)
`+largeGroup()), nil)
if err != nil {
t.Fatalf("Unexpected error: %v", err)
}
if len(groups) != 3 {
t.Fatalf("Expected 3 groups, got %v", len(groups))
if len(groups) != 4 {
t.Fatalf("Expected 4 groups, got %v", len(groups))
}
if groups[0].Gid != 0 || groups[0].Name != "root" || len(groups[0].List) != 1 {
t.Fatalf("Expected groups[0] to be 0 - root - 1 member, got %v - %v - %v", groups[0].Gid, groups[0].Name, len(groups[0].List))
Expand All @@ -103,16 +104,18 @@ root:x:0:0:root user:/root:/bin/bash
adm:x:42:43:adm:/var/adm:/bin/false
111:x:222:333::/var/garbage
odd:x:111:112::/home/odd:::::
user7456:x:7456:100:Vasya:/home/user7456
this is just some garbage data
`
const groupContent = `
groupContent := `
root:x:0:root
adm:x:43:
grp:x:1234:root,adm
grp:x:1234:root,adm,user7456
444:x:555:111
odd:x:444:
this is just some garbage data
`
` + largeGroup()

defaultExecUser := ExecUser{
Uid: 8888,
Gid: 8888,
Expand Down Expand Up @@ -216,6 +219,16 @@ this is just some garbage data
Home: "/home/odd",
},
},
// Test for #3036.
{
ref: "7456",
expected: ExecUser{
Uid: 7456,
Gid: 100,
Sgids: []int{1234, 1000}, // 1000 is largegroup GID
Home: "/home/user7456",
},
},
}

for _, test := range tests {
Expand Down Expand Up @@ -388,13 +401,13 @@ func TestGetAdditionalGroups(t *testing.T) {
hasError bool
}

const groupContent = `
groupContent := `
root:x:0:root
adm:x:43:
grp:x:1234:root,adm
adm:x:4343:root,adm-duplicate
this is just some garbage data
`
` + largeGroup()
tests := []foo{
{
// empty group
Expand Down Expand Up @@ -444,6 +457,11 @@ this is just some garbage data
expected: nil,
hasError: true,
},
{
// group with very long list of users
groups: []string{"largegroup"},
expected: []int{1000},
},
}

for _, test := range tests {
Expand Down Expand Up @@ -500,3 +518,13 @@ func TestGetAdditionalGroupsNumeric(t *testing.T) {
}
}
}

// Generate a proper "largegroup" entry for group tests.
func largeGroup() (res string) {
var b strings.Builder
b.WriteString("largegroup:x:1000:user1")
for i := 2; i <= 7500; i++ {
fmt.Fprintf(&b, ",user%d", i)
}
return b.String()
}

0 comments on commit 8e259d2

Please sign in to comment.