Allow PSA hash operations without psa_crypto_init #6470
Conversation
Our implementation of hash functions does not check whether the library has been initialized. Other implementations (including services built on top of Mbed TLS) might require initialization, but it's not our job to mention that (it's the job of these other implementations' documentation and of the PSA specification). Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
…alization The documentation dates back from when it doubled as the PSA specification. Nowadays this is just the documentation of Mbed TLS, so it should document implementation-specific behavior. All functions that access a key or the random generator require library initialization. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Hash functions don't actually require initializing the library. So don't do it when testing. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
When testing hash operations in situations that can return PSA_ERROR_BAD_STATE, do the testing both with an uninitialized library and with an initialized library, in case there was some interference between BAD_STATE due to the library initialization state and BAD_STATE due to the operation state. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
This is now tested, so we can promise it. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
gilles-peskine-arm
added
enhancement
needs-review
| @@ -0,0 +1,4 @@ | |||
| Features | |||
| * Document that PSA hash operations don't require psa_crypto_init(). This | |||
| has always been possible, but was not previously explicitly stated. This | |||
There was a problem hiding this comment.
Except when there's a driver that requires initialization, but that's a preexisting bug. Making this promise makes the bug more pressing though, so it would be good to fix it.
There was a problem hiding this comment.
I disagree with the reasoning here. As you say, it's technically a new feature, so I'm not sure how 6469 can be considered a pre-existing bug.
Regardless, we really shouldn't write things that we know to be false, so we need to either mention 6469 as temporary limitation that will be lifted in the future, or (much preferred) solve 6469 before we merge this.
There was a problem hiding this comment.
I am taking advantage of the fact that MBEDTLS_PSA_CRYPTO_DRIVERS is still considered experimental to declare that this a feature, not a bug.
It's still not clear to me that lifting the limitation is worth the effort (not in terms of coding, but in terms of impact).
In general terms: * Which operations are supported. * What entry point call sequences are possible. * What drivers can expect in terms of input validity. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
tom-daubney-arm
added
the
historical-reviewing
|
We are now converting older PRs to draft PRs where the following conditions are met: They have not been updated in the last 3 months, and they need work to complete; in this case there are conflicts. |
tom-daubney-arm
added
needs-work
historical-reviewed
Status: proposal — it turns out #6549 doesn't actually need this, at least at the moment.
Officially allow PSA hash operations (
psa_hash_compute,psa_hash_verify,psa_hash_setup, and functions on multipart hash operations) even if the library hasn't been initialized withpsa_crypto_init. This has always worked (with the exception of drivers requiring initialization), but was documented as “implementation-defined” (which doesn't really make sense since we are the implementation and our documentation should be defining the implementation-defined behavior).This is possibly on the critical path for simplifying the handling of builds where some algorithms are provided via PSA and others via legacy crypto, especially in algorithms such as RSA and deterministic ECDSA that use a hash inside.
Changelog: yes
Backport: maybe (it's technically a new feature, but the library code isn't changing)