From 53d4bf4fbf49ab7a6713b118eb98b2edfbb627dc Mon Sep 17 00:00:00 2001 From: beck <> Date: Sat, 25 Jun 2016 16:42:42 +0000 Subject: 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 and Kinichiro Inoguchi --- src/lib/libssl/src/crypto/ocsp/ocsp_cl.c | 38 +++++++++++++++++++++++++------ src/lib/libssl/src/crypto/ocsp/ocsp_srv.c | 4 ++-- 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..811101f385 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.6.1 2016/06/25 16:42:40 beck Exp $ */ /* Written by Tom Titchener for the OpenSSL * project. */ @@ -71,6 +71,9 @@ #include #include +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..a215c4ac0e 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.6.1 2016/06/25 16:42:40 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 -- cgit v1.2.3-55-g6feb