Commit Graph

12624 Commits

Author SHA1 Message Date
Gilles Peskine
bd326f93d4 Note that mbedtls_ctr_drbg_seed() must not be called twice
You can't reuse a CTR_DRBG context without free()ing it and
re-init()ing it. This generally happened to work, but was never
guaranteed. It could have failed with alternative implementations of
the AES module because mbedtls_ctr_drbg_seed() calls
mbedtls_aes_init() on a context which is already initialized if
mbedtls_ctr_drbg_seed() hasn't been called before, plausibly causing a
memory leak. Since the addition of mbedtls_ctr_drbg_set_nonce_len(),
the second call to mbedtls_ctr_drbg_seed() uses a nonsensical value as
the entropy nonce length.

Calling free() and seed() with no intervening init fails when
MBEDTLS_THREADING_C is enabled and all-bits-zero is not a valid mutex
representation.
2019-10-28 21:05:34 +01:00
Gilles Peskine
f0ebbfb3fc Fix CTR_DRBG benchmark
You can't reuse a CTR_DRBG context without free()ing it and
re-init()ing. This generally happened to work, but was never
guaranteed. It could have failed with alternative implementations of
the AES module because mbedtls_ctr_drbg_seed() calls
mbedtls_aes_init() on a context which is already initialized if
mbedtls_ctr_drbg_seed() hasn't been called before, plausibly causing a
memory leak. Calling free() and seed() with no intervening init fails
when MBEDTLS_THREADING_C is enabled and all-bits-zero is not a valid
mutex representation. So add the missing free() and init().
2019-10-28 21:03:52 +01:00
irwir
6c0da64094 Shorter version of mbedtls_ssl_send_fatal_handshake_failure 2019-10-28 19:54:04 +03:00
Jaeden Amero
2ce22a5079 Stop transactions from being reentrant
We want to explicitly disallow creating new transactions when a
transaction is already in progress. However, we were incorrectly
checking for the existence of the injected entropy file before
continuing with creating a transaction. This meant we could have a
transaction already in progress and would be able to still create a new
transaction. It also meant we couldn't start a new transaction if any
entropy had been injected. Check the transaction file instead of the
injected entropy file in order to prevent multiple concurrent
transactions.
2019-10-28 15:25:10 +00:00
Gilles Peskine
0eaf49c9be
Merge pull request #304 from gilles-peskine-arm/asan-test-fail-crypto
Make sure Asan failures are detected in 'make test'
2019-10-24 11:06:53 +02:00
Gilles Peskine
69971662bf CTR_DRBG: define a constant for the default entropy nonce length
The default entropy nonce length is either zero or nonzero depending
on the desired security strength and the entropy length.

The implementation calculates the actual entropy nonce length from the
actual entropy length, and therefore it doesn't need a constant that
indicates the default entropy nonce length. A portable application may
be interested in this constant, however. And our test code could
definitely use it.

Define a constant MBEDTLS_CTR_DRBG_ENTROPY_NONCE_LEN and use it in
test code. Previously, test_suite_ctr_drbg had knowledge about the
default entropy nonce length built in and test_suite_psa_crypto_init
failed. Now both use MBEDTLS_CTR_DRBG_ENTROPY_NONCE_LEN.

This change means that the test ctr_drbg_entropy_usage no longer
validates that the default entropy nonce length is sensible. So add a
new test that checks that the default entropy length and the default
entropy nonce length are sufficient to ensure the expected security
strength.
2019-10-23 19:47:05 +02:00
Gilles Peskine
e9a3454e09 CTR_DRBG: grab a nonce from the entropy source if needed
Change the default entropy nonce length to be nonzero in some cases.
Specifically, the default nonce length is now set in such a way that
the entropy input during the initial seeding always contains enough
entropy to achieve the maximum possible security strength per
NIST SP 800-90A given the key size and entropy length.

If MBEDTLS_CTR_DRBG_ENTROPY_LEN is kept to its default value,
mbedtls_ctr_drbg_seed() now grabs extra entropy for a nonce if
MBEDTLS_CTR_DRBG_USE_128_BIT_KEY is disabled and either
MBEDTLS_ENTROPY_FORCE_SHA256 is enabled or MBEDTLS_SHA512_C is
disabled. If MBEDTLS_CTR_DRBG_USE_128_BIT_KEY is enabled, or if
the entropy module uses SHA-512, then the default value of
MBEDTLS_CTR_DRBG_ENTROPY_LEN does not require a second call to the
entropy function to achieve the maximum security strength.

This choice of default nonce size guarantees NIST compliance with the
maximum security strength while keeping backward compatibility and
performance high: in configurations that do not require grabbing more
entropy, the code will not grab more entropy than before.
2019-10-23 19:46:57 +02:00
Gilles Peskine
0ed378aa02 CTR_DRBG: explicitly set entropy_nonce_len=0 when desired
No behavior change. Prepare for a future version that will set the
entropy nonce length to a nonzero value by default.
2019-10-23 19:46:56 +02:00
Gilles Peskine
c949de06ec Test mbedtls_ctr_drbg_set_nonce_len
Test mbedtls_ctr_drbg_set_nonce_len (good cases only, which is in
keeping with the coverage of other functions).
2019-10-23 19:46:56 +02:00
Gilles Peskine
4d2d4ff9b0 HMAC_DRBG entropy usage: test the exact amount of consumed entropy 2019-10-23 19:46:56 +02:00
Gilles Peskine
58b56ce444 CTR_DRBG entropy usage: test the exact amount of consumed entropy 2019-10-23 19:46:56 +02:00
Gilles Peskine
97f59ab527 CTR_DRBG: add the possibility of grabbing entropy for a nonce
Add a new function mbedtls_ctr_drbg_set_nonce_len() which configures
the DRBG instance to call f_entropy a second time during the initial
seeding to grab a nonce.

The default nonce length is 0, so there is no behavior change unless
the user calls the new function.
2019-10-23 19:46:56 +02:00
Gilles Peskine
9be5098061 CTR_DRBG: add the possibility of grabbing entropy for a nonce
Add a new function mbedtls_ctr_drbg_set_nonce_len() which configures
the DRBG instance to call f_entropy a second time during the initial
seeding to grab a nonce.

The default nonce length is 0, so there is no behavior change unless
the user calls the new function.
2019-10-23 19:46:56 +02:00
Gilles Peskine
dbd3f7c68d mbedtls_ctr_drbg_reseed: Minor readability improvement
No semantic change.
2019-10-23 19:46:56 +02:00
Gilles Peskine
c0ace355a4 mbedtls_ctr_drbg_context: minor documentation improvements 2019-10-23 19:46:56 +02:00
Gilles Peskine
460988a472 fixup! CTR_DRBG: support set_entropy_len() before seed()
Remove a comment that documented a now-removed restriction.
2019-10-23 19:46:56 +02:00
Gilles Peskine
379561feff fixup! CTR_DRBG: support set_entropy_len() before seed()
Update a comment that referred to a now-removed function.
2019-10-23 19:46:56 +02:00
Gilles Peskine
9d3baea439 fixup! HMAC_DRBG: support set_entropy_len() before seed() 2019-10-23 19:46:55 +02:00
Jaeden Amero
b23abcb38d
Merge pull request #2898 from gilles-peskine-arm/asan-test-fail-development
Make sure Asan failures are detected in 'make test'
2019-10-22 16:30:28 +01:00
k-stachowiak
67badb4451 Secure array index in its bounds 2019-10-22 13:25:06 +02:00
Gilles Peskine
ce35cb3cc7 'make test' must fail if Asan fails
When running 'make test' with GNU make, if a test suite program
displays "PASSED", this was automatically counted as a pass. This
would in particular count as passing:
* A test suite with the substring "PASSED" in a test description.
* A test suite where all the test cases succeeded, but the final
  cleanup failed, in particular if a sanitizer reported a memory leak.

Use the test executable's return status instead to determine whether
the test suite passed. It's always 0 on PASSED unless the executable's
cleanup code fails, and it's never 0 on any failure.

Fix ARMmbed/mbed-crypto#303
2019-10-21 20:10:21 +02:00
Gilles Peskine
5ca393f7b8 Asan make builds: avoid sanitizer recovery
Some sanitizers default to displaying an error message and recovering.
This could result in a test being recorded as passing despite a
complaint from the sanitizer. Turn off sanitizer recovery to avoid
this risk.
2019-10-21 20:10:12 +02:00
Gilles Peskine
2558d30f93 Use UBsan in addition to Asan with 'make test'
When building with make with the address sanitizer enabled, also
enable the undefined behavior sanitizer.
2019-10-21 20:09:22 +02:00
Gilles Peskine
8fd5942229 Unify ASan options in make builds
Use a common set of options when building with Asan without CMake.
2019-10-21 20:09:13 +02:00
Gilles Peskine
54d1937433 Fix memory leak in some SE HAL tests 2019-10-21 19:18:22 +02:00
Gilles Peskine
8b5389f360 'make test' must fail if Asan fails
When running 'make test' with GNU make, if a test suite program
displays "PASSED", this was automatically counted as a pass. This
would in particular count as passing:
* A test suite with the substring "PASSED" in a test description.
* A test suite where all the test cases succeeded, but the final
  cleanup failed, in particular if a sanitizer reported a memory leak.

Use the test executable's return status instead to determine whether
the test suite passed. It's always 0 on PASSED unless the executable's
cleanup code fails, and it's never 0 on any failure.

Fix ARMmbed/mbed-crypto#303
2019-10-21 19:17:42 +02:00
Gilles Peskine
bfeed663d2 Asan make builds: avoid sanitizer recovery
Some sanitizers default to displaying an error message and recovering.
This could result in a test being recorded as passing despite a
complaint from the sanitizer. Turn off sanitizer recovery to avoid
this risk.
2019-10-21 19:08:01 +02:00
Gilles Peskine
004206c7f5 Unify ASan options in make builds
Use a common set of options when building with Asan without CMake.
2019-10-21 17:13:45 +02:00
Gilles Peskine
dd61a2e3b8 Use the intended configuration in component_test_se_full 2019-10-21 15:58:20 +02:00
Gilles Peskine
ccfc5eaa81 Fix memory leak in some SE HAL tests 2019-10-21 15:57:51 +02:00
Jaeden Amero
b1c7197166
Merge pull request #299 from gilles-peskine-arm/drbg-set_entropy_len
Allow xxx_drbg_set_entropy_len before xxx_drbg_seed
2019-10-18 15:39:03 +01:00
Jaeden Amero
719ae74253
Merge pull request #2884 from AndrzejKurek/iotssl-2886-fix-parallel-make-jobs-2
Fix parallel make jobs for shared target
2019-10-18 14:18:55 +01:00
Janos Follath
247c4d3c88 ECDSA: Fix side channel vulnerability
The blinding applied to the scalar before modular inversion is
inadequate. Bignum is not constant time/constant trace, side channel
attacks can retrieve the blinded value, factor it (it is smaller than
RSA keys and not guaranteed to have only large prime factors). Then the
key can be recovered by brute force.

Reducing the blinded value makes factoring useless because the adversary
can only recover pk*t+z*N instead of pk*t.
2019-10-17 10:18:51 +01:00
Gilles Peskine
150d577780
Merge pull request #292 from gilles-peskine-arm/psa-destroy_0
Make psa_close_key(0) and psa_destroy_key(0) succeed
2019-10-14 11:21:54 +02:00
Jaeden Amero
1b717c9278
Merge pull request #2887 from gilles-peskine-arm/update-crypto-20191011
Update crypto submodule
2019-10-14 09:19:07 +01:00
Gilles Peskine
581bfcfc96 Create seedfile in test-ref-configs.pl
config-symmetric-only.h enables MBEDTLS_ENTROPY_NV_SEED so it needs a
seedfile. Create it in test-ref-configs.pl so that the script works on
its own, even if it is not invoked by all.sh.
2019-10-11 18:21:08 +02:00
Gilles Peskine
b16841ee69 Fixed -Wunused warnings when building without asymmetric crypto 2019-10-11 18:21:08 +02:00
Gilles Peskine
fec306452b Add a reference configuration without any asymmetric cryptography
Add a reference configuration with most symmetric cryptographic
algorithms enabled, but without any asymmetric cryptography. This
checks that we don't have spurious unexpected dependencies on
asymmetric-only modules such as bignum.

Keep HAVE_ASM disabled because it's platform-specific.

Keep HAVEGE disabled because it's untested and not recommended.

Keep MEMORY_BUFFER_ALLOC out because it isn't related to cryptography
at all and it makes memory sanitizers ineffective.

Keep THREADING disabled because it requires special build options.
2019-10-11 18:21:08 +02:00
Gilles Peskine
50ed86b6b9 CTR_DRBG: support set_entropy_len() before seed()
mbedtls_ctr_drbg_seed() always set the entropy length to the default,
so a call to mbedtls_ctr_drbg_set_entropy_len() before seed() had no
effect. Change this to the more intuitive behavior that
set_entropy_len() sets the entropy length and seed() respects that and
only uses the default entropy length if there was no call to
set_entropy_len().

This removes the need for the test-only function
mbedtls_ctr_drbg_seed_entropy_len(). Just call
mbedtls_ctr_drbg_set_entropy_len() followed by
mbedtls_ctr_drbg_seed(), it works now.
2019-10-11 18:04:12 +02:00
Gilles Peskine
8bf5613336 CTR_DRBG: Don't use functions before they're defined
Move the definitions of mbedtls_ctr_drbg_seed_entropy_len() and
mbedtls_ctr_drbg_seed() to after they are used. This makes the code
easier to read and to maintain.
2019-10-11 18:04:12 +02:00
Gilles Peskine
8f7921ec4b HMAC_DRBG: support set_entropy_len() before seed()
mbedtls_hmac_drbg_seed() always set the entropy length to the default,
so a call to mbedtls_hmac_drbg_set_entropy_len() before seed() had no
effect. Change this to the more intuitive behavior that
set_entropy_len() sets the entropy length and seed() respects that and
only uses the default entropy length if there was no call to
set_entropy_len().
2019-10-11 18:04:12 +02:00
Gilles Peskine
e601625b96 Update crypto submodule
* #272: Insert doxygen comments on old algorithms so they appear in PSA documentation
* #285: SE driver: make persistent data work
* #279: Include IANA reference in the definition of ECC curves and DH groups
* #287: DRBG documentation improvements
* #297: Fix int overflow in mbedtls_asn1_get_int (Credit to OSS-Fuzz)
2019-10-11 17:37:02 +02:00
Gilles Peskine
3cdb3da3a0
Merge pull request #297 from gilles-peskine-arm/asn1_get_int-undefined_shift
Fix int overflow in mbedtls_asn1_get_int
2019-10-11 17:31:16 +02:00
Gilles Peskine
e5e9081b76
Merge pull request #287 from gilles-peskine-arm/ctr_drbg-doc-nist-crypto
DRBG documentation improvements
2019-10-11 16:57:45 +02:00
Jaeden Amero
cb5fa8b4bd
Merge pull request #279 from athoelke/at-fix262
Include IANA reference in the definition of ECC curves and DH groups
2019-10-11 14:03:18 +01:00
Gilles Peskine
b8cde4ec03 Consolidate invalid-handle tests
Consolidate the invalid-handle tests from test_suite_psa_crypto and
test_suite_psa_crypto_slot_management. Start with the code in
test_suite_psa_crypto_slot_management and adapt it to test one invalid
handle value per run of the test function.
2019-10-11 11:44:48 +02:00
Gilles Peskine
37570e8152 mbedtls_asn1_get_int: fix int overflow
Fix a signed int overflow in mbedtls_asn1_get_int() for numbers
between INT_MAX+1 and UINT_MAX (typically 0x80000000..0xffffffff).
This was undefined behavior which in practice would typically have
resulted in an incorrect value, but which may plausibly also have
caused the postcondition (*p == initial<*p> + len) to be violated.

Credit to OSS-Fuzz.
2019-10-10 19:29:27 +02:00
Gilles Peskine
9fd9794d10 mbedtls_asn1_get_int: explain the logic
No behavior change.
2019-10-10 19:27:53 +02:00
Gilles Peskine
0370b1bd7d ASN1 tests: more INTEGER test cases
Test more INTEGER values, especially near the boundary of int (which
is at 2^31-1 on all our officially supported platforms).
2019-10-10 19:25:39 +02:00
Gilles Peskine
970dcbf453 ASN1 tests: Match negative INTEGERs with the actual library behavior
mbedtls_asn1_get_int() and mbedtls_asn1_get_mpi() behave differently
on negative INTEGERs (0200). Don't change the library behavior for now
because this might break interoperability in some applications. Change
the test function to the library behavior.

Fix the test data with negative INTEGERs. These test cases were
previously not run (they were introduced but deliberately deactivated
in 27d806fab4). The test data was
actually wrong: ASN.1 uses two's complement, which has no negative 0,
and some encodings were wrong. Now the tests have correct data, and
the test code rectifies the expected data to match the library
behavior.
2019-10-10 19:21:12 +02:00