Skip to content

Commit

Permalink
Get rid of time_t usage internally, change to int64_t
Browse files Browse the repository at this point in the history 
We still keep time_t stuff around for calling time() and
for external interfaces that are meant to give you time_t
values, but we stop using time_t internally. For publicly
exposed and used inputs that rely on time_t, _posix versions are
added to support providing times as an int64_t, and internal
use is changed to use the _posix version.

Several legacy functions which are extensivly used and
and use pointers to time_t are retained for compatibility,
along with posix time versions of them which we use exclusively.

This fixes the tests which were disabled on 32 bit platorms
to always run.

Update-Note: This is a potentially breaking change for things
that bind to the ASN1_[UTC|GENERALIZED]TIME_set and ASN1_TIME_adj
family of functions (and can not type convert a time_t to an
int64).

Bug: 416

Change-Id: Ic4daba5a299d8f35191853742640750a1ecc53d6
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54765
Commit-Queue: Bob Beck <bbe@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
(cherry picked from commit 6e20b77e6b79069e2468686bdc69169d3fa2252e)
  • Loading branch information
@samuel40791765
Bob Beck authored and samuel40791765 committed Mar 10, 2023
1 parent 039cd02 commit 31d5dce
Show file tree
Hide file tree
Showing 11 changed files with 233 additions and 120 deletions.
8 changes: 4 additions & 4 deletions crypto/asn1/a_gentm.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,15 +98,15 @@ int ASN1_GENERALIZEDTIME_set_string(ASN1_GENERALIZEDTIME *s, const char *str) {
}

ASN1_GENERALIZEDTIME *ASN1_GENERALIZEDTIME_set(ASN1_GENERALIZEDTIME *s,
time_t t) {
return ASN1_GENERALIZEDTIME_adj(s, t, 0, 0);
int64_t posix_time) {
return ASN1_GENERALIZEDTIME_adj(s, posix_time, 0, 0);
}

ASN1_GENERALIZEDTIME *ASN1_GENERALIZEDTIME_adj(ASN1_GENERALIZEDTIME *s,
time_t t, int offset_day,
int64_t posix_time, int offset_day,
long offset_sec) {
struct tm data;
if (!OPENSSL_gmtime(&t, &data)) {
if (!OPENSSL_posix_to_tm(posix_time, &data)) {
return NULL;
}

Expand Down
28 changes: 14 additions & 14 deletions crypto/asn1/a_time.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,29 +73,31 @@ IMPLEMENT_ASN1_MSTRING(ASN1_TIME, B_ASN1_TIME)

IMPLEMENT_ASN1_FUNCTIONS_const(ASN1_TIME)

ASN1_TIME *ASN1_TIME_set(ASN1_TIME *s, time_t t) {
return ASN1_TIME_adj(s, t, 0, 0);
ASN1_TIME *ASN1_TIME_set_posix(ASN1_TIME *s, int64_t posix_time) {
return ASN1_TIME_adj(s, posix_time, 0, 0);
}

ASN1_TIME *ASN1_TIME_adj(ASN1_TIME *s, time_t t, int offset_day,
ASN1_TIME *ASN1_TIME_set(ASN1_TIME *s, time_t time) {
return ASN1_TIME_adj(s, time, 0, 0);
}

ASN1_TIME *ASN1_TIME_adj(ASN1_TIME *s, int64_t posix_time, int offset_day,
long offset_sec) {
struct tm *ts;
struct tm data;
struct tm tm;

ts = OPENSSL_gmtime(&t, &data);
if (ts == NULL) {
if (!OPENSSL_posix_to_tm(posix_time, &tm)) {
OPENSSL_PUT_ERROR(ASN1, ASN1_R_ERROR_GETTING_TIME);
return NULL;
}
if (offset_day || offset_sec) {
if (!OPENSSL_gmtime_adj(ts, offset_day, offset_sec)) {
if (!OPENSSL_gmtime_adj(&tm, offset_day, offset_sec)) {
return NULL;
}
}
if ((ts->tm_year >= 50) && (ts->tm_year < 150)) {
return ASN1_UTCTIME_adj(s, t, offset_day, offset_sec);
if ((tm.tm_year >= 50) && (tm.tm_year < 150)) {
return ASN1_UTCTIME_adj(s, posix_time, offset_day, offset_sec);
}
return ASN1_GENERALIZEDTIME_adj(s, t, offset_day, offset_sec);
return ASN1_GENERALIZEDTIME_adj(s, posix_time, offset_day, offset_sec);
}

int ASN1_TIME_check(const ASN1_TIME *t) {
Expand Down Expand Up @@ -171,9 +173,7 @@ int ASN1_TIME_set_string(ASN1_TIME *s, const char *str) {
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);
if (OPENSSL_gmtime(&now_t, tm)) {
if (OPENSSL_posix_to_tm(time(NULL), tm)) {
return 1;
}
return 0;
Expand Down
10 changes: 5 additions & 5 deletions crypto/asn1/a_utctm.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,14 +98,14 @@ int ASN1_UTCTIME_set_string(ASN1_UTCTIME *s, const char *str) {
return 1;
}

ASN1_UTCTIME *ASN1_UTCTIME_set(ASN1_UTCTIME *s, time_t t) {
return ASN1_UTCTIME_adj(s, t, 0, 0);
ASN1_UTCTIME *ASN1_UTCTIME_set(ASN1_UTCTIME *s, int64_t posix_time) {
return ASN1_UTCTIME_adj(s, posix_time, 0, 0);
}

ASN1_UTCTIME *ASN1_UTCTIME_adj(ASN1_UTCTIME *s, time_t t, int offset_day,
ASN1_UTCTIME *ASN1_UTCTIME_adj(ASN1_UTCTIME *s, int64_t posix_time, int offset_day,
long offset_sec) {
struct tm data;
if (!OPENSSL_gmtime(&t, &data)) {
if (!OPENSSL_posix_to_tm(posix_time, &data)) {
return NULL;
}

Expand Down Expand Up @@ -151,7 +151,7 @@ int ASN1_UTCTIME_cmp_time_t(const ASN1_UTCTIME *s, time_t t) {
return -2;
}

if (!OPENSSL_gmtime(&t, &ttm)) {
if (!OPENSSL_posix_to_tm(t, &ttm)) {
return -2;
}

Expand Down
66 changes: 32 additions & 34 deletions crypto/asn1/asn1_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -921,7 +921,7 @@ static std::string ASN1StringToStdString(const ASN1_STRING *str) {
ASN1_STRING_get0_data(str) + ASN1_STRING_length(str));
}

static bool ASN1Time_check_time_t(const ASN1_TIME *s, time_t t) {
static bool ASN1Time_check_posix(const ASN1_TIME *s, int64_t t) {
struct tm stm, ttm;
int day, sec;

Expand All @@ -939,7 +939,7 @@ static bool ASN1Time_check_time_t(const ASN1_TIME *s, time_t t) {
default:
return false;
}
if (!OPENSSL_gmtime(&t, &ttm) ||
if (!OPENSSL_posix_to_tm(t, &ttm) ||
!OPENSSL_gmtime_diff(&day, &sec, &ttm, &stm)) {
return false;
}
Expand All @@ -963,46 +963,44 @@ static std::string PrintStringToBIO(const ASN1_STRING *str,

TEST(ASN1Test, SetTime) {
static const struct {
time_t time;
int64_t time;
const char *generalized;
const char *utc;
const char *printed;
} kTests[] = {
{-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"},
// 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,
// disable the tests on 32-bit. Re-enable them once the bug is fixed.
{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, "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
{-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"},
// 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"},
{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, "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},
};
for (const auto &t : kTests) {
time_t tt;
int64_t tt;
SCOPED_TRACE(t.time);

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_TRUE(ASN1Time_check_posix(utc.get(), t.time));
EXPECT_EQ(ASN1_TIME_to_posix(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);
Expand All @@ -1016,8 +1014,8 @@ TEST(ASN1Test, SetTime) {
ASSERT_TRUE(generalized);
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_TRUE(ASN1Time_check_posix(generalized.get(), t.time));
EXPECT_EQ(ASN1_TIME_to_posix(generalized.get(), &tt), 1);
EXPECT_EQ(tt, t.time);
EXPECT_EQ(
PrintStringToBIO(generalized.get(), &ASN1_GENERALIZEDTIME_print),
Expand All @@ -1028,7 +1026,7 @@ TEST(ASN1Test, SetTime) {
EXPECT_FALSE(generalized);
}

bssl::UniquePtr<ASN1_TIME> choice(ASN1_TIME_set(nullptr, t.time));
bssl::UniquePtr<ASN1_TIME> choice(ASN1_TIME_set_posix(nullptr, t.time));
if (t.generalized) {
ASSERT_TRUE(choice);
if (t.utc) {
Expand All @@ -1038,8 +1036,8 @@ TEST(ASN1Test, SetTime) {
EXPECT_EQ(V_ASN1_GENERALIZEDTIME, ASN1_STRING_type(choice.get()));
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_TRUE(ASN1Time_check_posix(choice.get(), t.time));
EXPECT_EQ(ASN1_TIME_to_posix(choice.get(), &tt), 1);
EXPECT_EQ(tt, t.time);
} else {
EXPECT_FALSE(choice);
Expand Down
2 changes: 1 addition & 1 deletion crypto/x509/internal.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ struct X509_crl_st {

struct X509_VERIFY_PARAM_st {
char *name;
time_t check_time; // Time to use
int64_t check_time; // POSIX time to use
unsigned long inh_flags; // Inheritance flags
unsigned long flags; // Various verify flags
int purpose; // purpose to check untrusted certificates
Expand Down
28 changes: 14 additions & 14 deletions crypto/x509/x509_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1146,7 +1146,7 @@ static bssl::UniquePtr<STACK_OF(X509_CRL)> CRLsToStack(
return stack;
}

static const time_t kReferenceTime = 1474934400 /* Sep 27th, 2016 */;
static const int64_t kReferenceTime = 1474934400 /* Sep 27th, 2016 */;

static int Verify(
X509 *leaf, const std::vector<X509 *> &roots,
Expand Down Expand Up @@ -1180,7 +1180,7 @@ static int Verify(
X509_STORE_CTX_set0_crls(ctx.get(), crls_stack.get());

X509_VERIFY_PARAM *param = X509_STORE_CTX_get0_param(ctx.get());
X509_VERIFY_PARAM_set_time(param, kReferenceTime);
X509_VERIFY_PARAM_set_time_posix(param, kReferenceTime);
if (configure_callback) {
configure_callback(param);
}
Expand Down Expand Up @@ -1552,7 +1552,7 @@ TEST(X509Test, TestCRL) {
EXPECT_EQ(X509_V_ERR_CRL_HAS_EXPIRED,
Verify(leaf.get(), {root.get()}, {root.get()}, {basic_crl.get()},
X509_V_FLAG_CRL_CHECK, [](X509_VERIFY_PARAM *param) {
X509_VERIFY_PARAM_set_time(
X509_VERIFY_PARAM_set_time_posix(
param, kReferenceTime + 2 * 30 * 24 * 3600);
}));

Expand All @@ -1561,7 +1561,7 @@ TEST(X509Test, TestCRL) {
Verify(leaf.get(), {root.get()}, {root.get()}, {basic_crl.get()},
X509_V_FLAG_CRL_CHECK | X509_V_FLAG_NO_CHECK_TIME,
[](X509_VERIFY_PARAM *param) {
X509_VERIFY_PARAM_set_time(
X509_VERIFY_PARAM_set_time_posix(
param, kReferenceTime + 2 * 30 * 24 * 3600);
}));

Expand Down Expand Up @@ -2117,7 +2117,7 @@ TEST(X509Test, SignCRL) {
ASSERT_TRUE(X509_CRL_set_version(crl.get(), X509_CRL_VERSION_2));
bssl::UniquePtr<ASN1_TIME> last_update(ASN1_TIME_new());
ASSERT_TRUE(last_update);
ASSERT_TRUE(ASN1_TIME_set(last_update.get(), kReferenceTime));
ASSERT_TRUE(ASN1_TIME_set_posix(last_update.get(), kReferenceTime));
ASSERT_TRUE(X509_CRL_set1_lastUpdate(crl.get(), last_update.get()));
bssl::UniquePtr<X509_NAME> issuer(X509_NAME_new());
ASSERT_TRUE(issuer);
Expand Down Expand Up @@ -3978,13 +3978,13 @@ TEST(X509Test, Expiry) {
// The following are measured in seconds relative to kReferenceTime. The
// validity periods are staggered so we can independently test both leaf and
// root time checks.
const time_t kSecondsInDay = 24 * 3600;
const time_t kRootStart = -30 * kSecondsInDay;
const time_t kIntermediateStart = -20 * kSecondsInDay;
const time_t kLeafStart = -10 * kSecondsInDay;
const time_t kIntermediateEnd = 10 * kSecondsInDay;
const time_t kLeafEnd = 20 * kSecondsInDay;
const time_t kRootEnd = 30 * kSecondsInDay;
const int64_t kSecondsInDay = 24 * 3600;
const int64_t kRootStart = -30 * kSecondsInDay;
const int64_t kIntermediateStart = -20 * kSecondsInDay;
const int64_t kLeafStart = -10 * kSecondsInDay;
const int64_t kIntermediateEnd = 10 * kSecondsInDay;
const int64_t kLeafEnd = 20 * kSecondsInDay;
const int64_t kRootEnd = 30 * kSecondsInDay;

bssl::UniquePtr<X509> root =
MakeTestCert("Root", "Root", key.get(), /*is_ca=*/true);
Expand Down Expand Up @@ -4022,9 +4022,9 @@ TEST(X509Test, Expiry) {
ASSERT_TRUE(X509_sign(leaf.get(), key.get(), EVP_sha256()));

struct VerifyAt {
time_t time;
int64_t time;
void operator()(X509_VERIFY_PARAM *param) const {
X509_VERIFY_PARAM_set_time(param, time);
X509_VERIFY_PARAM_set_time_posix(param, time);
}
};

Expand Down
Loading

0 comments on commit 31d5dce

Please sign in to comment.