Add mbedtls_ssl_cf_memcpy_offset() with tests
The tests are supposed to be failing now (in all.sh component
test_memsan_constant_flow), but they don't as apparently MemSan doesn't
complain when the src argument of memcpy() is uninitialized, see
https://github.com/google/sanitizers/issues/1296
The next commit will add an option to test constant flow with valgrind, which
will hopefully correctly flag the current non-constant-flow implementation.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
diff --git a/library/ssl_invasive.h b/library/ssl_invasive.h
index f04b816..60b6209 100644
--- a/library/ssl_invasive.h
+++ b/library/ssl_invasive.h
@@ -73,6 +73,30 @@
const unsigned char *data, size_t data_len_secret,
size_t min_data_len, size_t max_data_len,
unsigned char *output );
+
+/** \brief Copy data from a secret position with constant flow.
+ *
+ * This function copies \p len bytes from \p src_base + \p offset_secret to \p
+ * dst, with a code flow and memory access pattern that does not depend on \p
+ * offset_secret, but only on \p offset_min, \p offset_max and \p len.
+ *
+ * \param dst The destination buffer. This must point to a writable
+ * buffer of at least \p len bytes.
+ * \param src_base The base of the source buffer. This must point to a
+ * readable buffer of at least \p offset_max + \p len
+ * bytes.
+ * \param offset_secret The offset in the source buffer from which to copy.
+ * This must be no less than \p offset_min and no greater
+ * than \p offset_max.
+ * \param offset_min The minimal value of \p offset_secret.
+ * \param offset_max The maximal value of \p offset_secret.
+ * \param len The number of bytes to copy.
+ */
+void mbedtls_ssl_cf_memcpy_offset( unsigned char *dst,
+ const unsigned char *src_base,
+ size_t offset_secret,
+ size_t offset_min, size_t offset_max,
+ size_t len );
#endif /* MBEDTLS_TEST_HOOKS && MBEDTLS_SSL_SOME_SUITES_USE_TLS_CBC */
#endif /* MBEDTLS_SSL_INVASIVE_H */
diff --git a/library/ssl_msg.c b/library/ssl_msg.c
index e90e599..5dc8012 100644
--- a/library/ssl_msg.c
+++ b/library/ssl_msg.c
@@ -1193,6 +1193,24 @@
mbedtls_md_free( &aux );
return( ret );
}
+
+/*
+ * Constant-flow memcpy from variable position in buffer.
+ * - functionally equivalent to memcpy(dst, src + offset_secret, len)
+ * - but with execution flow independant from the value of offset_secret.
+ */
+MBEDTLS_STATIC_TESTABLE void mbedtls_ssl_cf_memcpy_offset(
+ unsigned char *dst,
+ const unsigned char *src_base,
+ size_t offset_secret,
+ size_t offset_min, size_t offset_max,
+ size_t len )
+{
+ /* WIP - THIS IS NOT ACTUALLY CONSTANT-FLOW!
+ * This is just to be able to write tests and check they work. */
+ ssl_read_memory( src_base + offset_min, offset_max - offset_min + len );
+ memcpy( dst, src_base + offset_secret, len );
+}
#endif /* MBEDTLS_SSL_SOME_SUITES_USE_TLS_CBC */
int mbedtls_ssl_decrypt_buf( mbedtls_ssl_context const *ssl,
@@ -1674,7 +1692,7 @@
{
/*
* The next two sizes are the minimum and maximum values of
- * in_msglen over all padlen values.
+ * data_len over all padlen values.
*
* They're independent of padlen, since we previously did
* data_len -= padlen.
@@ -1695,13 +1713,10 @@
return( ret );
}
- /* 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
- * attacks much tighter and hopefully impractical. */
- ssl_read_memory( data + min_len,
- max_len - min_len + transform->maclen );
- memcpy( mac_peer, data + rec->data_len, transform->maclen );
+ mbedtls_ssl_cf_memcpy_offset( mac_peer, data,
+ rec->data_len,
+ min_len, max_len,
+ transform->maclen );
}
else
#endif /* MBEDTLS_SSL_PROTO_TLS1 || MBEDTLS_SSL_PROTO_TLS1_1 || \
