From b3e6872c9381ed4ce020d631dda1e0126c42b64f Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Fri, 28 Apr 2017 17:08:27 +0100 Subject: [PATCH 1/4] Introduce macros and functions to characterize certain ciphersuites. The routine `mbedtls_ssl_write_server_key_exchange` heavily depends on what kind of cipher suite is active: some don't need a ServerKeyExchange at all, some need (EC)DH parameters but no server signature, some require both. Each time we want to restrict a certain piece of code to some class of ciphersuites, it is guarded by a lengthy concatentation of configuration checks determining whether at least one of the relevant cipher suites is enabled in the config; on the code level, it is guarded by the check whether one of these cipher suites is the active one. To ease readability of the code, this commit introduces several helper macros and helper functions that can be used to determine whether a certain class of ciphersuites (a) is active in the config, and (b) contains the currently present ciphersuite. --- include/mbedtls/ssl_ciphersuites.h | 199 ++++++++++++++++++++++++++++- include/mbedtls/ssl_internal.h | 4 +- library/ssl_ciphersuites.c | 35 ----- library/ssl_cli.c | 43 ++----- library/ssl_srv.c | 83 ++++++------ 5 files changed, 245 insertions(+), 119 deletions(-) diff --git a/include/mbedtls/ssl_ciphersuites.h b/include/mbedtls/ssl_ciphersuites.h index 55fae0b26..290050990 100644 --- a/include/mbedtls/ssl_ciphersuites.h +++ b/include/mbedtls/ssl_ciphersuites.h @@ -257,6 +257,46 @@ typedef enum { #define MBEDTLS_KEY_EXCHANGE__WITH_CERT__ENABLED #endif +/* Key exchanges allowing client certificate requests */ +#if defined(MBEDTLS_KEY_EXCHANGE_RSA_ENABLED) || \ + defined(MBEDTLS_KEY_EXCHANGE_DHE_RSA_ENABLED) || \ + defined(MBEDTLS_KEY_EXCHANGE_ECDH_RSA_ENABLED) || \ + defined(MBEDTLS_KEY_EXCHANGE_ECDHE_RSA_ENABLED) || \ + defined(MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA_ENABLED) || \ + defined(MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED) +#define MBEDTLS_KEY_EXCHANGE__CERT_REQ_ALLOWED__ENABLED +#endif + +/* Key exchanges involving server signature in ServerKeyExchange */ +#if defined(MBEDTLS_KEY_EXCHANGE_DHE_RSA_ENABLED) || \ + defined(MBEDTLS_KEY_EXCHANGE_ECDHE_RSA_ENABLED) || \ + defined(MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED) +#define MBEDTLS_KEY_EXCHANGE__WITH_SERVER_SIGNATURE__ENABLED +#endif + +/* Key exchanges using ECDH */ +#if defined(MBEDTLS_KEY_EXCHANGE_ECDH_RSA_ENABLED) || \ + defined(MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA_ENABLED) +#define MBEDTLS_KEY_EXCHANGE__SOME__ECDH_ENABLED +#endif + +/* Key exchanges that don't involve ephemeral keys */ +#if defined(MBEDTLS_KEY_EXCHANGE_RSA_ENABLED) || \ + defined(MBEDTLS_KEY_EXCHANGE_PSK_ENABLED) || \ + defined(MBEDTLS_KEY_EXCHANGE_RSA_PSK_ENABLED) || \ + defined(MBEDTLS_KEY_EXCHANGE__SOME__ECDH_ENABLED) +#define MBEDTLS_KEY_EXCHANGE__SOME_NON_PFS__ENABLED +#endif + +/* Key exchanges that involve ephemeral keys */ +#if defined(MBEDTLS_KEY_EXCHANGE_DHE_RSA_ENABLED) || \ + defined(MBEDTLS_KEY_EXCHANGE_DHE_PSK_ENABLED) || \ + defined(MBEDTLS_KEY_EXCHANGE_ECDHE_RSA_ENABLED) || \ + defined(MBEDTLS_KEY_EXCHANGE_ECDHE_PSK_ENABLED) || \ + defined(MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED) +#define MBEDTLS_KEY_EXCHANGE__SOME_PFS__ENABLED +#endif + /* Key exchanges using a PSK */ #if defined(MBEDTLS_KEY_EXCHANGE_PSK_ENABLED) || \ defined(MBEDTLS_KEY_EXCHANGE_RSA_PSK_ENABLED) || \ @@ -265,7 +305,13 @@ typedef enum { #define MBEDTLS_KEY_EXCHANGE__SOME__PSK_ENABLED #endif -/* Key exchanges using a ECDHE */ +/* Key exchanges using DHE */ +#if defined(MBEDTLS_KEY_EXCHANGE_DHE_RSA_ENABLED) || \ + defined(MBEDTLS_KEY_EXCHANGE_DHE_PSK_ENABLED) +#define MBEDTLS_KEY_EXCHANGE__SOME__DHE_ENABLED +#endif + +/* Key exchanges using ECDHE */ #if defined(MBEDTLS_KEY_EXCHANGE_ECDHE_RSA_ENABLED) || \ defined(MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED) || \ defined(MBEDTLS_KEY_EXCHANGE_ECDHE_PSK_ENABLED) @@ -308,8 +354,155 @@ const mbedtls_ssl_ciphersuite_t *mbedtls_ssl_ciphersuite_from_id( int ciphersuit mbedtls_pk_type_t mbedtls_ssl_get_ciphersuite_sig_pk_alg( const mbedtls_ssl_ciphersuite_t *info ); #endif -int mbedtls_ssl_ciphersuite_uses_ec( const mbedtls_ssl_ciphersuite_t *info ); -int mbedtls_ssl_ciphersuite_uses_psk( const mbedtls_ssl_ciphersuite_t *info ); +#if defined(MBEDTLS_ECDH_C) || defined(MBEDTLS_ECDSA_C) +static inline int mbedtls_ssl_ciphersuite_uses_ec( const mbedtls_ssl_ciphersuite_t *info ) +{ + switch( info->key_exchange ) + { + case MBEDTLS_KEY_EXCHANGE_ECDHE_RSA: + case MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA: + case MBEDTLS_KEY_EXCHANGE_ECDHE_PSK: + case MBEDTLS_KEY_EXCHANGE_ECDH_RSA: + case MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA: + return( 1 ); + + default: + return( 0 ); + } +} +#endif /* MBEDTLS_ECDH_C || MBEDTLS_ECDSA_C */ + +#if defined(MBEDTLS_KEY_EXCHANGE__SOME_PFS__ENABLED) +static inline int mbedtls_ssl_ciphersuite_has_pfs( const mbedtls_ssl_ciphersuite_t *info ) +{ + switch( info->key_exchange ) + { + case MBEDTLS_KEY_EXCHANGE_DHE_RSA: + case MBEDTLS_KEY_EXCHANGE_DHE_PSK: + case MBEDTLS_KEY_EXCHANGE_ECDHE_RSA: + case MBEDTLS_KEY_EXCHANGE_ECDHE_PSK: + case MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA: + return( 1 ); + + default: + return( 0 ); + } +} +#endif /* MBEDTLS_KEY_EXCHANGE__SOME_PFS__ENABLED */ + +#if defined(MBEDTLS_KEY_EXCHANGE__SOME_NON_PFS__ENABLED) +static inline int mbedtls_ssl_ciphersuite_no_pfs( const mbedtls_ssl_ciphersuite_t *info ) +{ + switch( info->key_exchange ) + { + case MBEDTLS_KEY_EXCHANGE_ECDH_RSA: + case MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA: + case MBEDTLS_KEY_EXCHANGE_RSA: + case MBEDTLS_KEY_EXCHANGE_PSK: + case MBEDTLS_KEY_EXCHANGE_RSA_PSK: + return( 1 ); + + default: + return( 0 ); + } +} +#endif /* MBEDTLS_KEY_EXCHANGE__SOME_NON_PFS__ENABLED */ + +#if defined(MBEDTLS_KEY_EXCHANGE__SOME__ECDH_ENABLED) +static inline int mbedtls_ssl_ciphersuite_uses_ecdh( const mbedtls_ssl_ciphersuite_t *info ) +{ + switch( info->key_exchange ) + { + case MBEDTLS_KEY_EXCHANGE_ECDH_RSA: + case MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA: + return( 1 ); + + default: + return( 0 ); + } +} +#endif /* MBEDTLS_KEY_EXCHANGE__SOME__ECDH_ENABLED */ + +#if defined(MBEDTLS_KEY_EXCHANGE__SOME__PSK_ENABLED) +static inline int mbedtls_ssl_ciphersuite_uses_psk( const mbedtls_ssl_ciphersuite_t *info ) +{ + switch( info->key_exchange ) + { + case MBEDTLS_KEY_EXCHANGE_PSK: + case MBEDTLS_KEY_EXCHANGE_RSA_PSK: + case MBEDTLS_KEY_EXCHANGE_DHE_PSK: + case MBEDTLS_KEY_EXCHANGE_ECDHE_PSK: + return( 1 ); + + default: + return( 0 ); + } +} +#endif /* MBEDTLS_KEY_EXCHANGE__SOME__PSK_ENABLED */ + +static inline int mbedtls_ssl_ciphersuite_cert_req_allowed( const mbedtls_ssl_ciphersuite_t *info ) +{ + switch( info->key_exchange ) + { + case MBEDTLS_KEY_EXCHANGE_RSA: + case MBEDTLS_KEY_EXCHANGE_DHE_RSA: + case MBEDTLS_KEY_EXCHANGE_ECDH_RSA: + case MBEDTLS_KEY_EXCHANGE_ECDHE_RSA: + case MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA: + case MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA: + return( 1 ); + + default: + return( 0 ); + } +} + +#if defined(MBEDTLS_KEY_EXCHANGE__SOME__DHE_ENABLED) +static inline int mbedtls_ssl_ciphersuite_uses_dhe( const mbedtls_ssl_ciphersuite_t *info ) +{ + switch( info->key_exchange ) + { + case MBEDTLS_KEY_EXCHANGE_DHE_RSA: + case MBEDTLS_KEY_EXCHANGE_DHE_PSK: + return( 1 ); + + default: + return( 0 ); + } +} +#endif /* MBEDTLS_KEY_EXCHANGE__SOME__DHE_ENABLED) */ + +#if defined(MBEDTLS_KEY_EXCHANGE__SOME__ECDHE_ENABLED) +static inline int mbedtls_ssl_ciphersuite_uses_ecdhe( const mbedtls_ssl_ciphersuite_t *info ) +{ + switch( info->key_exchange ) + { + case MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA: + case MBEDTLS_KEY_EXCHANGE_ECDHE_RSA: + case MBEDTLS_KEY_EXCHANGE_ECDHE_PSK: + return( 1 ); + + default: + return( 0 ); + } +} +#endif /* MBEDTLS_KEY_EXCHANGE__SOME__ECDHE_ENABLED) */ + +#if defined(MBEDTLS_KEY_EXCHANGE__WITH_SERVER_SIGNATURE__ENABLED) +static inline int mbedtls_ssl_ciphersuite_uses_server_signature( const mbedtls_ssl_ciphersuite_t *info ) +{ + switch( info->key_exchange ) + { + case MBEDTLS_KEY_EXCHANGE_DHE_RSA: + case MBEDTLS_KEY_EXCHANGE_ECDHE_RSA: + case MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA: + return( 1 ); + + default: + return( 0 ); + } +} +#endif /* MBEDTLS_KEY_EXCHANGE__WITH_SERVER_SIGNATURE__ENABLED */ #ifdef __cplusplus } diff --git a/include/mbedtls/ssl_internal.h b/include/mbedtls/ssl_internal.h index 712053ed6..a168746a9 100644 --- a/include/mbedtls/ssl_internal.h +++ b/include/mbedtls/ssl_internal.h @@ -183,7 +183,7 @@ struct mbedtls_ssl_handshake_params mbedtls_ssl_key_cert *sni_key_cert; /*!< key/cert list from SNI */ mbedtls_x509_crt *sni_ca_chain; /*!< trusted CAs from SNI callback */ mbedtls_x509_crl *sni_ca_crl; /*!< trusted CAs CRLs from SNI */ -#endif +#endif /* MBEDTLS_SSL_SERVER_NAME_INDICATION */ #endif /* MBEDTLS_X509_CRT_PARSE_C */ #if defined(MBEDTLS_SSL_PROTO_DTLS) unsigned int out_msg_seq; /*!< Outgoing handshake sequence number */ @@ -206,7 +206,7 @@ struct mbedtls_ssl_handshake_params resending messages */ unsigned char alt_out_ctr[8]; /*!< Alternative record epoch/counter for resending messages */ -#endif +#endif /* MBEDTLS_SSL_PROTO_DTLS */ /* * Checksum contexts diff --git a/library/ssl_ciphersuites.c b/library/ssl_ciphersuites.c index 8bad7abd7..9ea9e614c 100644 --- a/library/ssl_ciphersuites.c +++ b/library/ssl_ciphersuites.c @@ -1799,39 +1799,4 @@ mbedtls_pk_type_t mbedtls_ssl_get_ciphersuite_sig_pk_alg( const mbedtls_ssl_ciph } #endif /* MBEDTLS_PK_C */ -#if defined(MBEDTLS_ECDH_C) || defined(MBEDTLS_ECDSA_C) -int mbedtls_ssl_ciphersuite_uses_ec( const mbedtls_ssl_ciphersuite_t *info ) -{ - switch( info->key_exchange ) - { - case MBEDTLS_KEY_EXCHANGE_ECDHE_RSA: - case MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA: - case MBEDTLS_KEY_EXCHANGE_ECDHE_PSK: - case MBEDTLS_KEY_EXCHANGE_ECDH_RSA: - case MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA: - return( 1 ); - - default: - return( 0 ); - } -} -#endif /* MBEDTLS_ECDH_C || MBEDTLS_ECDSA_C */ - -#if defined(MBEDTLS_KEY_EXCHANGE__SOME__PSK_ENABLED) -int mbedtls_ssl_ciphersuite_uses_psk( const mbedtls_ssl_ciphersuite_t *info ) -{ - switch( info->key_exchange ) - { - case MBEDTLS_KEY_EXCHANGE_PSK: - case MBEDTLS_KEY_EXCHANGE_RSA_PSK: - case MBEDTLS_KEY_EXCHANGE_DHE_PSK: - case MBEDTLS_KEY_EXCHANGE_ECDHE_PSK: - return( 1 ); - - default: - return( 0 ); - } -} -#endif /* MBEDTLS_KEY_EXCHANGE__SOME__PSK_ENABLED */ - #endif /* MBEDTLS_SSL_TLS_C */ diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 3d7283c3d..d0d0bd81c 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -2170,12 +2170,8 @@ static int ssl_parse_server_key_exchange( mbedtls_ssl_context *ssl ) return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); } -#if defined(MBEDTLS_KEY_EXCHANGE_DHE_RSA_ENABLED) || \ - defined(MBEDTLS_KEY_EXCHANGE_ECDHE_RSA_ENABLED) || \ - defined(MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED) - if( ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_DHE_RSA || - ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_ECDHE_RSA || - ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA ) +#if defined(MBEDTLS_KEY_EXCHANGE__WITH_SERVER_SIGNATURE__ENABLED) + if( mbedtls_ssl_ciphersuite_uses_server_signature( ciphersuite_info ) ) { size_t sig_len, hashlen; unsigned char hash[64]; @@ -2344,9 +2340,7 @@ static int ssl_parse_server_key_exchange( mbedtls_ssl_context *ssl ) return( ret ); } } -#endif /* MBEDTLS_KEY_EXCHANGE_DHE_RSA_ENABLED || - MBEDTLS_KEY_EXCHANGE_ECDHE_RSA_ENABLED || - MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED */ +#endif /* MBEDTLS_KEY_EXCHANGE__WITH_SERVER_SIGNATURE__ENABLED */ exit: ssl->state++; @@ -2356,22 +2350,14 @@ exit: return( 0 ); } -#if !defined(MBEDTLS_KEY_EXCHANGE_RSA_ENABLED) && \ - !defined(MBEDTLS_KEY_EXCHANGE_DHE_RSA_ENABLED) && \ - !defined(MBEDTLS_KEY_EXCHANGE_ECDH_RSA_ENABLED) && \ - !defined(MBEDTLS_KEY_EXCHANGE_ECDHE_RSA_ENABLED) && \ - !defined(MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA_ENABLED)&& \ - !defined(MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED) +#if !defined(MBEDTLS_KEY_EXCHANGE__CERT_REQ_ALLOWED__ENABLED) static int ssl_parse_certificate_request( mbedtls_ssl_context *ssl ) { const mbedtls_ssl_ciphersuite_t *ciphersuite_info = ssl->transform_negotiate->ciphersuite_info; MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> parse certificate request" ) ); - if( ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_PSK || - ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_RSA_PSK || - ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_DHE_PSK || - ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_ECDHE_PSK ) + if( ! mbedtls_ssl_ciphersuite_cert_req_allowed( ciphersuite_info ) ) { MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= skip parse certificate request" ) ); ssl->state++; @@ -2381,7 +2367,7 @@ static int ssl_parse_certificate_request( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_DEBUG_MSG( 1, ( "should never happen" ) ); return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); } -#else +#else /* ! MBEDTLS_KEY_EXCHANGE__CERT_REQ_ALLOWED__ENABLED */ static int ssl_parse_certificate_request( mbedtls_ssl_context *ssl ) { int ret; @@ -2392,10 +2378,7 @@ static int ssl_parse_certificate_request( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> parse certificate request" ) ); - if( ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_PSK || - ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_RSA_PSK || - ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_DHE_PSK || - ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_ECDHE_PSK ) + if( ! mbedtls_ssl_ciphersuite_cert_req_allowed( ciphersuite_info ) ) { MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= skip parse certificate request" ) ); ssl->state++; @@ -2522,12 +2505,7 @@ exit: return( 0 ); } -#endif /* !MBEDTLS_KEY_EXCHANGE_RSA_ENABLED && - !MBEDTLS_KEY_EXCHANGE_DHE_RSA_ENABLED && - !MBEDTLS_KEY_EXCHANGE_ECDH_RSA_ENABLED && - !MBEDTLS_KEY_EXCHANGE_ECDHE_RSA_ENABLED && - !MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA_ENABLED && - !MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED */ +#endif /* MBEDTLS_KEY_EXCHANGE__CERT_REQ_ALLOWED__ENABLED */ static int ssl_parse_server_hello_done( mbedtls_ssl_context *ssl ) { @@ -2661,10 +2639,7 @@ static int ssl_write_client_key_exchange( mbedtls_ssl_context *ssl ) MBEDTLS_KEY_EXCHANGE_ECDH_RSA_ENABLED || MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA_ENABLED */ #if defined(MBEDTLS_KEY_EXCHANGE__SOME__PSK_ENABLED) - if( ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_PSK || - ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_RSA_PSK || - ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_DHE_PSK || - ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_ECDHE_PSK ) + if( mbedtls_ssl_ciphersuite_uses_psk( ciphersuite_info ) ) { /* * opaque psk_identity<0..2^16-1>; diff --git a/library/ssl_srv.c b/library/ssl_srv.c index 6f800038e..94f28cb48 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -2563,52 +2563,45 @@ static int ssl_write_server_key_exchange( mbedtls_ssl_context *ssl ) const mbedtls_ssl_ciphersuite_t *ciphersuite_info = ssl->transform_negotiate->ciphersuite_info; -#if defined(MBEDTLS_KEY_EXCHANGE_DHE_RSA_ENABLED) || \ - defined(MBEDTLS_KEY_EXCHANGE_DHE_PSK_ENABLED) || \ - defined(MBEDTLS_KEY_EXCHANGE_ECDHE_RSA_ENABLED) || \ - defined(MBEDTLS_KEY_EXCHANGE_ECDHE_PSK_ENABLED) || \ - defined(MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED) +#if defined(MBEDTLS_KEY_EXCHANGE__SOME_PFS__ENABLED) unsigned char *p = ssl->out_msg + 4; unsigned char *dig_signed = p; size_t dig_signed_len = 0, len; ((void) dig_signed); ((void) dig_signed_len); -#endif + ((void) len); +#endif /* MBEDTLS_KEY_EXCHANGE__SOME_PFS__ENABLED) */ MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> write server key exchange" ) ); -#if defined(MBEDTLS_KEY_EXCHANGE_RSA_ENABLED) || \ - defined(MBEDTLS_KEY_EXCHANGE_PSK_ENABLED) || \ - defined(MBEDTLS_KEY_EXCHANGE_RSA_PSK_ENABLED) - if( ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_RSA || - ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_PSK || - ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_RSA_PSK ) - { - MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= skip write server key exchange" ) ); - ssl->state++; - return( 0 ); - } -#endif - -#if defined(MBEDTLS_KEY_EXCHANGE_ECDH_RSA_ENABLED) || \ - defined(MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA_ENABLED) - if( ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_ECDH_RSA || - ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA ) + /* For key exchanges involving ECDH, extract DH parameters from certificate here. */ +#if defined(MBEDTLS_KEY_EXCHANGE__SOME__ECDH_ENABLED) + if( mbedtls_ssl_ciphersuite_uses_ecdh( ciphersuite_info ) ) { ssl_get_ecdh_params_from_cert( ssl ); + } +#endif /* MBEDTLS_KEY_EXCHANGE__SOME__ECDH_ENABLED */ + /* Key exchanges not involving ephemeral keys don't use ServerKeyExchange, so end here. */ +#if defined(MBEDTLS_KEY_EXCHANGE__SOME_NON_PFS__ENABLED) + if( mbedtls_ssl_ciphersuite_no_pfs( ciphersuite_info ) ) + { MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= skip write server key exchange" ) ); ssl->state++; return( 0 ); } -#endif +#endif /* MBEDTLS_KEY_EXCHANGE__NON_PFS__ENABLED */ -#if defined(MBEDTLS_KEY_EXCHANGE_DHE_PSK_ENABLED) || \ + /* + * For (EC)DHE key exchanges with PSK, parameters are prefixed by support + * identity hint (RFC 4279, Sec. 3). Until someone needs this feature, + * we use empty support identity hints here. + **/ +#if defined(MBEDTLS_KEY_EXCHANGE_DHE_PSK_ENABLED) || \ defined(MBEDTLS_KEY_EXCHANGE_ECDHE_PSK_ENABLED) if( ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_DHE_PSK || ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_ECDHE_PSK ) { - /* TODO: Support identity hints */ *(p++) = 0x00; *(p++) = 0x00; @@ -2617,10 +2610,11 @@ static int ssl_write_server_key_exchange( mbedtls_ssl_context *ssl ) #endif /* MBEDTLS_KEY_EXCHANGE_DHE_PSK_ENABLED || MBEDTLS_KEY_EXCHANGE_ECDHE_PSK_ENABLED */ -#if defined(MBEDTLS_KEY_EXCHANGE_DHE_RSA_ENABLED) || \ - defined(MBEDTLS_KEY_EXCHANGE_DHE_PSK_ENABLED) - if( ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_DHE_RSA || - ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_DHE_PSK ) + /* + * For DHE key exchanges, add the DH parameters here. + */ +#if defined(MBEDTLS_KEY_EXCHANGE__SOME__DHE_ENABLED) + if( mbedtls_ssl_ciphersuite_uses_dhe( ciphersuite_info ) ) { if( ssl->conf->dhm_P.p == NULL || ssl->conf->dhm_G.p == NULL ) { @@ -2663,13 +2657,13 @@ static int ssl_write_server_key_exchange( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_DEBUG_MPI( 3, "DHM: G ", &ssl->handshake->dhm_ctx.G ); MBEDTLS_SSL_DEBUG_MPI( 3, "DHM: GX", &ssl->handshake->dhm_ctx.GX ); } -#endif /* MBEDTLS_KEY_EXCHANGE_DHE_RSA_ENABLED || - MBEDTLS_KEY_EXCHANGE_DHE_PSK_ENABLED */ +#endif /* MBEDTLS_KEY_EXCHANGE__SOME__DHE_ENABLED */ + /* + * For ECDHE key exchanges, add the ECDH parameters here. + */ #if defined(MBEDTLS_KEY_EXCHANGE__SOME__ECDHE_ENABLED) - if( ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_ECDHE_RSA || - ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA || - ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_ECDHE_PSK ) + if( mbedtls_ssl_ciphersuite_uses_ecdhe( ciphersuite_info ) ) { /* * Ephemeral ECDH parameters: @@ -2722,12 +2716,12 @@ curve_matching_done: } #endif /* MBEDTLS_KEY_EXCHANGE__SOME__ECDHE_ENABLED */ -#if defined(MBEDTLS_KEY_EXCHANGE_DHE_RSA_ENABLED) || \ - defined(MBEDTLS_KEY_EXCHANGE_ECDHE_RSA_ENABLED) || \ - defined(MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED) - if( ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_DHE_RSA || - ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_ECDHE_RSA || - ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA ) + /* + * For key exchanges involving the server signing the (EC)DH parameters, + * compute and add the signature here. + */ +#if defined(MBEDTLS_KEY_EXCHANGE__WITH_SERVER_SIGNATURE__ENABLED) + if( mbedtls_ssl_ciphersuite_uses_server_signature( ciphersuite_info ) ) { size_t signature_len = 0; unsigned int hashlen = 0; @@ -2758,7 +2752,8 @@ curve_matching_done: md_alg = MBEDTLS_MD_SHA1; } else -#endif +#endif /* MBEDTLS_SSL_PROTO_SSL3 || MBEDTLS_SSL_PROTO_TLS1 || \ + MBEDTLS_SSL_PROTO_TLS1_1 */ { md_alg = MBEDTLS_MD_NONE; } @@ -2884,9 +2879,7 @@ curve_matching_done: n += signature_len; } -#endif /* MBEDTLS_KEY_EXCHANGE_DHE_RSA_ENABLED) || - MBEDTLS_KEY_EXCHANGE_ECDHE_RSA_ENABLED || - MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED */ +#endif /* MBEDTLS_KEY_EXCHANGE__WITH_SERVER_SIGNATURE__ENABLED */ ssl->out_msglen = 4 + n; ssl->out_msgtype = MBEDTLS_SSL_MSG_HANDSHAKE; From aa8a2bd05a3e89132d14fa9cc063d5b2b5c9d868 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Fri, 28 Apr 2017 17:15:26 +0100 Subject: [PATCH 2/4] Remember suitable hash function for any signature algorithm. This commit changes `ssl_parse_signature_algorithms_ext` to remember one suitable ( := supported by client and by our config ) hash algorithm per signature algorithm. It also modifies the ciphersuite checking function `ssl_ciphersuite_match` to refuse a suite if there is no suitable hash algorithm. Finally, it adds the corresponding entry to the ChangeLog. --- ChangeLog | 6 + include/mbedtls/ssl.h | 1 + include/mbedtls/ssl_ciphersuites.h | 1 + include/mbedtls/ssl_internal.h | 47 ++++++- library/ssl_ciphersuites.c | 17 +++ library/ssl_srv.c | 193 ++++++++++++++++++++++++----- library/ssl_tls.c | 70 ++++++++++- 7 files changed, 305 insertions(+), 30 deletions(-) diff --git a/ChangeLog b/ChangeLog index fe5ce6535..38c097a9a 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,11 @@ mbed TLS ChangeLog (Sorted per branch, date) +mbed TLS 2.x.x branch released xxxx-xx-xx + +Bugfix + * Fix insufficient support for signature-hash-algorithm extension, + resulting in compatibility problems with Chrome. Found by hfloyrd. #823 + = mbed TLS 2.1.7 branch released 2017-03-08 Security diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 1f885ee14..5775f8a49 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -414,6 +414,7 @@ typedef struct mbedtls_ssl_config mbedtls_ssl_config; /* Defined in ssl_internal.h */ typedef struct mbedtls_ssl_transform mbedtls_ssl_transform; typedef struct mbedtls_ssl_handshake_params mbedtls_ssl_handshake_params; +typedef struct mbedtls_ssl_sig_hash_set_t mbedtls_ssl_sig_hash_set_t; #if defined(MBEDTLS_X509_CRT_PARSE_C) typedef struct mbedtls_ssl_key_cert mbedtls_ssl_key_cert; #endif diff --git a/include/mbedtls/ssl_ciphersuites.h b/include/mbedtls/ssl_ciphersuites.h index 290050990..b57c5d1f6 100644 --- a/include/mbedtls/ssl_ciphersuites.h +++ b/include/mbedtls/ssl_ciphersuites.h @@ -352,6 +352,7 @@ const mbedtls_ssl_ciphersuite_t *mbedtls_ssl_ciphersuite_from_id( int ciphersuit #if defined(MBEDTLS_PK_C) mbedtls_pk_type_t mbedtls_ssl_get_ciphersuite_sig_pk_alg( const mbedtls_ssl_ciphersuite_t *info ); +mbedtls_pk_type_t mbedtls_ssl_get_ciphersuite_sig_alg( const mbedtls_ssl_ciphersuite_t *info ); #endif #if defined(MBEDTLS_ECDH_C) || defined(MBEDTLS_ECDSA_C) diff --git a/include/mbedtls/ssl_internal.h b/include/mbedtls/ssl_internal.h index a168746a9..6b9334f7b 100644 --- a/include/mbedtls/ssl_internal.h +++ b/include/mbedtls/ssl_internal.h @@ -152,6 +152,24 @@ extern "C" { #endif +#if defined(MBEDTLS_SSL_PROTO_TLS1_2) && \ + defined(MBEDTLS_KEY_EXCHANGE__WITH_CERT__ENABLED) +/* + * Abstraction for a grid of allowed signature-hash-algorithm pairs. + */ +struct mbedtls_ssl_sig_hash_set_t +{ + /* At the moment, we only need to remember a single suitable + * hash algorithm per signature algorithm. As long as that's + * the case - and we don't need a general lookup function - + * we can implement the sig-hash-set as a map from signatures + * to hash algorithms. */ + mbedtls_md_type_t rsa; + mbedtls_md_type_t ecdsa; +}; +#endif /* MBEDTLS_SSL_PROTO_TLS1_2 && + MBEDTLS_KEY_EXCHANGE__WITH_CERT__ENABLED */ + /* * This structure contains the parameters only needed during handshake. */ @@ -160,9 +178,14 @@ struct mbedtls_ssl_handshake_params /* * Handshake specific crypto variables */ - int sig_alg; /*!< Hash algorithm for signature */ int cert_type; /*!< Requested cert type */ int verify_sig_alg; /*!< Signature algorithm for verify */ + +#if defined(MBEDTLS_SSL_PROTO_TLS1_2) && \ + defined(MBEDTLS_KEY_EXCHANGE__WITH_CERT__ENABLED) + mbedtls_ssl_sig_hash_set_t hash_algs; /*!< Set of suitable sig-hash pairs */ +#endif + #if defined(MBEDTLS_DHM_C) mbedtls_dhm_context dhm_ctx; /*!< DHM key exchange */ #endif @@ -317,6 +340,27 @@ struct mbedtls_ssl_flight_item }; #endif /* MBEDTLS_SSL_PROTO_DTLS */ +#if defined(MBEDTLS_SSL_PROTO_TLS1_2) && \ + defined(MBEDTLS_KEY_EXCHANGE__WITH_CERT__ENABLED) + +/* Find an entry in a signature-hash set matching a given hash algorithm. */ +mbedtls_md_type_t mbedtls_ssl_sig_hash_set_find( mbedtls_ssl_sig_hash_set_t *set, + mbedtls_pk_type_t sig_alg ); +/* Add a signature-hash-pair to a signature-hash set */ +void mbedtls_ssl_sig_hash_set_add( mbedtls_ssl_sig_hash_set_t *set, + mbedtls_pk_type_t sig_alg, + mbedtls_md_type_t md_alg ); +/* Allow exactly one hash algorithm for each signature. */ +void mbedtls_ssl_sig_hash_set_const_hash( mbedtls_ssl_sig_hash_set_t *set, + mbedtls_md_type_t md_alg ); +/* Setup an empty signature-hash set */ +static inline void mbedtls_ssl_sig_hash_set_init( mbedtls_ssl_sig_hash_set_t *set ) +{ + mbedtls_ssl_sig_hash_set_const_hash( set, MBEDTLS_MD_NONE ); +} + +#endif /* MBEDTLS_SSL_PROTO_TLS1_2 && + MBEDTLS_KEY_EXCHANGE__WITH_CERT__ENABLED */ /** * \brief Free referenced items in an SSL transform context and clear @@ -368,6 +412,7 @@ int mbedtls_ssl_psk_derive_premaster( mbedtls_ssl_context *ssl, mbedtls_key_exch #if defined(MBEDTLS_PK_C) unsigned char mbedtls_ssl_sig_from_pk( mbedtls_pk_context *pk ); mbedtls_pk_type_t mbedtls_ssl_pk_alg_from_sig( unsigned char sig ); +unsigned char mbedtls_ssl_sig_from_pk_alg( mbedtls_pk_type_t type ); #endif mbedtls_md_type_t mbedtls_ssl_md_alg_from_hash( unsigned char hash ); diff --git a/library/ssl_ciphersuites.c b/library/ssl_ciphersuites.c index 9ea9e614c..5e10ef809 100644 --- a/library/ssl_ciphersuites.c +++ b/library/ssl_ciphersuites.c @@ -1797,6 +1797,23 @@ mbedtls_pk_type_t mbedtls_ssl_get_ciphersuite_sig_pk_alg( const mbedtls_ssl_ciph return( MBEDTLS_PK_NONE ); } } + +mbedtls_pk_type_t mbedtls_ssl_get_ciphersuite_sig_alg( const mbedtls_ssl_ciphersuite_t *info ) +{ + switch( info->key_exchange ) + { + case MBEDTLS_KEY_EXCHANGE_RSA: + case MBEDTLS_KEY_EXCHANGE_DHE_RSA: + case MBEDTLS_KEY_EXCHANGE_ECDHE_RSA: + return( MBEDTLS_PK_RSA ); + + case MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA: + return( MBEDTLS_PK_ECDSA ); + + default: + return( MBEDTLS_PK_NONE ); + } +} #endif /* MBEDTLS_PK_C */ #endif /* MBEDTLS_SSL_TLS_C */ diff --git a/library/ssl_srv.c b/library/ssl_srv.c index 94f28cb48..fae07c060 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -186,15 +186,30 @@ static int ssl_parse_renegotiation_info( mbedtls_ssl_context *ssl, #if defined(MBEDTLS_SSL_PROTO_TLS1_2) && \ defined(MBEDTLS_KEY_EXCHANGE__WITH_CERT__ENABLED) + +/* + * Status of the implementation of signature-algorithms extension: + * + * Currently, we are only considering the signature-algorithm extension + * to pick a ciphersuite which allows us to send the ServerKeyExchange + * message with a signature-hash combination that the user allows. + * + * We do *not* check whether all certificates in our certificate + * chain are signed with an allowed signature-hash pair. + * This needs to be done at a later stage. + * + */ static int ssl_parse_signature_algorithms_ext( mbedtls_ssl_context *ssl, const unsigned char *buf, size_t len ) { size_t sig_alg_list_size; + const unsigned char *p; const unsigned char *end = buf + len; - const int *md_cur; + mbedtls_md_type_t md_cur; + mbedtls_pk_type_t sig_cur; sig_alg_list_size = ( ( buf[0] << 8 ) | ( buf[1] ) ); if( sig_alg_list_size + 2 != len || @@ -204,29 +219,48 @@ static int ssl_parse_signature_algorithms_ext( mbedtls_ssl_context *ssl, return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_HELLO ); } - /* - * For now, ignore the SignatureAlgorithm part and rely on offered - * ciphersuites only for that part. To be fixed later. + /* Currently we only guarantee signing the ServerKeyExchange message according + * to the constraints specified in this extension (see above), so it suffices + * to remember only one suitable hash for each possible signature algorithm. * - * So, just look at the HashAlgorithm part. + * This will change when we also consider certificate signatures, + * in which case we will need to remember the whole signature-hash + * pair list from the extension. */ - for( md_cur = ssl->conf->sig_hashes; *md_cur != MBEDTLS_MD_NONE; md_cur++ ) { - for( p = buf + 2; p < end; p += 2 ) { - if( *md_cur == (int) mbedtls_ssl_md_alg_from_hash( p[0] ) ) { - ssl->handshake->sig_alg = p[0]; - goto have_sig_alg; - } + + for( p = buf + 2; p < end; p += 2 ) + { + /* Silently ignore unknown signature or hash algorithms. */ + + if( (sig_cur = mbedtls_ssl_pk_alg_from_sig( p[1] ) ) == MBEDTLS_PK_NONE ) + { + MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello v3, signature_algorithm ext: unknown sig alg encoding %d", + p[1] ) ); + continue; + } + + /* Check if we support the hash the user proposes */ + md_cur = mbedtls_ssl_md_alg_from_hash( p[0] ); + if( md_cur == MBEDTLS_MD_NONE ) + { + MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello v3, signature_algorithm ext: unknown hash alg encoding %d", + p[0] ) ); + continue; + } + + if( mbedtls_ssl_check_sig_hash( ssl, md_cur ) == 0 ) + { + mbedtls_ssl_sig_hash_set_add( &ssl->handshake->hash_algs, sig_cur, md_cur ); + MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello v3, signature_algorithm ext: match sig %d and hash %d", + sig_cur, md_cur ) ); + } + else + { + MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello v3, signature_algorithm ext: hash alg %d not supported", + md_cur ) ); } } - /* Some key echanges do not need signatures at all */ - MBEDTLS_SSL_DEBUG_MSG( 3, ( "no signature_algorithm in common" ) ); - return( 0 ); - -have_sig_alg: - MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello v3, signature_algorithm ext: %d", - ssl->handshake->sig_alg ) ); - return( 0 ); } #endif /* MBEDTLS_SSL_PROTO_TLS1_2 && @@ -676,6 +710,11 @@ static int ssl_ciphersuite_match( mbedtls_ssl_context *ssl, int suite_id, { const mbedtls_ssl_ciphersuite_t *suite_info; +#if defined(MBEDTLS_SSL_PROTO_TLS1_2) && \ + defined(MBEDTLS_KEY_EXCHANGE__WITH_CERT__ENABLED) + mbedtls_pk_type_t sig_type; +#endif + suite_info = mbedtls_ssl_ciphersuite_from_id( suite_id ); if( suite_info == NULL ) { @@ -731,6 +770,25 @@ static int ssl_ciphersuite_match( mbedtls_ssl_context *ssl, int suite_id, } #endif +#if defined(MBEDTLS_SSL_PROTO_TLS1_2) && \ + defined(MBEDTLS_KEY_EXCHANGE__WITH_CERT__ENABLED) + /* If the ciphersuite requires signing, check whether + * a suitable hash algorithm is present. */ + if( ssl->minor_ver == MBEDTLS_SSL_MINOR_VERSION_3 ) + { + sig_type = mbedtls_ssl_get_ciphersuite_sig_alg( suite_info ); + if( sig_type != MBEDTLS_PK_NONE && + mbedtls_ssl_sig_hash_set_find( &ssl->handshake->hash_algs, sig_type ) == MBEDTLS_MD_NONE ) + { + MBEDTLS_SSL_DEBUG_MSG( 3, ( "ciphersuite mismatch: no suitable hash algorithm " + "for signature algorithm %d", sig_type ) ); + return( 0 ); + } + } + +#endif /* MBEDTLS_SSL_PROTO_TLS1_2 && + MBEDTLS_KEY_EXCHANGE__WITH_CERT__ENABLED */ + #if defined(MBEDTLS_X509_CRT_PARSE_C) /* * Final check: if ciphersuite requires us to have a @@ -1041,6 +1099,15 @@ static int ssl_parse_client_hello( mbedtls_ssl_context *ssl ) const mbedtls_ssl_ciphersuite_t *ciphersuite_info; int major, minor; + /* If there is no signature-algorithm extension present, + * we need to fall back to the default values for allowed + * signature-hash pairs. */ +#if defined(MBEDTLS_SSL_PROTO_TLS1_2) && \ + defined(MBEDTLS_KEY_EXCHANGE__WITH_CERT__ENABLED) + int sig_hash_alg_ext_present = 0; +#endif /* MBEDTLS_SSL_PROTO_TLS1_2 && + MBEDTLS_KEY_EXCHANGE__WITH_CERT__ENABLED */ + MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> parse client hello" ) ); #if defined(MBEDTLS_SSL_DTLS_ANTI_REPLAY) @@ -1538,10 +1605,11 @@ read_record_header: if( ssl->renego_status == MBEDTLS_SSL_RENEGOTIATION_IN_PROGRESS ) break; #endif - ret = ssl_parse_signature_algorithms_ext( ssl, ext + 4, ext_size ); if( ret != 0 ) return( ret ); + + sig_hash_alg_ext_present = 1; break; #endif /* MBEDTLS_SSL_PROTO_TLS1_2 && MBEDTLS_KEY_EXCHANGE__WITH_CERT__ENABLED */ @@ -1665,6 +1733,26 @@ read_record_header: } #endif /* MBEDTLS_SSL_FALLBACK_SCSV */ +#if defined(MBEDTLS_SSL_PROTO_TLS1_2) && \ + defined(MBEDTLS_KEY_EXCHANGE__WITH_CERT__ENABLED) + + /* + * Try to fall back to default hash SHA1 if the client + * hasn't provided any preferred signature-hash combinations. + */ + if( sig_hash_alg_ext_present == 0 ) + { + mbedtls_md_type_t md_default = MBEDTLS_MD_SHA1; + + if( mbedtls_ssl_check_sig_hash( ssl, md_default ) != 0 ) + md_default = MBEDTLS_MD_NONE; + + mbedtls_ssl_sig_hash_set_const_hash( &ssl->handshake->hash_algs, md_default ); + } + +#endif /* MBEDTLS_SSL_PROTO_TLS1_2 && + MBEDTLS_KEY_EXCHANGE__WITH_CERT__ENABLED */ + /* * Check for TLS_EMPTY_RENEGOTIATION_INFO_SCSV */ @@ -1791,6 +1879,28 @@ have_ciphersuite: mbedtls_ssl_recv_flight_completed( ssl ); #endif + /* Debugging-only output for testsuite */ +#if defined(MBEDTLS_DEBUG_C) && \ + defined(MBEDTLS_SSL_PROTO_TLS1_2) && \ + defined(MBEDTLS_KEY_EXCHANGE__WITH_CERT__ENABLED) + if( ssl->minor_ver == MBEDTLS_SSL_MINOR_VERSION_3 ) + { + mbedtls_pk_type_t sig_alg = mbedtls_ssl_get_ciphersuite_sig_pk_alg( ciphersuite_info ); + if( sig_alg != MBEDTLS_PK_NONE ) + { + mbedtls_md_type_t md_alg = mbedtls_ssl_sig_hash_set_find( &ssl->handshake->hash_algs, + sig_alg ); + MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello v3, signature_algorithm ext: %d", + mbedtls_ssl_hash_from_md_alg( md_alg ) ) ); + } + else + { + MBEDTLS_SSL_DEBUG_MSG( 3, ( "no hash algorithm for signature algorithm %d - should not happen", + sig_alg ) ); + } + } +#endif + MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= parse client hello" ) ); return( 0 ); @@ -2706,7 +2816,7 @@ curve_matching_done: return( ret ); } - dig_signed = p; + dig_signed = p; dig_signed_len = len; p += len; @@ -2726,19 +2836,29 @@ curve_matching_done: size_t signature_len = 0; unsigned int hashlen = 0; unsigned char hash[64]; - mbedtls_md_type_t md_alg = MBEDTLS_MD_NONE; /* - * Choose hash algorithm. NONE means MD5 + SHA1 here. + * Choose hash algorithm: + * - For TLS 1.2, obey signature-hash-algorithm extension to choose appropriate hash. + * - For SSL3, TLS1.0, TLS1.1 and ECDHE_ECDSA, use SHA1 (RFC 4492, Sec. 5.4) + * - Otherwise, use MD5 + SHA1 (RFC 4346, Sec. 7.4.3) */ + + mbedtls_md_type_t md_alg; + #if defined(MBEDTLS_SSL_PROTO_TLS1_2) + mbedtls_pk_type_t sig_alg = mbedtls_ssl_get_ciphersuite_sig_pk_alg( ciphersuite_info ); if( ssl->minor_ver == MBEDTLS_SSL_MINOR_VERSION_3 ) { - md_alg = mbedtls_ssl_md_alg_from_hash( ssl->handshake->sig_alg ); + /* For TLS 1.2, obey signature-hash-algorithm extension + * (RFC 5246, Sec. 7.4.1.4.1). */ - if( md_alg == MBEDTLS_MD_NONE ) + if( sig_alg == MBEDTLS_PK_NONE || + ( md_alg = mbedtls_ssl_sig_hash_set_find( &ssl->handshake->hash_algs, sig_alg ) ) == MBEDTLS_MD_NONE ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "should never happen" ) ); + /* (... because we choose a cipher suite + * only if there is a matching hash.) */ return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); } } @@ -2758,6 +2878,8 @@ curve_matching_done: md_alg = MBEDTLS_MD_NONE; } + MBEDTLS_SSL_DEBUG_MSG( 3, ( "pick hash algorithm %d for signing", md_alg ) ); + /* * Compute the hash to be signed */ @@ -2845,7 +2967,7 @@ curve_matching_done: (unsigned int) ( mbedtls_md_get_size( mbedtls_md_info_from_type( md_alg ) ) ) ); /* - * Make the signature + * Compute and add the signature */ if( mbedtls_ssl_own_key( ssl ) == NULL ) { @@ -2856,8 +2978,23 @@ curve_matching_done: #if defined(MBEDTLS_SSL_PROTO_TLS1_2) if( ssl->minor_ver == MBEDTLS_SSL_MINOR_VERSION_3 ) { - *(p++) = ssl->handshake->sig_alg; - *(p++) = mbedtls_ssl_sig_from_pk( mbedtls_ssl_own_key( ssl ) ); + /* For TLS 1.2, we need to specify signature and hash algorithm + * explicitly through a prefix to the signature. + * + * struct { + * HashAlgorithm hash; + * SignatureAlgorithm signature; + * } SignatureAndHashAlgorithm; + * + * struct { + * SignatureAndHashAlgorithm algorithm; + * opaque signature<0..2^16-1>; + * } DigitallySigned; + * + */ + + *(p++) = mbedtls_ssl_hash_from_md_alg( md_alg ); + *(p++) = mbedtls_ssl_sig_from_pk_alg( sig_alg ); n += 2; } diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 43efe0ce3..4023854d5 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -5185,7 +5185,11 @@ static void ssl_handshake_params_init( mbedtls_ssl_handshake_params *handshake ) #endif /* MBEDTLS_SSL_PROTO_TLS1_2 */ handshake->update_checksum = ssl_update_checksum_start; - handshake->sig_alg = MBEDTLS_SSL_HASH_SHA1; + +#if defined(MBEDTLS_SSL_PROTO_TLS1_2) && \ + defined(MBEDTLS_KEY_EXCHANGE__WITH_CERT__ENABLED) + mbedtls_ssl_sig_hash_set_init( &handshake->hash_algs ); +#endif #if defined(MBEDTLS_DHM_C) mbedtls_dhm_init( &handshake->dhm_ctx ); @@ -7263,6 +7267,19 @@ unsigned char mbedtls_ssl_sig_from_pk( mbedtls_pk_context *pk ) return( MBEDTLS_SSL_SIG_ANON ); } +unsigned char mbedtls_ssl_sig_from_pk_alg( mbedtls_pk_type_t type ) +{ + switch( type ) { + case MBEDTLS_PK_RSA: + return( MBEDTLS_SSL_SIG_RSA ); + case MBEDTLS_PK_ECDSA: + case MBEDTLS_PK_ECKEY: + return( MBEDTLS_SSL_SIG_ECDSA ); + default: + return( MBEDTLS_SSL_SIG_ANON ); + } +} + mbedtls_pk_type_t mbedtls_ssl_pk_alg_from_sig( unsigned char sig ) { switch( sig ) @@ -7281,6 +7298,57 @@ mbedtls_pk_type_t mbedtls_ssl_pk_alg_from_sig( unsigned char sig ) } #endif /* MBEDTLS_PK_C */ +#if defined(MBEDTLS_SSL_PROTO_TLS1_2) && \ + defined(MBEDTLS_KEY_EXCHANGE__WITH_CERT__ENABLED) + +/* Find an entry in a signature-hash set matching a given hash algorithm. */ +mbedtls_md_type_t mbedtls_ssl_sig_hash_set_find( mbedtls_ssl_sig_hash_set_t *set, + mbedtls_pk_type_t sig_alg ) +{ + switch( sig_alg ) + { + case MBEDTLS_PK_RSA: + return( set->rsa ); + case MBEDTLS_PK_ECDSA: + return( set->ecdsa ); + default: + return( MBEDTLS_MD_NONE ); + } +} + +/* Add a signature-hash-pair to a signature-hash set */ +void mbedtls_ssl_sig_hash_set_add( mbedtls_ssl_sig_hash_set_t *set, + mbedtls_pk_type_t sig_alg, + mbedtls_md_type_t md_alg ) +{ + switch( sig_alg ) + { + case MBEDTLS_PK_RSA: + if( set->rsa == MBEDTLS_MD_NONE ) + set->rsa = md_alg; + break; + + case MBEDTLS_PK_ECDSA: + if( set->ecdsa == MBEDTLS_MD_NONE ) + set->ecdsa = md_alg; + break; + + default: + break; + } +} + +/* Allow exactly one hash algorithm for each signature. */ +void mbedtls_ssl_sig_hash_set_const_hash( mbedtls_ssl_sig_hash_set_t *set, + mbedtls_md_type_t md_alg ) +{ + set->rsa = md_alg; + set->ecdsa = md_alg; +} + +#endif /* MBEDTLS_SSL_PROTO_TLS1_2 && + MBEDTLS_KEY_EXCHANGE__WITH_CERT__ENABLED */ + /* * Convert from MBEDTLS_SSL_HASH_XXX to MBEDTLS_MD_XXX */ From 118848fd779649f959fe7bc016352ce1916a0ed2 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 1 May 2017 09:43:29 +0100 Subject: [PATCH 3/4] Split long lines. --- library/ssl_cli.c | 21 ++++++++++------ library/ssl_srv.c | 62 ++++++++++++++++++++++++++++------------------- 2 files changed, 51 insertions(+), 32 deletions(-) diff --git a/library/ssl_cli.c b/library/ssl_cli.c index d0d0bd81c..b281501b2 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -1845,7 +1845,8 @@ static int ssl_parse_server_psk_hint( mbedtls_ssl_context *ssl, if( (*p) + len > end ) { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad server key exchange message (psk_identity_hint length)" ) ); + MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad server key exchange message " + "(psk_identity_hint length)" ) ); return( MBEDTLS_ERR_SSL_BAD_HS_SERVER_KEY_EXCHANGE ); } @@ -2043,7 +2044,8 @@ static int ssl_get_ecdh_params_from_cert( mbedtls_ssl_context *ssl ) static int ssl_parse_server_key_exchange( mbedtls_ssl_context *ssl ) { int ret; - const mbedtls_ssl_ciphersuite_t *ciphersuite_info = ssl->transform_negotiate->ciphersuite_info; + const mbedtls_ssl_ciphersuite_t *ciphersuite_info = + ssl->transform_negotiate->ciphersuite_info; unsigned char *p, *end; MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> parse server key exchange" ) ); @@ -2353,7 +2355,8 @@ exit: #if !defined(MBEDTLS_KEY_EXCHANGE__CERT_REQ_ALLOWED__ENABLED) static int ssl_parse_certificate_request( mbedtls_ssl_context *ssl ) { - const mbedtls_ssl_ciphersuite_t *ciphersuite_info = ssl->transform_negotiate->ciphersuite_info; + const mbedtls_ssl_ciphersuite_t *ciphersuite_info = + ssl->transform_negotiate->ciphersuite_info; MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> parse certificate request" ) ); @@ -2374,7 +2377,8 @@ static int ssl_parse_certificate_request( mbedtls_ssl_context *ssl ) unsigned char *buf, *p; size_t n = 0, m = 0; size_t cert_type_len = 0, dn_len = 0; - const mbedtls_ssl_ciphersuite_t *ciphersuite_info = ssl->transform_negotiate->ciphersuite_info; + const mbedtls_ssl_ciphersuite_t *ciphersuite_info = + ssl->transform_negotiate->ciphersuite_info; MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> parse certificate request" ) ); @@ -2552,7 +2556,8 @@ static int ssl_write_client_key_exchange( mbedtls_ssl_context *ssl ) { int ret; size_t i, n; - const mbedtls_ssl_ciphersuite_t *ciphersuite_info = ssl->transform_negotiate->ciphersuite_info; + const mbedtls_ssl_ciphersuite_t *ciphersuite_info = + ssl->transform_negotiate->ciphersuite_info; MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> write client key exchange" ) ); @@ -2784,7 +2789,8 @@ static int ssl_write_client_key_exchange( mbedtls_ssl_context *ssl ) !defined(MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED) static int ssl_write_certificate_verify( mbedtls_ssl_context *ssl ) { - const mbedtls_ssl_ciphersuite_t *ciphersuite_info = ssl->transform_negotiate->ciphersuite_info; + const mbedtls_ssl_ciphersuite_t *ciphersuite_info = + ssl->transform_negotiate->ciphersuite_info; int ret; MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> write certificate verify" ) ); @@ -2812,7 +2818,8 @@ static int ssl_write_certificate_verify( mbedtls_ssl_context *ssl ) static int ssl_write_certificate_verify( mbedtls_ssl_context *ssl ) { int ret = MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE; - const mbedtls_ssl_ciphersuite_t *ciphersuite_info = ssl->transform_negotiate->ciphersuite_info; + const mbedtls_ssl_ciphersuite_t *ciphersuite_info = + ssl->transform_negotiate->ciphersuite_info; size_t n = 0, offset = 0; unsigned char hash[48]; unsigned char *hash_start = hash; diff --git a/library/ssl_srv.c b/library/ssl_srv.c index fae07c060..09a68d622 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -234,8 +234,8 @@ static int ssl_parse_signature_algorithms_ext( mbedtls_ssl_context *ssl, if( (sig_cur = mbedtls_ssl_pk_alg_from_sig( p[1] ) ) == MBEDTLS_PK_NONE ) { - MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello v3, signature_algorithm ext: unknown sig alg encoding %d", - p[1] ) ); + MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello v3, signature_algorithm ext" + " unknown sig alg encoding %d", p[1] ) ); continue; } @@ -243,21 +243,22 @@ static int ssl_parse_signature_algorithms_ext( mbedtls_ssl_context *ssl, md_cur = mbedtls_ssl_md_alg_from_hash( p[0] ); if( md_cur == MBEDTLS_MD_NONE ) { - MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello v3, signature_algorithm ext: unknown hash alg encoding %d", - p[0] ) ); + MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello v3, signature_algorithm ext:" + " unknown hash alg encoding %d", p[0] ) ); continue; } if( mbedtls_ssl_check_sig_hash( ssl, md_cur ) == 0 ) { mbedtls_ssl_sig_hash_set_add( &ssl->handshake->hash_algs, sig_cur, md_cur ); - MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello v3, signature_algorithm ext: match sig %d and hash %d", + MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello v3, signature_algorithm ext:" + " match sig %d and hash %d", sig_cur, md_cur ) ); } else { - MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello v3, signature_algorithm ext: hash alg %d not supported", - md_cur ) ); + MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello v3, signature_algorithm ext: " + "hash alg %d not supported", md_cur ) ); } } @@ -607,7 +608,8 @@ static int ssl_pick_cert( mbedtls_ssl_context *ssl, const mbedtls_ssl_ciphersuite_t * ciphersuite_info ) { mbedtls_ssl_key_cert *cur, *list, *fallback = NULL; - mbedtls_pk_type_t pk_alg = mbedtls_ssl_get_ciphersuite_sig_pk_alg( ciphersuite_info ); + mbedtls_pk_type_t pk_alg = + mbedtls_ssl_get_ciphersuite_sig_pk_alg( ciphersuite_info ); uint32_t flags; #if defined(MBEDTLS_SSL_SERVER_NAME_INDICATION) @@ -1227,7 +1229,8 @@ read_record_header: return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_HELLO ); } - if( ( ret = mbedtls_ssl_fetch_input( ssl, mbedtls_ssl_hdr_len( ssl ) + msg_len ) ) != 0 ) + if( ( ret = mbedtls_ssl_fetch_input( ssl, + mbedtls_ssl_hdr_len( ssl ) + msg_len ) ) != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_fetch_input", ret ); return( ret ); @@ -1764,7 +1767,8 @@ read_record_header: #if defined(MBEDTLS_SSL_RENEGOTIATION) if( ssl->renego_status == MBEDTLS_SSL_RENEGOTIATION_IN_PROGRESS ) { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "received RENEGOTIATION SCSV during renegotiation" ) ); + MBEDTLS_SSL_DEBUG_MSG( 1, ( "received RENEGOTIATION SCSV " + "during renegotiation" ) ); if( ( ret = mbedtls_ssl_send_fatal_handshake_failure( ssl ) ) != 0 ) return( ret ); @@ -1895,8 +1899,8 @@ have_ciphersuite: } else { - MBEDTLS_SSL_DEBUG_MSG( 3, ( "no hash algorithm for signature algorithm %d - should not happen", - sig_alg ) ); + MBEDTLS_SSL_DEBUG_MSG( 3, ( "no hash algorithm for signature algorithm " + "%d - should not happen", sig_alg ) ); } } #endif @@ -2455,7 +2459,8 @@ static int ssl_write_server_hello( mbedtls_ssl_context *ssl ) !defined(MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED) static int ssl_write_certificate_request( mbedtls_ssl_context *ssl ) { - const mbedtls_ssl_ciphersuite_t *ciphersuite_info = ssl->transform_negotiate->ciphersuite_info; + const mbedtls_ssl_ciphersuite_t *ciphersuite_info = + ssl->transform_negotiate->ciphersuite_info; MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> write certificate request" ) ); @@ -2476,7 +2481,8 @@ static int ssl_write_certificate_request( mbedtls_ssl_context *ssl ) static int ssl_write_certificate_request( mbedtls_ssl_context *ssl ) { int ret = MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE; - const mbedtls_ssl_ciphersuite_t *ciphersuite_info = ssl->transform_negotiate->ciphersuite_info; + const mbedtls_ssl_ciphersuite_t *ciphersuite_info = + ssl->transform_negotiate->ciphersuite_info; size_t dn_size, total_dn_size; /* excluding length bytes */ size_t ct_len, sa_len; /* including length bytes */ unsigned char *buf, *p; @@ -2692,7 +2698,8 @@ static int ssl_write_server_key_exchange( mbedtls_ssl_context *ssl ) } #endif /* MBEDTLS_KEY_EXCHANGE__SOME__ECDH_ENABLED */ - /* Key exchanges not involving ephemeral keys don't use ServerKeyExchange, so end here. */ + /* Key exchanges not involving ephemeral keys don't use + * ServerKeyExchange, so end here. */ #if defined(MBEDTLS_KEY_EXCHANGE__SOME_NON_PFS__ENABLED) if( mbedtls_ssl_ciphersuite_no_pfs( ciphersuite_info ) ) { @@ -2838,23 +2845,27 @@ curve_matching_done: unsigned char hash[64]; /* - * Choose hash algorithm: - * - For TLS 1.2, obey signature-hash-algorithm extension to choose appropriate hash. - * - For SSL3, TLS1.0, TLS1.1 and ECDHE_ECDSA, use SHA1 (RFC 4492, Sec. 5.4) - * - Otherwise, use MD5 + SHA1 (RFC 4346, Sec. 7.4.3) + * 3.1: Choose hash algorithm: + * A: For TLS 1.2, obey signature-hash-algorithm extension + * to choose appropriate hash. + * B: For SSL3, TLS1.0, TLS1.1 and ECDHE_ECDSA, use SHA1 + * (RFC 4492, Sec. 5.4) + * C: Otherwise, use MD5 + SHA1 (RFC 4346, Sec. 7.4.3) */ mbedtls_md_type_t md_alg; #if defined(MBEDTLS_SSL_PROTO_TLS1_2) - mbedtls_pk_type_t sig_alg = mbedtls_ssl_get_ciphersuite_sig_pk_alg( ciphersuite_info ); + mbedtls_pk_type_t sig_alg = + mbedtls_ssl_get_ciphersuite_sig_pk_alg( ciphersuite_info ); if( ssl->minor_ver == MBEDTLS_SSL_MINOR_VERSION_3 ) { /* For TLS 1.2, obey signature-hash-algorithm extension * (RFC 5246, Sec. 7.4.1.4.1). */ if( sig_alg == MBEDTLS_PK_NONE || - ( md_alg = mbedtls_ssl_sig_hash_set_find( &ssl->handshake->hash_algs, sig_alg ) ) == MBEDTLS_MD_NONE ) + ( md_alg = mbedtls_ssl_sig_hash_set_find( &ssl->handshake->hash_algs, + sig_alg ) ) == MBEDTLS_MD_NONE ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "should never happen" ) ); /* (... because we choose a cipher suite @@ -3001,8 +3012,7 @@ curve_matching_done: #endif /* MBEDTLS_SSL_PROTO_TLS1_2 */ if( ( ret = mbedtls_pk_sign( mbedtls_ssl_own_key( ssl ), md_alg, hash, hashlen, - p + 2 , &signature_len, - ssl->conf->f_rng, ssl->conf->p_rng ) ) != 0 ) + p + 2 , &signature_len, ssl->conf->f_rng, ssl->conf->p_rng ) ) != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_pk_sign", ret ); return( ret ); @@ -3510,7 +3520,8 @@ static int ssl_parse_client_key_exchange( mbedtls_ssl_context *ssl ) !defined(MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED) static int ssl_parse_certificate_verify( mbedtls_ssl_context *ssl ) { - const mbedtls_ssl_ciphersuite_t *ciphersuite_info = ssl->transform_negotiate->ciphersuite_info; + const mbedtls_ssl_ciphersuite_t *ciphersuite_info = + ssl->transform_negotiate->ciphersuite_info; MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> parse certificate verify" ) ); @@ -3539,7 +3550,8 @@ static int ssl_parse_certificate_verify( mbedtls_ssl_context *ssl ) mbedtls_pk_type_t pk_alg; #endif mbedtls_md_type_t md_alg; - const mbedtls_ssl_ciphersuite_t *ciphersuite_info = ssl->transform_negotiate->ciphersuite_info; + const mbedtls_ssl_ciphersuite_t *ciphersuite_info = + ssl->transform_negotiate->ciphersuite_info; MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> parse certificate verify" ) ); From 032aec05232156871f9ba25744e506c859a5dbea Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 1 May 2017 09:44:27 +0100 Subject: [PATCH 4/4] Improve documentation --- library/ssl_srv.c | 60 ++++++++++++++++++++++++++++++----------------- 1 file changed, 39 insertions(+), 21 deletions(-) diff --git a/library/ssl_srv.c b/library/ssl_srv.c index 09a68d622..b1a0b4095 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -2681,16 +2681,24 @@ static int ssl_write_server_key_exchange( mbedtls_ssl_context *ssl ) #if defined(MBEDTLS_KEY_EXCHANGE__SOME_PFS__ENABLED) unsigned char *p = ssl->out_msg + 4; + size_t len; +#if defined(MBEDTLS_KEY_EXCHANGE__WITH_SERVER_SIGNATURE__ENABLED) unsigned char *dig_signed = p; - size_t dig_signed_len = 0, len; - ((void) dig_signed); - ((void) dig_signed_len); - ((void) len); -#endif /* MBEDTLS_KEY_EXCHANGE__SOME_PFS__ENABLED) */ + size_t dig_signed_len = 0; +#endif /* MBEDTLS_KEY_EXCHANGE__WITH_SERVER_SIGNATURE__ENABLED */ +#endif /* MBEDTLS_KEY_EXCHANGE__SOME_PFS__ENABLED */ MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> write server key exchange" ) ); - /* For key exchanges involving ECDH, extract DH parameters from certificate here. */ + /* + * + * Part 1: Extract static ECDH parameters and abort + * if ServerKeyExchange not needed. + * + */ + + /* For suites involving ECDH, extract DH parameters + * from certificate at this point. */ #if defined(MBEDTLS_KEY_EXCHANGE__SOME__ECDH_ENABLED) if( mbedtls_ssl_ciphersuite_uses_ecdh( ciphersuite_info ) ) { @@ -2728,7 +2736,7 @@ static int ssl_write_server_key_exchange( mbedtls_ssl_context *ssl ) MBEDTLS_KEY_EXCHANGE_ECDHE_PSK_ENABLED */ /* - * For DHE key exchanges, add the DH parameters here. + * - DHE key exchanges */ #if defined(MBEDTLS_KEY_EXCHANGE__SOME__DHE_ENABLED) if( mbedtls_ssl_ciphersuite_uses_dhe( ciphersuite_info ) ) @@ -2763,8 +2771,10 @@ static int ssl_write_server_key_exchange( mbedtls_ssl_context *ssl ) return( ret ); } +#if defined(MBEDTLS_KEY_EXCHANGE__WITH_SERVER_SIGNATURE__ENABLED) dig_signed = p; dig_signed_len = len; +#endif p += len; n += len; @@ -2777,7 +2787,7 @@ static int ssl_write_server_key_exchange( mbedtls_ssl_context *ssl ) #endif /* MBEDTLS_KEY_EXCHANGE__SOME__DHE_ENABLED */ /* - * For ECDHE key exchanges, add the ECDH parameters here. + * - ECDHE key exchanges */ #if defined(MBEDTLS_KEY_EXCHANGE__SOME__ECDHE_ENABLED) if( mbedtls_ssl_ciphersuite_uses_ecdhe( ciphersuite_info ) ) @@ -2822,9 +2832,11 @@ curve_matching_done: MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecdh_make_params", ret ); return( ret ); } - + +#if defined(MBEDTLS_KEY_EXCHANGE__WITH_SERVER_SIGNATURE__ENABLED) dig_signed = p; dig_signed_len = len; +#endif p += len; n += len; @@ -2834,8 +2846,10 @@ curve_matching_done: #endif /* MBEDTLS_KEY_EXCHANGE__SOME__ECDHE_ENABLED */ /* - * For key exchanges involving the server signing the (EC)DH parameters, - * compute and add the signature here. + * + * Part 3: For key exchanges involving the server signing the + * exchange parameters, compute and add the signature here. + * */ #if defined(MBEDTLS_KEY_EXCHANGE__WITH_SERVER_SIGNATURE__ENABLED) if( mbedtls_ssl_ciphersuite_uses_server_signature( ciphersuite_info ) ) @@ -2860,16 +2874,15 @@ curve_matching_done: mbedtls_ssl_get_ciphersuite_sig_pk_alg( ciphersuite_info ); if( ssl->minor_ver == MBEDTLS_SSL_MINOR_VERSION_3 ) { - /* For TLS 1.2, obey signature-hash-algorithm extension - * (RFC 5246, Sec. 7.4.1.4.1). */ - + /* A: For TLS 1.2, obey signature-hash-algorithm extension + * (RFC 5246, Sec. 7.4.1.4.1). */ if( sig_alg == MBEDTLS_PK_NONE || ( md_alg = mbedtls_ssl_sig_hash_set_find( &ssl->handshake->hash_algs, sig_alg ) ) == MBEDTLS_MD_NONE ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "should never happen" ) ); - /* (... because we choose a cipher suite - * only if there is a matching hash.) */ + /* (... because we choose a cipher suite + * only if there is a matching hash.) */ return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); } } @@ -2877,22 +2890,23 @@ curve_matching_done: #endif /* MBEDTLS_SSL_PROTO_TLS1_2 */ #if defined(MBEDTLS_SSL_PROTO_SSL3) || defined(MBEDTLS_SSL_PROTO_TLS1) || \ defined(MBEDTLS_SSL_PROTO_TLS1_1) - if( ciphersuite_info->key_exchange == - MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA ) + if( ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA ) { + /* B: Default hash SHA1 */ md_alg = MBEDTLS_MD_SHA1; } else #endif /* MBEDTLS_SSL_PROTO_SSL3 || MBEDTLS_SSL_PROTO_TLS1 || \ MBEDTLS_SSL_PROTO_TLS1_1 */ { + /* C: MD5 + SHA1 */ md_alg = MBEDTLS_MD_NONE; } MBEDTLS_SSL_DEBUG_MSG( 3, ( "pick hash algorithm %d for signing", md_alg ) ); /* - * Compute the hash to be signed + * 3.2: Compute the hash to be signed */ #if defined(MBEDTLS_SSL_PROTO_SSL3) || defined(MBEDTLS_SSL_PROTO_TLS1) || \ defined(MBEDTLS_SSL_PROTO_TLS1_1) @@ -2917,6 +2931,7 @@ curve_matching_done: * SHA(ClientHello.random + ServerHello.random * + ServerParams); */ + mbedtls_md5_starts( &mbedtls_md5 ); mbedtls_md5_update( &mbedtls_md5, ssl->handshake->randbytes, 64 ); mbedtls_md5_update( &mbedtls_md5, dig_signed, dig_signed_len ); @@ -2978,7 +2993,7 @@ curve_matching_done: (unsigned int) ( mbedtls_md_get_size( mbedtls_md_info_from_type( md_alg ) ) ) ); /* - * Compute and add the signature + * 3.3: Compute and add the signature */ if( mbedtls_ssl_own_key( ssl ) == NULL ) { @@ -2989,7 +3004,8 @@ curve_matching_done: #if defined(MBEDTLS_SSL_PROTO_TLS1_2) if( ssl->minor_ver == MBEDTLS_SSL_MINOR_VERSION_3 ) { - /* For TLS 1.2, we need to specify signature and hash algorithm + /* + * For TLS 1.2, we need to specify signature and hash algorithm * explicitly through a prefix to the signature. * * struct { @@ -3028,6 +3044,8 @@ curve_matching_done: } #endif /* MBEDTLS_KEY_EXCHANGE__WITH_SERVER_SIGNATURE__ENABLED */ + /* Done with actual work; add header and send. */ + ssl->out_msglen = 4 + n; ssl->out_msgtype = MBEDTLS_SSL_MSG_HANDSHAKE; ssl->out_msg[0] = MBEDTLS_SSL_HS_SERVER_KEY_EXCHANGE;