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.

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

PSA_ALG_TLS12_PRF should have unlimited capacity #9744

Open
gilles-peskine-arm opened this issue Oct 31, 2024 · 2 comments
Open

PSA_ALG_TLS12_PRF should have unlimited capacity #9744

gilles-peskine-arm opened this issue Oct 31, 2024 · 2 comments
Labels
bug component-crypto Crypto primitives and low-level interfaces good-first-issue Good for newcomers size-xs Estimated task size: extra small (a few hours at most)

Comments

@gilles-peskine-arm
Copy link
Contributor

In psa_key_derivation_set_maximum_capacity, we correctly declare that the TLS1.2 PRF key derivation algorithm (PSA_ALG_TLS12_PRF) has unlimited capacity. We used to declare a more limited capacity of 256 hash-sized blocks, which was the right calculation for HKDF, but this was fixed in #8198.

However, in the TLS1.2 PRF calculation, we still have an artificial limitation of 256 blocks. This comes from the original implementation of the KDF, and may have been contamination from HKDF, but it was a mistake that we didn't catch until now. So if you try to derive more than 256 blocks, this raises PSA_ERROR_CORRUPTION_DETECTED. Fortunately that's just an implementation mistake.

The goal of this issue is to remove the limitation. We don't need to keep track of the block number for TLS12_PRF: only the first block needs to be special-cased. (However, when fixing on an LTS branch, we don't change the ABI, so the block_number field in the operation structure has to stay, even if it becomes useless.) Add a test case that goes beyond 256 blocks.

@gilles-peskine-arm gilles-peskine-arm added bug component-crypto Crypto primitives and low-level interfaces good-first-issue Good for newcomers size-xs Estimated task size: extra small (a few hours at most) labels Oct 31, 2024
@mfil
Copy link
Contributor

mfil commented Oct 31, 2024

If this is just a matter of deleting the check, would it make sense to include it in my PR?

@gilles-peskine-arm
Copy link
Contributor Author

@mfil Your PR is already large. In fact 35 commits is above what we normally consider to be viable for review, so we may have to ask you to split it. If you'd like to fix this bug, you're welcome, but please do it in a separate PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug component-crypto Crypto primitives and low-level interfaces good-first-issue Good for newcomers size-xs Estimated task size: extra small (a few hours at most)
Projects
Status: No status
Development

No branches or pull requests

2 participants