Fix several issues in the OCSP code that could result in the incorrect

generation and parsing of OCSP requests. This remediates a lack of error
checking on time parsing in these functions, and ensures that only
GENERALIZEDTIME formats are accepted for OCSP, as per RFC 6960

Issues reported, and fixes provided by  Kazuki Yamaguchi <k@rhe.jp>
and Kinichiro Inoguchi <kinichiro.inoguchi@gmail.com>

2 files changed, 33 insertions, 9 deletions

diff --git a/src/lib/libssl/src/crypto/ocsp/ocsp_cl.c b/src/lib/libssl/src/crypto/ocsp/ocsp_cl.c
index a4320d9278..1968550a7e 100644
--- a/src/lib/libssl/src/crypto/ocsp/ocsp_cl.c
+++ b/src/lib/libssl/src/crypto/ocsp/ocsp_cl.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ocsp_cl.c,v 1.8 2014/10/18 17:20:40 jsing Exp $ */
+/* $OpenBSD: ocsp_cl.c,v 1.8.2.1 2016/06/25 16:43:01 beck Exp $ */
 /* Written by Tom Titchener <Tom_Titchener@groove.net> for the OpenSSL
  * project. */
 
@@ -71,6 +71,9 @@
 #include <openssl/x509.h>
 #include <openssl/x509v3.h>
 
+int asn1_time_parse(const char *, size_t, struct tm *, int);
+int asn1_tm_cmp(struct tm *, struct tm *);
+
 /* Utility functions related to sending OCSP requests and extracting
  * relevant information from the response.
  */
@@ -329,25 +332,43 @@ OCSP_check_validity(ASN1_GENERALIZEDTIME *thisupd,
 {
         int ret = 1;
         time_t t_now, t_tmp;
+        struct tm tm_this, tm_next, tm_tmp;
 
         time(&t_now);
+
+        /*
+         * Times must explicitly be a GENERALIZEDTIME as per section
+         * 4.2.2.1 of RFC 6960 - It is invalid to accept other times
+         * (such as UTCTIME permitted/required by RFC 5280 for certificates)
+         */
+
         /* Check thisUpdate is valid and not more than nsec in the future */
-        if (!ASN1_GENERALIZEDTIME_check(thisupd)) {
+        if (asn1_time_parse(thisupd->data, thisupd->length, &tm_this,
+            V_ASN1_GENERALIZEDTIME) != V_ASN1_GENERALIZEDTIME) {
                 OCSPerr(OCSP_F_OCSP_CHECK_VALIDITY,
                     OCSP_R_ERROR_IN_THISUPDATE_FIELD);
                 ret = 0;
         } else {
                 t_tmp = t_now + nsec;
-                if (X509_cmp_time(thisupd, &t_tmp) > 0) {
+                if (gmtime_r(&t_tmp, &tm_tmp) == NULL)
+                        return 0;
+                if (asn1_tm_cmp(&tm_this, &tm_tmp) > 0) {
                         OCSPerr(OCSP_F_OCSP_CHECK_VALIDITY,
                             OCSP_R_STATUS_NOT_YET_VALID);
                         ret = 0;
                 }
 
-                /* If maxsec specified check thisUpdate is not more than maxsec in the past */
+                /*
+                 * If maxsec specified check thisUpdate is not more than maxsec
+                 * in the past
+                 */
                 if (maxsec >= 0) {
                         t_tmp = t_now - maxsec;
-                        if (X509_cmp_time(thisupd, &t_tmp) < 0) {
+                        if (gmtime_r(&t_tmp, &tm_tmp) == NULL)
+                                return 0;
+                        if (gmtime_r(&t_tmp, &tm_tmp) == NULL)
+                                return 0;
+                        if (asn1_tm_cmp(&tm_this, &tm_tmp) < 0) {
                                 OCSPerr(OCSP_F_OCSP_CHECK_VALIDITY,
                                     OCSP_R_STATUS_TOO_OLD);
                                 ret = 0;
@@ -359,13 +380,16 @@ OCSP_check_validity(ASN1_GENERALIZEDTIME *thisupd,
                 return ret;
 
         /* Check nextUpdate is valid and not more than nsec in the past */
-        if (!ASN1_GENERALIZEDTIME_check(nextupd)) {
+        if (asn1_time_parse(nextupd->data, nextupd->length, &tm_next,
+            V_ASN1_GENERALIZEDTIME) != V_ASN1_GENERALIZEDTIME) {
                 OCSPerr(OCSP_F_OCSP_CHECK_VALIDITY,
                     OCSP_R_ERROR_IN_NEXTUPDATE_FIELD);
                 ret = 0;
         } else {
                 t_tmp = t_now - nsec;
-                if (X509_cmp_time(nextupd, &t_tmp) < 0) {
+                if (gmtime_r(&t_tmp, &tm_tmp) == NULL)
+                        return 0;
+                if (asn1_tm_cmp(&tm_next, &tm_tmp) < 0) {
                         OCSPerr(OCSP_F_OCSP_CHECK_VALIDITY,
                             OCSP_R_STATUS_EXPIRED);
                         ret = 0;
diff --git a/src/lib/libssl/src/crypto/ocsp/ocsp_srv.c b/src/lib/libssl/src/crypto/ocsp/ocsp_srv.c
index 8f28916757..e300631eb1 100644
--- a/src/lib/libssl/src/crypto/ocsp/ocsp_srv.c
+++ b/src/lib/libssl/src/crypto/ocsp/ocsp_srv.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ocsp_srv.c,v 1.7 2014/10/18 17:20:40 jsing Exp $ */
+/* $OpenBSD: ocsp_srv.c,v 1.7.2.1 2016/06/25 16:43:01 beck Exp $ */
 /* Written by Dr Stephen N Henson (steve@openssl.org) for the OpenSSL
  * project 2001.
  */
@@ -260,7 +260,7 @@ OCSP_basic_sign(OCSP_BASICRESP *brsp, X509 *signer, EVP_PKEY *key,
         }
 
         if (!(flags & OCSP_NOTIME) &&
-            !X509_gmtime_adj(brsp->tbsResponseData->producedAt, 0))
+            !ASN1_GENERALIZEDTIME_set(brsp->tbsResponseData->producedAt, time(NULL)))
                 goto err;
 
         /* Right now, I think that not doing double hashing is the right