Add asn1_time_to_tm function and check days in month

Based on discussion in PR #3566. Reduce duplicated code in original
asn1_utctime_to_tm and asn1_generalizedtime_to_tm, and introduce a new
internal function asn1_time_to_tm. This function also checks if the days
in the input time string is valid or not for the corresponding month.

Test cases are also added.

Reviewed-by: Andy Polyakov <appro@openssl.org>
Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/3905)

diff --git a/crypto/asn1/a_gentm.c b/crypto/asn1/a_gentm.c
index 5cfc3ff15ec8c2535e154ab917f17d457771035c..8b2b66b96428e103416262773f29e81762584134 100644 (file)
--- a/crypto/asn1/a_gentm.c
+++ b/crypto/asn1/a_gentm.c
@@ -19,155 +19,11 @@
 
 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, min_l = 13, strict = 0;
-
+    /* wrapper around asn1_time_to_tm */
     if (d->type != V_ASN1_GENERALIZEDTIME)
-        return (0);
-    l = d->length;
-    a = (char *)d->data;
-    o = 0;
-    /*
-     * GENERALIZEDTIME is similar to UTCTIME except the year is represented
-     * as YYYY. This stuff treats everything as a two digit field so make
-     * first two fields 00 to 99
-     */
-
-    /*
-     * 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 (!strict && (i == 6) && ((a[o] == 'Z') || (a[o] == '+') || (a[o] == '-'))) {
-            i++;
-            if (tm)
-                tm->tm_sec = 0;
-            break;
-        }
-        if ((a[o] < '0') || (a[o] > '9'))
-            goto err;
-        n = a[o] - '0';
-        /* incomplete 2-digital number */
-        if (++o == l)
-            goto err;
-
-        if ((a[o] < '0') || (a[o] > '9'))
-            goto err;
-        n = (n * 10) + a[o] - '0';
-        /* 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]))
-            goto err;
-        if (tm) {
-            switch (i) {
-            case 0:
-                tm->tm_year = n * 100 - 1900;
-                break;
-            case 1:
-                tm->tm_year += n;
-                break;
-            case 2:
-                tm->tm_mon = n - 1;
-                break;
-            case 3:
-                tm->tm_mday = n;
-                break;
-            case 4:
-                tm->tm_hour = n;
-                break;
-            case 5:
-                tm->tm_min = n;
-                break;
-            case 6:
-                tm->tm_sec = n;
-                break;
-            }
-        }
-    }
-    /*
-     * Optional fractional seconds: decimal point followed by one or more
-     * digits.
-     */
-    if (a[o] == '.') {
-        if (strict)
-            /* RFC 5280 forbids fractional seconds */
-            goto err;
-        if (++o == l)
-            goto err;
-        i = o;
-        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;
-    }
-
-    /*
-     * '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 (!strict && ((a[o] == '+') || (a[o] == '-'))) {
-        int offsign = a[o] == '-' ? 1 : -1, offset = 0;
-        o++;
-        /*
-         * 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'))
-                goto err;
-            n = a[o] - '0';
-            o++;
-            if ((a[o] < '0') || (a[o] > '9'))
-                goto err;
-            n = (n * 10) + a[o] - '0';
-            if ((n < min[i]) || (n > max[i]))
-                goto err;
-            if (tm) {
-                if (i == 7)
-                    offset = n * 3600;
-                else if (i == 8)
-                    offset += n * 60;
-            }
-            o++;
-        }
-        if (offset && !OPENSSL_gmtime_adj(tm, 0, offset * offsign))
-            return 0;
-    } else if (a[o]) {
-        /* Missing time zone information. */
-        goto err;
-    }
-    return (o == l);
- err:
-    return (0);
-}
+        return 0;
+    return asn1_time_to_tm(tm, d);
+ }
 
 int ASN1_GENERALIZEDTIME_check(const ASN1_GENERALIZEDTIME *d)
 {

diff --git a/crypto/asn1/a_time.c b/crypto/asn1/a_time.c
index fc78e309a679be02668235ebb58dc4eaf9ccc79d..6e3fade2ee6bf84bcd332b9491367542a3f73fa4 100644 (file)
--- a/crypto/asn1/a_time.c
+++ b/crypto/asn1/a_time.c
@@ -24,6 +24,199 @@ IMPLEMENT_ASN1_MSTRING(ASN1_TIME, B_ASN1_TIME)
 
 IMPLEMENT_ASN1_FUNCTIONS(ASN1_TIME)
 
+static int leap_year(const int year)
+{
+    if (year % 400 == 0 || (year % 100 != 0 && year % 4 == 0))
+        return 1;
+    return 0;
+}
+
+int asn1_time_to_tm(struct tm *tm, const ASN1_TIME *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 };
+    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 = 11, strict = 0, end = 6, btz = 5, md;
+    struct tm tmp;
+
+    /*
+     * 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->type == V_ASN1_UTCTIME) {
+        if (d->flags & ASN1_STRING_FLAG_X509_TIME) {
+            min_l = 13;
+            strict = 1;
+        }
+    } else if (d->type == V_ASN1_GENERALIZEDTIME) {
+        end = 7;
+        btz = 6;
+        if (d->flags & ASN1_STRING_FLAG_X509_TIME) {
+            min_l = 15;
+            strict = 1;
+        } else {
+            min_l = 13;
+        }
+    } else {
+        return 0;
+    }
+
+    l = d->length;
+    a = (char *)d->data;
+    o = 0;
+    memset(&tmp, 0, sizeof(tmp));
+
+    /*
+     * GENERALIZEDTIME is similar to UTCTIME except the year is represented
+     * as YYYY. This stuff treats everything as a two digit field so make
+     * first two fields 00 to 99
+     */
+
+    if (l < min_l)
+        goto err;
+    for (i = 0; i < end; i++) {
+        if (!strict && (i == btz) && ((a[o] == 'Z') || (a[o] == '+') || (a[o] == '-'))) {
+            i++;
+            break;
+        }
+        if ((a[o] < '0') || (a[o] > '9'))
+            goto err;
+        n = a[o] - '0';
+        /* incomplete 2-digital number */
+        if (++o == l)
+            goto err;
+
+        if ((a[o] < '0') || (a[o] > '9'))
+            goto err;
+        n = (n * 10) + a[o] - '0';
+        /* no more bytes to read, but we haven't seen time-zone yet */
+        if (++o == l)
+            goto err;
+
+        i2 = (d->type == V_ASN1_UTCTIME) ? i + 1 : i;
+
+        if ((n < min[i2]) || (n > max[i2]))
+            goto err;
+        switch (i2) {
+        case 0:
+            /* UTC will never be here */
+            tmp.tm_year = n * 100 - 1900;
+            break;
+        case 1:
+            if (d->type == V_ASN1_UTCTIME)
+                tmp.tm_year = n < 50 ? n + 100 : n;
+            else
+                tmp.tm_year += n;
+            break;
+        case 2:
+            tmp.tm_mon = n - 1;
+            break;
+        case 3:
+            /* check if tm_mday is valid in tm_mon */
+            if (tmp.tm_mon == 1) {
+                /* it's February */
+                md = mdays[1] + leap_year(tmp.tm_year + 1900);
+            } else {
+                md = mdays[tmp.tm_mon];
+            }
+            if (n > md)
+                goto err;
+            tmp.tm_mday = n;
+            break;
+        case 4:
+            tmp.tm_hour = n;
+            break;
+        case 5:
+            tmp.tm_min = n;
+            break;
+        case 6:
+            tmp.tm_sec = n;
+            break;
+        }
+    }
+
+    /*
+     * Optional fractional seconds: decimal point followed by one or more
+     * digits.
+     */
+    if (d->type == V_ASN1_GENERALIZEDTIME && a[o] == '.') {
+        if (strict)
+            /* RFC 5280 forbids fractional seconds */
+            goto err;
+        if (++o == l)
+            goto err;
+        i = o;
+        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;
+    }
+
+    /*
+     * 'o' will never point to '\0' at this point, the only chance
+     * 'o' can point to '\0' is either the subsequent if or the first
+     * else if is true.
+     */
+    if (a[o] == 'Z') {
+        o++;
+    } else if (!strict && ((a[o] == '+') || (a[o] == '-'))) {
+        int offsign = a[o] == '-' ? 1 : -1;
+        int offset = 0;
+
+        o++;
+        /*
+         * 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 = end; i < end + 2; i++) {
+            if ((a[o] < '0') || (a[o] > '9'))
+                goto err;
+            n = a[o] - '0';
+            o++;
+            if ((a[o] < '0') || (a[o] > '9'))
+                goto err;
+            n = (n * 10) + a[o] - '0';
+            i2 = (d->type == V_ASN1_UTCTIME) ? i + 1 : i;
+            if ((n < min[i2]) || (n > max[i2]))
+                goto err;
+            /* if tm is NULL, no need to adjust */
+            if (tm != NULL) {
+                if (i == end)
+                    offset = n * 3600;
+                else if (i == end + 1)
+                    offset += n * 60;
+            }
+            o++;
+        }
+        if (offset && !OPENSSL_gmtime_adj(&tmp, 0, offset * offsign))
+            goto err;
+    } else {
+        /* not Z, or not +/- in non-strict mode */
+        goto err;
+    }
+    if (o == l) {
+        /* success, check if tm should be filled */
+        if (tm != NULL)
+            *tm = tmp;
+        return 1;
+    }
+ err:
+    return 0;
+}
+
 ASN1_TIME *ASN1_TIME_set(ASN1_TIME *s, time_t t)
 {
     return ASN1_TIME_adj(s, t, 0, 0);
@@ -162,7 +355,7 @@ int ASN1_TIME_set_string_X509(ASN1_TIME *s, const char *str)
      */
 
     if (s != NULL && t.type == V_ASN1_GENERALIZEDTIME) {
-        if (!asn1_generalizedtime_to_tm(&tm, &t))
+        if (!asn1_time_to_tm(&tm, &t))
             goto out;
         if (tm.tm_year >= 50 && tm.tm_year < 150) {
             t.length -= 2;
@@ -200,16 +393,7 @@ int ASN1_TIME_to_tm(const ASN1_TIME *s, struct tm *tm)
         return 0;
     }
 
-    if (s->type == V_ASN1_UTCTIME) {
-        memset(tm, 0, sizeof(*tm));
-        return asn1_utctime_to_tm(tm, s);
-    }
-    if (s->type == V_ASN1_GENERALIZEDTIME) {
-        memset(tm, 0, sizeof(*tm));
-        return asn1_generalizedtime_to_tm(tm, s);
-    }
-
-    return 0;
+    return asn1_time_to_tm(tm, s);
 }
 
 int ASN1_TIME_diff(int *pday, int *psec,

diff --git a/crypto/asn1/a_utctm.c b/crypto/asn1/a_utctm.c
index 5a4b1742f7391bd4108db49a1569685fbc60e860..2a864180e3e9a12525669602acbdad23f290cfd2 100644 (file)
--- a/crypto/asn1/a_utctm.c
+++ b/crypto/asn1/a_utctm.c
@@ -15,119 +15,10 @@
 
 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, min_l = 11, strict = 0;
-
+    /* wrapper around ans1_time_to_tm */
     if (d->type != V_ASN1_UTCTIME)
         return 0;
-    l = d->length;
-    a = (char *)d->data;
-    o = 0;
-
-    /*
-     * 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 (!strict && (i == 5) && ((a[o] == 'Z') || (a[o] == '+') || (a[o] == '-'))) {
-            i++;
-            if (tm)
-                tm->tm_sec = 0;
-            break;
-        }
-        if ((a[o] < '0') || (a[o] > '9'))
-            goto err;
-        n = a[o] - '0';
-        /* incomplete 2-digital number */
-        if (++o == l)
-            goto err;
-
-        if ((a[o] < '0') || (a[o] > '9'))
-            goto err;
-        n = (n * 10) + a[o] - '0';
-        /* 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]))
-            goto err;
-        if (tm) {
-            switch (i) {
-            case 0:
-                tm->tm_year = n < 50 ? n + 100 : n;
-                break;
-            case 1:
-                tm->tm_mon = n - 1;
-                break;
-            case 2:
-                tm->tm_mday = n;
-                break;
-            case 3:
-                tm->tm_hour = n;
-                break;
-            case 4:
-                tm->tm_min = n;
-                break;
-            case 5:
-                tm->tm_sec = n;
-                break;
-            }
-        }
-    }
-
-    /*
-     * '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 (!strict && ((a[o] == '+') || (a[o] == '-'))) {
-        int offsign = a[o] == '-' ? 1 : -1, offset = 0;
-        o++;
-        if (o + 4 != l)
-            goto err;
-        for (i = 6; i < 8; i++) {
-            if ((a[o] < '0') || (a[o] > '9'))
-                goto err;
-            n = a[o] - '0';
-            o++;
-            if ((a[o] < '0') || (a[o] > '9'))
-                goto err;
-            n = (n * 10) + a[o] - '0';
-            if ((n < min[i]) || (n > max[i]))
-                goto err;
-            if (tm) {
-                if (i == 6)
-                    offset = n * 3600;
-                else if (i == 7)
-                    offset += n * 60;
-            }
-            o++;
-        }
-        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:
-    return 0;
+    return asn1_time_to_tm(tm, d);
 }
 
 int ASN1_UTCTIME_check(const ASN1_UTCTIME *d)

diff --git a/crypto/asn1/asn1_locl.h b/crypto/asn1/asn1_locl.h
index 9470c7d0bf4cae5893d08b30a21b270682a88f93..bf095eaa9f4140563f74aa562d92dafa0be0ac21 100644 (file)
--- a/crypto/asn1/asn1_locl.h
+++ b/crypto/asn1/asn1_locl.h
@@ -9,6 +9,7 @@
 
 /* Internal ASN1 structures and functions: not for application use */
 
+int asn1_time_to_tm(struct tm *tm, const ASN1_TIME *d);
 int asn1_utctime_to_tm(struct tm *tm, const ASN1_UTCTIME *d);
 int asn1_generalizedtime_to_tm(struct tm *tm, const ASN1_GENERALIZEDTIME *d);
 

diff --git a/doc/man3/ASN1_TIME_set.pod b/doc/man3/ASN1_TIME_set.pod
index 5f041a575c926979348cd10c15d9d5eeaca30f0c..379f28a4aff8c8672aa540893281f90c9329ba3d 100644 (file)
--- a/doc/man3/ASN1_TIME_set.pod
+++ b/doc/man3/ASN1_TIME_set.pod
@@ -55,8 +55,10 @@ an error.
 
 ASN1_TIME_to_tm() converts the time B<s> to the standard B<tm> structure.
 If B<s> is NULL, then the current time is converted. The output time is GMT.
-Only the B<tm_sec>, B<tm_min>, B<tm_hour>, B<tm_mday>, B<tm_mon> and B<tm_year>
-fields are updated.
+The B<tm_sec>, B<tm_min>, B<tm_hour>, B<tm_mday>, B<tm_mon> and B<tm_year>
+fields of B<tm> structure are set to proper values, whereas all other fields
+are set to 0. If B<tm> is NULL this function performs a format check on B<s>
+only.
 
 ASN1_TIME_diff() sets B<*pday> and B<*psec> to the time difference between
 B<from> and B<to>. If B<to> represents a time later than B<from> then

diff --git a/test/x509_time_test.c b/test/x509_time_test.c
index 6812805847f93d01eb3d3fa76aca20ff0ad6fab2..21f6980a49a19ec6f6ff6934771ce77775f4c1af 100644 (file)
--- a/test/x509_time_test.c
+++ b/test/x509_time_test.c
@@ -55,6 +55,14 @@ static TESTDATA_FORMAT x509_format_tests[] = {
         /* good format, check only */
         "20170217180105Z", 0, 1, -1, NULL,
     },
+    {
+        /* not leap year, check only */
+        "20170229180105Z", 0, 0, -1, NULL,
+    },
+    {
+        /* leap year, check only */
+        "20160229180105Z", 0, 1, -1, NULL,
+    },
     {
         /* SS is missing, check only */
         "201702171801Z", 0, 0, -1, NULL,
@@ -96,6 +104,14 @@ static TESTDATA_FORMAT x509_format_tests[] = {
         /* SS is missing, check only */
         "1702171801Z", 0, 0, -1, NULL,
     },
+    {
+        /* not leap year, check only */
+        "050229180101Z", 0, 0, -1, NULL,
+    },
+    {
+        /* leap year, check only */
+        "040229180101Z", 0, 1, -1, NULL,
+    },
     {
         /* time zone, check only */
         "170217180154+0800", 0, 0, -1, NULL,