Summary:ASTERISK-18342: close() before SSL_Shutdown() in ssl_close()
Reporter:Stephane Chazelas (stephane.chazelas)Labels:
Date Opened:2011-08-24 15:09:51Date Closed:2012-08-27 10:41:47
Versions:SVN 1.8.4 Frequency of
is related toASTERISK-18345 [patch] sips connection dropped by asterisk with a large INVITE
is related toASTERISK-18700 chan_sip.c and tcptls.c - possible double close of file descriptor
Description:In http://svn.digium.com/svn/asterisk/trunk/main/tcptls.c

static int ssl_close(void *cookie)
       return 0;

If I understand correctly, that means that the SSL socket won't be torn down correctly, and the SSL_shutdown() is useless.

Is there any reason for it being that way?
Comments:By: Leif Madsen (lmadsen) 2011-08-30 14:00:20.278-0500

This is really a discussion item that should be brought up on the Asterisk-dev mailing list. Please reference this issue, but have the discussion on the mailing list. When it is determined to be a bug or not, please update the issue so it can be moved to the proper status. Thanks!

By: Leif Madsen (lmadsen) 2011-09-14 10:43:21.238-0500

Did you bring this up on the mailing list? Can you link to the discussion here? Thanks!

By: Stephane Chazelas (stephane.chazelas) 2011-09-14 14:39:28.796-0500

I did, see http://thread.gmane.org/gmane.comp.telephony.pbx.asterisk.devel/48151
Though I didn't get any reply.

Here is what I said on the list:

it says it all above. I noticed that behavior when observing
wireshark traces that showed an SSL connection ending up
abnormally (TCP connection close without "close notify" shutdown
alert), and saw that code above, so was wondering about the
rationale behind that (like forcing renegotiation of keys upon
reconnection or stuff like that).

It looks very much like a bug to me, or the lazy way to close a
non-blocking SSL connection (being non-blocking, the
SSL_shutdown would have to be done asynchronously which looks
like it's gonna be tricky with the current design).

See related Bug ASTERISK-18345

By: Jonathan Rose (jrose) 2011-11-30 16:08:24.741-0600

This issue was referenced in discussion on https://reviewboard.asterisk.org/r/1576/ and might possibly have been fixed from some of our changes there.

By: frank koster (notthematrix) 2012-06-04 18:08:16.622-0500


this https://issues.asterisk.org/jira/secure/attachment/43792/tls_read.patch
Seem to fix this issue in all version and 10.5.0.
please add theese lines to the code so TLS can be used again.

By: Matt Jordan (mjordan) 2012-08-27 10:41:11.929-0500

The ssl_close function was reworked to do the following:

* If the cookie file descriptor is not valid, do nothing.
* Call SSL_shutdown on the cookie
* Call SSL_free on the cookie
* Close the cookie file descriptor

A comment in the code addresses the asynchronous concern:

"According to the TLS standard, it is acceptable for an application to only send its shutdown alert and then close the underlying connection without waiting for the peer's response (this way resources can be saved, as the process can already terminate or serve another connection)"

Note that this explains why SSL_shutdown is now called first on the cookie, before closing the file descriptor.

By: Matt Jordan (mjordan) 2012-08-27 10:41:38.964-0500

The issue discovered with jitsi in ASTERISK-18345 is a separate issue.  As such, I'll be closing this issue out as fixed.