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/05/16 18:49:49 UTC
trafficserver git commit: TS-3610: Load reads certificate from file
multiple times.
Repository: trafficserver
Updated Branches:
refs/heads/master 39a980a49 -> 13396fd38
TS-3610: Load reads certificate from file multiple times.
Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/13396fd3
Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/13396fd3
Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/13396fd3
Branch: refs/heads/master
Commit: 13396fd387df0bdb0ce69595d8f65da9f5610fd2
Parents: 39a980a
Author: shinrich <sh...@yahoo-inc.com>
Authored: Sat May 16 11:49:25 2015 -0500
Committer: shinrich <sh...@yahoo-inc.com>
Committed: Sat May 16 11:49:25 2015 -0500
----------------------------------------------------------------------
CHANGES | 2 ++
iocore/net/OCSPStapling.cc | 11 +++------
iocore/net/P_OCSPStapling.h | 2 +-
iocore/net/P_SSLUtils.h | 2 +-
iocore/net/SSLUtils.cc | 53 +++++++++++++---------------------------
5 files changed, 25 insertions(+), 45 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/13396fd3/CHANGES
----------------------------------------------------------------------
diff --git a/CHANGES b/CHANGES
index 69be670..8a05998 100644
--- a/CHANGES
+++ b/CHANGES
@@ -1,6 +1,8 @@
-*- coding: utf-8 -*-
Changes with Apache Traffic Server 6.0.0
+ *) [TS-3610] Loading reads certificate from file multiple times.
+
*) [TS-3608] Client side SSL does not validate upstream hostname
*) [TS-3604] Transparent Mode does not work when accept_threads set to 0.
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/13396fd3/iocore/net/OCSPStapling.cc
----------------------------------------------------------------------
diff --git a/iocore/net/OCSPStapling.cc b/iocore/net/OCSPStapling.cc
index 2a1ef02..433fabe 100644
--- a/iocore/net/OCSPStapling.cc
+++ b/iocore/net/OCSPStapling.cc
@@ -103,17 +103,14 @@ stapling_get_issuer(SSL_CTX *ssl_ctx, X509 *x)
}
bool
-ssl_stapling_init_cert(SSL_CTX *ctx, const char *certfile)
+ssl_stapling_init_cert(SSL_CTX *ctx, X509 *cert, char *certname)
{
certinfo *cinf;
- scoped_X509 cert;
scoped_X509 issuer;
STACK_OF(OPENSSL_STRING) *aia = NULL;
- scoped_BIO bio(BIO_new_file(certfile, "r"));
- cert = PEM_read_bio_X509_AUX(bio.get(), NULL, NULL, NULL);
if (!cert) {
- Debug("ssl", "can not read cert from certfile %s!", certfile);
+ Debug("ssl", "Null cert passed in");
return false;
}
@@ -141,7 +138,7 @@ ssl_stapling_init_cert(SSL_CTX *ctx, const char *certfile)
issuer = stapling_get_issuer(ctx, cert);
if (issuer == NULL) {
- Debug("ssl", "can not get issuer certificate!");
+ Debug("ssl", "cannot get issuer certificate from %s!", certname);
return false;
}
@@ -154,7 +151,7 @@ ssl_stapling_init_cert(SSL_CTX *ctx, const char *certfile)
if (aia)
cinf->uri = sk_OPENSSL_STRING_pop(aia);
if (!cinf->uri) {
- Debug("ssl", "no responder URI");
+ Debug("ssl", "no responder URI in %s", certname);
}
if (aia)
X509_email_free(aia);
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/13396fd3/iocore/net/P_OCSPStapling.h
----------------------------------------------------------------------
diff --git a/iocore/net/P_OCSPStapling.h b/iocore/net/P_OCSPStapling.h
index 077529c..dedbfa2 100644
--- a/iocore/net/P_OCSPStapling.h
+++ b/iocore/net/P_OCSPStapling.h
@@ -28,7 +28,7 @@
#ifdef SSL_CTX_set_tlsext_status_cb
#define HAVE_OPENSSL_OCSP_STAPLING 1
void ssl_stapling_ex_init();
-bool ssl_stapling_init_cert(SSL_CTX *ctx, const char *certfile);
+bool ssl_stapling_init_cert(SSL_CTX *ctx, X509 *cert, char *certname);
void ocsp_update();
int ssl_callback_ocsp_stapling(SSL *);
#endif /* SSL_CTX_set_tlsext_status_cb */
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/13396fd3/iocore/net/P_SSLUtils.h
----------------------------------------------------------------------
diff --git a/iocore/net/P_SSLUtils.h b/iocore/net/P_SSLUtils.h
index efab041..7f78979 100644
--- a/iocore/net/P_SSLUtils.h
+++ b/iocore/net/P_SSLUtils.h
@@ -1,4 +1,4 @@
-/** @file
+/**
@section license License
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/13396fd3/iocore/net/SSLUtils.cc
----------------------------------------------------------------------
diff --git a/iocore/net/SSLUtils.cc b/iocore/net/SSLUtils.cc
index d81bd02..d15f0e5 100644
--- a/iocore/net/SSLUtils.cc
+++ b/iocore/net/SSLUtils.cc
@@ -1166,10 +1166,8 @@ SSLPrivateKeyHandler(SSL_CTX *ctx, const SSLConfigParams *params, const ats_scop
}
static int
-SSLCheckServerCertNow(const char *certFilename)
+SSLCheckServerCertNow(X509 *myCert, char *certname)
{
- BIO *bioFP = NULL;
- X509 *myCert;
int timeCmpValue;
time_t currentTime;
@@ -1181,27 +1179,9 @@ SSLCheckServerCertNow(const char *certFilename)
// - current time is between notBefore and notAfter dates of certificate
// if anything is not kosher, a negative value is returned and appropriate error logged.
- if ((!certFilename) || (!(*certFilename))) {
- return -2;
- }
-
- if ((bioFP = BIO_new(BIO_s_file())) == NULL) {
- Error("BIO_new() failed for server certificate check. Out of memory?");
- return -1;
- }
-
- if (BIO_read_filename(bioFP, certFilename) <= 0) {
- // file not found, or not accessible due to permissions
- Error("Can't open server certificate file: \"%s\"\n", certFilename);
- BIO_free(bioFP);
- return -2;
- }
-
- myCert = PEM_read_bio_X509(bioFP, NULL, 0, NULL);
- BIO_free(bioFP);
if (!myCert) {
// a truncated certificate would fall into here
- Error("Error during server certificate PEM read. Is this a PEM format certificate?: \"%s\"\n", certFilename);
+ Error("Checking NULL certificate from %s", certname);
return -3;
}
@@ -1212,7 +1192,7 @@ SSLCheckServerCertNow(const char *certFilename)
return -3;
} else if (0 < timeCmpValue) {
// cert contains a date before the notBefore
- Error("Server certificate notBefore date is in the future - INVALID CERTIFICATE: %s", certFilename);
+ Error("Server certificate notBefore date is in the future - INVALID CERTIFICATE");
return -4;
}
@@ -1222,11 +1202,11 @@ SSLCheckServerCertNow(const char *certFilename)
return -3;
} else if (0 > timeCmpValue) {
// cert is expired
- Error("Server certificate EXPIRED - INVALID CERTIFICATE: %s", certFilename);
+ Error("Server certificate EXPIRED - INVALID CERTIFICATE");
return -5;
}
- Debug("ssl", "Server certificate passed accessibility and date checks: \"%s\"", certFilename);
+ Debug("ssl", "Server certificate passed accessibility and date checks");
return 0; // all good
} /* CheckServerCertNow() */
@@ -1482,16 +1462,13 @@ asn1_strdup(ASN1_STRING *s)
// table aliases for subject CN and subjectAltNames DNS without wildcard,
// insert trie aliases for those with wildcard.
static bool
-ssl_index_certificate(SSLCertLookup *lookup, SSLCertContext const &cc, const char *certfile)
+ssl_index_certificate(SSLCertLookup *lookup, SSLCertContext const &cc, X509 *cert, char *certname)
{
X509_NAME *subject = NULL;
- X509 *cert;
- scoped_BIO bio(BIO_new_file(certfile, "r"));
bool inserted = false;
- cert = PEM_read_bio_X509_AUX(bio.get(), NULL, NULL, NULL);
if (NULL == cert) {
- Error("Failed to load certificate from file %s", certfile);
+ Error("Failed to load certificate %s", certname);
lookup->is_valid = false;
return false;
}
@@ -1511,7 +1488,7 @@ ssl_index_certificate(SSLCertLookup *lookup, SSLCertContext const &cc, const cha
ASN1_STRING *cn = X509_NAME_ENTRY_get_data(e);
subj_name = asn1_strdup(cn);
- Debug("ssl", "mapping '%s' to certificate %s", (const char *)subj_name, certfile);
+ Debug("ssl", "mapping '%s' to certificate %s", (const char *)subj_name, certname);
if (lookup->insert(subj_name, cc) >= 0)
inserted = true;
}
@@ -1530,7 +1507,7 @@ ssl_index_certificate(SSLCertLookup *lookup, SSLCertContext const &cc, const cha
ats_scoped_str dns(asn1_strdup(name->d.dNSName));
// only try to insert if the alternate name is not the main name
if (strcmp(dns, subj_name) != 0) {
- Debug("ssl", "mapping '%s' to certificate %s", (const char *)dns, certfile);
+ Debug("ssl", "mapping '%s' to certificates %s", (const char *)dns, certname);
if (lookup->insert(dns, cc) >= 0)
inserted = true;
}
@@ -1540,7 +1517,6 @@ ssl_index_certificate(SSLCertLookup *lookup, SSLCertContext const &cc, const cha
GENERAL_NAMES_free(names);
}
#endif // HAVE_OPENSSL_TS_H
- X509_free(cert);
return inserted;
}
@@ -1597,6 +1573,7 @@ ssl_store_ssl_context(const SSLConfigParams *params, SSLCertLookup *lookup, cons
ats_scoped_str session_key_path;
ssl_ticket_key_block *keyblock = NULL;
bool inserted = false;
+ scoped_X509 myCert;
if (!ctx) {
lookup->is_valid = false;
@@ -1621,7 +1598,11 @@ ssl_store_ssl_context(const SSLConfigParams *params, SSLCertLookup *lookup, cons
certpath = NULL;
}
- if (0 > SSLCheckServerCertNow((const char *)certpath)) {
+ scoped_BIO bio(BIO_new_file(certpath, "r"));
+ if (bio) {
+ myCert = PEM_read_bio_X509(bio, NULL, 0, NULL);
+ }
+ if (!myCert || 0 > SSLCheckServerCertNow(myCert, certpath)) {
/* At this point, we know cert is bad, and we've already printed a
descriptive reason as to why cert is bad to the log file */
Debug("ssl", "Marking certificate as NOT VALID: %s", (certpath) ? (const char *)certpath : "(null)");
@@ -1680,7 +1661,7 @@ ssl_store_ssl_context(const SSLConfigParams *params, SSLCertLookup *lookup, cons
if (SSLConfigParams::ssl_ocsp_enabled) {
Debug("ssl", "ssl ocsp stapling is enabled");
SSL_CTX_set_tlsext_status_cb(ctx, ssl_callback_ocsp_stapling);
- if (!ssl_stapling_init_cert(ctx, (const char *)certpath)) {
+ if (!ssl_stapling_init_cert(ctx, myCert, certpath)) {
Warning("fail to configure SSL_CTX for OCSP Stapling info for certificate at %s", (const char *)certpath);
}
} else {
@@ -1696,7 +1677,7 @@ ssl_store_ssl_context(const SSLConfigParams *params, SSLCertLookup *lookup, cons
// 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 (certpath != NULL && ssl_index_certificate(lookup, SSLCertContext(ctx, sslMultCertSettings.opt), certpath)) {
+ if (myCert != NULL && ssl_index_certificate(lookup, SSLCertContext(ctx, sslMultCertSettings.opt), myCert, certpath)) {
inserted = true;
}