commit 3adabaf3e925f3ad395a2a0a2dbc92aa1d018ec4 Author: David Goulet dgoulet@torproject.org Date: Fri Jun 19 10:35:50 2020 -0400

tls: Make buf_read_from_tls() read at most bytes

The buf_read_from_tls() function was designed to read up to a certain number of bytes a TLS socket using read_to_chunk_tls() which boils down to SSL_read() (with OpenSSL, common case).

However, at the end of the loop, the returned number of bytes from read_to_chunk_tls() was treated like the syscall read() for which if less bytes than the total asked are returned, it signals EOF.

But, with SSL_read(), it returns up to a TLS record which can be less than what was asked. The assumption that it was EOF was wrong which made the while loop exiting before it was able to consume all requested bytes (at_most parameter).

The general use case that Tor sees is that it will ask the network layer to give it at most 16KB (that is roughly 32 cells) but because of KIST scheduler, the highest possible TLS record we currently observe is 4096 bytes (4KB or 8 cells). Thus the loop would at best always return 8 cells even though much more could be on the TLS socket. See ticket #40006 for more details.

Fixes #40006

Signed-off-by: David Goulet dgoulet@torproject.org --- changes/ticket40006 | 6 ++++++ src/lib/tls/buffers_tls.c | 5 +++-- 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/changes/ticket40006 b/changes/ticket40006 new file mode 100644 index 000000000..ad10e236c --- /dev/null +++ b/changes/ticket40006 @@ -0,0 +1,6 @@ + o Major bugfix (TLS, buffer): + - When attempting to read N bytes on a TLS connection, really try to read + those N bytes. Before that, Tor would stop reading after the first TLS + record which can be smaller than N bytes even though more data was waiting + on the TLS connection socket. The remaining data would have been read at + the next mainloop event. Fixes bug 40006; bugfix on 0.1.0.5-rc. diff --git a/src/lib/tls/buffers_tls.c b/src/lib/tls/buffers_tls.c index b92a14d6a..1b99467d2 100644 --- a/src/lib/tls/buffers_tls.c +++ b/src/lib/tls/buffers_tls.c @@ -59,6 +59,9 @@ read_to_chunk_tls(buf_t *buf, chunk_t *chunk, tor_tls_t *tls, * Second, the TLS stream's events do not correspond directly to network * events: sometimes, before a TLS stream can read, the network must be * ready to write -- or vice versa. + * + * On success, return the number of bytes read. On error, a TOR_TLS_* negative + * code is returned (expect any of them except TOR_TLS_DONE). */ int buf_read_from_tls(buf_t *buf, tor_tls_t *tls, size_t at_most) @@ -92,8 +95,6 @@ buf_read_from_tls(buf_t *buf, tor_tls_t *tls, size_t at_most) return r; /* Error */ tor_assert(total_read+r <= BUF_MAX_LEN); total_read += r; - if ((size_t)r < readlen) /* eof, block, or no more to read. */ - break; } return (int)total_read; }