diff --git a/library/ecp.c b/library/ecp.c index f5d43d5d63..d907587abd 100644 --- a/library/ecp.c +++ b/library/ecp.c @@ -3280,16 +3280,14 @@ int mbedtls_ecp_read_key(mbedtls_ecp_group_id grp_id, mbedtls_ecp_keypair *key, ); } } - #endif #if defined(MBEDTLS_ECP_SHORT_WEIERSTRASS_ENABLED) if (mbedtls_ecp_get_type(&key->grp) == MBEDTLS_ECP_TYPE_SHORT_WEIERSTRASS) { MBEDTLS_MPI_CHK(mbedtls_mpi_read_binary(&key->d, buf, buflen)); - - MBEDTLS_MPI_CHK(mbedtls_ecp_check_privkey(&key->grp, &key->d)); } - #endif + MBEDTLS_MPI_CHK(mbedtls_ecp_check_privkey(&key->grp, &key->d)); + cleanup: if (ret != 0) { diff --git a/library/pkparse.c b/library/pkparse.c index e3d84c266a..fa0570c070 100644 --- a/library/pkparse.c +++ b/library/pkparse.c @@ -654,7 +654,7 @@ static int pk_parse_key_rfc8410_der(mbedtls_pk_context *pk, #else /* MBEDTLS_PK_USE_PSA_EC_DATA */ mbedtls_ecp_keypair *eck = mbedtls_pk_ec_rw(*pk); - if ((ret = mbedtls_mpi_read_binary_le(&eck->d, key, len)) != 0) { + if ((ret = mbedtls_ecp_read_key(eck->grp.id, eck, key, len)) != 0) { return MBEDTLS_ERROR_ADD(MBEDTLS_ERR_PK_KEY_INVALID_FORMAT, ret); } #endif /* MBEDTLS_PK_USE_PSA_EC_DATA */ @@ -666,14 +666,6 @@ static int pk_parse_key_rfc8410_der(mbedtls_pk_context *pk, return ret; } - /* When MBEDTLS_PK_USE_PSA_EC_DATA the key is checked while importing it - * into PSA. */ -#if !defined(MBEDTLS_PK_USE_PSA_EC_DATA) - if ((ret = mbedtls_ecp_check_privkey(&eck->grp, &eck->d)) != 0) { - return ret; - } -#endif /* !MBEDTLS_PK_USE_PSA_EC_DATA */ - return 0; } #endif /* MBEDTLS_PK_HAVE_RFC8410_CURVES */ @@ -1217,15 +1209,11 @@ static int pk_parse_key_sec1_der(mbedtls_pk_context *pk, return MBEDTLS_ERROR_ADD(MBEDTLS_ERR_PK_KEY_INVALID_FORMAT, ret); } + /* Keep a reference to the position fo the private key. It will be used + * later in this function. */ d = p; d_len = len; -#if !defined(MBEDTLS_PK_USE_PSA_EC_DATA) - if ((ret = mbedtls_mpi_read_binary(&eck->d, p, len)) != 0) { - return MBEDTLS_ERROR_ADD(MBEDTLS_ERR_PK_KEY_INVALID_FORMAT, ret); - } -#endif - p += len; pubkey_done = 0; @@ -1245,6 +1233,13 @@ static int pk_parse_key_sec1_der(mbedtls_pk_context *pk, } } + +#if !defined(MBEDTLS_PK_USE_PSA_EC_DATA) + if ((ret = mbedtls_ecp_read_key(eck->grp.id, eck, d, d_len)) != 0) { + return MBEDTLS_ERROR_ADD(MBEDTLS_ERR_PK_KEY_INVALID_FORMAT, ret); + } +#endif + if (p != end) { /* * Is 'publickey' present? If not, or if we can't read it (eg because it @@ -1307,12 +1302,6 @@ static int pk_parse_key_sec1_der(mbedtls_pk_context *pk, } } -#if !defined(MBEDTLS_PK_USE_PSA_EC_DATA) - if ((ret = mbedtls_ecp_check_privkey(&eck->grp, &eck->d)) != 0) { - return ret; - } -#endif /* !MBEDTLS_PK_USE_PSA_EC_DATA */ - return 0; } #endif /* MBEDTLS_PK_HAVE_ECC_KEYS */ diff --git a/tests/scripts/analyze_outcomes.py b/tests/scripts/analyze_outcomes.py index a2b1356188..f3a14a9d43 100755 --- a/tests/scripts/analyze_outcomes.py +++ b/tests/scripts/analyze_outcomes.py @@ -248,21 +248,7 @@ TASKS = { 'ECP test vectors secp384r1 rfc 5114', 'ECP test vectors secp521r1 rfc 5114', ], - 'test_suite_pkparse': [ - # This is a known difference for Montgomery curves: in - # reference component private keys are parsed using - # mbedtls_mpi_read_binary_le(), while in driver version they - # they are imported in PSA and there the parsing is done - # through mbedtls_ecp_read_key(). Unfortunately the latter - # fixes the errors which are intentionally set on the parsed - # key and therefore the following test case is not failing - # as expected. - # This cause the following test to be guarded by ECP_C and - # not being executed on the driver version. - ('Key ASN1 (OneAsymmetricKey X25519, doesn\'t match masking ' - 'requirements, from RFC8410 Appendix A but made into version 0)'), - ], - }, + } } }, 'analyze_driver_vs_reference_no_ecp_at_all': { @@ -298,10 +284,6 @@ TASKS = { 'PSA key derivation: bits=7 invalid for ECC SECT_R2 (ECC enabled)', ], 'test_suite_pkparse': [ - # See description provided for the analyze_driver_vs_reference_all_ec_algs - # case above. - ('Key ASN1 (OneAsymmetricKey X25519, doesn\'t match masking ' - 'requirements, from RFC8410 Appendix A but made into version 0)'), # When PK_PARSE_C and ECP_C are defined then PK_PARSE_EC_COMPRESSED # is automatically enabled in build_info.h (backward compatibility) # even if it is disabled in config_psa_crypto_no_ecp_at_all(). As a diff --git a/tests/suites/test_suite_pkparse.data b/tests/suites/test_suite_pkparse.data index ed5a576552..8e272bd107 100644 --- a/tests/suites/test_suite_pkparse.data +++ b/tests/suites/test_suite_pkparse.data @@ -1196,29 +1196,47 @@ Key ASN1 (ECPrivateKey, empty parameters) depends_on:MBEDTLS_PK_HAVE_ECC_KEYS pk_parse_key:"30070201010400a000":MBEDTLS_ERR_PK_KEY_INVALID_FORMAT -Key ASN1 (OneAsymmetricKey X25519, doesn't match masking requirements, from RFC8410 Appendix A but made into version 0) -depends_on:MBEDTLS_ECP_C -pk_parse_key:"302e020100300506032b656e04220420f8ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff3f":MBEDTLS_ERR_PK_KEY_INVALID_FORMAT - Key ASN1 (OneAsymmetricKey X25519, with invalid optional AlgorithIdentifier parameters) -depends_on:MBEDTLS_PK_HAVE_ECC_KEYS +depends_on:MBEDTLS_PK_HAVE_ECC_KEYS:MBEDTLS_ECP_DP_CURVE25519_ENABLED pk_parse_key:"3030020100300706032b656e050004220420b06d829655543a51cba36e53522bc0acfd60af59466555fb3e1e796872ab1a59":MBEDTLS_ERR_PK_KEY_INVALID_FORMAT Key ASN1 (OneAsymmetricKey X25519, with NULL private key) -depends_on:MBEDTLS_PK_HAVE_ECC_KEYS +depends_on:MBEDTLS_PK_HAVE_ECC_KEYS:MBEDTLS_ECP_DP_CURVE25519_ENABLED pk_parse_key:"300e020100300506032b656e04020500":MBEDTLS_ERR_PK_KEY_INVALID_FORMAT Key ASN1 (OneAsymmetricKey with invalid AlgorithIdentifier) pk_parse_key:"3013020100300a06082b0601040181fd5904020500":MBEDTLS_ERR_PK_KEY_INVALID_FORMAT Key ASN1 (OneAsymmetricKey X25519, with unsupported attributes) -depends_on:MBEDTLS_PK_HAVE_ECC_KEYS +depends_on:MBEDTLS_PK_HAVE_ECC_KEYS:MBEDTLS_ECP_DP_CURVE25519_ENABLED pk_parse_key:"304f020100300506032b656e04220420b06d829655543a51cba36e53522bc0acfd60af59466555fb3e1e796872ab1a59a01f301d060a2a864886f70d01090914310f0c0d437572646c6520436861697273":MBEDTLS_ERR_PK_KEY_INVALID_FORMAT Key ASN1 (OneAsymmetricKey X25519, unsupported version 2 with public key) -depends_on:MBEDTLS_PK_HAVE_ECC_KEYS +depends_on:MBEDTLS_PK_HAVE_ECC_KEYS:MBEDTLS_ECP_DP_CURVE25519_ENABLED pk_parse_key:"3051020101300506032b656e04220420b06d829655543a51cba36e53522bc0acfd60af59466555fb3e1e796872ab1a598121009bc3b0e93d8233fe6a8ba6138948cc12a91362d5c2ed81584db05ab5419c9d11":MBEDTLS_ERR_PK_KEY_INVALID_FORMAT Key ASN1 (OneAsymmetricKey X25519, unsupported version 2 with public key and unsupported attributes) -depends_on:MBEDTLS_PK_HAVE_ECC_KEYS +depends_on:MBEDTLS_PK_HAVE_ECC_KEYS:MBEDTLS_ECP_DP_CURVE25519_ENABLED pk_parse_key:"3072020101300506032b656e04220420b06d829655543a51cba36e53522bc0acfd60af59466555fb3e1e796872ab1a59a01f301d060a2a864886f70d01090914310f0c0d437572646c65204368616972738121009bc3b0e93d8233fe6a8ba6138948cc12a91362d5c2ed81584db05ab5419c9d11":MBEDTLS_ERR_PK_KEY_INVALID_FORMAT + +# From RFC8410 Appendix A but made into version 0 +OneAsymmetricKey X25519, doesn't match masking requirements #1 +depends_on:MBEDTLS_ECP_DP_CURVE25519_ENABLED +pk_parse_fix_montgomery:"302e020100300506032b656e04220420f8ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff3f":"302e020100300506032b656e04220420f8ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff7f" + +# Full inverse of the expected x25519 pattern: +# - 3 LSb to 1 +# - 1st MSb to 1 +# - 2nd MSb to 0 +# Note: Montgomery keys are written in Little endian format. +OneAsymmetricKey X25519, doesn't match masking requirements #2 +depends_on:MBEDTLS_ECP_DP_CURVE25519_ENABLED +pk_parse_fix_montgomery:"302e020100300506032b656e04220420ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffbf":"302e020100300506032b656e04220420f8ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff7f" + +# Full inverse of the expected x448 pattern: +# - 2 LSb to 1 +# - MSb to 0 +# Note: Montgomery keys are written in Little endian format. +OneAsymmetricKey X448, doesn't match masking requirements #3 +depends_on:MBEDTLS_ECP_DP_CURVE448_ENABLED +pk_parse_fix_montgomery:"3046020100300506032b656f043a0438ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff7f":"3046020100300506032b656f043a0438fcffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff" diff --git a/tests/suites/test_suite_pkparse.function b/tests/suites/test_suite_pkparse.function index fd098b043b..df139c60fc 100644 --- a/tests/suites/test_suite_pkparse.function +++ b/tests/suites/test_suite_pkparse.function @@ -3,6 +3,7 @@ #include "mbedtls/pem.h" #include "mbedtls/oid.h" #include "mbedtls/ecp.h" +#include "mbedtls/psa_util.h" #include "pk_internal.h" /* END_HEADER */ @@ -148,3 +149,39 @@ exit: USE_PSA_DONE(); } /* END_CASE */ + +/* BEGIN_CASE depends_on:MBEDTLS_PK_HAVE_ECC_KEYS:MBEDTLS_PK_WRITE_C */ +void pk_parse_fix_montgomery(data_t *input_key, data_t *exp_output) +{ + /* Montgomery keys have specific bits set to either 0 or 1 depending on + * their position. This is enforced during parsing (please see the implementation + * of mbedtls_ecp_read_key() for more details). The scope of this function + * is to verify this enforcing by feeding the parse algorithm with a x25519 + * key which does not have those bits set properly. */ + mbedtls_pk_context pk; + unsigned char *output_key = NULL; + size_t output_key_len = 0; + + mbedtls_pk_init(&pk); + USE_PSA_INIT(); + + TEST_EQUAL(mbedtls_pk_parse_key(&pk, input_key->x, input_key->len, NULL, 0, + mbedtls_test_rnd_std_rand, NULL), 0); + + output_key_len = input_key->len; + ASSERT_ALLOC(output_key, output_key_len); + /* output_key_len is updated with the real amount of data written to + * output_key buffer. */ + output_key_len = mbedtls_pk_write_key_der(&pk, output_key, output_key_len); + TEST_ASSERT(output_key_len > 0); + + ASSERT_COMPARE(exp_output->x, exp_output->len, output_key, output_key_len); + +exit: + if (output_key != NULL) { + mbedtls_free(output_key); + } + mbedtls_pk_free(&pk); + USE_PSA_DONE(); +} +/* END_CASE */