Address feedback on SSLv2 ClientHello processing
authorMatt Caswell <matt@openssl.org>
Wed, 3 Aug 2016 12:03:25 +0000 (13:03 +0100)
committerMatt Caswell <matt@openssl.org>
Mon, 15 Aug 2016 22:14:30 +0000 (23:14 +0100)
Reviewed-by: Tim Hudson <tjh@openssl.org>
ssl/record/rec_layer_s3.c
ssl/record/record_locl.h
ssl/record/ssl3_record.c

index 048f756..c2f9666 100644 (file)
@@ -33,7 +33,7 @@
 void RECORD_LAYER_init(RECORD_LAYER *rl, SSL *s)
 {
     rl->s = s;
 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);
 }
 
     SSL3_RECORD_clear(rl->rrec, SSL_MAX_PIPELINES);
 }
 
index 8f40430..f9dbc33 100644 (file)
@@ -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)
                                                 ((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);
 #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);
index f67b85f..5f9ce7a 100644 (file)
@@ -159,18 +159,9 @@ int ssl3_get_record(SSL *s)
             p = RECORD_LAYER_get_packet(&s->rlayer);
 
             /*
             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
                     && (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);
 
         /* 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)
     } while (num_recs < max_recs
              && rr[num_recs-1].type == SSL3_RT_APPLICATION_DATA
              && SSL_USE_EXPLICIT_IV(s)