Summary:	ASTERISK-18342: close() before SSL_Shutdown() in ssl_close()
Reporter:	Stephane Chazelas (stephane.chazelas)	Labels:
Date Opened:	2011-08-24 15:09:51	Date Closed:	2012-08-27 10:41:47
Priority:	Major	Regression?	No
Status:	Closed/Complete	Components:	Channels/chan_sip/TCP-TLS
Versions:	SVN 1.8.4	Frequency of Occurrence	Constant
Related Issues:	is related to ASTERISK-18345 [patch] sips connection dropped by asterisk with a large INVITE is related to ASTERISK-18700 chan_sip.c and tcptls.c - possible double close of file descriptor
Environment:		Attachments:
Description:	In http://svn.digium.com/svn/asterisk/trunk/main/tcptls.c {code} static int ssl_close(void *cookie) {        close(SSL_get_fd(cookie));        SSL_shutdown(cookie);        SSL_free(cookie);        return 0; } {code} 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 https://issues.asterisk.org/jira/browse/ASTERISK-18345?focusedCommentId=193382#comment-193382 this https://issues.asterisk.org/jira/secure/attachment/43792/tls_read.patch Seem to fix this issue in all version 1.8.12.2 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.