diff --git a/pam-u2f.c b/pam-u2f.c index e17470d..d154ae4 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,33 +306,14 @@ int pam_sm_authenticate(pam_handle_t *pamh, int flags, int argc, if (openasuser) { if (pam_modutil_regain_priv(pamh, &privs)) { debug_dbg(cfg, "could not restore privileges"); - retval = PAM_IGNORE; + retval = PAM_SYSTEM_ERR; goto done; } debug_dbg(cfg, "Restored privileges"); } - if (retval != 1) { - // for nouserok; make sure errors in get_devices_from_authfile don't - // result in valid devices - n_devices = 0; - } - - if (n_devices == 0) { - if (cfg->nouserok) { - debug_dbg(cfg, "Found no devices but nouserok specified. Skipping " - "authentication"); - retval = PAM_SUCCESS; - goto done; - } else if (retval != 1) { - debug_dbg(cfg, "Unable to get devices from authentication file"); - retval = PAM_AUTHINFO_UNAVAIL; - goto done; - } else { - debug_dbg(cfg, "Found no devices. Aborting."); - retval = PAM_AUTHINFO_UNAVAIL; - goto done; - } + if (retval != PAM_SUCCESS) { + goto done; } // Determine the full path for authpending_file in order to emit touch request @@ -388,14 +371,6 @@ int pam_sm_authenticate(pam_handle_t *pamh, int flags, int argc, } } - if (retval != 1) { - debug_dbg(cfg, "do_authentication returned %d", retval); - retval = PAM_AUTH_ERR; - goto done; - } - - retval = PAM_SUCCESS; - done: free_devices(devices, n_devices); diff --git a/util.c b/util.c index 10476b2..4ef3dcd 100644 --- a/util.c +++ b/util.c @@ -678,12 +678,9 @@ out: 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; - char buffer[BUFSIZE]; - int gpu_ret; FILE *opwfile = NULL; size_t opwfile_size; unsigned i; @@ -693,6 +690,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; } @@ -706,28 +706,10 @@ int get_devices_from_authfile(const cfg_t *cfg, const char *username, debug_dbg(cfg, "Authentication file is not a regular file"); 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); - if (gpu_ret != 0 || pw == NULL) { - debug_dbg(cfg, "Unable to retrieve credentials for uid %u, (%s)", st.st_uid, - strerror(errno)); - goto err; - } - - if (strcmp(pw->pw_name, username) != 0 && strcmp(pw->pw_name, "root") != 0) { - if (strcmp(username, "root") != 0) { - debug_dbg(cfg, - "The owner of the authentication file is neither %s nor root", - username); - } else { - debug_dbg(cfg, "The owner of the authentication file is not root"); - } + if (st.st_uid != 0 && st.st_uid != geteuid()) { + debug_dbg(cfg, "Authentication file has insecure ownership"); goto err; } @@ -740,26 +722,26 @@ int get_devices_from_authfile(const cfg_t *cfg, const char *username, } if (cfg->sshformat == 0) { - retval = parse_native_format(cfg, username, opwfile, devices, n_devs); + if (parse_native_format(cfg, username, opwfile, devices, n_devs) != 1) { + goto err; + } } else { - retval = parse_ssh_format(cfg, opwfile, opwfile_size, devices, n_devs); - } - - if (retval != 1) { - // NOTE(adma): error message is logged by the previous function - goto err; + if (parse_ssh_format(cfg, opwfile, opwfile_size, devices, n_devs) != 1) { + goto err; + } } debug_dbg(cfg, "Found %d device(s) for user %s", *n_devs, username); - - retval = 1; + r = PAM_SUCCESS; err: - if (retval != 1) { + if (r != PAM_SUCCESS) { for (i = 0; i < *n_devs; i++) { reset_device(&devices[i]); } *n_devs = 0; + } else if (*n_devs == 0) { + r = cfg->nouserok ? PAM_IGNORE : PAM_USER_UNKNOWN; } if (opwfile) @@ -768,7 +750,7 @@ err: if (fd != -1) close(fd); - return retval; + return r; } void free_devices(device_t *devices, const unsigned n_devs) { @@ -1139,7 +1121,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 +1165,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 +1234,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 +1359,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 +1426,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 +1440,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; } }