diff --git a/.github/workflows/alpine_builds.yml b/.github/workflows/alpine_builds.yml index badf72d..abd26f7 100644 --- a/.github/workflows/alpine_builds.yml +++ b/.github/workflows/alpine_builds.yml @@ -4,7 +4,7 @@ on: [push, pull_request] jobs: build: - runs-on: ubuntu-22.04 + runs-on: ubuntu-24.04 container: alpine:latest strategy: fail-fast: false diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml index dc41c4a..365c8f7 100644 --- a/.github/workflows/codeql-analysis.yml +++ b/.github/workflows/codeql-analysis.yml @@ -20,7 +20,7 @@ jobs: # Initializes the CodeQL tools for scanning. - name: Initialize CodeQL - uses: github/codeql-action/init@v2 + uses: github/codeql-action/init@v3 - name: Build project run: | @@ -34,4 +34,4 @@ jobs: make - name: Perform CodeQL Analysis - uses: github/codeql-action/analyze@v2 + uses: github/codeql-action/analyze@v3 diff --git a/.github/workflows/format.yml b/.github/workflows/format.yml index 535343a..e24c599 100644 --- a/.github/workflows/format.yml +++ b/.github/workflows/format.yml @@ -4,7 +4,7 @@ on: [push, pull_request] jobs: format: - runs-on: ubuntu-22.04 + runs-on: ubuntu-24.04 env: CLANG_FORMAT_VERSION: -15 steps: diff --git a/.github/workflows/linux_builds.yml b/.github/workflows/linux_builds.yml index a452c69..9250644 100644 --- a/.github/workflows/linux_builds.yml +++ b/.github/workflows/linux_builds.yml @@ -9,6 +9,8 @@ jobs: fail-fast: false matrix: include: + - { os: ubuntu-24.04, cc: gcc-13 } + - { os: ubuntu-24.04, cc: clang-18 } - { os: ubuntu-22.04, cc: gcc-12 } - { os: ubuntu-22.04, cc: clang-15 } - { os: ubuntu-20.04, cc: gcc-10 } diff --git a/.github/workflows/linux_fuzz.yml b/.github/workflows/linux_fuzz.yml index b11c3bd..9131a82 100644 --- a/.github/workflows/linux_fuzz.yml +++ b/.github/workflows/linux_fuzz.yml @@ -8,8 +8,8 @@ jobs: strategy: fail-fast: false matrix: - os: [ubuntu-22.04] - cc: [clang-15] + os: [ubuntu-24.04] + cc: [clang-18] sanitizer: [asan] steps: - uses: actions/checkout@v4 diff --git a/.github/workflows/macos_builds.yml b/.github/workflows/macos_builds.yml index 04faf3c..84d8cbc 100644 --- a/.github/workflows/macos_builds.yml +++ b/.github/workflows/macos_builds.yml @@ -8,12 +8,12 @@ jobs: strategy: fail-fast: false matrix: - os: [ macos-11, macos-12, macos-13 ] + os: [ macos-13, macos-14, macos-15 ] cc: [ clang ] steps: - uses: actions/checkout@v4 - name: dependencies - run: brew install check cmake help2man libfido2 mandoc pkg-config automake + run: brew install check cmake help2man libfido2 mandoc libtool automake - name: build env: CC: ${{ matrix.cc }} diff --git a/.gitignore b/.gitignore index 7c74d1a..85d22b9 100644 --- a/.gitignore +++ b/.gitignore @@ -1,10 +1,10 @@ *.bak +*.cred *.l[ao] *.log *.o *.trs *~ -*.cred .deps .dirstamp .libs @@ -14,7 +14,6 @@ Makefile Makefile.in aclocal.m4 autom4te.cache/ -build/ build-aux/ar-lib build-aux/compile build-aux/config.guess @@ -24,21 +23,24 @@ build-aux/install-sh build-aux/ltmain.sh build-aux/missing build-aux/test-driver +build/ config.log config.status configure +fuzz/fuzz_format_parsers libtool m4/libtool.m4 m4/ltoptions.m4 m4/ltsugar.m4 m4/ltversion.m4 m4/lt~obsolete.m4 -tests/.deps/ -tests/dlsym_check man/pam_u2f.8 +man/pamu2fcfg.1 pamu2fcfg/cmdline.c pamu2fcfg/cmdline.h pamu2fcfg/pamu2fcfg -man/pamu2fcfg.1 +tests/.deps/ +tests/cfg +tests/dlsym_check +tests/expand tests/get_devices -fuzz/fuzz_format_parsers diff --git a/Makefile.am b/Makefile.am index 16302c7..1b2b0cb 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 @@ -26,6 +27,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) @@ -44,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/NEWS b/NEWS index 3d84ba3..21ff517 100644 --- a/NEWS +++ b/NEWS @@ -1,8 +1,21 @@ -Copyright (c) 2014-2023 Yubico AB - See COPYING +Copyright (c) 2014-2025 Yubico AB - See COPYING pam-u2f NEWS -- History of user-visible changes. -*- outline -*- -* Version 1.3.1 (unreleased) +* Version 1.3.3 (unreleased) + +* Version 1.3.2 (released 2025-01-16) +** Relax authfile permission check to a warning instead of an error to +prevent a breaking change locking existing users out of their systems. + +* Version 1.3.1 (released 2025-01-14) +** Fix incorrect usage of PAM_IGNORE (YSA-2025-01, CVE-2025-23013). +** Changed return value when nouserok is enabled and the user has no +credentials, PAM_IGNORE is used instead of PAM_SUCCESS. +** Hardened checks of authfile permissions. +** Hardened checks for nouserok. +** Improved debug messages. +** Improved documentation. * Version 1.3.0 (released 2023-03-14) ** Add sanity checking of UV options to pamu2fcfg. diff --git a/README b/README index 595041b..5a0f9a5 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] @@ -161,9 +162,9 @@ disable this functionality, like so: `authpending_file=`. Default value: /var/run/user/$UID/pam-u2f-authpending nouserok:: -Set to enable authentication attempts to succeed even if the user -trying to authenticate is not found inside `authfile` or if `authfile` -is missing/malformed. +Set to make authentication attempts not fail if the user trying to +authenticate is not found inside `authfile`, is found but has no +credentials, or if the `authfile` is missing. openasuser:: Setuid to the authenticating user when opening the authfile. Useful @@ -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,30 @@ 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 diff --git a/build-aux/ci/fuzz-linux-asan.sh b/build-aux/ci/fuzz-linux-asan.sh index df18748..c140a5b 100755 --- a/build-aux/ci/fuzz-linux-asan.sh +++ b/build-aux/ci/fuzz-linux-asan.sh @@ -4,10 +4,10 @@ set -euxo pipefail CORPUS_URL="https://storage.googleapis.com/yubico-pam-u2f/corpus.tgz" LIBCBOR_URL="https://github.com/pjk/libcbor" -LIBCBOR_TAG="v0.10.2" +LIBCBOR_TAG="v0.11.0" LIBCBOR_CFLAGS="-fsanitize=address,alignment,bounds" LIBFIDO2_URL="https://github.com/Yubico/libfido2" -LIBFIDO2_TAG="1.14.0" +LIBFIDO2_TAG="1.15.0" LIBFIDO2_CFLAGS="-fsanitize=address,alignment,bounds" COMMON_CFLAGS="-g2 -fno-omit-frame-pointer" diff --git a/cfg.c b/cfg.c new file mode 100644 index 0000000..00cf82d --- /dev/null +++ b/cfg.c @@ -0,0 +1,309 @@ +/* + * Copyright (C) 2025 Yubico AB - See COPYING + */ + +#include +#include +#include +#include +#include +#include +#include + +#include + +#include "cfg.h" +#include "debug.h" + +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=", strlen("max_devices=")) == 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=", strlen("userpresence=")) == 0) { + sscanf(arg, "userpresence=%d", &cfg->userpresence); + } else if (strncmp(arg, "userverification=", strlen("userverification=")) == + 0) { + sscanf(arg, "userverification=%d", &cfg->userverification); + } else if (strncmp(arg, "pinverification=", strlen("pinverification=")) == + 0) { + sscanf(arg, "pinverification=%d", &cfg->pinverification); + } else if (strncmp(arg, "authfile=", strlen("authfile=")) == 0) { + cfg->auth_file = arg + strlen("authfile="); + } else if (strcmp(arg, "sshformat") == 0) { + cfg->sshformat = 1; + } else if (strncmp(arg, "authpending_file=", strlen("authpending_file=")) == + 0) { + cfg->authpending_file = arg + strlen("authpending_file="); + } else if (strncmp(arg, "origin=", strlen("origin=")) == 0) { + cfg->origin = arg + strlen("origin="); + } else if (strncmp(arg, "appid=", strlen("appid=")) == 0) { + cfg->appid = arg + strlen("appid="); + } else if (strncmp(arg, "prompt=", strlen("prompt=")) == 0) { + cfg->prompt = arg + strlen("prompt="); + } else if (strncmp(arg, "cue_prompt=", strlen("cue_prompt=")) == 0) { + cfg->cue_prompt = arg + strlen("cue_prompt="); + } 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 -= (size_t) 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, r = -EINVAL; + struct stat st; + + if (*path != '/') + return r; + + fd = open(path, O_RDONLY | O_CLOEXEC | O_NOCTTY | O_NOFOLLOW, 0); + if (fd == -1) + return -errno; + + if (fstat(fd, &st) != 0) + goto fail; + +#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; + if (st.st_size < 0) + goto fail; + + *outfd = fd; + *outsize = (size_t) st.st_size; + return 0; + +fail: + close(fd); + return r; +} + +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 = -1, r; + size_t fsize = 0; + char *buffer = NULL; + + r = open_safely(&fd, &fsize, config_path ? config_path : CFG_DEFAULT_PATH); + + /* Only the default config file is allowed to be missing. */ + if (r == -ENOENT && config_path == NULL) + return PAM_SUCCESS; + + if (r != 0) + return PAM_SERVICE_ERR; + + 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], "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++) + cfg_load_arg(cfg, argv[i]); + +exit: + 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)"); + } + + if (r != PAM_SUCCESS) + cfg_free(cfg); + + return r; +} + +void cfg_free(cfg_t *cfg) { + debug_close(cfg->debug_file); + free(cfg->defaults_buffer); + cfg_reset(cfg); +} diff --git a/cfg.h b/cfg.h new file mode 100644 index 0000000..d414969 --- /dev/null +++ b/cfg.h @@ -0,0 +1,42 @@ +/* + * Copyright (C) 2014-2019 Yubico AB - See COPYING + */ + +#ifndef CFG_H +#define CFG_H + +#include + +#define CFG_DEFAULT_PATH (SCONFDIR "/pam_u2f.conf") +#define CFG_MAX_FILE_SIZE 4096 + +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; + char *defaults_buffer; +} 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/configure.ac b/configure.ac index bf6611f..6b7d634 100644 --- a/configure.ac +++ b/configure.ac @@ -1,6 +1,6 @@ -# Copyright (C) 2014-2022 Yubico AB +#Copyright(C) 2014 - 2022 Yubico AB AC_PREREQ([2.65]) -AC_INIT([pam_u2f], [1.3.1], [https://github.com/Yubico/pam-u2f/issues], +AC_INIT([pam_u2f], [1.3.3], [https://github.com/Yubico/pam-u2f/issues], [pam_u2f], [https://developers.yubico.com/pam-u2f/]) AC_CONFIG_AUX_DIR([build-aux]) @@ -18,6 +18,12 @@ AM_PROG_AR LT_INIT([disable-static]) AC_PROG_CC +AX_CHECK_COMPILE_FLAG( + [-std=c11 -pedantic], + [CFLAGS="${CFLAGS} -std=c11 -pedantic"], + [AC_MSG_ERROR([C compiled does not support C11])] +) + AC_ARG_ENABLE([man], [AS_HELP_STRING([--disable-man], [Disable man page generation])], [:], @@ -40,7 +46,7 @@ AC_CHECK_HEADERS([security/pam_appl.h], [], [AC_MSG_ERROR([[PAM header files not found, install libpam-dev.]])]) AC_CHECK_HEADERS([security/pam_modules.h security/pam_modutil.h security/openpam.h], [], [], [#include - #include ]) +#include ]) AC_CHECK_LIB([pam], [pam_start]) AC_CHECK_FUNCS([pam_modutil_drop_priv openpam_borrow_cred]) @@ -59,6 +65,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], [], []) @@ -82,7 +102,7 @@ AX_CHECK_COMPILE_FLAG([-Wall], [CWFLAGS="-Wall"], [], [$check_extra_flags]) AX_CHECK_COMPILE_FLAG([-Wextra], [CWFLAGS="$CWFLAGS -Wextra"], [], [$check_extra_flags]) AX_CHECK_COMPILE_FLAG([-Wconversion], [CWFLAGS="$CWFLAGS -Wconversion"], [], [$check_extra_flags]) # Because pam headers are doing sign-conversion, see PAM_MODUTIL_DEF_PRIVS in pam_modutil.h -AX_CHECK_COMPILE_FLAG([-Wconversion], [CWFLAGS="$CWFLAGS -Wno-sign-conversion"], [], [$check_extra_flags]) +AX_CHECK_COMPILE_FLAG([-Wsign-conversion], [CWFLAGS="$CWFLAGS -Wno-sign-conversion"], [], [$check_extra_flags]) AX_CHECK_COMPILE_FLAG([-Wpedantic], [CWFLAGS="$CWFLAGS -Wpedantic"], [], [$check_extra_flags]) AX_CHECK_COMPILE_FLAG([-Wformat=2], [CWFLAGS="$CWFLAGS -Wformat=2"], [], [$check_extra_flags]) AX_CHECK_COMPILE_FLAG([-Wstrict-prototypes], [CWFLAGS="$CWFLAGS -Wstrict-prototypes"], [], [$check_extra_flags]) @@ -144,6 +164,7 @@ AC_CONFIG_FILES([tests/credentials/new_-V.cred]) AC_CONFIG_FILES([tests/credentials/old_credential.cred]) AC_CONFIG_FILES([tests/credentials/ssh_credential.cred]) AC_CONFIG_FILES([tests/credentials/new_limited_count.cred]) +AC_CONFIG_FILES([tests/credentials/empty.cred]) AC_OUTPUT @@ -162,4 +183,5 @@ AC_MSG_NOTICE([Summary of build options: LIBCRYPTO CFLAGS: $LIBCRYPTO_CFLAGS LIBCRYPTO LIBS: $LIBCRYPTO_LIBS PAMDIR: $PAMDIR + SCONFDIR: $SCONFDIR ]) diff --git a/expand.c b/expand.c index ad538c3..a0a470b 100644 --- a/expand.c +++ b/expand.c @@ -54,7 +54,7 @@ char *expand_variables(const char *str, const char *user) { buf_write(&head, &size, value, strlen(value)) != 0) { goto fail; } - } else if (buf_write_byte(&head, &size, *str) != 0) { + } else if (buf_write_byte(&head, &size, (uint8_t) *str) != 0) { goto fail; } } diff --git a/explicit_bzero.c b/explicit_bzero.c index 592e700..d681a27 100644 --- a/explicit_bzero.c +++ b/explicit_bzero.c @@ -5,6 +5,7 @@ * Written by Ted Unangst */ +#define __STDC_WANT_LIB_EXT1__ 1 #include /* 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 e6882e5..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); @@ -164,6 +193,8 @@ static void consume(const void *body, size_t len) { while (len--) x ^= *ptr++; + + (void) x; } static int conv_cb(int num_msg, const struct pam_message **msg, @@ -229,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)); @@ -254,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 8f7fd9f..98f392b 100644 --- a/fuzz/wrap.c +++ b/fuzz/wrap.c @@ -15,6 +15,7 @@ #include #include +#include "debug.h" #include "drop_privs.h" #include "fuzz/fuzz.h" @@ -34,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"; @@ -55,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)) @@ -64,8 +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(ssize_t, read, (int fd, void *buf, size_t count), -1, (fd, buf, count)) 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,7 +79,33 @@ 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 __wrap_asprintf(char **strp, const char *fmt, ...); +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) { + assert(fildes >= 0); + assert(buf != NULL); + return __real_read(fildes, buf, nbyte); +} + +extern int __wrap_asprintf(char **strp, const char *fmt, ...) + ATTRIBUTE_FORMAT(printf, 2, 3); extern int __wrap_asprintf(char **strp, const char *fmt, ...) { va_list ap; int r; @@ -100,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, diff --git a/man/Makefile.am b/man/Makefile.am index 888bb01..55aff6c 100644 --- a/man/Makefile.am +++ b/man/Makefile.am @@ -8,4 +8,7 @@ EXTRA_DIST = $(MANS:=.txt) SUFFIXES = .1.txt .1 .8.txt .8 .1.txt.1 .8.txt.8: - $(AM_V_GEN)$(A2X) --format=manpage -L -a revdate="Version $(VERSION)" $< + $(AM_V_GEN)$(A2X) --format=manpage -L \ + -a sconfdir="$(SCONFDIR)" \ + -a revdate="Version $(VERSION)" \ + $< diff --git a/man/pam_u2f.8.txt b/man/pam_u2f.8.txt index 4524d39..15c3924 100644 --- a/man/pam_u2f.8.txt +++ b/man/pam_u2f.8.txt @@ -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,27 @@ 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+. + 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* diff --git a/pam-u2f.c b/pam-u2f.c index e17470d..86faed0 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; @@ -176,7 +92,7 @@ int pam_sm_authenticate(pam_handle_t *pamh, int flags, int argc, cfg_t *cfg = &cfg_st; char buffer[BUFSIZE]; int pgu_ret, gpn_ret; - int retval = PAM_IGNORE; + int retval = PAM_ABORT; device_t *devices = NULL; unsigned n_devices = 0; int openasuser = 0; @@ -185,17 +101,19 @@ 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); + retval = cfg_init(cfg, flags, argc, argv); + if (retval != PAM_SUCCESS) + goto done; PAM_MODUTIL_DEF_PRIVS(privs); if (!cfg->origin) { if (!cfg->sshformat) { strcpy(buffer, DEFAULT_ORIGIN_PREFIX); - if (gethostname(buffer + strlen(DEFAULT_ORIGIN_PREFIX), BUFSIZE - strlen(DEFAULT_ORIGIN_PREFIX)) == -1) { debug_dbg(cfg, "Unable to get host name"); + retval = PAM_SYSTEM_ERR; goto done; } } else { @@ -205,6 +123,7 @@ int pam_sm_authenticate(pam_handle_t *pamh, int flags, int argc, cfg->origin = strdup(buffer); if (!cfg->origin) { debug_dbg(cfg, "Unable to allocate memory"); + retval = PAM_BUF_ERR; goto done; } else { should_free_origin = 1; @@ -217,6 +136,7 @@ int pam_sm_authenticate(pam_handle_t *pamh, int flags, int argc, cfg->appid = strdup(cfg->origin); if (!cfg->appid) { debug_dbg(cfg, "Unable to allocate memory"); + retval = PAM_BUF_ERR; goto done; } else { should_free_appid = 1; @@ -236,7 +156,7 @@ int pam_sm_authenticate(pam_handle_t *pamh, int flags, int argc, devices = calloc(cfg->max_devs, sizeof(device_t)); if (!devices) { debug_dbg(cfg, "Unable to allocate memory"); - retval = PAM_IGNORE; + retval = PAM_BUF_ERR; goto done; } @@ -254,7 +174,7 @@ int pam_sm_authenticate(pam_handle_t *pamh, int flags, int argc, pw->pw_dir[0] != '/') { debug_dbg(cfg, "Unable to retrieve credentials for user %s, (%s)", user, strerror(errno)); - retval = PAM_USER_UNKNOWN; + retval = PAM_SYSTEM_ERR; goto done; } @@ -265,7 +185,7 @@ int pam_sm_authenticate(pam_handle_t *pamh, int flags, int argc, if (cfg->expand && cfg->auth_file) { if ((cfg->auth_file = expand_variables(cfg->auth_file, user)) == NULL) { debug_dbg(cfg, "Failed to perform variable expansion"); - retval = PAM_AUTHINFO_UNAVAIL; + retval = PAM_BUF_ERR; goto done; } should_free_auth_file = 1; @@ -275,7 +195,7 @@ int pam_sm_authenticate(pam_handle_t *pamh, int flags, int argc, char *tmp = resolve_authfile_path(cfg, pw, &openasuser); if (tmp == NULL) { debug_dbg(cfg, "Could not resolve authfile path"); - retval = PAM_IGNORE; + retval = PAM_BUF_ERR; goto done; } if (should_free_auth_file) { @@ -294,7 +214,7 @@ int pam_sm_authenticate(pam_handle_t *pamh, int flags, int argc, debug_dbg(cfg, "Dropping privileges"); if (pam_modutil_drop_priv(pamh, &privs, pw)) { debug_dbg(cfg, "Unable to switch user to uid %i", pw->pw_uid); - retval = PAM_IGNORE; + retval = PAM_SYSTEM_ERR; goto done; } debug_dbg(cfg, "Switched to uid %i", pw->pw_uid); @@ -304,33 +224,14 @@ int pam_sm_authenticate(pam_handle_t *pamh, int flags, int argc, if (openasuser) { if (pam_modutil_regain_priv(pamh, &privs)) { debug_dbg(cfg, "could not restore privileges"); - retval = PAM_IGNORE; + retval = PAM_SYSTEM_ERR; goto done; } debug_dbg(cfg, "Restored privileges"); } - if (retval != 1) { - // for nouserok; make sure errors in get_devices_from_authfile don't - // result in valid devices - n_devices = 0; - } - - if (n_devices == 0) { - if (cfg->nouserok) { - debug_dbg(cfg, "Found no devices but nouserok specified. Skipping " - "authentication"); - retval = PAM_SUCCESS; - goto done; - } else if (retval != 1) { - debug_dbg(cfg, "Unable to get devices from authentication file"); - retval = PAM_AUTHINFO_UNAVAIL; - goto done; - } else { - debug_dbg(cfg, "Found no devices. Aborting."); - retval = PAM_AUTHINFO_UNAVAIL; - goto done; - } + if (retval != PAM_SUCCESS) { + goto done; } // Determine the full path for authpending_file in order to emit touch request @@ -388,14 +289,6 @@ int pam_sm_authenticate(pam_handle_t *pamh, int flags, int argc, } } - if (retval != 1) { - debug_dbg(cfg, "do_authentication returned %d", retval); - retval = PAM_AUTH_ERR; - goto done; - } - - retval = PAM_SUCCESS; - done: free_devices(devices, n_devices); @@ -425,9 +318,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/tests/Makefile.am b/tests/Makefile.am index 36b9532..55f6b1a 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -10,10 +10,17 @@ AM_LDFLAGS = -no-install check_PROGRAMS = dlsym_check dlsym_check_LDFLAGS = -ldl $(AM_LDFLAGS) +# XXX move openbsd-compat check_PROGRAMS += get_devices +get_devices_SOURCES = get_devices.c ../pamu2fcfg/strlcpy.c +get_devices_CPPFLAGS = -I$(srcdir)/../pamu2fcfg 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..d62c3b6 --- /dev/null +++ b/tests/cfg.c @@ -0,0 +1,397 @@ +/* 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_defaults); + 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_defaults); + 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(); +} diff --git a/tests/credentials/empty.cred.in b/tests/credentials/empty.cred.in new file mode 100644 index 0000000..08a398b --- /dev/null +++ b/tests/credentials/empty.cred.in @@ -0,0 +1 @@ +@USERNAME@: diff --git a/tests/get_devices.c b/tests/get_devices.c index 0296a98..b4b429e 100644 --- a/tests/get_devices.c +++ b/tests/get_devices.c @@ -13,6 +13,34 @@ #include #include "../util.h" +#include "openbsd-compat.h" + +static void test_nouserok(const char *username) { + device_t *dev; + unsigned ndevs; + cfg_t cfg; + int rc; + + memset(&cfg, 0, sizeof(cfg_t)); + cfg.auth_file = "credentials/this_file_does_not_exist.cred"; + cfg.debug = 1; + cfg.debug_file = stderr; + cfg.max_devs = 1; + cfg.nouserok = 1; + + dev = calloc(cfg.max_devs, sizeof(*dev)); + assert(dev != NULL); + + rc = get_devices_from_authfile(&cfg, username, dev, &ndevs); + assert(rc == PAM_IGNORE); + + cfg.auth_file = "credentials/empty.cred"; + rc = get_devices_from_authfile(&cfg, username, dev, &ndevs); + assert(rc == PAM_IGNORE); + + free_devices(dev, ndevs); +} + static void test_ssh_credential(const char *username) { device_t *dev; unsigned ndevs; @@ -30,7 +58,7 @@ static void test_ssh_credential(const char *username) { assert(dev != NULL); rc = get_devices_from_authfile(&cfg, username, dev, &ndevs); - assert(rc == 1); + assert(rc == PAM_SUCCESS); assert(ndevs == 1); assert(strcmp(dev[0].coseType, "es256") == 0); assert(strcmp(dev[0].attributes, "+presence") == 0); @@ -60,7 +88,7 @@ static void test_old_credential(const char *username) { dev = calloc(cfg.max_devs, sizeof(*dev)); rc = get_devices_from_authfile(&cfg, username, dev, &ndevs); - assert(rc == 1); + assert(rc == PAM_SUCCESS); assert(ndevs == 1); assert(strcmp(dev[0].coseType, "es256") == 0); assert(strcmp(dev[0].attributes, "+presence") == 0); @@ -93,7 +121,7 @@ static void test_limited_count(const char *username) { dev = calloc(cfg.max_devs, sizeof(*dev)); assert(dev != NULL); rc = get_devices_from_authfile(&cfg, username, dev, &ndevs); - assert(rc == 1); + assert(rc == PAM_SUCCESS); assert(ndevs == 1); assert(strcmp(dev[0].coseType, "eddsa") == 0); assert(strcmp(dev[0].keyHandle, @@ -111,7 +139,7 @@ static void test_limited_count(const char *username) { dev = calloc(cfg.max_devs, sizeof(*dev)); assert(dev != NULL); rc = get_devices_from_authfile(&cfg, username, dev, &ndevs); - assert(rc == 1); + assert(rc == PAM_SUCCESS); assert(ndevs == 2); assert(strcmp(dev[0].coseType, "eddsa") == 0); assert(strcmp(dev[0].keyHandle, @@ -152,7 +180,7 @@ static void test_new_credentials(const char *username) { assert(dev != NULL); cfg.auth_file = "credentials/new_.cred"; rc = get_devices_from_authfile(&cfg, username, dev, &n_devs); - assert(rc == 1); + assert(rc == PAM_SUCCESS); assert(n_devs == 1); assert(strcmp(dev[0].coseType, "es256") == 0); assert(strcmp(dev[0].keyHandle, "vlcWFQFik8gJySuxMTlRwSDvnq9u/mlMXRIqv4rd7Kq2CJj1V9Uh9PqbTF8UkY3EcQfHeS0G3nY0ibyxXE0pdw==") == 0); @@ -165,7 +193,7 @@ static void test_new_credentials(const char *username) { assert(dev != NULL); cfg.auth_file = "credentials/new_-V.cred"; rc = get_devices_from_authfile(&cfg, username, dev, &n_devs); - assert(rc == 1); + assert(rc == PAM_SUCCESS); assert(n_devs == 1); assert(strcmp(dev[0].coseType, "es256") == 0); assert(strcmp(dev[0].keyHandle, "qf/qcQqFloToNoUMnp2cWg8pUPKoJ0CJFyP0wqpbpOgcD+hzEOJEBaHFbnnYP9d/zLKuwTsQ1nRpSc/aDJTEeQ==") == 0); @@ -178,7 +206,7 @@ static void test_new_credentials(const char *username) { assert(dev != NULL); cfg.auth_file = "credentials/new_-N.cred"; rc = get_devices_from_authfile(&cfg, username, dev, &n_devs); - assert(rc == 1); + assert(rc == PAM_SUCCESS); assert(n_devs == 1); assert(strcmp(dev[0].coseType, "es256") == 0); assert(strcmp(dev[0].keyHandle, "IPbgFVDLguVOr5GzdV7C5MH4Ec+bWfG2hifOy0IWWvNsHUZyN5x0rqbAoGWQPgxbAuQTKfk/n+3U9h4AWf8QXg==") == 0); @@ -191,7 +219,7 @@ static void test_new_credentials(const char *username) { assert(dev != NULL); cfg.auth_file = "credentials/new_-V-N.cred"; rc = get_devices_from_authfile(&cfg, username, dev, &n_devs); - assert(rc == 1); + assert(rc == PAM_SUCCESS); assert(n_devs == 1); assert(strcmp(dev[0].coseType, "es256") == 0); assert(strcmp(dev[0].keyHandle, "HftI6IHewEFB4OhBMeT9WjnG097GYvpE4dTxSS33JTRzRP6V/oBPyj3vurnTRJwif98V8YhceMAH8lDePA1dxQ==") == 0); @@ -204,7 +232,7 @@ static void test_new_credentials(const char *username) { assert(dev != NULL); cfg.auth_file = "credentials/new_-P.cred"; rc = get_devices_from_authfile(&cfg, username, dev, &n_devs); - assert(rc == 1); + assert(rc == PAM_SUCCESS); assert(n_devs == 1); assert(strcmp(dev[0].coseType, "es256") == 0); assert(strcmp(dev[0].keyHandle, "yvFPHZBdPoBcdhF86mImwNQm2DUgfPw0s26QCpm4XQO0is4qlx3nIdyVP9WHszpJ5uFV/1mjd09L3P6ton1fAw==") == 0); @@ -217,7 +245,7 @@ static void test_new_credentials(const char *username) { assert(dev != NULL); cfg.auth_file = "credentials/new_-P-V.cred"; rc = get_devices_from_authfile(&cfg, username, dev, &n_devs); - assert(rc == 1); + assert(rc == PAM_SUCCESS); assert(n_devs == 1); assert(strcmp(dev[0].coseType, "es256") == 0); assert(strcmp(dev[0].keyHandle, "WSSDFwB8Bv4wg5pOLzYNRsqyJYi6/rbuxL6nzuvPOkpSslyNX/8lcZSsPfBmuWkRE1CNh7xvalAlBUz1/LUcbg==") == 0); @@ -230,7 +258,7 @@ static void test_new_credentials(const char *username) { assert(dev != NULL); cfg.auth_file = "credentials/new_-P-N.cred"; rc = get_devices_from_authfile(&cfg, username, dev, &n_devs); - assert(rc == 1); + assert(rc == PAM_SUCCESS); assert(n_devs == 1); assert(strcmp(dev[0].coseType, "es256") == 0); assert(strcmp(dev[0].keyHandle, "+/l9LJ6dwbnDLff0PqkDhMEOWsruM+aYP+bzQdaCq3QmTGnh0dbcblfLaYs86XgcirS9OEoEkohB5pd8mhwSMQ==") == 0); @@ -243,7 +271,7 @@ static void test_new_credentials(const char *username) { assert(dev != NULL); cfg.auth_file = "credentials/new_-P-V-N.cred"; rc = get_devices_from_authfile(&cfg, username, dev, &n_devs); - assert(rc == 1); + assert(rc == PAM_SUCCESS); assert(n_devs == 1); assert(strcmp(dev[0].coseType, "es256") == 0); assert(strcmp(dev[0].keyHandle, "vw9z9n3ndQkTKPY3+LDy1Fd2otIsV5LgcYE+dR0buViSZnKcLJ1kav46mQ47jtelw82/6q3Z2/VKQ44F763tVg==") == 0); @@ -256,7 +284,7 @@ static void test_new_credentials(const char *username) { assert(dev != NULL); cfg.auth_file = "credentials/new_-r.cred"; rc = get_devices_from_authfile(&cfg, username, dev, &n_devs); - assert(rc == 1); + assert(rc == PAM_SUCCESS); assert(n_devs == 1); assert(strcmp(dev[0].coseType, "es256") == 0); assert(strcmp(dev[0].keyHandle, "*") == 0); @@ -269,7 +297,7 @@ static void test_new_credentials(const char *username) { assert(dev != NULL); cfg.auth_file = "credentials/new_-r-V.cred"; rc = get_devices_from_authfile(&cfg, username, dev, &n_devs); - assert(rc == 1); + assert(rc == PAM_SUCCESS); assert(n_devs == 1); assert(strcmp(dev[0].coseType, "es256") == 0); assert(strcmp(dev[0].keyHandle, "*") == 0); @@ -282,7 +310,7 @@ static void test_new_credentials(const char *username) { assert(dev != NULL); cfg.auth_file = "credentials/new_-r-N.cred"; rc = get_devices_from_authfile(&cfg, username, dev, &n_devs); - assert(rc == 1); + assert(rc == PAM_SUCCESS); assert(n_devs == 1); assert(strcmp(dev[0].coseType, "es256") == 0); assert(strcmp(dev[0].keyHandle, "*") == 0); @@ -295,7 +323,7 @@ static void test_new_credentials(const char *username) { assert(dev != NULL); cfg.auth_file = "credentials/new_-r-V-N.cred"; rc = get_devices_from_authfile(&cfg, username, dev, &n_devs); - assert(rc == 1); + assert(rc == PAM_SUCCESS); assert(n_devs == 1); assert(strcmp(dev[0].coseType, "es256") == 0); assert(strcmp(dev[0].keyHandle, "*") == 0); @@ -308,7 +336,7 @@ static void test_new_credentials(const char *username) { assert(dev != NULL); cfg.auth_file = "credentials/new_-r-P.cred"; rc = get_devices_from_authfile(&cfg, username, dev, &n_devs); - assert(rc == 1); + assert(rc == PAM_SUCCESS); assert(n_devs == 1); assert(strcmp(dev[0].coseType, "es256") == 0); assert(strcmp(dev[0].keyHandle, "*") == 0); @@ -321,7 +349,7 @@ static void test_new_credentials(const char *username) { assert(dev != NULL); cfg.auth_file = "credentials/new_-r-P-V.cred"; rc = get_devices_from_authfile(&cfg, username, dev, &n_devs); - assert(rc == 1); + assert(rc == PAM_SUCCESS); assert(n_devs == 1); assert(strcmp(dev[0].coseType, "es256") == 0); assert(strcmp(dev[0].keyHandle, "*") == 0); @@ -334,7 +362,7 @@ static void test_new_credentials(const char *username) { assert(dev != NULL); cfg.auth_file = "credentials/new_-r-P-N.cred"; rc = get_devices_from_authfile(&cfg, username, dev, &n_devs); - assert(rc == 1); + assert(rc == PAM_SUCCESS); assert(n_devs == 1); assert(strcmp(dev[0].coseType, "es256") == 0); assert(strcmp(dev[0].keyHandle, "*") == 0); @@ -347,7 +375,7 @@ static void test_new_credentials(const char *username) { assert(dev != NULL); cfg.auth_file = "credentials/new_-r-P-V-N.cred"; rc = get_devices_from_authfile(&cfg, username, dev, &n_devs); - assert(rc == 1); + assert(rc == PAM_SUCCESS); assert(n_devs == 1); assert(strcmp(dev[0].coseType, "es256") == 0); assert(strcmp(dev[0].keyHandle, "*") == 0); @@ -360,7 +388,7 @@ static void test_new_credentials(const char *username) { assert(dev != NULL); cfg.auth_file = "credentials/new_double_.cred"; rc = get_devices_from_authfile(&cfg, username, dev, &n_devs); - assert(rc == 1); + assert(rc == PAM_SUCCESS); assert(n_devs == 2); assert(strcmp(dev[0].coseType, "es256") == 0); assert(strcmp(dev[0].keyHandle, "THwoppI4JkuHWwQsSvsH6E987xAokX4MjB8Vh/lVghzW3iBtMglBw1epdwjbVEpKMVNqwYq6h71p3sQqnaTgLQ==") == 0); @@ -378,7 +406,7 @@ static void test_new_credentials(const char *username) { assert(dev != NULL); cfg.auth_file = "credentials/new_double_-V.cred"; rc = get_devices_from_authfile(&cfg, username, dev, &n_devs); - assert(rc == 1); + assert(rc == PAM_SUCCESS); assert(n_devs == 2); assert(strcmp(dev[0].coseType, "es256") == 0); assert(strcmp(dev[0].keyHandle, "oBQ1hIWiYfhJ8g6DFWawe0xOAlKtcPiBDKyoS8ydd/zwXbIEU+fHfnzjh46gLjV67+rt1ycCTTMj+P/7EsLNhg==") == 0); @@ -396,7 +424,7 @@ static void test_new_credentials(const char *username) { assert(dev != NULL); cfg.auth_file = "credentials/new_double_-N.cred"; rc = get_devices_from_authfile(&cfg, username, dev, &n_devs); - assert(rc == 1); + assert(rc == PAM_SUCCESS); assert(n_devs == 2); assert(strcmp(dev[0].coseType, "es256") == 0); assert(strcmp(dev[0].keyHandle, "WWJqEWaCASU+nsp2bTFh4LbJVOnf1ZRgNxmDcBuThynSTxDgO1GxGcTYg0Ilo/RF4YXvVCur7gfALYZA69lDTg==") == 0); @@ -414,7 +442,7 @@ static void test_new_credentials(const char *username) { assert(dev != NULL); cfg.auth_file = "credentials/new_double_-V-N.cred"; rc = get_devices_from_authfile(&cfg, username, dev, &n_devs); - assert(rc == 1); + assert(rc == PAM_SUCCESS); assert(n_devs == 2); assert(strcmp(dev[0].coseType, "es256") == 0); assert(strcmp(dev[0].keyHandle, "5sVKkhoc+afHBtAp7csIg/Sq4RFi1arnr/Qi9quwpNZ4gPhlI6FFBP4CmH8HLw/n5xt8iQxUD83aue23WbrDVA==") == 0); @@ -432,7 +460,7 @@ static void test_new_credentials(const char *username) { assert(dev != NULL); cfg.auth_file = "credentials/new_double_-P.cred"; rc = get_devices_from_authfile(&cfg, username, dev, &n_devs); - assert(rc == 1); + assert(rc == PAM_SUCCESS); assert(n_devs == 2); assert(strcmp(dev[0].coseType, "es256") == 0); assert(strcmp(dev[0].keyHandle, "ACoC1fhEYhdOstzkaCb1PqcU4T6xMrXxe5GEQjPDsheOxJzWGXTpaA3abmHZ3khcJ8Off/ecyPq2kMMqh3l7Xg==") == 0); @@ -450,7 +478,7 @@ static void test_new_credentials(const char *username) { assert(dev != NULL); cfg.auth_file = "credentials/new_double_-P-V.cred"; rc = get_devices_from_authfile(&cfg, username, dev, &n_devs); - assert(rc == 1); + assert(rc == PAM_SUCCESS); assert(n_devs == 2); assert(strcmp(dev[0].coseType, "es256") == 0); assert(strcmp(dev[0].keyHandle, "7jPjHZzm/Ec6oKy6gpq+XXI3P435OLJFO4o3iGH8KUQlEw+1Zv0FmUtguJ2HIZifRsIyMILdu2rwCDgcqmuj9Q==") == 0); @@ -468,7 +496,7 @@ static void test_new_credentials(const char *username) { assert(dev != NULL); cfg.auth_file = "credentials/new_double_-P-N.cred"; rc = get_devices_from_authfile(&cfg, username, dev, &n_devs); - assert(rc == 1); + assert(rc == PAM_SUCCESS); assert(n_devs == 2); assert(strcmp(dev[0].coseType, "es256") == 0); assert(strcmp(dev[0].keyHandle, "USgDNJZ9Z8GXzQgWdrkFJ5S+WsqKhdg9zHmoMifow3xBd8Rn0ZH2udPuRs6Q8Y/13BOCL9lEhdxc+1JAoP0j8w==") == 0); @@ -486,7 +514,7 @@ static void test_new_credentials(const char *username) { assert(dev != NULL); cfg.auth_file = "credentials/new_double_-P-V-N.cred"; rc = get_devices_from_authfile(&cfg, username, dev, &n_devs); - assert(rc == 1); + assert(rc == PAM_SUCCESS); assert(n_devs == 2); assert(strcmp(dev[0].coseType, "es256") == 0); assert(strcmp(dev[0].keyHandle, "Ypw0/A5KEPshXH0zO72Qlgt1uHvB4VnVRBpObzVGDeS8LxR9smealISARIOo3rlOLgjqj6dkJxqu1LoLm22UpA==") == 0); @@ -504,7 +532,7 @@ static void test_new_credentials(const char *username) { assert(dev != NULL); cfg.auth_file = "credentials/new_double_-r.cred"; rc = get_devices_from_authfile(&cfg, username, dev, &n_devs); - assert(rc == 1); + assert(rc == PAM_SUCCESS); assert(n_devs == 2); assert(strcmp(dev[0].coseType, "es256") == 0); assert(strcmp(dev[0].keyHandle, "*") == 0); @@ -522,7 +550,7 @@ static void test_new_credentials(const char *username) { assert(dev != NULL); cfg.auth_file = "credentials/new_double_-r-V.cred"; rc = get_devices_from_authfile(&cfg, username, dev, &n_devs); - assert(rc == 1); + assert(rc == PAM_SUCCESS); assert(n_devs == 2); assert(strcmp(dev[0].coseType, "es256") == 0); assert(strcmp(dev[0].keyHandle, "*") == 0); @@ -540,7 +568,7 @@ static void test_new_credentials(const char *username) { assert(dev != NULL); cfg.auth_file = "credentials/new_double_-r-N.cred"; rc = get_devices_from_authfile(&cfg, username, dev, &n_devs); - assert(rc == 1); + assert(rc == PAM_SUCCESS); assert(n_devs == 2); assert(strcmp(dev[0].coseType, "es256") == 0); assert(strcmp(dev[0].keyHandle, "*") == 0); @@ -558,7 +586,7 @@ static void test_new_credentials(const char *username) { assert(dev != NULL); cfg.auth_file = "credentials/new_double_-r-V-N.cred"; rc = get_devices_from_authfile(&cfg, username, dev, &n_devs); - assert(rc == 1); + assert(rc == PAM_SUCCESS); assert(n_devs == 2); assert(strcmp(dev[0].coseType, "es256") == 0); assert(strcmp(dev[0].keyHandle, "*") == 0); @@ -576,7 +604,7 @@ static void test_new_credentials(const char *username) { assert(dev != NULL); cfg.auth_file = "credentials/new_double_-r-P.cred"; rc = get_devices_from_authfile(&cfg, username, dev, &n_devs); - assert(rc == 1); + assert(rc == PAM_SUCCESS); assert(n_devs == 2); assert(strcmp(dev[0].coseType, "es256") == 0); assert(strcmp(dev[0].keyHandle, "*") == 0); @@ -594,7 +622,7 @@ static void test_new_credentials(const char *username) { assert(dev != NULL); cfg.auth_file = "credentials/new_double_-r-P-V.cred"; rc = get_devices_from_authfile(&cfg, username, dev, &n_devs); - assert(rc == 1); + assert(rc == PAM_SUCCESS); assert(n_devs == 2); assert(strcmp(dev[0].coseType, "es256") == 0); assert(strcmp(dev[0].keyHandle, "*") == 0); @@ -612,7 +640,7 @@ static void test_new_credentials(const char *username) { assert(dev != NULL); cfg.auth_file = "credentials/new_double_-r-P-N.cred"; rc = get_devices_from_authfile(&cfg, username, dev, &n_devs); - assert(rc == 1); + assert(rc == PAM_SUCCESS); assert(n_devs == 2); assert(strcmp(dev[0].coseType, "es256") == 0); assert(strcmp(dev[0].keyHandle, "*") == 0); @@ -630,7 +658,7 @@ static void test_new_credentials(const char *username) { assert(dev != NULL); cfg.auth_file = "credentials/new_double_-r-P-V-N.cred"; rc = get_devices_from_authfile(&cfg, username, dev, &n_devs); - assert(rc == 1); + assert(rc == PAM_SUCCESS); assert(n_devs == 2); assert(strcmp(dev[0].coseType, "es256") == 0); assert(strcmp(dev[0].keyHandle, "*") == 0); @@ -648,7 +676,7 @@ static void test_new_credentials(const char *username) { assert(dev != NULL); cfg.auth_file = "credentials/new_mixed_12.cred"; rc = get_devices_from_authfile(&cfg, username, dev, &n_devs); - assert(rc == 1); + assert(rc == PAM_SUCCESS); assert(n_devs == 2); assert(strcmp(dev[0].coseType, "es256") == 0); assert(strcmp(dev[0].keyHandle, "ooq2bCWeHFXzWqKwWFRliREQjOtUWKtWJbr7KwSh3FLNiCFgBuie4tqq3Pee86o7ew32u1+ITLsCBEYPrTQMAg==") == 0); @@ -666,7 +694,7 @@ static void test_new_credentials(const char *username) { assert(dev != NULL); cfg.auth_file = "credentials/new_mixed_1-P2.cred"; rc = get_devices_from_authfile(&cfg, username, dev, &n_devs); - assert(rc == 1); + assert(rc == PAM_SUCCESS); assert(n_devs == 2); assert(strcmp(dev[0].coseType, "es256") == 0); assert(strcmp(dev[0].keyHandle, "9HY72OR/kQECy5PbwfJwSaWZFlLL1CHamlm1LMZFozCBj6hzq4V9BpkkkMObxNL9gFd8yOXKDflFiVVoGq7sWQ==") == 0); @@ -684,7 +712,7 @@ static void test_new_credentials(const char *username) { assert(dev != NULL); cfg.auth_file = "credentials/new_mixed_-P12.cred"; rc = get_devices_from_authfile(&cfg, username, dev, &n_devs); - assert(rc == 1); + assert(rc == PAM_SUCCESS); assert(n_devs == 2); assert(strcmp(dev[0].coseType, "es256") == 0); assert(strcmp(dev[0].keyHandle, "kNfZ8Uot7TcImjCXhji32Apur3172TYc4XLA0uDQsdW1lrIRecyZP5chyPrkNxIrRIZ58UgiMxD72fiaCiQghw==") == 0); @@ -702,7 +730,7 @@ static void test_new_credentials(const char *username) { assert(dev != NULL); cfg.auth_file = "credentials/new_mixed_-P1-P2.cred"; rc = get_devices_from_authfile(&cfg, username, dev, &n_devs); - assert(rc == 1); + assert(rc == PAM_SUCCESS); assert(n_devs == 2); assert(strcmp(dev[0].coseType, "es256") == 0); assert(strcmp(dev[0].keyHandle, "gqCuXGhiA9P4PhXPgrMjQCdgBPkLHHmQcDF/AMOp9vMuCoreRgwWlckMvCdHnsRTohdGqKZgVT/M3HVu4/UiXA==") == 0); @@ -721,12 +749,13 @@ static void test_new_credentials(const char *username) { } int main(void) { - struct passwd *pwd; + const struct passwd *pwd; char *username; assert((pwd = getpwuid(geteuid())) != NULL); assert((username = strdup(pwd->pw_name)) != NULL); + test_nouserok(username); test_ssh_credential(username); test_old_credential(username); test_limited_count(username); diff --git a/tests/regenerate_credentials.py b/tests/regenerate_credentials.py index 47e083a..bc5eabc 100755 --- a/tests/regenerate_credentials.py +++ b/tests/regenerate_credentials.py @@ -6,8 +6,9 @@ import re import subprocess import sys +import os -PUC = "../pamu2fcfg/pamu2fcfg" +PUC = os.getenv("PAMU2FCFG", "../pamu2fcfg/pamu2fcfg") resident = ["", "-r"] @@ -46,7 +47,7 @@ def print_test_case(filename, credentials): assert(dev != NULL); cfg.auth_file = "{authfile}"; rc = get_devices_from_authfile(&cfg, username, dev, &n_devs); - assert(rc == 1); + assert(rc == PAM_SUCCESS); assert(n_devs == {devices}); """ diff --git a/util.c b/util.c index 8dfe98c..0bf21d0 100644 --- a/util.c +++ b/util.c @@ -6,6 +6,7 @@ #include #include #include +#include #include #include @@ -214,8 +215,8 @@ static int parse_native_format(const cfg_t *cfg, const char *username, FILE *opwfile, device_t *devices, unsigned *n_devs) { - char *s_user, *s_credential; - char *buf = NULL; + const char *s_user; + char *buf = NULL, *s_credential; size_t bufsiz = 0; ssize_t len; unsigned i; @@ -680,7 +681,7 @@ static int parse_ssh_format(const cfg_t *cfg, FILE *opwfile, int get_devices_from_authfile(const cfg_t *cfg, const char *username, device_t *devices, unsigned *n_devs) { - int retval = 0; + int r = PAM_AUTHINFO_UNAVAIL; int fd = -1; struct stat st; struct passwd *pw = NULL, pw_s; @@ -695,6 +696,9 @@ int get_devices_from_authfile(const cfg_t *cfg, const char *username, fd = open(cfg->auth_file, O_RDONLY | O_CLOEXEC | O_NOCTTY); if (fd < 0) { + if (errno == ENOENT && cfg->nouserok) { + r = PAM_IGNORE; + } debug_dbg(cfg, "Cannot open authentication file: %s", strerror(errno)); goto err; } @@ -709,11 +713,31 @@ int get_devices_from_authfile(const cfg_t *cfg, const char *username, goto err; } - if (st.st_size == 0) { - debug_dbg(cfg, "Authentication file is empty"); + if ((st.st_mode & (S_IWGRP | S_IWOTH)) != 0) { + /* XXX: attempt to prevent two messages to syslog */ + if (cfg->debug_file) { + debug_dbg(cfg, + "Permissions %04o for '%s' are too open. Please change the " + "file mode bits to 0644 or more restrictive. This may become " + "an error in the future!", + (unsigned int) st.st_mode & 0777, cfg->auth_file); + } +#ifndef WITH_FUZZING + /* XXX: force a message to syslog, regardless of the debug level */ + syslog(LOG_AUTHPRIV | LOG_WARNING, + "warning(pam_u2f): Permissions %04o for '%s' are too open. Please " + "change the file mode bits to 0644 or more restrictive. This may " + "become an error in the future!", + (unsigned int) st.st_mode & 0777, cfg->auth_file); +#endif + } + + if (st.st_size < 0) { + debug_dbg(cfg, "Invalid stat size for %s: %jd", cfg->auth_file, + (intmax_t) st.st_size); goto err; } - opwfile_size = st.st_size; + opwfile_size = (size_t) st.st_size; gpu_ret = getpwuid_r(st.st_uid, &pw_s, buffer, sizeof(buffer), &pw); if (gpu_ret != 0 || pw == NULL) { @@ -742,26 +766,26 @@ int get_devices_from_authfile(const cfg_t *cfg, const char *username, } if (cfg->sshformat == 0) { - retval = parse_native_format(cfg, username, opwfile, devices, n_devs); + if (parse_native_format(cfg, username, opwfile, devices, n_devs) != 1) { + goto err; + } } else { - retval = parse_ssh_format(cfg, opwfile, opwfile_size, devices, n_devs); - } - - if (retval != 1) { - // NOTE(adma): error message is logged by the previous function - goto err; + if (parse_ssh_format(cfg, opwfile, opwfile_size, devices, n_devs) != 1) { + goto err; + } } debug_dbg(cfg, "Found %d device(s) for user %s", *n_devs, username); - - retval = 1; + r = PAM_SUCCESS; err: - if (retval != 1) { + if (r != PAM_SUCCESS) { for (i = 0; i < *n_devs; i++) { reset_device(&devices[i]); } *n_devs = 0; + } else if (*n_devs == 0) { + r = cfg->nouserok ? PAM_IGNORE : PAM_USER_UNKNOWN; } if (opwfile) @@ -770,7 +794,7 @@ int get_devices_from_authfile(const cfg_t *cfg, const char *username, if (fd != -1) close(fd); - return retval; + return r; } void free_devices(device_t *devices, const unsigned n_devs) { @@ -784,7 +808,6 @@ void free_devices(device_t *devices, const unsigned n_devs) { } free(devices); - devices = NULL; } static int get_authenticators(const cfg_t *cfg, const fido_dev_info_t *devlist, @@ -883,13 +906,14 @@ static void parse_opts(const cfg_t *cfg, const char *attr, struct opts *opts) { static int get_device_opts(fido_dev_t *dev, int *pin, int *uv) { fido_cbor_info_t *info = NULL; - char *const *ptr; const bool *val; - size_t len; *pin = *uv = -1; /* unsupported */ if (fido_dev_is_fido2(dev)) { + char *const *ptr; + size_t len; + if ((info = fido_cbor_info_new()) == NULL || fido_dev_get_cbor_info(dev, info) != FIDO_OK) { fido_cbor_info_free(&info); @@ -1141,7 +1165,7 @@ int do_authentication(const cfg_t *cfg, const device_t *devices, fido_dev_t **authlist = NULL; int cued = 0; int r; - int retval = -2; + int retval = PAM_AUTH_ERR; size_t ndevs = 0; size_t ndevs_prev = 0; unsigned i = 0; @@ -1185,8 +1209,6 @@ int do_authentication(const cfg_t *cfg, const device_t *devices, i = 0; while (i < n_devs) { - retval = -2; - debug_dbg(cfg, "Attempting authentication with device number %d", i + 1); init_opts(&opts); /* used during authenticator discovery */ @@ -1259,7 +1281,7 @@ int do_authentication(const cfg_t *cfg, const device_t *devices, syslog(LOG_AUTHPRIV | LOG_INFO, "Successful FIDO authentication with publicKey %s (idx %u)", devices[i].publicKey, i); - retval = 1; + retval = PAM_SUCCESS; goto out; } } @@ -1384,7 +1406,7 @@ int do_manual_authentication(const cfg_t *cfg, const device_t *devices, char *b64_challenge = NULL; char prompt[MAX_PROMPT_LEN]; char buf[MAX_PROMPT_LEN]; - int retval = -2; + int retval = PAM_AUTH_ERR; int n; int r; unsigned i = 0; @@ -1426,7 +1448,7 @@ int do_manual_authentication(const cfg_t *cfg, const device_t *devices, debug_dbg(cfg, "Challenge: %s", b64_challenge); - n = snprintf(prompt, sizeof(prompt), "Challenge #%d:", i + 1); + n = snprintf(prompt, sizeof(prompt), "Challenge #%u:", i + 1); if (n <= 0 || (size_t) n >= sizeof(prompt)) { debug_dbg(cfg, "Failed to print challenge prompt"); goto out; @@ -1451,10 +1473,8 @@ int do_manual_authentication(const cfg_t *cfg, const device_t *devices, "Please pass the challenge(s) above to fido2-assert, and " "paste the results in the prompt below."); - retval = -1; - for (i = 0; i < n_devs; ++i) { - n = snprintf(prompt, sizeof(prompt), "Response #%d: ", i + 1); + n = snprintf(prompt, sizeof(prompt), "Response #%u: ", i + 1); if (n <= 0 || (size_t) n >= sizeof(prompt)) { debug_dbg(cfg, "Failed to print response prompt"); goto out; @@ -1467,7 +1487,7 @@ int do_manual_authentication(const cfg_t *cfg, const device_t *devices, r = fido_assert_verify(assert[i], 0, pk[i].type, pk[i].ptr); if (r == FIDO_OK) { - retval = 1; + retval = PAM_SUCCESS; break; } } 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;