From 575fb08cd8dcb7471037f5ad6b711c153acc88f1 Mon Sep 17 00:00:00 2001 From: Ludvig Michaelsson Date: Mon, 25 Nov 2024 10:59:37 +0100 Subject: [PATCH 01/40] ci: replace macos-11 with macos-14 --- .github/workflows/macos_builds.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/macos_builds.yml b/.github/workflows/macos_builds.yml index 04faf3c..9b0a2dc 100644 --- a/.github/workflows/macos_builds.yml +++ b/.github/workflows/macos_builds.yml @@ -8,7 +8,7 @@ jobs: strategy: fail-fast: false matrix: - os: [ macos-11, macos-12, macos-13 ] + os: [ macos-12, macos-13, macos-14 ] cc: [ clang ] steps: - uses: actions/checkout@v4 From 6e0ca93d08d282fcefd830991479304420a9291a Mon Sep 17 00:00:00 2001 From: Ludvig Michaelsson Date: Mon, 25 Nov 2024 11:04:01 +0100 Subject: [PATCH 02/40] ci: update to 24.04 runners --- .github/workflows/alpine_builds.yml | 2 +- .github/workflows/format.yml | 2 +- .github/workflows/linux_builds.yml | 2 ++ .github/workflows/linux_fuzz.yml | 4 ++-- 4 files changed, 6 insertions(+), 4 deletions(-) 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/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 From 1fa10483f92af6d5008b305cde6c2757487d3492 Mon Sep 17 00:00:00 2001 From: Ludvig Michaelsson Date: Mon, 25 Nov 2024 11:06:27 +0100 Subject: [PATCH 03/40] ci: fixup macos workflow - Attempting to install `pkg-config` or `pkgconf` results in brew complaining about conflicts. Rely on whatever version already present. - macos-14 seemingly needs an explicit libtool install. --- .github/workflows/macos_builds.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/macos_builds.yml b/.github/workflows/macos_builds.yml index 9b0a2dc..15af99d 100644 --- a/.github/workflows/macos_builds.yml +++ b/.github/workflows/macos_builds.yml @@ -13,7 +13,7 @@ jobs: 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 }} From 8fcb1bf30f6daa8e14798661da109f1e4cc2bcde Mon Sep 17 00:00:00 2001 From: Ludvig Michaelsson Date: Mon, 25 Nov 2024 11:13:46 +0100 Subject: [PATCH 04/40] ci: bump libfido2, libcbor --- build-aux/ci/fuzz-linux-asan.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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" From e86db10cc2f2b4f9098a5d7e7d00ca9c1a8c4752 Mon Sep 17 00:00:00 2001 From: Ludvig Michaelsson Date: Mon, 2 Dec 2024 17:18:10 +0100 Subject: [PATCH 05/40] ci: update CodeQL CodeQL action v2 will be deprecated on 2024-12-05. --- .github/workflows/codeql-analysis.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 From a4073bf699853c80eb27c9150e1c5869a3644d65 Mon Sep 17 00:00:00 2001 From: Ludvig Michaelsson Date: Mon, 16 Dec 2024 11:41:10 +0100 Subject: [PATCH 06/40] ci: rotate out macos-12, bring in macos-15 --- .github/workflows/macos_builds.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/macos_builds.yml b/.github/workflows/macos_builds.yml index 15af99d..84d8cbc 100644 --- a/.github/workflows/macos_builds.yml +++ b/.github/workflows/macos_builds.yml @@ -8,7 +8,7 @@ jobs: strategy: fail-fast: false matrix: - os: [ macos-12, macos-13, macos-14 ] + os: [ macos-13, macos-14, macos-15 ] cc: [ clang ] steps: - uses: actions/checkout@v4 From 4ae31ac984f28074f936294c337adcc2d3c824a5 Mon Sep 17 00:00:00 2001 From: Giovanni Simoni Date: Mon, 16 Dec 2024 11:49:21 +0100 Subject: [PATCH 07/40] fuzz: defend against read(fd, NULL, size) and read(-1, buf, size) Under Linux the read call seems to accept NULL as a parameter: $ cat try.c #include #include int main(void) { int i = read(0, NULL, 4); err(1, "read"); return 0; } $ make try cc try.c -o try $ ./try = 0); + assert(buf != NULL); + return __real_read(fildes, buf, nbyte); +} + extern int __wrap_asprintf(char **strp, const char *fmt, ...); extern int __wrap_asprintf(char **strp, const char *fmt, ...) { va_list ap; From ba1fb88c42206acc5f341dd733737a2166ba14c1 Mon Sep 17 00:00:00 2001 From: Giovanni Simoni Date: Mon, 16 Dec 2024 23:56:20 +0100 Subject: [PATCH 08/40] fuzz: Fix compiler warnings --- fuzz/fuzz_auth.c | 2 ++ fuzz/wrap.c | 4 +++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/fuzz/fuzz_auth.c b/fuzz/fuzz_auth.c index e6882e5..3c76119 100644 --- a/fuzz/fuzz_auth.c +++ b/fuzz/fuzz_auth.c @@ -164,6 +164,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, diff --git a/fuzz/wrap.c b/fuzz/wrap.c index a4613a9..ee1f2ce 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" @@ -82,7 +83,8 @@ extern ssize_t __wrap_read(int fildes, void *buf, size_t nbyte) { return __real_read(fildes, buf, nbyte); } -extern int __wrap_asprintf(char **strp, const char *fmt, ...); +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; From a96ef17f74b8e4ed80a97322120af1a228a1ffb7 Mon Sep 17 00:00:00 2001 From: Ludvig Michaelsson Date: Wed, 20 Nov 2024 13:36:31 +0100 Subject: [PATCH 09/40] pam: do not return PAM_IGNORE on system errors Instead, use more meaningful status codes: - PAM_SYSTEM_ERR if getpwuid_r(), gethostname(), or pam_modutil_{drop,regain}_priv() fails; - PAM_BUF_ERR if memory allocation routines fails; and - PAM_ABORT for any uncaught errors. This commit is part of a fix for YSA-2025-01 / CVE-2025-23013. --- pam-u2f.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/pam-u2f.c b/pam-u2f.c index e17470d..27d8c0a 100644 --- a/pam-u2f.c +++ b/pam-u2f.c @@ -176,7 +176,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; @@ -192,10 +192,10 @@ int pam_sm_authenticate(pam_handle_t *pamh, int flags, int argc, 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 +205,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 +218,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 +238,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 +256,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 +267,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 +277,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 +296,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,7 +306,7 @@ 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"); From 08199144d870a63275a4601dbc6751ac68d48301 Mon Sep 17 00:00:00 2001 From: Ludvig Michaelsson Date: Thu, 21 Nov 2024 09:40:52 +0100 Subject: [PATCH 10/40] pam: tighten down nouserok Move PAM return value handling to get_devices_from_authfile(): If `nouserok` is set, return - PAM_IGNORE if open() returns ENOENT; - PAM_IGNORE if user is not found in the authfile; - PAM_IGNORE if user is found in the but have no credentials; - PAM_AUTHINFO_UNAVAIL otherwise. If `nouserok` is *not* set, return - PAM_USER_UNKNOWN if user is not found in the authfile; - PAM_USER_UNKNOWN if user is found but have no credentials; - PAM_AUTHINFO_UNAVAIL otherwise. This commit is part of a fix for YSA-2025-01 / CVE-2025-23013. --- README | 6 +++--- pam-u2f.c | 23 ++--------------------- util.c | 30 +++++++++++++++--------------- 3 files changed, 20 insertions(+), 39 deletions(-) diff --git a/README b/README index 595041b..17c2f40 100644 --- a/README +++ b/README @@ -161,9 +161,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 diff --git a/pam-u2f.c b/pam-u2f.c index 27d8c0a..5dd9e10 100644 --- a/pam-u2f.c +++ b/pam-u2f.c @@ -312,27 +312,8 @@ int pam_sm_authenticate(pam_handle_t *pamh, int flags, int argc, 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 diff --git a/util.c b/util.c index 10476b2..9acf474 100644 --- a/util.c +++ b/util.c @@ -678,7 +678,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; @@ -693,6 +693,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; } @@ -707,10 +710,6 @@ 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"); - goto err; - } opwfile_size = st.st_size; gpu_ret = getpwuid_r(st.st_uid, &pw_s, buffer, sizeof(buffer), &pw); @@ -740,26 +739,27 @@ 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 (parse_ssh_format(cfg, opwfile, opwfile_size, devices, n_devs) != 1) { + goto err; + } } - if (retval != 1) { - // NOTE(adma): error message is logged by the previous function - 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) @@ -768,7 +768,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) { From 73e7bc16679eca077ca5ddc96bf2c090a17f60e5 Mon Sep 17 00:00:00 2001 From: Ludvig Michaelsson Date: Thu, 21 Nov 2024 09:46:45 +0100 Subject: [PATCH 11/40] pam: let do_*authentication() return PAM_* This simplifies reasoning about the return value in pam_sm_authenticate(). Function returns PAM_SUCCESS if successful, PAM_AUTH_ERR otherwise. --- pam-u2f.c | 5 +---- util.c | 13 ++++--------- 2 files changed, 5 insertions(+), 13 deletions(-) diff --git a/pam-u2f.c b/pam-u2f.c index 5dd9e10..91a0712 100644 --- a/pam-u2f.c +++ b/pam-u2f.c @@ -371,14 +371,11 @@ int pam_sm_authenticate(pam_handle_t *pamh, int flags, int argc, } } - if (retval != 1) { + if (retval != PAM_SUCCESS) { debug_dbg(cfg, "do_authentication returned %d", retval); - retval = PAM_AUTH_ERR; goto done; } - retval = PAM_SUCCESS; - done: free_devices(devices, n_devices); diff --git a/util.c b/util.c index 9acf474..9d4f9e6 100644 --- a/util.c +++ b/util.c @@ -748,7 +748,6 @@ int get_devices_from_authfile(const cfg_t *cfg, const char *username, } } - debug_dbg(cfg, "Found %d device(s) for user %s", *n_devs, username); r = PAM_SUCCESS; @@ -1139,7 +1138,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; @@ -1183,8 +1182,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 */ @@ -1254,7 +1251,7 @@ int do_authentication(const cfg_t *cfg, const device_t *devices, } r = fido_assert_verify(assert, 0, pk.type, pk.ptr); if (r == FIDO_OK) { - retval = 1; + retval = PAM_SUCCESS; goto out; } } @@ -1379,7 +1376,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; @@ -1446,8 +1443,6 @@ 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); if (n <= 0 || (size_t) n >= sizeof(prompt)) { @@ -1462,7 +1457,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; } } From cf68862af2dbe7730ed7c5fd8a02ac8aada9e7b5 Mon Sep 17 00:00:00 2001 From: Ludvig Michaelsson Date: Thu, 21 Nov 2024 13:53:14 +0100 Subject: [PATCH 12/40] tests: update return value --- tests/get_devices.c | 80 ++++++++++++++++----------------- tests/regenerate_credentials.py | 5 ++- 2 files changed, 43 insertions(+), 42 deletions(-) diff --git a/tests/get_devices.c b/tests/get_devices.c index 0296a98..3cc12bf 100644 --- a/tests/get_devices.c +++ b/tests/get_devices.c @@ -30,7 +30,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 +60,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 +93,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 +111,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 +152,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 +165,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 +178,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 +191,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 +204,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 +217,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 +230,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 +243,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 +256,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 +269,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 +282,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 +295,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 +308,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 +321,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 +334,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 +347,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 +360,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 +378,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 +396,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 +414,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 +432,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 +450,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 +468,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 +486,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 +504,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 +522,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 +540,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 +558,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 +576,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 +594,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 +612,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 +630,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 +648,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 +666,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 +684,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 +702,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); 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}); """ From 542e3766d7c06040634c14e66a8e1bfe6136569a Mon Sep 17 00:00:00 2001 From: Ludvig Michaelsson Date: Tue, 3 Dec 2024 11:16:17 +0100 Subject: [PATCH 13/40] tests: add nouserok test - test that ENOENT return PAM_IGNORE; and - test that missing user return PAM_IGNORE. --- configure.ac | 1 + tests/Makefile.am | 3 +++ tests/credentials/empty.cred.in | 1 + tests/get_devices.c | 29 +++++++++++++++++++++++++++++ 4 files changed, 34 insertions(+) create mode 100644 tests/credentials/empty.cred.in diff --git a/configure.ac b/configure.ac index bf6611f..a1ea9ca 100644 --- a/configure.ac +++ b/configure.ac @@ -144,6 +144,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 diff --git a/tests/Makefile.am b/tests/Makefile.am index 36b9532..7a8124a 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -10,7 +10,10 @@ 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 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 3cc12bf..f131d65 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; @@ -727,6 +755,7 @@ int main(void) { 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); From fceb6c7a1058047adef5b429aeb475112ee35e55 Mon Sep 17 00:00:00 2001 From: Ludvig Michaelsson Date: Thu, 21 Nov 2024 15:45:14 +0100 Subject: [PATCH 14/40] misc: drop unnecessary debug statement, goto --- pam-u2f.c | 5 ----- 1 file changed, 5 deletions(-) diff --git a/pam-u2f.c b/pam-u2f.c index 91a0712..8602218 100644 --- a/pam-u2f.c +++ b/pam-u2f.c @@ -371,11 +371,6 @@ int pam_sm_authenticate(pam_handle_t *pamh, int flags, int argc, } } - if (retval != PAM_SUCCESS) { - debug_dbg(cfg, "do_authentication returned %d", retval); - goto done; - } - done: free_devices(devices, n_devices); From 51cea61c89b750cad899eb2d34299d5d41d04090 Mon Sep 17 00:00:00 2001 From: Ludvig Michaelsson Date: Mon, 13 Jan 2025 12:32:25 +0100 Subject: [PATCH 15/40] util: check permissions of authfile --- util.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/util.c b/util.c index 9d4f9e6..29e91b1 100644 --- a/util.c +++ b/util.c @@ -710,6 +710,11 @@ int get_devices_from_authfile(const cfg_t *cfg, const char *username, goto err; } + if ((st.st_mode & (S_IWGRP | S_IWOTH)) != 0) { + debug_dbg(cfg, "Authentication file has insecure permissions"); + goto err; + } + opwfile_size = st.st_size; gpu_ret = getpwuid_r(st.st_uid, &pw_s, buffer, sizeof(buffer), &pw); From 94d884a5a1c42a10d2360af117c8904ea71bf316 Mon Sep 17 00:00:00 2001 From: Ludvig Michaelsson Date: Wed, 8 Jan 2025 11:24:05 +0100 Subject: [PATCH 16/40] news: prepare for 1.3.1 --- NEWS | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index 3d84ba3..83899b4 100644 --- a/NEWS +++ b/NEWS @@ -1,8 +1,15 @@ -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) +** 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. +** Herdened 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. From b4b955072b374b71b51d844a5d456ebc2380f1ee Mon Sep 17 00:00:00 2001 From: Ludvig Michaelsson Date: Tue, 14 Jan 2025 13:07:00 +0100 Subject: [PATCH 17/40] release 1.3.1 --- NEWS | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/NEWS b/NEWS index 83899b4..88bf8fc 100644 --- a/NEWS +++ b/NEWS @@ -2,11 +2,11 @@ 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.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. -** Herdened checks of authfile permissions. +** Hardened checks of authfile permissions. ** Hardened checks for nouserok. ** Improved debug messages. ** Improved documentation. From 79349bf082dc191be87740dfb4f6b428f15cc415 Mon Sep 17 00:00:00 2001 From: Ludvig Michaelsson Date: Tue, 14 Jan 2025 16:30:07 +0100 Subject: [PATCH 18/40] Bump version --- NEWS | 2 ++ configure.ac | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index 88bf8fc..cecc925 100644 --- a/NEWS +++ b/NEWS @@ -2,6 +2,8 @@ Copyright (c) 2014-2025 Yubico AB - See COPYING pam-u2f NEWS -- History of user-visible changes. -*- outline -*- +* Version 1.3.2 (unreleased) + * 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 diff --git a/configure.ac b/configure.ac index a1ea9ca..37c742c 100644 --- a/configure.ac +++ b/configure.ac @@ -1,6 +1,6 @@ # 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.2], [https://github.com/Yubico/pam-u2f/issues], [pam_u2f], [https://developers.yubico.com/pam-u2f/]) AC_CONFIG_AUX_DIR([build-aux]) From f573707012f92e31172a7b14b6e36f8e93a02478 Mon Sep 17 00:00:00 2001 From: Ludvig Michaelsson Date: Wed, 15 Jan 2025 16:57:18 +0100 Subject: [PATCH 19/40] util: soften authfile permission check to a warning We'd like to make this a hard error but it has proven to break existing installations. To avoid breaking changes, revert to trying our hardest to inform the administrator that this user is authenticating with a potentially unsafe authfile. --- util.c | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/util.c b/util.c index 29e91b1..25dbedd 100644 --- a/util.c +++ b/util.c @@ -6,6 +6,7 @@ #include #include #include +#include #include #include @@ -711,8 +712,22 @@ int get_devices_from_authfile(const cfg_t *cfg, const char *username, } if ((st.st_mode & (S_IWGRP | S_IWOTH)) != 0) { - debug_dbg(cfg, "Authentication file has insecure permissions"); - goto err; + /* 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 } opwfile_size = st.st_size; From 5abd0d7019c6618d7dfdad9ec591a3cd58c9d349 Mon Sep 17 00:00:00 2001 From: Ludvig Michaelsson Date: Wed, 15 Jan 2025 17:03:37 +0100 Subject: [PATCH 20/40] NEWS: prepare for 1.3.2 --- NEWS | 2 ++ 1 file changed, 2 insertions(+) diff --git a/NEWS b/NEWS index cecc925..d415c54 100644 --- a/NEWS +++ b/NEWS @@ -3,6 +3,8 @@ Copyright (c) 2014-2025 Yubico AB - See COPYING pam-u2f NEWS -- History of user-visible changes. -*- outline -*- * Version 1.3.2 (unreleased) +** 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). From c81841d905362ab9c4be814789590b76447f03a1 Mon Sep 17 00:00:00 2001 From: Ludvig Michaelsson Date: Thu, 16 Jan 2025 13:14:14 +0100 Subject: [PATCH 21/40] release 1.3.2 --- NEWS | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/NEWS b/NEWS index d415c54..4f724c4 100644 --- a/NEWS +++ b/NEWS @@ -2,7 +2,7 @@ Copyright (c) 2014-2025 Yubico AB - See COPYING pam-u2f NEWS -- History of user-visible changes. -*- outline -*- -* Version 1.3.2 (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. From d21267de395881426abf66a00bd7619211427367 Mon Sep 17 00:00:00 2001 From: Ludvig Michaelsson Date: Thu, 16 Jan 2025 13:45:32 +0100 Subject: [PATCH 22/40] Bump version --- NEWS | 2 ++ configure.ac | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index 4f724c4..21ff517 100644 --- a/NEWS +++ b/NEWS @@ -2,6 +2,8 @@ Copyright (c) 2014-2025 Yubico AB - See COPYING pam-u2f NEWS -- History of user-visible changes. -*- outline -*- +* 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. diff --git a/configure.ac b/configure.ac index 37c742c..e80b0e0 100644 --- a/configure.ac +++ b/configure.ac @@ -1,6 +1,6 @@ # Copyright (C) 2014-2022 Yubico AB AC_PREREQ([2.65]) -AC_INIT([pam_u2f], [1.3.2], [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]) From 76b7fcd27d80e95a71f5578c7af6830babb74241 Mon Sep 17 00:00:00 2001 From: Giovanni Simoni Date: Thu, 16 Jan 2025 10:57:35 +0100 Subject: [PATCH 23/40] gitignore: sort alphabetically --- .gitignore | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/.gitignore b/.gitignore index 7c74d1a..f6a4c64 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,22 @@ 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/dlsym_check tests/get_devices -fuzz/fuzz_format_parsers From 689fe4af49d72f4a98df77e87ac42a3bc37bdc98 Mon Sep 17 00:00:00 2001 From: Giovanni Simoni Date: Thu, 16 Jan 2025 10:58:00 +0100 Subject: [PATCH 24/40] gitignore: add tests/expand --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index f6a4c64..6b64339 100644 --- a/.gitignore +++ b/.gitignore @@ -41,4 +41,5 @@ pamu2fcfg/cmdline.h pamu2fcfg/pamu2fcfg tests/.deps/ tests/dlsym_check +tests/expand tests/get_devices From 8f3923af1912de73c2af2a08d975368630781fe9 Mon Sep 17 00:00:00 2001 From: Giovanni Simoni Date: Thu, 16 Jan 2025 14:21:33 +0100 Subject: [PATCH 25/40] util: pointer can be const --- util.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/util.c b/util.c index 25dbedd..540e384 100644 --- a/util.c +++ b/util.c @@ -213,8 +213,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; From 7213e8aee8b66c9ad5b99cef5d4f0c570adbcddd Mon Sep 17 00:00:00 2001 From: Giovanni Simoni Date: Thu, 16 Jan 2025 14:21:33 +0100 Subject: [PATCH 26/40] util: remove assignment that has no effect --- util.c | 1 - 1 file changed, 1 deletion(-) diff --git a/util.c b/util.c index 540e384..4199e78 100644 --- a/util.c +++ b/util.c @@ -801,7 +801,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, From 83a1fd226ca8cba9e6543c7ea4edc4130216f1a8 Mon Sep 17 00:00:00 2001 From: Giovanni Simoni Date: Thu, 16 Jan 2025 14:21:33 +0100 Subject: [PATCH 27/40] util: reduce the scope of variable --- util.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/util.c b/util.c index 4199e78..96af172 100644 --- a/util.c +++ b/util.c @@ -899,13 +899,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); From e610df3ca7908014fcb9006a647494c71fb89fca Mon Sep 17 00:00:00 2001 From: Giovanni Simoni Date: Thu, 16 Jan 2025 14:21:33 +0100 Subject: [PATCH 28/40] util: use %u for unsigned integer --- util.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/util.c b/util.c index 96af172..97880e3 100644 --- a/util.c +++ b/util.c @@ -1438,7 +1438,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; @@ -1464,7 +1464,7 @@ int do_manual_authentication(const cfg_t *cfg, const device_t *devices, "paste the results in the prompt below."); 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; From ad11405fac3d5157c84a6c4bc4051e33735115cc Mon Sep 17 00:00:00 2001 From: Giovanni Simoni Date: Thu, 16 Jan 2025 23:42:57 +0100 Subject: [PATCH 29/40] test: use const pointer --- tests/get_devices.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/get_devices.c b/tests/get_devices.c index f131d65..b4b429e 100644 --- a/tests/get_devices.c +++ b/tests/get_devices.c @@ -749,7 +749,7 @@ 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); From abe14391fb7722bf00bc24f4e1f51037eef42fae Mon Sep 17 00:00:00 2001 From: Giovanni Simoni Date: Wed, 22 Jan 2025 11:26:14 +0100 Subject: [PATCH 30/40] Require -std=c11 --- configure.ac | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/configure.ac b/configure.ac index e80b0e0..f89106c 100644 --- a/configure.ac +++ b/configure.ac @@ -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])], [:], From 7dbddf0b1f9e2a0dfc5901fa0bf3af44c43f6e20 Mon Sep 17 00:00:00 2001 From: Giovanni Simoni Date: Thu, 23 Jan 2025 16:22:07 +0100 Subject: [PATCH 31/40] Enable __STDC_WANT_LIB_EXT1__ to get memset_s As we switch to C11, the availability of the memset_s function under Macos is detected by autoconf. The explicit_bzero.c module will use it to implement explicit_bzero(3) on that system. According to N1570, Annex K, in order for the string.h header to declare memset_s(), we need to define __STDC_WANT_LIB_EXT1__ as 1 before including the header file. The libc implementation is in turn supposed to define __STDC_LIB_EXT1__ in the header, to claim that the interfaces of Annex K are implemented as dictated by the standard. In theory, we should be using memset_s() only if __STDC_LIB_EXT1__ is defined, but we know that memset_s() exists thanks to autoconf. Since the Macos libc does not define __STDC_LIB_EXT1__, we can not be 100% sure that the memset_s() function is implemented in a standard way. --- explicit_bzero.c | 1 + 1 file changed, 1 insertion(+) 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 /* From a3706cdf4e224dd133246b2fbe318f7cb12fccb4 Mon Sep 17 00:00:00 2001 From: Giovanni Simoni Date: Tue, 28 Jan 2025 14:07:25 +0100 Subject: [PATCH 32/40] expand: fix sign-conversion warning --- expand.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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; } } From acd5fbd2c7d9306f971996bbc8ce3bae2423fe9d Mon Sep 17 00:00:00 2001 From: Giovanni Simoni Date: Tue, 28 Jan 2025 14:07:36 +0100 Subject: [PATCH 33/40] util: fix sign-conversion warning --- util.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/util.c b/util.c index 97880e3..f4a7791 100644 --- a/util.c +++ b/util.c @@ -730,7 +730,12 @@ int get_devices_from_authfile(const cfg_t *cfg, const char *username, #endif } - opwfile_size = st.st_size; + 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 = (size_t) st.st_size; gpu_ret = getpwuid_r(st.st_uid, &pw_s, buffer, sizeof(buffer), &pw); if (gpu_ret != 0 || pw == NULL) { From a12b982cb5ab0fb06e9ec2af4701213e3d1e03a4 Mon Sep 17 00:00:00 2001 From: Giovanni Simoni Date: Tue, 28 Jan 2025 14:52:11 +0100 Subject: [PATCH 34/40] configure.ac: fix -W{ => sign-}conversion --- configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index f89106c..6af19bf 100644 --- a/configure.ac +++ b/configure.ac @@ -88,7 +88,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]) From ee20e22368caee3933ea3d8ab4acf96ea746ddb2 Mon Sep 17 00:00:00 2001 From: Giovanni Simoni Date: Tue, 3 Dec 2024 21:47:10 +0100 Subject: [PATCH 35/40] 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 b54ffd4cf938de39a623c190fb52aa3227ec43c8 Mon Sep 17 00:00:00 2001 From: Giovanni Simoni Date: Mon, 16 Dec 2024 16:31:47 +0100 Subject: [PATCH 36/40] 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 | 310 +++++++++++++++++++++++++++++++++++++++++++-------- cfg.h | 4 + configure.ac | 19 +++- pam-u2f.c | 4 +- 5 files changed, 286 insertions(+), 52 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..00cf82d 100644 --- a/cfg.c +++ b/cfg.c @@ -1,65 +1,273 @@ +/* + * Copyright (C) 2025 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=", 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], "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++) + 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 +296,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..d414969 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 + 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 6af19bf..6b7d634 100644 --- a/configure.ac +++ b/configure.ac @@ -1,4 +1,4 @@ -# Copyright (C) 2014-2022 Yubico AB +#Copyright(C) 2014 - 2022 Yubico AB AC_PREREQ([2.65]) AC_INIT([pam_u2f], [1.3.3], [https://github.com/Yubico/pam-u2f/issues], [pam_u2f], [https://developers.yubico.com/pam-u2f/]) @@ -46,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]) @@ -65,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], [], []) @@ -169,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/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 3e6a2b8f568cc5bcd964893a4f23358b0563cc82 Mon Sep 17 00:00:00 2001 From: Giovanni Simoni Date: Thu, 5 Dec 2024 11:25:34 +0100 Subject: [PATCH 37/40] tests: Add unit tests for conf file --- .gitignore | 1 + tests/Makefile.am | 4 + tests/cfg.c | 397 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 402 insertions(+) create mode 100644 tests/cfg.c diff --git a/.gitignore b/.gitignore index 6b64339..85d22b9 100644 --- a/.gitignore +++ b/.gitignore @@ -40,6 +40,7 @@ pamu2fcfg/cmdline.c pamu2fcfg/cmdline.h pamu2fcfg/pamu2fcfg tests/.deps/ +tests/cfg tests/dlsym_check tests/expand tests/get_devices 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..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(); +} From e94e0750155bbe7c1d1908e81b851f95295627e4 Mon Sep 17 00:00:00 2001 From: Giovanni Simoni Date: Wed, 11 Dec 2024 09:01:20 +0100 Subject: [PATCH 38/40] 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 c7d280a88201d058e2e0b8927c15da4521409778 Mon Sep 17 00:00:00 2001 From: Giovanni Simoni Date: Wed, 18 Dec 2024 10:49:29 +0100 Subject: [PATCH 39/40] README: update with info about conf file --- README | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/README b/README index 17c2f40..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] @@ -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 From 3bd2d7de6841dc62bdeda1121c7506403770164e Mon Sep 17 00:00:00 2001 From: Giovanni Simoni Date: Wed, 18 Dec 2024 10:49:41 +0100 Subject: [PATCH 40/40] man: update with info about conf file The SCONFDIR variable is expanded by the build system within the pam_u2f.8 manpage. --- man/Makefile.am | 5 ++++- man/pam_u2f.8.txt | 27 +++++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) 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*