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 4551e8069a5c119ae0d10eabe3588595bbaf3c77..4b639957970aa6120af4afbf85a84fef838dded8 100644 (file)
--- 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 d3fd25646bad3a599d2a0d7df5cfd6858dfe68af..78a38cc1f83f6234b664d7d0ec4d699ec52ee8dc 100644 (file)
--- 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 2e8ad07f45b7be7ad337ff9d045e9b0be852a635..24b3f4d6c47f2405d87af4224894651d2c450276 100644 (file)
--- 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 8188b8db1d2631279bf77cf30dbc244a5a9bc3a3..9ea7bd26d8f29bc9740de04e9ae329f8cf7b0a47 100644 (file)
--- a/lib/gnutls_handshake.c
+++ b/lib/gnutls_handshake.c
@@ -49,6 +49,7 @@
 #include <gnutls_state.h>
 #include <ext_srp.h>
 #include <ext_session_ticket.h>
+#include <ext_safe_renegotiation.h>
 #include <gnutls_rsa_export.h> /* for gnutls_get_rsa_params() */
 #include <auth_anon.h>         /* for gnutls_anon_server_credentials_t */
 #include <auth_psk.h>          /* 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 ba8ed9b6265349eac473d7f9bcf23d775631ac89..fec71a7fef174bd8bc4079ef2f2076696bec24ed 100644 (file)
--- 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 3ba533b3c68759641ff735d31c723c153ed8e2e4..0545757aec97becad7ab836e91ccea9d3251f24e 100644 (file)
--- 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 80a4c7d82e8404a9fdbe81d21a58dc5459f51f04..cf26669be0e03f1ee89bf443aa32d60e410a148d 100644 (file)
--- 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 5f1645494e0ed729d2590ceee4e4c45b05381434..a3774e5d6eebdf644b5715267519ad78a23730ba 100644 (file)
--- 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])));