From 59e6963a37a615b137ee4f9824f798dc704fdd96 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 26 Jun 2017 13:26:58 +0100 Subject: [PATCH 1/2] Prevent clever optimization to prematurely quit loop in safe memcmp The previous version of `mbedtls_ssl_safer_memcmp` did not qualify the pointers to the arrays to be compared as volatile, theoretically opening the possibility for the compiler to notice that the loop operation `diff |= A[i] ^ B[i]` is pointless if `diff = -1`. This commit changes this. It also declares the stack variable `diff` as volatile, to force read and write in every loop; omitting that, the compiler would still be allowed to get away with reading `A[i]` and `B[i]` but not doing the XOR and not updating `diff`. --- include/mbedtls/ssl_internal.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/include/mbedtls/ssl_internal.h b/include/mbedtls/ssl_internal.h index 756360b18..8d3ab61ef 100644 --- a/include/mbedtls/ssl_internal.h +++ b/include/mbedtls/ssl_internal.h @@ -600,9 +600,9 @@ void mbedtls_ssl_dtls_replay_update( mbedtls_ssl_context *ssl ); static inline int mbedtls_ssl_safer_memcmp( const void *a, const void *b, size_t n ) { size_t i; - const unsigned char *A = (const unsigned char *) a; - const unsigned char *B = (const unsigned char *) b; - unsigned char diff = 0; + volatile const unsigned char *A = (volatile const unsigned char *) a; + volatile const unsigned char *B = (volatile const unsigned char *) b; + volatile unsigned char diff = 0; for( i = 0; i < n; i++ ) diff |= A[i] ^ B[i]; From d4755deafac99abe708edb1ba13bcac0bbc4f007 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 28 Nov 2017 13:31:12 +0100 Subject: [PATCH 2/2] add changelog entry --- ChangeLog | 1 + 1 file changed, 1 insertion(+) diff --git a/ChangeLog b/ChangeLog index 84a05d003..9cb4430a0 100644 --- a/ChangeLog +++ b/ChangeLog @@ -15,6 +15,7 @@ Security detect it sometimes. Reported by Hugo Leisink. #810 * Tighten parsing of RSA PKCS#1 v1.5 signatures, to avoid a potential Bleichenbacher/BERserk-style attack. + * Tighten should-be-constant-time memcmp against compiler optimizations. Bugfix * Remove size zero arrays from ECJPAKE test suite. Size zero arrays are not