docs: add WPA3-SAE (PSA Crypto API 1.4) design proposal #837
Open
krish2718 wants to merge 1 commit from
krish2718/wpa3-sae-design-note into development
pull from: krish2718/wpa3-sae-design-note
merge into: gh-Mbed-TLS:development
gh-Mbed-TLS:development
gh-Mbed-TLS:tf-psa-crypto-1.1
gh-Mbed-TLS:archive/20261005T153337Z-force/pull/927
gh-Mbed-TLS:archive/20261001T141055Z-force/pull/927
gh-Mbed-TLS:archive/20261005T153337Z-force/pull/931
gh-Mbed-TLS:archive/20261001T083616Z-force/pull/927
gh-Mbed-TLS:archive/20260930T232327Z-force/pull/927
gh-Mbed-TLS:archive/20260930T154618Z-force/pull/927
gh-Mbed-TLS:archive/20260929T231919Z-rescue/rohankl-arm/mldsa-pubkey-899
gh-Mbed-TLS:archive/20260929T231919Z-rescue/sigvartmh/pr/spake2p-9342-public-key-import-export
gh-Mbed-TLS:archive/20260929T231919Z-rescue/ManoharPaturi/fix-pem-ct-padding
gh-Mbed-TLS:archive/20260929T231919Z-rescue/ManoharPaturi/fix-asn1write-null-guard
gh-Mbed-TLS:archive/20260929T231919Z-rescue/ManoharPaturi/fix-pbes2-params-validation
gh-Mbed-TLS:archive/20260929T231919Z-rescue/valeriosetti/blake2
gh-Mbed-TLS:archive/20260929T231919Z-rescue/jfdev001/v1.1.1_bump-cmake-minimum-version
gh-Mbed-TLS:archive/20260929T231919Z-rescue/jfdev001/bump-cmake-minimum-version
gh-Mbed-TLS:archive/20260929T231919Z-rescue/ronald-cron-arm/sm3-base
gh-Mbed-TLS:archive/20260929T231919Z-rescue/valeriosetti/blake2-hmac
gh-Mbed-TLS:archive/20260929T231919Z-rescue/v1rtu3x/enforce-hash-length-sign-verify
gh-Mbed-TLS:archive/20260929T231919Z-rescue/bjwtaylor/file-backed-keystore-doc
gh-Mbed-TLS:archive/20260929T231919Z-rescue/sigvartmh/pr/spake2p-9344-set-context
gh-Mbed-TLS:archive/20260929T231919Z-rescue/sigvartmh/pr/spake2p-9343-key-pair-import-export-depends-on-9342
gh-Mbed-TLS:archive/20260929T231919Z-rescue/romart/development
gh-Mbed-TLS:archive/20260929T231919Z-rescue/sweetlilmre/constant-time-embedded-perf
gh-Mbed-TLS:archive/20260929T231919Z-rescue/JulioLoayzaM/add-wycheproof-rsa-pss
gh-Mbed-TLS:archive/20260929T231919Z-rescue/JulioLoayzaM/add-wycheproof-ecdsa
gh-Mbed-TLS:archive/20260929T231919Z-rescue/JulioLoayzaM/add-wycheproof-tests
gh-Mbed-TLS:archive/20260929T231919Z-rescue/oubaid-a/dev_psa_driver_wrapper
gh-Mbed-TLS:archive/20260929T231919Z-rescue/gilles-peskine-arm/keystoreless-prototype-1
gh-Mbed-TLS:archive/20260929T231919Z-rescue/valeriosetti/issue9580
gh-Mbed-TLS:archive/20260929T231919Z-rescue/LeonieReichertABB/hpke-functionality
gh-Mbed-TLS:archive/20260929T231919Z-rescue/gilles-peskine-arm/psa-hash-no-buffer-copy-1.1
gh-Mbed-TLS:archive/20260929T231919Z-rescue/vanhauser-thc/development
gh-Mbed-TLS:archive/20260929T231919Z-rescue/tpambor/key-wrap
gh-Mbed-TLS:archive/20260929T231919Z-rescue/ruiliio/psa_xts
gh-Mbed-TLS:archive/20261011T063515Z-force/pull/538
gh-Mbed-TLS:archive/20260929T231919Z-rescue/gilles-peskine-arm/fix-broken-links-1.1
gh-Mbed-TLS:archive/20260929T231919Z-rescue/valeriosetti/issue669
gh-Mbed-TLS:archive/20260929T231919Z-rescue/gilles-peskine-arm/prepare-release-test
gh-Mbed-TLS:archive/20260929T231919Z-rescue/krish2718/wpa3-sae-design-note
gh-Mbed-TLS:archive/20260929T231919Z-rescue/nzzlinh/fix_iar_endianness_1.1
gh-Mbed-TLS:archive/20260929T231919Z-rescue/nzzlinh/fix-iar-endianness
gh-Mbed-TLS:archive/20260929T231919Z-rescue/abtom87/fix-pbkdf2-iter-count
gh-Mbed-TLS:archive/20260929T231919Z-rescue/minosgalanakis/tools/prepare_release_lite
gh-Mbed-TLS:archive/20260929T231919Z-rescue/Harshal5/fix/aes-xts-in-place-cts
gh-Mbed-TLS:archive/20260929T231919Z-rescue/sigvartmh/pr/spake2p-9347-prover-depends-on-9342-9343-9344
gh-Mbed-TLS:archive/20260929T231919Z-rescue/crabel99/async-hardware-support-c35df962
gh-Mbed-TLS:tf-psa-crypto-1.2.0_mergeback
gh-Mbed-TLS:tf-psa-crypto-1.1.1_mergeback
gh-Mbed-TLS:archive/20260929T231919Z-rescue/asmellby/feature/dispatch-library
gh-Mbed-TLS:tf-psa-crypto-release-sync
gh-Mbed-TLS:archive/20260929T231919Z-rescue/ChakshuGupta13/fix/ecdsa-zeroize-stack-key
gh-Mbed-TLS:archive/20260929T231919Z-rescue/gilles-peskine-arm/security-md-option-names
gh-Mbed-TLS:archive/20260929T231919Z-rescue/parmi93/patch-2
gh-Mbed-TLS:archive/20260929T231919Z-rescue/bjwtaylor/remove-mldsa-sign-from-build
gh-Mbed-TLS:archive/20260929T231919Z-rescue/bjwtaylor/tf-m-build-issues
gh-Mbed-TLS:archive/20260929T231919Z-rescue/bjwtaylor/test-context-population
gh-Mbed-TLS:archive/20260929T231919Z-rescue/gilles-peskine-arm/iar-1.1.0
gh-Mbed-TLS:archive/20260929T231919Z-rescue/gilles-peskine-arm/fork-helper-unit-tests-create-crypto
gh-Mbed-TLS:archive/20260929T231919Z-rescue/bjwtaylor/redundant-declarations
gh-Mbed-TLS:archive/20260929T231919Z-rescue/hasheddan/docs/fix-broken-links
gh-Mbed-TLS:main
gh-Mbed-TLS:archive/20260929T231919Z-rescue/bjwtaylor/reintroduce-dh
gh-Mbed-TLS:archive/20260929T231919Z-rescue/bjwtaylor/cipher-update-incorrect-error
gh-Mbed-TLS:archive/20260929T231919Z-rescue/Marandil/fix/mbedtls-4314-parse-explicit-curve
gh-Mbed-TLS:archive/20260929T231919Z-rescue/schneider-electric/dev/plaj-se/parse-rsassa-pss
gh-Mbed-TLS:archive/20260929T231919Z-rescue/minosgalanakis/bugfix/program-fuzz-mscv
gh-Mbed-TLS:archive/20260929T231919Z-rescue/daverodgman/aesgcm
gh-Mbed-TLS:archive/20260929T231919Z-rescue/gilles-peskine-arm/sha3-coverity-20260224
gh-Mbed-TLS:archive/20260929T231919Z-rescue/bensze01/undef
gh-Mbed-TLS:archive/20260929T231919Z-rescue/valeriosetti/issue523
gh-Mbed-TLS:archive/20260929T231919Z-rescue/dannytsen/ppc-dev
gh-Mbed-TLS:archive/20260929T231919Z-rescue/daverodgman/aes12
gh-Mbed-TLS:archive/20260929T231919Z-rescue/gilles-peskine-arm/ascon-prototype
gh-Mbed-TLS:archive/20260929T231919Z-rescue/daverodgman/chacha10
gh-Mbed-TLS:archive/20260929T231919Z-rescue/kusumitg/implement_psa_pake_set_context_for_spake2p
gh-Mbed-TLS:archive/20260929T231919Z-rescue/davidhorstmann-arm/add-code-size-target
gh-Mbed-TLS:archive/20260929T231919Z-rescue/bjwtaylor/simplify-sign_func
gh-Mbed-TLS:archive/20260929T231919Z-rescue/mfil/feature/aes_ni_parallel
gh-Mbed-TLS:archive/20260929T231919Z-rescue/ivv19041994/patch-1
gh-Mbed-TLS:archive/20260929T231919Z-rescue/gilles-peskine-arm/psa-verify-policy-relax
gh-Mbed-TLS:public_2/main
gh-Mbed-TLS:archive/20260929T231919Z-rescue/mpg/pk-sign-proto
gh-Mbed-TLS:release/tf-psa-crypto1.0.0-documentation
gh-Mbed-TLS:archive/20260929T231919Z-rescue/gilles-peskine-arm/0e-plans-july-updates
gh-Mbed-TLS:archive/20260929T231919Z-rescue/davidhorstmann-arm/guard-context-types-in-client-only
gh-Mbed-TLS:archive/20260929T231919Z-rescue/gilles-peskine-arm/psa-driver-interface-spec-remove
gh-Mbed-TLS:archive/20260929T231919Z-rescue/gilles-peskine-arm/ecp-self-test-race-crypto
gh-Mbed-TLS:archive/20260929T231919Z-rescue/KiruthikPrakash/feature/9342_add-support-for-spake2p-public-key-import-export
gh-Mbed-TLS:archive/20260929T231919Z-rescue/ruiliio/chacha20_16
gh-Mbed-TLS:archive/20260929T231919Z-rescue/irwir/winsock2
gh-Mbed-TLS:archive/20260929T231919Z-rescue/aphroteus/development
gh-Mbed-TLS:archive/20260929T231919Z-rescue/amjoul01/amjoul01/psa-mac-out-of-bound-fix
gh-Mbed-TLS:archive/20260929T231919Z-rescue/gilles-peskine-arm/everest-submodule-crypto
gh-Mbed-TLS:archive/20260929T231919Z-rescue/yamt/xtensa-ccount
gh-Mbed-TLS:archive/20260929T231919Z-rescue/gilles-peskine-arm/all.sh-generate-only-before-checks-crypto
gh-Mbed-TLS:archive/20260929T231919Z-rescue/Harshal5/feat/builtin_cmac_driver_use_psa_crypto_interface
gh-Mbed-TLS:archive/20260929T231919Z-rescue/gilles-peskine-arm/test-data_t-empty_is_empty-crypto
gh-Mbed-TLS:archive/20260929T231919Z-rescue/jean-baptisteboric-eaton/oids-pkcs7-authenticated-attributes
gh-Mbed-TLS:dev/bensze01/public-only-commit
gh-Mbed-TLS:archive/20260929T231919Z-rescue/waleed-elmelegy-arm/psa-key-wrap-aes-kw
gh-Mbed-TLS:archive/20260929T231919Z-rescue/tfloch/psa_add_xts_algorithm
gh-Mbed-TLS:archive/20260929T231919Z-rescue/gilles-peskine-arm/msancf-clang16-crypto
gh-Mbed-TLS:archive/20260929T231919Z-rescue/DemiMarie/document-unexpected-tags
gh-Mbed-TLS:archive/20260929T231919Z-rescue/DemiMarie/strict-len
gh-Mbed-TLS:archive/20260929T231919Z-rescue/valeriosetti/issue9255-tfpsacrypto
gh-Mbed-TLS:archive/20260929T231919Z-rescue/daantimmer/use-compiler-frontend
gh-Mbed-TLS:archive/20260929T231919Z-rescue/benmcollins/enable-rsassa-pss-pkparse
gh-Mbed-TLS:dev/paul-elliott-arm/add_iop_key_agreement_negative_tests
gh-Mbed-TLS:add-iop-export-pub-key-get-num-ops
gh-Mbed-TLS:dev/rc/new-2-d635ea1
gh-Mbed-TLS:dev/bensze01/jenkins
No reviewers
Labels
Clear labels
api-break
This issue/PR breaks the API and must wait for a new major version
approved
Design and code approved - may be waiting for CI or backports
bug
Something isn't working
component-docs
Docs / website issues filed here for tracking
component-platform
Portability layer and build scripts
component-test
Test framework and CI scripts
dependencies
Pull requests that update a dependency file
deprecation
needs deprecation announcement
DO-NOT-MERGE
enhancement
New feature or request
good first issue
Good for newcomers
help wanted
This issue is not being actively worked on, but PRs welcome.
needs-adoption
stalled PR that someone should pick up and complete
needs-backports
Backports are missing or are pending review and approval.
needs-ci
Needs to pass CI tests
needs-design-approval
Needs design discussion / approval
needs-info
An issue or PR which needs further info from the reporter / author
needs-preceding-pr
Requires another PR to be merged first
needs-review
Every commit must be reviewed by at least two team members
needs-reviewer
This PR needs someone to pick it up for review
needs-work
priority-high
High priority - will be reviewed soon
priority-low
Low priority - this may not receive review soon
priority-medium
Medium priority - this can be reviewed as time permits
priority-scheduled
This PR is big - it will require time to be scheduled for review
priority-very-high
Highest priority - prioritise this over other review work
resolved-after-1.1
The issue is resolved in development, but still present in 1.1 LTS.
single-reviewer
This PR qualifies for having only one reviewer
size-l
Estimated task size: large (2w+)
size-m
Estimated task size: medium (~1w)
size-optimisation
size-s
Estimated task size: small (~2d)
size-xs
Estimated task size: extra small (a few hours at most)
No labels
api-break
approved
bug
component-docs
component-platform
component-test
dependencies
deprecation
DO-NOT-MERGE
enhancement
good first issue
help wanted
needs-adoption
needs-backports
needs-ci
needs-design-approval
needs-info
needs-preceding-pr
needs-review
needs-reviewer
needs-work
priority-high
priority-low
priority-medium
priority-scheduled
priority-very-high
resolved-after-1.1
single-reviewer
size-l
size-m
size-optimisation
size-s
size-xs
Milestone
Clear milestone
No items
No milestone
Assignees
Clear assignees
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!837
Loading…
Reference in a new issue
No description provided.
Delete branch "krish2718/wpa3-sae-design-note"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
This adds a Phase 0 design proposal (
docs/proposed/wpa3-sae.md) forimplementing the WPA3-SAE PAKE from the PSA Certified Crypto API 1.4
(§10.13.12–13) in the built-in driver.
It is a design note for discussion — no implementation code — intended
to reach agreement on scope, API surface, new primitives, constant-time
strategy, and PR phasing before any code is written. Relates to #709.
What it covers
PSA_DH_FAMILY_RFC3526; theFIXEDandGDHalgorithm variants and theWPA3_SAE_H2EKDF. Protocol-level concerns (anti-clogging, groupnegotiation, SAE-PK, mesh) are out of scope — they belong in the Wi-Fi stack.
modular square root, and the SAE engine.
motivating threat).
Open questions for maintainers (see §9 of the note)
PSA_DH_FAMILY_RFC3526(IANA 15–18) for SAE FFDH tokens; confirm this is the intended set.
PWE; H2E supersedes it) and return
PSA_ERROR_NOT_SUPPORTED.(SPAKE2+ is currently API-encoding-only).
Related
GlobalPlatform/psa-api#372.
Thank you for providing a detailed design plan.
I've reviewed it as someone who is familiar with PSA APIs in general and with TF-PSA-Crypto as a product, but not with the PAKE API and not with the WPA3 algorithm family. In particular, I have not confronted the proposal with the WPA3 and related specifications.
@ -0,0 +1,441 @@WPA3-SAE (PSA Certified Crypto API 1.4) — design noteAlso a new flag
ALG_IS_WPA3_SAEintests/suites/test_suite_psa_crypto_metadata.functionAdding new files in an existing directory should not require any changes to
**/CMakeLists.txt.Note: changelog entries only for things visible in the API, not for internal elements that are not reachable from the API yet.
Specifically uncrustify 0.75.1. But it's almost K&R2 so the details shouldn't be a problem.
The framework PR will be merged first, and needs to work both with the current code and with the new code (but this shouldn't be an issue when adding a new feature). It only needs to be coordinated in the sense that we like the CI on the using PR to pass before we merge the framework PR, because that gives assurance that the framework PR isn't missing something.
There is already a square root implementation in
mbedtls_ecp_sw_derive_y()inecp.c. Prefer refactoring this to make it usable for SAE.Please avoid landing test vectors before the corresponding algorithm. In particular, merging tests that are not executed is technically possible, but it's a hassle.
As far as I can tell from reading this document, but without being familiar with the mechanisms that are implemented, it seems to me that
sae.c(+ unit tests) andhash_to_curve.c(+ unit tests) should be separate PR. I'm not sure about the other bits.Constant-time annotations don't involve much code. They should preferably be present from the start.
Fuzzing, on the other hand, can be added as a separate later PR.
Documentation should be provided as early as possible, not as an afterthought. Writing good documentation often helps remove ambiguities from the specification and helps with the test plan.
PSA_DH_FAMILY_RFC3526should be added in a PR that doesn't add H2E/SAE code.@ -0,0 +144,4 @@- `drivers/builtin/include/mbedtls/private/crypto_adjust_config_enable_builtins.h`— derive `MBEDTLS_PSA_BUILTIN_ALG_WPA3_SAE_*` / `MBEDTLS_PSA_BUILTIN_PAKE`,pull in ECP/bignum (and, P3, FFDH/RFC3526) modules.- `core/check_crypto_config.h` — dependency `#error` checks.Also
scripts/data_files/config-options-current.txt@ -0,0 +181,4 @@**Framework submodule (separate repo)**- `framework/scripts/mbedtls_framework/crypto_knowledge.py` — register the newAlso probably a block in
framework/tests/src/psa_exercise_key.cfor the new KDF.@ -0,0 +294,4 @@and would put SAE on the critical path of a bignum API extension. See §9, openquestion 5.Existing building blocks we rely on: the constant-time moduleThe library has several internal bignum interfaces, from lowest level to highest level:
bignum_core.h,bignum_mod_raw.h,bignum_mod.h,mbedtls/private/bignum.h. Lower-level interfaces generally have less overhead and are easier to make constant-time, but have a higher risk of memory management mistakes.The existing code in
ecp.cmostly relies onmbedtls/private/bignum.hbecause that was the only one available whenecp.cwas originally written. However, for new code,bignum_mod.his generally preferred, withbignum_mod_raw.horbignum_core.havailable ifbignum_mod.his not flexible enough or induces too much overhead.Given that the new code for this feature will have a large adherence to the existing code in
ecp.c, it's not obvious to me which abstraction level to prefer.@ -0,0 +336,4 @@for `…_GDH(SHA_256)` and for `…_FIXED(SHA_384)`), and the exact extent of`PSA_ALG_WPA3_SAE_ANY` — which algorithms it does and does not permit,including that it does not permit non-SAE algorithms.- **Opacity (acceptance criteria)**: (a) PMK from `get_shared_key()` withAlso test algorithm policies: non-wildcard policies require an exact match, and test the extent of
PSA_ALG_WPA3_SAE_ANY.@ -0,0 +402,4 @@merge at a major/minor release per the API-extension policy. The frameworksubmodule change is a separate PR — see §3.## 9. Open questions for maintainersThe design choices made here match Arm's preferences.
@ -0,0 +1,441 @@WPA3-SAE (PSA Certified Crypto API 1.4) — design noteAdded.
@ -0,0 +1,441 @@WPA3-SAE (PSA Certified Crypto API 1.4) — design noteRight, it globs
src/*.c— dropped that line.@ -0,0 +144,4 @@- `drivers/builtin/include/mbedtls/private/crypto_adjust_config_enable_builtins.h`— derive `MBEDTLS_PSA_BUILTIN_ALG_WPA3_SAE_*` / `MBEDTLS_PSA_BUILTIN_PAKE`,pull in ECP/bignum (and, P3, FFDH/RFC3526) modules.- `core/check_crypto_config.h` — dependency `#error` checks.Added.
@ -0,0 +181,4 @@**Framework submodule (separate repo)**- `framework/scripts/mbedtls_framework/crypto_knowledge.py` — register the newAdded.
@ -0,0 +336,4 @@for `…_GDH(SHA_256)` and for `…_FIXED(SHA_384)`), and the exact extent of`PSA_ALG_WPA3_SAE_ANY` — which algorithms it does and does not permit,including that it does not permit non-SAE algorithms.- **Opacity (acceptance criteria)**: (a) PMK from `get_shared_key()` withAdded — exact-match on non-wildcard policies (
FIXED(SHA_256)key rejected forGDH(SHA_256)and forFIXED(SHA_384)), plus whatPSA_ALG_WPA3_SAE_ANYdoes and does not cover.@ -0,0 +1,441 @@WPA3-SAE (PSA Certified Crypto API 1.4) — design noteNoted. P1/P2 are internal-only, so no entry there; the ChangeLog fragment goes with the PSA-integration PR.
@ -0,0 +1,441 @@WPA3-SAE (PSA Certified Crypto API 1.4) — design notePinned 0.75.1 in the note.
@ -0,0 +1,441 @@WPA3-SAE (PSA Certified Crypto API 1.4) — design noteThanks, that's clearer than what I had — reworded: framework merges first, written to work with both old and new code, and CI green on the consuming PR as the signal it isn't missing anything.
@ -0,0 +1,441 @@WPA3-SAE (PSA Certified Crypto API 1.4) — design noteMissed that one — will refactor it rather than add a second sqrt. Three deltas for the SSWU side, none changing existing behaviour: the operand becomes a parameter (SSWU needs the sqrt of an arbitrary field element, not
ecp_sw_rhs()); it returns a CTis_squareflag (cheapest by squaring the result back, not a secondexp_mod); and root selection stays in the callers, sincesw_derive_yselects on a public parity bit and SSWU selects on secret data. Written up as §4.1, planned atbignum_core.hlevel — ecp.c can call it by growing togrp->P.nand passingY->p, Y->n.@ -0,0 +294,4 @@and would put SAE on the critical path of a bignum API extension. See §9, openquestion 5.Existing building blocks we rely on: the constant-time moduleI went through the headers on this and I think the choice is narrower than it first looks.
bignum_mod.hfits SSWU conceptually — one fixed modulus, a long run of ops — but as it stands it can't express it:w^((p+1)/4) mod p.exp_modexists only asmbedtls_mpi_exp_mod()/mbedtls_mpi_core_exp_mod();mbedtls_mpi_mod_raw_cond_assign(), a layer down, and SSWU needs it throughout;lt_ct/check_zero_ctare inbignum_core.h;sgn0(a parity test on the canonical value) reads the wrong thing without converting out.So it's really
bignum_core.hvs extendingbignum_mod.hfirst. I'd leanbignum_core.hforhash_to_curve.cand the shared sqrt (it has exp_mod, cond_assign, lt_ct, check_zero_ct and explicit mont conversion, at fixed limb counts), and keepsae.conmbedtls_mpi, since it's mostly orchestratingmbedtls_ecp_*and anything else just buys conversions at every boundary.Fixed-limb matters here beyond style:
mbedtls_mpinormalizes its limb count, so a secret value that's numerically small runs shorter and leaks its magnitude — same class as Dragonblood — so I'd rather not have it near the field arithmetic. (mbedtls_mpi_exp_mod()is fine on that point; it grows toN->nfirst.)Happy to extend
bignum_mod.hinstead if you'd prefer the library to move that way, but I'd rather that be its own piece of work than a dependency of this one. Written up as §4.3 / open question 5 — leaving this thread open for your call.@ -0,0 +1,441 @@WPA3-SAE (PSA Certified Crypto API 1.4) — design noteDropped the vectors-only phase — tests now land in the PR with the code they exercise.
@ -0,0 +1,441 @@WPA3-SAE (PSA Certified Crypto API 1.4) — design noteAgreed, split. P1 = sqrt refactor +
hash_to_curve.c(RFC 9380 App J vectors), P2 =sae.c(hostap vectors). P2 depends on P1; nothing else does.@ -0,0 +1,441 @@WPA3-SAE (PSA Certified Crypto API 1.4) — design noteDone — CT annotations are a per-phase rule now, not a phase; fuzzing is its own later PR.
@ -0,0 +1,441 @@WPA3-SAE (PSA Certified Crypto API 1.4) — design noteAgreed — docs land with the code that introduces them.
@ -0,0 +1,441 @@WPA3-SAE (PSA Certified Crypto API 1.4) — design noteDone — it's a standalone phase now, independent of the SAE work, can land in any order relative to it.
Thanks, this was a useful pass — all 16 points are in the updated note (force-pushed). Substantive changes:
hash_to_curve.candsae.csplit into separate PRs,PSA_DH_FAMILY_RFC3526as a standalone phase with no SAE code, constant-time annotations and documentation as per-phase rules rather than a trailing hardening pass, fuzzing last.mbedtls_ecp_sw_derive_y()rather than new code.On that last one — the answer came out more definite than "open question" suggests, so flagging it here too:
bignum_mod.hhas no exponentiation, no conditional assign and no comparison, and its Montgomery representation breaks RFC 9380sgn0, so the real choice isbignum_core.hvs extendingbignum_mod.hfirst. We'd propose the former; detail in the thread on §4. That's the one thread I've left open for your call — the rest of the review is addressed and resolved.The three maintainer-facing open questions (§9: FFDH group set, omitting Looping, built-in vs encoding-only) are unchanged and still open.
Unrelated: the spec example errata (#372) merged, targeted for 1.5.1 — the note now reflects that.
Thanks for the update. I'm generally happy with the plan now, except that I'm unsure about
bignum_corevsbignum_mod: the arguments given againstbignum_modfeel very weak to me. It's hard for me to judge because I'm not at all familiar with the algorithm.Do you have a reading suggestion (code or math+English) that would make it clear how the representation of numbers matter? Are all the numbers involved in the same
Z_p, or is there a mix? At which steps are conversions to/from Montgomery form needed?Note: when I say I'm happy with the plan, that's as a general plan for implementing some calculations as a new cryptographic mechanism in TF-PSA-Crypto. I'm not familiar with the WPA3-SAE family of algorithms, so I can't vouch for difficulties or incompleteness for implementing the desired algorithms.
@ -0,0 +1,441 @@WPA3-SAE (PSA Certified Crypto API 1.4) — design noteIt's ok for this time because all the changes are in a single new file, but for future reference: please avoid force-pushing to a pull request after the first review has started. Make new changes in separate commits. Each commit should only do one thing (no big commit “address all the review comments”).
“the one Gilles noted” won't make sense outside of the context of the review of this pull request.
@ -0,0 +262,4 @@- **No conditional assign.** CMOV is `mbedtls_mpi_mod_raw_cond_assign()`, onelayer down; SSWU needs it throughout.- **No comparison.** The CT predicates (`mbedtls_mpi_core_lt_ct`,`mbedtls_mpi_core_check_zero_ct`) live in `bignum_core.h`.It would be easy to add comparison and conditional assignment, and not hard to add exponentiation. So I don't think those missing functions should weigh much in the decision.
@ -0,0 +266,4 @@- **Montgomery representation gets in the way of `sgn0`.** Residues may be heldin Montgomery form depending on the modulus `int_rep`, so a bit-level paritytest — which RFC 9380 `sgn0` needs — reads the wrong value unless convertedout first.A conversion step can be done. And conversion has to be done manually anyway when using bignum_core. How is bignum_mod worse in this respect?
@ -0,0 +400,4 @@0.75.1** and the in-tree `.uncrustify.cfg`.- **API stability**: this is additive (new macros/types/functions); it can onlymerge at a major/minor release per the API-extension policy. The frameworksubmodule change is a separate PR — see §3.To be precise, the changes inside the framework are a separate PR since it's a separate repository. Updating the framework submodule in the TF-PSA-Crypto repository should be done in the TF-PSA-Crypto PR that needs it, but the need may go away if another TF-PSA-Crypto PR updates the framework for its own needs first.
@ -0,0 +435,4 @@extend `bignum_mod.h` first. We propose `bignum_core.h` for`hash_to_curve.c` and the shared square root, and `mbedtls_mpi` for`sae.c`. If extending `bignum_mod.h` is wanted instead, we would rather doit as its own piece of work than on this feature's critical path.Many functions in the bignum modules and in the ecp module have automatically generated tests (
framework/scripts/generate_bignum_tests.py,framework/scripts/generate_ecp_tests.py). If this work introduces new functions that should have automatically generated tests, we'll need to arrange. to add new tested functionsBecause the framework code and the crypto code can't be updated atomically, the first time those scripts need to cover a new test function, this will require preliminary work to split the script into two parts:
framework/scripts/mbedtls_framwork/xxx_generator.pyhaving almost all the code.tests/scripts/generate_xxx.pythat calls an entry point in the framework and passes it arguments that describe which test functions should be covered.framework/scripts/generate_xxx.pystill exists. It callstests/scripts/generate_xxx.pyfrom the consuming branch if it exists, and otherwise generates test cases for the historical set of test functions.See
scripts/maintainer/generate_mldsa_tests.pyandframework/util/mbedtls_maintainer/mldsa_test_generator.pyfor an example of this split. Note that the file paths are different for mldsa because it needs extra python packages, and that would break some validation scripts that we run onframework/scripts,scriptsandtests/scripts; that doesn't apply for bignum and ecp.@ -0,0 +1,441 @@WPA3-SAE (PSA Certified Crypto API 1.4) — design noteAssitant (LLM) leakage :), will fix it thanks.
@ -0,0 +294,4 @@and would put SAE on the critical path of a bignum API extension. See §9, openquestion 5.Existing building blocks we rely on: the constant-time moduleYou're right, and two of my three arguments don't hold up:
add/sub/mul/invare there and cmp / cond-assign / exp are easy to add, so that shouldn't weigh much.mbedtls_mpi, not againstbignum_mod.bignum_modresidues are fixed-size (limbs == modulus limbs), so it doesn't distinguish the two. I conflated two axes; scratch that one.So it's really about ergonomics and blast radius, not capability.
Your questions:
All in the same Z_p, or a mix? For
hash_to_curve.cit's essentially one field. Every step ofhash_to_field+ simple-SWU + the square root is arithmetic modp, the curve's base-field prime — mul/add/sub/inv/sqrt/sgn0, all modp; cofactor clearing is a no-op (h=1 for P-256/384/521). The only non-field integers are the exponents fed toexp_mod(e.g.(p+1)/4), which are bit-strings not residues, and the final point addPT = P1 + P2, a curve-group op best handed tombedtls_ecp. Scalars mod the group ordernshow up in the SAE protocol (commit scalar etc.) but those live insae.c, not here. So: single modulus, long run of ops — which is exactly thebignum_modsweet spot, so your instinct is the right one.Where are Montgomery conversions needed? With
bignum_modin Montgomeryint_rep: convert in once when the field element is read (mbedtls_mpi_mod_read), then every mul/add/sub/inv stays in Montgomery — no per-op conversion — and convert out only at (a)sgn0, since RFC 9380's sign is the parity of the canonical representative, and (b) the final handoff to point arithmetic.exp_modfor the sqrt wants the base in Montgomery plus a precomputedRR, which the modulus object already carries. Withbignum_coreyou do the identical conversions by hand and manageRRyourself — so on conversions the two are a wash, which was your point on the other thread.So I'll drop the objection:
bignum_modis a good fit forhash_to_curve.cand the shared sqrt. The one tradeoff left is scope, not correctness — see my reply on the generated-tests comment.Reading, if useful: RFC 9380 §4.1 (
hash_to_field,sgn0) and §6.6.2 (simple SWU) for what the numbers are; thebignum_mod.hheader doc block (int_rep/ residue model) plus HAC §14.3.2 (Montgomery reduction) for why representation matters — short version: Montgomery form storesx·R mod pso reduction is shifts instead of division; it's stable under +/-/×, but any step that reads the integer itself (parity, comparison, serialization) has to convert back first, which is whysgn0is the one place we drop out.@ -0,0 +262,4 @@- **No conditional assign.** CMOV is `mbedtls_mpi_mod_raw_cond_assign()`, onelayer down; SSWU needs it throughout.- **No comparison.** The CT predicates (`mbedtls_mpi_core_lt_ct`,`mbedtls_mpi_core_check_zero_ct`) live in `bignum_core.h`.Agreed — conceded on the main thread; that argument doesn't carry weight, the functions are cheap to add.
@ -0,0 +266,4 @@- **Montgomery representation gets in the way of `sgn0`.** Residues may be heldin Montgomery form depending on the modulus `int_rep`, so a bit-level paritytest — which RFC 9380 `sgn0` needs — reads the wrong value unless convertedout first.Right — conversions are manual either way; I walked through where they land on the main thread. Not a point in
bignum_core's favour.@ -0,0 +435,4 @@extend `bignum_mod.h` first. We propose `bignum_core.h` for`hash_to_curve.c` and the shared square root, and `mbedtls_mpi` for`sae.c`. If extending `bignum_mod.h` is wanted instead, we would rather doit as its own piece of work than on this feature's critical path.This is the real deciding factor now that capability isn't. If we go
bignum_modwe're addingexp_mod(and probably cmp / cond-assign) as first-classbignum_modfunctions, which per your note means doing the generator split — ambedtls_framework/..._generator.pyplus a thintests/scripts/generate_*.pyharness, transition shim and all — following the mldsa example.bignum_corealready has all of these, so that path adds no bignum API and touches no generator.So the choice is: grow
bignum_modproperly (better long-term home, but the first code PR then also owns a bignum-module extension + the generator split), or keep the first SSWU PR tight onbignum_coreand leavebignum_modgrowth as separate future work. I lean toward doing it properly onbignum_modsince you'd want those functions eventually anyway — but you're better placed to say whether you want that coupled into the first SSWU PR or kept out of it. Either way I'll write it up to match.@ -0,0 +400,4 @@0.75.1** and the in-tree `.uncrustify.cfg`.- **API stability**: this is additive (new macros/types/functions); it can onlymerge at a major/minor release per the API-extension policy. The frameworksubmodule change is a separate PR — see §3.Understood — I'll fix §3 to say it precisely: the framework changes are a separate PR (separate repo), and the submodule bump lands in whichever TF-PSA-Crypto PR first needs it, so the "merged first" framing was too rigid.
@ -0,0 +1,441 @@WPA3-SAE (PSA Certified Crypto API 1.4) — design noteRemoved in
986f205.@gilles-peskine-arm — I answered your three questions (representation/reading, single-field vs mix, where the Montgomery conversions land) on the bignum thread rather than here so they wouldn't get buried: https://github.com/Mbed-TLS/TF-PSA-Crypto/pull/837#discussion_r3641366480
Short version: it's a single field (F_p) throughout
hash_to_curve.c, conversions are in-once and out-only-at-sgn0, and I've dropped the objection —bignum_modis a fine fit. The one open point is scope: growbignum_mod(which pulls in the generator split) vs keep the first PR tight onbignum_core. Your call.LGTM at
986f205106, with the caveat that I am not familiar with WPA3-SAE and I have not checked the plan against the specification of the mechanisms.On
bignum_corevsbignum_mod, I could go either way based on my knowledge. I'll ask a colleague who's more familiar with ECC math and the planned evolution of bignum and ECP modules.@gilles-peskine-arm Should I add any other reviewer to get this merged?
We do need a second reviewer. But I have no idea when someone will be available or who it will be.
By the way, given the size of the contribution, have you made arrangements with our product management to have the review of the code scheduled? We currently have very little bandwidth since most of the team is triaging and fixing LLM-assisted vulnerability reports.
Ah, any process to followup here or a list of people to nudge :)?
No, but if you can guide me, I am happy to connect and take this forward with the product management.
View command line instructions
Checkout
From your project repository, check out a new branch and test the changes.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.