From 875d1eb2c925e6e31ffe48d122f3a8d97c6eee5b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 6 Dec 2022 10:42:44 +0100 Subject: [PATCH 1/4] Document all effects of MBEDTLS_ECP_RESTARTABLE MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It might not be obvious that this option goes beyond adding new functions, but also automagically modifies the behaviour of TLS in some circumstances. Moreover, the exact modifications and circumstances were not documented anywhere outside the ChangeLog. Fix that. While at it, adjust the test that checks no restartable behaviour with other key exchanges, to use a key exchange that allows cert-based client authentication so that we can check that this is not restartable either. We don't have any automated test checking that the server is never affected. That would require adding an ec_max_ops command-line option to ssl_server2 that never has any effect, just to check that it indeed doesn't. I'm not sure that's worth it. I tested manually and could confirm that the server never has restartable behaviour, even for the parts that are shared between client and server such as cert chain verification. Note (from re-reading the code): all restartable behaviour is controlled by the flag ssl->handshake->ecrs_enabled which is only client-side with the ECDHE-ECDSA key exchange (TLS 1.2). Note: this commit is backported from development, which has more dependency declarations in tests/ssl-opt.sh. While at it, add them to the existing tests. Signed-off-by: Manuel Pégourié-Gonnard --- include/mbedtls/config.h | 27 ++++++++++++++++++++++++--- tests/ssl-opt.sh | 34 ++++++++++++++++++++++++++++++---- 2 files changed, 54 insertions(+), 7 deletions(-) diff --git a/include/mbedtls/config.h b/include/mbedtls/config.h index 61db79362f..dff4522574 100644 --- a/include/mbedtls/config.h +++ b/include/mbedtls/config.h @@ -859,12 +859,33 @@ * This is useful in non-threaded environments if you want to avoid blocking * for too long on ECC (and, hence, X.509 or SSL/TLS) operations. * - * Uncomment this macro to enable restartable ECC computations. + * This option: + * - Adds xxx_restartable() variants of existing operations in the + * following modules, with corresponding restart context types: + * - ECP: scalar multiplication (mult), linear combination (muladd); + * - ECDSA: signature generation & verification; + * - PK: signature generation & verification; + * - X509: certificate chain verification. + * - Adds mbedtls_ecdh_enable_restart() in the ECDH module. + * - Changes the behaviour of TLS 1.2 clients (not servers) when using the + * ECDHE-ECDSA key exchange (not other key exchanges) to make all ECC + * computations restartable: + * - ECDH operations from the key exchange; + * - verification of the server's key exchange signature; + * - verification of the server's certificate chain; + * - generation of our signature if client authentication is used, with an + * ECC key/certificate. + * + * \note In the cases above, the usual SSL/TLS functions, such as + * mbedtls_ssl_handshake(), can now return + * MBEDTLS_ERR_SSL_CRYPTO_IN_PROGRESS. * * \note This option only works with the default software implementation of * elliptic curve functionality. It is incompatible with - * MBEDTLS_ECP_ALT, MBEDTLS_ECDH_XXX_ALT, MBEDTLS_ECDSA_XXX_ALT - * and MBEDTLS_ECDH_LEGACY_CONTEXT. + * MBEDTLS_ECP_ALT, MBEDTLS_ECDH_XXX_ALT, MBEDTLS_ECDSA_XXX_ALT, + * MBEDTLS_ECDH_LEGACY_CONTEXT, and MBEDTLS_USE_PSA_CRYPTO. + * + * Uncomment this macro to enable restartable ECC computations. */ //#define MBEDTLS_ECP_RESTARTABLE diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index c1fffa9ede..9e5dc6c84e 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -7720,6 +7720,8 @@ run_test "Large server packet TLS 1.2 AEAD shorter tag" \ # Tests for restartable ECC +requires_config_enabled MBEDTLS_ECP_DP_SECP256R1_ENABLED +requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2 requires_config_enabled MBEDTLS_ECP_RESTARTABLE run_test "EC restart: TLS, default" \ "$P_SRV auth_mode=required" \ @@ -7732,6 +7734,8 @@ run_test "EC restart: TLS, default" \ -C "mbedtls_ecdh_make_public.*4b00" \ -C "mbedtls_pk_sign.*4b00" +requires_config_enabled MBEDTLS_ECP_DP_SECP256R1_ENABLED +requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2 requires_config_enabled MBEDTLS_ECP_RESTARTABLE run_test "EC restart: TLS, max_ops=0" \ "$P_SRV auth_mode=required" \ @@ -7744,6 +7748,8 @@ run_test "EC restart: TLS, max_ops=0" \ -C "mbedtls_ecdh_make_public.*4b00" \ -C "mbedtls_pk_sign.*4b00" +requires_config_enabled MBEDTLS_ECP_DP_SECP256R1_ENABLED +requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2 requires_config_enabled MBEDTLS_ECP_RESTARTABLE run_test "EC restart: TLS, max_ops=65535" \ "$P_SRV auth_mode=required" \ @@ -7756,6 +7762,8 @@ run_test "EC restart: TLS, max_ops=65535" \ -C "mbedtls_ecdh_make_public.*4b00" \ -C "mbedtls_pk_sign.*4b00" +requires_config_enabled MBEDTLS_ECP_DP_SECP256R1_ENABLED +requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2 requires_config_enabled MBEDTLS_ECP_RESTARTABLE run_test "EC restart: TLS, max_ops=1000" \ "$P_SRV auth_mode=required" \ @@ -7768,6 +7776,8 @@ run_test "EC restart: TLS, max_ops=1000" \ -c "mbedtls_ecdh_make_public.*4b00" \ -c "mbedtls_pk_sign.*4b00" +requires_config_enabled MBEDTLS_ECP_DP_SECP256R1_ENABLED +requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2 requires_config_enabled MBEDTLS_ECP_RESTARTABLE run_test "EC restart: TLS, max_ops=1000, badsign" \ "$P_SRV auth_mode=required \ @@ -7785,6 +7795,8 @@ run_test "EC restart: TLS, max_ops=1000, badsign" \ -c "! mbedtls_ssl_handshake returned" \ -c "X509 - Certificate verification failed" +requires_config_enabled MBEDTLS_ECP_DP_SECP256R1_ENABLED +requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2 requires_config_enabled MBEDTLS_ECP_RESTARTABLE run_test "EC restart: TLS, max_ops=1000, auth_mode=optional badsign" \ "$P_SRV auth_mode=required \ @@ -7802,6 +7814,8 @@ run_test "EC restart: TLS, max_ops=1000, auth_mode=optional badsign" \ -C "! mbedtls_ssl_handshake returned" \ -C "X509 - Certificate verification failed" +requires_config_enabled MBEDTLS_ECP_DP_SECP256R1_ENABLED +requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2 requires_config_enabled MBEDTLS_ECP_RESTARTABLE run_test "EC restart: TLS, max_ops=1000, auth_mode=none badsign" \ "$P_SRV auth_mode=required \ @@ -7819,6 +7833,8 @@ run_test "EC restart: TLS, max_ops=1000, auth_mode=none badsign" \ -C "! mbedtls_ssl_handshake returned" \ -C "X509 - Certificate verification failed" +requires_config_enabled MBEDTLS_ECP_DP_SECP256R1_ENABLED +requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2 requires_config_enabled MBEDTLS_ECP_RESTARTABLE run_test "EC restart: DTLS, max_ops=1000" \ "$P_SRV auth_mode=required dtls=1" \ @@ -7831,6 +7847,8 @@ run_test "EC restart: DTLS, max_ops=1000" \ -c "mbedtls_ecdh_make_public.*4b00" \ -c "mbedtls_pk_sign.*4b00" +requires_config_enabled MBEDTLS_ECP_DP_SECP256R1_ENABLED +requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2 requires_config_enabled MBEDTLS_ECP_RESTARTABLE run_test "EC restart: TLS, max_ops=1000 no client auth" \ "$P_SRV" \ @@ -7842,11 +7860,19 @@ run_test "EC restart: TLS, max_ops=1000 no client auth" \ -c "mbedtls_ecdh_make_public.*4b00" \ -C "mbedtls_pk_sign.*4b00" + +# Restartable is only for ECDHE-ECDSA, with another ciphersuite we expect no +# restartable behaviour at all (not even client auth). +# This is the same as "EC restart: TLS, max_ops=1000" except with ECDHE-RSA, +# and all 4 assertions negated. requires_config_enabled MBEDTLS_ECP_RESTARTABLE -run_test "EC restart: TLS, max_ops=1000, ECDHE-PSK" \ - "$P_SRV psk=abc123" \ - "$P_CLI force_ciphersuite=TLS-ECDHE-PSK-WITH-AES-128-CBC-SHA256 \ - psk=abc123 debug_level=1 ec_max_ops=1000" \ +requires_config_enabled MBEDTLS_ECP_DP_SECP256R1_ENABLED +requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2 +run_test "EC restart: TLS, max_ops=1000, ECDHE-RSA" \ + "$P_SRV curves=secp256r1 auth_mode=required" \ + "$P_CLI force_ciphersuite=TLS-ECDHE-RSA-WITH-AES-128-GCM-SHA256 \ + key_file=data_files/server5.key crt_file=data_files/server5.crt \ + debug_level=1 ec_max_ops=1000" \ 0 \ -C "x509_verify_cert.*4b00" \ -C "mbedtls_pk_verify.*4b00" \ From 3dc7f238e62ef697b0a697a1bbec6bdca38586e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 6 Dec 2022 13:20:06 +0100 Subject: [PATCH 2/4] Document that ECP_RESTARTABLE depends on ECP_C MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is not new, it had always been the case, just not documented. Pointed out by depends.py pkalgs (again, now that restartable is part of full). Signed-off-by: Manuel Pégourié-Gonnard --- include/mbedtls/check_config.h | 5 +++++ include/mbedtls/config.h | 2 ++ tests/scripts/depends.py | 1 + 3 files changed, 8 insertions(+) diff --git a/include/mbedtls/check_config.h b/include/mbedtls/check_config.h index 7ae1ff94db..25df257a94 100644 --- a/include/mbedtls/check_config.h +++ b/include/mbedtls/check_config.h @@ -143,6 +143,11 @@ #error "MBEDTLS_ECDH_VARIANT_EVEREST_ENABLED defined, but MBEDTLS_ECDH_LEGACY_CONTEXT not disabled" #endif +#if defined(MBEDTLS_ECP_RESTARTABLE) && \ + !defined(MBEDTLS_ECP_C) +#error "MBEDTLS_ECP_RESTARTABLE defined, but not all prerequisites" +#endif + #if defined(MBEDTLS_ECDSA_DETERMINISTIC) && !defined(MBEDTLS_HMAC_DRBG_C) #error "MBEDTLS_ECDSA_DETERMINISTIC defined, but not all prerequisites" #endif diff --git a/include/mbedtls/config.h b/include/mbedtls/config.h index dff4522574..f6d2c612b7 100644 --- a/include/mbedtls/config.h +++ b/include/mbedtls/config.h @@ -885,6 +885,8 @@ * MBEDTLS_ECP_ALT, MBEDTLS_ECDH_XXX_ALT, MBEDTLS_ECDSA_XXX_ALT, * MBEDTLS_ECDH_LEGACY_CONTEXT, and MBEDTLS_USE_PSA_CRYPTO. * + * Requires: MBEDTLS_ECP_C + * * Uncomment this macro to enable restartable ECC computations. */ //#define MBEDTLS_ECP_RESTARTABLE diff --git a/tests/scripts/depends.py b/tests/scripts/depends.py index 2c38f9d705..ed9caa41de 100755 --- a/tests/scripts/depends.py +++ b/tests/scripts/depends.py @@ -229,6 +229,7 @@ REVERSE_DEPENDENCIES = { 'MBEDTLS_ECP_C': ['MBEDTLS_ECDSA_C', 'MBEDTLS_ECDH_C', 'MBEDTLS_ECJPAKE_C', + 'MBEDTLS_ECP_RESTARTABLE', 'MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA_ENABLED', 'MBEDTLS_KEY_EXCHANGE_ECDH_RSA_ENABLED', 'MBEDTLS_KEY_EXCHANGE_ECDHE_PSK_ENABLED', From b884f7e3dcb6a41c8986699a040dcdd4538183f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Fri, 9 Dec 2022 09:53:55 +0100 Subject: [PATCH 3/4] Clarify documentation of ECP_RESTARTABLE MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- include/mbedtls/config.h | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/include/mbedtls/config.h b/include/mbedtls/config.h index f6d2c612b7..9a2de676d6 100644 --- a/include/mbedtls/config.h +++ b/include/mbedtls/config.h @@ -862,7 +862,8 @@ * This option: * - Adds xxx_restartable() variants of existing operations in the * following modules, with corresponding restart context types: - * - ECP: scalar multiplication (mult), linear combination (muladd); + * - ECP (for Short Weierstrass curves only): scalar multiplication (mul), + * linear combination (muladd); * - ECDSA: signature generation & verification; * - PK: signature generation & verification; * - X509: certificate chain verification. @@ -870,11 +871,12 @@ * - Changes the behaviour of TLS 1.2 clients (not servers) when using the * ECDHE-ECDSA key exchange (not other key exchanges) to make all ECC * computations restartable: - * - ECDH operations from the key exchange; + * - ECDH operations from the key exchange, only for Short Weierstass + * curves; * - verification of the server's key exchange signature; * - verification of the server's certificate chain; - * - generation of our signature if client authentication is used, with an - * ECC key/certificate. + * - generation of the client's signature if client authentication is used, + * with an ECC key/certificate. * * \note In the cases above, the usual SSL/TLS functions, such as * mbedtls_ssl_handshake(), can now return From df310768c8905ebd471533373ff1b3cbf6319e75 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 6 Dec 2022 12:14:49 +0100 Subject: [PATCH 4/4] Fix missing dependency declaration in test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit muladd() (restartable or not) is only available when at least one short weirstrass curve is enabled. Found by depends.py curves in development. Signed-off-by: Manuel Pégourié-Gonnard --- tests/suites/test_suite_ecp.function | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/suites/test_suite_ecp.function b/tests/suites/test_suite_ecp.function index 59812b4efb..7e9c6d2813 100644 --- a/tests/suites/test_suite_ecp.function +++ b/tests/suites/test_suite_ecp.function @@ -513,7 +513,7 @@ exit: } /* END_CASE */ -/* BEGIN_CASE depends_on:MBEDTLS_ECP_RESTARTABLE */ +/* BEGIN_CASE depends_on:MBEDTLS_ECP_RESTARTABLE:MBEDTLS_ECP_SHORT_WEIERSTRASS_ENABLED */ void ecp_muladd_restart( int id, char *xR_str, char *yR_str, char *u1_str, char *u2_str, char *xQ_str, char *yQ_str,