Hiltjo Posthuma wrote:
> I agree, it's better to check if crypt() returns NULL.
>
> Maybe something like this (untested):
>
> …
Heyho,
looks good to me. Do we also want to check the format of pws pre-locking? This
would give the benefit of being able to error out early if the network auth
mechanism is down when locking the screen, so the user can actually react and
try to fix his network issue. If we just apply the patch as you proposed, the
user has no chance of noticing the failure until he is coming back to his
slocked screen and not able to unlock it with his password. Just adding in the
proposed fix linked earlier with an additional check for `NULL != pws` should do
the trick in this case. We might also consider adding something like this for
the allowed character range in crypt():
char *cur;
for (cur = pws; *cur; cur++) {
if (!((*cur >= '.' && *cur <= '9') ||
(*cur >= 'A' && *cur <= 'Z') ||
(*cur >= 'a' && *cur <= 'z')))
die("slock: failed to get password hash for user");
}
This check however is rather annoying and I think we can rely on
getpwuid()->pw_passwd and getspnam()->sp_pwdp to return correct information or a
NULL or too short string on failure.
> should we change:
> running = !auth_userokay(getlogin(), NULL, "auth-xlock", passwd);
> to:
> running = !auth_userokay(getlogin(), NULL, "auth-slock", passwd);
I'm not a BSD user, but from this manpage[0] it seams we can use an arbitrary
string here. If this is the case then sure, just push the change.
--Markus
0:
http://man.openbsd.org/authenticate.3
Received on Wed Aug 24 2016 - 08:18:39 CEST