From 02ba34f9c30b1ca181ba7d022364cc0c076e074c Mon Sep 17 00:00:00 2001 From: beck Date: Wed, 29 Jun 2022 08:56:44 +0000 Subject: [PATCH] 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@ --- lib/libcrypto/asn1/a_time_tm.c | 210 ++++++++++++++++++++++----------- lib/libcrypto/asn1/asn1_locl.h | 4 +- lib/libcrypto/asn1/tasn_dec.c | 9 +- 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 * @@ -24,6 +24,7 @@ #include #include +#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, ©); + + if (is_gentime) { + if (!cbs_get_two_digit_value(©, &val)) + return 0; + year = val * 100; + if (!cbs_get_two_digit_value(©, &val)) + return 0; + year += val; + } else { + year = 1900; + if (!cbs_get_two_digit_value(©, &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(©, &month)) + return 0; + if (month < 1 || month > 12) + return 0; /* Reject invalid months. */ + + if (!cbs_get_two_digit_value(©, &day)) + return 0; + if (!is_valid_day(year, month, day)) + return 0; /* Reject invalid days. */ + + if (!cbs_get_two_digit_value(©, &hour)) + return 0; + if (hour > 23) + return 0; /* Reject invalid hours. */ + + if (!cbs_get_two_digit_value(©, &min)) + return 0; + if (min > 59) + return 0; /* Reject invalid minutes. */ + + if (!cbs_get_two_digit_value(©, &sec)) + return 0; + if (sec > 59) + return 0; /* Reject invalid seconds. Leap seconds are invalid. */ + + if (!CBS_get_u8(©, &tz)) + return 0; + if ( tz != 'Z') + return 0; /* Reject anything but Z on the end. */ + + if (CBS_len(©) != 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 = <m; - 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) { -- 2.20.1