We don't need to reinvent these, especially in a confusing form
(bool type like stdbool.h, but TRUE and FALSE constants like GLib).
stdbool.h was available in the gcc 4.6 that is the default compiler in
Ubuntu 12.04, more than a decade ago, so it seems sufficiently
ubiquitous.
Signed-off-by: Simon McVittie <smcv@collabora.com>
If a blocking operation is interrupted by a signal, including SIGCHLD,
various things can fail with EINTR. This is not a "real" error and can
result in spurious failures.
Resolves: https://github.com/containers/bubblewrap/issues/657
Signed-off-by: Simon McVittie <smcv@collabora.com>
This prepends a severity level such as <3> to each line of diagnostic
output, with numeric severity levels taken from matching syslog(3)
(such as LOG_ERR = 3), so that the diagnostic output can be parsed by
tools like `logger --prio-prefix` and `systemd-cat --level-prefix=1`
that support that encoding.
The facility (LOG_USER, etc.) is not included, since it makes little
sense to vary on a per-message basis. logger(1) supports prefixes
with or without a facility, and systemd-cat(1) only supports prefixes
without a facility, so this is compatible with both.
A future version of Steam's pressure-vessel is likely to use this to
make warnings and fatal errors from bubblewrap more visible.
Signed-off-by: Simon McVittie <smcv@collabora.com>
This takes a syslog-style severity level, allowing a larger program
that runs bwrap and reads a pipe from its stderr to filter or highlight
messages based on the severity.
Take the opportunity to make the __debug__ macro (which normally expands
to nothing, but can be enabled by changing a `#if 0` to `#if 1`) less
weird and easier to use, by taking it out of the reserved-for-the-compiler
namespace, adding a newline automatically, and not requiring nested
parentheses.
Signed-off-by: Simon McVittie <smcv@collabora.com>
This is useful for example if you for some reason don't have the real
path. It is also a way to make bind-mounts race-free (i.e. to have the
mount actually be the thing you wanted to be mounted, avoiding issues
where some other process replaces the target in parallel with the bwrap
launch.
Unfortunately due to some technical details we can't actually directly
mount the dirfd, as they come from different user namespace which is not
permitted, but at least we can delay resolving the fd to a path as much as
possible, and then validate after mount that we actually mounted the right
thing.
Signed-off-by: Alexander Larsson <alexl@redhat.com>
For some reason the second flags is "2<<0", but really flags should
be 1<<N, and in this case 1<<1. Both happen to be the same value, so its
not like this matter deeply, but lets fix it if we do later changes.
Signed-off-by: Alexander Larsson <alexl@redhat.com>
When creating symlinks specified with --symlink, check errno if
creation fails. If the reason was EEXIST, check the path of the
existing symlink. If it matches what was desired, continue as
normal.
Resolves: containers/bubblewrap#549
Signed-off-by: Jakob Kaivo <jkk@ung.org>
[Anders Blomdell: use readlink_malloc()]
[smcv: Squash multiple changes into one; coding style fixes]
Co-authored-by: Anders Blomdell
Co-authored-by: Simon McVittie <smcv@collabora.com>
Signed-off-by: Simon McVittie <smcv@collabora.com>
Fixes containers/bubblewrap#91
Add the ability to overwrite argv[0] when starting a process in a
container. Using --argv0 to be consistent with ld.so --argv0.
Overwriting argv[0] is useful as some tools change their behavior based
on the value of argv[0]. For example, when bash is symlinked to sh it
behaves as sh. Similarly, unxz is a symlink to xz and changes the
default from compressing to decompressing. An extreme example is on many
systems, date, df, cat and so on are all symlinks to the coreutils
binary.
Example usage: bwrap --bind / / --argv0 sh bash
Signed-off-by: Jonathan Wright <quaggy@gmail.com>
Found by running under pedantic UBSAN:
../bubblewrap.c:968:21: runtime error: implicit conversion from type 'int' of value -1 (32-bit, signed) to type 'uid_t' (aka 'unsigned int') changed the value to 4294967295 (32-bit, unsigned)
../bubblewrap.c:1210:28: runtime error: implicit conversion from type 'int' of value -41 (32-bit, signed) to type 'unsigned int' changed the value to 4294967255 (32-bit, unsigned)
../bubblewrap.c:1215:28: runtime error: implicit conversion from type 'int' of value -41 (32-bit, signed) to type 'unsigned int' changed the value to 4294967255 (32-bit, unsigned)
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Comparisson of different signedness can result in unexpected results due
to implicit conversions.
../network.c:81:34: warning: comparison of integer expressions of different signedness: ‘__u32’ {aka ‘unsigned int’} and ‘int’ [-Wsign-compare]
81 | if (rheader->nlmsg_seq != seq_nr)
| ^~
../network.c:83:34: warning: comparison of integer expressions of different signedness: ‘__u32’ {aka ‘unsigned int’} and ‘__pid_t’ {aka ‘int’} [-Wsign-compare]
83 | if (rheader->nlmsg_pid != getpid ())
| ^~
../bind-mount.c:268:17: warning: comparison of integer expressions of different signedness: ‘int’ and ‘unsigned int’ [-Wsign-compare]
268 | assert (i < n_lines);
| ^
../bind-mount.c:309:13: warning: comparison of integer expressions of different signedness: ‘int’ and ‘unsigned int’ [-Wsign-compare]
309 | assert (i == n_lines);
| ^~
../bind-mount.c:318:17: warning: comparison of integer expressions of different signedness: ‘int’ and ‘unsigned int’ [-Wsign-compare]
318 | for (i = 0; i < n_lines; i++)
| ^
../bind-mount.c:321:17: warning: comparison of integer expressions of different signedness: ‘int’ and ‘unsigned int’ [-Wsign-compare]
321 | for (i = 0; i < n_lines; i++)
| ^
../utils.c:818:19: warning: comparison of integer expressions of different signedness: ‘size_t’ {aka ‘long unsigned int’} and ‘ssize_t’ {aka ‘long int’} [-Wsign-compare]
818 | while (size - 2 < n);
| ^
../bubblewrap.c:489:13: warning: comparison of integer expressions of different signedness: ‘int’ and ‘long unsigned int’ [-Wsign-compare]
489 | assert (j < sizeof(dont_close)/sizeof(*dont_close));
| ^
../bubblewrap.c:994:25: warning: comparison of integer expressions of different signedness: ‘int’ and ‘uid_t’ {aka ‘unsigned int’} [-Wsign-compare]
994 | if (setfsuid (-1) != real_uid)
| ^~
../bubblewrap.c:1042:61: warning: comparison of integer expressions of different signedness: ‘ssize_t’ {aka ‘long int’} and ‘size_t’ {aka ‘long unsigned int’} [-Wsign-compare]
1042 | if (write (privileged_op_socket, buffer, buffer_size) != buffer_size)
| ^~
../bubblewrap.c:1232:25: warning: comparison of integer expressions of different signedness: ‘int’ and ‘long unsigned int’ [-Wsign-compare]
1232 | for (i = 0; i < N_ELEMENTS (cover_proc_dirs); i++)
| ^
../bubblewrap.c:1260:25: warning: comparison of integer expressions of different signedness: ‘int’ and ‘long unsigned int’ [-Wsign-compare]
1260 | for (i = 0; i < N_ELEMENTS (devnodes); i++)
| ^
../bubblewrap.c:1272:25: warning: comparison of integer expressions of different signedness: ‘int’ and ‘long unsigned int’ [-Wsign-compare]
1272 | for (i = 0; i < N_ELEMENTS (stdionodes); i++)
| ^
../bubblewrap.c: In function ‘read_priv_sec_op’:
../bubblewrap.c:1556:15: warning: comparison of integer expressions of different signedness: ‘ssize_t’ {aka ‘long int’} and ‘long unsigned int’ [-Wsign-compare]
1556 | if (rec_len < sizeof (PrivSepOp))
| ^
../bubblewrap.c:1626:28: warning: comparison of integer expressions of different signedness: ‘int’ and ‘uint32_t’ {aka ‘unsigned int’} [-Wsign-compare]
1626 | if (*total_parsed_argc_p > MAX_ARGS)
| ^
../bubblewrap.c:1681:40: warning: comparison of integer expressions of different signedness: ‘int’ and ‘uint32_t’ {aka ‘unsigned int’} [-Wsign-compare]
1681 | if (*total_parsed_argc_p > MAX_ARGS)
| ^
../bubblewrap.c:2265:31: warning: comparison of integer expressions of different signedness: ‘uid_t’ {aka ‘unsigned int’} and ‘int’ [-Wsign-compare]
2265 | if (opt_sandbox_uid != -1)
| ^~
../bubblewrap.c:2285:31: warning: comparison of integer expressions of different signedness: ‘gid_t’ {aka ‘unsigned int’} and ‘int’ [-Wsign-compare]
2285 | if (opt_sandbox_gid != -1)
| ^~
../bubblewrap.c:2678:23: warning: comparison of integer expressions of different signedness: ‘uid_t’ {aka ‘unsigned int’} and ‘int’ [-Wsign-compare]
2678 | if (opt_sandbox_uid == -1)
| ^~
../bubblewrap.c:2680:23: warning: comparison of integer expressions of different signedness: ‘gid_t’ {aka ‘unsigned int’} and ‘int’ [-Wsign-compare]
2680 | if (opt_sandbox_gid == -1)
| ^~
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
General-purpose desktop distributions are compiled with CONFIG_SECCOMP
and CONFIG_SECCOMP_FILTER, but vendor kernels for phones and other
assorted embedded devices don't necessarily enable these options. These
kernels are unsuitable for running Flatpak, or anything else that relies
on `bwrap --seccomp` or `bwrap --add-seccomp-fd`.
Missing CONFIG_SECCOMP or CONFIG_SECCOMP_FILTER is not the *only* reason
why we could get EINVAL here: I think we'd also get EINVAL if the seccomp
program is syntatically invalid. However, it's a relatively likely reason,
so it seems worth providing a hint.
Helps: flatpak/flatpak#3069
Signed-off-by: Simon McVittie <smcv@collabora.com>
We can't combine --disable-userns with entering an existing user
namespace via --userns if the existing user namespace was created with
--disable-userns, because its ability to create nested user namespaces
has already been disabled. However, the next best thing is to verify
that we are already in the desired state.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Some use-cases of bubblewrap want to ensure that the subprocess can't
further re-arrange the filesystem namespace, or do other more complex
namespace modification. For example, Flatpak wants to prevent sandboxed
processes from altering their /proc/$pid/root/.flatpak-info, so that
/.flatpak-info can safely be used as an indicator that a process is part
of a Flatpak app.
This approach was suggested by lukts30 on containers/bubblewrap#452.
The sysctl-controlled maximum numbers of namespaces are themselves
namespaced, so we can disable nested user namespaces by setting the
limit to 1 and then entering a new, nested user namespace. The resulting
process loses its privileges in the namespace where the limit was set
to 1, so it is unable to move the limit back up.
Co-authored-by: Alexander Larsson <alexl@redhat.com>
Signed-off-by: Simon McVittie <smcv@collabora.com>
Prompted by flatpak/flatpak#4731, in which a misconfigured SMB automount
was failing to be remounted with ENODEV. This would have been easier to
debug if we knew which path could not be remounted.
Signed-off-by: Simon McVittie <smcv@collabora.com>
I'm about to add a third linked list, for seccomp programs, which would
seem like too much duplication.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Unfortunately it's possible for argc to be 0, so error out pretty early
on in that case. I don't think this is a security issue in this case.
Signed-off-by: Phaedrus Leeds <mwleeds@protonmail.com>
When building with -Wjump-misses-init as part of a larger project, gcc
reports that we jump past initialization of cover_proc_dirs. This is
technically true, but we only use this variable in the case where it's
initialized, so that's harmless.
However, we can avoid this altogether by making the array static and
constant, which allows it to be moved from initialized data to read-only
data.
Signed-off-by: Simon McVittie <smcv@collabora.com>
This is a step towards REUSE compliance. Third-party files that we do
not otherwise edit (git.mk, m4/attributes.m4) are excluded here.
Signed-off-by: Simon McVittie <smcv@collabora.com>
This gives us better diagnostic messages on failure, particularly for
BIND_MOUNT_ERROR_FIND_DEST_MOUNT where we previously said "Invalid
argument".
Signed-off-by: Simon McVittie <smcv@collabora.com>
This makes it clearer that we have thought about whether they need
symlinks for source resolved, and concluded that they do not.
Signed-off-by: Simon McVittie <smcv@collabora.com>
This allows environment variables to be set when running bwrap itself
(perhaps a custom LD_LIBRARY_PATH), but cleared for the command that
runs in the container, without having to enumerate all the variables.
Because PWD is set later, as a side-effect of changing directory, this
actually clears everything except PWD.
A portable program would check for clearenv() (and if not found, fall
back to using environ = NULL), but bubblewrap is Linux-specific, and
Linux C libraries (at least glibc and musl) do have clearenv().
Signed-off-by: Simon McVittie <smcv@collabora.com>
If two mount namespaces can both see a directory, and we bind-mount a
non-directory into that directory, we have to create a non-directory
to mount it onto:
$ ls -l ~/tmp/mountpoint
ls: cannot access '/home/smcv/tmp/mountpoint': No such file or directory
$ bwrap --bind / / --bind /etc/os-release ~/tmp/mountpoint true
$ ls -l ~/tmp/mountpoint
-rw-rw-rw- 1 smcv smcv 0 Feb 16 10:27 /home/smcv/tmp/mountpoint
The mount point is currently created as an empty world-writable file,
but this doesn't seem like a least-astonishment thing to do.
Create it with read-only permissions instead, to make it clearer that
it's just a placeholder and prevent other users from filling it.
Resolves: https://github.com/containers/bubblewrap/issues/337
Signed-off-by: Simon McVittie <smcv@collabora.com>
If we are using a case-insensitive filesystem the bind-mount operation
might fail when `/proc/self/mountinfo` is checked.
In a case-insensitive filesystem, if we ask to mount a certain
directory, e.g. '/CI_fs/foo', the kernel might add its entry in
`mountinfo` as '/CI_fs/FOO'. This happens because the kernel populates
`mountinfo` with whatever case combination first appeared in the dcache.
With this patch we open the requested path and look at its
`/proc/self/fd`, using readlink(), to get the path case combination that
the kernel is also expected to be using.
Signed-off-by: Ludovico de Nittis <ludovico.denittis@collabora.com>
