trafficserver
trafficserver copied to clipboard
Fix an error on SSL config reload (plus some cleanup).
This seems to have eliminated some ERROR diags we were seeing in Yahoo Prod when doing config reloads.
The SSLSecret public functions no longer return pointers into the unorded_map of secrets, they return a copy of the secret data. This seemed thread unsafe. A periodic poll running in the background can update the secret data for an entry for a secret name in the map.
To avoid exporting pointers, I had to change the prototype of TSSslSecretGet(). Hopefully there are no existing plugins that are already using this TS API function, so breaking this rule will be moot. I added a new TS API handle, TSHeapBuf, in order to be able to cleanly return a copy of the secret data.
I'll undraft this after letting it soak in Yahoo Prod for a while.
Sorry, I accidentally made the PR branch in the Apache repo and not my fork. I'll delete the branch once this PR is closed.
I disagree this is incompatible, since, as Masakazu found, it's only incompatible with commits which have not been released yet.
This has been running in Yahoo Prod for more than 2 weeks, with no issues/problems. These changes fixed errors that were happening when doing a config reload.
"I would not call this general cleanup."
What title do you prefer then?
What title do you prefer then?
I don't know what happened exactly, so I can only say "Fix an error on SSL config reload".
As mentioned already, at a minimum this should be split into two PRs, and a discussion should be opened on the mailing list about adding the new set of APIs.
We're seeing a deadlock with our plugin that uses SSL_SECRET_HOOK, so making this a draft for now.
Brian Neradt has a fix for the deadlock problem. We're going to let it ripen internally for a bit, then I'll pull it into this PR.
[approve ci autest]
I still think we should fix the API without introducing additional data structure to avoid the unfortunate release that has an unusable API.
Even if we are going to have TSAllocatedVarLenData
, using it just for TSSslSecretGet
causes inconsistency. TSUrlStringGet
could use it too. We can continue the discussion about the data structure on the list and have it later. Let's focus on the issue on this PR.
We have this in Yahoo internal TS. If anyone else sees errors like:
[Jan 20 00:01:47.669] [ET_TASK 1] ERROR: SSL::139889882617600:error:0909006C:PEM routines:get_name:no start line:crypto/pem/pem_lib.c:745:Expecting: TRUSTED CERTIFICATE
[Jan 20 00:01:47.669] [ET_TASK 1] ERROR: SSL::139889882617600:error:0909006C:PEM routines:get_name:no start line:crypto/pem/pem_lib.c:745:Expecting: ANY PRIVATE KEY
[Jan 20 00:01:47.669] [ET_TASK 1] ERROR: failed to load server private key from /home/y/conf/trafficserver/ssl/ycpi.tls.yahooinc.com.ec.key
we should reopen this PR.
Weren't you going to update this PR with the simplest interface? I think this PR was closed because the branch was on the upstream repo.
I was planning to, but it was delayed by production issues. Leif didn't comment on why he was closing it.