Reimplement ASN1_TIME_print with the new parser.
No sense in keeping two around. This does cause the functions to reject
some previously accepted invalid inputs. These were intentionally
accepted by
https://boringssl-review.googlesource.com/c/boringssl/+/13082 for
an old version of M2Crypto, but I belive we no longer need to be
compatible with that.
Update-Note: ASN1_TIME_print, ASN1_UTCTIME_print, and
ASN1_GENERALIZEDTIME_print will no longer accept various invalid inputs.
Change-Id: I4606d0b39585a19eb4b984ac809706e497a3f799
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53090
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
diff --git a/crypto/asn1/a_strex.c b/crypto/asn1/a_strex.c
index 52d979e..e6aacf0 100644
--- a/crypto/asn1/a_strex.c
+++ b/crypto/asn1/a_strex.c
@@ -60,8 +60,10 @@
#include <ctype.h>
#include <inttypes.h>
#include <string.h>
+#include <time.h>
#include <openssl/bio.h>
+#include <openssl/bytestring.h>
#include <openssl/mem.h>
#include "internal.h"
@@ -464,7 +466,7 @@
if (tm->type == V_ASN1_GENERALIZEDTIME) {
return ASN1_GENERALIZEDTIME_print(bp, tm);
}
- BIO_write(bp, "Bad time value", 14);
+ BIO_puts(bp, "Bad time value");
return 0;
}
@@ -472,136 +474,29 @@
"Jul", "Aug", "Sep", "Oct", "Nov", "Dec"};
int ASN1_GENERALIZEDTIME_print(BIO *bp, const ASN1_GENERALIZEDTIME *tm) {
- char *v;
- int gmt = 0;
- int i;
- int y = 0, M = 0, d = 0, h = 0, m = 0, s = 0;
- char *f = NULL;
- int f_len = 0;
-
- i = tm->length;
- v = (char *)tm->data;
-
- if (i < 12) {
- goto err;
- }
- if (v[i - 1] == 'Z') {
- gmt = 1;
- }
- for (i = 0; i < 12; i++) {
- if ((v[i] > '9') || (v[i] < '0')) {
- goto err;
- }
- }
- y = (v[0] - '0') * 1000 + (v[1] - '0') * 100 + (v[2] - '0') * 10 +
- (v[3] - '0');
- M = (v[4] - '0') * 10 + (v[5] - '0');
- if ((M > 12) || (M < 1)) {
- goto err;
- }
- d = (v[6] - '0') * 10 + (v[7] - '0');
- h = (v[8] - '0') * 10 + (v[9] - '0');
- m = (v[10] - '0') * 10 + (v[11] - '0');
- if (tm->length >= 14 && (v[12] >= '0') && (v[12] <= '9') && (v[13] >= '0') &&
- (v[13] <= '9')) {
- s = (v[12] - '0') * 10 + (v[13] - '0');
- // Check for fractions of seconds.
- if (tm->length >= 15 && v[14] == '.') {
- int l = tm->length;
- f = &v[14]; // The decimal point.
- f_len = 1;
- while (14 + f_len < l && f[f_len] >= '0' && f[f_len] <= '9') {
- ++f_len;
- }
- }
- }
-
- if (BIO_printf(bp, "%s %2d %02d:%02d:%02d%.*s %d%s", mon[M - 1], d, h, m, s,
- f_len, f, y, (gmt) ? " GMT" : "") <= 0) {
- return 0;
- } else {
- return 1;
- }
-err:
- BIO_write(bp, "Bad time value", 14);
- return 0;
-}
-
-// consume_two_digits is a helper function for ASN1_UTCTIME_print. If |*v|,
-// assumed to be |*len| bytes long, has two leading digits, updates |*out| with
-// their value, updates |v| and |len|, and returns one. Otherwise, returns
-// zero.
-static int consume_two_digits(int *out, const char **v, int *len) {
- if (*len < 2 || !isdigit((unsigned char)((*v)[0])) ||
- !isdigit((unsigned char)((*v)[1]))) {
- return 0;
- }
- *out = ((*v)[0] - '0') * 10 + ((*v)[1] - '0');
- *len -= 2;
- *v += 2;
- return 1;
-}
-
-// consume_zulu_timezone is a helper function for ASN1_UTCTIME_print. If |*v|,
-// assumed to be |*len| bytes long, starts with "Z" then it updates |*v| and
-// |*len| and returns one. Otherwise returns zero.
-static int consume_zulu_timezone(const char **v, int *len) {
- if (*len == 0 || (*v)[0] != 'Z') {
+ CBS cbs;
+ CBS_init(&cbs, tm->data, tm->length);
+ struct tm utc;
+ if (!CBS_parse_generalized_time(&cbs, &utc, /*allow_timezone_offset=*/0)) {
+ BIO_puts(bp, "Bad time value");
return 0;
}
- *len -= 1;
- *v += 1;
- return 1;
+ return BIO_printf(bp, "%s %2d %02d:%02d:%02d %d GMT", mon[utc.tm_mon],
+ utc.tm_mday, utc.tm_hour, utc.tm_min, utc.tm_sec,
+ utc.tm_year + 1900) > 0;
}
int ASN1_UTCTIME_print(BIO *bp, const ASN1_UTCTIME *tm) {
- const char *v = (const char *)tm->data;
- int len = tm->length;
- int Y = 0, M = 0, D = 0, h = 0, m = 0, s = 0;
-
- // YYMMDDhhmm are required to be present.
- if (!consume_two_digits(&Y, &v, &len) || !consume_two_digits(&M, &v, &len) ||
- !consume_two_digits(&D, &v, &len) || !consume_two_digits(&h, &v, &len) ||
- !consume_two_digits(&m, &v, &len)) {
- goto err;
- }
- // https://tools.ietf.org/html/rfc5280, section 4.1.2.5.1, requires seconds
- // to be present, but historically this code has forgiven its absence.
- consume_two_digits(&s, &v, &len);
-
- // https://tools.ietf.org/html/rfc5280, section 4.1.2.5.1, specifies this
- // interpretation of the year.
- if (Y < 50) {
- Y += 2000;
- } else {
- Y += 1900;
- }
- if (M > 12 || M == 0) {
- goto err;
- }
- if (D > 31 || D == 0) {
- goto err;
- }
- if (h > 23 || m > 59 || s > 60) {
- goto err;
+ CBS cbs;
+ CBS_init(&cbs, tm->data, tm->length);
+ struct tm utc;
+ if (!CBS_parse_utc_time(&cbs, &utc, /*allow_timezone_offset=*/0)) {
+ BIO_puts(bp, "Bad time value");
+ return 0;
}
- // https://tools.ietf.org/html/rfc5280, section 4.1.2.5.1, requires the "Z"
- // to be present, but historically this code has forgiven its absence.
- const int is_gmt = consume_zulu_timezone(&v, &len);
-
- // https://tools.ietf.org/html/rfc5280, section 4.1.2.5.1, does not permit
- // the specification of timezones using the +hhmm / -hhmm syntax, which is
- // the only other thing that might legitimately be found at the end.
- if (len) {
- goto err;
- }
-
- return BIO_printf(bp, "%s %2d %02d:%02d:%02d %d%s", mon[M - 1], D, h, m, s, Y,
- is_gmt ? " GMT" : "") > 0;
-
-err:
- BIO_write(bp, "Bad time value", 14);
- return 0;
+ return BIO_printf(bp, "%s %2d %02d:%02d:%02d %d GMT", mon[utc.tm_mon],
+ utc.tm_mday, utc.tm_hour, utc.tm_min, utc.tm_sec,
+ utc.tm_year + 1900) > 0;
}
diff --git a/crypto/asn1/asn1_test.cc b/crypto/asn1/asn1_test.cc
index c4f47bb..442c1ea 100644
--- a/crypto/asn1/asn1_test.cc
+++ b/crypto/asn1/asn1_test.cc
@@ -23,6 +23,7 @@
#include <openssl/asn1.h>
#include <openssl/asn1t.h>
+#include <openssl/bio.h>
#include <openssl/bytestring.h>
#include <openssl/err.h>
#include <openssl/mem.h>
@@ -926,28 +927,46 @@
return day == 0 && sec ==0;
}
+static std::string PrintStringToBIO(const ASN1_STRING *str,
+ int (*print_func)(BIO *,
+ const ASN1_STRING *)) {
+ const uint8_t *data;
+ size_t len;
+ bssl::UniquePtr<BIO> bio(BIO_new(BIO_s_mem()));
+ if (!bio || //
+ !print_func(bio.get(), str) ||
+ !BIO_mem_contents(bio.get(), &data, &len)) {
+ ADD_FAILURE() << "Could not print to BIO";
+ return "";
+ }
+ return std::string(data, data + len);
+}
+
TEST(ASN1Test, SetTime) {
static const struct {
time_t time;
const char *generalized;
const char *utc;
+ const char *printed;
} kTests[] = {
- {-631152001, "19491231235959Z", nullptr},
- {-631152000, "19500101000000Z", "500101000000Z"},
- {0, "19700101000000Z", "700101000000Z"},
- {981173106, "20010203040506Z", "010203040506Z"},
- {951804000, "20000229060000Z", "000229060000Z"},
+ {-631152001, "19491231235959Z", nullptr, "Dec 31 23:59:59 1949 GMT"},
+ {-631152000, "19500101000000Z", "500101000000Z",
+ "Jan 1 00:00:00 1950 GMT"},
+ {0, "19700101000000Z", "700101000000Z", "Jan 1 00:00:00 1970 GMT"},
+ {981173106, "20010203040506Z", "010203040506Z", "Feb 3 04:05:06 2001 GMT"},
+ {951804000, "20000229060000Z", "000229060000Z", "Feb 29 06:00:00 2000 GMT"},
#if defined(OPENSSL_64_BIT)
// TODO(https://crbug.com/boringssl/416): These cases overflow 32-bit
// |time_t| and do not consistently work on 32-bit platforms. For now,
// disable the tests on 32-bit. Re-enable them once the bug is fixed.
- {2524607999, "20491231235959Z", "491231235959Z"},
- {2524608000, "20500101000000Z", nullptr},
+ {2524607999, "20491231235959Z", "491231235959Z",
+ "Dec 31 23:59:59 2049 GMT"},
+ {2524608000, "20500101000000Z", nullptr, "Jan 1 00:00:00 2050 GMT"},
// Test boundary conditions.
- {-62167219200, "00000101000000Z", nullptr},
- {-62167219201, nullptr, nullptr},
- {253402300799, "99991231235959Z", nullptr},
- {253402300800, nullptr, nullptr},
+ {-62167219200, "00000101000000Z", nullptr, "Jan 1 00:00:00 0 GMT"},
+ {-62167219201, nullptr, nullptr, nullptr},
+ {253402300799, "99991231235959Z", nullptr, "Dec 31 23:59:59 9999 GMT"},
+ {253402300800, nullptr, nullptr, nullptr},
#endif
};
for (const auto &t : kTests) {
@@ -966,6 +985,8 @@
EXPECT_EQ(V_ASN1_UTCTIME, ASN1_STRING_type(utc.get()));
EXPECT_EQ(t.utc, ASN1StringToStdString(utc.get()));
EXPECT_TRUE(ASN1Time_check_time_t(utc.get(), t.time));
+ EXPECT_EQ(PrintStringToBIO(utc.get(), &ASN1_UTCTIME_print), t.printed);
+ EXPECT_EQ(PrintStringToBIO(utc.get(), &ASN1_TIME_print), t.printed);
} else {
EXPECT_FALSE(utc);
}
@@ -977,6 +998,11 @@
EXPECT_EQ(V_ASN1_GENERALIZEDTIME, ASN1_STRING_type(generalized.get()));
EXPECT_EQ(t.generalized, ASN1StringToStdString(generalized.get()));
EXPECT_TRUE(ASN1Time_check_time_t(generalized.get(), t.time));
+ EXPECT_EQ(
+ PrintStringToBIO(generalized.get(), &ASN1_GENERALIZEDTIME_print),
+ t.printed);
+ EXPECT_EQ(PrintStringToBIO(generalized.get(), &ASN1_TIME_print),
+ t.printed);
} else {
EXPECT_FALSE(generalized);
}
diff --git a/crypto/x509/x509_test.cc b/crypto/x509/x509_test.cc
index ce70ae3..fe4a775 100644
--- a/crypto/x509/x509_test.cc
+++ b/crypto/x509/x509_test.cc
@@ -2129,10 +2129,10 @@
{"000000000000Z", "Bad time value"},
{"999999999999Z", "Bad time value"},
- // Missing components. Not legal RFC 5280, but permitted.
- {"090303125425", "Mar 3 12:54:25 2009"},
- {"9003031254", "Mar 3 12:54:00 1990"},
- {"9003031254Z", "Mar 3 12:54:00 1990 GMT"},
+ // Missing components.
+ {"090303125425", "Bad time value"},
+ {"9003031254", "Bad time value"},
+ {"9003031254Z", "Bad time value"},
// GENERALIZEDTIME confused for UTCTIME.
{"20090303125425Z", "Bad time value"},
diff --git a/include/openssl/bytestring.h b/include/openssl/bytestring.h
index 846ab24..1d47f60 100644
--- a/include/openssl/bytestring.h
+++ b/include/openssl/bytestring.h
@@ -362,16 +362,17 @@
// corresponding time in UTC. This function does not compute |out_tm->tm_wday|
// or |out_tm->tm_yday|.
OPENSSL_EXPORT int CBS_parse_generalized_time(const CBS *cbs, struct tm *out_tm,
- int allow_timezeone_offset);
+ int allow_timezone_offset);
// CBS_parse_utc_time returns one if |cbs| is a valid DER-encoded, ASN.1
// UTCTime body within the limitations imposed by RFC 5280, or zero otherwise.
// If |allow_timezone_offset| is non-zero, four-digit timezone offsets, which
// would not be allowed by DER, are permitted. On success, if |out_tm| is
// non-NULL, |*out_tm| will be zeroed, and then set to the corresponding time
-// in UTC. This function does not compute |out_tm->tm_wday| or |out_tm->tm_yday|.
+// in UTC. This function does not compute |out_tm->tm_wday| or
+// |out_tm->tm_yday|.
OPENSSL_EXPORT int CBS_parse_utc_time(const CBS *cbs, struct tm *out_tm,
- int allow_timezeone_offset);
+ int allow_timezone_offset);
// CRYPTO ByteBuilder.
//
