From 17a5f85cbbb8e637af0abfccfd8f21eb1054c9a3 Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Sun, 11 Jan 2015 19:46:52 -0500 Subject: [PATCH] Clarify dtls1_do_write's interaction with the buffering BIO. The existing comments are not very helpful. This code is also quite buggy. Document two of them as TODOs. Change-Id: Idfaf93d9c3b8b1ee92f2fb0d292ef513b5f6d824 Reviewed-on: https://boringssl-review.googlesource.com/2830 Reviewed-by: Adam Langley --- ssl/d1_both.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/ssl/d1_both.c b/ssl/d1_both.c index 26044660..5edc93fe 100644 --- a/ssl/d1_both.c +++ b/ssl/d1_both.c @@ -278,15 +278,27 @@ int dtls1_do_write(SSL *s, int type) { frag_off = 0; while (s->init_num) { + /* Account for data in the buffering BIO; multiple records may be packed + * into a single packet during the handshake. + * + * TODO(davidben): This is buggy; if the MTU is larger than the buffer size, + * the large record will be split across two packets. Moreover, in that + * case, the |dtls1_write_bytes| call may not return synchronously. This + * will break on retry as the |s->init_off| and |s->init_num| adjustment + * will run a second time. */ curr_mtu = s->d1->mtu - BIO_wpending(SSL_get_wbio(s)) - DTLS1_RT_HEADER_LENGTH - max_overhead; if (curr_mtu <= DTLS1_HM_HEADER_LENGTH) { - /* grr.. we could get an error if MTU picked was wrong */ + /* Flush the buffer and continue with a fresh packet. + * + * TODO(davidben): If |BIO_flush| is not synchronous and requires multiple + * calls to |dtls1_do_write|, |frag_off| will be wrong. */ ret = BIO_flush(SSL_get_wbio(s)); if (ret <= 0) { return ret; } + assert(BIO_wpending(SSL_get_wbio(s)) == 0); curr_mtu = s->d1->mtu - DTLS1_RT_HEADER_LENGTH - max_overhead; }