Skip to content

[Bug fix 3711] Close the file descriptor when TLS handshake fails with a TLS server …#3918

Open
GouthamPMadhava wants to merge 1 commit into
OpenSIPS:3.4from
GouthamPMadhava:fix/OpenSIPsbug3711-FixFDLeakOnTLSConnFailure
Open

[Bug fix 3711] Close the file descriptor when TLS handshake fails with a TLS server …#3918
GouthamPMadhava wants to merge 1 commit into
OpenSIPS:3.4from
GouthamPMadhava:fix/OpenSIPsbug3711-FixFDLeakOnTLSConnFailure

Conversation

@GouthamPMadhava

Copy link
Copy Markdown

…successfully listening on the ports.

Summary

Details

Solution

Compatibility

Closing issues

sh_log(c->hist, TCP_SEND2MAIN, "send 1, (%d)", c->refcnt);
/* close the fd if this process is not meant to own it */
if (c->proc_id != process_no)
close(fd);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is here a close(fd) and both reported cases (where you did your extra closes) are landing here - so it shouldn't be needed the close you added .
If so, could you check the value of the c->proc_id ? (just add a LM_DBG or let me know how to do it).

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right in 3.4 branch, but I do not see the code to close the socket in 3.4.9 as I see the following in there:
con_release: sh_log(c->hist, TCP_SEND2MAIN, "send 1, (%d)", c->refcnt); tcp_conn_release(c, (rlen < 0)?0:1); return rlen;
Looks like this is fixed in 3.13 and above. See 849bd63

My PR Is based on 3.4 like you suggested as I couldn't figure out a way to fix in 3.4.9.

The scenario is quite simple. Ensure a TLS client tries to create a TLS connection with TLS server where TLS server is listening on the port but the handshake fails (may be due to unknow CA or any other error), and that was creating a leak in 3.4.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants