Simplify fragmented HelloRequest state.
With server-side renegotiation gone, handshake_fragment's only purpose
in life is to handle a fragmented HelloRequest (we probably do need to
support those if some server does 1/n-1 record-splitting on handshake
records). The logic to route the data into
ssl3_read_bytes(SSL3_RT_HANDSHAKE) never happens, and the contents are
always a HelloRequest prefix.
This also trims a tiny bit of per-connection state.
Change-Id: Ia1b0dda5b7e79d817c28da1478640977891ebc97
Reviewed-on: https://boringssl-review.googlesource.com/6641
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
index 885dea2..f4b9535 100644
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -3949,10 +3949,9 @@
SSL3_RECORD rrec; /* each decoded record goes in here */
- /* storage for Handshake protocol data received but not yet processed by
- * ssl3_read_bytes: */
- uint8_t handshake_fragment[4];
- unsigned int handshake_fragment_len;
+ /* hello_request_len is the number of bytes of HelloRequest received, possibly
+ * split over multiple records. */
+ uint8_t hello_request_len;
/* partial write - check the numbers match */
unsigned int wnum; /* number of bytes sent so far */
diff --git a/ssl/s3_pkt.c b/ssl/s3_pkt.c
index 7416d0e..722041e 100644
--- a/ssl/s3_pkt.c
+++ b/ssl/s3_pkt.c
@@ -317,11 +317,10 @@
/* ssl3_expect_change_cipher_spec informs the record layer that a
* ChangeCipherSpec record is required at this point. If a Handshake record is
- * received before ChangeCipherSpec, the connection will fail. Moreover, if
- * there are unprocessed handshake bytes, the handshake will also fail and the
- * function returns zero. Otherwise, the function returns one. */
+ * received before ChangeCipherSpec, the connection will fail. If there is an
+ * unprocessed handshake message, it returns zero. Otherwise, it returns one. */
int ssl3_expect_change_cipher_spec(SSL *s) {
- if (s->s3->handshake_fragment_len > 0 || s->s3->tmp.reuse_message) {
+ if (s->s3->tmp.reuse_message) {
OPENSSL_PUT_ERROR(SSL, SSL_R_UNPROCESSED_HANDSHAKE_DATA);
return 0;
}
@@ -393,29 +392,6 @@
return -1;
}
- if (type == SSL3_RT_HANDSHAKE && s->s3->handshake_fragment_len > 0) {
- /* (partially) satisfy request from storage */
- uint8_t *src = s->s3->handshake_fragment;
- uint8_t *dst = buf;
- unsigned int k;
-
- /* peek == 0 */
- n = 0;
- while (len > 0 && s->s3->handshake_fragment_len > 0) {
- *dst++ = *src++;
- len--;
- s->s3->handshake_fragment_len--;
- n++;
- }
- /* move any remaining fragment bytes: */
- for (k = 0; k < s->s3->handshake_fragment_len; k++) {
- s->s3->handshake_fragment[k] = *src++;
- }
- return n;
- }
-
- /* Now s->s3->handshake_fragment_len == 0 if type == SSL3_RT_HANDSHAKE. */
-
/* This may require multiple iterations. False Start will cause
* |s->handshake_func| to signal success one step early, but the handshake
* must be completely finished before other modes are accepted.
@@ -532,33 +508,28 @@
goto f_err;
}
- /* HelloRequests may be fragmented across multiple records. */
- const size_t size = sizeof(s->s3->handshake_fragment);
- const size_t avail = size - s->s3->handshake_fragment_len;
- const size_t todo = (rr->length < avail) ? rr->length : avail;
- memcpy(s->s3->handshake_fragment + s->s3->handshake_fragment_len,
- &rr->data[rr->off], todo);
- rr->off += todo;
- rr->length -= todo;
- s->s3->handshake_fragment_len += todo;
- if (s->s3->handshake_fragment_len < size) {
- goto start; /* fragment was too small */
+ /* This must be a HelloRequest, possibly fragmented over multiple records.
+ * Consume data from the handshake protocol until it is complete. */
+ static const uint8_t kHelloRequest[] = {SSL3_MT_HELLO_REQUEST, 0, 0, 0};
+ while (s->s3->hello_request_len < sizeof(kHelloRequest)) {
+ if (rr->length == 0) {
+ /* Get a new record. */
+ goto start;
+ }
+ if (rr->data[rr->off] != kHelloRequest[s->s3->hello_request_len]) {
+ al = SSL_AD_DECODE_ERROR;
+ OPENSSL_PUT_ERROR(SSL, SSL_R_BAD_HELLO_REQUEST);
+ goto f_err;
+ }
+ rr->off++;
+ rr->length--;
+ s->s3->hello_request_len++;
}
-
- /* Parse out and consume a HelloRequest. */
- if (s->s3->handshake_fragment[0] != SSL3_MT_HELLO_REQUEST ||
- s->s3->handshake_fragment[1] != 0 ||
- s->s3->handshake_fragment[2] != 0 ||
- s->s3->handshake_fragment[3] != 0) {
- al = SSL_AD_DECODE_ERROR;
- OPENSSL_PUT_ERROR(SSL, SSL_R_BAD_HELLO_REQUEST);
- goto f_err;
- }
- s->s3->handshake_fragment_len = 0;
+ s->s3->hello_request_len = 0;
if (s->msg_callback) {
- s->msg_callback(0, s->version, SSL3_RT_HANDSHAKE,
- s->s3->handshake_fragment, 4, s, s->msg_callback_arg);
+ s->msg_callback(0, s->version, SSL3_RT_HANDSHAKE, kHelloRequest,
+ sizeof(kHelloRequest), s, s->msg_callback_arg);
}
if (!SSL_is_init_finished(s) || !s->s3->initial_handshake_complete) {