You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@trafficserver.apache.org by persiaAziz <gi...@git.apache.org> on 2016/11/17 22:00:13 UTC
[GitHub] trafficserver pull request #1226: TS-5022: Allow multiple client cert for AT...
GitHub user persiaAziz opened a pull request:
https://github.com/apache/trafficserver/pull/1226
TS-5022: Allow multiple client cert for ATS
Allow ATS to choose the certificate to use while making ssl connection with an origin server. Remap plugin can set "proxy.config.ssl.client.cert.path" and "proxy.config.ssl.client.cert.filename" which are overridable configs now. This code has been tested with [microserver](https://bitbucket.org/persiaAziz/ats_tests/src/23ed086bfb426761d8836d282cbb7c2327e5d0ab/tests/tools/microServer?at=microserver)
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/persiaAziz/trafficserver TS-5022
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/trafficserver/pull/1226.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1226
----
commit 0122ed112a4d52e9ae6cdd97b7d3e1d71efb9454
Author: Persia Aziz <pe...@yahoo-inc.com>
Date: 2016-11-14T17:51:27Z
TS-5022: Allow multiple client cert for ATS
----
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #1226: TS-5022: Allow multiple client cert for AT...
Posted by persiaAziz <gi...@git.apache.org>.
Github user persiaAziz commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/1226#discussion_r94827291
--- Diff: iocore/net/SSLNetProcessor.cc ---
@@ -67,7 +67,8 @@ SSLNetProcessor::start(int, size_t stacksize)
// Acquire a SSLConfigParams instance *after* we start SSL up.
SSLConfig::scoped_config params;
-
+ // freeing the CTX Map
+ params->freeCTXmap();
--- End diff --
You are right, I forgot to remove this call
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1226: TS-5022: Allow multiple client cert for ATS
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1226
FreeBSD build *failed*! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/1206/ for details.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1226: TS-5022: Allow multiple client cert for ATS
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1226
Linux build *successful*! See https://ci.trafficserver.apache.org/job/Github-Linux/1115/ for details.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #1226: TS-5022: Allow multiple client cert for AT...
Posted by persiaAziz <gi...@git.apache.org>.
Github user persiaAziz commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/1226#discussion_r94828732
--- Diff: proxy/http/HttpSM.cc ---
@@ -4059,6 +4061,16 @@ HttpSM::do_remap_request(bool run_inline)
pending_action = remap_action_handle;
}
+ // check if the overridden client cert filename is already attached to an existing ssl context
+ ats_scoped_str clientCert(Layout::relative_to(t_state.txn_conf->client_cert_filepath, t_state.txn_conf->client_cert_filename));
+ auto tCTX = params->getCTX(clientCert);
+
+ if (tCTX == nullptr) {
+ // make new client ctx and add it to the ctx list
+ auto tctx = ssl_NetProcessor.getNewCTX(clientCert);
+ params->InsertCTX(clientCert, tctx);
--- End diff --
Yes it would be cleaner. I reused the SSLInitClientContext logic to create the new CTX. That is why I kept it here. I will see if I can move the whole client context thing to SSLconfig
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #1226: TS-5022: Allow multiple client cert for AT...
Posted by shinrich <gi...@git.apache.org>.
Github user shinrich commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/1226#discussion_r94824666
--- Diff: iocore/net/SSLNetProcessor.cc ---
@@ -67,7 +67,8 @@ SSLNetProcessor::start(int, size_t stacksize)
// Acquire a SSLConfigParams instance *after* we start SSL up.
SSLConfig::scoped_config params;
-
+ // freeing the CTX Map
+ params->freeCTXmap();
--- End diff --
Shouldn't you just rely on the SSLConfig::cleanup to call freeCTXmap?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1226: TS-5022: Allow multiple client cert for ATS
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1226
FreeBSD build *successful*! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/1303/ for details.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1226: TS-5022: Allow multiple client cert for ATS
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1226
Linux build *successful*! See https://ci.trafficserver.apache.org/job/Github-Linux/1200/ for details.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1226: TS-5022: Allow multiple client cert for ATS
Posted by shinrich <gi...@git.apache.org>.
Github user shinrich commented on the issue:
https://github.com/apache/trafficserver/pull/1226
I chatted with Persia about cleanly dealing with the config reload case.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1226: TS-5022: Allow multiple client cert for ATS
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1226
FreeBSD build *successful*! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/1307/ for details.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1226: TS-5022: Allow multiple client cert for ATS
Posted by persiaAziz <gi...@git.apache.org>.
Github user persiaAziz commented on the issue:
https://github.com/apache/trafficserver/pull/1226
Please review @shinrich @SolidWallOfCode
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1226: TS-5022: Allow multiple client cert for ATS
Posted by persiaAziz <gi...@git.apache.org>.
Github user persiaAziz commented on the issue:
https://github.com/apache/trafficserver/pull/1226
Please review @shinrich
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1226: TS-5022: Allow multiple client cert for ATS
Posted by shinrich <gi...@git.apache.org>.
Github user shinrich commented on the issue:
https://github.com/apache/trafficserver/pull/1226
[approve ci]
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1226: TS-5022: Allow multiple client cert for ATS
Posted by persiaAziz <gi...@git.apache.org>.
Github user persiaAziz commented on the issue:
https://github.com/apache/trafficserver/pull/1226
Please review @shinrich . The map containing client ctxs have been moved to sslconfig
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1226: TS-5022: Allow multiple client cert for ATS
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1226
Linux build *successful*! See https://ci.trafficserver.apache.org/job/Github-Linux/1099/ for details.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1226: TS-5022: Allow multiple client cert for ATS
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1226
Linux build *successful*! See https://ci.trafficserver.apache.org/job/Github-Linux/1198/ for details.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #1226: TS-5022: Allow multiple client cert for AT...
Posted by shinrich <gi...@git.apache.org>.
Github user shinrich commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/1226#discussion_r94824973
--- Diff: iocore/net/SSLNetProcessor.cc ---
@@ -76,6 +77,7 @@ SSLNetProcessor::start(int, size_t stacksize)
SSLError("Can't initialize the SSL client, HTTPS in remap rules will not function");
}
+ params->InsertCTX(params->clientCertPath, client_ctx);
--- End diff --
And maybe the InsertCTX should be moved to the logic that initializes/sets up the SSLConfig?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #1226: TS-5022: Allow multiple client cert for AT...
Posted by shinrich <gi...@git.apache.org>.
Github user shinrich commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/1226#discussion_r94605355
--- Diff: iocore/net/P_SSLNetProcessor.h ---
@@ -63,6 +64,90 @@ struct SSLNetProcessor : public UnixNetProcessor {
return client_ctx;
}
+ // InsertCTX hashes on the absolute path to the client certificate file and stores in the map
+ bool
+ InsertCTX(cchar *client_cert, SSL_CTX *cctx)
+ {
+ ink_mutex_acquire(&ctxMapLock);
+ if (client_cert == nullptr) {
+ ctx_map.put(nullptr, cctx);
+ return true;
--- End diff --
Are you leaking the lock here too?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #1226: TS-5022: Allow multiple client cert for AT...
Posted by shinrich <gi...@git.apache.org>.
Github user shinrich commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/1226#discussion_r94825933
--- Diff: iocore/net/SSLNetVConnection.cc ---
@@ -980,7 +980,20 @@ SSLNetVConnection::sslStartHandShake(int event, int &err)
case SSL_EVENT_CLIENT:
if (this->ssl == nullptr) {
- this->ssl = make_ssl_connection(ssl_NetProcessor.client_ctx, this);
+ SSL_CTX *clientCTX = nullptr;
+ if (this->options.clientCertificate) {
+ const char *certfile = (const char *)this->options.clientCertificate;
+ if (certfile != nullptr) {
+ clientCTX = params->getCTX(certfile);
+ if (clientCTX != nullptr)
+ Debug("ssl", "context for %s is found at %p", this->options.clientCertificate.get(), (void *)clientCTX);
+ else
+ Debug("ssl", "failed to find context for %s", this->options.clientCertificate.get());
+ }
+ } else {
+ clientCTX = ssl_NetProcessor.client_ctx;
+ }
--- End diff --
Should we use the same logic to fetch the default client cert as well?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1226: TS-5022: Allow multiple client cert for ATS
Posted by shinrich <gi...@git.apache.org>.
Github user shinrich commented on the issue:
https://github.com/apache/trafficserver/pull/1226
Looks Good!
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #1226: TS-5022: Allow multiple client cert for AT...
Posted by shinrich <gi...@git.apache.org>.
Github user shinrich closed the pull request at:
https://github.com/apache/trafficserver/pull/1226
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1226: TS-5022: Allow multiple client cert for ATS
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1226
FreeBSD build *successful*! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/1306/ for details.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1226: TS-5022: Allow multiple client cert for ATS
Posted by shinrich <gi...@git.apache.org>.
Github user shinrich commented on the issue:
https://github.com/apache/trafficserver/pull/1226
[approve ci]
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1226: TS-5022: Allow multiple client cert for ATS
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1226
FreeBSD build *successful*! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/1222/ for details.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1226: TS-5022: Allow multiple client cert for ATS
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1226
Linux build *successful*! See https://ci.trafficserver.apache.org/job/Github-Linux/1197/ for details.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #1226: TS-5022: Allow multiple client cert for AT...
Posted by shinrich <gi...@git.apache.org>.
Github user shinrich commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/1226#discussion_r94826573
--- Diff: proxy/http/HttpSM.cc ---
@@ -4059,6 +4061,16 @@ HttpSM::do_remap_request(bool run_inline)
pending_action = remap_action_handle;
}
+ // check if the overridden client cert filename is already attached to an existing ssl context
+ ats_scoped_str clientCert(Layout::relative_to(t_state.txn_conf->client_cert_filepath, t_state.txn_conf->client_cert_filename));
+ auto tCTX = params->getCTX(clientCert);
+
+ if (tCTX == nullptr) {
+ // make new client ctx and add it to the ctx list
+ auto tctx = ssl_NetProcessor.getNewCTX(clientCert);
+ params->InsertCTX(clientCert, tctx);
--- End diff --
Maybe it would be cleaner to have a version of InsertCTX() that only took the certificate name and did the certificate creation and insert?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #1226: TS-5022: Allow multiple client cert for AT...
Posted by persiaAziz <gi...@git.apache.org>.
Github user persiaAziz commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/1226#discussion_r94827614
--- Diff: iocore/net/SSLNetProcessor.cc ---
@@ -76,6 +77,7 @@ SSLNetProcessor::start(int, size_t stacksize)
SSLError("Can't initialize the SSL client, HTTPS in remap rules will not function");
}
+ params->InsertCTX(params->clientCertPath, client_ctx);
--- End diff --
I put it here because client_ctx is being initialized here which belongs to SSLNetprocessor
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1226: TS-5022: Allow multiple client cert for ATS
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1226
Linux build *successful*! See https://ci.trafficserver.apache.org/job/Github-Linux/1201/ for details.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1226: TS-5022: Allow multiple client cert for ATS
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1226
FreeBSD build *successful*! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/1304/ for details.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1226: TS-5022: Allow multiple client cert for ATS
Posted by persiaAziz <gi...@git.apache.org>.
Github user persiaAziz commented on the issue:
https://github.com/apache/trafficserver/pull/1226
Please review @shinrich. The client context initialization and creation parts have been moved to SSLConfig
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1226: TS-5022: Allow multiple client cert for ATS
Posted by shinrich <gi...@git.apache.org>.
Github user shinrich commented on the issue:
https://github.com/apache/trafficserver/pull/1226
I think there still needs to be some locking around the CtxMap to safely reload.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #1226: TS-5022: Allow multiple client cert for AT...
Posted by shinrich <gi...@git.apache.org>.
Github user shinrich commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/1226#discussion_r94591982
--- Diff: iocore/net/P_SSLNetProcessor.h ---
@@ -63,6 +64,90 @@ struct SSLNetProcessor : public UnixNetProcessor {
return client_ctx;
}
+ // InsertCTX hashes on the absolute path to the client certificate file and stores in the map
+ bool
+ InsertCTX(cchar *client_cert, SSL_CTX *cctx)
+ {
+ ink_mutex_acquire(&ctxMapLock);
+ if (client_cert == nullptr) {
+ ctx_map.put(nullptr, cctx);
+ return true;
+ }
+ // dup is required here to avoid the nullifying of the keys stored in the map.
+ // client_cert is coming from the overridable clientcert config retrieved by the remap plugin.
+ cchar *cert = ats_strdup(client_cert);
+ // Hashmap has no delete functionality :(
+ ctx_map.put(cert, cctx);
+ ink_mutex_release(&ctxMapLock);
+ return true;
+ }
+
+ void
+ printCTXmap()
+ {
+ Vec<cchar *> keys;
+ ctx_map.get_keys(keys);
+ for (size_t i = 0; i < keys.length(); i++)
+ Debug("ssl", "Client certificates in the map %s", keys.get(i));
+ }
+ void
+ freeCTXmap()
+ {
+ ink_mutex_acquire(&ctxMapLock);
+ Vec<cchar *> keys;
+ ctx_map.get_keys(keys);
+ size_t n = keys.length();
+ Debug("ssl", "freeing CTX Map");
+ for (size_t i = 0; i < n; i++) {
+ deleteKey(keys.get(i));
+ ats_free((char *)keys.get(i));
+ }
+ ctx_map.clear();
+ ink_mutex_release(&ctxMapLock);
+ }
+
+ void
+ deleteKey(cchar *key)
+ {
+ SSL_CTX_free((SSL_CTX *)ctx_map.get(key));
+ }
+ // creates a new context attaching the provided certificate
+ SSL_CTX *
+ getNewCTX(char *client_cert)
+ {
+ SSL_CTX *client_ctx = nullptr;
+
+ SSLConfig::scoped_config params;
+
+ client_ctx = SSLInitClientContext(params);
+ if (!client_ctx) {
+ SSLError("Can't initialize the SSL client, HTTPS in remap rules will not function");
+ }
+ if (client_ctx && client_cert != nullptr) {
+ if (!SSL_CTX_use_certificate_chain_file(client_ctx, (const char *)client_cert)) {
+ SSLError("failed to load client certificate from %s", params->clientCertPath);
+ goto fail;
+ }
+ }
+ return client_ctx;
+ fail:
+ SSLReleaseContext(client_ctx);
+ ::exit(1);
+ }
+
+ // getCTX: returns the context attached to the given certificate
+ SSL_CTX *
+ getCTX(cchar *client_cert)
+ {
+ ink_mutex_acquire(&ctxMapLock);
+ if (client_cert == nullptr) {
+ return ctx_map.get(nullptr);
--- End diff --
Are we returning without dropping the mutex?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1226: TS-5022: Allow multiple client cert for ATS
Posted by persiaAziz <gi...@git.apache.org>.
Github user persiaAziz commented on the issue:
https://github.com/apache/trafficserver/pull/1226
@shinrich Lock problem has been fixed. Please review again. Checking for nullptr is unnecessary since we are allowing nullptr to be key.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---