On 13.12.2013 22:01, Jed Laundry wrote:
The problem is that parse_usersfile() is called first to find the
matching usersfile entry, and uses parse_type() to find the type of OTP;
#HOTP doesn't match any known type and therefore skipped_users isn't
incremented. Later, once the correct username is found and the OTP token
verifies, update_usersfile2() then only uses the username and
skipped_users counter to find the line to replace (tokenising the type,
secret, pin etc), and because skipped_users wasn't incremented, writes
the update to the commented out line.
Trivial fix: call parse_type in both places:
- if (type == NULL)
- continue;
+
+ /* Read token type */
+ if (parse_type (type, &digits, &totpstepsize) != 0)
+ continue;
Though it would probably not be a bad idea to merge the loops in
parse_usersfile and update_usersfile2, since they do parse the same
data. (or do something else to make this kind of bug impossible)
Simon, how do you want to proceed? AFAICT, comments in the usersfile
aren't explicitly supported and one is supposed to maintain separation
between the usersfile, which controls authentication, and an
authorisation file/mechanism, but I imagine that because it Just Works
for usersfiles that don't contain duplicate usernames that there are a
few people using it in this way...
How does one maintain this separation with pam_oath? I can't see
anything about it in the documentation, so I think this is a valid
question. The types accepted by parse_type are not listed either.
As far as I can tell it also works with duplicate usernames (If all the
type fields are valid according to parse_type and this bug isn't hit)
