Commit graph

16940 commits

Author SHA1 Message Date
Gilles Peskine
4a02cef402 Test restartable ECC with a curve that supports it
The default curve is now Curve25519, which doesn't support restartable ECC.
So run the restartable ECC tests with a curve that does support it. Use
secp256r1 which is required for these tests anyway for the server's
certificate.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-17 21:46:29 +02:00
Gilles Peskine
3b3aa36962 Indicate that the truncation from size_t to int is deliberate
MPI sizes do fit in int. Let MSVC know this conversion is deliberate.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-17 21:46:29 +02:00
Gilles Peskine
799eee65fd Update the expected default curve in ssl-opt.sh
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-17 21:46:29 +02:00
Gilles Peskine
b26696bafb Simplify mbedtls_debug_print_mpi and fix the case of empty bignums
Rewrite mbedtls_debug_print_mpi to be simpler and smaller. Leverage
mbedtls_mpi_bitlen() instead of manually looking for the leading
zeros.

Fix #4608: the old code made an invalid memory dereference when
X->n==0 (freshly initialized bignum with the value 0).

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-17 21:46:29 +02:00
Gilles Peskine
3beb72eeaf Add mbedtls_debug_print_mpi test case for 0
There was already a test case for 0 but with a non-empty representation
(X->n == 1). Add a test case with X->n == 0 (freshly initialized mpi).

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-17 21:46:29 +02:00
Gilles Peskine
c5b9510114 Clarify test case descriptions
Reorder test cases and make their descriptions more explicit. No
change in test data.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-17 21:46:29 +02:00
Gilles Peskine
a28f0f5082 Leave the preference order for hashes unspecified
We don't seem to have strong feelings about this, so allow ourselves to
change the order later.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-17 21:46:29 +02:00
Gilles Peskine
b1940a76ad In TLS, order curves by resource usage, not size
TLS used to prefer larger curves, under the idea that a larger curve has a
higher security strength and is therefore harder to attack. However, brute
force attacks are not a practical concern, so this was not particularly
meaningful. If a curve is considered secure enough to be allowed, then we
might as well use it.

So order curves by resource usage. The exact definition of what this means
is purposefully left open. It may include criteria such as performance and
memory usage. Risk of side channels could be a factor as well, although it
didn't affect the current choice.

The current list happens to exactly correspond to the numbers reported by
one run of the benchmark program for "full handshake/s" on my machine.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-17 21:46:29 +02:00
Gilles Peskine
377c91e1b7 Remove meaningless clause
We stated that curves were listed "in order of preference", but we never
explained what the preference was, so this was not meaningful.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-17 21:46:29 +02:00
Gilles Peskine
5752e599b3 Reduce the default ECP window size
MBEDTLS_ECP_WINDOW_SIZE is a compromise between memory usage (growing based
on the value) and performance (faster with larger values). There are
disminishing returns as the value grows larger. Based on Manuel's benchmarks
recorded in https://github.com/ARMmbed/mbedtls/issues/4127, 4 is a good
compromise point, with larger values bringing little advantage. So reduce
the default from 6 to 4.

Document the default value as in optimized for performance mostly, but don't
document the specific value, so we may change it later or make it
platform-dependent.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-17 21:46:29 +02:00
Gilles Peskine
12b5b38981 Fix "PSA - ECDH with [non-default curve]"
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-17 21:46:14 +02:00
Gilles Peskine
2c69fa245c Initializer element was not constant
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-17 21:46:14 +02:00
Gilles Peskine
3758fd6b79 Changelog entry and migration guide for hash and curve profile upgrades
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-17 21:46:14 +02:00
Gilles Peskine
ae270bf386 Upgrade the default TLS hash and curve selection, matching X.509
Upgrade the default list of hashes and curves allowed for TLS. The list is
now aligned with X.509 certificate verification: hashes and curves with at
least 255 bits (Curve25519 included), and RSA 2048 and above.

Remove MBEDTLS_TLS_DEFAULT_ALLOW_SHA1_IN_KEY_EXCHANGE which would no
longer do anything.

Document more precisely what is allowed by default.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-17 21:46:14 +02:00
Gilles Peskine
ffb92da622 Upgrade the default X.509 profile to the former "next" profile
Upgrade the default X.509 certificate verification profile
mbedtls_x509_crt_profile_default to the former value of
mbedtls_x509_crt_profile_next, which is hashes and curves with at least 255
bits (Curve25519 included), and RSA 2048 and above.

Document more precisely what goes into the default profile.

Keep the "next" profile unchanged for now.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-17 21:46:14 +02:00
JoeSubbiani
57face2f0b Free context in at the end of aes_crypt_xts_size()
in file tests/suite/test_suite_aes.function, aes_crypt_xts_size()
did not free the context upon the function exit.
The function now frees the context on exit.

Fixes #4176

Signed-off-by: JoeSubbiani <Joe.Subbiani@arm.com>
2021-06-17 15:55:00 +01:00
Manuel Pégourié-Gonnard
a48b16a449 Homogenize coding patterns
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2021-06-17 13:25:03 +02:00
Gilles Peskine
e96c5854d0 Move the inclusion of crypto_spe.h to psa/crypto_platform.h
This makes it easier to ensure that crypto_spe.h is included everywhere it
needs to be, and that it's included early enough to do its job (it must be
included before any mention of psa_xxx() functions with external linkage,
because it defines macros to rename these functions).

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-17 11:43:58 +02:00
Gilles Peskine
a354867399 In the SPM test build, fail if a symbol wasn't renamed
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-17 11:40:51 +02:00
Gilles Peskine
d1dcfd53aa Do a test build with MBEDTLS_PSA_CRYPTO_SPM
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-17 11:40:51 +02:00
Manuel Pégourié-Gonnard
55a7fb8322 Improve documentation
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2021-06-17 10:42:03 +02:00
Manuel Pégourié-Gonnard
5314e08e08 Make the fields of mbedtls_x509_crt_profile public
These fields are supposed to be manipulated directly, that's how people
create custom profiles.

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2021-06-17 10:32:01 +02:00
Thomas Daubney
50afb4378f Adds Migration guide
Commit adds a migraiton guide entry that was
missing.

Signed-off-by: Thomas Daubney <thomas.daubney@arm.com>
2021-06-17 09:23:41 +01:00
Gilles Peskine
532327b429
Merge pull request #4576 from gilles-peskine-arm/psa_key_derivation-bad_workflow-20210527
PSA key derivation bad-workflow tests
2021-06-17 09:55:39 +02:00
Manuel Pégourié-Gonnard
609ab6478b Fix warning in some configurations
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2021-06-17 09:41:01 +02:00
Manuel Pégourié-Gonnard
6ff9ef56a5 Fix cmake build of fuzz_privkey
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2021-06-17 09:41:01 +02:00
Manuel Pégourié-Gonnard
6f19ce317b Fix async support in ssl_server2
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2021-06-17 09:41:00 +02:00
Manuel Pégourié-Gonnard
8707259318 Improve ChangeLog and migration guide entries
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2021-06-17 09:41:00 +02:00
Manuel Pégourié-Gonnard
1503a9adab Use a proper DRBG in programs
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2021-06-17 09:40:15 +02:00
Manuel Pégourié-Gonnard
7f93da1265 Use the dedicated dummy_random in fuzzing programs
Also make sure to initialize the DRBG before using it in fuzz_server
(dummy_random uses ctr_drbg internally).

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2021-06-17 09:38:38 +02:00
Manuel Pégourié-Gonnard
36a8963b3b Fix cmake build of programs
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2021-06-17 09:38:38 +02:00
Manuel Pégourié-Gonnard
e6e51aab55 Add ChangeLog and migration guide entries
Merge part of the RSA entries into this one, as I think it's easier for
users to have all similar changes in one place regardless of whether
they were introduce in the same PR or not.

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2021-06-17 09:38:38 +02:00
Manuel Pégourié-Gonnard
02b5705aa3 Simplify internal code
We know that Montgomery multiplication will never be called without an
RNG, so make that clear from the beginning of the function.

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2021-06-17 09:38:38 +02:00
Manuel Pégourié-Gonnard
7962bfaa79 Remove "internal RNG" code from ECP
This is no longer needed, as the RNG param is now mandatory.

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2021-06-17 09:38:38 +02:00
Manuel Pégourié-Gonnard
d51aaad4c9 Remove config option MBEDTLS_ECP_NO_INTERNAL_RNG
It was used to remove the code used when mbedtls_ecp_mul() received a
NULL RNG parameter. This code is no longer relevant (as the RNG may no
longer be NULL) and will be unconditionally removed in the next commit.

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2021-06-17 09:38:38 +02:00
Manuel Pégourié-Gonnard
84dea01f36 Add RNG params to private key parsing
This is necessary for the case where the public part of an EC keypair
needs to be computed from the private part - either because it was not
included (it's an optional component) or because it was compressed (a
format we can't parse).

This changes the API of two public functions: mbedtls_pk_parse_key() and
mbedtls_pk_parse_keyfile().

Tests and programs have been adapted. Some programs use a non-secure RNG
(from the test library) just to get things to compile and run; in a
future commit this should be improved in order to demonstrate best
practice.

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2021-06-17 09:38:38 +02:00
Manuel Pégourié-Gonnard
39be1410fd Add RNG parameter to check_pair functions
- mbedtls_ecp_check_pub_priv() because it calls ecp_mul()
- mbedtls_pk_check_pair() because it calls the former

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2021-06-17 09:38:38 +02:00
Manuel Pégourié-Gonnard
f8c24bf507 Fix signature of check_pub_priv
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2021-06-17 09:38:38 +02:00
Manuel Pégourié-Gonnard
75525aec52 Fix mbedtls_ecp_muladd()
It was indirectly calling ecp_mul() without an RNG. That's actually the
rare case where this should be allowed, as ecp_muladd() is typically
used on non-secret data (to verify signatures or ZKPs) and documented as
not being constant-time.

Refactor a bit in order to keep the ability to call ecp_mul() without a
RNG, but not exposed publicly (except though muladd).

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2021-06-17 09:38:38 +02:00
Manuel Pégourié-Gonnard
aa3ed6f987 Make RNG parameters mandatory in ECP functions
Fix trivial faulty calls in ECP test suite and ECP/ECJPAKE self-tests (by
adding a dummy RNG).

Several tests suites are not passing yet, as a couple of library
function do call ecp_mul() with a NULL RNG. The complexity of the fixes
range from "simple refactoring" to "requires API changes", so these will
be addressed in separate commits.

This makes the option MBEDTLS_ECP_NO_INTERNAL_RNG, as well as the whole
"internal RNG" code, obsolete. This will be addressed in a future
commit, after getting the test suites to pass again.

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2021-06-17 09:38:38 +02:00
Manuel Pégourié-Gonnard
7861ecf838 Make RNG parameters mandatory in ECDH functions
Again, no check in the code - will be checked by ECP

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2021-06-17 09:38:38 +02:00
Manuel Pégourié-Gonnard
1a87722bb6 Make RNG parameters mandatory in DHM functions
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2021-06-17 09:38:38 +02:00
Manuel Pégourié-Gonnard
f035904060 Check for mandatory RNG parameters in RSA private
(This commit is best reviewed using `git show -b` as indentation levels
have changed.)

The documentation already states that the RNG parameter is mandatory,
since PRs #4488 and #4515. There are several families of functions to
consider here:

- private-key operations (sign, decrypt) all call
mbedtls_rsa_private() where this commit adds a non-NULL check;
- encrypt operations need an RNG for masking/padding and already had a
non-NULL check since #4515 (conditional on \p mode before that)
- verify operations no longer take an RNG parameter since #4515

So, after this commit, all RSA functions that accept an RNG will reach a
non-NULL check before the RNG is used.

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2021-06-17 09:37:55 +02:00
Manuel Pégourié-Gonnard
34d3756457 Make RNG parameters mandatory in PK functions
Again, no check here, will be checked by lower-level modules.

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2021-06-17 09:37:55 +02:00
Manuel Pégourié-Gonnard
c305b72ed1 Make RNG parameters mandatory in X.509 functions
Not adding a check in the code here, as this will be checked by the
lower-level modules.

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2021-06-17 09:37:55 +02:00
Manuel Pégourié-Gonnard
ad5390fc4f Clarify that RNG parameters are mandatory in SSL
No change here, these were already mandatory, it just wasn't explicit in
the documentation.

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2021-06-17 09:37:55 +02:00
Hanno Becker
ab4ecfcc2c Improve wording of documentation of new session resumption API
Signed-off-by: Hanno Becker <hanno.becker@arm.com>
2021-06-17 05:12:16 +01:00
Thomas Daubney
22ecf49e9a Adds ChangeLog entry
Commit adds ChangeLog entry for removal of
MBEDTLS_SSL_TRUNCATED_HMAC.

Signed-off-by: Thomas Daubney <thomas.daubney@arm.com>
2021-06-16 16:26:59 +01:00
Thomas Daubney
4a7010d1aa Removes MBEDTLS_SSL_TRUNCATED_HMAC config option
Commit removes the
MBEDTLS_SSL_TRUNCATED_HMAC config option from config.h
and places a check that it is unset in check_config.h.

Signed-off-by: Thomas Daubney <thomas.daubney@arm.com>
2021-06-16 16:26:59 +01:00
Thomas Daubney
d7171e9f59 Removes truncated HMAC code from ssl.h
Commit removes conditionally compiled code
relating to MBEDTLS_SSL_TRUNCATED_HMAC from
ssl.h.

Signed-off-by: Thomas Daubney <thomas.daubney@arm.com>
2021-06-16 16:26:59 +01:00