Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dnsdist: add sessionTimeout setting for TLS session lifetime #8882

Merged
merged 2 commits into from Mar 3, 2020

Conversation

qvr
Copy link
Contributor

@qvr qvr commented Mar 2, 2020

Short description

Adds a new sessionTimeout setting for TLS session lifetime. It is used both for sessions stored server-side and also to indicate to clients for how long their given TLS ticket is valid for. If unset or set to 0, will use OpenSSL's default (which for TLS protocols is 2 hours).

It makes sense to not tell the client to throw the ticket away if the ticket key hasn't yet been rotated.

Checklist

I have:

  • read the CONTRIBUTING.md document
  • compiled this code
  • tested this code
  • included documentation (including possible behaviour changes)
  • documented the code
  • added or modified regression test(s)
  • added or modified unit test(s)
  • checked that this code was merged to master

Copy link
Member

@rgacogne rgacogne left a comment

Thanks for this PR! The code looks good and I agree it makes sense to be able to configure the lifetime of tickets. I'm not sure we should change the default value, though, because there are some concerns that tickets can be used to track users. So I'd prefer we keep the default value set by the underlying OpenSSL library, and let the administrator change it if they so desire.

@qvr qvr force-pushed the ssl_ctx_set_timeout branch from 327953b to 2542667 Compare Mar 2, 2020
@qvr
Copy link
Contributor Author

@qvr qvr commented Mar 2, 2020

Ok, the default is now left up to OpenSSL, and I changed the setting name as well because the timeout is also used for sessions stored server-side (and that's a good reason to decouple it from ticket key rotation too).

And the default value for all TLS protocols is 2 hours:
https://github.com/openssl/openssl/blob/master/ssl/t1_lib.c#L106

@qvr qvr changed the title dnsdist: add ticketLifetime setting for TLS ticket lifetime dnsdist: add sessionTimeout setting for TLS session lifetime Mar 2, 2020
Copy link
Member

@rgacogne rgacogne left a comment

Looks great! Documentation would be nice :-)

@qvr qvr force-pushed the ssl_ctx_set_timeout branch from c4b2174 to 557855f Compare Mar 3, 2020
Copy link
Member

@rgacogne rgacogne left a comment

Thanks!

@rgacogne rgacogne merged commit b736b2e into PowerDNS:master Mar 3, 2020
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants