search

TechnikEmpire / HttpFilteringEngine

Transparent filtering TLS proxy.
Mozilla Public License 2.0
60 stars 33 forks source link

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

Open TechnikEmpire opened 7 years ago

TechnikEmpire commented 7 years ago

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 commented 7 years ago

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

TechnikEmpire commented 7 years ago

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 commented 7 years ago

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.