From a73b57774451017a0499e24b22c99ae850093471 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 19 Jul 2021 14:36:03 +0200 Subject: [PATCH 01/14] Make the fields of mbedtls_ecp_curve_info public The whole point of this structure is to provide information, both for the library's own sake and to applications. Signed-off-by: Gilles Peskine --- include/mbedtls/ecp.h | 12 ++++++++---- programs/pkey/gen_key.c | 12 ++++++------ 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/include/mbedtls/ecp.h b/include/mbedtls/ecp.h index 384d0608a..b2a2e3256 100644 --- a/include/mbedtls/ecp.h +++ b/include/mbedtls/ecp.h @@ -147,13 +147,17 @@ typedef enum /** * Curve information, for use by other modules. + * + * The fields of this structure are part of the public API and can be + * accessed directly by applications. Future versions of the library may + * add extra fields or reorder existing fields. */ typedef struct mbedtls_ecp_curve_info { - mbedtls_ecp_group_id MBEDTLS_PRIVATE(grp_id); /*!< An internal identifier. */ - uint16_t MBEDTLS_PRIVATE(tls_id); /*!< The TLS NamedCurve identifier. */ - uint16_t MBEDTLS_PRIVATE(bit_size); /*!< The curve size in bits. */ - const char *MBEDTLS_PRIVATE(name); /*!< A human-friendly name. */ + mbedtls_ecp_group_id grp_id; /*!< An internal identifier. */ + uint16_t tls_id; /*!< The TLS NamedCurve identifier. */ + uint16_t bit_size; /*!< The curve size in bits. */ + const char *name; /*!< A human-friendly name. */ } mbedtls_ecp_curve_info; /** diff --git a/programs/pkey/gen_key.c b/programs/pkey/gen_key.c index 4043dfa6e..7535eee3f 100644 --- a/programs/pkey/gen_key.c +++ b/programs/pkey/gen_key.c @@ -86,7 +86,7 @@ int dev_random_entropy_poll( void *data, unsigned char *output, #endif #if defined(MBEDTLS_ECP_C) -#define DFL_EC_CURVE mbedtls_ecp_curve_list()->MBEDTLS_PRIVATE(grp_id) +#define DFL_EC_CURVE mbedtls_ecp_curve_list()->grp_id #else #define DFL_EC_CURVE 0 #endif @@ -219,9 +219,9 @@ int main( int argc, char *argv[] ) #if defined(MBEDTLS_ECP_C) mbedtls_printf( " available ec_curve values:\n" ); curve_info = mbedtls_ecp_curve_list(); - mbedtls_printf( " %s (default)\n", curve_info->MBEDTLS_PRIVATE(name) ); - while( ( ++curve_info )->MBEDTLS_PRIVATE(name) != NULL ) - mbedtls_printf( " %s\n", curve_info->MBEDTLS_PRIVATE(name) ); + mbedtls_printf( " %s (default)\n", curve_info->name ); + while( ( ++curve_info )->name != NULL ) + mbedtls_printf( " %s\n", curve_info->name ); #endif /* MBEDTLS_ECP_C */ goto exit; } @@ -270,7 +270,7 @@ int main( int argc, char *argv[] ) { if( ( curve_info = mbedtls_ecp_curve_info_from_name( q ) ) == NULL ) goto usage; - opt.ec_curve = curve_info->MBEDTLS_PRIVATE(grp_id); + opt.ec_curve = curve_info->grp_id; } #endif else if( strcmp( p, "filename" ) == 0 ) @@ -391,7 +391,7 @@ int main( int argc, char *argv[] ) { mbedtls_ecp_keypair *ecp = mbedtls_pk_ec( key ); mbedtls_printf( "curve: %s\n", - mbedtls_ecp_curve_info_from_grp_id( ecp->MBEDTLS_PRIVATE(grp).id )->MBEDTLS_PRIVATE(name) ); + mbedtls_ecp_curve_info_from_grp_id( ecp->MBEDTLS_PRIVATE(grp).id )->name ); mbedtls_mpi_write_file( "X_Q: ", &ecp->MBEDTLS_PRIVATE(Q).MBEDTLS_PRIVATE(X), 16, NULL ); mbedtls_mpi_write_file( "Y_Q: ", &ecp->MBEDTLS_PRIVATE(Q).MBEDTLS_PRIVATE(Y), 16, NULL ); mbedtls_mpi_write_file( "D: ", &ecp->MBEDTLS_PRIVATE(d) , 16, NULL ); From 0be02bd823c07737775a2251e7426270f2c4f902 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 19 Jul 2021 16:32:54 +0200 Subject: [PATCH 02/14] Add accessor functions for cipher_info fields Add functions to read the type, mode, name and key_bitlen fields from mbedtls_cipher_info_t. These are the fields that applications are most likely to care about. TLS code also uses iv_size and block_size, which it might make sense to expose, but most applications shouldn't need those, so I'm not exposing them for now. Call the new functions in unit tests, so they're at least smoke-tested. Signed-off-by: Gilles Peskine --- include/mbedtls/cipher.h | 76 +++++++++++++++++++++++++ tests/suites/test_suite_cipher.function | 42 +++++++++++++- 2 files changed, 117 insertions(+), 1 deletion(-) diff --git a/include/mbedtls/cipher.h b/include/mbedtls/cipher.h index 9c9a2e88c..7921f4d85 100644 --- a/include/mbedtls/cipher.h +++ b/include/mbedtls/cipher.h @@ -414,6 +414,82 @@ const mbedtls_cipher_info_t *mbedtls_cipher_info_from_values( const mbedtls_ciph int key_bitlen, const mbedtls_cipher_mode_t mode ); +/** + * \brief Retrieve the identifier for a cipher info structure. + * + * \param[in] info The cipher info structure to query. + * This may be \c NULL. + * + * \return The full cipher identifier (\c MBEDTLS_CIPHER_xxx). + * \return #MBEDTLS_CIPHER_NONE if \p info is \c NULL. + */ +static inline mbedtls_cipher_type_t mbedtls_cipher_info_get_type( + const mbedtls_cipher_info_t *info ) +{ + if( info == NULL ) + return( MBEDTLS_CIPHER_NONE ); + else + return( info->MBEDTLS_PRIVATE(type) ); +} + +/** + * \brief Retrieve the operation mode for a cipher info structure. + * + * \param[in] info The cipher info structure to query. + * This may be \c NULL. + * + * \return The cipher mode (\c MBEDTLS_MODE_xxx). + * \return #MBEDTLS_MODE_NONE if \p info is \c NULL. + */ +static inline mbedtls_cipher_mode_t mbedtls_cipher_info_get_mode( + const mbedtls_cipher_info_t *info ) +{ + if( info == NULL ) + return( MBEDTLS_MODE_NONE ); + else + return( info->MBEDTLS_PRIVATE(mode) ); +} + +/** + * \brief Retrieve the key size for a cipher info structure. + * + * \param[in] info The cipher info structure to query. + * This may be \c NULL. + * + * \return The key length in bits. + * For variable-sized ciphers, this is the default length. + * For DES, this includes the parity bits. + * \return \c 0 if \p info is \c NULL. + */ +static inline size_t mbedtls_cipher_info_get_key_bitlen( + const mbedtls_cipher_info_t *info ) +{ + if( info == NULL ) + return( 0 ); + else + return( info->MBEDTLS_PRIVATE(key_bitlen) ); +} + +/** + * \brief Retrieve the human-readable name for a + * cipher info structure. + * + * \param[in] info The cipher info structure to query. + * This may be \c NULL. + * + * \return The cipher name, which is a human readable string, + * with static storage duration. + * \return \c NULL if \c info is \p NULL. + */ +static inline const char *mbedtls_cipher_info_get_name( + const mbedtls_cipher_info_t *info ) +{ + if( info == NULL ) + return( NULL ); + else + return( info->MBEDTLS_PRIVATE(name) ); +} + /** * \brief This function initializes a \p cipher_context as NONE. * diff --git a/tests/suites/test_suite_cipher.function b/tests/suites/test_suite_cipher.function index 674349f76..94ea88f79 100644 --- a/tests/suites/test_suite_cipher.function +++ b/tests/suites/test_suite_cipher.function @@ -13,6 +13,38 @@ #define MBEDTLS_CIPHER_AUTH_CRYPT #endif +/* Check the internal consistency of a cipher info structure, and + * check it against mbedtls_cipher_info_from_xxx(). */ +static int check_cipher_info( mbedtls_cipher_type_t type, + const mbedtls_cipher_info_t *info ) +{ + size_t key_bitlen; + + TEST_ASSERT( info != NULL ); + TEST_EQUAL( type, mbedtls_cipher_info_get_type( info ) ); + TEST_EQUAL( type, info->type ); + TEST_ASSERT( mbedtls_cipher_info_from_type( type ) == info ); + + TEST_EQUAL( info->mode, mbedtls_cipher_info_get_mode( info ) ); + + /* Insist that get_name() return the string from the structure and + * not a copy. A copy would have an unknown storage duration. */ + TEST_ASSERT( mbedtls_cipher_info_get_name( info ) == info->name ); + TEST_ASSERT( mbedtls_cipher_info_from_string( info->name ) == info ); + + key_bitlen = mbedtls_cipher_info_get_key_bitlen( info ); + TEST_ASSERT( key_bitlen % 8 == 0 ); + /* All current and plausible supported ciphers use a 64-bit, 128-bit + * or 256-bit key, except XTS which uses a double AES key. */ + TEST_ASSERT( key_bitlen >= 64 ); + TEST_ASSERT( key_bitlen <= 512 ); + + return( 1 ); + +exit: + return( 0 ); +} + #if defined(MBEDTLS_CIPHER_AUTH_CRYPT) /* Helper for resetting key/direction * @@ -81,7 +113,13 @@ void mbedtls_cipher_list( ) const int *cipher_type; for( cipher_type = mbedtls_cipher_list(); *cipher_type != 0; cipher_type++ ) - TEST_ASSERT( mbedtls_cipher_info_from_type( *cipher_type ) != NULL ); + { + const mbedtls_cipher_info_t *info = + mbedtls_cipher_info_from_type( *cipher_type ); + mbedtls_test_set_step( *cipher_type ); + if( ! check_cipher_info( *cipher_type, info ) ) + goto exit; + } } /* END_CASE */ @@ -309,6 +347,8 @@ void enc_dec_buf( int cipher_id, char * cipher_string, int key_len, cipher_info = mbedtls_cipher_info_from_type( cipher_id ); TEST_ASSERT( NULL != cipher_info ); TEST_ASSERT( mbedtls_cipher_info_from_string( cipher_string ) == cipher_info ); + TEST_ASSERT( strcmp( mbedtls_cipher_info_get_name( cipher_info ), + cipher_string ) == 0 ); /* Initialise enc and dec contexts */ TEST_ASSERT( 0 == mbedtls_cipher_setup( &ctx_dec, cipher_info ) ); From 80932fa9448cb6c9016041f660f0d2444b1e27af Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 19 Jul 2021 17:34:02 +0200 Subject: [PATCH 03/14] Don't access cipher_info private fields in sample programs Use the new accessor functions. Signed-off-by: Gilles Peskine --- programs/aes/crypt_and_hash.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/programs/aes/crypt_and_hash.c b/programs/aes/crypt_and_hash.c index a8026a335..ba9827f45 100644 --- a/programs/aes/crypt_and_hash.c +++ b/programs/aes/crypt_and_hash.c @@ -124,7 +124,7 @@ int main( int argc, char *argv[] ) while( *list ) { cipher_info = mbedtls_cipher_info_from_type( *list ); - mbedtls_printf( " %s\n", cipher_info->MBEDTLS_PRIVATE(name) ); + mbedtls_printf( " %s\n", mbedtls_cipher_info_get_name( cipher_info ) ); list++; } @@ -309,7 +309,9 @@ int main( int argc, char *argv[] ) } - if( mbedtls_cipher_setkey( &cipher_ctx, digest, cipher_info->MBEDTLS_PRIVATE(key_bitlen), + if( mbedtls_cipher_setkey( &cipher_ctx, + digest, + mbedtls_cipher_info_get_key_bitlen( cipher_info ), MBEDTLS_ENCRYPT ) != 0 ) { mbedtls_fprintf( stderr, "mbedtls_cipher_setkey() returned error\n"); @@ -408,7 +410,7 @@ int main( int argc, char *argv[] ) /* * Check the file size. */ - if( cipher_info->MBEDTLS_PRIVATE(mode) != MBEDTLS_MODE_GCM && + if( mbedtls_cipher_info_get_mode( cipher_info ) != MBEDTLS_MODE_GCM && ( ( filesize - mbedtls_md_get_size( md_info ) ) % mbedtls_cipher_get_block_size( &cipher_ctx ) ) != 0 ) { @@ -448,7 +450,9 @@ int main( int argc, char *argv[] ) mbedtls_md_finish( &md_ctx, digest ); } - if( mbedtls_cipher_setkey( &cipher_ctx, digest, cipher_info->MBEDTLS_PRIVATE(key_bitlen), + if( mbedtls_cipher_setkey( &cipher_ctx, + digest, + mbedtls_cipher_info_get_key_bitlen( cipher_info ), MBEDTLS_DECRYPT ) != 0 ) { mbedtls_fprintf( stderr, "mbedtls_cipher_setkey() returned error\n" ); From e720dbe17762655b86e46b5968ba4e029c78c378 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 19 Jul 2021 17:37:46 +0200 Subject: [PATCH 04/14] Use cipher_info accessor functions in TLS code Signed-off-by: Gilles Peskine --- library/ssl_ticket.c | 6 +++--- library/ssl_tls.c | 22 +++++++++++----------- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/library/ssl_ticket.c b/library/ssl_ticket.c index bce9a1cd7..db2bb52b3 100644 --- a/library/ssl_ticket.c +++ b/library/ssl_ticket.c @@ -141,13 +141,13 @@ int mbedtls_ssl_ticket_setup( mbedtls_ssl_ticket_context *ctx, if( cipher_info == NULL ) return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA ); - if( cipher_info->mode != MBEDTLS_MODE_GCM && - cipher_info->mode != MBEDTLS_MODE_CCM ) + if( mbedtls_cipher_info_get_mode( cipher_info ) != MBEDTLS_MODE_GCM && + mbedtls_cipher_info_get_mode( cipher_info ) != MBEDTLS_MODE_CCM ) { return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA ); } - if( cipher_info->key_bitlen > 8 * MAX_KEY_BYTES ) + if( mbedtls_cipher_info_get_key_bitlen( cipher_info ) > 8 * MAX_KEY_BYTES ) return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA ); #if defined(MBEDTLS_USE_PSA_CRYPTO) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 07b51003a..1e81384aa 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -789,14 +789,14 @@ static int ssl_tls12_populate_transform( mbedtls_ssl_transform *transform, * Determine the appropriate key, IV and MAC length. */ - keylen = cipher_info->key_bitlen / 8; + keylen = mbedtls_cipher_info_get_key_bitlen( cipher_info ) / 8; #if defined(MBEDTLS_GCM_C) || \ defined(MBEDTLS_CCM_C) || \ defined(MBEDTLS_CHACHAPOLY_C) - if( cipher_info->mode == MBEDTLS_MODE_GCM || - cipher_info->mode == MBEDTLS_MODE_CCM || - cipher_info->mode == MBEDTLS_MODE_CHACHAPOLY ) + if( mbedtls_cipher_info_get_mode( cipher_info ) == MBEDTLS_MODE_GCM || + mbedtls_cipher_info_get_mode( cipher_info ) == MBEDTLS_MODE_CCM || + mbedtls_cipher_info_get_mode( cipher_info ) == MBEDTLS_MODE_CHACHAPOLY ) { size_t explicit_ivlen; @@ -814,7 +814,7 @@ static int ssl_tls12_populate_transform( mbedtls_ssl_transform *transform, * sequence number). */ transform->ivlen = 12; - if( cipher_info->mode == MBEDTLS_MODE_CHACHAPOLY ) + if( mbedtls_cipher_info_get_mode( cipher_info ) == MBEDTLS_MODE_CHACHAPOLY ) transform->fixed_ivlen = 12; else transform->fixed_ivlen = 4; @@ -826,8 +826,8 @@ static int ssl_tls12_populate_transform( mbedtls_ssl_transform *transform, else #endif /* MBEDTLS_GCM_C || MBEDTLS_CCM_C || MBEDTLS_CHACHAPOLY_C */ #if defined(MBEDTLS_SSL_SOME_SUITES_USE_MAC) - if( cipher_info->mode == MBEDTLS_MODE_STREAM || - cipher_info->mode == MBEDTLS_MODE_CBC ) + if( mbedtls_cipher_info_get_mode( cipher_info ) == MBEDTLS_MODE_STREAM || + mbedtls_cipher_info_get_mode( cipher_info ) == MBEDTLS_MODE_CBC ) { /* Initialize HMAC contexts */ if( ( ret = mbedtls_md_setup( &transform->md_ctx_enc, md_info, 1 ) ) != 0 || @@ -845,7 +845,7 @@ static int ssl_tls12_populate_transform( mbedtls_ssl_transform *transform, transform->ivlen = cipher_info->iv_size; /* Minimum length */ - if( cipher_info->mode == MBEDTLS_MODE_STREAM ) + if( mbedtls_cipher_info_get_mode( cipher_info ) == MBEDTLS_MODE_STREAM ) transform->minlen = transform->maclen; else { @@ -1060,7 +1060,7 @@ static int ssl_tls12_populate_transform( mbedtls_ssl_transform *transform, } if( ( ret = mbedtls_cipher_setkey( &transform->cipher_ctx_enc, key1, - cipher_info->key_bitlen, + mbedtls_cipher_info_get_key_bitlen( cipher_info ), MBEDTLS_ENCRYPT ) ) != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_cipher_setkey", ret ); @@ -1068,7 +1068,7 @@ static int ssl_tls12_populate_transform( mbedtls_ssl_transform *transform, } if( ( ret = mbedtls_cipher_setkey( &transform->cipher_ctx_dec, key2, - cipher_info->key_bitlen, + mbedtls_cipher_info_get_key_bitlen( cipher_info ), MBEDTLS_DECRYPT ) ) != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_cipher_setkey", ret ); @@ -1076,7 +1076,7 @@ static int ssl_tls12_populate_transform( mbedtls_ssl_transform *transform, } #if defined(MBEDTLS_CIPHER_MODE_CBC) - if( cipher_info->mode == MBEDTLS_MODE_CBC ) + if( mbedtls_cipher_info_get_mode( cipher_info ) == MBEDTLS_MODE_CBC ) { if( ( ret = mbedtls_cipher_set_padding_mode( &transform->cipher_ctx_enc, MBEDTLS_PADDING_NONE ) ) != 0 ) From ce9e3a92fe3db6f20b267005e9f12b3e6ffbd389 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 19 Jul 2021 17:38:09 +0200 Subject: [PATCH 05/14] Remove redundant null check mbedtls_cipher_info_get_xxx has well-defined behavior on NULL, so no need to check first. Signed-off-by: Gilles Peskine --- library/ssl_ticket.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/library/ssl_ticket.c b/library/ssl_ticket.c index db2bb52b3..e998111d9 100644 --- a/library/ssl_ticket.c +++ b/library/ssl_ticket.c @@ -138,8 +138,6 @@ int mbedtls_ssl_ticket_setup( mbedtls_ssl_ticket_context *ctx, ctx->ticket_lifetime = lifetime; cipher_info = mbedtls_cipher_info_from_type( cipher); - if( cipher_info == NULL ) - return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA ); if( mbedtls_cipher_info_get_mode( cipher_info ) != MBEDTLS_MODE_GCM && mbedtls_cipher_info_get_mode( cipher_info ) != MBEDTLS_MODE_CCM ) From b11d61e095dd656afec02716f350db1e2063a8b9 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 4 Aug 2021 20:38:59 +0200 Subject: [PATCH 06/14] mbedtls_net_context: make fd public on Unix/POSIX platforms On platforms with BSD-like sockets, it is useful for applications to have access to the underlying file descriptor so that they can use functions like select() and poll(). Do not promise that the field will exist on other platforms such as Windows (where the type and name of the field are technically wrong because Windows socket handles are actually not file descriptors). Signed-off-by: Gilles Peskine --- include/mbedtls/net_sockets.h | 8 +++++++- programs/ssl/mini_client.c | 4 ++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/include/mbedtls/net_sockets.h b/include/mbedtls/net_sockets.h index c8214a233..0c754b122 100644 --- a/include/mbedtls/net_sockets.h +++ b/include/mbedtls/net_sockets.h @@ -94,7 +94,13 @@ extern "C" { */ typedef struct mbedtls_net_context { - int MBEDTLS_PRIVATE(fd); /**< The underlying file descriptor */ + /** The underlying file descriptor. + * + * This field is only guaranteed to be present on POSIX/Unix-like platforms. + * On other platforms, it may have a different type, have a different + * meaning, or be absent altogether. + */ + int fd; } mbedtls_net_context; diff --git a/programs/ssl/mini_client.c b/programs/ssl/mini_client.c index 1e0bef6b1..97bfe6806 100644 --- a/programs/ssl/mini_client.c +++ b/programs/ssl/mini_client.c @@ -246,13 +246,13 @@ int main( void ) addr.sin_addr.s_addr = *((char *) &ret) == ret ? ADDR_LE : ADDR_BE; ret = 0; - if( ( server_fd.MBEDTLS_PRIVATE(fd) = socket( AF_INET, SOCK_STREAM, 0 ) ) < 0 ) + if( ( server_fd.fd = socket( AF_INET, SOCK_STREAM, 0 ) ) < 0 ) { ret = socket_failed; goto exit; } - if( connect( server_fd.MBEDTLS_PRIVATE(fd), + if( connect( server_fd.fd, (const struct sockaddr *) &addr, sizeof( addr ) ) < 0 ) { ret = connect_failed; From b89d9c05990355b9a7522d62380a41c237195a3b Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 4 Aug 2021 20:55:33 +0200 Subject: [PATCH 07/14] Make fields of ASN.1 data structures public The structures mbedtls_asn1_buf, mbedtls_asn1_bitstring, mbedtls_asn1_sequence and mbedtls_asn1_named_data are designed to allow access to data after parsing. Make their fields public. Document that chaining fields are essentially read-only. Signed-off-by: Gilles Peskine --- include/mbedtls/asn1.h | 46 +++++++++++++++++++++++++++++++----------- 1 file changed, 34 insertions(+), 12 deletions(-) diff --git a/include/mbedtls/asn1.h b/include/mbedtls/asn1.h index f45fc1751..34a39d9eb 100644 --- a/include/mbedtls/asn1.h +++ b/include/mbedtls/asn1.h @@ -152,9 +152,9 @@ extern "C" { */ typedef struct mbedtls_asn1_buf { - int MBEDTLS_PRIVATE(tag); /**< ASN1 type, e.g. MBEDTLS_ASN1_UTF8_STRING. */ - size_t MBEDTLS_PRIVATE(len); /**< ASN1 length, in octets. */ - unsigned char *MBEDTLS_PRIVATE(p); /**< ASN1 data, e.g. in ASCII. */ + int tag; /**< ASN1 type, e.g. MBEDTLS_ASN1_UTF8_STRING. */ + size_t len; /**< ASN1 length, in octets. */ + unsigned char *p; /**< ASN1 data, e.g. in ASCII. */ } mbedtls_asn1_buf; @@ -163,9 +163,9 @@ mbedtls_asn1_buf; */ typedef struct mbedtls_asn1_bitstring { - size_t MBEDTLS_PRIVATE(len); /**< ASN1 length, in octets. */ - unsigned char MBEDTLS_PRIVATE(unused_bits); /**< Number of unused bits at the end of the string */ - unsigned char *MBEDTLS_PRIVATE(p); /**< Raw ASN1 data for the bit string */ + size_t len; /**< ASN1 length, in octets. */ + unsigned char unused_bits; /**< Number of unused bits at the end of the string */ + unsigned char *p; /**< Raw ASN1 data for the bit string */ } mbedtls_asn1_bitstring; @@ -174,8 +174,16 @@ mbedtls_asn1_bitstring; */ typedef struct mbedtls_asn1_sequence { - mbedtls_asn1_buf MBEDTLS_PRIVATE(buf); /**< Buffer containing the given ASN.1 item. */ - struct mbedtls_asn1_sequence *MBEDTLS_PRIVATE(next); /**< The next entry in the sequence. */ + mbedtls_asn1_buf buf; /**< Buffer containing the given ASN.1 item. */ + + /** The next entry in the sequence. + * + * The details memory management for sequences are not documented and + * may change in future versions. Set this field to \p NULL when + * initializing a structure, and do not modify it except via Mbed TLS + * library functions. + */ + struct mbedtls_asn1_sequence *next; } mbedtls_asn1_sequence; @@ -184,10 +192,24 @@ mbedtls_asn1_sequence; */ typedef struct mbedtls_asn1_named_data { - mbedtls_asn1_buf MBEDTLS_PRIVATE(oid); /**< The object identifier. */ - mbedtls_asn1_buf MBEDTLS_PRIVATE(val); /**< The named value. */ - struct mbedtls_asn1_named_data *MBEDTLS_PRIVATE(next); /**< The next entry in the sequence. */ - unsigned char MBEDTLS_PRIVATE(next_merged); /**< Merge next item into the current one? */ + mbedtls_asn1_buf oid; /**< The object identifier. */ + mbedtls_asn1_buf val; /**< The named value. */ + + /** The next entry in the sequence. + * + * The details memory management for named data sequences are not documented + * and may change in future versions. Set this field to \p NULL when + * initializing a structure, and do not modify it except via Mbed TLS + * library functions. + */ + struct mbedtls_asn1_named_data *next; + + /** Merge next item into the current one? + * + * This field exists for the sake of Mbed TLS's X.509 certificate parsing + * code and may change in future versions of the library. + */ + unsigned char MBEDTLS_PRIVATE(next_merged); } mbedtls_asn1_named_data; From 842edf474c7c4844dd28aab059b931b0793d276d Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 4 Aug 2021 21:56:10 +0200 Subject: [PATCH 08/14] Make many fields of X.509 structures public The structures mbedtls_x509_time, mbedtls_x509_crl_entry, mbedtls_x509_crl, mbedtls_x509_crt, mbedtls_x509_san_other_name, mbedtls_x509_subject_alternative_name, mbedtls_x509_csr are designed to expose the result of parsing X.509 data. Document many of their fields as being publicly readable. Signed-off-by: Gilles Peskine --- include/mbedtls/x509.h | 4 +-- include/mbedtls/x509_crl.h | 43 ++++++++++++++---------- include/mbedtls/x509_crt.h | 69 ++++++++++++++++++++++---------------- include/mbedtls/x509_csr.h | 18 ++++++---- programs/x509/cert_write.c | 8 ++--- 5 files changed, 83 insertions(+), 59 deletions(-) diff --git a/include/mbedtls/x509.h b/include/mbedtls/x509.h index df187cb09..9a4be95a3 100644 --- a/include/mbedtls/x509.h +++ b/include/mbedtls/x509.h @@ -246,8 +246,8 @@ typedef mbedtls_asn1_sequence mbedtls_x509_sequence; /** Container for date and time (precision in seconds). */ typedef struct mbedtls_x509_time { - int MBEDTLS_PRIVATE(year), MBEDTLS_PRIVATE(mon), MBEDTLS_PRIVATE(day); /**< Date. */ - int MBEDTLS_PRIVATE(hour), MBEDTLS_PRIVATE(min), MBEDTLS_PRIVATE(sec); /**< Time. */ + int year, mon, day; /**< Date. */ + int hour, min, sec; /**< Time. */ } mbedtls_x509_time; diff --git a/include/mbedtls/x509_crl.h b/include/mbedtls/x509_crl.h index 9331827bb..f65e9847b 100644 --- a/include/mbedtls/x509_crl.h +++ b/include/mbedtls/x509_crl.h @@ -43,16 +43,25 @@ extern "C" { /** * Certificate revocation list entry. * Contains the CA-specific serial numbers and revocation dates. + * + * Some fields of this structure are publicly readable. Do not modify + * them except via Mbed TLS library functions: the effect of modifying + * those fields or the data that those fields points to is unspecified. */ typedef struct mbedtls_x509_crl_entry { - mbedtls_x509_buf MBEDTLS_PRIVATE(raw); - - mbedtls_x509_buf MBEDTLS_PRIVATE(serial); - - mbedtls_x509_time MBEDTLS_PRIVATE(revocation_date); - - mbedtls_x509_buf MBEDTLS_PRIVATE(entry_ext); + /** Direct access to the whole entry inside the containing buffer. */ + mbedtls_x509_buf raw; + /** The serial number of the revoked certificate. */ + mbedtls_x509_buf serial; + /** The revocation date of this entry. */ + mbedtls_x509_time revocation_date; + /** Direct access to the list of CRL entry extensions + * (an ASN.1 constructed sequence). + * + * If there are no extensions, `entry_ext.len == 0` and + * `entry_ext.p == NULL`. */ + mbedtls_x509_buf entry_ext; struct mbedtls_x509_crl_entry *MBEDTLS_PRIVATE(next); } @@ -64,22 +73,22 @@ mbedtls_x509_crl_entry; */ typedef struct mbedtls_x509_crl { - mbedtls_x509_buf MBEDTLS_PRIVATE(raw); /**< The raw certificate data (DER). */ - mbedtls_x509_buf MBEDTLS_PRIVATE(tbs); /**< The raw certificate body (DER). The part that is To Be Signed. */ + mbedtls_x509_buf raw; /**< The raw certificate data (DER). */ + mbedtls_x509_buf tbs; /**< The raw certificate body (DER). The part that is To Be Signed. */ - int MBEDTLS_PRIVATE(version); /**< CRL version (1=v1, 2=v2) */ - mbedtls_x509_buf MBEDTLS_PRIVATE(sig_oid); /**< CRL signature type identifier */ + int version; /**< CRL version (1=v1, 2=v2) */ + mbedtls_x509_buf sig_oid; /**< CRL signature type identifier */ - mbedtls_x509_buf MBEDTLS_PRIVATE(issuer_raw); /**< The raw issuer data (DER). */ + mbedtls_x509_buf issuer_raw; /**< The raw issuer data (DER). */ - mbedtls_x509_name MBEDTLS_PRIVATE(issuer); /**< The parsed issuer data (named information object). */ + mbedtls_x509_name issuer; /**< The parsed issuer data (named information object). */ - mbedtls_x509_time MBEDTLS_PRIVATE(this_update); - mbedtls_x509_time MBEDTLS_PRIVATE(next_update); + mbedtls_x509_time this_update; + mbedtls_x509_time next_update; - mbedtls_x509_crl_entry MBEDTLS_PRIVATE(entry); /**< The CRL entries containing the certificate revocation times for this CA. */ + mbedtls_x509_crl_entry entry; /**< The CRL entries containing the certificate revocation times for this CA. */ - mbedtls_x509_buf MBEDTLS_PRIVATE(crl_ext); + mbedtls_x509_buf crl_ext; mbedtls_x509_buf MBEDTLS_PRIVATE(sig_oid2); mbedtls_x509_buf MBEDTLS_PRIVATE(sig); diff --git a/include/mbedtls/x509_crt.h b/include/mbedtls/x509_crt.h index 49211a948..6731100f2 100644 --- a/include/mbedtls/x509_crt.h +++ b/include/mbedtls/x509_crt.h @@ -45,36 +45,40 @@ extern "C" { /** * Container for an X.509 certificate. The certificate may be chained. + * + * Some fields of this structure are publicly readable. Do not modify + * them except via Mbed TLS library functions: the effect of modifying + * those fields or the data that those fields points to is unspecified. */ typedef struct mbedtls_x509_crt { int MBEDTLS_PRIVATE(own_buffer); /**< Indicates if \c raw is owned * by the structure or not. */ - mbedtls_x509_buf MBEDTLS_PRIVATE(raw); /**< The raw certificate data (DER). */ - mbedtls_x509_buf MBEDTLS_PRIVATE(tbs); /**< The raw certificate body (DER). The part that is To Be Signed. */ + mbedtls_x509_buf raw; /**< The raw certificate data (DER). */ + mbedtls_x509_buf tbs; /**< The raw certificate body (DER). The part that is To Be Signed. */ - int MBEDTLS_PRIVATE(version); /**< The X.509 version. (1=v1, 2=v2, 3=v3) */ - mbedtls_x509_buf MBEDTLS_PRIVATE(serial); /**< Unique id for certificate issued by a specific CA. */ - mbedtls_x509_buf MBEDTLS_PRIVATE(sig_oid); /**< Signature algorithm, e.g. sha1RSA */ + int version; /**< The X.509 version. (1=v1, 2=v2, 3=v3) */ + mbedtls_x509_buf serial; /**< Unique id for certificate issued by a specific CA. */ + mbedtls_x509_buf sig_oid; /**< Signature algorithm, e.g. sha1RSA */ - mbedtls_x509_buf MBEDTLS_PRIVATE(issuer_raw); /**< The raw issuer data (DER). Used for quick comparison. */ - mbedtls_x509_buf MBEDTLS_PRIVATE(subject_raw); /**< The raw subject data (DER). Used for quick comparison. */ + mbedtls_x509_buf issuer_raw; /**< The raw issuer data (DER). Used for quick comparison. */ + mbedtls_x509_buf subject_raw; /**< The raw subject data (DER). Used for quick comparison. */ - mbedtls_x509_name MBEDTLS_PRIVATE(issuer); /**< The parsed issuer data (named information object). */ - mbedtls_x509_name MBEDTLS_PRIVATE(subject); /**< The parsed subject data (named information object). */ + mbedtls_x509_name issuer; /**< The parsed issuer data (named information object). */ + mbedtls_x509_name subject; /**< The parsed subject data (named information object). */ - mbedtls_x509_time MBEDTLS_PRIVATE(valid_from); /**< Start time of certificate validity. */ - mbedtls_x509_time MBEDTLS_PRIVATE(valid_to); /**< End time of certificate validity. */ + mbedtls_x509_time valid_from; /**< Start time of certificate validity. */ + mbedtls_x509_time valid_to; /**< End time of certificate validity. */ - mbedtls_x509_buf MBEDTLS_PRIVATE(pk_raw); - mbedtls_pk_context MBEDTLS_PRIVATE(pk); /**< Container for the public key context. */ + mbedtls_x509_buf pk_raw; + mbedtls_pk_context pk; /**< Container for the public key context. */ - mbedtls_x509_buf MBEDTLS_PRIVATE(issuer_id); /**< Optional X.509 v2/v3 issuer unique identifier. */ - mbedtls_x509_buf MBEDTLS_PRIVATE(subject_id); /**< Optional X.509 v2/v3 subject unique identifier. */ - mbedtls_x509_buf MBEDTLS_PRIVATE(v3_ext); /**< Optional X.509 v3 extensions. */ - mbedtls_x509_sequence MBEDTLS_PRIVATE(subject_alt_names); /**< Optional list of raw entries of Subject Alternative Names extension (currently only dNSName and OtherName are listed). */ + mbedtls_x509_buf issuer_id; /**< Optional X.509 v2/v3 issuer unique identifier. */ + mbedtls_x509_buf subject_id; /**< Optional X.509 v2/v3 subject unique identifier. */ + mbedtls_x509_buf v3_ext; /**< Optional X.509 v3 extensions. */ + mbedtls_x509_sequence subject_alt_names; /**< Optional list of raw entries of Subject Alternative Names extension (currently only dNSName and OtherName are listed). */ - mbedtls_x509_sequence MBEDTLS_PRIVATE(certificate_policies); /**< Optional list of certificate policies (Only anyPolicy is printed and enforced, however the rest of the policies are still listed). */ + mbedtls_x509_sequence certificate_policies; /**< Optional list of certificate policies (Only anyPolicy is printed and enforced, however the rest of the policies are still listed). */ int MBEDTLS_PRIVATE(ext_types); /**< Bit string containing detected and parsed extensions */ int MBEDTLS_PRIVATE(ca_istrue); /**< Optional Basic Constraint extension value: 1 if this certificate belongs to a CA, 0 otherwise. */ @@ -82,7 +86,7 @@ typedef struct mbedtls_x509_crt unsigned int MBEDTLS_PRIVATE(key_usage); /**< Optional key usage extension value: See the values in x509.h */ - mbedtls_x509_sequence MBEDTLS_PRIVATE(ext_key_usage); /**< Optional list of extended key usage OIDs. */ + mbedtls_x509_sequence ext_key_usage; /**< Optional list of extended key usage OIDs. */ unsigned char MBEDTLS_PRIVATE(ns_cert_type); /**< Optional Netscape certificate type extension value: See the values in x509.h */ @@ -100,6 +104,9 @@ mbedtls_x509_crt; * OtherName ::= SEQUENCE { * type-id OBJECT IDENTIFIER, * value [0] EXPLICIT ANY DEFINED BY type-id } + * + * Future versions of the library may add new fields to this structure or + * to its embedded union and structure. */ typedef struct mbedtls_x509_san_other_name { @@ -108,7 +115,7 @@ typedef struct mbedtls_x509_san_other_name * To check the value of the type id, you should use * \p MBEDTLS_OID_CMP with a known OID mbedtls_x509_buf. */ - mbedtls_x509_buf MBEDTLS_PRIVATE(type_id); /**< The type id. */ + mbedtls_x509_buf type_id; /**< The type id. */ union { /** @@ -119,26 +126,30 @@ typedef struct mbedtls_x509_san_other_name */ struct { - mbedtls_x509_buf MBEDTLS_PRIVATE(oid); /**< The object identifier. */ - mbedtls_x509_buf MBEDTLS_PRIVATE(val); /**< The named value. */ + mbedtls_x509_buf oid; /**< The object identifier. */ + mbedtls_x509_buf val; /**< The named value. */ } - MBEDTLS_PRIVATE(hardware_module_name); + hardware_module_name; } - MBEDTLS_PRIVATE(value); + value; } mbedtls_x509_san_other_name; /** - * A structure for holding the parsed Subject Alternative Name, according to type + * A structure for holding the parsed Subject Alternative Name, + * according to type. + * + * Future versions of the library may add new fields to this structure or + * to its embedded union and structure. */ typedef struct mbedtls_x509_subject_alternative_name { - int MBEDTLS_PRIVATE(type); /**< The SAN type, value of MBEDTLS_X509_SAN_XXX. */ + int type; /**< The SAN type, value of MBEDTLS_X509_SAN_XXX. */ union { - mbedtls_x509_san_other_name MBEDTLS_PRIVATE(other_name); /**< The otherName supported type. */ - mbedtls_x509_buf MBEDTLS_PRIVATE(unstructured_name); /**< The buffer for the un constructed types. Only dnsName currently supported */ + mbedtls_x509_san_other_name other_name; /**< The otherName supported type. */ + mbedtls_x509_buf unstructured_name; /**< The buffer for the un constructed types. Only dnsName currently supported */ } - MBEDTLS_PRIVATE(san); /**< A union of the supported SAN types */ + san; /**< A union of the supported SAN types */ } mbedtls_x509_subject_alternative_name; diff --git a/include/mbedtls/x509_csr.h b/include/mbedtls/x509_csr.h index 674f9ce79..5d1ce0e41 100644 --- a/include/mbedtls/x509_csr.h +++ b/include/mbedtls/x509_csr.h @@ -42,20 +42,24 @@ extern "C" { /** * Certificate Signing Request (CSR) structure. + * + * Some fields of this structure are publicly readable. Do not modify + * them except via Mbed TLS library functions: the effect of modifying + * those fields or the data that those fields points to is unspecified. */ typedef struct mbedtls_x509_csr { - mbedtls_x509_buf MBEDTLS_PRIVATE(raw); /**< The raw CSR data (DER). */ - mbedtls_x509_buf MBEDTLS_PRIVATE(cri); /**< The raw CertificateRequestInfo body (DER). */ + mbedtls_x509_buf raw; /**< The raw CSR data (DER). */ + mbedtls_x509_buf cri; /**< The raw CertificateRequestInfo body (DER). */ - int MBEDTLS_PRIVATE(version); /**< CSR version (1=v1). */ + int version; /**< CSR version (1=v1). */ - mbedtls_x509_buf MBEDTLS_PRIVATE(subject_raw); /**< The raw subject data (DER). */ - mbedtls_x509_name MBEDTLS_PRIVATE(subject); /**< The parsed subject data (named information object). */ + mbedtls_x509_buf subject_raw; /**< The raw subject data (DER). */ + mbedtls_x509_name subject; /**< The parsed subject data (named information object). */ - mbedtls_pk_context MBEDTLS_PRIVATE(pk); /**< Container for the public key context. */ + mbedtls_pk_context pk; /**< Container for the public key context. */ - mbedtls_x509_buf MBEDTLS_PRIVATE(sig_oid); + mbedtls_x509_buf sig_oid; mbedtls_x509_buf MBEDTLS_PRIVATE(sig); mbedtls_md_type_t MBEDTLS_PRIVATE(sig_md); /**< Internal representation of the MD algorithm of the signature algorithm, e.g. MBEDTLS_MD_SHA256 */ mbedtls_pk_type_t MBEDTLS_PRIVATE(sig_pk); /**< Internal representation of the Public Key algorithm of the signature algorithm, e.g. MBEDTLS_PK_RSA */ diff --git a/programs/x509/cert_write.c b/programs/x509/cert_write.c index 9a20d6341..763f8684f 100644 --- a/programs/x509/cert_write.c +++ b/programs/x509/cert_write.c @@ -514,7 +514,7 @@ int main( int argc, char *argv[] ) } ret = mbedtls_x509_dn_gets( issuer_name, sizeof(issuer_name), - &issuer_crt.MBEDTLS_PRIVATE(subject) ); + &issuer_crt.subject ); if( ret < 0 ) { mbedtls_strerror( ret, buf, 1024 ); @@ -548,7 +548,7 @@ int main( int argc, char *argv[] ) } ret = mbedtls_x509_dn_gets( subject_name, sizeof(subject_name), - &csr.MBEDTLS_PRIVATE(subject) ); + &csr.subject ); if( ret < 0 ) { mbedtls_strerror( ret, buf, 1024 ); @@ -558,7 +558,7 @@ int main( int argc, char *argv[] ) } opt.subject_name = subject_name; - subject_key = &csr.MBEDTLS_PRIVATE(pk); + subject_key = &csr.pk; mbedtls_printf( " ok\n" ); } @@ -602,7 +602,7 @@ int main( int argc, char *argv[] ) // if( strlen( opt.issuer_crt ) ) { - if( mbedtls_pk_check_pair( &issuer_crt.MBEDTLS_PRIVATE(pk), issuer_key, + if( mbedtls_pk_check_pair( &issuer_crt.pk, issuer_key, mbedtls_ctr_drbg_random, &ctr_drbg ) != 0 ) { mbedtls_printf( " failed\n ! issuer_key does not match " From 44ffc79d298c172e9bf2000acec9edc0ba0e95f6 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 31 Aug 2021 22:59:35 +0200 Subject: [PATCH 09/14] Copyediting in comments Signed-off-by: Gilles Peskine --- include/mbedtls/asn1.h | 10 +++++----- include/mbedtls/x509_csr.h | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/include/mbedtls/asn1.h b/include/mbedtls/asn1.h index 34a39d9eb..4746c1cb4 100644 --- a/include/mbedtls/asn1.h +++ b/include/mbedtls/asn1.h @@ -178,9 +178,9 @@ typedef struct mbedtls_asn1_sequence /** The next entry in the sequence. * - * The details memory management for sequences are not documented and + * The details of memory management for sequences are not documented and * may change in future versions. Set this field to \p NULL when - * initializing a structure, and do not modify it except via Mbed TLS + * initializing a structure, and do not modify it except via Mbed TLS * library functions. */ struct mbedtls_asn1_sequence *next; @@ -197,9 +197,9 @@ typedef struct mbedtls_asn1_named_data /** The next entry in the sequence. * - * The details memory management for named data sequences are not documented - * and may change in future versions. Set this field to \p NULL when - * initializing a structure, and do not modify it except via Mbed TLS + * The details of memory management for named data sequences are not + * documented and may change in future versions. Set this field to \p NULL + * when initializing a structure, and do not modify it except via Mbed TLS * library functions. */ struct mbedtls_asn1_named_data *next; diff --git a/include/mbedtls/x509_csr.h b/include/mbedtls/x509_csr.h index 5d1ce0e41..f80a1a130 100644 --- a/include/mbedtls/x509_csr.h +++ b/include/mbedtls/x509_csr.h @@ -45,7 +45,7 @@ extern "C" { * * Some fields of this structure are publicly readable. Do not modify * them except via Mbed TLS library functions: the effect of modifying - * those fields or the data that those fields points to is unspecified. + * those fields or the data that those fields point to is unspecified. */ typedef struct mbedtls_x509_csr { From 2e9d65f928be9287e5e2c5a443beaa845982b8bb Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 31 Aug 2021 23:05:19 +0200 Subject: [PATCH 10/14] Note that custom info structures are not supported This was already documented for mbedtls_md_info_t. Also document it for mbedtls_pk_info_t (where it's fairly obvious since the structure is not defined in a public header) and for mbedtls_cipher_info_t (where it's not obvious since the structure is defined in a public header). Signed-off-by: Gilles Peskine --- include/mbedtls/cipher.h | 7 +++++++ include/mbedtls/pk.h | 4 ++++ 2 files changed, 11 insertions(+) diff --git a/include/mbedtls/cipher.h b/include/mbedtls/cipher.h index 7921f4d85..b4630f63c 100644 --- a/include/mbedtls/cipher.h +++ b/include/mbedtls/cipher.h @@ -258,6 +258,13 @@ typedef struct mbedtls_cmac_context_t mbedtls_cmac_context_t; /** * Cipher information. Allows calling cipher functions * in a generic way. + * + * \note The library does not support custom cipher info structures, + * only built-in structures returned by the functions + * mbedtls_cipher_info_from_string(), + * mbedtls_cipher_info_from_type(), + * mbedtls_cipher_info_from_values(), + * mbedtls_cipher_info_from_psa(). */ typedef struct mbedtls_cipher_info_t { diff --git a/include/mbedtls/pk.h b/include/mbedtls/pk.h index ded52225f..5f9f29ff6 100644 --- a/include/mbedtls/pk.h +++ b/include/mbedtls/pk.h @@ -186,6 +186,10 @@ typedef struct mbedtls_pk_debug_item /** * \brief Public key information and operations + * + * \note The library does not support custom pk info structures, + * only built-in structures returned by + * mbedtls_cipher_info_from_type(). */ typedef struct mbedtls_pk_info_t mbedtls_pk_info_t; From ca939959e4d33174176c81900a4a8fb343f357b7 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 31 Aug 2021 23:18:07 +0200 Subject: [PATCH 11/14] Allow read-only access to lists of certificates, CRL, CRL entries Signed-off-by: Gilles Peskine --- include/mbedtls/x509_crl.h | 10 ++++++++-- include/mbedtls/x509_crt.h | 5 ++++- programs/ssl/dtls_server.c | 2 +- programs/ssl/ssl_fork_server.c | 2 +- programs/ssl/ssl_server.c | 2 +- programs/x509/cert_app.c | 2 +- 6 files changed, 16 insertions(+), 7 deletions(-) diff --git a/include/mbedtls/x509_crl.h b/include/mbedtls/x509_crl.h index f65e9847b..52bd43cd3 100644 --- a/include/mbedtls/x509_crl.h +++ b/include/mbedtls/x509_crl.h @@ -63,7 +63,10 @@ typedef struct mbedtls_x509_crl_entry * `entry_ext.p == NULL`. */ mbedtls_x509_buf entry_ext; - struct mbedtls_x509_crl_entry *MBEDTLS_PRIVATE(next); + /** Next element in the linked list of entries. + * \p NULL indicates the end of the list. + * Do not modify this field directly. */ + struct mbedtls_x509_crl_entry *next; } mbedtls_x509_crl_entry; @@ -96,7 +99,10 @@ typedef struct mbedtls_x509_crl mbedtls_pk_type_t MBEDTLS_PRIVATE(sig_pk); /**< Internal representation of the Public Key algorithm of the signature algorithm, e.g. MBEDTLS_PK_RSA */ void *MBEDTLS_PRIVATE(sig_opts); /**< Signature options to be passed to mbedtls_pk_verify_ext(), e.g. for RSASSA-PSS */ - struct mbedtls_x509_crl *MBEDTLS_PRIVATE(next); + /** Next element in the linked list of CRL. + * \p NULL indicates the end of the list. + * Do not modify this field directly. */ + struct mbedtls_x509_crl *next; } mbedtls_x509_crl; diff --git a/include/mbedtls/x509_crt.h b/include/mbedtls/x509_crt.h index 6731100f2..3c11a9989 100644 --- a/include/mbedtls/x509_crt.h +++ b/include/mbedtls/x509_crt.h @@ -95,7 +95,10 @@ typedef struct mbedtls_x509_crt mbedtls_pk_type_t MBEDTLS_PRIVATE(sig_pk); /**< Internal representation of the Public Key algorithm of the signature algorithm, e.g. MBEDTLS_PK_RSA */ void *MBEDTLS_PRIVATE(sig_opts); /**< Signature options to be passed to mbedtls_pk_verify_ext(), e.g. for RSASSA-PSS */ - struct mbedtls_x509_crt *MBEDTLS_PRIVATE(next); /**< Next certificate in the CA-chain. */ + /** Next certificate in the linked list that constitutes the CA chain. + * \p NULL indicates the end of the list. + * Do not modify this field directly. */ + struct mbedtls_x509_crt *next; } mbedtls_x509_crt; diff --git a/programs/ssl/dtls_server.c b/programs/ssl/dtls_server.c index f2570490f..5d1cccbe6 100644 --- a/programs/ssl/dtls_server.c +++ b/programs/ssl/dtls_server.c @@ -226,7 +226,7 @@ int main( void ) mbedtls_ssl_cache_set ); #endif - mbedtls_ssl_conf_ca_chain( &conf, srvcert.MBEDTLS_PRIVATE(next), NULL ); + mbedtls_ssl_conf_ca_chain( &conf, srvcert.next, NULL ); if( ( ret = mbedtls_ssl_conf_own_cert( &conf, &srvcert, &pkey ) ) != 0 ) { printf( " failed\n ! mbedtls_ssl_conf_own_cert returned %d\n\n", ret ); diff --git a/programs/ssl/ssl_fork_server.c b/programs/ssl/ssl_fork_server.c index 542a33460..694fc3b7a 100644 --- a/programs/ssl/ssl_fork_server.c +++ b/programs/ssl/ssl_fork_server.c @@ -190,7 +190,7 @@ int main( void ) mbedtls_ssl_conf_rng( &conf, mbedtls_ctr_drbg_random, &ctr_drbg ); mbedtls_ssl_conf_dbg( &conf, my_debug, stdout ); - mbedtls_ssl_conf_ca_chain( &conf, srvcert.MBEDTLS_PRIVATE(next), NULL ); + mbedtls_ssl_conf_ca_chain( &conf, srvcert.next, NULL ); if( ( ret = mbedtls_ssl_conf_own_cert( &conf, &srvcert, &pkey ) ) != 0 ) { mbedtls_printf( " failed! mbedtls_ssl_conf_own_cert returned %d\n\n", ret ); diff --git a/programs/ssl/ssl_server.c b/programs/ssl/ssl_server.c index ace657ceb..95557fb05 100644 --- a/programs/ssl/ssl_server.c +++ b/programs/ssl/ssl_server.c @@ -212,7 +212,7 @@ int main( void ) mbedtls_ssl_cache_set ); #endif - mbedtls_ssl_conf_ca_chain( &conf, srvcert.MBEDTLS_PRIVATE(next), NULL ); + mbedtls_ssl_conf_ca_chain( &conf, srvcert.next, NULL ); if( ( ret = mbedtls_ssl_conf_own_cert( &conf, &srvcert, &pkey ) ) != 0 ) { mbedtls_printf( " failed\n ! mbedtls_ssl_conf_own_cert returned %d\n\n", ret ); diff --git a/programs/x509/cert_app.c b/programs/x509/cert_app.c index aab15db2b..3d8f37b64 100644 --- a/programs/x509/cert_app.c +++ b/programs/x509/cert_app.c @@ -331,7 +331,7 @@ int main( int argc, char *argv[] ) mbedtls_printf( "%s\n", buf ); - cur = cur->MBEDTLS_PRIVATE(next); + cur = cur->next; } /* From 6ac8f94a72cb75071c79797908c4927b37e2f85a Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 1 Sep 2021 08:31:49 +0200 Subject: [PATCH 12/14] Fix cipher info key length sanity checks Most supported ciphers have a 128-bit, 192-bit or 256-bit keys. List the exceptions explicitly. This commit fixes a test failure with the null cipher and an incorrect comment that omitted several key lengths. Signed-off-by: Gilles Peskine --- tests/suites/test_suite_cipher.function | 31 +++++++++++++++++++++---- 1 file changed, 26 insertions(+), 5 deletions(-) diff --git a/tests/suites/test_suite_cipher.function b/tests/suites/test_suite_cipher.function index 94ea88f79..c809d9a28 100644 --- a/tests/suites/test_suite_cipher.function +++ b/tests/suites/test_suite_cipher.function @@ -33,11 +33,32 @@ static int check_cipher_info( mbedtls_cipher_type_t type, TEST_ASSERT( mbedtls_cipher_info_from_string( info->name ) == info ); key_bitlen = mbedtls_cipher_info_get_key_bitlen( info ); - TEST_ASSERT( key_bitlen % 8 == 0 ); - /* All current and plausible supported ciphers use a 64-bit, 128-bit - * or 256-bit key, except XTS which uses a double AES key. */ - TEST_ASSERT( key_bitlen >= 64 ); - TEST_ASSERT( key_bitlen <= 512 ); + if( info->type == MBEDTLS_CIPHER_NULL ) + TEST_ASSERT( key_bitlen == 0 ); + else if( info->mode == MBEDTLS_MODE_XTS ) + { + TEST_ASSERT( key_bitlen == 256 || + key_bitlen == 384 || + key_bitlen == 512 ); + } + else if( ! strncmp( info->name, "DES-EDE3-", 9 ) ) + { + TEST_ASSERT( key_bitlen == 192 ); + } + else if( ! strncmp( info->name, "DES-EDE-", 8 ) ) + { + TEST_ASSERT( key_bitlen == 128 ); + } + else if( ! strncmp( info->name, "DES-", 4 ) ) + { + TEST_ASSERT( key_bitlen == 64 ); + } + else + { + TEST_ASSERT( key_bitlen == 128 || + key_bitlen == 192 || + key_bitlen == 256 ); + } return( 1 ); From 88d681ca35a11644a4c492c8a6d8cfd90e0ef391 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 1 Sep 2021 11:19:33 +0200 Subject: [PATCH 13/14] Make size_t -> int downcasts explicit mbedtls_cipher_setkey takes an int argument. Cast explicitly, otherwise MSVC complains. Where possible, just stick to size_t. Signed-off-by: Gilles Peskine --- library/ssl_tls.c | 6 +++--- programs/aes/crypt_and_hash.c | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 1e81384aa..754c76f80 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -689,7 +689,7 @@ static int ssl_tls12_populate_transform( mbedtls_ssl_transform *transform, unsigned char *mac_dec; size_t mac_key_len = 0; size_t iv_copy_len; - unsigned keylen; + size_t keylen; const mbedtls_ssl_ciphersuite_t *ciphersuite_info; const mbedtls_cipher_info_t *cipher_info; const mbedtls_md_info_t *md_info; @@ -1060,7 +1060,7 @@ static int ssl_tls12_populate_transform( mbedtls_ssl_transform *transform, } if( ( ret = mbedtls_cipher_setkey( &transform->cipher_ctx_enc, key1, - mbedtls_cipher_info_get_key_bitlen( cipher_info ), + (int) mbedtls_cipher_info_get_key_bitlen( cipher_info ), MBEDTLS_ENCRYPT ) ) != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_cipher_setkey", ret ); @@ -1068,7 +1068,7 @@ static int ssl_tls12_populate_transform( mbedtls_ssl_transform *transform, } if( ( ret = mbedtls_cipher_setkey( &transform->cipher_ctx_dec, key2, - mbedtls_cipher_info_get_key_bitlen( cipher_info ), + (int) mbedtls_cipher_info_get_key_bitlen( cipher_info ), MBEDTLS_DECRYPT ) ) != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_cipher_setkey", ret ); diff --git a/programs/aes/crypt_and_hash.c b/programs/aes/crypt_and_hash.c index ba9827f45..5ed2ece70 100644 --- a/programs/aes/crypt_and_hash.c +++ b/programs/aes/crypt_and_hash.c @@ -311,7 +311,7 @@ int main( int argc, char *argv[] ) if( mbedtls_cipher_setkey( &cipher_ctx, digest, - mbedtls_cipher_info_get_key_bitlen( cipher_info ), + (int) mbedtls_cipher_info_get_key_bitlen( cipher_info ), MBEDTLS_ENCRYPT ) != 0 ) { mbedtls_fprintf( stderr, "mbedtls_cipher_setkey() returned error\n"); @@ -452,7 +452,7 @@ int main( int argc, char *argv[] ) if( mbedtls_cipher_setkey( &cipher_ctx, digest, - mbedtls_cipher_info_get_key_bitlen( cipher_info ), + (int) mbedtls_cipher_info_get_key_bitlen( cipher_info ), MBEDTLS_DECRYPT ) != 0 ) { mbedtls_fprintf( stderr, "mbedtls_cipher_setkey() returned error\n" ); From 5b8618b44cc272e4f851612f1143c038c894a40d Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 28 Sep 2021 12:34:53 +0200 Subject: [PATCH 14/14] fixup: Make the fields of mbedtls_ecp_curve_info public Remove more places where MBEDTLS_PRIVATE() was used on grp_id, which is now public. Signed-off-by: Gilles Peskine --- include/mbedtls/psa_util.h | 2 +- programs/pkey/ecdsa.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/include/mbedtls/psa_util.h b/include/mbedtls/psa_util.h index f6f2e5805..6f6354591 100644 --- a/include/mbedtls/psa_util.h +++ b/include/mbedtls/psa_util.h @@ -369,7 +369,7 @@ static inline psa_key_type_t mbedtls_psa_parse_tls_ecc_group( if( curve_info == NULL ) return( 0 ); return( PSA_KEY_TYPE_ECC_KEY_PAIR( - mbedtls_ecc_group_to_psa( curve_info->MBEDTLS_PRIVATE(grp_id), bits ) ) ); + mbedtls_ecc_group_to_psa( curve_info->grp_id, bits ) ) ); } #endif /* MBEDTLS_ECP_C */ diff --git a/programs/pkey/ecdsa.c b/programs/pkey/ecdsa.c index 6b6e9517d..550a230e8 100644 --- a/programs/pkey/ecdsa.c +++ b/programs/pkey/ecdsa.c @@ -51,7 +51,7 @@ #define ECPARAMS MBEDTLS_ECP_DP_SECP192R1 #if !defined(ECPARAMS) -#define ECPARAMS mbedtls_ecp_curve_list()->MBEDTLS_PRIVATE(grp_id) +#define ECPARAMS mbedtls_ecp_curve_list()->grp_id #endif #if !defined(MBEDTLS_ECDSA_C) || !defined(MBEDTLS_SHA256_C) || \