From 9fc15ea4cc726ebfaf3a572cea42750e87961b4c Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 4 May 2020 12:00:47 +0100 Subject: [PATCH 01/38] Introduce config option for experimental TLS 1.3 specific features Signed-off-by: Hanno Becker --- include/mbedtls/config.h | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/include/mbedtls/config.h b/include/mbedtls/config.h index 901e26d89..da8b6b31e 100644 --- a/include/mbedtls/config.h +++ b/include/mbedtls/config.h @@ -1706,6 +1706,22 @@ */ #define MBEDTLS_SSL_PROTO_TLS1_2 +/** + * \def MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL + * + * This is an experimental macro used to selectively enable parts + * of the code that solely contribute to the ongoing development of + * a prototype implementation for TLS 1.3 and DTLS 1.3 but aren't + * used otherwise. + * + * \warning Features under the control of this macro are experimental + * and don't come with any stability guarantees. + * + * Uncomment this macro to enable experimental and partial + * functionality specific to TLS 1.3. + */ +#define MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL + /** * \def MBEDTLS_SSL_PROTO_DTLS * From a711f6e27798db931636ac8ced1666ac1bd5576a Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 4 May 2020 12:03:51 +0100 Subject: [PATCH 02/38] Add 'build+unit test' test for experimental TLS 1.3 code to all.sh Signed-off-by: Hanno Becker --- tests/scripts/all.sh | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 5ea1c35d1..8761c284b 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -1701,6 +1701,15 @@ component_test_allow_sha1 () { if_build_succeeded tests/ssl-opt.sh -f SHA-1 } +component_test_tls13_experimental () { + msg "build: default config with MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL enabled" + scripts/config.pl set MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL + CC=gcc cmake -D CMAKE_BUILD_TYPE:String=Asan . + make + msg "test: default config with MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL enabled" + make test +} + component_build_mingw () { msg "build: Windows cross build - mingw64, make (Link Library)" # ~ 30s make CC=i686-w64-mingw32-gcc AR=i686-w64-mingw32-ar LD=i686-w64-minggw32-ld CFLAGS='-Werror -Wall -Wextra' WINDOWS_BUILD=1 lib programs From 2ab47dc5cdc22f9800b00243d157937fbc960137 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 4 May 2020 12:19:12 +0100 Subject: [PATCH 03/38] Add internal version identifier for TLS 1.3 Signed-off-by: Hanno Becker --- include/mbedtls/ssl.h | 1 + 1 file changed, 1 insertion(+) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 7fec65e1d..5869e1560 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -138,6 +138,7 @@ #define MBEDTLS_SSL_MINOR_VERSION_1 1 /*!< TLS v1.0 */ #define MBEDTLS_SSL_MINOR_VERSION_2 2 /*!< TLS v1.1 */ #define MBEDTLS_SSL_MINOR_VERSION_3 3 /*!< TLS v1.2 */ +#define MBEDTLS_SSL_MINOR_VERSION_4 4 /*!< TLS v1.3 (experimental) */ #define MBEDTLS_SSL_TRANSPORT_STREAM 0 /*!< TLS */ #define MBEDTLS_SSL_TRANSPORT_DATAGRAM 1 /*!< DTLS */ From 581bc1b90895e658774ff2640bb258803cc767ff Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 4 May 2020 12:20:03 +0100 Subject: [PATCH 04/38] Remove ref to CID from inner plaintext parsing/building functions The internal functions `ssl_cid_{build/parse}_inner_plaintext()` implement the TLSInnerPlaintext mechanism used by DTLS 1.2 + CID in order to allow for flexible length padding and to protect the true content type of a record. This feature is also present in TLS 1.3 support for which is under development. As a preparatory step towards sharing the code between the case of DTLS 1.2 + CID and TLS 1.3, this commit renames `ssl_cid_{build/parse}_inner_plaintext()` to `ssl_{build/parse}_inner_plaintext()`. Signed-off-by: Hanno Becker --- library/ssl_msg.c | 34 ++++++++++++++++++---------------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/library/ssl_msg.c b/library/ssl_msg.c index 976fc7b00..e5331a746 100644 --- a/library/ssl_msg.c +++ b/library/ssl_msg.c @@ -342,14 +342,16 @@ static void ssl_read_memory( unsigned char *p, size_t len ) */ #if defined(MBEDTLS_SSL_DTLS_CONNECTION_ID) -/* This functions transforms a DTLS plaintext fragment and a record content - * type into an instance of the DTLSInnerPlaintext structure: +/* This functions transforms a (D)TLS plaintext fragment and a record content + * type into an instance of the (D)TLSInnerPlaintext structure. This is used + * in DTLS 1.2 + CID and within TLS 1.3 to allow flexible padding and to protect + * a record's content type. * * struct { * opaque content[DTLSPlaintext.length]; * ContentType real_type; * uint8 zeros[length_of_padding]; - * } DTLSInnerPlaintext; + * } (D)TLSInnerPlaintext; * * Input: * - `content`: The beginning of the buffer holding the @@ -360,18 +362,18 @@ static void ssl_read_memory( unsigned char *p, size_t len ) * - `rec_type`: The desired record content type. * * Output: - * - `content`: The beginning of the resulting DTLSInnerPlaintext structure. - * - `*content_size`: The length of the resulting DTLSInnerPlaintext structure. + * - `content`: The beginning of the resulting (D)TLSInnerPlaintext structure. + * - `*content_size`: The length of the resulting (D)TLSInnerPlaintext structure. * * Returns: * - `0` on success. * - A negative error code if `max_len` didn't offer enough space * for the expansion. */ -static int ssl_cid_build_inner_plaintext( unsigned char *content, - size_t *content_size, - size_t remaining, - uint8_t rec_type ) +static int ssl_build_inner_plaintext( unsigned char *content, + size_t *content_size, + size_t remaining, + uint8_t rec_type ) { size_t len = *content_size; size_t pad = ( MBEDTLS_SSL_CID_PADDING_GRANULARITY - @@ -395,9 +397,9 @@ static int ssl_cid_build_inner_plaintext( unsigned char *content, return( 0 ); } -/* This function parses a DTLSInnerPlaintext structure. - * See ssl_cid_build_inner_plaintext() for details. */ -static int ssl_cid_parse_inner_plaintext( unsigned char const *content, +/* This function parses a (D)TLSInnerPlaintext structure. + * See ssl_build_inner_plaintext() for details. */ +static int ssl_parse_inner_plaintext( unsigned char const *content, size_t *content_size, uint8_t *rec_type ) { @@ -586,12 +588,12 @@ int mbedtls_ssl_encrypt_buf( mbedtls_ssl_context *ssl, { /* * Wrap plaintext into DTLSInnerPlaintext structure. - * See ssl_cid_build_inner_plaintext() for more information. + * See ssl_build_inner_plaintext() for more information. * * Note that this changes `rec->data_len`, and hence * `post_avail` needs to be recalculated afterwards. */ - if( ssl_cid_build_inner_plaintext( data, + if( ssl_build_inner_plaintext( data, &rec->data_len, post_avail, rec->type ) != 0 ) @@ -1552,8 +1554,8 @@ int mbedtls_ssl_decrypt_buf( mbedtls_ssl_context const *ssl, #if defined(MBEDTLS_SSL_DTLS_CONNECTION_ID) if( rec->cid_len != 0 ) { - ret = ssl_cid_parse_inner_plaintext( data, &rec->data_len, - &rec->type ); + ret = ssl_parse_inner_plaintext( data, &rec->data_len, + &rec->type ); if( ret != 0 ) return( MBEDTLS_ERR_SSL_INVALID_RECORD ); } From 7d343ecf06c45d8d99d3fd8bd7368a5b4f737e14 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 4 May 2020 12:29:05 +0100 Subject: [PATCH 05/38] Add note on inner plaintext parsing to ssl_transform documentation Signed-off-by: Hanno Becker --- include/mbedtls/ssl_internal.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/include/mbedtls/ssl_internal.h b/include/mbedtls/ssl_internal.h index e92381c33..cd881eb02 100644 --- a/include/mbedtls/ssl_internal.h +++ b/include/mbedtls/ssl_internal.h @@ -554,6 +554,10 @@ typedef struct mbedtls_ssl_hs_buffer mbedtls_ssl_hs_buffer; * time with the 8-byte record sequence number, without prepending the * latter to the encrypted record. * + * Additionally, DTLS 1.2 + CID as well as TLS 1.3 use an inner plaintext + * which allows to add flexible length padding and to hide a record's true + * content type. + * * In addition to type and version, the following parameters are relevant: * - The symmetric cipher algorithm to be used. * - The (static) encryption/decryption keys for the cipher. From ccc13d03c3b0f8c73031d4036075466c426f5eee Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 4 May 2020 12:30:04 +0100 Subject: [PATCH 06/38] TLS 1.3: Implement TLSInnerPlaintext parsing/building Signed-off-by: Hanno Becker --- library/ssl_msg.c | 40 ++++++++++++++++++++++++++++++++++++++-- 1 file changed, 38 insertions(+), 2 deletions(-) diff --git a/library/ssl_msg.c b/library/ssl_msg.c index e5331a746..e739e2fb6 100644 --- a/library/ssl_msg.c +++ b/library/ssl_msg.c @@ -341,7 +341,8 @@ static void ssl_read_memory( unsigned char *p, size_t len ) * Encryption/decryption functions */ -#if defined(MBEDTLS_SSL_DTLS_CONNECTION_ID) +#if defined(MBEDTLS_SSL_DTLS_CONNECTION_ID) || \ + defined(MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL) /* This functions transforms a (D)TLS plaintext fragment and a record content * type into an instance of the (D)TLSInnerPlaintext structure. This is used * in DTLS 1.2 + CID and within TLS 1.3 to allow flexible padding and to protect @@ -418,7 +419,8 @@ static int ssl_parse_inner_plaintext( unsigned char const *content, return( 0 ); } -#endif /* MBEDTLS_SSL_DTLS_CONNECTION_ID */ +#endif /* MBEDTLS_SSL_DTLS_CONNECTION_ID || + MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL */ /* `add_data` must have size 13 Bytes if the CID extension is disabled, * and 13 + 1 + CID-length Bytes if the CID extension is enabled. */ @@ -576,6 +578,28 @@ int mbedtls_ssl_encrypt_buf( mbedtls_ssl_context *ssl, return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA ); } +#if defined(MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL) + if( transform->minor_ver == MBEDTLS_SSL_MINOR_VERSION_4 ) + { + /* + * Wrap plaintext into TLSInnerPlaintext structure. + * See ssl_build_inner_plaintext() for more information. + * + * Note that this changes `rec->data_len`, and hence + * `post_avail` needs to be recalculated afterwards. + */ + if( ssl_build_inner_plaintext( data, + &rec->data_len, + post_avail, + rec->type ) != 0 ) + { + return( MBEDTLS_ERR_SSL_BUFFER_TOO_SMALL ); + } + + rec->type = MBEDTLS_SSL_MSG_APPLICATION_DATA; + } +#endif /* MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL */ + #if defined(MBEDTLS_SSL_DTLS_CONNECTION_ID) /* * Add CID information @@ -1551,6 +1575,18 @@ int mbedtls_ssl_decrypt_buf( mbedtls_ssl_context const *ssl, return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); } +#if defined(MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL) + if( transform->minor_ver == MBEDTLS_SSL_MINOR_VERSION_4 ) + { + /* Remove inner padding and infer true content type. */ + ret = ssl_parse_inner_plaintext( data, &rec->data_len, + &rec->type ); + + if( ret != 0 ) + return( MBEDTLS_ERR_SSL_INVALID_RECORD ); + } +#endif /* MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL */ + #if defined(MBEDTLS_SSL_DTLS_CONNECTION_ID) if( rec->cid_len != 0 ) { From 3169dad48bf0afa0694823c986c45735ca800904 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Thu, 7 May 2020 14:54:07 +0100 Subject: [PATCH 07/38] Add unit tests for TLS 1.3 record encryption Signed-off-by: Hanno Becker --- tests/suites/test_suite_ssl.data | 56 ++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/tests/suites/test_suite_ssl.data b/tests/suites/test_suite_ssl.data index 9af6a5ca0..e71a4cb87 100644 --- a/tests/suites/test_suite_ssl.data +++ b/tests/suites/test_suite_ssl.data @@ -4154,6 +4154,10 @@ Record crypt, AES-128-GCM, 1.2 depends_on:MBEDTLS_AES_C:MBEDTLS_SSL_PROTO_TLS1_2:MBEDTLS_GCM_C ssl_crypt_record:MBEDTLS_CIPHER_AES_128_GCM:MBEDTLS_MD_MD5:0:0:MBEDTLS_SSL_MINOR_VERSION_3:0:0 +Record crypt, AES-128-GCM, 1.3 +depends_on:MBEDTLS_AES_C:MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL:MBEDTLS_GCM_C +ssl_crypt_record:MBEDTLS_CIPHER_AES_128_GCM:MBEDTLS_MD_MD5:0:0:MBEDTLS_SSL_MINOR_VERSION_4:0:0 + Record crypt, AES-128-GCM, 1.2, CID 4+4 depends_on:MBEDTLS_SSL_DTLS_CONNECTION_ID:MBEDTLS_AES_C:MBEDTLS_SSL_PROTO_TLS1_2:MBEDTLS_GCM_C ssl_crypt_record:MBEDTLS_CIPHER_AES_128_GCM:MBEDTLS_MD_MD5:0:0:MBEDTLS_SSL_MINOR_VERSION_3:4:4 @@ -4178,6 +4182,10 @@ Record crypt, AES-192-GCM, 1.2 depends_on:MBEDTLS_AES_C:MBEDTLS_SSL_PROTO_TLS1_2:MBEDTLS_GCM_C ssl_crypt_record:MBEDTLS_CIPHER_AES_192_GCM:MBEDTLS_MD_MD5:0:0:MBEDTLS_SSL_MINOR_VERSION_3:0:0 +Record crypt, AES-192-GCM, 1.3 +depends_on:MBEDTLS_AES_C:MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL:MBEDTLS_GCM_C +ssl_crypt_record:MBEDTLS_CIPHER_AES_192_GCM:MBEDTLS_MD_MD5:0:0:MBEDTLS_SSL_MINOR_VERSION_4:0:0 + Record crypt, AES-192-GCM, 1.2, CID 4+4 depends_on:MBEDTLS_SSL_DTLS_CONNECTION_ID:MBEDTLS_AES_C:MBEDTLS_SSL_PROTO_TLS1_2:MBEDTLS_GCM_C ssl_crypt_record:MBEDTLS_CIPHER_AES_192_GCM:MBEDTLS_MD_MD5:0:0:MBEDTLS_SSL_MINOR_VERSION_3:4:4 @@ -4202,6 +4210,10 @@ Record crypt, AES-256-GCM, 1.2 depends_on:MBEDTLS_AES_C:MBEDTLS_SSL_PROTO_TLS1_2:MBEDTLS_GCM_C ssl_crypt_record:MBEDTLS_CIPHER_AES_256_GCM:MBEDTLS_MD_MD5:0:0:MBEDTLS_SSL_MINOR_VERSION_3:0:0 +Record crypt, AES-192-GCM, 1.3 +depends_on:MBEDTLS_AES_C:MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL:MBEDTLS_GCM_C +ssl_crypt_record:MBEDTLS_CIPHER_AES_256_GCM:MBEDTLS_MD_MD5:0:0:MBEDTLS_SSL_MINOR_VERSION_4:0:0 + Record crypt, AES-256-GCM, 1.2, CID 4+4 depends_on:MBEDTLS_SSL_DTLS_CONNECTION_ID:MBEDTLS_AES_C:MBEDTLS_SSL_PROTO_TLS1_2:MBEDTLS_GCM_C ssl_crypt_record:MBEDTLS_CIPHER_AES_256_GCM:MBEDTLS_MD_MD5:0:0:MBEDTLS_SSL_MINOR_VERSION_3:4:4 @@ -4298,6 +4310,10 @@ Record crypt, AES-128-CCM, 1.2 depends_on:MBEDTLS_AES_C:MBEDTLS_SSL_PROTO_TLS1_2:MBEDTLS_CCM_C ssl_crypt_record:MBEDTLS_CIPHER_AES_128_CCM:MBEDTLS_MD_MD5:0:0:MBEDTLS_SSL_MINOR_VERSION_3:0:0 +Record crypt, AES-128-CCM, 1.3 +depends_on:MBEDTLS_AES_C:MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL:MBEDTLS_CCM_C +ssl_crypt_record:MBEDTLS_CIPHER_AES_128_CCM:MBEDTLS_MD_MD5:0:0:MBEDTLS_SSL_MINOR_VERSION_4:0:0 + Record crypt, AES-128-CCM, 1.2, CID 4+4 depends_on:MBEDTLS_SSL_DTLS_CONNECTION_ID:MBEDTLS_AES_C:MBEDTLS_SSL_PROTO_TLS1_2:MBEDTLS_CCM_C ssl_crypt_record:MBEDTLS_CIPHER_AES_128_CCM:MBEDTLS_MD_MD5:0:0:MBEDTLS_SSL_MINOR_VERSION_3:4:4 @@ -4322,6 +4338,10 @@ Record crypt, AES-192-CCM, 1.2 depends_on:MBEDTLS_AES_C:MBEDTLS_SSL_PROTO_TLS1_2:MBEDTLS_CCM_C ssl_crypt_record:MBEDTLS_CIPHER_AES_192_CCM:MBEDTLS_MD_MD5:0:0:MBEDTLS_SSL_MINOR_VERSION_3:0:0 +Record crypt, AES-192-CCM, 1.3 +depends_on:MBEDTLS_AES_C:MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL:MBEDTLS_CCM_C +ssl_crypt_record:MBEDTLS_CIPHER_AES_192_CCM:MBEDTLS_MD_MD5:0:0:MBEDTLS_SSL_MINOR_VERSION_4:0:0 + Record crypt, AES-192-CCM, 1.2, CID 4+4 depends_on:MBEDTLS_SSL_DTLS_CONNECTION_ID:MBEDTLS_AES_C:MBEDTLS_SSL_PROTO_TLS1_2:MBEDTLS_CCM_C ssl_crypt_record:MBEDTLS_CIPHER_AES_192_CCM:MBEDTLS_MD_MD5:0:0:MBEDTLS_SSL_MINOR_VERSION_3:4:4 @@ -4346,6 +4366,10 @@ Record crypt, AES-256-CCM, 1.2 depends_on:MBEDTLS_AES_C:MBEDTLS_SSL_PROTO_TLS1_2:MBEDTLS_CCM_C ssl_crypt_record:MBEDTLS_CIPHER_AES_256_CCM:MBEDTLS_MD_MD5:0:0:MBEDTLS_SSL_MINOR_VERSION_3:0:0 +Record crypt, AES-256-CCM, 1.3 +depends_on:MBEDTLS_AES_C:MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL:MBEDTLS_CCM_C +ssl_crypt_record:MBEDTLS_CIPHER_AES_256_CCM:MBEDTLS_MD_MD5:0:0:MBEDTLS_SSL_MINOR_VERSION_4:0:0 + Record crypt, AES-256-CCM, 1.2, CID 4+4 depends_on:MBEDTLS_SSL_DTLS_CONNECTION_ID:MBEDTLS_AES_C:MBEDTLS_SSL_PROTO_TLS1_2:MBEDTLS_CCM_C ssl_crypt_record:MBEDTLS_CIPHER_AES_256_CCM:MBEDTLS_MD_MD5:0:0:MBEDTLS_SSL_MINOR_VERSION_3:4:4 @@ -5018,10 +5042,18 @@ Record crypt, ChachaPoly depends_on:MBEDTLS_CHACHAPOLY_C:MBEDTLS_SSL_PROTO_TLS1_2 ssl_crypt_record:MBEDTLS_CIPHER_CHACHA20_POLY1305:MBEDTLS_MD_MD5:0:0:MBEDTLS_SSL_MINOR_VERSION_3:0:0 +Record crypt, ChachaPoly, 1.3 +depends_on:MBEDTLS_CHACHAPOLY_C:MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL +ssl_crypt_record:MBEDTLS_CIPHER_CHACHA20_POLY1305:MBEDTLS_MD_MD5:0:0:MBEDTLS_SSL_MINOR_VERSION_4:0:0 + Record crypt, little space, ChachaPoly depends_on:MBEDTLS_CHACHAPOLY_C:MBEDTLS_SSL_PROTO_TLS1_2 ssl_crypt_record_small:MBEDTLS_CIPHER_CHACHA20_POLY1305:MBEDTLS_MD_MD5:0:0:MBEDTLS_SSL_MINOR_VERSION_3:0:0 +Record crypt, little space, ChachaPoly, 1.3 +depends_on:MBEDTLS_CHACHAPOLY_C:MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL +ssl_crypt_record_small:MBEDTLS_CIPHER_CHACHA20_POLY1305:MBEDTLS_MD_MD5:0:0:MBEDTLS_SSL_MINOR_VERSION_4:0:0 + Record crypt, little space, ChachaPoly, CID 4+4 depends_on:MBEDTLS_SSL_DTLS_CONNECTION_ID:MBEDTLS_CHACHAPOLY_C:MBEDTLS_SSL_PROTO_TLS1_2 ssl_crypt_record_small:MBEDTLS_CIPHER_CHACHA20_POLY1305:MBEDTLS_MD_MD5:0:0:MBEDTLS_SSL_MINOR_VERSION_3:4:4 @@ -8554,6 +8586,10 @@ Record crypt, little space, AES-128-GCM, 1.2 depends_on:MBEDTLS_AES_C:MBEDTLS_SSL_PROTO_TLS1_2:MBEDTLS_GCM_C ssl_crypt_record_small:MBEDTLS_CIPHER_AES_128_GCM:MBEDTLS_MD_MD5:0:0:MBEDTLS_SSL_MINOR_VERSION_3:0:0 +Record crypt, little space, AES-128-GCM, 1.3 +depends_on:MBEDTLS_AES_C:MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL:MBEDTLS_GCM_C +ssl_crypt_record_small:MBEDTLS_CIPHER_AES_128_GCM:MBEDTLS_MD_MD5:0:0:MBEDTLS_SSL_MINOR_VERSION_4:0:0 + Record crypt, little space, AES-128-GCM, 1.2, CID 4+4 depends_on:MBEDTLS_SSL_DTLS_CONNECTION_ID:MBEDTLS_AES_C:MBEDTLS_SSL_PROTO_TLS1_2:MBEDTLS_GCM_C ssl_crypt_record_small:MBEDTLS_CIPHER_AES_128_GCM:MBEDTLS_MD_MD5:0:0:MBEDTLS_SSL_MINOR_VERSION_3:4:4 @@ -8578,6 +8614,10 @@ Record crypt, little space, AES-192-GCM, 1.2 depends_on:MBEDTLS_AES_C:MBEDTLS_SSL_PROTO_TLS1_2:MBEDTLS_GCM_C ssl_crypt_record_small:MBEDTLS_CIPHER_AES_192_GCM:MBEDTLS_MD_MD5:0:0:MBEDTLS_SSL_MINOR_VERSION_3:0:0 +Record crypt, little space, AES-192-GCM, 1.3 +depends_on:MBEDTLS_AES_C:MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL:MBEDTLS_GCM_C +ssl_crypt_record_small:MBEDTLS_CIPHER_AES_192_GCM:MBEDTLS_MD_MD5:0:0:MBEDTLS_SSL_MINOR_VERSION_4:0:0 + Record crypt, little space, AES-192-GCM, 1.2, CID 4+4 depends_on:MBEDTLS_SSL_DTLS_CONNECTION_ID:MBEDTLS_AES_C:MBEDTLS_SSL_PROTO_TLS1_2:MBEDTLS_GCM_C ssl_crypt_record_small:MBEDTLS_CIPHER_AES_192_GCM:MBEDTLS_MD_MD5:0:0:MBEDTLS_SSL_MINOR_VERSION_3:4:4 @@ -8602,6 +8642,10 @@ Record crypt, little space, AES-256-GCM, 1.2 depends_on:MBEDTLS_AES_C:MBEDTLS_SSL_PROTO_TLS1_2:MBEDTLS_GCM_C ssl_crypt_record_small:MBEDTLS_CIPHER_AES_256_GCM:MBEDTLS_MD_MD5:0:0:MBEDTLS_SSL_MINOR_VERSION_3:0:0 +Record crypt, little space, AES-256-GCM, 1.3 +depends_on:MBEDTLS_AES_C:MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL:MBEDTLS_GCM_C +ssl_crypt_record_small:MBEDTLS_CIPHER_AES_256_GCM:MBEDTLS_MD_MD5:0:0:MBEDTLS_SSL_MINOR_VERSION_4:0:0 + Record crypt, little space, AES-256-GCM, 1.2, CID 4+4 depends_on:MBEDTLS_SSL_DTLS_CONNECTION_ID:MBEDTLS_AES_C:MBEDTLS_SSL_PROTO_TLS1_2:MBEDTLS_GCM_C ssl_crypt_record_small:MBEDTLS_CIPHER_AES_256_GCM:MBEDTLS_MD_MD5:0:0:MBEDTLS_SSL_MINOR_VERSION_3:4:4 @@ -8698,6 +8742,10 @@ Record crypt, little space, AES-128-CCM, 1.2 depends_on:MBEDTLS_AES_C:MBEDTLS_SSL_PROTO_TLS1_2:MBEDTLS_CCM_C ssl_crypt_record_small:MBEDTLS_CIPHER_AES_128_CCM:MBEDTLS_MD_MD5:0:0:MBEDTLS_SSL_MINOR_VERSION_3:0:0 +Record crypt, little space, AES-128-CCM, 1.3 +depends_on:MBEDTLS_AES_C:MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL:MBEDTLS_CCM_C +ssl_crypt_record_small:MBEDTLS_CIPHER_AES_128_CCM:MBEDTLS_MD_MD5:0:0:MBEDTLS_SSL_MINOR_VERSION_4:0:0 + Record crypt, little space, AES-128-CCM, 1.2, CID 4+4 depends_on:MBEDTLS_SSL_DTLS_CONNECTION_ID:MBEDTLS_AES_C:MBEDTLS_SSL_PROTO_TLS1_2:MBEDTLS_CCM_C ssl_crypt_record_small:MBEDTLS_CIPHER_AES_128_CCM:MBEDTLS_MD_MD5:0:0:MBEDTLS_SSL_MINOR_VERSION_3:4:4 @@ -8722,6 +8770,10 @@ Record crypt, little space, AES-192-CCM, 1.2 depends_on:MBEDTLS_AES_C:MBEDTLS_SSL_PROTO_TLS1_2:MBEDTLS_CCM_C ssl_crypt_record_small:MBEDTLS_CIPHER_AES_192_CCM:MBEDTLS_MD_MD5:0:0:MBEDTLS_SSL_MINOR_VERSION_3:0:0 +Record crypt, little space, AES-192-CCM, 1.3 +depends_on:MBEDTLS_AES_C:MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL:MBEDTLS_CCM_C +ssl_crypt_record_small:MBEDTLS_CIPHER_AES_192_CCM:MBEDTLS_MD_MD5:0:0:MBEDTLS_SSL_MINOR_VERSION_4:0:0 + Record crypt, little space, AES-192-CCM, 1.2, CID 4+4 depends_on:MBEDTLS_SSL_DTLS_CONNECTION_ID:MBEDTLS_AES_C:MBEDTLS_SSL_PROTO_TLS1_2:MBEDTLS_CCM_C ssl_crypt_record_small:MBEDTLS_CIPHER_AES_192_CCM:MBEDTLS_MD_MD5:0:0:MBEDTLS_SSL_MINOR_VERSION_3:4:4 @@ -8746,6 +8798,10 @@ Record crypt, little space, AES-256-CCM, 1.2 depends_on:MBEDTLS_AES_C:MBEDTLS_SSL_PROTO_TLS1_2:MBEDTLS_CCM_C ssl_crypt_record_small:MBEDTLS_CIPHER_AES_256_CCM:MBEDTLS_MD_MD5:0:0:MBEDTLS_SSL_MINOR_VERSION_3:0:0 +Record crypt, little space, AES-256-CCM, 1.3 +depends_on:MBEDTLS_AES_C:MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL:MBEDTLS_CCM_C +ssl_crypt_record_small:MBEDTLS_CIPHER_AES_256_CCM:MBEDTLS_MD_MD5:0:0:MBEDTLS_SSL_MINOR_VERSION_4:0:0 + Record crypt, little space, AES-256-CCM, 1.2, CID 4+4 depends_on:MBEDTLS_SSL_DTLS_CONNECTION_ID:MBEDTLS_AES_C:MBEDTLS_SSL_PROTO_TLS1_2:MBEDTLS_CCM_C ssl_crypt_record_small:MBEDTLS_CIPHER_AES_256_CCM:MBEDTLS_MD_MD5:0:0:MBEDTLS_SSL_MINOR_VERSION_3:4:4 From b2713abb8f4af13a1eaead65ef025fca9d756da6 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Thu, 7 May 2020 14:54:22 +0100 Subject: [PATCH 08/38] Enhance record encryption unit tests by checking hidden content type TLS 1.3 and DTLS 1.2 + CID hide the real content type of a record within the record's inner plaintext, while always using the same content type for the protected record: - TLS 1.3 always uses ApplicationData - DTLS 1.2 + CID always uses a special CID content type. This commit enhances the record encryption unit test to check that the record content type is indeed correctly hidden. Signed-off-by: Hanno Becker --- tests/suites/test_suite_ssl.function | 40 ++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function index e59a1677c..d902abd2b 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -3178,6 +3178,26 @@ void ssl_crypt_record( int cipher_type, int hash_id, continue; } +#if defined(MBEDTLS_SSL_DTLS_CONNECTION_ID) + if( rec.cid_len != 0 ) + { + /* DTLS 1.2 + CID hides the real content type and + * uses a special CID content type in the protected + * record. Double-check this. */ + TEST_ASSERT( rec.type == MBEDTLS_SSL_MSG_CID ); + } +#endif /* MBEDTLS_SSL_DTLS_CONNECTION_ID */ + +#if defined(MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL) + if( t_enc->minor_ver == MBEDTLS_SSL_MINOR_VERSION_4 ) + { + /* TLS 1.3 hides the real content type and + * always uses Application Data as the content type + * for protected records. Double-check this. */ + TEST_ASSERT( rec.type == MBEDTLS_SSL_MSG_APPLICATION_DATA ); + } +#endif /* MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL */ + /* Decrypt record with t_dec */ ret = mbedtls_ssl_decrypt_buf( &ssl, t_dec, &rec ); TEST_ASSERT( ret == 0 ); @@ -3321,6 +3341,26 @@ void ssl_crypt_record_small( int cipher_type, int hash_id, if( ret != 0 ) continue; +#if defined(MBEDTLS_SSL_DTLS_CONNECTION_ID) + if( rec.cid_len != 0 ) + { + /* DTLS 1.2 + CID hides the real content type and + * uses a special CID content type in the protected + * record. Double-check this. */ + TEST_ASSERT( rec.type == MBEDTLS_SSL_MSG_CID ); + } +#endif /* MBEDTLS_SSL_DTLS_CONNECTION_ID */ + +#if defined(MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL) + if( t_enc->minor_ver == MBEDTLS_SSL_MINOR_VERSION_4 ) + { + /* TLS 1.3 hides the real content type and + * always uses Application Data as the content type + * for protected records. Double-check this. */ + TEST_ASSERT( rec.type == MBEDTLS_SSL_MSG_APPLICATION_DATA ); + } +#endif /* MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL */ + /* Decrypt record with t_dec */ TEST_ASSERT( mbedtls_ssl_decrypt_buf( &ssl, t_dec, &rec ) == 0 ); From e41606001bb53aa15564bda2ccf256d4802bed05 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Thu, 7 May 2020 15:11:45 +0100 Subject: [PATCH 09/38] Disable experimental TLS 1.3 features in default config Signed-off-by: Hanno Becker --- include/mbedtls/config.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/mbedtls/config.h b/include/mbedtls/config.h index da8b6b31e..5474bbd5f 100644 --- a/include/mbedtls/config.h +++ b/include/mbedtls/config.h @@ -1720,7 +1720,7 @@ * Uncomment this macro to enable experimental and partial * functionality specific to TLS 1.3. */ -#define MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL +//#define MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL /** * \def MBEDTLS_SSL_PROTO_DTLS From 3c358d4e12bd48f1fae13d891b8d5f2f168ec7a2 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Wed, 20 May 2020 13:54:41 +0100 Subject: [PATCH 10/38] Improve documentation of MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL Signed-off-by: Hanno Becker --- include/mbedtls/config.h | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/include/mbedtls/config.h b/include/mbedtls/config.h index 5474bbd5f..e1de6f82e 100644 --- a/include/mbedtls/config.h +++ b/include/mbedtls/config.h @@ -1709,13 +1709,16 @@ /** * \def MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL * - * This is an experimental macro used to selectively enable parts - * of the code that solely contribute to the ongoing development of - * a prototype implementation for TLS 1.3 and DTLS 1.3 but aren't - * used otherwise. + * This macro is used to selectively enable experimental parts + * of the code that contribute to the ongoing development of + * the prototype TLS 1.3 and DTLS 1.3 implementation, and provide + * no other purpose. * - * \warning Features under the control of this macro are experimental - * and don't come with any stability guarantees. + * \warning TLS 1.3 and DTLS 1.3 aren't yet supported in Mbed TLS, + * and no feature exposed through this macro is part of the + * public API. In particular, features under the control + * of this macro are experimental and don't come with any + * stability guarantees. * * Uncomment this macro to enable experimental and partial * functionality specific to TLS 1.3. From 9231340d7176e5f10db6b92b810d66394301b56e Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Wed, 20 May 2020 13:58:58 +0100 Subject: [PATCH 11/38] Improve documentation of (D)TLSInnerPlaintext handling Signed-off-by: Hanno Becker --- library/ssl_msg.c | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/library/ssl_msg.c b/library/ssl_msg.c index e739e2fb6..6151cbd80 100644 --- a/library/ssl_msg.c +++ b/library/ssl_msg.c @@ -578,16 +578,21 @@ int mbedtls_ssl_encrypt_buf( mbedtls_ssl_context *ssl, return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA ); } + /* The following two code paths implement the (D)TLSInnerPlaintext + * structure present in TLS 1.3 and DTLS 1.2 + CID. + * + * See ssl_build_inner_plaintext() for more information. + * + * Note that this changes `rec->data_len`, and hence + * `post_avail` needs to be recalculated afterwards. + * + * Note also that the two code paths cannot occur simultaneously + * since they apply to different versions of the protocol. There + * is hence no risk of double-addition of the inner plaintext. + */ #if defined(MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL) if( transform->minor_ver == MBEDTLS_SSL_MINOR_VERSION_4 ) { - /* - * Wrap plaintext into TLSInnerPlaintext structure. - * See ssl_build_inner_plaintext() for more information. - * - * Note that this changes `rec->data_len`, and hence - * `post_avail` needs to be recalculated afterwards. - */ if( ssl_build_inner_plaintext( data, &rec->data_len, post_avail, From 81f3662ae8270a8ca55a299982fe609ea5e5473d Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Wed, 20 May 2020 14:00:29 +0100 Subject: [PATCH 12/38] Update query_config.c Signed-off-by: Hanno Becker --- programs/test/query_config.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/programs/test/query_config.c b/programs/test/query_config.c index bd3f638a7..7389605b6 100644 --- a/programs/test/query_config.c +++ b/programs/test/query_config.c @@ -1426,6 +1426,14 @@ int query_config( const char *config ) } #endif /* MBEDTLS_SSL_PROTO_TLS1_2 */ +#if defined(MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL) + if( strcmp( "MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL", config ) == 0 ) + { + MACRO_EXPANSION_TO_STR( MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL ); + return( 0 ); + } +#endif /* MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL */ + #if defined(MBEDTLS_SSL_PROTO_DTLS) if( strcmp( "MBEDTLS_SSL_PROTO_DTLS", config ) == 0 ) { From 1cb6c2a69d72f26ead7657f4d05433cf4b787989 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Thu, 21 May 2020 15:25:21 +0100 Subject: [PATCH 13/38] TLS record protection: Rewrite AAD setup and add case of TLS 1.3 Signed-off-by: Hanno Becker --- library/ssl_msg.c | 69 ++++++++++++++++++++++++++++++++++------------- 1 file changed, 51 insertions(+), 18 deletions(-) diff --git a/library/ssl_msg.c b/library/ssl_msg.c index 6151cbd80..958071c2a 100644 --- a/library/ssl_msg.c +++ b/library/ssl_msg.c @@ -426,7 +426,8 @@ static int ssl_parse_inner_plaintext( unsigned char const *content, * and 13 + 1 + CID-length Bytes if the CID extension is enabled. */ static void ssl_extract_add_data_from_record( unsigned char* add_data, size_t *add_data_len, - mbedtls_record *rec ) + mbedtls_record *rec, + unsigned minor_ver ) { /* Quoting RFC 5246 (TLS 1.2): * @@ -442,28 +443,50 @@ static void ssl_extract_add_data_from_record( unsigned char* add_data, * cid + * cid_length + * length_of_DTLSInnerPlaintext; + * + * For TLS 1.3, the record sequence number is dropped from the AAD + * and encoded within the nonce of the AEAD operation instead. */ - memcpy( add_data, rec->ctr, sizeof( rec->ctr ) ); - add_data[8] = rec->type; - memcpy( add_data + 9, rec->ver, sizeof( rec->ver ) ); + unsigned char *cur = add_data; + +#if defined(MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL) + if( minor_ver != MBEDTLS_SSL_MINOR_VERSION_4 ) +#endif /* MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL */ + { + ((void) minor_ver); + memcpy( cur, rec->ctr, sizeof( rec->ctr ) ); + cur += sizeof( rec->ctr ); + } + + *cur = rec->type; + cur++; + + memcpy( cur, rec->ver, sizeof( rec->ver ) ); + cur += sizeof( rec->ver ); #if defined(MBEDTLS_SSL_DTLS_CONNECTION_ID) if( rec->cid_len != 0 ) { - memcpy( add_data + 11, rec->cid, rec->cid_len ); - add_data[11 + rec->cid_len + 0] = rec->cid_len; - add_data[11 + rec->cid_len + 1] = ( rec->data_len >> 8 ) & 0xFF; - add_data[11 + rec->cid_len + 2] = ( rec->data_len >> 0 ) & 0xFF; - *add_data_len = 13 + 1 + rec->cid_len; + memcpy( cur, rec->cid, rec->cid_len ); + cur += rec->cid_len; + + *cur = rec->cid_len; + cur++; + + cur[0] = ( rec->data_len >> 8 ) & 0xFF; + cur[1] = ( rec->data_len >> 0 ) & 0xFF; + cur += 2; } else #endif /* MBEDTLS_SSL_DTLS_CONNECTION_ID */ { - add_data[11 + 0] = ( rec->data_len >> 8 ) & 0xFF; - add_data[11 + 1] = ( rec->data_len >> 0 ) & 0xFF; - *add_data_len = 13; + cur[0] = ( rec->data_len >> 8 ) & 0xFF; + cur[1] = ( rec->data_len >> 0 ) & 0xFF; + cur += 2; } + + *add_data_len = cur - add_data; } #if defined(MBEDTLS_SSL_PROTO_SSL3) @@ -669,7 +692,8 @@ int mbedtls_ssl_encrypt_buf( mbedtls_ssl_context *ssl, { unsigned char mac[MBEDTLS_SSL_MAC_ADD]; - ssl_extract_add_data_from_record( add_data, &add_data_len, rec ); + ssl_extract_add_data_from_record( add_data, &add_data_len, rec, + transform->minor_ver ); mbedtls_md_hmac_update( &transform->md_ctx_enc, add_data, add_data_len ); @@ -775,7 +799,12 @@ int mbedtls_ssl_encrypt_buf( mbedtls_ssl_context *ssl, return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); } - ssl_extract_add_data_from_record( add_data, &add_data_len, rec ); + /* + * Build additional data for AEAD encryption. + * This depends on the TLS version. + */ + ssl_extract_add_data_from_record( add_data, &add_data_len, rec, + transform->minor_ver ); MBEDTLS_SSL_DEBUG_BUF( 4, "IV used (internal)", iv, transform->ivlen ); @@ -929,7 +958,8 @@ int mbedtls_ssl_encrypt_buf( mbedtls_ssl_context *ssl, return( MBEDTLS_ERR_SSL_BUFFER_TOO_SMALL ); } - ssl_extract_add_data_from_record( add_data, &add_data_len, rec ); + ssl_extract_add_data_from_record( add_data, &add_data_len, + rec, transform->minor_ver ); MBEDTLS_SSL_DEBUG_MSG( 3, ( "using encrypt then mac" ) ); MBEDTLS_SSL_DEBUG_BUF( 4, "MAC'd meta-data", add_data, @@ -1097,7 +1127,8 @@ int mbedtls_ssl_decrypt_buf( mbedtls_ssl_context const *ssl, rec->data_offset += explicit_iv_len; rec->data_len -= explicit_iv_len + transform->taglen; - ssl_extract_add_data_from_record( add_data, &add_data_len, rec ); + ssl_extract_add_data_from_record( add_data, &add_data_len, rec, + transform->minor_ver ); MBEDTLS_SSL_DEBUG_BUF( 4, "additional data used for AEAD", add_data, add_data_len ); @@ -1209,7 +1240,8 @@ int mbedtls_ssl_decrypt_buf( mbedtls_ssl_context const *ssl, * * Further, we still know that data_len > minlen */ rec->data_len -= transform->maclen; - ssl_extract_add_data_from_record( add_data, &add_data_len, rec ); + ssl_extract_add_data_from_record( add_data, &add_data_len, rec, + transform->minor_ver ); /* Calculate expected MAC. */ MBEDTLS_SSL_DEBUG_BUF( 4, "MAC'd meta-data", add_data, @@ -1428,7 +1460,8 @@ int mbedtls_ssl_decrypt_buf( mbedtls_ssl_context const *ssl, * hence data_len >= maclen in any case. */ rec->data_len -= transform->maclen; - ssl_extract_add_data_from_record( add_data, &add_data_len, rec ); + ssl_extract_add_data_from_record( add_data, &add_data_len, rec, + transform->minor_ver ); #if defined(MBEDTLS_SSL_PROTO_SSL3) if( transform->minor_ver == MBEDTLS_SSL_MINOR_VERSION_0 ) From bd5ed1d11bd7a124f65fa6430d757a6f0438968b Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Thu, 21 May 2020 15:26:39 +0100 Subject: [PATCH 14/38] TLS record protection: Add explicit IV after record protection. The previous record protection code added the explicit part of the record nonce prior to encrypting the record. This temporarily leaves the record structure in the undesireable state that the data outsie of the interval `rec->data_offset, .., rec->data_offset + rec->data_len` has already been written. This commit moves the addition of the explicit IV past record encryption. Signed-off-by: Hanno Becker --- library/ssl_msg.c | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/library/ssl_msg.c b/library/ssl_msg.c index 958071c2a..57b39c74c 100644 --- a/library/ssl_msg.c +++ b/library/ssl_msg.c @@ -761,10 +761,8 @@ int mbedtls_ssl_encrypt_buf( mbedtls_ssl_context *ssl, unsigned char iv[12]; size_t explicit_iv_len = transform->ivlen - transform->fixed_ivlen; - /* Check that there's space for both the authentication tag - * and the explicit IV before and after the record content. */ - if( post_avail < transform->taglen || - rec->data_offset < explicit_iv_len ) + /* Check that there's space for the authentication tag. */ + if( post_avail < transform->taglen ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "Buffer provided for encrypted record not large enough" ) ); return( MBEDTLS_ERR_SSL_BUFFER_TOO_SMALL ); @@ -779,8 +777,6 @@ int mbedtls_ssl_encrypt_buf( mbedtls_ssl_context *ssl, memcpy( iv, transform->iv_enc, transform->fixed_ivlen ); memcpy( iv + transform->fixed_ivlen, rec->ctr, explicit_iv_len ); - /* Prefix record content with explicit IV. */ - memcpy( data - explicit_iv_len, rec->ctr, explicit_iv_len ); } else if( transform->ivlen == 12 && transform->fixed_ivlen == 12 ) { @@ -831,9 +827,20 @@ int mbedtls_ssl_encrypt_buf( mbedtls_ssl_context *ssl, return( ret ); } + /* + * Prefix record content with explicit IV. + */ + if( rec->data_offset < explicit_iv_len ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "Buffer provided for encrypted record not large enough" ) ); + return( MBEDTLS_ERR_SSL_BUFFER_TOO_SMALL ); + } + memcpy( data - explicit_iv_len, rec->ctr, explicit_iv_len ); + MBEDTLS_SSL_DEBUG_BUF( 4, "after encrypt: tag", data + rec->data_len, transform->taglen ); + /* Account for tag and explicit IV. */ rec->data_len += transform->taglen + explicit_iv_len; rec->data_offset -= explicit_iv_len; post_avail -= transform->taglen; From df8be226bacf81b2827dea6624339d95f6b02368 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Thu, 21 May 2020 15:30:57 +0100 Subject: [PATCH 15/38] TLS record protection: Add helper function for nonce derivation The computation of the per-record nonce for AEAD record protection varies with the AEAD algorithm and the TLS version in use. This commit introduces a helper function for the nonce computation to ease readability of the quite monolithic record encrytion routine. Signed-off-by: Hanno Becker --- library/ssl_msg.c | 255 ++++++++++++++++++++++++++++++---------------- 1 file changed, 169 insertions(+), 86 deletions(-) diff --git a/library/ssl_msg.c b/library/ssl_msg.c index 57b39c74c..420e539c7 100644 --- a/library/ssl_msg.c +++ b/library/ssl_msg.c @@ -536,6 +536,78 @@ static void ssl_mac( mbedtls_md_context_t *md_ctx, } #endif /* MBEDTLS_SSL_PROTO_SSL3 */ +#define SSL_RECORD_AEAD_NONCE_UNKNOWN 0u +#define SSL_RECORD_AEAD_NONCE_CONCAT 1u +#define SSL_RECORD_AEAD_NONCE_XOR 2u + +static int ssl_transform_get_nonce_mode( mbedtls_ssl_transform const *transform ) +{ +#if defined(MBEDTLS_CHACHAPOLY_C) + if( transform->ivlen == 12 && transform->fixed_ivlen == 12 ) + { + return( SSL_RECORD_AEAD_NONCE_XOR ); + } +#endif /* MBEDTLS_CHACHAPOLY_C */ + +#if defined(MBEDTLS_GCM_C) || defined(MBEDTLS_CCM_C) + if( transform->ivlen == 12 && transform->fixed_ivlen == 4 ) + { + return( SSL_RECORD_AEAD_NONCE_CONCAT ); + } +#endif /* MBEDTLS_GCM_C || MBEDTLS_CCM_C */ + + return( SSL_RECORD_AEAD_NONCE_UNKNOWN ); +} + +/* Preconditions: + * - If mode == SSL_RECORD_AEAD_NONCE_CONCAT, then + * dst_nonce_len == fixed_iv_len + dynamic_iv_len + * - If mode == SSL_RECORD_AEAD_NONCE_XOR, then + * dst_nonce_len == fixed_iv_len && + * dynamic_iv_len < dst_nonce + */ +static int ssl_build_record_nonce( unsigned char *dst_nonce, + size_t dst_nonce_len, + unsigned char const *fixed_iv, + size_t fixed_iv_len, + unsigned char const *dynamic_iv, + size_t dynamic_iv_len, + unsigned mode ) +{ + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; + + ((void) dst_nonce_len); + + /* Start with Fixed IV || 0 */ + memcpy( dst_nonce, fixed_iv, fixed_iv_len ); + dst_nonce += fixed_iv_len; + + if( mode == SSL_RECORD_AEAD_NONCE_CONCAT ) + { + /* Nonce := Fixed IV || Dynamic IV */ + memcpy( dst_nonce, dynamic_iv, dynamic_iv_len ); + ret = 0; + } + else if( mode == SSL_RECORD_AEAD_NONCE_XOR ) + { + /* Nonce := Fixed IV XOR ( 0 || Dynamic IV ) */ + unsigned char i; + + /* This is safe by the second precondition above. */ + dst_nonce -= dynamic_iv_len; + for( i = 0; i < dynamic_iv_len; i++ ) + dst_nonce[i] ^= dynamic_iv[i]; + + ret = 0; + } + else + { + ret = MBEDTLS_ERR_SSL_INTERNAL_ERROR; + } + + return( ret ); +} + int mbedtls_ssl_encrypt_buf( mbedtls_ssl_context *ssl, mbedtls_ssl_transform *transform, mbedtls_record *rec, @@ -759,7 +831,13 @@ int mbedtls_ssl_encrypt_buf( mbedtls_ssl_context *ssl, { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; unsigned char iv[12]; - size_t explicit_iv_len = transform->ivlen - transform->fixed_ivlen; + unsigned char *dynamic_iv; + size_t dynamic_iv_len; + + unsigned const nonce_mode + = ssl_transform_get_nonce_mode( transform ); + unsigned const dynamic_iv_is_explicit + = nonce_mode == SSL_RECORD_AEAD_NONCE_CONCAT; /* Check that there's space for the authentication tag. */ if( post_avail < transform->taglen ) @@ -769,31 +847,28 @@ int mbedtls_ssl_encrypt_buf( mbedtls_ssl_context *ssl, } /* - * Generate IV + * Build nonce for AEAD encryption. + * + * Note: In the case of CCM and GCM in TLS 1.2, the dynamic + * part of the IV is prepended to the ciphertext and + * can be chosen freely - in particular, it need not + * agree with the record sequence number. + * However, since ChaChaPoly as well as all AEAD modes + * in TLS 1.3 use the record sequence number as the + * dynamic part of the nonce, we uniformly use the + * record sequence number here in all cases. */ - if( transform->ivlen == 12 && transform->fixed_ivlen == 4 ) - { - /* GCM and CCM: fixed || explicit (=seqnum) */ - memcpy( iv, transform->iv_enc, transform->fixed_ivlen ); - memcpy( iv + transform->fixed_ivlen, rec->ctr, - explicit_iv_len ); - } - else if( transform->ivlen == 12 && transform->fixed_ivlen == 12 ) - { - /* ChachaPoly: fixed XOR sequence number */ - unsigned char i; + dynamic_iv = rec->ctr; + dynamic_iv_len = sizeof( rec->ctr ); - memcpy( iv, transform->iv_enc, transform->fixed_ivlen ); - - for( i = 0; i < 8; i++ ) - iv[i+4] ^= rec->ctr[i]; - } - else - { - /* Reminder if we ever add an AEAD mode with a different size */ - MBEDTLS_SSL_DEBUG_MSG( 1, ( "should never happen" ) ); - return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); - } + ret = ssl_build_record_nonce( iv, sizeof( iv ), + transform->iv_enc, + transform->fixed_ivlen, + dynamic_iv, + dynamic_iv_len, + nonce_mode ); + if( ret != 0 ) + return( ret ); /* * Build additional data for AEAD encryption. @@ -805,7 +880,8 @@ int mbedtls_ssl_encrypt_buf( mbedtls_ssl_context *ssl, MBEDTLS_SSL_DEBUG_BUF( 4, "IV used (internal)", iv, transform->ivlen ); MBEDTLS_SSL_DEBUG_BUF( 4, "IV used (transmitted)", - data - explicit_iv_len, explicit_iv_len ); + data - dynamic_iv_len * dynamic_iv_is_explicit, + dynamic_iv_len * dynamic_iv_is_explicit ); MBEDTLS_SSL_DEBUG_BUF( 4, "additional data used for AEAD", add_data, add_data_len ); MBEDTLS_SSL_DEBUG_MSG( 3, ( "before encrypt: msglen = %d, " @@ -826,24 +902,28 @@ int mbedtls_ssl_encrypt_buf( mbedtls_ssl_context *ssl, MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_cipher_auth_encrypt", ret ); return( ret ); } - - /* - * Prefix record content with explicit IV. - */ - if( rec->data_offset < explicit_iv_len ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "Buffer provided for encrypted record not large enough" ) ); - return( MBEDTLS_ERR_SSL_BUFFER_TOO_SMALL ); - } - memcpy( data - explicit_iv_len, rec->ctr, explicit_iv_len ); - MBEDTLS_SSL_DEBUG_BUF( 4, "after encrypt: tag", data + rec->data_len, transform->taglen ); - - /* Account for tag and explicit IV. */ - rec->data_len += transform->taglen + explicit_iv_len; - rec->data_offset -= explicit_iv_len; + /* Account for authentication tag. */ + rec->data_len += transform->taglen; post_avail -= transform->taglen; + + /* + * Prefix record content with dynamic IV in case it is explicit. + */ + if( dynamic_iv_is_explicit == 1 ) + { + if( rec->data_offset < dynamic_iv_len ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "Buffer provided for encrypted record not large enough" ) ); + return( MBEDTLS_ERR_SSL_BUFFER_TOO_SMALL ); + } + + memcpy( data - dynamic_iv_len, dynamic_iv, dynamic_iv_len ); + rec->data_offset -= dynamic_iv_len; + rec->data_len += dynamic_iv_len; + } + auth_done++; } else @@ -1080,60 +1160,63 @@ int mbedtls_ssl_decrypt_buf( mbedtls_ssl_context const *ssl, mode == MBEDTLS_MODE_CHACHAPOLY ) { unsigned char iv[12]; - size_t explicit_iv_len = transform->ivlen - transform->fixed_ivlen; + unsigned const nonce_mode = ssl_transform_get_nonce_mode( transform ); + unsigned char *dynamic_iv; + size_t dynamic_iv_len; /* - * Prepare IV from explicit and implicit data. + * Extract dynamic part of nonce for AEAD decryption. + * + * Note: In the case of CCM and GCM in TLS 1.2, the dynamic + * part of the IV is prepended to the ciphertext and + * can be chosen freely - in particular, it need not + * agree with the record sequence number. */ - - /* Check that there's enough space for the explicit IV - * (at the beginning of the record) and the MAC (at the - * end of the record). */ - if( rec->data_len < explicit_iv_len + transform->taglen ) + dynamic_iv_len = sizeof( rec->ctr ); + if( nonce_mode == SSL_RECORD_AEAD_NONCE_XOR ) { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "msglen (%d) < explicit_iv_len (%d) " - "+ taglen (%d)", rec->data_len, - explicit_iv_len, transform->taglen ) ); + dynamic_iv = rec->ctr; + } + else + { + if( rec->data_len < dynamic_iv_len ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "msglen (%d) < explicit_iv_len (%d) ", + rec->data_len, + dynamic_iv_len ) ); + return( MBEDTLS_ERR_SSL_INVALID_MAC ); + } + dynamic_iv = data; + + data += dynamic_iv_len; + rec->data_offset += dynamic_iv_len; + rec->data_len -= dynamic_iv_len; + } + + /* Check that there's space for the authentication tag. */ + if( rec->data_len < transform->taglen ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "msglen (%d) < taglen (%d) " ) ); return( MBEDTLS_ERR_SSL_INVALID_MAC ); } + rec->data_len -= transform->taglen; -#if defined(MBEDTLS_GCM_C) || defined(MBEDTLS_CCM_C) - if( transform->ivlen == 12 && transform->fixed_ivlen == 4 ) - { - /* GCM and CCM: fixed || explicit */ - - /* Fixed */ - memcpy( iv, transform->iv_dec, transform->fixed_ivlen ); - /* Explicit */ - memcpy( iv + transform->fixed_ivlen, data, 8 ); - } - else -#endif /* MBEDTLS_GCM_C || MBEDTLS_CCM_C */ -#if defined(MBEDTLS_CHACHAPOLY_C) - if( transform->ivlen == 12 && transform->fixed_ivlen == 12 ) - { - /* ChachaPoly: fixed XOR sequence number */ - unsigned char i; - - memcpy( iv, transform->iv_dec, transform->fixed_ivlen ); - - for( i = 0; i < 8; i++ ) - iv[i+4] ^= rec->ctr[i]; - } - else -#endif /* MBEDTLS_CHACHAPOLY_C */ - { - /* Reminder if we ever add an AEAD mode with a different size */ - MBEDTLS_SSL_DEBUG_MSG( 1, ( "should never happen" ) ); - return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); - } - - /* Group changes to data, data_len, and add_data, because - * add_data depends on data_len. */ - data += explicit_iv_len; - rec->data_offset += explicit_iv_len; - rec->data_len -= explicit_iv_len + transform->taglen; + /* + * Prepare nonce from dynamic and static parts. + */ + ret = ssl_build_record_nonce( iv, sizeof( iv ), + transform->iv_dec, + transform->fixed_ivlen, + dynamic_iv, + dynamic_iv_len, + nonce_mode ); + if( ret != 0 ) + return( ret ); + /* + * Build additional data for AEAD encryption. + * This depends on the TLS version. + */ ssl_extract_add_data_from_record( add_data, &add_data_len, rec, transform->minor_ver ); MBEDTLS_SSL_DEBUG_BUF( 4, "additional data used for AEAD", From 17263803aa034122603ef864a536751f018a2fbd Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Thu, 28 May 2020 07:05:48 +0100 Subject: [PATCH 16/38] Simplify AEAD nonce derivation This commit simplifies nonce derivation for AEAD based record protection routines in the following way. So far, code distinguished between the cases of GCM+CCM and ChachaPoly: - In the case of GCM+CCM, the AEAD nonce is the concatentation of a 4-byte Fixed IV and a dynamically chosen 8-byte IV which is prepended to the record. In Mbed TLS, this is always chosen to be the record sequence number, but it need not to. - In the case of ChaChaPoly, the AEAD nonce is derived as `( 12-byte Fixed IV ) XOR ( 0 || 8-byte dynamic IV == record seq nr )` and the dynamically chosen IV is no longer prepended to the record. This commit removes this distinction by always computing the record nonce via the formula `IV == ( Fixed IV || 0 ) XOR ( 0 || Dynamic IV )` The ChaChaPoly case is recovered in case `Len(Fixed IV) == Len(IV)`, and GCM+CCM is recovered when `Len(IV) == Len(Fixed IV) + Len(Dynamic IV)`. Moreover, a getter stub `ssl_transform_aead_dynamic_iv_is_explicit()` is introduced which infers from a transform whether the dynamically chosen part of the IV is explicit, which in the current implementation of `mbedtls_ssl_transform` can be derived from the helper field `mbedtls_ssl_transform::fixed_ivlen`. Signed-off-by: Hanno Becker --- library/ssl_msg.c | 135 +++++++++++++++------------------------------- 1 file changed, 44 insertions(+), 91 deletions(-) diff --git a/library/ssl_msg.c b/library/ssl_msg.c index 420e539c7..1c56e538f 100644 --- a/library/ssl_msg.c +++ b/library/ssl_msg.c @@ -536,76 +536,39 @@ static void ssl_mac( mbedtls_md_context_t *md_ctx, } #endif /* MBEDTLS_SSL_PROTO_SSL3 */ -#define SSL_RECORD_AEAD_NONCE_UNKNOWN 0u -#define SSL_RECORD_AEAD_NONCE_CONCAT 1u -#define SSL_RECORD_AEAD_NONCE_XOR 2u - -static int ssl_transform_get_nonce_mode( mbedtls_ssl_transform const *transform ) +static int ssl_transform_aead_dynamic_iv_is_explicit( + mbedtls_ssl_transform const *transform ) { -#if defined(MBEDTLS_CHACHAPOLY_C) - if( transform->ivlen == 12 && transform->fixed_ivlen == 12 ) - { - return( SSL_RECORD_AEAD_NONCE_XOR ); - } -#endif /* MBEDTLS_CHACHAPOLY_C */ - -#if defined(MBEDTLS_GCM_C) || defined(MBEDTLS_CCM_C) - if( transform->ivlen == 12 && transform->fixed_ivlen == 4 ) - { - return( SSL_RECORD_AEAD_NONCE_CONCAT ); - } -#endif /* MBEDTLS_GCM_C || MBEDTLS_CCM_C */ - - return( SSL_RECORD_AEAD_NONCE_UNKNOWN ); + return( transform->ivlen != transform->fixed_ivlen ); } -/* Preconditions: - * - If mode == SSL_RECORD_AEAD_NONCE_CONCAT, then - * dst_nonce_len == fixed_iv_len + dynamic_iv_len - * - If mode == SSL_RECORD_AEAD_NONCE_XOR, then - * dst_nonce_len == fixed_iv_len && - * dynamic_iv_len < dst_nonce - */ -static int ssl_build_record_nonce( unsigned char *dst_nonce, - size_t dst_nonce_len, - unsigned char const *fixed_iv, - size_t fixed_iv_len, - unsigned char const *dynamic_iv, - size_t dynamic_iv_len, - unsigned mode ) -{ - int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; - ((void) dst_nonce_len); +/* Compute IV := ( fixed_iv || 0 ) XOR ( 0 || dynamic_IV ) + * + * Concretely, this occurs in two variants: + * + * a) Fixed and dynamic IV lengths add up to total IV length, giving + * IV = fixed_iv || dynamic_iv + * + * b) Fixed IV lengths matches total IV length, giving + * IV = fixed_iv XOR ( 0 || dynamic_iv ) + */ +static void ssl_build_record_nonce( unsigned char *dst_iv, + size_t dst_iv_len, + unsigned char const *fixed_iv, + size_t fixed_iv_len, + unsigned char const *dynamic_iv, + size_t dynamic_iv_len ) +{ + size_t i; /* Start with Fixed IV || 0 */ - memcpy( dst_nonce, fixed_iv, fixed_iv_len ); - dst_nonce += fixed_iv_len; + memset( dst_iv, 0, dst_iv_len ); + memcpy( dst_iv, fixed_iv, fixed_iv_len ); - if( mode == SSL_RECORD_AEAD_NONCE_CONCAT ) - { - /* Nonce := Fixed IV || Dynamic IV */ - memcpy( dst_nonce, dynamic_iv, dynamic_iv_len ); - ret = 0; - } - else if( mode == SSL_RECORD_AEAD_NONCE_XOR ) - { - /* Nonce := Fixed IV XOR ( 0 || Dynamic IV ) */ - unsigned char i; - - /* This is safe by the second precondition above. */ - dst_nonce -= dynamic_iv_len; - for( i = 0; i < dynamic_iv_len; i++ ) - dst_nonce[i] ^= dynamic_iv[i]; - - ret = 0; - } - else - { - ret = MBEDTLS_ERR_SSL_INTERNAL_ERROR; - } - - return( ret ); + dst_iv += dst_iv_len - dynamic_iv_len; + for( i = 0; i < dynamic_iv_len; i++ ) + dst_iv[i] ^= dynamic_iv[i]; } int mbedtls_ssl_encrypt_buf( mbedtls_ssl_context *ssl, @@ -833,11 +796,8 @@ int mbedtls_ssl_encrypt_buf( mbedtls_ssl_context *ssl, unsigned char iv[12]; unsigned char *dynamic_iv; size_t dynamic_iv_len; - - unsigned const nonce_mode - = ssl_transform_get_nonce_mode( transform ); - unsigned const dynamic_iv_is_explicit - = nonce_mode == SSL_RECORD_AEAD_NONCE_CONCAT; + int dynamic_iv_is_explicit = + ssl_transform_aead_dynamic_iv_is_explicit( transform ); /* Check that there's space for the authentication tag. */ if( post_avail < transform->taglen ) @@ -861,14 +821,11 @@ int mbedtls_ssl_encrypt_buf( mbedtls_ssl_context *ssl, dynamic_iv = rec->ctr; dynamic_iv_len = sizeof( rec->ctr ); - ret = ssl_build_record_nonce( iv, sizeof( iv ), - transform->iv_enc, - transform->fixed_ivlen, - dynamic_iv, - dynamic_iv_len, - nonce_mode ); - if( ret != 0 ) - return( ret ); + ssl_build_record_nonce( iv, sizeof( iv ), + transform->iv_enc, + transform->fixed_ivlen, + dynamic_iv, + dynamic_iv_len ); /* * Build additional data for AEAD encryption. @@ -911,7 +868,7 @@ int mbedtls_ssl_encrypt_buf( mbedtls_ssl_context *ssl, /* * Prefix record content with dynamic IV in case it is explicit. */ - if( dynamic_iv_is_explicit == 1 ) + if( dynamic_iv_is_explicit ) { if( rec->data_offset < dynamic_iv_len ) { @@ -1160,7 +1117,6 @@ int mbedtls_ssl_decrypt_buf( mbedtls_ssl_context const *ssl, mode == MBEDTLS_MODE_CHACHAPOLY ) { unsigned char iv[12]; - unsigned const nonce_mode = ssl_transform_get_nonce_mode( transform ); unsigned char *dynamic_iv; size_t dynamic_iv_len; @@ -1173,11 +1129,7 @@ int mbedtls_ssl_decrypt_buf( mbedtls_ssl_context const *ssl, * agree with the record sequence number. */ dynamic_iv_len = sizeof( rec->ctr ); - if( nonce_mode == SSL_RECORD_AEAD_NONCE_XOR ) - { - dynamic_iv = rec->ctr; - } - else + if( ssl_transform_aead_dynamic_iv_is_explicit( transform ) == 1 ) { if( rec->data_len < dynamic_iv_len ) { @@ -1192,6 +1144,10 @@ int mbedtls_ssl_decrypt_buf( mbedtls_ssl_context const *ssl, rec->data_offset += dynamic_iv_len; rec->data_len -= dynamic_iv_len; } + else + { + dynamic_iv = rec->ctr; + } /* Check that there's space for the authentication tag. */ if( rec->data_len < transform->taglen ) @@ -1204,14 +1160,11 @@ int mbedtls_ssl_decrypt_buf( mbedtls_ssl_context const *ssl, /* * Prepare nonce from dynamic and static parts. */ - ret = ssl_build_record_nonce( iv, sizeof( iv ), - transform->iv_dec, - transform->fixed_ivlen, - dynamic_iv, - dynamic_iv_len, - nonce_mode ); - if( ret != 0 ) - return( ret ); + ssl_build_record_nonce( iv, sizeof( iv ), + transform->iv_dec, + transform->fixed_ivlen, + dynamic_iv, + dynamic_iv_len ); /* * Build additional data for AEAD encryption. From c0eefa8b921f2e08f0357b6bc2ef446c5aac3095 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Thu, 28 May 2020 07:17:36 +0100 Subject: [PATCH 17/38] Introduce helper function to retrieve explicit IV len for transform The structure `mbedtls_ssl_transform` representing record protection transformations should ideally be used through a function-based interface only, as this will ease change of implementation as well as the addition of new record protection routines in the future. This commit makes a step in that direction by introducing the helper function `ssl_transform_get_explicit_iv_len()` which returns the size of the pre-expansion during record encryption due to the potential addition of an explicit IV. Signed-off-by: Hanno Becker --- library/ssl_msg.c | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/library/ssl_msg.c b/library/ssl_msg.c index 1c56e538f..669a33d4b 100644 --- a/library/ssl_msg.c +++ b/library/ssl_msg.c @@ -4985,6 +4985,15 @@ int mbedtls_ssl_parse_change_cipher_spec( mbedtls_ssl_context *ssl ) * and the caller has to make sure there's space for this. */ +static size_t ssl_transform_get_explicit_iv_len( + mbedtls_ssl_transform const *transform ) +{ + if( transform->minor_ver < MBEDTLS_SSL_MINOR_VERSION_2 ) + return( 0 ); + + return( transform->ivlen - transform->fixed_ivlen ); +} + void mbedtls_ssl_update_out_pointers( mbedtls_ssl_context *ssl, mbedtls_ssl_transform *transform ) { @@ -5013,14 +5022,10 @@ void mbedtls_ssl_update_out_pointers( mbedtls_ssl_context *ssl, ssl->out_iv = ssl->out_hdr + 5; } + ssl->out_msg = ssl->out_iv; /* Adjust out_msg to make space for explicit IV, if used. */ - if( transform != NULL && - ssl->minor_ver >= MBEDTLS_SSL_MINOR_VERSION_2 ) - { - ssl->out_msg = ssl->out_iv + transform->ivlen - transform->fixed_ivlen; - } - else - ssl->out_msg = ssl->out_iv; + if( transform != NULL ) + ssl->out_msg += ssl_transform_get_explicit_iv_len( transform ); } /* Once ssl->in_hdr as the address of the beginning of the From 447558df12ba054818569cf8b35869ea8bb136de Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Thu, 28 May 2020 07:36:33 +0100 Subject: [PATCH 18/38] Improve documentation of ssl_populate_transform() Signed-off-by: Hanno Becker --- library/ssl_tls.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index fd0c8a7ab..116d2a26c 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -973,9 +973,12 @@ static int ssl_populate_transform( mbedtls_ssl_transform *transform, transform->taglen = ciphersuite_info->flags & MBEDTLS_CIPHERSUITE_SHORT_TAG ? 8 : 16; - /* All modes haves 96-bit IVs; - * GCM and CCM has 4 implicit and 8 explicit bytes - * ChachaPoly has all 12 bytes implicit + /* All modes haves 96-bit IVs, but the length of the static parts vary + * with mode and version: + * - For GCM and CCM in TLS 1.2, there's a static IV of 4 Bytes + * (to be concatenated with a dynamically chosen IV of 8 Bytes) + * - For ChaChaPoly in TLS 1.2, there's a static IV of 12 Bytes + * (to be XOR'ed with the 8 Byte record sequence number). */ transform->ivlen = 12; if( cipher_info->mode == MBEDTLS_MODE_CHACHAPOLY ) From f93c2d7ca551661a851b503122222b38cc2b5189 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Thu, 28 May 2020 07:39:43 +0100 Subject: [PATCH 19/38] Add support for TLS 1.3 record protection to ssl_populate_transform() Signed-off-by: Hanno Becker --- library/ssl_tls.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 116d2a26c..30c917bb1 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -977,14 +977,24 @@ static int ssl_populate_transform( mbedtls_ssl_transform *transform, * with mode and version: * - For GCM and CCM in TLS 1.2, there's a static IV of 4 Bytes * (to be concatenated with a dynamically chosen IV of 8 Bytes) - * - For ChaChaPoly in TLS 1.2, there's a static IV of 12 Bytes - * (to be XOR'ed with the 8 Byte record sequence number). + * - For ChaChaPoly in TLS 1.2, and all modes in TLS 1.3, there's + * a static IV of 12 Bytes (to be XOR'ed with the 8 Byte record + * sequence number). */ transform->ivlen = 12; - if( cipher_info->mode == MBEDTLS_MODE_CHACHAPOLY ) +#if defined(MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL) + if( minor_ver == MBEDTLS_SSL_MINOR_VERSION_4 ) + { transform->fixed_ivlen = 12; + } else - transform->fixed_ivlen = 4; +#endif /* MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL */ + { + if( cipher_info->mode == MBEDTLS_MODE_CHACHAPOLY ) + transform->fixed_ivlen = 12; + else + transform->fixed_ivlen = 4; + } /* Minimum length of encrypted record */ explicit_ivlen = transform->ivlen - transform->fixed_ivlen; From e683287710779c7487f3aab607d13c5cfef82f10 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Thu, 28 May 2020 08:29:58 +0100 Subject: [PATCH 20/38] Adapt SSL record protection unit test to setup TLS 1.3 transforms Signed-off-by: Hanno Becker --- tests/suites/test_suite_ssl.function | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function index d902abd2b..205abc60e 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -1306,8 +1306,18 @@ static int build_transforms( mbedtls_ssl_transform *t_in, { case MBEDTLS_MODE_GCM: case MBEDTLS_MODE_CCM: - t_out->fixed_ivlen = 4; - t_in->fixed_ivlen = 4; +#if defined(MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL) + if( ver == MBEDTLS_SSL_MINOR_VERSION_4 ) + { + t_out->fixed_ivlen = 12; + t_in->fixed_ivlen = 12; + } + else +#endif /* MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL */ + { + t_out->fixed_ivlen = 4; + t_in->fixed_ivlen = 4; + } t_out->maclen = 0; t_in->maclen = 0; switch( tag_mode ) From a0c65d84daa965f729936284d2fbfc66d57db8cb Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Thu, 28 May 2020 08:59:39 +0100 Subject: [PATCH 21/38] Update version_features.c Signed-off-by: Hanno Becker --- library/version_features.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/library/version_features.c b/library/version_features.c index d16ad1bac..f5ea7989e 100644 --- a/library/version_features.c +++ b/library/version_features.c @@ -519,6 +519,9 @@ static const char * const features[] = { #if defined(MBEDTLS_SSL_PROTO_TLS1_2) "MBEDTLS_SSL_PROTO_TLS1_2", #endif /* MBEDTLS_SSL_PROTO_TLS1_2 */ +#if defined(MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL) + "MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL", +#endif /* MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL */ #if defined(MBEDTLS_SSL_PROTO_DTLS) "MBEDTLS_SSL_PROTO_DTLS", #endif /* MBEDTLS_SSL_PROTO_DTLS */ From b54094bd7c083fbf555ad839f445573a4e8df045 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Thu, 28 May 2020 15:41:44 +0100 Subject: [PATCH 22/38] Fix copy-pasta in TLS 1.3 record protection unit test names Signed-off-by: Hanno Becker --- tests/suites/test_suite_ssl.data | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/suites/test_suite_ssl.data b/tests/suites/test_suite_ssl.data index e71a4cb87..aa314dd32 100644 --- a/tests/suites/test_suite_ssl.data +++ b/tests/suites/test_suite_ssl.data @@ -4210,7 +4210,7 @@ Record crypt, AES-256-GCM, 1.2 depends_on:MBEDTLS_AES_C:MBEDTLS_SSL_PROTO_TLS1_2:MBEDTLS_GCM_C ssl_crypt_record:MBEDTLS_CIPHER_AES_256_GCM:MBEDTLS_MD_MD5:0:0:MBEDTLS_SSL_MINOR_VERSION_3:0:0 -Record crypt, AES-192-GCM, 1.3 +Record crypt, AES-256-GCM, 1.3 depends_on:MBEDTLS_AES_C:MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL:MBEDTLS_GCM_C ssl_crypt_record:MBEDTLS_CIPHER_AES_256_GCM:MBEDTLS_MD_MD5:0:0:MBEDTLS_SSL_MINOR_VERSION_4:0:0 From 13996927cbb10615b0db4df2c1d6494ab3f3b29b Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Thu, 28 May 2020 16:15:19 +0100 Subject: [PATCH 23/38] Introduce configuration option for TLS 1.3 padding granularity TLS 1.3 record protection allows the addition of an arbitrary amount of padding. This commit introduces a configuration option ``` MBEDTLS_SSL_TLS13_PADDING_GRANULARITY ``` The semantics of this option is that padding is chosen in a minimal way so that the padded plaintext has a length which is a multiple of MBEDTLS_SSL_TLS13_PADDING_GRANULARITY. For example, setting MBEDTLS_SSL_TLS13_PADDING_GRANULARITY to 1024 means that padded plaintexts will have length 1024, 2048, ..., while setting it to 1 means that no padding will be used. Signed-off-by: Hanno Becker --- include/mbedtls/config.h | 16 ++++++++++++++++ include/mbedtls/ssl.h | 4 ++++ library/ssl_msg.c | 29 +++++++++++++++++++++-------- 3 files changed, 41 insertions(+), 8 deletions(-) diff --git a/include/mbedtls/config.h b/include/mbedtls/config.h index e1de6f82e..9d700df8e 100644 --- a/include/mbedtls/config.h +++ b/include/mbedtls/config.h @@ -3554,6 +3554,22 @@ */ //#define MBEDTLS_SSL_CID_PADDING_GRANULARITY 16 +/** \def MBEDTLS_SSL_TLS13_PADDING_GRANULARITY + * + * This option controls the use of record plaintext padding + * in TLS 1.3. + * + * The padding will always be chosen so that the length of the + * padded plaintext is a multiple of the value of this option. + * + * Note: A value of \c 1 means that no padding will be used + * for outgoing records. + * + * Note: On systems lacking division instructions, + * a power of two should be preferred. + */ +//#define MBEDTLS_SSL_TLS13_PADDING_GRANULARITY 16 + /** \def MBEDTLS_SSL_OUT_CONTENT_LEN * * Maximum length (in bytes) of outgoing plaintext fragments. diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 5869e1560..4a6ea6cab 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -277,6 +277,10 @@ #define MBEDTLS_SSL_CID_PADDING_GRANULARITY 16 #endif +#if !defined(MBEDTLS_SSL_TLS13_PADDING_GRANULARITY) +#define MBEDTLS_SSL_TLS13_PADDING_GRANULARITY 16 +#endif + /* \} name SECTION: Module settings */ /* diff --git a/library/ssl_msg.c b/library/ssl_msg.c index 669a33d4b..4ae1b8f08 100644 --- a/library/ssl_msg.c +++ b/library/ssl_msg.c @@ -343,6 +343,13 @@ static void ssl_read_memory( unsigned char *p, size_t len ) #if defined(MBEDTLS_SSL_DTLS_CONNECTION_ID) || \ defined(MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL) + +static size_t ssl_compute_padding_length( size_t len, + size_t granularity ) +{ + return( ( granularity - ( len + 1 ) % granularity ) % granularity ); +} + /* This functions transforms a (D)TLS plaintext fragment and a record content * type into an instance of the (D)TLSInnerPlaintext structure. This is used * in DTLS 1.2 + CID and within TLS 1.3 to allow flexible padding and to protect @@ -374,12 +381,10 @@ static void ssl_read_memory( unsigned char *p, size_t len ) static int ssl_build_inner_plaintext( unsigned char *content, size_t *content_size, size_t remaining, - uint8_t rec_type ) + uint8_t rec_type, + size_t pad ) { size_t len = *content_size; - size_t pad = ( MBEDTLS_SSL_CID_PADDING_GRANULARITY - - ( len + 1 ) % MBEDTLS_SSL_CID_PADDING_GRANULARITY ) % - MBEDTLS_SSL_CID_PADDING_GRANULARITY; /* Write real content type */ if( remaining == 0 ) @@ -651,10 +656,14 @@ int mbedtls_ssl_encrypt_buf( mbedtls_ssl_context *ssl, #if defined(MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL) if( transform->minor_ver == MBEDTLS_SSL_MINOR_VERSION_4 ) { + size_t padding = + ssl_compute_padding_length( rec->data_len, + MBEDTLS_SSL_TLS13_PADDING_GRANULARITY ); if( ssl_build_inner_plaintext( data, - &rec->data_len, - post_avail, - rec->type ) != 0 ) + &rec->data_len, + post_avail, + rec->type, + padding ) != 0 ) { return( MBEDTLS_ERR_SSL_BUFFER_TOO_SMALL ); } @@ -673,6 +682,9 @@ int mbedtls_ssl_encrypt_buf( mbedtls_ssl_context *ssl, if( rec->cid_len != 0 ) { + size_t padding = + ssl_compute_padding_length( rec->data_len, + MBEDTLS_SSL_CID_PADDING_GRANULARITY ); /* * Wrap plaintext into DTLSInnerPlaintext structure. * See ssl_build_inner_plaintext() for more information. @@ -683,7 +695,8 @@ int mbedtls_ssl_encrypt_buf( mbedtls_ssl_context *ssl, if( ssl_build_inner_plaintext( data, &rec->data_len, post_avail, - rec->type ) != 0 ) + rec->type, + padding ) != 0 ) { return( MBEDTLS_ERR_SSL_BUFFER_TOO_SMALL ); } From 67a37db2d2a38664f9624c4e3fac08db23117870 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Thu, 28 May 2020 16:27:07 +0100 Subject: [PATCH 24/38] Add missing configuration guards to SSL record protection helpers Signed-off-by: Hanno Becker --- library/ssl_msg.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/library/ssl_msg.c b/library/ssl_msg.c index 4ae1b8f08..5146605e2 100644 --- a/library/ssl_msg.c +++ b/library/ssl_msg.c @@ -541,13 +541,15 @@ static void ssl_mac( mbedtls_md_context_t *md_ctx, } #endif /* MBEDTLS_SSL_PROTO_SSL3 */ +#if defined(MBEDTLS_GCM_C) || \ + defined(MBEDTLS_CCM_C) || \ + defined(MBEDTLS_CHACHAPOLY_C) static int ssl_transform_aead_dynamic_iv_is_explicit( mbedtls_ssl_transform const *transform ) { return( transform->ivlen != transform->fixed_ivlen ); } - /* Compute IV := ( fixed_iv || 0 ) XOR ( 0 || dynamic_IV ) * * Concretely, this occurs in two variants: @@ -575,6 +577,7 @@ static void ssl_build_record_nonce( unsigned char *dst_iv, for( i = 0; i < dynamic_iv_len; i++ ) dst_iv[i] ^= dynamic_iv[i]; } +#endif /* MBEDTLS_GCM_C || MBEDTLS_CCM_C || MBEDTLS_CHACHAPOLY_C */ int mbedtls_ssl_encrypt_buf( mbedtls_ssl_context *ssl, mbedtls_ssl_transform *transform, From c3f7b0b16b98780708ccf03fe2b6d8bd573b11ae Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Thu, 28 May 2020 16:27:16 +0100 Subject: [PATCH 25/38] Fix #endif indicator comment Signed-off-by: Hanno Becker --- library/ssl_msg.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/ssl_msg.c b/library/ssl_msg.c index 5146605e2..0d1619248 100644 --- a/library/ssl_msg.c +++ b/library/ssl_msg.c @@ -900,7 +900,7 @@ int mbedtls_ssl_encrypt_buf( mbedtls_ssl_context *ssl, auth_done++; } else -#endif /* MBEDTLS_GCM_C || MBEDTLS_CCM_C */ +#endif /* MBEDTLS_GCM_C || MBEDTLS_CCM_C || MBEDTLS_CHACHAPOLY_C */ #if defined(MBEDTLS_CIPHER_MODE_CBC) && \ ( defined(MBEDTLS_AES_C) || defined(MBEDTLS_CAMELLIA_C) || defined(MBEDTLS_ARIA_C) ) if( mode == MBEDTLS_MODE_CBC ) From 3427f1dad2559e03850b1098e1a20066477dd0eb Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Fri, 29 May 2020 07:24:32 +0100 Subject: [PATCH 26/38] Update query_config.c Signed-off-by: Hanno Becker --- programs/test/query_config.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/programs/test/query_config.c b/programs/test/query_config.c index 7389605b6..7f9410cf7 100644 --- a/programs/test/query_config.c +++ b/programs/test/query_config.c @@ -2610,6 +2610,14 @@ int query_config( const char *config ) } #endif /* MBEDTLS_SSL_CID_PADDING_GRANULARITY */ +#if defined(MBEDTLS_SSL_TLS13_PADDING_GRANULARITY) + if( strcmp( "MBEDTLS_SSL_TLS13_PADDING_GRANULARITY", config ) == 0 ) + { + MACRO_EXPANSION_TO_STR( MBEDTLS_SSL_TLS13_PADDING_GRANULARITY ); + return( 0 ); + } +#endif /* MBEDTLS_SSL_TLS13_PADDING_GRANULARITY */ + #if defined(MBEDTLS_SSL_OUT_CONTENT_LEN) if( strcmp( "MBEDTLS_SSL_OUT_CONTENT_LEN", config ) == 0 ) { From 29e9895faa3852ec37fb6c9632088b23a61af138 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Sun, 31 May 2020 07:30:00 +0100 Subject: [PATCH 27/38] Change TLS 1.3 default padding to no padding Signed-off-by: Hanno Becker --- include/mbedtls/config.h | 2 +- include/mbedtls/ssl.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/include/mbedtls/config.h b/include/mbedtls/config.h index 9d700df8e..874cfb774 100644 --- a/include/mbedtls/config.h +++ b/include/mbedtls/config.h @@ -3568,7 +3568,7 @@ * Note: On systems lacking division instructions, * a power of two should be preferred. */ -//#define MBEDTLS_SSL_TLS13_PADDING_GRANULARITY 16 +//#define MBEDTLS_SSL_TLS13_PADDING_GRANULARITY 1 /** \def MBEDTLS_SSL_OUT_CONTENT_LEN * diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 4a6ea6cab..2e552b81c 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -278,7 +278,7 @@ #endif #if !defined(MBEDTLS_SSL_TLS13_PADDING_GRANULARITY) -#define MBEDTLS_SSL_TLS13_PADDING_GRANULARITY 16 +#define MBEDTLS_SSL_TLS13_PADDING_GRANULARITY 1 #endif /* \} name SECTION: Module settings */ From 9338f9f7187af0c12a9a7519bf29101f345af7a0 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Sun, 31 May 2020 07:39:50 +0100 Subject: [PATCH 28/38] Add documentation on state of upstreaming of TLS 1.3 prototype Signed-off-by: Hanno Becker --- docs/architecture/tls13-experimental.md | 36 +++++++++++++++++++++++++ 1 file changed, 36 insertions(+) create mode 100644 docs/architecture/tls13-experimental.md diff --git a/docs/architecture/tls13-experimental.md b/docs/architecture/tls13-experimental.md new file mode 100644 index 000000000..ee8452be7 --- /dev/null +++ b/docs/architecture/tls13-experimental.md @@ -0,0 +1,36 @@ +Experimental TLS 1.3 develpoments +================================= + +Overview +-------- + +Mbed TLS doesn't support the TLS 1.3 protocol yet, but a prototype is in development. +Stable parts of this prototype that can be independently tested are being successively +upstreamed under the guard of the following macro: + +``` +MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL +``` + +This macro will likely be renamed to `MBEDTLS_SSL_PROTO_TLS1_3` once a minimal viable +implementation of the TLS 1.3 protocol is available. + +See the [documentation of `MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL`](../../include/mbedtls/config.h) +for more information. + +Status +------ + +The following lists which parts of the TLS 1.3 prototype have already been upstreamed +together with their level of testing: + +* TLS 1.3 record protection mechanisms + + The record protection routines `mbedtls_ssl_{encrypt|decrypt}_buf()` have been extended + to support the modified TLS 1.3 record protection mechanism, including modified computation + of AAD, IV, and the introduction of a flexible padding. + + Those record protection routines have unit tests in `test_suite_ssl` alongside the + tests for the other record protection routines. + + TODO: Add some test vectors from RFC 8448. From ceef848eb6e8975d452596f36342b793b1edb947 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 2 Jun 2020 06:16:00 +0100 Subject: [PATCH 29/38] Rename TLS 1.3 padding granularity macro This is to avoid confusion with the class of macros MBEDTLS_SSL_PROTO_TLS1_X which have an underscore between major and minor version number. Signed-off-by: Hanno Becker --- include/mbedtls/config.h | 4 ++-- include/mbedtls/ssl.h | 4 ++-- library/ssl_msg.c | 2 +- programs/test/query_config.c | 8 ++++---- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/include/mbedtls/config.h b/include/mbedtls/config.h index 874cfb774..9ba566e4f 100644 --- a/include/mbedtls/config.h +++ b/include/mbedtls/config.h @@ -3554,7 +3554,7 @@ */ //#define MBEDTLS_SSL_CID_PADDING_GRANULARITY 16 -/** \def MBEDTLS_SSL_TLS13_PADDING_GRANULARITY +/** \def MBEDTLS_SSL_TLS1_3_PADDING_GRANULARITY * * This option controls the use of record plaintext padding * in TLS 1.3. @@ -3568,7 +3568,7 @@ * Note: On systems lacking division instructions, * a power of two should be preferred. */ -//#define MBEDTLS_SSL_TLS13_PADDING_GRANULARITY 1 +//#define MBEDTLS_SSL_TLS1_3_PADDING_GRANULARITY 1 /** \def MBEDTLS_SSL_OUT_CONTENT_LEN * diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 2e552b81c..65424d6d0 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -277,8 +277,8 @@ #define MBEDTLS_SSL_CID_PADDING_GRANULARITY 16 #endif -#if !defined(MBEDTLS_SSL_TLS13_PADDING_GRANULARITY) -#define MBEDTLS_SSL_TLS13_PADDING_GRANULARITY 1 +#if !defined(MBEDTLS_SSL_TLS1_3_PADDING_GRANULARITY) +#define MBEDTLS_SSL_TLS1_3_PADDING_GRANULARITY 1 #endif /* \} name SECTION: Module settings */ diff --git a/library/ssl_msg.c b/library/ssl_msg.c index 0d1619248..2c296561d 100644 --- a/library/ssl_msg.c +++ b/library/ssl_msg.c @@ -661,7 +661,7 @@ int mbedtls_ssl_encrypt_buf( mbedtls_ssl_context *ssl, { size_t padding = ssl_compute_padding_length( rec->data_len, - MBEDTLS_SSL_TLS13_PADDING_GRANULARITY ); + MBEDTLS_SSL_TLS1_3_PADDING_GRANULARITY ); if( ssl_build_inner_plaintext( data, &rec->data_len, post_avail, diff --git a/programs/test/query_config.c b/programs/test/query_config.c index 7f9410cf7..58aa87527 100644 --- a/programs/test/query_config.c +++ b/programs/test/query_config.c @@ -2610,13 +2610,13 @@ int query_config( const char *config ) } #endif /* MBEDTLS_SSL_CID_PADDING_GRANULARITY */ -#if defined(MBEDTLS_SSL_TLS13_PADDING_GRANULARITY) - if( strcmp( "MBEDTLS_SSL_TLS13_PADDING_GRANULARITY", config ) == 0 ) +#if defined(MBEDTLS_SSL_TLS1_3_PADDING_GRANULARITY) + if( strcmp( "MBEDTLS_SSL_TLS1_3_PADDING_GRANULARITY", config ) == 0 ) { - MACRO_EXPANSION_TO_STR( MBEDTLS_SSL_TLS13_PADDING_GRANULARITY ); + MACRO_EXPANSION_TO_STR( MBEDTLS_SSL_TLS1_3_PADDING_GRANULARITY ); return( 0 ); } -#endif /* MBEDTLS_SSL_TLS13_PADDING_GRANULARITY */ +#endif /* MBEDTLS_SSL_TLS1_3_PADDING_GRANULARITY */ #if defined(MBEDTLS_SSL_OUT_CONTENT_LEN) if( strcmp( "MBEDTLS_SSL_OUT_CONTENT_LEN", config ) == 0 ) From 6055a17d7d95c62c2bb4763a108646a4700cf265 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 2 Jun 2020 06:20:23 +0100 Subject: [PATCH 30/38] Add dependencies for experimental TLS 1.3 features in check_config.h Signed-off-by: Hanno Becker --- include/mbedtls/check_config.h | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/include/mbedtls/check_config.h b/include/mbedtls/check_config.h index fa3caa7c4..4111b37f6 100644 --- a/include/mbedtls/check_config.h +++ b/include/mbedtls/check_config.h @@ -619,6 +619,11 @@ #error "MBEDTLS_SSL_PROTO_TLS1_2 defined, but not all prerequisites" #endif +#if defined(MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL) && ( !defined(MBEDTLS_HKDF_C) \ + !defined(MBEDTLS_SHA256_C) && !defined(MBEDTLS_SHA512_C) ) +#error "MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL defined, but not all prerequisites" +#endif + #if (defined(MBEDTLS_SSL_PROTO_SSL3) || defined(MBEDTLS_SSL_PROTO_TLS1) || \ defined(MBEDTLS_SSL_PROTO_TLS1_1) || defined(MBEDTLS_SSL_PROTO_TLS1_2)) && \ !(defined(MBEDTLS_KEY_EXCHANGE_RSA_ENABLED) || \ From 0c3bebfa1503c165efea47d3f0de5f9dca52e05b Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 2 Jun 2020 06:32:43 +0100 Subject: [PATCH 31/38] Fix typo in header of TLS 1.3 experimental features document Signed-off-by: Hanno Becker --- docs/architecture/tls13-experimental.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/architecture/tls13-experimental.md b/docs/architecture/tls13-experimental.md index ee8452be7..0e3c7cc9e 100644 --- a/docs/architecture/tls13-experimental.md +++ b/docs/architecture/tls13-experimental.md @@ -1,4 +1,4 @@ -Experimental TLS 1.3 develpoments +TLS 1.3 Experimental Developments ================================= Overview From 5a83d29114c0dad0cee8cfef3f56d1f4d5c791e4 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 2 Jun 2020 06:33:00 +0100 Subject: [PATCH 32/38] Mention HKDF in TLS 1.3 feature document Signed-off-by: Hanno Becker --- docs/architecture/tls13-experimental.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/architecture/tls13-experimental.md b/docs/architecture/tls13-experimental.md index 0e3c7cc9e..bcf3e34f9 100644 --- a/docs/architecture/tls13-experimental.md +++ b/docs/architecture/tls13-experimental.md @@ -34,3 +34,7 @@ together with their level of testing: tests for the other record protection routines. TODO: Add some test vectors from RFC 8448. + +- The HKDF key derivation function on which the TLS 1.3 key schedule is based, + is already present as an independent module controlled by `MBEDTLS_HKDF_C` + independently of the development of the TLS 1.3 prototype. From afca47a6b9521ea49984bc2850233d420cb67ab5 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 2 Jun 2020 07:51:26 +0100 Subject: [PATCH 33/38] Fix typo in check_config.h Signed-off-by: Hanno Becker --- include/mbedtls/check_config.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/mbedtls/check_config.h b/include/mbedtls/check_config.h index 4111b37f6..e2e45ac98 100644 --- a/include/mbedtls/check_config.h +++ b/include/mbedtls/check_config.h @@ -619,7 +619,7 @@ #error "MBEDTLS_SSL_PROTO_TLS1_2 defined, but not all prerequisites" #endif -#if defined(MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL) && ( !defined(MBEDTLS_HKDF_C) \ +#if defined(MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL) && ( !defined(MBEDTLS_HKDF_C) && \ !defined(MBEDTLS_SHA256_C) && !defined(MBEDTLS_SHA512_C) ) #error "MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL defined, but not all prerequisites" #endif From 7cca3589cb8b6bc4c8bbc9d7bed42e9d2b520d13 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Thu, 4 Jun 2020 13:27:22 +0100 Subject: [PATCH 34/38] Fix indentation in debug statement in ssl_msg.c Signed-off-by: Hanno Becker --- library/ssl_msg.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/ssl_msg.c b/library/ssl_msg.c index 2c296561d..33e9a4ef7 100644 --- a/library/ssl_msg.c +++ b/library/ssl_msg.c @@ -851,7 +851,7 @@ int mbedtls_ssl_encrypt_buf( mbedtls_ssl_context *ssl, transform->minor_ver ); MBEDTLS_SSL_DEBUG_BUF( 4, "IV used (internal)", - iv, transform->ivlen ); + iv, transform->ivlen ); MBEDTLS_SSL_DEBUG_BUF( 4, "IV used (transmitted)", data - dynamic_iv_len * dynamic_iv_is_explicit, dynamic_iv_len * dynamic_iv_is_explicit ); From 16bf0e234663936ff533292ef64fba7001f694dc Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Thu, 4 Jun 2020 13:27:34 +0100 Subject: [PATCH 35/38] Fix debug print of explicit IV The previous version attempted to write the explicit IV from the destination buffer before it has been written there. Signed-off-by: Hanno Becker --- library/ssl_msg.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library/ssl_msg.c b/library/ssl_msg.c index 33e9a4ef7..26f30b5d1 100644 --- a/library/ssl_msg.c +++ b/library/ssl_msg.c @@ -853,8 +853,8 @@ int mbedtls_ssl_encrypt_buf( mbedtls_ssl_context *ssl, MBEDTLS_SSL_DEBUG_BUF( 4, "IV used (internal)", iv, transform->ivlen ); MBEDTLS_SSL_DEBUG_BUF( 4, "IV used (transmitted)", - data - dynamic_iv_len * dynamic_iv_is_explicit, - dynamic_iv_len * dynamic_iv_is_explicit ); + dynamic_iv, + dynamic_iv_is_explicit ? dynamic_iv_len : 0 ); MBEDTLS_SSL_DEBUG_BUF( 4, "additional data used for AEAD", add_data, add_data_len ); MBEDTLS_SSL_DEBUG_MSG( 3, ( "before encrypt: msglen = %d, " From 1cda2667afb0bb654db712321b1cd84b6028c34a Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Thu, 4 Jun 2020 13:28:28 +0100 Subject: [PATCH 36/38] Spell out check for non-zero'ness Signed-off-by: Hanno Becker --- library/ssl_msg.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/ssl_msg.c b/library/ssl_msg.c index 26f30b5d1..65eab4b40 100644 --- a/library/ssl_msg.c +++ b/library/ssl_msg.c @@ -884,7 +884,7 @@ int mbedtls_ssl_encrypt_buf( mbedtls_ssl_context *ssl, /* * Prefix record content with dynamic IV in case it is explicit. */ - if( dynamic_iv_is_explicit ) + if( dynamic_iv_is_explicit != 0 ) { if( rec->data_offset < dynamic_iv_len ) { From 15952814d837cc5687230861d1defed0458ce0aa Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Thu, 4 Jun 2020 13:31:46 +0100 Subject: [PATCH 37/38] Improve documentation of nonce-generating function in ssl_msg.c Signed-off-by: Hanno Becker --- library/ssl_msg.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/library/ssl_msg.c b/library/ssl_msg.c index 65eab4b40..32bbc97be 100644 --- a/library/ssl_msg.c +++ b/library/ssl_msg.c @@ -557,8 +557,14 @@ static int ssl_transform_aead_dynamic_iv_is_explicit( * a) Fixed and dynamic IV lengths add up to total IV length, giving * IV = fixed_iv || dynamic_iv * + * This variant is used in TLS 1.2 when used with GCM or CCM. + * * b) Fixed IV lengths matches total IV length, giving * IV = fixed_iv XOR ( 0 || dynamic_iv ) + * + * This variant occurs in TLS 1.3 and for TLS 1.2 when using ChaChaPoly. + * + * See also the documentation of mbedtls_ssl_transform. */ static void ssl_build_record_nonce( unsigned char *dst_iv, size_t dst_iv_len, From f486e286948bb96fcaf746f440a1cf936ba048f2 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Thu, 4 Jun 2020 13:33:08 +0100 Subject: [PATCH 38/38] Document precondition of nonce-generating function in ssl_msg.c Signed-off-by: Hanno Becker --- library/ssl_msg.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/library/ssl_msg.c b/library/ssl_msg.c index 32bbc97be..ae8d07653 100644 --- a/library/ssl_msg.c +++ b/library/ssl_msg.c @@ -565,6 +565,13 @@ static int ssl_transform_aead_dynamic_iv_is_explicit( * This variant occurs in TLS 1.3 and for TLS 1.2 when using ChaChaPoly. * * See also the documentation of mbedtls_ssl_transform. + * + * This function has the precondition that + * + * dst_iv_len >= max( fixed_iv_len, dynamic_iv_len ) + * + * which has to be ensured by the caller. If this precondition + * violated, the behavior of this function is undefined. */ static void ssl_build_record_nonce( unsigned char *dst_iv, size_t dst_iv_len,