Only read 5 bytes (record header length) in sniffing for V2ClientHello.
This guarantees that we never read beyond the first record, even if the
first record is empty. Between removing SSL_set_read_ahead and DTLS
enforcing record boundaries, this means the buffer need never memmove
data.
The memmove isn't really much of a burden and we can probably just put
SSL_set_read_ahead back after the cleanup if desired. But while the
non-existant read_ahead is off, we should avoid reading more than needed.
(Also the current memmove logic is completely wrong for TLS. Checking
align != 0 doesn't make sense. The real reason to memmove is that the
next record may still be full size. So now line 209 of s3_pkt.c should
*actually* be unreachable.)
SSL_R_HTTPS_PROXY_REQUEST detection is now slightly less accurate, but
OpenSSL was already not parsing HTTP completely. We could asynchronously
read the extra 3 bytes once the first 5 match, but that seems
unnecessary. (Shall we just get rid of all these HTTP detectors? The
only consumer of those error codes is some diagnostics logic.)
BUG=468889
Change-Id: Ie3bf148ae7274795e1d048d78282d1d8063278ea
Reviewed-on: https://boringssl-review.googlesource.com/5714
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/ssl/s3_srvr.c b/ssl/s3_srvr.c
index a1f6a41..8045260 100644
--- a/ssl/s3_srvr.c
+++ b/ssl/s3_srvr.c
@@ -172,10 +172,6 @@
#include "../crypto/dh/internal.h"
-/* INITIAL_SNIFF_BUFFER_SIZE is the number of bytes read in the initial sniff
- * buffer. */
-#define INITIAL_SNIFF_BUFFER_SIZE 8
-
int ssl3_accept(SSL *s) {
BUF_MEM *buf = NULL;
uint32_t alg_a;
@@ -590,13 +586,14 @@
}
int ssl3_get_initial_bytes(SSL *s) {
- /* Read the first 8 bytes. To recognize a V2ClientHello only needs the first 4
- * bytes, but 8 is needed to recognize CONNECT below. */
- int ret = ssl3_read_n(s, INITIAL_SNIFF_BUFFER_SIZE, 0 /* new packet */);
+ /* Read the first 5 bytes, the size of the TLS record header. This is
+ * sufficient to detect a V2ClientHello and ensures that we never read beyond
+ * the first record. */
+ int ret = ssl3_read_n(s, SSL3_RT_HEADER_LENGTH, 0 /* new packet */);
if (ret <= 0) {
return ret;
}
- assert(s->packet_length == INITIAL_SNIFF_BUFFER_SIZE);
+ assert(s->packet_length == SSL3_RT_HEADER_LENGTH);
const uint8_t *p = s->packet;
/* Some dedicated error codes for protocol mixups should the application wish
@@ -609,7 +606,7 @@
OPENSSL_PUT_ERROR(SSL, SSL_R_HTTP_REQUEST);
return -1;
}
- if (strncmp("CONNECT ", (const char *)p, 8) == 0) {
+ if (strncmp("CONNE", (const char *)p, 5) == 0) {
OPENSSL_PUT_ERROR(SSL, SSL_R_HTTPS_PROXY_REQUEST);
return -1;
}
@@ -625,9 +622,9 @@
/* Fall through to the standard logic. Unread what's been read to re-process
* it. */
assert(s->rstate == SSL_ST_READ_HEADER);
- assert(s->s3->rbuf.offset >= INITIAL_SNIFF_BUFFER_SIZE);
- s->s3->rbuf.offset -= INITIAL_SNIFF_BUFFER_SIZE;
- s->s3->rbuf.left += INITIAL_SNIFF_BUFFER_SIZE;
+ assert(s->s3->rbuf.offset >= SSL3_RT_HEADER_LENGTH);
+ s->s3->rbuf.offset -= SSL3_RT_HEADER_LENGTH;
+ s->s3->rbuf.left += SSL3_RT_HEADER_LENGTH;
s->packet = NULL;
s->packet_length = 0;
@@ -646,24 +643,24 @@
uint8_t random[SSL3_RANDOM_SIZE];
/* Determine the length of the V2ClientHello. */
- assert(s->packet_length >= INITIAL_SNIFF_BUFFER_SIZE);
+ assert(s->packet_length >= SSL3_RT_HEADER_LENGTH);
p = (const uint8_t *)s->packet;
msg_length = ((p[0] & 0x7f) << 8) | p[1];
if (msg_length > (1024 * 4)) {
OPENSSL_PUT_ERROR(SSL, SSL_R_RECORD_TOO_LARGE);
return -1;
}
- if (msg_length < INITIAL_SNIFF_BUFFER_SIZE - 2) {
+ if (msg_length < SSL3_RT_HEADER_LENGTH - 2) {
/* Reject lengths that are too short early. We have already read
- * |INITIAL_SNIFF_BUFFER_SIZE| bytes, so we should not attempt to process an
+ * |SSL3_RT_HEADER_LENGTH| bytes, so we should not attempt to process an
* (invalid) V2ClientHello which would be shorter than that. */
OPENSSL_PUT_ERROR(SSL, SSL_R_RECORD_LENGTH_MISMATCH);
return -1;
}
/* Read the remainder of the V2ClientHello. We have previously read
- * |INITIAL_SNIFF_BUFFER_SIZE| bytes in ssl3_get_initial_bytes. */
- ret = ssl3_read_n(s, msg_length - (INITIAL_SNIFF_BUFFER_SIZE - 2),
+ * |SSL3_RT_HEADER_LENGTH| bytes in ssl3_get_initial_bytes. */
+ ret = ssl3_read_n(s, msg_length - (SSL3_RT_HEADER_LENGTH - 2),
1 /* extend */);
if (ret <= 0) {
return ret;
