Makes sense -- but perhaps justify the arc4random with a comment, explaining what is being done, so that people don't need to look in the commitlog?
Ted Unangst <t...@tedunangst.com> wrote: > In the event that a program uses invalid parameters, I think we should > overwrite the key with random data. Otherwise, there's a chance the program > will continue with a zero key. It may even appear to work, encrypting and > decrypting data, but with a weak key. Random data means it fails closed, and > should also make it easier to detect such errors since it no longer > interoperates. > > > Index: bcrypt_pbkdf.c > =================================================================== > RCS file: /home/cvs/src/lib/libutil/bcrypt_pbkdf.c,v > retrieving revision 1.13 > diff -u -p -r1.13 bcrypt_pbkdf.c > --- bcrypt_pbkdf.c 12 Jan 2015 03:20:04 -0000 1.13 > +++ bcrypt_pbkdf.c 15 Oct 2019 19:14:12 -0000 > @@ -110,10 +110,10 @@ bcrypt_pbkdf(const char *pass, size_t pa > > /* nothing crazy */ > if (rounds < 1) > - return -1; > + goto bad; > if (passlen == 0 || saltlen == 0 || keylen == 0 || > keylen > sizeof(out) * sizeof(out)) > - return -1; > + goto bad; > stride = (keylen + sizeof(out) - 1) / sizeof(out); > amt = (keylen + stride - 1) / stride; > > @@ -166,4 +166,8 @@ bcrypt_pbkdf(const char *pass, size_t pa > explicit_bzero(out, sizeof(out)); > > return 0; > + > +bad: > + arc4random_buf(key, keylen); > + return -1; > } > Index: pkcs5_pbkdf2.c > =================================================================== > RCS file: /home/cvs/src/lib/libutil/pkcs5_pbkdf2.c,v > retrieving revision 1.10 > diff -u -p -r1.10 pkcs5_pbkdf2.c > --- pkcs5_pbkdf2.c 18 Apr 2017 04:06:21 -0000 1.10 > +++ pkcs5_pbkdf2.c 15 Oct 2019 19:17:08 -0000 > @@ -84,11 +84,11 @@ pkcs5_pbkdf2(const char *pass, size_t pa > size_t r; > > if (rounds < 1 || key_len == 0) > - return -1; > + goto bad; > if (salt_len == 0 || salt_len > SIZE_MAX - 4) > - return -1; > + goto bad; > if ((asalt = malloc(salt_len + 4)) == NULL) > - return -1; > + goto bad; > > memcpy(asalt, salt, salt_len); > > @@ -118,4 +118,8 @@ pkcs5_pbkdf2(const char *pass, size_t pa > explicit_bzero(obuf, sizeof(obuf)); > > return 0; > + > +bad: > + arc4random_buf(key, key_len); > + return -1; > } >
