Merge pull request #3019 from mpg/fix-ssl-opt-gnutls-no-sha1-2.16
[backport 2.16] Fix ssl-opt.sh for GnuTLS versions rejecting SHA-1
diff --git a/ChangeLog b/ChangeLog
index f03b83d..a8f9a79 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -2,11 +2,6 @@
= mbed TLS 2.16.X branch released XXXX-XX-XX
-Bugfix
- * Allow loading symlinked certificates. Fixes #3005. Reported and fixed
- by Jonathan Bennett <JBennett@incomsystems.biz> via #3008.
- * Fix an unchecked call to mbedtls_md() in the x509write module.
-
Security
* Fix potential memory overread when performing an ECDSA signature
operation. The overread only happens with cryptographically low
@@ -14,8 +9,16 @@
unless the RNG is broken, and could result in information disclosure or
denial of service (application crash or extra resource consumption).
Found by Auke Zeilstra and Peter Schwabe, using static analysis.
+ * To avoid a side channel vulnerability when parsing an RSA private key,
+ read all the CRT parameters from the DER structure rather than
+ reconstructing them. Found by Alejandro Cabrera Aldaya and Billy Bob
+ Brumley. Reported and fix contributed by Jack Lloyd.
+ ARMmbed/mbed-crypto#352
Bugfix
+ * Allow loading symlinked certificates. Fixes #3005. Reported and fixed
+ by Jonathan Bennett <JBennett@incomsystems.biz> via #3008.
+ * Fix an unchecked call to mbedtls_md() in the x509write module.
= mbed TLS 2.16.4 branch released 2020-01-15
diff --git a/library/pkparse.c b/library/pkparse.c
index ae210bc..0ae2402 100644
--- a/library/pkparse.c
+++ b/library/pkparse.c
@@ -768,14 +768,40 @@
goto cleanup;
p += len;
- /* Complete the RSA private key */
- if( ( ret = mbedtls_rsa_complete( rsa ) ) != 0 )
- goto cleanup;
+#if !defined(MBEDTLS_RSA_NO_CRT)
+ /*
+ * The RSA CRT parameters DP, DQ and QP are nominally redundant, in
+ * that they can be easily recomputed from D, P and Q. However by
+ * parsing them from the PKCS1 structure it is possible to avoid
+ * recalculating them which both reduces the overhead of loading
+ * RSA private keys into memory and also avoids side channels which
+ * can arise when computing those values, since all of D, P, and Q
+ * are secret. See https://eprint.iacr.org/2020/055 for a
+ * description of one such attack.
+ */
- /* Check optional parameters */
+ /* Import DP */
+ if( ( ret = mbedtls_asn1_get_mpi( &p, end, &rsa->DP ) ) != 0)
+ goto cleanup;
+
+ /* Import DQ */
+ if( ( ret = mbedtls_asn1_get_mpi( &p, end, &rsa->DQ ) ) != 0)
+ goto cleanup;
+
+ /* Import QP */
+ if( ( ret = mbedtls_asn1_get_mpi( &p, end, &rsa->QP ) ) != 0)
+ goto cleanup;
+
+#else
+ /* Verify existance of the CRT params */
if( ( ret = mbedtls_asn1_get_mpi( &p, end, &T ) ) != 0 ||
( ret = mbedtls_asn1_get_mpi( &p, end, &T ) ) != 0 ||
( ret = mbedtls_asn1_get_mpi( &p, end, &T ) ) != 0 )
+ goto cleanup;
+#endif
+
+ /* Complete the RSA private key */
+ if( ( ret = mbedtls_rsa_complete( rsa ) ) != 0 )
goto cleanup;
if( p != end )
diff --git a/library/rsa.c b/library/rsa.c
index af1a878..09fd379 100644
--- a/library/rsa.c
+++ b/library/rsa.c
@@ -249,6 +249,9 @@
{
int ret = 0;
int have_N, have_P, have_Q, have_D, have_E;
+#if !defined(MBEDTLS_RSA_NO_CRT)
+ int have_DP, have_DQ, have_QP;
+#endif
int n_missing, pq_missing, d_missing, is_pub, is_priv;
RSA_VALIDATE_RET( ctx != NULL );
@@ -259,6 +262,12 @@
have_D = ( mbedtls_mpi_cmp_int( &ctx->D, 0 ) != 0 );
have_E = ( mbedtls_mpi_cmp_int( &ctx->E, 0 ) != 0 );
+#if !defined(MBEDTLS_RSA_NO_CRT)
+ have_DP = ( mbedtls_mpi_cmp_int( &ctx->DP, 0 ) != 0 );
+ have_DQ = ( mbedtls_mpi_cmp_int( &ctx->DQ, 0 ) != 0 );
+ have_QP = ( mbedtls_mpi_cmp_int( &ctx->QP, 0 ) != 0 );
+#endif
+
/*
* Check whether provided parameters are enough
* to deduce all others. The following incomplete
@@ -324,7 +333,7 @@
*/
#if !defined(MBEDTLS_RSA_NO_CRT)
- if( is_priv )
+ if( is_priv && ! ( have_DP && have_DQ && have_QP ) )
{
ret = mbedtls_rsa_deduce_crt( &ctx->P, &ctx->Q, &ctx->D,
&ctx->DP, &ctx->DQ, &ctx->QP );
diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh
index 8f25c6b..ee0547c 100755
--- a/tests/scripts/all.sh
+++ b/tests/scripts/all.sh
@@ -652,6 +652,45 @@
if_build_succeeded tests/compat.sh
}
+component_test_zlib_make() {
+ msg "build: zlib enabled, make"
+ scripts/config.pl set MBEDTLS_ZLIB_SUPPORT
+ make ZLIB=1 CFLAGS='-Werror -O1'
+
+ msg "test: main suites (zlib, make)"
+ make test
+
+ msg "test: ssl-opt.sh (zlib, make)"
+ if_build_succeeded tests/ssl-opt.sh
+}
+support_test_zlib_make () {
+ base=support_test_zlib_$$
+ cat <<'EOF' > ${base}.c
+#include "zlib.h"
+int main(void) { return 0; }
+EOF
+ gcc -o ${base}.exe ${base}.c -lz 2>/dev/null
+ ret=$?
+ rm -f ${base}.*
+ return $ret
+}
+
+component_test_zlib_cmake() {
+ msg "build: zlib enabled, cmake"
+ scripts/config.pl set MBEDTLS_ZLIB_SUPPORT
+ cmake -D ENABLE_ZLIB_SUPPORT=On -D CMAKE_BUILD_TYPE:String=Check .
+ make
+
+ msg "test: main suites (zlib, cmake)"
+ make test
+
+ msg "test: ssl-opt.sh (zlib, cmake)"
+ if_build_succeeded tests/ssl-opt.sh
+}
+support_test_zlib_cmake () {
+ support_test_zlib_make "$@"
+}
+
component_test_ref_configs () {
msg "test/build: ref-configs (ASan build)" # ~ 6 min 20s
CC=gcc cmake -D CMAKE_BUILD_TYPE:String=Asan .
diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh
index f833ef0..3a5cad2 100755
--- a/tests/ssl-opt.sh
+++ b/tests/ssl-opt.sh
@@ -905,6 +905,18 @@
-s "Protocol is DTLSv1.2" \
-s "Ciphersuite is TLS-ECDHE-RSA-WITH-CHACHA20-POLY1305-SHA256"
+requires_config_enabled MBEDTLS_ZLIB_SUPPORT
+run_test "Default (compression enabled)" \
+ "$P_SRV debug_level=3" \
+ "$P_CLI debug_level=3" \
+ 0 \
+ -s "Allocating compression buffer" \
+ -c "Allocating compression buffer" \
+ -s "Record expansion is unknown (compression)" \
+ -c "Record expansion is unknown (compression)" \
+ -S "error" \
+ -C "error"
+
# Test current time in ServerHello
requires_config_enabled MBEDTLS_HAVE_TIME
run_test "ServerHello contains gmt_unix_time" \
