From ad4a1379651a442316df77275c14f53f67ef53e8 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Fri, 3 May 2019 13:06:44 +0100 Subject: [PATCH] Add CID configuration API Context: The CID draft does not require that the length of CIDs used for incoming records must not change in the course of a connection. Since the record header does not contain a length field for the CID, this means that if CIDs of varying lengths are used, the CID length must be inferred from other aspects of the record header (such as the epoch) and/or by means outside of the protocol, e.g. by coding its length in the CID itself. Inferring the CID length from the record's epoch is theoretically possible in DTLS 1.2, but it requires the information about the epoch to be present even if the epoch is no longer used: That's because one should silently drop records from old epochs, but not the entire datagrams to which they belong (there might be entire flights in a single datagram, including a change of epoch); however, in order to do so, one needs to parse the record's content length, the position of which is only known once the CID length for the epoch is known. In conclusion, it puts a significant burden on the implementation to infer the CID length from the record epoch, which moreover mangles record processing with the high-level logic of the protocol (determining which epochs are in use in which flights, when they are changed, etc. -- this would normally determine when we drop epochs). Moreover, with DTLS 1.3, CIDs are no longer uniquely associated to epochs, but every epoch may use a set of CIDs of varying lengths -- in that case, it's even theoretically impossible to do record header parsing based on the epoch configuration only. We must therefore seek a way for standalone record header parsing, which means that we must either (a) fix the CID lengths for incoming records, or (b) allow the application-code to configure a callback to implement an application-specific CID parsing which would somehow infer the length of the CID from the CID itself. Supporting multiple lengths for incoming CIDs significantly increases complexity while, on the other hand, the restriction to a fixed CID length for incoming CIDs (which the application controls - in contrast to the lengths of the CIDs used when writing messages to the peer) doesn't appear to severely limit the usefulness of the CID extension. Therefore, the initial implementation of the CID feature will require a fixed length for incoming CIDs, which is what this commit enforces, in the following way: In order to avoid a change of API in case support for variable lengths CIDs shall be added at some point, we keep mbedtls_ssl_set_cid(), which includes a CID length parameter, but add a new API mbedtls_ssl_conf_cid_len() which applies to an SSL configuration, and which fixes the CID length that any call to mbetls_ssl_set_cid() which applies to an SSL context that is bound to the given SSL configuration must use. While this creates a slight redundancy of parameters, it allows to potentially add an API like mbedtls_ssl_conf_cid_len_cb() later which could allow users to register a callback which dynamically infers the length of a CID at record header parsing time, without changing the rest of the API. --- include/mbedtls/config.h | 11 ++++++---- include/mbedtls/ssl.h | 41 ++++++++++++++++++++++++++++++++++++++ library/ssl_tls.c | 22 +++++++++++++++----- programs/ssl/ssl_client2.c | 13 ++++++++++++ programs/ssl/ssl_server2.c | 13 ++++++++++++ 5 files changed, 91 insertions(+), 9 deletions(-) diff --git a/include/mbedtls/config.h b/include/mbedtls/config.h index 262a9be97..6f6d7f064 100644 --- a/include/mbedtls/config.h +++ b/include/mbedtls/config.h @@ -1334,15 +1334,18 @@ * which allows to identify DTLS connections across changes * in the underlying transport. * - * Setting this option enables the SSL APIs `mbedtls_ssl_set_cid()` - * and `mbedtls_ssl_get_peer_cid()`. See their documentation for more - * information. + * Setting this option enables the SSL APIs `mbedtls_ssl_set_cid()`, + * `mbedtls_ssl_get_peer_cid()` and `mbedtls_ssl_conf_cid_len()`. + * See their documentation for more information. * * \warning The Connection ID extension is still in draft state. * We make no stability promises for the availability * or the shape of the API controlled by this option. * - * See also MBEDTLS_SSL_CID_OUT_LEN_MAX and MBEDTLS_SSL_CID_IN_LEN_MAX. + * The maximum lengths of outgoing and incoming CIDs can be configured + * through the options + * - MBEDTLS_SSL_CID_OUT_LEN_MAX + * - MBEDTLS_SSL_CID_IN_LEN_MAX. * * Requires: MBEDTLS_SSL_PROTO_DTLS * diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index fc84d628f..1ae8cf16a 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -959,6 +959,10 @@ struct mbedtls_ssl_config void *p_export_keys; /*!< context for key export callback */ #endif +#if defined(MBEDTLS_SSL_CID) + size_t cid_len; /*!< The length of CIDs for incoming DTLS records. */ +#endif /* MBEDTLS_SSL_CID */ + #if defined(MBEDTLS_X509_CRT_PARSE_C) const mbedtls_x509_crt_profile *cert_profile; /*!< verification profile */ mbedtls_ssl_key_cert *key_cert; /*!< own certificate/key pair(s) */ @@ -1559,6 +1563,12 @@ void mbedtls_ssl_set_bio( mbedtls_ssl_context *ssl, * This parameter is unused if \p enabled is set to * MBEDTLS_SSL_CID_DISABLED. * + * \note The value of \p own_cid_len must match the value of the + * \c len parameter passed to mbedtls_ssl_conf_cid_len() + * when configuring the ::mbedtls_ssl_config that \p ssl + * is bound to. See the documentation of + * mbedtls_ssl_conf_cid_len() for more information. + * * \note This CID configuration applies to subsequent handshakes * performed on the SSL context \p ssl, but does not trigger * one. You still have to call `mbedtls_ssl_handshake()` @@ -2293,6 +2303,37 @@ int mbedtls_ssl_set_session( mbedtls_ssl_context *ssl, const mbedtls_ssl_session void mbedtls_ssl_conf_ciphersuites( mbedtls_ssl_config *conf, const int *ciphersuites ); +#if defined(MBEDTLS_SSL_CID) +/** + * \brief (STUB) Specify the length of CIDs for incoming encrypted + * DTLS records. (Default: \c 0) + * + * \param conf The SSL configuration to modify. + * \param len The length in Bytes of the CID fields in encrypted + * DTLS records using the CID mechanism. This must + * not be larger than #MBEDTLS_SSL_CID_OUT_LEN_MAX. + * + * \note The CID draft does not mandate that incoming CIDs + * have equal lengths, but support for varying lengths + * significantly complicates record header parsing by + * requiring a user-specified callback to perform the + * CID parsing, and Mbed TLS doesn't currently support it. + * + * \note The connection-specific API mbedtls_ssl_set_cid() + * must use the value of \p len as the value for its + * \c own_cid_len parameter, rendering the latter + * redundant at the moment. However, once variable + * length incoming CIDs are supported, the \c own_cid_len + * parameter in mbedtls_ssl_set_cid() will be flexible, and + * it is added already now to avoid a change of API. + * + * \return \c 0 on success. + * \return #MBEDTLS_ERR_SSL_BAD_INPUT_DATA if \p own_cid_len + * is too large. + */ +int mbedtls_ssl_conf_cid_len( mbedtls_ssl_config *conf, size_t len ); +#endif /* MBEDTLS_SSL_CID */ + /** * \brief Set the list of allowed ciphersuites and the * preference order for a specific version of the protocol. diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 8fb98ca96..ab2ae6fe2 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -120,6 +120,18 @@ static void ssl_update_in_pointers( mbedtls_ssl_context *ssl ); #if defined(MBEDTLS_SSL_CID) /* Top-level Connection ID API */ +/* WARNING: The CID feature isn't fully implemented yet + * and will not be used. */ +int mbedtls_ssl_conf_cid_len( mbedtls_ssl_config *conf, + size_t len ) +{ + if( len > MBEDTLS_SSL_CID_IN_LEN_MAX ) + return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA ); + + conf->cid_len = len; + return( 0 ); +} + /* WARNING: The CID feature isn't fully implemented yet * and will not be used. */ int mbedtls_ssl_set_cid( mbedtls_ssl_context *ssl, @@ -137,12 +149,13 @@ int mbedtls_ssl_set_cid( mbedtls_ssl_context *ssl, return( 0 ); } MBEDTLS_SSL_DEBUG_MSG( 3, ( "Enable use of CID extension." ) ); + MBEDTLS_SSL_DEBUG_BUF( 3, "Own CID", own_cid, own_cid_len ); - if( own_cid_len > MBEDTLS_SSL_CID_IN_LEN_MAX ) + if( own_cid_len != ssl->conf->cid_len ) { - MBEDTLS_SSL_DEBUG_MSG( 3, ( "CID too large: Maximum %u, actual %u", - (unsigned) MBEDTLS_SSL_CID_IN_LEN_MAX, - (unsigned) own_cid_len ) ); + MBEDTLS_SSL_DEBUG_MSG( 3, ( "CID length %u does not match CID length %u in config", + (unsigned) own_cid_len, + (unsigned) ssl->conf->cid_len ) ); return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA ); } @@ -151,7 +164,6 @@ int mbedtls_ssl_set_cid( mbedtls_ssl_context *ssl, * MBEDTLS_SSL_CID_IN_LEN_MAX at most 255. */ ssl->own_cid_len = (uint8_t) own_cid_len; - MBEDTLS_SSL_DEBUG_BUF( 3, "Own CID", own_cid, own_cid_len ); return( 0 ); } diff --git a/programs/ssl/ssl_client2.c b/programs/ssl/ssl_client2.c index cb5bea7dc..9080f9e80 100644 --- a/programs/ssl/ssl_client2.c +++ b/programs/ssl/ssl_client2.c @@ -1738,6 +1738,19 @@ int main( int argc, char *argv[] ) memset( peer_crt_info, 0, sizeof( peer_crt_info ) ); #endif /* MBEDTLS_X509_CRT_PARSE_C */ +#if defined(MBEDTLS_SSL_CID) + if( opt.cid_enabled == 1 ) + { + ret = mbedtls_ssl_conf_cid_len( &conf, cid_len ); + if( ret != 0 ) + { + mbedtls_printf( " failed\n ! mbedtls_ssl_conf_cid_len returned %d\n\n", + ret ); + goto exit; + } + } +#endif /* MBEDTLS_SSL_CID */ + if( opt.auth_mode != DFL_AUTH_MODE ) mbedtls_ssl_conf_authmode( &conf, opt.auth_mode ); diff --git a/programs/ssl/ssl_server2.c b/programs/ssl/ssl_server2.c index a94ddac8e..2a791fc47 100644 --- a/programs/ssl/ssl_server2.c +++ b/programs/ssl/ssl_server2.c @@ -2637,6 +2637,19 @@ int main( int argc, char *argv[] ) }; #endif +#if defined(MBEDTLS_SSL_CID) + if( opt.cid_enabled == 1 ) + { + ret = mbedtls_ssl_conf_cid_len( &conf, cid_len ); + if( ret != 0 ) + { + mbedtls_printf( " failed\n ! mbedtls_ssl_conf_cid_len returned %d\n\n", + ret ); + goto exit; + } + } +#endif /* MBEDTLS_SSL_CID */ + #if defined(MBEDTLS_SSL_TRUNCATED_HMAC) if( opt.trunc_hmac != DFL_TRUNC_HMAC ) mbedtls_ssl_conf_truncated_hmac( &conf, opt.trunc_hmac );