Before #5083, when running with chroot isolation ro mounts like secrets
from env vars would explicitly have the unix.MS_NOEXEC, unix.MS_NOSUID
and unix.MS_NODEV flags set when they were remounted. Now when running
with chroot isolation ro mounts like secrets from env vars are not
getting those same flags set and so the remount operation fails.
Specifically it looks like we are missing the unix.MS_NOSUID and
unix.MS_NODEV flags.
This change adds special handling for read-only mounts when we need to do
a remount to try to get the desired flags to stick. If we've requested
a read-only mount (unix.ST_RDONLY is set in requestFlags), then we add any
possibleImportantFlags that are set in fs.Flags to remountFlags so the remount
operation doesn't fail because they are missing. I've also added a test to
bud.bats that covers this case.
Signed-off-by: Jonah Bull <jonah.bull@elastic.co>
... in an attempt to try to get UID 0 in a user namespace to stop trying
to read files from root's home directory, where the permissions error is
treated as a hard failure.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Pay better attention to dev/nodev/exec/noexec/suid/nosuid/ro/rw flags on
bind, overlay, and tmpfs mounts when any of them are specified. Stop
quietly adding "nodev" when it isn't asked for.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
In golang 1.19, `io/ioutil` is fully deprecated preventing Buildah from
compiling. Replace all calls with equivalent calls from the `os`
package.
Signed-off-by: Chris Evich <cevich@redhat.com>
a bind mount cannot be made RDONLY in the same mount operation as it
is created. For that we need a second operation.
Closes: https://github.com/containers/buildah/issues/4203
[NO NEW TESTS NEEDED] it fails in Buildah in a container
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Podman adds an Error: to every error message. So starting an error
message with "error" ends up being reported to the user as
Error: error ...
This patch removes the stutter.
Also ioutil.ReadFile errors report the Path, so wrapping the err message
with the path causes a stutter.
Signed-off-by: Daniel J Walsh dwalsh@redhat.com
[NO NEW TESTS NEEDED]
Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
This adds no-op stubs for various things and adds an optional override
for creating the container chroot - on FreeBSD we use a jail to allow
setting the container hostname.
Signed-off-by: Doug Rabson <dfr@rabson.org>
This intentionally breaks the FreeBSD so that I can move the code
unmodified which will help with future merge conflicts. A subsequent
commit will resolve this by factoring out Linux-specific code.
Signed-off-by: Doug Rabson <dfr@rabson.org>
This leaves runUsingChrootSubprocOptions in the platform-specific file
since syscall.SysProcIDMap isn't available on FreeBSD.
Signed-off-by: Doug Rabson <dfr@rabson.org>
This copies a large amount of code from run_linux.go. Later diffs in
this stack will factor out duplicate code where possible.
Terminal handling is implemented using the posix_openpt API. We could
use the Linux implementation which uses the /dev/ptmx but that is not
present on standard FreeBSD installs - its supplied by an optional
kernel module. Conversely, posix_openpt could be used for both platforms
but has a downside of requiring cgo so its probably better to use this
only on FreeBSD.
Signed-off-by: Doug Rabson <dfr@rabson.org>
This commit replaces `ioutil.TempDir` with `t.TempDir` in tests. The
directory created by `t.TempDir` is automatically removed when the test
and all its subtests complete.
Prior to this commit, temporary directory created using `ioutil.TempDir`
needs to be removed manually by calling `os.RemoveAll`, which is omitted
in some tests. The error handling boilerplate e.g.
defer func() {
if err := os.RemoveAll(dir); err != nil {
t.Fatal(err)
}
}
is also tedious, but `t.TempDir` handles this for us nicely.
Reference: https://pkg.go.dev/testing#T.TempDir
Signed-off-by: Eng Zer Jun <engzerjun@gmail.com>
If errors for which os.IsExist() or os.IsNotExist() would have returned
true have been wrapped using fmt.Errorf()'s "%w" verb, os.IsExist() and
os.IsNotExist(), not having been retrofitted to use errors.Is(), will
return false.
Use errors.Is() to check if an error is an os.ErrExist or os.ErrNotExist
error instead of calling os.IsExist() or os.IsNotExist().
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
We now use the golang error wrapping format specifier `%w` instead of
the deprecated github.com/pkg/errors package.
Signed-off-by: Sascha Grunert <sgrunert@redhat.com>
Restore setting of the parent-death signal when we're running
subprocesses in Run(), so that if we get killed, the child processes
will also get killed.
While a child process is running, if we receive SIGHUP, SIGINT, or
SIGTERM, forward the signal to our child process unless it's the command
we're executing, which we SIGKILL without mercy, and finish the current
routine, which will then notice that the child process has exited and
return an error to its caller.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
The kernel never sets the inheritable capabilities for a process, they
are only set by userspace. Emulate the same behavior.
Closes: CVE-2022-27651
Signed-off-by: Giuseppe Scrivano <gscrivan@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 we link our test helper statically using the external linker, the
hardwired default seccomp filter we get from the runtime-tools generator
triggers a hang in it at startup.
Rather than switch to the internal linker, which seems to work around
this, start using the same seccomp filter for unit tests that we
actually use in real life, leaving analysis of which difference between
the two is responsible for it for another day.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
crun and runc accept a "rw" option, so we should expect to see it from
time to time. Recognize "dev", "suid", and "exec", too.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
The golang.org/x/crypto/ssh/terminal package has been deprecated and
replaced upstream by golang.org/x/term, so switch to that. It's a
simple 1:1 replacement.
[NO NEW TESTS NEEDED]
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Blake Burkhart reports that when running processes using "chroot"
isolation, the process being run can examine the environment of its
immediate parent and grandparent processes (CVE-2021-3602).
When run in a container in a CI/CD environment, the environment may
include sensitive information which was shared with the container in
order to be used only by buildah itself. The command being executed is
able to read such information.
This patch reduces the set of environment variables passed to these
intermediate processes, from all variables to the one which is used to
control the level of debug logging. It also corrects a misleading debug
message and expands the description of chroot isolation in man pages.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Check if a builtin volume is a symlink. If it is,
follow the symlink and ensure that the destination
exists.
Add tests for symlink and no symlink case.
Signed-off-by: Urvashi Mohnani <umohnani@redhat.com>
If you are in a rootless environment using chroot builds, you are
likely to get failures when mounting /sys file systems onto your
container. The problem is certain directories are not able to be
mounted on by rootless users. Since we are logging at Warn level
now, and users can not do anything to fix this situation, I am
dropping this message to info.
Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
It should work fine on linux and not linux boxes. Since there
is no glibc added, we can safely compile and run this code
on non SELinux boxes.
Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
2644: chroot: fix handling of errno seccomp rules r=rhatdan a=nalind
#### What type of PR is this?
/kind bug
#### What this PR does / why we need it:
When converting seccomp rules from the runtime spec to the structure that we can feed to libseccomp, combine the prescribed errno value with the action when we're mapping the "return an errno" action from one to the other.
#### How to verify it
Currently, chroot isolation hits an error processing this seccomp rule:
```
{
"names": [
"socket"
],
"action": "SCMP_ACT_ERRNO",
"args": [
{
"index": 0,
"value": 16,
"valueTwo": 0,
"op": "SCMP_CMP_EQ"
},
{
"index": 2,
"value": 9,
"valueTwo": 0,
"op": "SCMP_CMP_EQ"
}
],
"comment": "",
"includes": {},
"excludes": {
"caps": [
"CAP_AUDIT_WRITE"
]
},
"errnoRet": 22
},
```
on Fedora 33.
#### Which issue(s) this PR fixes:
None
#### Special notes for your reviewer:
Definitely going to need to backport this to older branches.
#### Does this PR introduce a user-facing change?
```
None
```
Co-authored-by: Nalin Dahyabhai <nalin@redhat.com>
Create the target mountpoints for bind mounts, when they don't already
exist, with 0755 permissions, for better consistency with runc.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
When converting seccomp rules from the runtime spec to the structure
that we can feed to libseccomp, combine the prescribed errno value with
the action when we're mapping the "return an errno" action from one to
the other.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
if setgroups is blocked to set up the user namespace, do not attempt
to use it to clear the additional groups.
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Jonah Bull
Giuseppe Scrivano
Nalin Dahyabhai
Philip Dubé
Michal Biesek
Chris Evich
Daniel J Walsh
Doug Rabson
Eng Zer Jun
Sascha Grunert
dependabot[bot]
Ruben Jenster
Kir Kolyshkin
Urvashi Mohnani
OpenShift Merge Robot
bors[bot]