Fix horrible (and hard to track down) bug in ssl23_get_client_hello:

In case of a restart, v[0] and v[1] were incorrectly initialised.
This was interpreted by ssl3_get_client_key_exchange as an RSA decryption
failure (don't ask me why) and caused it to create a _random_ master key
instead (even weirder), which obviously led to incorrect input to
ssl3_generate_master_secret and thus caused "block cipher pad is
wrong" error messages from ssl3_enc for the client's Finished message.
Arrgh.

diff --git a/CHANGES b/CHANGES
index 93c314e64fb31b9f174c9f178905c82cc590bd6c..7b6970eb458f25aaf7656523e0e1d1bb750233e7 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -4,6 +4,12 @@
 
  Changes between 0.9.4 and 0.9.5  [xx XXX 1999]
 
+  *) Bugfix: ssl23_get_client_hello did not work properly when called in
+     state SSL23_ST_SR_CLNT_HELLO_B, i.e. when the first 7 bytes of
+     a SSLv2-compatible client hello for SSLv3 or TLSv1 could be read,
+     but a retry condition occured while trying to read the rest.
+     [Bodo Moeller]
+
   *) The PKCS7_ENC_CONTENT_new() function was setting the content type as
      NID_pkcs7_encrypted by default: this was wrong since this should almost
      always be NID_pkcs7_data. Also modified the PKCS7_set_type() to handle

diff --git a/ssl/s23_srvr.c b/ssl/s23_srvr.c
index e4122f2d78df2032fe729bc818a5add03891a8ed..1a9e5fd867066bc670d2a05e673596c4d7015e2a 100644 (file)
--- a/ssl/s23_srvr.c
+++ b/ssl/s23_srvr.c
@@ -195,10 +195,11 @@ int ssl23_get_client_hello(SSL *s)
        int type=0,use_sslv2_strong=0;
        int v[2];
 
-       /* read the initial header */
-       v[0]=v[1]=0;
        if (s->state == SSL23_ST_SR_CLNT_HELLO_A)
                {
+               /* read the initial header */
+               v[0]=v[1]=0;
+
                if (!ssl3_setup_buffers(s)) goto err;
 
                n=ssl23_read_bytes(s,7);
@@ -244,6 +245,7 @@ int ssl23_get_client_hello(SSL *s)
                                        type=1;
 
                                if (s->options & SSL_OP_NON_EXPORT_FIRST)
+                                       /* not only confusing, but broken! */
                                        {
                                        STACK_OF(SSL_CIPHER) *sk;
                                        SSL_CIPHER *c;
@@ -337,6 +339,8 @@ next_bit:
                /* we have a SSLv3/TLSv1 in a SSLv2 header */
                type=2;
                p=s->packet;
+               v[0] = p[3];
+               v[1] = p[4];
                n=((p[0]&0x7f)<<8)|p[1];
                if (n > (1024*4))
                        {

diff --git a/ssl/ssl.h b/ssl/ssl.h
index fbe4f667fa13d04c37cdcd71b175223210822b5b..424b195f5ce31ac30d0872b92e3390f6d74ca80f 100644 (file)
--- a/ssl/ssl.h
+++ b/ssl/ssl.h
@@ -291,6 +291,7 @@ typedef struct ssl_session_st
 #define SSL_OP_PKCS1_CHECK_1                           0x08000000L
 #define SSL_OP_PKCS1_CHECK_2                           0x10000000L
 #define SSL_OP_NETSCAPE_CA_DN_BUG                      0x20000000L
+/* SSL_OP_NON_EXPORT_FIRST looks utterly broken .. */
 #define SSL_OP_NON_EXPORT_FIRST                        0x40000000L
 #define SSL_OP_NETSCAPE_DEMO_CIPHER_CHANGE_BUG         0x80000000L
 #define SSL_OP_ALL                                     0x000FFFFFL