Commit graph

15605 commits

Author SHA1 Message Date
Janos Follath b23b5745b5 PSA RSA PSS: pass pre-hash algorithm to Mbed TLS
PSA Crypto always passed MBEDTLS_MD_NONE to Mbed TLS, which worked well
as Mbed TLS does not use this parameter for anything beyond determining
the input lengths.

Some alternative implementations however check the consistency of the
algorithm used for pre-hash and for other uses in verification (verify
operation and mask generation) and fail if they don't match. This makes
all such verifications fail.

Furthermore, the PSA Crypto API mandates that the pre-hash and internal
uses are aligned as well.

Fixes #3990.

Signed-off-by: Janos Follath <janos.follath@arm.com>
2021-06-22 12:57:00 +02:00
Janos Follath 456d7e055f mbedtls_rsa_rsassa_pss_*: improve documentation
Hashes used in RSA-PSS encoding (EMSA-PSS-ENCODE, see §9.1.1 in RFC
8017):

- H1: Hashing the message (step 2)
- H2: Hashing in the salt (step 6)
- H3: Mask generation function (step 9)

According to the standard:

- H1 and H2 MUST be done by the same hash function
- H3 is RECOMMENDED to be the same as the hash used for H1 and H2.

According to the implementation:

- H1 happens outside of the function call. It might or might not happen
and the implementation might or might not be aware of the hash used.
- H2 happens inside the function call, consistency with H1 is not
enforced and might not even be possible to detect.
- H3 is done with the same hash as H2 (with the exception of
mbedtls_rsassa_pss_verify_ext(), which takes a dedicated parameter for
the hash used in the MGF).

Issues with the documentation:

- The comments weren't always clear about the three hashes involved and
often only mentioned two of them (which two varied from function to
function).
- The documentation was giving the impression that the standard
recommends aligning H2 and H1 (which is not a recommendation but a
must).

Signed-off-by: Janos Follath <janos.follath@arm.com>
2021-06-22 12:50:26 +02:00
Manuel Pégourié-Gonnard 6a55de9057
Merge pull request #4623 from gilles-peskine-arm/debug-print-mpi-null-2.x
Backport 2.x: Fix mbedtls_debug_print_mpi crash on 0
2021-06-22 12:08:57 +02:00
Manuel Pégourié-Gonnard 9a11ac9cc1
Merge pull request #4621 from gilles-peskine-arm/default-hashes-curves-2.x
Backport 2.x: Curve and hash selection for X.509 and TLS
2021-06-22 12:08:43 +02:00
Gilles Peskine 5ea63a31c4 Mention the Montgomery curve exception
Montgomery curves are not in the expected place in the curve list.
This is a bug (https://github.com/ARMmbed/mbedtls/issues/4698), but
until this bug is fixed, document the current behavior and indicate
that it's likely to change.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-22 10:50:04 +02:00
Dave Rodgman c158213b2e
Merge pull request #4678 from JoeSubbiani/FixedMissingContextFree-test_suite_aes
Backport 2.x: Add missing free context at the end of aes_crypt_xts_size()
2021-06-22 09:24:14 +01:00
Manuel Pégourié-Gonnard 3f0538d7b7
Merge pull request #4688 from gilles-peskine-arm/winsock-fd-range-2.x
Backport 2.x: Fix net_sockets regression on Windows
2021-06-22 09:29:33 +02:00
Joe Subbiani 7d5fa2be81 Reword changelog - Test Resource Leak
- “Fix an issue where X happens” → ”Fix X“
  the extra words are just a distraction.
- “resource” → “a resource”
- “where resource is never freed” has a name: it's a resource leak
- “when running one particular test suite” → “in a test suite”

Signed-off-by: Joe Subbiani <joe.subbiani@arm.com>
2021-06-21 16:57:28 +01:00
Gilles Peskine 138d9f52cf SHA-1 is allowed for handshake signatures by default
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-21 09:53:25 +02:00
Gilles Peskine 51859aaff2 Fix fd range for select on Windows
Fix mbedtls_net_poll() and mbedtls_net_recv_timeout() often failing with
MBEDTLS_ERR_NET_POLL_FAILED on Windows: they were testing that the file
descriptor is in range for fd_set, but on Windows socket descriptors are not
limited to a small range. Fixes #4465.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-20 23:17:39 +02:00
Gilles Peskine 0f6351f8a9 Refactor file descriptor checks into a common function
This will make it easier to change the behavior uniformly.

No behavior change.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-20 23:17:39 +02:00
Joe Subbiani 02945bcab4 Update changelog formatting - Missing Free Context
Missing trailing full stop added to the end of the fixed issue number

Signed-off-by: Joe Subbiani <joe.subbiani@arm.com>
2021-06-18 18:55:57 +01:00
Joe Subbiani 707186d179 Update changelog formatting Missing Free Context
Trailing white space causing check_files.py to fail

Signed-off-by: Joe Subbiani <joe.subbiani@arm.com>
2021-06-18 17:45:34 +01:00
Joe Subbiani 5e1fac8b28 Update changelog formatting - Missing Free Context
The original formatting was in dos and the changelog
assembler would fail. The length of the description was
too long horizontally. This has been updated.

Signed-off-by: Joe Subbiani <joe.subbiani@arm.com>
2021-06-18 15:42:42 +01:00
Joe Subbiani 2af8d04085 Changelog entry for Free Context in test_suite_aes fix
Signed-off-by: Joe Subbiani <joe.subbiani@arm.com>
2021-06-18 12:06:31 +01:00
JoeSubbiani 67889a5e64 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 16:15:31 +01:00
Gilles Peskine f97a963037
Merge pull request #4656 from gilles-peskine-arm/psa_key_derivation-bad_workflow-20210527-2.x
Backport 2.x: PSA key derivation bad-workflow tests
2021-06-17 09:55:37 +02:00
Gilles Peskine 8d54b69c96 Fix copypasta in test data
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-14 18:08:26 +02:00
Gilles Peskine a172cf53f7 Use UNUSED wherever applicable in derive_input tests
Exhaustivity check:
```
<tests/suites/test_suite_psa_crypto.data awk -F: '$1=="derive_input" { for (step=1; step<=3; step++) { if ($(4*step-1) == "0") { if ($(4*step) != "UNUSED" || $(4*step+1) != "\"\"" || $(4*step+2) != "UNUSED") print NR, step, $(4*step), $(4*step+1), $(4*step+2) } } }'
```

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-14 18:08:26 +02:00
Gilles Peskine f216f0d5d4 Fix missing state check for tls12_prf output
Fix PSA_ALG_TLS12_PRF and PSA_ALG_TLS12_PSK_TO_MS being too permissive
about missing inputs.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-14 18:08:26 +02:00
Gilles Peskine d40a21cff1 Key derivation: add test cases where the secret is missing
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-14 18:08:26 +02:00
Gilles Peskine f627931cde Add bad-workflow key derivation tests
Add HKDF tests where the sequence of inputs differs from the nominal
case: missing step, duplicate step, step out of order, or invalid step.

There were already similar tests for TLS 1.2 PRF. Add one with a key
agreement which has slightly different code.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-14 18:08:26 +02:00
Gilles Peskine 0faba4e8c5 More explicit names for some bad-workflow key derivation tests
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-14 18:08:26 +02:00
Ronald Cron 766edb8476
Merge pull request #4635 from Patater/mbed-can-do-timing-2.x
Backport 2.x: config: Allow Mbed to implement TIMING_C

Trivial backport, only one reviewer is ok.
2021-06-11 09:14:00 +02:00
Dave Rodgman 78719eaa7b
Merge pull request #4646 from daverodgman/travis-disable-osx-development_2.x
Backport 2.x: Disable OS X builds on Travis
2021-06-10 17:48:27 +01:00
Dave Rodgman fcf958afc7 Disable OS X builds on Travis
Signed-off-by: Dave Rodgman <dave.rodgman@arm.com>
2021-06-10 15:48:20 +01:00
Jaeden Amero 128c94dd87 config: Allow Mbed to implement TIMING_C
Mbed OS now provides POSIX-like time functions, although not alarm() nor
signal(). It is possible to implement MBEDTLS_TIMING_ALT on Mbed OS, so
we should not artificially prevent this in check-config. Remove the the
check that prevents implementing MBEDTLS_TIMING_ALT on Mbed OS.

Note that this limitation originally was added in the following commit,
although there isn't much context around why the restriction was
imposed: 63e7ebaaa1 ("Add material for generating yotta module"). In
2015, Mbed OS was quite a different thing: no RTOS, no threads, just an
asynchronous event loop model. I'd suppose the asynchronous event loop
model made it difficult before to implement MBEDTLS_TIMING_C on Mbed OS,
but that is no longer the case.

Fixes #4633

Signed-off-by: Jaeden Amero <jaeden.amero@arm.com>
2021-06-09 14:09:11 +01:00
Gilles Peskine 4de5a6096b Add missing parentheses
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-07 21:42:15 +02:00
Gilles Peskine e247b10cd6 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-07 21:42:08 +02:00
Gilles Peskine 2ee0bb333c 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-07 21:41:59 +02:00
Gilles Peskine fba257d391 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-07 21:41:53 +02:00
Gilles Peskine b37abdcb07 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-07 21:41:38 +02:00
Gilles Peskine da728b31b0 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-07 21:28:14 +02:00
Gilles Peskine b3ca90bc44 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-07 21:27:51 +02:00
Gilles Peskine 646b78b927 Document more precisely what goes into the default preset
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-07 21:26:41 +02:00
Gilles Peskine 0ecd719edf Document more precisely what goes into the default profile
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-07 21:24:26 +02:00
Gilles Peskine 7a4c7589c8
Merge pull request #4541 from mpg/fix-ssl-cf-hmac-alt-2.x
[Backport 2.x] Fix misuse of MD API in SSL constant-flow HMAC
2021-06-07 20:53:48 +02:00
Manuel Pégourié-Gonnard 62da8ac37a
Merge pull request #4276 from gilles-peskine-arm/random-range-uniformity
Backport 2.x: Fix non-uniform random generation in a range
2021-06-04 10:43:25 +02:00
Gilles Peskine 23422e424c Note that the byte order in mpi_fill_random_internal() is deliberate
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-03 11:51:09 +02:00
Gilles Peskine c0b68bf03a Use MBEDTLS_MPI_CHK where warranted
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-03 11:38:26 +02:00
Gilles Peskine 3130ce24f4 New internal function mbedtls_mpi_resize_clear
The idiom "resize an mpi to a given size" appeared 4 times. Unify it
in a single function. Guarantee that the value is set to 0, which is
required by some of the callers and not a significant expense where
not required.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-02 23:48:07 +02:00
Gilles Peskine e4f937f5d3 Lift function call out of inner loop
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-02 22:28:27 +02:00
Gilles Peskine f37b9f73c7 Fix mistakes in test case descriptions
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-02 22:28:14 +02:00
Gilles Peskine 11779077a0 Use ternary operator with the most common case first
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-02 21:39:31 +02:00
Gilles Peskine b72b7e6b9d Fix long-standing obsolete comment
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-02 21:39:31 +02:00
Gilles Peskine 3f61363f8d Correct some comments about ECC in mbedtls_mpi_random
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-02 21:39:31 +02:00
Gilles Peskine 346d20d209 DHM: add test case with x_size < 0
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-02 21:39:31 +02:00
Gilles Peskine 9e96679548 DHM tests: add some explanations
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-02 21:39:31 +02:00
Gilles Peskine 104eb82ec1 DHM: add notes about leading zeros
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-02 21:39:31 +02:00
Gilles Peskine a16001e2d9 mpi_fill_random_internal: remove spurious grow() call
Since the internal function mpi_fill_random_internal() assumes that X
has the right size, there is no need to call grow().

To further simplify the function, set the sign outside, and zero out
the non-randomized part directly.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-02 21:39:31 +02:00