Skip to content

cp: keep host-to-container writes inside the container mount - #5336

Open
danielcho02 wants to merge 1 commit into
containerd:mainfrom
danielcho02:cp-pin-destination
Open

danielcho02 wants to merge 1 commit into
containerd:mainfrom
danielcho02:cp-pin-destination

Conversation

@danielcho02

Copy link
Copy Markdown

nerdctl cp <host> <container>:<path> resolves the destination in userspace (resolvePath), then hands the resulting host path string (/proc/<pid>/root/..., or a volume's host path) to os.Mkdir and to tar -x -C. Both resolve that string again in the kernel. If a process in the running container replaces a path component with a symlink in between, the write follows it, and absolute symlinks under /proc/<pid>/root resolve against the host root, so the file lands on the host.

This keeps host-to-container writes inside the mount that holds the destination:

  • getPathSpecFromContainer records the mount's host path and the destination's path relative to it.
  • A new openExtractDir opens that mount with os.OpenRoot, creates a missing destination with Root.Mkdir, and opens the directory tar extracts into with Root.OpenFile(..., O_PATH|O_DIRECTORY).
  • A new tarExtractCmd, used by CopyFiles, hands that handle to tar through ExtraFiles as -C /proc/self/fd/3, so no path is left for tar to resolve again.

No new dependency (os.OpenInRoot is already used in pkg/cmd/container/run_mount.go). The container-to-host direction is unchanged, and so is the documented warning that nerdctl cp "is designed only for use with trusted, cooperating containers"; this only narrows the host-to-container case.

Details for copies into the container:

  • Errors from creating or opening the destination are still ErrFilesystem, but now come from os.Root and name the path relative to the mount, e.g. mkdirat data/x/proof: no such file or directory rather than mkdir /proc/<pid>/root/data/x/proof: .... Likewise, tar errors and the debug log show tar -x -C /proc/self/fd/3 -f - instead of the host path.
  • os.Root opens each directory on the way to the destination, so it needs read as well as search permission on them, where path strings needed search only. This only matters for a caller without CAP_DAC_OVERRIDE over those directories: as a non-root user, with an intermediate directory of mode 0333, mkdir and tar -C by path succeed while Root.Mkdir and Root.OpenFile fail with permission denied. A directory that cannot be searched is not a new case: resolving the destination already fails there with ErrFilesystem (lstat ...: permission denied), before and after this change, so the destination is never created or opened.
  • A single-file bind mount such as /etc/hosts is opened on its parent directory on the host, which is where tar already wrote.
  • For a mount whose source is not a path (/dev has source tmpfs, /dev/shm has shm), the source name used to be resolved against nerdctl's working directory. Usually that fails with destination parent does not exist, before and after this change. But run from a directory that contains tmpfs/, nerdctl cp f c:/dev/foo succeeded and wrote ./tmpfs/foo on the host. openExtractDir now refuses such a mount with cannot copy into mount with non-path source "tmpfs".

Testing

  • New unit tests in pkg/containerutil/cp_linux_test.go, on a directory standing in for /proc/<pid>/root with a volume:
    • getPathSpecFromContainer records the mount; openExtractDir opens the right directory, creates a missing destination inside the mount, and opens a file bind mount on its parent.
    • After the destination is resolved, a component is swapped for an absolute or a relative symlink to a directory outside: openExtractDir fails and nothing is created outside, for an existing destination, a created one, and a file copied to a new name.
    • tarExtractCmd extracts into the handle after the directory has been swapped for a symlink, and without a handle builds the same -C <dir> -f - command as before.
    • Inside the two helpers, reverting the mkdir or the open to path strings, dropping the handle from the tar command, or giving tar the path, makes these tests fail. They pass as root and as a non-root user, also with TMPDIR behind a symlink. The calls in CopyFiles are not unit-tested: the integration tests show copies still work through them, and the live race below that they keep writes inside.
  • go test ./cmd/nerdctl/container -run TestCopy passes: 201 passed (3 tests, 198 subtests), 0 failed, 1 skipped (rootful, Ubuntu on WSL2, GNU tar 1.35).
  • Live race, as a smoke test: a busybox container flipping /data/x between a directory and a symlink to a host-only directory, 300 × nerdctl cp /tmp/payload cpf:/data/x/proof. Across six runs on successive revisions of this branch, the unpatched binary wrote 7, 1, 1, 2, 1 and 2 files on the host and the patched one none; the last run is on this revision. Escapes are rare per attempt, so the unit tests carry the weight. While the container keeps flipping, the patched copies all fail, mostly with destination parent does not exist, otherwise with tar exiting with status 2 or the mkdir finding no parent; copies into containers that do not do this succeed as before.

Scope: this pins the directory tar extracts into and the mkdir. Paths below it are resolved by tar itself. With an archive member sub/file.txt and sub a symlink in the pinned directory, GNU tar 1.35 refuses (Cannot open: Invalid cross-device link) when the symlink is absolute or leads out of the directory, and follows it when it is relative and stays inside. Other tar implementations were not checked.

Checked by hand outside nerdctl: an O_PATH|O_DIRECTORY descriptor passed as fd 3 through nsenter -t <pid> -U --preserve-credentials -- tar -x -C /proc/self/fd/3, into a user namespace created with unshare -U -r, extracted into the pinned directory after it had been swapped for a symlink.

Not tested: a rootless nerdctl run end to end, and tar 1.30-13 on AlmaLinux 8 (the getcwd failure in #5237, worked around by #5238); tar still opens the -C directory itself here.

The destination was resolved in userspace and then passed as a host path
string to os.Mkdir and to `tar -x -C`, which resolve it again. A process
in the container could replace a component with a symlink in between;
absolute symlinks under /proc/<pid>/root resolve against the host root,
so the write could land on the host.

Record the mount holding the destination and the path relative to it.
openExtractDir opens the mount with os.OpenRoot, creates a missing
destination with Root.Mkdir and opens the directory tar extracts into
with O_PATH; tarExtractCmd hands that descriptor to tar (ExtraFiles,
-C /proc/self/fd/3) instead of a path.

A single-file bind mount (e.g. /etc/hosts) is opened on its parent
directory, which is where tar already wrote. A mount whose source is
not a path (e.g. /dev, source "tmpfs") is refused; its source used to be
resolved against the working directory, so copying to /dev/foo from a
directory containing tmpfs/ wrote ./tmpfs/foo on the host. The
container-to-host direction is unchanged.

Errors from creating or opening the destination now name the path
relative to the mount, and tar errors show -C /proc/self/fd/3. os.Root
needs read as well as search permission on the directories it walks,
which matters only to a caller without CAP_DAC_OVERRIDE (e.g. a 0333
intermediate directory now fails with permission denied).

Signed-off-by: Minkyu Cho <chominkyu3290@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants