Skip to content

Commit

Permalink
Make time_t conversions. Give up on the OS provided ones.
Browse files Browse the repository at this point in the history 
We only care about dates within years 0000 to 9999 for
RFC5280. timegm() is only semi-standard. Some things require the
setting awkward defines to get libc to give it to you. Other things
let you have it but make it stop working at year 3000. Still other
things have 32 bit time_t.....

Let's just make our own that actually works. all the time, does
everything with an int64_t, and fails if you want to send something
out that would overflow a 32 bit time_t.

In the process of doing this, we get rid of the old Julian date stuff
from OpenSSL, which while functional was a bit awkward dealing only
with days, and using the Julian calendar as the reference point instead of potentially something more useful. Julian seconds since Jan 1 1970
00:00:00 UCT are much more useful to us than Julian days since a
Julian epoch.

The OS implementations of timegm() and gmtime() also can be pretty
complex, due to the nature of needing multiple timezone, daylight
saving, day of week, and other stuff we simply do not need for
doing things with certificate times. A small microbenchmark of
10000000 of each operation comparing this implementation to
the system version on my M1 mac gives:

bbe-macbookpro:tmp bbe$ time ./openssl_gmtime

real    0m0.152s
user    0m0.127s
sys     0m0.018s
bbe-macbookpro:tmp bbe$ time ./gmtime

real    0m0.422s
user    0m0.403s
sys     0m0.014s
bbe-macbookpro:tmp bbe$ time ./openssl_timegm

real    0m0.041s
user    0m0.015s
sys     0m0.019s
bbe-macbookpro:tmp bbe$ time ./timegm

real    0m30.432s
user    0m30.383s
sys     0m0.040s

Similarly On a glinux machine:

bbe@bbe-glinux1:~$ time ./openssl_gmtime

real    0m0.157s
user    0m0.152s
sys     0m0.008s
bbe@bbe-glinux1:~$ time ./gmtime

real    0m0.336s
user    0m0.336s
sys     0m0.002s
bbe@bbe-glinux1:~$ time ./openssl_timegm

real    0m0.018s
user    0m0.019s
sys     0m0.002s
bbe@bbe-glinux1:~$ time ./timegm

real    0m0.680s
user    0m0.671s
sys     0m0.011s
bbe@bbe-glinux1:~$


Bug: 501

Change-Id: If445272d365f2c9673b5f3264d082af1a342e0a1
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53245
Commit-Queue: Bob Beck <[email protected]>
Reviewed-by: David Benjamin <[email protected]>
  • Loading branch information
Bob Beck authored and Boringssl LUCI CQ committed Jul 29, 2022
1 parent 5cb597e commit ccd665d
Show file tree
Hide file tree
Showing 9 changed files with 365 additions and 288 deletions.
2 changes: 1 addition & 1 deletion crypto/CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ add_library(
asn1/tasn_new.c
asn1/tasn_typ.c
asn1/tasn_utl.c
asn1/time_support.c
asn1/posix_time.c
base64/base64.c
bio/bio.c
bio/bio_mem.c
Expand Down
33 changes: 29 additions & 4 deletions crypto/asn1/a_time.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,8 @@ int ASN1_TIME_set_string(ASN1_TIME *s, const char *str) {
return 1;
}

static int asn1_time_to_tm(struct tm *tm, const ASN1_TIME *t) {
static int asn1_time_to_tm(struct tm *tm, const ASN1_TIME *t,
int allow_timezone_offset) {
if (t == NULL) {
time_t now_t;
time(&now_t);
Expand All @@ -198,7 +199,7 @@ static int asn1_time_to_tm(struct tm *tm, const ASN1_TIME *t) {
}

if (t->type == V_ASN1_UTCTIME) {
return asn1_utctime_to_tm(tm, t);
return asn1_utctime_to_tm(tm, t, allow_timezone_offset);
} else if (t->type == V_ASN1_GENERALIZEDTIME) {
return asn1_generalizedtime_to_tm(tm, t);
}
Expand All @@ -209,11 +210,35 @@ static int asn1_time_to_tm(struct tm *tm, const ASN1_TIME *t) {
int ASN1_TIME_diff(int *out_days, int *out_seconds, const ASN1_TIME *from,
const ASN1_TIME *to) {
struct tm tm_from, tm_to;
if (!asn1_time_to_tm(&tm_from, from)) {
if (!asn1_time_to_tm(&tm_from, from, /*allow_timezone_offset=*/1)) {
return 0;
}
if (!asn1_time_to_tm(&tm_to, to)) {
if (!asn1_time_to_tm(&tm_to, to, /*allow_timezone_offset=*/1)) {
return 0;
}
return OPENSSL_gmtime_diff(out_days, out_seconds, &tm_from, &tm_to);
}

// The functions below do *not* permissively allow the use of four digit
// timezone offsets in UTC times, as is done elsewhere in the code. They are
// both new API, and used internally to X509_cmp_time. This is to discourage the
// use of nonstandard times in new code, and to ensure that this code behaves
// correctly in X509_cmp_time which historically did its own time validations
// slightly different than the many other copies of X.509 time validation
// sprinkled through the codebase. The custom checks in X509_cmp_time meant that
// it did not allow four digit timezone offsets in UTC times.
int ASN1_TIME_to_time_t(const ASN1_TIME *t, time_t *out_time) {
struct tm tm;
if (!asn1_time_to_tm(&tm, t, /*allow_timezone_offset=*/0)) {
return 0;
}
return OPENSSL_timegm(&tm, out_time);
}

int ASN1_TIME_to_posix(const ASN1_TIME *t, int64_t *out_time) {
struct tm tm;
if (!asn1_time_to_tm(&tm, t, /*allow_timezone_offset=*/0)) {
return 0;
}
return OPENSSL_tm_to_posix(&tm, out_time);
}
9 changes: 5 additions & 4 deletions crypto/asn1/a_utctm.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,20 +64,21 @@

#include "internal.h"

int asn1_utctime_to_tm(struct tm *tm, const ASN1_UTCTIME *d) {
int asn1_utctime_to_tm(struct tm *tm, const ASN1_UTCTIME *d,
int allow_timezone_offset) {
if (d->type != V_ASN1_UTCTIME) {
return 0;
}
CBS cbs;
CBS_init(&cbs, d->data, (size_t)d->length);
if (!CBS_parse_utc_time(&cbs, tm, /*allow_timezone_offset=*/1)) {
if (!CBS_parse_utc_time(&cbs, tm, allow_timezone_offset)) {
return 0;
}
return 1;
}

int ASN1_UTCTIME_check(const ASN1_UTCTIME *d) {
return asn1_utctime_to_tm(NULL, d);
return asn1_utctime_to_tm(NULL, d, /*allow_timezone_offset=*/1);
}

int ASN1_UTCTIME_set_string(ASN1_UTCTIME *s, const char *str) {
Expand Down Expand Up @@ -148,7 +149,7 @@ int ASN1_UTCTIME_cmp_time_t(const ASN1_UTCTIME *s, time_t t) {
struct tm stm, ttm;
int day, sec;

if (!asn1_utctime_to_tm(&stm, s)) {
if (!asn1_utctime_to_tm(&stm, s, /*allow_timezone_offset=*/1)) {
return -2;
}

Expand Down
69 changes: 60 additions & 9 deletions crypto/asn1/asn1_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -913,12 +913,12 @@ static bool ASN1Time_check_time_t(const ASN1_TIME *s, time_t t) {
}
break;
case V_ASN1_UTCTIME:
if (!asn1_utctime_to_tm(&stm, s)) {
if (!asn1_utctime_to_tm(&stm, s, /*allow_timezone_offset=*/1)) {
return false;
}
break;
default:
return 0;
return false;
}
if (!OPENSSL_gmtime(&t, &ttm) ||
!OPENSSL_gmtime_diff(&day, &sec, &ttm, &stm)) {
Expand Down Expand Up @@ -955,6 +955,10 @@ TEST(ASN1Test, SetTime) {
{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"},
// NASA says this is the correct time for posterity.
{-16751025, "19690621025615Z", "690621025615Z", "Jun 21 02:56:15 1969 GMT"},
// -1 is sometimes used as an error value. Ensure we correctly handle it.
{-1, "19691231235959Z", "691231235959Z", "Dec 31 23:59:59 1969 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,
Expand All @@ -970,21 +974,17 @@ TEST(ASN1Test, SetTime) {
#endif
};
for (const auto &t : kTests) {
time_t tt;
SCOPED_TRACE(t.time);
#if defined(OPENSSL_WINDOWS)
// Windows |time_t| functions can only handle 1970 through 3000. See
// https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/gmtime-s-gmtime32-s-gmtime64-s?view=msvc-160
if (t.time < 0 || int64_t{t.time} > 32535215999) {
continue;
}
#endif

bssl::UniquePtr<ASN1_UTCTIME> utc(ASN1_UTCTIME_set(nullptr, t.time));
if (t.utc) {
ASSERT_TRUE(utc);
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(ASN1_TIME_to_time_t(utc.get(), &tt), 1);
EXPECT_EQ(tt, t.time);
EXPECT_EQ(PrintStringToBIO(utc.get(), &ASN1_UTCTIME_print), t.printed);
EXPECT_EQ(PrintStringToBIO(utc.get(), &ASN1_TIME_print), t.printed);
} else {
Expand All @@ -998,6 +998,8 @@ 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(ASN1_TIME_to_time_t(generalized.get(), &tt), 1);
EXPECT_EQ(tt, t.time);
EXPECT_EQ(
PrintStringToBIO(generalized.get(), &ASN1_GENERALIZEDTIME_print),
t.printed);
Expand All @@ -1018,6 +1020,8 @@ TEST(ASN1Test, SetTime) {
EXPECT_EQ(t.generalized, ASN1StringToStdString(choice.get()));
}
EXPECT_TRUE(ASN1Time_check_time_t(choice.get(), t.time));
EXPECT_EQ(ASN1_TIME_to_time_t(choice.get(), &tt), 1);
EXPECT_EQ(tt, t.time);
} else {
EXPECT_FALSE(choice);
}
Expand Down Expand Up @@ -2256,6 +2260,53 @@ TEST(ASN1Test, StringEncoding) {
}
}

// Exhaustively test POSIX time conversions for every day across the millenium.
TEST(ASN1Test, POSIXTime) {
const int kDaysInMonth[] = {31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31};

// Test the epoch explicitly, to confirm our baseline is correct.
struct tm civil_time;
ASSERT_TRUE(OPENSSL_posix_to_tm(0, &civil_time));
ASSERT_EQ(civil_time.tm_year + 1900, 1970);
ASSERT_EQ(civil_time.tm_mon + 1, 1);
ASSERT_EQ(civil_time.tm_mday, 1);
ASSERT_EQ(civil_time.tm_hour, 0);
ASSERT_EQ(civil_time.tm_min, 0);
ASSERT_EQ(civil_time.tm_sec, 0);

int64_t posix_time = -11676096000; // Sat, 01 Jan 1600 00:00:00 +0000
for (int year = 1600; year < 3000; year++) {
SCOPED_TRACE(year);
bool is_leap_year = (year % 4 == 0 && year % 100 != 0) || year % 400 == 0;
for (int month = 1; month <= 12; month++) {
SCOPED_TRACE(month);
int days = kDaysInMonth[month - 1];
if (month == 2 && is_leap_year) {
days++;
}
for (int day = 1; day <= days; day++) {
SCOPED_TRACE(day);
SCOPED_TRACE(posix_time);

ASSERT_TRUE(OPENSSL_posix_to_tm(posix_time, &civil_time));
ASSERT_EQ(civil_time.tm_year + 1900, year);
ASSERT_EQ(civil_time.tm_mon + 1, month);
ASSERT_EQ(civil_time.tm_mday, day);
ASSERT_EQ(civil_time.tm_hour, 0);
ASSERT_EQ(civil_time.tm_min, 0);
ASSERT_EQ(civil_time.tm_sec, 0);

int64_t posix_time_computed;
ASSERT_TRUE(OPENSSL_tm_to_posix(&civil_time, &posix_time_computed));
ASSERT_EQ(posix_time_computed, posix_time);

// Advance to the next day.
posix_time += 24 * 60 * 60;
}
}
}
}

// The ASN.1 macros do not work on Windows shared library builds, where usage of
// |OPENSSL_EXPORT| is a bit stricter.
#if !defined(OPENSSL_WINDOWS) || !defined(BORINGSSL_SHARED_LIBRARY)
Expand Down
37 changes: 30 additions & 7 deletions crypto/asn1/internal.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,16 +71,38 @@ extern "C" {

// Wrapper functions for time functions.

// OPENSSL_gmtime wraps |gmtime_r|. See the manual page for that function.
// OPENSSL_posix_to_tm converts a int64_t POSIX time value in |time| whuch must
// be in the range of year 0000 to 9999 to a broken out time value in |tm|. It
// returns one on success and zero on error.
OPENSSL_EXPORT int OPENSSL_posix_to_tm(int64_t time, struct tm *out_tm);

// OPENSSL_tm_to_posix converts a time value between the years 0 and 9999 in
// |tm| to a POSIX time value in |out|. One is returned on success, zero is
// returned on failure. It is a failure if the tm contains out of range values.
OPENSSL_EXPORT int OPENSSL_tm_to_posix(const struct tm *tm, int64_t *out);

// OPENSSL_gmtime converts a time_t value in |time| which must be in the range
// of year 0000 to 9999 to a broken out time value in |tm|. On success |tm| is
// returned. On failure NULL is returned.
OPENSSL_EXPORT struct tm *OPENSSL_gmtime(const time_t *time, struct tm *result);

// OPENSSL_gmtime_adj updates |tm| by adding |offset_day| days and |offset_sec|
// seconds.
// OPENSSL_timegm converts a time value between the years 0 and 9999 in |tm| to
// a time_t value in |out|. One is returned on success, zero is returned on
// failure. It is a failure if the converted time can not be represented in a
// time_t, or if the tm contains out of range values.
OPENSSL_EXPORT int OPENSSL_timegm(const struct tm *tm, time_t *out);

// OPENSSL_gmtime_adj returns one on success, and updates |tm| by adding
// |offset_day| days and |offset_sec| seconds. It returns zero on failure. |tm|
// must be in the range of year 0000 to 9999 both before and after the update or
// a failure will be returned.
int OPENSSL_gmtime_adj(struct tm *tm, int offset_day, long offset_sec);

// OPENSSL_gmtime_diff calculates the difference between |from| and |to| and
// outputs the difference as a number of days and seconds in |*out_days| and
// |*out_secs|.
// OPENSSL_gmtime_diff calculates the difference between |from| and |to|. It
// returns one, and outputs the difference as a number of days and seconds in
// |*out_days| and |*out_secs| on success. It returns zero on failure. Both
// |from| and |to| must be in the range of year 0000 to 9999 or a failure will
// be returned.
OPENSSL_EXPORT int OPENSSL_gmtime_diff(int *out_days, int *out_secs,
const struct tm *from,
const struct tm *to);
Expand Down Expand Up @@ -126,7 +148,8 @@ typedef struct ASN1_ENCODING_st {
unsigned alias_only_on_next_parse : 1;
} ASN1_ENCODING;

OPENSSL_EXPORT int asn1_utctime_to_tm(struct tm *tm, const ASN1_UTCTIME *d);
OPENSSL_EXPORT int asn1_utctime_to_tm(struct tm *tm, const ASN1_UTCTIME *d,
int allow_timezone_offset);
OPENSSL_EXPORT int asn1_generalizedtime_to_tm(struct tm *tm,
const ASN1_GENERALIZEDTIME *d);

Expand Down
Loading

0 comments on commit ccd665d

Please sign in to comment.