From: beck <beck@openbsd.org>
Date: Sat, 16 Jul 2016 16:14:28 +0000 (+0000)
Subject: Clean up OCSP_check_validity() a bit more.
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=7465f45506b6064d9ec5bdc220c782306aa866dd;p=openbsd

Clean up OCSP_check_validity() a bit more.
- Return on first failure rather than continuing.
- Don't compare times by comparing strings that possibly were not parsable as a time.
ok deraadt@
---

diff --git a/lib/libcrypto/ocsp/ocsp_cl.c b/lib/libcrypto/ocsp/ocsp_cl.c
index 5616ae1bb5d..86baed87247 100644
--- a/lib/libcrypto/ocsp/ocsp_cl.c
+++ b/lib/libcrypto/ocsp/ocsp_cl.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ocsp_cl.c,v 1.10 2016/07/05 03:24:38 beck Exp $ */
+/* $OpenBSD: ocsp_cl.c,v 1.11 2016/07/16 16:14:28 beck Exp $ */
 /* Written by Tom Titchener <Tom_Titchener@groove.net> for the OpenSSL
  * project. */
 
@@ -330,7 +330,6 @@ int
 OCSP_check_validity(ASN1_GENERALIZEDTIME *thisupd,
     ASN1_GENERALIZEDTIME *nextupd, long nsec, long maxsec)
 {
-	int ret = 1;
 	time_t t_now, t_tmp;
 	struct tm tm_this, tm_next, tm_tmp;
 
@@ -347,7 +346,7 @@ OCSP_check_validity(ASN1_GENERALIZEDTIME *thisupd,
 	    V_ASN1_GENERALIZEDTIME) != V_ASN1_GENERALIZEDTIME) {
 		OCSPerr(OCSP_F_OCSP_CHECK_VALIDITY,
 		    OCSP_R_ERROR_IN_THISUPDATE_FIELD);
-		ret = 0;
+		return 0;
 	} else {
 		t_tmp = t_now + nsec;
 		if (gmtime_r(&t_tmp, &tm_tmp) == NULL)
@@ -355,7 +354,7 @@ OCSP_check_validity(ASN1_GENERALIZEDTIME *thisupd,
 		if (asn1_tm_cmp(&tm_this, &tm_tmp) > 0) {
 			OCSPerr(OCSP_F_OCSP_CHECK_VALIDITY,
 			    OCSP_R_STATUS_NOT_YET_VALID);
-			ret = 0;
+			return 0;
 		}
 
 		/*
@@ -369,20 +368,20 @@ OCSP_check_validity(ASN1_GENERALIZEDTIME *thisupd,
 			if (asn1_tm_cmp(&tm_this, &tm_tmp) < 0) {
 				OCSPerr(OCSP_F_OCSP_CHECK_VALIDITY,
 				    OCSP_R_STATUS_TOO_OLD);
-				ret = 0;
+				return 0;
 			}
 		}
 	}
 
 	if (!nextupd)
-		return ret;
+		return 1;
 
 	/* Check nextUpdate is valid and not more than nsec in the past */
 	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;
+		return 0;
 	} else {
 		t_tmp = t_now - nsec;
 		if (gmtime_r(&t_tmp, &tm_tmp) == NULL)
@@ -390,16 +389,16 @@ OCSP_check_validity(ASN1_GENERALIZEDTIME *thisupd,
 		if (asn1_tm_cmp(&tm_next, &tm_tmp) < 0) {
 			OCSPerr(OCSP_F_OCSP_CHECK_VALIDITY,
 			    OCSP_R_STATUS_EXPIRED);
-			ret = 0;
+			return 0;
 		}
 	}
 
 	/* Also don't allow nextUpdate to precede thisUpdate */
-	if (ASN1_STRING_cmp(nextupd, thisupd) < 0) {
+	if (asn1_tm_cmp(&tm_next, &tm_this) < 0) {
 		OCSPerr(OCSP_F_OCSP_CHECK_VALIDITY,
 		    OCSP_R_NEXTUPDATE_BEFORE_THISUPDATE);
-		ret = 0;
+		return 0;
 	}
 
-	return ret;
+	return 1;
 }
diff --git a/lib/libssl/src/crypto/ocsp/ocsp_cl.c b/lib/libssl/src/crypto/ocsp/ocsp_cl.c
index 5616ae1bb5d..86baed87247 100644
--- a/lib/libssl/src/crypto/ocsp/ocsp_cl.c
+++ b/lib/libssl/src/crypto/ocsp/ocsp_cl.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ocsp_cl.c,v 1.10 2016/07/05 03:24:38 beck Exp $ */
+/* $OpenBSD: ocsp_cl.c,v 1.11 2016/07/16 16:14:28 beck Exp $ */
 /* Written by Tom Titchener <Tom_Titchener@groove.net> for the OpenSSL
  * project. */
 
@@ -330,7 +330,6 @@ int
 OCSP_check_validity(ASN1_GENERALIZEDTIME *thisupd,
     ASN1_GENERALIZEDTIME *nextupd, long nsec, long maxsec)
 {
-	int ret = 1;
 	time_t t_now, t_tmp;
 	struct tm tm_this, tm_next, tm_tmp;
 
@@ -347,7 +346,7 @@ OCSP_check_validity(ASN1_GENERALIZEDTIME *thisupd,
 	    V_ASN1_GENERALIZEDTIME) != V_ASN1_GENERALIZEDTIME) {
 		OCSPerr(OCSP_F_OCSP_CHECK_VALIDITY,
 		    OCSP_R_ERROR_IN_THISUPDATE_FIELD);
-		ret = 0;
+		return 0;
 	} else {
 		t_tmp = t_now + nsec;
 		if (gmtime_r(&t_tmp, &tm_tmp) == NULL)
@@ -355,7 +354,7 @@ OCSP_check_validity(ASN1_GENERALIZEDTIME *thisupd,
 		if (asn1_tm_cmp(&tm_this, &tm_tmp) > 0) {
 			OCSPerr(OCSP_F_OCSP_CHECK_VALIDITY,
 			    OCSP_R_STATUS_NOT_YET_VALID);
-			ret = 0;
+			return 0;
 		}
 
 		/*
@@ -369,20 +368,20 @@ OCSP_check_validity(ASN1_GENERALIZEDTIME *thisupd,
 			if (asn1_tm_cmp(&tm_this, &tm_tmp) < 0) {
 				OCSPerr(OCSP_F_OCSP_CHECK_VALIDITY,
 				    OCSP_R_STATUS_TOO_OLD);
-				ret = 0;
+				return 0;
 			}
 		}
 	}
 
 	if (!nextupd)
-		return ret;
+		return 1;
 
 	/* Check nextUpdate is valid and not more than nsec in the past */
 	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;
+		return 0;
 	} else {
 		t_tmp = t_now - nsec;
 		if (gmtime_r(&t_tmp, &tm_tmp) == NULL)
@@ -390,16 +389,16 @@ OCSP_check_validity(ASN1_GENERALIZEDTIME *thisupd,
 		if (asn1_tm_cmp(&tm_next, &tm_tmp) < 0) {
 			OCSPerr(OCSP_F_OCSP_CHECK_VALIDITY,
 			    OCSP_R_STATUS_EXPIRED);
-			ret = 0;
+			return 0;
 		}
 	}
 
 	/* Also don't allow nextUpdate to precede thisUpdate */
-	if (ASN1_STRING_cmp(nextupd, thisupd) < 0) {
+	if (asn1_tm_cmp(&tm_next, &tm_this) < 0) {
 		OCSPerr(OCSP_F_OCSP_CHECK_VALIDITY,
 		    OCSP_R_NEXTUPDATE_BEFORE_THISUPDATE);
-		ret = 0;
+		return 0;
 	}
 
-	return ret;
+	return 1;
 }