Enforce hash length in psa_sign_hash and psa_verify_hash #892

Open
v1rtu3x wants to merge 1 commit from v1rtu3x/enforce-hash-length-sign-verify into development
v1rtu3x commented 2026-09-02 10:38:05 -05:00 (Migrated from github.com)

For hash-and-sign algorithms, validate that the hash input length matches PSA_HASH_LENGTH(PSA_ALG_SIGN_GET_HASH(alg)). Return PSA_ERROR_INVALID_ARGUMENT if it doesn't.

This is a requirement in the PSA Crypto API 1.1 specification. PSA_ALG_RSA_PKCS1V15_SIGN_RAW is exempt as it has no specific hash length requirement.

The validation is applied to:

  • psa_sign_hash() and psa_verify_hash()
  • psa_sign_hash_start() and psa_verify_hash_start()
  • mbedtls_psa_sign_hash_start() and mbedtls_psa_verify_hash_start()

Added negative test cases for wrong hash length in all variants.

Fixes #869

Description

This PR enforces the hash input length in psa_sign_hash() and psa_verify_hash() (and their interruptible variants) for hash-and-sign algorithms, as required by the PSA Crypto API 1.1 specification. Previously, the library accepted hash inputs of any length, which was non-compliant and error-prone.

A new static helper function psa_validate_sign_hash_length() checks that hash_length == PSA_HASH_LENGTH(PSA_ALG_SIGN_GET_HASH(alg)) for all hash-and-sign algorithms except PSA_ALG_RSA_PKCS1V15_SIGN_RAW, returning PSA_ERROR_INVALID_ARGUMENT on mismatch.

Negative test cases have been added to tests/suites/test_suite_psa_crypto.data covering:

  • ECDSA with SHA-256: too-short and too-long hash inputs
  • RSA-PSS and RSA-PKCS1v15 with SHA-256: wrong hash length
  • Interruptible sign/verify variants with wrong hash length

PR checklist

  • changelog provided | not required because:
  • framework PR provided Mbed-TLS/mbedtls-framework# | not required
  • TF-PSA-Crypto development PR provided # | not required because: this is the development PR
  • TF-PSA-Crypto 1.1 PR provided # | not required because:
  • mbedtls development PR provided Mbed-TLS/mbedtls# | not required because:
  • mbedtls 4.1 PR provided Mbed-TLS/mbedtls# | not required because:
  • mbedtls 3.6 PR provided Mbed-TLS/mbedtls# | not required because:
  • tests provided | not required because:
For hash-and-sign algorithms, validate that the hash input length matches PSA_HASH_LENGTH(PSA_ALG_SIGN_GET_HASH(alg)). Return PSA_ERROR_INVALID_ARGUMENT if it doesn't. This is a requirement in the PSA Crypto API 1.1 specification. PSA_ALG_RSA_PKCS1V15_SIGN_RAW is exempt as it has no specific hash length requirement. The validation is applied to: - psa_sign_hash() and psa_verify_hash() - psa_sign_hash_start() and psa_verify_hash_start() - mbedtls_psa_sign_hash_start() and mbedtls_psa_verify_hash_start() Added negative test cases for wrong hash length in all variants. Fixes #869 ## Description This PR enforces the hash input length in psa_sign_hash() and psa_verify_hash() (and their interruptible variants) for hash-and-sign algorithms, as required by the PSA Crypto API 1.1 specification. Previously, the library accepted hash inputs of any length, which was non-compliant and error-prone. A new static helper function `psa_validate_sign_hash_length()` checks that `hash_length == PSA_HASH_LENGTH(PSA_ALG_SIGN_GET_HASH(alg))` for all hash-and-sign algorithms except `PSA_ALG_RSA_PKCS1V15_SIGN_RAW`, returning `PSA_ERROR_INVALID_ARGUMENT` on mismatch. Negative test cases have been added to `tests/suites/test_suite_psa_crypto.data` covering: - ECDSA with SHA-256: too-short and too-long hash inputs - RSA-PSS and RSA-PKCS1v15 with SHA-256: wrong hash length - Interruptible sign/verify variants with wrong hash length ## PR checklist - [x] **changelog** provided | not required because: - [ ] **framework PR** provided Mbed-TLS/mbedtls-framework# | not required - [ ] **TF-PSA-Crypto development PR** provided # | not required because: this is the development PR - [ ] **TF-PSA-Crypto 1.1 PR** provided # | not required because: - [ ] **mbedtls development PR** provided Mbed-TLS/mbedtls# | not required because: - [ ] **mbedtls 4.1 PR** provided Mbed-TLS/mbedtls# | not required because: - [ ] **mbedtls 3.6 PR** provided Mbed-TLS/mbedtls# | not required because: - **tests** provided | not required because:
v1rtu3x commented 2026-09-03 10:15:42 -05:00 (Migrated from github.com)

Pushed fixes for the CI issues from build. Changes:

  • Fixed parameter alignment in psa_validate_sign_hash_length()
  • Moved test cases into proper sections with depends_on: headers
  • Added guard for PSA_ALG_ANY_HASH in the helper

psa_crypto-suite passes locally. I wasn't able to run uncrustify or the full changelog checker locally — happy to fix any remaining issues if they come up.

The X509/PKCS7 failures appear unrelated to this change.

Pushed fixes for the CI issues from build. Changes: - Fixed parameter alignment in `psa_validate_sign_hash_length()` - Moved test cases into proper sections with `depends_on:` headers - Added guard for `PSA_ALG_ANY_HASH` in the helper `psa_crypto-suite` passes locally. I wasn't able to run `uncrustify` or the full changelog checker locally — happy to fix any remaining issues if they come up. The X509/PKCS7 failures appear unrelated to this change.
v1rtu3x commented 2026-09-05 03:09:22 -05:00 (Migrated from github.com)

I may be missing something, but I tried to reproduce the CI failure locally and couldn’t.

On a clean combined Mbed TLS development tree with tf-psa-crypto at this PR head (74c03494d), built with CMAKE_BUILD_TYPE=MemSan, the relevant suites pass for me: the PSA crypto suites and also pkcs7-suite / x509parse-suite — 31/31 passed.

In the failed CI log, the error seemed to be MBEDTLS_ERR_PK_FILE_IO_ERROR (-0x3E00) from file-based PKCS7/X509 tests rather than from the PSA sign/verify hash-length path, so I’m wondering whether this could be a CI workspace/container issue or an unrelated base-branch failure.

If possible, could someone rerun the job or point me at what I should check next? Happy to adjust the patch if there’s something I overlooked.

I may be missing something, but I tried to reproduce the CI failure locally and couldn’t. On a clean combined Mbed TLS `development` tree with `tf-psa-crypto` at this PR head (`74c03494d`), built with `CMAKE_BUILD_TYPE=MemSan`, the relevant suites pass for me: the PSA crypto suites and also `pkcs7-suite` / `x509parse-suite` — 31/31 passed. In the failed CI log, the error seemed to be `MBEDTLS_ERR_PK_FILE_IO_ERROR` (`-0x3E00`) from file-based PKCS7/X509 tests rather than from the PSA sign/verify hash-length path, so I’m wondering whether this could be a CI workspace/container issue or an unrelated base-branch failure. If possible, could someone rerun the job or point me at what I should check next? Happy to adjust the patch if there’s something I overlooked.
gilles-peskine-arm commented 2026-09-05 05:55:30 -05:00 (Migrated from github.com)

There are many failing jobs, but most of them probably have the same cause. At first glance, it looks like the tests pass in the default configuration, but not in the full config. Try something like this:

cmake -B build-full-debug -DCMAKE_BUILD_TYPE=Debug -DTF_PSA_CRYPTO_CONFIG_NAME=full
cmake --build build-full-debug test_suite_psa_crypto
gdb build-full-debug/tests/test_suite_psa_crypto
There are many failing jobs, but most of them probably have the same cause. At first glance, it looks like the tests pass in the default configuration, but not in the full config. Try something like this: ``` cmake -B build-full-debug -DCMAKE_BUILD_TYPE=Debug -DTF_PSA_CRYPTO_CONFIG_NAME=full cmake --build build-full-debug test_suite_psa_crypto gdb build-full-debug/tests/test_suite_psa_crypto ```
v1rtu3x commented 2026-09-18 04:18:27 -05:00 (Migrated from github.com)

Hi, it looks like the current PR head (4618440c) only has the DCO check and the full CI hasn't been triggered again. Would someone with the necessary permissions be able to retrigger the CI for the current head? Thanks!

Hi, it looks like the current PR head (4618440c) only has the DCO check and the full CI hasn't been triggered again. Would someone with the necessary permissions be able to retrigger the CI for the current head? Thanks!
gilles-peskine-arm commented 2026-09-18 04:56:18 -05:00 (Migrated from github.com)

Triggered. Expect results in ~2 hours.

Triggered. Expect results in ~2 hours.
v1rtu3x commented 2026-09-21 06:23:40 -05:00 (Migrated from github.com)

Hi,
When you have a chance, could you please rerun the CI on the latest commit?

Thank you for your time and help

Hi, When you have a chance, could you please rerun the CI on the latest commit? Thank you for your time and help
ronald-cron-arm commented 2026-09-21 08:20:20 -05:00 (Migrated from github.com)

I've just triggered it.

I've just triggered it.
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 'v1rtu3x/enforce-hash-length-sign-verify:v1rtu3x/enforce-hash-length-sign-verify'
git switch 'v1rtu3x/enforce-hash-length-sign-verify'

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 'v1rtu3x/enforce-hash-length-sign-verify'
git switch 'v1rtu3x/enforce-hash-length-sign-verify'
git rebase 'development'
git switch 'development'
git merge --ff-only 'v1rtu3x/enforce-hash-length-sign-verify'
git switch 'v1rtu3x/enforce-hash-length-sign-verify'
git rebase 'development'
git switch 'development'
git merge --no-ff 'v1rtu3x/enforce-hash-length-sign-verify'
git switch 'development'
git merge --squash 'v1rtu3x/enforce-hash-length-sign-verify'
git switch 'development'
git merge --ff-only 'v1rtu3x/enforce-hash-length-sign-verify'
git switch 'development'
git merge 'v1rtu3x/enforce-hash-length-sign-verify'
git push origin 'development'
Sign in to join this conversation.
No reviewers
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!892
No description provided.