--netns-only is supposed to make pasta use only a network namespace, not
a user namespace. However, pasta_start_ns() has this backwards, and if
--netns-only is specified it creates a user namespace but *not* a network
namespace. Correct this.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
conf_ns_open() opens file descriptors for the namespaces pasta needs, but
it doesnt really have anything to do with configuration any more. For
better clarity, move it to pasta.c and rename it pasta_open_ns(). This
makes the symmetry between it and pasta_start_ns() more clear, since these
represent the two basic ways that pasta can operate, either attaching to
an existing namespace/process or spawning a new one.
Since its no longer validating options, the errors it could return
shouldn't cause a usage message. Just exit directly with an error instead.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
There are a number of different ways to specify namespaces for pasta to
use. Some combinations are valid and some are not. Currently validation
for these is spread across several places: conf_ns_pid() validates PID
options specifically. Near its callsite in conf() several other checks
are made. Some additional checks are made in conf_ns_open() and finally
theres a check just before the call to pasta_start_ns().
This is quite hard to follow. Make it easier by putting all the validation
logic together in a new conf_pasta_ns() function, which subsumes
conf_ns_pid(). This reveals that some of the checks were redundant with
each other, so remove those.
For good measure, rename conf_netns() to conf_netns_opt() to make it
clearer its handling just the --netns option specifically, not overall
configuration of the netns.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
passt/pasta contains a number of routines designed to isolate passt from
the rest of the system for security. These are spread through util.c and
passt.c. Move them together into a new isolation.c file.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
passt is allowed to run as "root" (UID 0) in a user namespace, but notas
real root in the init namespace. We read /proc/self/uid_map to determine
if we're in the init namespace or not.
If we're unable to open /proc/self/uid_map we assume we're ok and
continue running as UID 0. This seems unwise. The only instances I
can think of where uid_map won't be available are if the host kernel
doesn't support namespaces, or /proc is not mounted. In neither case
is it safe to assume we're "not really" root and continue (although in
practice we'd likely fail for other reasons pretty soon anyway).
Therefore, fail with an error in this case, instead of carrying on.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Currently the logic to work out what UID and GID we will run as is spread
across conf(). If --runas is specified it's handled in conf_runas(),
otherwise it's handled by check_root(), which depends on initialization of
the uid and gid variables by either conf() itself or conf_runas().
Make this clearer by putting all the UID and GID logic into a single
conf_ugid() function.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
check_root() both checks to see if we are root (in the init namespace),
and if we are drops to an unprivileged user. To make future cleanups
simpler, split the checking for root (now in check_root()) from the actual
dropping of privilege (now in drop_root()).
Note that this does slightly alter semantics. Previously we would only
setuid() if we were originally root (in the init namespace). Now we will
always setuid() and setgid(), though it won't actually change anything if
we weren't privileged to begin with. This also means that we will now
always attempt to switch to the user specified with --runas, even if we
aren't (init namespace) root to begin with. Obviously this will fail with
an error if we weren't privileged to start with. --help and the man page
are updated accordingly.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
c->uid and c->gid are first set in conf(), and last used in check_root()
itself called from conf(). Therefore these don't need to be fields in the
long lived context structure and can instead be locals in conf().
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
test_iperf3() is a pretty inscrutable mess of nested background processes.
It has a number of ugly sleeps needed to wait for things to complete.
Rewrite it to be cleaner:
* Use the construct (a & b & wait) to run 'a' and 'b' in parallel, but
then wait for them both to complete before continuing
* This allows us to wait for both the server and client to finish, rather
than sleeping
* Use jq to do all the math we need to get the final result, rather than
jq followed by some complicated 'bc' mangling
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Currently all the throughput tests are run for 30s. This is reflected in
both the actual parameters given to the iperf commands, but also in the
matching sleeps in test_iperf3.
Allow this to be adjusted more easily with a new parameter to test_iperf3.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
[sbrivio: Reflect new parameter in comment to test_iperf3()]
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
These two commands in the DSL to run an iperf client and server are always
used together, and some of the parameters must match between them. The
iperf3s must also be run more or less immediately after iperf3c, since
iperf3c will run a client in the background after a sleep and requires a
server to be running before it will work.
A bunch of things can be made cleaner if we make a single DSL command that
runs both sides of the test. For now make the combined command work
exactly like the two commands together did, warts and all.
This does lose the ability for the DSL scripts to give additional options
to the iperf3 server, but we weren't using that anyway.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
The tests now use a number of pidfiles for qemu and pasta as well as
passt.pid. Broaden the .gitignore file so these aren't unintentially
committed.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
We don't set any, but we should use them if they are passed in the
environment. On a Fedora Rawhide package build, annocheck
(https://sourceware.org/annobin/) reports:
Hardened: /usr/bin/passt: FAIL: bind-now test because not linked with -Wl,-z,now
...despite the build system exporting -Wl,-z,now in LDFLAGS.
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
On new Ubuntu 22.04 images, stopping systemd-resolved to get the
dhclient script override resolv.conf doesn't work anymore. I
originally used that hack to avoid introducing a delay which is
needed when running it on TCG.
Keep systemd-resolved running instead, and wait for it to be ready
by retrying to resolve a domain a few times before installing
packages, so that we don't add another ugly delay that might
unnecessarily slow down things even further.
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
fedora-review says:
Note: Directories without known owners:
/usr/share/selinux/packages/passt, /usr/share/doc/passt,
/usr/share/selinux, /usr/share/selinux/packages
and selinux-policy owns those two last ones.
While at it, split Requires: tags also for post and preun actions
onto different lines, for consistency.
Reported-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
fedora-review says:
Note: No known owner of /usr/share/selinux/packages/passt,
/usr/share/doc/passt
While at it, replace "passt" by "%{name}" in a few places for
consistency.
Reported-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
Declaring them as required_argument in the longopts array specifies
validation, but doesn't affect how optind is increased after parsing
their values.
Currently, passing one of these options as last option causes pasta
to handle their own values as path to a binary to execute.
Fixes: aae2a9bbf7 ("conf: Use "-D none" and "-S none" instead of missing empty option arguments")
Fixes: bf95322fc1 ("conf: Make the argument to --pcap option mandatory")
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
Performance tests use iperf3(1) with large windows, and these sysctl
entries are needed to run them unmodified.
The passt demo uses perf(1) to report syscall overhead, and that
needs access to hardware performance counters for unprivileged
users.
Reported-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
...instead of PATH. This seems to be the only change needed in
existing pasta integrations after patch:
Use explicit --netns option rather than multiplexing with PID
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
When not given an existing PID or network namspace to attach to, pasta
spawns a shell. Most commands which can spawn a shell in an altered
environment can also run other commands in that same environment, which can
be useful in automation.
Allow pasta to do the same thing; it can be given an arbitrary command to
run in the network and user namespace which pasta creates. If neither a
command nor an existing PID or netns to attach to is given, continue to
spawn a default shell, as before.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
When attaching to an existing namespace, pasta can take a PID or the name
or path of a network namespace as a non-option parameter. We disambiguate
based on what the parameter looks like. Make this more explicit by using
a --netns option for explicitly giving the path or name, and treating a
non-option argument always as a PID.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
[sbrivio: Fix typo in man page]
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
pasta takes as its only non-option argument either a PID to attach to the
namespaces of, a PATH to a network namespace or a NAME of a network
namespace (relative to /run/netns). Currently to determine which it is
we try all 3 in that order, and if anything goes wrong we move onto the
next.
This has the potential to cause very confusing failure modes. e.g. if the
argument is intended to be a network namespace name, but a (non-namespace)
file of the same name exists in the current directory.
Make behaviour more predictable by choosing how to treat the argument based
only on the argument's contents, not anything else on the system:
- If it's a decimal integer treat it as a PID
- Otherwise, if it has no '/' characters, treat it as a netns name
(ip-netns doesn't allow '/' in netns names)
- Otherwise, treat it as a netns path
If you want to open a persistent netns in the current directory, you can
use './netns'.
This also allows us to split the parsing of the PID|PATH|NAME option from
the actual opening of the namespaces. In turn that allows us to put the
opening of existing namespaces next to the opening of new namespaces in
pasta_start_ns. That makes the logical flow easier to follow and will
enable later cleanups.
Caveats:
- The separation of functions mean we will always generate the basename
and dirname for the netns_quit system, even when using PID namespaces.
This is pointless, since the netns_quit system doesn't work for non
persistent namespaces, but is harmless.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
After calling conf_ns_opt() we check for -ENOENT and print an error
message, but conf_ns_opt() prints messages for other errors itself. For
consistency move the ENOENT message into conf_ns_opt() as well.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
pasta can identify a netns as a "name", which is to say a path relative to
(usually) /run/netns, which is the place that ip(8) creates persistent
network namespaces. Alternatively a full path to a netns can be given.
The --nsrun-dir option allows the user to change the standard path where
netns names are resolved. However, there's no real point to this, if the
user wants to override the location of the netns, they can just as easily
use the full path to specify the netns.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
The man page states that the --userns option can be given either as a path
or as a name relative to --nsrun-dir. This is not correct: as the name
suggests --nsrun-dir is (correctly) used only for *netns* resolution, not
*userns* resolution.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Both the -D (--dns) and -S (--search) options take an optional argument.
If the argument is omitted the option is disabled entirely. However,
handling the optional argument requires some ugly special case handling if
it's the last option on the command line, and has potential ambiguity with
non-option arguments used with pasta. It can also make it more confusing
to read command lines.
Simplify the logic here by replacing the non-argument versions with an
explicit "-D none" or "-S none".
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
[sbrivio: Reworked logic to exclude redundant/conflicting options]
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
The --pcap or -p option can be used with or without an argument. If given,
the argument gives the name of the file to save a packet trace to. If
omitted, we generate a default name in /tmp.
Generating the default name isn't particularly useful though, since making
a suitable name can easily be done by the caller. Remove this feature.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Fedora's parameters currently match the ones from the Makefile (which
is based on GNU recommendations), but that's not necessarily
guaranteed.
This should make the OpenSUSE Tumbleweed override for docdir
unnecessary: drop it.
Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
...as it's used twice. The short version, however, appears hardcoded
only once in the output, and it comes straight from the rpkg macro
building the version string -- leave that macro as it is.
Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
...which makes it fall under MIT licensing terms. Daniel reports that
it's very unusual for spec files to contain explicit licensing terms
and might cause minor inconveniences later on, on mass changes to
spec files.
I originally added licensing information using SPDX identifiers to
make the project fully compliant with the REUSE Specification 3.0
(https://reuse.software/spec/), but there are anyway a few more files
not including explicit licensing information. It might be worth to
fix that later on, in any case.
Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
Commit a951e0b9ef ("conf: Add --runas option, changing to given UID
and GID if started as root") dropped the call to initgroups() that
used to add supplementary groups corresponding to the user we'll
eventually run as -- we don't need those.
However, if the original user belongs to supplementary groups
(usually not the case, if started as root), we don't drop those,
now, and rpmlint says:
passt.x86_64: E: missing-call-to-setgroups-before-setuid /usr/bin/passt
passt.x86_64: E: missing-call-to-setgroups-before-setuid /usr/bin/passt.avx2
Add a call to setgroups() with an empty set, to drop any
supplementary group we might currently have, before changing GID
and UID.
Reported-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
There are several places in the passt code where we have lint overrides
because we're not adding CLOEXEC flags to open or other operations.
Comments suggest this is because it's before we fork() into the background
but we'll need those file descriptors after we're in the background.
However, as the name suggests CLOEXEC closes on exec(), not on fork(). The
only place we exec() is either super early invoke the avx2 version of the
binary, or when we start a shell in pasta mode, which certainly *doesn't*
require the fds in question.
Add the CLOEXEC flag in those places, and remove the lint overrides.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Looks like a copy-paste error where we're checking against the size of the
pcap field, rather than the sock_path field.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
It turns out that, while on most distributions "docdir" would be
/usr/share/doc, it's /usr/share/doc/packages/ on OpenSUSE Tumbleweed.
Use an explicit docdir as shown in:
https://en.opensuse.org/openSUSE:Build_Service_cross_distribution_howto
and don't unnecessarily hardcode directory variables in the Makefile.
Otherwise, RPM builds for OpenSUSE will fail now that we have a README
there.
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
We start getting prompts about restarting outdated services: we're
using daily images but they might have been cached for a while now.
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
Currently in at least some of the testcases we kill qemu processes we're
done with by issuing a Control-C to the tmux panel it's running in. That
makes things harder as we try to move towards allowing "headless" testing
without tmux.
So, instead always use an explicit kill on a pid derived from a pidfile
for killing qemu. Note that we don't need to remove the pidfiles
afterwards, because qemu does that itself when terminated.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
The test scripts run with sh -e, which means they will stop if any commands
return an error. That's generally desirable, because we won't continue
after things are hopeless due to an earlier step failing.
Unfortunately, the tmux setup we run the script in means it's not obvious
where any error messages related to such a failure will go. Depending on
exactly where the error occurs they might go to the original terminal
hidden behind tmux, or they might go to a tmux panel that's not visible in
the normal layouts.
To make it easier to find such error message, redirect direct output and
errors from the test script itself to a 'script.log' file in the logs
directory. When in DEBUG=1 mode, additionaly 'set -x' so we log all the
commands we execute to that file.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
For the passt and passt_in_ns tests we have a "shutdown" testcase that
checks for any errors from the passt process we were using (including
valgrind warnings). Do the same for pasta tests, so that we catch any
error codes from the pasta process.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
The "valgrind" test cases are designed to pick up errors reported when
passt is running under valgrind. But what it actually does is just kill
the passt process, then see if it had a non-zero exit code. That means it
will equally well pick up any other problems which caused passt to exit
with an error status: either something detected within passt or as a result
of passt being killed by an unexpected signal.
The fact that the "valgrind" test is actually responsible for shutting down
the passt process is non-obvious and can lead to problems when selectively
running tests during debugging.
Rename the "valgrind" tests to "shutdown" tests and run it regardless of
whether we're using valgrind or not. This allows us to remove an ugly
speacial case in the passt_in_ns teardown code.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
The queries we use in the test scripts to locate the external interface
or gateway can return multiple results. We get away with this because the
way we parse command output only looks at the last line. It's not really
correct, though, and improvements to our handling of command output will
mean it breaks.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Currently the build tests and distro tests share a common setup function.
That works for now, but changes we want to make will mean they need
slightly different setup, so split the setup functions in preparation.
Currently, neither build nor distro tests have any teardown function.
Again, future changes are going to mean we need to do some teardown, so
create some empty for now teardown functions in preparation.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
When recording tests or demos with asciinema we generate several temporary
files during post-processing. Add these to the .gitignore file so they're
not accidentally comitted.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
The DEMO_XTERM and CI_XTERM variables defined in test/lib/term aren't used
anywhere. Remove them.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Both clang-tidy and cppcheck linting are handled by the same test file,
test/build/static_checkers. The two linters are independent of each other
though, and each one takes quite a long time. Split them into separate
files to make it easier to control which are executed from the top level
test script.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
