response write to socket

11 views
Skip to first unread message

Solar Designer

unread,
Mar 16, 2016, 9:27:37 PM3/16/16
to onion-dev
David,

When reviewing the poller code recently, I found it suspicious that you
don't poll for readiness to write (no EPOLLOUT, nor the handling this
would require).

This probably means that large or just unlucky responses may block the
thread or/and be truncated (or worse, have portions missing).

The sockets don't appear to be set to non-blocking, or are they? So
blocking on response write is possible, correct? If so, just a handful
of malicious or unlucky client connections (matching the number of
threads on the server) may result in a denial-of-service, even if the
number of fd's on the server is much larger. (In practice, this would
be tricky or impossible to do when responses are small. But some
servers may have URLs with sufficiently large responses.)

http.c: onion_http_write() simply uses the write(2) syscall, and returns
its return value. response.c sometimes uses ->write()'s return value,
and sometimes not. When it does not, a portion of the data may end up
being omitted or the response truncated. In particular, this is the
case when sending headers and short strings specific to chunked-encoding,

The code sending headers will require some effort to repair because
there are multiple internal APIs that don't pass a status back - e.g.,
even if write_header() is changed to have a non-void return type,
there's also onion_dict_preorder() to which write_header is passed, and
three other functions in the file call onion_response_write_headers()
ignoring its return value.

For a long-term fix, I think it'd be better for the poller (rewritten
from scratch) to manage both reads and writes.

Alexander
Reply all
Reply to author
Forward
0 new messages