aboutgitcodebugslistschat
path: root/conf.c
Commit message (Collapse)AuthorAgeFilesLines
...
* conf, udp: Drop mostly duplicated dns_send arrays, rename related fieldsStefano Brivio2022-11-161-43/+33
| | | | | | | | | | | | | | | | | | | | | Given that we use just the first valid DNS resolver address configured, or read from resolv.conf(5) on the host, to forward DNS queries to, in case --dns-forward is used, we don't need to duplicate dns[] to dns_send[]: - rename dns_send[] back to dns[]: those are the resolvers we advertise to the guest/container - for forwarding purposes, instead of dns[], use a single field (for each protocol version): dns_host - and rename dns_fwd to dns_match, so that it's clear this is the address we are matching DNS queries against, to decide if they need to be forwarded Suggested-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>
* conf: Fix mask calculation from prefix_len in conf_print()2022_11_10.4129764Stefano Brivio2022-11-101-1/+4
| | | | | | | Reported-by: Paul Holzinger <pholzing@redhat.com> Fixes: dd09cceaee21 ("Minor improvements to IPv4 netmask handling") Signed-off-by: Stefano Brivio <sbrivio@redhat.com> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
* conf: Split the notions of read DNS addresses and offered onesStefano Brivio2022-11-041-14/+35
| | | | | | | | | | | | | | | | | | | | | | | | | | | With --dns-forward, if the host has a loopback address configured as DNS server, we should actually use it to forward queries, but, if --no-map-gw is passed, we shouldn't offer the same address via DHCP, NDP and DHCPv6, because it's not going to be reachable. Problematic configuration: * systemd-resolved configuring the usual 127.0.0.53 on the host: we read that from /etc/resolv.conf * --dns-forward specified with an unrelated address, for example 198.51.100.1 We still want to forward queries to 127.0.0.53, if we receive one directed to 198.51.100.1, so we can't drop 127.0.0.53 from our list: we want to use it for forwarding. At the same time, we shouldn't offer 127.0.0.53 to the guest or container either. With this change, I'm only covering the case of automatically configured DNS servers from /etc/resolv.conf. We could extend this to addresses configured with command-line options, but I don't really see a likely use case at this point. Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* conf: Adjust netmask on mismatch between IPv4 address/netmask and gatewayStefano Brivio2022-11-041-1/+24
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Seen in a Google Compute Engine environment with a machine configured via cloud-init-dhcp, while testing Podman integration for pasta: the assigned address has a /32 netmask, and there's a default route, which can be added on the host because there's another route, also /32, pointing to the default gateway. For example, on the host: ip -4 address add 10.156.0.2/32 dev eth0 ip -4 route add 10.156.0.1/32 dev eth0 ip -4 route add default via 10.156.0.1 This is not a valid configuration as far as I can tell: if the address is configured as /32, it shouldn't be used to reach a gateway outside its derived netmask. However, Linux allows that, and everything works. The problem comes when pasta --config-net sources address and default route from the host, and it can't configure the route in the target namespace because the gateway is invalid. That is, we would skip configuring the first route in the example, which results in the equivalent of doing: ip -4 address add 10.156.0.2/32 dev eth0 ip -4 route add default via 10.156.0.1 where, at this point, 10.156.0.1 is unreachable, and hence invalid as a gateway. Sourcing more routes than just the default is doable, but probably undesirable: pasta users want to provide connectivity to a container, not reflect exactly whatever trickery is configured on the host. Add a consistency check and an adjustment: if the configured default gateway is not reachable, shrink the given netmask until we can reach it. Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* Use typing to reduce chances of IPv4 endianness errorsDavid Gibson2022-11-041-26/+29
| | | | | | | | | | | | | | | | | | | We recently corrected some errors handling the endianness of IPv4 addresses. These are very easy errors to make since although we mostly store them in network endianness, we sometimes need to manipulate them in host endianness. To reduce the chances of making such mistakes again, change to always using a (struct in_addr) instead of a bare in_addr_t or uint32_t to store network endian addresses. This makes it harder to accidentally do arithmetic or comparisons on such addresses as if they were host endian. We introduce a number of IN4_IS_ADDR_*() helpers to make it easier to directly work with struct in_addr values. This has the additional benefit of making the IPv4 and IPv6 paths more visually similar. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* Use IPV4_IS_LOOPBACK more widelyDavid Gibson2022-11-041-4/+4
| | | | | | | | | | | | | This macro checks if an IPv4 address is in the loopback network (127.0.0.0/8). There are two places where we open code an identical check, use the macro instead. There are also a number of places we specifically exclude the loopback address (127.0.0.1), but we should actually be excluding anything in the loopback network. Change those sites to use the macro as well. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* Minor improvements to IPv4 netmask handlingDavid Gibson2022-11-041-24/+39
| | | | | | | | | | | | | | | | There are several minor problems with our parsing of IPv4 netmasks (-n). First, we don't reject nonsensical netmasks like 0.255.0.255. Address this structurally by using prefix length instead of netmask as the primary variable, only converting (and validating) when we need to. This has the added benefit of making some things more uniform with the IPv6 path. Second, when the user specifies a prefix length, we truncate the output from strtol() to an integer, which means we would treat -n 4294967320 as valid (equivalent to 24). Fix types to check for this. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* Correct some missing endian conversions of IPv4 addressesDavid Gibson2022-11-041-13/+13
| | | | | | | | | | | | | | | | The INADDR_LOOPBACK constant is in host endianness, and similarly the IN_MULTICAST macro expects a host endian address. However, there are some places in passt where we use those with network endian values. This means that passt will incorrectly allow you to set 127.0.0.1 or a multicast address as the guest address or DNS forwarding address. Add the necessary conversions to correct this. INADDR_ANY and INADDR_BROADCAST logically behave the same way, although because they're palindromes it doesn't have an effect in practice. Change them to be logically correct while we're there, though. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* conf, passt.1: Don't imply --foreground with --debugStefano Brivio2022-10-271-4/+3
| | | | | | | | | | Having -f implied by -d (and --trace) usually saves some typing, but debug mode in background (with a log file) is quite useful if pasta is started by Podman, and is probably going to be handy for passt with libvirt later, too. Signed-off-by: Stefano Brivio <sbrivio@redhat.com> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
* conf: Don't pass leading ~ to parse_port_range() on exclusions2022_10_24.c11277bStefano Brivio2022-10-241-0/+1
| | | | | | | | | | | | | | | | | | | Commit 84fec4e998b6 ("Clean up parsing of port ranges") drops the strspn() call before the parsing of excluded port ranges, because now we're checking against any stray characters at every step. However, that also has the effect of passing ~ as first character to the new parse_port_range(), which makes no sense: we already checked that ~ is the first character before the call, so skip it. Alona reported this output: Invalid port specifier ~15000,~15001,~15006,~15008,~15020,~15021,~15090 while the whole specifier is indeed valid. Reported-by: Alona Paz <alkaplan@redhat.com> Fixes: 84fec4e998b6 ("Clean up parsing of port ranges") Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* conf: Bind inbound ports with CAP_NET_BIND_SERVICE before isolate_user()Stefano Brivio2022-10-151-36/+35
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Even if CAP_NET_BIND_SERVICE is granted, we'll lose the capability in the target user namespace as we isolate the process, which means we're unable to bind to low ports at that point. Bind inbound ports, and only those, before isolate_user(). Keep the handling of outbound ports (for pasta mode only) after the setup of the namespace, because that's where we'll bind them. To this end, initialise the netlink socket for the init namespace before isolate_user() as well, as we actually need to know the addresses of the upstream interface before binding ports, in case they're not explicitly passed by the user. As we now call nl_sock_init() twice, checking its return code from conf() twice looks a bit heavy: make it exit(), instead, as we can't do much if we don't have netlink sockets. While at it: - move the v4_only && v6_only options check just after the first option processing loop, as this is more strictly related to option parsing proper - update the man page, explaining that CAP_NET_BIND_SERVICE is *not* the preferred way to bind ports, because passt and pasta can be abused to allow other processes to make effective usage of it. Add a note about the recommended sysctl instead - simplify nl_sock_init_do() now that it's called once for each case Reported-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* isolation: Only configure UID/GID mappings in userns when spawning shellDavid Gibson2022-10-151-1/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When in passt mode, or pasta mode spawning a command, we create a userns for ourselves. This is used both to isolate the pasta/passt process itself and to run the spawned command, if any. Since eed17a47 "Handle userns isolation and dropping root at the same time" we've handled both cases the same, configuring the UID and GID mappings in the new userns to map whichever UID we're running as to root within the userns. This mapping is desirable when spawning a shell or other command, so that the user gets a root shell with reasonably clear abilities within the userns and netns. It's not necessarily essential, though. When not spawning a shell, it doesn't really have any purpose: passt itself doesn't need to be root and can operate fine with an unmapped user (using some of the capabilities we get when entering the userns instead). Configuring the uid_map can cause problems if passt is running with any capabilities in the initial namespace, such as CAP_NET_BIND_SERVICE to allow it to forward low ports. In this case the kernel makes files in /proc/pid owned by root rather than the starting user to prevent the user from interfering with the operation of the capability-enhanced process. This includes uid_map meaning we are not able to write to it. Whether this behaviour is correct in the kernel is debatable, but in any case we might as well avoid problems by only initializing the user mappings when we really want them. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* isolation: Replace drop_caps() with a version that actually does somethingDavid Gibson2022-10-151-1/+1
| | | | | | | | | | | | | | | | | | | | The current implementation of drop_caps() doesn't really work because it attempts to drop capabilities from the bounding set. That's not the set that really matters, it's about limiting the abilities of things we might later exec() rather than our own capabilities. It also requires CAP_SETPCAP which we won't usually have. Replace it with a new version which uses setcap(2) to drop capabilities from the effective and permitted sets. For now we leave the inheritable set as is, since we don't want to preclude the user from passing inheritable capabilities to the command spawed by pasta. Correctly dropping caps reveals that we were relying on some capabilities we'd supposedly dropped. Re-divide the dropping of capabilities between isolate_initial(), isolate_user() and isolate_prefork() to make this work. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* conf: Drop excess colons in usage for DHCP and DNS optionsStefano Brivio2022-10-151-4/+4
| | | | | Signed-off-by: Stefano Brivio <sbrivio@redhat.com> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
* conf: Report usage for --no-netns-quitStefano Brivio2022-10-151-0/+2
| | | | | | Fixes: 745a9ba4284c ("pasta: By default, quit if filesystem-bound net namespace goes away") Signed-off-by: Stefano Brivio <sbrivio@redhat.com> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
* conf, tcp, udp: Allow specification of interface to bind toStefano Brivio2022-10-151-10/+21
| | | | | | | | | | | | | | | | Since kernel version 5.7, commit c427bfec18f2 ("net: core: enable SO_BINDTODEVICE for non-root users"), we can bind sockets to interfaces, if they haven't been bound yet (as in bind()). Introduce an optional interface specification for forwarded ports, prefixed by %, that can be passed together with an address. Reported use case: running local services that use ports we want to have externally forwarded: https://github.com/containers/podman/issues/14425 Signed-off-by: Stefano Brivio <sbrivio@redhat.com> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
* conf, tap: Add option to quit once the client closes the connectionStefano Brivio2022-10-151-1/+16
| | | | | | | | This is practical to avoid explicit lifecycle management in users, e.g. libvirtd, and is trivial to implement. Signed-off-by: Stefano Brivio <sbrivio@redhat.com> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
* conf, log, Makefile: Add versioning informationStefano Brivio2022-10-151-0/+8
| | | | | | | Add a --version option displaying that, and also include this information in the log files. Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* log, conf: Add support for logging to fileStefano Brivio2022-10-141-3/+46
| | | | | | | | | | | | | | | | | | | | | | In some environments, such as KubeVirt pods, we might not have a system logger available. We could choose to run in foreground, but this takes away the convenient synchronisation mechanism derived from forking to background when interfaces are ready. Add optional logging to file with -l/--log-file and --log-size. Unfortunately, this means we need to duplicate features that are more appropriately implemented by a system logger, such as rotation. Keep that reasonably simple, by using fallocate() with range collapsing where supported (Linux kernel >= 3.15, extent-based ext4 and XFS) and falling back to an unsophisticated block-by-block moving of entries toward the beginning of the file once we reach the (mandatory) size limit. While at it, clarify the role of LOG_EMERG in passt.c. Signed-off-by: Stefano Brivio <sbrivio@redhat.com> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
* conf: Drop duplicate, diverging optstring assignmentsStefano Brivio2022-10-141-14/+6
| | | | | | | | | | | | | This originated as a result of copy and paste to introduce a second stage for processing options related to port forwarding, has already bitten David in the past, and just gave me hours of fun. As a matter of fact, the second set of optstring assignments was already incorrect, but it didn't matter because the first one was more restrictive, not allowing optional arguments for -P, -D, -S. Signed-off-by: Stefano Brivio <sbrivio@redhat.com> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
* Move logging functions to a new file, log.cStefano Brivio2022-10-141-0/+1
| | | | | | | | Logging to file is going to add some further complexity that we don't want to squeeze into util.c. Signed-off-by: Stefano Brivio <sbrivio@redhat.com> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
* cppcheck: Use inline suppression for strtok() in conf.cDavid Gibson2022-09-291-0/+2
| | | | | | | | | | | strtok() is non-reentrant and old-fashioned, so cppcheck would complains about its use in conf.c if it weren't suppressed. We're single threaded and strtok() is convenient though, so it's not really worth reworking at this time. Convert this to an inline suppression so it's adjacent to the code its annotating. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* Don't shadow 'i' in conf_ports()David Gibson2022-09-291-2/+5
| | | | | | | | | | | | | | The counter 'i' is used in a number of places in conf_ports(), but in one of those we unnecessarily shadow it in an inner scope. We could re-use the same 'i' every time, but each use is logically separate, so instead remove the outer declaration and declare it locally in each of the clauses where we need it. While we're there change it from a signed to unsigned int, since it's used to iterate over port numbers which are generally treated as unsigned. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* Clean up parsing in conf_runas()David Gibson2022-09-291-45/+50
| | | | | | | | | | | | | | | | | | | | | | | | | conf_runas() handles several of the different possible cases for the --runas argument in a slightly odd order. Although it can parse both numeric UIDs/GIDs and user/group names, it can't parse a numeric UID combined with a group name or vice versa. That's not obviously useful, but it's slightly surprising gap to have. Rework the parsing to be more systematic: first split the option into user and (optional) group parts, then separately parse each part as either numeric or a name. As a bonus this removes some clang-tidy warnings. While we're there also add cppcheck suppressions for getpwnam() and getgrnam(). It complains about those because they're not reentrant. passt is single threaded though, and is always likely to be during this initialization code, even if we multithread later. There were some existing suppressions for these in the cppcheck invocation but they're no longer up to date. Replace them with inline suppressions which, being next to the code, are more likely to stay correct. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* Clean up parsing of port rangesDavid Gibson2022-09-291-140/+102
| | | | | | | | | | | | | | | | | | | | conf_ports() parses ranges of ports for the -t, -u, -T and -U options. The code is quite difficult to the follow, to the point that clang-tidy and cppcheck disagree on whether one of the pointers can be NULL at some points. Rework the code with the use of two new helper functions: * parse_port_range() operates a bit like strtoul(), but can parse a whole port range specification (e.g. '80' or '1000-1015') * next_chunk() does the necessary wrapping around strchr() to advance to just after the next given delimiter, while cleanly handling if there are no more delimiters The new version is easier to follow, and also removes some cppcheck warnings. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* Fix widespread off-by-one error dealing with port numbersDavid Gibson2022-09-241-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | Port numbers (for both TCP and UDP) are 16-bit, and so fit exactly into a 'short'. USHRT_MAX is therefore the maximum port number and this is widely used in the code. Unfortunately, a lot of those places don't actually want the maximum port number (USHRT_MAX == 65535), they want the total number of ports (65536). This leads to a number of potentially nasty consequences: * We have buffer overruns on the port_fwd::delta array if we try to use port 65535 * We have similar potential overruns for the tcp_sock_* arrays * Interestingly udp_act had the correct size, but we can calculate it in a more direct manner * We have a logical overrun of the ports bitmap as well, although it will just use an unused bit in the last byte so isnt harmful * Many loops don't consider port 65535 (which does mitigate some but not all of the buffer overruns above) * In udp_invert_portmap() we incorrectly compute the reverse port translation for return packets Correct all these by using a new NUM_PORTS defined explicitly for this purpose. Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
* Treat port numbers as unsignedDavid Gibson2022-09-241-5/+6
| | | | | | | | | | | Port numbers are unsigned values, but we're storing them in (signed) int variables in some places. This isn't actually harmful, because int is large enough to hold the entire range of ports. However in places we don't want to use an in_port_t (usually to avoid overflow on the last iteration of a loop) it makes more conceptual sense to use an unsigned int. This will also avoid some problems with later cleanups. Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
* Pass entire port forwarding configuration substructure to conf_ports()David Gibson2022-09-241-40/+22
| | | | | | | | | conf_ports() switches on the optname argument to select the target array for several updates. Now that all these maps are in a common structure, we can simplify by just passing in a pointer to the whole struct port_fwd to update. Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
* Don't use indirect remap functions for conf_ports()David Gibson2022-09-241-7/+7
| | | | | | | | | | Now that we've delayed initialization of the UDP specific "reverse" map until udp_init(), the only difference between the various 'remap' functions used in conf_ports() is which array they target. So, simplify by open coding the logic into conf_ports() with a pointer to the correct mapping array. Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
* Consolidate port forwarding configuration into a common structureDavid Gibson2022-09-241-37/+37
| | | | | | | | | | | | | | | The configuration for how to forward ports in and out of the guest/ns is divided between several different variables. For each connect direction and protocol we have a mode in the udp/tcp context structure, a bitmap of which ports to forward also in the context structure and an array of deltas to apply if the outward facing and inward facing port numbers are different. This last is a separate global variable, rather than being in the context structure, for no particular reason. UDP also requires an additional array which has the reverse mapping used for return packets. Consolidate these into a re-used substructure in the context structure. Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
* Improve types and names for port forwarding configurationDavid Gibson2022-09-241-36/+37
| | | | | | | | | | | | | | | | | | | | | | | | | enum conf_port_type is local to conf.c and is used to track the port forwarding mode during configuration. We don't keep it around in the context structure, however the 'init_detect_ports' and 'ns_detect_ports' fields in the context are based solely on this. Rather than changing encoding, just include the forwarding mode into the context structure. Move the type definition to a new port_fwd.h, which is kind of trivial at the moment but will have more stuff later. While we're there, "conf_port_type" doesn't really convey that this enum is describing how port forwarding is configured. Rename it to port_fwd_mode. The variables (now fields) of this type also have mildly confusing names since it's not immediately obvious whether 'ns' and 'init' refer to the source or destination of the packets. Use "in" (host to guest / init to ns) and "out" (guest to host / ns to init) instead. This has the added bonus that we no longer have locals 'udp_init' and 'tcp_init' which shadow global functions. In addition, add a typedef 'port_fwd_map' for a bitmap of each port number, which is used in several places. Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
* conf, tcp, udp: Arrays for ports need 2^16 values, not 2^16-8Stefano Brivio2022-09-221-1/+1
| | | | | | | | | | | | Reported by David but also by Coverity (CWE-119): In conf_ports: Out-of-bounds access to a buffer ...not in practice, because the allocation size is rounded up anyway, but not nice either. Reported-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* Allow --userns when pasta spawns a commandDavid Gibson2022-09-131-5/+0
| | | | | | | | | | | | Currently --userns is only allowed when pasta is attaching to an existing netns or PID, and is prohibited when creating a new netns by spawning a command or shell. With the new handling of userns, this check isn't neccessary. I'm not sure if there's any use case for --userns with a spawned command, but it's strictly more flexible and requires zero extra code, so we might as well. Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
* Handle userns isolation and dropping root at the same timeDavid Gibson2022-09-131-5/+6
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | passt/pasta can interact with user namespaces in a number of ways: 1) With --netns-only we'll remain in our original user namespace 2) With --userns or a PID option to pasta we'll join either the given user namespace or that of the PID 3) When pasta spawns a shell or command we'll start a new user namespace for the command and then join it 4) With passt we'll create a new user namespace when we sandbox() ourself However (3) and (4) turn out to have essentially the same effect. In both cases we create one new user namespace. The spawned command starts there, and passt/pasta itself will live there from sandbox() onwards. Because of this, we can simplify user namespace handling by moving the userns handling earlier, to the same point we drop root in the original namespace. Extend the drop_user() function to isolate_user() which does both. After switching UID and GID in the original userns, isolate_user() will either join or create the userns we require. When we spawn a command with pasta_start_ns()/pasta_setup_ns() we no longer need to create a userns, because we're already made one. sandbox() likewise no longer needs to create (or join) an userns because we're already in the one we need. We no longer need c->pasta_userns_fd, since the fd is only used locally in isolate_user(). Likewise we can replace c->netns_only with a local in conf(), since it's not used outside there. Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
* Clean up and rename conf_ns_open()David Gibson2022-09-131-71/+1
| | | | | | | | | | | | | | 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>
* Consolidate validation of pasta namespace optionsDavid Gibson2022-09-131-41/+42
| | | | | | | | | | | | | | | | | | | | 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>
* Move self-isolation code into a separate fileDavid Gibson2022-09-131-0/+1
| | | | | | | | 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>
* Safer handling if we can't open /proc/self/uid_mapDavid Gibson2022-09-131-2/+6
| | | | | | | | | | | | | | | | | 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>
* Consolidate determination of UID/GID to run asDavid Gibson2022-09-131-8/+73
| | | | | | | | | | | | 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>
* Split checking for root from dropping root privilegeDavid Gibson2022-09-131-2/+3
| | | | | | | | | | | | | | | | | | 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>
* Don't store UID & GID persistently in the context structureDavid Gibson2022-09-131-3/+5
| | | | | | | | 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>
* conf: Fix getopt_long() optstring for current semantics of -D, -S, -pStefano Brivio2022-09-021-2/+2
| | | | | | | | | | | | | 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: aae2a9bbf7d1 ("conf: Use "-D none" and "-S none" instead of missing empty option arguments") Fixes: bf95322fc1ef ("conf: Make the argument to --pcap option mandatory") Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* Allow pasta to take a command to executeDavid Gibson2022-08-301-9/+18
| | | | | | | | | | | | | | 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>
* Use explicit --netns option rather than multiplexing with PIDDavid Gibson2022-08-301-25/+60
| | | | | | | | | | | | 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>
* More deterministic detection of whether argument is a PID, PATH or NAMEDavid Gibson2022-08-301-82/+90
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
* Move ENOENT error message into conf_ns_opt()David Gibson2022-08-301-2/+1
| | | | | | | | 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>
* Remove --nsrun-dir optionDavid Gibson2022-08-301-20/+4
| | | | | | | | | | | | | 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>
* conf: Use "-D none" and "-S none" instead of missing empty option argumentsDavid Gibson2022-08-301-24/+32
| | | | | | | | | | | | | | | | 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>
* conf: Make the argument to --pcap option mandatoryDavid Gibson2022-08-301-15/+3
| | | | | | | | | | | 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>
* Don't unnecessarily avoid CLOEXEC flags2022_08_24.60ffc5bDavid Gibson2022-08-241-7/+3
| | | | | | | | | | | | | | | | 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>