Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Investigate ECP light #7357

Closed
wants to merge 9 commits into from
Closed

Investigate ECP light #7357

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
fc2b797
WIP
mpg Mar 30, 2023
fc0c64e
rename FULL symbol to ECP_FULL to be more future proof
valeriosetti Mar 31, 2023
2bcbc76
pkparse: adding temporary ECP_HAS_KEY_GENERATION symbol to pass all t…
valeriosetti Mar 31, 2023
e45ea62
pk: adding temporary symbol ECP_HAS_CHECK_PAIR for passing tests
valeriosetti Mar 31, 2023
55e297e
pk: add alternate (PSA based) function for EC key pair verification
valeriosetti Mar 31, 2023
95b3003
pkparse: adding a function for generating public key from private
valeriosetti Mar 31, 2023
0c22798
ecp: adding alternate function for key-pair derivation based on PSA
valeriosetti Mar 31, 2023
d064b4a
pk: keep function for generating EC keypair in test_suite_pk
valeriosetti Apr 3, 2023
1b10c2b
pk_wrap: improve eckey_alt_check_pair() function
valeriosetti Apr 3, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
106 changes: 101 additions & 5 deletions library/pk_wrap.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1095,6 +1095,103 @@ static int eckey_sign_rs_wrap(void *ctx, mbedtls_md_type_t md_alg,
}
#endif /* MBEDTLS_ECDSA_C && MBEDTLS_ECP_RESTARTABLE */

#if !defined(ECP_FULL)
/*
* Alternative function used to verify that the EC private/public key pair
* is valid using PSA functions instead of ECP ones.
* The flow is:
* - sign a hash message using the provided private key
* - verify the signature using the public key
*/
static int eckey_alt_check_pair(const void *pub, const void *prv,
int (*f_rng)(void *, unsigned char *, size_t),
void *p_rng)
{
(void)f_rng;
(void)p_rng;
psa_status_t status;
psa_key_attributes_t key_attr = PSA_KEY_ATTRIBUTES_INIT;
mbedtls_ecp_keypair *prv_ctx = (mbedtls_ecp_keypair *) prv;
mbedtls_ecp_keypair *pub_ctx = (mbedtls_ecp_keypair *) pub;
unsigned char sig[MBEDTLS_MPI_MAX_SIZE];
size_t sig_len = 0;
unsigned char hash[32];
int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
size_t curve_bits;
psa_ecc_family_t curve =
mbedtls_ecc_group_to_psa(prv_ctx->grp.id, &curve_bits);
unsigned char key_buf[MBEDTLS_PSA_MAX_EC_KEY_PAIR_LENGTH];
size_t key_len = PSA_BITS_TO_BYTES(curve_bits);
mbedtls_svc_key_id_t key_id = MBEDTLS_SVC_KEY_ID_INIT;

memset(hash, 0x2a, sizeof(hash));

psa_set_key_type(&key_attr, PSA_KEY_TYPE_ECC_KEY_PAIR(curve));
psa_set_key_usage_flags(&key_attr, PSA_KEY_USAGE_SIGN_HASH);
// TODO: forcing SHA256 because this is included by default when building
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, this is a problem with the "sign-verify" approach I suggested, indeed. In the meantime, I had another idea, inspired by what you did for key completion: we could just serialize the public key for both prv and pub (using mbedtls_pk_write_pubkey() I think) and see if the results are identical.

(Then later when #7202 is done, that would be mbedtls_pk_write_pubkey() on prv and directly compare to the byte array in pub, then when and its private counterpart is done, that would be psa_import() + psa_export_public() on prv and compare to byte array in pub.)

Wdyt?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this idea!
However I have a question: since also for the key creation we are already importing the private key into PSA to get its public part, why can't we go that way directly also here? In theory also 7202 is a sort of middle step toward having everything on the PSA side (or at least there is a discussion on this)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's continue this discussion in #7387.

// the library (even though it's not granted that the built-in version
// is supported). Is there a more general purpose solution?
psa_set_key_algorithm(&key_attr, PSA_ALG_ECDSA(PSA_ALG_SHA_256));

ret = mbedtls_mpi_write_binary(&prv_ctx->d, key_buf, key_len);
if (ret != 0) {
return ret;
}

status = psa_import_key(&key_attr, key_buf, key_len, &key_id);
if (status != PSA_SUCCESS) {
ret = PSA_PK_TO_MBEDTLS_ERR(status);
return ret;
}

status = psa_sign_hash(key_id, PSA_ALG_ECDSA(PSA_ALG_SHA_256),
hash, sizeof(hash), sig, sizeof(sig), &sig_len);
if (status != PSA_SUCCESS) {
ret = PSA_PK_TO_MBEDTLS_ERR(status);
status = psa_destroy_key(key_id);
return (status != PSA_SUCCESS) ? PSA_PK_TO_MBEDTLS_ERR(status) : ret;
}

status = psa_destroy_key(key_id);
if (status != PSA_SUCCESS) {
return PSA_PK_TO_MBEDTLS_ERR(status);
}
psa_reset_key_attributes(&key_attr);
mbedtls_platform_zeroize(key_buf, sizeof(key_buf));

psa_set_key_type(&key_attr, PSA_KEY_TYPE_ECC_PUBLIC_KEY(curve));
psa_set_key_usage_flags(&key_attr, PSA_KEY_USAGE_VERIFY_HASH);
psa_set_key_algorithm(&key_attr, PSA_ALG_ECDSA(PSA_ALG_SHA_256));

ret = mbedtls_ecp_point_write_binary(&pub_ctx->grp, &pub_ctx->Q,
MBEDTLS_ECP_PF_UNCOMPRESSED,
&key_len, key_buf, sizeof(key_buf));
if (ret != 0) {
return ret;
}

status = psa_import_key(&key_attr, key_buf, key_len, &key_id);
if (status != PSA_SUCCESS) {
ret = PSA_PK_TO_MBEDTLS_ERR(status);
return ret;
}

status = psa_verify_hash(key_id, PSA_ALG_ECDSA(PSA_ALG_SHA_256),
hash, sizeof(hash), sig, sig_len);
if (status != PSA_SUCCESS) {
ret = PSA_PK_TO_MBEDTLS_ERR(status);
status = psa_destroy_key(key_id);
return (status != PSA_SUCCESS) ? PSA_PK_TO_MBEDTLS_ERR(status) : ret;
}
status = psa_destroy_key(key_id);
if (status != PSA_SUCCESS) {
return PSA_PK_TO_MBEDTLS_ERR(status);
}

return 0;
}
#endif /* ECP_HAS_CHECK_PAIR */

static int eckey_check_pair(const void *pub, const void *prv,
int (*f_rng)(void *, unsigned char *, size_t),
void *p_rng)
Expand All @@ -1104,12 +1201,11 @@ static int eckey_check_pair(const void *pub, const void *prv,
(const mbedtls_ecp_keypair *) prv,
f_rng, p_rng);
#else /* ECP_FULL */
(void) pub;
(void) prv;
(void) f_rng;
(void) p_rng;
return MBEDTLS_ERR_PK_FEATURE_UNAVAILABLE;
return eckey_alt_check_pair((const mbedtls_ecp_keypair *) pub,
(const mbedtls_ecp_keypair *) prv,
f_rng, p_rng);
#endif /* ECP_FULL */
return MBEDTLS_ERR_PK_FEATURE_UNAVAILABLE;
}

static void *eckey_alloc_wrap(void)
Expand Down
2 changes: 2 additions & 0 deletions library/psa_util.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,8 @@ int psa_pk_status_to_mbedtls(psa_status_t status)
return MBEDTLS_ERR_PK_ALLOC_FAILED;
case PSA_ERROR_BAD_STATE:
return MBEDTLS_ERR_PK_BAD_INPUT_DATA;
case PSA_ERROR_INVALID_SIGNATURE:
return MBEDTLS_ERR_PK_BAD_INPUT_DATA;
case PSA_ERROR_DATA_CORRUPT:
case PSA_ERROR_DATA_INVALID:
case PSA_ERROR_STORAGE_FAILURE:
Expand Down
6 changes: 3 additions & 3 deletions tests/suites/test_suite_pk.data
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -525,12 +525,12 @@ depends_on:MBEDTLS_PKCS1_V21:MBEDTLS_MD_CAN_SHA256
pk_rsa_verify_ext_test_vec:"ae6e43dd387c25741e42fc3570cdfc52e4f51a2343294f3b677dfe01cd5339f6":MBEDTLS_MD_SHA256:1024:"00dd118a9f99bab068ca2aea3b6a6d5997ed4ec954e40deecea07da01eaae80ec2bb1340db8a128e891324a5c5f5fad8f590d7c8cacbc5fe931dafda1223735279461abaa0572b761631b3a8afe7389b088b63993a0a25ee45d21858bab9931aedd4589a631b37fcf714089f856549f359326dd1e0e86dde52ed66b4a90bda4095":"010001":"0d2bdb0456a3d651d5bd48a4204493898f72cf1aaddd71387cc058bc3f4c235ea6be4010fd61b28e1fbb275462b53775c04be9022d38b6a2e0387dddba86a3f8554d2858044a59fddbd594753fc056fe33c8daddb85dc70d164690b1182209ff84824e0be10e35c379f2f378bf176a9f7cb94d95e44d90276a298c8810f741c9":MBEDTLS_PK_RSASSA_PSS:MBEDTLS_MD_SHA256:94:129:MBEDTLS_ERR_RSA_VERIFY_FAILED

Check pair #1 (EC, OK)
depends_on:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_PEM_PARSE_C:ECP_HAS_CHECK_PAIR
depends_on:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_PEM_PARSE_C
mbedtls_pk_check_pair:"data_files/ec_256_pub.pem":"data_files/ec_256_prv.pem":0

Check pair #2 (EC, bad)
depends_on:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_PEM_PARSE_C:ECP_HAS_CHECK_PAIR
mbedtls_pk_check_pair:"data_files/ec_256_pub.pem":"data_files/server5.key":MBEDTLS_ERR_ECP_BAD_INPUT_DATA
depends_on:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_PEM_PARSE_C
mbedtls_pk_check_pair:"data_files/ec_256_pub.pem":"data_files/server5.key":MBEDTLS_ERR_PK_BAD_INPUT_DATA

Check pair #3 (RSA, OK)
depends_on:MBEDTLS_RSA_C:MBEDTLS_PKCS1_V15:MBEDTLS_PEM_PARSE_C
Expand Down