Commit Graph

5721 Commits

Author SHA1 Message Date
Gilles Peskine
d9f694960f
Merge pull request #3190 from gilles-peskine-arm/config-full-clarify-development
Clarify that the full config enables everything that can be tested together
2020-05-04 12:29:09 +02:00
Manuel Pégourié-Gonnard
1d632c26a4
Merge pull request #3239 from opatomic/win2kdns
add support for win2k
2020-05-04 12:19:45 +02:00
Gilles Peskine
9515d76171
Merge pull request #3219 from aggarg/err_optimization
Remove error_description variable from strerr functions
2020-05-04 09:37:13 +02:00
danh-arm
3b1c072bfc
Merge pull request #3217 from irwir/fix_ssl_tls.c
Avoid re-assigning zero to `ret` variable.
2020-04-29 18:07:10 +01:00
danh-arm
924a1f2f47
Merge pull request #3147 from gocarlos/gocarlos-patch-1
cmake: add option to treat compiler warnings as errors
2020-04-29 17:14:48 +01:00
opatomic
2b2acf1111 add support for win2k
getaddrinfo() is not available on win2k. By including wspiapi.h (if
_WIN32_WINNT is defined as value < 0x0501) then a compatibility layer
will be used when running on win2k. For more details, refer to Microsoft
docs for getaddrinfo().

Signed-off-by: opatomic <j@opatomic.com>
2020-04-24 10:40:24 -04:00
Manuel Pégourié-Gonnard
b1c8e41ae3
Merge pull request #3209 from aurel32/fix-ecp_double_add_mxz
Fix wrong modulo call in ecp_double_add_mxz
2020-04-24 09:28:17 +02:00
Aurelien Jarno
66deb38d64 Fix wrong modulo call in ecp_double_add_mxz
ecp_double_add_mxz wrongly does an MPI addition followed by a call to
MOD_MUL instead of MOD_ADD. This is more visible since the
mbedtls_mpi_xxx_mod functions have been added in commit 3b3b34f608
("Replace some macros by functions").

Fix that by using mbedtls_mpi_add_mod instead. The testsuite still
passes after that change.

Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
2020-04-23 23:16:11 +02:00
Gaurav Aggarwal
cabde25289 Align with coding style
return statements use parentheses to contain their value.

Signed-off-by: Gaurav Aggarwal <aggarg@amazon.com>
2020-04-22 08:13:25 -07:00
Kenneth Soerensen
518d435e7b Fix GCC format-signedness warnings
Signed-off-by: Kenneth Soerensen <knnthsrnsn@gmail.com>
2020-04-22 16:01:48 +02:00
Manuel Pégourié-Gonnard
6bd4c79999
Merge pull request #3150 from irwir/fix_ssl_srv
Simplify checks in ssl_write_certificate_request
2020-04-22 09:31:00 +02:00
Gaurav Aggarwal
8663c7415a Remove error_description variable from strerr functions
This was suggested on this PR: https://github.com/ARMmbed/mbedtls/pull/3176

Signed-off-by: Gaurav Aggarwal <aggarg@amazon.com>
2020-04-21 14:04:19 -07:00
irwir
d3085ab2b8 Avoid re-assigning zero to ret variable.
Resolve #3182.

Signed-off-by: irwir <irwir@users.noreply.github.com>
2020-04-21 22:26:59 +03:00
Gilles Peskine
5cac337228
Merge pull request #3208 from irwir/fix_md.c
Fix "same actions in two cases of a switch statement" warning
2020-04-21 16:25:20 +02:00
Gilles Peskine
dc9c47da6c
Merge pull request #3176 from aggarg/development
Add support for const error description strings
2020-04-21 13:57:11 +02:00
irwir
c9bc300724 Simplify bounds check in ssl_write_certificate_request
It is sufficient to check for the strongest limit only. Using a smaller
type ensures there is no overflow (assuming size_t is at least 32 bits).

Fixes #2916

Signed-off-by: irwir <irwir@users.noreply.github.com>
2020-04-21 14:51:08 +03:00
Gaurav Aggarwal
6ea4fc7b53 Address review comments
1. The functions mbedtls_high_level_strerr and mbedtls_low_level_strerr
   accept any error code and extract the high-level and low-level parts
   respectively.
2. Documentation updates.

Signed-off-by: Gaurav Aggarwal <aggarg@amazon.com>
2020-04-20 16:03:46 -07:00
Gilles Peskine
1a75d0c155 Strict C99: don't use an anonymous union field
GCC and Clang accept anonymous union fields, but this is not valid ISO
C. Use a named field.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2020-04-20 15:39:36 +02:00
Gilles Peskine
01fd875b32 Strict C99: Don't repeat the typedef for psa_se_drv_table_entry_t
GCC and Clang accept
```
typedef struct foo foo_t;
typedef struct foo { ... } foo_t;
```
But this is not valid ISO C due to the redefinition of `foo_t`.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2020-04-20 15:39:35 +02:00
irwir
f0fec77e3e Fix "same actions in two cases of a switch statement" warning
Resolves #3202

Signed-off-by: irwir <irwir@users.noreply.github.com>
2020-04-20 13:56:09 +03:00
Manuel Pégourié-Gonnard
10a5b535f1 Sort lists in Makefiles and have one item per line
A file generated based on the output of `make list` from programs has been
re-generated.

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-04-17 11:57:50 +02:00
Jaeden Amero
31f4cd9de2
Merge pull request #3192 from AndrzejKurek/max_pathlen_overflow
Guard from undefined behaviour in case of an INT_MAX max_pathlen
2020-04-16 16:29:44 +01:00
Andrzej Kurek
1605074f97
Guard from undefined behaviour in case of an INT_MAX max_pathlen
When parsing a certificate with the basic constraints extension
the max_pathlen that was read from it was incremented regardless
of its value. However, if the max_pathlen is equal to INT_MAX (which
is highly unlikely), an undefined behaviour would occur.
This commit adds a check to ensure that such value is not accepted
as valid. Relevant tests for INT_MAX and INT_MAX-1 are also introduced.
Certificates added in this commit were generated using the
test_suite_x509write, function test_x509_crt_check. Input data taken 
from the "Certificate write check Server1 SHA1" test case, so the generated
files are like the "server1.crt", but with the "is_ca" field set to 1 and
max_pathlen as described by the file name.
Signed-off-by: Andrzej Kurek <andrzej.kurek@arm.com>
2020-04-15 06:15:45 -04:00
Janos Follath
a18c4f35c9
Merge pull request #686 from ARMmbed/merge-2.22.0-release-to-development
Merge 2.22.0 release to development
2020-04-14 16:21:53 +01:00
Manuel Pégourié-Gonnard
5d318cc22b
Merge pull request #3130 from gbryant-arm/generic-psk
Clarify and simplify PSK
2020-04-14 10:54:01 +02:00
Janos Follath
ac15f842a5 Merge remote-tracking branch 'restricted/pr/684' into mbedtls-2.22.0r0 2020-04-14 08:16:27 +01:00
Gaurav Aggarwal
3d02db23e8 Add auto-generated code markers
The presence of these markers in the original code was helpful to me in
figuring out that this portion of the code is auto-generated.
Therefore, I think those are useful and should be present.

Signed-off-by: Gaurav Aggarwal <aggarg@amazon.com>
2020-04-11 17:14:03 -07:00
Gaurav Aggarwal
a4a2aa5169 Address review comments
- Use switch case instead of loop to generate faster code
- Add #if defined to address compiler error

Signed-off-by: Gaurav Aggarwal <aggarg@amazon.com>
2020-04-09 11:58:24 -07:00
Jaeden Amero
66e21efe47
Merge pull request #3163 from AndrzejKurek/variable-buffers-renegotiation
Variable buffers & renegotiation - fixes
2020-04-09 12:11:02 +01:00
Manuel Pégourié-Gonnard
28482397d3 Merge branch 'development-restricted' into prepare-rc-2.22.0-updated
* development-restricted:
  Parse HelloVerifyRequest buffer overread: add changelog entry
  Parse HelloVerifyRequest: avoid buffer overread at the start
  Parse HelloVerifyRequest: avoid buffer overread on the cookie
2020-04-09 12:17:11 +02:00
Manuel Pégourié-Gonnard
215d2e14a3 Merge remote-tracking branch 'restricted/pr/662' into development-restricted
* restricted/pr/662:
  Parse HelloVerifyRequest buffer overread: add changelog entry
  Parse HelloVerifyRequest: avoid buffer overread at the start
  Parse HelloVerifyRequest: avoid buffer overread on the cookie
2020-04-09 11:53:01 +02:00
Gaurav Aggarwal
a9f64006ea Add support for const error description strings
Problem
-------
mbedtls_strerror is a utility function which converts an mbedTLS error code
into a human readable string. It requires the caller to allocate a buffer every
time an error code needs to be converted to a string. It is an overkill and a
waste of RAM for resource constrained microcontrollers - where the most common
use case is to use these strings for logging.

Solution
--------
The proposed commit adds two functions:

* const char * mbedtls_high_level_strerr( int error_code );
* const char * mbedtls_low_level_strerr( int error_code );

The above two functions convert the high level and low level parts of an mbedTLS
error code to human readable strings. They return a const pointer to an
unmodifiable string which is not supposed to be modified by the caller and only
to be used for logging purposes. The caller no longer needs to allocate a
buffer.

Backward Compatibility
----------------------
The proposed change is completely backward compatible as it does not change
the existing mbedtls_strerror function and ensures that it continues to behave
the same way.

Signed-off-by: Gaurav Aggarwal <aggarg@amazon.com>
2020-04-09 01:44:52 -07:00
Andrzej Kurek
8ea6872889
Improve pointer calculations when resizing I/O buffers
This commit introduces two changes:
- Add in_msg and out_msg calculations for buffer upsizing. This was previously
considered as unnecessary, but renegotiation using certain ciphersuites needs
this.
- Improving the way out_msg and in_msg pointers are calculated, so that even 
if no resizing is introduced, the pointers remain the same;

New tests added:
- various renegotiation schemes with a range of MFL's and ciphersuites;
- an ssl-opt.sh test exercising two things that were problematic: renegotiation
with TLS-ECDHE-ECDSA-WITH-AES-128-CCM-8 and a server MFL that's smaller
than the one negotiated by the client.
Signed-off-by: Andrzej Kurek <andrzej.kurek@arm.com>
2020-04-09 04:33:00 -04:00
Andrzej Kurek
90c6e84a9c
Split the maximum fragment length into two - an input and output MFL
Since the server might want to have a different maximum fragment length
for the outgoing messages than the negotiated one - introduce a new way of
computing it. This commit also adds additional ssl-opt.sh tests ensuring
that the maximum fragment lengths are set as expected. 
mbedtls_ssl_get_max_frag_len() is now a deprecated function,
being an alias to mbedtls_ssl_get_output_max_frag_len(). The behaviour
of this function is the same as before.
Signed-off-by: Andrzej Kurek <andrzej.kurek@arm.com>
2020-04-09 04:30:34 -04:00
Janos Follath
876e0259d5 Bump version to Mbed TLS 2.22.0
Signed-off-by: Janos Follath <janos.follath@arm.com>
2020-04-08 17:15:18 +01:00
Janos Follath
dca973bbeb Merge branch 'development-restricted' into mbedtls-2.22.0r0
Signed-off-by: Janos Follath <janos.follath@arm.com>
2020-04-08 15:11:40 +01:00
Guilhem Bryant
0c9b1955ca Return internal error if no PSK is found when deriving the premaster secret
Signed-off-by: Guilhem Bryant <Guilhem.Bryant@arm.com>
2020-04-08 13:06:49 +01:00
Gilles Peskine
54e1c30d1b
Merge pull request #3160 from gilles-peskine-arm/hkdf_expand-initialize_t-development
hkdf_expand: explicitly initialize t
2020-04-06 16:34:31 +02:00
Manuel Pégourié-Gonnard
15f30dc7e6 Merge remote-tracking branch 'public/pr/2856' into development
* public/pr/2856:
  Fix issue  #2718 (condition always false)
2020-04-06 10:15:49 +02:00
Guilhem Bryant
03d3711bb1 Fix bracket style
Signed-off-by: Guilhem Bryant <Guilhem.Bryant@arm.com>
2020-04-03 18:21:19 +01:00
Carlos Gomes Martinho
a5f0bd34b6 build: add option to treat compiler warnings as errors
Signed-off-by: Carlos Gomes Martinho <carlos.gomes_martinho@siemens.com>
2020-04-03 09:44:54 +02:00
Gilles Peskine
3ab121a603 hkdf_expand: explicitly initialize t
t is never used uninitialized, since the first loop iteration reads 0
bytes of it and then writes hash_len bytes, and subsequent iterations
read and write hash_len bytes. However this is somewhat fragile, and
it would be legitimate for a static analyzer to be unsure.

Initialize t explicitly, to make the code clearer and more robust, at
negligible cost.

Reported by Vasily Evseenko in
https://github.com/ARMmbed/mbedtls/pull/2942
with a slightly different fix.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2020-04-02 19:54:51 +02:00
Gilles Peskine
60d6516c60 Fix whitespace style
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2020-04-02 19:50:00 +02:00
Gilles Peskine
3124164f81
Merge pull request #3132 from mpg/fix-reconnect
Fix issues in handling of client reconnecting from the same port
2020-04-02 19:21:01 +02:00
Guilhem Bryant
b5f04e4d84 Properly initialise psk and psk_len
Signed-off-by: Guilhem Bryant <Guilhem.Bryant@arm.com>
2020-04-01 11:25:12 +01:00
Manuel Pégourié-Gonnard
243d70f2a5 Improve debug logging of client hard reconnect
The current logging was sub-standard, in particular there was no trace
whatsoever of the HelloVerifyRequest being sent. Now it's being logged with
the usual levels: 4 for full content, 2 return of f_send, 1 decision about
sending it (or taking other branches in the same function) because that's the
same level as state changes in the handshake, and also same as the "possible
client reconnect" message" to which it's the logical continuation (what are we
doing about it?).

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-03-31 12:33:57 +02:00
Simon Butcher
3aa6405007 Correct comment on the configuration option in x509.c
In x509.c, the self-test code is dependent on MBEDTLS_CERTS_C and
MBEDTLS_SHA256_C being enabled. At some point in the recent past that dependency
was on MBEDTLS_SHA1_C but changed to SHA256, but the comment wasn't updated.

This commit updates the comment.

Signed-off-by: Simon Butcher <simon.butcher@arm.com>
2020-03-27 17:16:15 +00:00
Guilhem Bryant
61b0fe617e Initialise psk_len in mbedtls_ssl_psk_derive_premaster()
Signed-off-by: Guilhem Bryant <Guilhem.Bryant@arm.com>
2020-03-27 11:15:28 +00:00
Manuel Pégourié-Gonnard
824655c837 Fix lack of cookie check on hard reconnect
Section 4.2.8 of RFC 6347 describes how to handle the case of a DTLS client
establishing a new connection using the same UDP quartet as an already active
connection, which we implement under the compile option
MBEDTLS_SSL_DLTS_CLIENT_PORT_REUSE. Relevant excerpts:

    [the server] MUST NOT destroy the existing
    association until the client has demonstrated reachability either by
    completing a cookie exchange or by completing a complete handshake
    including delivering a verifiable Finished message.
    [...]
    The reachability requirement prevents
    off-path/blind attackers from destroying associations merely by
    sending forged ClientHellos.

Our code chooses to use a cookie exchange for establishing reachability, but
unfortunately that check was effectively removed in a recent refactoring,
which changed what value ssl_handle_possible_reconnect() needs to return in
order for ssl_get_next_record() (introduced in that refactoring) to take the
proper action. Unfortunately, in addition to changing the value, the
refactoring also changed a return statement to an assignment to the ret
variable, causing the function to reach the code for a valid cookie, which
immediately destroys the existing association, effectively bypassing the
cookie verification.

This commit fixes that by immediately returning after sending a
HelloVerifyRequest when a ClientHello without a valid cookie is found. It also
updates the description of the function to reflect the new return value
convention (the refactoring updated the code but not the documentation).

The commit that changed the return value convention (and introduced the bug)
is 2fddd3765e, whose commit message explains the
change.

Note: this bug also indirectly caused the ssl-opt.sh test case "DTLS client
reconnect from same port: reconnect" to occasionally fail due to a race
condition between the reception of the ClientHello carrying a valid cookie and
the closure of the connection by the server after noticing the ClientHello
didn't carry a valid cookie after it incorrectly destroyed the previous
connection, that could cause that ClientHello to be invisible to the server
(if that message reaches the server just before it does `net_close()`). A
welcome side effect of this commit is to remove that race condition, as the
new connection will immediately start with a ClientHello carrying a valid
cookie in the SSL input buffer, so the server will not call `net_close()` and
not risk discarding a better ClientHello that arrived in the meantime.

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-03-27 10:50:05 +01:00
Manuel Pégourié-Gonnard
a4aa89b16e Fix leakage of projective coordinates in ECC
See the comments in the code for how an attack would go, and the ChangeLog
entry for an impact assessment. (For ECDSA, leaking a few bits of the scalar
over several signatures translates to full private key recovery using a
lattice attack.)

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-03-27 09:43:34 +01:00