Opened 3 years ago
Closed 23 months ago
#8247 enhancement closed fixed (fixed)
we should always set MODE_RELEASE_BUFFERS on OpenSSL contexts by default
| Reported by: | Glyph | Owned by: | hawkowl |
|---|---|---|---|
| Priority: | normal | Milestone: | |
| Component: | core | Keywords: | |
| Cc: | Branch: |
8247-tls-mode-release-buffers
branch-diff, diff-cov, branch-cov, buildbot |
|
| Author: |
Description
Ben Bangert of Mozilla discovered that TLS connections can be quite memory-intensive within Twisted. As part of his investigation into why, he discovered that the majority of the overhead can be eliminated with the SSL_MODE_RELEASE_BUFFERS flag, documented here as:
When we no longer need a read buffer or a write buffer for a given SSL, then release the memory we were using to hold it. Using this flag can save around 34k per idle SSL connection. This flag has no effect on SSL v2 connections, or on DTLS connections.
Given that Python copies every string like a zillion times anyway, and modern malloc implementations are quite fast, I doubt that freeing these buffers would be significant CPU overhead; however, it might save significant amounts of memory in concurrent servers. We should always set it.
It is exported by PyOpenSSL as OpenSSL.SSL.MODE_RELEASE_BUFFERS. I am not sure about what versions may or may not support it.
Change History (5)
comment:1 Changed 3 years ago by
comment:2 Changed 23 months ago by
| Branch: | → 8247-tls-mode-release-buffers |
|---|
comment:3 Changed 23 months ago by
| Keywords: | review added |
|---|
PR at https://github.com/twisted/twisted/pull/602
It does not do the OpenSSL checking that Python 3.5+ does, it's been long enough that all supported OSs with those versions should have patched it (and you can bring your own OpenSSL with Twisted, too).
comment:4 Changed 23 months ago by
| Keywords: | review removed |
|---|---|
| Owner: | set to hawkowl |
So, my position on those checks is complex. I think in general the checks are a good idea: older OpenSSL versions most certainly do still float around, and exposing people to CVEs is bad.
On the other hand, there's a solid school of thought that says that if you're still running a broken OpenSSL then you're in a world of pain of your own making. Given that having such a branch would be annoying to test, I think I'm ok with not providing the checks.
Please resolve the txchecker failures and merge. :)
In OpenSSL v0.13 it is not available.
It was introduced here https://github.com/pyca/pyopenssl/commit/a63714c6ef6915b662a5fa53094fd8c6e3aa9be8 ... or 0.14