Skip to content

psa: Check generator validity before read #57

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 1 commit into from
Feb 18, 2019

Conversation

Patater
Copy link
Contributor

@Patater Patater commented Feb 15, 2019

Check generator validity (i.e. that alg has been initialized) before
allowing reads from the generator or allowing reads of the generator's
capacity.

This aligns our implementation with the documented error code behavior
in our crypto.h and the PSA Crypto API.

Fixes https://github.com/ARMmbed/mbedtls-psa/issues/183

Copy link
Contributor

@NirSonnenschein NirSonnenschein left a comment

Choose a reason for hiding this comment

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

The changes look good to me.
Besides the tests fixed here are there any other instances that could affected by the change in behavior? (e.g. example repo, IPC parameter checks in IPC)?

@Patater Patater added bug Something isn't working needs: review The pull request is ready for review. This generally means that it has no known issues. compliance Discrepancy between the library and the PSA Crypto API specification labels Feb 15, 2019
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.

A single typo, otherwise looks good to me

@@ -3911,7 +3922,13 @@ psa_status_t psa_generator_read( psa_crypto_generator_t *generator,
exit:
if( status != PSA_SUCCESS )
{
/* Preserve the algorithm upon errors, but clear all sensitive state.
* This allows us to differentiate between exhaused generators and
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo: exhaused -> exhausted

@Patater
Copy link
Contributor Author

Patater commented Feb 18, 2019

Rebasing to address review comments and to resolve merge conflicts.

Check generator validity (i.e. that alg has been initialized) before
allowing reads from the generator or allowing reads of the generator's
capacity.

This aligns our implementation with the documented error code behavior
in our crypto.h and the PSA Crypto API.
@Patater
Copy link
Contributor Author

Patater commented Feb 18, 2019

Rebased. Unfortunately, the base change had to happen in order to get the merge conflicts resolved, and we've merged the latest Mbed TLS code in (so the base is quite different).

Probably easiest to diff between diffs on this one instead of using GitHub's "force-pushed" diff.

Old branch up at https://github.com/Patater/mbed-crypto/tree/check-generator-validity-1

@Patater
Copy link
Contributor Author

Patater commented Feb 18, 2019

CI failure is the FreeBSD timing test, a known flaky test.

@Patater Patater merged commit 065c426 into ARMmbed:development Feb 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working compliance Discrepancy between the library and the PSA Crypto API specification needs: review The pull request is ready for review. This generally means that it has no known issues.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants