Commit graph

1041 commits

Author SHA1 Message Date
Jarno Lamsa 29f2dd0a7b Address review comments 2019-07-02 15:12:29 +02:00
Jarno Lamsa 59bd12bf14 Add new config MBEDTLS_SSL_SESSION_RESUMPTION
Add a new configuration option MBEDTLS_SSL_SESSION_RESUMPTION
to enable/disable the session resumption feature including
ticket and cache based session resumption.
2019-07-02 15:12:29 +02:00
Jarno Lamsa 7be14065e2 Add config MBEDTLS_SSL_SESSION_CACHE
Add configuration option MBEDTLS_SSL_SESSION_CACHE to control
enabling/disabling of the cache based session resumption.
2019-07-02 15:12:29 +02:00
Hanno Becker f765ce617f Remove ExtendedMS configuration API if hardcoded at compile-time
If the ExtendedMasterSecret extension is configured at compile-time
by setting MBEDTLS_SSL_CONF_EXTENDED_MASTER_SECRET and/or
MBEDTLS_SSL_CONF_ENFORCE_EXTENDED_MASTER_SECRET, the runtime
configuration APIs mbedtls_ssl_conf_extended_master_secret()
and mbedtls_ssl_conf_extended_master_secret_enforce() must
either be removed or modified to take no effect (or at most
check that the runtime value matches the hardcoded one, but
that would undermine the code-size benefits the hardcoding
is supposed to bring in the first place).

Previously, the API was kept but modified to have no effect.
While convenient for us because we don't have to adapt example
applications, this comes at the danger of users calling the runtime
configuration API, forgetting that the respective fields are
potentially already hardcoded at compile-time - and hence silently
using a configuration they don't intend to use.

This commit changes the approach to removing the configuration
API in case the respective field is hardcoded at compile-time,
and exemplifies it in the only case implemented so far, namely
the configuration of the ExtendedMasterSecret extension.

It adapts ssl_client2 and ssl_server2 by omitting the call to
the corresponding API if MBEDTLS_SSL_CONF_XXX are defined and
removing the command line parameters for the runtime configuration
of the ExtendedMasterSecret extension.
2019-06-25 08:42:20 +01:00
Hanno Becker a49ec56f51 Introduce getter function for extended_ms field in HS struct 2019-06-25 08:42:20 +01:00
Hanno Becker aabbb582eb Exemplify harcoding SSL config at compile-time in example of ExtMS
This commit is the first in a series demonstrating how code-size
can be reduced by hardcoding parts of the SSL configuration at
compile-time, focusing on the example of the configuration of
the ExtendedMasterSecret extension.

The flexibility of an SSL configuration defined a runtime vs.
compile-time is necessary for the use of Mbed TLS as a
dynamically linked library, but is undesirable in constrained
environments because it introduces the following overhead:
- Definition of SSL configuration API (code-size overhead)
  (and on the application-side: The API needs to be called)
- Additional fields in the SSL configuration (RAM overhead,
  and potentially code-size overhead if structures grow
  beyond immediate-offset bounds).
- Dereferencing is needed to obtain configuration settings.
- Code contains branches and potentially additional structure
  fields to distinguish between different configurations.

Considering the example of the ExtendedMasterSecret extension,
this instantiates as follows:
- mbedtls_ssl_conf_extended_master_secret() and
  mbedtls_ssl_conf_extended_master_secret_enforced()
  are introduced to configure the ExtendedMasterSecret extension.
- mbedtls_ssl_config contains bitflags `extended_ms` and
  `enforce_extended_master_secret` reflecting the runtime
  configuration of the ExtendedMasterSecret extension.
- Whenever we need to access these fields, we need a chain
  of dereferences `ssl->conf->extended_ms`.
- Determining whether Client/Server should write the
  ExtendedMasterSecret extension needs a branch
  depending on `extended_ms`, and the state of the
  ExtendedMasterSecret negotiation needs to be stored in a new
  handshake-local variable mbedtls_ssl_handshake_params::extended_ms.
  Finally (that's the point of ExtendedMasterSecret) key derivation
  depends on this handshake-local state of ExtendedMasterSecret.

All this is unnecessary if it is known at compile-time that the
ExtendedMasterSecret extension is used and enforced:
- No API calls are necessary because the configuration is fixed
  at compile-time.
- No SSL config fields are necessary because there are corresponding
  compile-time constants instead.
- Accordingly, no dereferences for field accesses are necessary,
  and these accesses can instead be replaced by the corresponding
  compile-time constants.
- Branches can be eliminated at compile-time because the compiler
  knows the configuration. Also, specifically for the ExtendedMasterSecret
  extension, the field `extended_ms` in the handshake structure
  is unnecessary, because we can fail immediately during the Hello-
  stage of the handshake if the ExtendedMasterSecret extension
  is not negotiated; accordingly, the non-ExtendedMS code-path
  can be eliminated from the key derivation logic.

A way needs to be found to allow fixing parts of the SSL configuration
at compile-time which removes this overhead in case it is used,
while at the same time maintaining readability and backwards
compatibility.

This commit proposes the following approach:

From the user perspective, for aspect of the SSL configuration
mbedtls_ssl_config that should be configurable at compile-time,
introduce a compile-time option MBEDTLS_SSL_CONF_FIELD_NAME.
If this option is not defined, the field is kept and configurable
at runtime as usual. If the option is defined, the field is logically
forced to the value of the option at compile time.

Internally, read-access to fields in the SSL configuration which are
configurable at compile-time gets replaced by new `static inline` getter
functions which evaluate to the corresponding field access or to the
constant MBEDTLS_SSL_CONF_FIELD_NAME, depending on whether the latter
is defined or not.

Write-access to fields which are configurable at compile-time needs
to be removed: Specifically, the corresponding API itself either
needs to be removed or replaced by a stub function without effect.
This commit takes the latter approach, which has the benefit of
not requiring any change on the example applications, but introducing
the risk of mismatching API calls and compile-time configuration,
in case a user doesn't correctly keep track of which parts of the
configuration have been fixed at compile-time, and which haven't.
Write-access for the purpose of setting defaults is simply omitted.
2019-06-25 08:42:20 +01:00
Manuel Pégourié-Gonnard 393338ca78
Merge pull request #586 from ARMmbed/remove_peer_crt_after_handshake_no_digest-baremetal
[Baremetal] Don't store peer CRT digest if renegotiation is disabled
2019-06-24 18:12:00 +02:00
Manuel Pégourié-Gonnard 79cf74a95f
Merge pull request #583 from ARMmbed/remove_peer_crt_after_handshake-baremetal
[Baremetal] Allow removal of peer certificate to reduce RAM usage
2019-06-24 18:11:46 +02:00
Manuel Pégourié-Gonnard cc3b7ccb04
Merge pull request #579 from Patater/bm-dont-use-non-existent-encrypt-then-mac
[Baremetal] ssl: Don't access non-existent encrypt_then_mac field
2019-06-24 18:06:53 +02:00
Hanno Becker 5882dd0856 Remove CRT digest from SSL session if !RENEGO + !KEEP_PEER_CERT
If `MBEDTLS_SSL_KEEP_PEER_CERTIFICATE` is not set, `mbedtls_ssl_session`
contains the digest of the peer's certificate for the sole purpose of
detecting a CRT change on renegotiation. Hence, it is not needed if
renegotiation is disabled.

This commit removes the `peer_cert_digest` fields (and friends) from
`mbedtls_ssl_session` if
   `!MBEDTLS_SSL_KEEP_PEER_CERTIFICATE + !MBEDTLS_SSL_RENEGOTIATION`,
which is a sensible configuration for constrained devices.

Apart from straightforward replacements of
   `if !defined(MBEDTLS_SSL_KEEP_PEER_CERTIFICATE)`
by
   `if !defined(MBEDTLS_SSL_KEEP_PEER_CERTIFICATE) && \
        defined(MBEDTLS_SSL_RENEGOTIATION)`,
there's one notable change: On the server-side, the CertificateVerify
parsing function is a no-op if the client hasn't sent a certificate.
So far, this was determined by either looking at the peer CRT or the
peer CRT digest in the SSL session structure (depending on the setting
of `MBEDTLS_SSL_KEEP_PEER_CERTIFICATE`), which now no longer works if
`MBEDTLS_SSL_KEEP_PEER_CERTIFICATE` is unset. Instead, this function
now checks whether the temporary copy of the peer's public key within
the handshake structure is initialized or not (which is also a
beneficial simplification in its own right, because the pubkey is
all the function needs anyway).
2019-06-19 16:56:51 +01:00
Hanno Becker 0528f82fa9 Clarify documentation of serialized session format 2019-06-19 14:59:42 +01:00
Hanno Becker 17daaa5cc6 Move return statement in ssl_srv_check_client_no_crt_notification
The previous placing of the return statement made it look like there
are configurations for which no return statement is emitted; while
that's not true (if this function is used, at least some version of
TLS must be enabled), it's still clearer to move the failing return
statement to outside of all preprocessor guards.
2019-06-19 14:59:42 +01:00
Hanno Becker 2326d20361 Validate consistency of certificate hash type and length in session 2019-06-19 14:59:42 +01:00
Hanno Becker fd5dc8ae07 Fix unused variable warning in ssl_parse_certificate_coordinate()
This was triggered in client-only builds.
2019-06-19 14:59:42 +01:00
Hanno Becker 42de8f8a42 Fix typo in documentation of ssl_parse_certificate_chain() 2019-06-19 14:59:41 +01:00
Hanno Becker 9d64b789cf Set peer CRT length only after successful allocation 2019-06-19 10:26:50 +01:00
Hanno Becker 257ef65d94 Remove question in comment about verify flags on cli vs. server 2019-06-19 10:26:50 +01:00
Hanno Becker 34106f6ae2 Free peer CRT chain immediately after verifying it
If we don't need to store the peer's CRT chain permanently, we may
free it immediately after verifying it. Moreover, since we parse the
CRT chain in-place from the input buffer in this case, pointers from
the CRT structure remain valid after freeing the structure, and we
use that to extract the digest and pubkey from the CRT after freeing
the structure.
2019-06-19 10:26:50 +01:00
Hanno Becker 0cc7af5be5 Parse peer's CRT chain in-place from the input buffer 2019-06-19 10:26:50 +01:00
Hanno Becker 17572473c6 Correct compile-time guards for ssl_clear_peer_cert()
It is used in `mbedtls_ssl_session_free()` under
`MBEDTLS_X509_CRT_PARSE_C`, but defined only if
`MBEDTLS_KEY_EXCHANGE__WITH_CERT__ENABLED`.

Issue #2422 tracks the use of
`MBEDTLS_KEY_EXCHANGE__WITH_CERT_ENABLED` instead of
`MBEDTLS_X509_CRT_PARSE_C` for code and fields
related to CRT-based ciphersuites.
2019-06-19 10:26:50 +01:00
Hanno Becker bfab9dfea1 Guard mbedtls_ssl_get_peer_cert() by new compile-time option 2019-06-19 10:26:17 +01:00
Hanno Becker 81d11aa640 Adapt mbedtls_ssl_parse_certificate() to removal of peer_cert field 2019-06-19 10:25:02 +01:00
Hanno Becker 5062897507 Adapt ssl_clear_peer_cert() to removal of peer_cert field 2019-06-19 10:25:02 +01:00
Hanno Becker d5258faa29 Adapt mbedtls_ssl_session_copy() to removal of peer_cert field 2019-06-19 10:25:02 +01:00
Hanno Becker cf291d63dd Make a copy of peer's raw public key after verifying its CRT chain
This commit modifies `mbedtls_ssl_parse_certificate()` to store a
copy of the peer's public key after parsing and verifying the peer's
CRT chain.

So far, this leads to heavy memory duplication: We have the CRT chain
in the I/O buffer, then parse (and, thereby, copy) it to a
`mbedtls_x509_crt` structure, and then make another copy of the
peer's public key, plus the overhead from the MPI and ECP structures.

This inefficiency will soon go away to a significant extend, because:
- Another PR adds functionality to parse CRTs without taking
  ownership of the input buffers. Applying this here will allow
  parsing and verifying the peer's chain without making an additional
  raw copy. The overhead reduces to the size of `mbedtls_x509_crt`,
  the public key, and the DN structures referenced in the CRT.
- Once copyless parsing is in place and the removal of the peer CRT
  is fully implemented, we can extract the public key bounds from
  the parsed certificate and then free the entire chain before
  parsing the public key again. This means that we never store
  the parsed public key twice at the same time.
2019-06-19 10:25:02 +01:00
Hanno Becker 3bf8cdf2f8 Add field for peer's raw public key to TLS handshake param structure
When removing the (session-local) copy of the peer's CRT chain, we must
keep a handshake-local copy of the peer's public key, as (naturally) every
key exchange will make use of that public key at some point to verify that
the peer actually owns the corresponding private key (e.g., verify signatures
from ServerKeyExchange or CertificateVerify, or encrypt a PMS in a RSA-based
exchange, or extract static (EC)DH parameters).

This commit adds a PK context field `peer_pubkey` to the handshake parameter
structure `mbedtls_handshake_params_init()` and adapts the init and free
functions accordingly. It does not yet make actual use of the new field.
2019-06-19 10:25:01 +01:00
Hanno Becker 2e6d34761f Remove peer CRT from mbedtls_ssl_session if !KEEP_PEER_CERT 2019-06-19 10:25:01 +01:00
Hanno Becker 4a2f8e584f Add peer CRT digest to session tickets
This commit changes the format of session tickets to include
the digest of the peer's CRT if MBEDTLS_SSL_KEEP_PEER_CERTIFICATE
is disabled.

This commit does not yet remove the peer CRT itself.
2019-06-19 10:25:01 +01:00
Hanno Becker e4aeb76a2c Parse and verify peer CRT chain in local variable
`mbedtls_ssl_parse_certificate()` parses the peer's certificate chain
directly into the `peer_cert` field of the `mbedtls_ssl_session`
structure being established. To allow to optionally remove this field
from the session structure, this commit changes this to parse the peer's
chain into a local variable instead first, which can then either be freed
after CRT verification - in case the chain should not be stored - or
mapped to the `peer_cert` if it should be kept. For now, only the latter
is implemented.
2019-06-19 10:25:01 +01:00
Hanno Becker df75938b84 Mitigate triple handshake attack by comparing digests only
This paves the way for the removal of the peer CRT chain from
`mbedtls_ssl_session`.
2019-06-19 10:25:01 +01:00
Hanno Becker 3008d2869f Compute digest of peer's end-CRT in mbedtls_ssl_parse_certificate() 2019-06-19 10:25:01 +01:00
Hanno Becker 9fb6e2e203 Extend mbedtls_ssl_session by buffer holding peer CRT digest 2019-06-19 10:25:01 +01:00
Hanno Becker 58fccf2f62 Give ssl_session_copy() external linkage
A subsequent commit will need this function in the session ticket
and session cache implementations. As the latter are server-side,
this commit also removes the MBEDTLS_SSL_CLI_C guard.

For now, the function is declared in ssl_internal.h and hence not
part of the public API.
2019-06-19 10:25:01 +01:00
Hanno Becker 35e41771fe Allow passing any X.509 CRT chain to ssl_parse_certificate_chain()
This commit modifies the helper `ssl_parse_certificate_chain()` to
accep any target X.509 CRT chain instead of hardcoding it to
`session_negotiate->peer_cert`. This increases modularity and paves
the way towards removing `mbedtls_ssl_session::peer_cert`.
2019-06-19 10:25:01 +01:00
Hanno Becker 3cf5061091 Introduce helper function for peer CRT chain verification 2019-06-19 10:25:01 +01:00
Hanno Becker a7c1df6321 Don't progress TLS state machine on peer CRT chain parsing error 2019-06-19 10:25:01 +01:00
Hanno Becker 6b9a6f3f37 Add helper function to check whether a CRT msg is expected
This commit adds a helper function `ssl_parse_certificate_coordinate()`
which checks whether a `Certificate` message is expected from the peer.

The logic is the following:
- For ciphersuites which don't use server-side CRTs, no Certificate
  message is expected (neither for the server, nor the client).
- On the server, no client certificate is expected in the following cases:
  * The server server didn't request a Certificate, which is controlled
    by the `authmode` setting.
  * A RSA-PSK suite is used; this is the only suite using server CRTs
    but not allowing client-side authentication.
2019-06-19 10:25:01 +01:00
Hanno Becker 5097cba93c Introduce helper function to determine whether suite uses server CRT
This commit introduces a static helper function

   `mbedtls_ssl_ciphersuite_uses_srv_cert()`

which determines whether a ciphersuite may make use of server-side CRTs.

This function is in turn uses in `mbedtls_ssl_parse_certificate()` to
skip certificate parsing for ciphersuites which don't involve CRTs.

Note: Ciphersuites not using server-side CRTs don't allow client-side CRTs
either, so it is safe to guard `mbedtls_ssl_{parse/write}_certificate()`
this way.

Note: Previously, the code uses a positive check over the suites

- MBEDTLS_KEY_EXCHANGE_PSK
- MBEDTLS_KEY_EXCHANGE_DHE_PSK
- MBEDTLS_KEY_EXCHANGE_ECDHE_PSK
- MBEDTLS_KEY_EXCHANGE_ECJPAKE,

while now, it uses a negative check over `mbedtls_ssl_ciphersuite_uses_srv_cert()`,
which checks for the suites

- MBEDTLS_KEY_EXCHANGE_RSA
- MBEDTLS_KEY_EXCHANGE_RSA_PSK
- MBEDTLS_KEY_EXCHANGE_DHE_RSA
- MBEDTLS_KEY_EXCHANGE_ECDH_RSA
- MBEDTLS_KEY_EXCHANGE_ECDHE_RSA
- MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA
- MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA

This is equivalent since, together, those are all ciphersuites.
Quoting ssl_ciphersuites.h:

```
typedef enum {
    MBEDTLS_KEY_EXCHANGE_NONE = 0,
    MBEDTLS_KEY_EXCHANGE_RSA,
    MBEDTLS_KEY_EXCHANGE_DHE_RSA,
    MBEDTLS_KEY_EXCHANGE_ECDHE_RSA,
    MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA,
    MBEDTLS_KEY_EXCHANGE_PSK,
    MBEDTLS_KEY_EXCHANGE_DHE_PSK,
    MBEDTLS_KEY_EXCHANGE_RSA_PSK,
    MBEDTLS_KEY_EXCHANGE_ECDHE_PSK,
    MBEDTLS_KEY_EXCHANGE_ECDH_RSA,
    MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA,
    MBEDTLS_KEY_EXCHANGE_ECJPAKE,
} mbedtls_key_exchange_type_t;
```
2019-06-19 10:25:01 +01:00
Hanno Becker b71e90acc5 Use helper macro to detect whether some ciphersuite uses CRTs 2019-06-19 10:25:01 +01:00
Hanno Becker 613d490bf1 Unify state machine update in mbedtls_ssl_parse_certificate()
The handler `mbedtls_ssl_parse_certificate()` for incoming `Certificate`
messages contains many branches updating the handshake state. For easier
reasoning about state evolution, this commit introduces a single code-path
updating the state machine at the end of `mbedtls_ssl_parse_certificate()`.
2019-06-19 10:25:01 +01:00
Hanno Becker a46c287796 Clear peer's CRT chain outside before parsing new one
If an attempt for session resumption fails, the `session_negotiate` structure
might be partially filled, and in particular already contain a peer certificate
structure. This certificate structure needs to be freed before parsing the
certificate sent in the `Certificate` message.

This commit moves the code-path taking care of this from the helper
function `ssl_parse_certificate_chain()`, whose purpose should be parsing
only, to the top-level handler `mbedtls_ssl_parse_certificate()`.

The fact that we don't know the state of `ssl->session_negotiate` after
a failed attempt for session resumption is undesirable, and a separate
issue #2414 has been opened to improve on this.
2019-06-19 10:25:01 +01:00
Hanno Becker b8a085744f Introduce helper to check for no-CRT notification from client
This commit introduces a server-side static helper function
`ssl_srv_check_client_no_crt_notification()`, which checks if
the message we received during the incoming certificate state
notifies the server of the lack of certificate on the client.

For SSLv3, such a notification comes as a specific alert,
while for all other TLS versions, it comes as a `Certificate`
handshake message with an empty CRT list.
2019-06-19 10:25:01 +01:00
Hanno Becker 8794fd927c Introduce CRT counter to CRT chain parsing function
So far, we've used the `peer_cert` pointer to detect whether
we're parsing the first CRT, but that will soon be removed
if `MBEDTLS_SSL_KEEP_PEER_CERTIFICATE` is unset.
2019-06-19 10:25:01 +01:00
Hanno Becker 22141593e1 Introduce helper function to clear peer CRT from session structure
This commit introduces a helper function `ssl_clear_peer_cert()`
which frees all data related to the peer's certificate from an
`mbedtls_ssl_session` structure. Currently, this is the peer's
certificate itself, while eventually, it'll be its digest only.
2019-06-19 10:25:01 +01:00
Hanno Becker 933b9fc815 Break overly long line in definition of mbedtls_ssl_get_session() 2019-06-19 10:25:01 +01:00
Hanno Becker 1332f35a4e Don't reuse CRT from initial handshake during renegotiation
After mitigating the 'triple handshake attack' by checking that
the peer's end-CRT didn't change during renegotation, the current
code avoids re-parsing the CRT by moving the CRT-pointer from the
old session to the new one. While efficient, this will no longer
work once only the hash of the peer's CRT is stored beyond the
handshake.

This commit removes the code-path moving the old CRT, and instead
frees the entire peer CRT chain from the initial handshake as soon
as the 'triple handshake attack' protection has completed.
2019-06-19 10:25:01 +01:00
Simon Butcher f2ef573f2b Merge remote-tracking branch 'origin/pr/598' into baremetal 2019-06-18 15:00:02 +01:00
Manuel Pégourié-Gonnard 889bbc70b6 Fix unreachable code warnings with armc5
Some TLS-only code paths were not protected by an #ifdef and while some
compiler are happy to just silently remove them, armc5 complains:

Warning:  #111-D: statement is unreachable

Let's make armc5 happy.
2019-06-18 10:56:09 +02:00
Manuel Pégourié-Gonnard cc71c77b5d Fix typos, grammar and wording in documentation 2019-06-18 10:09:27 +02:00
Manuel Pégourié-Gonnard d87601e3ef Declare and document ssl_context_save()/load()
Also introduce stub definitions so that things compile and link.
2019-06-18 10:09:27 +02:00