diff --git a/crypto/x509/t_x509.c b/crypto/x509/t_x509.c index d5d48ba05..d4f6bba58 100644 --- a/crypto/x509/t_x509.c +++ b/crypto/x509/t_x509.c @@ -54,6 +54,7 @@ * copied and put under another distribution licence * [including the GNU Public Licence.] */ +#include #include #include #include @@ -415,45 +416,84 @@ int ASN1_GENERALIZEDTIME_print(BIO *bp, const ASN1_GENERALIZEDTIME *tm) return (0); } -int ASN1_UTCTIME_print(BIO *bp, const ASN1_UTCTIME *tm) -{ - const char *v; - int gmt = 0; - int i; - int y = 0, M = 0, d = 0, h = 0, m = 0, s = 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((*v)[0]) || !isdigit((*v)[1])) { + return 0; + } + *out = ((*v)[0] - '0') * 10 + ((*v)[1] - '0'); + *len -= 2; + *v += 2; + return 1; +} - i = tm->length; - v = (const char *)tm->data; +// 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') { + return 0; + } - if (i < 10) - goto err; - if (v[i - 1] == 'Z') - gmt = 1; - for (i = 0; i < 10; i++) - if ((v[i] > '9') || (v[i] < '0')) - goto err; - y = (v[0] - '0') * 10 + (v[1] - '0'); - if (y < 50) - y += 100; - M = (v[2] - '0') * 10 + (v[3] - '0'); - if ((M > 12) || (M < 1)) - goto err; - d = (v[4] - '0') * 10 + (v[5] - '0'); - h = (v[6] - '0') * 10 + (v[7] - '0'); - m = (v[8] - '0') * 10 + (v[9] - '0'); - if (tm->length >= 12 && - (v[10] >= '0') && (v[10] <= '9') && (v[11] >= '0') && (v[11] <= '9')) - s = (v[10] - '0') * 10 + (v[11] - '0'); + *len -= 1; + *v += 1; + return 1; +} - if (BIO_printf(bp, "%s %2d %02d:%02d:%02d %d%s", - mon[M - 1], d, h, m, s, y + 1900, - (gmt) ? " GMT" : "") <= 0) - return (0); - else - return (1); - err: - BIO_write(bp, "Bad time value", 14); - return (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; + } + + // 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; } int X509_NAME_print(BIO *bp, X509_NAME *name, int obase) diff --git a/crypto/x509/x509_test.cc b/crypto/x509/x509_test.cc index 3629c13e0..4b80af825 100644 --- a/crypto/x509/x509_test.cc +++ b/crypto/x509/x509_test.cc @@ -17,6 +17,7 @@ #include #include +#include #include #include #include @@ -1011,6 +1012,73 @@ static bool TestFailedParseFromBuffer() { return true; } +static bool TestPrintUTCTIME() { + static const struct { + const char *val, *want; + } asn1_utctime_tests[] = { + {"", "Bad time value"}, + + // Correct RFC 5280 form. Test years < 2000 and > 2000. + {"090303125425Z", "Mar 3 12:54:25 2009 GMT"}, + {"900303125425Z", "Mar 3 12:54:25 1990 GMT"}, + {"000303125425Z", "Mar 3 12:54:25 2000 GMT"}, + + // Correct form, bad values. + {"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"}, + + // GENERALIZEDTIME confused for UTCTIME. + {"20090303125425Z", "Bad time value"}, + + // Legal ASN.1, but not legal RFC 5280. + {"9003031254+0800", "Bad time value"}, + {"9003031254-0800", "Bad time value"}, + + // Trailing garbage. + {"9003031254Z ", "Bad time value"}, + }; + + for (auto t : asn1_utctime_tests) { + bssl::UniquePtr tm(ASN1_UTCTIME_new()); + bssl::UniquePtr bio(BIO_new(BIO_s_mem())); + + // Use this instead of ASN1_UTCTIME_set() because some callers get + // type-confused and pass ASN1_GENERALIZEDTIME to ASN1_UTCTIME_print(). + // ASN1_UTCTIME_set_string() is stricter, and would reject the inputs in + // question. + if (!ASN1_STRING_set(tm.get(), t.val, strlen(t.val))) { + fprintf(stderr, "ASN1_STRING_set\n"); + return false; + } + const int ok = ASN1_UTCTIME_print(bio.get(), tm.get()); + + const uint8_t *contents; + size_t len; + if (!BIO_mem_contents(bio.get(), &contents, &len)) { + fprintf(stderr, "BIO_mem_contents\n"); + return false; + } + + if (ok != (strcmp(t.want, "Bad time value") != 0)) { + fprintf(stderr, "ASN1_UTCTIME_print(%s): bad return value\n", t.val); + return false; + } + if (len != strlen(t.want) || memcmp(contents, t.want, len)) { + fprintf(stderr, "ASN1_UTCTIME_print(%s): got %.*s, want %s\n", t.val, + static_cast(len), + reinterpret_cast(contents), t.want); + return false; + } + } + + return true; +} + int main() { CRYPTO_library_init(); @@ -1023,7 +1091,8 @@ int main() { !TestFromBufferTrailingData() || !TestFromBufferModified() || !TestFromBufferReused() || - !TestFailedParseFromBuffer()) { + !TestFailedParseFromBuffer() || + !TestPrintUTCTIME()) { return 1; }