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@

diff --git a/lib/libcrypto/asn1/a_time_tm.c b/lib/libcrypto/asn1/a_time_tm.c
index 23e2ce4..9ddae82 100644 (file)
--- 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 a0a1842..ec85325 100644 (file)
--- 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 375425a..2356854 100644 (file)
--- 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) {