From 78fcddbb8d690c515f4525b5220fc63448ecd1a9 Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Wed, 3 Aug 2016 13:03:25 +0100 Subject: [PATCH] Address feedback on SSLv2 ClientHello processing Reviewed-by: Tim Hudson --- ssl/record/rec_layer_s3.c | 2 +- ssl/record/record_locl.h | 3 ++- ssl/record/ssl3_record.c | 15 +++------------ 3 files changed, 6 insertions(+), 14 deletions(-) diff --git a/ssl/record/rec_layer_s3.c b/ssl/record/rec_layer_s3.c index 048f756577..c2f96669cb 100644 --- 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 8f40430745..f9dbc33466 100644 --- 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 f67b85f0a9..5f9ce7a065 100644 --- 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) -- 2.25.1