Commit graph

9619 commits

Author SHA1 Message Date
Hanno Becker a61925fa51 Don't send an alert when receiving a record of unknown ContentType
We don't send alerts on other instances of ill-formed records,
so why should we do it here? If we want to keep it, the alerts
should rather be sent ssl_get_next_record().
2019-08-01 09:51:49 +02:00
Hanno Becker dc4d62748c Don't call ssl_fetch_input for record content fetch in DTLS
As explained in the previous commit, if mbedtls_ssl_fetch_input()
is called multiple times, all but the first call are equivalent to
bounds checks in the incoming datagram.
2019-08-01 09:50:27 +02:00
Hanno Becker 29823466a1 Don't call ssl_fetch_input for record hdr size check in DTLS
In DTLS, if mbedtls_ssl_fetch_input() is called multiple times without
resetting the input buffer in between, the non-initial calls are functionally
equivalent to mere bounds checks ensuring that the incoming datagram is
large enough to hold the requested data. In the interest of code-size
and modularity (removing a call to a non-const function which is logically
const in this instance), this commit replaces such a call to
mbedtls_ssl_fetch_input() by an explicit bounds check in
ssl_parse_record_header().
2019-08-01 09:50:27 +02:00
Hanno Becker de7d6d33e5 Move size-check for DTLS record header with CID to DTLS-only branch 2019-08-01 09:50:27 +02:00
Hanno Becker 87b5626d73 Check same-port-reconnect from client outside of record hdr parsing
Previously, `ssl_handle_possible_reconnect()` was part of
`ssl_parse_record_header()`, which was required to return a non-zero error
code to indicate a record which should not be further processed because it
was invalid, unexpected, duplicate, .... In this case, some error codes
would lead to some actions to be taken, e.g. `MBEDTLS_ERR_SSL_EARLY_MESSAGE`
to potential buffering of the record, but eventually, the record would be
dropped regardless of the precise value of the error code. The error code
`MBEDTLS_ERR_SSL_HELLO_VERIFY_REQUIRED` returned from
`ssl_handle_possible_reconnect()` did not receive any special treatment and
lead to silent dopping of the record - in particular, it was never returned
to the user.

In the new logic this commit introduces, `ssl_handle_possible_reconnect()` is
part of `ssl_check_client_reconnect()` which is triggered _after_
`ssl_parse_record_header()` found an unexpected record, which is already in
the code-path eventually dropping the record; we want to leave this code-path
only if a valid cookie has been found and we want to reset, but do nothing
otherwise. That's why `ssl_handle_possible_reconnect()` now returns `0` unless
a valid cookie has been found or a fatal error occurred.
2019-08-01 09:50:27 +02:00
Hanno Becker 07d420d6ad Remove unnecessary backup of explicit IV in AEAD record decryption
There is no need to hold back the explicit IV for AEAD ciphers.
2019-08-01 09:50:27 +02:00
Hanno Becker 8244cfa8bc Remove redundant minimum length check
Availability of sufficient incoming data should be checked when
it is needed, which is in mbedtls_ssl_fetch_input(), and this
function has the necessary bounds checks in place.
2019-08-01 09:50:27 +02:00
Hanno Becker 6d3db0fa25 Improve documentation of mbedtls_ssl_decrypt_buf() 2019-08-01 09:50:26 +02:00
Hanno Becker 9520b31860 Remove misleading comment in mbedtls_ssl_decrypt_buf()
The comment doesn't seem to relate to the code that follows.
2019-08-01 09:50:26 +02:00
Hanno Becker b603bd34bc Remove assertion in mbedtls_ssl_decrypt_buf()
mbedtls_ssl_decrypt_buf() asserts that the passed transform is not NULL,
but the function is only invoked in a single place, and this invocation
is clearly visible to be within a branch ensuring that the incoming
transform isn't NULL. Remove the assertion for the benefit of code-size.
2019-08-01 09:50:26 +02:00
Hanno Becker f024285034 Check architectural bound for max record payload len in one place
The previous code performed architectural maximum record length checks
both before and after record decryption. Since MBEDTLS_SSL_IN_CONTENT_LEN
bounds the maximum length of the record plaintext, it suffices to check
only once after (potential) decryption.

This must not be confused with the internal check that the record
length is small enough to make the record fit into the internal input
buffer; this is done in mbedtls_ssl_fetch_input().
2019-08-01 09:50:26 +02:00
Hanno Becker 408a2742b3 Remove redundant length-0 checks for incoming unprotected records 2019-08-01 09:50:26 +02:00
Hanno Becker 1c26845777 Remove redundant length check during record header parsing
The check is in terms of the internal input buffer length and is
hence likely to be originally intended to protect against overflow
of the input buffer when fetching data from the underlying
transport in mbedtls_ssl_fetch_input(). For locality of reasoning,
it's better to perform such a check close to where it's needed,
and in fact, mbedtls_ssl_fetch_input() _does_ contain an equivalent
bounds check, too, rendering the bounds check in question redundant.
2019-08-01 09:50:26 +02:00
Manuel Pégourié-Gonnard 6852e95c2a
Merge pull request #618 from hanno-arm/record_checking_api-baremetal
[Baremetal] Record checking: API, Documentation and Stubs
2019-07-30 17:11:46 +02:00
Manuel Pégourié-Gonnard 2b29a37c91 Fix compile bugs in examples with hardcoded I/O callbacks
These were found by `all.sh test_baremetal`, so no need for a new test.
2019-07-30 17:07:38 +02:00
Manuel Pégourié-Gonnard f1358acdc7 Fix bug in MBEDTLS_X509_CRT_REMOVE_TIME
When looking for a parent, all candidates were considered time-invalid due to
the #ifdef incorrectly including the `parent_valid = 1` line.

When MBEDTLS_HAVE_TIME_DATE is unset the time-validity of certificates is
never checked and always treated as valid. This is usually achieved by proper
usage of mbedtls_x509_time_is_past() and mbedtls_x509_time_is_future() (and
their definition when we don't HAVE_TIME_DATE).

Here the calls to these functions needs to be guarded by
MBEDTLS_X509_CRT_REMOVE_TIME as they access struct members whose presence is
controlled by this option. But the "valid" branch should still always be taken.

(Note: MBEDTLS_X509_CRT_REMOVE_TIME being set forces MBEDTLS_HAVE_TIME_DATE to
be unset, as enforce by check_config.h.)

This bug was found by `all.sh test_baremetal` - no need for a new test.
2019-07-30 16:56:58 +02:00
Manuel Pégourié-Gonnard 80eaddfc36 Clean generated *.su file and gitignore them 2019-07-30 16:56:58 +02:00
Manuel Pégourié-Gonnard d64a2f72e6 Fix wrong ifdef in ssl_server2 & add test for it
This was found as a warning when running scripts/baremetal.sh --ram
--build-only manually, but it should have been found in a more automated way.
Adding -Werror so that future such issues will be caught by all.sh
(component_test_baremetal already invokes baremetal.sh --ram --build-only).
2019-07-30 16:56:43 +02:00
Manuel Pégourié-Gonnard 0d1db20490 Fix bug in skip_date() (MBEDTLS_X509_CRT_REMOVE_TIME)
Asserting `*p == end` right after setting `end = *p + len` will always fail
unless `len == 0`, which is never the case with properly-formed certificates.

The function x509_skip_dates() is modelled after x509_get_dates() which between
setting `end` and comparing it to `*p` calls mbedtls_x509_get_time() which
advances `*p` to the expected value, which is why this test works in
get_dates().

Since `skip_dates()` has `skip`, not `validate` in its name, and the entire
point of `MBEDTLS_X509_CRT_REMOVE_TIME` is to save code, we don't want to
call the relatively large functions needed to properly parse (and validate)
dates before throwing the parsed dates away, we can just fast-forward to the
end of the sequence.

This makes updating `end` and comparing it to `*p` after the fast-forward
redundant, as the comparison will always be true (unlike the case where we
actually parse the contents of the sequence).

This bug was found by `all.sh test_baremetal` - no need for a new test.
2019-07-30 16:56:25 +02:00
Hanno Becker 62daad3b9a all.sh: Adhere to convention that build_ prefixes build-only tests 2019-07-30 16:56:25 +02:00
Hanno Becker 93de2965d0 Fix rebase slip 2019-07-30 16:56:25 +02:00
Hanno Becker b7769e4fc0 Remove wrong reference to ARMCC in all.sh test name 2019-07-30 16:56:25 +02:00
Hanno Becker c4296a3bbb Modify all.sh test names to reflect use of ARMCC 2019-07-30 16:56:25 +02:00
Hanno Becker 28d2a88173 Fix indentation in three example programs 2019-07-30 16:56:25 +02:00
Hanno Becker e7895aae2c Add all.sh test for baremetal.h runtime test 2019-07-30 16:56:25 +02:00
Hanno Becker a1f3c521e7 Add --build-only option to baremetal.sh --ram
This option builds the library, tests and example programs
in a minimally modified baremetal.h configuration (modifications
from baremetal_test.h) but doesn't execute any tests.
2019-07-30 16:56:25 +02:00
Hanno Becker fe1bd781de Add all.sh test for hardcoded elliptic curve 2019-07-30 16:56:25 +02:00
Hanno Becker 7cedd8bed2 Remove overly strict guard in ssl_server2
get_auth_mode() is needed for a change of authmode through SNI,
which is possible even if the original authmode is hardcoded.
2019-07-30 16:56:18 +02:00
Hanno Becker 6dd8e1c54d Add all.sh test for hardcoded miscellaneous SSL config options 2019-07-30 16:50:27 +02:00
Hanno Becker 41e5a6871d Example apps: Don't use runtime IO config API if CBs are hardcoded
Multiple example applications still use mbedtls_ssl_set_bio() even
if the I/O callbacks are hardcoded. This commit fixes this.
2019-07-30 16:50:27 +02:00
Hanno Becker 2a0cd5a031 Add all.sh test for hardcoded IO callbacks 2019-07-30 16:48:20 +02:00
Hanno Becker 9fb3f1eaf2 Add all.sh test for hardcoded SSL version 2019-07-30 16:48:20 +02:00
Hanno Becker 32bbe4a66b Remove unused label in ssl_client2/ssl_server2 2019-07-30 16:33:40 +03:00
Hanno Becker e29dfb2157 Add missing word in documentation of mbedtls_ssl_check_record() 2019-07-30 16:33:40 +03:00
Hanno Becker 83b8c3b8eb cli/srv ex: Add dbg msg if record checking gives inconsistent result 2019-07-30 16:33:40 +03:00
Hanno Becker c2b08d1251 Fix minor issues in documentation of mbedtls_ssl_check_record() 2019-07-30 16:33:40 +03:00
Hanno Becker bec8885b7d State that record checking is DTLS only and doesn't check content type 2019-07-30 16:33:40 +03:00
Hanno Becker 82ff6f1e17 Update version_features.c 2019-07-30 16:33:40 +03:00
Hanno Becker de9e36e6b3 Pass dgrams to mbedtls_ssl_check_record in ssl_client2/server2 2019-07-30 16:33:40 +03:00
Hanno Becker fe24b3b269 Add IO wrappers to ssl_server2 as interm's between NET and SSL layer 2019-07-30 16:33:40 +03:00
Hanno Becker 14219feb27 Add IO wrappers to ssl_client2 as interm's between NET and SSL layer 2019-07-30 15:44:43 +03:00
Hanno Becker 02f2609551 Introduce configuration option and API for SSL record checking 2019-07-30 15:38:40 +03:00
Manuel Pégourié-Gonnard f010eba833
Merge pull request #632 from hanno-arm/baremetal_sh_debug-baremetal
[Baremetal] Add `--debug` option to `baremetal.sh`
2019-07-30 00:07:45 +02:00
Manuel Pégourié-Gonnard cdb83e7c88
Merge pull request #616 from mpg/context-s11n
[baremetal] Implement context serialization
2019-07-30 00:07:23 +02:00
Manuel Pégourié-Gonnard 69a3e417d8 Improve reability and debugability of large if
Breaking into a series of statements makes things easier when stepping through
the code in a debugger.

Previous comments we stating the opposite or what the code tested for (what we
want vs what we're erroring out on) which was confusing.

Also expand a bit on the reasons for these restrictions.
2019-07-29 12:32:02 +02:00
Manuel Pégourié-Gonnard 18332c5c6c Improve getter for renegotiation enabled 2019-07-29 12:17:52 +02:00
Manuel Pégourié-Gonnard d04850507d
Merge pull request #634 from hanno-arm/single_ec_doc-baremetal
[Baremetal] Fix single-EC documentation
2019-07-29 11:59:12 +02:00
Manuel Pégourié-Gonnard 7c575d29dc
Merge pull request #605 from ARMmbed/x509_ondemand_remove_unneeded_fields
[Baremetal] Allow removal of unneeded fields in X.509 CRT structures
2019-07-29 11:58:58 +02:00
Manuel Pégourié-Gonnard 7d33b7e2b9
Merge pull request #610 from ARMmbed/delay_alerts-baremetal
[Baremetal] Delay sending alerts
2019-07-29 11:58:44 +02:00
Manuel Pégourié-Gonnard b3bb31bd90 Introduce getter function for disable_renego 2019-07-26 16:37:45 +02:00