diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h index df227fbce..d272b5896 100644 --- a/include/openssl/ssl.h +++ b/include/openssl/ssl.h @@ -4106,17 +4106,6 @@ struct ssl_st { BIO *rbio; /* used by SSL_read */ BIO *wbio; /* used by SSL_write */ - /* bbio, if non-NULL, is a buffer placed in front of |wbio| to pack handshake - * messages within one flight into a single |BIO_write|. In this case, |wbio| - * and |bbio| are equal and the true caller-configured BIO is - * |bbio->next_bio|. - * - * TODO(davidben): This does not work right for DTLS. It assumes the MTU is - * smaller than the buffer size so that the buffer's internal flushing never - * kicks in. It also doesn't kick in for DTLS retransmission. Replace this - * with a better mechanism. */ - BIO *bbio; - int (*handshake_func)(SSL_HANDSHAKE *hs); BUF_MEM *init_buf; /* buffer used during init */ diff --git a/ssl/d1_both.c b/ssl/d1_both.c index 1d896360e..bbb6b3283 100644 --- a/ssl/d1_both.c +++ b/ssl/d1_both.c @@ -579,7 +579,7 @@ static int add_outgoing(SSL *ssl, int is_ccs, uint8_t *data, size_t len) { return 1; } -int dtls1_queue_message(SSL *ssl, uint8_t *data, size_t len) { +int dtls1_add_message(SSL *ssl, uint8_t *data, size_t len) { return add_outgoing(ssl, 0 /* handshake */, data, len); } @@ -588,10 +588,19 @@ int dtls1_write_message(SSL *ssl) { return 1; } -int dtls1_send_change_cipher_spec(SSL *ssl) { +int dtls1_add_change_cipher_spec(SSL *ssl) { return add_outgoing(ssl, 1 /* ChangeCipherSpec */, NULL, 0); } +int dtls1_add_alert(SSL *ssl, uint8_t level, uint8_t desc) { + /* The |add_alert| path is only used for warning alerts for now, which DTLS + * never sends. This will be implemented later once closure alerts are + * converted. */ + assert(0); + OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR); + return 0; +} + /* dtls1_update_mtu updates the current MTU from the BIO, ensuring it is above * the minimum. */ static void dtls1_update_mtu(SSL *ssl) { diff --git a/ssl/dtls_method.c b/ssl/dtls_method.c index a774c828b..e7cd21ef2 100644 --- a/ssl/dtls_method.c +++ b/ssl/dtls_method.c @@ -151,14 +151,15 @@ static const SSL_PROTOCOL_METHOD kDTLSProtocolMethod = { dtls1_supports_cipher, dtls1_init_message, dtls1_finish_message, - dtls1_queue_message, - dtls1_write_message, - dtls1_send_change_cipher_spec, + dtls1_add_message, + dtls1_add_change_cipher_spec, + dtls1_add_alert, dtls1_flush_flight, dtls1_expect_flight, dtls1_received_flight, dtls1_set_read_state, dtls1_set_write_state, + dtls1_write_message, }; const SSL_METHOD *DTLS_method(void) { diff --git a/ssl/handshake_client.c b/ssl/handshake_client.c index 3389d6cd4..29b59d44e 100644 --- a/ssl/handshake_client.c +++ b/ssl/handshake_client.c @@ -206,12 +206,6 @@ int ssl3_connect(SSL_HANDSHAKE *hs) { case SSL_ST_CONNECT: ssl_do_info_callback(ssl, SSL_CB_HANDSHAKE_START, 1); - - if (!ssl_init_wbio_buffer(ssl)) { - ret = -1; - goto end; - } - hs->state = SSL3_ST_CW_CLNT_HELLO_A; break; @@ -362,18 +356,13 @@ int ssl3_connect(SSL_HANDSHAKE *hs) { break; case SSL3_ST_CW_CHANGE: - ret = ssl->method->send_change_cipher_spec(ssl); - if (ret <= 0) { - goto end; - } - - hs->state = SSL3_ST_CW_NEXT_PROTO_A; - - if (!tls1_change_cipher_state(hs, SSL3_CHANGE_CIPHER_CLIENT_WRITE)) { + if (!ssl->method->add_change_cipher_spec(ssl) || + !tls1_change_cipher_state(hs, SSL3_CHANGE_CIPHER_CLIENT_WRITE)) { ret = -1; goto end; } + hs->state = SSL3_ST_CW_NEXT_PROTO_A; break; case SSL3_ST_CW_NEXT_PROTO_A: @@ -437,8 +426,6 @@ int ssl3_connect(SSL_HANDSHAKE *hs) { case SSL3_ST_FALSE_START: hs->state = SSL3_ST_CR_SESSION_TICKET_A; hs->in_false_start = 1; - - ssl_free_wbio_buffer(ssl); ret = 1; goto end; @@ -527,9 +514,6 @@ int ssl3_connect(SSL_HANDSHAKE *hs) { ssl->s3->new_session = NULL; } - /* Remove write buffering now. */ - ssl_free_wbio_buffer(ssl); - const int is_initial_handshake = !ssl->s3->initial_handshake_complete; ssl->s3->initial_handshake_complete = 1; if (is_initial_handshake) { @@ -713,7 +697,7 @@ int ssl_write_client_hello(SSL_HANDSHAKE *hs) { goto err; } - return ssl->method->queue_message(ssl, msg, len); + return ssl->method->add_message(ssl, msg, len); err: CBB_cleanup(&cbb); @@ -1469,9 +1453,12 @@ static int ssl3_send_client_certificate(SSL_HANDSHAKE *hs) { /* Without a client certificate, the handshake buffer may be released. */ ssl3_free_handshake_buffer(ssl); + /* In SSL 3.0, the Certificate message is replaced with a warning alert. */ if (ssl->version == SSL3_VERSION) { - /* In SSL 3.0, send no certificate by skipping both messages. */ - ssl3_send_alert(ssl, SSL3_AL_WARNING, SSL_AD_NO_CERTIFICATE); + if (!ssl->method->add_alert(ssl, SSL3_AL_WARNING, + SSL_AD_NO_CERTIFICATE)) { + return -1; + } return 1; } } @@ -1647,7 +1634,7 @@ static int ssl3_send_client_key_exchange(SSL_HANDSHAKE *hs) { /* The message must be added to the finished hash before calculating the * master secret. */ - if (!ssl_complete_message(ssl, &cbb)) { + if (!ssl_add_message_cbb(ssl, &cbb)) { goto err; } hs->state = SSL3_ST_CW_KEY_EXCH_B; @@ -1765,7 +1752,7 @@ static int ssl3_send_cert_verify(SSL_HANDSHAKE *hs) { } if (!CBB_did_write(&child, sig_len) || - !ssl_complete_message(ssl, &cbb)) { + !ssl_add_message_cbb(ssl, &cbb)) { goto err; } @@ -1795,7 +1782,7 @@ static int ssl3_send_next_proto(SSL_HANDSHAKE *hs) { ssl->s3->next_proto_negotiated_len) || !CBB_add_u8_length_prefixed(&body, &child) || !CBB_add_bytes(&child, kZero, padding_len) || - !ssl_complete_message(ssl, &cbb)) { + !ssl_add_message_cbb(ssl, &cbb)) { OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR); CBB_cleanup(&cbb); return -1; @@ -1825,7 +1812,7 @@ static int ssl3_send_channel_id(SSL_HANDSHAKE *hs) { CBB cbb, body; if (!ssl->method->init_message(ssl, &cbb, &body, SSL3_MT_CHANNEL_ID) || !tls1_write_channel_id(ssl, &body) || - !ssl_complete_message(ssl, &cbb)) { + !ssl_add_message_cbb(ssl, &cbb)) { OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR); CBB_cleanup(&cbb); return -1; diff --git a/ssl/handshake_server.c b/ssl/handshake_server.c index a4396f40b..00b374cf7 100644 --- a/ssl/handshake_server.c +++ b/ssl/handshake_server.c @@ -206,13 +206,6 @@ int ssl3_accept(SSL_HANDSHAKE *hs) { case SSL_ST_ACCEPT: ssl_do_info_callback(ssl, SSL_CB_HANDSHAKE_START, 1); - /* Enable a write buffer. This groups handshake messages within a flight - * into a single write. */ - if (!ssl_init_wbio_buffer(ssl)) { - ret = -1; - goto end; - } - if (!ssl3_init_handshake_buffer(ssl)) { OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR); ret = -1; @@ -423,16 +416,13 @@ int ssl3_accept(SSL_HANDSHAKE *hs) { break; case SSL3_ST_SW_CHANGE: - ret = ssl->method->send_change_cipher_spec(ssl); - if (ret <= 0) { - goto end; - } - hs->state = SSL3_ST_SW_FINISHED_A; - - if (!tls1_change_cipher_state(hs, SSL3_CHANGE_CIPHER_SERVER_WRITE)) { + if (!ssl->method->add_change_cipher_spec(ssl) || + !tls1_change_cipher_state(hs, SSL3_CHANGE_CIPHER_SERVER_WRITE)) { ret = -1; goto end; } + + hs->state = SSL3_ST_SW_FINISHED_A; break; case SSL3_ST_SW_FINISHED_A: @@ -493,9 +483,6 @@ int ssl3_accept(SSL_HANDSHAKE *hs) { ssl->s3->new_session = NULL; } - /* remove buffering on output */ - ssl_free_wbio_buffer(ssl); - ssl->s3->initial_handshake_complete = 1; ssl_update_cache(hs, SSL_SESS_CACHE_SERVER); @@ -1101,7 +1088,7 @@ static int ssl3_send_server_hello(SSL_HANDSHAKE *hs) { !CBB_add_u16(&body, ssl_cipher_get_value(ssl->s3->tmp.new_cipher)) || !CBB_add_u8(&body, 0 /* no compression */) || !ssl_add_serverhello_tlsext(hs, &body) || - !ssl_complete_message(ssl, &cbb)) { + !ssl_add_message_cbb(ssl, &cbb)) { OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR); CBB_cleanup(&cbb); return -1; @@ -1142,7 +1129,7 @@ static int ssl3_send_certificate_status(SSL_HANDSHAKE *hs) { !CBB_add_u24_length_prefixed(&body, &ocsp_response) || !CBB_add_bytes(&ocsp_response, CRYPTO_BUFFER_data(ssl->ocsp_response), CRYPTO_BUFFER_len(ssl->ocsp_response)) || - !ssl_complete_message(ssl, &cbb)) { + !ssl_add_message_cbb(ssl, &cbb)) { OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR); CBB_cleanup(&cbb); return -1; @@ -1317,7 +1304,7 @@ static int ssl3_send_server_key_exchange(SSL_HANDSHAKE *hs) { } } - if (!ssl_complete_message(ssl, &cbb)) { + if (!ssl_add_message_cbb(ssl, &cbb)) { goto err; } @@ -1400,7 +1387,7 @@ static int ssl3_send_certificate_request(SSL_HANDSHAKE *hs) { } if (!ssl_add_client_CA_list(ssl, &body) || - !ssl_complete_message(ssl, &cbb)) { + !ssl_add_message_cbb(ssl, &cbb)) { goto err; } @@ -1421,7 +1408,7 @@ static int ssl3_send_server_hello_done(SSL_HANDSHAKE *hs) { CBB cbb, body; if (!ssl->method->init_message(ssl, &cbb, &body, SSL3_MT_SERVER_HELLO_DONE) || - !ssl_complete_message(ssl, &cbb)) { + !ssl_add_message_cbb(ssl, &cbb)) { OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR); CBB_cleanup(&cbb); return -1; @@ -1970,7 +1957,7 @@ static int ssl3_send_new_session_ticket(SSL_HANDSHAKE *hs) { CBB_add_u32(&body, session->timeout) && CBB_add_u16_length_prefixed(&body, &ticket) && ssl_encrypt_ticket(ssl, &ticket, session) && - ssl_complete_message(ssl, &cbb); + ssl_add_message_cbb(ssl, &cbb); SSL_SESSION_free(session_copy); CBB_cleanup(&cbb); diff --git a/ssl/internal.h b/ssl/internal.h index ffe4d62d8..877822dd2 100644 --- a/ssl/internal.h +++ b/ssl/internal.h @@ -1333,16 +1333,17 @@ struct ssl_protocol_method_st { * release it with |OPENSSL_free| when done. It returns one on success and * zero on error. */ int (*finish_message)(SSL *ssl, CBB *cbb, uint8_t **out_msg, size_t *out_len); - /* queue_message queues a handshake message and prepares it to be written. It - * takes ownership of |msg| and releases it with |OPENSSL_free| when done. It - * returns one on success and zero on error. */ - int (*queue_message)(SSL *ssl, uint8_t *msg, size_t len); - /* write_message writes the next message to the transport. It returns one on - * success and <= 0 on error. */ - int (*write_message)(SSL *ssl); - /* send_change_cipher_spec sends a ChangeCipherSpec message. */ - int (*send_change_cipher_spec)(SSL *ssl); - /* flush_flight flushes the current flight to the transport. It returns one on + /* add_message adds a handshake message to the pending flight. It returns one + * on success and zero on error. In either case, it takes ownership of |msg| + * and releases it with |OPENSSL_free| when done. */ + int (*add_message)(SSL *ssl, uint8_t *msg, size_t len); + /* add_change_cipher_spec adds a ChangeCipherSpec record to the pending + * flight. It returns one on success and zero on error. */ + int (*add_change_cipher_spec)(SSL *ssl); + /* add_alert adds an alert to the pending flight. It returns one on success + * and zero on error. */ + int (*add_alert)(SSL *ssl, uint8_t level, uint8_t desc); + /* flush_flight flushes the pending flight to the transport. It returns one on * success and <= 0 on error. */ int (*flush_flight)(SSL *ssl); /* expect_flight is called when the handshake expects a flight of messages from @@ -1359,6 +1360,8 @@ struct ssl_protocol_method_st { * ownership of |aead_ctx|. It returns one on success and zero if changing the * write state is forbidden at this point. */ int (*set_write_state)(SSL *ssl, SSL_AEAD_CTX *aead_ctx); + /* write_message returns one. */ + int (*write_message)(SSL *ssl); }; /* This is for the SSLv3/TLSv1.0 differences in crypto/hash stuff It is a bit @@ -1491,9 +1494,13 @@ typedef struct ssl3_state_st { uint8_t send_alert[2]; - /* pending_message is the current outgoing handshake message. */ - uint8_t *pending_message; - uint32_t pending_message_len; + /* pending_flight is the pending outgoing flight. This is used to flush each + * handshake flight in a single write. */ + BUF_MEM *pending_flight; + + /* pending_flight_offset is the number of bytes of |pending_flight| which have + * been successfully written. */ + uint32_t pending_flight_offset; /* aead_read_ctx is the current read cipher state. */ SSL_AEAD_CTX *aead_read_ctx; @@ -1750,7 +1757,6 @@ void ssl_update_cache(SSL_HANDSHAKE *hs, int mode); int ssl_verify_alarm_type(long type); int ssl3_get_finished(SSL_HANDSHAKE *hs); -int ssl3_send_change_cipher_spec(SSL *ssl); int ssl3_send_alert(SSL *ssl, int level, int desc); int ssl3_get_message(SSL *ssl, int msg_type, enum ssl_hash_message_t hash_message); @@ -1782,20 +1788,24 @@ int ssl3_connect(SSL_HANDSHAKE *hs); int ssl3_init_message(SSL *ssl, CBB *cbb, CBB *body, uint8_t type); int ssl3_finish_message(SSL *ssl, CBB *cbb, uint8_t **out_msg, size_t *out_len); -int ssl3_queue_message(SSL *ssl, uint8_t *msg, size_t len); +int ssl3_add_message(SSL *ssl, uint8_t *msg, size_t len); +int ssl3_add_change_cipher_spec(SSL *ssl); +int ssl3_add_alert(SSL *ssl, uint8_t level, uint8_t desc); int ssl3_write_message(SSL *ssl); +int ssl3_flush_flight(SSL *ssl); int dtls1_init_message(SSL *ssl, CBB *cbb, CBB *body, uint8_t type); int dtls1_finish_message(SSL *ssl, CBB *cbb, uint8_t **out_msg, size_t *out_len); -int dtls1_queue_message(SSL *ssl, uint8_t *msg, size_t len); +int dtls1_add_message(SSL *ssl, uint8_t *msg, size_t len); +int dtls1_add_change_cipher_spec(SSL *ssl); +int dtls1_add_alert(SSL *ssl, uint8_t level, uint8_t desc); int dtls1_write_message(SSL *ssl); -int dtls1_send_change_cipher_spec(SSL *ssl); int dtls1_flush_flight(SSL *ssl); -/* ssl_complete_message calls |finish_message| and |queue_message| on |cbb| to - * queue the message for writing. */ -int ssl_complete_message(SSL *ssl, CBB *cbb); +/* ssl_add_message_cbb finishes the handshake message in |cbb| and adds it to + * the pending flight. It returns one on success and zero on error. */ +int ssl_add_message_cbb(SSL *ssl, CBB *cbb); /* ssl_hash_current_message incorporates the current handshake message into the * handshake hash. It returns one on success and zero on allocation failure. */ @@ -1842,13 +1852,6 @@ void dtls1_get_current_message(const SSL *ssl, CBS *out); void dtls1_release_current_message(SSL *ssl, int free_buffer); int dtls1_dispatch_alert(SSL *ssl); -/* ssl_is_wbio_buffered returns one if |ssl|'s write BIO is buffered and zero - * otherwise. */ -int ssl_is_wbio_buffered(const SSL *ssl); - -int ssl_init_wbio_buffer(SSL *ssl); -void ssl_free_wbio_buffer(SSL *ssl); - int tls1_change_cipher_state(SSL_HANDSHAKE *hs, int which); int tls1_handshake_digest(SSL *ssl, uint8_t *out, size_t out_len); int tls1_generate_master_secret(SSL *ssl, uint8_t *out, const uint8_t *premaster, diff --git a/ssl/s3_both.c b/ssl/s3_both.c index 492884f78..133cd2d9d 100644 --- a/ssl/s3_both.c +++ b/ssl/s3_both.c @@ -180,33 +180,44 @@ void ssl_handshake_free(SSL_HANDSHAKE *hs) { OPENSSL_free(hs); } -/* ssl3_do_write sends |ssl->init_buf| in records of type 'type' - * (SSL3_RT_HANDSHAKE or SSL3_RT_CHANGE_CIPHER_SPEC). It returns 1 on success - * and <= 0 on error. */ -static int ssl3_do_write(SSL *ssl, int type, const uint8_t *data, size_t len) { - int ret = ssl3_write_bytes(ssl, type, data, len); - if (ret <= 0) { - return ret; +static int add_record_to_flight(SSL *ssl, uint8_t type, const uint8_t *in, + size_t in_len) { + /* We'll never add a flight while in the process of writing it out. */ + assert(ssl->s3->pending_flight_offset == 0); + + if (ssl->s3->pending_flight == NULL) { + ssl->s3->pending_flight = BUF_MEM_new(); + if (ssl->s3->pending_flight == NULL) { + return 0; + } } - /* ssl3_write_bytes writes the data in its entirety. */ - assert((size_t)ret == len); - ssl_do_msg_callback(ssl, 1 /* write */, type, data, len); + size_t max_out = in_len + SSL_max_seal_overhead(ssl); + size_t new_cap = ssl->s3->pending_flight->length + max_out; + if (max_out < in_len || new_cap < max_out) { + OPENSSL_PUT_ERROR(SSL, ERR_R_OVERFLOW); + return 0; + } + + size_t len; + if (!BUF_MEM_reserve(ssl->s3->pending_flight, new_cap) || + !tls_seal_record(ssl, (uint8_t *)ssl->s3->pending_flight->data + + ssl->s3->pending_flight->length, + &len, max_out, type, in, in_len)) { + return 0; + } + + ssl->s3->pending_flight->length += len; return 1; } int ssl3_init_message(SSL *ssl, CBB *cbb, CBB *body, uint8_t type) { - CBB_zero(cbb); - if (ssl->s3->pending_message != NULL) { - OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR); - return 0; - } - /* Pick a modest size hint to save most of the |realloc| calls. */ if (!CBB_init(cbb, 64) || !CBB_add_u8(cbb, type) || !CBB_add_u24_length_prefixed(cbb, body)) { OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR); + CBB_cleanup(cbb); return 0; } @@ -223,47 +234,111 @@ int ssl3_finish_message(SSL *ssl, CBB *cbb, uint8_t **out_msg, return 1; } -int ssl3_queue_message(SSL *ssl, uint8_t *msg, size_t len) { - if (ssl->s3->pending_message != NULL || - len > 0xffffffffu) { - OPENSSL_free(msg); - OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR); +int ssl3_add_message(SSL *ssl, uint8_t *msg, size_t len) { + /* Add the message to the current flight, splitting into several records if + * needed. */ + int ret = 0; + size_t added = 0; + do { + size_t todo = len - added; + if (todo > ssl->max_send_fragment) { + todo = ssl->max_send_fragment; + } + + if (!add_record_to_flight(ssl, SSL3_RT_HANDSHAKE, msg + added, todo)) { + goto err; + } + added += todo; + } while (added < len); + + ssl_do_msg_callback(ssl, 1 /* write */, SSL3_RT_HANDSHAKE, msg, len); + ssl3_update_handshake_hash(ssl, msg, len); + ret = 1; + +err: + OPENSSL_free(msg); + return ret; +} + +int ssl3_add_change_cipher_spec(SSL *ssl) { + static const uint8_t kChangeCipherSpec[1] = {SSL3_MT_CCS}; + + if (!add_record_to_flight(ssl, SSL3_RT_CHANGE_CIPHER_SPEC, kChangeCipherSpec, + sizeof(kChangeCipherSpec))) { return 0; } - ssl3_update_handshake_hash(ssl, msg, len); - - ssl->s3->pending_message = msg; - ssl->s3->pending_message_len = (uint32_t)len; + ssl_do_msg_callback(ssl, 1 /* write */, SSL3_RT_CHANGE_CIPHER_SPEC, + kChangeCipherSpec, sizeof(kChangeCipherSpec)); return 1; } -int ssl_complete_message(SSL *ssl, CBB *cbb) { +int ssl3_add_alert(SSL *ssl, uint8_t level, uint8_t desc) { + uint8_t alert[2] = {level, desc}; + if (!add_record_to_flight(ssl, SSL3_RT_ALERT, alert, sizeof(alert))) { + return 0; + } + + ssl_do_msg_callback(ssl, 1 /* write */, SSL3_RT_ALERT, alert, sizeof(alert)); + ssl_do_info_callback(ssl, SSL_CB_WRITE_ALERT, ((int)level << 8) | desc); + return 1; +} + +int ssl_add_message_cbb(SSL *ssl, CBB *cbb) { uint8_t *msg; size_t len; if (!ssl->method->finish_message(ssl, cbb, &msg, &len) || - !ssl->method->queue_message(ssl, msg, len)) { + !ssl->method->add_message(ssl, msg, len)) { return 0; } return 1; } -int ssl3_write_message(SSL *ssl) { - if (ssl->s3->pending_message == NULL) { - OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR); - return 0; +int ssl3_write_message(SSL *ssl) { return 1; } + +int ssl3_flush_flight(SSL *ssl) { + if (ssl->s3->pending_flight == NULL) { + return 1; } - int ret = ssl3_do_write(ssl, SSL3_RT_HANDSHAKE, ssl->s3->pending_message, - ssl->s3->pending_message_len); + if (ssl->s3->pending_flight->length > 0xffffffff || + ssl->s3->pending_flight->length > INT_MAX) { + OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR); + return -1; + } + + /* The handshake flight buffer is mutually exclusive with application data. + * + * TODO(davidben): This will not be true when closure alerts use this. */ + if (ssl_write_buffer_is_pending(ssl)) { + OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR); + return -1; + } + + /* Write the pending flight. */ + while (ssl->s3->pending_flight_offset < ssl->s3->pending_flight->length) { + int ret = BIO_write( + ssl->wbio, + ssl->s3->pending_flight->data + ssl->s3->pending_flight_offset, + ssl->s3->pending_flight->length - ssl->s3->pending_flight_offset); + if (ret <= 0) { + ssl->rwstate = SSL_WRITING; + return ret; + } + + ssl->s3->pending_flight_offset += ret; + } + + int ret = BIO_flush(ssl->wbio); if (ret <= 0) { + ssl->rwstate = SSL_WRITING; return ret; } - OPENSSL_free(ssl->s3->pending_message); - ssl->s3->pending_message = NULL; - ssl->s3->pending_message_len = 0; + BUF_MEM_free(ssl->s3->pending_flight); + ssl->s3->pending_flight = NULL; + ssl->s3->pending_flight_offset = 0; return 1; } @@ -307,7 +382,7 @@ int ssl3_send_finished(SSL_HANDSHAKE *hs, int a, int b) { CBB cbb, body; if (!ssl->method->init_message(ssl, &cbb, &body, SSL3_MT_FINISHED) || !CBB_add_bytes(&body, finished, finished_len) || - !ssl_complete_message(ssl, &cbb)) { + !ssl_add_message_cbb(ssl, &cbb)) { OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR); CBB_cleanup(&cbb); return -1; @@ -365,18 +440,11 @@ int ssl3_get_finished(SSL_HANDSHAKE *hs) { return 1; } -int ssl3_send_change_cipher_spec(SSL *ssl) { - static const uint8_t kChangeCipherSpec[1] = {SSL3_MT_CCS}; - - return ssl3_do_write(ssl, SSL3_RT_CHANGE_CIPHER_SPEC, kChangeCipherSpec, - sizeof(kChangeCipherSpec)); -} - int ssl3_output_cert_chain(SSL *ssl) { CBB cbb, body; if (!ssl->method->init_message(ssl, &cbb, &body, SSL3_MT_CERTIFICATE) || !ssl_add_cert_chain(ssl, &body) || - !ssl_complete_message(ssl, &cbb)) { + !ssl_add_message_cbb(ssl, &cbb)) { OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR); CBB_cleanup(&cbb); return 0; diff --git a/ssl/s3_lib.c b/ssl/s3_lib.c index 703941828..3f44629cb 100644 --- a/ssl/s3_lib.c +++ b/ssl/s3_lib.c @@ -209,7 +209,7 @@ void ssl3_free(SSL *ssl) { OPENSSL_free(ssl->s3->alpn_selected); SSL_AEAD_CTX_free(ssl->s3->aead_read_ctx); SSL_AEAD_CTX_free(ssl->s3->aead_write_ctx); - OPENSSL_free(ssl->s3->pending_message); + BUF_MEM_free(ssl->s3->pending_flight); OPENSSL_cleanse(ssl->s3, sizeof *ssl->s3); OPENSSL_free(ssl->s3); diff --git a/ssl/s3_pkt.c b/ssl/s3_pkt.c index 9bd9f1fdd..5a31e1f70 100644 --- a/ssl/s3_pkt.c +++ b/ssl/s3_pkt.c @@ -267,6 +267,14 @@ static int do_ssl3_write(SSL *ssl, int type, const uint8_t *buf, unsigned len) { return ssl3_write_pending(ssl, type, buf, len); } + /* The handshake flight buffer is mutually exclusive with application data. + * + * TODO(davidben): This will not be true when closure alerts use this. */ + if (ssl->s3->pending_flight != NULL) { + OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR); + return -1; + } + /* If we have an alert to send, lets send it */ if (ssl->s3->alert_dispatch) { int ret = ssl->method->dispatch_alert(ssl); diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c index d653378fc..101ee4dae 100644 --- a/ssl/ssl_lib.c +++ b/ssl/ssl_lib.c @@ -504,9 +504,6 @@ void SSL_free(SSL *ssl) { CRYPTO_free_ex_data(&g_ex_data_class_ssl, ssl, &ssl->ex_data); - ssl_free_wbio_buffer(ssl); - assert(ssl->bbio == NULL); - BIO_free_all(ssl->rbio); BIO_free_all(ssl->wbio); @@ -553,18 +550,8 @@ void SSL_set0_rbio(SSL *ssl, BIO *rbio) { } void SSL_set0_wbio(SSL *ssl, BIO *wbio) { - /* If the output buffering BIO is still in place, remove it. */ - if (ssl->bbio != NULL) { - ssl->wbio = BIO_pop(ssl->wbio); - } - BIO_free_all(ssl->wbio); ssl->wbio = wbio; - - /* Re-attach |bbio| to the new |wbio|. */ - if (ssl->bbio != NULL) { - ssl->wbio = BIO_push(ssl->bbio, ssl->wbio); - } } void SSL_set_bio(SSL *ssl, BIO *rbio, BIO *wbio) { @@ -603,14 +590,7 @@ void SSL_set_bio(SSL *ssl, BIO *rbio, BIO *wbio) { BIO *SSL_get_rbio(const SSL *ssl) { return ssl->rbio; } -BIO *SSL_get_wbio(const SSL *ssl) { - if (ssl->bbio != NULL) { - /* If |bbio| is active, the true caller-configured BIO is its |next_bio|. */ - assert(ssl->bbio == ssl->wbio); - return ssl->bbio->next_bio; - } - return ssl->wbio; -} +BIO *SSL_get_wbio(const SSL *ssl) { return ssl->wbio; } void ssl_reset_error_state(SSL *ssl) { /* Functions which use |SSL_get_error| must reset I/O and error state on @@ -2023,48 +2003,6 @@ const COMP_METHOD *SSL_get_current_expansion(SSL *ssl) { return NULL; } int *SSL_get_server_tmp_key(SSL *ssl, EVP_PKEY **out_key) { return 0; } -int ssl_is_wbio_buffered(const SSL *ssl) { - return ssl->bbio != NULL; -} - -int ssl_init_wbio_buffer(SSL *ssl) { - if (SSL_is_dtls(ssl)) { - /* DTLS does not use the BIO buffer. - * TODO(davidben): Remove this altogether when TLS no longer uses it. - * https://crbug.com/boringssl/72. */ - return 1; - } - - if (ssl->bbio != NULL) { - /* Already buffered. */ - assert(ssl->bbio == ssl->wbio); - return 1; - } - - BIO *bbio = BIO_new(BIO_f_buffer()); - if (bbio == NULL || - !BIO_set_read_buffer_size(bbio, 1)) { - BIO_free(bbio); - return 0; - } - - ssl->bbio = bbio; - ssl->wbio = BIO_push(bbio, ssl->wbio); - return 1; -} - -void ssl_free_wbio_buffer(SSL *ssl) { - if (ssl->bbio == NULL) { - return; - } - - assert(ssl->bbio == ssl->wbio); - - ssl->wbio = BIO_pop(ssl->wbio); - BIO_free(ssl->bbio); - ssl->bbio = NULL; -} - void SSL_CTX_set_quiet_shutdown(SSL_CTX *ctx, int mode) { ctx->quiet_shutdown = (mode != 0); } diff --git a/ssl/tls13_both.c b/ssl/tls13_both.c index 142566520..3805bb14a 100644 --- a/ssl/tls13_both.c +++ b/ssl/tls13_both.c @@ -453,7 +453,7 @@ int tls13_prepare_certificate(SSL_HANDSHAKE *hs) { } if (!ssl_has_certificate(ssl)) { - if (!ssl_complete_message(ssl, &cbb)) { + if (!ssl_add_message_cbb(ssl, &cbb)) { goto err; } @@ -507,7 +507,7 @@ int tls13_prepare_certificate(SSL_HANDSHAKE *hs) { } } - if (!ssl_complete_message(ssl, &cbb)) { + if (!ssl_add_message_cbb(ssl, &cbb)) { goto err; } @@ -569,7 +569,7 @@ enum ssl_private_key_result_t tls13_prepare_certificate_verify( } if (!CBB_did_write(&child, sig_len) || - !ssl_complete_message(ssl, &cbb)) { + !ssl_add_message_cbb(ssl, &cbb)) { goto err; } @@ -595,7 +595,7 @@ int tls13_prepare_finished(SSL_HANDSHAKE *hs) { CBB cbb, body; if (!ssl->method->init_message(ssl, &cbb, &body, SSL3_MT_FINISHED) || !CBB_add_bytes(&body, verify_data, verify_data_len) || - !ssl_complete_message(ssl, &cbb)) { + !ssl_add_message_cbb(ssl, &cbb)) { CBB_cleanup(&cbb); return 0; } diff --git a/ssl/tls13_client.c b/ssl/tls13_client.c index 6f2bb218c..2d46bf2e5 100644 --- a/ssl/tls13_client.c +++ b/ssl/tls13_client.c @@ -542,7 +542,7 @@ static enum ssl_hs_wait_t do_send_channel_id(SSL_HANDSHAKE *hs) { CBB cbb, body; if (!ssl->method->init_message(ssl, &cbb, &body, SSL3_MT_CHANNEL_ID) || !tls1_write_channel_id(ssl, &body) || - !ssl_complete_message(ssl, &cbb)) { + !ssl_add_message_cbb(ssl, &cbb)) { CBB_cleanup(&cbb); return ssl_hs_error; } diff --git a/ssl/tls13_server.c b/ssl/tls13_server.c index 750e47fa4..1f10038f3 100644 --- a/ssl/tls13_server.c +++ b/ssl/tls13_server.c @@ -349,7 +349,7 @@ static enum ssl_hs_wait_t do_send_hello_retry_request(SSL_HANDSHAKE *hs) { !CBB_add_u16(&extensions, TLSEXT_TYPE_key_share) || !CBB_add_u16(&extensions, 2 /* length */) || !CBB_add_u16(&extensions, group_id) || - !ssl_complete_message(ssl, &cbb)) { + !ssl_add_message_cbb(ssl, &cbb)) { CBB_cleanup(&cbb); return ssl_hs_error; } @@ -417,7 +417,7 @@ static enum ssl_hs_wait_t do_send_server_hello(SSL_HANDSHAKE *hs) { } } - if (!ssl_complete_message(ssl, &cbb)) { + if (!ssl_add_message_cbb(ssl, &cbb)) { goto err; } @@ -443,7 +443,7 @@ static enum ssl_hs_wait_t do_send_encrypted_extensions(SSL_HANDSHAKE *hs) { if (!ssl->method->init_message(ssl, &cbb, &body, SSL3_MT_ENCRYPTED_EXTENSIONS) || !ssl_add_serverhello_tlsext(hs, &body) || - !ssl_complete_message(ssl, &cbb)) { + !ssl_add_message_cbb(ssl, &cbb)) { CBB_cleanup(&cbb); return ssl_hs_error; } @@ -488,7 +488,7 @@ static enum ssl_hs_wait_t do_send_certificate_request(SSL_HANDSHAKE *hs) { if (!ssl_add_client_CA_list(ssl, &body) || !CBB_add_u16(&body, 0 /* empty certificate_extensions. */) || - !ssl_complete_message(ssl, &cbb)) { + !ssl_add_message_cbb(ssl, &cbb)) { goto err; } @@ -692,7 +692,7 @@ static enum ssl_hs_wait_t do_send_new_session_ticket(SSL_HANDSHAKE *hs) { goto err; } - if (!ssl_complete_message(ssl, &cbb)) { + if (!ssl_add_message_cbb(ssl, &cbb)) { goto err; } diff --git a/ssl/tls_method.c b/ssl/tls_method.c index a6584c192..21f9e2ad2 100644 --- a/ssl/tls_method.c +++ b/ssl/tls_method.c @@ -100,14 +100,6 @@ static uint16_t ssl3_version_to_wire(uint16_t version) { static int ssl3_supports_cipher(const SSL_CIPHER *cipher) { return 1; } -static int ssl3_flush_flight(SSL *ssl) { - int ret = BIO_flush(ssl->wbio); - if (ret <= 0) { - ssl->rwstate = SSL_WRITING; - } - return ret; -} - static void ssl3_expect_flight(SSL *ssl) {} static void ssl3_received_flight(SSL *ssl) {} @@ -155,14 +147,15 @@ static const SSL_PROTOCOL_METHOD kTLSProtocolMethod = { ssl3_supports_cipher, ssl3_init_message, ssl3_finish_message, - ssl3_queue_message, - ssl3_write_message, - ssl3_send_change_cipher_spec, + ssl3_add_message, + ssl3_add_change_cipher_spec, + ssl3_add_alert, ssl3_flush_flight, ssl3_expect_flight, ssl3_received_flight, ssl3_set_read_state, ssl3_set_write_state, + ssl3_write_message, }; const SSL_METHOD *TLS_method(void) {