Removing `newSession`/`resumeSession` #1462

indutny · 2015-04-18T08:08:35Z

SSL session ids are so outdated, let's remove them. TLS tickets is a modern age and they are very widely deployed!

I'd like to remove it to get rid of the ClientHelloParser. SSL_set_cert_cb is a new OpenSSL-1.0.2 API that we are going to use instead.

The list of the modules that will no longer work:

Please comment if you have a use case for this API, or know a module that does depend on it.

cc @iojs/collaborators @iojs/community-members @iojs/crypto

The text was updated successfully, but these errors were encountered:

silverwind · 2015-04-18T11:06:59Z

I'm using id resumption, but meant to switch over to tickets eventually.

Could you provide a bit of info on where the ticket keys are stored and if there is a user-exposed API to manage the invalidation? I think if we want to do the switch to tickets, the user should have control over this important aspect.

Also, how is the client support on tickets? Such data seems hard to find. Does IE support them yet?

indutny · 2015-04-18T11:15:39Z

@silverwind tickets are stored on client-side, the client should provide them when attempting to do a new connection. There is a validity timeout, but it is not configurable in io.js yet. Please file a bug for it ;)

The client-side session API in io.js remains the same even when using TLS tickets. You could call .getSession() and supply it as a session option to the tls.connect().

silverwind · 2015-04-18T11:17:34Z

But the server uses a key to encrypt the ticket, and I think best practice dictates this key to be rotated, no?

indutny · 2015-04-18T11:23:05Z

@silverwind that's true. We do not provide the API to change the the ticket key in runtime.

Speaking of, I just figured that sessionTimeout controls TLS ticket lifetime, but it is only an advisory. So technically the TLS tickets are valid until the key is rotated.

silverwind · 2015-04-18T11:31:51Z

On the server, I could see forcing a ticket key refresh by instantiating a new server with a provided ticketKeys (The docs to that option seem confusing, it's called keys but says it only takes a single key). I think there should probably be a better api to this.

indutny · 2015-04-18T11:47:57Z

@silverwind well, this is some OpenSSL weirdness :) It takes one buffer and splits it into 3 parts, each used for a different kind of key for TLS tickets. That's why it is called that way.

There is an API for changing it in runtime. I'd really appreciate if you'll open a ticket for it and mention me there.

silverwind · 2015-04-18T12:04:22Z

Will do.

My primary concern is we're deprecating a important performance functionality to which there is no universally supported alternative to. Could you provide some data on browser support?

indutny · 2015-04-18T12:16:08Z

I think @pquerna has some insights on this: https://journal.paul.querna.org/articles/2012/09/07/adoption-of-tls-extensions/ . Though the situation might have been changed much since 2012 ;)

I believe that chrome/firefox/opera/... support this, while old IE versions most likely do not. The command line utilities do not support TLS tickets in their majority, but!.. they do not support sessions either :)

indutny · 2015-04-18T12:16:43Z

@silverwind to conclude, I believe that in most situations if the client support the SSL session, it support TLS session tickets too.

Do not enable ClientHello parser for async SNI/OCSP. Use new OpenSSL-1.0.2's API `SSL_set_cert_cb` to pause the handshake process and load the cert/OCSP response asynchronously. Hopefuly this will make whole async SNI/OCSP process much faster and will eventually let us remove the ClientHello parser itself (which is currently used only for async session, see nodejs#1462 for the discussion of removing it). NOTE: Ported our code to `SSL_CTX_add1_chain_cert` to use `SSL_CTX_get0_chain_certs` in `CertCbDone`. Test provided for this feature. Fix: nodejs#1423

tlivings · 2015-04-18T15:07:50Z

Why remove it? It's a valid method for resume, and it having the option for tickets doesn't mean it needs to be removed.

silverwind · 2015-04-18T15:21:17Z

Looks like Safari and IE on Windows < 8 don't support tickets. I'm not sure if deprecating session ids is such a good idea, yet.

pquerna · 2015-04-18T16:32:13Z

Removing support for sessions doesn't seem quite right yet. Tickets are better for most cases, but esp in non-browser stacks (eg, API clients) TLS feature adoption has always lagged. Getting real and updated numbers would be helpful in validating this of course.
Extending the tickets API to allow multiple keys and key rotation is a good idea.

Do not enable ClientHello parser for async SNI/OCSP. Use new OpenSSL-1.0.2's API `SSL_set_cert_cb` to pause the handshake process and load the cert/OCSP response asynchronously. Hopefuly this will make whole async SNI/OCSP process much faster and will eventually let us remove the ClientHello parser itself (which is currently used only for async session, see nodejs#1462 for the discussion of removing it). NOTE: Ported our code to `SSL_CTX_add1_chain_cert` to use `SSL_CTX_get0_chain_certs` in `CertCbDone`. Test provided for this feature. Fix: nodejs#1423

bajtos · 2015-04-20T07:22:04Z

TLS session keys have one major issue: if you are running in a cluster, you have to either synchronize session keys across all cluster nodes (workers) or provide sticky HTTP sessions (sticky connections). The former ensures that all server nodes recognize all client sessions. The latter ensures that the same client connects always to the same server node, to the node which issued the session key and thus it's not necessary to synchronize the keys.

AFAIK, sticky HTTP sessions / sticky connections are not supported by Node.js native cluster yet, thus users have to synchronize the TLS sessions.

Back in 2013, I wrote a small module to synchronize TLS session keys across Node cluster workers: strong-cluster-tls-store, blog post. IIRC, the performance gains were not great, i.e. the cost of synchronizing TLS session keys is similar to the cost of re-establishing a fresh TLS session.

If you are running multiple Node.js server nodes and cluster them differently, then you need to synchronize session keys via external service (messaging queue, Redis, etc.), which makes things even more slow.

In my opinion, TLS session keys are so difficult to configure correctly, that most people are actually not using this feature, even though they may believe that they are using it.

Let's not forget that the client code must be aware of TLS sessions too. For example, if I'm reading the code correctly, the default Node.js HTTPS client does not supply session option to tls.connect() and thus it does not reuse TLS sessions at all.

It's a wild guess on my side, but if non-browser stacks (eg, API clients) are lagging in TLS feature adoption, then I doubt they will be supporting TLS sessions, considering that they require explicit effort to enable.

indutny · 2015-04-20T07:49:17Z

@bajtos I think you are missing the point. TLS session tickets doesn't need to be stored. The module that you have put here: https://github.com/strongloop/strong-cluster-tls-store/blob/master/lib/cluster-store.js , is working with the SSL sessions.

I believe that we support session option to tls.connect and AFAIK PayPal is using it for it's internal needs quite heavily.

The thing about TLS tickets is that everyone are already using them, they are enabled implicitly. Synchronization of keys between workers in cluster is happening automatically when they are forking, but there are no APIs for changing the key yet. Anyway, changing it very often is useless, key rotation might happen once a day, or a couple of hours, depending on the needs. This process is quite fast and should involve just sending a one IPC message to every worker.

I'd say that with this heartbleed thing many and many has updated their OpenSSL from 0.9.8 to something fresh. And as far as I remember 0.9.8 will reach it's EOL this year:

Back in October we announced the End Of Life of version 0.9.8. This version is currently only receiving security updates, and support will cease completely on 31st December 2015.

So eventually many will update to the next version, especially in corporate environments, where people care about the security.

Anyway, managing SSL sessions across the cluster is hard indeed as you already said, and is most likely not worth it considering the costs of storing the sessions in a shared storage. In both cases of TLS tickets and SSL sessions, the client needs to store the session id/ticket somewhere and send it on a next connection. So for the client situation won't change. But for the server with TLS tickets - there is nothing to store, the performance will bloom!

bajtos · 2015-04-20T09:12:58Z

I think you are missing the point. TLS session tickets doesn't need to be stored. The module that you have put here: https://github.com/strongloop/strong-cluster-tls-store/blob/master/lib/cluster-store.js , is working with the SSL sessions.

Let's unify the terminology first. Per this blog post from 2011, there are

session identifiers as described in RFC 5246 (I believe you call them "SSL sessions", I have incorrectly called them "session keys").
and session tickets as depicted in RFC 5077 (I believe you call them "TLS session tickets")

I believe I understand you correctly. What I was trying to say: "session identifiers" are difficult to implement right on the server, most people don't have their servers configured correctly and thus they are not using this feature, even though they think they are. Therefore it should not be a big deal to drop the support for them, at least IMO.

I think your last comment is saying the same thing, just in other words, thus we seem to be in agreement here.

Originally I wanted to point out a module that depends on this API, per you request in the issue description:

Please comment if you have a use case for this API, or know a module that does depend on it.

So I have a module that depends on the API you are going to remove. When the API is gone, I will deprecate our module and explain that people should switch to "session tickets". Since we have only like one download per day, I don't think this will be an issue.

I believe that we support session option to tls.connect and AFAIK PayPal is using it for it's internal needs quite heavily.

Yes, that option is supported, but AFAICT it is not used by default.

In other words, the following mockup code will initiate a new TLS session for each request.

get(get);

function get(cb) {
  require('https').request(
    { host: 'localhost', port: 3000, headers: { connection: 'close' } })
    function(res) {
      res.resume();
      res.on('end', function() { cb && cb(); });
    })
    .end();
}

indutny · 2015-04-20T09:23:48Z

Sorry @bajtos, looks like I have misread most of your comment :(

/me will never write a reply in a morning.

Good to see that you agree with me! :)

Regarding the session - it is not used in core, indeed. I guess we might want to explore this behaviour in HTTPS Agent. Please file a bug!

tlivings · 2015-04-20T14:31:24Z

@bajtos at PayPal, as @indutny mentioned, we use this feature and have our own method of synchronizing our sessions between clusters.

Although we intend to move to TLS tickets, the effort must be coordinated across several stacks, which has its own challenges.

Removing the features for session resume (resumeSession namely) will definitely affect us and effectively keep us off of io.js (for now).

This isn't to say that this is likely a good direction to go long term, because as you mentioned, it isn't particularly easy to distribute sessions across clusters, and doing it in any sort of centralized store isn't going to be a performance enhancement anyway.

On the other hand, I am not sure how removing it really benefits anyone either.

bajtos · 2015-04-22T09:39:40Z

Regarding the session - it is not used in core, indeed. I guess we might want to explore this behaviour in HTTPS Agent. Please file a bug!

#1499

Do not enable ClientHello parser for async SNI/OCSP. Use new OpenSSL-1.0.2's API `SSL_set_cert_cb` to pause the handshake process and load the cert/OCSP response asynchronously. Hopefuly this will make whole async SNI/OCSP process much faster and will eventually let us remove the ClientHello parser itself (which is currently used only for async session, see nodejs#1462 for the discussion of removing it). NOTE: Ported our code to `SSL_CTX_add1_chain_cert` to use `SSL_CTX_get0_chain_certs` in `CertCbDone`. Test provided for this feature. Fix: nodejs#1423

Do not enable ClientHello parser for async SNI/OCSP. Use new OpenSSL-1.0.2's API `SSL_set_cert_cb` to pause the handshake process and load the cert/OCSP response asynchronously. Hopefuly this will make whole async SNI/OCSP process much faster and will eventually let us remove the ClientHello parser itself (which is currently used only for async session, see #1462 for the discussion of removing it). NOTE: Ported our code to `SSL_CTX_add1_chain_cert` to use `SSL_CTX_get0_chain_certs` in `CertCbDone`. Test provided for this feature. Fix: #1423 PR-URL: #1464 Reviewed-By: Shigeki Ohtsu <ohtsu@iij.ad.jp>

Do not enable ClientHello parser for async SNI/OCSP. Use new OpenSSL-1.0.2's API `SSL_set_cert_cb` to pause the handshake process and load the cert/OCSP response asynchronously. Hopefuly this will make whole async SNI/OCSP process much faster and will eventually let us remove the ClientHello parser itself (which is currently used only for async session, see nodejs#1462 for the discussion of removing it). NOTE: Ported our code to `SSL_CTX_add1_chain_cert` to use `SSL_CTX_get0_chain_certs` in `CertCbDone`. Test provided for this feature. Fix: nodejs#1423 PR-URL: nodejs#1464 Reviewed-By: Shigeki Ohtsu <ohtsu@iij.ad.jp>

Trott · 2016-02-25T05:30:06Z

Hello, friends! Can someone tell me if this is still a thing?

(Doing some poking around on issues that haven't been touched in a long time and trying to figure out what, if anything, to do with them. This one hasn't seen any comments in over 10 months.)

indutny · 2016-02-25T06:15:20Z

This is still a thing, and I firmly believe that we should eventually do this. There are lots of code that supports it, and it doesn't provide that much use in the presence of TLS session tickets (which are very common nowadays).

To support @tlivings case, I think it should be enough to leave a synchronous version of newSession/resumeSession as a compatibility shim.

indutny · 2016-03-15T20:40:23Z

Alright, I just spoke to @tlivings, and it looks like we will be able to figure it out with a synchronous newSession/resumeSession. Going to file a PR with proposed changes soon.

Deprecate asynchronous `newSession`/`resumeSession` events, introduce a synchronous APIs via `newSession`/`resumeSession` option-callback for `tls.createServer`. The reason for this transition is rather simple. There is a quite big amount of code that was added to support this construction, and not that much users of it. Additionally, that code chunk is running in front of OpenSSL, so it makes the process of asynchronous session resumption twice as ineffective. See: nodejs#1462

justinegreene · 2016-03-30T03:18:43Z

I am a little late to this thread, but unless I am missing something, I am pretty sure that Safari on iOS and OSX lacks Session Ticket Support and only supports Session IDs, so removing new Session and Resume Session functionality would cripple SSL/TLS for iPhones and iPads which account for 40%-60% of the mobile web traffic in the US (and Safari on OS X which is about 3%).

indutny · 2016-03-30T03:42:14Z

@justinegreene I'm not suggesting removal of support of session ids... just asynchronous APIs for loading/storing them.

justinegreene · 2016-03-30T04:11:32Z

Thanks for the clarification. The thread started with "SSL session ids are so outdated, let's remove them" and much of the discussion was about using Session Tickets instead of Session IDs, but this isn't an option with Safari.

bnoordhuis · 2017-02-07T18:05:25Z

@indutny What's the status of this? If it's inactive, can you close it out?

silverwind · 2017-02-07T18:17:03Z

BTW, Safari 10 still does not support session tickets according to https://www.ssllabs.com/ssltest/clients.html.

jasnell · 2017-05-30T04:08:39Z

Closing due to lack of further activity. @indutny feel free to reopen if you get back to this.

indutny added the semver-major PRs that contain breaking changes and should be released in the next major version. label Apr 18, 2015

silverwind added the tls Issues and PRs related to the tls subsystem. label Apr 18, 2015

indutny mentioned this issue Apr 18, 2015

tls: use SSL_set_cert_cb for async SNI/OCSP #1464

Closed

silverwind mentioned this issue Apr 18, 2015

tls: api to change tls ticket keys #1465

Closed

yosuke-furukawa mentioned this issue Apr 20, 2015

Evangelism: Weekly Update April 24th nodejs/evangelism#64

Closed

Fishrock123 added the discuss Issues opened for discussions and feedbacks. label Apr 28, 2015

indutny mentioned this issue Mar 18, 2016

tls: deprecate newSession/resumeSession events #5774

Closed

4 tasks

jasnell added the stalled Issues and PRs that are stalled. label Mar 24, 2017

jasnell closed this as completed May 30, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Removing `newSession`/`resumeSession` #1462

Removing `newSession`/`resumeSession` #1462

indutny commented Apr 18, 2015

silverwind commented Apr 18, 2015

indutny commented Apr 18, 2015

silverwind commented Apr 18, 2015

indutny commented Apr 18, 2015

silverwind commented Apr 18, 2015

indutny commented Apr 18, 2015

silverwind commented Apr 18, 2015

indutny commented Apr 18, 2015

indutny commented Apr 18, 2015

tlivings commented Apr 18, 2015

silverwind commented Apr 18, 2015

pquerna commented Apr 18, 2015

bajtos commented Apr 20, 2015

indutny commented Apr 20, 2015

bajtos commented Apr 20, 2015

indutny commented Apr 20, 2015

tlivings commented Apr 20, 2015

bajtos commented Apr 22, 2015

Trott commented Feb 25, 2016

indutny commented Feb 25, 2016

indutny commented Mar 15, 2016

justinegreene commented Mar 30, 2016

indutny commented Mar 30, 2016

justinegreene commented Mar 30, 2016

bnoordhuis commented Feb 7, 2017

silverwind commented Feb 7, 2017

jasnell commented May 30, 2017

Removing newSession/resumeSession #1462

Removing newSession/resumeSession #1462

Comments

indutny commented Apr 18, 2015

silverwind commented Apr 18, 2015

indutny commented Apr 18, 2015

silverwind commented Apr 18, 2015

indutny commented Apr 18, 2015

silverwind commented Apr 18, 2015

indutny commented Apr 18, 2015

silverwind commented Apr 18, 2015

indutny commented Apr 18, 2015

indutny commented Apr 18, 2015

tlivings commented Apr 18, 2015

silverwind commented Apr 18, 2015

pquerna commented Apr 18, 2015

bajtos commented Apr 20, 2015

indutny commented Apr 20, 2015

bajtos commented Apr 20, 2015

indutny commented Apr 20, 2015

tlivings commented Apr 20, 2015

bajtos commented Apr 22, 2015

Trott commented Feb 25, 2016

indutny commented Feb 25, 2016

indutny commented Mar 15, 2016

justinegreene commented Mar 30, 2016

indutny commented Mar 30, 2016

justinegreene commented Mar 30, 2016

bnoordhuis commented Feb 7, 2017

silverwind commented Feb 7, 2017

jasnell commented May 30, 2017

Removing `newSession`/`resumeSession` #1462

Removing `newSession`/`resumeSession` #1462