[Help-gnutls] Re: Unknown type '0' for SNI: 'foo.domain.bar'

[Simon Josefsson]

Nikos Mavrogiannopoulos <n.mavrogiannopoulos at gmail.com> writes:

> On Tuesday 21 August 2007, Simon Josefsson wrote:
>
>> I found the error message, it is from mod_gnutls.  The cause is a
>> bug... and I'm not sure if it is in mod_gnutls or GnuTLS.  The code in
>> mod_gnutls is:
>>
>>     rv = gnutls_server_name_get(ctxt->session, sni_name,
>>                                 &data_len, &sni_type, 0);
>>
>>     if (rv != 0) {
>>         return NULL;
>>     }
>>
>>     if (sni_type != GNUTLS_NAME_DNS) {
>>         ap_log_error(APLOG_MARK, APLOG_CRIT, 0,
>>                      ctxt->c->base_server,
>>                      "GnuTLS: Unknown type '%d' for SNI: "
>>                      "'%s'", sni_type, sni_name);
>>         return NULL;
>>     }
>>
>> This looks correct, but unfortunately, the value of GNUTLS_NAME_DNS is
>> 1, and the RFC uses the value 0 for this, and that is the value that is
>> returned in the type variable from the gnutls_server_name_get
>> function.

Never mind, my analysis was bogus.  Please don't bother trying my patch.
>
> Hi,
>  It seems that the type is set properly in _gnutls_server_name_recv_params():
>
>           switch (type)  
>             {
>             case 0:             /* NAME_DNS */
>               if (len <= MAX_SERVER_NAME_SIZE)
>                 {
>                   memcpy (session->security_parameters.extensions.
>                           server_names[i].name, p, len);
>                   session->security_parameters.extensions.
>                     server_names[i].name_length = len;
>                   session->security_parameters.extensions.
>                     server_names[i].type = GNUTLS_NAME_DNS;
>                   break;
>                 }
>             }
>
> So this error should be from a case where server name is set and type is not 
> updated for some reason (maybe is left uninitialized because of a long server 
> name?). I'm checking it but so far no clue :)

Yeah, if the code receives a type!=0, the type stored in gnutls will be
0, and no part of the string is stored.  That seems sub-optimal.  How
about this patch?

/Simon

diff --git a/lib/ext_server_name.c b/lib/ext_server_name.c
index f9ca429..2ef7905 100644
--- a/lib/ext_server_name.c
+++ b/lib/ext_server_name.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2002, 2003, 2004, 2005 Free Software Foundation
+ * Copyright (C) 2002, 2003, 2004, 2005, 2007 Free Software Foundation
  *
  * Author: Nikos Mavroyanopoulos
  *
@@ -93,26 +93,22 @@ _gnutls_server_name_recv_params (gnutls_session_t session,
       p = data + 2;
       for (i = 0; i < server_names; i++)
 	{
-	  type = *p;
+	  uint16_t trunc_len;
+
+	  session->security_parameters.extensions.
+	    server_names[i].type = *p + 1;
 	  p++;
 
-	  len = _gnutls_read_uint16 (p);
+	  trunc_len = len = _gnutls_read_uint16 (p);
 	  p += 2;
 
-	  switch (type)
-	    {
-	    case 0:		/* NAME_DNS */
-	      if (len <= MAX_SERVER_NAME_SIZE)
-		{
-		  memcpy (session->security_parameters.extensions.
-			  server_names[i].name, p, len);
-		  session->security_parameters.extensions.
-		    server_names[i].name_length = len;
-		  session->security_parameters.extensions.
-		    server_names[i].type = GNUTLS_NAME_DNS;
-		  break;
-		}
-	    }
+	  if (trunc_len > MAX_SERVER_NAME_SIZE)
+	    trunc_len = MAX_SERVER_NAME_SIZE;
+
+	  memcpy (session->security_parameters.extensions.
+		  server_names[i].name, p, trunc_len);
+	  session->security_parameters.extensions.
+	    server_names[i].name_length = trunc_len;
 
 	  /* move to next record */
 	  p += len;