Skip to content

Tls tickets#452

Closed
cconroy wants to merge 2 commits intoopenssl:masterfrom
cconroy:tls-tickets
Closed

Tls tickets#452
cconroy wants to merge 2 commits intoopenssl:masterfrom
cconroy:tls-tickets

Conversation

@cconroy
Copy link
Copy Markdown

@cconroy cconroy commented Oct 27, 2015

The existing API for managing RFC 5077 TLS ticket keys is cumbersome: callers must either specify a key once at startup or they must implement a complicated callback API.

This new API allows a caller to set a list of TLS ticket keys. The first key in the list is preferred, and any other keys in the list will be accepted with an upgrade to a ticket with the preferred key. This scheme allows groups of servers to implement seamless key rotation strategies.

The original patch comes from Twitter's https://github.com/twitter/sslconfig

RT: [openssl.org #4108]

@cconroy
Copy link
Copy Markdown
Author

cconroy commented Oct 27, 2015

cc: @tkordas

@cconroy
Copy link
Copy Markdown
Author

cconroy commented Oct 27, 2015

hmm. seems tests are only failing under --debug..i'll have to dig on this.

@mattcaswell
Copy link
Copy Markdown
Member

Ping @cconroy...what is the status of this PR? The last message indicated you were working on failing tests. Is this PR still relevant or can it be closed?
Also, I note that sslconfig that you link to is licensed under Apache 2 - not the OpenSSL Licence. So there could be a licence issue with accepting this patch.

@mattcaswell mattcaswell added the issue: feature request The issue was opened to request a feature label May 6, 2016
@mattcaswell mattcaswell added this to the Post 1.1.0 milestone May 6, 2016
@cconroy
Copy link
Copy Markdown
Author

cconroy commented May 6, 2016

@mattcaswell I am definitely still interested in upstreaming this PR. However, I had lost hope that I would have a chance of doing so since I got only 1 message with initial feedback. If the general approach is acceptable to the maintainers, then I will definitely pick this back up!

I will reach out to the folks that maintain Twitter's sslconfig so that we can sort out the License issue.

@mattcaswell
Copy link
Copy Markdown
Member

@cconroy, well I haven't looked at your patch in detail so I can't guarantee that it will eventually be accepted, but if it is rebased and the licence issue resolved then I would be happy to look at it. Note though that as a new feature it is too late for inclusion in the 1.1.0 release.

@cconroy
Copy link
Copy Markdown
Author

cconroy commented May 11, 2016

@mattcaswell Twitter just updated the license for the original patch to MIT which should work for our purposes here.

Updated license here: https://github.com/twitter/sslconfig/blob/master/LICENSE
In this commit: twitter/sslconfig@bc55f39
Reference text: https://spdx.org/licenses/MIT

@richsalz
Copy link
Copy Markdown
Contributor

We will need a CLA from Twitter so that we can use this code. But there's no rush, it's too late for 1.1 sorry.

@cconroy
Copy link
Copy Markdown
Author

cconroy commented May 11, 2016

@richsalz really? The MIT license allows unrestricted copying and modification. So, I would think a CLA from myself should suffice? I can definitely ask them for a CLA if it's truly required, but that would be surprising to me..

@richsalz
Copy link
Copy Markdown
Contributor

Since there's no rush, we can wait a bit until we get it sorted out.

@levitte
Copy link
Copy Markdown
Member

levitte commented Jan 12, 2017

This could use a rebase and a new review

@cconroy
Copy link
Copy Markdown
Author

cconroy commented Jan 12, 2017

I'm going to go ahead and decline this PR as I was not able to get a CLA from Twitter, and we've just gone ahead and integrated with the existing API in the meantime. However, should someone want to revive this in the future, the patch is MIT licensed so feel free to pick it back up.

@cconroy cconroy closed this Jan 12, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

issue: feature request The issue was opened to request a feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants