Always buffer DTLS retransmits.
The DTLS bbio logic is rather problematic, but this shouldn't make things
worse. In the in-handshake case, the new code merges the per-message
(unchecked) BIO_flush calls into one call at the end but otherwise the BIO is
treated as is. Otherwise any behavior around non-block writes should be
preserved.
In the post-handshake case, we now install the buffer when we didn't
previously. On write error, the buffer will have garbage in it, but it will be
discarded, so that will preserve any existing retry behavior. (Arguably the
existing retry behavior is a bug, but that's another matter.)
Add a test for all this, otherwise it is sure to regress. Testing for
record-packing is a little fuzzy, but we can assert ChangeCipherSpec always
shares a record with something.
BUG=57
Change-Id: I8603f20811d502c71ded2943b0e72a8bdc4e46f2
Reviewed-on: https://boringssl-review.googlesource.com/7871
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/ssl/d1_both.c b/ssl/d1_both.c
index cc95a70..7bd2824 100644
--- a/ssl/d1_both.c
+++ b/ssl/d1_both.c
@@ -737,25 +737,38 @@
ret = dtls1_do_handshake_write(ssl, use_epoch);
}
- /* TODO(davidben): Check return value? */
- (void)BIO_flush(SSL_get_wbio(ssl));
return ret;
}
-
int dtls1_retransmit_buffered_messages(SSL *ssl) {
- pqueue sent = ssl->d1->sent_messages;
- piterator iter = pqueue_iterator(sent);
- pitem *item;
+ /* Ensure we are packing handshake messages. */
+ const int was_buffered = ssl_is_wbio_buffered(ssl);
+ assert(was_buffered == SSL_in_init(ssl));
+ if (!was_buffered && !ssl_init_wbio_buffer(ssl, 1)) {
+ return -1;
+ }
+ assert(ssl_is_wbio_buffered(ssl));
+ int ret = -1;
+ piterator iter = pqueue_iterator(ssl->d1->sent_messages);
+ pitem *item;
for (item = pqueue_next(&iter); item != NULL; item = pqueue_next(&iter)) {
hm_fragment *frag = (hm_fragment *)item->data;
if (dtls1_retransmit_message(ssl, frag) <= 0) {
- return -1;
+ goto err;
}
}
- return 1;
+ /* TODO(davidben): Check return value? */
+ (void)BIO_flush(SSL_get_wbio(ssl));
+
+ ret = 1;
+
+err:
+ if (!was_buffered) {
+ ssl_free_wbio_buffer(ssl);
+ }
+ return ret;
}
/* dtls1_buffer_change_cipher_spec adds a ChangeCipherSpec to the current