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

Jump to bottom

Add PSA interruptible key generation get num ops API #9665

Merged
merged 5 commits into from
Dec 2, 2024
Merged

Add PSA interruptible key generation get num ops API #9665

merged 5 commits into from
Dec 2, 2024

Conversation

waleed-elmelegy-arm
Copy link
Contributor

@waleed-elmelegy-arm waleed-elmelegy-arm commented Oct 2, 2024 •
edited
Loading

Description

fixes #9106
fixes #9644

Add PSA interruptible key generation get num ops API

PR checklist

Please remove the segment/s on either side of the | symbol as appropriate, and add any relevant link/s to the end of the line.
If the provided content is part of the present PR remove the # symbol.

  • changelog provided
  • development PR not required because:
  • framework PR not required
  • 3.6 PR not required because:
  • 2.28 PR not required because:
  • tests provided

@waleed-elmelegy-arm waleed-elmelegy-arm added DO-NOT-MERGE needs-ci Needs to pass CI tests component-psa PSA keystore/dispatch layer (storage, drivers, …) size-s Estimated task size: small (~2d) labels Oct 2, 2024
@waleed-elmelegy-arm waleed-elmelegy-arm force-pushed the add-iop-key-gen-get-num-ops branch from 6c807b7 to 7e7eda9 Compare October 2, 2024 17:01
@waleed-elmelegy-arm waleed-elmelegy-arm force-pushed the add-iop-key-gen-get-num-ops branch from 7e7eda9 to 05cb194 Compare October 22, 2024 10:27
@waleed-elmelegy-arm waleed-elmelegy-arm added needs-review Every commit must be reviewed by at least two team members, needs-preceding-pr Requires another PR to be merged first needs-reviewer This PR needs someone to pick it up for review and removed DO-NOT-MERGE needs-ci Needs to pass CI tests labels Oct 22, 2024
@gilles-peskine-arm gilles-peskine-arm added the priority-high High priority - will be reviewed soon label Oct 28, 2024
@waleed-elmelegy-arm waleed-elmelegy-arm force-pushed the add-iop-key-gen-get-num-ops branch 3 times, most recently from bd0df25 to fafae23 Compare November 18, 2024 17:43
@waleed-elmelegy-arm waleed-elmelegy-arm force-pushed the add-iop-key-gen-get-num-ops branch from fafae23 to d4276fb Compare November 21, 2024 18:49
@waleed-elmelegy-arm waleed-elmelegy-arm force-pushed the add-iop-key-gen-get-num-ops branch from d4276fb to f7ebd73 Compare November 27, 2024 16:57
@waleed-elmelegy-arm waleed-elmelegy-arm removed the needs-preceding-pr Requires another PR to be merged first label Nov 27, 2024
@waleed-elmelegy-arm waleed-elmelegy-arm added needs-ci Needs to pass CI tests and removed needs-ci Needs to pass CI tests labels Nov 27, 2024
@gilles-peskine-arm gilles-peskine-arm requested review from gilles-peskine-arm and removed request for paul-elliott-arm November 28, 2024 17:12
@gilles-peskine-arm gilles-peskine-arm removed the needs-reviewer This PR needs someone to pick it up for review label Nov 28, 2024
gilles-peskine-arm
gilles-peskine-arm requested changes Nov 29, 2024
View reviewed changes
Copy link
Contributor

@gilles-peskine-arm gilles-peskine-arm left a comment

Choose a reason for hiding this comment

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

Looks mostly good to me, but there's a design decision that I 100% agree with, but that isn't documented, and I'd like it to be documented.

tf-psa-crypto/tests/suites/test_suite_psa_crypto.function Outdated
if (status == PSA_SUCCESS || status == PSA_OPERATION_INCOMPLETE) {
num_ops = psa_generate_key_iop_get_num_ops(&operation);

TEST_ASSERT(num_ops > num_ops_prior);
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor: we have TEST_LE_U which is similar to TEST_EQUAL, but for <=, the advantage being that it displays the values on failure.

Suggested change
TEST_ASSERT(num_ops > num_ops_prior);
TEST_LE_U(num_ops_prior + 1, num_ops);

We could define TEST_GT_U and variants for better readability.

tf-psa-crypto/tests/suites/test_suite_psa_crypto.function Show resolved Hide resolved
@gilles-peskine-arm gilles-peskine-arm removed the needs-review Every commit must be reviewed by at least two team members, label Nov 29, 2024
gilles-peskine-arm
gilles-peskine-arm approved these changes Nov 29, 2024
View reviewed changes
Copy link
Contributor

@gilles-peskine-arm gilles-peskine-arm left a comment

Choose a reason for hiding this comment

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

Thanks for the updates, LGTM

@gilles-peskine-arm gilles-peskine-arm added needs-review Every commit must be reviewed by at least two team members, and removed needs-work labels Nov 29, 2024
yanesca
yanesca approved these changes Dec 2, 2024
View reviewed changes
Copy link
Contributor

@yanesca yanesca left a comment

Choose a reason for hiding this comment

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

LGTM

@yanesca yanesca added this pull request to the merge queue Dec 2, 2024
Merged via the queue into Mbed-TLS:development with commit 62e79dc Dec 2, 2024
6 checks passed
@waleed-elmelegy-arm waleed-elmelegy-arm mentioned this pull request Dec 2, 2024
Closed
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gilles-peskine-arm gilles-peskine-arm gilles-peskine-arm approved these changes

@yanesca yanesca yanesca approved these changes

Assignees
No one assigned
Labels
component-psa PSA keystore/dispatch layer (storage, drivers, …) needs-review Every commit must be reviewed by at least two team members, priority-high High priority - will be reviewed soon size-s Estimated task size: small (~2d)
Projects
Roadmap pull requests (new board)
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Implement interruptible ECC Key Generation (iop tests) Implement Interruptible ECC Keypair Generation
3 participants
@waleed-elmelegy-arm @yanesca @gilles-peskine-arm