login: Fix no-pam authorization regression #1174
Conversation
The list_match function handles EXCEPT entries through recursive calls. It calls itself with NULL, which was then passed to strtok so parsing continued at current position. Replacing strtok with strsep, this means that EXCEPT entries never match, because strsep(NULL, ...) always returns NULL, i.e. the code treats everything after EXCEPT as non-existing. Fix this by passing current list pointer to recursive call. Fixes: 90afe61 (2024-07-04; "lib/, src/: Use strsep(3) instead of strtok(3)") Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
|
I've noticed this while further investigating |
|
BTW, a minor issue I noticed in the commit message after merging. The syntax I'm using for git commit references is: So it would be either of these: The reason I use the commit date is that it makes it easy to know if a release is affected by a bug and needs cherrypicking just by looking at the commit date. The author date can differ significantly from the commit date. The author date I usually omit it, and only write it if it's important for some reason. I have a couple of git aliases that make it easy to print those references: [alias]
ref = show --no-patch --abbrev=12 --date=short \
--format=tformat:'%C(auto)%h%C(reset) %C(white)(%cd%x3B \"%C(reset)%C(auto)%s%C(reset)%C(white)\")%C(reset)'
ref2 = show --no-patch --abbrev=12 --date=short \
--format=tformat:'%C(auto)%h%C(reset) %C(white)(%ad, %cd%x3B \"%C(reset)%C(auto)%s%C(reset)%C(white)\")%C(reset)'Which produces: $ git ref 90afe61003
90afe61003ef (2024-12-05; "lib/, src/: Use strsep(3) instead of strtok(3)")
$ git ref2 90afe61003
90afe61003ef (2024-07-04, 2024-12-05; "lib/, src/: Use strsep(3) instead of strtok(3)")Also, in relation with https://lwn.net/Articles/1001526/, assuming a collision in the future where both a hash (prefix) and a subject collide (the subject may collide for trivial cosmetic fixes), the commit date makes it easy to search the commit in the log: commit dates are strictly increasing in a branch. The author date doesn't help searching. |
Makes sense.
Wonderful! Thank you for those aliases ;)
I'm not deeply preoccupied by this right now since I'm assuming git itself will create a way to avoid this problem. But maybe I'm too optimistic. |
A regression in
/etc/login.accessparsing could allow or deny more users than configured. It affects shadow 4.17.x.The regression takes place in
logintool if compiled without PAM support. Basically all distributions ship either with PAM support or use util-linux login, so this affects rather few systems, let alone it requires theEXCEPTkeyword in/etc/login.access. Affected right now:And these distributions could be affected depending on compile configuration, but have older unaffected versions:
The
list_matchfunction handlesEXCEPTentries in/etc/login.accessthrough recursive calls. It calls itself withNULL, which was then passed tostrtokso parsing continued at current position.Replacing
strtokwithstrsep, this means thatEXCEPTentries never match, becausestrsep(NULL, ...)always returnsNULL, i.e. the code treats everything afterEXCEPTas non-existing.Fix this by passing current list pointer to recursive call.
Proof of Concept:
--without-libpam, no need to install login into system/etc/login.accessfile (replacemyuserwith a user which is okay if access is denied):If you try to log in as denied user, you can actually log in. This happens because the first rule matches
myuserbecause EXCEPT handling is broken. With this patch,myuseris correctly blocked because the first rule does not match, but the second.Fixes: 90afe61 (2024-07-04; "lib/, src/: Use strsep(3) instead of strtok(3)")