Commit graph

1035 commits

Author SHA1 Message Date
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
Simon Butcher c725e4b34e Merge remote-tracking branch 'origin/pr/590' into baremetal 2019-06-17 17:57:26 +01:00
Simon Butcher 01a8eb21d3 Merge remote-tracking branch 'origin/pr/585' into baremetal 2019-06-17 17:53:41 +01:00
Manuel Pégourié-Gonnard ec1c222947 Fix a few style issues 2019-06-12 10:18:26 +02:00
Manuel Pégourié-Gonnard 8794a4290d Clarify a few more comments and documentation 2019-06-11 10:04:57 +02:00
Jarno Lamsa 18b9a491e1 Disable the enforce flag by default 2019-06-10 15:23:29 +03:00
Jarno Lamsa 842be16800 Check for the enforcing and fail handshake if the peer doesn't support 2019-06-10 15:05:33 +03:00