Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Get correct username in pod when using --userns=keep-id #17174

Merged
merged 1 commit into from
Jan 24, 2023
Merged

Get correct username in pod when using --userns=keep-id #17174

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions libpod/container_internal_common.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2465,6 +2465,10 @@ func (c *Container) generateUserPasswdEntry(addedUID int) (string, error) {
return entry, nil
}

u, err := user.LookupId(fmt.Sprintf("%d", uid))
if err == nil {
return fmt.Sprintf("%s:*:%d:%d:%s:%s:/bin/sh\n", u.Username, uid, gid, u.Name, c.WorkingDir()), nil
}
return fmt.Sprintf("%d:*:%d:%d:container user:%s:/bin/sh\n", uid, uid, gid, c.WorkingDir()), nil
}

Expand Down
8 changes: 5 additions & 3 deletions test/e2e/pod_create_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -711,12 +711,14 @@ ENTRYPOINT ["sleep","99999"]
session.WaitWithDefaultTimeout()
Expect(session).Should(Exit(0))

// container inside pod inherits user form infra container if --user is not set
// etc/passwd entry will look like 1000:*:1000:1000:container user:/:/bin/sh
u, err := user.Current()
Expect(err).ToNot(HaveOccurred())
// container inside pod inherits user from infra container if --user is not set
// etc/passwd entry will look like USERNAME:*:1000:1000:Full User Name:/:/bin/sh
exec1 := podmanTest.Podman([]string{"exec", ctrName, "cat", "/etc/passwd"})
exec1.WaitWithDefaultTimeout()
Expect(exec1).Should(Exit(0))
Expect(exec1.OutputToString()).To(ContainSubstring("container"))
Expect(exec1.OutputToString()).To(ContainSubstring(u.Name))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just out of principle, I think a better test would be:

    .....{"exec", ctrName, "grep", "^"+u.Name+":", "/etc/passwd"})

or even (cleaner)

    .....{"exec", ... , "id", u.Name})

Basically, to avoid a false match on username against (something else in passwd). Not that I can think of a real-world example, so this is paranoia. Anyone concur/disagree? Should I open an update/cleanup PR?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we would probably catch it in the CI if the passwd file of any image breaks this test. Said so, nothing against paranoia and fixing it earlier :-)


exec2 := podmanTest.Podman([]string{"exec", ctrName, "useradd", "testuser"})
exec2.WaitWithDefaultTimeout()
Expand Down
13 changes: 13 additions & 0 deletions test/system/170-run-userns.bats
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,3 +140,16 @@ EOF
is "${output}" "Error: keep-id is only supported in rootless mode" "Container should fail to start since keep-id is not supported in rootful mode"
fi
}

@test "podman userns=keep-id in a pod" {
if is_rootless; then
user=$(id -u)
run_podman pod create --userns keep-id
pid=$output
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Eek, am I the only one who read this as "process id" and got severely confused for a second or two?

run_podman run --rm --pod $pid $IMAGE id -u
is "${output}" "$user" "Container should run as the current user"
else
run_podman 125 pod create --userns keep-id
is "${output}" 'Error:.*keep-id is only supported in rootless mode' "pod should fail to be created since keep-id is not supported in rootful mode"
fi
}