Remove the ASN1_STRING_FLAG_X509_TIME flag

It's only use was to do some somewhat confused cruftery
inside of ossl_asn1_time_to_tm as a special case to
implement ASN1_TIME_set_string_X509.

As it turns out, you don't need the cruftery of a special
case inside of ossl_asn1_time_to_tm to implement this
function, so the flag is completely unnecessary.

This removes flag, and simplifies this to work without it.

It removes the cruft only from ossl_asn1_time_to_tm,
minimally. This function really needs some cleanup and
makes my eyes bleed but I am resisting the temptation
to do that with this PR and making this a the minimal
change needed for review. I will clean up that function
in a follow on pr.

As tests on the behaviour of ASN1_TIME_set_string_X509
were added with it, Beyonce dances happily for me and I
only need to pass the existing tests, not write as bunch
of new ones.. .

Reviewed-by: Neil Horman <nhorman@openssl.org>
Reviewed-by: Nikola Pajkovsky <nikolap@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/29187)

diff --git a/CHANGES.md b/CHANGES.md
index 23046079ec6b3df435a249f93874d07ff274259b..afd7f7101085aea05c52d7fb9b28afbd44c3c9ae 100644 (file)
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -76,6 +76,10 @@ OpenSSL 4.0
 
    *Bob Beck*
 
+* The ASN1_STRING_FLAG_X509_TIME define has been removed.
+
+   *Bob Beck*
+
  * various function parameters have been constified,
    in particular for X509-related functions.
 

diff --git a/crypto/asn1/a_time.c b/crypto/asn1/a_time.c
index 51536eb630c14f72699096789b083035d75f0159..26e9b158992756e7784a1955de8d27f7fec842a7 100644 (file)
--- a/crypto/asn1/a_time.c
+++ b/crypto/asn1/a_time.c
@@ -79,33 +79,20 @@ int ossl_asn1_time_to_tm(struct tm *tm, const ASN1_TIME *d)
     static const int max[9] = { 99, 99, 12, 31, 23, 59, 59, 12, 59 };
     static const int mdays[12] = { 31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31 };
     char *a;
-    int n, i, i2, l, o, min_l, strict = 0, end = 6, btz = 5, md;
+    int n, i, i2, l, o, min_l, end = 6, btz = 5, md;
     struct tm tmp;
 #if defined(CHARSET_EBCDIC)
     const char upper_z = 0x5A, num_zero = 0x30, period = 0x2E, minus = 0x2D, plus = 0x2B;
 #else
     const char upper_z = 'Z', num_zero = '0', period = '.', minus = '-', plus = '+';
 #endif
-    /*
-     * ASN1_STRING_FLAG_X509_TIME is used to enforce RFC 5280
-     * time string format, in which:
-     *
-     * 1. "seconds" is a 'MUST'
-     * 2. "Zulu" timezone is a 'MUST'
-     * 3. "+|-" is not allowed to indicate a timezone
-     */
+
     if (d->type == V_ASN1_UTCTIME) {
         min_l = 13;
-        if (d->flags & ASN1_STRING_FLAG_X509_TIME) {
-            strict = 1;
-        }
     } else if (d->type == V_ASN1_GENERALIZEDTIME) {
         end = 7;
         btz = 6;
         min_l = 15;
-        if (d->flags & ASN1_STRING_FLAG_X509_TIME) {
-            strict = 1;
-        }
     } else {
         return 0;
     }
@@ -124,7 +111,7 @@ int ossl_asn1_time_to_tm(struct tm *tm, const ASN1_TIME *d)
     if (l < min_l)
         goto err;
     for (i = 0; i < end; i++) {
-        if (!strict && (i == btz) && ((a[o] == upper_z) || (a[o] == plus) || (a[o] == minus))) {
+        if ((i == btz) && ((a[o] == upper_z) || (a[o] == plus) || (a[o] == minus))) {
             i++;
             break;
         }
@@ -190,9 +177,6 @@ int ossl_asn1_time_to_tm(struct tm *tm, const ASN1_TIME *d)
      * digits.
      */
     if (d->type == V_ASN1_GENERALIZEDTIME && a[o] == period) {
-        if (strict)
-            /* RFC 5280 forbids fractional seconds */
-            goto err;
         if (++o == l)
             goto err;
         i = o;
@@ -213,7 +197,7 @@ int ossl_asn1_time_to_tm(struct tm *tm, const ASN1_TIME *d)
      */
     if (a[o] == upper_z) {
         o++;
-    } else if (!strict && ((a[o] == plus) || (a[o] == minus))) {
+    } else if (((a[o] == plus) || (a[o] == minus))) {
         int offsign = a[o] == minus ? 1 : -1;
         int offset = 0;
 
@@ -249,7 +233,7 @@ int ossl_asn1_time_to_tm(struct tm *tm, const ASN1_TIME *d)
         if (offset && !OPENSSL_gmtime_adj(&tmp, 0, offset * offsign))
             goto err;
     } else {
-        /* not Z, or not +/- in non-strict mode */
+        /* not Z, or not +/- */
         goto err;
     }
     if (o == l) {
@@ -385,62 +369,53 @@ int ASN1_TIME_set_string_X509(ASN1_TIME *s, const char *str)
 {
     ASN1_TIME t;
     struct tm tm;
-    int rv = 0;
     size_t len;
 
-    if ((len = strlen(str)) >= INT_MAX)
-        goto out;
-    t.length = (int)len;
-    t.data = (unsigned char *)str;
-    t.flags = ASN1_STRING_FLAG_X509_TIME;
-
-    t.type = V_ASN1_UTCTIME;
-
-    if (!ASN1_TIME_check(&t)) {
+    /* RFC 5280 4.1.2.5: Valid RFC5280 times must be either length 13 or 15. */
+    len = strlen(str);
+    switch (len) {
+    case 13:
+        t.type = V_ASN1_UTCTIME;
+        break;
+    case 15:
         t.type = V_ASN1_GENERALIZEDTIME;
-        if (!ASN1_TIME_check(&t))
-            goto out;
+        break;
+    default:
+        return 0;
     }
 
+    /* RFC 5280 4.1.2.5 Valid RFC5280 times must end in 'Z'. */
+    if (str[len - 1] != 0x5A)
+        return 0;
+
+    t.length = (int)len;
+    t.data = (unsigned char *)str;
+
     /*
-     * Per RFC 5280 (section 4.1.2.5.), the valid input time
-     * strings should be encoded with the following rules:
-     *
-     * 1. UTC: YYMMDDHHMMSSZ, if YY < 50 (20YY) --> UTC: YYMMDDHHMMSSZ
-     * 2. UTC: YYMMDDHHMMSSZ, if YY >= 50 (19YY) --> UTC: YYMMDDHHMMSSZ
-     * 3. G'd: YYYYMMDDHHMMSSZ, if YYYY >= 2050 --> G'd: YYYYMMDDHHMMSSZ
-     * 4. G'd: YYYYMMDDHHMMSSZ, if YYYY < 2050 --> UTC: YYMMDDHHMMSSZ
-     *
-     * Only strings of the 4th rule should be reformatted, but since a
-     * UTC can only present [1950, 2050), so if the given time string
-     * is less than 1950 (e.g. 19230419000000Z), we do nothing...
+     * RFC 5280 Section 4.1.2.5 The following function is permissive
+     * and allows time zone offsets and time zones not Z, etc. As we
+     * have already failed and excluded anything not the correct length
+     * for the type, and anything not ending in a 'Z', Our time may
+     * not be any of these other cases, and still parse as a time.
      */
+    if (!ossl_asn1_time_to_tm(&tm, &t))
+        return 0;
 
-    if (s != NULL && t.type == V_ASN1_GENERALIZEDTIME) {
-        if (!ossl_asn1_time_to_tm(&tm, &t))
-            goto out;
-        if (is_utc(tm.tm_year)) {
-            t.length -= 2;
-            /*
-             * it's OK to let original t.data go since that's assigned
-             * to a piece of memory allocated outside of this function.
-             * new t.data would be freed after ASN1_STRING_copy is done.
-             */
-            t.data = OPENSSL_zalloc(t.length + 1);
-            if (t.data == NULL)
-                goto out;
-            memcpy(t.data, str + 2, t.length);
-            t.type = V_ASN1_UTCTIME;
-        }
-    }
+    if (s != NULL) {
+        /*
+         * Unlike every other type of nonconforming to RFC5280 time
+         * string, which causes this function to fail, a 15 character
+         * input string that ends up being in the UTC time range is not
+         * rejected. Instead, two digits of the year is removed from
+         * start of the input string so the result is a UTC time.
+         */
+        if (is_utc(tm.tm_year) && len == 15)
+            return ASN1_TIME_set_string(s, str + 2);
 
-    if (s == NULL || ASN1_STRING_copy((ASN1_STRING *)s, (ASN1_STRING *)&t))
-        rv = 1;
+        return ASN1_TIME_set_string(s, str);
+    }
 
-    if (t.data != (unsigned char *)str)
-        OPENSSL_free(t.data);
-out:
-    return rv;
+    return 1;
 }
 
 int ASN1_TIME_to_tm(const ASN1_TIME *s, struct tm *tm)

diff --git a/include/openssl/asn1.h.in b/include/openssl/asn1.h.in
index 1b7dd66d68c861285db15a3902f7be0839eb995f..08d1c27817474c297f2737553ac101f8427519e0 100644 (file)
--- a/include/openssl/asn1.h.in
+++ b/include/openssl/asn1.h.in
@@ -160,8 +160,6 @@ extern "C" {
 #define ASN1_STRING_FLAG_MSTRING 0x040
 /* String is embedded and only content should be freed */
 #define ASN1_STRING_FLAG_EMBED 0x080
-/* String should be parsed in RFC 5280's time format */
-#define ASN1_STRING_FLAG_X509_TIME 0x100
 /* This is the base type that holds just about everything :-) */
 struct asn1_string_st {
     int length;