Commit Graph

5520 Commits

Author SHA1 Message Date
Gilles Peskine
28cd41676e Fix possible error code mangling in psa_mac_verify_finish
If psa_mac_finish_internal fails (which can only happen due to bad
parameters or hardware problem), the error code was converted to
PSA_ERROR_INVALID_SIGNATURE if the uninitialized stack variable
actual_mac happened to contain the expected MAC. This is a minor bug
but it may be possible to leverage it as part of a longer attack path
in some scenarios.

Reported externally. Found by static analysis.
2020-01-20 16:31:06 +01:00
Gilles Peskine
180850a229
Merge pull request #291 from gilles-peskine-arm/ctr_drbg-test_aes_128
Test MBEDTLS_CTR_DRBG_USE_128_BIT_KEY
2019-12-20 10:43:44 +01:00
Jaeden Amero
795c6bab62
Merge pull request #323 from yanesca/iotcrypt-942-initialise-return-values
Initialise return values to an error
2019-12-19 10:24:22 +00:00
Gilles Peskine
4cd3277656 Factor common code of psa_import_ec_{public,private}_key 2019-12-12 09:00:27 +01:00
Gilles Peskine
81f7909497
Merge pull request #325 from gilles-peskine-arm/psa-sign_hash
Rename psa_asymmetric_{sign_verify} to psa_{sign,verify}_hash
2019-12-06 10:10:14 +01:00
Janos Follath
24eed8d2d2 Initialise return values to an error
Initialising the return values to and error is best practice and makes
the library more robust.
2019-12-03 16:07:18 +00:00
Janos Follath
a13b905d8d Map the new Mbed TLS error value in PSA 2019-12-03 16:03:11 +00:00
Janos Follath
60f6b64b8f Add two error codes to the Error module
One of the error codes was already reserved, this commit just makes it
explicit. The other one is a new error code for initializing return
values in the library: `MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED` should
not be returned by the library. If it is returned, then it is surely a
bug in the library or somebody is tampering with the device.
2019-12-03 16:01:06 +00:00
Gilles Peskine
7bb1a7e0bf
Merge pull request #307 from msopiha-linaro/development
Add ASN.1 ENUMERATED tag support
2019-11-28 10:20:18 +01:00
Gilles Peskine
8f4df81506
Merge pull request #293 from gilles-peskine-arm/entropy-min
Always gather MBEDTLS_ENTROPY_BLOCK_SIZE bytes of entropy
2019-11-26 18:43:45 +01:00
Gilles Peskine
89d8c5c447 Rename some signature-related identifiers
Rename some macros and functions related to signature which are
changing as part of the addition of psa_sign_message and
psa_verify_message.

perl -i -pe '%t = (
PSA_KEY_USAGE_SIGN => PSA_KEY_USAGE_SIGN_HASH,
PSA_KEY_USAGE_VERIFY => PSA_KEY_USAGE_VERIFY_HASH,
PSA_ASYMMETRIC_SIGNATURE_MAX_SIZE => PSA_SIGNATURE_MAX_SIZE,
PSA_ASYMMETRIC_SIGN_OUTPUT_SIZE => PSA_SIGN_OUTPUT_SIZE,
psa_asymmetric_sign => psa_sign_hash,
psa_asymmetric_verify => psa_verify_hash,
); s/\b(@{[join("|", keys %t)]})\b/$t{$1}/ge' $(git ls-files . ':!:**/crypto_compat.h')
2019-11-26 18:20:59 +01:00
Gilles Peskine
02e79a4e43 MBEDTLS_CTR_DRBG_USE_128_BIT_KEY: add selftest data
In the CTR_DRBG module, add selftest data for when
MBEDTLS_CTR_DRBG_USE_128_BIT_KEY is enabled.

I generated the test data by running our own code. This is ok because
we have other tests that ensure that the algorithm is implemented
correctly.

This makes programs/self/selftest pass when
MBEDTLS_CTR_DRBG_USE_128_BIT_KEY is enabled.
2019-11-21 13:49:20 +01:00
Gilles Peskine
2e6cbcd931
Merge pull request #308 from tempesta-tech/ak-mpi_div_mpi-opt
Small performance improvement of mbedtls_mpi_div_mpi()
2019-11-19 11:32:29 +01:00
Gilles Peskine
9afbfdc833 Merge remote-tracking branch 'myfork-public/development' into merge-crypto-development-20191115
First deal with deleted files.

* Files deleted by us: keep them deleted.
* Files deleted by them, whether modified by us or not: keep our version.

```
git rm $(git status -s | sed -n 's/^DU //p')
git reset -- $(git status -s | sed -n 's/^D  //p')
git checkout -- $(git status -s | sed -n 's/^ D //p')
git add -- $(git status -s | sed -n 's/^UD //p')
```

Individual files with conflicts:

* `3rdparty/everest/library/Hacl_Curve25519_joined.c`: spurious conflict because git mistakenly identified this file as a rename. Keep our version.
* `README.md`: conflict due to their change in a paragraph that doesn't exist in our version. Keep our version of this paragraph.
* `docs/architecture/Makefile`: near-identical additions. Adapt the definition of `all_markdown` and include the clean target.
* `doxygen/input/docs_mainpage.h`: conflict in the version number. Keep our version number.
* `include/mbedtls/config.h`: two delete/modify conflicts. Keep the removed chunks out.
* `library/CMakeLists.txt`: discard all their changes as they are not relevant.
* `library/Makefile`:
    * Discard the added chunk about the crypto submodule starting with `INCLUDING_FROM_MBEDTLS:=1`.
    * delete/modify: keep the removed chunk out.
    * library build: This is almost delete/modify. Their changes are mostly not applicable. Do keep the `libmbedcrypto.$(DLEXT): | libmbedcrypto.a` order dependency.
    * `.c.o`: `-o` was added on both sides but in a different place. Change to their place.
* `library/error.c`: to be regenerated.
* `library/version_features.c`: to be regenerated.
* `programs/Makefile`: Most of the changes are not relevant. The one relevant change is in the `clean` target for Windows; adapt it by removing `/S` from our version.
* `programs/test/query_config.c`: to be regenerated.
* `scripts/config.py`: added in parallel on both sides. Keep our version.
* `scripts/footprint.sh`: parallel changes. Keep our version.
* `scripts/generate_visualc_files.pl`: one delete/modify conflict. Keep the removed chunks out.
* `tests/Makefile`: discard all of their changes.
* `tests/scripts/all.sh`:
    * `pre_initialize_variables` add `append_outcome`: add it.
    * `pre_initialize_variables` add `ASAN_CFLAGS`: already there, keep our version.
    * `pre_parse_command_line` add `--no-append-outcome`: add it.
    * `pre_parse_command_line` add `--outcome-file`: add it.
    * `pre_print_configuration`: add `MBEDTLS_TEST_OUTCOME_FILE`.
    * Several changes in SSL-specific components: keep our version without them.
    * Several changes where `config.pl` was changed to `config.py` and there was an adjacent difference: keep our version.
    * Changes regarding the inclusion of `MBEDTLS_MEMORY_xxx`: ignore them here, they will be normalized in a subsequent commit.
    * `component_test_full_cmake_gcc_asan`: add it without the TLS tests.
    * `component_test_no_use_psa_crypto_full_cmake_asan`: keep the fixed `msg`, discard other changes.
    * `component_test_memory_buffer_allocator_backtrace`, `component_test_memory_buffer_allocator`: add them without the TLS tests.
    * `component_test_m32_everest`: added in parallel on both sides. Keep our version.
* `tests/scripts/check-names.sh`, `tests/scripts/list-enum-consts.pl`, `tests/scripts/list-identifiers.sh`, ``tests/scripts/list-macros.sh`: discard all of their changes.
* `tests/scripts/test-ref-configs.pl`: the change in the conflict is not relevant, so keep our version there.
* `visualc/VS2010/*.vcxproj`: to be regenerated.

Regenerate files:

```
scripts/generate_visualc_files.pl
git add visualc/VS2010/*.vcxproj
scripts/generate_errors.pl
git add library/error.c
scripts/generate_features.pl
git add library/version_features.c
scripts/generate_query_config.pl
git add programs/test/query_config.c
```

Rejected changes in non-conflicting files:

* `CMakeLists.txt`: discard their addition which has already been side-ported.
* `doxygen/mbedtls.doxyfile`: keep the version number change. Discard the changes related to `../crypto` paths.

Keep the following changes after examination:

* `.travis.yml`: all of their changes are relevant.
* `include/mbedtls/error.h`: do keep their changes. Even though Crypto doesn't use TLS errors, it must not encroach on TLS's allocated numbers.
* `tests/scripts/check-test-cases.py`: keep the code dealing with `ssl-opt.sh`. It works correctly when the file is not present.
2019-11-15 11:47:14 +01:00
Jaeden Amero
df8185eae3
Merge pull request #2923 from Patater/reduce-ram-pem-csr
x509write_csr: Reduce stack usage of mbedtls_x509write_csr_pem()
2019-11-14 16:39:48 +00:00
Gilles Peskine
b9b5f493d3
Merge pull request #2920 from gilles-peskine-arm/pk_signature_max_size-tls
Use MBEDTLS_PK_SIGNATURE_MAX_SIZE
2019-11-14 10:22:32 +01:00
Gilles Peskine
bf88780e64 Use MBEDTLS_PK_SIGNATURE_MAX_SIZE in X.509
Use the constant that is now provided by the crypto submodule instead
of rolling our own definition which is not correct in all cases.
2019-11-13 15:32:11 +01:00
Jaeden Amero
6ffac75995 x509write_csr: Reduce stack usage of mbedtls_x509write_csr_pem()
Using 4096 bytes of stack for the temporary buffer used for holding a
throw-away DER-formatted CSR limits the portability of generating
certificate signing requests to only devices with lots of stack space.
To increase portability, use the mbedtls_pem_write_buffer() in-place
capability instead, using the same buffer for input and output. This
works since the DER encoding for some given data is always smaller than
that same data PEM-encoded.

PEM format is desirable to use even on stack-constrained devices as the
format is easy to work with (for example, copy-pasting from a tiny
device's serial console output, for CSRs generated on tiny devices
without the private key leaving said tiny device).
2019-11-13 14:29:38 +00:00
Gilles Peskine
f0d87001d0
Merge pull request #315 from gilles-peskine-arm/pk_signature_max_size
Define MBEDTLS_PK_SIGNATURE_MAX_SIZE
2019-11-13 12:13:57 +01:00
Gilles Peskine
5bcb24b56e Fix output buffer length check in pk_opaque_sign_wrap 2019-11-13 10:57:59 +01:00
Gilles Peskine
7a9899f1bd
Merge pull request #284 from gilles-peskine-arm/bk-warning-fixes-crypto
Fix some possibly-undefined variable warnings
2019-11-12 19:45:13 +01:00
Gilles Peskine
cb0101ff33
Merge pull request #298 from gilles-peskine-arm/config-symmetric-only
Test a build without any asymmetric cryptography
2019-11-12 19:37:13 +01:00
Gilles Peskine
f48d6f2320 Add sanity checks for the mbedtls_pk_sign output size
mbedtls_pk_sign does not take the size of its output buffer as a
parameter. We guarantee that MBEDTLS_PK_SIGNATURE_MAX_SIZE is enough.
For RSA and ECDSA signatures made in software, this is ensured by the
way MBEDTLS_PK_SIGNATURE_MAX_SIZE is defined at compile time. For
signatures made through RSA-alt and PSA, this is not guaranteed
robustly at compile time, but we can test it at runtime, so do that.
2019-11-12 13:21:53 +01:00
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
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
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
Alexander K
d19a193738 Fix code review comments:
1. variable name accoriding to the Mbed TLS coding style;
2. add a comment explaining safety of the optimization;
3. safer T2 initialization and memory zeroing on the function exit;
2019-11-01 18:20:42 +03:00
Mykhailo Sopiha
20180ca919 Add ASN.1 ENUMERATED tag support
Add ASN.1 ENUMERATED [1] tag to supported tag list.

1. https://tools.ietf.org/html/rfc3641#page-8

Signed-off-by: Mykhailo Sopiha <mykhailo.sopiha@linaro.org>
2019-10-31 19:17:26 +02:00
Alexander K
35d6d46169 Small performance improvement of mbedtls_mpi_div_mpi():
1. don't use dynamic allocator for fixed size T2;
2. move T2 initialization out of the inner loop.
2019-10-31 14:46:45 +03:00
Arto Kinnunen
7f8089b2ec Fix mbedtls_ssl_check_record usage with ext buf
Record checking fails if mbedtls_ssl_check_record() is called with
external buffer. Received record sequence number is available in the
incoming record but it is not available in the ssl contexts `in_ctr`-
variable that is used when decoding the sequence number.

To fix the problem, temporarily update ssl context `in_ctr` to
point to the received record header and restore value later.
2019-10-29 13:51:37 +02: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
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
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
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
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
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
b16841ee69 Fixed -Wunused warnings when building without asymmetric crypto 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
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
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
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
85485c7338 Always gather MBEDTLS_ENTROPY_BLOCK_SIZE bytes of entropy
mbedtls_entropy_func returns up to MBEDTLS_ENTROPY_BLOCK_SIZE bytes.
This is the output of a hash function and does not indicate how many
bytes of entropy went into the hash computation.

Enforce that mbedtls_entropy_func gathers a total of
MBEDTLS_ENTROPY_BLOCK_SIZE bytes or more from strong sources. Weak
sources don't count for this calculation. This is complementary to the
per-source threshold mechanism.

In particular, we define system sources with a threshold of 32. But
when using SHA-512 for the entropy accumulator,
MBEDTLS_ENTROPY_BLOCK_SIZE = 64, so users can expect 64 bytes' worth
of entropy. Before, you only got 64 bytes of entropy if there were two
sources. Now you get 64 bytes of entropy even with a single source
with a threshold of 32.
2019-10-09 13:53:47 +02:00