Plugin Certifier: Load, manage and/or dynamically generate certificates #3832
Conversation
| Certifier | ||
| This plugin performs two basic tasks - | ||
|
|
||
| Load SSL certificates from file storage on demand. The total number of loaded certificates can be configured. |
There was a problem hiding this comment.
"loaded certificates kept in memory"...
| @@ -0,0 +1,709 @@ | |||
| /** @file | |||
| A brief file description | |||
| /// Add a new ssl_data to dict if not exist | ||
| scoped_SslData scoped_ssl_data(new SslData); | ||
| ssl_data = scoped_ssl_data.get(); | ||
| ssl_data->commonName = commonName; |
There was a problem hiding this comment.
Might be good to std::move(commonName) and then use ssl_data->commonName later instead, avoiding a string duplication.
| size -= 1; | ||
| auto iter = cnDataMap.find(tail->commonName); | ||
| if (iter != cnDataMap.end()) { | ||
| local = std::move(iter->second); // copy ownership |
There was a problem hiding this comment.
What is the point of this? Isn't the SslData in local lost anyway when this goes out of scope?
| return req; | ||
| } | ||
|
|
||
| // /// Local helper function that signs a X509 certificate with (CA) private key |
| } | ||
| } | ||
|
|
||
| // Registre plugin and create callback |
| SSL_set_SSL_CTX(ssl, ref_ctx); | ||
| TSVConnReenable(ssl_vc); | ||
| } | ||
| /// For queued up connections, the schduled continuation will handle the reenabling |
|
This has a couple of clang-analyzer errors, see e.g. https://ci.trafficserver.apache.org/clang-analyzer/github/3832/2018-06-15-211301-11710-1/ |
|
Yes, @dyrock is working on them. I'm waiting for those fixes before I do another pass. |
|
At some point we should remove the |
| TSCont cb_shadow = nullptr; | ||
| info.plugin_name = "certifier"; | ||
| info.vendor_name = "Oath"; | ||
| info.support_email = "zeyuany@oath.com"; |
There was a problem hiding this comment.
This should be dev@trafficserver.apache.org.
| TSPluginRegistrationInfo info; | ||
| TSCont cb_shadow = nullptr; | ||
| info.plugin_name = "certifier"; | ||
| info.vendor_name = "Oath"; |
There was a problem hiding this comment.
This should be "Apache Software Foundation"
| ssl_data->commonName = std::move(commonName); | ||
| ssl_data->vconnQ.push(edata); | ||
|
|
||
| auto &target_data = cnDataMap[ssl_data->commonName]; |
There was a problem hiding this comment.
This is a bit odd - why not just cnDatamap[ssl_data->commonName] = std::move(scoped_ssl_data);?
There was a problem hiding this comment.
since we want to first check if it exists at all? In case we lose the old pointer by replacing it.
There was a problem hiding this comment.
In order to generate the debug message? In that case you'd do better to use find, it's cleaner.
| { | ||
| TSMutexLock(list_mutex); | ||
| std::unique_ptr<SslData> local = nullptr; | ||
| if (data != nullptr) { |
There was a problem hiding this comment.
Why not call remove to remove the item, then always insert at the front?
There was a problem hiding this comment.
the remove method does removal of both list node and dict node. Here in prepend the first part is only removing from list.
There was a problem hiding this comment.
It might be possible to write a method/function that just did the part of remove that's common to both.
|
Plus, please squash these commits. |
5 participants
This plugin performs two basic tasks -