Comment 4 for bug 1695899

Revision history for this message
Tyler Hicks (tyhicks) wrote :

Hello! This is a very accelerated security review of python-scrypt. I
didn't look at the scrypt implementation itself but did have a quick
look at a few important areas of the project.

1) crypto_entropy_read() eventually calls entropy_read() which directly
   reads from /dev/urandom. New code that needs to fetch random data
   should be using the getrandom(2) syscall available in 3.17 and newer
   kernels. The main downside of entropy_read()'s implementation is that
   it can't detect if the urandom pool has not yet been initialized. It
   would be nice if the function were converted to use getrandom(2) when
   it is available.

2) It is great to see that tests/hashvectors.csv is inspired by the test
   vectors found in rfc7914:

    https://tools.ietf.org/html/rfc7914#section-12

   However, it only includes three of the four test vectors. It would be
   nice if hashvectors.csv could be updated to include the
   scrypt(P="pleaseletmein", S="SodiumChloride", N=1048576, r=8, p=1,
          dkLen=64) vector.

3) It is strongly recommended that BINDNOW hardening be enabled at build
   time.

Security team ack for pre-promotion but I'm requesting that you fix #2
and #3 ASAP (before 17.10 is released).