Remove redundant setup buffer calls.
Nothing should call ssl3_setup_read_buffer or ssl3_setup_write_buffer unless it intends to write into the buffer. This way buffer management can later be an implementation detail of the record layer. Change-Id: Idb0effba00e77c6169764843793f40ec37868b61 Reviewed-on: https://boringssl-review.googlesource.com/4687 Reviewed-by: Adam Langley <agl@google.com>
This commit is contained in:
parent
4a5982813f
commit
6a08da2cf8
@ -176,8 +176,7 @@ int dtls1_connect(SSL *s) {
|
||||
buf = NULL;
|
||||
}
|
||||
|
||||
if (!ssl3_setup_buffers(s) ||
|
||||
!ssl_init_wbio_buffer(s, 0)) {
|
||||
if (!ssl_init_wbio_buffer(s, 0)) {
|
||||
ret = -1;
|
||||
goto end;
|
||||
}
|
||||
|
@ -446,11 +446,6 @@ int dtls1_read_bytes(SSL *s, int type, unsigned char *buf, int len, int peek) {
|
||||
}
|
||||
}
|
||||
|
||||
if (s->s3->rbuf.buf == NULL && !ssl3_setup_buffers(s)) {
|
||||
/* TODO(davidben): Is this redundant with the calls in the handshake? */
|
||||
return -1;
|
||||
}
|
||||
|
||||
start:
|
||||
s->rwstate = SSL_NOTHING;
|
||||
|
||||
|
@ -179,11 +179,6 @@ int dtls1_accept(SSL *s) {
|
||||
buf = NULL;
|
||||
}
|
||||
|
||||
if (!ssl3_setup_buffers(s)) {
|
||||
ret = -1;
|
||||
goto end;
|
||||
}
|
||||
|
||||
s->init_num = 0;
|
||||
|
||||
if (s->state != SSL_ST_RENEGOTIATE) {
|
||||
|
@ -861,7 +861,6 @@ int ssl3_output_cert_chain(SSL *s, CERT_PKEY *cpk);
|
||||
const SSL_CIPHER *ssl3_choose_cipher(
|
||||
SSL *ssl, STACK_OF(SSL_CIPHER) *clnt,
|
||||
struct ssl_cipher_preference_list_st *srvr);
|
||||
int ssl3_setup_buffers(SSL *s);
|
||||
int ssl3_setup_read_buffer(SSL *s);
|
||||
int ssl3_setup_write_buffer(SSL *s);
|
||||
int ssl3_release_read_buffer(SSL *s);
|
||||
|
@ -654,15 +654,6 @@ err:
|
||||
return 0;
|
||||
}
|
||||
|
||||
|
||||
int ssl3_setup_buffers(SSL *s) {
|
||||
if (!ssl3_setup_read_buffer(s) ||
|
||||
!ssl3_setup_write_buffer(s)) {
|
||||
return 0;
|
||||
}
|
||||
return 1;
|
||||
}
|
||||
|
||||
int ssl3_release_write_buffer(SSL *s) {
|
||||
OPENSSL_free(s->s3->wbuf.buf);
|
||||
s->s3->wbuf.buf = NULL;
|
||||
|
@ -224,8 +224,7 @@ int ssl3_connect(SSL *s) {
|
||||
buf = NULL;
|
||||
}
|
||||
|
||||
if (!ssl3_setup_buffers(s) ||
|
||||
!ssl_init_wbio_buffer(s, 0)) {
|
||||
if (!ssl_init_wbio_buffer(s, 0)) {
|
||||
ret = -1;
|
||||
goto end;
|
||||
}
|
||||
|
28
ssl/s3_pkt.c
28
ssl/s3_pkt.c
@ -272,19 +272,6 @@ static int ssl3_get_record(SSL *s) {
|
||||
|
||||
rr = &s->s3->rrec;
|
||||
|
||||
if (s->options & SSL_OP_MICROSOFT_BIG_SSLV3_BUFFER) {
|
||||
extra = SSL3_RT_MAX_EXTRA;
|
||||
} else {
|
||||
extra = 0;
|
||||
}
|
||||
|
||||
if (extra && !s->s3->init_extra) {
|
||||
/* An application error: SSL_OP_MICROSOFT_BIG_SSLV3_BUFFER set after
|
||||
* ssl3_setup_buffers() was done */
|
||||
OPENSSL_PUT_ERROR(SSL, ssl3_get_record, ERR_R_INTERNAL_ERROR);
|
||||
return -1;
|
||||
}
|
||||
|
||||
again:
|
||||
/* check if we have the header */
|
||||
if (s->rstate != SSL_ST_READ_BODY ||
|
||||
@ -295,6 +282,11 @@ again:
|
||||
}
|
||||
s->rstate = SSL_ST_READ_BODY;
|
||||
|
||||
/* Some bytes were read, so the read buffer must be existant and
|
||||
* |s->s3->init_extra| is defined. */
|
||||
assert(s->s3->rbuf.buf != NULL);
|
||||
extra = s->s3->init_extra ? SSL3_RT_MAX_EXTRA : 0;
|
||||
|
||||
p = s->packet;
|
||||
if (s->msg_callback) {
|
||||
s->msg_callback(0, 0, SSL3_RT_HEADER, p, 5, s, s->msg_callback_arg);
|
||||
@ -325,6 +317,11 @@ again:
|
||||
}
|
||||
|
||||
/* now s->rstate == SSL_ST_READ_BODY */
|
||||
} else {
|
||||
/* |packet_length| is non-zero and |s->rstate| is |SSL_ST_READ_BODY|. The
|
||||
* read buffer must be existant and |s->s3->init_extra| is defined. */
|
||||
assert(s->s3->rbuf.buf != NULL);
|
||||
extra = s->s3->init_extra ? SSL3_RT_MAX_EXTRA : 0;
|
||||
}
|
||||
|
||||
/* s->rstate == SSL_ST_READ_BODY, get and decode the data */
|
||||
@ -778,11 +775,6 @@ int ssl3_read_bytes(SSL *s, int type, uint8_t *buf, int len, int peek) {
|
||||
}
|
||||
}
|
||||
|
||||
if (s->s3->rbuf.buf == NULL && !ssl3_setup_read_buffer(s)) {
|
||||
/* TODO(davidben): Is this redundant with the calls in the handshake? */
|
||||
return -1;
|
||||
}
|
||||
|
||||
start:
|
||||
s->rwstate = SSL_NOTHING;
|
||||
|
||||
|
@ -227,11 +227,6 @@ int ssl3_accept(SSL *s) {
|
||||
}
|
||||
s->init_num = 0;
|
||||
|
||||
if (!ssl3_setup_buffers(s)) {
|
||||
ret = -1;
|
||||
goto end;
|
||||
}
|
||||
|
||||
if (!s->s3->send_connection_binding &&
|
||||
!(s->options & SSL_OP_ALLOW_UNSAFE_LEGACY_RENEGOTIATION)) {
|
||||
/* Server attempting to renegotiate with client that doesn't support
|
||||
@ -296,6 +291,13 @@ int ssl3_accept(SSL *s) {
|
||||
}
|
||||
s->init_num = 0;
|
||||
|
||||
/* Enable a write buffer. This groups handshake messages within a flight
|
||||
* into a single write. */
|
||||
if (!ssl_init_wbio_buffer(s, 1)) {
|
||||
ret = -1;
|
||||
goto end;
|
||||
}
|
||||
|
||||
if (!ssl3_init_finished_mac(s)) {
|
||||
OPENSSL_PUT_ERROR(SSL, ssl3_accept, ERR_R_INTERNAL_ERROR);
|
||||
ret = -1;
|
||||
@ -303,19 +305,8 @@ int ssl3_accept(SSL *s) {
|
||||
}
|
||||
|
||||
if (!s->s3->have_version) {
|
||||
/* This is the initial handshake. The record layer has not been
|
||||
* initialized yet. Sniff for a V2ClientHello before reading a
|
||||
* ClientHello normally. */
|
||||
assert(s->s3->rbuf.buf == NULL);
|
||||
assert(s->s3->wbuf.buf == NULL);
|
||||
s->state = SSL3_ST_SR_INITIAL_BYTES;
|
||||
} else {
|
||||
/* Enable a write buffer. This groups handshake messages within a
|
||||
* flight into a single write. */
|
||||
if (!ssl3_setup_buffers(s) || !ssl_init_wbio_buffer(s, 1)) {
|
||||
ret = -1;
|
||||
goto end;
|
||||
}
|
||||
s->state = SSL3_ST_SR_CLNT_HELLO_A;
|
||||
}
|
||||
break;
|
||||
@ -751,10 +742,12 @@ int ssl3_get_initial_bytes(SSL *s) {
|
||||
p[5] == SSL3_MT_CLIENT_HELLO) {
|
||||
/* This is a ClientHello. Initialize the record layer with the already
|
||||
* consumed data and continue the handshake. */
|
||||
if (!ssl3_setup_buffers(s) || !ssl_init_wbio_buffer(s, 1)) {
|
||||
if (!ssl3_setup_read_buffer(s)) {
|
||||
return -1;
|
||||
}
|
||||
assert(s->rstate == SSL_ST_READ_HEADER);
|
||||
/* There cannot have already been data in the record layer. */
|
||||
assert(s->s3->rbuf.left == 0);
|
||||
memcpy(s->s3->rbuf.buf, p, s->s3->sniff_buffer_len);
|
||||
s->s3->rbuf.offset = 0;
|
||||
s->s3->rbuf.left = s->s3->sniff_buffer_len;
|
||||
@ -897,11 +890,6 @@ int ssl3_get_v2_client_hello(SSL *s) {
|
||||
/* The handshake message header is 4 bytes. */
|
||||
s->s3->tmp.message_size = len - 4;
|
||||
|
||||
/* Initialize the record layer. */
|
||||
if (!ssl3_setup_buffers(s) || !ssl_init_wbio_buffer(s, 1)) {
|
||||
return -1;
|
||||
}
|
||||
|
||||
/* Drop the sniff buffer. */
|
||||
BUF_MEM_free(s->s3->sniff_buffer);
|
||||
s->s3->sniff_buffer = NULL;
|
||||
|
Loading…
Reference in New Issue
Block a user