passt, util: Close any open file that the parent might have leaked

If a parent accidentally or due to implementation reasons leaks any
open file, we don't want to have access to them, except for the file
passed via --fd, if any.

This is the case for Podman when Podman's parent leaks files into
Podman: it's not practical for Podman to close unrelated files before
starting pasta, as reported by Paul.

Use close_range(2) to close all open files except for standard streams
and the one from --fd.

Given that parts of conf() depend on other files to be already opened,
such as the epoll file descriptor, we can't easily defer this to a
more convenient point, where --fd was already parsed. Introduce a
minimal, duplicate version of --fd parsing to keep this simple.

As we need to check that the passed --fd option doesn't exceed
INT_MAX, because we'll parse it with strtol() but file descriptor
indices are signed ints (regardless of the arguments close_range()
take), extend the existing check in the actual --fd parsing in conf(),
also rejecting file descriptors numbers that match standard streams,
while at it.

Suggested-by: Paul Holzinger <pholzing@redhat.com>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Paul Holzinger <pholzing@redhat.com>
This commit is contained in:
Stefano Brivio 2024-08-06 20:32:11 +02:00
parent 755f9fd911
commit 09603cab28
6 changed files with 59 additions and 7 deletions

8
conf.c
View file

@ -1245,6 +1245,7 @@ void conf(struct ctx *c, int argc, char **argv)
const char *optstring; const char *optstring;
size_t logsize = 0; size_t logsize = 0;
char *runas = NULL; char *runas = NULL;
long fd_tap_opt;
int name, ret; int name, ret;
uid_t uid; uid_t uid;
gid_t gid; gid_t gid;
@ -1260,6 +1261,7 @@ void conf(struct ctx *c, int argc, char **argv)
c->tcp.fwd_in.mode = c->tcp.fwd_out.mode = FWD_UNSET; c->tcp.fwd_in.mode = c->tcp.fwd_out.mode = FWD_UNSET;
c->udp.fwd_in.mode = c->udp.fwd_out.mode = FWD_UNSET; c->udp.fwd_in.mode = c->udp.fwd_out.mode = FWD_UNSET;
optind = 1;
do { do {
name = getopt_long(argc, argv, optstring, options, NULL); name = getopt_long(argc, argv, optstring, options, NULL);
@ -1424,11 +1426,13 @@ void conf(struct ctx *c, int argc, char **argv)
break; break;
case 'F': case 'F':
errno = 0; errno = 0;
c->fd_tap = strtol(optarg, NULL, 0); fd_tap_opt = strtol(optarg, NULL, 0);
if (c->fd_tap < 0 || errno) if (errno ||
fd_tap_opt <= STDERR_FILENO || fd_tap_opt > INT_MAX)
die("Invalid --fd: %s", optarg); die("Invalid --fd: %s", optarg);
c->fd_tap = fd_tap_opt;
c->one_off = true; c->one_off = true;
*c->sock_path = 0; *c->sock_path = 0;
break; break;

View file

@ -29,7 +29,8 @@
* *
* Executed immediately after startup, drops capabilities we don't * Executed immediately after startup, drops capabilities we don't
* need at any point during execution (or which we gain back when we * need at any point during execution (or which we gain back when we
* need by joining other namespaces). * need by joining other namespaces), and closes any leaked file we
* might have inherited from the parent process.
* *
* 2. isolate_user() * 2. isolate_user()
* ================= * =================
@ -166,14 +167,17 @@ static void clamp_caps(void)
} }
/** /**
* isolate_initial() - Early, config independent self isolation * isolate_initial() - Early, mostly config independent self isolation
* @argc: Argument count
* @argv: Command line options: only --fd (if present) is relevant here
* *
* Should: * Should:
* - drop unneeded capabilities * - drop unneeded capabilities
* - close all open files except for standard streams and the one from --fd
* Musn't: * Musn't:
* - remove filesytem access (we need to access files during setup) * - remove filesytem access (we need to access files during setup)
*/ */
void isolate_initial(void) void isolate_initial(int argc, char **argv)
{ {
uint64_t keep; uint64_t keep;
@ -207,6 +211,8 @@ void isolate_initial(void)
keep |= BIT(CAP_SETFCAP) | BIT(CAP_SYS_PTRACE); keep |= BIT(CAP_SETFCAP) | BIT(CAP_SYS_PTRACE);
drop_caps_ep_except(keep); drop_caps_ep_except(keep);
close_open_files(argc, argv);
} }
/** /**

View file

@ -7,7 +7,7 @@
#ifndef ISOLATION_H #ifndef ISOLATION_H
#define ISOLATION_H #define ISOLATION_H
void isolate_initial(void); void isolate_initial(int argc, char **argv);
void isolate_user(uid_t uid, gid_t gid, bool use_userns, const char *userns, void isolate_user(uid_t uid, gid_t gid, bool use_userns, const char *userns,
enum passt_modes mode); enum passt_modes mode);
int isolate_prefork(const struct ctx *c); int isolate_prefork(const struct ctx *c);

View file

@ -211,7 +211,7 @@ int main(int argc, char **argv)
arch_avx2_exec(argv); arch_avx2_exec(argv);
isolate_initial(); isolate_initial(argc, argv);
c.pasta_netns_fd = c.fd_tap = c.pidfile_fd = -1; c.pasta_netns_fd = c.fd_tap = c.pidfile_fd = -1;

41
util.c
View file

@ -26,6 +26,7 @@
#include <errno.h> #include <errno.h>
#include <stdbool.h> #include <stdbool.h>
#include <linux/errqueue.h> #include <linux/errqueue.h>
#include <getopt.h>
#include "util.h" #include "util.h"
#include "iov.h" #include "iov.h"
@ -694,3 +695,43 @@ const char *str_ee_origin(const struct sock_extended_err *ee)
return "<invalid>"; return "<invalid>";
} }
/**
* close_open_files() - Close leaked files, but not --fd, stdin, stdout, stderr
* @argc: Argument count
* @argv: Command line options, as we need to skip any file given via --fd
*/
void close_open_files(int argc, char **argv)
{
const struct option optfd[] = { { "fd", required_argument, NULL, 'F' },
{ 0 },
};
long fd = -1;
int name, rc;
do {
name = getopt_long(argc, argv, ":F", optfd, NULL);
if (name == 'F') {
errno = 0;
fd = strtol(optarg, NULL, 0);
if (errno || fd <= STDERR_FILENO || fd > INT_MAX)
die("Invalid --fd: %s", optarg);
}
} while (name != -1);
if (fd == -1) {
rc = close_range(STDERR_FILENO + 1, ~0U, CLOSE_RANGE_UNSHARE);
} else if (fd == STDERR_FILENO + 1) { /* Still a single range */
rc = close_range(STDERR_FILENO + 2, ~0U, CLOSE_RANGE_UNSHARE);
} else {
rc = close_range(STDERR_FILENO + 1, fd - 1,
CLOSE_RANGE_UNSHARE);
if (!rc)
rc = close_range(fd + 1, ~0U, CLOSE_RANGE_UNSHARE);
}
if (rc)
die_perror("Failed to close files leaked by parent");
}

1
util.h
View file

@ -183,6 +183,7 @@ int __daemon(int pidfile_fd, int devnull_fd);
int fls(unsigned long x); int fls(unsigned long x);
int write_file(const char *path, const char *buf); int write_file(const char *path, const char *buf);
int write_remainder(int fd, const struct iovec *iov, size_t iovcnt, size_t skip); int write_remainder(int fd, const struct iovec *iov, size_t iovcnt, size_t skip);
void close_open_files(int argc, char **argv);
/** /**
* af_name() - Return name of an address family * af_name() - Return name of an address family