ssl: consistent certificate expiry error messages

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

Branch: refs/heads/master
Commit: c7ea5ac5a45e14f7510e12f1ff125ffd529ac0e1
Parents: 6f7e00e
Author: James Peach <[email protected]>
Authored: Sat Oct 24 11:27:08 2015 -0700
Committer: James Peach <[email protected]>
Committed: Sat Oct 24 11:30:51 2015 -0700

----------------------------------------------------------------------
 iocore/net/OCSPStapling.cc  |  2 +-
 iocore/net/P_OCSPStapling.h |  2 +-
 iocore/net/SSLUtils.cc      | 36 +++++++++++++++++++-----------------
 3 files changed, 21 insertions(+), 19 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/trafficserver/blob/c7ea5ac5/iocore/net/OCSPStapling.cc
----------------------------------------------------------------------
diff --git a/iocore/net/OCSPStapling.cc b/iocore/net/OCSPStapling.cc
index 2c42f8f..67e7fe6 100644
--- a/iocore/net/OCSPStapling.cc
+++ b/iocore/net/OCSPStapling.cc
@@ -103,7 +103,7 @@ stapling_get_issuer(SSL_CTX *ssl_ctx, X509 *x)
 }
 
 bool
-ssl_stapling_init_cert(SSL_CTX *ctx, X509 *cert, char *certname)
+ssl_stapling_init_cert(SSL_CTX *ctx, X509 *cert, const char *certname)
 {
   certinfo *cinf;
   scoped_X509 issuer;

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/c7ea5ac5/iocore/net/P_OCSPStapling.h
----------------------------------------------------------------------
diff --git a/iocore/net/P_OCSPStapling.h b/iocore/net/P_OCSPStapling.h
index dedbfa2..ddb8425 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, X509 *cert, char *certname);
+bool ssl_stapling_init_cert(SSL_CTX *ctx, X509 *cert, const 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/c7ea5ac5/iocore/net/SSLUtils.cc
----------------------------------------------------------------------
diff --git a/iocore/net/SSLUtils.cc b/iocore/net/SSLUtils.cc
index 4282bae..ec44b47 100644
--- a/iocore/net/SSLUtils.cc
+++ b/iocore/net/SSLUtils.cc
@@ -1208,10 +1208,9 @@ SSLPrivateKeyHandler(SSL_CTX *ctx, const SSLConfigParams 
*params, const ats_scop
 }
 
 static int
-SSLCheckServerCertNow(X509 *myCert, char *certname)
+SSLCheckServerCertNow(X509 *cert, const char *certname)
 {
   int timeCmpValue;
-  time_t currentTime;
 
   // SSLCheckServerCertNow() -  returns 0 on OK or negative value on failure
   // and update log as appropriate.
@@ -1221,34 +1220,37 @@ SSLCheckServerCertNow(X509 *myCert, char *certname)
   // - 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 (!myCert) {
+  if (!cert) {
     // a truncated certificate would fall into here
-    Error("Checking NULL certificate from %s", certname);
+    Error("invalid certificate %s: file is truncated or corrupted", certname);
     return -3;
   }
 
-  time(&currentTime);
-  if (!(timeCmpValue = X509_cmp_time(X509_get_notBefore(myCert), 
&currentTime))) {
+  // XXX we should log the notBefore and notAfter dates in the errors ...
+
+  timeCmpValue = X509_cmp_current_time(X509_get_notBefore(cert));
+  if (timeCmpValue == 0) {
     // an error occured parsing the time, which we'll call a bogosity
-    Error("Error occured while parsing server certificate notBefore time. %s", 
certname);
+    Error("invalid certificate %s: unable to parse notBefore time", certname);
     return -3;
-  } else if (0 < timeCmpValue) {
+  } else if (timeCmpValue > 0) {
     // cert contains a date before the notBefore
-    Error("Server certificate notBefore date is in the future - INVALID 
CERTIFICATE %s", certname);
+    Error("invalid certificate %s: notBefore date is in the future", certname);
     return -4;
   }
 
-  if (!(timeCmpValue = X509_cmp_time(X509_get_notAfter(myCert), 
&currentTime))) {
+  timeCmpValue = X509_cmp_current_time(X509_get_notAfter(cert));
+  if (timeCmpValue == 0) {
     // an error occured parsing the time, which we'll call a bogosity
-    Error("Error occured while parsing server certificate notAfter time.");
+    Error("invalid certificate %s: unable to parse notAfter time", certname);
     return -3;
-  } else if (0 > timeCmpValue) {
+  } else if (timeCmpValue < 0) {
     // cert is expired
-    Error("Server certificate EXPIRED - INVALID CERTIFICATE %s", certname);
+    Error("invalid certificate %s: certificate expired", certname);
     return -5;
   }
 
-  Debug("ssl", "Server certificate %s passed accessibility and date checks", 
certname);
+  Debug("ssl", "server certificate %s passed accessibility and date checks", 
certname);
   return 0; // all good
 
 } /* CheckServerCertNow() */
@@ -1540,7 +1542,7 @@ 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, X509 
*cert, char *certname)
+ssl_index_certificate(SSLCertLookup *lookup, SSLCertContext const &cc, X509 
*cert, const char *certname)
 {
   X509_NAME *subject = NULL;
   bool inserted = false;
@@ -1668,13 +1670,13 @@ ssl_store_ssl_context(const SSLConfigParams *params, 
SSLCertLookup *lookup, cons
 #if TS_USE_TLS_ALPN
   SSL_CTX_set_alpn_select_cb(ctx, SSLNetVConnection::select_next_protocol, 
NULL);
 #endif /* TS_USE_TLS_ALPN */
-  char *certname = sslMultCertSettings.cert.get();
 
+  const char *certname = sslMultCertSettings.cert.get();
   for (unsigned i = 0; i < cert_list.length(); ++i) {
     if (0 > SSLCheckServerCertNow(cert_list[i], certname)) {
       /* 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", (certname) ? (const 
char *)certname : "(null)");
+      Debug("ssl", "Marking certificate as NOT VALID: %s", certname);
       lookup->is_valid = false;
     }
   }

Reply via email to