When noting that a non-symlink has setuid/setgid/sticky bits, switch
from using "syscall" package bits and syscall.Chmod() to using "os"
package bits and os.Chmod(), and if the item's a directory, record the
updated mode information in the "directoryModes" map that we'll use to
reset its permissions later.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Signed-off-by: Aditya R <arajan@redhat.com>
Similar to the `NoOverwriteDirNonDir` one, add an option that disables
non-directories from being overwritten by directories.
Required-for: containers/podman/issues/14420
Signed-off-by: Valentin Rothberg <vrothberg@redhat.com>
Since we started calling into the standard library's io/fs package
directly, we effectively made Go 1.16 our minimum Go version, so we
don't need to keep the workaround for compiling with Go 1.12.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Add StripSetuidBit/StripSetgidBit/StripStickyBit flags to
copier.PutOptions, that are interpreted similarly to their counterparts
in copier.GetOptions.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Try to improve our ability to write to directories that aren't
writable. If we encounter an EPERM error while attempting to create an
item, attempt to temporarily make writable the directory that we're
writing the item to, and restore its permissions on our way out.
The error usually isn't seen when run as UID 0, whether in a user
namespace or not, which is usually how we're called, but running the
unit tests as an unprivileged user will verify it.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Handing Put() an archive which uses host IDs while also including ID
maps for mapping from container IDs to host IDs is a mistake, and the
tests have been failing when run by non-root users since storage v1.38
started flagging it as such.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
it is needed to fix a failing test.
commit 985eec5391 changed the behavior
since the directory is not opened first before adding it.
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Should speed up most walks escpecially if they don't need to
stat every directory entry.
[ NO NEW TESTS NEEDED]
Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
Since commit 3eef1ed0bd (January 2019) using
capabilities.NewPid() is deprecated.
Replace with NewPid2().
Note that in chroot/run.go we used to load then clear all capabilities
bits. With NewPid2, this is no longer needed -- we do not load caps, so
there is no need to clear.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
When checking if something that we want to overwrite with a directory is
already a directory or not, use lstat instead of stat. If it's a
symbolic link, it's not a directory.
This is a subtle behavior change, but it's in line with docker build.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
When we attempt to remove a directory to make way for a non-directory as
part of extracting content, use RemoveAll() instead of Remove().
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
When processing a directory tree, only descend into a directory that is
marked for exclusion if its path is literally a prefix of an exception
pattern.
Subtly, but in a way that's compatible with docker, this means that if
we exclude directory "subdir", but we've been told to also include
"**/file" (with an exclusion pattern of "!**/file"), we won't descend
into "subdir" and find a file named "subdir/file", because "**/file"
doesn't start with "subdir/".
More generally, exclusion patterns that start with "!" which include any
wildcards before their final component technically won't be treated
correctly.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Set extended attributes on files _after_ setting their ownership, so
that security-sensitive attributes ("security.capability" among them)
won't get quietly cleared from under us if we set them before calling
chown().
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
During a copy operation which descends through a directory tree,
It's possible for a referenced file to become inaccessible (by unlink
or permission change or whatever). During the walk of paths to copy,
an `Lstat()` is run on each item, and any error passed into the handler
function to deal with. Subsequently, if there is no error, the file
is examined for inclusion/exclusion by the handler.
Unfortunately, this introduces a TOCTOU race condition for files which
become inaccessible even if they would otherwise be excluded. For
example a file or directory under /proc or /sys (which frequently and
unpredictably change). This was the original cause encountered during
podman integration testing.
It's impractical to actually fix this race at the file-level, without
introducing negative effects to any source-container operations. It's
also questionably useful to offer a command-line option to offload the
choice to the user. Instead, follow the behavior of the `tar` command
for this situation: Issue a warning to the user, and ignore the
problematic item (don't copy it).
Also add a test resembling the podman test which originally caught this
race. While not reliable, it does introduce a non-zero chance of
hitting the race condition - and handling the new warning properly.
Signed-off-by: Chris Evich <cevich@redhat.com>
In Docker if you are copying more then one object, and
one of them is successful, then the command is successful. Currently in
buildah each glob has to be successful. This PR matches Buildah to
Docker.
Fixes: https://github.com/containers/podman/issues/9594
Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
Add a NoCrossDevice flag to GetOptions, telling it to ignore
subdirectories on devices different than the top reference directory
that we start from, i.e., ignore the contents of mounted filesystems.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Add an IgnoreUnreadable flag to copier.GetOptions to suppress errors
from copier.Get() that would pass the os.IsPermission() test, if they're
encountered while attempting to read files or descend into directories.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Always create the destination directory first when ADDing or COPYing
content into a container, then extract contents into it using the
destination directory as the chroot instead of the container's root
directory.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Add a NoDerefSymlinks flag to force items that are matched to the Globs
we're given to be treated as symlinks, rather than dereferencing them as
we would need to do for sources for ADD or COPY.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Add copier.Eval(), for expanding paths using symbolic links in a
chrooted scope, without failing if a path component doesn't exist.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
When attempting to handle renames, we'd fail to correctly handle renames
of prefixes of a given item's path because of a string handling error,
and add a unit test for the rename logic (finally).
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
If the subprocess exits with an error, but we can't decode its stdout as
a proper status result, check if it produced error output. If it did,
then return its error output as the error.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Prior to 1.16, when ADDing contents to a working container, if we were
being run by an unprivileged user using a user namespace, content that
was a device node would be ignored.
Add a flag in copier.PutOptions that tells copier.Put() to ignore
entries that are either a device, or a hard link to a device.
Make buildah.Add() set the IgnoreDevices flag in PutOptions when
libcontainer says we're running in a user namespace.
Together, these two changes should restore the earlier behavior.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Fall back to non-chroot behavior on Unixy systems when we're not started
as UID 0. Break the unit tests that exercise chroot functionality into
a separate file so that we don't even try to run those cases on non-Unix
OSes.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Add a flag for controlling overwriting-directories-with-non-directories
behavior in PutOptions, and fields for controlling name mappings to both
GetOptions and PutOptions.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Improve handling of cases where extracting an archive requires us to
replace a directory with something that is not a directory, or vice-
versa:
* when replacing a directory with something that isn't a directory,
remove the directory even if it has contents
* don't fail when replacing something that isn't a directory with a
directory
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
When extracting archives, if we encounter an entry with a zero-length
name, skip it, instead of attempting to Join the entry's name with the
name of the parent directory where it should be placed (which just
yields the parent directory's name again), and attempting to create that
directory again, possibly as a file.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
When trying to read the list of extended attributes on content that
we're reading, if we get an ENOTSUP or ENOSYS error while attempting to
list the attributes, treat it as if we succeeded and got an empty list
of attributes.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Attempt to address CVE-2019-14271 by having the copier module attempt to
trigger loading of dynamic nsswitch modules, that it'll end up using
while chrooted, before it chroots into a target rootfs.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Ignore PAX global headers when extracting archives, like the archive
package does, instead of erroring out.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
When attempting to create a file as part of extracting a tar stream,
don't check if the number of bytes written that our helper function
(createFile) reports back to us matches our expectations if it also
reported an error, in which case the byte count is irrelevant.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
When a directly-named (or globbed) source directory for ADD or COPY is
marked for exclusion by .dockerignore, check if its name is a prefix for
any exceptions in the .dockerignore file, and if it is, check the
directory for things we need to include anyway.
This will miss exceptions where the pattern uses a wildcard for anything
but the final component.
When adding items, count items that are actually passed over the tar
pipe, rather than items scanned, so that we can correctly diagnose not
having found anything that we needed to copy under a directory that
would otherwise have been excluded.
In copierHandlerGet(), just don't discount any globbed directories that
are excluded.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
copier: when returning an error that we didn't copy any files, wrap that
messages a syscall.ENOENT rather than just returning our own text.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
In copierHandlerGet(), instead of waiting until after we've made another
function call and checked its error result, check the error value that
we're passed right away.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
When a subprocess exits successfully, log its stderr output at debug
priority instead of just discarding it.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
When ADD was used to ADD a directory, the contents of archives that we
found inside of it were incorrectly being expanded at the destination
for the ADD.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
When extracting archives that are added using ADD, don't override
permissions and ownership information. We regressed on this when we
switched to using the copier package to handle them.
Add a conformance test to prevent regressions on this.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
When adding a hard link entry to an archive, don't try to make the path
of the link's target name be relative to the link's location in the
filesystem if they're in the same directory, since receivers don't
interpret it that way.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Use the copier package to rework how we handle ADD and COPY.
When evaluating cache for content that's being copied/added in, switch
from (digest the data, check for a cache entry, then maybe copy the data
and create the new layer) to (copy the data and create the new layer,
digesting as we go, check for a cache entry, either commit or discard
the new layer).
Use the copier package for ADD, COPY, and for ensuring that a specified
directory exists in the working container's rootfs.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
The internal types don't need to be exported in order for us to be able
to encode them as JSON; the individual fields need to be, but not their
types.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Add a function for doing Mkdir-possibly-in-a-chroot, for ensuring that a
directory exists without having to possibly create it directly from
outside of a chroot.
Make use of filepath.ToSlash() and filepath.FromSlash() where it's
appropriate.
Add more unit tests.
Address some review comments:
* Check for ERANGE instead of E2BIG errors from llistxattr() and
lgetxattr() as indicators that the buffer we passed to them is too
small.
* Factor an `isRelevantXattr` helper out of Lgetxattrs() and
Lsetxattrs().
* Drop our getcwd() function in favor of using os.Getwd().
* Adjust the comment describing the GetOptions.KeepDirectoryNames field.
* Clean hdr.Name before attempting to compute where an item being
extracted should go.
* When writing items to the filesystem, create with 0?00 permissions,
set ownership, then set the correct permissions.
* Merge StatResponse.Error, GetResponse.Error, and PutResponse.Error
into Response.Error.
* When reading items from the filesystem, if a glob matches multiple
items, and one of the items is excluded, continue checking the other
items.
* Make sure we always Wait() on a child process if we spawned one.
* Clean up the cleanup logic for pipes that we use to communicate with a
child process.
* Clean up the kill-the-child-process logic we call when we encounter an
error communicating with the child process.
* Drop the separate Options structure, use helper methods to simplify
pulling out the right ID maps and exclusions list for the request.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
For the sake of conformance tests, callers need to be able to strip
setuid and setgid bits from contents being copied from the build context
while leaving the sticky bit intact. Split the StripSetidBits option
for copier.Get() into three separate flags (StripSetuidBit /
StripSetgidBit / StripStickyBit).
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Add new primitives for reading and writing data, represented as tar
streams, from and to possibly-chrooted directories via subprocesses.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Nalin Dahyabhai
Valentin Rothberg
OpenShift Merge Robot
Giuseppe Scrivano
Daniel J Walsh
Kir Kolyshkin
Chris Evich
Josh Soref
Hironori Shiina