Skip to content

Commit

Permalink
Backport X509 certificate verification optimizations to AWS-LC-FIPS-2…
Browse files Browse the repository at this point in the history 
….x (#1611)

31d5dce: 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.

4e32cc5: When looking for the issuer of a certificate, if the current
certificate candidate is expired, X509_verify_cert will
continue searching for a valid cert. An expired certificate is
only returned if no valid certificates are found. This lets
AWS-LC gain feature parity with OpenSSL 1.1.1.

9bed1c9: Tweak test introduced by 4e32cc5.

All changes above reside outside our FIPS boundary and do not change
the integrity hash of our static build on validated platforms.

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license and the ISC license.
  • Loading branch information
@samuel40791765
samuel40791765 authored May 31, 2024
2 parents b103ec8 + 9270acd commit df03fe1
Show file tree
Hide file tree
Showing 12 changed files with 382 additions and 134 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
4 changes: 3 additions & 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 Expand Up @@ -367,6 +367,8 @@ ASN1_TYPE *ASN1_generate_v3(const char *str, const X509V3_CTX *cnf);
int X509_CERT_AUX_print(BIO *bp, X509_CERT_AUX *x, int indent);


int x509_check_cert_time(X509_STORE_CTX *ctx, X509 *x, int suppress_error);

// RSA-PSS functions.

// x509_rsa_pss_to_ctx configures |ctx| for an RSA-PSS operation based on
Expand Down
19 changes: 15 additions & 4 deletions crypto/x509/x509_lu.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -583,14 +583,17 @@ int X509_STORE_CTX_get1_issuer(X509 **issuer, X509_STORE_CTX *ctx, X509 *x) {
X509_OBJECT obj, *pobj;
int idx, ret;
size_t i;
*issuer = NULL;
xn = X509_get_issuer_name(x);
if (!X509_STORE_get_by_subject(ctx, X509_LU_X509, xn, &obj)) {
return 0;
}
// If certificate matches all OK
if (ctx->check_issued(ctx, x, obj.data.x509)) {
*issuer = obj.data.x509;
return 1;
if (x509_check_cert_time(ctx, obj.data.x509, /*suppress_error*/1)) {
*issuer = obj.data.x509;
return 1;
}
}
X509_OBJECT_free_contents(&obj);

Expand All @@ -612,13 +615,21 @@ int X509_STORE_CTX_get1_issuer(X509 **issuer, X509_STORE_CTX *ctx, X509 *x) {
}
if (ctx->check_issued(ctx, x, pobj->data.x509)) {
*issuer = pobj->data.x509;
X509_OBJECT_up_ref_count(pobj);
ret = 1;
break;
// Break the loop with a match if the time check is valid, otherwise
// we continue searching. We leave the last tested issuer certificate in
// |issuer| on purpose. This returns the closest match if none of the
// candidate issuer certificates' timestamps were valid.
if (x509_check_cert_time(ctx, *issuer, /*suppress_error*/1)) {
break;
}
}
}
}
CRYPTO_MUTEX_unlock_write(&ctx->ctx->objs_lock);
if(*issuer) {
X509_up_ref(*issuer);
}
return ret;
}

Expand Down
Loading

0 comments on commit df03fe1

Please sign in to comment.