pam_access: handle the 'LOCAL' keyword more securely#343
pam_access: handle the 'LOCAL' keyword more securely#343jo-he wants to merge 1 commit intolinux-pam:masterfrom
Conversation
Previously, for remote connections (e.g. SSH) it slipped through to 'network_netmask_match', where it luckily was just rejected, because it does does not mach a netmask pattern or IP address.
|
Pull request #226 is the SUSE patch I was referring to. Without fixing the |
| } else if (item->from_remote_host == 0) { /* local: no PAM_RHOSTS */ | ||
| if (strcasecmp(tok, "LOCAL") == 0) | ||
| return (YES); | ||
| } else if (strcasecmp(tok, "LOCAL") == 0) { | ||
| return NO; |
There was a problem hiding this comment.
This can be rewritten in a more logical way:
} else if (strcasecmp(tok, "LOCAL") == 0) {
if (item->from_remote_host == 0)
return YES;
return NO;
} else if ....
There was a problem hiding this comment.
That is not the same logic. With your suggestion, local accesses would fall through, when LOCAL is not configured.
There was a problem hiding this comment.
You're right. But I still prefer doing it differently so that the comparison of tok with LOCAL is not done twice. My alternate PR: #345
There was a problem hiding this comment.
Okay, that clean-up makes it more readable. I just added some minor comments.
|
Closing as alternative PR was merged. But thank you for the heads up! |
2 participants
Previously, for remote connections (e.g. SSH) it slipped through to 'network_netmask_match', where it was rejected just luckily, because it does does not mach a netmask pattern or an IP address.
This can become a security issue, when Linux vendors extend 'network_netmask_match', e.g. with hostname resolution (like SUSE did in SLES 15). So better prefer the code to be locally correct.