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

Letting OpenSSL handle automatic ECDH negotiation keys gives us a memory leak #134

Open
TechnikEmpire opened this issue Oct 15, 2017 · 3 comments
Assignees
Labels

Comments

@TechnikEmpire
Copy link
Owner

#131 may not be accurate in describing what the problem is. I believe this is actually the problem, but a solution as described in 131 might allow us to keep both systems and fix this issue.

It appears that when auto generating keys, one copy doesn't get freed. There's some arguing about this being proper behavior in some openSSL dev threads. We used to create a single EC key for client and server contexts. We could revert back to this, but I'm concerned about security doing that.

If we implement the fix in 131 then we can potentially leave auto tmp key negotiation on per-context. However, that's only IF one of the openSSL dev's is accurate about there in fact being no true "leak" and that the full memory is deallocated in SSL_free. Assuming that SSL_free ends up dereferencing the per-host contexts to the point of triggering SSL_ctx_free, and assuming that SSL_ctx_free will trigger full key de-allocation, then this should be OK.

If the dev is wrong, then we may be forced to go straight back to our single, one time tmp key per global context system.

@TechnikEmpire TechnikEmpire self-assigned this Oct 15, 2017
@TechnikEmpire
Copy link
Owner Author

In the thread arguing about this, there's a suggestion that setting SSL_SESS_CACHE_OFF will prevent this leak.

@TechnikEmpire
Copy link
Owner Author

Well, I implemented the above solution and this didn't fix the memory leak. So, only possibility is that we remove SSL_CTX_set_ecdh_auto completely and see if this takes away our memory leak. It would help if we had some clue as to when this started happening, le-sigh.

@TechnikEmpire
Copy link
Owner Author

The assumption here is almost certainly wrong. I was thrown off by a very recent openSSL ticket discussion specifically related to this system I put in place JUST before realizing we had a leak.

The leak appears to indeed be coming from either the asio reactor hanging on to wrapped async callbacks indefinitely, or some madness self referencing via shared_from_this, although I removed all lambda captures and still encountered this problem with no other obvious cause showing and the problem persisted. That is to say, I'm leaning more toward asio holding the callbacks, but I also doubt this theory to a degree because asio is battle tested. Either way, using the std::future system given first-class support in later asio version should alleviate this problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant