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)

1 files 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 <beck@openbsd.org>
  *
@@ -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);