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>
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>
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>
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>
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>
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>
- 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>
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>
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>
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>
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>
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>
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>
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>
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>
This change was first introduced in 8af3923 - see this commit for more background.
After the removal of crypto directory, there are no targets that require a
crypto library with the directory prefix, so there's also no need for the priority
dependency to be declared. This commit removes it.
Signed-off-by: Andrzej Kurek <andrzej.kurek@arm.com>
Merge the latest state of the target branch (mbedtls/development) into the
pull request to merge mbed-crypto into mbedtls.
Conflicts:
* ChangeLog: add/add conflict. Resolve by using the usual section order.
Rename identifiers containing double-underscore (`__`) to avoid `__`.
The reason to avoid double-underscore is that all identifiers
containing double-underscore are reserved in C++. Rename all such
identifiers that appear in any public header, including ssl_internal.h
which is in principle private but in practice is installed with the
public headers.
This commit makes check-names.sh pass.
```
perl -i -pe 's/\bMBEDTLS_SSL__ECP_RESTARTABLE\b/MBEDTLS_SSL_ECP_RESTARTABLE_ENABLED/g; s/\bMBEDTLS_KEY_EXCHANGE_(_\w+)_(_\w+)\b/MBEDTLS_KEY_EXCHANGE${1}${2}/g' include/mbedtls/*.h library/*.c programs/*/*.c scripts/data_files/rename-1.3-2.0.txt tests/suites/*.function
```
