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.

Sign up for GitHub

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

Jump to bottom

psa_rsa_verify always fails with some compliant alternative implementations #3990

Closed
yanesca opened this issue Jan 7, 2021 · 2 comments · Fixed by #4665
Closed

psa_rsa_verify always fails with some compliant alternative implementations #3990

yanesca opened this issue Jan 7, 2021 · 2 comments · Fixed by #4665
Assignees
@yanesca
Labels
bug size-s Estimated task size: small (~2d)

Comments

@yanesca
Copy link
Contributor

yanesca commented Jan 7, 2021 •
edited
Loading

Description

  • Type: Bug
  • Priority: Major

psa_rsa_verify always passes MBEDTLS_MD_NONE to mbedtls_rsa_rsassa_pss_verify()
https://github.com/ARMmbed/mbedtls/blob/2b759626a90da9eaa821a6294a12e9a655ca46cf/library/psa_crypto.c#L3760

This works well with the Mbed TLS implementation, but causes all verifications to fail in some alternative implementations. (The standard allows for some flexibility and this can happen with perfectly compliant implementations.)

In particular the Mbed TLS implementation only uses this parameter for determining the input length, but some implementations might enforce that the hash functions match.

Bug

Expected behavior

psa_rsa_verify works with compliant alternative implementations.

Actual behavior

psa_rsa_verify always fails with a compliant alternative implementation.
@gilles-peskine-arm
Copy link
Contributor

Could you give a concrete example of a correct signature that our implementation of psa_rsa_verify rejects or vice versa (it's not even clear to me which direction the bug is in)?

Note that the definition of PSA_ALG_RSA_PSS requires all three hash algorithms involved (hashing the message, hashing the hash, and the hash used in MGF1) to be the same. However implementations are permitted to have extensions if they don't breach security, so accepting signatures of something that isn't a hash made with h2 in psa_verify_message(alg=PSA_ALG_RSA_PSS(h2)) is somewhat unhygienic but still compliant.

@yanesca
Copy link
Contributor Author

yanesca commented Jan 11, 2021 •
edited
Loading

However implementations are permitted to have extensions if they don't breach security, so accepting signatures of something that isn't a hash made with h2 in psa_verify_message(alg=PSA_ALG_RSA_PSS(h2)) is somewhat unhygienic but still compliant.

Exactly. But strict implementations would reject any signature when called with different hash parameters. So if we set the padding to md_alg and pass MBEDTLS_MD_NONE to mbedtls_rsa_rsassa_pss_verify() (as we do it in psa_rsa_verify()), then such implementations will reject it unless md_alg==MBEDTLS_MD_NONE.

@laumor01 laumor01 added the size-s Estimated task size: small (~2d) label May 4, 2021
@yanesca yanesca self-assigned this Jun 7, 2021
yanesca added a commit to yanesca/mbedtls that referenced this issue Jun 15, 2021
@yanesca
PSA RSA PSS: pass pre-hash algorithm to Mbed TLS
43d1938 
PSA Crypto always passed MBEDTLS_MD_NONE to Mbed TLS, which worked well
as Mbed TLS does not use this parameter for anything beyond determining
the input lengths.

Some alternative implementations however check the consistency of the
algorithm used for pre-hash and for other uses in verification (verify
operation and mask generation) and fail if they don't match. This makes
all such verifications fail.

Furthermore, the PSA Crypto API mandates that the pre-hash and internal
uses are aligned as well.

Fixes Mbed-TLS#3990.

Signed-off-by: Janos Follath <janos.follath@arm.com>
@yanesca yanesca mentioned this issue Jun 15, 2021
Merged
@mpg mpg mentioned this issue Jun 16, 2021
Closed
yanesca added a commit to yanesca/mbedtls that referenced this issue Jun 17, 2021
@yanesca
PSA RSA PSS: pass pre-hash algorithm to Mbed TLS
d366b89 
PSA Crypto always passed MBEDTLS_MD_NONE to Mbed TLS, which worked well
as Mbed TLS does not use this parameter for anything beyond determining
the input lengths.

Some alternative implementations however check the consistency of the
algorithm used for pre-hash and for other uses in verification (verify
operation and mask generation) and fail if they don't match. This makes
all such verifications fail.

Furthermore, the PSA Crypto API mandates that the pre-hash and internal
uses are aligned as well.

Fixes Mbed-TLS#3990.

Signed-off-by: Janos Follath <janos.follath@arm.com>
yanesca added a commit to yanesca/mbedtls that referenced this issue Jun 17, 2021
@yanesca
PSA RSA PSS: pass pre-hash algorithm to Mbed TLS
bf179e5 
PSA Crypto always passed MBEDTLS_MD_NONE to Mbed TLS, which worked well
as Mbed TLS does not use this parameter for anything beyond determining
the input lengths.

Some alternative implementations however check the consistency of the
algorithm used for pre-hash and for other uses in verification (verify
operation and mask generation) and fail if they don't match. This makes
all such verifications fail.

Furthermore, the PSA Crypto API mandates that the pre-hash and internal
uses are aligned as well.

Fixes Mbed-TLS#3990.

Signed-off-by: Janos Follath <janos.follath@arm.com>
yanesca added a commit to yanesca/mbedtls that referenced this issue Jun 21, 2021
@yanesca
PSA RSA PSS: pass pre-hash algorithm to Mbed TLS
0af093b 
PSA Crypto always passed MBEDTLS_MD_NONE to Mbed TLS, which worked well
as Mbed TLS does not use this parameter for anything beyond determining
the input lengths.

Some alternative implementations however check the consistency of the
algorithm used for pre-hash and for other uses in verification (verify
operation and mask generation) and fail if they don't match. This makes
all such verifications fail.

Furthermore, the PSA Crypto API mandates that the pre-hash and internal
uses are aligned as well.

Fixes Mbed-TLS#3990.

Signed-off-by: Janos Follath <janos.follath@arm.com>
mpg pushed a commit to mpg/mbedtls that referenced this issue Jun 22, 2021
@yanesca @mpg
PSA RSA PSS: pass pre-hash algorithm to Mbed TLS
b23b574 
PSA Crypto always passed MBEDTLS_MD_NONE to Mbed TLS, which worked well
as Mbed TLS does not use this parameter for anything beyond determining
the input lengths.

Some alternative implementations however check the consistency of the
algorithm used for pre-hash and for other uses in verification (verify
operation and mask generation) and fail if they don't match. This makes
all such verifications fail.

Furthermore, the PSA Crypto API mandates that the pre-hash and internal
uses are aligned as well.

Fixes Mbed-TLS#3990.

Signed-off-by: Janos Follath <janos.follath@arm.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@yanesca yanesca

Labels
bug size-s Estimated task size: small (~2d)
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix PSA RSA PSS verify with ALT implementations yanesca/mbedtls
3 participants
@yanesca @gilles-peskine-arm @laumor01