PK: inline and simplify get_bitlen functions #522
Closed
opened 2025-10-14 08:21:59 -05:00 by mpg
·
4 comments
No Branch/Tag specified
development
tf-psa-crypto-1.1
archive/20261005T153337Z-force/pull/927
archive/20261001T141055Z-force/pull/927
archive/20261005T153337Z-force/pull/931
archive/20261001T083616Z-force/pull/927
archive/20260930T232327Z-force/pull/927
archive/20260930T154618Z-force/pull/927
archive/20260929T231919Z-rescue/rohankl-arm/mldsa-pubkey-899
archive/20260929T231919Z-rescue/sigvartmh/pr/spake2p-9342-public-key-import-export
archive/20260929T231919Z-rescue/ManoharPaturi/fix-pem-ct-padding
archive/20260929T231919Z-rescue/ManoharPaturi/fix-asn1write-null-guard
archive/20260929T231919Z-rescue/ManoharPaturi/fix-pbes2-params-validation
archive/20260929T231919Z-rescue/valeriosetti/blake2
archive/20260929T231919Z-rescue/jfdev001/v1.1.1_bump-cmake-minimum-version
archive/20260929T231919Z-rescue/jfdev001/bump-cmake-minimum-version
archive/20260929T231919Z-rescue/ronald-cron-arm/sm3-base
archive/20260929T231919Z-rescue/valeriosetti/blake2-hmac
archive/20260929T231919Z-rescue/v1rtu3x/enforce-hash-length-sign-verify
archive/20260929T231919Z-rescue/bjwtaylor/file-backed-keystore-doc
archive/20260929T231919Z-rescue/sigvartmh/pr/spake2p-9344-set-context
archive/20260929T231919Z-rescue/sigvartmh/pr/spake2p-9343-key-pair-import-export-depends-on-9342
archive/20260929T231919Z-rescue/romart/development
archive/20260929T231919Z-rescue/sweetlilmre/constant-time-embedded-perf
archive/20260929T231919Z-rescue/JulioLoayzaM/add-wycheproof-rsa-pss
archive/20260929T231919Z-rescue/JulioLoayzaM/add-wycheproof-ecdsa
archive/20260929T231919Z-rescue/JulioLoayzaM/add-wycheproof-tests
archive/20260929T231919Z-rescue/oubaid-a/dev_psa_driver_wrapper
archive/20260929T231919Z-rescue/gilles-peskine-arm/keystoreless-prototype-1
archive/20260929T231919Z-rescue/valeriosetti/issue9580
archive/20260929T231919Z-rescue/LeonieReichertABB/hpke-functionality
archive/20260929T231919Z-rescue/gilles-peskine-arm/psa-hash-no-buffer-copy-1.1
archive/20260929T231919Z-rescue/vanhauser-thc/development
archive/20260929T231919Z-rescue/tpambor/key-wrap
archive/20260929T231919Z-rescue/ruiliio/psa_xts
archive/20261011T063515Z-force/pull/538
archive/20260929T231919Z-rescue/gilles-peskine-arm/fix-broken-links-1.1
archive/20260929T231919Z-rescue/valeriosetti/issue669
archive/20260929T231919Z-rescue/gilles-peskine-arm/prepare-release-test
archive/20260929T231919Z-rescue/krish2718/wpa3-sae-design-note
archive/20260929T231919Z-rescue/nzzlinh/fix_iar_endianness_1.1
archive/20260929T231919Z-rescue/nzzlinh/fix-iar-endianness
archive/20260929T231919Z-rescue/abtom87/fix-pbkdf2-iter-count
archive/20260929T231919Z-rescue/minosgalanakis/tools/prepare_release_lite
archive/20260929T231919Z-rescue/Harshal5/fix/aes-xts-in-place-cts
archive/20260929T231919Z-rescue/sigvartmh/pr/spake2p-9347-prover-depends-on-9342-9343-9344
archive/20260929T231919Z-rescue/crabel99/async-hardware-support-c35df962
tf-psa-crypto-1.2.0_mergeback
tf-psa-crypto-1.1.1_mergeback
archive/20260929T231919Z-rescue/asmellby/feature/dispatch-library
tf-psa-crypto-release-sync
archive/20260929T231919Z-rescue/ChakshuGupta13/fix/ecdsa-zeroize-stack-key
archive/20260929T231919Z-rescue/gilles-peskine-arm/security-md-option-names
archive/20260929T231919Z-rescue/parmi93/patch-2
archive/20260929T231919Z-rescue/bjwtaylor/remove-mldsa-sign-from-build
archive/20260929T231919Z-rescue/bjwtaylor/tf-m-build-issues
archive/20260929T231919Z-rescue/bjwtaylor/test-context-population
archive/20260929T231919Z-rescue/gilles-peskine-arm/iar-1.1.0
archive/20260929T231919Z-rescue/gilles-peskine-arm/fork-helper-unit-tests-create-crypto
archive/20260929T231919Z-rescue/bjwtaylor/redundant-declarations
archive/20260929T231919Z-rescue/hasheddan/docs/fix-broken-links
main
archive/20260929T231919Z-rescue/bjwtaylor/reintroduce-dh
archive/20260929T231919Z-rescue/bjwtaylor/cipher-update-incorrect-error
archive/20260929T231919Z-rescue/Marandil/fix/mbedtls-4314-parse-explicit-curve
archive/20260929T231919Z-rescue/schneider-electric/dev/plaj-se/parse-rsassa-pss
archive/20260929T231919Z-rescue/minosgalanakis/bugfix/program-fuzz-mscv
archive/20260929T231919Z-rescue/daverodgman/aesgcm
archive/20260929T231919Z-rescue/gilles-peskine-arm/sha3-coverity-20260224
archive/20260929T231919Z-rescue/bensze01/undef
archive/20260929T231919Z-rescue/valeriosetti/issue523
archive/20260929T231919Z-rescue/dannytsen/ppc-dev
archive/20260929T231919Z-rescue/daverodgman/aes12
archive/20260929T231919Z-rescue/gilles-peskine-arm/ascon-prototype
archive/20260929T231919Z-rescue/daverodgman/chacha10
archive/20260929T231919Z-rescue/kusumitg/implement_psa_pake_set_context_for_spake2p
archive/20260929T231919Z-rescue/davidhorstmann-arm/add-code-size-target
archive/20260929T231919Z-rescue/bjwtaylor/simplify-sign_func
archive/20260929T231919Z-rescue/mfil/feature/aes_ni_parallel
archive/20260929T231919Z-rescue/ivv19041994/patch-1
archive/20260929T231919Z-rescue/gilles-peskine-arm/psa-verify-policy-relax
public_2/main
archive/20260929T231919Z-rescue/mpg/pk-sign-proto
release/tf-psa-crypto1.0.0-documentation
archive/20260929T231919Z-rescue/gilles-peskine-arm/0e-plans-july-updates
archive/20260929T231919Z-rescue/davidhorstmann-arm/guard-context-types-in-client-only
archive/20260929T231919Z-rescue/gilles-peskine-arm/psa-driver-interface-spec-remove
archive/20260929T231919Z-rescue/gilles-peskine-arm/ecp-self-test-race-crypto
archive/20260929T231919Z-rescue/KiruthikPrakash/feature/9342_add-support-for-spake2p-public-key-import-export
archive/20260929T231919Z-rescue/ruiliio/chacha20_16
archive/20260929T231919Z-rescue/irwir/winsock2
archive/20260929T231919Z-rescue/aphroteus/development
archive/20260929T231919Z-rescue/amjoul01/amjoul01/psa-mac-out-of-bound-fix
archive/20260929T231919Z-rescue/gilles-peskine-arm/everest-submodule-crypto
archive/20260929T231919Z-rescue/yamt/xtensa-ccount
archive/20260929T231919Z-rescue/gilles-peskine-arm/all.sh-generate-only-before-checks-crypto
archive/20260929T231919Z-rescue/Harshal5/feat/builtin_cmac_driver_use_psa_crypto_interface
archive/20260929T231919Z-rescue/gilles-peskine-arm/test-data_t-empty_is_empty-crypto
archive/20260929T231919Z-rescue/jean-baptisteboric-eaton/oids-pkcs7-authenticated-attributes
dev/bensze01/public-only-commit
archive/20260929T231919Z-rescue/waleed-elmelegy-arm/psa-key-wrap-aes-kw
archive/20260929T231919Z-rescue/tfloch/psa_add_xts_algorithm
archive/20260929T231919Z-rescue/gilles-peskine-arm/msancf-clang16-crypto
archive/20260929T231919Z-rescue/DemiMarie/document-unexpected-tags
archive/20260929T231919Z-rescue/DemiMarie/strict-len
archive/20260929T231919Z-rescue/valeriosetti/issue9255-tfpsacrypto
archive/20260929T231919Z-rescue/daantimmer/use-compiler-frontend
archive/20260929T231919Z-rescue/benmcollins/enable-rsassa-pss-pkparse
dev/paul-elliott-arm/add_iop_key_agreement_negative_tests
add-iop-export-pub-key-get-num-ops
dev/rc/new-2-d635ea1
dev/bensze01/jenkins
v1.2.0
tf-psa-crypto-1.2.0
v1.1.1
tf-psa-crypto-1.1.1
v1.1.0
tf-psa-crypto-1.1.0
v1.0.0
tf-psa-crypto-1.0.0
v1.0.0-beta
tf-psa-crypto-1.0.0-beta
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#522
Loading…
Reference in a new issue
No description provided.
Delete branch "%!s()"
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?
Same as #515 and #519 but with
get_bitlenfunction pointers. Should be simpler.Here's an attempt to define a test matrix for this task. @mpg @bjwtaylor feel free to extend/integrated/fix if needed.
PK generating functions
Test matrix:
negative: verify that a NULL PK context pointer and NULL pk_info return 0 length
non-wrapping -> mbedtls_pk_parse_key
non-wrapping -> mbedtls_pk_copy_from_psa
non-wrapping -> mbedtls_pk_copy_public_from_psa
non-wrapping -> mbedtls_pk_parse_public_key
wrapping -> mbedtls_pk_wrap_psa
Notes
EC curve#1andEC curve#2are 2 curves of different bit size. For example secp256r1 and secp521r1.RSA bit-size#1andRSA bit-size#2are 2 different bit size for RSA. For example 1024 and 4096Thanks @valeriosetti for making the test matrix explicit! Just one point though: I don't think the negative needs to be repeated: there are 5 ways to populate a context, but only 1 way not to populate it (that is, try using it right after
pk_init()).In terms of implementing this test matrix, I was thinking we could use the same strategy as in https://github.com/Mbed-TLS/TF-PSA-Crypto/pull/577:
parse_key()orparse_public_key(), with an enum parameter telling which one it is, and the key material in the test datamake_psa_key_from_predefined()(see also https://github.com/Mbed-TLS/TF-PSA-Crypto/issues/598)For (1) I think we already have a general function for this kind of negative testing, and probably it already tests those two things already, we should just double-check and add (b) if missing. For (2) and (3) we'd create new test functions for clarity. That way it's obvious from the data file that
get_bitlen()is properly tested, because we can see the expanded test matrix right there in the data file. (As is the case in #577)Wdyt @valeriosetti @bjwtaylor ?
That's fine for me. I updated the test matrix above.
Ok, let me look into it and I'll update the PR as per the above.