OpenSSL - allow session reuse via a 'reuse' ssl_bind method or bind string query parameter
Description
This PR allows enabling OpenSSL TLSv1.2 session reuse.
This PR only affects code related to https connections and does not change current behavior.
OpenSSL and TLSv1.2 support session reuse via what is referred to as session id. Opening a tls connection with 'reuse' uses less network resources but requires additional memory on the server. It may also take cpu time when the session cache is flushed, which happens every 255 connections.
Note that session reuse on TLSv1.3 uses what is referred to as a session ticket, rather than a session id.
To switch session reuse on, pass an appropriate string to reuse if using ssl_bind in a config file, or do the same using a 'reuse' query parameter. See the comments to the Puma::MiniSSL#reuse= method for details. A string of 'dflt' turns it on with OpenSSL's default values for the cache.
Closes #2845.
Your checklist for this pull request
- [x] My pull request is 100 lines added/removed or less so that it can be easily reviewed.
- [ ] If this PR doesn't need tests (docs change), I added
[ci skip]to the title of the PR. - [x] If this closes any issues, I have added "Closes
#issue" to the PR description or my commit messages. - [x] I have updated the documentation accordingly.
- [x] All new and existing tests passed, including Rubocop.
PUMA_REUSE_ID — can we make it less ambiguous by including TLS or SSL somewhere in that string?
PUMA_REUSE_ID— can we make it less ambiguous by including TLS or SSL somewhere in that string?
Yeah, thought about that, figured I'd change before (and if) this becomes a real PR. Maybe start all ssl related compile flags with PUMA_SSL, so this would be PUMA_SSL_REUSE_ID?
A more general discussion, but the question of what should be 'compile time' options vs runtime options? This could be runtime, so Puma could have an ssl listener using it, and another not. But, that keeps adding to the bind parameters, and makes things rather messy...
Which may lead to the idea of whether Puma should load Ruby's OpenSSL, which would allow finer grained configuration, and probably allow us to remove minissl. Thought about it, not sure. If most Puma app's are already loading it, we're not really gaining that much...
Hi!
I tried adding
ENV['PUMA_SSL_REUSE_ID'] = 'YES'
gem "puma", "~> 5.0", git: 'https://github.com/MSP-Greg/puma.git'
to my Gemfile, but it seems that does not compile the C extension. Is that right?
Do I need to pack a gemfile, or is there a way to use the granch directly from Git?
Update
Seems like the following works:
ENV['PUMA_SSL_REUSE_ID'] = 'YES'
gem "puma", "~> 5.0", git: 'https://github.com/MSP-Greg/puma.git', branch: '00_issue-2845'
SUCCESS!
I followed the excellent contribution guide and compiled and used the 00_issue-2845 branch, and our tests passed!
It would be awesome if this could be merged to master.
I must ask why this option should not always be enabled? Why the need for an environment variable?
@donv
Thank you for testing the PR/patch.
I must ask why this option should not always be enabled?
Good question. It's a breaking change, but some quick testing showed that some large sites (google, etc) support it.
Caching has a few defaults, some of which could be exposed. Two in particular are TTL for each session, and the max cache size. I believe defaults are 300 secs for TTL, and size is 20k. Some may find those default limits are higher than needed.
Depending on the clients, caching may result in fewer network 'trips', and may reduce CPU time. But, it also takes memory, and when OpenSSL flushes the cache, other operations may be impacted.
Note that the above does not apply to TLSv1.3 reuse, which needs to be enabled (which this PR does) and defaults to 'stateless' session reuse. Also, if most connections are using keep-alive, session reuse may not really be beneficial.
Lastly, I'm thinking about using a string like '120,8192' or 't120s8192` to be used for configuring caching, which would set the TTL and max size. Using zero for the value would use the default. Not sure whether it should be compile time or runtime.
If anyone has configured other servers for this (nginx, etc) and have any thoughts, looking for input...
I think it should be runtime, if possible, sounds more useful.
Re: nginx, ssl_session_cache shared:SSL:10m; ("about 40000 sessions") seems to be a common configuration: https://ssl-config.mozilla.org/#server=nginx&version=1.17.7&config=intermediate&openssl=1.1.1k&guideline=5.6
Some more links
- the nginx docs https://nginx.org/en/docs/http/ngx_http_ssl_module.html#ssl_session_cache
- a guide https://haydenjames.io/nginx-tuning-tips-tls-ssl-https-ttfb-latency/
@dentarg
Thanks for looking into it, links are helpful.
I think it should be runtime, if possible, sounds more useful.
Ok. I'll look into adding to the bind parameters, similar to what I mentioned above. Probably this weekend. I may see if ALPN can be added also, tired of seeing the messages every time I run curl...
This sounds really great, we need this as well; any chance that this can be merged in sometime soon? :)
@seriousCookies
Sorry for the delay; a few things came up that I needed to work on. I've started work on changing it to a runtime option. So, should be a few days...
@donv @seriousCookies
If you have time to check this, the update requires one to add the following line to the ssl_bind hash in one's config file:
reuse: 'dflt'
or add &reuse=dflt to the bind string.
Doing so should turn on session caching/reuse.
thanks for that, it works! :)