Skip to content

Commit

Permalink
#51: fix terminal multiplexers bypass remote sudo password (#54)
Browse files Browse the repository at this point in the history
The tty only approach had downsides (see #8), its fix had downsides too (see #39), and the name approach had downsides (see #51) too, let's try all together plus some additional magic.

This modifies local check to:
* check for sshd/telnetd in process chain - for the obvious cases
* check for tmux in process chain, if found parse its environment to determine tmux client id to determine session tty for utmp check
* if tmux found, but session tty not, check for remote clients attached to tmux
* check for DISPLAY, if found use that for utmp check
* in case no remote daemon was found, tmux wasn't detected, and DISPLAY is not set - fall back to good ol' ttyname() which should now be safe since we handled all edge cases before

Closes #51 


* #51: process:c add get_process_tty()

* #51: local.c: Re-add utmp code, to be used by parent pid using process.c [WIP]

* #51: Add @todo

* #51: [WIP] Rework get_process_tty(), check for X session, add more debug logging

* #51: [WIP] If tmux detected use it to detect the login tty

* #51: local.c use new tmux/display/tty approach in all cases

* #51: process.c: remove get_process_tty(), local.c: rename from to session_tty

* #51: [Debian] [Packaging] Re-add 'Standards-Version', got lost somehow

* #51: local.c: replace 4-spaces with tabs to keep uniform formatting

* #51: remove libprocps depency again

* #51: local.c: remove current_tty - used only for logging / making the code order nicer

* local.c: spaces...

* local.c: fix alt-tab-typo and some formatting

* #51: Test for open udp port 177 (XDMCP negotiation), if not found allow (when display manager is found)

* #51: Remove port check stuff again, XDMCP is a pain to setup for testing and is insecure anyway

* #51: local.c: whitelist graphical logins by service tag, remove xdmcp leftovers

* #51: Make ttyname() approach default fallback for all cases

* #51: local.c: extract tmux magic to tmux.c

* #51: local.c/Makefile: make use of tmux.c, adjust to new chain

* #51: Iterate all tty methods, add 'tmux var from parent proc', cleanup formatting

* #51: local.c: add pusb_get_tty_by_xorg_display(), used to get tty by DISPLAY var (for SDDM sessions)

* #51: Fix DISPLAY fallback, add more debug, expect console and pts

* #51: Add pusb_ prefix to new functions

* #51: Add pusb_ prefix to new functions 2nd edition

* #51: Whitelist sddm too

* #51: [WIP] [deb} Update news and changelog

* #51: Fix incorrect return handling reported in #51 (comment)

* #51: Fix derp

* actions: make sure no previously build debs are installed again...

* #51: Check for remotely connected clients to local tmux sessions

* #51: This and that

* #51: Fix v6 detection of connected tmux sessions

* #51: Cleanup

* #51: 'Fix' debug output

* #51: Fix warning unitialized for tmux_pid

* #51/#64: Replace utmp with utmpx stuff / posix compliance

* #51: tmux.c: extend regex to capture full 'attach' argument too

* #51: Fix last case of byobu/tmux

* #51: Remove version update, will be done in seperate PR

* #51: Cleanup
  • Loading branch information
mcdope authored Aug 31, 2021
1 parent ce02bb6 commit b5ec9eb
Show file tree
Hide file tree
Showing 10 changed files with 456 additions and 20 deletions.
6 changes: 6 additions & 0 deletions .github/workflows/build-and-test-on-own-server.yml
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,12 @@ jobs:
TEST_RUNNER_USER: ${{ secrets.TEST_RUNNER_USER }}
TEST_RUNNER_PASS: ${{ secrets.TEST_RUNNER_PASS }}
run: sshpass -p "$TEST_RUNNER_PASS" ssh -o StrictHostKeyChecking=no $TEST_RUNNER_USER@$TEST_RUNNER_HOST "sudo apt purge --yes libpam-usb || exit 0"
- name: remove previously built packages
env:
TEST_RUNNER_HOST: ${{ secrets.TEST_RUNNER_HOST }}
TEST_RUNNER_USER: ${{ secrets.TEST_RUNNER_USER }}
TEST_RUNNER_PASS: ${{ secrets.TEST_RUNNER_PASS }}
run: sshpass -p "$TEST_RUNNER_PASS" ssh -o StrictHostKeyChecking=no $TEST_RUNNER_USER@$TEST_RUNNER_HOST "cd ~ && rm *.deb"
- name: make deb
env:
TEST_RUNNER_HOST: ${{ secrets.TEST_RUNNER_HOST }}
Expand Down
1 change: 1 addition & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ SRCS := src/conf.c \
src/pad.c \
src/volume.c \
src/process.c \
src/tmux.c \
src/local.c \
src/device.c
OBJS := $(SRCS:.c=.o)
Expand Down
249 changes: 238 additions & 11 deletions src/local.c
Original file line number Diff line number Diff line change
Expand Up @@ -18,33 +18,260 @@
#include <stdio.h>
#include <string.h>
#include <unistd.h>
#include <utmpx.h>
#include <stdlib.h>
#include <dirent.h>
#include <fcntl.h>
#include "log.h"
#include "conf.h"
#include "process.h"
#include "tmux.h"

int pusb_local_login(t_pusb_options *opts, const char *user)
int pusb_is_tty_local(char *tty)
{
struct utmpx utsearch;
struct utmpx *utent;

if (strstr(tty, "/dev/") != NULL) {
tty += strlen("/dev/");
}

strncpy(utsearch.ut_line, tty, sizeof(utsearch.ut_line) - 1);

setutxent();
utent = getutxline(&utsearch);
endutxent();

if (!utent)
{
log_debug(" No utmp entry found for tty \"%s\"\n", utsearch.ut_line);
return (0);
} else {
log_debug(" utmp entry for tty \"%s\" found\n", tty);
log_debug(" utmp->ut_pid: %d\n", utent->ut_pid);
log_debug(" utmp->ut_user: %s\n", utent->ut_user);
}

for (int i = 0; i < 4; ++i)
{
/**
* Note: despite the property name this also works for IPv4, v4 addr would be in ut_addr_v6[0] solely.
* See utmp man (http://manpages.ubuntu.com/manpages/bionic/de/man5/utmp.5.html)
**/
if (utent->ut_addr_v6[i] != 0)
{
log_error("Remote authentication request: %s\n", utent->ut_host);
return (-1);
} else {
log_debug(" Checking utmp->ut_addr_v6[%d]\n", i);
}
}

log_debug(" utmp check successful, request originates from a local source!\n");
return (1);
}

char *pusb_get_tty_from_xorg_process(const char *display)
{
DIR *d_proc = opendir("/proc");
if (d_proc == NULL)
return NULL;

char *expected_core = (char *)malloc(12);
char *cmdline_path = (char *)malloc(32);
char *cmdline = (char *)malloc(4096);
char *fd_path = (char *)malloc(32);
char *link_path = (char *)malloc(32);
char *fd_target = (char *)malloc(32);

sprintf(expected_core, "-core %s", display);

struct dirent *dent_proc;
while ((dent_proc = readdir(d_proc)) != NULL)
{
if (dent_proc->d_type == DT_DIR && atoi(dent_proc->d_name) != 0 && strcmp(dent_proc->d_name, ".") != 0 && strcmp(dent_proc->d_name, "..") != 0)
{
memset(cmdline_path, 0, 32);
sprintf(cmdline_path, "/proc/%s/cmdline", dent_proc->d_name);

memset(cmdline, 0, 4096);
int cmdline_file = open(cmdline_path, O_RDONLY | O_CLOEXEC);
int bytes_read = read(cmdline_file, cmdline, 4096);
close(cmdline_file);
for (int i = 0 ; i < bytes_read; i++) {
if (!cmdline[i] && i != bytes_read) cmdline[i] = ' '; // replace \0 with [space]
}

if (strstr(cmdline, "Xorg") != NULL && strstr(cmdline, expected_core) != NULL)
{
memset(fd_path, 0, 32);
sprintf(fd_path, "/proc/%s/fd", dent_proc->d_name);

DIR *d_fd = opendir(fd_path);
if (d_fd == NULL) {
log_debug(" Determining tty by Xorg failed (running 'pamusb-check' as user?)\n", fd_path);
return NULL;
}

struct dirent *dent_fd;
while ((dent_fd = readdir(d_fd)) != NULL)
{
if (dent_fd->d_type == DT_LNK && strcmp(dent_fd->d_name, ".") != 0 && strcmp(dent_fd->d_name, "..") != 0)
{
memset(link_path, 0, 32);
memset(fd_target, 0, 32);

sprintf(link_path, "/proc/%s/fd/%s", dent_proc->d_name, dent_fd->d_name);
if (readlink(link_path, fd_target, 32) != -1)
{
if (strstr(fd_target, "/dev/tty") != NULL)
{
closedir(d_fd);
closedir(d_proc);

return fd_target;
}
}
}
}
closedir(d_fd);
}
}
}
closedir(d_proc);

return NULL;
}

char *pusb_get_tty_by_xorg_display(const char *display, const char *user)
{
struct utmpx *utent;

setutxent();
while ((utent = getutxent())) {
if (strncmp(utent->ut_host, display, strlen(display)) == 0
&& strncmp(utent->ut_user, user, strlen(user)) == 0
&& (
strncmp(utent->ut_line, "tty", strlen("tty")) == 0
|| strncmp(utent->ut_line, "console", strlen("console")) == 0
|| strncmp(utent->ut_line, "pts", strlen("pts")) == 0
)
) {
endutxent();
return utent->ut_line;
}
}

endutxent();
return NULL;
}

int pusb_local_login(t_pusb_options *opts, const char *user, const char *service)
{
if (!opts->deny_remote)
{
log_debug("deny_remote is disabled. Skipping local check.\n");
return (1);
log_debug("deny_remote is disabled. Skipping local check.\n");
return (1);
}

log_debug("Checking whether the caller is local or not...\n");

log_debug("Checking whether the caller (%s) is local or not...\n", service);

char name[BUFSIZ];
pid_t pid = getpid();
pid_t previous_pid = 0;
pid_t tmux_pid = 0;
int local_request = 0;

while (pid != 0) {
char name[BUFSIZ];
get_process_name(pid, name);
log_debug(" Checking pid %6d (%s)...\n", pid, name);
get_process_parent_id(pid, & pid);
pusb_get_process_name(pid, name);
log_debug(" Checking pid %6d (%s)...\n", pid, name);

if (strstr(name, "tmux") != NULL) {
log_debug(" Setting pid %d as fallback for tmux check\n", previous_pid);
tmux_pid = previous_pid;
}

previous_pid = pid;
pusb_get_process_parent_id(pid, & pid);
if (strstr(name, "sshd") != NULL || strstr(name, "telnetd") != NULL) {
log_error("One of the parent processes found to be a remote access daemon, denying.\n");
return (0);
}
}

log_debug("No remote daemons found in parent process list, seems to be local request - allowing.\n");
return (1);
if (strcmp(service, "gdm-password") == 0 || strcmp(service, "xdm") == 0 || strcmp(service, "lightdm") == 0 || strcmp(service, "sddm") == 0) {
log_debug(" Graphical login request detected, assuming local.");
local_request = 1;
}

const char *session_tty;
const char *display = getenv("DISPLAY");

if (local_request == 0 && strstr(name, "tmux") != NULL && tmux_pid != 0) {
char *tmux_client_tty = pusb_tmux_get_client_tty(tmux_pid);
if (tmux_client_tty != NULL && tmux_client_tty != 0) {
local_request = pusb_is_tty_local(tmux_client_tty);
} else if (tmux_client_tty == 0) {
return 0;
}
}

if (local_request == 0 && strstr(name, "tmux") != NULL) {
log_debug(" Checking for remote clients attached to tmux...\n");
if (pusb_tmux_has_remote_clients(user) != 0) { // tmux has at least one remote client, can't be sure it isn't this one so denying...
return 0;
}
}

if (local_request == 0 && display != NULL) {
log_debug(" Using DISPLAY %s for utmp search\n", display);
local_request = pusb_is_tty_local((char *) display);

char *xorg_tty = (char *)malloc(32);
if (local_request == 0)
{
log_debug(" Trying to get tty from Xorg process\n");
xorg_tty = pusb_get_tty_from_xorg_process(display);

if (xorg_tty != NULL)
{
log_debug(" Retrying with tty %s, obtained from Xorg, for utmp search\n", xorg_tty);
local_request = pusb_is_tty_local(xorg_tty);
}

if (local_request == 0)
{
log_debug(" Trying to get tty by DISPLAY\n");
xorg_tty = pusb_get_tty_by_xorg_display(display, user);

if (xorg_tty != NULL)
{
log_debug(" Retrying with tty %s, obtained by DISPLAY, for utmp search\n", xorg_tty);
local_request = pusb_is_tty_local(xorg_tty);
}
}
}
}

if (local_request == 0) {
session_tty = ttyname(STDIN_FILENO);
if (!session_tty || !(*session_tty))
{
log_error("Couldn't retrieve login tty, assuming remote\n");
} else {
log_debug(" Using TTY %s for search\n", session_tty);
local_request = pusb_is_tty_local((char *) session_tty);
}
}

if (local_request == 1) {
log_debug("No remote access detected, seems to be local request - allowing.\n");
} else if (local_request == 0) {
log_debug("Couldn't confirm login tty to be neither local or remote - denying.\n");
} else if (local_request == -1) {
log_debug("Confirmed remote request - denying.\n");
}

return local_request;
}

8 changes: 7 additions & 1 deletion src/local.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,12 @@
#ifndef PUSB_LOCAL_H_
# define PUSB_LOCAL_H_

int pusb_local_login(t_pusb_options *opts, const char *user);
int pusb_local_login(t_pusb_options *opts, const char *user, const char *service);

int pusb_is_tty_local(char *tty);

char *pusb_get_tty_from_xorg_process(const char *display);

char *pusb_get_tty_by_xorg_display(const char *display, const char *user);

#endif /* !PUSB_LOCAL_H_ */
4 changes: 2 additions & 2 deletions src/pam.c
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ int pam_sm_authenticate(pam_handle_t *pamh, int flags,
log_info("Authentication request for user \"%s\" (%s)\n",
user, service);

if (!pusb_local_login(&opts, user))
if (pusb_local_login(&opts, user, service) != 1)
{
log_error("Access denied.\n");
return (PAM_AUTH_ERR);
Expand Down Expand Up @@ -131,7 +131,7 @@ int pam_sm_acct_mgmt(pam_handle_t *pamh, int flags,
log_info("Account request for user \"%s\" (%s)\n",
user, service);

if (!pusb_local_login(&opts, user))
if (pusb_local_login(&opts, user, service) != 1)
{
log_error("Access denied.\n");
return (PAM_AUTH_ERR);
Expand Down
3 changes: 1 addition & 2 deletions src/pamusb-check.c
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
*/

#include <stdio.h>
#include <unistd.h>
#include <string.h>
#include <getopt.h>
#include "conf.h"
Expand Down Expand Up @@ -59,7 +58,7 @@ static int pusb_check_perform_authentication(t_pusb_options *opts,
}
log_info("Authentication request for user \"%s\" (%s)\n",
user, service);
if (!pusb_local_login(opts, user))
if (pusb_local_login(opts, user, service) != 1)
{
log_error("Access denied.\n");
return (0);
Expand Down
Loading

1 comment on commit b5ec9eb

@mcdope
Copy link
Owner Author

@mcdope mcdope commented on b5ec9eb Oct 5, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aptx4869 any reason you have deleted your comment? was about to check & fix that but wondering about the deletion...

Please sign in to comment.