You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by sh...@apache.org on 2015/02/16 18:53:04 UTC
[1/2] trafficserver git commit: TS-3375 - Further refining error
actions on bad SSL configuration.
Repository: trafficserver
Updated Branches:
refs/heads/master 53d5c6dfb -> a8e0c5e17
TS-3375 - Further refining error actions on bad SSL configuration.
Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/8152dbfc
Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/8152dbfc
Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/8152dbfc
Branch: refs/heads/master
Commit: 8152dbfc2af3c85b545d7c2b34eb6c05cc189fe8
Parents: 53d5c6d
Author: shinrich <sh...@yahoo-inc.com>
Authored: Mon Feb 16 07:39:39 2015 -0600
Committer: shinrich <sh...@yahoo-inc.com>
Committed: Mon Feb 16 10:08:51 2015 -0600
----------------------------------------------------------------------
iocore/net/P_SSLCertLookup.h | 1 +
iocore/net/SSLCertLookup.cc | 2 +-
iocore/net/SSLConfig.cc | 8 ++++++--
iocore/net/SSLUtils.cc | 24 ++++++++++++++----------
4 files changed, 22 insertions(+), 13 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/8152dbfc/iocore/net/P_SSLCertLookup.h
----------------------------------------------------------------------
diff --git a/iocore/net/P_SSLCertLookup.h b/iocore/net/P_SSLCertLookup.h
index 5c176c0..23fad46 100644
--- a/iocore/net/P_SSLCertLookup.h
+++ b/iocore/net/P_SSLCertLookup.h
@@ -80,6 +80,7 @@ struct SSLCertLookup : public ConfigInfo
{
SSLContextStorage * ssl_storage;
SSL_CTX * ssl_default;
+ bool is_valid;
int insert(const char *name, SSLCertContext const &cc);
int insert(const IpEndpoint& address, SSLCertContext const &cc);
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/8152dbfc/iocore/net/SSLCertLookup.cc
----------------------------------------------------------------------
diff --git a/iocore/net/SSLCertLookup.cc b/iocore/net/SSLCertLookup.cc
index 18d4873..84caa6e 100644
--- a/iocore/net/SSLCertLookup.cc
+++ b/iocore/net/SSLCertLookup.cc
@@ -117,7 +117,7 @@ private:
};
SSLCertLookup::SSLCertLookup()
- : ssl_storage(new SSLContextStorage()), ssl_default(NULL)
+ : ssl_storage(new SSLContextStorage()), ssl_default(NULL), is_valid(true)
{
}
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/8152dbfc/iocore/net/SSLConfig.cc
----------------------------------------------------------------------
diff --git a/iocore/net/SSLConfig.cc b/iocore/net/SSLConfig.cc
index 98214d2..627ccd2 100644
--- a/iocore/net/SSLConfig.cc
+++ b/iocore/net/SSLConfig.cc
@@ -342,7 +342,10 @@ SSLCertificateConfig::startup()
sslCertUpdate->attach("proxy.config.ssl.server.private_key.path");
sslCertUpdate->attach("proxy.config.ssl.server.cert_chain.filename");
- return reconfigure();
+ if (!reconfigure()) {
+ _exit(1);
+ }
+ return true;
}
bool
@@ -360,7 +363,8 @@ SSLCertificateConfig::reconfigure()
ink_hrtime_sleep(HRTIME_SECONDS(secs));
}
- if (SSLParseCertificateConfiguration(params, lookup)) {
+ SSLParseCertificateConfiguration(params, lookup);
+ if (lookup->is_valid) {
configid = configProcessor.set(configid, lookup);
} else {
retStatus = false;
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/8152dbfc/iocore/net/SSLUtils.cc
----------------------------------------------------------------------
diff --git a/iocore/net/SSLUtils.cc b/iocore/net/SSLUtils.cc
index 4bf88fd..bcde35b 100644
--- a/iocore/net/SSLUtils.cc
+++ b/iocore/net/SSLUtils.cc
@@ -1224,22 +1224,22 @@ SSLPrivateKeyHandler(
// assume private key is contained in cert obtained from multicert file.
if (!SSL_CTX_use_PrivateKey_file(ctx, completeServerCertPath, SSL_FILETYPE_PEM)) {
SSLError("failed to load server private key from %s", (const char *) completeServerCertPath);
- _exit(1);
+ return false;
}
} else if (params->serverKeyPathOnly != NULL) {
ats_scoped_str completeServerKeyPath(Layout::get()->relative_to(params->serverKeyPathOnly, keyPath));
if (!SSL_CTX_use_PrivateKey_file(ctx, completeServerKeyPath, SSL_FILETYPE_PEM)) {
SSLError("failed to load server private key from %s", (const char *) completeServerKeyPath);
- _exit(1);
+ return false;
}
} else {
SSLError("empty SSL private key path in records.config");
- _exit(1);
+ return false;
}
if (!SSL_CTX_check_private_key(ctx)) {
SSLError("server private key does not match the certificate public key");
- _exit(1);
+ return false;
}
return true;
@@ -1345,7 +1345,7 @@ SSLInitServerContext(const SSLConfigParams * params, const ssl_user_config & ssl
completeServerCertPath = Layout::relative_to(params->serverCertPathOnly, certname);
if (SSL_CTX_use_certificate_chain_file(ctx, completeServerCertPath) <= 0) {
SSLError("failed to load certificate chain from %s", (const char *)completeServerCertPath);
- _exit(1);
+ goto fail;
}
const char * keyPath = key_tok.getNext();
@@ -1359,7 +1359,7 @@ SSLInitServerContext(const SSLConfigParams * params, const ssl_user_config & ssl
ats_scoped_str completeServerCertChainPath(Layout::relative_to(params->serverCertPathOnly, params->serverCertChainFilename));
if (!SSL_CTX_add_extra_chain_cert_file(ctx, completeServerCertChainPath)) {
SSLError("failed to load global certificate chain from %s", (const char *) completeServerCertChainPath);
- _exit(1);
+ goto fail;
}
}
@@ -1368,7 +1368,7 @@ SSLInitServerContext(const SSLConfigParams * params, const ssl_user_config & ssl
ats_scoped_str completeServerCertChainPath(Layout::relative_to(params->serverCertPathOnly, sslMultCertSettings.ca));
if (!SSL_CTX_add_extra_chain_cert_file(ctx, completeServerCertChainPath)) {
SSLError("failed to load certificate chain from %s", (const char *) completeServerCertChainPath);
- _exit(1);
+ goto fail;
}
}
}
@@ -1596,7 +1596,8 @@ ssl_index_certificate(SSLCertLookup * lookup, SSLCertContext const& cc, const ch
cert = PEM_read_bio_X509_AUX(bio.get(), NULL, NULL, NULL);
if (NULL == cert) {
Error("Failed to load certificate from file %s", certfile);
- _exit(1);
+ lookup->is_valid = false;
+ return false;
}
// Insert a key for the subject CN.
@@ -1701,7 +1702,10 @@ ssl_store_ssl_context(
ssl_ticket_key_block *keyblock = NULL;
bool inserted = false;
- if (!ctx) return ctx;
+ if (!ctx) {
+ lookup->is_valid = false;
+ return ctx;
+ }
// The certificate callbacks are set by the caller only
// for the default certificate
@@ -1746,7 +1750,7 @@ ssl_store_ssl_context(
}
} else {
Error("'%s' is not a valid IPv4 or IPv6 address", (const char *)sslMultCertSettings.addr);
- _exit(1);
+ lookup->is_valid = false;
}
}
}
[2/2] trafficserver git commit: TS-3389 - Fix configure error
handling in SSL configuration.
Posted by sh...@apache.org.
TS-3389 - Fix configure error handling in SSL configuration.
Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/a8e0c5e1
Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/a8e0c5e1
Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/a8e0c5e1
Branch: refs/heads/master
Commit: a8e0c5e173f32855685cede5a112d6041d1849b8
Parents: 8152dbf
Author: shinrich <sh...@yahoo-inc.com>
Authored: Mon Feb 16 11:52:09 2015 -0600
Committer: shinrich <sh...@yahoo-inc.com>
Committed: Mon Feb 16 11:52:09 2015 -0600
----------------------------------------------------------------------
iocore/net/SSLUtils.cc | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/a8e0c5e1/iocore/net/SSLUtils.cc
----------------------------------------------------------------------
diff --git a/iocore/net/SSLUtils.cc b/iocore/net/SSLUtils.cc
index bcde35b..038e200 100644
--- a/iocore/net/SSLUtils.cc
+++ b/iocore/net/SSLUtils.cc
@@ -1719,8 +1719,11 @@ ssl_store_ssl_context(
#if TS_USE_TLS_ALPN
SSL_CTX_set_alpn_select_cb(ctx, SSLNetVConnection::select_next_protocol, NULL);
#endif /* TS_USE_TLS_ALPN */
-
- certpath = Layout::relative_to(params->serverCertPathOnly, sslMultCertSettings.first_cert);
+ if (sslMultCertSettings.first_cert) {
+ certpath = Layout::relative_to(params->serverCertPathOnly, sslMultCertSettings.first_cert);
+ } else {
+ certpath = NULL;
+ }
// Load the session ticket key if session tickets are not disabled and we have key name.
if (sslMultCertSettings.session_ticket_enabled != 0 && sslMultCertSettings.ticket_key_filename) {
@@ -1745,7 +1748,7 @@ ssl_store_ssl_context(
if (ats_ip_pton(sslMultCertSettings.addr, &ep) == 0) {
Debug("ssl", "mapping '%s' to certificate %s", (const char *)sslMultCertSettings.addr, (const char *)certpath);
- if (lookup->insert(ep, SSLCertContext(ctx, sslMultCertSettings.opt, keyblock)) >= 0) {
+ if (certpath != NULL && lookup->insert(ep, SSLCertContext(ctx, sslMultCertSettings.opt, keyblock)) >= 0) {
inserted = true;
}
} else {
@@ -1783,7 +1786,7 @@ ssl_store_ssl_context(
// this code is updated to reconfigure the SSL certificates, it will need some sort of
// refcounting or alternate way of avoiding double frees.
Debug("ssl", "importing SNI names from %s", (const char *)certpath);
- if (ssl_index_certificate(lookup, SSLCertContext(ctx, sslMultCertSettings.opt), certpath)) {
+ if (certpath != NULL && ssl_index_certificate(lookup, SSLCertContext(ctx, sslMultCertSettings.opt), certpath)) {
inserted = true;
}
Re: [1/2] trafficserver git commit: TS-3375 - Further refining error actions on bad SSL configuration.
Posted by James Peach <jp...@apache.org>.
> On Feb 16, 2015, at 9:53 AM, shinrich@apache.org wrote:
>
> Repository: trafficserver
> Updated Branches:
> refs/heads/master 53d5c6dfb -> a8e0c5e17
>
>
> TS-3375 - Further refining error actions on bad SSL configuration.
>
[snip]
>
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/8152dbfc/iocore/net/SSLConfig.cc
> ----------------------------------------------------------------------
> diff --git a/iocore/net/SSLConfig.cc b/iocore/net/SSLConfig.cc
> index 98214d2..627ccd2 100644
> --- a/iocore/net/SSLConfig.cc
> +++ b/iocore/net/SSLConfig.cc
> @@ -342,7 +342,10 @@ SSLCertificateConfig::startup()
> sslCertUpdate->attach("proxy.config.ssl.server.private_key.path");
> sslCertUpdate->attach("proxy.config.ssl.server.cert_chain.filename");
>
> - return reconfigure();
> + if (!reconfigure()) {
> + _exit(1);
> + }
> + return true;
> }
This should just return the status. It's up to the caller to decide whether to exit or not. It looks like we don't typically plumb these failures all the way to main(), but I think it would we reasonable for the SSLNetProcessor to exit. It should exit by using Fatal().
> bool
> @@ -360,7 +363,8 @@ SSLCertificateConfig::reconfigure()
> ink_hrtime_sleep(HRTIME_SECONDS(secs));
> }
>
> - if (SSLParseCertificateConfiguration(params, lookup)) {
> + SSLParseCertificateConfiguration(params, lookup);
You still need to check the return value of SSLParseCertificateConfiguration() here.
> + if (lookup->is_valid) {
> configid = configProcessor.set(configid, lookup);
> } else {
> retStatus = false;
Re: [1/2] trafficserver git commit: TS-3375 - Further refining error actions on bad SSL configuration.
Posted by James Peach <jp...@apache.org>.
> On Feb 16, 2015, at 9:53 AM, shinrich@apache.org wrote:
>
> Repository: trafficserver
> Updated Branches:
> refs/heads/master 53d5c6dfb -> a8e0c5e17
>
>
> TS-3375 - Further refining error actions on bad SSL configuration.
>
[snip]
>
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/8152dbfc/iocore/net/SSLConfig.cc
> ----------------------------------------------------------------------
> diff --git a/iocore/net/SSLConfig.cc b/iocore/net/SSLConfig.cc
> index 98214d2..627ccd2 100644
> --- a/iocore/net/SSLConfig.cc
> +++ b/iocore/net/SSLConfig.cc
> @@ -342,7 +342,10 @@ SSLCertificateConfig::startup()
> sslCertUpdate->attach("proxy.config.ssl.server.private_key.path");
> sslCertUpdate->attach("proxy.config.ssl.server.cert_chain.filename");
>
> - return reconfigure();
> + if (!reconfigure()) {
> + _exit(1);
> + }
> + return true;
> }
This should just return the status. It's up to the caller to decide whether to exit or not. It looks like we don't typically plumb these failures all the way to main(), but I think it would we reasonable for the SSLNetProcessor to exit. It should exit by using Fatal().
> bool
> @@ -360,7 +363,8 @@ SSLCertificateConfig::reconfigure()
> ink_hrtime_sleep(HRTIME_SECONDS(secs));
> }
>
> - if (SSLParseCertificateConfiguration(params, lookup)) {
> + SSLParseCertificateConfiguration(params, lookup);
You still need to check the return value of SSLParseCertificateConfiguration() here.
> + if (lookup->is_valid) {
> configid = configProcessor.set(configid, lookup);
> } else {
> retStatus = false;