Hi,
Using strcmp(3) to check a password is just asking for a timing
attack.
I admit that setting up such an attack on a custom lock(1) key at,
say, a physical terminal would be cumbersome, so maybe this is just
paranoia.
However, passwords *do* get reused all the time, so I think it
makes sense to hash the key, even if it's a "one-off" key. That
way in the worst case a nefarious actor only has the hash.
CC'd tedu@ because I'm not sure if I'm using crypt_newhash(3)
correctly.
Ted: In other places people use _PASSWORD_LEN for the length
of the hash buffer. Clearly this works, but it feels off.
_PASSWORD_LEN is meant to be an upper bound on length of
the plaintext, not the hash output, right?
Is there a better way to size my buffer for use with
crypt_newhash(3)?
--
Scott Cheloha
Index: usr.bin/lock/lock.c
===================================================================
RCS file: /cvs/src/usr.bin/lock/lock.c,v
retrieving revision 1.34
diff -u -p -r1.34 lock.c
--- usr.bin/lock/lock.c 3 May 2017 09:51:39 -0000 1.34
+++ usr.bin/lock/lock.c 27 Jun 2017 02:20:33 -0000
@@ -76,6 +76,7 @@ int
main(int argc, char *argv[])
{
char hostname[HOST_NAME_MAX+1], s[BUFSIZ], s1[BUFSIZ], date[256];
+ char hash[_PASSWORD_LEN];
char *p, *style, *nstyle, *ttynam;
struct itimerval ntimer, otimer;
int ch, sectimeout, usemine, cnt, tries = 10, backoff = 3;
@@ -162,7 +163,9 @@ main(int argc, char *argv[])
warnx("\apasswords didn't match.");
exit(1);
}
+ (void)crypt_newhash(s, "bcrypt,a", hash, sizeof(hash));
explicit_bzero(s, sizeof(s));
+ explicit_bzero(s1, sizeof(s1));
}
/* set signal handlers */
@@ -210,9 +213,9 @@ main(int argc, char *argv[])
explicit_bzero(s, sizeof(s));
break;
}
- } else if (strcmp(s, s1) == 0) {
+ } else if (crypt_checkpass(s, hash) == 0) {
explicit_bzero(s, sizeof(s));
- explicit_bzero(s1, sizeof(s1));
+ explicit_bzero(hash, sizeof(hash));
break;
}
(void)putc('\a', stderr);
