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>
chromium-5359
David Benjamin 2 years ago committed by Boringssl LUCI CQ
parent 0378578cd4
commit fdeb4aa925
  1. 145
      crypto/asn1/a_strex.c
  2. 48
      crypto/asn1/asn1_test.cc
  3. 8
      crypto/x509/x509_test.cc
  4. 7
      include/openssl/bytestring.h

@ -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 @@ int ASN1_TIME_print(BIO *bp, const ASN1_TIME *tm) {
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 @@ static const char *const mon[12] = {"Jan", "Feb", "Mar", "Apr", "May", "Jun",
"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;
}
// 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;
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;
}
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;
}

@ -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 @@ static bool ASN1Time_check_time_t(const ASN1_TIME *s, time_t t) {
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 @@ TEST(ASN1Test, SetTime) {
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 @@ TEST(ASN1Test, SetTime) {
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);
}

@ -2129,10 +2129,10 @@ TEST(X509Test, TestPrintUTCTIME) {
{"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"},

@ -362,16 +362,17 @@ OPENSSL_EXPORT char *CBS_asn1_oid_to_text(const CBS *cbs);
// 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.
//

Loading…
Cancel
Save