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

Jump to bottom

User from "generateCurrentUserPasswdEntry" created with duplicate UID #7503

Closed
njam opened this issue Aug 30, 2020 · 1 comment · Fixed by #7541
Closed

User from "generateCurrentUserPasswdEntry" created with duplicate UID #7503

njam opened this issue Aug 30, 2020 · 1 comment · Fixed by #7541
Assignees
@mheon
Labels
kind/bug Categorizes issue or PR as related to a bug. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.

Comments

@njam
Copy link

njam commented Aug 30, 2020

Is this a BUG REPORT or FEATURE REQUEST? (leave only one on its own line)

/kind bug

Description

Since 2.0.5 podman creates a user account when --userns=keep-id is set, with the UID of the user on the host system. The algorithm doesn't check if there's already a user with the same UID inside the container. As a result in the end there might be two user accounts with the same UID.

Steps to reproduce the issue:

  1. Given your UID is 1000, run a container off "quay.io/podman/stable" (which already has a user with UID 1000)
$ id -u
1000
$ podman run --rm -it --user=root --userns=keep-id quay.io/podman/stable tail -n2 /etc/passwd
podman:x:1000:1000::/home/podman:/bin/bash
USER:x:1000:995:USER:/:/bin/sh

Describe the results you received:
The /etc/passwd in the containers now has two users with UID 1000.

Describe the results you expected:
Not sure how this should be handled, I guess either:

  • A: Not create a new entry, if there's already an entry with this UID
  • B: Abort the creation of the container with an error.

Additional information you deem important (e.g. issue happens only occasionally):
I've noticed this behaviour when investigating #7499.
I'm not sure if the duplicate UIDs are a problem, and how severe. Maybe it's actually ok and this ticket can be closed. Just wanted to bring it to your attention.

This functionality was added in 3dfd863 cc @rhatdan @vrothberg

Output of podman version:

Version:      2.0.5
API Version:  1
Go Version:   go1.15
Git Commit:   776abc52106ec7652ced6dbc0869020123ed393d
Built:        Tue Aug 25 20:58:48 2020
OS/Arch:      linux/amd64

Output of podman info --debug:

host:
  arch: amd64
  buildahVersion: 1.15.1
  cgroupVersion: v1
  conmon:
    package: Unknown
    path: /usr/bin/conmon
    version: 'conmon version 2.0.20, commit: 13244db638cf987c415298a3c23393ae5abeb885'
  cpus: 12
  distribution:
    distribution: arch
    version: unknown
  eventLogger: file
  hostname: njamdesk
  idMappings:
    gidmap:
    - container_id: 0
      host_id: 995
      size: 1
    - container_id: 1
      host_id: 100000
      size: 65536
    uidmap:
    - container_id: 0
      host_id: 1000
      size: 1
    - container_id: 1
      host_id: 100000
      size: 65536
  kernel: 5.8.5-arch1-1
  linkmode: dynamic
  memFree: 5362950144
  memTotal: 33520361472
  ociRuntime:
    name: runc
    package: Unknown
    path: /usr/bin/runc
    version: |-
      runc version 1.0.0-rc92
      commit: ff819c7e9184c13b7c2607fe6c30ae19403a7aff
      spec: 1.0.2-dev
  os: linux
  remoteSocket:
    path: /run/user/1000/podman/podman.sock
  rootless: true
  slirp4netns:
    executable: /usr/bin/slirp4netns
    package: Unknown
    version: |-
      slirp4netns version 1.1.4
      commit: b66ffa8e262507e37fca689822d23430f3357fe8
      libslirp: 4.3.1
      SLIRP_CONFIG_VERSION_MAX: 3
  swapFree: 0
  swapTotal: 0
  uptime: 5h 45m 58.77s (Approximately 0.21 days)
registries:
  search:
  - docker.io
  - registry.fedoraproject.org
  - quay.io
  - registry.access.redhat.com
  - registry.centos.org
store:
  configFile: /home/reto/.config/containers/storage.conf
  containerStore:
    number: 29
    paused: 0
    running: 3
    stopped: 26
  graphDriverName: overlay
  graphOptions:
    overlay.mount_program:
      Executable: /usr/bin/fuse-overlayfs
      Package: Unknown
      Version: |-
        fusermount3 version: 3.9.3
        fuse-overlayfs: version 1.1.0
        FUSE library version 3.9.3
        using FUSE kernel interface version 7.31
  graphRoot: /home/reto/.local/share/containers/storage
  graphStatus:
    Backing Filesystem: btrfs
    Native Overlay Diff: "false"
    Supports d_type: "true"
    Using metacopy: "false"
  imageStore:
    number: 95
  runRoot: /run/user/1000/containers
  volumePath: /home/reto/.local/share/containers/storage/volumes
version:
  APIVersion: 1
  Built: 1598381928
  BuiltTime: Tue Aug 25 20:58:48 2020
  GitCommit: 776abc52106ec7652ced6dbc0869020123ed393d
  GoVersion: go1.15
  OsArch: linux/amd64
  Version: 2.0.5

Package info (e.g. output of rpm -q podman or apt list podman):

$ pacman -Q podman       
podman 2.0.5-1

Have you tested with the latest version of Podman and have you checked the Podman Troubleshooting Guide?

Yes

Additional environment details (AWS, VirtualBox, physical, etc.):

physical
@openshift-ci-robot openshift-ci-robot added the kind/bug Categorizes issue or PR as related to a bug. label Aug 30, 2020
@njam njam mentioned this issue Aug 30, 2020
Closed
@mheon
Copy link
Member

mheon commented Sep 2, 2020

I'm working on this now.

@mheon mheon self-assigned this Sep 2, 2020
mheon added a commit to mheon/libpod that referenced this issue Sep 4, 2020
@mheon
Make an entry in /etc/group when we modify /etc/passwd
31f26c7 
To ensure that the user running in the container ahs a valid
entry in /etc/passwd so lookup functions for the current user
will not error, Podman previously began adding entries to the
passwd file. We did not, however, add entries to the group file,
and this created problems - our passwd entries included the group
the user is in, but said group might not exist. The solution is
to mirror our logic for /etc/passwd modifications to also edit
/etc/group in the container.

Unfortunately, this is not a catch-all solution. Our logic here
is only advanced enough to *add* to the group file - so if the
group already exists but we add a user not a part of it, we will
not modify that existing entry, and things remain inconsistent.
We can look into adding this later if we absolutely need to, but
it would involve adding significant complexity to this already
massively complicated function.

While we're here, address an edge case where Podman could add a
user or group whose UID overlapped with an existing user or
group.

Fixes containers#7503
Fixes containers#7389

Signed-off-by: Matthew Heon <matthew.heon@pm.me>
@mheon mheon mentioned this issue Sep 4, 2020
Merged
mheon added a commit to mheon/libpod that referenced this issue Sep 4, 2020
@mheon
Make an entry in /etc/group when we modify /etc/passwd
51ecd03 
To ensure that the user running in the container ahs a valid
entry in /etc/passwd so lookup functions for the current user
will not error, Podman previously began adding entries to the
passwd file. We did not, however, add entries to the group file,
and this created problems - our passwd entries included the group
the user is in, but said group might not exist. The solution is
to mirror our logic for /etc/passwd modifications to also edit
/etc/group in the container.

Unfortunately, this is not a catch-all solution. Our logic here
is only advanced enough to *add* to the group file - so if the
group already exists but we add a user not a part of it, we will
not modify that existing entry, and things remain inconsistent.
We can look into adding this later if we absolutely need to, but
it would involve adding significant complexity to this already
massively complicated function.

While we're here, address an edge case where Podman could add a
user or group whose UID overlapped with an existing user or
group.

Fixes containers#7503
Fixes containers#7389

Signed-off-by: Matthew Heon <matthew.heon@pm.me>
mheon added a commit to mheon/libpod that referenced this issue Sep 4, 2020
@mheon
Make an entry in /etc/group when we modify /etc/passwd
56a4a50 
To ensure that the user running in the container ahs a valid
entry in /etc/passwd so lookup functions for the current user
will not error, Podman previously began adding entries to the
passwd file. We did not, however, add entries to the group file,
and this created problems - our passwd entries included the group
the user is in, but said group might not exist. The solution is
to mirror our logic for /etc/passwd modifications to also edit
/etc/group in the container.

Unfortunately, this is not a catch-all solution. Our logic here
is only advanced enough to *add* to the group file - so if the
group already exists but we add a user not a part of it, we will
not modify that existing entry, and things remain inconsistent.
We can look into adding this later if we absolutely need to, but
it would involve adding significant complexity to this already
massively complicated function.

While we're here, address an edge case where Podman could add a
user or group whose UID overlapped with an existing user or
group.

Fixes containers#7503
Fixes containers#7389

Signed-off-by: Matthew Heon <matthew.heon@pm.me>
mheon added a commit to mheon/libpod that referenced this issue Sep 4, 2020
@mheon
Make an entry in /etc/group when we modify /etc/passwd
9870b5d 
To ensure that the user running in the container ahs a valid
entry in /etc/passwd so lookup functions for the current user
will not error, Podman previously began adding entries to the
passwd file. We did not, however, add entries to the group file,
and this created problems - our passwd entries included the group
the user is in, but said group might not exist. The solution is
to mirror our logic for /etc/passwd modifications to also edit
/etc/group in the container.

Unfortunately, this is not a catch-all solution. Our logic here
is only advanced enough to *add* to the group file - so if the
group already exists but we add a user not a part of it, we will
not modify that existing entry, and things remain inconsistent.
We can look into adding this later if we absolutely need to, but
it would involve adding significant complexity to this already
massively complicated function.

While we're here, address an edge case where Podman could add a
user or group whose UID overlapped with an existing user or
group.

Fixes containers#7503
Fixes containers#7389

Signed-off-by: Matthew Heon <matthew.heon@pm.me>
mheon added a commit to mheon/libpod that referenced this issue Sep 9, 2020
@mheon
Make an entry in /etc/group when we modify /etc/passwd
a5cac7c 
To ensure that the user running in the container ahs a valid
entry in /etc/passwd so lookup functions for the current user
will not error, Podman previously began adding entries to the
passwd file. We did not, however, add entries to the group file,
and this created problems - our passwd entries included the group
the user is in, but said group might not exist. The solution is
to mirror our logic for /etc/passwd modifications to also edit
/etc/group in the container.

Unfortunately, this is not a catch-all solution. Our logic here
is only advanced enough to *add* to the group file - so if the
group already exists but we add a user not a part of it, we will
not modify that existing entry, and things remain inconsistent.
We can look into adding this later if we absolutely need to, but
it would involve adding significant complexity to this already
massively complicated function.

While we're here, address an edge case where Podman could add a
user or group whose UID overlapped with an existing user or
group.

Also, let's make users able to log into users we added. Instead
of generating user entries with an 'x' in the password field,
indicating they have an entry in /etc/shadow, generate a '*'
indicating the user has no password but can be logged into by
other means e.g. ssh key, su.

Fixes containers#7503
Fixes containers#7389
Fixes containers#7499

Signed-off-by: Matthew Heon <matthew.heon@pm.me>
mheon added a commit to mheon/libpod that referenced this issue Sep 9, 2020
@mheon
Make an entry in /etc/group when we modify /etc/passwd
a7294a7 
To ensure that the user running in the container ahs a valid
entry in /etc/passwd so lookup functions for the current user
will not error, Podman previously began adding entries to the
passwd file. We did not, however, add entries to the group file,
and this created problems - our passwd entries included the group
the user is in, but said group might not exist. The solution is
to mirror our logic for /etc/passwd modifications to also edit
/etc/group in the container.

Unfortunately, this is not a catch-all solution. Our logic here
is only advanced enough to *add* to the group file - so if the
group already exists but we add a user not a part of it, we will
not modify that existing entry, and things remain inconsistent.
We can look into adding this later if we absolutely need to, but
it would involve adding significant complexity to this already
massively complicated function.

While we're here, address an edge case where Podman could add a
user or group whose UID overlapped with an existing user or
group.

Also, let's make users able to log into users we added. Instead
of generating user entries with an 'x' in the password field,
indicating they have an entry in /etc/shadow, generate a '*'
indicating the user has no password but can be logged into by
other means e.g. ssh key, su.

Fixes containers#7503
Fixes containers#7389
Fixes containers#7499

Signed-off-by: Matthew Heon <matthew.heon@pm.me>
mheon added a commit to mheon/libpod that referenced this issue Sep 9, 2020
@mheon
Make an entry in /etc/group when we modify /etc/passwd
436ff92 
To ensure that the user running in the container ahs a valid
entry in /etc/passwd so lookup functions for the current user
will not error, Podman previously began adding entries to the
passwd file. We did not, however, add entries to the group file,
and this created problems - our passwd entries included the group
the user is in, but said group might not exist. The solution is
to mirror our logic for /etc/passwd modifications to also edit
/etc/group in the container.

Unfortunately, this is not a catch-all solution. Our logic here
is only advanced enough to *add* to the group file - so if the
group already exists but we add a user not a part of it, we will
not modify that existing entry, and things remain inconsistent.
We can look into adding this later if we absolutely need to, but
it would involve adding significant complexity to this already
massively complicated function.

While we're here, address an edge case where Podman could add a
user or group whose UID overlapped with an existing user or
group.

Also, let's make users able to log into users we added. Instead
of generating user entries with an 'x' in the password field,
indicating they have an entry in /etc/shadow, generate a '*'
indicating the user has no password but can be logged into by
other means e.g. ssh key, su.

Fixes containers#7503
Fixes containers#7389
Fixes containers#7499

Signed-off-by: Matthew Heon <matthew.heon@pm.me>
mheon added a commit to mheon/libpod that referenced this issue Sep 9, 2020
@mheon
Make an entry in /etc/group when we modify /etc/passwd
d457cba 
To ensure that the user running in the container ahs a valid
entry in /etc/passwd so lookup functions for the current user
will not error, Podman previously began adding entries to the
passwd file. We did not, however, add entries to the group file,
and this created problems - our passwd entries included the group
the user is in, but said group might not exist. The solution is
to mirror our logic for /etc/passwd modifications to also edit
/etc/group in the container.

Unfortunately, this is not a catch-all solution. Our logic here
is only advanced enough to *add* to the group file - so if the
group already exists but we add a user not a part of it, we will
not modify that existing entry, and things remain inconsistent.
We can look into adding this later if we absolutely need to, but
it would involve adding significant complexity to this already
massively complicated function.

While we're here, address an edge case where Podman could add a
user or group whose UID overlapped with an existing user or
group.

Also, let's make users able to log into users we added. Instead
of generating user entries with an 'x' in the password field,
indicating they have an entry in /etc/shadow, generate a '*'
indicating the user has no password but can be logged into by
other means e.g. ssh key, su.

Fixes containers#7503
Fixes containers#7389
Fixes containers#7499

Signed-off-by: Matthew Heon <matthew.heon@pm.me>
mheon added a commit to mheon/libpod that referenced this issue Sep 9, 2020
@mheon
Make an entry in /etc/group when we modify /etc/passwd
099feb5 
To ensure that the user running in the container ahs a valid
entry in /etc/passwd so lookup functions for the current user
will not error, Podman previously began adding entries to the
passwd file. We did not, however, add entries to the group file,
and this created problems - our passwd entries included the group
the user is in, but said group might not exist. The solution is
to mirror our logic for /etc/passwd modifications to also edit
/etc/group in the container.

Unfortunately, this is not a catch-all solution. Our logic here
is only advanced enough to *add* to the group file - so if the
group already exists but we add a user not a part of it, we will
not modify that existing entry, and things remain inconsistent.
We can look into adding this later if we absolutely need to, but
it would involve adding significant complexity to this already
massively complicated function.

While we're here, address an edge case where Podman could add a
user or group whose UID overlapped with an existing user or
group.

Also, let's make users able to log into users we added. Instead
of generating user entries with an 'x' in the password field,
indicating they have an entry in /etc/shadow, generate a '*'
indicating the user has no password but can be logged into by
other means e.g. ssh key, su.

Fixes containers#7503
Fixes containers#7389
Fixes containers#7499

Signed-off-by: Matthew Heon <matthew.heon@pm.me>
mheon added a commit to mheon/libpod that referenced this issue Sep 10, 2020
@mheon
Make an entry in /etc/group when we modify /etc/passwd
d79693b 
To ensure that the user running in the container ahs a valid
entry in /etc/passwd so lookup functions for the current user
will not error, Podman previously began adding entries to the
passwd file. We did not, however, add entries to the group file,
and this created problems - our passwd entries included the group
the user is in, but said group might not exist. The solution is
to mirror our logic for /etc/passwd modifications to also edit
/etc/group in the container.

Unfortunately, this is not a catch-all solution. Our logic here
is only advanced enough to *add* to the group file - so if the
group already exists but we add a user not a part of it, we will
not modify that existing entry, and things remain inconsistent.
We can look into adding this later if we absolutely need to, but
it would involve adding significant complexity to this already
massively complicated function.

While we're here, address an edge case where Podman could add a
user or group whose UID overlapped with an existing user or
group.

Also, let's make users able to log into users we added. Instead
of generating user entries with an 'x' in the password field,
indicating they have an entry in /etc/shadow, generate a '*'
indicating the user has no password but can be logged into by
other means e.g. ssh key, su.

Fixes containers#7503
Fixes containers#7389
Fixes containers#7499

Signed-off-by: Matthew Heon <matthew.heon@pm.me>
mheon added a commit to mheon/libpod that referenced this issue Sep 10, 2020
@mheon
Make an entry in /etc/group when we modify /etc/passwd
f57c39f 
To ensure that the user running in the container ahs a valid
entry in /etc/passwd so lookup functions for the current user
will not error, Podman previously began adding entries to the
passwd file. We did not, however, add entries to the group file,
and this created problems - our passwd entries included the group
the user is in, but said group might not exist. The solution is
to mirror our logic for /etc/passwd modifications to also edit
/etc/group in the container.

Unfortunately, this is not a catch-all solution. Our logic here
is only advanced enough to *add* to the group file - so if the
group already exists but we add a user not a part of it, we will
not modify that existing entry, and things remain inconsistent.
We can look into adding this later if we absolutely need to, but
it would involve adding significant complexity to this already
massively complicated function.

While we're here, address an edge case where Podman could add a
user or group whose UID overlapped with an existing user or
group.

Also, let's make users able to log into users we added. Instead
of generating user entries with an 'x' in the password field,
indicating they have an entry in /etc/shadow, generate a '*'
indicating the user has no password but can be logged into by
other means e.g. ssh key, su.

Fixes containers#7503
Fixes containers#7389
Fixes containers#7499

Signed-off-by: Matthew Heon <matthew.heon@pm.me>
edsantiago pushed a commit to edsantiago/libpod that referenced this issue Sep 14, 2020
@mheon @edsantiago
Make an entry in /etc/group when we modify /etc/passwd
bb9f49c 
To ensure that the user running in the container ahs a valid
entry in /etc/passwd so lookup functions for the current user
will not error, Podman previously began adding entries to the
passwd file. We did not, however, add entries to the group file,
and this created problems - our passwd entries included the group
the user is in, but said group might not exist. The solution is
to mirror our logic for /etc/passwd modifications to also edit
/etc/group in the container.

Unfortunately, this is not a catch-all solution. Our logic here
is only advanced enough to *add* to the group file - so if the
group already exists but we add a user not a part of it, we will
not modify that existing entry, and things remain inconsistent.
We can look into adding this later if we absolutely need to, but
it would involve adding significant complexity to this already
massively complicated function.

While we're here, address an edge case where Podman could add a
user or group whose UID overlapped with an existing user or
group.

Also, let's make users able to log into users we added. Instead
of generating user entries with an 'x' in the password field,
indicating they have an entry in /etc/shadow, generate a '*'
indicating the user has no password but can be logged into by
other means e.g. ssh key, su.

Fixes containers#7503
Fixes containers#7389
Fixes containers#7499

Signed-off-by: Matthew Heon <matthew.heon@pm.me>
@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 22, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@mheon mheon

Labels
kind/bug Categorizes issue or PR as related to a bug. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Make an entry in /etc/group when we modify /etc/passwd mheon/libpod
3 participants
@njam @mheon @openshift-ci-robot