Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix tls_connection shutdown cleanup #163

Closed
wants to merge 1 commit into from

Conversation

proxyles
Copy link
Contributor

The SSL library maintains an internal table of CA certificates
(ssl_otp_cacertificate_db). This is supposed to be cleaned up when the
last connection using a certificate closes, however there's two problems
in R16B02 (and in the current master branch on github):

  • When CA certificates are provided as binary blobs, rather than by
    filename (ie, #ssl_options.cacerts is set, but #ssl_options.cacertfile
    is undefined) the cleanup never occurs due to an incorrect pattern
    match in tls_connection:handle_trusted_certs_db/1. This causes the
    table to grow unchecked because each connection adds a new entry.
  • When the process exits abnormally, tls_connection:terminate/1 is never
    called because the trap_exit process flag is not set and so similarly
    the table (and everything else cleaned in terminate/1, for that
    matter) is not cleaned up. This doesn't affect "normal" termination
    caused by the connection closing because terminate/1 is called
    explicitly from handle_sync_event/4, rather that relying on gen_fsm's
    automatic calling.

The SSL library maintains an internal table of CA certificates
(ssl_otp_cacertificate_db). This is supposed to be cleaned up when the
last connection using a certificate closes, however there's two problems
in R16B02 (and in the current master branch on github):

* When CA certificates are provided as binary blobs, rather than by
filename (ie, #ssl_options.cacerts is set, but #ssl_options.cacertfile
is undefined) the cleanup never occurs due to an incorrect pattern
match in tls_connection:handle_trusted_certs_db/1. This causes the
table to grow unchecked because each connection adds a new entry.

* When the process exits abnormally, tls_connection:terminate/1 is never
called because the trap_exit process flag is not set and so similarly
the table (and everything else cleaned in terminate/1, for that
matter) is not cleaned up. This doesn't affect "normal" termination
caused by the connection closing because terminate/1 is called
explicitly from handle_sync_event/4, rather that relying on gen_fsm's
automatic calling.
@OTP-Maintainer
Copy link

Patch has passed first testings and has been assigned to be reviewed

@IngelaAndin
Copy link
Contributor

The trap_exit problem is already solved, the other part of the patch we want. Can you please rebase that part onto master? Also if you could provide a test case that would be great. I think this should have been communicated already, I do not now how come it is not in the pull request log. We will have to look into that.

@proxyles
Copy link
Contributor Author

Merged

@proxyles proxyles closed this Feb 24, 2014
@bernardd bernardd deleted the ssl_cert_cache_fix branch March 31, 2014 09:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants