Commit Graph

11850 Commits

Author SHA1 Message Date
Gilles Peskine
2ad5e45de6
Merge pull request #314 from gilles-peskine-arm/pkwrite_ECPrivateKey_size-crypto
Fix pk_write with EC key to use a constant size for the private value
2019-11-08 19:30:51 +01:00
Gilles Peskine
c82ed6fbf4
Merge pull request #317 from Patater/reduce-ram-rsa
getting_started: Make it clear that keys are passed in
2019-11-08 17:44:10 +01:00
Jaeden Amero
b14a4ff840
Merge pull request #316 from Patater/stop-reentrant-transaction
Stop transactions from being reentrant
2019-11-08 14:59:39 +00:00
Jaeden Amero
fbdf150080 getting_started: Make it clear that keys are passed in
It was not obvious before that `AES_KEY` and `RSA_KEY` were shorthand
for key material. A user copy pasting the code snippet would run into a
compilation error if they didn't realize this. Make it more obvious that
key material must come from somewhere external by making the snippets
which use global keys into functions that take a key as a parameter.
2019-11-08 10:22:15 +00:00
Gilles Peskine
2700cfbdd5 Fix pk_write with an EC key to write a constant-length private value
When writing a private EC key, use a constant size for the private
value, as specified in RFC 5915. Previously, the value was written
as an ASN.1 INTEGER, which caused the size of the key to leak
about 1 bit of information on average, and could cause the value to be
1 byte too large for the output buffer.
2019-11-05 15:32:53 +01:00
Gilles Peskine
c212166171 pk_write test cases with short/long private key
Add pk_write test cases where the ASN.1 INTEGER encoding of the
private value would not have the mandatory size for the OCTET STRING
that contains the value.

ec_256_long_prv.pem is a random secp256r1 private key, selected so
that the private value is >= 2^255, i.e. the top bit of the first byte
is set (which would cause the INTEGER encoding to have an extra
leading 0 byte).

ec_521_short_prv.pem is a random secp521r1 private key, selected so
that the private value is < 2^519, i.e. the first byte is 0 and the
top bit of the second byte is 0 (which would cause the INTEGER
encoding to have one less 0 byte at the start).
2019-11-05 15:32:53 +01:00
Gilles Peskine
22589f0a72
Merge pull request #305 from gilles-peskine-arm/ctr_drbg-grab_nonce_from_entropy-set_nonce_length
CTR_DRBG: grab a nonce from the entropy source if needed
2019-11-04 10:39:42 +01:00
Gilles Peskine
08c674dfe3
Merge pull request #288 from gilles-peskine-arm/psa-ecdsa_longer_hash
Add ECDSA tests with hash and key of different lengths
2019-10-31 17:03:28 +01:00
Gilles Peskine
ccde952df0
Merge pull request #259 from k-stachowiak/bounds-check-asn1-len
Check `len` against buffers size upper bound in PSA tests
2019-10-29 17:47:47 +01:00
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
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
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
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
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
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
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
Gilles Peskine
321adb297c ASN1 tests: Match "Empty INTEGER" with the actual library behavior
mbedtls_asn1_get_int() and mbedtls_asn1_get_mpi() behave differently
on an empty INTEGER (0200). Don't change the library behavior for now
because this might break interoperability in some applications. Write
a test function that matches the library behavior.
2019-10-10 19:18:21 +02:00
Gilles Peskine
03c165e1e1 Fix the build and the tests when MBEDTLS_BIGNUM_C is unset
When the asn1parse module is enabled but the bignum module is
disabled, the asn1parse test suite did not work. Fix this.

* Fix a syntax error in get_integer() (label immediately followed by a
  closing brace).
* Fix an unused variable in get_integer().
* Fix `TEST_ASSERT( *p == q );` in nested_parse() failing because `*p`
  was not set.
* Fix nested_parse() not outputting the length of what it parsed.
2019-10-10 19:15:18 +02:00
Gilles Peskine
36029387de
Merge pull request #285 from gilles-peskine-arm/psa-se_driver-validate_save_persistent
SE driver: make persistent data work
2019-10-09 18:35:33 +02:00
Gilles Peskine
1bbe284d63
Merge pull request #272 from adrianlshaw/document_old_algs
Insert doxygen comments on old algorithms so they appear in PSA documentation
2019-10-09 17:18:11 +02:00
Gilles Peskine
43326f0d1e Change PSA_DH_GROUP_CUSTOM to not be in the vendor-defined range 2019-10-09 16:43:39 +02:00
Gilles Peskine
04129a0d96 Update slot management tests now that {close,destroy}_key(0) succeed 2019-10-09 16:23:49 +02:00
Gilles Peskine
cb25cdd53a Add ECDSA tests with hash and key of different lengths
Add some ECDSA test cases where the hash is shorter or longer than the
key length, to check that the API doesn't enforce a relationship
between the two.

For the sign_deterministic tests, the keys are
tests/data_files/ec_256_prv.pem and tests/data_files/ec_384_prv.pem
and the signatures were obtained with Python Cryptodome:

from binascii import hexlify, unhexlify
from Crypto.Hash import SHA256, SHA384
from Crypto.PublicKey import ECC
from Crypto.Signature import DSS
k2 = ECC.import_key(unhexlify("3077020101042049c9a8c18c4b885638c431cf1df1c994131609b580d4fd43a0cab17db2f13eeea00a06082a8648ce3d030107a144034200047772656f814b399279d5e1f1781fac6f099a3c5ca1b0e35351834b08b65e0b572590cdaf8f769361bcf34acfc11e5e074e8426bdde04be6e653945449617de45"))
SHA384.new(b'hello').hexdigest()
hexlify(DSS.new(k2, 'deterministic-rfc6979').sign(SHA384.new(b'hello')))
k3 = ECC.import_key(unhexlify("3081a402010104303f5d8d9be280b5696cc5cc9f94cf8af7e6b61dd6592b2ab2b3a4c607450417ec327dcdcaed7c10053d719a0574f0a76aa00706052b81040022a16403620004d9c662b50ba29ca47990450e043aeaf4f0c69b15676d112f622a71c93059af999691c5680d2b44d111579db12f4a413a2ed5c45fcfb67b5b63e00b91ebe59d09a6b1ac2c0c4282aa12317ed5914f999bc488bb132e8342cc36f2ca5e3379c747"))
SHA256.new(b'hello').hexdigest()
hexlify(DSS.new(k3, 'deterministic-rfc6979').sign(SHA256.new(b'hello')))
2019-10-08 16:05:02 +02:00