Scott Cheloha wrote: > 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.
this seems quite reasonable to me. > > 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)? yes, but there is no better define for the output buffer. perhaps PASS_MAX, i'm not sure why everything settled on _PASSWORD_LEN. > > -- > 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); >