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>

1 files changed, 3 insertions, 2 deletions

diff --git a/src/lib/tls/buffers_tls.c b/src/lib/tls/buffers_tls.c
index b92a14d6a1..1b99467d2b 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;
 }