SPAKE2+: Implement public key import and export #817

2026-06-21T04:54:22-05:00

sigvartmh commented

2026-06-21 04:54:22 -05:00

(Migrated from github.com)

Description

Adds support for the PSA_KEY_TYPE_SPAKE2P_PUBLIC_KEY key type in psa_import_key() / psa_export_key(), following the PSA Crypto API for SPAKE2+ encoding (the registration record w0 ‖ L).

Implements Mbed-TLS issue https://github.com/Mbed-TLS/mbedtls/issues/9342 Implement SPAKE2+ public key import and export.

PR checklist

changelog provided
framework PR provided Mbed-TLS/mbedtls-framework#323
TF-PSA-Crypto development PR provided HERE
TF-PSA-Crypto 1.1 PR not required because: new feature
mbedtls development PR not required because: crypto only
mbedtls 4.1 PR not required because: crypto only
mbedtls 3.6 PR not required because: crypto only

## Description Adds support for the PSA_KEY_TYPE_SPAKE2P_PUBLIC_KEY key type in psa_import_key() / psa_export_key(), following the PSA Crypto API for SPAKE2+ encoding (the registration record w0 ‖ L). Implements Mbed-TLS issue https://github.com/Mbed-TLS/mbedtls/issues/9342 Implement SPAKE2+ public key import and export. ## PR checklist - [x] **changelog** provided - [x] **framework PR** provided Mbed-TLS/mbedtls-framework#323 - [x] **TF-PSA-Crypto development PR** provided HERE - [ ] **TF-PSA-Crypto 1.1 PR** not required because: new feature - [ ] **mbedtls development PR** not required because: crypto only - [ ] **mbedtls 4.1 PR** not required because: crypto only - [ ] **mbedtls 3.6 PR** not required because: crypto only

yanesca commented

2026-07-20 06:16:44 -05:00

(Migrated from github.com)

@sigvartmh we have some CI failures, could you please fix them?

👍 1

silabs-johnh (Migrated from github.com) approved these changes

2026-07-27 03:59:30 -05:00

silabs-johnh (Migrated from github.com) requested changes

2026-07-27 04:35:24 -05:00

silabs-johnh commented

2026-07-28 04:52:37 -05:00

(Migrated from github.com)

@sigvartmh we have some CI failures, could you please fix them?

howdy @yanesca - I'm taking over for sigvart and trying to look at the CI failures but it seems I am not on "the allow list:"

Commit message: "Merge pull request #254 from Mbed-TLS/dev/gilles-peskine-arm/crypto-add-outcome-analysis"

git rev-list --no-walk ef487b44da4a42093213e3ff9d90176938e15019 # timeout=10
Trying to pass milestone 6
Trying to pass milestone 7
PR author not found on allowlist - not building

I was looking at CONTRIBUTING.md to try and see if there's a guide - maybe this is happening because I'm doing work on someone else's fork?

> @sigvartmh we have some CI failures, could you please fix them? howdy @yanesca - I'm taking over for sigvart and trying to look at the CI failures but it seems I am not on "the allow list:" > > Commit message: "Merge pull request #254 from Mbed-TLS/dev/gilles-peskine-arm/crypto-add-outcome-analysis" > > git rev-list --no-walk ef487b44da4a42093213e3ff9d90176938e15019 # timeout=10 > Trying to pass milestone 6 > Trying to pass milestone 7 > **PR author not found on allowlist - not building** I was looking at [CONTRIBUTING.md](https://github.com/Mbed-TLS/TF-PSA-Crypto/blob/a92cb6a97d84762d2f4355eae99af7b9ff5c6ae1/CONTRIBUTING.md) to try and see if there's a guide - maybe this is happening because I'm doing work on someone else's fork?

yanesca commented

2026-07-28 05:06:14 -05:00

(Migrated from github.com)

howdy @yanesca - I'm taking over for sigvart and trying to look at the CI failures but it seems I am not on "the allow list:"

Hi @silabs-johnh, the CI only runs automatically for maintainers and reviewers, for everyone else we need to kick it off manually. I have kicked it off now.

> howdy @yanesca - I'm taking over for sigvart and trying to look at the CI failures but it seems I am not on "the allow list:" Hi @silabs-johnh, the CI only runs automatically for maintainers and reviewers, for everyone else we need to kick it off manually. I have kicked it off now.

❤️ 1

silabs-johnh (Migrated from github.com) approved these changes

2026-08-19 03:46:59 -05:00

silabs-johnh commented

2026-08-24 03:30:33 -05:00

(Migrated from github.com)

seems like origin/development is missing PKCS7 datafiles? in framework@c550de0d99e7221d5fcafe40622cc1883ba8422e?
should I update the framework pointer in this PR, @yanesca ?

seems like origin/development is missing PKCS7 datafiles? in [framework](https://github.com/Mbed-TLS/mbedtls-framework/commit/c550de0d99e7221d5fcafe40622cc1883ba8422e)@c550de0d99e7221d5fcafe40622cc1883ba8422e? should I update the framework pointer in this PR, @yanesca ?

gilles-peskine-arm commented

2026-08-24 03:44:34 -05:00

(Migrated from github.com)

Yes, if this PR needs a framework update, please add a commit to update the framework pointer to the head of the framwwork PR.

👍 1

sigvartmh commented

2026-09-07 06:10:29 -05:00

(Migrated from github.com)

I see that this has lost a bit of love I'll try to get it up-to-date this week.

Thomasdjb commented

2026-09-08 11:08:44 -05:00

(Migrated from github.com)

hello @yanesca, would it be possible to trigger CI for this PR? ty

yanesca commented

2026-09-08 11:18:45 -05:00

(Migrated from github.com)

I have kicked the CI now, but it looks like that the DCO check has failed as well, could you please make sure that every commit has a Signed-off-by: line?

I have kicked the CI now, but it looks like that the DCO check has failed as well, could you please make sure that every commit has a `Signed-off-by:` line?

👍 1

Thomasdjb commented

2026-09-09 10:42:18 -05:00

(Migrated from github.com)

I have kicked the CI now, but it looks like that the DCO check has failed as well, could you please make sure that every commit has a Signed-off-by: line?

Sorry I didn't paid attention to this policy, it's good now. I tried to fix the existing CI errors, could you run again?

> I have kicked the CI now, but it looks like that the DCO check has failed as well, could you please make sure that every commit has a `Signed-off-by:` line? Sorry I didn't paid attention to this policy, it's good now. I tried to fix the existing CI errors, could you run again?

👍 1

Thomasdjb (Migrated from github.com) reviewed

2026-09-23 08:28:31 -05:00

core/psa_crypto.c

					
				@ -27,3 +27,3 @@

				#include "psa_crypto_rsa.h"

				#include "psa_crypto_ecp.h"

				#include "psa_crypto_spake2p.h"

				#include "psa_crypto_slot_management.h"

psa_crypto_ecp.h is already included line 23

`psa_crypto_ecp.h` is already included line 23

stevew817 (Migrated from github.com) reviewed

2026-09-24 09:32:28 -05:00

core/psa_crypto.c

Superfluous newline

drivers/builtin/src/psa_crypto_spake2p.c

					
				@ -0,0 +1,187 @@

				/*

If the intention is to have knowledge about all known curves, but reject the ones for which we don't have backend support (no PSA_WANT_ECC_*) early, then IMO it would be cleaner to fold this into supported_curves table as an extra struct member (and have the table be a known_curves table with a flag supported per entry). That way we don't risk the table above and the code below getting out of sync.

Alternatively, you could rely on the result of mbedtls_ecp_group_load to tell you whether or not the requested curve was supported, and do the algorithm vs key type check before doing the 'do we support this curve' check.

If the intention is to have knowledge about all known curves, but reject the ones for which we don't have backend support (no `PSA_WANT_ECC_*`) early, then IMO it would be cleaner to fold this into supported_curves table as an extra struct member (and have the table be a known_curves table with a flag `supported` per entry). That way we don't risk the table above and the code below getting out of sync. Alternatively, you could rely on the result of mbedtls_ecp_group_load to tell you whether or not the requested curve was supported, and do the algorithm vs key type check before doing the 'do we support this curve' check.

scripts/data_files/config-options-current.txt

There are some places in this PR where (currently dead) definitions are added in anticipation for adding support for SPAKE2P private keys - though support for those private keys is not supposed to be a part of this PR.

I don't mind getting the definitions in earlier, but if we're being strict they should only be introduced together with support & test of the private keys. @gilles-peskine-arm thoughts?

There are some places in this PR where (currently dead) definitions are added in anticipation for adding support for SPAKE2P private keys - though support for those private keys is not supposed to be a part of this PR. I don't mind getting the definitions in earlier, but if we're being strict they should only be introduced together with support & test of the private keys. @gilles-peskine-arm thoughts?

gilles-peskine-arm (Migrated from github.com) reviewed

2026-09-24 15:59:52 -05:00

include/psa/crypto_extra.h

					
				@ -641,4 +642,4 @@

				 *

				 * Not implemented yet.

				 */

				#define PSA_KEY_TYPE_SPAKE2P_KEY_PAIR(curve)            \

Preexisting, but becoming more important now that this is getting implemented:

tests/suites/test_suite_psa_crypto_metadata.data is supposed to validate the classification of algorithm and key type constructors. Please add the missing entries related to SPAKE2+. Note, this may require additional classification macros in the .function file.

There are some places in this PR where (currently dead) definitions are added in anticipation for adding support for SPAKE2P private keys - though support for those private keys is not supposed to be a part of this PR.

I don't mind getting the definitions in earlier, but if we're being strict they should only be introduced together with support & test of the private keys. @gilles-peskine-arm thoughts?

I don't mind adding declarations before they're fully supported. It's happened before, and it isn't a big deal. For example, we used to declare identifiers for finite-field DSA, which never made it into the API.

However, you may run into problems with partial support, due to automatically generated test cases. When framework/scripts/generate_psa_tests.py sees new identifiers, it might generate test cases that validate support that isn't ready yet. Due to the repository split, it's now difficult to add exceptions in te test generator. If the problematic test cases are never executed because they depend on a macro that is never enabled, you can declare them as exceptions for the coverage analysis check in tests/scripts/analyze_outcomes.py.

We are missing a lot of support for systematically generated tests involving PAKE keys and algorithms, so it's possible that this just won't be a problem for PAKE.

Preexisting, but becoming more important now that this is getting implemented: `tests/suites/test_suite_psa_crypto_metadata.data` is supposed to validate the classification of algorithm and key type constructors. Please add the missing entries related to SPAKE2+. Note, this may require additional classification macros in the `.function` file. ---- > There are some places in this PR where (currently dead) definitions are added in anticipation for adding support for SPAKE2P private keys - though support for those private keys is not supposed to be a part of this PR. > > I don't mind getting the definitions in earlier, but if we're being strict they should only be introduced together with support & test of the private keys. @gilles-peskine-arm thoughts? I don't mind adding declarations before they're fully supported. It's happened before, and it isn't a big deal. For example, we used to declare identifiers for finite-field DSA, which never made it into the API. However, you may run into problems with partial support, due to automatically generated test cases. When `framework/scripts/generate_psa_tests.py` sees new identifiers, it might generate test cases that validate support that isn't ready yet. Due to the repository split, it's now difficult to add exceptions in te test generator. If the problematic test cases are never executed because they depend on a macro that is never enabled, you can declare them as exceptions for the coverage analysis check in `tests/scripts/analyze_outcomes.py`. We are missing a lot of support for systematically generated tests involving PAKE keys and algorithms, so it's possible that this just won't be a problem for PAKE.

stevew817 (Migrated from github.com) reviewed

2026-09-25 07:18:05 -05:00

scripts/data_files/config-options-current.txt

Comment resolved by Gilles' answer below.

Thomasdjb commented

2026-09-25 08:28:26 -05:00

(Migrated from github.com)

Needed to force push to fix DCO... sorry for reviewers

mpg commented

2026-09-28 02:22:58 -05:00

(Migrated from github.com)

changelog provided | not required because:

It makes no sense for the checkbox to be checked while the template is still intact with its two branches. Please either remove "provided" and fill in a reason why it's not required, or remove "not required because" and make sure the PR actually includes a ChangeLog entry.

Similar notes apply to the other lines.

> * **changelog** provided | not required because: It makes no sense for the checkbox to be checked while the template is still intact with its two branches. Please either remove "provided" and fill in a reason why it's not required, or remove "not required because" and make sure the PR actually includes a ChangeLog entry. Similar notes apply to the other lines.

Thomasdjb commented

2026-09-28 03:06:18 -05:00

(Migrated from github.com)

changelog provided | not required because:

It makes no sense for the checkbox to be checked while the template is still intact with its two branches. Please either remove "provided" and fill in a reason why it's not required, or remove "not required because" and make sure the PR actually includes a ChangeLog entry.

Similar notes apply to the other lines.

Unfortunately, I cannot modify the description of this PR as I'm not the original author.
Is this changelog entry not correct for this PR? what should be added?

> > * **changelog** provided | not required because: > > It makes no sense for the checkbox to be checked while the template is still intact with its two branches. Please either remove "provided" and fill in a reason why it's not required, or remove "not required because" and make sure the PR actually includes a ChangeLog entry. > > Similar notes apply to the other lines. Unfortunately, I cannot modify the description of this PR as I'm not the original author. Is this [changelog entry](https://github.com/Mbed-TLS/TF-PSA-Crypto/pull/817/changes#diff-dee9f7d3c37d79051a46207bd9026a0f96b95c454d82f696159549fece7ff21d) not correct for this PR? what should be added?

mpg commented

2026-09-28 03:46:22 -05:00

(Migrated from github.com)

Unfortunately, I cannot modify the description of this PR as I'm not the original author.

Ok, I edited it then.

Is this changelog entry not correct for this PR? what should be added?

A changelog entry is present, so I checked "provided". I'll leave the actual contents to the reviewers. (I was just passing by while triaging the associated framework PR, and noticed the inconsistencies in the PR checklist.)

> Unfortunately, I cannot modify the description of this PR as I'm not the original author. Ok, I edited it then. > Is this [changelog entry](https://github.com/Mbed-TLS/TF-PSA-Crypto/pull/817/changes#diff-dee9f7d3c37d79051a46207bd9026a0f96b95c454d82f696159549fece7ff21d) not correct for this PR? what should be added? A changelog entry is present, so I checked "provided". I'll leave the actual contents to the reviewers. (I was just passing by while triaging the associated framework PR, and noticed the inconsistencies in the PR checklist.)

👍 1

This pull request is broken due to missing fork information.

View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.

git fetch -u origin 'sigvartmh/pr/spake2p-9342-public-key-import-export:sigvartmh/pr/spake2p-9342-public-key-import-export'

git switch 'sigvartmh/pr/spake2p-9342-public-key-import-export'

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.

git switch 'development'

git merge --no-ff 'sigvartmh/pr/spake2p-9342-public-key-import-export'

git switch 'sigvartmh/pr/spake2p-9342-public-key-import-export'

git rebase 'development'

git switch 'development'

git merge --ff-only 'sigvartmh/pr/spake2p-9342-public-key-import-export'

git switch 'sigvartmh/pr/spake2p-9342-public-key-import-export'

git rebase 'development'

git switch 'development'

git merge --no-ff 'sigvartmh/pr/spake2p-9342-public-key-import-export'

git switch 'development'

git merge --squash 'sigvartmh/pr/spake2p-9342-public-key-import-export'

git switch 'development'

git merge --ff-only 'sigvartmh/pr/spake2p-9342-public-key-import-export'

git switch 'development'

git merge 'sigvartmh/pr/spake2p-9342-public-key-import-export'

git push origin 'development'

Sign in to join this conversation.

No reviewers

silabs-johnh

No milestone

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

gh-Mbed-TLS/TF-PSA-Crypto!817

				`@ -0,0 +1,187 @@`
				`/*`

Rows
Columns