Support x_drbg_set_reseed_interval before x_drbg_seed #379

Closed

gavacq wants to merge 3 commits from development into development

gavacq commented

2020-03-01 22:46:06 -06:00

(Migrated from github.com)

Description

Fixes ARMmbed/mbedtls#2927

Summary of change

mbedtls_hmac_drbg_seed() and mbedtls_ctr_drbg_seed() were setting the entropy context reseed_interval to the default value of 10 000 even when set_reseed_interval() had been called prior to seed(). The documentation did not describe the API behaving this way, and it doesn't make sense that seed() would remove any prior preference for reseed_interval so this behavior has been fixed. Now, the value set by set_reseed_interval() will persist through after a call to seed().

Impact of changes

This change would break any application that

set the reseed_interval and expected it to be reset to the default value once they called seed()
set the reseed_interval and was unknowingly using the default value for the lifetime of the DRBG object

The first situation could be dangerous if the set interval was very high and the PRNG was never reseeded. It is an unlikely scenario however because a user that understood the prior behavior would probably not design their application in this way anyways.

In the second situation, this change could expose a vulnerability in the user's application, so this could provide them with a net benefit.

Migration actions required

None

Documentation

Comments have been updated.

Pull request type

[x] Patch update (Bug fix / Target update / Docs update / Test update / Refactor)
[] Feature update (New feature / Functionality change / New API)
[] Major update (Breaking change E.g. Return code change / API behaviour change)

Test results

[] No Tests required for this change (E.g docs only update)
[] Covered by existing mbed-os tests (Greentea or Unittest)
[x] Tests / results supplied as part of this PR

The first commit in the PR modifies *_drbg_entropy_usage() so that set_reseed_interval() is called before seed(). This will fail before including the next two commits which fix the bug in mbedtls_hmac_drbg_seed() and mbedtls_ctr_drbg_seed().

### Description Fixes ARMmbed/mbedtls#2927 ##### Summary of change `mbedtls_hmac_drbg_seed()` and `mbedtls_ctr_drbg_seed()` were setting the entropy context `reseed_interval` to the default value of 10 000 even when `set_reseed_interval()` had been called prior to `seed()`. The documentation did not describe the API behaving this way, and it doesn't make sense that `seed()` would remove any prior preference for `reseed_interval` so this behavior has been fixed. Now, the value set by `set_reseed_interval()` will persist through after a call to `seed()`. ###### Impact of changes This change would break any application that - set the `reseed_interval` and expected it to be reset to the default value once they called `seed()` - set the `reseed_interval` and was unknowingly using the default value for the lifetime of the `DRBG` object The first situation could be dangerous if the set interval was very high and the PRNG was never reseeded. It is an unlikely scenario however because a user that understood the prior behavior would probably not design their application in this way anyways. In the second situation, this change could expose a vulnerability in the user's application, so this could provide them with a net benefit. ###### Migration actions required None ##### Documentation Comments have been updated. ---------------------------------------------------------------------------------------------------------------- ### Pull request type [x] Patch update (Bug fix / Target update / Docs update / Test update / Refactor) [] Feature update (New feature / Functionality change / New API) [] Major update (Breaking change E.g. Return code change / API behaviour change) ---------------------------------------------------------------------------------------------------------------- ### Test results [] No Tests required for this change (E.g docs only update) [] Covered by existing mbed-os tests (Greentea or Unittest) [x] Tests / results supplied as part of this PR The first commit in the PR modifies `*_drbg_entropy_usage()` so that `set_reseed_interval()` is called before `seed()`. This will fail before including the next two commits which fix the bug in `mbedtls_hmac_drbg_seed()` and `mbedtls_ctr_drbg_seed()`.

gilles-peskine-arm commented

2020-03-24 15:45:00 -05:00

(Migrated from github.com)

Also, there are CI failures in tests/scripts/doxygen.sh and tests/scripts/check-files.py.

For legal reasons, we cannot merge this pull request unless you indicate that we are allowed to redistribute your contribution by including a Signed-off-by: line in each commit message. For more information, see https://github.com/ARMmbed/mbed-crypto/issues/371 and [`CONTRIBUTING.md`](https://github.com/ARMmbed/mbed-crypto/blob/development/CONTRIBUTING.md). Also, there are CI failures in `tests/scripts/doxygen.sh` and `tests/scripts/check-files.py`.

gavacq commented

2020-06-03 11:06:28 -05:00

(Migrated from github.com)