When a TLS session is resumed, t1_lib.c:ssl_parse_clienthello_tlsext()
currently disregards an SNI extension which is potentially
included in the ClientHello, because the following if condition is false:

        switch (servname_type)
                {
        case TLSEXT_NAMETYPE_host_name:
                if (s->session->tlsext_hostname == NULL)
                        {
                        [...ClientHello data is parsed here...]

I.e., OpenSSL will use the servername as retrieved from the
existing session (s->session->tlsext_hostname), and will leave
s->tlsext_hostname unset. With mod_ssl in Apache 2.2.12 and later, this
has an unfortunate effect when a client tries to resume a session with
the same host, but a different host name. Let's assume a configuration like

  <VirtualHost *:443>
    ServerName www.example.org
    ServerAlias www.example.net
    [...]
  </VirtualHost>

and a server certificate with two subjectAltName entries, one for
www.example.org and one for www.example.net.

A client first connecting to www.example.org and then to
www.example.net has legitimate reasons for reusing the session - it's
the same IP address and port, and the client has a certificate (received
during the initial handshake) which is valid for both host names. In
fact, this is exactly what Mozilla NSS is doing by default. [1]

Now, as mentioned above, this has an unfortunate consequence in case of
Apache/mod_ssl: OpenSSL discards the server name sent in the second
ClientHello (www.example.net), and SSL_get_servername() will only report
the host name of the initial connection (i.e. www.example.org) to the
application (mod_ssl, in this case). Apache/mod_ssl will therefore
reject the request because [OpenSSL makes it believe that] the Host:
header and the SNI extension do not match. The client will receive "400
Bad Request", and Apache will log "Hostname www.example.org provided via
SNI and hostname www.example.net provided via HTTP are different".

This problem can be solved by optimizing ssl_parse_clienthello_tlsext(),
I think. The proposed patch (attached to this message) slightly changes
the server-side handling of the SNI extension at session resumption: if
s->session->tlsext_hostname is set, but s->tlsext_hostname is unset
(i.e., we are resuming a session, and the application did not specify a
"desirable" servername), then the content of the SNI extension is stored
in s->tlsext_hostname. Note that even with the patch applied, the
s->tlsext_hostname value set by the application (through
SSL_set_tlsext_host_name) will never be overwritten, so there is no
change to the current behavior in this situation.

There is currently one browser which can be used to reproduce the
problem: Chrome/Chromium, when running under Linux (only on this
platform, since only in this case Mozilla NSS is used for TLS [2]). A
publicly available test site is https://alice.sni.velox.ch - if followed
by a request to https://carol.sni.velox.ch, the Linux version of
Chrome/Chromium will trigger the "400 Bad Request" result.

Would it possible to get such a fix into 1.0.0 (and backported to
0.9.8)...? I don't think the patch introduces an insecure way of
handling the SNI extension, but am of course interested in opinions of
experts in this area of OpenSSL's code (Peter?).

Kaspar

[1]
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/security/nss/lib/ssl/sslnonce.c&rev=1.25&mark=310-314#267

[2] Note that Firefox and Seamonkey don't exhibit this problem because
they use NSS in a slightly different way.



Index: ssl/t1_lib.c
===================================================================
RCS file: /openssl-cvsrepo/openssl/ssl/t1_lib.c,v
retrieving revision 1.65
diff -p -u -r1.65 t1_lib.c
--- ssl/t1_lib.c        28 Apr 2009 22:10:54 -0000      1.65
+++ ssl/t1_lib.c        23 Aug 2009 06:23:36 -0000
@@ -601,28 +601,27 @@ int ssl_parse_clienthello_tlsext(SSL *s,
 
    - Only the hostname type is supported with a maximum length of 255.
    - The servername is rejected if too long or if it contains zeros,
-     in which case an fatal alert is generated.
+     in which case a fatal alert is generated.
    - The servername field is maintained together with the session cache.
-   - When a session is resumed, the servername call back invoked in order
-     to allow the application to position itself to the right context. 
-   - The servername is acknowledged if it is new for a session or when 
-     it is identical to a previously used for the same session. 
-     Applications can control the behaviour.  They can at any time
-     set a 'desirable' servername for a new SSL object. This can be the
+   - When a session is started or resumed, the servername callback is invoked
+     in order to allow the application to position itself to the right 
context. 
+   - Applications can control session resumption behavior. They can at any
+     time set a 'desirable' servername for a new SSL object. This can be the
      case for example with HTTPS when a Host: header field is received and
      a renegotiation is requested. In this case, a possible servername
      presented in the new client hello is only acknowledged if it matches
      the value of the Host: field. 
-   - Applications must  use SSL_OP_NO_SESSION_RESUMPTION_ON_RENEGOTIATION
+   - Applications must use SSL_OP_NO_SESSION_RESUMPTION_ON_RENEGOTIATION
      if they provide for changing an explicit servername context for the 
session,
      i.e. when the session has been established with a servername extension. 
-   - On session reconnect, the servername extension may be absent. 
+   - On session resumption, the servername extension may be absent. 
 
 */      
 
                if (type == TLSEXT_TYPE_server_name)
                        {
                        unsigned char *sdata;
+                       char **q;
                        int servname_type;
                        int dsize; 
                
@@ -655,19 +654,21 @@ int ssl_parse_clienthello_tlsext(SSL *s,
                                switch (servname_type)
                                        {
                                case TLSEXT_NAMETYPE_host_name:
-                                       if (s->session->tlsext_hostname == NULL)
+                                       q = s->session->tlsext_hostname ?
+                                           &s->tlsext_hostname : 
&s->session->tlsext_hostname;
+                                       if (*q == NULL)
                                                {
                                                if (len > 
TLSEXT_MAXLEN_host_name || 
-                                                       
((s->session->tlsext_hostname = OPENSSL_malloc(len+1)) == NULL))
+                                                       ((*q = 
OPENSSL_malloc(len+1)) == NULL))
                                                        {
                                                        *al = 
TLS1_AD_UNRECOGNIZED_NAME;
                                                        return 0;
                                                        }
-                                               
memcpy(s->session->tlsext_hostname, sdata, len);
-                                               
s->session->tlsext_hostname[len]='\0';
-                                               if 
(strlen(s->session->tlsext_hostname) != len) {
-                                                       
OPENSSL_free(s->session->tlsext_hostname);
-                                                       
s->session->tlsext_hostname = NULL;
+                                               memcpy(*q, sdata, len);
+                                               (*q)[len]='\0';
+                                               if (strlen(*q) != len) {
+                                                       OPENSSL_free(*q);
+                                                       *q = NULL;
                                                        *al = 
TLS1_AD_UNRECOGNIZED_NAME;
                                                        return 0;
                                                }






Reply via email to