Re: TLS renegociation bug: time for OpenSSL upgrade?

[Manuel Bouyer <bouyer%antioche.eu.org@localhost>]

On Thu, Apr 01, 2010 at 04:02:53PM +1100, Luke Mewburn wrote:
> At the firefox client end; yes.
> 
> At the server end; I'm not sure if disabling TLSv1 in apache2
> avoids the problem. 
> 
> IMHO, it is not acceptable that a remote client can cause a core dump
> in a server application, or library that the latter uses...

Can you see if the attached patch fixes your problem ?
It does for a similar issue with freeradius for me.
I reported this to openssl, but their anserw so far has been "try 1.0.0,
this may have been fixed". I didn't see anything in the changelog or
sources that would confirm it's fixed.

-- 
Manuel Bouyer <bouyer%antioche.eu.org@localhost>
     NetBSD: 26 ans d'experience feront toujours la difference
--

Index: ssl/s3_enc.c
===================================================================
RCS file: /cvsroot/src/crypto/dist/openssl/ssl/Attic/s3_enc.c,v
retrieving revision 1.1.1.12
diff -u -p -u -r1.1.1.12 s3_enc.c
--- ssl/s3_enc.c        9 May 2008 21:34:44 -0000       1.1.1.12
+++ ssl/s3_enc.c        31 Mar 2010 10:49:02 -0000
@@ -570,7 +570,7 @@ void ssl3_finish_mac(SSL *s, const unsig
                {
                BIO_write (s->s3->handshake_buffer,(void *)buf,len);
                } 
-       else 
+       else if (s->s3->handshake_dgst != NULL)
                {
                int i;
                for (i=0;i< SSL_MAX_DIGEST;i++) 
Index: ssl/s3_srvr.c
===================================================================
RCS file: /cvsroot/src/crypto/dist/openssl/ssl/Attic/s3_srvr.c,v
retrieving revision 1.15.4.2
diff -u -p -u -r1.15.4.2 s3_srvr.c
--- ssl/s3_srvr.c       12 Jan 2010 09:07:51 -0000      1.15.4.2
+++ ssl/s3_srvr.c       31 Mar 2010 10:49:02 -0000
@@ -532,12 +532,14 @@ int ssl3_accept(SSL *s)
                                 */
                                if (s->s3->handshake_buffer)
                                        ssl3_digest_cached_records(s);
-                               for (dgst_num=0; 
dgst_num<SSL_MAX_DIGEST;dgst_num++)    
-                                       if (s->s3->handshake_dgst[dgst_num]) 
-                                               {
-                                               
s->method->ssl3_enc->cert_verify_mac(s,EVP_MD_CTX_type(s->s3->handshake_dgst[dgst_num]),&(s->s3->tmp.cert_verify_md[offset]));
-                                               
offset+=EVP_MD_CTX_size(s->s3->handshake_dgst[dgst_num]);
-                                               }               
+                               if (s->s3->handshake_dgst != NULL) {
+                                       for (dgst_num=0; 
dgst_num<SSL_MAX_DIGEST;dgst_num++)    
+                                               if 
(s->s3->handshake_dgst[dgst_num]) 
+                                                       {
+                                                       
s->method->ssl3_enc->cert_verify_mac(s,EVP_MD_CTX_type(s->s3->handshake_dgst[dgst_num]),&(s->s3->tmp.cert_verify_md[offset]));
+                                                       
offset+=EVP_MD_CTX_size(s->s3->handshake_dgst[dgst_num]);
+                                                       }               
+                                       }
                                }
                        break;
 
Index: ssl/t1_enc.c
===================================================================
RCS file: /cvsroot/src/crypto/dist/openssl/ssl/Attic/t1_enc.c,v
retrieving revision 1.1.1.12
diff -u -p -u -r1.1.1.12 t1_enc.c
--- ssl/t1_enc.c        9 May 2008 21:34:46 -0000       1.1.1.12
+++ ssl/t1_enc.c        31 Mar 2010 10:49:02 -0000
@@ -750,14 +750,16 @@ int tls1_cert_verify_mac(SSL *s, int md_
 
        if (s->s3->handshake_buffer) 
                ssl3_digest_cached_records(s);
-       for (i=0;i<SSL_MAX_DIGEST;i++) 
-               {
-                 if 
(s->s3->handshake_dgst[i]&&EVP_MD_CTX_type(s->s3->handshake_dgst[i])==md_nid) 
-                       {
-                       d=s->s3->handshake_dgst[i];
-                       break;
+       if (s->s3->handshake_dgst) {
+               for (i=0;i<SSL_MAX_DIGEST;i++) 
+                       {
+                         if 
(s->s3->handshake_dgst[i]&&EVP_MD_CTX_type(s->s3->handshake_dgst[i])==md_nid) 
+                               {
+                               d=s->s3->handshake_dgst[i];
+                               break;
+                               }
                        }
-               }
+       }
        if (!d) {
                SSLerr(SSL_F_TLS1_CERT_VERIFY_MAC,SSL_R_NO_REQUIRED_DIGEST);
                return 0;
@@ -794,7 +796,8 @@ int tls1_final_finish_mac(SSL *s,
                if (mask & s->s3->tmp.new_cipher->algorithm2)
                        {
                        unsigned int hashsize = EVP_MD_size(md);
-                       if (hashsize > (sizeof buf - (size_t)(q-buf)))
+                       if (hashsize > (sizeof buf - (size_t)(q-buf)) ||
+                           s->s3->handshake_dgst == NULL)
                                {
                                /* internal error: 'buf' is too small for this 
cipersuite! */
                                err = 1;