Github user jpeach commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/1024#discussion_r79039044
--- Diff: iocore/net/SSLUtils.cc ---
@@ -2052,6 +2052,7 @@ SSLParseCertificateConfiguration(const
SSLConfigParams *params, SSLCertLookup *l
// Load the global ticket key for later use.
REC_ReadConfigStringAlloc(ticket_key_filename,
"proxy.config.ssl.server.ticket_key.filename");
+ ticket_block_free(global_default_keyblock);
--- End diff --
Unfortunately this is not going to work correctly. At the same time you
have frees the ticket block here, other threads are accessing it to perform SSL
handshaking, so they will read a stale pointer.
One way to fix this is to make the ticket block part of the
``SSLCertLookup`` object and then point the global pointer that that object.
That might get a little nasty since updating the certificate lookup can fail
after you assign the global pointer. You might need to find a different way.
In general, I think what needs to happen here is:
- Only update the ticket block if it has changed. Updating is each time
will reduce the possibility of SSL session resumption.
- If you update the ticket block, swap the pointers to the old and new
blocks atomically, then schedule the old block for deletion
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
