Re: [PATCH] Improve application data throughput

[Enrico Scholz]

Simon Josefsson <address@hidden> writes:

>> There was added an overflow check too which assumes 1MiB as maximum
>> size for a GSASL message.  This value is a blind guess; please adjust
>> when specifications require a higher value.
>
> I don't understand this part -- can you explain why it is needed?  

Both

|-                sockpos++;
|+                sockpos += len;

operations can wrap and they (resp. the called recv functions) can cause
buffer overflows.  We could check for this wrap, or (as I did) limit the
maximum buffer size to prevent DOS attacks which allocate huge amounts
of memory.

Such limits should be chosen that they do not violate specifications
resp. real-world implementations.  I chose 1 MiB based upon my
observations.


>> I see only one problem with the patch:  when remote site stops sending
>> in the middle of a TLS record, the 'gsasl' application will hang in
>> gnutls_record_recv() forever.
>
> When would that happen?  If the network connection goes down? 

yes; e.g. somebody unplugs a cable


> I thought there were some TCP timeouts that would kick in after some
> time without any data.

afaik, recv() does not have such a timeout (it does not send packets
which require an ACK and won't get information about dropped connection
hence).


With old code, waiting would happen exclusively in select() which listens
both for network and for application events.  When application sees that
there did not came data for a certain time, it could close its input
pipe, select() returns and 'gsasl' sees that application hung up and
exits.

With new code, application would have to send a signal and abort 'gsasl'
process.

It should be possible to put network and perhaps application sockets in
non-blocking mode and restore the old behaviour.  But I did not tested
it.



Enrico