From 4216abc1a9b002557a09a3b5be0c9ddd7c30a6c6 Mon Sep 17 00:00:00 2001 From: Giovanni Simoni Date: Tue, 3 Dec 2024 21:47:10 +0100 Subject: [PATCH 1/6] pam: Introduce cfg module Having it into another module will prevent the code from being messy later. The parsing procedure is taken verbatim: no semantic change, no behavioural change. --- Makefile.am | 1 + cfg.c | 97 +++++++++++++++++++++++++++++++++++++++++++++++++++++ cfg.h | 38 +++++++++++++++++++++ pam-u2f.c | 90 ++----------------------------------------------- util.h | 26 ++------------ 5 files changed, 140 insertions(+), 112 deletions(-) create mode 100644 cfg.c create mode 100644 cfg.h diff --git a/Makefile.am b/Makefile.am index 16302c7..e67d64c 100644 --- a/Makefile.am +++ b/Makefile.am @@ -26,6 +26,7 @@ libmodule_la_SOURCES += drop_privs.h libmodule_la_SOURCES += expand.c libmodule_la_SOURCES += explicit_bzero.c libmodule_la_SOURCES += util.c util.h +libmodule_la_SOURCES += cfg.c cfg.h libmodule_la_LIBADD = -lpam $(LIBFIDO2_LIBS) $(LIBCRYPTO_LIBS) pampluginexecdir = $(PAMDIR) diff --git a/cfg.c b/cfg.c new file mode 100644 index 0000000..68ad532 --- /dev/null +++ b/cfg.c @@ -0,0 +1,97 @@ +#include + +#include "cfg.h" +#include "debug.h" + +int cfg_init(cfg_t *cfg, int flags, int argc, const char **argv) { + int i; + + memset(cfg, 0, sizeof(cfg_t)); + cfg->debug_file = DEFAULT_DEBUG_FILE; + cfg->userpresence = -1; + cfg->userverification = -1; + cfg->pinverification = -1; + + for (i = 0; i < argc; i++) { + if (strncmp(argv[i], "max_devices=", 12) == 0) { + sscanf(argv[i], "max_devices=%u", &cfg->max_devs); + } else if (strcmp(argv[i], "manual") == 0) { + cfg->manual = 1; + } else if (strcmp(argv[i], "debug") == 0) { + cfg->debug = 1; + } else if (strcmp(argv[i], "nouserok") == 0) { + cfg->nouserok = 1; + } else if (strcmp(argv[i], "openasuser") == 0) { + cfg->openasuser = 1; + } else if (strcmp(argv[i], "alwaysok") == 0) { + cfg->alwaysok = 1; + } else if (strcmp(argv[i], "interactive") == 0) { + cfg->interactive = 1; + } else if (strcmp(argv[i], "cue") == 0) { + cfg->cue = 1; + } else if (strcmp(argv[i], "nodetect") == 0) { + cfg->nodetect = 1; + } else if (strcmp(argv[i], "expand") == 0) { + cfg->expand = 1; + } else if (strncmp(argv[i], "userpresence=", 13) == 0) { + sscanf(argv[i], "userpresence=%d", &cfg->userpresence); + } else if (strncmp(argv[i], "userverification=", 17) == 0) { + sscanf(argv[i], "userverification=%d", &cfg->userverification); + } else if (strncmp(argv[i], "pinverification=", 16) == 0) { + sscanf(argv[i], "pinverification=%d", &cfg->pinverification); + } else if (strncmp(argv[i], "authfile=", 9) == 0) { + cfg->auth_file = argv[i] + 9; + } else if (strcmp(argv[i], "sshformat") == 0) { + cfg->sshformat = 1; + } else if (strncmp(argv[i], "authpending_file=", 17) == 0) { + cfg->authpending_file = argv[i] + 17; + } else if (strncmp(argv[i], "origin=", 7) == 0) { + cfg->origin = argv[i] + 7; + } else if (strncmp(argv[i], "appid=", 6) == 0) { + cfg->appid = argv[i] + 6; + } else if (strncmp(argv[i], "prompt=", 7) == 0) { + cfg->prompt = argv[i] + 7; + } else if (strncmp(argv[i], "cue_prompt=", 11) == 0) { + cfg->cue_prompt = argv[i] + 11; + } else if (strncmp(argv[i], "debug_file=", 11) == 0) { + const char *filename = argv[i] + 11; + debug_close(cfg->debug_file); + cfg->debug_file = debug_open(filename); + } + } + + if (cfg->debug) { + debug_dbg(cfg, "called."); + debug_dbg(cfg, "flags %d argc %d", flags, argc); + for (i = 0; i < argc; i++) { + debug_dbg(cfg, "argv[%d]=%s", i, argv[i]); + } + debug_dbg(cfg, "max_devices=%d", cfg->max_devs); + debug_dbg(cfg, "debug=%d", cfg->debug); + debug_dbg(cfg, "interactive=%d", cfg->interactive); + debug_dbg(cfg, "cue=%d", cfg->cue); + debug_dbg(cfg, "nodetect=%d", cfg->nodetect); + debug_dbg(cfg, "userpresence=%d", cfg->userpresence); + debug_dbg(cfg, "userverification=%d", cfg->userverification); + debug_dbg(cfg, "pinverification=%d", cfg->pinverification); + debug_dbg(cfg, "manual=%d", cfg->manual); + debug_dbg(cfg, "nouserok=%d", cfg->nouserok); + debug_dbg(cfg, "openasuser=%d", cfg->openasuser); + debug_dbg(cfg, "alwaysok=%d", cfg->alwaysok); + debug_dbg(cfg, "sshformat=%d", cfg->sshformat); + debug_dbg(cfg, "expand=%d", cfg->expand); + debug_dbg(cfg, "authfile=%s", cfg->auth_file ? cfg->auth_file : "(null)"); + debug_dbg(cfg, "authpending_file=%s", + cfg->authpending_file ? cfg->authpending_file : "(null)"); + debug_dbg(cfg, "origin=%s", cfg->origin ? cfg->origin : "(null)"); + debug_dbg(cfg, "appid=%s", cfg->appid ? cfg->appid : "(null)"); + debug_dbg(cfg, "prompt=%s", cfg->prompt ? cfg->prompt : "(null)"); + } + + return 0; +} + +void cfg_free(cfg_t *cfg) { + debug_close(cfg->debug_file); + cfg->debug_file = DEFAULT_DEBUG_FILE; +} diff --git a/cfg.h b/cfg.h new file mode 100644 index 0000000..32ac16c --- /dev/null +++ b/cfg.h @@ -0,0 +1,38 @@ +/* + * Copyright (C) 2014-2019 Yubico AB - See COPYING + */ + +#ifndef CFG_H +#define CFG_H + +#include + +typedef struct { + unsigned max_devs; + int manual; + int debug; + int nouserok; + int openasuser; + int alwaysok; + int interactive; + int cue; + int nodetect; + int userpresence; + int userverification; + int pinverification; + int sshformat; + int expand; + const char *auth_file; + const char *authpending_file; + const char *origin; + const char *appid; + const char *prompt; + const char *cue_prompt; + FILE *debug_file; +} cfg_t; + +int cfg_init(cfg_t *cfg, int flags, int argc, const char **argv); + +void cfg_free(cfg_t *cfg); + +#endif diff --git a/pam-u2f.c b/pam-u2f.c index 8602218..4b02b1c 100644 --- a/pam-u2f.c +++ b/pam-u2f.c @@ -35,90 +35,6 @@ char *secure_getenv(const char *name) { } #endif -static void parse_cfg(int flags, int argc, const char **argv, cfg_t *cfg) { - int i; - - memset(cfg, 0, sizeof(cfg_t)); - cfg->debug_file = DEFAULT_DEBUG_FILE; - cfg->userpresence = -1; - cfg->userverification = -1; - cfg->pinverification = -1; - - for (i = 0; i < argc; i++) { - if (strncmp(argv[i], "max_devices=", 12) == 0) { - sscanf(argv[i], "max_devices=%u", &cfg->max_devs); - } else if (strcmp(argv[i], "manual") == 0) { - cfg->manual = 1; - } else if (strcmp(argv[i], "debug") == 0) { - cfg->debug = 1; - } else if (strcmp(argv[i], "nouserok") == 0) { - cfg->nouserok = 1; - } else if (strcmp(argv[i], "openasuser") == 0) { - cfg->openasuser = 1; - } else if (strcmp(argv[i], "alwaysok") == 0) { - cfg->alwaysok = 1; - } else if (strcmp(argv[i], "interactive") == 0) { - cfg->interactive = 1; - } else if (strcmp(argv[i], "cue") == 0) { - cfg->cue = 1; - } else if (strcmp(argv[i], "nodetect") == 0) { - cfg->nodetect = 1; - } else if (strcmp(argv[i], "expand") == 0) { - cfg->expand = 1; - } else if (strncmp(argv[i], "userpresence=", 13) == 0) { - sscanf(argv[i], "userpresence=%d", &cfg->userpresence); - } else if (strncmp(argv[i], "userverification=", 17) == 0) { - sscanf(argv[i], "userverification=%d", &cfg->userverification); - } else if (strncmp(argv[i], "pinverification=", 16) == 0) { - sscanf(argv[i], "pinverification=%d", &cfg->pinverification); - } else if (strncmp(argv[i], "authfile=", 9) == 0) { - cfg->auth_file = argv[i] + 9; - } else if (strcmp(argv[i], "sshformat") == 0) { - cfg->sshformat = 1; - } else if (strncmp(argv[i], "authpending_file=", 17) == 0) { - cfg->authpending_file = argv[i] + 17; - } else if (strncmp(argv[i], "origin=", 7) == 0) { - cfg->origin = argv[i] + 7; - } else if (strncmp(argv[i], "appid=", 6) == 0) { - cfg->appid = argv[i] + 6; - } else if (strncmp(argv[i], "prompt=", 7) == 0) { - cfg->prompt = argv[i] + 7; - } else if (strncmp(argv[i], "cue_prompt=", 11) == 0) { - cfg->cue_prompt = argv[i] + 11; - } else if (strncmp(argv[i], "debug_file=", 11) == 0) { - const char *filename = argv[i] + 11; - debug_close(cfg->debug_file); - cfg->debug_file = debug_open(filename); - } - } - - debug_dbg(cfg, "called."); - debug_dbg(cfg, "flags %d argc %d", flags, argc); - for (i = 0; i < argc; i++) { - debug_dbg(cfg, "argv[%d]=%s", i, argv[i]); - } - debug_dbg(cfg, "max_devices=%d", cfg->max_devs); - debug_dbg(cfg, "debug=%d", cfg->debug); - debug_dbg(cfg, "interactive=%d", cfg->interactive); - debug_dbg(cfg, "cue=%d", cfg->cue); - debug_dbg(cfg, "nodetect=%d", cfg->nodetect); - debug_dbg(cfg, "userpresence=%d", cfg->userpresence); - debug_dbg(cfg, "userverification=%d", cfg->userverification); - debug_dbg(cfg, "pinverification=%d", cfg->pinverification); - debug_dbg(cfg, "manual=%d", cfg->manual); - debug_dbg(cfg, "nouserok=%d", cfg->nouserok); - debug_dbg(cfg, "openasuser=%d", cfg->openasuser); - debug_dbg(cfg, "alwaysok=%d", cfg->alwaysok); - debug_dbg(cfg, "sshformat=%d", cfg->sshformat); - debug_dbg(cfg, "expand=%d", cfg->expand); - debug_dbg(cfg, "authfile=%s", cfg->auth_file ? cfg->auth_file : "(null)"); - debug_dbg(cfg, "authpending_file=%s", - cfg->authpending_file ? cfg->authpending_file : "(null)"); - debug_dbg(cfg, "origin=%s", cfg->origin ? cfg->origin : "(null)"); - debug_dbg(cfg, "appid=%s", cfg->appid ? cfg->appid : "(null)"); - debug_dbg(cfg, "prompt=%s", cfg->prompt ? cfg->prompt : "(null)"); -} - static void interactive_prompt(pam_handle_t *pamh, const cfg_t *cfg) { char *tmp = NULL; @@ -185,7 +101,7 @@ int pam_sm_authenticate(pam_handle_t *pamh, int flags, int argc, int should_free_auth_file = 0; int should_free_authpending_file = 0; - parse_cfg(flags, argc, argv, cfg); + cfg_init(cfg, flags, argc, argv); PAM_MODUTIL_DEF_PRIVS(privs); @@ -400,9 +316,7 @@ int pam_sm_authenticate(pam_handle_t *pamh, int flags, int argc, } debug_dbg(cfg, "done. [%s]", pam_strerror(pamh, retval)); - debug_close(cfg->debug_file); - cfg->debug_file = DEFAULT_DEBUG_FILE; - + cfg_free(cfg); return retval; } diff --git a/util.h b/util.h index f3dac94..409c446 100644 --- a/util.h +++ b/util.h @@ -8,6 +8,8 @@ #include #include +#include "cfg.h" + #define BUFSIZE 1024 #define MAX_DEVS 24 #define DEFAULT_AUTHFILE_DIR_VAR "XDG_CONFIG_HOME" @@ -23,30 +25,6 @@ #define DEVLIST_LEN 64 -typedef struct { - unsigned max_devs; - int manual; - int debug; - int nouserok; - int openasuser; - int alwaysok; - int interactive; - int cue; - int nodetect; - int userpresence; - int userverification; - int pinverification; - int sshformat; - int expand; - const char *auth_file; - const char *authpending_file; - const char *origin; - const char *appid; - const char *prompt; - const char *cue_prompt; - FILE *debug_file; -} cfg_t; - typedef struct { char *publicKey; char *keyHandle; From 787f30ba7062da53ca11e1f1039d09058af5d638 Mon Sep 17 00:00:00 2001 From: Giovanni Simoni Date: Mon, 16 Dec 2024 16:31:47 +0100 Subject: [PATCH 2/6] pam: Config file support The configuration file defines the default behaviour of pam_u2f. Individual module invocations under /etc/pam.d can override settings. The file-system location of the config file is by default $sysconfdir/security/pam_u2f.conf, where $sysconfdir is supplied at build time. A new module configuration, "conf=", allows to override it at runtime. Only absolute paths are accepted. --- Makefile.am | 1 + cfg.c | 311 +++++++++++++++++++++++++++++++++++++++++++-------- cfg.h | 4 + configure.ac | 15 +++ pam-u2f.c | 4 +- 5 files changed, 285 insertions(+), 50 deletions(-) diff --git a/Makefile.am b/Makefile.am index e67d64c..f1413dc 100644 --- a/Makefile.am +++ b/Makefile.am @@ -17,6 +17,7 @@ AM_CPPFLAGS = $(LIBFIDO2_CFLAGS) $(LIBCRYPTO_CFLAGS) if ENABLE_FUZZING AM_CPPFLAGS += -fsanitize=fuzzer-no-link endif +AM_CPPFLAGS += -D SCONFDIR='"@SCONFDIR@"' noinst_LTLIBRARIES = libmodule.la libmodule_la_SOURCES = pam-u2f.c diff --git a/cfg.c b/cfg.c index 68ad532..98a213d 100644 --- a/cfg.c +++ b/cfg.c @@ -1,65 +1,274 @@ +/* Copyright (C) 2021-2024 Yubico AB - See COPYING */ +#include +#include +#include +#include #include +#include +#include + +#include #include "cfg.h" #include "debug.h" -int cfg_init(cfg_t *cfg, int flags, int argc, const char **argv) { - int i; +static void cfg_load_arg_debug(cfg_t *cfg, const char *arg) { + if (strcmp(arg, "debug") == 0) + cfg->debug = 1; + else if (strncmp(arg, "debug_file=", strlen("debug_file=")) == 0) { + debug_close(cfg->debug_file); + cfg->debug_file = debug_open(arg + strlen("debug_file=")); + } +} + +static void cfg_load_arg(cfg_t *cfg, const char *arg) { + if (strncmp(arg, "max_devices=", 12) == 0) { + sscanf(arg, "max_devices=%u", &cfg->max_devs); + } else if (strcmp(arg, "manual") == 0) { + cfg->manual = 1; + } else if (strcmp(arg, "nouserok") == 0) { + cfg->nouserok = 1; + } else if (strcmp(arg, "openasuser") == 0) { + cfg->openasuser = 1; + } else if (strcmp(arg, "alwaysok") == 0) { + cfg->alwaysok = 1; + } else if (strcmp(arg, "interactive") == 0) { + cfg->interactive = 1; + } else if (strcmp(arg, "cue") == 0) { + cfg->cue = 1; + } else if (strcmp(arg, "nodetect") == 0) { + cfg->nodetect = 1; + } else if (strcmp(arg, "expand") == 0) { + cfg->expand = 1; + } else if (strncmp(arg, "userpresence=", 13) == 0) { + sscanf(arg, "userpresence=%d", &cfg->userpresence); + } else if (strncmp(arg, "userverification=", 17) == 0) { + sscanf(arg, "userverification=%d", &cfg->userverification); + } else if (strncmp(arg, "pinverification=", 16) == 0) { + sscanf(arg, "pinverification=%d", &cfg->pinverification); + } else if (strncmp(arg, "authfile=", 9) == 0) { + cfg->auth_file = arg + 9; + } else if (strcmp(arg, "sshformat") == 0) { + cfg->sshformat = 1; + } else if (strncmp(arg, "authpending_file=", 17) == 0) { + cfg->authpending_file = arg + 17; + } else if (strncmp(arg, "origin=", 7) == 0) { + cfg->origin = arg + 7; + } else if (strncmp(arg, "appid=", 6) == 0) { + cfg->appid = arg + 6; + } else if (strncmp(arg, "prompt=", 7) == 0) { + cfg->prompt = arg + 7; + } else if (strncmp(arg, "cue_prompt=", 11) == 0) { + cfg->cue_prompt = arg + 11; + } else + cfg_load_arg_debug(cfg, arg); +} + +static int slurp(int fd, size_t to_read, char **dst) { + char *buffer, *w; + + if (to_read > CFG_MAX_FILE_SIZE) + return PAM_SERVICE_ERR; + + buffer = malloc(to_read + 1); + if (!buffer) + return PAM_BUF_ERR; + + w = buffer; + while (to_read) { + ssize_t r; + + r = read(fd, w, to_read); + if (r < 0) { + free(buffer); + return PAM_SYSTEM_ERR; + } + + if (r == 0) + break; + + w += r; + to_read -= r; + } + + *w = '\0'; + *dst = buffer; + return PAM_SUCCESS; +} + +// Open the given path while ensuring certain security properties hold. +// +// On success returns PAM_SUCCESS +// On failure returns PAM_SERVICE_ERR and sets errno to indicate the error. +static int open_safely(int *outfd, size_t *outsize, const char *path) { + int fd, serrno; + size_t len; + struct stat st; + + len = strlen(path); + if (!len || path[0] != '/' || path[len - 1] == '/') { + errno = EINVAL; + return PAM_SERVICE_ERR; + } + + fd = open(path, O_RDONLY | O_CLOEXEC | O_NOCTTY | O_NOFOLLOW, 0); + if (fd == -1) + return PAM_SERVICE_ERR; + + if (fstat(fd, &st)) + goto fail; + + errno = EINVAL; +#ifndef PAM_U2F_TESTING + if (st.st_uid != 0) + goto fail; +#endif + if (!S_ISREG(st.st_mode) || st.st_mode & (S_IWGRP | S_IWOTH)) + goto fail; + + errno = 0; + *outfd = fd; + *outsize = st.st_size; + return PAM_SUCCESS; + +fail: + serrno = errno; + close(fd); + errno = serrno; + return PAM_SERVICE_ERR; +} + +static char *ltrim(char *s) { + while (isspace((unsigned char) *s)) + s++; + return s; +} + +static char *rtrim(char *s) { + size_t l; + + l = strlen(s); + + while (l > 0 && isspace((unsigned char) s[l - 1])) + s[--l] = '\0'; + + return s; +} + +// Transform a line from the configuration file in an equivalent +// module command line value. Comments are stripped. +// +// E.g. +// 'foo = bar' => 'foo=bar' +// 'baz' => 'baz' +// 'baz # etc' => 'baz' +static const char *pack(char *s) { + size_t n; + char *v; + + s[strcspn(s, "#")] = '\0'; + s = ltrim(s); + v = strchr(s, '='); + if (!v) + return rtrim(s); + + *v++ = '\0'; + v = ltrim(rtrim(v)); + + s = rtrim(s); + n = strlen(s); + s[n++] = '='; + + memmove(s + n, v, strlen(v) + 1); + + return s; +} + +static void cfg_load_buffer(cfg_t *cfg, char *buffer) { + char *saveptr_out = NULL, *line; + + line = strtok_r(buffer, "\n", &saveptr_out); + while (line) { + char *buf; + const char *arg; + + // Pin the next line before messing with the buffer. + buf = line; + line = strtok_r(NULL, "\n", &saveptr_out); + + arg = pack(buf); + if (!*arg) + continue; + + cfg_load_arg(cfg, arg); + } +} + +static int cfg_load_defaults(cfg_t *cfg, const char *config_path) { + int fd, r; + size_t fsize; + char *buffer = NULL; + + r = open_safely(&fd, &fsize, config_path ? config_path : CFG_DEFAULT_PATH); + if (r) + return r; + + if (errno == ENOENT) { + // Only the default config file is allowed to be missing + return config_path ? PAM_SERVICE_ERR : PAM_SUCCESS; + } + + r = slurp(fd, fsize, &buffer); + if (r) + goto exit; + + cfg_load_buffer(cfg, buffer); + cfg->defaults_buffer = buffer; + buffer = NULL; + r = PAM_SUCCESS; + +exit: + free(buffer); + close(fd); + return r; +} + +static void cfg_reset(cfg_t *cfg) { memset(cfg, 0, sizeof(cfg_t)); cfg->debug_file = DEFAULT_DEBUG_FILE; cfg->userpresence = -1; cfg->userverification = -1; cfg->pinverification = -1; +} + +int cfg_init(cfg_t *cfg, int flags, int argc, const char **argv) { + int i, r; + const char *config_path = NULL; + + (void) flags; // prevent unused warning when unit-testing. + + cfg_reset(cfg); for (i = 0; i < argc; i++) { - if (strncmp(argv[i], "max_devices=", 12) == 0) { - sscanf(argv[i], "max_devices=%u", &cfg->max_devs); - } else if (strcmp(argv[i], "manual") == 0) { - cfg->manual = 1; - } else if (strcmp(argv[i], "debug") == 0) { - cfg->debug = 1; - } else if (strcmp(argv[i], "nouserok") == 0) { - cfg->nouserok = 1; - } else if (strcmp(argv[i], "openasuser") == 0) { - cfg->openasuser = 1; - } else if (strcmp(argv[i], "alwaysok") == 0) { - cfg->alwaysok = 1; - } else if (strcmp(argv[i], "interactive") == 0) { - cfg->interactive = 1; - } else if (strcmp(argv[i], "cue") == 0) { - cfg->cue = 1; - } else if (strcmp(argv[i], "nodetect") == 0) { - cfg->nodetect = 1; - } else if (strcmp(argv[i], "expand") == 0) { - cfg->expand = 1; - } else if (strncmp(argv[i], "userpresence=", 13) == 0) { - sscanf(argv[i], "userpresence=%d", &cfg->userpresence); - } else if (strncmp(argv[i], "userverification=", 17) == 0) { - sscanf(argv[i], "userverification=%d", &cfg->userverification); - } else if (strncmp(argv[i], "pinverification=", 16) == 0) { - sscanf(argv[i], "pinverification=%d", &cfg->pinverification); - } else if (strncmp(argv[i], "authfile=", 9) == 0) { - cfg->auth_file = argv[i] + 9; - } else if (strcmp(argv[i], "sshformat") == 0) { - cfg->sshformat = 1; - } else if (strncmp(argv[i], "authpending_file=", 17) == 0) { - cfg->authpending_file = argv[i] + 17; - } else if (strncmp(argv[i], "origin=", 7) == 0) { - cfg->origin = argv[i] + 7; - } else if (strncmp(argv[i], "appid=", 6) == 0) { - cfg->appid = argv[i] + 6; - } else if (strncmp(argv[i], "prompt=", 7) == 0) { - cfg->prompt = argv[i] + 7; - } else if (strncmp(argv[i], "cue_prompt=", 11) == 0) { - cfg->cue_prompt = argv[i] + 11; - } else if (strncmp(argv[i], "debug_file=", 11) == 0) { - const char *filename = argv[i] + 11; - debug_close(cfg->debug_file); - cfg->debug_file = debug_open(filename); - } + if (strncmp(argv[i], "conf=", strlen("conf=")) == 0) + config_path = argv[i] + strlen("conf="); + else + cfg_load_arg_debug(cfg, argv[i]); + } + + r = cfg_load_defaults(cfg, config_path); + if (r != PAM_SUCCESS) + goto exit; + + for (i = 0; i < argc; i++) { + if (strncmp(argv[i], "conf=", strlen("conf=")) == 0) + continue; + + cfg_load_arg(cfg, argv[i]); } +exit: if (cfg->debug) { debug_dbg(cfg, "called."); debug_dbg(cfg, "flags %d argc %d", flags, argc); @@ -88,10 +297,14 @@ int cfg_init(cfg_t *cfg, int flags, int argc, const char **argv) { debug_dbg(cfg, "prompt=%s", cfg->prompt ? cfg->prompt : "(null)"); } - return 0; + if (r != PAM_SUCCESS) + cfg_free(cfg); + + return r; } void cfg_free(cfg_t *cfg) { debug_close(cfg->debug_file); - cfg->debug_file = DEFAULT_DEBUG_FILE; + free(cfg->defaults_buffer); + cfg_reset(cfg); } diff --git a/cfg.h b/cfg.h index 32ac16c..07d242e 100644 --- a/cfg.h +++ b/cfg.h @@ -7,6 +7,9 @@ #include +#define CFG_DEFAULT_PATH (SCONFDIR "/pam_u2f.conf") +#define CFG_MAX_FILE_SIZE 4096 // Arbitrary + typedef struct { unsigned max_devs; int manual; @@ -29,6 +32,7 @@ typedef struct { const char *prompt; const char *cue_prompt; FILE *debug_file; + char *defaults_buffer; } cfg_t; int cfg_init(cfg_t *cfg, int flags, int argc, const char **argv); diff --git a/configure.ac b/configure.ac index 37c742c..b006241 100644 --- a/configure.ac +++ b/configure.ac @@ -59,6 +59,20 @@ AC_ARG_WITH(pam-dir, ]) AC_SUBST(PAMDIR, "$PAMDIR") +SCONFDIR="${sysconfdir}/security" +AC_ARG_WITH(sconf-dir, + AS_HELP_STRING( + [--with-sconf-dir=DIR], + [Path to module conf file] + ), [ + case "${withval}" in + /*) SCONFDIR="${withval}";; + *) AC_MSG_ERROR(expected an absolute directory name for --with-sconf-dir: ${withval});; + esac + ] +) +AC_SUBST(SCONFDIR, "$SCONFDIR") + PKG_CHECK_MODULES([LIBCRYPTO], [libcrypto], [], []) PKG_CHECK_MODULES([LIBFIDO2], [libfido2 >= 1.3.0], [], []) @@ -163,4 +177,5 @@ AC_MSG_NOTICE([Summary of build options: LIBCRYPTO CFLAGS: $LIBCRYPTO_CFLAGS LIBCRYPTO LIBS: $LIBCRYPTO_LIBS PAMDIR: $PAMDIR + SCONFDIR: $SCONFDIR ]) diff --git a/pam-u2f.c b/pam-u2f.c index 4b02b1c..86faed0 100644 --- a/pam-u2f.c +++ b/pam-u2f.c @@ -101,7 +101,9 @@ int pam_sm_authenticate(pam_handle_t *pamh, int flags, int argc, int should_free_auth_file = 0; int should_free_authpending_file = 0; - cfg_init(cfg, flags, argc, argv); + retval = cfg_init(cfg, flags, argc, argv); + if (retval != PAM_SUCCESS) + goto done; PAM_MODUTIL_DEF_PRIVS(privs); From dabe69e289e111abd467cf692da7788a719da8ef Mon Sep 17 00:00:00 2001 From: Giovanni Simoni Date: Thu, 5 Dec 2024 11:25:34 +0100 Subject: [PATCH 3/6] tests: Add unit tests for conf file --- .gitignore | 1 + tests/Makefile.am | 4 + tests/cfg.c | 395 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 400 insertions(+) create mode 100644 tests/cfg.c diff --git a/.gitignore b/.gitignore index 7c74d1a..89fdbb0 100644 --- a/.gitignore +++ b/.gitignore @@ -41,4 +41,5 @@ pamu2fcfg/cmdline.h pamu2fcfg/pamu2fcfg man/pamu2fcfg.1 tests/get_devices +tests/cfg fuzz/fuzz_format_parsers diff --git a/tests/Makefile.am b/tests/Makefile.am index 7a8124a..55f6b1a 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -19,4 +19,8 @@ get_devices_LDADD = $(top_builddir)/libmodule.la check_PROGRAMS += expand expand_LDADD = $(top_builddir)/libmodule.la +check_PROGRAMS += cfg +cfg_SOURCES = ./cfg.c ../cfg.c ../debug.c +cfg_CFLAGS = -DPAM_U2F_TESTING -DSCONFDIR='"@SCONFDIR@"' $(AM_CFLAGS) + TESTS = $(check_PROGRAMS) diff --git a/tests/cfg.c b/tests/cfg.c new file mode 100644 index 0000000..49f1891 --- /dev/null +++ b/tests/cfg.c @@ -0,0 +1,395 @@ +/* Copyright (C) 2021-2024 Yubico AB - See COPYING */ +#undef NDEBUG + +#include +#include +#include +#include +#include + +#ifndef PATH_MAX +#define PATH_MAX 4096 +#endif + +#include + +#include "cfg.h" + +static char *generate_template(void) { + // Generate a conf= argument + // + // The function returns a string which is: + // - suitable as argv item for pam_u2f + // - suitable as template argument for mkstemp + // - optionally referring to the absolute path of the temporary file. + + char *template; + char cwd[PATH_MAX]; + int err; + + err = !getcwd(cwd, sizeof(cwd)); + assert(!err); + + err = asprintf(&template, "conf=%.*s/test_config_XXXXXX", (int) sizeof(cwd), + cwd) == -1; + assert(!err); + + return template; +} + +struct conf_file { + char *arg; + const char *path; + FILE *out; +}; + +static void conf_file_init(struct conf_file *cf, const char *template) { + int fd; + char *path; + + memset(cf, 0, sizeof *cf); + + if (template) { + cf->arg = strdup(template); + assert(cf->arg); + } else + cf->arg = generate_template(); + + path = cf->arg + strlen("conf="); + fd = mkstemp(path); + assert(fd != -1); + + cf->path = path; + cf->out = fdopen(fd, "w"); + assert(cf->out); +} + +static void conf_file_clear(struct conf_file *cf) { + unlink(cf->path); + fclose(cf->out); + free(cf->arg); +} + +static void config_different_str(FILE *conf_out, const char *key, + const char *default_value) { + // Adding '!' to make it different. + fprintf(conf_out, "%s=%s!\n", key, default_value ? default_value : ""); +} + +static void config_different_bool(FILE *conf_out, const char *key, + int default_value) { + if (!default_value) + fprintf(conf_out, "%s\n", key); +} + +static void config_different_treestate(FILE *conf_out, const char *key, + int default_value) { + int new_value; + + assert(default_value >= -1 && default_value <= 1); + + // -1 => 0 + // 0 => 1 + // 1 => -1 + new_value = ((default_value + 2) % 3) - 1; + + if (new_value >= 0) + fprintf(conf_out, "%s=%d\n", key, new_value); +} + +static void config_flip_all(const struct conf_file *cf, const cfg_t *cfg) { + // Loads hard-wired defaults, and dumps + // into conf_fd a config file that changes all of them. + + FILE *conf_out = cf->out; + + config_different_bool(conf_out, "alwaysok", cfg->alwaysok); + config_different_bool(conf_out, "cue", cfg->cue); + config_different_bool(conf_out, "debug", cfg->debug); + config_different_bool(conf_out, "expand", cfg->expand); + config_different_bool(conf_out, "interactive", cfg->interactive); + config_different_bool(conf_out, "manual", cfg->manual); + config_different_bool(conf_out, "nodetect", cfg->nodetect); + config_different_bool(conf_out, "nouserok", cfg->nouserok); + config_different_bool(conf_out, "openasuser", cfg->openasuser); + config_different_bool(conf_out, "sshformat", cfg->sshformat); + + config_different_str(conf_out, "appid", cfg->appid); + config_different_str(conf_out, "authfile", cfg->auth_file); + config_different_str(conf_out, "authpending_file", cfg->authpending_file); + config_different_str(conf_out, "cue_prompt", cfg->cue_prompt); + config_different_str(conf_out, "origin", cfg->origin); + config_different_str(conf_out, "prompt", cfg->prompt); + + config_different_treestate(conf_out, "pinverification", cfg->pinverification); + config_different_treestate(conf_out, "userpresence", cfg->userpresence); + config_different_treestate(conf_out, "userverification", + cfg->userverification); + + fprintf(conf_out, "max_devices=%d\n", cfg->max_devs + 1); + + if (cfg->debug_file) + fprintf(conf_out, "debug_file=syslog\n"); + else + fprintf(conf_out, "debug_file=stderr\n"); + + fflush(conf_out); +} + +static int str_opt_cmp(const char *s1, const char *s2) { + if ((!s1) != (!s2)) + return s1 ? -1 : 1; + + if (!s1) + return 0; + + return strcmp(s1, s2); +} + +static void test_regular(void) { + // Ensure that all configuration options are loaded into the configuration: + + const char *argv[] = {NULL, + "debug", // So we have a log file for the test + "prompt=hi"}; + + struct conf_file cf; + int r; + cfg_t cfg, cfg_defaults; + + conf_file_init(&cf, NULL); + argv[0] = cf.arg; + + // 1. Load the default + r = cfg_init(&cfg_defaults, 0, 1, argv); + assert(r == PAM_SUCCESS); + + // 2. Write the configuration file, changing every field. + config_flip_all(&cf, &cfg_defaults); + + // 3. Load from the file + r = cfg_init(&cfg, 0, sizeof(argv) / sizeof(*argv), argv); + assert(r == PAM_SUCCESS); + conf_file_clear(&cf); + + // 4. Assert that every field is different from the default. + assert(cfg.max_devs != cfg_defaults.max_devs); + assert(cfg.manual != cfg_defaults.manual); + assert(cfg.debug != cfg_defaults.debug); + assert(cfg.nouserok != cfg_defaults.nouserok); + assert(cfg.openasuser != cfg_defaults.openasuser); + assert(cfg.alwaysok != cfg_defaults.alwaysok); + assert(cfg.interactive != cfg_defaults.interactive); + assert(cfg.cue != cfg_defaults.cue); + assert(cfg.nodetect != cfg_defaults.nodetect); + assert(cfg.userpresence != cfg_defaults.userpresence); + assert(cfg.userverification != cfg_defaults.userverification); + assert(cfg.pinverification != cfg_defaults.pinverification); + assert(cfg.sshformat != cfg_defaults.sshformat); + assert(cfg.expand != cfg_defaults.expand); + + assert(str_opt_cmp(cfg.auth_file, cfg_defaults.auth_file)); + assert(str_opt_cmp(cfg.authpending_file, cfg_defaults.authpending_file)); + assert(str_opt_cmp(cfg.origin, cfg_defaults.origin)); + assert(str_opt_cmp(cfg.appid, cfg_defaults.appid)); + assert(str_opt_cmp(cfg.prompt, cfg_defaults.prompt)); + assert(str_opt_cmp(cfg.cue_prompt, cfg_defaults.cue_prompt)); + + assert(cfg.debug_file != cfg_defaults.debug_file); + + cfg_free(&cfg); +} + +static void test_config_abspath(void) { + /* Ensuring that the library rejects the conf= argument + * unless it points to an absolute path. + */ + + struct conf_file cf; + const char *argv[] = { + NULL, // replaced with config_arg_{...} + "debug", // So we have a log file for the test + }; + int r; + cfg_t cfg; + + // 1. Generate a valid configuration and pass it around + // as relative path. Assert failure. + conf_file_init(&cf, "conf=test_config_XXXXXX"); + fputs("alwaysok\n" + "prompt=hello", + cf.out); + r = fflush(cf.out); + assert(r == 0); + + argv[0] = cf.arg; + r = cfg_init(&cfg, 0, sizeof(argv) / sizeof(*argv), argv); + assert(r == PAM_SERVICE_ERR); + conf_file_clear(&cf); + + // 2. Generate a same configuration and pass it around + // as absolute path. Assert success. + conf_file_init(&cf, NULL); + fputs("alwaysok\n" + "prompt=hello", + cf.out); + r = fflush(cf.out); + assert(r == 0); + + argv[0] = cf.arg; + r = cfg_init(&cfg, 0, sizeof(argv) / sizeof(*argv), argv); + assert(r == PAM_SUCCESS); + + assert(strcmp(cfg.prompt, "hello") == 0); + conf_file_clear(&cf); + + cfg_free(&cfg); +} + +static void test_last_config_wins(void) { + // If conf= is used multiple times, only + // the last one is honored. + + const char *argv[3] = {NULL, NULL, "debug"}; + struct conf_file cf_1, cf_2; + int r; + cfg_t cfg; + + conf_file_init(&cf_1, NULL); + conf_file_init(&cf_2, NULL); + + fputs("max_devices=10\n", cf_1.out); + fflush(cf_1.out); + fputs("max_devices=12\n", cf_2.out); + fflush(cf_2.out); + + argv[0] = cf_1.arg; + argv[1] = cf_2.arg; + r = cfg_init(&cfg, 0, sizeof(argv) / sizeof(*argv), argv); + assert(r == PAM_SUCCESS); + assert(cfg.max_devs == 12); + cfg_free(&cfg); + + argv[0] = cf_2.arg; + argv[1] = cf_1.arg; + r = cfg_init(&cfg, 0, sizeof(argv) / sizeof(*argv), argv); + assert(r == PAM_SUCCESS); + assert(cfg.max_devs == 10); + cfg_free(&cfg); + + conf_file_clear(&cf_1); + conf_file_clear(&cf_2); +} + +static void test_file_corner_cases(void) { + // Testng config file corner cases. + + const char *argv[] = {NULL, "debug"}; + struct conf_file cf; + int r; + cfg_t cfg; + char buffer[CFG_MAX_FILE_SIZE]; + + conf_file_init(&cf, NULL); + argv[0] = cf.arg; + + // 1. Empty file -> Success + r = cfg_init(&cfg, 0, sizeof(argv) / sizeof(*argv), argv); + assert(r == PAM_SUCCESS); + cfg_free(&cfg); + + // 2. File size within limit -> Success + memset(buffer, ' ', sizeof(buffer)); + memcpy(buffer, "manual\n", strlen("manual\n")); + r = fwrite(buffer, sizeof(buffer), 1, cf.out) != 1; + assert(!r); + r = fflush(cf.out); + assert(r == 0); + r = cfg_init(&cfg, 0, sizeof(argv) / sizeof(*argv), argv); + assert(r == PAM_SUCCESS); + cfg_free(&cfg); + + // 3. File size beyond limit -> Failure + r = fwrite("manual\n", strlen("manual\n"), 1, cf.out) != 1; + assert(!r); + r = fflush(cf.out); + assert(r == 0); + r = cfg_init(&cfg, 0, sizeof(argv) / sizeof(*argv), argv); + assert(r == PAM_SERVICE_ERR); + + // 4. Missing file -> Failure + argv[0] = "conf=/not/the/droids/you/are/looking/for"; + r = cfg_init(&cfg, 0, sizeof(argv) / sizeof(*argv), argv); + assert(r == PAM_SERVICE_ERR); + + conf_file_clear(&cf); +} + +static void test_file_parser(void) { + cfg_t cfg_defaults, cfg; + const char *argv[] = { + NULL, "debug", + "cu", // not 'cue' + }; + struct conf_file cf; + int r; + + conf_file_init(&cf, NULL); + argv[0] = cf.arg; + + r = cfg_init(&cfg_defaults, 0, 1, argv); + assert(r == PAM_SUCCESS); + + // Defaults are unlikely to change, but if they do + // the test might be invalidated. + assert(!cfg_defaults.alwaysok); + assert(!cfg_defaults.prompt); + assert(!cfg_defaults.cue_prompt); + assert(!cfg_defaults.auth_file); + assert(!cfg_defaults.interactive); + assert(!cfg_defaults.cue); + assert(!cfg_defaults.origin); + assert(!cfg_defaults.appid); + assert(!cfg_defaults.appid); + assert(!cfg_defaults.authpending_file); + + fputs(" \n", cf.out); + fputs(" # interactive \n", cf.out); + fputs(" alwaysok # I really mean it.\n", cf.out); + fputs("prompt = C:/> # DOS like a boss.\n", cf.out); + fputs("cue_prompt = =C:/ > # DOS in space.\n", cf.out); + fputs("authfile = /dev/null \n", cf.out); + fputs("interactive \n", cf.out); + fputs("cu # Not 'cue'\n", cf.out); + fputs("cu\n", cf.out); + fputs("origin unknown\n", cf.out); + fputs("appid= something\n", cf.out); + fputs("authpending_file =else\n", cf.out); + fflush(cf.out); + + r = cfg_init(&cfg, 0, sizeof(argv) / sizeof(*argv), argv); + assert(r == PAM_SUCCESS); + + assert(cfg.alwaysok); + assert(strcmp(cfg.prompt, "C:/>") == 0); + assert(strcmp(cfg.cue_prompt, "=C:/ >") == 0); + assert(strcmp(cfg.auth_file, "/dev/null") == 0); + assert(cfg.interactive); + assert(!cfg.cue); + assert(!cfg.origin); + assert(strcmp(cfg.appid, "something") == 0); + assert(strcmp(cfg.authpending_file, "else") == 0); + + cfg_free(&cfg); + conf_file_clear(&cf); +} + +int main(int argc, char **argv) { + (void) argc, (void) argv; + + test_regular(); + test_config_abspath(); + test_last_config_wins(); + test_file_corner_cases(); + test_file_parser(); +} From 0be95f15f1a64c7702b714c0b42cb4f88b05864b Mon Sep 17 00:00:00 2001 From: Giovanni Simoni Date: Wed, 11 Dec 2024 09:01:20 +0100 Subject: [PATCH 4/6] fuzz: Integrate cfg with libfuzzer testing - split-input format: add trailing blob for config file The corpus needs some update. - wrappers (-Wl,--wrap) integrate fuzzing of the configuration file. The configuration file, mutated by the fuzzer, is made available to the cfg.c implementation. The mock-up works under the assumption that only the cfg.c module works by opening "/" with open(3), and follows up with an alternation of openat(3) and fstat(3) calls. --- Makefile.am | 1 + fuzz/Makefile.am | 1 + fuzz/export.sym | 2 ++ fuzz/fuzz.h | 2 ++ fuzz/fuzz_auth.c | 90 +++++++++++++++++++++++++++++++++++++++++++----- fuzz/wrap.c | 41 ++++++++++++++++++---- 6 files changed, 122 insertions(+), 15 deletions(-) diff --git a/Makefile.am b/Makefile.am index f1413dc..1b2b0cb 100644 --- a/Makefile.am +++ b/Makefile.am @@ -46,6 +46,7 @@ pam_u2f_la_LDFLAGS += -Wl,--wrap=strdup pam_u2f_la_LDFLAGS += -Wl,--wrap=calloc pam_u2f_la_LDFLAGS += -Wl,--wrap=malloc pam_u2f_la_LDFLAGS += -Wl,--wrap=open +pam_u2f_la_LDFLAGS += -Wl,--wrap=openat pam_u2f_la_LDFLAGS += -Wl,--wrap=close pam_u2f_la_LDFLAGS += -Wl,--wrap=fdopen pam_u2f_la_LDFLAGS += -Wl,--wrap=fstat diff --git a/fuzz/Makefile.am b/fuzz/Makefile.am index 70b4c3a..bf89634 100644 --- a/fuzz/Makefile.am +++ b/fuzz/Makefile.am @@ -1,6 +1,7 @@ # Copyright (C) 2020 Yubico AB - See COPYING AM_CFLAGS = $(CWFLAGS) $(CSFLAGS) -fsanitize=fuzzer AM_CPPFLAGS = $(LIBFIDO2_CFLAGS) $(LIBCRYPTO_CFLAGS) -I$(srcdir)/.. +AM_CPPFLAGS += -D SCONFDIR='"@SCONFDIR@"' AM_LDFLAGS = -no-install -fsanitize=fuzzer fuzz_format_parsers_SOURCES = fuzz_format_parsers.c diff --git a/fuzz/export.sym b/fuzz/export.sym index a36d378..afecd3a 100644 --- a/fuzz/export.sym +++ b/fuzz/export.sym @@ -5,3 +5,5 @@ set_authfile set_conv set_user set_wiredata +set_conf_file_fd +set_conf_file_path diff --git a/fuzz/fuzz.h b/fuzz/fuzz.h index 461b3e6..32afb52 100644 --- a/fuzz/fuzz.h +++ b/fuzz/fuzz.h @@ -21,6 +21,8 @@ void set_wiredata(uint8_t *, size_t); void set_user(const char *); void set_conv(struct pam_conv *); void set_authfile(int); +void set_conf_file_path(const char *); +void set_conf_file_fd(int); int pack_u32(uint8_t **, size_t *, uint32_t); int unpack_u32(const uint8_t **, size_t *, uint32_t *); diff --git a/fuzz/fuzz_auth.c b/fuzz/fuzz_auth.c index 3c76119..7d6928a 100644 --- a/fuzz/fuzz_auth.c +++ b/fuzz/fuzz_auth.c @@ -11,6 +11,7 @@ #include #include +#include "cfg.h" #include "fuzz/fuzz.h" #include "fuzz/wiredata.h" #include "fuzz/authfile.h" @@ -32,6 +33,7 @@ struct param { char conv[MAXSTR]; struct blob authfile; struct blob wiredata; + struct blob conf_file; }; struct conv_appdata { @@ -48,6 +50,29 @@ static const char dummy_authfile[] = AUTHFILE_SSH; /* module configuration split by fuzzer on semicolon */ static const char *dummy_conf = "sshformat;pinverification=0;manual;"; +/* module configuration file */ +static const char dummy_conf_file[] = "max_devices=10\n" + "manual\n" + "debug\n" + "nouserok\n" + "openasuser\n" + "alwaysok\n" + "interactive\n" + "cue\n" + "nodetect\n" + "expand\n" + "userpresence=0\n" + "userverification=0\n" + "pinverification=0\n" + "authfile=/foo/bar\n" + "sshformat\n" + "authpending_file=/baz/quux\n" + "origin=pam://lolcalhost\n" + "appid=pam://lolcalhost\n" + "prompt=hello\n" + "cue_prompt=howdy\n" + "debug_file=stdout\n"; + /* conversation dummy for manual authentication */ static const char *dummy_conv = "94/ZgCC5htEl9SRmTRfUffKCzU/2ScRJYNFSlC5U+ik=\n" @@ -72,7 +97,8 @@ static size_t pack(uint8_t *data, size_t len, const struct param *p) { pack_string(&data, &len, p->conf) != 1 || pack_string(&data, &len, p->conv) != 1 || pack_blob(&data, &len, &p->authfile) != 1 || - pack_blob(&data, &len, &p->wiredata) != 1) { + pack_blob(&data, &len, &p->wiredata) != 1 || + pack_blob(&data, &len, &p->conf_file) != 1) { return 0; } @@ -106,7 +132,8 @@ static size_t pack_dummy(uint8_t *data, size_t len) { !set_string(dummy.conf, dummy_conf, MAXSTR) || !set_string(dummy.conv, dummy_conv, MAXSTR) || !set_blob(&dummy.authfile, dummy_authfile, sizeof(dummy_authfile)) || - !set_blob(&dummy.wiredata, dummy_wiredata, sizeof(dummy_wiredata))) { + !set_blob(&dummy.wiredata, dummy_wiredata, sizeof(dummy_wiredata)) || + !set_blob(&dummy.conf_file, dummy_conf_file, sizeof(dummy_conf_file))) { assert(0); /* dummy couldn't be prepared */ return 0; } @@ -125,7 +152,8 @@ static struct param *unpack(const uint8_t *data, size_t len) { unpack_string(&data, &len, p->conf) != 1 || unpack_string(&data, &len, p->conv) != 1 || unpack_blob(&data, &len, &p->authfile) != 1 || - unpack_blob(&data, &len, &p->wiredata) != 1) { + unpack_blob(&data, &len, &p->wiredata) != 1 || + unpack_blob(&data, &len, &p->conf_file) != 1) { free(p); return NULL; } @@ -153,6 +181,7 @@ static void mutate(struct param *p, uint32_t seed) { mutate_string(p->conf, MAXSTR); mutate_string(p->conv, MAXSTR); mutate_blob(&p->authfile); + mutate_blob(&p->conf_file); } if (flags & MUTATE_WIREDATA) mutate_blob(&p->wiredata); @@ -231,14 +260,47 @@ static int prepare_authfile(const unsigned char *data, size_t len) { return fd; } +static int prepare_conf_file(const struct blob *conf_file, int argc, + const char **argv, const char **conf_file_path) { + int i, fd; + ssize_t w; + + *conf_file_path = CFG_DEFAULT_PATH; + for (i = 0; i < argc; i++) { + const char *value; + + if (strncmp(argv[i], "conf=", strlen("conf="))) + continue; + + value = argv[i] + strlen("conf="); + *conf_file_path = value; + } + + if ((fd = memfd_create("pam_u2f.conf", MFD_CLOEXEC)) == -1) + return -1; + + w = write(fd, conf_file->body, conf_file->len); + if (w == -1 || (size_t) w != conf_file->len) + goto fail; + + if (lseek(fd, 0, SEEK_SET) == -1) + goto fail; + + return fd; + +fail: + close(fd); + return -1; +} + int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) { struct param *param = NULL; struct pam_conv conv; struct conv_appdata conv_data; - const char *argv[32]; + const char *argv[32], *conf_file_path; int argc = 32; - int fd = -1; + int authfile_fd = -1, conf_file_fd = -1; memset(&argv, 0, sizeof(*argv)); memset(&conv, 0, sizeof(conv)); @@ -256,16 +318,26 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) { set_user(param->user); set_wiredata(param->wiredata.body, param->wiredata.len); - if ((fd = prepare_authfile(param->authfile.body, param->authfile.len)) == -1) + if ((authfile_fd = + prepare_authfile(param->authfile.body, param->authfile.len)) == -1) goto err; - set_authfile(fd); + set_authfile(authfile_fd); prepare_argv(param->conf, &argv[0], &argc); + + if ((conf_file_fd = prepare_conf_file(¶m->conf_file, argc, argv, + &conf_file_path)) == -1) + goto err; + set_conf_file_path(conf_file_path); + set_conf_file_fd(conf_file_fd); + pam_sm_authenticate((void *) FUZZ_PAM_HANDLE, 0, argc, argv); err: - if (fd != -1) - close(fd); + if (authfile_fd != -1) + close(authfile_fd); + if (conf_file_fd != -1) + close(conf_file_fd); free(param); return 0; } diff --git a/fuzz/wrap.c b/fuzz/wrap.c index ee1f2ce..98f392b 100644 --- a/fuzz/wrap.c +++ b/fuzz/wrap.c @@ -35,6 +35,9 @@ static const char *user_ptr = NULL; static struct pam_conv *conv_ptr = NULL; static uint8_t *wiredata_ptr = NULL; static size_t wiredata_len = 0; +static const char *conf_file_path = NULL; +static int conf_file_fd = -1; +static int conf_file_fd_lastdup = -1; static int authfile_fd = -1; static char env[] = "value"; @@ -56,6 +59,8 @@ void set_wiredata(uint8_t *data, size_t len) { } void set_user(const char *user) { user_ptr = user; } void set_conv(struct pam_conv *conv) { conv_ptr = conv; } +void set_conf_file_path(const char *path) { conf_file_path = path; } +void set_conf_file_fd(int fd) { conf_file_fd = fd; } void set_authfile(int fd) { authfile_fd = fd; } WRAP(int, close, (int fd), -1, (fd)) @@ -65,7 +70,6 @@ WRAP(void *, malloc, (size_t size), NULL, (size)) WRAP(int, gethostname, (char *name, size_t len), -1, (name, len)) WRAP(ssize_t, getline, (char **s, size_t *n, FILE *fp), -1, (s, n, fp)) WRAP(FILE *, fdopen, (int fd, const char *mode), NULL, (fd, mode)) -WRAP(int, fstat, (int fd, struct stat *st), -1, (fd, st)) WRAP(BIO *, BIO_new, (const BIO_METHOD *type), NULL, (type)) WRAP(int, BIO_write, (BIO * b, const void *data, int len), -1, (b, data, len)) WRAP(int, BIO_read, (BIO * b, void *data, int len), -1, (b, data, len)) @@ -75,6 +79,23 @@ WRAP(BIO *, BIO_new_mem_buf, (const void *buf, int len), NULL, (buf, len)) WRAP(EC_KEY *, EC_KEY_new_by_curve_name, (int nid), NULL, (nid)) WRAP(const EC_GROUP *, EC_KEY_get0_group, (const EC_KEY *key), NULL, (key)) +extern int __real_fstat(int fildes, struct stat *buf); +extern int __wrap_fstat(int fildes, struct stat *buf); +extern int __wrap_fstat(int fildes, struct stat *buf) { + int r; + + assert(fildes >= 0); + assert(buf != NULL); + + r = __real_fstat(fildes, buf); + if (!r && (fildes == conf_file_fd_lastdup)) { + buf->st_uid = 0; + buf->st_mode &= ~(S_IWGRP | S_IWOTH); + } + + return r; +} + extern ssize_t __real_read(int fildes, void *buf, size_t nbyte); extern ssize_t __wrap_read(int fildes, void *buf, size_t nbyte); extern ssize_t __wrap_read(int fildes, void *buf, size_t nbyte) { @@ -109,19 +130,27 @@ extern uid_t __wrap_geteuid(void) { extern int __real_open(const char *pathname, int flags); extern int __wrap_open(const char *pathname, int flags); extern int __wrap_open(const char *pathname, int flags) { + if (prng_up && uniform_random(400) < 1) return -1; + /* open write-only files as /dev/null */ if ((flags & O_ACCMODE) == O_WRONLY) return __real_open("/dev/null", flags); + + assert((flags & O_ACCMODE) == O_RDONLY); + /* FIXME: special handling for /dev/random */ if (strcmp(pathname, "/dev/urandom") == 0) return __real_open(pathname, flags); - /* open read-only files using a shared fd for the authfile */ - if ((flags & O_ACCMODE) == O_RDONLY) - return dup(authfile_fd); - assert(0); /* unsupported */ - return -1; + + if (conf_file_path && strcmp(pathname, conf_file_path) == 0) { + assert(*pathname == '/'); /* should not load config from relative path */ + conf_file_fd_lastdup = dup(conf_file_fd); + return conf_file_fd_lastdup; + } + + return dup(authfile_fd); } extern int __wrap_getpwuid_r(uid_t, struct passwd *, char *, size_t, From 727d605d63ed0d776efe4a4caa57ac910c02fddc Mon Sep 17 00:00:00 2001 From: Giovanni Simoni Date: Wed, 18 Dec 2024 10:49:29 +0100 Subject: [PATCH 5/6] README: update with info about conf file --- README | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/README b/README index 17c2f40..2a10ffd 100644 --- a/README +++ b/README @@ -108,6 +108,7 @@ recommended that you start a separate shell with root privileges while configuring PAM to be able to revert changes if something goes wrong. Test your configuration thoroughly before closing the root shell. +[[moduleArguments]] === Module Arguments [horizontal] @@ -240,6 +241,14 @@ FIDO devices. It is not possible to mix native credentials and SSH credentials. Once this option is enabled all credentials will be parsed as SSH. +conf=/path/to/pam_u2f.conf:: +Set an alternative location for the <>. +The supplied path must be absolute and must correspond to an existing +regular file. + +The options specified on the module command line override the values +from the <>. + IMPORTANT: On dynamic networks (e.g. where hostnames are set by DHCP), users should not rely on the default origin and appid ("pam://$HOSTNAME") but set those parameters explicitly to the same @@ -404,6 +413,29 @@ defined in the authorization mapping file. If during an authentication attempt a connected device is removed or a new device is plugged in, the authentication restarts from the top of the list. +[[confFile]] +== Configuration file + +A configuration file can be used to set the default +<>. + +The file has a `name = value` format, with comments starting with the `#` +character. +White spaces at the beginning of line, end of line, and around +the `=` sign are ignored. + +Any `conf` argument in the configuration file is ignored. + +The maximum size for the configuration file is 4 KiB. + +The default path for the configuration file is +`/etc/security/pam_u2f.conf`. Note that it may have been set to another +value by the distribution. The default file is allowed to not exist. An +alternative path may be set in the module command line options. + +The options specified on the module command line override the values +from the configuration file. + == SELinux Note Due to an issue with Fedora Linux, and possibly with other From 6faab0d1901344e0b5926419f646458e83eb92d0 Mon Sep 17 00:00:00 2001 From: Giovanni Simoni Date: Wed, 18 Dec 2024 10:49:41 +0100 Subject: [PATCH 6/6] man: update with info about conf file Generate pam_u2f.8.txt from pam_u2f.8.txt.in, replacing SCONFDIR --- .gitignore | 1 + configure.ac | 1 + man/Makefile.am | 1 + man/{pam_u2f.8.txt => pam_u2f.8.txt.in} | 28 +++++++++++++++++++++++++ 4 files changed, 31 insertions(+) rename man/{pam_u2f.8.txt => pam_u2f.8.txt.in} (90%) diff --git a/.gitignore b/.gitignore index 89fdbb0..8a7ddcb 100644 --- a/.gitignore +++ b/.gitignore @@ -36,6 +36,7 @@ m4/lt~obsolete.m4 tests/.deps/ tests/dlsym_check man/pam_u2f.8 +man/pam_u2f.8.txt pamu2fcfg/cmdline.c pamu2fcfg/cmdline.h pamu2fcfg/pamu2fcfg diff --git a/configure.ac b/configure.ac index b006241..e4b2df0 100644 --- a/configure.ac +++ b/configure.ac @@ -115,6 +115,7 @@ AC_CONFIG_FILES([ tests/Makefile fuzz/Makefile man/Makefile + man/pam_u2f.8.txt ]) creduser=$(whoami) diff --git a/man/Makefile.am b/man/Makefile.am index 888bb01..c14e893 100644 --- a/man/Makefile.am +++ b/man/Makefile.am @@ -4,6 +4,7 @@ dist_man1_MANS = pamu2fcfg.1 dist_man8_MANS = pam_u2f.8 MAINTAINERCLEANFILES = $(MANS) EXTRA_DIST = $(MANS:=.txt) +DISTCLEANFILES = pam_u2f.8 SUFFIXES = .1.txt .1 .8.txt .8 diff --git a/man/pam_u2f.8.txt b/man/pam_u2f.8.txt.in similarity index 90% rename from man/pam_u2f.8.txt rename to man/pam_u2f.8.txt.in index 4524d39..41e8b42 100644 --- a/man/pam_u2f.8.txt +++ b/man/pam_u2f.8.txt.in @@ -134,6 +134,12 @@ FIDO devices. It is not possible to mix native credentials and SSH credentials. Once this option is enabled all credentials will be parsed as SSH. +*conf*=_path/to/pam_u2f.conf_:: +Set an alternative location for the configuration file. +The supplied path must be absolute and must correspond to an existing +regular file. +See *CONFIGURATION FILE*. + == EXAMPLES Second factor authentication deferring user verification configuration to the @@ -162,6 +168,28 @@ mapping file in an encrypted home directory, will result in the impossibility of logging into the system. The partition is decrypted after login and the mapping file can not be accessed. +== CONFIGURATION FILE + +A configuration file can be used to set the default module arguments. + +The file has a `name = value` format, with comments starting with the `#` +character. +White spaces at the beginning of line, end of line, and around +the `=` sign are ignored. + +Any `conf` argument in the configuration file is ignored. + +The maximum size for the configuration file is 4 KiB. + +The default path for the configuration file is +`@SCONFDIR@/pam_u2f.conf`. +Note that it may have been set to another value by the distribution. +The default file is allowed to not exist. +An alternative path may be set in the module command line options. + +The options specified on the module command line override the values +from the configuration file. + == NOTES *Nodetect*