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/src/lib/libcrypto/asn1/a_time_tm.c b/src/lib/libcrypto/asn1/a_time_tm.c
index 23e2ce4b4c..9ddae82768 100644
--- a/src/lib/libcrypto/asn1/a_time_tm.c
+++ b/src/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/src/lib/libcrypto/asn1/asn1_locl.h b/src/lib/libcrypto/asn1/asn1_locl.h
index a0a1842d99..ec853250f2 100644
--- a/src/lib/libcrypto/asn1/asn1_locl.h
+++ b/src/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/src/lib/libcrypto/asn1/tasn_dec.c b/src/lib/libcrypto/asn1/tasn_dec.c
index 375425a9f2..235685484c 100644
--- a/src/lib/libcrypto/asn1/tasn_dec.c
+++ b/src/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) {