-
Notifications
You must be signed in to change notification settings - Fork 28
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
Integrate the PAKE Extension into the Crypto API specification #177
Integrate the PAKE Extension into the Crypto API specification #177
Conversation
* PAKE uses multi-part functions, so the description of single-part functions and multi-part operations is no longer specific to Symmetric operations
cdeccb5
to
a9e947a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One nit / wording comment on the functionality section.
I think I also found a preexisting copypasta in the PAKE spec:
* Assign the result of the function `psa_pake_cipher_suite_init()` to the object, for example:
.. code-block:: xref
psa_pake_operation_t operation;
operation = psa_pake_operation_init();
I believe the psa_pake_cipher_suite_init()
should be psa_pake_operation_init()
.
If it's out of scope for this PR I can raise a followup.
Otherwise, the code motion looks good and psa_pake_get_shared_key()
seems to be listed everywhere that it should.
Co-authored-by: David Horstmann <[email protected]> Signed-off-by: Andrew Thoelke <[email protected]>
Well spotted - I have added a commit for this. This is a good PR to fix that error. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes look good!
I noticed one other small editorial nit when looking through the reworked functionality section, LGTM otherwise
@@ -168,7 +157,7 @@ use of a `multi-part operation <multi-part-operations>`. | |||
.. _multi-part-operations: | |||
|
|||
Multi-part operations | |||
^^^^^^^^^^^^^^^^^^^^^ | |||
~~~~~~~~~~~~~~~~~~~~~ | |||
|
|||
Multi-part operations are APIs which split a single cryptographic operation into | |||
a sequence of separate steps. This enables fine control over the configuration |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small wording / editorial nit further below, we say:
The update function can provide additional parameters, supply data for processing or generate outputs.
This implies that there is only one update function. In light of PAKE's many update functions (set_role()
, set_user()
, input()
, output()
etc) it might be good to clarify that there can be more than one. Would something like:
Each operation has one or more update functions, which are used to provide additional parameters, supply data for processing or generate outputs.
Work as a clearer wording?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've updated the text along these lines, but keeping consistency with the wording of the setup functions.
I also took the opportunity to add PAKE as an example of why multi-part operations are needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
[Updated: rebased the PR after publication of Crypto API 1.2.1, and updated the rendered PDF]
As an initial step for the Crypto API 1.3 release, integrate the API for PAKE operations into the primary specification, now that the PAKE Extension is Final.
Fixes #170.
A draft PDF render of the integrated specification: IHI0086-PSA_Certified_Crypto_API-1.3.0-integrate-pake-draft.2.pdf
Notes on the integration
psa_pake_get_shared_key()
.