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.

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

Merged
merged 9 commits into from
May 9, 2024

Conversation

athoelke
Copy link
Contributor

@athoelke athoelke commented Feb 28, 2024

[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

  • Most of the extension forms a new sub-chapter within Cryptographic operations, but the key type and encoding information for SPAKE2+ is integrated into the respective sections.
  • Previous indirect references to the Crypto API, have been replaced with direct cross-references. (I hope I didn't miss any)
  • I have reworked parts of the Functionality chapter, as multi-part operations are now used with asymmetric cryptography as well.
  • Lists of 'key creation functions' all include the psa_pake_get_shared_key().

@athoelke athoelke added the Crypto API Issue or PR related to the Cryptography API label Feb 28, 2024
@athoelke athoelke added this to the Crypto API 1.3 milestone Feb 28, 2024
@athoelke athoelke self-assigned this Feb 28, 2024
Copy link
Contributor

@davidhorstmann-arm davidhorstmann-arm left a 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.

doc/crypto/overview/functionality.rst Outdated Show resolved Hide resolved
athoelke and others added 2 commits May 8, 2024 16:21
@athoelke
Copy link
Contributor Author

athoelke commented May 8, 2024

I think I also found a preexisting copypasta in the PAKE spec:

Well spotted - I have added a commit for this. This is a good PR to fix that error.

Copy link
Contributor

@davidhorstmann-arm davidhorstmann-arm left a 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
Copy link
Contributor

@davidhorstmann-arm davidhorstmann-arm May 8, 2024

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?

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'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.

Copy link
Contributor

@davidhorstmann-arm davidhorstmann-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, thanks!

@athoelke athoelke merged commit d6dc284 into ARM-software:main May 9, 2024
@athoelke athoelke deleted the crypto-1.3-integrate-pake branch May 9, 2024 10:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Crypto API Issue or PR related to the Cryptography API
Projects
Development

Successfully merging this pull request may close these issues.

Integrate the PAKE Extension into the Crypto API specification
2 participants