From: Nikos Mavrogiannopoulos Date: Wed, 13 Jan 2010 19:38:26 +0000 (+0100) Subject: Specify in detail what to be copied when resuming. It seems there X-Git-Tag: gnutls_2_9_10~165 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=4160e1087f3ca75a306bd6afab68f45a0ef97124;p=thirdparty%2Fgnutls.git Specify in detail what to be copied when resuming. It seems there are extensions (like safe renegotiation) that do not need to read the stored values. Moreover this might overcome any bugs by the extensions that used to store pointers in the extension structure. --- diff --git a/lib/ext_safe_renegotiation.h b/lib/ext_safe_renegotiation.h index 4551e8069a..4b63995797 100644 --- a/lib/ext_safe_renegotiation.h +++ b/lib/ext_safe_renegotiation.h @@ -22,7 +22,12 @@ * */ +#ifndef EXT_SAFE_RENEGOTIATION_H +# define EXT_SAFE_RENEGOTIATION_H + int _gnutls_safe_renegotiation_recv_params (gnutls_session_t state, const opaque * data, size_t data_size); int _gnutls_safe_renegotiation_send_params (gnutls_session_t state, opaque * data, size_t); + +#endif /* EXT_SAFE_RENEGOTIATION_H */ diff --git a/lib/gnutls_constate.c b/lib/gnutls_constate.c index d3fd25646b..78a38cc1f8 100644 --- a/lib/gnutls_constate.c +++ b/lib/gnutls_constate.c @@ -380,6 +380,19 @@ _gnutls_set_write_keys (gnutls_session_t session) export_flag); } +#define CPY_EXTENSIONS \ + memcpy(dst->extensions.server_names, src->extensions.server_names, sizeof(src->extensions.server_names)); \ + dst->extensions.server_names_size = src->extensions.server_names_size; \ + memcpy(dst->extensions.srp_username, src->extensions.srp_username, sizeof(src->extensions.srp_username)); \ + memcpy(dst->extensions.sign_algorithms, src->extensions.sign_algorithms, sizeof(src->extensions.sign_algorithms)); \ + dst->extensions.sign_algorithms_size = src->extensions.sign_algorithms_size; \ + dst->extensions.gnutls_ia_enable = src->extensions.gnutls_ia_enable; \ + dst->extensions.gnutls_ia_peer_enable = src->extensions.gnutls_ia_peer_enable; \ + dst->extensions.gnutls_ia_allowskip = src->extensions.gnutls_ia_allowskip; \ + dst->extensions.gnutls_ia_peer_allowskip = src->extensions.gnutls_ia_peer_allowskip; \ + dst->extensions.do_recv_supplemental = src->extensions.do_recv_supplemental; \ + dst->extensions.do_send_supplemental = src->extensions.do_send_supplemental + #define CPY_COMMON dst->entity = src->entity; \ dst->kx_algorithm = src->kx_algorithm; \ memcpy( &dst->current_cipher_suite, &src->current_cipher_suite, sizeof(cipher_suite_st)); \ @@ -393,8 +406,8 @@ _gnutls_set_write_keys (gnutls_session_t session) dst->max_record_recv_size = src->max_record_recv_size; \ dst->max_record_send_size = src->max_record_send_size; \ dst->version = src->version; \ - memcpy( &dst->extensions, &src->extensions, sizeof(tls_ext_st)); \ - memcpy( &dst->inner_secret, &src->inner_secret, GNUTLS_MASTER_SIZE); + CPY_EXTENSIONS; \ + memcpy( &dst->inner_secret, &src->inner_secret, GNUTLS_MASTER_SIZE) static void _gnutls_cpy_read_security_parameters (security_parameters_st * @@ -486,12 +499,9 @@ _gnutls_read_connection_state_init (gnutls_session_t session) } else { /* RESUME_TRUE */ - opaque *session_ticket = - session->security_parameters.extensions.session_ticket; _gnutls_cpy_read_security_parameters (&session->security_parameters, &session->internals. resumed_security_parameters); - session->security_parameters.extensions.session_ticket = session_ticket; } @@ -671,12 +681,9 @@ _gnutls_write_connection_state_init (gnutls_session_t session) } else { /* RESUME_TRUE */ - opaque *session_ticket = - session->security_parameters.extensions.session_ticket; _gnutls_cpy_write_security_parameters (&session->security_parameters, &session->internals. resumed_security_parameters); - session->security_parameters.extensions.session_ticket = session_ticket; } rc = _gnutls_set_write_keys (session); diff --git a/lib/gnutls_extensions.c b/lib/gnutls_extensions.c index 2e8ad07f45..24b3f4d6c4 100644 --- a/lib/gnutls_extensions.c +++ b/lib/gnutls_extensions.c @@ -148,7 +148,7 @@ _gnutls_parse_extensions (gnutls_session_t session, type = _gnutls_read_uint16 (&data[pos]); pos += 2; - _gnutls_debug_log ("EXT[%p]: Received extension '%s/%d'\n", session, + _gnutls_debug_log ("EXT[%p]: Found extension '%s/%d'\n", session, _gnutls_extension_get_name (type), type); if ((ret = _gnutls_extension_list_check (session, type)) < 0) @@ -168,6 +168,8 @@ _gnutls_parse_extensions (gnutls_session_t session, ext_recv = _gnutls_ext_func_recv (type, parse_type); if (ext_recv == NULL) continue; + + if ((ret = ext_recv (session, sdata, size)) < 0) { gnutls_assert (); diff --git a/lib/gnutls_handshake.c b/lib/gnutls_handshake.c index 8188b8db1d..9ea7bd26d8 100644 --- a/lib/gnutls_handshake.c +++ b/lib/gnutls_handshake.c @@ -49,6 +49,7 @@ #include #include #include +#include #include /* for gnutls_get_rsa_params() */ #include /* for gnutls_anon_server_credentials_t */ #include /* for gnutls_psk_server_credentials_t */ @@ -106,6 +107,9 @@ _gnutls_handshake_hash_buffers_clear (gnutls_session_t session) static void resume_copy_required_values (gnutls_session_t session) { + tls_ext_st *newext; + tls_ext_st *resext; + /* get the new random values */ memcpy (session->internals.resumed_security_parameters.server_random, session->security_parameters.server_random, GNUTLS_RANDOM_SIZE); @@ -143,13 +147,13 @@ resume_copy_required_values (gnutls_session_t session) session->internals.resumed_security_parameters.session_id_size; /* safe renegotiation */ - tls_ext_st *newext = &session->security_parameters.extensions; - tls_ext_st *resext = &session->internals.resumed_security_parameters.extensions; + newext = &session->security_parameters.extensions; + resext = &session->internals.resumed_security_parameters.extensions; newext->connection_using_safe_renegotiation = resext->connection_using_safe_renegotiation; - newext->initial_negotiation_completed = TRUE; + session->internals.initial_negotiation_completed = TRUE; newext->client_verify_data_len = resext->client_verify_data_len; memcpy (newext->client_verify_data, resext->client_verify_data, @@ -743,6 +747,7 @@ _gnutls_recv_finished (gnutls_session_t session) int data_size; int ret; int vrfysize; + tls_ext_st *ext; ret = _gnutls_recv_handshake (session, &vrfy, &vrfysize, @@ -807,7 +812,7 @@ _gnutls_recv_finished (gnutls_session_t session) return GNUTLS_E_UNEXPECTED_PACKET_LENGTH; } - tls_ext_st *ext = &session->security_parameters.extensions; + ext = &session->security_parameters.extensions; if (session->security_parameters.entity == GNUTLS_CLIENT) { @@ -820,7 +825,7 @@ _gnutls_recv_finished (gnutls_session_t session) ext->client_verify_data_len = data_size; } - ext->initial_negotiation_completed = 1; + session->internals.initial_negotiation_completed = 1; return ret; } @@ -887,7 +892,7 @@ _gnutls_server_select_suite (gnutls_session_t session, opaque * data, data[offset+1] == GNUTLS_RENEGO_PROTECTION_REQUEST_MINOR) { _gnutls_handshake_log ("HSK[%p]: Received safe renegotiation CS\n", session); - session->security_parameters.extensions.safe_renegotiation_received = 1; + session->internals.safe_renegotiation_received = 1; session->security_parameters.extensions.connection_using_safe_renegotiation = 1; break; } @@ -1673,6 +1678,8 @@ _gnutls_client_check_if_resuming (gnutls_session_t session, opaque * session_id, int session_id_len) { opaque buf[2 * TLS_MAX_SESSION_ID_SIZE + 1]; + tls_ext_st *newext; + tls_ext_st *resext; _gnutls_handshake_log ("HSK[%p]: SessionID length: %d\n", session, session_id_len); @@ -1695,13 +1702,13 @@ _gnutls_client_check_if_resuming (gnutls_session_t session, session->internals.resumed = RESUME_TRUE; /* we are resuming */ /* safe renegotiation after resumption */ - tls_ext_st *newext = &session->security_parameters.extensions; - tls_ext_st *resext = &session->internals.resumed_security_parameters.extensions; + newext = &session->security_parameters.extensions; + resext = &session->internals.resumed_security_parameters.extensions; newext->connection_using_safe_renegotiation = resext->connection_using_safe_renegotiation; - newext->initial_negotiation_completed = TRUE; + session->internals.initial_negotiation_completed = TRUE; newext->client_verify_data_len = resext->client_verify_data_len; memcpy (newext->client_verify_data, resext->client_verify_data, @@ -1780,7 +1787,6 @@ _gnutls_read_server_hello (gnutls_session_t session, } DECR_LEN (len, session_id_len); - /* check if we are resuming and set the appropriate * values; */ @@ -1815,8 +1821,6 @@ _gnutls_read_server_hello (gnutls_session_t session, } pos += 2; - - /* move to compression */ DECR_LEN (len, 1); @@ -1855,6 +1859,7 @@ _gnutls_copy_ciphersuites (gnutls_session_t session, cipher_suite_st *cipher_suites; uint16_t cipher_num; int datalen, pos; + uint16_t loop_max; ret = _gnutls_supported_ciphersuites_sorted (session, &cipher_suites); if (ret < 0) @@ -1905,7 +1910,7 @@ _gnutls_copy_ciphersuites (gnutls_session_t session, _gnutls_write_uint16 (cipher_num, ret_data); pos += 2; - uint16_t loop_max = add_scsv ? cipher_num - 2 : cipher_num; + loop_max = add_scsv ? cipher_num - 2 : cipher_num; for (i = 0; i < (loop_max / 2); i++) { @@ -2099,7 +2104,7 @@ _gnutls_send_client_hello (gnutls_session_t session, int again) * prevention on initial negotiation (but not renegotiation; that's * handled with the RI extension below). */ - if(!session->security_parameters.extensions.initial_negotiation_completed && + if(!session->internals.initial_negotiation_completed && session->security_parameters.entity == GNUTLS_CLIENT && gnutls_protocol_get_version (session) == GNUTLS_SSL3) { @@ -2190,8 +2195,10 @@ _gnutls_send_client_hello (gnutls_session_t session, int again) return ret; } } - else if(session->security_parameters.extensions.initial_negotiation_completed) + else if(session->internals.initial_negotiation_completed != 0) { + opaque buf[256]; /* opaque renegotiated_connection<0..255> */ + /* For SSLv3 only, we will (only) to send the RI extension; we must * send it every time we renegotiate. We don't want to send anything * else, out of concern for interoperability. @@ -2199,7 +2206,6 @@ _gnutls_send_client_hello (gnutls_session_t session, int again) * If this is an initial negotiation, we already sent SCSV above. */ - opaque buf[256]; /* opaque renegotiated_connection<0..255> */ ret = _gnutls_safe_renegotiation_send_params (session, buf, sizeof(buf)); if (ret < 0) @@ -2362,7 +2368,7 @@ _gnutls_send_hello (gnutls_session_t session, int again) { int ret; - session->security_parameters.extensions.safe_renegotiation_received = 0; + session->internals.safe_renegotiation_received = 0; if (session->security_parameters.entity == GNUTLS_CLIENT) { @@ -2385,6 +2391,7 @@ int _gnutls_recv_hello (gnutls_session_t session, opaque * data, int datalen) { int ret; + tls_ext_st *ext; if (session->security_parameters.entity == GNUTLS_CLIENT) { @@ -2407,9 +2414,9 @@ _gnutls_recv_hello (gnutls_session_t session, opaque * data, int datalen) } /* Safe renegotiation */ - tls_ext_st *ext = &session->security_parameters.extensions; + ext = &session->security_parameters.extensions; - if (ext->safe_renegotiation_received) + if (session->internals.safe_renegotiation_received) { if ((ext->ri_extension_data_len < ext->client_verify_data_len) || (memcmp (ext->ri_extension_data, @@ -2417,7 +2424,7 @@ _gnutls_recv_hello (gnutls_session_t session, opaque * data, int datalen) ext->client_verify_data_len))) { gnutls_assert(); - _gnutls_handshake_log ("Safe renegotiation failed (1)\n"); + _gnutls_handshake_log ("Safe renegotiation failed [1]\n"); return GNUTLS_E_SAFE_RENEGOTIATION_FAILED; } if (session->security_parameters.entity == GNUTLS_CLIENT) @@ -2425,10 +2432,10 @@ _gnutls_recv_hello (gnutls_session_t session, opaque * data, int datalen) if ((ext->ri_extension_data_len != ext->client_verify_data_len + ext->server_verify_data_len) || memcmp (ext->ri_extension_data + ext->client_verify_data_len, - ext->server_verify_data, ext->server_verify_data_len)) + ext->server_verify_data, ext->server_verify_data_len) != 0) { gnutls_assert(); - _gnutls_handshake_log ("Safe renegotiation failed (2)\n"); + _gnutls_handshake_log ("Safe renegotiation failed [2]\n"); return GNUTLS_E_SAFE_RENEGOTIATION_FAILED; } } @@ -2437,7 +2444,7 @@ _gnutls_recv_hello (gnutls_session_t session, opaque * data, int datalen) if (ext->ri_extension_data_len != ext->client_verify_data_len) { gnutls_assert(); - _gnutls_handshake_log ("Safe renegotiation failed (3)\n"); + _gnutls_handshake_log ("Safe renegotiation failed [3]\n"); return GNUTLS_E_SAFE_RENEGOTIATION_FAILED; } } @@ -2454,7 +2461,7 @@ _gnutls_recv_hello (gnutls_session_t session, opaque * data, int datalen) } /* Clients can't tell if it's an initial negotiation */ - if (ext->initial_negotiation_completed || + if (session->internals.initial_negotiation_completed || session->security_parameters.entity == GNUTLS_CLIENT) { if (session->internals.priorities.unsafe_renegotiation != 0) diff --git a/lib/gnutls_int.h b/lib/gnutls_int.h index ba8ed9b626..fec71a7fef 100644 --- a/lib/gnutls_int.h +++ b/lib/gnutls_int.h @@ -309,6 +309,9 @@ struct gnutls_session_ticket_key_st { #define MAX_VERIFY_DATA_SIZE 36 /* in SSL 3.0, 12 in TLS 1.0 */ +/* If you want the extension data to be kept across resuming sessions + * then modify CPY_EXTENSIONS in gnutls_constate.c + */ typedef struct { server_name_st server_names[MAX_SERVER_NAME_EXTENSIONS]; @@ -328,6 +331,9 @@ typedef struct /* Used by extensions that enable supplemental data. */ int do_recv_supplemental, do_send_supplemental; + /*** Those below do not get copied when resuming session + ***/ + /* Opaque PRF input. */ gnutls_oprfi_callback_func oprfi_cb; void *oprfi_userdata; @@ -343,9 +349,6 @@ typedef struct opaque session_ticket_IV[SESSION_TICKET_IV_SIZE]; /* Safe renegotiation. */ - int connection_using_safe_renegotiation:1; - int safe_renegotiation_received:1; - int initial_negotiation_completed:1; uint8_t client_verify_data[MAX_VERIFY_DATA_SIZE]; size_t client_verify_data_len; uint8_t server_verify_data[MAX_VERIFY_DATA_SIZE]; @@ -353,6 +356,8 @@ typedef struct uint8_t ri_extension_data[MAX_VERIFY_DATA_SIZE*2]; /* max signal is 72 bytes in s->c sslv3 */ size_t ri_extension_data_len; + int connection_using_safe_renegotiation:1; + } tls_ext_st; /* auth_info_t structures now MAY contain malloced @@ -366,7 +371,7 @@ typedef struct */ /* if you add anything in Security_Parameters struct, then - * also modify CPY_COMMON in gnutls_constate.c + * also modify CPY_COMMON in gnutls_constate.c. */ /* Note that the security parameters structure is set up after the @@ -411,6 +416,7 @@ typedef struct /* holds the negotiated certificate type */ gnutls_certificate_type_t cert_type; gnutls_protocol_t version; /* moved here */ + /* For TLS/IA. XXX: Move to IA credential? */ opaque inner_secret[GNUTLS_MASTER_SIZE]; } security_parameters_st; @@ -740,6 +746,9 @@ typedef struct int session_ticket_enable, session_ticket_renew; + int safe_renegotiation_received:1; + int initial_negotiation_completed:1; + /* If you add anything here, check _gnutls_handshake_internal_state_clear(). */ } internals_st; diff --git a/lib/gnutls_state.c b/lib/gnutls_state.c index 3ba533b3c6..0545757aec 100644 --- a/lib/gnutls_state.c +++ b/lib/gnutls_state.c @@ -226,6 +226,8 @@ _gnutls_handshake_internal_state_init (gnutls_session_t session) session->internals.adv_version_minor = 0; session->internals.adv_version_minor = 0; session->internals.direction = 0; + session->internals.safe_renegotiation_received = 0; + session->internals.initial_negotiation_completed = 0; /* use out of band data for the last * handshake messages received. diff --git a/tests/resume.c b/tests/resume.c index 80a4c7d82e..cf26669be0 100644 --- a/tests/resume.c +++ b/tests/resume.c @@ -100,7 +100,7 @@ client (struct params_res *params) if (debug) { gnutls_global_set_log_function (tls_log_func); - gnutls_global_set_log_level (4); + gnutls_global_set_log_level (2); } gnutls_global_init (); @@ -178,7 +178,7 @@ client (struct params_res *params) if (params->expect_resume) fail ("*** Previous session was NOT resumed\n"); else - success ("*** Previous session was NOT resumed\n"); + success ("*** Previous session was NOT resumed (expected)\n"); } } @@ -352,7 +352,7 @@ server (struct params_res *params) if (debug) { gnutls_global_set_log_function (tls_log_func); - gnutls_global_set_log_level (4); + gnutls_global_set_log_level (2); } gnutls_global_init (); diff --git a/tests/simple.c b/tests/simple.c index 5f1645494e..a3774e5d6e 100644 --- a/tests/simple.c +++ b/tests/simple.c @@ -50,7 +50,7 @@ doit (void) for (i = 0; algs[i]; i++) { - printf ("pk_list[%d] = %d = %s = %d\n", i, algs[i], + printf ("pk_list[%d] = %d = %s = %d\n", (int)i, algs[i], gnutls_pk_algorithm_get_name (algs[i]), gnutls_pk_get_id (gnutls_pk_algorithm_get_name (algs[i]))); if (gnutls_pk_get_id (gnutls_pk_algorithm_get_name (algs[i])) @@ -76,7 +76,7 @@ doit (void) for (i = 0; algs[i]; i++) { - printf ("sign_list[%d] = %d = %s = %d\n", i, algs[i], + printf ("sign_list[%d] = %d = %s = %d\n", (int)i, algs[i], gnutls_sign_algorithm_get_name (algs[i]), gnutls_sign_get_id (gnutls_sign_algorithm_get_name (algs[i])));