Add appropriate lower bound checks for GeneralizedTime and UTCTime

ITU-T X.690 / ISO/IEC 8825-1 section 11.7 and section 11.8 impose specific constraints on how GeneralizedTime and UTCTime can be encoded in BER/CER/DER. Following from these constraints a minimum length can be derived. Checking the length in this context can potentially help prevent applications from interpreting an invalid GeneralizedTime as a valid UTCTime. Reviewed-by: Bernd Edlinger <[email protected]> Reviewed-by: Tomas Mraz <[email protected]> (Merged from openssl#23483)
jspricke · Feb 25, 2024 · eadd8c4 · eadd8c4
1 parent 5d70f11
commit eadd8c4
Show file tree

Hide file tree

Showing 9 changed files with 86 additions and 11 deletions.
diff --git a/CHANGES.md b/CHANGES.md
@@ -34,6 +34,12 @@ OpenSSL 3.3
 
    *Ijtaba Hussain*
 
+ * The d2i_ASN1_GENERALIZEDTIME(), d2i_ASN1_UTCTIME(), ASN1_TIME_check(), and
+   related functions have been augmented to check for a minimum length of
+   the input string, in accordance with ITU-T X.690 section 11.7 and 11.8.
+
+   *Job Snijders*
+
  * The EVP_PKEY_fromdata function has been augmented to allow for the derivation
    of CRT (Chinese Remainder Theorem) parameters when requested.  See the
    OSSL_PKEY_PARAM_RSA_DERIVE_FROM_PQ param in the EVP_PKEY-RSA documentation.

diff --git a/crypto/asn1/a_time.c b/crypto/asn1/a_time.c
@@ -79,7 +79,7 @@ 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 = 11, strict = 0, end = 6, btz = 5, md;
+    int n, i, i2, l, o, min_l, strict = 0, 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;
@@ -95,18 +95,16 @@ int ossl_asn1_time_to_tm(struct tm *tm, const ASN1_TIME *d)
      * 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) {
-            min_l = 13;
             strict = 1;
         }
     } else if (d->type == V_ASN1_GENERALIZEDTIME) {
         end = 7;
         btz = 6;
+        min_l = 15;
         if (d->flags & ASN1_STRING_FLAG_X509_TIME) {
-            min_l = 15;
             strict = 1;
-        } else {
-            min_l = 13;
         }
     } else {
         return 0;

diff --git a/crypto/asn1/asn1_err.c b/crypto/asn1/asn1_err.c
@@ -55,6 +55,8 @@ static const ERR_STRING_DATA ASN1_str_reasons[] = {
     {ERR_PACK(ERR_LIB_ASN1, 0, ASN1_R_FIELD_MISSING), "field missing"},
     {ERR_PACK(ERR_LIB_ASN1, 0, ASN1_R_FIRST_NUM_TOO_LARGE),
     "first num too large"},
+    {ERR_PACK(ERR_LIB_ASN1, 0, ASN1_R_GENERALIZEDTIME_IS_TOO_SHORT),
+    "generalizedtime is too short"},
     {ERR_PACK(ERR_LIB_ASN1, 0, ASN1_R_HEADER_TOO_LONG), "header too long"},
     {ERR_PACK(ERR_LIB_ASN1, 0, ASN1_R_ILLEGAL_BITSTRING_FORMAT),
     "illegal bitstring format"},
@@ -192,6 +194,8 @@ static const ERR_STRING_DATA ASN1_str_reasons[] = {
     {ERR_PACK(ERR_LIB_ASN1, 0, ASN1_R_UNSUPPORTED_PUBLIC_KEY_TYPE),
     "unsupported public key type"},
     {ERR_PACK(ERR_LIB_ASN1, 0, ASN1_R_UNSUPPORTED_TYPE), "unsupported type"},
+    {ERR_PACK(ERR_LIB_ASN1, 0, ASN1_R_UTCTIME_IS_TOO_SHORT),
+    "utctime is too short"},
     {ERR_PACK(ERR_LIB_ASN1, 0, ASN1_R_WRONG_INTEGER_TYPE),
     "wrong integer type"},
     {ERR_PACK(ERR_LIB_ASN1, 0, ASN1_R_WRONG_PUBLIC_KEY_TYPE),

diff --git a/crypto/asn1/tasn_dec.c b/crypto/asn1/tasn_dec.c
@@ -921,6 +921,14 @@ static int asn1_ex_c2i(ASN1_VALUE **pval, const unsigned char *cont, int len,
             ERR_raise(ERR_LIB_ASN1, ASN1_R_UNIVERSALSTRING_IS_WRONG_LENGTH);
             goto err;
         }
+        if (utype == V_ASN1_GENERALIZEDTIME && (len < 15)) {
+            ERR_raise(ERR_LIB_ASN1, ASN1_R_GENERALIZEDTIME_IS_TOO_SHORT);
+            goto err;
+        }
+        if (utype == V_ASN1_UTCTIME && (len < 13)) {
+            ERR_raise(ERR_LIB_ASN1, ASN1_R_UTCTIME_IS_TOO_SHORT);
+            goto err;
+        }
         /* All based on ASN1_STRING and handled the same */
         if (*pval == NULL) {
             stmp = ASN1_STRING_type_new(utype);

diff --git a/crypto/err/openssl.txt b/crypto/err/openssl.txt
@@ -32,6 +32,7 @@ ASN1_R_EXPLICIT_LENGTH_MISMATCH:119:explicit length mismatch
 ASN1_R_EXPLICIT_TAG_NOT_CONSTRUCTED:120:explicit tag not constructed
 ASN1_R_FIELD_MISSING:121:field missing
 ASN1_R_FIRST_NUM_TOO_LARGE:122:first num too large
+ASN1_R_GENERALIZEDTIME_IS_TOO_SHORT:232:generalizedtime is too short
 ASN1_R_HEADER_TOO_LONG:123:header too long
 ASN1_R_ILLEGAL_BITSTRING_FORMAT:175:illegal bitstring format
 ASN1_R_ILLEGAL_BOOLEAN:176:illegal boolean
@@ -119,6 +120,7 @@ ASN1_R_UNSUPPORTED_ANY_DEFINED_BY_TYPE:164:unsupported any defined by type
 ASN1_R_UNSUPPORTED_CIPHER:228:unsupported cipher
 ASN1_R_UNSUPPORTED_PUBLIC_KEY_TYPE:167:unsupported public key type
 ASN1_R_UNSUPPORTED_TYPE:196:unsupported type
+ASN1_R_UTCTIME_IS_TOO_SHORT:233:utctime is too short
 ASN1_R_WRONG_INTEGER_TYPE:225:wrong integer type
 ASN1_R_WRONG_PUBLIC_KEY_TYPE:200:wrong public key type
 ASN1_R_WRONG_TAG:168:wrong tag

diff --git a/include/openssl/asn1err.h b/include/openssl/asn1err.h
@@ -47,6 +47,7 @@
 # define ASN1_R_EXPLICIT_TAG_NOT_CONSTRUCTED              120
 # define ASN1_R_FIELD_MISSING                             121
 # define ASN1_R_FIRST_NUM_TOO_LARGE                       122
+# define ASN1_R_GENERALIZEDTIME_IS_TOO_SHORT              232
 # define ASN1_R_HEADER_TOO_LONG                           123
 # define ASN1_R_ILLEGAL_BITSTRING_FORMAT                  175
 # define ASN1_R_ILLEGAL_BOOLEAN                           176
@@ -133,6 +134,7 @@
 # define ASN1_R_UNSUPPORTED_CIPHER                        228
 # define ASN1_R_UNSUPPORTED_PUBLIC_KEY_TYPE               167
 # define ASN1_R_UNSUPPORTED_TYPE                          196
+# define ASN1_R_UTCTIME_IS_TOO_SHORT                      233
 # define ASN1_R_WRONG_INTEGER_TYPE                        225
 # define ASN1_R_WRONG_PUBLIC_KEY_TYPE                     200
 # define ASN1_R_WRONG_TAG                                 168

diff --git a/test/asn1_decode_test.c b/test/asn1_decode_test.c
@@ -11,6 +11,7 @@
 #include <string.h>
 
 #include <openssl/rand.h>
+#include <openssl/asn1.h>
 #include <openssl/asn1t.h>
 #include <openssl/obj_mac.h>
 #include "internal/numbers.h"
@@ -161,6 +162,56 @@ static int test_uint64(void)
     return 1;
 }
 
+/* GeneralizedTime underflow *********************************************** */
+
+static int test_gentime(void)
+{
+    /* Underflowing GeneralizedTime 161208193400Z (YYMMDDHHMMSSZ) */
+    const unsigned char der[] = {
+        0x18, 0x0d, 0x31, 0x36, 0x31, 0x32, 0x30, 0x38, 0x31,
+        0x39, 0x33, 0x34, 0x30, 0x30, 0x5a,
+    };
+    const unsigned char *p;
+    int der_len, rc = 1;
+    ASN1_GENERALIZEDTIME *gentime;
+
+    p = der;
+    der_len = sizeof(der);
+    gentime = d2i_ASN1_GENERALIZEDTIME(NULL, &p, der_len);
+
+    if (!TEST_ptr_null(gentime))
+        rc = 0; /* fail */
+
+    ASN1_GENERALIZEDTIME_free(gentime);
+    return rc;
+}
+
+/* UTCTime underflow ******************************************************* */
+
+static int test_utctime(void)
+{
+    /* Underflowing UTCTime 0205104700Z (MMDDHHMMSSZ) */
+    const unsigned char der[] = {
+        0x17, 0x0b, 0x30, 0x32, 0x30, 0x35, 0x31, 0x30,
+        0x34, 0x37, 0x30, 0x30, 0x5a,
+    };
+    const unsigned char *p;
+    int der_len, rc = 1;
+    ASN1_UTCTIME *utctime;
+
+    p = der;
+    der_len = sizeof(der);
+    utctime = d2i_ASN1_UTCTIME(NULL, &p, der_len);
+
+    if (!TEST_ptr_null(utctime))
+        rc = 0; /* fail */
+
+    ASN1_UTCTIME_free(utctime);
+    return rc;
+}
+
+/* Invalid template ******************************************************** */
+
 typedef struct {
     ASN1_STRING *invalidDirString;
 } INVALIDTEMPLATE;
@@ -229,6 +280,8 @@ int setup_tests(void)
     ADD_TEST(test_uint32);
     ADD_TEST(test_int64);
     ADD_TEST(test_uint64);
+    ADD_TEST(test_gentime);
+    ADD_TEST(test_utctime);
     ADD_TEST(test_invalid_template);
     ADD_TEST(test_reuse_asn1_object);
     return 1;

diff --git a/test/asn1_time_test.c b/test/asn1_time_test.c
@@ -101,6 +101,10 @@ static struct testdata tbl_testdata_pos[] = {
     { "1970010100000AZ",   V_ASN1_GENERALIZEDTIME, V_ASN1_GENERALIZEDTIME, 0,           0,  0, 0, },
     { "700101000000X",     V_ASN1_UTCTIME,         V_ASN1_UTCTIME,         0,           0,  0, 0, },
     { "19700101000000X",   V_ASN1_GENERALIZEDTIME, V_ASN1_GENERALIZEDTIME, 0,           0,  0, 0, },
+    { "209912312359Z",     V_ASN1_GENERALIZEDTIME, V_ASN1_GENERALIZEDTIME, 0,           0,  0, 0, },
+    { "199912310000Z",     V_ASN1_GENERALIZEDTIME, V_ASN1_GENERALIZEDTIME, 0,           0,  0, 0, },
+    { "9912312359Z",       V_ASN1_UTCTIME,         V_ASN1_UTCTIME,         0,           0,  0, 0, },
+    { "9912310000Z",       V_ASN1_UTCTIME,         V_ASN1_UTCTIME,         0,           0,  0, 0, },
     { "19700101000000Z",   V_ASN1_GENERALIZEDTIME, V_ASN1_UTCTIME,         1,           0, -1, 1, }, /* Epoch begins */
     { "700101000000Z",     V_ASN1_UTCTIME,         V_ASN1_UTCTIME,         1,           0, -1, 1, }, /* ditto */
     { "20380119031407Z",   V_ASN1_GENERALIZEDTIME, V_ASN1_UTCTIME,         1,  0x7FFFFFFF,  1, 1, }, /* Max 32bit time_t */
@@ -111,9 +115,7 @@ static struct testdata tbl_testdata_pos[] = {
     { "19701006121456Z",   V_ASN1_GENERALIZEDTIME, V_ASN1_UTCTIME,         1,    24063296, -1, 1, },
     { "701006121456Z",     V_ASN1_UTCTIME,         V_ASN1_UTCTIME,         1,    24063296, -1, 1, },
     { "19991231000000Z",   V_ASN1_GENERALIZEDTIME, V_ASN1_UTCTIME,         1,   946598400,  0, 1, }, /* Match baseline */
-    { "199912310000Z",     V_ASN1_GENERALIZEDTIME, V_ASN1_UTCTIME,         1,   946598400,  0, 1, }, /* In various flavors */
     { "991231000000Z",     V_ASN1_UTCTIME,         V_ASN1_UTCTIME,         1,   946598400,  0, 1, },
-    { "9912310000Z",       V_ASN1_UTCTIME,         V_ASN1_UTCTIME,         1,   946598400,  0, 1, },
     { "9912310000+0000",   V_ASN1_UTCTIME,         V_ASN1_UTCTIME,         1,   946598400,  0, 1, },
     { "199912310000+0000", V_ASN1_GENERALIZEDTIME, V_ASN1_UTCTIME,         1,   946598400,  0, 1, },
     { "9912310000-0000",   V_ASN1_UTCTIME,         V_ASN1_UTCTIME,         1,   946598400,  0, 1, },

diff --git a/test/x509_time_test.c b/test/x509_time_test.c
@@ -490,7 +490,7 @@ static const struct {
             "Jul 31 22:20:50 2017 GMT"),
     /* Generalized Time, no seconds */
     construct_asn1_time("201707312220Z", V_ASN1_GENERALIZEDTIME,
-            "Jul 31 22:20:00 2017 GMT"),
+            "Bad time value"),
     /* Generalized Time, fractional seconds (3 digits) */
     construct_asn1_time("20170731222050.123Z", V_ASN1_GENERALIZEDTIME,
             "Jul 31 22:20:50.123 2017 GMT"),
@@ -505,7 +505,7 @@ static const struct {
             "Jul 31 22:20:50 2017 GMT"),
     /* UTC Time, no seconds */
     construct_asn1_time("1707312220Z", V_ASN1_UTCTIME,
-            "Jul 31 22:20:00 2017 GMT"),
+            "Bad time value"),
 };
 
 static const struct {
@@ -517,7 +517,7 @@ static const struct {
             "2017-07-31 22:20:50Z"),
     /* Generalized Time, no seconds */
     construct_asn1_time("201707312220Z", V_ASN1_GENERALIZEDTIME,
-            "2017-07-31 22:20:00Z"),
+            "Bad time value"),
     /* Generalized Time, fractional seconds (3 digits) */
     construct_asn1_time("20170731222050.123Z", V_ASN1_GENERALIZEDTIME,
             "2017-07-31 22:20:50.123Z"),
@@ -532,7 +532,7 @@ static const struct {
             "2017-07-31 22:20:50Z"),
     /* UTC Time, no seconds */
     construct_asn1_time("1707312220Z", V_ASN1_UTCTIME,
-            "2017-07-31 22:20:00Z"),
+            "Bad time value"),
 };
 
 static int test_x509_time_print_rfc_822(int idx)