From 2bc58748746401e0d3519f48e9a9c9f6d271f101 Mon Sep 17 00:00:00 2001 From: Frank Denis Date: Tue, 17 Nov 2015 00:43:12 +0100 Subject: [PATCH] Check that the output of X25519 is not the all-zero value Return -1 if this happens, and mark crypto_scalarmult() as warn_unused_result Mark dependent functions with warn_unused_result as well --- src/libsodium/crypto_box/crypto_box_easy.c | 8 ++++++-- .../ref/before_curve25519xsalsa20poly1305.c | 4 +++- .../ref/box_curve25519xsalsa20poly1305.c | 8 ++++++-- .../curve25519/scalarmult_curve25519.c | 11 ++++++++++- src/libsodium/include/sodium/crypto_box.h | 12 ++++++++---- .../crypto_box_curve25519xsalsa20poly1305.h | 6 ++++-- .../include/sodium/crypto_scalarmult.h | 3 ++- .../sodium/crypto_scalarmult_curve25519.h | 3 ++- test/default/box.c | 7 +++++-- test/default/box2.c | 4 +++- test/default/box7.c | 4 +++- test/default/box8.c | 6 ++++-- test/default/box_easy.c | 4 +++- test/default/box_easy2.c | 18 ++++++++++++------ test/default/scalarmult.c | 7 +++++-- test/default/scalarmult5.c | 4 +++- test/default/scalarmult6.c | 4 +++- test/default/scalarmult7.c | 8 ++++++-- 18 files changed, 88 insertions(+), 33 deletions(-) diff --git a/src/libsodium/crypto_box/crypto_box_easy.c b/src/libsodium/crypto_box/crypto_box_easy.c index a4066c5c..e1cba835 100644 --- a/src/libsodium/crypto_box/crypto_box_easy.c +++ b/src/libsodium/crypto_box/crypto_box_easy.c @@ -26,7 +26,9 @@ crypto_box_detached(unsigned char *c, unsigned char *mac, (void) sizeof(int[crypto_box_BEFORENMBYTES >= crypto_secretbox_KEYBYTES ? 1 : -1]); - crypto_box_beforenm(k, pk, sk); + if (crypto_box_beforenm(k, pk, sk) != 0) { + return -1; + } ret = crypto_box_detached_afternm(c, mac, m, mlen, n, k); sodium_memzero(k, sizeof k); @@ -75,7 +77,9 @@ crypto_box_open_detached(unsigned char *m, const unsigned char *c, unsigned char k[crypto_box_BEFORENMBYTES]; int ret; - crypto_box_beforenm(k, pk, sk); + if (crypto_box_beforenm(k, pk, sk) != 0) { + return -1; + } ret = crypto_box_open_detached_afternm(m, c, mac, clen, n, k); sodium_memzero(k, sizeof k); diff --git a/src/libsodium/crypto_box/curve25519xsalsa20poly1305/ref/before_curve25519xsalsa20poly1305.c b/src/libsodium/crypto_box/curve25519xsalsa20poly1305/ref/before_curve25519xsalsa20poly1305.c index 40d4300e..d8e2f7b3 100644 --- a/src/libsodium/crypto_box/curve25519xsalsa20poly1305/ref/before_curve25519xsalsa20poly1305.c +++ b/src/libsodium/crypto_box/curve25519xsalsa20poly1305/ref/before_curve25519xsalsa20poly1305.c @@ -14,6 +14,8 @@ int crypto_box_beforenm( ) { unsigned char s[32]; - crypto_scalarmult_curve25519(s,sk,pk); + if (crypto_scalarmult_curve25519(s,sk,pk) != 0) { + return -1; + } return crypto_core_hsalsa20(k,n,s,sigma); } diff --git a/src/libsodium/crypto_box/curve25519xsalsa20poly1305/ref/box_curve25519xsalsa20poly1305.c b/src/libsodium/crypto_box/curve25519xsalsa20poly1305/ref/box_curve25519xsalsa20poly1305.c index ebc208a3..dc616799 100644 --- a/src/libsodium/crypto_box/curve25519xsalsa20poly1305/ref/box_curve25519xsalsa20poly1305.c +++ b/src/libsodium/crypto_box/curve25519xsalsa20poly1305/ref/box_curve25519xsalsa20poly1305.c @@ -12,7 +12,9 @@ int crypto_box( unsigned char k[crypto_box_BEFORENMBYTES]; int ret; - crypto_box_beforenm(k,pk,sk); + if (crypto_box_beforenm(k,pk,sk) != 0) { + return -1; + } ret = crypto_box_afternm(c,m,mlen,n,k); sodium_memzero(k, sizeof k); @@ -30,7 +32,9 @@ int crypto_box_open( unsigned char k[crypto_box_BEFORENMBYTES]; int ret; - crypto_box_beforenm(k,pk,sk); + if (crypto_box_beforenm(k,pk,sk) != 0) { + return -1; + } ret = crypto_box_open_afternm(m,c,clen,n,k); sodium_memzero(k, sizeof k); diff --git a/src/libsodium/crypto_scalarmult/curve25519/scalarmult_curve25519.c b/src/libsodium/crypto_scalarmult/curve25519/scalarmult_curve25519.c index 3075747a..e9a44a91 100644 --- a/src/libsodium/crypto_scalarmult/curve25519/scalarmult_curve25519.c +++ b/src/libsodium/crypto_scalarmult/curve25519/scalarmult_curve25519.c @@ -20,7 +20,16 @@ int crypto_scalarmult_curve25519(unsigned char *q, const unsigned char *n, const unsigned char *p) { - return implementation->mult(q, n, p); + size_t i; + unsigned char d = 0; + + if (implementation->mult(q, n, p) != 0) { + return -1; + } + for (i = 0; i < crypto_scalarmult_curve25519_BYTES; i++) { + d |= q[i]; + } + return -(1 & ((d - 1) >> 8)); } int diff --git a/src/libsodium/include/sodium/crypto_box.h b/src/libsodium/include/sodium/crypto_box.h index 25b1bfce..473f922d 100644 --- a/src/libsodium/include/sodium/crypto_box.h +++ b/src/libsodium/include/sodium/crypto_box.h @@ -54,7 +54,8 @@ int crypto_box_keypair(unsigned char *pk, unsigned char *sk); SODIUM_EXPORT int crypto_box_easy(unsigned char *c, const unsigned char *m, unsigned long long mlen, const unsigned char *n, - const unsigned char *pk, const unsigned char *sk); + const unsigned char *pk, const unsigned char *sk) + __attribute__ ((warn_unused_result)); SODIUM_EXPORT int crypto_box_open_easy(unsigned char *m, const unsigned char *c, @@ -66,7 +67,8 @@ SODIUM_EXPORT int crypto_box_detached(unsigned char *c, unsigned char *mac, const unsigned char *m, unsigned long long mlen, const unsigned char *n, const unsigned char *pk, - const unsigned char *sk); + const unsigned char *sk) + __attribute__ ((warn_unused_result)); SODIUM_EXPORT int crypto_box_open_detached(unsigned char *m, const unsigned char *c, @@ -85,7 +87,8 @@ size_t crypto_box_beforenmbytes(void); SODIUM_EXPORT int crypto_box_beforenm(unsigned char *k, const unsigned char *pk, - const unsigned char *sk); + const unsigned char *sk) + __attribute__ ((warn_unused_result)); SODIUM_EXPORT int crypto_box_easy_afternm(unsigned char *c, const unsigned char *m, @@ -139,7 +142,8 @@ size_t crypto_box_boxzerobytes(void); SODIUM_EXPORT int crypto_box(unsigned char *c, const unsigned char *m, unsigned long long mlen, const unsigned char *n, - const unsigned char *pk, const unsigned char *sk); + const unsigned char *pk, const unsigned char *sk) + __attribute__ ((warn_unused_result)); SODIUM_EXPORT int crypto_box_open(unsigned char *m, const unsigned char *c, diff --git a/src/libsodium/include/sodium/crypto_box_curve25519xsalsa20poly1305.h b/src/libsodium/include/sodium/crypto_box_curve25519xsalsa20poly1305.h index e58d8a53..e9c38d6d 100644 --- a/src/libsodium/include/sodium/crypto_box_curve25519xsalsa20poly1305.h +++ b/src/libsodium/include/sodium/crypto_box_curve25519xsalsa20poly1305.h @@ -51,7 +51,8 @@ int crypto_box_curve25519xsalsa20poly1305(unsigned char *c, unsigned long long mlen, const unsigned char *n, const unsigned char *pk, - const unsigned char *sk); + const unsigned char *sk) + __attribute__ ((warn_unused_result)); SODIUM_EXPORT int crypto_box_curve25519xsalsa20poly1305_open(unsigned char *m, @@ -74,7 +75,8 @@ int crypto_box_curve25519xsalsa20poly1305_keypair(unsigned char *pk, SODIUM_EXPORT int crypto_box_curve25519xsalsa20poly1305_beforenm(unsigned char *k, const unsigned char *pk, - const unsigned char *sk); + const unsigned char *sk) + __attribute__ ((warn_unused_result)); SODIUM_EXPORT int crypto_box_curve25519xsalsa20poly1305_afternm(unsigned char *c, diff --git a/src/libsodium/include/sodium/crypto_scalarmult.h b/src/libsodium/include/sodium/crypto_scalarmult.h index 3d59b3a2..830c10f6 100644 --- a/src/libsodium/include/sodium/crypto_scalarmult.h +++ b/src/libsodium/include/sodium/crypto_scalarmult.h @@ -27,7 +27,8 @@ int crypto_scalarmult_base(unsigned char *q, const unsigned char *n); SODIUM_EXPORT int crypto_scalarmult(unsigned char *q, const unsigned char *n, - const unsigned char *p); + const unsigned char *p) + __attribute__ ((warn_unused_result)); #ifdef __cplusplus } diff --git a/src/libsodium/include/sodium/crypto_scalarmult_curve25519.h b/src/libsodium/include/sodium/crypto_scalarmult_curve25519.h index 4cd622e3..953f8923 100644 --- a/src/libsodium/include/sodium/crypto_scalarmult_curve25519.h +++ b/src/libsodium/include/sodium/crypto_scalarmult_curve25519.h @@ -19,7 +19,8 @@ size_t crypto_scalarmult_curve25519_scalarbytes(void); SODIUM_EXPORT int crypto_scalarmult_curve25519(unsigned char *q, const unsigned char *n, - const unsigned char *p); + const unsigned char *p) + __attribute__ ((warn_unused_result)); SODIUM_EXPORT int crypto_scalarmult_curve25519_base(unsigned char *q, const unsigned char *n); diff --git a/test/default/box.c b/test/default/box.c index b9ba1cc8..68656db5 100644 --- a/test/default/box.c +++ b/test/default/box.c @@ -39,8 +39,10 @@ int main(void) { unsigned char k[crypto_box_BEFORENMBYTES]; int i; + int ret; - crypto_box(c, m, 163, nonce, bobpk, alicesk); + ret = crypto_box(c, m, 163, nonce, bobpk, alicesk); + assert(ret == 0); for (i = 16; i < 163; ++i) { printf(",0x%02x", (unsigned int)c[i]); if (i % 8 == 7) @@ -49,7 +51,8 @@ int main(void) printf("\n"); memset(c, 0, sizeof c); - crypto_box_beforenm(k, bobpk, alicesk); + ret = crypto_box_beforenm(k, bobpk, alicesk); + assert(ret == 0); crypto_box_afternm(c, m, 163, nonce, k); for (i = 16; i < 163; ++i) { printf(",0x%02x", (unsigned int)c[i]); diff --git a/test/default/box2.c b/test/default/box2.c index 93785f83..d83690c6 100644 --- a/test/default/box2.c +++ b/test/default/box2.c @@ -39,6 +39,7 @@ int main(void) { unsigned char k[crypto_box_BEFORENMBYTES]; int i; + int ret; if (crypto_box_open(m, c, 163, nonce, alicepk, bobsk) == 0) { for (i = 32; i < 163; ++i) { @@ -50,7 +51,8 @@ int main(void) } memset(m, 0, sizeof m); - crypto_box_beforenm(k, alicepk, bobsk); + ret = crypto_box_beforenm(k, alicepk, bobsk); + assert(ret == 0); if (crypto_box_open_afternm(m, c, 163, nonce, k) == 0) { for (i = 32; i < 163; ++i) { printf(",0x%02x", (unsigned int)m[i]); diff --git a/test/default/box7.c b/test/default/box7.c index e9b1db57..1f4e6afe 100644 --- a/test/default/box7.c +++ b/test/default/box7.c @@ -15,6 +15,7 @@ int main(void) { size_t mlen; size_t i; + int ret; for (mlen = 0; mlen < 1000 && mlen + crypto_box_ZEROBYTES < sizeof m; ++mlen) { @@ -22,7 +23,8 @@ int main(void) crypto_box_keypair(bobpk, bobsk); randombytes_buf(n, crypto_box_NONCEBYTES); randombytes_buf(m + crypto_box_ZEROBYTES, mlen); - crypto_box(c, m, mlen + crypto_box_ZEROBYTES, n, bobpk, alicesk); + ret = crypto_box(c, m, mlen + crypto_box_ZEROBYTES, n, bobpk, alicesk); + assert(ret == 0); if (crypto_box_open(m2, c, mlen + crypto_box_ZEROBYTES, n, alicepk, bobsk) == 0) { for (i = 0; i < mlen + crypto_box_ZEROBYTES; ++i) { diff --git a/test/default/box8.c b/test/default/box8.c index 75b59a24..ace56d7b 100644 --- a/test/default/box8.c +++ b/test/default/box8.c @@ -15,7 +15,8 @@ int main(void) { size_t mlen; size_t i; - int caught; + int caught; + int ret; for (mlen = 0; mlen < 1000 && mlen + crypto_box_ZEROBYTES < sizeof m; ++mlen) { @@ -23,7 +24,8 @@ int main(void) crypto_box_keypair(bobpk, bobsk); randombytes_buf(n, crypto_box_NONCEBYTES); randombytes_buf(m + crypto_box_ZEROBYTES, mlen); - crypto_box(c, m, mlen + crypto_box_ZEROBYTES, n, bobpk, alicesk); + ret = crypto_box(c, m, mlen + crypto_box_ZEROBYTES, n, bobpk, alicesk); + assert(ret == 0); caught = 0; while (caught < 10) { c[rand() % (mlen + crypto_box_ZEROBYTES)] = rand(); diff --git a/test/default/box_easy.c b/test/default/box_easy.c index e4ce8a67..fca723d0 100644 --- a/test/default/box_easy.c +++ b/test/default/box_easy.c @@ -34,8 +34,10 @@ unsigned char c[147 + crypto_box_MACBYTES]; int main(void) { size_t i; + int ret; - crypto_box_easy(c, m, 131, nonce, bobpk, alicesk); + ret = crypto_box_easy(c, m, 131, nonce, bobpk, alicesk); + assert(ret == 0); for (i = 0; i < 131 + crypto_box_MACBYTES; ++i) { printf(",0x%02x", (unsigned int)c[i]); if (i % 8 == 7) diff --git a/test/default/box_easy2.c b/test/default/box_easy2.c index b0e79de8..5b0c2245 100644 --- a/test/default/box_easy2.c +++ b/test/default/box_easy2.c @@ -18,6 +18,7 @@ int main(void) unsigned char *k2; size_t mlen; size_t i; + int ret; alicepk = (unsigned char *) sodium_malloc(crypto_box_PUBLICKEYBYTES); alicesk = (unsigned char *) sodium_malloc(crypto_box_SECRETKEYBYTES); @@ -32,7 +33,8 @@ int main(void) mlen = (size_t) randombytes_uniform((uint32_t)sizeof m); randombytes_buf(m, mlen); randombytes_buf(nonce, crypto_box_NONCEBYTES); - crypto_box_easy(c, m, mlen, nonce, bobpk, alicesk); + ret = crypto_box_easy(c, m, mlen, nonce, bobpk, alicesk); + assert(ret == 0); if (crypto_box_open_easy(m2, c, (unsigned long long) mlen + crypto_box_MACBYTES, nonce, alicepk, bobsk) != 0) { @@ -50,7 +52,8 @@ int main(void) } memcpy(c, m, mlen); - crypto_box_easy(c, c, (unsigned long long) mlen, nonce, bobpk, alicesk); + ret = crypto_box_easy(c, c, (unsigned long long) mlen, nonce, bobpk, alicesk); + assert(ret == 0); printf("%d\n", memcmp(m, c, mlen) == 0); printf("%d\n", memcmp(m, c + crypto_box_MACBYTES, mlen) == 0); if (crypto_box_open_easy(c, c, @@ -59,8 +62,10 @@ int main(void) printf("crypto_box_open_easy() failed\n"); } - crypto_box_beforenm(k1, alicepk, bobsk); - crypto_box_beforenm(k2, bobpk, alicesk); + ret = crypto_box_beforenm(k1, alicepk, bobsk); + assert(ret == 0); + ret = crypto_box_beforenm(k2, bobpk, alicesk); + assert(ret == 0); memset(m2, 0, sizeof m2); @@ -79,8 +84,9 @@ int main(void) printf("crypto_box_open_easy_afternm() with a huge ciphertext should have failed\n"); } memset(m2, 0, sizeof m2); - crypto_box_detached(c, mac, m, (unsigned long long) mlen, - nonce, alicepk, bobsk); + ret = crypto_box_detached(c, mac, m, (unsigned long long) mlen, + nonce, alicepk, bobsk); + assert(ret == 0); if (crypto_box_open_detached(m2, c, mac, (unsigned long long) mlen, nonce, bobpk, alicesk) != 0) { printf("crypto_box_open_detached() failed\n"); diff --git a/test/default/scalarmult.c b/test/default/scalarmult.c index d476783a..6b6e0336 100644 --- a/test/default/scalarmult.c +++ b/test/default/scalarmult.c @@ -22,6 +22,7 @@ int main(void) (unsigned char *) sodium_malloc(crypto_scalarmult_BYTES); unsigned char *k = (unsigned char *) sodium_malloc(crypto_scalarmult_BYTES); + int ret; assert(alicepk != NULL && bobpk != NULL && k != NULL); @@ -33,11 +34,13 @@ int main(void) sodium_bin2hex(hex, sizeof hex, bobpk, crypto_scalarmult_BYTES); printf("%s\n", hex); - crypto_scalarmult(k, alicesk, bobpk); + ret = crypto_scalarmult(k, alicesk, bobpk); + assert(ret == 0); sodium_bin2hex(hex, sizeof hex, k, crypto_scalarmult_BYTES); printf("%s\n", hex); - crypto_scalarmult(k, bobsk, alicepk); + ret = crypto_scalarmult(k, bobsk, alicepk); + assert(ret == 0); sodium_bin2hex(hex, sizeof hex, k, crypto_scalarmult_BYTES); printf("%s\n", hex); diff --git a/test/default/scalarmult5.c b/test/default/scalarmult5.c index 3dc69775..a3d6f85e 100644 --- a/test/default/scalarmult5.c +++ b/test/default/scalarmult5.c @@ -17,8 +17,10 @@ unsigned char k[32]; int main(void) { int i; + int ret; - crypto_scalarmult(k, alicesk, bobpk); + ret = crypto_scalarmult(k, alicesk, bobpk); + assert(ret == 0); for (i = 0; i < 32; ++i) { if (i > 0) { diff --git a/test/default/scalarmult6.c b/test/default/scalarmult6.c index 69fdc079..3af459f2 100644 --- a/test/default/scalarmult6.c +++ b/test/default/scalarmult6.c @@ -18,6 +18,7 @@ int main(void) unsigned char *bobsk; unsigned char *alicepk; int i; + int ret; k = (unsigned char *) sodium_malloc(crypto_scalarmult_BYTES); bobsk = (unsigned char *) sodium_malloc(crypto_scalarmult_SCALARBYTES); @@ -27,7 +28,8 @@ int main(void) memcpy(bobsk, bobsk_, crypto_scalarmult_SCALARBYTES); memcpy(alicepk, alicepk_, crypto_scalarmult_SCALARBYTES); - crypto_scalarmult(k, bobsk, alicepk); + ret = crypto_scalarmult(k, bobsk, alicepk); + assert(ret == 0); sodium_free(alicepk); sodium_free(bobsk); diff --git a/test/default/scalarmult7.c b/test/default/scalarmult7.c index e83493ef..517be78e 100644 --- a/test/default/scalarmult7.c +++ b/test/default/scalarmult7.c @@ -22,9 +22,13 @@ unsigned char out2[32]; int main(void) { + int ret; + scalar[0] = 1U; - crypto_scalarmult_curve25519(out1, scalar, p1); - crypto_scalarmult_curve25519(out2, scalar, p2); + ret = crypto_scalarmult_curve25519(out1, scalar, p1); + assert(ret == 0); + ret = crypto_scalarmult_curve25519(out2, scalar, p2); + assert(ret == 0); printf("%d\n", !!memcmp(out1, out2, 32)); return 0;