From 928287b7826ae8670da78a221c72f5380bc58973 Mon Sep 17 00:00:00 2001 From: Timothy Flynn Date: Thu, 7 Mar 2024 10:34:45 -0500 Subject: [PATCH] LibCrypto: Store ASN1 certificate timestamps as UnixDateTime We are currently using Core::DateTime, which is meant to represent local time. However, we are doing no conversion between the parsed time in UTC and local time, so we end up comparing time stamps from different time zones. Instead, store the parsed times as UnixDateTime, which is UTC. Then we can always compare the parsed times against the current UTC time. This also lets us store parsed milliseconds. --- Tests/LibCrypto/TestASN1.cpp | 96 +++++++++---------- .../CertificateStoreWidget.cpp | 3 +- Userland/Libraries/LibCrypto/ASN1/ASN1.cpp | 10 +- Userland/Libraries/LibCrypto/ASN1/ASN1.h | 6 +- Userland/Libraries/LibTLS/Certificate.cpp | 2 +- Userland/Libraries/LibTLS/Certificate.h | 6 +- Userland/Libraries/LibTLS/TLSv12.cpp | 6 +- 7 files changed, 64 insertions(+), 65 deletions(-) diff --git a/Tests/LibCrypto/TestASN1.cpp b/Tests/LibCrypto/TestASN1.cpp index 3b752d8e4b5..730e8db2f20 100644 --- a/Tests/LibCrypto/TestASN1.cpp +++ b/Tests/LibCrypto/TestASN1.cpp @@ -5,11 +5,12 @@ */ #include +#include #include #include #define EXPECT_DATETIME(sv, y, mo, d, h, mi, s) \ - EXPECT_EQ(Crypto::ASN1::parse_utc_time(sv).value(), Core::DateTime::create(y, mo, d, h, mi, s)) + EXPECT_EQ(Crypto::ASN1::parse_utc_time(sv).value(), UnixDateTime::from_unix_time_parts(y, mo, d, h, mi, s, 0)) TEST_CASE(test_utc_boring) { @@ -65,79 +66,76 @@ TEST_CASE(test_utc_missing_z) } #undef EXPECT_DATETIME -#define EXPECT_DATETIME(sv, y, mo, d, h, mi, s) \ - EXPECT_EQ(Crypto::ASN1::parse_generalized_time(sv).value(), Core::DateTime::create(y, mo, d, h, mi, s)) +#define EXPECT_DATETIME(sv, y, mo, d, h, mi, s, ms) \ + EXPECT_EQ(Crypto::ASN1::parse_generalized_time(sv).value(), UnixDateTime::from_unix_time_parts(y, mo, d, h, mi, s, ms)) TEST_CASE(test_generalized_boring) { // YYYYMMDDhh[mm[ss[.fff]]] - EXPECT_DATETIME("20010101010101Z"sv, 2001, 1, 1, 1, 1, 1); - EXPECT_DATETIME("20010203040506Z"sv, 2001, 2, 3, 4, 5, 6); - EXPECT_DATETIME("20020406081012Z"sv, 2002, 4, 6, 8, 10, 12); - EXPECT_DATETIME("200204060810Z"sv, 2002, 4, 6, 8, 10, 0); - EXPECT_DATETIME("2002040608Z"sv, 2002, 4, 6, 8, 0, 0); - // TODO: We probably should not discard the milliseconds. - EXPECT_DATETIME("20020406081012.567Z"sv, 2002, 4, 6, 8, 10, 12); - EXPECT_DATETIME("20220911220000Z"sv, 2022, 9, 11, 22, 0, 0); + EXPECT_DATETIME("20010101010101Z"sv, 2001, 1, 1, 1, 1, 1, 0); + EXPECT_DATETIME("20010203040506Z"sv, 2001, 2, 3, 4, 5, 6, 0); + EXPECT_DATETIME("20020406081012Z"sv, 2002, 4, 6, 8, 10, 12, 0); + EXPECT_DATETIME("200204060810Z"sv, 2002, 4, 6, 8, 10, 0, 0); + EXPECT_DATETIME("2002040608Z"sv, 2002, 4, 6, 8, 0, 0, 0); + EXPECT_DATETIME("20020406081012.567Z"sv, 2002, 4, 6, 8, 10, 12, 567); + EXPECT_DATETIME("20220911220000Z"sv, 2022, 9, 11, 22, 0, 0, 0); } TEST_CASE(test_generalized_offset) { // YYYYMMDDhh[mm[ss[.fff]]](+|-)hhmm // We don't yet support storing the offset anywhere and instead just assume that the offset is just +0000. - EXPECT_DATETIME("20010101010101+0000"sv, 2001, 1, 1, 1, 1, 1); - EXPECT_DATETIME("20010203040506+0000"sv, 2001, 2, 3, 4, 5, 6); - EXPECT_DATETIME("20020406081012+0000"sv, 2002, 4, 6, 8, 10, 12); - EXPECT_DATETIME("200204060810+0000"sv, 2002, 4, 6, 8, 10, 0); - EXPECT_DATETIME("2002040608+0000"sv, 2002, 4, 6, 8, 0, 0); - // TODO: We probably should not discard the milliseconds. - EXPECT_DATETIME("20020406081012.567+0000"sv, 2002, 4, 6, 8, 10, 12); - EXPECT_DATETIME("20220911220000+0000"sv, 2022, 9, 11, 22, 0, 0); + EXPECT_DATETIME("20010101010101+0000"sv, 2001, 1, 1, 1, 1, 1, 0); + EXPECT_DATETIME("20010203040506+0000"sv, 2001, 2, 3, 4, 5, 6, 0); + EXPECT_DATETIME("20020406081012+0000"sv, 2002, 4, 6, 8, 10, 12, 0); + EXPECT_DATETIME("200204060810+0000"sv, 2002, 4, 6, 8, 10, 0, 0); + EXPECT_DATETIME("2002040608+0000"sv, 2002, 4, 6, 8, 0, 0, 0); + EXPECT_DATETIME("20020406081012.567+0000"sv, 2002, 4, 6, 8, 10, 12, 567); + EXPECT_DATETIME("20220911220000+0000"sv, 2022, 9, 11, 22, 0, 0, 0); // Designed to fail once we support offsets: - EXPECT_DATETIME("20220911220000+0600"sv, 2022, 9, 11, 22, 0, 0); + EXPECT_DATETIME("20220911220000+0600"sv, 2022, 9, 11, 22, 0, 0, 0); } TEST_CASE(test_generalized_missing_z) { // YYYYMMDDhh[mm[ss[.fff]]] - EXPECT_DATETIME("20010101010101"sv, 2001, 1, 1, 1, 1, 1); - EXPECT_DATETIME("20010203040506"sv, 2001, 2, 3, 4, 5, 6); - EXPECT_DATETIME("20020406081012"sv, 2002, 4, 6, 8, 10, 12); - EXPECT_DATETIME("200204060810"sv, 2002, 4, 6, 8, 10, 0); - EXPECT_DATETIME("2002040608"sv, 2002, 4, 6, 8, 0, 0); - // TODO: We probably should not discard the milliseconds. - EXPECT_DATETIME("20020406081012.567"sv, 2002, 4, 6, 8, 10, 12); - EXPECT_DATETIME("20220911220000"sv, 2022, 9, 11, 22, 0, 0); + EXPECT_DATETIME("20010101010101"sv, 2001, 1, 1, 1, 1, 1, 0); + EXPECT_DATETIME("20010203040506"sv, 2001, 2, 3, 4, 5, 6, 0); + EXPECT_DATETIME("20020406081012"sv, 2002, 4, 6, 8, 10, 12, 0); + EXPECT_DATETIME("200204060810"sv, 2002, 4, 6, 8, 10, 0, 0); + EXPECT_DATETIME("2002040608"sv, 2002, 4, 6, 8, 0, 0, 0); + EXPECT_DATETIME("20020406081012.567"sv, 2002, 4, 6, 8, 10, 12, 567); + EXPECT_DATETIME("20220911220000"sv, 2022, 9, 11, 22, 0, 0, 0); } TEST_CASE(test_generalized_unusual_year) { // Towards the positive - EXPECT_DATETIME("20010203040506Z"sv, 2001, 2, 3, 4, 5, 6); - EXPECT_DATETIME("20110203040506Z"sv, 2011, 2, 3, 4, 5, 6); - EXPECT_DATETIME("21010203040506Z"sv, 2101, 2, 3, 4, 5, 6); - EXPECT_DATETIME("30010203040506Z"sv, 3001, 2, 3, 4, 5, 6); - EXPECT_DATETIME("40010203040506Z"sv, 4001, 2, 3, 4, 5, 6); - EXPECT_DATETIME("90010203040506Z"sv, 9001, 2, 3, 4, 5, 6); - EXPECT_DATETIME("99990203040506Z"sv, 9999, 2, 3, 4, 5, 6); + EXPECT_DATETIME("20010203040506Z"sv, 2001, 2, 3, 4, 5, 6, 0); + EXPECT_DATETIME("20110203040506Z"sv, 2011, 2, 3, 4, 5, 6, 0); + EXPECT_DATETIME("21010203040506Z"sv, 2101, 2, 3, 4, 5, 6, 0); + EXPECT_DATETIME("30010203040506Z"sv, 3001, 2, 3, 4, 5, 6, 0); + EXPECT_DATETIME("40010203040506Z"sv, 4001, 2, 3, 4, 5, 6, 0); + EXPECT_DATETIME("90010203040506Z"sv, 9001, 2, 3, 4, 5, 6, 0); + EXPECT_DATETIME("99990203040506Z"sv, 9999, 2, 3, 4, 5, 6, 0); // Towards zero - EXPECT_DATETIME("20010203040506Z"sv, 2001, 2, 3, 4, 5, 6); - EXPECT_DATETIME("19990203040506Z"sv, 1999, 2, 3, 4, 5, 6); - EXPECT_DATETIME("19500203040506Z"sv, 1950, 2, 3, 4, 5, 6); - EXPECT_DATETIME("19010203040506Z"sv, 1901, 2, 3, 4, 5, 6); - EXPECT_DATETIME("18010203040506Z"sv, 1801, 2, 3, 4, 5, 6); - EXPECT_DATETIME("15010203040506Z"sv, 1501, 2, 3, 4, 5, 6); - EXPECT_DATETIME("10010203040506Z"sv, 1001, 2, 3, 4, 5, 6); - EXPECT_DATETIME("01010203040506Z"sv, 101, 2, 3, 4, 5, 6); - EXPECT_DATETIME("00110203040506Z"sv, 11, 2, 3, 4, 5, 6); - EXPECT_DATETIME("00010203040506Z"sv, 1, 2, 3, 4, 5, 6); - EXPECT_DATETIME("00000203040506Z"sv, 0, 2, 3, 4, 5, 6); + EXPECT_DATETIME("20010203040506Z"sv, 2001, 2, 3, 4, 5, 6, 0); + EXPECT_DATETIME("19990203040506Z"sv, 1999, 2, 3, 4, 5, 6, 0); + EXPECT_DATETIME("19500203040506Z"sv, 1950, 2, 3, 4, 5, 6, 0); + EXPECT_DATETIME("19010203040506Z"sv, 1901, 2, 3, 4, 5, 6, 0); + EXPECT_DATETIME("18010203040506Z"sv, 1801, 2, 3, 4, 5, 6, 0); + EXPECT_DATETIME("15010203040506Z"sv, 1501, 2, 3, 4, 5, 6, 0); + EXPECT_DATETIME("10010203040506Z"sv, 1001, 2, 3, 4, 5, 6, 0); + EXPECT_DATETIME("01010203040506Z"sv, 101, 2, 3, 4, 5, 6, 0); + EXPECT_DATETIME("00110203040506Z"sv, 11, 2, 3, 4, 5, 6, 0); + EXPECT_DATETIME("00010203040506Z"sv, 1, 2, 3, 4, 5, 6, 0); + EXPECT_DATETIME("00000203040506Z"sv, 0, 2, 3, 4, 5, 6, 0); // Problematic dates - EXPECT_DATETIME("20200229040506Z"sv, 2020, 2, 29, 4, 5, 6); - EXPECT_DATETIME("20000229040506Z"sv, 2000, 2, 29, 4, 5, 6); - EXPECT_DATETIME("24000229040506Z"sv, 2400, 2, 29, 4, 5, 6); + EXPECT_DATETIME("20200229040506Z"sv, 2020, 2, 29, 4, 5, 6, 0); + EXPECT_DATETIME("20000229040506Z"sv, 2000, 2, 29, 4, 5, 6, 0); + EXPECT_DATETIME("24000229040506Z"sv, 2400, 2, 29, 4, 5, 6, 0); } TEST_CASE(test_generalized_nonexistent_dates) diff --git a/Userland/Applications/CertificateSettings/CertificateStoreWidget.cpp b/Userland/Applications/CertificateSettings/CertificateStoreWidget.cpp index 388f1ea8be0..0f31d4eed01 100644 --- a/Userland/Applications/CertificateSettings/CertificateStoreWidget.cpp +++ b/Userland/Applications/CertificateSettings/CertificateStoreWidget.cpp @@ -6,6 +6,7 @@ #include "CertificateStoreWidget.h" #include +#include #include #include #include @@ -74,7 +75,7 @@ GUI::Variant CertificateStoreModel::data(GUI::ModelIndex const& index, GUI::Mode return issued_by; } case Column::Expire: - return cert.validity.not_after.to_byte_string("%Y-%m-%d"sv); + return Core::DateTime::from_timestamp(cert.validity.not_after.seconds_since_epoch()).to_byte_string("%Y-%m-%d"sv); default: VERIFY_NOT_REACHED(); } diff --git a/Userland/Libraries/LibCrypto/ASN1/ASN1.cpp b/Userland/Libraries/LibCrypto/ASN1/ASN1.cpp index 4bba1bc032f..b49e276f674 100644 --- a/Userland/Libraries/LibCrypto/ASN1/ASN1.cpp +++ b/Userland/Libraries/LibCrypto/ASN1/ASN1.cpp @@ -119,7 +119,7 @@ ByteString type_name(Type type) return "InvalidType"; } -Optional parse_utc_time(StringView time) +Optional parse_utc_time(StringView time) { // YYMMDDhhmm[ss]Z or YYMMDDhhmm[ss](+|-)hhmm GenericLexer lexer(time); @@ -164,10 +164,10 @@ Optional parse_utc_time(StringView time) if (offset_hours.has_value() || offset_minutes.has_value()) dbgln("FIXME: Implement UTCTime with offset!"); - return Core::DateTime::create(full_year, month.value(), day.value(), hour.value(), minute.value(), full_seconds); + return UnixDateTime::from_unix_time_parts(full_year, month.value(), day.value(), hour.value(), minute.value(), full_seconds, 0); } -Optional parse_generalized_time(StringView time) +Optional parse_generalized_time(StringView time) { // YYYYMMDDhh[mm[ss[.fff]]] or YYYYMMDDhh[mm[ss[.fff]]]Z or YYYYMMDDhh[mm[ss[.fff]]](+|-)hhmm GenericLexer lexer(time); @@ -177,6 +177,7 @@ Optional parse_generalized_time(StringView time) auto hour = lexer.consume(2).to_number(); Optional minute, seconds, milliseconds, offset_hours, offset_minutes; [[maybe_unused]] bool negative_offset = false; + if (!lexer.is_eof()) { if (lexer.consume_specific('Z')) goto done_parsing; @@ -233,7 +234,6 @@ done_parsing:; if (offset_hours.has_value() || offset_minutes.has_value()) dbgln("FIXME: Implement GeneralizedTime with offset!"); - // Unceremoniously drop the milliseconds on the floor. - return Core::DateTime::create(year.value(), month.value(), day.value(), hour.value(), minute.value_or(0), seconds.value_or(0)); + return UnixDateTime::from_unix_time_parts(year.value(), month.value(), day.value(), hour.value(), minute.value_or(0), seconds.value_or(0), milliseconds.value_or(0)); } } diff --git a/Userland/Libraries/LibCrypto/ASN1/ASN1.h b/Userland/Libraries/LibCrypto/ASN1/ASN1.h index 4c4f0ac7322..dea932271a9 100644 --- a/Userland/Libraries/LibCrypto/ASN1/ASN1.h +++ b/Userland/Libraries/LibCrypto/ASN1/ASN1.h @@ -6,8 +6,8 @@ #pragma once +#include #include -#include #include namespace Crypto::ASN1 { @@ -75,7 +75,7 @@ ByteString kind_name(Kind); ByteString class_name(Class); ByteString type_name(Type); -Optional parse_utc_time(StringView); -Optional parse_generalized_time(StringView); +Optional parse_utc_time(StringView); +Optional parse_generalized_time(StringView); } diff --git a/Userland/Libraries/LibTLS/Certificate.cpp b/Userland/Libraries/LibTLS/Certificate.cpp index 793de9edf00..0d21ec074c9 100644 --- a/Userland/Libraries/LibTLS/Certificate.cpp +++ b/Userland/Libraries/LibTLS/Certificate.cpp @@ -296,7 +296,7 @@ static ErrorOr parse_name(Crypto::ASN1::Decoder& deco return rdn; } -static ErrorOr parse_time(Crypto::ASN1::Decoder& decoder, Vector current_scope) +static ErrorOr parse_time(Crypto::ASN1::Decoder& decoder, Vector current_scope) { // Time ::= Choice { // utc_time UTCTime, diff --git a/Userland/Libraries/LibTLS/Certificate.h b/Userland/Libraries/LibTLS/Certificate.h index e5bcde99816..f7bd33b5f4d 100644 --- a/Userland/Libraries/LibTLS/Certificate.h +++ b/Userland/Libraries/LibTLS/Certificate.h @@ -9,9 +9,9 @@ #include #include #include +#include #include #include -#include #include #include #include @@ -233,8 +233,8 @@ private: }; struct Validity { - Core::DateTime not_before; - Core::DateTime not_after; + UnixDateTime not_before; + UnixDateTime not_after; }; class SubjectPublicKey { diff --git a/Userland/Libraries/LibTLS/TLSv12.cpp b/Userland/Libraries/LibTLS/TLSv12.cpp index 8af2d9b410f..dbe32e1fdf3 100644 --- a/Userland/Libraries/LibTLS/TLSv12.cpp +++ b/Userland/Libraries/LibTLS/TLSv12.cpp @@ -104,15 +104,15 @@ void TLSv12::consume(ReadonlyBytes record) bool Certificate::is_valid() const { - auto now = Core::DateTime::now(); + auto now = UnixDateTime::now(); if (now < validity.not_before) { - dbgln("certificate expired (not yet valid, signed for {})", validity.not_before.to_byte_string()); + dbgln("certificate expired (not yet valid, signed for {})", Core::DateTime::from_timestamp(validity.not_before.seconds_since_epoch())); return false; } if (validity.not_after < now) { - dbgln("certificate expired (expiry date {})", validity.not_after.to_byte_string()); + dbgln("certificate expired (expiry date {})", Core::DateTime::from_timestamp(validity.not_after.seconds_since_epoch())); return false; }