archive: disable looking up usernames and groupnames on the host #8220
Conversation
archive/tar.go
Outdated
| // uname and gname strings are from the host filesystem and meaningless for containers | ||
| // (https://github.com/moby/buildkit/issues/3688) | ||
| hdr.Uname = "" | ||
| hdr.Gname = "" |
There was a problem hiding this comment.
Should we disable the lookup altogether? In moby/moby we previously had a fork of pkg/archive to disable lookup of usernames (related to CVE-2019-14271 / GHSA-v2cv-wwxq-qq97), but in moby/moby@e9bbc41, we implemented a new approach to disable it (while using Go's stdlib)
/cc @corhere
There was a problem hiding this comment.
What would be the benefit compared to just unsetting uname and gname? (slightly faster?)
There was a problem hiding this comment.
ISTR the lookup not just looks up in /etc/passwd, but (depending on the host) also looks up in other ways.
Not sure if it applies to this as well, but I recall cases on Windows where this could even result in connecting to Active Directory (something which has been known to cause 10 seconds or more delays in some setups).
|
@thaJeztah: GitHub didn't allow me to request PR reviews from the following users: corhere. Note that only containerd members and repo collaborators can review this PR, and authors cannot review their own PRs. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
AkihiroSuda
force-pushed
the
archive-discard-uname-gname
branch
2 times, most recently
from
f42ab87 to
bfe198b
Compare
AkihiroSuda
force-pushed
the
archive-discard-uname-gname
branch
from
bfe198b to
a4cac89
Compare
|
only nit: should the commit message reflect the latest changes (e.g., "disable user-lookup ...")? |
These strings were resolved using the user information on the host filesystem and were decreasing reproducibility. For moby/buildkit issue 3688 Signed-off-by: Akihiro Suda <[email protected]>
AkihiroSuda
force-pushed
the
archive-discard-uname-gname
branch
from
a4cac89 to
d26587c
Compare
AkihiroSuda
changed the title
|
(Do we need to assign a CVE for that we were leaking user names on the host? 🤔 ) |
Not sure if needed; it seems very minimal. If we would, then the CVE should probably be for Go itself for including this information by default (and until recently no good way to disable it) |
thaJeztah
added
cherry-picked/1.6.x
5 participants
These strings were resolved using the user information on the host filesystem and were decreasing reproducibility.
For :
/etc/{passwd,group}on the buildkit daemon host moby/buildkit#3688