From b1f5bca53816ad4b9a684e285aefa9d935aac400 Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Fri, 8 May 2015 22:54:02 -0400 Subject: [PATCH] Remove max parameter to ssl3_read_n. It's completely redundant with the extend bit. If extend is 0, we're reading a new record, and rbuf.len is passed. Then it needs to get clamped by ssl3_read_n post alignment anyway. If extend is 1, we're reading the rest of the current record and max is always n. (For TLS, we actually could just read more, but not for DTLS. Basically no one sets it on the TLS side of things, so instead, after WebRTC's broken DTLS handling is fixed, read_ahead can go away altogether and DTLS/TLS record layers can be separated.) This removes ssl3_read_n's callers' dependency on ssl3_setup_read_buffer setting up rbuf.len. Change-Id: Iaf11535d01017507a52a33b19240f42984d6cf52 Reviewed-on: https://boringssl-review.googlesource.com/4686 Reviewed-by: Adam Langley --- ssl/d1_pkt.c | 4 ++-- ssl/internal.h | 2 +- ssl/s3_pkt.c | 25 +++++++++---------------- 3 files changed, 12 insertions(+), 19 deletions(-) diff --git a/ssl/d1_pkt.c b/ssl/d1_pkt.c index 80847af4..c169cb11 100644 --- a/ssl/d1_pkt.c +++ b/ssl/d1_pkt.c @@ -273,7 +273,7 @@ again: /* check if we have the header */ if ((s->rstate != SSL_ST_READ_BODY) || (s->packet_length < DTLS1_RT_HEADER_LENGTH)) { - n = ssl3_read_n(s, DTLS1_RT_HEADER_LENGTH, s->s3->rbuf.len, 0); + n = ssl3_read_n(s, DTLS1_RT_HEADER_LENGTH, 0); /* read timeout is handled by dtls1_read_bytes */ if (n <= 0) { return n; /* error or non-blocking */ @@ -345,7 +345,7 @@ again: if (rr->length > s->packet_length - DTLS1_RT_HEADER_LENGTH) { /* now s->packet_length == DTLS1_RT_HEADER_LENGTH */ i = rr->length; - n = ssl3_read_n(s, i, i, 1); + n = ssl3_read_n(s, i, 1); if (n <= 0) { return n; /* error or non-blocking io */ } diff --git a/ssl/internal.h b/ssl/internal.h index 1d8b2bf6..430b323c 100644 --- a/ssl/internal.h +++ b/ssl/internal.h @@ -895,7 +895,7 @@ int ssl3_set_handshake_header(SSL *s, int htype, unsigned long len); int ssl3_handshake_write(SSL *s); int dtls1_do_write(SSL *s, int type); -int ssl3_read_n(SSL *s, int n, int max, int extend); +int ssl3_read_n(SSL *s, int n, int extend); int dtls1_read_bytes(SSL *s, int type, uint8_t *buf, int len, int peek); int ssl3_write_pending(SSL *s, int type, const uint8_t *buf, unsigned int len); void dtls1_set_message_header(SSL *s, uint8_t mt, unsigned long len, diff --git a/ssl/s3_pkt.c b/ssl/s3_pkt.c index 56e3ec6f..913c3ba5 100644 --- a/ssl/s3_pkt.c +++ b/ssl/s3_pkt.c @@ -124,14 +124,14 @@ static int do_ssl3_write(SSL *s, int type, const uint8_t *buf, unsigned int len, char fragment); static int ssl3_get_record(SSL *s); -int ssl3_read_n(SSL *s, int n, int max, int extend) { +int ssl3_read_n(SSL *s, int n, int extend) { /* If |extend| is 0, obtain new n-byte packet; * if |extend| is 1, increase packet by another n bytes. * * The packet will be in the sub-array of |s->s3->rbuf.buf| specified by - * |s->packet| and |s->packet_length|. (If |s->read_ahead| is set, |max| - * bytes may be stored in |rbuf| (plus |s->packet_length| bytes if |extend| - * is one.) */ + * |s->packet| and |s->packet_length|. (If |s->read_ahead| is set and |extend| + * is 0, additional bytes may be read into |rbuf|, up to the size of the + * buffer.) */ int i, len, left; uintptr_t align = 0; uint8_t *pkt; @@ -206,16 +206,9 @@ int ssl3_read_n(SSL *s, int n, int max, int extend) { return -1; } - if (!s->read_ahead) { - /* ignore max parameter */ - max = n; - } else { - if (max < n) { - max = n; - } - if (max > (int)(rb->len - rb->offset)) { - max = rb->len - rb->offset; - } + int max = n; + if (s->read_ahead && !extend) { + max = rb->len - rb->offset; } while (left < n) { @@ -296,7 +289,7 @@ again: /* check if we have the header */ if (s->rstate != SSL_ST_READ_BODY || s->packet_length < SSL3_RT_HEADER_LENGTH) { - n = ssl3_read_n(s, SSL3_RT_HEADER_LENGTH, s->s3->rbuf.len, 0); + n = ssl3_read_n(s, SSL3_RT_HEADER_LENGTH, 0); if (n <= 0) { return n; /* error or non-blocking */ } @@ -339,7 +332,7 @@ again: if (rr->length > s->packet_length - SSL3_RT_HEADER_LENGTH) { /* now s->packet_length == SSL3_RT_HEADER_LENGTH */ i = rr->length; - n = ssl3_read_n(s, i, i, 1); + n = ssl3_read_n(s, i, 1); if (n <= 0) { /* Error or non-blocking IO. Now |n| == |rr->length|, and * |s->packet_length| == |SSL3_RT_HEADER_LENGTH| + |rr->length|. */