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

sys/psa_crypto: Fix macro for public key max size and SE example #19995

Merged
merged 2 commits into from
Oct 20, 2023
Merged

sys/psa_crypto: Fix macro for public key max size and SE example #19995

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1304202
sys/psa_crypto: Fix conditions for public key max export size
Einhornhool Oct 19, 2023
b2e1c69
examples/psa_crypto: Update example to work with SEs
Einhornhool Oct 19, 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
11 changes: 10 additions & 1 deletion examples/psa_crypto/example_ecdsa_p256.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,10 +81,13 @@ psa_status_t example_ecdsa_p256(void)
}

#ifdef SECURE_ELEMENT
/* Currently there is no support for message signature and verification on secure elements */
psa_set_key_lifetime(&pubkey_attr, lifetime);
psa_set_key_usage_flags(&pubkey_attr, PSA_KEY_USAGE_VERIFY_HASH);
#else
psa_set_key_usage_flags(&pubkey_attr, PSA_KEY_USAGE_VERIFY_MESSAGE);
#endif
psa_set_key_algorithm(&pubkey_attr, ECC_ALG);
psa_set_key_usage_flags(&pubkey_attr, PSA_KEY_USAGE_VERIFY_MESSAGE);
psa_set_key_bits(&pubkey_attr, PSA_BYTES_TO_BITS(pubkey_length));
psa_set_key_type(&pubkey_attr, PSA_KEY_TYPE_ECC_PUBLIC_KEY(PSA_ECC_FAMILY_SECP_R1));

Expand All @@ -99,6 +102,12 @@ psa_status_t example_ecdsa_p256(void)
return status;
}

#ifdef SECURE_ELEMENT
/* Currently there is only support for hash signature and verification on secure elements,
so we can't verify the message, but only the hash */
return psa_verify_hash(pubkey_id, ECC_ALG, hash, sizeof(hash), signature, sig_length);
Copy link
Contributor

Choose a reason for hiding this comment

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

Would be great to have a comment here how this differs from psa_verify_message()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added a comment on why it's necessary to call psa_verify_hash.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah so the example really does two different things depending on whether SECURE_ELEMENT is set.

Is there no software fallback for message verification?

Copy link
Contributor Author

@Einhornhool Einhornhool Oct 19, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

There is, but since this is supposed to be the Secure Element example, I would like to use the available SE functions.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm just surprised because psa_verify_message() has no reference to a secure element, so I would expect it would work either way.

Copy link
Contributor Author

@Einhornhool Einhornhool Oct 19, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Aaah, I didn't understand your question correctly.
In this case there are two issues:

  1. In this build there is no fallback. Secure Element modules are selected by different symbols than regular PSA Crypto modules. This configuration only selects psa_secure_element_ateccx08a_ecc_p256. To have a fallback in this case, I would have to explicitly select psa_asymmetric_ecc_p256r1.
    This could happen automatically, but I deliberately chose to build SEs without fallbacks, since they are also meant to be a memory efficient alternative to software implementations, especially on platforms that don't have enough memory for them. Also, if keys are stored on the SE, a software fallback couldn't do anything anyways.
  2. In this case the key for verification is stored on the secure element, which means that a software backend couldn't access it. I could change the example to store the public key in RAM and make it usable for a different backend. But then it's not a "secure element example" anymore, but a "secure element signature with software verification example". It's not a big problem, but a different use case, I guess.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think a simple fallback for secure elements or other configurations where signing/verifying messages is not directly supported could be the following: internally calculate the hash using psa_hash_compute() (on the secure element, or in software if not supported) and then call psa_verify_hash() on the hash.

That's what I did for ed25519 on nRF52840dk iirc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can do that, but I'd rather submit a separate PR for it, since it would be a new feature =)

#endif

/* verify on original message with internal hashing operation */
return psa_verify_message(pubkey_id, ECC_ALG, msg, sizeof(msg), signature, sig_length);
}
Expand Down
5 changes: 5 additions & 0 deletions examples/psa_crypto/main.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,10 @@
extern psa_status_t example_cipher_aes_128(void);
extern psa_status_t example_hmac_sha256(void);
extern psa_status_t example_ecdsa_p256(void);

#ifndef SECURE_ELEMENT
extern psa_status_t example_eddsa(void);
#endif

#ifdef MULTIPLE_SE
extern psa_status_t example_cipher_aes_128_sec_se(void);
Expand Down Expand Up @@ -61,12 +64,14 @@ int main(void)
printf("ECDSA failed: %s\n", psa_status_to_humanly_readable(status));
}

#ifndef SECURE_ELEMENT
start = ztimer_now(ZTIMER_USEC);
status = example_eddsa();
printf("EdDSA took %d us\n", (int)(ztimer_now(ZTIMER_USEC) - start));
if (status != PSA_SUCCESS) {
printf("EdDSA failed: %s\n", psa_status_to_humanly_readable(status));
}
#endif

#ifdef MULTIPLE_SE
puts("Running Examples with secondary SE:");
Expand Down
4 changes: 3 additions & 1 deletion sys/include/psa_crypto/psa/crypto_sizes.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -965,7 +965,9 @@ extern "C" {
*
* See also @ref PSA_EXPORT_PUBLIC_KEY_OUTPUT_SIZE(@p key_type, @p key_bits).
*/
#if IS_USED(MODULE_PSA_ASYMMETRIC_ECC_P256R1) || IS_USED(MODULE_PSA_ASYMMETRIC_ECC_P192R1)
#if (IS_USED(MODULE_PSA_ASYMMETRIC_ECC_P256R1) || \
IS_USED(MODULE_PSA_ASYMMETRIC_ECC_P192R1) || \
IS_USED(MODULE_PSA_SECURE_ELEMENT_ATECCX08A_ECC_P256))
#define PSA_EXPORT_PUBLIC_KEY_MAX_SIZE \
(PSA_KEY_EXPORT_ECC_PUBLIC_KEY_MAX_SIZE(PSA_ECC_FAMILY_SECT_R1, PSA_MAX_PRIV_KEY_SIZE))
#else
Expand Down