Merge branch 'mbedtls-2.7' into mbedtls-2.7-restricted
* mbedtls-2.7: (28 commits)
A different approach of signed-to-unsigned comparison
Update the copy of tests/data_files/server2-sha256.crt in certs.c
Fix bug in redirection of unit test outputs
Backport e2k support to mbedtls-2.7
Don't forget to free G, P, Q, ctr_drbg, and entropy
Regenerate server2-sha256.crt with a PrintableString issuer
Regenerate test client certificates with a PrintableString issuer
cert_write: support all hash algorithms
compat.sh: stop using allow_sha1
compat.sh: quit using SHA-1 certificates
compat.sh: enable CBC-SHA-2 suites for GnuTLS
Fix license header in pre-commit hook
Update copyright notices to use Linux Foundation guidance
Fix building on NetBSD 9.0
Remove obsolete buildbot reference in compat.sh
Fix misuse of printf in shell script
Fix added proxy command when IPv6 is used
Simplify test syntax
Fix logic error in setting client port
ssl-opt.sh: include test name in log files
...
diff --git a/ChangeLog.d/protect-base-blinding.txt b/ChangeLog.d/protect-base-blinding.txt
new file mode 100644
index 0000000..ca0600c
--- /dev/null
+++ b/ChangeLog.d/protect-base-blinding.txt
@@ -0,0 +1,6 @@
+Security
+ * Fix side channel in RSA private key operations and static (finite-field)
+ Diffie-Hellman. An adversary with precise enough timing and memory access
+ information (typically an untrusted operating system attacking a secure
+ enclave) could bypass an existing counter-measure (base blinding) and
+ potentially fully recover the private key.
diff --git a/ChangeLog.d/x509parse_crl-empty_entry.txt b/ChangeLog.d/x509parse_crl-empty_entry.txt
new file mode 100644
index 0000000..483abb1
--- /dev/null
+++ b/ChangeLog.d/x509parse_crl-empty_entry.txt
@@ -0,0 +1,4 @@
+Security
+ * Fix a 1-byte buffer overread in mbedtls_x509_crl_parse_der().
+ Credit to OSS-Fuzz for detecting the problem and to Philippe Antoine
+ for pinpointing the problematic code.
diff --git a/ChangeLog.d/zeroising_of_plaintext_buffer.txt b/ChangeLog.d/zeroising_of_plaintext_buffer.txt
new file mode 100644
index 0000000..f618beb
--- /dev/null
+++ b/ChangeLog.d/zeroising_of_plaintext_buffer.txt
@@ -0,0 +1,4 @@
+Security
+ * Zeroising of plaintext buffers in mbedtls_ssl_read() to erase unused
+ application data from memory. Reported in #689 by
+ Johan Uppman Bruce of Sectra.
diff --git a/include/mbedtls/check_config.h b/include/mbedtls/check_config.h
index c671133..654c6ba 100644
--- a/include/mbedtls/check_config.h
+++ b/include/mbedtls/check_config.h
@@ -180,6 +180,16 @@
#error "MBEDTLS_ENTROPY_FORCE_SHA256 defined, but not all prerequisites"
#endif
+#if defined(__has_feature)
+#if __has_feature(memory_sanitizer)
+#define MBEDTLS_HAS_MEMSAN
+#endif
+#endif
+#if defined(MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN) && !defined(MBEDTLS_HAS_MEMSAN)
+#error "MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN requires building with MemorySanitizer"
+#endif
+#undef MBEDTLS_HAS_MEMSAN
+
#if defined(MBEDTLS_TEST_NULL_ENTROPY) && \
( !defined(MBEDTLS_ENTROPY_C) || !defined(MBEDTLS_NO_DEFAULT_ENTROPY_SOURCES) )
#error "MBEDTLS_TEST_NULL_ENTROPY defined, but not all prerequisites"
diff --git a/include/mbedtls/config.h b/include/mbedtls/config.h
index bee23c0..82d8d8b 100644
--- a/include/mbedtls/config.h
+++ b/include/mbedtls/config.h
@@ -443,6 +443,22 @@
//#define MBEDTLS_ECP_NORMALIZE_MXZ_ALT
/**
+ * \def MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN
+ *
+ * Enable testing of the constant-flow nature of some sensitive functions with
+ * clang's MemorySanitizer. This causes some existing tests to also test
+ * non-functional properties of the code under test.
+ *
+ * This setting requires compiling with clang -fsanitize=memory.
+ *
+ * \warning This macro is only used for extended testing; it is not considered
+ * part of the library's API, so it may change or disappear at any time.
+ *
+ * Uncomment to enable testing of the constant-flow nature of selected code.
+ */
+//#define MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN
+
+/**
* \def MBEDTLS_TEST_NULL_ENTROPY
*
* Enables testing and use of mbed TLS without any configured entropy sources.
diff --git a/include/mbedtls/ssl_internal.h b/include/mbedtls/ssl_internal.h
index 6dbec1b..c48ba98 100644
--- a/include/mbedtls/ssl_internal.h
+++ b/include/mbedtls/ssl_internal.h
@@ -142,6 +142,23 @@
#define MBEDTLS_SSL_RETRANS_WAITING 2
#define MBEDTLS_SSL_RETRANS_FINISHED 3
+/* This macro determines whether CBC is supported. */
+#if defined(MBEDTLS_CIPHER_MODE_CBC) && \
+ ( defined(MBEDTLS_AES_C) || \
+ defined(MBEDTLS_CAMELLIA_C) || \
+ defined(MBEDTLS_DES_C) )
+#define MBEDTLS_SSL_SOME_SUITES_USE_CBC
+#endif
+
+/* This macro determines whether the CBC construct used in TLS 1.0-1.2 (as
+ * opposed to the very different CBC construct used in SSLv3) is supported. */
+#if defined(MBEDTLS_SSL_SOME_SUITES_USE_CBC) && \
+ ( defined(MBEDTLS_SSL_PROTO_TLS1) || \
+ defined(MBEDTLS_SSL_PROTO_TLS1_1) || \
+ defined(MBEDTLS_SSL_PROTO_TLS1_2) )
+#define MBEDTLS_SSL_SOME_SUITES_USE_TLS_CBC
+#endif
+
/*
* Allow extra bytes for record, authentication and encryption overhead:
* counter (8) + header (5) + IV(16) + MAC (16-48) + padding (0-256)
@@ -730,6 +747,49 @@
#endif /* MBEDTLS_SSL_PROTO_TLS1 || MBEDTLS_SSL_PROTO_TLS1_1 || \
MBEDTLS_SSL_PROTO_TLS1_2 */
+#if defined(MBEDTLS_SSL_SOME_SUITES_USE_TLS_CBC)
+/** \brief Compute the HMAC of variable-length data with constant flow.
+ *
+ * This function computes the HMAC of the concatenation of \p add_data and \p
+ * data, and does with a code flow and memory access pattern that does not
+ * depend on \p data_len_secret, but only on \p min_data_len and \p
+ * max_data_len. In particular, this function always reads exactly \p
+ * max_data_len bytes from \p data.
+ *
+ * \param ctx The HMAC context. It must have keys configured
+ * with mbedtls_md_hmac_starts() and use one of the
+ * following hashes: SHA-384, SHA-256, SHA-1 or MD-5.
+ * It is reset using mbedtls_md_hmac_reset() after
+ * the computation is complete to prepare for the
+ * next computation.
+ * \param add_data The additional data prepended to \p data. This
+ * must point to a readable buffer of \p add_data_len
+ * bytes.
+ * \param add_data_len The length of \p add_data in bytes.
+ * \param data The data appended to \p add_data. This must point
+ * to a readable buffer of \p max_data_len bytes.
+ * \param data_len_secret The length of the data to process in \p data.
+ * This must be no less than \p min_data_len and no
+ * greater than \p max_data_len.
+ * \param min_data_len The minimal length of \p data in bytes.
+ * \param max_data_len The maximal length of \p data in bytes.
+ * \param output The HMAC will be written here. This must point to
+ * a writable buffer of sufficient size to hold the
+ * HMAC value.
+ *
+ * \retval 0
+ * Success.
+ * \retval non-zero
+ * Failure.
+ */
+int mbedtls_ssl_cf_hmac(
+ mbedtls_md_context_t *ctx,
+ const unsigned char *add_data, size_t add_data_len,
+ const unsigned char *data, size_t data_len_secret,
+ size_t min_data_len, size_t max_data_len,
+ unsigned char *output );
+#endif /* MBEDTLS_SSL_SOME_SUITES_USE_TLS_CBC */
+
#ifdef __cplusplus
}
#endif
diff --git a/library/dhm.c b/library/dhm.c
index 79ef116..e15cc8e 100644
--- a/library/dhm.c
+++ b/library/dhm.c
@@ -334,6 +334,32 @@
}
/*
+ * Pick a random R in the range [2, M) for blinding purposes
+ */
+static int dhm_random_below( mbedtls_mpi *R, const mbedtls_mpi *M,
+ int (*f_rng)(void *, unsigned char *, size_t), void *p_rng )
+{
+ int ret, count;
+
+ count = 0;
+ do
+ {
+ MBEDTLS_MPI_CHK( mbedtls_mpi_fill_random( R, mbedtls_mpi_size( M ), f_rng, p_rng ) );
+
+ while( mbedtls_mpi_cmp_mpi( R, M ) >= 0 )
+ MBEDTLS_MPI_CHK( mbedtls_mpi_shift_r( R, 1 ) );
+
+ if( count++ > 10 )
+ return( MBEDTLS_ERR_MPI_NOT_ACCEPTABLE );
+ }
+ while( mbedtls_mpi_cmp_int( R, 1 ) <= 0 );
+
+cleanup:
+ return( ret );
+}
+
+
+/*
* Use the blinding method and optimisation suggested in section 10 of:
* KOCHER, Paul C. Timing attacks on implementations of Diffie-Hellman, RSA,
* DSS, and other systems. In : Advances in Cryptology-CRYPTO'96. Springer
@@ -342,7 +368,10 @@
static int dhm_update_blinding( mbedtls_dhm_context *ctx,
int (*f_rng)(void *, unsigned char *, size_t), void *p_rng )
{
- int ret, count;
+ int ret;
+ mbedtls_mpi R;
+
+ mbedtls_mpi_init( &R );
/*
* Don't use any blinding the first time a particular X is used,
@@ -377,24 +406,23 @@
*/
/* Vi = random( 2, P-1 ) */
- count = 0;
- do
- {
- MBEDTLS_MPI_CHK( mbedtls_mpi_fill_random( &ctx->Vi, mbedtls_mpi_size( &ctx->P ), f_rng, p_rng ) );
+ MBEDTLS_MPI_CHK( dhm_random_below( &ctx->Vi, &ctx->P, f_rng, p_rng ) );
- while( mbedtls_mpi_cmp_mpi( &ctx->Vi, &ctx->P ) >= 0 )
- MBEDTLS_MPI_CHK( mbedtls_mpi_shift_r( &ctx->Vi, 1 ) );
+ /* Vf = Vi^-X mod P
+ * First compute Vi^-1 = R * (R Vi)^-1, (avoiding leaks from inv_mod),
+ * then elevate to the Xth power. */
+ MBEDTLS_MPI_CHK( dhm_random_below( &R, &ctx->P, f_rng, p_rng ) );
+ MBEDTLS_MPI_CHK( mbedtls_mpi_mul_mpi( &ctx->Vf, &ctx->Vi, &R ) );
+ MBEDTLS_MPI_CHK( mbedtls_mpi_mod_mpi( &ctx->Vf, &ctx->Vf, &ctx->P ) );
+ MBEDTLS_MPI_CHK( mbedtls_mpi_inv_mod( &ctx->Vf, &ctx->Vf, &ctx->P ) );
+ MBEDTLS_MPI_CHK( mbedtls_mpi_mul_mpi( &ctx->Vf, &ctx->Vf, &R ) );
+ MBEDTLS_MPI_CHK( mbedtls_mpi_mod_mpi( &ctx->Vf, &ctx->Vf, &ctx->P ) );
- if( count++ > 10 )
- return( MBEDTLS_ERR_MPI_NOT_ACCEPTABLE );
- }
- while( mbedtls_mpi_cmp_int( &ctx->Vi, 1 ) <= 0 );
-
- /* Vf = Vi^-X mod P */
- MBEDTLS_MPI_CHK( mbedtls_mpi_inv_mod( &ctx->Vf, &ctx->Vi, &ctx->P ) );
MBEDTLS_MPI_CHK( mbedtls_mpi_exp_mod( &ctx->Vf, &ctx->Vf, &ctx->X, &ctx->P, &ctx->RP ) );
cleanup:
+ mbedtls_mpi_free( &R );
+
return( ret );
}
diff --git a/library/rsa.c b/library/rsa.c
index 376afa4..c5dbdac 100644
--- a/library/rsa.c
+++ b/library/rsa.c
@@ -744,6 +744,9 @@
int (*f_rng)(void *, unsigned char *, size_t), void *p_rng )
{
int ret, count = 0;
+ mbedtls_mpi R;
+
+ mbedtls_mpi_init( &R );
if( ctx->Vf.p != NULL )
{
@@ -759,18 +762,41 @@
/* Unblinding value: Vf = random number, invertible mod N */
do {
if( count++ > 10 )
- return( MBEDTLS_ERR_RSA_RNG_FAILED );
+ {
+ ret = MBEDTLS_ERR_RSA_RNG_FAILED;
+ goto cleanup;
+ }
MBEDTLS_MPI_CHK( mbedtls_mpi_fill_random( &ctx->Vf, ctx->len - 1, f_rng, p_rng ) );
- MBEDTLS_MPI_CHK( mbedtls_mpi_gcd( &ctx->Vi, &ctx->Vf, &ctx->N ) );
- } while( mbedtls_mpi_cmp_int( &ctx->Vi, 1 ) != 0 );
- /* Blinding value: Vi = Vf^(-e) mod N */
- MBEDTLS_MPI_CHK( mbedtls_mpi_inv_mod( &ctx->Vi, &ctx->Vf, &ctx->N ) );
+ /* Compute Vf^-1 as R * (R Vf)^-1 to avoid leaks from inv_mod. */
+ MBEDTLS_MPI_CHK( mbedtls_mpi_fill_random( &R, ctx->len - 1, f_rng, p_rng ) );
+ MBEDTLS_MPI_CHK( mbedtls_mpi_mul_mpi( &ctx->Vi, &ctx->Vf, &R ) );
+ MBEDTLS_MPI_CHK( mbedtls_mpi_mod_mpi( &ctx->Vi, &ctx->Vi, &ctx->N ) );
+
+ /* At this point, Vi is invertible mod N if and only if both Vf and R
+ * are invertible mod N. If one of them isn't, we don't need to know
+ * which one, we just loop and choose new values for both of them.
+ * (Each iteration succeeds with overwhelming probability.) */
+ ret = mbedtls_mpi_inv_mod( &ctx->Vi, &ctx->Vi, &ctx->N );
+ if( ret == MBEDTLS_ERR_MPI_NOT_ACCEPTABLE )
+ continue;
+ if( ret != 0 )
+ goto cleanup;
+
+ /* Finish the computation of Vf^-1 = R * (R Vf)^-1 */
+ MBEDTLS_MPI_CHK( mbedtls_mpi_mul_mpi( &ctx->Vi, &ctx->Vi, &R ) );
+ MBEDTLS_MPI_CHK( mbedtls_mpi_mod_mpi( &ctx->Vi, &ctx->Vi, &ctx->N ) );
+ } while( 0 );
+
+ /* Blinding value: Vi = Vf^(-e) mod N
+ * (Vi already contains Vf^-1 at this point) */
MBEDTLS_MPI_CHK( mbedtls_mpi_exp_mod( &ctx->Vi, &ctx->Vi, &ctx->E, &ctx->N, &ctx->RN ) );
cleanup:
+ mbedtls_mpi_free( &R );
+
return( ret );
}
diff --git a/library/ssl_tls.c b/library/ssl_tls.c
index ee7a1cf..fdd1a71 100644
--- a/library/ssl_tls.c
+++ b/library/ssl_tls.c
@@ -1299,8 +1299,7 @@
#endif /* MBEDTLS_SSL_PROTO_SSL3 */
#if defined(MBEDTLS_ARC4_C) || defined(MBEDTLS_CIPHER_NULL_CIPHER) || \
- ( defined(MBEDTLS_CIPHER_MODE_CBC) && \
- ( defined(MBEDTLS_AES_C) || defined(MBEDTLS_CAMELLIA_C) ) )
+ defined(MBEDTLS_SSL_SOME_SUITES_USE_CBC)
#define SSL_SOME_MODES_USE_MAC
#endif
@@ -1312,7 +1311,7 @@
defined(MBEDTLS_SSL_PROTO_TLS1_2) )
/* This function makes sure every byte in the memory region is accessed
* (in ascending addresses order) */
-static void ssl_read_memory( unsigned char *p, size_t len )
+static void ssl_read_memory( const unsigned char *p, size_t len )
{
unsigned char acc = 0;
volatile unsigned char force;
@@ -1521,8 +1520,7 @@
}
else
#endif /* MBEDTLS_GCM_C || MBEDTLS_CCM_C */
-#if defined(MBEDTLS_CIPHER_MODE_CBC) && \
- ( defined(MBEDTLS_AES_C) || defined(MBEDTLS_CAMELLIA_C) )
+#if defined(MBEDTLS_SSL_SOME_SUITES_USE_CBC)
if( mode == MBEDTLS_MODE_CBC )
{
int ret;
@@ -1641,8 +1639,7 @@
#endif /* MBEDTLS_SSL_ENCRYPT_THEN_MAC */
}
else
-#endif /* MBEDTLS_CIPHER_MODE_CBC &&
- ( MBEDTLS_AES_C || MBEDTLS_CAMELLIA_C ) */
+#endif /* MBEDTLS_SSL_SOME_SUITES_USE_CBC */
{
MBEDTLS_SSL_DEBUG_MSG( 1, ( "should never happen" ) );
return( MBEDTLS_ERR_SSL_INTERNAL_ERROR );
@@ -1660,6 +1657,136 @@
return( 0 );
}
+#if defined(MBEDTLS_SSL_SOME_SUITES_USE_TLS_CBC)
+/*
+ * Constant-flow conditional memcpy:
+ * - if c1 == c2, equivalent to memcpy(dst, src, len),
+ * - otherwise, a no-op,
+ * but with execution flow independent of the values of c1 and c2.
+ *
+ * Use only bit operations to avoid branches that could be used by some
+ * compilers on some platforms to translate comparison operators.
+ */
+static void mbedtls_ssl_cf_memcpy_if_eq( unsigned char *dst,
+ const unsigned char *src,
+ size_t len,
+ size_t c1, size_t c2 )
+{
+ /* diff = 0 if c1 == c2, non-zero otherwise */
+ const size_t diff = c1 ^ c2;
+
+ /* MSVC has a warning about unary minus on unsigned integer types,
+ * but this is well-defined and precisely what we want to do here. */
+#if defined(_MSC_VER)
+#pragma warning( push )
+#pragma warning( disable : 4146 )
+#endif
+
+ /* diff_msb's most significant bit is equal to c1 != c2 */
+ const size_t diff_msb = ( diff | -diff );
+
+ /* diff1 = c1 != c2 */
+ const size_t diff1 = diff_msb >> ( sizeof( diff_msb ) * 8 - 1 );
+
+ /* mask = c1 != c2 ? 0xff : 0x00 */
+ const unsigned char mask = (unsigned char) -diff1;
+
+#if defined(_MSC_VER)
+#pragma warning( pop )
+#endif
+
+ /* dst[i] = c1 != c2 ? dst[i] : src[i] */
+ size_t i;
+ for( i = 0; i < len; i++ )
+ dst[i] = ( dst[i] & mask ) | ( src[i] & ~mask );
+}
+
+/*
+ * Compute HMAC of variable-length data with constant flow.
+ *
+ * Only works with MD-5, SHA-1, SHA-256 and SHA-384.
+ * (Otherwise, computation of block_size needs to be adapted.)
+ */
+int mbedtls_ssl_cf_hmac(
+ mbedtls_md_context_t *ctx,
+ const unsigned char *add_data, size_t add_data_len,
+ const unsigned char *data, size_t data_len_secret,
+ size_t min_data_len, size_t max_data_len,
+ unsigned char *output )
+{
+ /*
+ * This function breaks the HMAC abstraction and uses the md_clone()
+ * extension to the MD API in order to get constant-flow behaviour.
+ *
+ * HMAC(msg) is defined as HASH(okey + HASH(ikey + msg)) where + means
+ * concatenation, and okey/ikey are the XOR of the key with some fixed bit
+ * patterns (see RFC 2104, sec. 2), which are stored in ctx->hmac_ctx.
+ *
+ * We'll first compute inner_hash = HASH(ikey + msg) by hashing up to
+ * minlen, then cloning the context, and for each byte up to maxlen
+ * finishing up the hash computation, keeping only the correct result.
+ *
+ * Then we only need to compute HASH(okey + inner_hash) and we're done.
+ */
+ const mbedtls_md_type_t md_alg = mbedtls_md_get_type( ctx->md_info );
+ /* TLS 1.0-1.2 only support SHA-384, SHA-256, SHA-1, MD-5,
+ * all of which have the same block size except SHA-384. */
+ const size_t block_size = md_alg == MBEDTLS_MD_SHA384 ? 128 : 64;
+ const unsigned char * const ikey = ctx->hmac_ctx;
+ const unsigned char * const okey = ikey + block_size;
+ const size_t hash_size = mbedtls_md_get_size( ctx->md_info );
+
+ unsigned char aux_out[MBEDTLS_MD_MAX_SIZE];
+ mbedtls_md_context_t aux;
+ size_t offset;
+ int ret;
+
+ mbedtls_md_init( &aux );
+
+#define MD_CHK( func_call ) \
+ do { \
+ ret = (func_call); \
+ if( ret != 0 ) \
+ goto cleanup; \
+ } while( 0 )
+
+ MD_CHK( mbedtls_md_setup( &aux, ctx->md_info, 0 ) );
+
+ /* After hmac_start() of hmac_reset(), ikey has already been hashed,
+ * so we can start directly with the message */
+ MD_CHK( mbedtls_md_update( ctx, add_data, add_data_len ) );
+ MD_CHK( mbedtls_md_update( ctx, data, min_data_len ) );
+
+ /* For each possible length, compute the hash up to that point */
+ for( offset = min_data_len; offset <= max_data_len; offset++ )
+ {
+ MD_CHK( mbedtls_md_clone( &aux, ctx ) );
+ MD_CHK( mbedtls_md_finish( &aux, aux_out ) );
+ /* Keep only the correct inner_hash in the output buffer */
+ mbedtls_ssl_cf_memcpy_if_eq( output, aux_out, hash_size,
+ offset, data_len_secret );
+
+ if( offset < max_data_len )
+ MD_CHK( mbedtls_md_update( ctx, data + offset, 1 ) );
+ }
+
+ /* Now compute HASH(okey + inner_hash) */
+ MD_CHK( mbedtls_md_starts( ctx ) );
+ MD_CHK( mbedtls_md_update( ctx, okey, block_size ) );
+ MD_CHK( mbedtls_md_update( ctx, output, hash_size ) );
+ MD_CHK( mbedtls_md_finish( ctx, output ) );
+
+ /* Done, get ready for next time */
+ MD_CHK( mbedtls_md_hmac_reset( ctx ) );
+
+#undef MD_CHK
+
+cleanup:
+ mbedtls_md_free( &aux );
+ return( ret );
+}
+#endif /* MBEDTLS_SSL_SOME_SUITES_USE_TLS_CBC */
+
static int ssl_decrypt_buf( mbedtls_ssl_context *ssl )
{
size_t i;
@@ -1785,8 +1912,7 @@
}
else
#endif /* MBEDTLS_GCM_C || MBEDTLS_CCM_C */
-#if defined(MBEDTLS_CIPHER_MODE_CBC) && \
- ( defined(MBEDTLS_AES_C) || defined(MBEDTLS_CAMELLIA_C) )
+#if defined(MBEDTLS_SSL_SOME_SUITES_USE_CBC)
if( mode == MBEDTLS_MODE_CBC )
{
/*
@@ -1997,8 +2123,7 @@
ssl->in_msglen -= padlen;
}
else
-#endif /* MBEDTLS_CIPHER_MODE_CBC &&
- ( MBEDTLS_AES_C || MBEDTLS_CAMELLIA_C ) */
+#endif /* MBEDTLS_SSL_SOME_SUITES_USE_CBC) */
{
MBEDTLS_SSL_DEBUG_MSG( 1, ( "should never happen" ) );
return( MBEDTLS_ERR_SSL_INTERNAL_ERROR );
@@ -2038,34 +2163,8 @@
defined(MBEDTLS_SSL_PROTO_TLS1_2)
if( ssl->minor_ver > MBEDTLS_SSL_MINOR_VERSION_0 )
{
- /*
- * Process MAC and always update for padlen afterwards to make
- * total time independent of padlen.
- *
- * Known timing attacks:
- * - Lucky Thirteen (http://www.isg.rhul.ac.uk/tls/TLStiming.pdf)
- *
- * To compensate for different timings for the MAC calculation
- * depending on how much padding was removed (which is determined
- * by padlen), process extra_run more blocks through the hash
- * function.
- *
- * The formula in the paper is
- * extra_run = ceil( (L1-55) / 64 ) - ceil( (L2-55) / 64 )
- * where L1 is the size of the header plus the decrypted message
- * plus CBC padding and L2 is the size of the header plus the
- * decrypted message. This is for an underlying hash function
- * with 64-byte blocks.
- * We use ( (Lx+8) / 64 ) to handle 'negative Lx' values
- * correctly. We round down instead of up, so -56 is the correct
- * value for our calculations instead of -55.
- *
- * Repeat the formula rather than defining a block_size variable.
- * This avoids requiring division by a variable at runtime
- * (which would be marginally less efficient and would require
- * linking an extra division function in some builds).
- */
- size_t j, extra_run = 0;
+ int ret;
+ unsigned char add_data[13];
/*
* The next two sizes are the minimum and maximum values of
@@ -2080,60 +2179,21 @@
const size_t max_len = ssl->in_msglen + padlen;
const size_t min_len = ( max_len > 256 ) ? max_len - 256 : 0;
- switch( ssl->transform_in->ciphersuite_info->mac )
+ memcpy( add_data + 0, ssl->in_ctr, 8 );
+ memcpy( add_data + 8, ssl->in_hdr, 3 );
+ memcpy( add_data + 11, ssl->in_len, 2 );
+
+ ret = mbedtls_ssl_cf_hmac( &ssl->transform_in->md_ctx_dec,
+ add_data, sizeof( add_data ),
+ ssl->in_msg, ssl->in_msglen,
+ min_len, max_len,
+ mac_expect );
+ if( ret != 0 )
{
-#if defined(MBEDTLS_MD5_C) || defined(MBEDTLS_SHA1_C) || \
- defined(MBEDTLS_SHA256_C)
- case MBEDTLS_MD_MD5:
- case MBEDTLS_MD_SHA1:
- case MBEDTLS_MD_SHA256:
- /* 8 bytes of message size, 64-byte compression blocks */
- extra_run = ( 13 + ssl->in_msglen + padlen + 8 ) / 64 -
- ( 13 + ssl->in_msglen + 8 ) / 64;
- break;
-#endif
-#if defined(MBEDTLS_SHA512_C)
- case MBEDTLS_MD_SHA384:
- /* 16 bytes of message size, 128-byte compression blocks */
- extra_run = ( 13 + ssl->in_msglen + padlen + 16 ) / 128 -
- ( 13 + ssl->in_msglen + 16 ) / 128;
- break;
-#endif
- default:
- MBEDTLS_SSL_DEBUG_MSG( 1, ( "should never happen" ) );
- return( MBEDTLS_ERR_SSL_INTERNAL_ERROR );
+ MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_cf_hmac", ret );
+ return( ret );
}
- extra_run &= correct * 0xFF;
-
- mbedtls_md_hmac_update( &ssl->transform_in->md_ctx_dec, ssl->in_ctr, 8 );
- mbedtls_md_hmac_update( &ssl->transform_in->md_ctx_dec, ssl->in_hdr, 3 );
- mbedtls_md_hmac_update( &ssl->transform_in->md_ctx_dec, ssl->in_len, 2 );
- mbedtls_md_hmac_update( &ssl->transform_in->md_ctx_dec, ssl->in_msg,
- ssl->in_msglen );
- /* Make sure we access everything even when padlen > 0. This
- * makes the synchronisation requirements for just-in-time
- * Prime+Probe attacks much tighter and hopefully impractical. */
- ssl_read_memory( ssl->in_msg + ssl->in_msglen, padlen );
- mbedtls_md_hmac_finish( &ssl->transform_in->md_ctx_dec, mac_expect );
-
- /* Dummy calls to compression function.
- * Call mbedtls_md_process at least once due to cache attacks
- * that observe whether md_process() was called of not.
- * Respect the usual start-(process|update)-finish sequence for
- * the sake of hardware accelerators that might require it. */
- mbedtls_md_starts( &ssl->transform_in->md_ctx_dec );
- for( j = 0; j < extra_run + 1; j++ )
- mbedtls_md_process( &ssl->transform_in->md_ctx_dec, ssl->in_msg );
- {
- /* The switch statement above already checks that we're using
- * one of MD-5, SHA-1, SHA-256 or SHA-384. */
- unsigned char tmp[384 / 8];
- mbedtls_md_finish( &ssl->transform_in->md_ctx_dec, tmp );
- }
-
- mbedtls_md_hmac_reset( &ssl->transform_in->md_ctx_dec );
-
/* Make sure we access all the memory that could contain the MAC,
* before we check it in the next code block. This makes the
* synchronisation requirements for just-in-time Prime+Probe
@@ -7345,6 +7405,10 @@
memcpy( buf, ssl->in_offt, n );
ssl->in_msglen -= n;
+ /* Zeroising the plaintext buffer to erase unused application data
+ from the memory. */
+ mbedtls_zeroize( ssl->in_offt, n );
+
if( ssl->in_msglen == 0 )
{
/* all bytes consumed */
diff --git a/library/version_features.c b/library/version_features.c
index 6ed248f..dbe6ba6 100644
--- a/library/version_features.c
+++ b/library/version_features.c
@@ -253,6 +253,9 @@
#if defined(MBEDTLS_ECP_NORMALIZE_MXZ_ALT)
"MBEDTLS_ECP_NORMALIZE_MXZ_ALT",
#endif /* MBEDTLS_ECP_NORMALIZE_MXZ_ALT */
+#if defined(MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN)
+ "MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN",
+#endif /* MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN */
#if defined(MBEDTLS_TEST_NULL_ENTROPY)
"MBEDTLS_TEST_NULL_ENTROPY",
#endif /* MBEDTLS_TEST_NULL_ENTROPY */
diff --git a/library/x509_crl.c b/library/x509_crl.c
index d890f2c..73a300d 100644
--- a/library/x509_crl.c
+++ b/library/x509_crl.c
@@ -287,13 +287,13 @@
size_t len2;
const unsigned char *end2;
+ cur_entry->raw.tag = **p;
if( ( ret = mbedtls_asn1_get_tag( p, end, &len2,
MBEDTLS_ASN1_SEQUENCE | MBEDTLS_ASN1_CONSTRUCTED ) ) != 0 )
{
return( ret );
}
- cur_entry->raw.tag = **p;
cur_entry->raw.p = *p;
cur_entry->raw.len = len2;
end2 = *p + len2;
diff --git a/scripts/config.pl b/scripts/config.pl
index 0fd5190..2dadc87 100755
--- a/scripts/config.pl
+++ b/scripts/config.pl
@@ -124,6 +124,7 @@
MBEDTLS_REMOVE_ARC4_CIPHERSUITES
MBEDTLS_RSA_NO_CRT
MBEDTLS_SSL_HW_RECORD_ACCEL
+MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN
MBEDTLS_TEST_NULL_ENTROPY
MBEDTLS_X509_ALLOW_UNSUPPORTED_CRITICAL_EXTENSION
MBEDTLS_ZLIB_SUPPORT
diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh
index 4673b80..8139694 100755
--- a/tests/scripts/all.sh
+++ b/tests/scripts/all.sh
@@ -961,6 +961,24 @@
if_build_succeeded env OPENSSL_CMD="$OPENSSL_LEGACY" GNUTLS_CLI="$GNUTLS_LEGACY_CLI" GNUTLS_SERV="$GNUTLS_LEGACY_SERV" tests/compat.sh -e '^$' -f 'NULL\|DES\|RC4\|ARCFOUR'
}
+component_test_memsan_constant_flow () {
+ # This tests both (1) accesses to undefined memory, and (2) branches or
+ # memory access depending on secret values. To distinguish between those:
+ # - unset MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN - does the failure persist?
+ # - or alternatively, change the build type to MemSanDbg, which enables
+ # origin tracking and nicer stack traces (which are useful for debugging
+ # anyway), and check if the origin was TEST_CF_SECRET() or something else.
+ msg "build: cmake MSan (clang), full config with constant flow testing"
+ scripts/config.pl full
+ scripts/config.pl set MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN
+ scripts/config.pl unset MBEDTLS_AESNI_C # memsan doesn't grok asm
+ CC=clang cmake -D CMAKE_BUILD_TYPE:String=MemSan .
+ make
+
+ msg "test: main suites (Msan + constant flow)"
+ make test
+}
+
component_test_default_no_deprecated () {
# Test that removing the deprecated features from the default
# configuration leaves something consistent.
diff --git a/tests/suites/helpers.function b/tests/suites/helpers.function
index d205786..3309173 100644
--- a/tests/suites/helpers.function
+++ b/tests/suites/helpers.function
@@ -38,6 +38,22 @@
#include <unistd.h>
#endif
+#if defined(MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN)
+#include <sanitizer/msan_interface.h>
+
+/* Use macros to avoid messing up with origin tracking */
+#define TEST_CF_SECRET __msan_allocated_memory
+// void __msan_allocated_memory(const volatile void* data, size_t size);
+#define TEST_CF_PUBLIC __msan_unpoison
+// void __msan_unpoison(const volatile void *a, size_t size);
+
+#else /* MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN */
+
+#define TEST_CF_SECRET(ptr, size)
+#define TEST_CF_PUBLIC(ptr, size)
+
+#endif /* MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN */
+
/*----------------------------------------------------------------------------*/
/* Constants */
diff --git a/tests/suites/test_suite_ssl.data b/tests/suites/test_suite_ssl.data
index b92c1fe..f85d26b 100644
--- a/tests/suites/test_suite_ssl.data
+++ b/tests/suites/test_suite_ssl.data
@@ -57,3 +57,19 @@
SSL SET_HOSTNAME memory leak: call ssl_set_hostname twice
ssl_set_hostname_twice:"server0":"server1"
+
+Constant-flow HMAC: MD5
+depends_on:MBEDTLS_MD5_C
+ssl_cf_hmac:MBEDTLS_MD_MD5
+
+Constant-flow HMAC: SHA1
+depends_on:MBEDTLS_SHA1_C
+ssl_cf_hmac:MBEDTLS_MD_SHA1
+
+Constant-flow HMAC: SHA256
+depends_on:MBEDTLS_SHA256_C
+ssl_cf_hmac:MBEDTLS_MD_SHA256
+
+Constant-flow HMAC: SHA384
+depends_on:MBEDTLS_SHA512_C:!MBEDTLS_SHA512_NO_SHA384
+ssl_cf_hmac:MBEDTLS_MD_SHA384
diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function
index 1cd2ed5..0987374 100644
--- a/tests/suites/test_suite_ssl.function
+++ b/tests/suites/test_suite_ssl.function
@@ -54,3 +54,94 @@
}
/* END_CASE */
+/* BEGIN_CASE depends_on:MBEDTLS_SSL_SOME_SUITES_USE_TLS_CBC */
+void ssl_cf_hmac( int hash )
+{
+ /*
+ * Test the function mbedtls_ssl_cf_hmac() against a reference
+ * implementation.
+ */
+ mbedtls_md_context_t ctx, ref_ctx;
+ const mbedtls_md_info_t *md_info;
+ size_t out_len, block_size;
+ size_t min_in_len, in_len, max_in_len, i;
+ /* TLS additional data is 13 bytes (hence the "lucky 13" name) */
+ unsigned char add_data[13];
+ unsigned char ref_out[MBEDTLS_MD_MAX_SIZE];
+ unsigned char *data = NULL;
+ unsigned char *out = NULL;
+ unsigned char rec_num = 0;
+
+ mbedtls_md_init( &ctx );
+ mbedtls_md_init( &ref_ctx );
+
+ md_info = mbedtls_md_info_from_type( hash );
+ TEST_ASSERT( md_info != NULL );
+ out_len = mbedtls_md_get_size( md_info );
+ TEST_ASSERT( out_len != 0 );
+ block_size = hash == MBEDTLS_MD_SHA384 ? 128 : 64;
+
+ /* Use allocated out buffer to catch overwrites */
+ out = mbedtls_calloc( 1, out_len );
+ TEST_ASSERT( out != NULL );
+
+ /* Set up contexts with the given hash and a dummy key */
+ TEST_ASSERT( 0 == mbedtls_md_setup( &ctx, md_info, 1 ) );
+ TEST_ASSERT( 0 == mbedtls_md_setup( &ref_ctx, md_info, 1 ) );
+ memset( ref_out, 42, sizeof( ref_out ) );
+ TEST_ASSERT( 0 == mbedtls_md_hmac_starts( &ctx, ref_out, out_len ) );
+ TEST_ASSERT( 0 == mbedtls_md_hmac_starts( &ref_ctx, ref_out, out_len ) );
+ memset( ref_out, 0, sizeof( ref_out ) );
+
+ /*
+ * Test all possible lengths up to a point. The difference between
+ * max_in_len and min_in_len is at most 255, and make sure they both vary
+ * by at least one block size.
+ */
+ for( max_in_len = 0; max_in_len <= 255 + block_size; max_in_len++ )
+ {
+ /* Use allocated in buffer to catch overreads */
+ data = mbedtls_calloc( 1, max_in_len );
+ TEST_ASSERT( data != NULL || max_in_len == 0 );
+
+ min_in_len = max_in_len > 255 ? max_in_len - 255 : 0;
+ for( in_len = min_in_len; in_len <= max_in_len; in_len++ )
+ {
+ /* Set up dummy data and add_data */
+ rec_num++;
+ memset( add_data, rec_num, sizeof( add_data ) );
+ for( i = 0; i < in_len; i++ )
+ data[i] = ( i & 0xff ) ^ rec_num;
+
+ /* Get the function's result */
+ TEST_CF_SECRET( &in_len, sizeof( in_len ) );
+ TEST_ASSERT( 0 == mbedtls_ssl_cf_hmac( &ctx, add_data, sizeof( add_data ),
+ data, in_len,
+ min_in_len, max_in_len,
+ out ) );
+ TEST_CF_PUBLIC( &in_len, sizeof( in_len ) );
+ TEST_CF_PUBLIC( out, out_len );
+
+ /* Compute the reference result */
+ TEST_ASSERT( 0 == mbedtls_md_hmac_update( &ref_ctx, add_data,
+ sizeof( add_data ) ) );
+ TEST_ASSERT( 0 == mbedtls_md_hmac_update( &ref_ctx, data, in_len ) );
+ TEST_ASSERT( 0 == mbedtls_md_hmac_finish( &ref_ctx, ref_out ) );
+ TEST_ASSERT( 0 == mbedtls_md_hmac_reset( &ref_ctx ) );
+
+ /* Compare */
+ TEST_ASSERT( 0 == memcmp( out, ref_out, out_len ) );
+ }
+
+ mbedtls_free( data );
+ data = NULL;
+ }
+
+exit:
+ mbedtls_md_free( &ref_ctx );
+ mbedtls_md_free( &ctx );
+
+ mbedtls_free( data );
+ mbedtls_free( out );
+}
+/* END_CASE */
diff --git a/tests/suites/test_suite_x509parse.data b/tests/suites/test_suite_x509parse.data
index e5d5fed..b87859e 100644
--- a/tests/suites/test_suite_x509parse.data
+++ b/tests/suites/test_suite_x509parse.data
@@ -1332,10 +1332,60 @@
depends_on:MBEDTLS_RSA_C:MBEDTLS_SHA256_C
x509parse_crl:"305d3047020100300d06092a864886f70d01010e0500300f310d300b0603550403130441424344170c303930313031303030303030301430128202abcd170c303831323331323335393539300d06092a864886f70d01010e05000302000100":"":MBEDTLS_ERR_X509_INVALID_FORMAT + MBEDTLS_ERR_ASN1_LENGTH_MISMATCH
+# 305c
+# 3047 tbsCertList TBSCertList
+# 020100 version INTEGER OPTIONAL
+# 300d signatureAlgorithm AlgorithmIdentifi
+# 06092a864886f70d01010e
+# 0500
+# 300f issuer Name
+# 310d300b0603550403130441424344
+# 170c303930313031303030303030 thisUpdate Time
+# 3014 revokedCertificates
+# 3012 entry 1
+# 8202abcd userCertificate CertificateSerialNum
+# 170c303831323331323335393539 revocationDate Time
+# 300d signatureAlgorithm AlgorithmIdentifi
+# 06092a864886f70d01010e
+# 0500
+# 03020001 signatureValue BIT STRING
+# The subsequent TBSCertList negative tests remove or modify some elements.
X509 CRL ASN1 (TBSCertList, sig present)
depends_on:MBEDTLS_RSA_C:MBEDTLS_SHA256_C
x509parse_crl:"305c3047020100300d06092a864886f70d01010e0500300f310d300b0603550403130441424344170c303930313031303030303030301430128202abcd170c303831323331323335393539300d06092a864886f70d01010e050003020001":"CRL version \: 1\nissuer name \: CN=ABCD\nthis update \: 2009-01-01 00\:00\:00\nnext update \: 0000-00-00 00\:00\:00\nRevoked certificates\:\nserial number\: AB\:CD revocation date\: 2008-12-31 23\:59\:59\nsigned using \: RSA with SHA-224\n":0
+X509 CRL ASN1 (TBSCertList, signatureValue missing)
+depends_on:MBEDTLS_RSA_C:MBEDTLS_SHA256_C
+x509parse_crl:"30583047020100300d06092a864886f70d01010e0500300f310d300b0603550403130441424344170c303930313031303030303030301430128202abcd170c303831323331323335393539300d06092a864886f70d01010e0500":"":MBEDTLS_ERR_X509_INVALID_SIGNATURE + MBEDTLS_ERR_ASN1_OUT_OF_DATA
+
+X509 CRL ASN1 (TBSCertList, signatureAlgorithm missing)
+depends_on:MBEDTLS_RSA_C:MBEDTLS_SHA256_C
+x509parse_crl:"30493047020100300d06092a864886f70d01010e0500300f310d300b0603550403130441424344170c303930313031303030303030301430128202abcd170c303831323331323335393539":"":MBEDTLS_ERR_X509_INVALID_ALG + MBEDTLS_ERR_ASN1_OUT_OF_DATA
+
+X509 CRL ASN1 (TBSCertList, single empty entry at end)
+depends_on:MBEDTLS_RSA_C:MBEDTLS_SHA256_C
+x509parse_crl:"30373035020100300d06092a864886f70d01010e0500300f310d300b0603550403130441424344170c30393031303130303030303030023000":"":MBEDTLS_ERR_X509_INVALID_SERIAL + MBEDTLS_ERR_ASN1_OUT_OF_DATA
+
+X509 CRL ASN1 (TBSCertList, good entry then empty entry at end)
+depends_on:MBEDTLS_RSA_C:MBEDTLS_SHA256_C
+x509parse_crl:"304b3049020100300d06092a864886f70d01010e0500300f310d300b0603550403130441424344170c303930313031303030303030301630128202abcd170c3038313233313233353935393000":"":MBEDTLS_ERR_X509_INVALID_SERIAL + MBEDTLS_ERR_ASN1_OUT_OF_DATA
+
+X509 CRL ASN1 (TBSCertList, missing time in entry)
+depends_on:MBEDTLS_RSA_C:MBEDTLS_SHA256_C
+x509parse_crl:"304e3039020100300d06092a864886f70d01010e0500300f310d300b0603550403130441424344170c303930313031303030303030300630048202abcd300d06092a864886f70d01010e050003020001":"":MBEDTLS_ERR_X509_INVALID_DATE + MBEDTLS_ERR_ASN1_OUT_OF_DATA
+
+X509 CRL ASN1 (TBSCertList, missing time in entry at end)
+depends_on:MBEDTLS_RSA_C:MBEDTLS_SHA256_C
+x509parse_crl:"303b3039020100300d06092a864886f70d01010e0500300f310d300b0603550403130441424344170c303930313031303030303030300630048202abcd":"":MBEDTLS_ERR_X509_INVALID_DATE + MBEDTLS_ERR_ASN1_OUT_OF_DATA
+
+X509 CRL ASN1 (TBSCertList, invalid tag for time in entry)
+depends_on:MBEDTLS_RSA_C:MBEDTLS_SHA256_C
+x509parse_crl:"305c3047020100300d06092a864886f70d01010e0500300f310d300b0603550403130441424344170c303930313031303030303030301430128202abcd190c303831323331323335393539300d06092a864886f70d01010e050003020001":"":MBEDTLS_ERR_X509_INVALID_DATE + MBEDTLS_ERR_ASN1_UNEXPECTED_TAG
+
+X509 CRL ASN1 (TBSCertList, invalid tag for serial)
+depends_on:MBEDTLS_RSA_C:MBEDTLS_SHA256_C
+x509parse_crl:"305c3047020100300d06092a864886f70d01010e0500300f310d300b0603550403130441424344170c303930313031303030303030301430128402abcd170c303831323331323335393539300d06092a864886f70d01010e050003020001":"":MBEDTLS_ERR_X509_INVALID_SERIAL + MBEDTLS_ERR_ASN1_UNEXPECTED_TAG
+
X509 CRL ASN1 (TBSCertList, no entries)
depends_on:MBEDTLS_RSA_C:MBEDTLS_SHA256_C
x509parse_crl:"30463031020100300d06092a864886f70d01010e0500300f310d300b0603550403130441424344170c303930313031303030303030300d06092a864886f70d01010e050003020001":"CRL version \: 1\nissuer name \: CN=ABCD\nthis update \: 2009-01-01 00\:00\:00\nnext update \: 0000-00-00 00\:00\:00\nRevoked certificates\:\nsigned using \: RSA with SHA-224\n":0