[asterisk-dev] [Code Review] 2783: Fix SIP/TLS reading - random connection drop

[Matt Jordan]

> On Oct. 13, 2014, 7:32 a.m., Olle E Johansson wrote:
> > What's the status of this patch? Should it be closed?

I think it should be closed out. The long, tortuous history is as follows:

(1) Mark Michelson, under ASTERISK-20212 (https://reviewboard.asterisk.org/r/2123) - as part of fixing chan_sip's usage of a FILE handle in this portion of the code - also fixed chan_sip mangling partial packets received on a TCP read. The usage of an overflow buffer allows for portions of a SIP message that aren't part of the current read to be deferred until a subsequent read operation. While that worked for TCP, it didn't resolve Asterisk's invalid handling of the messages under a TLS read, which had larger issues - namely, our usage of OpenSSL in the TCP/TLS layer. This was largely addressed by...
(2) Richard Mudgett, under ASTERISK-23673 (which was a security vulnerability). Richard had to re-work substantial portions of the TCP/TLS code as part of this vulnerability. A portion of that included rewriting much of the TLS read/write routines to better follow David Lee's suggestions on this issue. Unfortunately, chan_sip still didn't perform the TLS read correctly, which led to...
(3) Alexander Traud, in https://reviewboard.asterisk.org/r/3653/, tweaking some of the polling issues in chan_sip's usage of the TLS read. That still left an issue where we would not read all of the bytes on a read attempt, in a similar fashion to what the TCP reads used to do prior to Mark's patch. That led to...
(4) Elazar Broad consolidating the TCP/TLS reads into a single function in chan_sip in https://reviewboard.asterisk.org/r/3882, which resolved the issue where we would fail to read all of the bytes waiting for us.

And we should now properly handle large packets received over a TLS connection.


- Matt


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/2783/#review13494
-----------------------------------------------------------


On Aug. 21, 2013, 3:24 p.m., Tzafrir Cohen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2783/
> -----------------------------------------------------------
> 
> (Updated Aug. 21, 2013, 3:24 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-18345
>     https://issues.asterisk.org/jira/browse/ASTERISK-18345
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> Symptom: Asterisk drops a SIP/TLS connection: debugging reports that it has failed to read it.
> 
> I can reproduce this on my system when the TLS client is Asterisk 11.5 (installed from the Debian package) set with 'allow=all' to get a long list of codecs.
> 
> Calling ast_wait_for_input before every fgets is not sufficient.
> Function fgets internally calls read (=> SSL_read) until either "\n" or
> eof is found. And because the socket is polled only before the first
> SSL_read call, consequent calls can fail and return <=0 even though the
> data are on the way.
>     
> This is fixed by adding a read() loop inside the ssl_read() hook.
> 
> I came accross this patch today and it looks like it fixes my problem (see my comment at the end). The patch I used is by Filip Jenicek. See the bug report for the full log.
> 
> 
> Diffs
> -----
> 
>   /trunk/main/tcptls.c 397346 
> 
> Diff: https://reviewboard.asterisk.org/r/2783/diff/
> 
> 
> Testing
> -------
> 
> Work on trunk.
> 
> 
> Thanks,
> 
> Tzafrir Cohen
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20141013/9daabc14/attachment-0001.html>