From 8560366cd8da3245f564a69e31dfec1ee4a3e161 Mon Sep 17 00:00:00 2001 From: Frank Denis Date: Sat, 7 Jun 2014 10:16:03 -0700 Subject: [PATCH 1/7] Disable the pwhash_scrypt_ll test that requires 1 Gb RAM. This is way too much for many devices and for VPS users. --- test/default/pwhash_scrypt_ll.c | 7 ------- test/default/pwhash_scrypt_ll.exp | 5 ----- 2 files changed, 12 deletions(-) diff --git a/test/default/pwhash_scrypt_ll.c b/test/default/pwhash_scrypt_ll.c index 4a40d7e9..db72e4e9 100644 --- a/test/default/pwhash_scrypt_ll.c +++ b/test/default/pwhash_scrypt_ll.c @@ -25,12 +25,6 @@ static uint64_t N3 = 16384U; static uint32_t r3 = 8U; static uint32_t p3 = 1U; -static const char *password4 = "pleaseletmein"; -static const char *salt4 = "SodiumChloride"; -static uint64_t N4 = 1048576U; -static uint32_t r4 = 8U; -static uint32_t p4 = 1U; - static void test_vector(const char *password, const char *salt, uint64_t N, uint32_t r, uint32_t p) { @@ -68,7 +62,6 @@ int main(void) test_vector(password1, salt1, N1, r1, p1); test_vector(password2, salt2, N2, r2, p2); test_vector(password3, salt3, N3, r3, p3); - test_vector(password4, salt4, N4, r4, p4); return 0; } diff --git a/test/default/pwhash_scrypt_ll.exp b/test/default/pwhash_scrypt_ll.exp index 44fe199e..9b7f6a74 100644 --- a/test/default/pwhash_scrypt_ll.exp +++ b/test/default/pwhash_scrypt_ll.exp @@ -13,8 +13,3 @@ scrypt('pleaseletmein', 'SodiumChloride', 16384, 8, 1, 64) = fd a8 fb ba 90 4f 8e 3e a9 b5 43 f6 54 5d a1 f2 d5 43 29 55 61 3f 0f cf 62 d4 97 05 24 2a 9a f9 e6 1e 85 dc 0d 65 1e 40 df cf 01 7b 45 57 58 87 -scrypt('pleaseletmein', 'SodiumChloride', 1048576, 8, 1, 64) = -21 01 cb 9b 6a 51 1a ae ad db be 09 cf 70 f8 81 -ec 56 8d 57 4a 2f fd 4d ab e5 ee 98 20 ad aa 47 -8e 56 fd 8f 4b a5 d0 9f fa 1c 6d 92 7c 40 f4 c3 -37 30 40 49 e8 a9 52 fb cb f4 5c 6f a7 7a 41 a4 From 06e089ef6a18ba1d461fe7dcfc82b72b8b16e43d Mon Sep 17 00:00:00 2001 From: Frank Denis Date: Mon, 9 Jun 2014 18:43:54 -0700 Subject: [PATCH 2/7] Make crypto_sign() test code more explicit. crypto_sign() doesn't just need the secret key. The public key has to follow. Which is why the test vectors are laid out in this order. But this can confuse static analysis, as well as people looking at the test in order to better understand how crypto_sign() works. So, just copy the sk and the pk into a dedicated buffer, for clarity. --- test/default/sign.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/test/default/sign.c b/test/default/sign.c index b711027b..bb5ca988 100644 --- a/test/default/sign.c +++ b/test/default/sign.c @@ -1054,6 +1054,8 @@ int main(void) { unsigned char sm[1024 + crypto_sign_BYTES]; unsigned char m[1024]; + unsigned char skpk[crypto_sign_SECRETKEYBYTES + + crypto_sign_PUBLICKEYBYTES]; unsigned char pk[crypto_sign_PUBLICKEYBYTES]; unsigned char sk[crypto_sign_SECRETKEYBYTES]; char pk_hex[crypto_sign_PUBLICKEYBYTES * 2 + 1]; @@ -1063,9 +1065,11 @@ int main(void) unsigned int i; for (i = 0U; i < (sizeof test_data) / (sizeof test_data[0]); i++) { + memcpy(skpk, test_data[i].sk, crypto_sign_SECRETKEYBYTES_WITHOUT_PK); + memcpy(skpk + crypto_sign_SECRETKEYBYTES_WITHOUT_PK, + test_data[i].pk, crypto_sign_PUBLICKEYBYTES); if (crypto_sign(sm, &smlen, - (const unsigned char *) test_data[i].m, i, - test_data[i].sk) != 0) { + (const unsigned char *) test_data[i].m, i, skpk) != 0) { printf("crypto_sign() failure: [%u]\n", i); continue; } From 1fcb676aed2e55156402218b54ee797beb6eb4ca Mon Sep 17 00:00:00 2001 From: Frank Denis Date: Mon, 9 Jun 2014 18:48:26 -0700 Subject: [PATCH 3/7] test/default/sign.c: make the test vectors static --- test/default/sign.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/default/sign.c b/test/default/sign.c index bb5ca988..b634171f 100644 --- a/test/default/sign.c +++ b/test/default/sign.c @@ -23,7 +23,7 @@ typedef struct TestData_ { const char *m; } TestData; -TestData test_data[] = { +static TestData test_data[] = { {{0x9d,0x61,0xb1,0x9d,0xef,0xfd,0x5a,0x60,0xba,0x84,0x4a,0xf4,0x92,0xec,0x2c,0xc4,0x44,0x49,0xc5,0x69,0x7b,0x32,0x69,0x19,0x70,0x3b,0xac,0x03,0x1c,0xae,0x7f,0x60,},{0xd7,0x5a,0x98,0x01,0x82,0xb1,0x0a,0xb7,0xd5,0x4b,0xfe,0xd3,0xc9,0x64,0x07,0x3a,0x0e,0xe1,0x72,0xf3,0xda,0xa6,0x23,0x25,0xaf,0x02,0x1a,0x68,0xf7,0x07,0x51,0x1a,},{0xe5,0x56,0x43,0x00,0xc3,0x60,0xac,0x72,0x90,0x86,0xe2,0xcc,0x80,0x6e,0x82,0x8a,0x84,0x87,0x7f,0x1e,0xb8,0xe5,0xd9,0x74,0xd8,0x73,0xe0,0x65,0x22,0x49,0x01,0x55,0x5f,0xb8,0x82,0x15,0x90,0xa3,0x3b,0xac,0xc6,0x1e,0x39,0x70,0x1c,0xf9,0xb4,0x6b,0xd2,0x5b,0xf5,0xf0,0x59,0x5b,0xbe,0x24,0x65,0x51,0x41,0x43,0x8e,0x7a,0x10,0x0b,},""}, {{0x4c,0xcd,0x08,0x9b,0x28,0xff,0x96,0xda,0x9d,0xb6,0xc3,0x46,0xec,0x11,0x4e,0x0f,0x5b,0x8a,0x31,0x9f,0x35,0xab,0xa6,0x24,0xda,0x8c,0xf6,0xed,0x4f,0xb8,0xa6,0xfb,},{0x3d,0x40,0x17,0xc3,0xe8,0x43,0x89,0x5a,0x92,0xb7,0x0a,0xa7,0x4d,0x1b,0x7e,0xbc,0x9c,0x98,0x2c,0xcf,0x2e,0xc4,0x96,0x8c,0xc0,0xcd,0x55,0xf1,0x2a,0xf4,0x66,0x0c,},{0x92,0xa0,0x09,0xa9,0xf0,0xd4,0xca,0xb8,0x72,0x0e,0x82,0x0b,0x5f,0x64,0x25,0x40,0xa2,0xb2,0x7b,0x54,0x16,0x50,0x3f,0x8f,0xb3,0x76,0x22,0x23,0xeb,0xdb,0x69,0xda,0x08,0x5a,0xc1,0xe4,0x3e,0x15,0x99,0x6e,0x45,0x8f,0x36,0x13,0xd0,0xf1,0x1d,0x8c,0x38,0x7b,0x2e,0xae,0xb4,0x30,0x2a,0xee,0xb0,0x0d,0x29,0x16,0x12,0xbb,0x0c,0x00,},"\x72"}, {{0xc5,0xaa,0x8d,0xf4,0x3f,0x9f,0x83,0x7b,0xed,0xb7,0x44,0x2f,0x31,0xdc,0xb7,0xb1,0x66,0xd3,0x85,0x35,0x07,0x6f,0x09,0x4b,0x85,0xce,0x3a,0x2e,0x0b,0x44,0x58,0xf7,},{0xfc,0x51,0xcd,0x8e,0x62,0x18,0xa1,0xa3,0x8d,0xa4,0x7e,0xd0,0x02,0x30,0xf0,0x58,0x08,0x16,0xed,0x13,0xba,0x33,0x03,0xac,0x5d,0xeb,0x91,0x15,0x48,0x90,0x80,0x25,},{0x62,0x91,0xd6,0x57,0xde,0xec,0x24,0x02,0x48,0x27,0xe6,0x9c,0x3a,0xbe,0x01,0xa3,0x0c,0xe5,0x48,0xa2,0x84,0x74,0x3a,0x44,0x5e,0x36,0x80,0xd7,0xdb,0x5a,0xc3,0xac,0x18,0xff,0x9b,0x53,0x8d,0x16,0xf2,0x90,0xae,0x67,0xf7,0x60,0x98,0x4d,0xc6,0x59,0x4a,0x7c,0x15,0xe9,0x71,0x6e,0xd2,0x8d,0xc0,0x27,0xbe,0xce,0xea,0x1e,0xc4,0x0a,},"\xaf\x82"}, From befd9c257d6538c126077f942bd27e7c9a89ff7d Mon Sep 17 00:00:00 2001 From: Frank Denis Date: Mon, 9 Jun 2014 19:03:47 -0700 Subject: [PATCH 4/7] Add annotations for Coverity FPs --- .../ref/sc25519_edwards25519sha512batch.c | 12 ++++++++---- .../randombytes/salsa20/randombytes_salsa20_random.c | 1 + .../randombytes/sysrandom/randombytes_sysrandom.c | 1 + 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/libsodium/crypto_sign/edwards25519sha512batch/ref/sc25519_edwards25519sha512batch.c b/src/libsodium/crypto_sign/edwards25519sha512batch/ref/sc25519_edwards25519sha512batch.c index ad232d78..085e3f92 100644 --- a/src/libsodium/crypto_sign/edwards25519sha512batch/ref/sc25519_edwards25519sha512batch.c +++ b/src/libsodium/crypto_sign/edwards25519sha512batch/ref/sc25519_edwards25519sha512batch.c @@ -46,10 +46,14 @@ static void barrett_reduce(sc25519 *r, const crypto_uint32 x[64]) q2[33] += carry; for(i=0;i<33;i++)r1[i] = x[i]; - for(i=0;i<32;i++) - for(j=0;j<33;j++) - if(i+j < 33) r2[i+j] += m[i]*q3[j]; - + for(i=0;i<32;i++) { + for(j=0;j<33;j++) { + if(i+j < 33) { + /* coverity[overrun-local] */ + r2[i+j] += m[i]*q3[j]; + } + } + } for(i=0;i<32;i++) { carry = r2[i] >> 8; diff --git a/src/libsodium/randombytes/salsa20/randombytes_salsa20_random.c b/src/libsodium/randombytes/salsa20/randombytes_salsa20_random.c index 6538c0d4..61f9e855 100644 --- a/src/libsodium/randombytes/salsa20/randombytes_salsa20_random.c +++ b/src/libsodium/randombytes/salsa20/randombytes_salsa20_random.c @@ -279,6 +279,7 @@ randombytes_salsa20_random_buf(void * const buf, const size_t size) randombytes_salsa20_random_stir_if_needed(); COMPILER_ASSERT(sizeof stream.nonce == crypto_stream_salsa20_NONCEBYTES); #ifdef ULONG_LONG_MAX + /* coverity[result_independent_of_operands] */ assert(size <= ULONG_LONG_MAX); #endif ret = crypto_stream_salsa20((unsigned char *) buf, (unsigned long long) size, diff --git a/src/libsodium/randombytes/sysrandom/randombytes_sysrandom.c b/src/libsodium/randombytes/sysrandom/randombytes_sysrandom.c index 3a5ec65e..f38c681f 100644 --- a/src/libsodium/randombytes/sysrandom/randombytes_sysrandom.c +++ b/src/libsodium/randombytes/sysrandom/randombytes_sysrandom.c @@ -202,6 +202,7 @@ randombytes_sysrandom_buf(void * const buf, const size_t size) { randombytes_sysrandom_stir_if_needed(); #ifdef ULONG_LONG_MAX + /* coverity[result_independent_of_operands] */ assert(size <= ULONG_LONG_MAX); #endif #ifndef _WIN32 From 3acdfa99a6922a1f4eed699f301131ae98820c2d Mon Sep 17 00:00:00 2001 From: Frank Denis Date: Mon, 9 Jun 2014 19:06:07 -0700 Subject: [PATCH 5/7] Remove useless access() before open() --- src/libsodium/randombytes/salsa20/randombytes_salsa20_random.c | 3 +-- src/libsodium/randombytes/sysrandom/randombytes_sysrandom.c | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/libsodium/randombytes/salsa20/randombytes_salsa20_random.c b/src/libsodium/randombytes/salsa20/randombytes_salsa20_random.c index 61f9e855..970ce750 100644 --- a/src/libsodium/randombytes/salsa20/randombytes_salsa20_random.c +++ b/src/libsodium/randombytes/salsa20/randombytes_salsa20_random.c @@ -119,8 +119,7 @@ randombytes_salsa20_random_random_dev_open(void) int fd; do { - if (access(*device, F_OK | R_OK) == 0 && - (fd = open(*device, O_RDONLY)) != -1) { + if ((fd = open(*device, O_RDONLY)) != -1) { if (fstat(fd, &st) == 0 && S_ISCHR(st.st_mode)) { return fd; } diff --git a/src/libsodium/randombytes/sysrandom/randombytes_sysrandom.c b/src/libsodium/randombytes/sysrandom/randombytes_sysrandom.c index f38c681f..efb7dbef 100644 --- a/src/libsodium/randombytes/sysrandom/randombytes_sysrandom.c +++ b/src/libsodium/randombytes/sysrandom/randombytes_sysrandom.c @@ -111,8 +111,7 @@ randombytes_sysrandom_random_dev_open(void) int fd; do { - if (access(*device, F_OK | R_OK) == 0 && - (fd = open(*device, O_RDONLY)) != -1) { + if ((fd = open(*device, O_RDONLY)) != -1) { if (fstat(fd, &st) == 0 && S_ISCHR(st.st_mode)) { return fd; } From 9b3da167fda244a5f4b2cb5f9420d6e85fed4629 Mon Sep 17 00:00:00 2001 From: Frank Denis Date: Mon, 9 Jun 2014 19:17:14 -0700 Subject: [PATCH 6/7] Comment dead code generated by qhasm --- src/libsodium/crypto_sign/ed25519/ref10/pow22523.h | 4 ++-- src/libsodium/crypto_sign/ed25519/ref10/pow225521.h | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libsodium/crypto_sign/ed25519/ref10/pow22523.h b/src/libsodium/crypto_sign/ed25519/ref10/pow22523.h index 60ffe0d3..9bd45f13 100644 --- a/src/libsodium/crypto_sign/ed25519/ref10/pow22523.h +++ b/src/libsodium/crypto_sign/ed25519/ref10/pow22523.h @@ -50,7 +50,7 @@ /* qhasm: z2 = z1^2^1 */ /* asm 1: fe_sq(>z2=fe#1,z2=fe#1,>z2=fe#1); */ /* asm 2: fe_sq(>z2=t0,z2=t0,>z2=t0); */ -fe_sq(t0,z); for (i = 1;i < 1;++i) fe_sq(t0,t0); +fe_sq(t0,z); /* for (i = 1;i < 1;++i) fe_sq(t0,t0); */ /* qhasm: z8 = z2^2^2 */ /* asm 1: fe_sq(>z8=fe#2,z8=fe#2,>z8=fe#2); */ @@ -70,7 +70,7 @@ fe_mul(t0,t0,t1); /* qhasm: z22 = z11^2^1 */ /* asm 1: fe_sq(>z22=fe#1,z22=fe#1,>z22=fe#1); */ /* asm 2: fe_sq(>z22=t0,z22=t0,>z22=t0); */ -fe_sq(t0,t0); for (i = 1;i < 1;++i) fe_sq(t0,t0); +fe_sq(t0,t0); /* for (i = 1;i < 1;++i) fe_sq(t0,t0); */ /* qhasm: z_5_0 = z9*z22 */ /* asm 1: fe_mul(>z_5_0=fe#1,z2=fe#1,z2=fe#1,>z2=fe#1); */ /* asm 2: fe_sq(>z2=t0,z2=t0,>z2=t0); */ -fe_sq(t0,z); for (i = 1;i < 1;++i) fe_sq(t0,t0); +fe_sq(t0,z); /* for (i = 1;i < 1;++i) fe_sq(t0,t0); */ /* qhasm: z8 = z2^2^2 */ /* asm 1: fe_sq(>z8=fe#2,z8=fe#2,>z8=fe#2); */ @@ -70,7 +70,7 @@ fe_mul(t0,t0,t1); /* qhasm: z22 = z11^2^1 */ /* asm 1: fe_sq(>z22=fe#3,z22=fe#3,>z22=fe#3); */ /* asm 2: fe_sq(>z22=t2,z22=t2,>z22=t2); */ -fe_sq(t2,t0); for (i = 1;i < 1;++i) fe_sq(t2,t2); +fe_sq(t2,t0); /* for (i = 1;i < 1;++i) fe_sq(t2,t2); */ /* qhasm: z_5_0 = z9*z22 */ /* asm 1: fe_mul(>z_5_0=fe#2, Date: Mon, 9 Jun 2014 19:29:08 -0700 Subject: [PATCH 7/7] Avoid naming a function select() in order to avoid confusion with select(2) --- .../crypto_sign/ed25519/ref10/ge_scalarmult_base.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libsodium/crypto_sign/ed25519/ref10/ge_scalarmult_base.c b/src/libsodium/crypto_sign/ed25519/ref10/ge_scalarmult_base.c index 54d7d6ab..f3490a94 100644 --- a/src/libsodium/crypto_sign/ed25519/ref10/ge_scalarmult_base.c +++ b/src/libsodium/crypto_sign/ed25519/ref10/ge_scalarmult_base.c @@ -37,7 +37,7 @@ static ge_precomp base[32][8] = { #include "base.h" } ; -static void select(ge_precomp *t,int pos,signed char b) +static void ge_select(ge_precomp *t,int pos,signed char b) { ge_precomp minust; unsigned char bnegative = negative(b); @@ -95,7 +95,7 @@ void ge_scalarmult_base(ge_p3 *h,const unsigned char *a) ge_p3_0(h); for (i = 1;i < 64;i += 2) { - select(&t,i / 2,e[i]); + ge_select(&t,i / 2,e[i]); ge_madd(&r,h,&t); ge_p1p1_to_p3(h,&r); } @@ -105,7 +105,7 @@ void ge_scalarmult_base(ge_p3 *h,const unsigned char *a) ge_p2_dbl(&r,&s); ge_p1p1_to_p3(h,&r); for (i = 0;i < 64;i += 2) { - select(&t,i / 2,e[i]); + ge_select(&t,i / 2,e[i]); ge_madd(&r,h,&t); ge_p1p1_to_p3(h,&r); } }