Skip to content

Crypto API update - slots to handles #13

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.

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

Merged
merged 2 commits into from
Feb 1, 2019

Conversation

itayzafrir
Copy link
Contributor

@itayzafrir itayzafrir commented Jan 29, 2019

Update the examples to use the latest crypto API.
Depends on ARMmbed/mbed-os#9529.

PLEASE DO NOT MERGE until ARMmbed/mbed-os#9529 has been merged (or the crypto API has been updated in Mbed OS).

This PR also fixes a typo in the README.md file.

Note: The hash in file mbed-os.lib will need to be updated before merging.

@itayzafrir itayzafrir requested a review from Patater January 29, 2019 14:31
main.cpp Outdated
@@ -214,7 +214,9 @@ static psa_status_t cipher_example_encrypt_decrypt_aes_cbc_nopad_1_block(void)
ASSERT_STATUS(status, PSA_SUCCESS);

exit:
psa_destroy_key(key_slot_cipher);
if (key_handle != 0) {
psa_destroy_key(key_handle);
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it not safe to call psa_destroy_key(0)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

double checked, it is safe, rebased.

Copy link
Contributor

@Patater Patater left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@dgreen-arm dgreen-arm left a comment

Choose a reason for hiding this comment

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

LGTM. Builds and runs fine with the tip of ARMmbed/mbed-os#9529

psa_key_usage_t key_usage,
psa_algorithm_t alg)
{
psa_status_t status;
psa_key_policy_t policy;
psa_key_policy_t policy = psa_key_policy_init();

Choose a reason for hiding this comment

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

crypto code uses psa_key_policy_t policy = PSA_KEY_POLICY_INIT;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You can do it either way, it's the same

Choose a reason for hiding this comment

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

unless macro hides another stuff. and when the internals will change using macro will still allow abstraction...

Copy link
Contributor Author

@itayzafrir itayzafrir Jan 30, 2019

Choose a reason for hiding this comment

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

psa_key_policy_init() is an API function, it's not internal.

The macro can be only used as an initializer.

@0xc0170
Copy link
Contributor

0xc0170 commented Feb 1, 2019

I believe this can't be merged to master, as this example would fail for 5.11.x releases (this is on 5.11 branch https://github.com/ARMmbed/mbed-os/blob/mbed-os-5.11/tools/test/examples/examples.json#L292 ) As soon as this lands on master, it breaks 5.11.x release candidates

@itayzafrir
Copy link
Contributor Author

OK got it, I'll keep my fork as is for the time being.

@Patater Patater changed the base branch from master to development February 1, 2019 10:39
@Patater
Copy link
Contributor

Patater commented Feb 1, 2019

Merging to development instead of master, to allow us to build up, test, and integrate a branch suitable for 5.12 well in advance of the 5.12 code freeze, without breaking patch releases.

@Patater Patater merged commit 06ede8d into ARMmbed:development Feb 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants