Validate reference scrypt parameters

FORMAT

@@ -5,8 +5,8 @@ offset	length
 0	6	"scrypt"
 6	1	scrypt data file version number (== 0)
 7	1	log2(N) (must be between 1 and 63 inclusive)
-8	4	r (big-endian integer; must satisfy r * p < 2^30)
-12	4	p (big-endian integer; must satisfy r * p < 2^30)
+8	4	r (big-endian integer; must satisfy 0 < r * p < 2^30)
+12	4	p (big-endian integer; must satisfy 0 < r * p < 2^30)
 16	32	salt
 48	16	first 16 bytes of SHA256(bytes 0 .. 47)
 64	32	HMAC-SHA256(bytes 0 .. 63)

Makefile.am

@@ -1,6 +1,7 @@
 bin_PROGRAMS=	scrypt
 noinst_PROGRAMS=							\
 	tests/valgrind/potential-memleaks				\
+	tests/verify-strings/test_scrypt_ref_params			\
 	tests/verify-strings/test_scrypt
 dist_man_MANS=$(scrypt_man_MANS)
 ACLOCAL_AMFLAGS=	-I m4
@@ -287,11 +288,30 @@ tests_verify_strings_test_scrypt_LDADD=					\
 	libscrypt_sse2.la						\
 	${LDADD_POSIX}
 
+# Binary to test parameter validation in the reference crypto_scrypt().
+tests_verify_strings_test_scrypt_ref_params_SOURCES=			\
+	tests/verify-strings/test_scrypt_ref_params.c			\
+	lib/crypto/crypto_scrypt-ref.c					\
+	libcperciva/alg/sha256.c					\
+	libcperciva/alg/sha256.h					\
+	libcperciva/util/insecure_memzero.c				\
+	libcperciva/util/insecure_memzero.h				\
+	libcperciva/util/warnp.c					\
+	libcperciva/util/warnp.h
+
+tests_verify_strings_test_scrypt_ref_params_LDADD=			\
+	libcperciva_arm_sha256.la					\
+	libcperciva_cpusupport_detect.la				\
+	libcperciva_shani.la						\
+	libscrypt_sse2.la						\
+	${LDADD_POSIX}
+
 # Eliminate false positives while memory-checking for the test framework.
 tests_valgrind_potential_memleaks_SOURCES= tests/valgrind/potential-memleaks.c
 
 .PHONY: test
 # we can't only build "scrypt tests/verify-strings/test_scrypt" because that
 # won't build the BUILT_SOURCES.
 test: all
+	$(top_builddir)/tests/verify-strings/test_scrypt_ref_params
 	$(top_srcdir)/tests/test_scrypt.sh .

README.md

@@ -91,8 +91,8 @@ https://en.wikipedia.org/wiki/Key_derivation_function) (KDF) with
  * scrypt_kdf(passwd, passwdlen, salt, saltlen, N, r, p, buf, buflen):
  * Compute scrypt(passwd[0 .. passwdlen - 1], salt[0 .. saltlen - 1], N, r,
  * p, buflen) and write the result into buf.  The parameters r, p, and buflen
- * must satisfy r * p < 2^30 and buflen <= (2^32 - 1) * 32.  The parameter N
- * must be a power of 2 greater than 1.
+ * must satisfy 0 < r * p < 2^30 and buflen <= (2^32 - 1) * 32.  The parameter
+ * N must be a power of 2 greater than 1.
  *
  * Return 0 on success; or -1 on error.
  */

lib/crypto/crypto_scrypt-ref.c

@@ -165,7 +165,8 @@ integerify(uint8_t * B, size_t r)
  * smix(B, r, N, V, XY):
  * Compute B = SMix_r(B, N).  The input B must be 128r bytes in length; the
  * temporary storage V must be 128rN bytes in length; the temporary storage
- * XY must be 256r bytes in length.  The value N must be a power of 2.
+ * XY must be 256r bytes in length.  The value N must be a power of 2 greater
+ * than 1.
  */
 static void
 smix(uint8_t * B, size_t r, uint64_t N, uint8_t * V, uint8_t * XY)
@@ -205,8 +206,8 @@ smix(uint8_t * B, size_t r, uint64_t N, uint8_t * V, uint8_t * XY)
  * crypto_scrypt(passwd, passwdlen, salt, saltlen, N, r, p, buf, buflen):
  * Compute scrypt(passwd[0 .. passwdlen - 1], salt[0 .. saltlen - 1], N, r,
  * p, buflen) and write the result into buf.  The parameters r, p, and buflen
- * must satisfy r * p < 2^30 and buflen <= (2^32 - 1) * 32.  The parameter N
- * must be a power of 2.
+ * must satisfy 0 < r * p < 2^30 and buflen <= (2^32 - 1) * 32.  The parameter
+ * N must be a power of 2 greater than 1.
  *
  * Return 0 on success; or -1 on error.
  */
@@ -222,6 +223,10 @@ crypto_scrypt(const uint8_t * passwd, size_t passwdlen,
 	uint32_t i;
 
 	/* Sanity-check parameters. */
+	if ((r == 0) || (p == 0)) {
+		errno = EINVAL;
+		goto err0;
+	}
 #if SIZE_MAX > UINT32_MAX
 	if (buflen > (((uint64_t)(1) << 32) - 1) * 32) {
 		errno = EFBIG;
@@ -232,7 +237,7 @@ crypto_scrypt(const uint8_t * passwd, size_t passwdlen,
 		errno = EFBIG;
 		goto err0;
 	}
-	if (((N & (N - 1)) != 0) || (N == 0)) {
+	if (((N & (N - 1)) != 0) || (N < 2)) {
 		errno = EINVAL;
 		goto err0;
 	}

libscrypt-kdf/scrypt-kdf.h

@@ -44,8 +44,8 @@ extern "C" {
  * scrypt_kdf(passwd, passwdlen, salt, saltlen, N, r, p, buf, buflen):
  * Compute scrypt(passwd[0 .. passwdlen - 1], salt[0 .. saltlen - 1], N, r,
  * p, buflen) and write the result into buf.  The parameters r, p, and buflen
- * must satisfy r * p < 2^30 and buflen <= (2^32 - 1) * 32.  The parameter N
- * must be a power of 2 greater than 1.
+ * must satisfy 0 < r * p < 2^30 and buflen <= (2^32 - 1) * 32.  The parameter
+ * N must be a power of 2 greater than 1.
  *
  * Return 0 on success; or -1 on error.
  */

tests/verify-strings/test_scrypt_ref_params.c

@@ -0,0 +1,40 @@
+#include <errno.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <string.h>
+
+#include "crypto_scrypt.h"
+
+static int
+check_invalid_params(uint64_t N, uint32_t r, uint32_t p)
+{
+	uint8_t kbuf[16];
+
+	errno = 0;
+	if (crypto_scrypt((const uint8_t *)"passwd", 6,
+	    (const uint8_t *)"salt", 4, N, r, p, kbuf, sizeof(kbuf)) != -1) {
+		printf("crypto_scrypt(%llu, %u, %u) succeeded\n",
+		    (unsigned long long)N, (unsigned int)r, (unsigned int)p);
+		return (1);
+	}
+	if (errno != EINVAL) {
+		printf("crypto_scrypt(%llu, %u, %u) failed with errno %d\n",
+		    (unsigned long long)N, (unsigned int)r, (unsigned int)p,
+		    errno);
+		return (1);
+	}
+
+	return (0);
+}
+
+int
+main(void)
+{
+	int failures = 0;
+
+	failures += check_invalid_params(16, 0, 1);
+	failures += check_invalid_params(16, 1, 0);
+	failures += check_invalid_params(1, 1, 1);
+
+	return (failures ? 1 : 0);
+}