Fix: UTCTime interpreted as GeneralizedTime

Example:
"200305100350Z" interpreted as "2003/05/10 03:50:00 UTC"
correct is "2020/03/05 10:03:50 UTC"

According to RFC 5280:
UTCTime values ... MUST include seconds (i.e., times are YYMMDDHHMMSSZ) -> length 13
GeneralizedTime values ...  MUST include seconds (i.e., times are YYYYMMDDHHMMSSZ) -> length 15

I think we should enforce the RFC5280 rules even if the ASN.1 rules are not that strict.

2 files changed, 97 insertions, 24 deletions

diff --git a/src/lib/asn1/asn1_time.cpp b/src/lib/asn1/asn1_time.cpp
index ef259740b..5890ac7ca 100644
--- a/src/lib/asn1/asn1_time.cpp
+++ b/src/lib/asn1/asn1_time.cpp
@@ -171,12 +171,12 @@ void X509_Time::set_to(const std::string& t_spec, ASN1_Tag spec_tag)
 
    if(spec_tag == GENERALIZED_TIME)
       {
-      if(t_spec.size() != 13 && t_spec.size() != 15)
+      if(t_spec.size() != 15)
          throw Invalid_Argument("Invalid GeneralizedTime string: '" + t_spec + "'");
       }
    else if(spec_tag == UTC_TIME)
       {
-      if(t_spec.size() != 11 && t_spec.size() != 13)
+      if(t_spec.size() != 13)
          throw Invalid_Argument("Invalid UTCTime string: '" + t_spec + "'");
       }
 
diff --git a/src/tests/unit_x509.cpp b/src/tests/unit_x509.cpp
index b51914ee8..e23017738 100644
--- a/src/tests/unit_x509.cpp
+++ b/src/tests/unit_x509.cpp
@@ -57,8 +57,8 @@ Botan::X509_Cert_Options req_opts1(const std::string& algo)
    opts.dns = "botan.randombit.net";
    opts.email = "[email protected]";
 
-   opts.not_before("1601012000Z");
-   opts.not_after("3001012000Z");
+   opts.not_before("160101200000Z");
+   opts.not_after("300101200000Z");
 
    if(algo == "RSA")
       {
@@ -170,23 +170,21 @@ Test::Result test_x509_dates()
 
    Botan::X509_Time time;
    result.confirm("unset time not set", !time.time_is_set());
-   time = Botan::X509_Time("0802011822Z", Botan::ASN1_Tag::UTC_TIME);
+   time = Botan::X509_Time("080201182200Z", Botan::ASN1_Tag::UTC_TIME);
    result.confirm("time set after construction", time.time_is_set());
    result.test_eq("time readable_string", time.readable_string(), "2008/02/01 18:22:00 UTC");
 
-   const std::vector<std::string> valid = {
-      "0802010000Z",
-      "0802011724Z",
-      "0406142334Z",
-      "9906142334Z",
-      "0006142334Z",
+   time = Botan::X509_Time("200305100350Z", Botan::ASN1_Tag::UTC_TIME);
+   result.test_eq("UTC_TIME readable_string", time.readable_string(), "2020/03/05 10:03:50 UTC");
 
-      "080201000000Z",
-      "080201172412Z",
-      "040614233433Z",
-      "990614233444Z",
-      "000614233455Z",
-   };
+   time = Botan::X509_Time("200305100350Z", Botan::ASN1_Tag::UTC_OR_GENERALIZED_TIME);
+   result.test_eq("UTC_OR_GENERALIZED_TIME from UTC_TIME readable_string", time.readable_string(), "2020/03/05 10:03:50 UTC");
+
+   time = Botan::X509_Time("20200305100350Z", Botan::ASN1_Tag::UTC_OR_GENERALIZED_TIME);
+   result.test_eq("UTC_OR_GENERALIZED_TIME from GENERALIZED_TIME readable_string", time.readable_string(), "2020/03/05 10:03:50 UTC");
+
+   time = Botan::X509_Time("20200305100350Z", Botan::ASN1_Tag::GENERALIZED_TIME);
+   result.test_eq("GENERALIZED_TIME readable_string", time.readable_string(), "2020/03/05 10:03:50 UTC");
 
    // Dates that are valid per X.500 but rejected as unsupported
    const std::vector<std::string> valid_but_unsup = {
@@ -205,7 +203,16 @@ Test::Result test_x509_dates()
       "000614233455+0530",
    };
 
-   const std::vector<std::string> invalid = {
+   // valid length 13
+   const std::vector<std::string> valid_utc = {
+      "080201000000Z",
+      "080201172412Z",
+      "040614233433Z",
+      "990614233444Z",
+      "000614233455Z",
+   };
+
+   const std::vector<std::string> invalid_utc = {
       "",
       " ",
       "2008`02-01",
@@ -213,6 +220,13 @@ Test::Result test_x509_dates()
       "2000-02-01 17",
       "999921",
 
+      // No seconds
+      "0802010000Z",
+      "0802011724Z",
+      "0406142334Z",
+      "9906142334Z",
+      "0006142334Z",
+
       // valid length 13 -> range check
       "080201000061Z", // seconds too big (61)
       "080201000060Z", // seconds too big (60, leap seconds not covered by the standard)
@@ -247,25 +261,84 @@ Test::Result test_x509_dates()
       "2\n2211221122Z",
 
       // wrong time zone
-      "0802010000",
-      "0802010000z"
+      "080201000000",
+      "080201000000z",
+
+      // Fractional seconds
+      "170217180154.001Z",
+
+      // Timezone offset
+      "170217180154+0100",
+
+      // Extra digits
+      "17021718015400Z",
+
+      // Non-digits
+      "17021718015aZ",
+
+      // Trailing garbage
+      "170217180154Zlongtrailinggarbage",
+
+      // Swapped type
+      "20170217180154Z",
+   };
+
+   // valid length 15
+   const std::vector<std::string> valid_generalized_time = {
+      "20000305100350Z",
+   };
+
+   const std::vector<std::string> invalid_generalized = {
+      // No trailing Z
+      "20000305100350",
+
+      // No seconds
+      "200003051003Z",
+
+      // Fractional seconds
+      "20000305100350.001Z",
+
+      // Timezone offset
+      "20170217180154+0100",
+
+      // Extra digits
+      "2017021718015400Z",
+
+      // Non-digits
+      "2017021718015aZ",
+
+      // Trailing garbage
+      "20170217180154Zlongtrailinggarbage",
+
+      // Swapped type
+      "170217180154Z",
    };
 
-   for(auto&& v : valid)
+   for(auto&& v : valid_but_unsup)
+      {
+      result.test_throws("valid but unsupported", [v]() { Botan::X509_Time t(v, Botan::ASN1_Tag::UTC_TIME); });
+      }
+
+   for(auto&& v : valid_utc)
       {
       Botan::X509_Time t(v, Botan::ASN1_Tag::UTC_TIME);
       }
 
-   for(auto&& v : valid_but_unsup)
+   for(auto&& v : valid_generalized_time)
       {
-      result.test_throws("valid but unsupported", [v]() { Botan::X509_Time t(v, Botan::ASN1_Tag::UTC_TIME); });
+      Botan::X509_Time t(v, Botan::ASN1_Tag::GENERALIZED_TIME);
       }
 
-   for(auto&& v : invalid)
+   for(auto&& v : invalid_utc)
       {
       result.test_throws("invalid", [v]() { Botan::X509_Time t(v, Botan::ASN1_Tag::UTC_TIME); });
       }
 
+   for (auto&& v : invalid_generalized)
+      {
+      result.test_throws("invalid", [v]() { Botan::X509_Time t(v, Botan::ASN1_Tag::GENERALIZED_TIME); });
+      }
+
    return result;
    }