From: Nikos Mavrogiannopoulos <nmav@gnutls.org>
Date: Tue, 12 Jan 2010 19:17:07 +0000 (+0100)
Subject: When resuming no extensions were parsed thus the safe
X-Git-Tag: gnutls_2_9_10~172
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=5a92d28f74951356705d4e6698b9315980911f87;p=thirdparty%2Fgnutls.git

When resuming no extensions were parsed thus the safe
renegotiation extension was ignored as well causing a
false detection of unsafe session. Corrected by making
a special class of extensions called RESUMED. Those are
parsed even when resuming (normally we don't do it to
prevent clients overwriting capabilities and credentials).
---

diff --git a/lib/gnutls_extensions.c b/lib/gnutls_extensions.c
index 4e9db89e7c..2e8ad07f45 100644
--- a/lib/gnutls_extensions.c
+++ b/lib/gnutls_extensions.c
@@ -324,7 +324,7 @@ _gnutls_ext_init (void)
 
   ret = gnutls_ext_register (GNUTLS_EXTENSION_SAFE_RENEGOTIATION,
 			     "SAFE_RENEGOTIATION",
-			     GNUTLS_EXT_TLS,
+			     GNUTLS_EXT_RESUMED,
 			     _gnutls_safe_renegotiation_recv_params,
 			     _gnutls_safe_renegotiation_send_params);
   if (ret != GNUTLS_E_SUCCESS)
diff --git a/lib/gnutls_handshake.c b/lib/gnutls_handshake.c
index 9830278762..3620a2991a 100644
--- a/lib/gnutls_handshake.c
+++ b/lib/gnutls_handshake.c
@@ -450,6 +450,33 @@ _gnutls_read_client_hello (gnutls_session_t session, opaque * data,
 
   if (ret == 0)
     {				/* resumed! */
+      /* Parse only the safe renegotiation extension
+       * We don't want to parse any other extensions since
+       * we don't want new extension values to overwrite the
+       * resumed ones.
+       */
+       
+      /* move forward to extensions */
+      DECR_LEN (len, 2);
+      suite_size = _gnutls_read_uint16 (&data[pos]);
+      pos += 2;
+
+      DECR_LEN (len, suite_size);
+      pos += suite_size;
+      
+      DECR_LEN (len, 1);
+      comp_size = data[pos++];	/* z is the number of compression methods */
+      DECR_LEN (len, comp_size);
+      pos += comp_size;
+
+      ret = _gnutls_parse_extensions (session, GNUTLS_EXT_RESUMED,
+				  &data[pos], len);
+      if (ret < 0)
+        {
+          gnutls_assert ();
+          return ret;
+        }
+
       resume_copy_required_values (session);
       session->internals.resumed = RESUME_TRUE;
       
@@ -504,9 +531,16 @@ _gnutls_read_client_hello (gnutls_session_t session, opaque * data,
       return ret;
     }
 
+  ret = _gnutls_parse_extensions (session, GNUTLS_EXT_RESUMED,
+				  &data[pos], len);
+  if (ret < 0)
+    {
+      gnutls_assert ();
+      return ret;
+    }
+
   ret = _gnutls_parse_extensions (session, GNUTLS_EXT_TLS,
 				  &data[pos], len);
-  /* len is the rest of the parsed length */
   if (ret < 0)
     {
       gnutls_assert ();
@@ -1718,9 +1752,21 @@ _gnutls_read_server_hello (gnutls_session_t session,
    */
   if (_gnutls_client_check_if_resuming
       (session, &data[pos], session_id_len) == 0)
-    return 0;
-  pos += session_id_len;
+    {
+      pos += session_id_len + 2 + 1;
+      DECR_LEN (len, 2+1);
+
+      ret = _gnutls_parse_extensions (session, GNUTLS_EXT_RESUMED,
+				  &data[pos], len);
+      if (ret < 0)
+        {
+          gnutls_assert ();
+          return ret;
+        }
+      return 0;
+    }
 
+  pos += session_id_len;
 
   /* Check if the given cipher suite is supported and copy
    * it to the session.
@@ -1752,7 +1798,6 @@ _gnutls_read_server_hello (gnutls_session_t session,
    */
   ret = _gnutls_parse_extensions (session, GNUTLS_EXT_ANY,
 				  &data[pos], len);
-  /* len is the rest of the parsed length */
   if (ret < 0)
     {
       gnutls_assert ();
diff --git a/lib/includes/gnutls/gnutls.h.in b/lib/includes/gnutls/gnutls.h.in
index cbcf493332..ea691e6667 100644
--- a/lib/includes/gnutls/gnutls.h.in
+++ b/lib/includes/gnutls/gnutls.h.in
@@ -511,7 +511,8 @@ extern "C" {
     {
       GNUTLS_EXT_ANY,
       GNUTLS_EXT_APPLICATION,
-      GNUTLS_EXT_TLS
+      GNUTLS_EXT_TLS,
+      GNUTLS_EXT_RESUMED, /* parse even if resuming */
     } gnutls_ext_parse_type_t;
 
   int gnutls_ext_register (int type,
diff --git a/tests/resume.c b/tests/resume.c
index c52c1d6bed..80a4c7d82e 100644
--- a/tests/resume.c
+++ b/tests/resume.c
@@ -58,6 +58,8 @@ struct params_res
   int expect_resume;
 };
 
+pid_t child;
+
 struct params_res resume_tests[] = {
   {"try to resume from db", 50, 0, 0, 1},
 #ifdef ENABLE_SESSION_TICKET
@@ -74,6 +76,12 @@ struct params_res resume_tests[] = {
 #define MAX_BUF 1024
 #define MSG "Hello TLS"
 
+static void
+tls_log_func (int level, const char *str)
+{
+  fprintf (stderr, "%s |<%d>| %s", child ? "server" : "client", level, str);
+}
+
 static void
 client (struct params_res *params)
 {
@@ -89,6 +97,11 @@ client (struct params_res *params)
   int t;
   gnutls_datum_t session_data;
 
+  if (debug)
+    {
+      gnutls_global_set_log_function (tls_log_func);
+      gnutls_global_set_log_level (4);
+    }
   gnutls_global_init ();
 
   gnutls_anon_allocate_client_credentials (&anoncred);
@@ -336,8 +349,13 @@ server (struct params_res *params)
 
   /* this must be called once in the program, it is mostly for the server.
    */
-  gnutls_global_init ();
+  if (debug)
+    {
+      gnutls_global_set_log_function (tls_log_func);
+      gnutls_global_set_log_level (4);
+    }
 
+  gnutls_global_init ();
   gnutls_anon_allocate_server_credentials (&anoncred);
 
   success ("Launched, generating DH parameters...\n");
@@ -430,7 +448,6 @@ server (struct params_res *params)
 void
 doit (void)
 {
-  pid_t child;
   int i;
 
   for (i = 0; resume_tests[i].desc; i++)