Refactor asn1 time parsing to use CBS - enforce valid times in ASN.1 parsing.

While we're here enforce valid days for months and leap years.

Inspired by same in boringssl.

ok jsing@

3 files changed, 155 insertions, 68 deletions

diff --git a/lib/libcrypto/asn1/a_time_tm.c b/lib/libcrypto/asn1/a_time_tm.c
index 23e2ce4b4c0..9ddae82768e 100644
--- a/lib/libcrypto/asn1/a_time_tm.c
+++ b/lib/libcrypto/asn1/a_time_tm.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: a_time_tm.c,v 1.21 2022/06/27 13:54:57 beck Exp $ */
+/* $OpenBSD: a_time_tm.c,v 1.22 2022/06/29 08:56:44 beck Exp $ */
 /*
  * Copyright (c) 2015 Bob Beck <beck@openbsd.org>
  *
@@ -24,6 +24,7 @@
 #include <openssl/asn1t.h>
 #include <openssl/err.h>
 
+#include "bytestring.h"
 #include "o_time.h"
 
 #define RFC5280 0
@@ -173,6 +174,137 @@ tm_to_rfc5280_time(struct tm *tm, ASN1_TIME *atime)
 	return (tm_to_gentime(tm, atime));
 }
 
+
+static int
+cbs_get_two_digit_value(CBS *cbs, int *out)
+{
+	uint8_t first_digit, second_digit;
+
+	if (!CBS_get_u8(cbs, &first_digit))
+		return 0;
+	if (!isdigit(first_digit))
+		return 0;
+	if (!CBS_get_u8(cbs, &second_digit))
+		return 0;
+	if (!isdigit(second_digit))
+		return 0;
+
+	*out = (first_digit - '0') * 10 + (second_digit - '0');
+
+	return 1;
+}
+
+static int
+is_valid_day(int year, int month, int day)
+{
+	if (day < 1)
+		return 0;
+	switch (month) {
+	case 1:
+	case 3:
+	case 5:
+	case 7:
+	case 8:
+	case 10:
+	case 12:
+		return day <= 31;
+	case 4:
+	case 6:
+	case 9:
+	case 11:
+		return day <= 30;
+	case 2:
+		if ((year % 4 == 0 && year % 100 != 0) || year % 400 == 0)
+			return day <= 29;
+		 else
+			return day <= 28;
+	default:
+		return 0;
+	}
+}
+
+/*
+ * asn1_time_parse_cbs returns one if |cbs| is a valid DER-encoded, ASN.1 Time
+ * body within the limitations imposed by RFC 5280, or zero otherwise. The time
+ * is expected to parse as a Generalized Time if is_gentime is true, and as a
+ * UTC Time otherwise. If |out_tm| is non-NULL, |*out_tm| will be zeroed, and
+ * then set to the corresponding time in UTC. This function does not compute
+ * |out_tm->tm_wday| or |out_tm->tm_yday|. |cbs| is not consumed.
+ */
+int
+asn1_time_parse_cbs(const CBS *cbs, int is_gentime, struct tm *out_tm)
+{
+	int year, month, day, hour, min, sec, val;
+	CBS copy;
+	uint8_t tz;
+
+	CBS_dup(cbs, &copy);
+
+	if (is_gentime) {
+		if (!cbs_get_two_digit_value(&copy, &val))
+			return 0;
+		year = val * 100;
+		if (!cbs_get_two_digit_value(&copy, &val))
+			return 0;
+		year += val;
+	} else {
+		year = 1900;
+		if (!cbs_get_two_digit_value(&copy, &val))
+			return 0;
+		year += val;
+		if (year < 1950)
+			year += 100;
+		if (year >= 2050)
+			return 0;  /* A Generalized time must be used. */
+	}
+
+	if (!cbs_get_two_digit_value(&copy, &month))
+		return 0;
+	if (month < 1 || month > 12)
+		return 0; /* Reject invalid months. */
+
+	if (!cbs_get_two_digit_value(&copy, &day))
+		return 0;
+	if (!is_valid_day(year, month, day))
+		return 0; /* Reject invalid days. */
+
+	if (!cbs_get_two_digit_value(&copy, &hour))
+		return 0;
+	if (hour > 23)
+		return 0; /* Reject invalid hours. */
+
+	if (!cbs_get_two_digit_value(&copy, &min))
+		return 0;
+	if (min > 59)
+		return 0; /* Reject invalid minutes. */
+
+	if (!cbs_get_two_digit_value(&copy, &sec))
+		return 0;
+	if (sec > 59)
+		return 0; /* Reject invalid seconds. Leap seconds are invalid. */
+
+	if (!CBS_get_u8(&copy, &tz))
+		return 0;
+	if ( tz != 'Z')
+		return 0; /* Reject anything but Z on the end. */
+
+	if (CBS_len(&copy) != 0)
+		return 0;  /* Reject invalid lengths. */
+
+	if (out_tm != NULL) {
+		memset(out_tm, 0, sizeof(*out_tm));
+		/* Fill in the tm fields corresponding to what we validated. */
+		out_tm->tm_year = year - 1900;
+		out_tm->tm_mon = month - 1;
+		out_tm->tm_mday = day;
+		out_tm->tm_hour = hour;
+		out_tm->tm_min = min;
+		out_tm->tm_sec = sec;
+	}
+
+	return 1;
+}
+
 /*
  * Parse an RFC 5280 format ASN.1 time string.
  *
@@ -188,83 +320,29 @@ tm_to_rfc5280_time(struct tm *tm, ASN1_TIME *atime)
  *
  * Fills in *tm with the corresponding time if tm is non NULL.
  */
-#define	ATOI2(ar)	((ar) += 2, ((ar)[-2] - '0') * 10 + ((ar)[-1] - '0'))
 int
 ASN1_time_parse(const char *bytes, size_t len, struct tm *tm, int mode)
 {
-	size_t i;
+	struct tm tml, *tmp = tm ? tm : &tml;
 	int type = 0;
-	struct tm ltm;
-	struct tm *lt;
-	const char *p;
+	CBS cbs;
 
 	if (bytes == NULL)
 		return (-1);
 
-	/* Constrain to valid lengths. */
-	if (len != UTCTIME_LENGTH && len != GENTIME_LENGTH)
-		return (-1);
-
-	lt = tm;
-	if (lt == NULL)
-		lt = &ltm;
-	memset(lt, 0, sizeof(*lt));
+	CBS_init(&cbs, bytes, len);
 
-	/* Timezone is required and must be GMT (Zulu). */
-	if (bytes[len - 1] != 'Z')
-		return (-1);
-
-	/* Make sure everything else is digits. */
-	for (i = 0; i < len - 1; i++) {
-		if (isdigit((unsigned char)bytes[i]))
-			continue;
-		return (-1);
-	}
-
-	/*
-	 * Validate and convert the time
-	 */
-	p = bytes;
-	switch (len) {
-	case GENTIME_LENGTH:
-		if (mode == V_ASN1_UTCTIME)
-			return (-1);
-		lt->tm_year = (ATOI2(p) * 100) - 1900;	/* cc */
+	if (CBS_len(&cbs) == UTCTIME_LENGTH)
+		type = V_ASN1_UTCTIME;
+	if (CBS_len(&cbs) == GENTIME_LENGTH)
 		type = V_ASN1_GENERALIZEDTIME;
-		/* FALLTHROUGH */
-	case UTCTIME_LENGTH:
-		if (type == 0) {
-			if (mode == V_ASN1_GENERALIZEDTIME)
-				return (-1);
-			type = V_ASN1_UTCTIME;
-		}
-		lt->tm_year += ATOI2(p);		/* yy */
-		if (type == V_ASN1_UTCTIME) {
-			if (lt->tm_year < 50)
-				lt->tm_year += 100;
-		}
-		lt->tm_mon = ATOI2(p) - 1;		/* mm */
-		if (lt->tm_mon < 0 || lt->tm_mon > 11)
-			return (-1);
-		lt->tm_mday = ATOI2(p);			/* dd */
-		if (lt->tm_mday < 1 || lt->tm_mday > 31)
-			return (-1);
-		lt->tm_hour = ATOI2(p);			/* HH */
-		if (lt->tm_hour < 0 || lt->tm_hour > 23)
-			return (-1);
-		lt->tm_min = ATOI2(p);			/* MM */
-		if (lt->tm_min < 0 || lt->tm_min > 59)
-			return (-1);
-		lt->tm_sec = ATOI2(p);			/* SS */
-		/* Leap second 60 is not accepted. Reconsider later? */
-		if (lt->tm_sec < 0 || lt->tm_sec > 59)
-			return (-1);
-		break;
-	default:
-		return (-1);
+	if (asn1_time_parse_cbs(&cbs, type == V_ASN1_GENERALIZEDTIME, tmp)) {
+		if (mode != 0 && mode != type)
+			return -1;
+		return type;
 	}
 
-	return (type);
+	return -1;
 }
 
 /*
diff --git a/lib/libcrypto/asn1/asn1_locl.h b/lib/libcrypto/asn1/asn1_locl.h
index a0a1842d991..ec853250f20 100644
--- a/lib/libcrypto/asn1/asn1_locl.h
+++ b/lib/libcrypto/asn1/asn1_locl.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: asn1_locl.h,v 1.34 2022/06/27 12:36:05 tb Exp $ */
+/* $OpenBSD: asn1_locl.h,v 1.35 2022/06/29 08:56:44 beck Exp $ */
 /* Written by Dr Stephen N Henson (steve@openssl.org) for the OpenSSL
  * project 2006.
  */
@@ -219,4 +219,6 @@ int i2t_ASN1_OBJECT_internal(const ASN1_OBJECT *aobj, char *buf, int buf_len,
     int no_name);
 ASN1_OBJECT *t2i_ASN1_OBJECT_internal(const char *oid);
 
+int asn1_time_parse_cbs(const CBS *cbs, int is_gentime, struct tm *out_tm);
+
 __END_HIDDEN_DECLS
diff --git a/lib/libcrypto/asn1/tasn_dec.c b/lib/libcrypto/asn1/tasn_dec.c
index 375425a9f2a..235685484cc 100644
--- a/lib/libcrypto/asn1/tasn_dec.c
+++ b/lib/libcrypto/asn1/tasn_dec.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: tasn_dec.c,v 1.77 2022/06/25 17:43:56 jsing Exp $ */
+/* $OpenBSD: tasn_dec.c,v 1.78 2022/06/29 08:56:44 beck Exp $ */
 /* Written by Dr Stephen N Henson (steve@openssl.org) for the OpenSSL
  * project 2000.
  */
@@ -353,6 +353,13 @@ asn1_c2i_primitive(ASN1_VALUE **pval, CBS *content, int utype, const ASN1_ITEM *
 			ASN1error(ASN1_R_UNIVERSALSTRING_IS_WRONG_LENGTH);
 			goto err;
 		}
+		if (utype == V_ASN1_UTCTIME || utype == V_ASN1_GENERALIZEDTIME) {
+			if (!asn1_time_parse_cbs(content,
+			    utype == V_ASN1_GENERALIZEDTIME, NULL))  {
+				ASN1error(ASN1_R_INVALID_TIME_FORMAT);
+				goto err;
+			}
+		}
 		/* All based on ASN1_STRING and handled the same way. */
 		if (*pval == NULL) {
 			if ((stmp = ASN1_STRING_type_new(utype)) == NULL) {