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

[Simon Josefsson]

Nikos Mavrogiannopoulos <address@hidden> 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;