Currently we trust the LDAP server to only return the `yubi_attr`
attribute, yet we loop over all possible attributes when there should
only be one.
Since the bundled test LDAP server ignores the requested attributes list,
we must make sure to only match against the `yubi_attr` attibute as
opposed to "all of them".
This also fixes an issue where AUTH_NOT_FOUND was returned instead
of AUTH_NO_TOKENS when there were no values returned for `yubi_attr`
but another attribute's value was considered as a candidate token.
Currently, if a user has no associated tokens, we still prompt for an
OTP challenge and attempt to verify it.
This adds a check earlier to avoid the useless prompt in that case.
The `nullok` option is also added. It changes the return value from
PAM_USER_UNKNOWN to PAM_IGNORE. (fixes#97)
Finally, some constants have been turned to symbolic form for clarity
and debugging output is improved.
When using `try_first_pass` or `use_first_pass`, the password we inherit
from PAM might not actually be an OTP challenge.
Currently, we happily leak it to the validation server without first
checking if it matches an authorized token ID.
This postpones sending the actual request until we know the token ID is
authorized.
add a debug_file option for where to write debug info (default to stdout)
stop compiling with DEBUG_PAM and PAM_DEBUG
make debugging behave the same way on linux-pam and openpam
Some of the printf conversion specifications were wrong when used on
size_t, causing
> pam_yubico.c:957:57: warning: format specifies type 'int' but the argument has type 'size_t' (aka 'unsigned long') [-Wformat]
> DBG (("OTP too short to be considered : %i < %i", password_len, (cfg->token_id_length + TOKEN_OTP_LEN)));
> ~~ ^~~~~~~~~~~~
> %zu
> pam_yubico.c:132:36: note: expanded from macro 'DBG'
> #define DBG(x) if (cfg->debug) { D(x); }
> ^
> ./util.h:47:12: note: expanded from macro 'D'
> printf x; \
> ^
and
> pam_yubico.c:967:14: warning: format specifies type 'int' but the argument has type 'size_t' (aka 'unsigned long') [-Wformat]
> skip_bytes, password_len, cfg->token_id_length, TOKEN_OTP_LEN));
> ^~~~~~~~~~~~
> pam_yubico.c:132:36: note: expanded from macro 'DBG'
> #define DBG(x) if (cfg->debug) { D(x); }
> ^
> ./util.h:47:12: note: expanded from macro 'D'
> printf x; \
> ^
Fix these by using the appropriate %zu conversions for size_t. While
looking through the code, there are a couple more places where format
string specifiers could be improved, e.g. using %zu instead of casting
the result of sizeof(x) or strlen(x) to unsigned long.
In addition, convert TOKEN_OTP_LEN, MAX_TOKEN_ID_LEN and
DEFAULT_TOKEN_ID_LEN to unsigned numbers, because negative values would
not make any sense for those.
On OS X and FreeBSD, struct pam_message does not declare its msg member
as constant. This causes a warning when assigning a constant string to
it:
pam_yubico.c:403:14: warning: assigning to 'char *' from 'const char *' discards qualifiers [-Wincompatible-pointer-types-discards-qualifiers]
msg[0].msg = message;
^ ~~~~~~~
Sending the users password out in another direction is scary and
requires more thought. Ideally this should be solved by using ldap
GSSAPI or similar instead of sending a cleartext password over the
network.