trafficserver git commit: TS-3538: ATS should perform validity checks on certificate prior to serving. This closes #195.

[sh...@apache.org]

Repository: trafficserver
Updated Branches:
  refs/heads/master c2ed99714 -> ac5f73a03


TS-3538: ATS should perform validity checks on certificate prior to serving.
This closes #195.


Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/ac5f73a0
Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/ac5f73a0
Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/ac5f73a0

Branch: refs/heads/master
Commit: ac5f73a0398df7581ccb34b0d6c6d400ea810336
Parents: c2ed997
Author: Dave Thompson <da...@yahoo-inc.com>
Authored: Thu Apr 30 10:19:32 2015 -0500
Committer: shinrich <sh...@yahoo-inc.com>
Committed: Thu Apr 30 10:19:32 2015 -0500

----------------------------------------------------------------------
 CHANGES                |  2 ++
 iocore/net/SSLUtils.cc | 76 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 78 insertions(+)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/trafficserver/blob/ac5f73a0/CHANGES
----------------------------------------------------------------------
diff --git a/CHANGES b/CHANGES
index 7760948..eae4f99 100644
--- a/CHANGES
+++ b/CHANGES
@@ -1,6 +1,8 @@
                                                          -*- coding: utf-8 -*-
 Changes with Apache Traffic Server 6.0.0
 
+  *) [TS-3538]: Perform server certificate validity check.
+
   *) [TS-3549]: Configurable option to avoid thundering herd problem 
      for multiple concurrent requests. The initial POC patch for this
      solution came from Justin Laue. This patch will further be

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/ac5f73a0/iocore/net/SSLUtils.cc
----------------------------------------------------------------------
diff --git a/iocore/net/SSLUtils.cc b/iocore/net/SSLUtils.cc
index 76727c5..2fae482 100644
--- a/iocore/net/SSLUtils.cc
+++ b/iocore/net/SSLUtils.cc
@@ -1148,6 +1148,74 @@ SSLPrivateKeyHandler(SSL_CTX *ctx, const SSLConfigParams *params, const ats_scop
   return true;
 }
 
+static int
+SSLCheckServerCertNow(const char *certFilename)
+{
+BIO    *bioFP = NULL;
+X509   *myCert;
+int    timeCmpValue;
+time_t currentTime;
+
+// SSLCheckServerCertNow() -  returns 0 on OK or negative value on failure
+// and update log as appropriate.
+// Will check:
+// - if file exists, and has read permissions 
+// - for truncation or other PEM read fail 
+// - 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);
+        return -3;
+    }
+
+    time(&currentTime);
+    if (!(timeCmpValue = X509_cmp_time(X509_get_notBefore(myCert), &currentTime))) {
+        // an error occured parsing the time, which we'll call a bogosity 
+        Error("Error occured while parsing server certificate notBefore time.");
+        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);
+        return -4;
+    }
+
+    if (!(timeCmpValue = X509_cmp_time(X509_get_notAfter(myCert), &currentTime))) {
+        // an error occured parsing the time, which we'll call a bogosity 
+        Error("Error occured while parsing server certificate notAfter time.");
+        return -3;
+    } else if ( 0 > timeCmpValue) {
+        // cert is expired
+        Error("Server certificate EXPIRED - INVALID CERTIFICATE: %s",certFilename);
+        return -5;
+    }
+
+    Debug("ssl","Server certificate passed accessibility and date checks: \"%s\"",certFilename);
+    return 0; // all good 
+
+} /* CheckServerCertNow() */
+
+
+
 SSL_CTX *
 SSLInitServerContext(const SSLConfigParams *params, const ssl_user_config &sslMultCertSettings)
 {
@@ -1624,6 +1692,14 @@ ssl_store_ssl_context(const SSLConfigParams *params, SSLCertLookup *lookup, cons
     certpath = NULL;
   }
 
+  if (0 > SSLCheckServerCertNow((const char *) 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)" );
+    lookup->is_valid = false; 
+  }
+
   // 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) {
     ats_scoped_str ticket_key_path(Layout::relative_to(params->serverCertPathOnly, sslMultCertSettings.ticket_key_filename));