Address feedback on SSLv2 ClientHello processing

Reviewed-by: Tim Hudson <tjh@openssl.org>

diff --git a/ssl/record/rec_layer_s3.c b/ssl/record/rec_layer_s3.c
index 048f756577d5c54eb37a6acc8c7cc15c76cee978..c2f96669cbdcf70a4ff0b5a417a76586a0c231dc 100644 (file)
--- a/ssl/record/rec_layer_s3.c
+++ b/ssl/record/rec_layer_s3.c
@@ -33,7 +33,7 @@
 void RECORD_LAYER_init(RECORD_LAYER *rl, SSL *s)
 {
     rl->s = s;
-    RECORD_LAYER_set_first_record(&s->rlayer, 1);
+    RECORD_LAYER_set_first_record(&s->rlayer);
     SSL3_RECORD_clear(rl->rrec, SSL_MAX_PIPELINES);
 }
 

diff --git a/ssl/record/record_locl.h b/ssl/record/record_locl.h
index 8f404307456f224d085fbd3e7eca15a39dd2a5f6..f9dbc334663dd4ae1e594384bc88e1c5fbe442db 100644 (file)
--- a/ssl/record/record_locl.h
+++ b/ssl/record/record_locl.h
@@ -32,7 +32,8 @@
                                                 ((rl)->empty_record_count = 0)
 #define RECORD_LAYER_get_empty_record_count(rl) ((rl)->empty_record_count)
 #define RECORD_LAYER_is_first_record(rl)        ((rl)->is_first_record)
-#define RECORD_LAYER_set_first_record(rl, val)  ((rl)->is_first_record = (val))
+#define RECORD_LAYER_set_first_record(rl)       ((rl)->is_first_record = 1)
+#define RECORD_LAYER_clear_first_record(rl)     ((rl)->is_first_record = 0)
 #define DTLS_RECORD_LAYER_get_r_epoch(rl)       ((rl)->d->r_epoch)
 
 __owur int ssl3_read_n(SSL *s, int n, int max, int extend, int clearold);

diff --git a/ssl/record/ssl3_record.c b/ssl/record/ssl3_record.c
index f67b85f0a9ae799ce3c886df2fcce15cc6228643..5f9ce7a0653e60314181d3c776fde5c4d2c19d80 100644 (file)
--- a/ssl/record/ssl3_record.c
+++ b/ssl/record/ssl3_record.c
@@ -159,18 +159,9 @@ int ssl3_get_record(SSL *s)
             p = RECORD_LAYER_get_packet(&s->rlayer);
 
             /*
-             * Check whether this is a regular record or an SSLv2 style record.
-             * The latter can only be used in the first record of an initial
-             * ClientHello for old clients. Initial ClientHello means
-             * s->first_packet is set and s->server is true.  The first record
-             * means s->rlayer.is_first_record is true. Probably this is
-             * sufficient in itself instead of s->first_packet, but I am
-             * cautious. We check s->read_hash and s->enc_read_ctx to ensure
-             * this does not apply during renegotiation.
+             * The first record received by the server may be a V2ClientHello.
              */
-            if (s->first_packet && s->server
-                    && RECORD_LAYER_is_first_record(&s->rlayer)
-                    && s->read_hash == NULL && s->enc_read_ctx == NULL
+            if (s->server && RECORD_LAYER_is_first_record(&s->rlayer)
                     && (p[0] & 0x80) && (p[2] == SSL2_MT_CLIENT_HELLO)) {
                 /*
                  *  SSLv2 style record
@@ -342,7 +333,7 @@ int ssl3_get_record(SSL *s)
 
         /* we have pulled in a full packet so zero things */
         RECORD_LAYER_reset_packet_length(&s->rlayer);
-        RECORD_LAYER_set_first_record(&s->rlayer, 0);
+        RECORD_LAYER_clear_first_record(&s->rlayer);
     } while (num_recs < max_recs
              && rr[num_recs-1].type == SSL3_RT_APPLICATION_DATA
              && SSL_USE_EXPLICIT_IV(s)