Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

session: Use pidfd for determining ws peer cgroup #21341

Merged
merged 3 commits into from
Nov 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions configure.ac
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ AC_CHECK_TOOL(AR, ar)

AC_CHECK_FUNCS(
closefrom
pidfd_getpid
)

AM_SILENT_RULES([yes])
Expand Down
140 changes: 102 additions & 38 deletions src/session/client-certificate.c
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,11 @@
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

#ifdef HAVE_PIDFD_GETPID
#include <sys/pidfd.h>
#endif

#include <sys/socket.h>
#include <sys/stat.h>
#include <sys/un.h>
Expand Down Expand Up @@ -127,6 +132,99 @@ unsigned long long get_proc_pid_start_time (int dirfd)
return start_time;
}

/* Fallback for get_ws_proc_fd() on older kernels which don't support enough pidfd API */
static int
get_ws_proc_fd_pid_time (int unix_fd)
{
struct ucred ucred;
socklen_t ucred_len = sizeof ucred;
if (getsockopt (unix_fd, SOL_SOCKET, SO_PEERCRED, &ucred, &ucred_len) != 0 ||
/* this is an inout parameter, be extra suspicious */
ucred_len != sizeof ucred)
{
debug ("failed to read stdin peer credentials: %m; not in socket mode?");
warnx ("Certificate authentication only supported with cockpit-session.socket");
exit_init_problem ("authentication-unavailable", "Certificate authentication only supported with cockpit-session.socket");
}

debug ("unix socket mode, ws peer pid %d", ucred.pid);
int ws_proc_dirfd = open_proc_pid (ucred.pid);
unsigned long long ws_start_time = get_proc_pid_start_time (ws_proc_dirfd);

int my_pid_dirfd = open_proc_pid (getpid ());
unsigned long long my_start_time = get_proc_pid_start_time (my_pid_dirfd);
close (my_pid_dirfd);

debug ("peer start time: %llu, my start time: %llu", ws_start_time, my_start_time);

/* Guard against pid recycling: If a malicious user captures ws, keeps the socket in a forked child and exits
* the original pid, they can trick a different user to login, get the old pid (pointing to their cgroup), and
* capture their session. To prevent that, require that ws must have started earlier than ourselves. */
if (my_start_time < ws_start_time)
{
warnx ("start time of this process (%llu) is older than cockpit-ws (%llu), pid recycling attack?",
my_start_time, ws_start_time);
close (ws_proc_dirfd);
exit_init_problem ("access-denied", "implausible cockpit-ws start time");
}

return ws_proc_dirfd;
}

/* Get a /proc/[pid] dirfd for our Unix socket peer (i.e. cockpit-ws).
* We only support being called via cockpit-session.socket (i.e. Unix socket).
*/
static int
get_ws_proc_fd (int unix_fd)
{
#if defined(SO_PEERPIDFD) && defined(HAVE_PIDFD_GETPID)
martinpitt marked this conversation as resolved.
Show resolved Hide resolved
int pidfd = -1;
socklen_t socklen = sizeof pidfd;
/* this is always the pidfd for the process that started the communication, it cannot be recycled */
if (getsockopt (unix_fd, SOL_SOCKET, SO_PEERPIDFD, &pidfd, &socklen) < 0)
{
if (errno == ENOPROTOOPT)
{
debug ("SO_PEERPIDFD not supported: %m, falling back to pid/time check");
return get_ws_proc_fd_pid_time (unix_fd);
}

warn ("Failed to get peer pidfd");
exit_init_problem ("access-denied", "Failed to get peer pidfd");
}
/* this is an inout parameter, be extra suspicious; this really Should Not Happen™, so bomb out */
if (socklen != sizeof pidfd)
errx (EX, "SO_PEERPIDFD returned too small result");

/* get pid for pidfd; from here on this is racy and could suffer from PID recycling */
pid_t pid = pidfd_getpid (pidfd);
martinpitt marked this conversation as resolved.
Show resolved Hide resolved
if (pid < 0)
martinpitt marked this conversation as resolved.
Show resolved Hide resolved
{
/* be *very* strict here. This could theoretically ENOSYS if glibc has pidfd_getpid() but the kernel doesn't
* support it; but err on the side of denying access rather than falling back */
warn ("Failed to get pid from pidfd");
exit_init_problem ("access-denied", "Failed to get pid from pidfd");
}

debug ("pid from ws peer pidfd: %i", (int) pid);
int ws_proc_dirfd = open_proc_pid (pid);

/* check that the pid is still valid to guard against recycling */
if (pidfd_getpid (pidfd) != pid)
martinpitt marked this conversation as resolved.
Show resolved Hide resolved
{
warn ("original pid %i is not valid any more", (int) pid);
exit_init_problem ("access-denied", "Failed to get cockpit-ws pid");
}

close (pidfd);
return ws_proc_dirfd;

#else
debug ("not built with pidfd support, falling back to pid/time check");
return get_ws_proc_fd_pid_time (unix_fd);
#endif
}

/* valid_256_bit_hex_string:
* @str: a string
*
Expand Down Expand Up @@ -354,46 +452,12 @@ cockpit_session_client_certificate_map_user (const char *client_certificate_file
exit_init_problem ("authentication-unavailable", "No https instance certificate present");
}

/* We only support being called via cockpit-session.socket (i.e. Unix socket).
* We need to check the cgroup of cockpit-ws (our peer) */
struct ucred ucred;
socklen_t ucred_len = sizeof ucred;
if (getsockopt (STDIN_FILENO, SOL_SOCKET, SO_PEERCRED, &ucred, &ucred_len) != 0 ||
/* this is an inout parameter, be extra suspicious */
ucred_len < sizeof ucred)
{
debug ("failed to read stdin peer credentials: %m; not in socket mode?");
warnx ("Certificate authentication only supported with cockpit-session.socket");
exit_init_problem ("authentication-unavailable", "Certificate authentication only supported with cockpit-session.socket");
}

/* this is an inout parameter, be extra suspicious */
assert (ucred_len >= sizeof ucred);
debug ("unix socket mode, reading cgroup from peer pid %d", ucred.pid);
int ws_pid_dirfd = open_proc_pid (ucred.pid);
unsigned long long ws_start_time = get_proc_pid_start_time (ws_pid_dirfd);

int my_pid_dirfd = open_proc_pid (getpid ());
unsigned long long my_start_time = get_proc_pid_start_time (my_pid_dirfd);
close (my_pid_dirfd);

debug ("peer start time: %llu, my start time: %llu", ws_start_time, my_start_time);

/* Guard against pid recycling: If a malicious user captures ws, keeps the socket in a forked child and exits
* the original pid, they can trap a different user to login, get the old pid (pointing ot their cgroup), and
* capture their session. To prevent that, require that ws must have started earlier than ourselves. */
if (my_start_time < ws_start_time)
{
warnx ("start time of this process (%llu) is older than cockpit-ws (%llu), pid recycling attack?",
my_start_time, ws_start_time);
close (ws_pid_dirfd);
exit_init_problem ("access-denied", "implausible cockpit-ws start time");
}

/* We need to check the cgroup of cockpit-ws (our peer); we are systemd socket activated, so stdin is a socket */
int ws_proc_dirfd = get_ws_proc_fd (STDIN_FILENO);
size_t ws_cgroup_length;
char *ws_cgroup = read_proc_pid_cgroup (ws_pid_dirfd, &ws_cgroup_length);
char *ws_cgroup = read_proc_pid_cgroup (ws_proc_dirfd, &ws_cgroup_length);
assert (ws_cgroup);
close (ws_pid_dirfd);
close (ws_proc_dirfd);

/* read_proc_pid_cgroup() already ensures that, but just in case we refactor this: this is *essential* for the
* subsequent comparison */
Expand Down