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);
> 

Reply via email to