From a5d336bcec0a35c84b3fe4b177d4f46bf83bf87b Mon Sep 17 00:00:00 2001 From: Gergely Budai Date: Mon, 27 Jan 2014 23:27:06 +0100 Subject: [PATCH 01/13] Increase title size (fits to increased curve names). Give verbose errors on failures. --- programs/test/benchmark.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/programs/test/benchmark.c b/programs/test/benchmark.c index b1974fed1..56baa58fe 100644 --- a/programs/test/benchmark.c +++ b/programs/test/benchmark.c @@ -50,6 +50,7 @@ #include "polarssl/dhm.h" #include "polarssl/ecdsa.h" #include "polarssl/ecdh.h" +#include "polarssl/error.h" #if defined _MSC_VER && !defined snprintf #define snprintf _snprintf @@ -57,7 +58,7 @@ #define BUFSIZE 1024 #define HEADER_FORMAT " %-24s : " -#define TITLE_LEN 15 +#define TITLE_LEN 25 #if !defined(POLARSSL_TIMING_C) int main( int argc, char *argv[] ) @@ -132,7 +133,10 @@ do { \ } \ \ if( ret != 0 ) \ - printf( "FAILED\n" ); \ + { \ + polarssl_strerror( ret, ( char * )tmp, sizeof( tmp ) ); \ + printf( "FAILED: %s\n", tmp ); \ + } \ else \ printf( "%9lu " TYPE "/s\n", i / 3 ); \ } while( 0 ) From 987bfb510bb84c893a67b134564109cbe4615af5 Mon Sep 17 00:00:00 2001 From: Gergely Budai Date: Sun, 19 Jan 2014 21:48:42 +0100 Subject: [PATCH 02/13] Added the possibility to define the allowed curves for ECDHE handshake. It also defines the preference of the curves. --- include/polarssl/ssl.h | 22 +++++++++++++++++ library/ssl_srv.c | 44 +++++++++++++++++++++++++++------- library/ssl_tls.c | 54 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 112 insertions(+), 8 deletions(-) diff --git a/include/polarssl/ssl.h b/include/polarssl/ssl.h index 1bda2b395..2b50304aa 100644 --- a/include/polarssl/ssl.h +++ b/include/polarssl/ssl.h @@ -83,6 +83,12 @@ #define POLARSSL_KEY_EXCHANGE__SOME__PSK_ENABLED #endif +#if defined(POLARSSL_KEY_EXCHANGE_ECDHE_RSA_ENABLED) || \ + defined(POLARSSL_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED) || \ + defined(POLARSSL_KEY_EXCHANGE_ECDHE_PSK_ENABLED) +#define POLARSSL_KEY_EXCHANGE__SOME__ECDHE_ENABLED +#endif + #if defined(_MSC_VER) && !defined(inline) #define inline _inline #else @@ -721,6 +727,9 @@ struct _ssl_context int disable_renegotiation; /*!< enable/disable renegotiation */ int allow_legacy_renegotiation; /*!< allow legacy renegotiation */ const int *ciphersuite_list[4]; /*!< allowed ciphersuites / version */ +#if defined(POLARSSL_KEY_EXCHANGE__SOME__ECDHE_ENABLED) + const ecp_group_id *ecdh_curve_list;/*!< allowed curves for ECDH */ +#endif #if defined(POLARSSL_SSL_TRUNCATED_HMAC) int trunc_hmac; /*!< negotiate truncated hmac? */ #endif @@ -1149,6 +1158,19 @@ int ssl_set_dh_param( ssl_context *ssl, const char *dhm_P, const char *dhm_G ); int ssl_set_dh_param_ctx( ssl_context *ssl, dhm_context *dhm_ctx ); #endif +#if defined(POLARSSL_KEY_EXCHANGE__SOME__ECDHE_ENABLED) +/** + * \brief Set the allowed ECDH curves. + * + * The sequence of the curves in the list also determines the + * handshake curve preference. + * + * \param ssl SSL context + * \param ecdh_curve_list Zero terminated list of the allowed ECDH curves + */ +void ssl_set_ecdh_curves( ssl_context *ssl, const ecp_group_id *ecdh_curve_list ); +#endif + #if defined(POLARSSL_SSL_SERVER_NAME_INDICATION) /** * \brief Set hostname for ServerName TLS extension diff --git a/library/ssl_srv.c b/library/ssl_srv.c index 0abded139..dfae8c5a1 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -2092,10 +2092,7 @@ static int ssl_write_server_key_exchange( ssl_context *ssl ) #endif /* POLARSSL_KEY_EXCHANGE_DHE_RSA_ENABLED || POLARSSL_KEY_EXCHANGE_DHE_PSK_ENABLED */ -#if defined(POLARSSL_KEY_EXCHANGE_ECDHE_RSA_ENABLED) || \ - defined(POLARSSL_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED) || \ - defined(POLARSSL_KEY_EXCHANGE_ECDHE_PSK_ENABLED) - +#if defined(POLARSSL_KEY_EXCHANGE__SOME__ECDHE_ENABLED) if( ciphersuite_info->key_exchange == POLARSSL_KEY_EXCHANGE_ECDHE_RSA || ciphersuite_info->key_exchange == POLARSSL_KEY_EXCHANGE_ECDHE_ECDSA || ciphersuite_info->key_exchange == POLARSSL_KEY_EXCHANGE_ECDHE_PSK ) @@ -2108,8 +2105,41 @@ static int ssl_write_server_key_exchange( ssl_context *ssl ) * ECPoint public; * } ServerECDHParams; */ + + unsigned int pref_idx, curv_idx, found; + + /* Match our preference list against the agreed curves */ + for( pref_idx = 0, found = 0; + ssl->ecdh_curve_list[pref_idx] != POLARSSL_ECP_DP_NONE; + pref_idx++ ) + { + /* Look through the agreed curve list */ + for( curv_idx = 0; + ssl->handshake->curves[curv_idx] != NULL; + curv_idx++ ) + { + if (ssl->handshake->curves[curv_idx]->grp_id == + ssl->ecdh_curve_list[pref_idx] ) + { + /* We found our most preferred curve */ + found = 1; + break; + } + } + + /* Exit the search if we have found our curve */ + if( found == 1 ) + { + break; + } + } + /* If we haven't found any allowed / preferred curve, + * ssl->ecdh_curve_list[pref_idx] will contain POLARSSL_ECP_DP_NONE and + * ecp_use_known_dp() will fail. + */ + if( ( ret = ecp_use_known_dp( &ssl->handshake->ecdh_ctx.grp, - ssl->handshake->curves[0]->grp_id ) ) != 0 ) + ssl->ecdh_curve_list[pref_idx] ) ) != 0 ) { SSL_DEBUG_RET( 1, "ecp_use_known_dp", ret ); return( ret ); @@ -2134,9 +2164,7 @@ static int ssl_write_server_key_exchange( ssl_context *ssl ) SSL_DEBUG_ECP( 3, "ECDH: Q ", &ssl->handshake->ecdh_ctx.Q ); } -#endif /* POLARSSL_KEY_EXCHANGE_ECDHE_RSA_ENABLED || - POLARSSL_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED || - POLARSSL_KEY_EXCHANGE_ECDHE_PSK_ENABLED */ +#endif /* POLARSSL_KEY_EXCHANGE__SOME__ECDHE_ENABLED */ #if defined(POLARSSL_KEY_EXCHANGE_DHE_RSA_ENABLED) || \ defined(POLARSSL_KEY_EXCHANGE_ECDHE_RSA_ENABLED) || \ diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 36378ef1b..02f24a18f 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -3325,6 +3325,46 @@ static int ssl_handshake_init( ssl_context *ssl ) */ int ssl_init( ssl_context *ssl ) { + +#if defined(POLARSSL_KEY_EXCHANGE__SOME__ECDHE_ENABLED) + /* + * ECDHE allowed curves and preference list + * + * We start with the most secure curves. From the same size curves, we prefer + * the SECP ones because they are much faster. + * + * TODO: Add the Montgomery curves + */ + static const ecp_group_id ecdh_default_curve_list[] = + { +#if defined(POLARSSL_ECP_DP_SECP521R1_ENABLED) + POLARSSL_ECP_DP_SECP521R1, +#endif +#if defined(POLARSSL_ECP_DP_BP512R1_ENABLED) + POLARSSL_ECP_DP_BP512R1, +#endif +#if defined(POLARSSL_ECP_DP_SECP384R1_ENABLED) + POLARSSL_ECP_DP_SECP384R1, +#endif +#if defined(POLARSSL_ECP_DP_BP384R1_ENABLED) + POLARSSL_ECP_DP_BP384R1, +#endif +#if defined(POLARSSL_ECP_DP_SECP256R1_ENABLED) + POLARSSL_ECP_DP_SECP256R1, +#endif +#if defined(POLARSSL_ECP_DP_BP256R1_ENABLED) + POLARSSL_ECP_DP_BP256R1, +#endif +#if defined(POLARSSL_ECP_DP_SECP224R1_ENABLED) + POLARSSL_ECP_DP_SECP224R1, +#endif +#if defined(POLARSSL_ECP_DP_SECP192R1_ENABLED) + POLARSSL_ECP_DP_SECP192R1, +#endif + POLARSSL_ECP_DP_NONE + }; +#endif + int ret; int len = SSL_BUFFER_LEN; @@ -3384,6 +3424,10 @@ int ssl_init( ssl_context *ssl ) ssl->ticket_lifetime = SSL_DEFAULT_TICKET_LIFETIME; #endif +#if defined(POLARSSL_KEY_EXCHANGE__SOME__ECDHE_ENABLED) + ssl->ecdh_curve_list = ecdh_default_curve_list; +#endif + if( ( ret = ssl_handshake_init( ssl ) ) != 0 ) return( ret ); @@ -4610,3 +4654,13 @@ md_type_t ssl_md_alg_from_hash( unsigned char hash ) } #endif + +#if defined(POLARSSL_KEY_EXCHANGE__SOME__ECDHE_ENABLED) +/* + * Set the allowed ECDH curves. + */ +void ssl_set_ecdh_curves( ssl_context *ssl, const ecp_group_id *ecdh_curve_list ) +{ + ssl->ecdh_curve_list = ecdh_curve_list; +} +#endif From 5de2580563109decf0e883f7e7c414b933d35333 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Mon, 3 Feb 2014 15:56:49 +0100 Subject: [PATCH 03/13] Make ssl_set_ecdh_curves() a compile-time option --- include/polarssl/config.h | 16 ++++++++++++++++ include/polarssl/ssl.h | 10 +++++++--- library/ssl_srv.c | 9 +++++++-- library/ssl_tls.c | 6 ++++-- 4 files changed, 34 insertions(+), 7 deletions(-) diff --git a/include/polarssl/config.h b/include/polarssl/config.h index 1ed203c73..8c95c426a 100644 --- a/include/polarssl/config.h +++ b/include/polarssl/config.h @@ -813,6 +813,22 @@ */ #define POLARSSL_SSL_TRUNCATED_HMAC +/** + * \def POLARSSL_SSL_SET_ECDH_CURVES + * + * Enable ssl_set_ecdh_curves(). + * + * This is disabled by default since it breaks binary compatibility with the + * 1.3.x line. If you choose to enable it, you will need to rebuild your + * application against the new header files, relinking will not be enough. + * It will be enabled by default, or no longer an option, in the 1.4 branch. + * + * TODO: actually disable it when done working on this branch ,) + * + * Uncomment to make ssl_set_ecdh_curves() available. + */ +#define POLARSSL_SSL_SET_ECDH_CURVES + /** * \def POLARSSL_THREADING_ALT * diff --git a/include/polarssl/ssl.h b/include/polarssl/ssl.h index 2b50304aa..2fdc01df4 100644 --- a/include/polarssl/ssl.h +++ b/include/polarssl/ssl.h @@ -727,7 +727,8 @@ struct _ssl_context int disable_renegotiation; /*!< enable/disable renegotiation */ int allow_legacy_renegotiation; /*!< allow legacy renegotiation */ const int *ciphersuite_list[4]; /*!< allowed ciphersuites / version */ -#if defined(POLARSSL_KEY_EXCHANGE__SOME__ECDHE_ENABLED) +#if defined(POLARSSL_KEY_EXCHANGE__SOME__ECDHE_ENABLED) && \ + defined(POLARSSL_SSL_SET_ECDH_CURVES) const ecp_group_id *ecdh_curve_list;/*!< allowed curves for ECDH */ #endif #if defined(POLARSSL_SSL_TRUNCATED_HMAC) @@ -1158,9 +1159,11 @@ int ssl_set_dh_param( ssl_context *ssl, const char *dhm_P, const char *dhm_G ); int ssl_set_dh_param_ctx( ssl_context *ssl, dhm_context *dhm_ctx ); #endif -#if defined(POLARSSL_KEY_EXCHANGE__SOME__ECDHE_ENABLED) +#if defined(POLARSSL_KEY_EXCHANGE__SOME__ECDHE_ENABLED) && \ + defined(POLARSSL_SSL_SET_ECDH_CURVES) /** * \brief Set the allowed ECDH curves. + * (Default: all defined curves.) * * The sequence of the curves in the list also determines the * handshake curve preference. @@ -1168,7 +1171,8 @@ int ssl_set_dh_param_ctx( ssl_context *ssl, dhm_context *dhm_ctx ); * \param ssl SSL context * \param ecdh_curve_list Zero terminated list of the allowed ECDH curves */ -void ssl_set_ecdh_curves( ssl_context *ssl, const ecp_group_id *ecdh_curve_list ); +void ssl_set_ecdh_curves( ssl_context *ssl, + const ecp_group_id *ecdh_curve_list ); #endif #if defined(POLARSSL_SSL_SERVER_NAME_INDICATION) diff --git a/library/ssl_srv.c b/library/ssl_srv.c index dfae8c5a1..ac5f8028f 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -2105,7 +2105,8 @@ static int ssl_write_server_key_exchange( ssl_context *ssl ) * ECPoint public; * } ServerECDHParams; */ - + ecp_group_id grp_id; +#if defined(POLARSSL_SSL_SET_ECDH_CURVES) unsigned int pref_idx, curv_idx, found; /* Match our preference list against the agreed curves */ @@ -2137,9 +2138,13 @@ static int ssl_write_server_key_exchange( ssl_context *ssl ) * ssl->ecdh_curve_list[pref_idx] will contain POLARSSL_ECP_DP_NONE and * ecp_use_known_dp() will fail. */ + grp_id = ssl->ecdh_curve_list[pref_idx]; +#else + grp_id = ssl->handshake->curves[0]->grp_id; +#endif /* POLARSSL_SSL_SET_ECDH_CURVES */ if( ( ret = ecp_use_known_dp( &ssl->handshake->ecdh_ctx.grp, - ssl->ecdh_curve_list[pref_idx] ) ) != 0 ) + grp_id ) ) != 0 ) { SSL_DEBUG_RET( 1, "ecp_use_known_dp", ret ); return( ret ); diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 02f24a18f..29977d789 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -3424,7 +3424,8 @@ int ssl_init( ssl_context *ssl ) ssl->ticket_lifetime = SSL_DEFAULT_TICKET_LIFETIME; #endif -#if defined(POLARSSL_KEY_EXCHANGE__SOME__ECDHE_ENABLED) +#if defined(POLARSSL_KEY_EXCHANGE__SOME__ECDHE_ENABLED) && \ + defined(POLARSSL_SSL_SET_ECDH_CURVES) ssl->ecdh_curve_list = ecdh_default_curve_list; #endif @@ -4655,7 +4656,8 @@ md_type_t ssl_md_alg_from_hash( unsigned char hash ) #endif -#if defined(POLARSSL_KEY_EXCHANGE__SOME__ECDHE_ENABLED) +#if defined(POLARSSL_KEY_EXCHANGE__SOME__ECDHE_ENABLED) && \ + defined(POLARSSL_SSL_SET_ECDH_CURVES) /* * Set the allowed ECDH curves. */ From de05390c856b9f2b616f42c1741360528f24e384 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 4 Feb 2014 13:58:39 +0100 Subject: [PATCH 04/13] Rename ecdh_curve_list to curve_list --- include/polarssl/config.h | 8 ++++---- include/polarssl/ssl.h | 11 +++++------ library/ssl_srv.c | 18 +++++++++--------- library/ssl_tls.c | 12 ++++++------ 4 files changed, 24 insertions(+), 25 deletions(-) diff --git a/include/polarssl/config.h b/include/polarssl/config.h index 8c95c426a..0a64f6ede 100644 --- a/include/polarssl/config.h +++ b/include/polarssl/config.h @@ -814,9 +814,9 @@ #define POLARSSL_SSL_TRUNCATED_HMAC /** - * \def POLARSSL_SSL_SET_ECDH_CURVES + * \def POLARSSL_SSL_SET_CURVES * - * Enable ssl_set_ecdh_curves(). + * Enable ssl_set_curves(). * * This is disabled by default since it breaks binary compatibility with the * 1.3.x line. If you choose to enable it, you will need to rebuild your @@ -825,9 +825,9 @@ * * TODO: actually disable it when done working on this branch ,) * - * Uncomment to make ssl_set_ecdh_curves() available. + * Uncomment to make ssl_set_curves() available. */ -#define POLARSSL_SSL_SET_ECDH_CURVES +#define POLARSSL_SSL_SET_CURVES /** * \def POLARSSL_THREADING_ALT diff --git a/include/polarssl/ssl.h b/include/polarssl/ssl.h index 2fdc01df4..3ab362921 100644 --- a/include/polarssl/ssl.h +++ b/include/polarssl/ssl.h @@ -728,8 +728,8 @@ struct _ssl_context int allow_legacy_renegotiation; /*!< allow legacy renegotiation */ const int *ciphersuite_list[4]; /*!< allowed ciphersuites / version */ #if defined(POLARSSL_KEY_EXCHANGE__SOME__ECDHE_ENABLED) && \ - defined(POLARSSL_SSL_SET_ECDH_CURVES) - const ecp_group_id *ecdh_curve_list;/*!< allowed curves for ECDH */ + defined(POLARSSL_SSL_SET_CURVES) + const ecp_group_id *curve_list; /*!< allowed curves */ #endif #if defined(POLARSSL_SSL_TRUNCATED_HMAC) int trunc_hmac; /*!< negotiate truncated hmac? */ @@ -1160,7 +1160,7 @@ int ssl_set_dh_param_ctx( ssl_context *ssl, dhm_context *dhm_ctx ); #endif #if defined(POLARSSL_KEY_EXCHANGE__SOME__ECDHE_ENABLED) && \ - defined(POLARSSL_SSL_SET_ECDH_CURVES) + defined(POLARSSL_SSL_SET_CURVES) /** * \brief Set the allowed ECDH curves. * (Default: all defined curves.) @@ -1169,10 +1169,9 @@ int ssl_set_dh_param_ctx( ssl_context *ssl, dhm_context *dhm_ctx ); * handshake curve preference. * * \param ssl SSL context - * \param ecdh_curve_list Zero terminated list of the allowed ECDH curves + * \param curves Zero terminated list of the allowed ECDH curves */ -void ssl_set_ecdh_curves( ssl_context *ssl, - const ecp_group_id *ecdh_curve_list ); +void ssl_set_curves( ssl_context *ssl, const ecp_group_id *curves ); #endif #if defined(POLARSSL_SSL_SERVER_NAME_INDICATION) diff --git a/library/ssl_srv.c b/library/ssl_srv.c index ac5f8028f..20a6be591 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -2106,12 +2106,12 @@ static int ssl_write_server_key_exchange( ssl_context *ssl ) * } ServerECDHParams; */ ecp_group_id grp_id; -#if defined(POLARSSL_SSL_SET_ECDH_CURVES) +#if defined(POLARSSL_SSL_SET_CURVES) unsigned int pref_idx, curv_idx, found; /* Match our preference list against the agreed curves */ for( pref_idx = 0, found = 0; - ssl->ecdh_curve_list[pref_idx] != POLARSSL_ECP_DP_NONE; + ssl->curve_list[pref_idx] != POLARSSL_ECP_DP_NONE; pref_idx++ ) { /* Look through the agreed curve list */ @@ -2120,7 +2120,7 @@ static int ssl_write_server_key_exchange( ssl_context *ssl ) curv_idx++ ) { if (ssl->handshake->curves[curv_idx]->grp_id == - ssl->ecdh_curve_list[pref_idx] ) + ssl->curve_list[pref_idx] ) { /* We found our most preferred curve */ found = 1; @@ -2130,18 +2130,18 @@ static int ssl_write_server_key_exchange( ssl_context *ssl ) /* Exit the search if we have found our curve */ if( found == 1 ) - { break; - } } - /* If we haven't found any allowed / preferred curve, - * ssl->ecdh_curve_list[pref_idx] will contain POLARSSL_ECP_DP_NONE and + + /* + * If we haven't found any allowed / preferred curve, + * ssl->curve_list[pref_idx] will contain POLARSSL_ECP_DP_NONE and * ecp_use_known_dp() will fail. */ - grp_id = ssl->ecdh_curve_list[pref_idx]; + grp_id = ssl->curve_list[pref_idx]; #else grp_id = ssl->handshake->curves[0]->grp_id; -#endif /* POLARSSL_SSL_SET_ECDH_CURVES */ +#endif /* POLARSSL_SSL_SET_CURVES */ if( ( ret = ecp_use_known_dp( &ssl->handshake->ecdh_ctx.grp, grp_id ) ) != 0 ) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 29977d789..79b4bb75c 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -3335,7 +3335,7 @@ int ssl_init( ssl_context *ssl ) * * TODO: Add the Montgomery curves */ - static const ecp_group_id ecdh_default_curve_list[] = + static const ecp_group_id default_curve_list[] = { #if defined(POLARSSL_ECP_DP_SECP521R1_ENABLED) POLARSSL_ECP_DP_SECP521R1, @@ -3425,8 +3425,8 @@ int ssl_init( ssl_context *ssl ) #endif #if defined(POLARSSL_KEY_EXCHANGE__SOME__ECDHE_ENABLED) && \ - defined(POLARSSL_SSL_SET_ECDH_CURVES) - ssl->ecdh_curve_list = ecdh_default_curve_list; + defined(POLARSSL_SSL_SET_CURVES) + ssl->curve_list = default_curve_list; #endif if( ( ret = ssl_handshake_init( ssl ) ) != 0 ) @@ -4657,12 +4657,12 @@ md_type_t ssl_md_alg_from_hash( unsigned char hash ) #endif #if defined(POLARSSL_KEY_EXCHANGE__SOME__ECDHE_ENABLED) && \ - defined(POLARSSL_SSL_SET_ECDH_CURVES) + defined(POLARSSL_SSL_SET_CURVES) /* * Set the allowed ECDH curves. */ -void ssl_set_ecdh_curves( ssl_context *ssl, const ecp_group_id *ecdh_curve_list ) +void ssl_set_curves( ssl_context *ssl, const ecp_group_id *curve_list ) { - ssl->ecdh_curve_list = ecdh_curve_list; + ssl->curve_list = curve_list; } #endif From e40c469ad3d8b7b6cd8b6e21dfc0227dc73492b5 Mon Sep 17 00:00:00 2001 From: Gergely Budai Date: Wed, 22 Jan 2014 11:22:20 +0100 Subject: [PATCH 05/13] The default ECDH curve list will be dynamically built in the ecp module based on ecp_supported_curves[]. --- include/polarssl/ecp.h | 7 +++++++ library/ecp.c | 43 +++++++++++++++++++++++++++++++++--------- library/ssl_tls.c | 42 +---------------------------------------- 3 files changed, 42 insertions(+), 50 deletions(-) diff --git a/include/polarssl/ecp.h b/include/polarssl/ecp.h index 1635b702f..e79b0bced 100644 --- a/include/polarssl/ecp.h +++ b/include/polarssl/ecp.h @@ -255,6 +255,13 @@ const ecp_curve_info *ecp_curve_info_from_tls_id( uint16_t tls_id ); */ const ecp_curve_info *ecp_curve_info_from_name( const char *name ); +/** + * \brief Get the default ECDH curve list + * + * \return The default ECDH curve list + */ +ecp_group_id *ecp_get_default_echd_curve_list( void ); + /** * \brief Initialize a point (as zero) */ diff --git a/library/ecp.c b/library/ecp.c index a27d30e2a..992c43697 100644 --- a/library/ecp.c +++ b/library/ecp.c @@ -114,27 +114,33 @@ typedef enum * - TLS NamedCurve ID (RFC 4492 sec. 5.1.1, RFC 7071 sec. 2) * - size in bits * - readable name + * + * The sequence of elements in this list also determines the default preference + * of the curves used by an ECHDE handshake. + * We start with the most secure curves. From the same sized curves, we prefer + * the SECP ones because they are much faster. + * */ static const ecp_curve_info ecp_supported_curves[] = { -#if defined(POLARSSL_ECP_DP_BP512R1_ENABLED) - { POLARSSL_ECP_DP_BP512R1, 28, 512, "brainpoolP512r1" }, -#endif -#if defined(POLARSSL_ECP_DP_BP384R1_ENABLED) - { POLARSSL_ECP_DP_BP384R1, 27, 384, "brainpoolP384r1" }, -#endif -#if defined(POLARSSL_ECP_DP_BP256R1_ENABLED) - { POLARSSL_ECP_DP_BP256R1, 26, 256, "brainpoolP256r1" }, -#endif #if defined(POLARSSL_ECP_DP_SECP521R1_ENABLED) { POLARSSL_ECP_DP_SECP521R1, 25, 521, "secp521r1" }, #endif +#if defined(POLARSSL_ECP_DP_BP512R1_ENABLED) + { POLARSSL_ECP_DP_BP512R1, 28, 512, "brainpoolP512r1" }, +#endif #if defined(POLARSSL_ECP_DP_SECP384R1_ENABLED) { POLARSSL_ECP_DP_SECP384R1, 24, 384, "secp384r1" }, #endif +#if defined(POLARSSL_ECP_DP_BP384R1_ENABLED) + { POLARSSL_ECP_DP_BP384R1, 27, 384, "brainpoolP384r1" }, +#endif #if defined(POLARSSL_ECP_DP_SECP256R1_ENABLED) { POLARSSL_ECP_DP_SECP256R1, 23, 256, "secp256r1" }, #endif +#if defined(POLARSSL_ECP_DP_BP256R1_ENABLED) + { POLARSSL_ECP_DP_BP256R1, 26, 256, "brainpoolP256r1" }, +#endif #if defined(POLARSSL_ECP_DP_SECP224R1_ENABLED) { POLARSSL_ECP_DP_SECP224R1, 21, 224, "secp224r1" }, #endif @@ -152,6 +158,8 @@ static const ecp_curve_info ecp_supported_curves[] = #endif { POLARSSL_ECP_DP_NONE, 0, 0, NULL }, }; +#define ECP_NUM_SUPPORTED_CURVES ( sizeof( ecp_supported_curves ) / \ + sizeof( ecp_curve_info ) ) /* * List of supported curves and associated info @@ -215,6 +223,23 @@ const ecp_curve_info *ecp_curve_info_from_name( const char *name ) return( NULL ); } +/* + * Get the default ECDH curve list + */ +ecp_group_id *ecp_get_default_echd_curve_list( void ) +{ + static ecp_group_id ecdh_default_curve_list[ECP_NUM_SUPPORTED_CURVES]; + int i; + + /* Build the list of default curves based on ecp_supported_curves[] */ + for( i = 0; i < ECP_NUM_SUPPORTED_CURVES; i++) + { + ecdh_default_curve_list[i] = ecp_supported_curves[i].grp_id; + } + + return ecdh_default_curve_list; +} + /* * Get the type of a curve */ diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 79b4bb75c..dd84daa08 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -3325,46 +3325,6 @@ static int ssl_handshake_init( ssl_context *ssl ) */ int ssl_init( ssl_context *ssl ) { - -#if defined(POLARSSL_KEY_EXCHANGE__SOME__ECDHE_ENABLED) - /* - * ECDHE allowed curves and preference list - * - * We start with the most secure curves. From the same size curves, we prefer - * the SECP ones because they are much faster. - * - * TODO: Add the Montgomery curves - */ - static const ecp_group_id default_curve_list[] = - { -#if defined(POLARSSL_ECP_DP_SECP521R1_ENABLED) - POLARSSL_ECP_DP_SECP521R1, -#endif -#if defined(POLARSSL_ECP_DP_BP512R1_ENABLED) - POLARSSL_ECP_DP_BP512R1, -#endif -#if defined(POLARSSL_ECP_DP_SECP384R1_ENABLED) - POLARSSL_ECP_DP_SECP384R1, -#endif -#if defined(POLARSSL_ECP_DP_BP384R1_ENABLED) - POLARSSL_ECP_DP_BP384R1, -#endif -#if defined(POLARSSL_ECP_DP_SECP256R1_ENABLED) - POLARSSL_ECP_DP_SECP256R1, -#endif -#if defined(POLARSSL_ECP_DP_BP256R1_ENABLED) - POLARSSL_ECP_DP_BP256R1, -#endif -#if defined(POLARSSL_ECP_DP_SECP224R1_ENABLED) - POLARSSL_ECP_DP_SECP224R1, -#endif -#if defined(POLARSSL_ECP_DP_SECP192R1_ENABLED) - POLARSSL_ECP_DP_SECP192R1, -#endif - POLARSSL_ECP_DP_NONE - }; -#endif - int ret; int len = SSL_BUFFER_LEN; @@ -3426,7 +3386,7 @@ int ssl_init( ssl_context *ssl ) #if defined(POLARSSL_KEY_EXCHANGE__SOME__ECDHE_ENABLED) && \ defined(POLARSSL_SSL_SET_CURVES) - ssl->curve_list = default_curve_list; + ssl->curve_list = ecp_get_default_echd_curve_list( ); #endif if( ( ret = ssl_handshake_init( ssl ) ) != 0 ) From ac7194133ec165bccf56dc614321bc3012f8184e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 4 Feb 2014 14:48:50 +0100 Subject: [PATCH 06/13] Renamings and other fixes --- include/polarssl/ecp.h | 19 +++++++----- library/ecp.c | 70 +++++++++++++++++++++++------------------- library/ssl_tls.c | 2 +- 3 files changed, 50 insertions(+), 41 deletions(-) diff --git a/include/polarssl/ecp.h b/include/polarssl/ecp.h index e79b0bced..b35b0d1d0 100644 --- a/include/polarssl/ecp.h +++ b/include/polarssl/ecp.h @@ -222,12 +222,22 @@ ecp_keypair; #define POLARSSL_ECP_TLS_NAMED_CURVE 3 /**< ECCurveType's named_curve */ /** - * \brief Return the list of supported curves with associated info + * \brief Get the list of supported curves in order of preferrence + * (full information) * * \return A statically allocated array, the last entry is 0. */ const ecp_curve_info *ecp_curve_list( void ); +/** + * \brief Get the list of supported curves in order of preferrence + * (grp_id only) + * + * \return A statically allocated array, + * terminated with POLARSSL_ECP_DP_NONE. + */ +const ecp_group_id *ecp_grp_id_list( void ); + /** * \brief Get curve information from an internal group identifier * @@ -255,13 +265,6 @@ const ecp_curve_info *ecp_curve_info_from_tls_id( uint16_t tls_id ); */ const ecp_curve_info *ecp_curve_info_from_name( const char *name ); -/** - * \brief Get the default ECDH curve list - * - * \return The default ECDH curve list - */ -ecp_group_id *ecp_get_default_echd_curve_list( void ); - /** * \brief Initialize a point (as zero) */ diff --git a/library/ecp.c b/library/ecp.c index 992c43697..ad6e5f586 100644 --- a/library/ecp.c +++ b/library/ecp.c @@ -115,13 +115,10 @@ typedef enum * - size in bits * - readable name * - * The sequence of elements in this list also determines the default preference - * of the curves used by an ECHDE handshake. - * We start with the most secure curves. From the same sized curves, we prefer - * the SECP ones because they are much faster. - * + * Curves are listed in order: largest curves first, and for a given size, + * fastest curves first. This provides the default order for the SSL module. */ -static const ecp_curve_info ecp_supported_curves[] = +static const ecp_curve_info ecp_supported_curves[POLARSSL_ECP_DP_MAX] = { #if defined(POLARSSL_ECP_DP_SECP521R1_ENABLED) { POLARSSL_ECP_DP_SECP521R1, 25, 521, "secp521r1" }, @@ -138,28 +135,28 @@ static const ecp_curve_info ecp_supported_curves[] = #if defined(POLARSSL_ECP_DP_SECP256R1_ENABLED) { POLARSSL_ECP_DP_SECP256R1, 23, 256, "secp256r1" }, #endif +#if defined(POLARSSL_ECP_DP_SECP256K1_ENABLED) + { POLARSSL_ECP_DP_SECP256K1, 22, 256, "secp256k1" }, +#endif #if defined(POLARSSL_ECP_DP_BP256R1_ENABLED) { POLARSSL_ECP_DP_BP256R1, 26, 256, "brainpoolP256r1" }, #endif #if defined(POLARSSL_ECP_DP_SECP224R1_ENABLED) { POLARSSL_ECP_DP_SECP224R1, 21, 224, "secp224r1" }, #endif -#if defined(POLARSSL_ECP_DP_SECP192R1_ENABLED) - { POLARSSL_ECP_DP_SECP192R1, 19, 192, "secp192r1" }, -#endif -#if defined(POLARSSL_ECP_DP_SECP256K1_ENABLED) - { POLARSSL_ECP_DP_SECP256K1, 22, 256, "secp256k1" }, -#endif #if defined(POLARSSL_ECP_DP_SECP224K1_ENABLED) { POLARSSL_ECP_DP_SECP224K1, 20, 224, "secp224k1" }, #endif +#if defined(POLARSSL_ECP_DP_SECP192R1_ENABLED) + { POLARSSL_ECP_DP_SECP192R1, 19, 192, "secp192r1" }, +#endif #if defined(POLARSSL_ECP_DP_SECP192K1_ENABLED) { POLARSSL_ECP_DP_SECP192K1, 18, 192, "secp192k1" }, #endif { POLARSSL_ECP_DP_NONE, 0, 0, NULL }, }; -#define ECP_NUM_SUPPORTED_CURVES ( sizeof( ecp_supported_curves ) / \ - sizeof( ecp_curve_info ) ) + +static ecp_group_id ecp_supported_grp_id[POLARSSL_ECP_DP_MAX]; /* * List of supported curves and associated info @@ -170,7 +167,33 @@ const ecp_curve_info *ecp_curve_list( void ) } /* - * Get the curve info for the internal identifer + * List of supported curves, group ID only + */ +const ecp_group_id *ecp_grp_id_list( void ) +{ + static int init_done = 0; + + if( ! init_done ) + { + size_t i = 0; + const ecp_curve_info *curve_info; + + for( curve_info = ecp_curve_list(); + curve_info->grp_id != POLARSSL_ECP_DP_NONE; + curve_info++ ) + { + ecp_supported_grp_id[i++] = curve_info->grp_id; + } + ecp_supported_grp_id[i] = POLARSSL_ECP_DP_NONE; + + init_done = 1; + } + + return ecp_supported_grp_id; +} + +/* + * Get the curve info for the internal identifier */ const ecp_curve_info *ecp_curve_info_from_grp_id( ecp_group_id grp_id ) { @@ -223,23 +246,6 @@ const ecp_curve_info *ecp_curve_info_from_name( const char *name ) return( NULL ); } -/* - * Get the default ECDH curve list - */ -ecp_group_id *ecp_get_default_echd_curve_list( void ) -{ - static ecp_group_id ecdh_default_curve_list[ECP_NUM_SUPPORTED_CURVES]; - int i; - - /* Build the list of default curves based on ecp_supported_curves[] */ - for( i = 0; i < ECP_NUM_SUPPORTED_CURVES; i++) - { - ecdh_default_curve_list[i] = ecp_supported_curves[i].grp_id; - } - - return ecdh_default_curve_list; -} - /* * Get the type of a curve */ diff --git a/library/ssl_tls.c b/library/ssl_tls.c index dd84daa08..987e2cfa1 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -3386,7 +3386,7 @@ int ssl_init( ssl_context *ssl ) #if defined(POLARSSL_KEY_EXCHANGE__SOME__ECDHE_ENABLED) && \ defined(POLARSSL_SSL_SET_CURVES) - ssl->curve_list = ecp_get_default_echd_curve_list( ); + ssl->curve_list = ecp_grp_id_list( ); #endif if( ( ret = ssl_handshake_init( ssl ) ) != 0 ) From cd49f76898446c6766886d4eb6614531b9e4c7ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 4 Feb 2014 15:14:13 +0100 Subject: [PATCH 07/13] Make ssl_set_curves() work client-side too. --- include/polarssl/ssl.h | 13 +++++++++---- library/ssl_cli.c | 21 ++++++++++++++------- 2 files changed, 23 insertions(+), 11 deletions(-) diff --git a/include/polarssl/ssl.h b/include/polarssl/ssl.h index 3ab362921..f93abefa7 100644 --- a/include/polarssl/ssl.h +++ b/include/polarssl/ssl.h @@ -1162,14 +1162,19 @@ int ssl_set_dh_param_ctx( ssl_context *ssl, dhm_context *dhm_ctx ); #if defined(POLARSSL_KEY_EXCHANGE__SOME__ECDHE_ENABLED) && \ defined(POLARSSL_SSL_SET_CURVES) /** - * \brief Set the allowed ECDH curves. + * \brief Set the allowed curves in order of preference. * (Default: all defined curves.) * - * The sequence of the curves in the list also determines the - * handshake curve preference. + * On server: this only affects selection of the ECDHE curve; + * the curves used for ECDH and ECDSA are determined by the + * list of available certificates instead. + * + * On client: this affects the list of curves offered for any + * use. The server can override our preferences. * * \param ssl SSL context - * \param curves Zero terminated list of the allowed ECDH curves + * \param curves Ordered list of allowed curves, + * terminated by POLARSSL_ECP_DP_NONE. */ void ssl_set_curves( ssl_context *ssl, const ecp_group_id *curves ); #endif diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 62df85747..fa3b7a89f 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -233,19 +233,26 @@ static void ssl_write_supported_elliptic_curves_ext( ssl_context *ssl, unsigned char *p = buf; unsigned char *elliptic_curve_list = p + 6; size_t elliptic_curve_len = 0; - const ecp_curve_info *curve; - ((void) ssl); + const ecp_curve_info *info; +#if defined(POLARSSL_SSL_SET_CURVES) + const ecp_group_id *grp_id; +#endif *olen = 0; SSL_DEBUG_MSG( 3, ( "client hello, adding supported_elliptic_curves extension" ) ); - for( curve = ecp_curve_list(); - curve->grp_id != POLARSSL_ECP_DP_NONE; - curve++ ) +#if defined(POLARSSL_SSL_SET_CURVES) + for( grp_id = ssl->curve_list; *grp_id != POLARSSL_ECP_DP_NONE; grp_id++ ) { - elliptic_curve_list[elliptic_curve_len++] = curve->tls_id >> 8; - elliptic_curve_list[elliptic_curve_len++] = curve->tls_id & 0xFF; + info = ecp_curve_info_from_grp_id( *grp_id ); +#else + for( info = ecp_curve_list(); info->grp_id != POLARSSL_ECP_DP_NONE; info++ ) + { +#endif + + elliptic_curve_list[elliptic_curve_len++] = info->tls_id >> 8; + elliptic_curve_list[elliptic_curve_len++] = info->tls_id & 0xFF; } if( elliptic_curve_len == 0 ) From 7f38ed0bfa18ea236d2cf21e502ab1a2ad5af3d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 4 Feb 2014 15:52:33 +0100 Subject: [PATCH 08/13] ssl_set_curves is no longer ECDHE only --- include/polarssl/ssl.h | 6 ++---- library/ssl_tls.c | 23 +++++++++++------------ 2 files changed, 13 insertions(+), 16 deletions(-) diff --git a/include/polarssl/ssl.h b/include/polarssl/ssl.h index f93abefa7..f4084e8fd 100644 --- a/include/polarssl/ssl.h +++ b/include/polarssl/ssl.h @@ -727,8 +727,7 @@ struct _ssl_context int disable_renegotiation; /*!< enable/disable renegotiation */ int allow_legacy_renegotiation; /*!< allow legacy renegotiation */ const int *ciphersuite_list[4]; /*!< allowed ciphersuites / version */ -#if defined(POLARSSL_KEY_EXCHANGE__SOME__ECDHE_ENABLED) && \ - defined(POLARSSL_SSL_SET_CURVES) +#if defined(POLARSSL_SSL_SET_CURVES) const ecp_group_id *curve_list; /*!< allowed curves */ #endif #if defined(POLARSSL_SSL_TRUNCATED_HMAC) @@ -1159,8 +1158,7 @@ int ssl_set_dh_param( ssl_context *ssl, const char *dhm_P, const char *dhm_G ); int ssl_set_dh_param_ctx( ssl_context *ssl, dhm_context *dhm_ctx ); #endif -#if defined(POLARSSL_KEY_EXCHANGE__SOME__ECDHE_ENABLED) && \ - defined(POLARSSL_SSL_SET_CURVES) +#if defined(POLARSSL_SSL_SET_CURVES) /** * \brief Set the allowed curves in order of preference. * (Default: all defined curves.) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 987e2cfa1..0178c5e5a 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -3384,8 +3384,7 @@ int ssl_init( ssl_context *ssl ) ssl->ticket_lifetime = SSL_DEFAULT_TICKET_LIFETIME; #endif -#if defined(POLARSSL_KEY_EXCHANGE__SOME__ECDHE_ENABLED) && \ - defined(POLARSSL_SSL_SET_CURVES) +#if defined(POLARSSL_SSL_SET_CURVES) ssl->curve_list = ecp_grp_id_list( ); #endif @@ -3801,6 +3800,16 @@ int ssl_set_dh_param_ctx( ssl_context *ssl, dhm_context *dhm_ctx ) } #endif /* POLARSSL_DHM_C */ +#if defined(POLARSSL_SSL_SET_CURVES) +/* + * Set the allowed elliptic curves + */ +void ssl_set_curves( ssl_context *ssl, const ecp_group_id *curve_list ) +{ + ssl->curve_list = curve_list; +} +#endif + #if defined(POLARSSL_SSL_SERVER_NAME_INDICATION) int ssl_set_hostname( ssl_context *ssl, const char *hostname ) { @@ -4616,13 +4625,3 @@ md_type_t ssl_md_alg_from_hash( unsigned char hash ) #endif -#if defined(POLARSSL_KEY_EXCHANGE__SOME__ECDHE_ENABLED) && \ - defined(POLARSSL_SSL_SET_CURVES) -/* - * Set the allowed ECDH curves. - */ -void ssl_set_curves( ssl_context *ssl, const ecp_group_id *curve_list ) -{ - ssl->curve_list = curve_list; -} -#endif From ab24010b543abb65f0c73e24ee68615e6f24bd05 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 4 Feb 2014 16:18:07 +0100 Subject: [PATCH 09/13] Enforce our choice of allowed curves. --- include/polarssl/ssl.h | 9 ++++++++- library/ssl_cli.c | 17 ++++++++++++----- library/ssl_tls.c | 32 ++++++++++++++++++++++++++++++++ 3 files changed, 52 insertions(+), 6 deletions(-) diff --git a/include/polarssl/ssl.h b/include/polarssl/ssl.h index f4084e8fd..d610052c9 100644 --- a/include/polarssl/ssl.h +++ b/include/polarssl/ssl.h @@ -1168,7 +1168,10 @@ int ssl_set_dh_param_ctx( ssl_context *ssl, dhm_context *dhm_ctx ); * list of available certificates instead. * * On client: this affects the list of curves offered for any - * use. The server can override our preferences. + * use. The server can override our preference order. + * + * Both sides: limits the set of curves used by peer to the + * listed curves for any use (ECDH(E), certificates). * * \param ssl SSL context * \param curves Ordered list of allowed curves, @@ -1589,6 +1592,10 @@ pk_type_t ssl_pk_alg_from_sig( unsigned char sig ); md_type_t ssl_md_alg_from_hash( unsigned char hash ); +#if defined(POLARSSL_SSL_SET_CURVES) +int ssl_curve_is_acceptable( const ssl_context *ssl, ecp_group_id grp_id ); +#endif + #if defined(POLARSSL_X509_CRT_PARSE_C) static inline pk_context *ssl_own_key( ssl_context *ssl ) { diff --git a/library/ssl_cli.c b/library/ssl_cli.c index fa3b7a89f..5947e5c1b 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -1125,14 +1125,17 @@ static int ssl_parse_server_dh_params( ssl_context *ssl, unsigned char **p, defined(POLARSSL_KEY_EXCHANGE_ECDH_ECDSA_ENABLED) static int ssl_check_server_ecdh_params( const ssl_context *ssl ) { + // TODO: print name instead SSL_DEBUG_MSG( 2, ( "ECDH curve size: %d", (int) ssl->handshake->ecdh_ctx.grp.nbits ) ); +#if defined(POLARSSL_SSL_ECP_SET_CURVES) + if( ! ssl_curve_is_acceptable( ssl, ssl->handshake->ecdh_ctx.grp.id ) ) +#else if( ssl->handshake->ecdh_ctx.grp.nbits < 163 || ssl->handshake->ecdh_ctx.grp.nbits > 521 ) - { +#endif return( -1 ); - } SSL_DEBUG_ECP( 3, "ECDH: Qp", &ssl->handshake->ecdh_ctx.Qp ); @@ -1167,7 +1170,7 @@ static int ssl_parse_server_ecdh_params( ssl_context *ssl, if( ssl_check_server_ecdh_params( ssl ) != 0 ) { - SSL_DEBUG_MSG( 1, ( "bad server key exchange message (ECDH length)" ) ); + SSL_DEBUG_MSG( 1, ( "bad server key exchange message (ECDHE curve)" ) ); return( POLARSSL_ERR_SSL_BAD_HS_SERVER_KEY_EXCHANGE ); } @@ -1355,7 +1358,7 @@ static int ssl_get_ecdh_params_from_cert( ssl_context *ssl ) if( ssl_check_server_ecdh_params( ssl ) != 0 ) { - SSL_DEBUG_MSG( 1, ( "bad server certificate (ECDH length)" ) ); + SSL_DEBUG_MSG( 1, ( "bad server certificate (ECDH curve)" ) ); return( POLARSSL_ERR_SSL_BAD_HS_CERTIFICATE ); } @@ -1397,7 +1400,11 @@ static int ssl_parse_server_key_exchange( ssl_context *ssl ) if( ciphersuite_info->key_exchange == POLARSSL_KEY_EXCHANGE_ECDH_RSA || ciphersuite_info->key_exchange == POLARSSL_KEY_EXCHANGE_ECDH_ECDSA ) { - ssl_get_ecdh_params_from_cert( ssl ); + if( ( ret = ssl_get_ecdh_params_from_cert( ssl ) ) != 0 ) + { + SSL_DEBUG_RET( 1, "ssl_get_ecdh_params_from_cert", ret ); + return( ret ); + } SSL_DEBUG_MSG( 2, ( "<= skip parse server key exchange" ) ); ssl->state++; diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 0178c5e5a..a5205836c 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -2664,7 +2664,23 @@ int ssl_parse_certificate( ssl_context *ssl ) ssl->f_vrfy, ssl->p_vrfy ); if( ret != 0 ) + { SSL_DEBUG_RET( 1, "x509_verify_cert", ret ); + } +#if defined(POLARSSL_SSL_SET_CURVES) + else + { + pk_context *pk = &ssl->session_negotiate->peer_cert->pk; + + /* If certificate uses an EC key, make sure the curve is OK */ + if( pk_can_do( pk, POLARSSL_PK_ECKEY ) && + ! ssl_curve_is_acceptable( ssl, pk_ec( *pk )->grp.id ) ) + { + SSL_DEBUG_MSG( 1, ( "bad server certificate (EC key curve)" ) ); + ret = POLARSSL_ERR_SSL_BAD_HS_CERTIFICATE; + } + } +#endif if( ssl->authmode != SSL_VERIFY_REQUIRED ) ret = 0; @@ -4625,3 +4641,19 @@ md_type_t ssl_md_alg_from_hash( unsigned char hash ) #endif +#if defined(POLARSSL_SSL_SET_CURVES) +/* + * Check is a curve proposed by the peer is in our list. + * Return 1 if we're willing to use it, 0 otherwise. + */ +int ssl_curve_is_acceptable( const ssl_context *ssl, ecp_group_id grp_id ) +{ + const ecp_group_id *gid; + + for( gid = ssl->curve_list; *gid != POLARSSL_ECP_DP_NONE; gid++ ) + if( *gid == grp_id ) + return( 1 ); + + return( 0 ); +} +#endif From c3f6b62ccc0c425895309084bc424fcdf954d1c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 6 Feb 2014 10:13:09 +0100 Subject: [PATCH 10/13] Print curve name instead of size in debugging Also refactor server-side curve selection --- library/ssl_cli.c | 13 ++++++++--- library/ssl_srv.c | 56 ++++++++++++++++------------------------------- 2 files changed, 29 insertions(+), 40 deletions(-) diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 5947e5c1b..bdd2d9502 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -1125,9 +1125,16 @@ static int ssl_parse_server_dh_params( ssl_context *ssl, unsigned char **p, defined(POLARSSL_KEY_EXCHANGE_ECDH_ECDSA_ENABLED) static int ssl_check_server_ecdh_params( const ssl_context *ssl ) { - // TODO: print name instead - SSL_DEBUG_MSG( 2, ( "ECDH curve size: %d", - (int) ssl->handshake->ecdh_ctx.grp.nbits ) ); + const ecp_curve_info *curve_info; + + curve_info = ecp_curve_info_from_grp_id( ssl->handshake->ecdh_ctx.grp.id ); + if( curve_info == NULL ) + { + SSL_DEBUG_MSG( 1, ( "Should never happen" ) ); + return( -1 ); + } + + SSL_DEBUG_MSG( 2, ( "ECDH curve: %s", curve_info->name ) ); #if defined(POLARSSL_SSL_ECP_SET_CURVES) if( ! ssl_curve_is_acceptable( ssl, ssl->handshake->ecdh_ctx.grp.id ) ) diff --git a/library/ssl_srv.c b/library/ssl_srv.c index 20a6be591..5acb64b96 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -536,7 +536,7 @@ static int ssl_parse_supported_elliptic_curves( ssl_context *ssl, return( POLARSSL_ERR_SSL_BAD_HS_CLIENT_HELLO ); } - /* Don't allow our peer to make use allocated too much memory, + /* Don't allow our peer to make us allocate too much memory, * and leave room for a final 0 */ our_size = list_size / 2 + 1; if( our_size > POLARSSL_ECP_DP_MAX ) @@ -2105,54 +2105,36 @@ static int ssl_write_server_key_exchange( ssl_context *ssl ) * ECPoint public; * } ServerECDHParams; */ - ecp_group_id grp_id; + const ecp_curve_info **curve; #if defined(POLARSSL_SSL_SET_CURVES) - unsigned int pref_idx, curv_idx, found; + const ecp_group_id *gid; - /* Match our preference list against the agreed curves */ - for( pref_idx = 0, found = 0; - ssl->curve_list[pref_idx] != POLARSSL_ECP_DP_NONE; - pref_idx++ ) + /* Match our preference list against the offered curves */ + for( gid = ssl->curve_list; *gid != POLARSSL_ECP_DP_NONE; gid++ ) + for( curve = ssl->handshake->curves; *curve != NULL; curve++ ) + if( (*curve)->grp_id == *gid ) + goto curve_matching_done; + +curve_matching_done: +#else + curve = ssl->handshake->curves; +#endif + + if( *curve == NULL ) { - /* Look through the agreed curve list */ - for( curv_idx = 0; - ssl->handshake->curves[curv_idx] != NULL; - curv_idx++ ) - { - if (ssl->handshake->curves[curv_idx]->grp_id == - ssl->curve_list[pref_idx] ) - { - /* We found our most preferred curve */ - found = 1; - break; - } - } - - /* Exit the search if we have found our curve */ - if( found == 1 ) - break; + SSL_DEBUG_MSG( 1, ( "no matching curve for ECDHE" ) ); + return( POLARSSL_ERR_SSL_NO_CIPHER_CHOSEN ); } - /* - * If we haven't found any allowed / preferred curve, - * ssl->curve_list[pref_idx] will contain POLARSSL_ECP_DP_NONE and - * ecp_use_known_dp() will fail. - */ - grp_id = ssl->curve_list[pref_idx]; -#else - grp_id = ssl->handshake->curves[0]->grp_id; -#endif /* POLARSSL_SSL_SET_CURVES */ + SSL_DEBUG_MSG( 2, ( "ECDHE curve: %s", (*curve)->name ) ); if( ( ret = ecp_use_known_dp( &ssl->handshake->ecdh_ctx.grp, - grp_id ) ) != 0 ) + (*curve)->grp_id ) ) != 0 ) { SSL_DEBUG_RET( 1, "ecp_use_known_dp", ret ); return( ret ); } - SSL_DEBUG_MSG( 2, ( "ECDH curve size: %d", - (int) ssl->handshake->ecdh_ctx.grp.nbits ) ); - if( ( ret = ecdh_make_params( &ssl->handshake->ecdh_ctx, &len, p, SSL_MAX_CONTENT_LEN - n, ssl->f_rng, ssl->p_rng ) ) != 0 ) From f6dc5e1d169111d6443ef376553dfa0a1df1a5d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 6 Feb 2014 10:14:25 +0100 Subject: [PATCH 11/13] Remove temporary debug code --- library/ssl_srv.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/library/ssl_srv.c b/library/ssl_srv.c index 5acb64b96..e045fdc98 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -2616,10 +2616,6 @@ static int ssl_parse_client_key_exchange( ssl_context *ssl ) SSL_DEBUG_ECP( 3, "ECDH: Qp ", &ssl->handshake->ecdh_ctx.Qp ); - SSL_DEBUG_MSG( 0, ( "ECDH: id %d", ssl->handshake->ecdh_ctx.grp.id ) ); - SSL_DEBUG_ECP( 0, "ECDH: Q ", &ssl->handshake->ecdh_ctx.Q ); - SSL_DEBUG_MPI( 0, "ECDH: d ", &ssl->handshake->ecdh_ctx.d ); - if( ( ret = ecdh_calc_secret( &ssl->handshake->ecdh_ctx, &ssl->handshake->pmslen, ssl->handshake->premaster, From f07031aa989ee8e4204687b0bd30d823727c9950 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 6 Feb 2014 10:16:47 +0100 Subject: [PATCH 12/13] debug_ecp: don't print Z, always 1 --- library/debug.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/library/debug.c b/library/debug.c index 371cbf95c..4e674e1e2 100644 --- a/library/debug.c +++ b/library/debug.c @@ -149,10 +149,6 @@ void debug_print_ecp( const ssl_context *ssl, int level, snprintf( str, maxlen, "%s(Y)", text ); str[maxlen] = '\0'; debug_print_mpi( ssl, level, file, line, str, &X->Y ); - - snprintf( str, maxlen, "%s(Z)", text ); - str[maxlen] = '\0'; - debug_print_mpi( ssl, level, file, line, str, &X->Z ); } #endif /* POLARSSL_ECP_C */ From 792657045b89a713df575face0b35c6139897e9d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 6 Feb 2014 10:23:14 +0100 Subject: [PATCH 13/13] Disable ecp_set_curve() for compatibility --- include/polarssl/config.h | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/include/polarssl/config.h b/include/polarssl/config.h index 0a64f6ede..1a68f2a62 100644 --- a/include/polarssl/config.h +++ b/include/polarssl/config.h @@ -823,11 +823,9 @@ * application against the new header files, relinking will not be enough. * It will be enabled by default, or no longer an option, in the 1.4 branch. * - * TODO: actually disable it when done working on this branch ,) - * * Uncomment to make ssl_set_curves() available. */ -#define POLARSSL_SSL_SET_CURVES +//#define POLARSSL_SSL_SET_CURVES /** * \def POLARSSL_THREADING_ALT