Alternative random generator support for PSA #3895
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
gilles-peskine-arm
added
enhancement
needs-review
Subsequent commits will move declarations and definitions there. Signed-off-by: Gilles Peskine <[email protected]>
Create wrapper functions around calls to CTR_DRBG and around calls to entropy+DRBG. This is in preparation for allowing alternative DRBG implementations that use the Mbed TLS entropy module, or complete RNG implementations that bypass the entropy module as well. This is purely a refactoring commit. No behavior change. Signed-off-by: Gilles Peskine <[email protected]>
Create a configuration option for autonomous random drivers, i.e. PSA crypto drivers that provide a random generator, that have their own entropy source and do not support injecting entropy from another source. This commit only creates the configuration option. Subsequent commits will add the implementation and tests. Signed-off-by: Gilles Peskine <[email protected]>
Signed-off-by: Gilles Peskine <[email protected]>
Define a sample type mbedtls_psa_external_random_context_t in psa/crypto_platform.h and define the prototype of mbedtls_psa_external_get_random() in a public header. Signed-off-by: Gilles Peskine <[email protected]>
psa_crypto must be able to convert error codes even from modules that it doesn't call directly. Signed-off-by: Gilles Peskine <[email protected]>
gilles-peskine-arm
force-pushed
the
psa-external-random
branch
from
ab30977 to
f82acaa
Compare
|
I rebased on top of Next up: another rebase for |
Implement support for MBEDTLS_PSA_CRYPTO_EXTERNAL_RNG. For test purposes, write an implementation that uses libc rand(). Signed-off-by: Gilles Peskine <[email protected]>
Support using HMAC_DRBG instead of CTR_DRBG in the PSA subsystem. Use HMAC_DRBG if CTR_DRBG is available. Choose between SHA-256 and SHA-512 based on availability. Signed-off-by: Gilles Peskine <[email protected]>
Allow the user to configure PSA to use HMAC_DRBG even if CTR_DRBG is available, or to explicitly select the hash algorithm to use for HMAC_DRBG, by setting MBEDTLS_PSA_HMAC_DRBG_MD_TYPE in config.h. Signed-off-by: Gilles Peskine <[email protected]>
Signed-off-by: Gilles Peskine <[email protected]>
We generate the Doxygen documentation in a configuration where part of config.h is excluded. See Mbed-TLS#520 ``` /var/lib/build/include/mbedtls/config.h:3635: warning: documentation for unknown define MBEDTLS_PSA_HMAC_DRBG_MD_TYPE found. ``` This is a more general issue and fixing it is out of scope of my current work. Therefore, just do something simple to silence Doxygen, and never mind that this causes the documentation of `MBEDTLS_PSA_HMAC_DRBG_MD_TYPE` to be omitted from the rendered documentation. We'll fix that when we fix all the configuration macros with a similar problem. Signed-off-by: Gilles Peskine <[email protected]>
Signed-off-by: Gilles Peskine <[email protected]>
Signed-off-by: Gilles Peskine <[email protected]>
daverodgman
removed
the
needs-reviewer
Add and document PSA_ERROR_INSUFFICIENT_ENTROPY. Signed-off-by: Gilles Peskine <[email protected]>
gilles-peskine-arm
force-pushed
the
psa-external-random
branch
from
38c6080 to
e995b9b
Compare
|
Coming back at this from my "review requested" list, I can see that both Ronald and Gabor already did some review on this PR, while I don't think I did yet, so I'm removing myself from the reviewers list. If it turns out that Ronald or Gabor's no longer available, or that for some reason a review from me specifically was desired, please let me know. |
Hide the obtention of the pointer to the RNG state behind a macro. To make it possible to use this macro in a module other than psa_crypto.c, which will happen in the future, make sure that the definition of the macro does not reference internal variables of psa_crypto.c. For this purpose, in the internal-DRBG case, export a symbol containing the address of the DRBG state. When the RNG state is a pointer a DRBG state, just keep this pointer in a variable: there's no need to store a pointer to a larger structure. Signed-off-by: Gilles Peskine <[email protected]>
In the external RNG case, don't make mbedtls_psa_get_random() a static inline function: this would likely result in identical instances of this function in every module that uses it. Instead, make it a single function with external linkage. In the non-external case, instead of a trivial wrapper function, make mbedtls_psa_get_random a constant pointer to whichever DRBG function is being used. Signed-off-by: Gilles Peskine <[email protected]>
Make it clear that this is an abstraction of the random generator abstraction, and not an abstraction of the PSA random generator. mbedtls_psa_get_random and MBEDTLS_PSA_RANDOM_STATE are public-facing definitions and will be moved in a subsequent commit. Signed-off-by: Gilles Peskine <[email protected]>
Signed-off-by: Gilles Peskine <[email protected]>
ronald-cron-arm
requested changes
Jan 4, 2021
Signed-off-by: Gilles Peskine <[email protected]>
Make the code slightly more readable and slightly smaller. Signed-off-by: Gilles Peskine <[email protected]>
ronald-cron-arm
requested changes
Jan 5, 2021
library/psa_crypto.c
Outdated
| MBEDTLS_PSA_RANDOM_MAX_REQUEST : | ||
| output_size ); | ||
| ret = mbedtls_psa_get_random( MBEDTLS_PSA_RANDOM_STATE, | ||
| output, request_size ); |
There was a problem hiding this comment.
We have to get out of the loop if ret is not equal to zero.
If a call to mbedtls_psa_get_random other than the last one failed, this went undetected. Signed-off-by: Gilles Peskine <[email protected]>
Signed-off-by: Gilles Peskine <[email protected]>
ronald-cron-arm
approved these changes
Jan 5, 2021
| * The PSA crypto subsystem can now use HMAC_DRBG instead of CTR_DRBG. | ||
| CTR_DRBG is used by default if it is available, but you can override | ||
| this choice by setting MBEDTLS_PSA_HMAC_DRBG_MD_TYPE at compile time. | ||
| Fix #3354. |
There was a problem hiding this comment.
By the way, I'm deliberately not mentioning external RNG support in the changelog in this PR. I'll add a changelog entry in the follow-up that makes an external PSA RNG usable from TLS code.
gabor-mezei-arm
approved these changes
Jan 6, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Allow the PSA subsystem to use a random generator other than what is provided by
ctr_drbg.c+entropy.c.psa_crypto.c.add_entropy) as specified in PSA: Specification for random generation and entropy drivers #3882 (only the C part, not the JSON-to-C transpilation). This fixes Break the dependency of psa_crypto on software AES #3675 with the DRBG abstraction method. To use a DRBG driver, enableMBEDTLS_PSA_CRYPTO_EXTERNAL_RNGand implementmbedtls_psa_external_get_random; this allows you to disableMBEDTLS_CTR_DRBG_C,MBEDTLS_AES_CandMBEDTLS_ENTROPY_C(assuming no application code requires these modules).Future work, out of scope of this PR:
MBEDTLS_USE_PSA_CRYPTO) to use the PSA RNG no matter how it's implemented. (Define an RNG instance based on the PSA RNG #3883).add_entropy.get_entropyinterface.