Introduce ASN1_TIME_set_string_X509 API

Make funcs to deal with non-null-term'd string
in both asn1_generalizedtime_to_tm() and asn1_utctime_to_tm().

Fixes issue #3444.

This one is used to enforce strict format (RFC 5280) check and to
convert GeneralizedTime to UTCTime.

apps/ca has been changed to use the new API.

Test cases and documentation are updated/added

Signed-off-by: Paul Yang <paulyang.inf@gmail.com>

Reviewed-by: Kurt Roeckx <kurt@openssl.org>
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3566)

3 files changed, 141 insertions, 19 deletions

diff --git a/crypto/asn1/a_gentm.c b/crypto/asn1/a_gentm.c
index ff1b695475..bd90d05114 100644
--- a/crypto/asn1/a_gentm.c
+++ b/crypto/asn1/a_gentm.c
@@ -22,7 +22,7 @@ int asn1_generalizedtime_to_tm(struct tm *tm, const ASN1_GENERALIZEDTIME *d)
     static const int min[9] = { 0, 0, 1, 1, 0, 0, 0, 0, 0 };
     static const int max[9] = { 99, 99, 12, 31, 23, 59, 59, 12, 59 };
     char *a;
-    int n, i, l, o;
+    int n, i, l, o, min_l = 13, strict = 0;
 
     if (d->type != V_ASN1_GENERALIZEDTIME)
         return (0);
@@ -34,10 +34,26 @@ int asn1_generalizedtime_to_tm(struct tm *tm, const ASN1_GENERALIZEDTIME *d)
      * as YYYY. This stuff treats everything as a two digit field so make
      * first two fields 00 to 99
      */
-    if (l < 13)
+
+    /*
+     * 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 time zone
+     * 4. fractional seconds are not allowed in GeneralizedTime
+     */
+
+    if (d->flags & ASN1_STRING_FLAG_X509_TIME) {
+        min_l = 15;
+        strict = 1;
+    }
+
+    if (l < min_l)
         goto err;
     for (i = 0; i < 7; i++) {
-        if ((i == 6) && ((a[o] == 'Z') || (a[o] == '+') || (a[o] == '-'))) {
+        if (!strict && (i == 6) && ((a[o] == 'Z') || (a[o] == '+') || (a[o] == '-'))) {
             i++;
             if (tm)
                 tm->tm_sec = 0;
@@ -46,13 +62,15 @@ int asn1_generalizedtime_to_tm(struct tm *tm, const ASN1_GENERALIZEDTIME *d)
         if ((a[o] < '0') || (a[o] > '9'))
             goto err;
         n = a[o] - '0';
-        if (++o > l)
+        /* incomplete 2-digital number */
+        if (++o == l)
             goto err;
 
         if ((a[o] < '0') || (a[o] > '9'))
             goto err;
         n = (n * 10) + a[o] - '0';
-        if (++o > l)
+        /* no more bytes to read, but we haven't seen time-zone yet */
+        if (++o == l)
             goto err;
 
         if ((n < min[i]) || (n > max[i]))
@@ -88,22 +106,39 @@ int asn1_generalizedtime_to_tm(struct tm *tm, const ASN1_GENERALIZEDTIME *d)
      * digits.
      */
     if (a[o] == '.') {
-        if (++o > l)
+        if (strict)
+            /* RFC 5280 forbids fractional seconds */
+            goto err;
+        if (++o == l)
             goto err;
         i = o;
-        while ((a[o] >= '0') && (a[o] <= '9') && (o <= l))
+        while ((o < l) && (a[o] >= '0') && (a[o] <= '9'))
             o++;
         /* Must have at least one digit after decimal point */
         if (i == o)
             goto err;
+        /* no more bytes to read, but we haven't seen time-zone yet */
+        if (o == l)
+            goto err;
     }
 
-    if (a[o] == 'Z')
+    /*
+     * 'o' will never point to '\0' at this point, the only chance
+     * 'o' can point th '\0' is either the subsequent if or the first
+     * else if is true.
+     */
+    if (a[o] == 'Z') {
         o++;
-    else if ((a[o] == '+') || (a[o] == '-')) {
+    } else if (!strict && ((a[o] == '+') || (a[o] == '-'))) {
         int offsign = a[o] == '-' ? 1 : -1, offset = 0;
         o++;
-        if (o + 4 > l)
+        /*
+         * if not equal, no need to do subsequent checks
+         * since the following for-loop will add 'o' by 4
+         * and the final return statement will check if 'l'
+         * and 'o' are equal.
+         */
+        if (o + 4 != l)
             goto err;
         for (i = 7; i < 9; i++) {
             if ((a[o] < '0') || (a[o] > '9'))
@@ -146,6 +181,8 @@ int ASN1_GENERALIZEDTIME_set_string(ASN1_GENERALIZEDTIME *s, const char *str)
     t.type = V_ASN1_GENERALIZEDTIME;
     t.length = strlen(str);
     t.data = (unsigned char *)str;
+    t.flags = 0;
+
     if (ASN1_GENERALIZEDTIME_check(&t)) {
         if (s != NULL) {
             if (!ASN1_STRING_set((ASN1_STRING *)s, str, t.length))
diff --git a/crypto/asn1/a_time.c b/crypto/asn1/a_time.c
index 27f9bc6808..12b1ff5890 100644
--- a/crypto/asn1/a_time.c
+++ b/crypto/asn1/a_time.c
@@ -107,7 +107,6 @@ ASN1_GENERALIZEDTIME *ASN1_TIME_to_generalizedtime(const ASN1_TIME *t,
     return NULL;
 }
 
-
 int ASN1_TIME_set_string(ASN1_TIME *s, const char *str)
 {
     ASN1_TIME t;
@@ -130,6 +129,65 @@ int ASN1_TIME_set_string(ASN1_TIME *s, const char *str)
     return 1;
 }
 
+int ASN1_TIME_set_string_X509(ASN1_TIME *s, const char *str)
+{
+    ASN1_TIME t;
+    struct tm tm;
+    int rv = 0;
+
+    t.length = strlen(str);
+    t.data = (unsigned char *)str;
+    t.flags = ASN1_STRING_FLAG_X509_TIME;
+
+    t.type = V_ASN1_UTCTIME;
+
+    if (!ASN1_TIME_check(&t)) {
+        t.type = V_ASN1_GENERALIZEDTIME;
+        if (!ASN1_TIME_check(&t))
+            goto out;
+    }
+
+    /*
+     * 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...
+     */
+
+    if (s != NULL && t.type == V_ASN1_GENERALIZEDTIME) {
+        if (!asn1_generalizedtime_to_tm(&tm, &t))
+            goto out;
+        if (tm.tm_year >= 50 && tm.tm_year < 150) {
+            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 || ASN1_STRING_copy((ASN1_STRING *)s, (ASN1_STRING *)&t))
+        rv = 1;
+
+    if (t.data != (unsigned char *)str)
+        OPENSSL_free(t.data);
+out:
+    return rv;
+}
+
 int ASN1_TIME_to_tm(const ASN1_TIME *s, struct tm *tm)
 {
     if (s == NULL) {
diff --git a/crypto/asn1/a_utctm.c b/crypto/asn1/a_utctm.c
index 9797aa8a1e..ee98e4b489 100644
--- a/crypto/asn1/a_utctm.c
+++ b/crypto/asn1/a_utctm.c
@@ -18,7 +18,7 @@ int asn1_utctime_to_tm(struct tm *tm, const ASN1_UTCTIME *d)
     static const int min[8] = { 0, 1, 1, 0, 0, 0, 0, 0 };
     static const int max[8] = { 99, 12, 31, 23, 59, 59, 12, 59 };
     char *a;
-    int n, i, l, o;
+    int n, i, l, o, min_l = 11, strict = 0;
 
     if (d->type != V_ASN1_UTCTIME)
         return (0);
@@ -26,10 +26,24 @@ int asn1_utctime_to_tm(struct tm *tm, const ASN1_UTCTIME *d)
     a = (char *)d->data;
     o = 0;
 
-    if (l < 11)
+    /*
+     * 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 time zone
+     */
+
+    if (d->flags & ASN1_STRING_FLAG_X509_TIME) {
+        min_l = 13;
+        strict = 1;
+    }
+
+    if (l < min_l)
         goto err;
     for (i = 0; i < 6; i++) {
-        if ((i == 5) && ((a[o] == 'Z') || (a[o] == '+') || (a[o] == '-'))) {
+        if (!strict && (i == 5) && ((a[o] == 'Z') || (a[o] == '+') || (a[o] == '-'))) {
             i++;
             if (tm)
                 tm->tm_sec = 0;
@@ -38,13 +52,15 @@ int asn1_utctime_to_tm(struct tm *tm, const ASN1_UTCTIME *d)
         if ((a[o] < '0') || (a[o] > '9'))
             goto err;
         n = a[o] - '0';
-        if (++o > l)
+        /* incomplete 2-digital number */
+        if (++o == l)
             goto err;
 
         if ((a[o] < '0') || (a[o] > '9'))
             goto err;
         n = (n * 10) + a[o] - '0';
-        if (++o > l)
+        /* no more bytes to read, but we haven't seen time-zone yet */
+        if (++o == l)
             goto err;
 
         if ((n < min[i]) || (n > max[i]))
@@ -72,12 +88,18 @@ int asn1_utctime_to_tm(struct tm *tm, const ASN1_UTCTIME *d)
             }
         }
     }
-    if (a[o] == 'Z')
+
+    /*
+     * 'o' will never point to '\0' at this point, the only chance
+     * 'o' can point th '\0' is either the subsequent if or the first
+     * else if is true.
+     */
+    if (a[o] == 'Z') {
         o++;
-    else if ((a[o] == '+') || (a[o] == '-')) {
+    } else if (!strict && ((a[o] == '+') || (a[o] == '-'))) {
         int offsign = a[o] == '-' ? 1 : -1, offset = 0;
         o++;
-        if (o + 4 > l)
+        if (o + 4 != l)
             goto err;
         for (i = 6; i < 8; i++) {
             if ((a[o] < '0') || (a[o] > '9'))
@@ -99,6 +121,9 @@ int asn1_utctime_to_tm(struct tm *tm, const ASN1_UTCTIME *d)
         }
         if (offset && !OPENSSL_gmtime_adj(tm, 0, offset * offsign))
             return 0;
+    } else {
+        /* not Z, or not +/- in non-strict mode */
+        return 0;
     }
     return o == l;
  err:
@@ -117,6 +142,8 @@ int ASN1_UTCTIME_set_string(ASN1_UTCTIME *s, const char *str)
     t.type = V_ASN1_UTCTIME;
     t.length = strlen(str);
     t.data = (unsigned char *)str;
+    t.flags = 0;
+
     if (ASN1_UTCTIME_check(&t)) {
         if (s != NULL) {
             if (!ASN1_STRING_set((ASN1_STRING *)s, str, t.length))