From f93ff4217d84706c56c31646a99ce08785b8286d Mon Sep 17 00:00:00 2001 From: tb Date: Thu, 11 Apr 2024 06:42:09 +0000 Subject: [PATCH] Rework internal tm_to_*() converters Make them static. Don't make them allocate if passed a NULL ASN1_TIME to avoid leaks. This currently means that we accept a NULL and succeed. That's very ugly but better than what we have now. Simplify ASN1_TIME_set_string_internal() accordingly and allocate an ASN1_TIME at the API boundary of ASN1_TIME_adj_internal() and of ASN1_TIME_to_generalized_time(). ok beck (after a lot of squealing and distress) --- lib/libcrypto/asn1/a_time_tm.c | 135 ++++++++++++++++----------------- 1 file changed, 66 insertions(+), 69 deletions(-) diff --git a/lib/libcrypto/asn1/a_time_tm.c b/lib/libcrypto/asn1/a_time_tm.c index 49d6ea2097b..59fd32e9caa 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.36 2024/04/10 14:55:12 beck Exp $ */ +/* $OpenBSD: a_time_tm.c,v 1.37 2024/04/11 06:42:09 tb Exp $ */ /* * Copyright (c) 2015 Bob Beck * @@ -78,31 +78,24 @@ ASN1_time_tm_clamp_notafter(struct tm *tm) } /* Convert time to GeneralizedTime, X.690, 11.7. */ -ASN1_TIME * +static int tm_to_gentime(struct tm *tm, ASN1_TIME *atime) { char *time_str = NULL; - int year; - year = tm->tm_year + 1900; - if (year < 0 || year > 9999) { + if (tm->tm_year < -1900 || tm->tm_year > 9999 - 1900) { ASN1error(ASN1_R_ILLEGAL_TIME_VALUE); - goto err; + return 0; } - if (asprintf(&time_str, "%04u%02u%02u%02u%02u%02uZ", year, + if (atime == NULL) + return 1; + + if (asprintf(&time_str, "%04u%02u%02u%02u%02u%02uZ", tm->tm_year + 1900, tm->tm_mon + 1, tm->tm_mday, tm->tm_hour, tm->tm_min, tm->tm_sec) == -1) { - time_str = NULL; - ASN1error(ERR_R_MALLOC_FAILURE); - goto err; - } - - if (atime == NULL) - atime = ASN1_TIME_new(); - if (atime == NULL) { ASN1error(ERR_R_MALLOC_FAILURE); - goto err; + return 0; } free(atime->data); @@ -110,38 +103,28 @@ tm_to_gentime(struct tm *tm, ASN1_TIME *atime) atime->length = GENTIME_LENGTH; atime->type = V_ASN1_GENERALIZEDTIME; - return (atime); - - err: - free(time_str); - - return (NULL); + return 1; } /* Convert time to UTCTime, X.690, 11.8. */ -ASN1_TIME * +static int tm_to_utctime(struct tm *tm, ASN1_TIME *atime) { char *time_str = NULL; if (tm->tm_year >= 150 || tm->tm_year < 50) { ASN1error(ASN1_R_ILLEGAL_TIME_VALUE); - goto err; + return 0; } + if (atime == NULL) + return 1; + if (asprintf(&time_str, "%02u%02u%02u%02u%02u%02uZ", tm->tm_year % 100, tm->tm_mon + 1, tm->tm_mday, tm->tm_hour, tm->tm_min, tm->tm_sec) == -1) { - time_str = NULL; - ASN1error(ERR_R_MALLOC_FAILURE); - goto err; - } - - if (atime == NULL) - atime = ASN1_TIME_new(); - if (atime == NULL) { ASN1error(ERR_R_MALLOC_FAILURE); - goto err; + return 0; } free(atime->data); @@ -149,15 +132,10 @@ tm_to_utctime(struct tm *tm, ASN1_TIME *atime) atime->length = UTCTIME_LENGTH; atime->type = V_ASN1_UTCTIME; - return (atime); - - err: - free(time_str); - - return (NULL); + return 1; } -ASN1_TIME * +static int tm_to_rfc5280_time(struct tm *tm, ASN1_TIME *atime) { if (tm->tm_year >= 50 && tm->tm_year < 150) @@ -344,58 +322,67 @@ ASN1_time_parse(const char *bytes, size_t len, struct tm *tm, int mode) static int ASN1_TIME_set_string_internal(ASN1_TIME *s, const char *str, int mode) { - ASN1_TIME *atime = s; struct tm tm; int type; - int ret = 0; if ((type = ASN1_time_parse(str, strlen(str), &tm, mode)) == -1) return (0); switch (mode) { case V_ASN1_UTCTIME: - ret = (type == mode && (atime = tm_to_utctime(&tm, s)) != NULL); - break; + return (type == mode && tm_to_utctime(&tm, s)); case V_ASN1_GENERALIZEDTIME: - ret = (type == mode && (atime = tm_to_gentime(&tm, s)) != NULL); - break; + return (type == mode && tm_to_gentime(&tm, s)); case RFC5280: - ret = ((atime = tm_to_rfc5280_time(&tm, s)) != NULL); - break; + return (tm_to_rfc5280_time(&tm, s)); default: - ret = 0; - break; + return (0); } - - if (atime != s) - ASN1_TIME_free(atime); - - return ret; } static ASN1_TIME * ASN1_TIME_adj_internal(ASN1_TIME *s, time_t t, int offset_day, long offset_sec, int mode) { + ASN1_TIME *atime = s; struct tm tm; if (!asn1_time_time_t_to_tm(&t, &tm)) - return (NULL); + goto err; if (offset_day != 0 || offset_sec != 0) { if (!OPENSSL_gmtime_adj(&tm, offset_day, offset_sec)) - return (NULL); + goto err; } + if (atime == NULL) + atime = ASN1_TIME_new(); + if (atime == NULL) + goto err; + switch (mode) { case V_ASN1_UTCTIME: - return (tm_to_utctime(&tm, s)); + if (!tm_to_utctime(&tm, atime)) + goto err; + break; case V_ASN1_GENERALIZEDTIME: - return (tm_to_gentime(&tm, s)); + if (!tm_to_gentime(&tm, atime)) + goto err; + break; case RFC5280: - return (tm_to_rfc5280_time(&tm, s)); + if (!tm_to_rfc5280_time(&tm, atime)) + goto err; + break; default: - return (NULL); + goto err; } + + return atime; + + err: + if (atime != s) + ASN1_TIME_free(atime); + + return NULL; } ASN1_TIME * @@ -428,19 +415,29 @@ ASN1_TIME_to_generalizedtime(const ASN1_TIME *t, ASN1_GENERALIZEDTIME **out) struct tm tm; if (t->type != V_ASN1_GENERALIZEDTIME && t->type != V_ASN1_UTCTIME) - return (NULL); + goto err; if (t->type != ASN1_time_parse(t->data, t->length, &tm, t->type)) - return (NULL); + goto err; + + if (out == NULL || (agt = *out) == NULL) + agt = ASN1_TIME_new(); + if (agt == NULL) + goto err; + + if (!tm_to_gentime(&tm, agt)) + goto err; - if (out != NULL) - agt = *out; - if ((agt = tm_to_gentime(&tm, agt)) == NULL) - return (NULL); if (out != NULL) *out = agt; - return (agt); + return agt; + + err: + if (out == NULL || *out != agt) + ASN1_TIME_free(agt); + + return NULL; } LCRYPTO_ALIAS(ASN1_TIME_to_generalizedtime); @@ -601,7 +598,7 @@ ASN1_TIME_normalize(ASN1_TIME *t) return 0; if (!ASN1_TIME_to_tm(t, &tm)) return 0; - return tm_to_rfc5280_time(&tm, t) != NULL; + return tm_to_rfc5280_time(&tm, t); } LCRYPTO_ALIAS(ASN1_TIME_normalize); -- 2.20.1