Skip to content

Commit f74cddb

Browse files
yhabteabyhabteab
committed
tlsutility: normalize cert valid for ts on 32-bit systems
1 parent ec26a2d commit f74cddb

File tree

3 files changed

+67
-61
lines changed

3 files changed

+67
-61
lines changed

‎lib/base/tlsutility.cpp‎

Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -478,7 +478,7 @@ std::shared_ptr<X509> GetX509Certificate(const String& pemfile)
478478
return std::shared_ptr<X509>(cert, X509_free);
479479
}
480480

481-
int MakeX509CSR(const String& cn, const String& keyfile, const String& csrfile, const String& certfile, int validFor, bool ca)
481+
int MakeX509CSR(const String& cn, const String& keyfile, const String& csrfile, const String& certfile, long validFor, bool ca)
482482
{
483483
char errbuf[256];
484484

@@ -640,7 +640,7 @@ int MakeX509CSR(const String& cn, const String& keyfile, const String& csrfile,
640640
return 1;
641641
}
642642

643-
std::shared_ptr<X509> CreateCert(EVP_PKEY *pubkey, X509_NAME *subject, X509_NAME *issuer, EVP_PKEY *cakey, int validFor, bool ca)
643+
std::shared_ptr<X509> CreateCert(EVP_PKEY *pubkey, X509_NAME *subject, X509_NAME *issuer, EVP_PKEY *cakey, long validFor, bool ca)
644644
{
645645
X509 *cert = X509_new();
646646
X509_set_version(cert, 2);
@@ -649,7 +649,7 @@ std::shared_ptr<X509> CreateCert(EVP_PKEY *pubkey, X509_NAME *subject, X509_NAME
649649
notBefore = validFor - validFor / 4; // Add 25% of the validity period to the past
650650
}
651651
X509_gmtime_adj(X509_get_notBefore(cert), notBefore);
652-
X509_gmtime_adj(X509_get_notAfter(cert), validFor);
652+
X509_gmtime_adj(X509_get_notAfter(cert), NormalizeCertValidFor(validFor));
653653
X509_set_pubkey(cert, pubkey);
654654

655655
X509_set_subject_name(cert, subject);
@@ -732,7 +732,7 @@ String GetIcingaCADir()
732732
return Configuration::DataDir + "/ca";
733733
}
734734

735-
std::shared_ptr<X509> CreateCertIcingaCA(EVP_PKEY *pubkey, X509_NAME *subject, int validFor, bool ca)
735+
std::shared_ptr<X509> CreateCertIcingaCA(EVP_PKEY *pubkey, X509_NAME *subject, long validFor, bool ca)
736736
{
737737
char errbuf[256];
738738

@@ -779,7 +779,7 @@ std::shared_ptr<X509> CreateCertIcingaCA(EVP_PKEY *pubkey, X509_NAME *subject, i
779779
* @param validFor The validity period in seconds. Defaults to LEAF_VALID_FOR.
780780
* @returns The new X509 certificate or an empty shared_ptr on error.
781781
*/
782-
std::shared_ptr<X509> CreateCertIcingaCA(const std::shared_ptr<X509>& cert, int validFor)
782+
std::shared_ptr<X509> CreateCertIcingaCA(const std::shared_ptr<X509>& cert, long validFor)
783783
{
784784
std::shared_ptr<EVP_PKEY> pkey = std::shared_ptr<EVP_PKEY>(X509_get_pubkey(cert.get()), EVP_PKEY_free);
785785
return CreateCertIcingaCA(pkey.get(), X509_get_subject_name(cert.get()), validFor);
@@ -788,7 +788,7 @@ std::shared_ptr<X509> CreateCertIcingaCA(const std::shared_ptr<X509>& cert, int
788788
static inline
789789
bool CertExpiresWithin(X509* cert, int seconds)
790790
{
791-
time_t renewalStart = time(nullptr) + seconds;
791+
time_t renewalStart = time(nullptr) + NormalizeCertValidFor(seconds);
792792

793793
return X509_cmp_time(X509_get_notAfter(cert), &renewalStart) < 0;
794794
}
@@ -812,6 +812,30 @@ bool IsCaUptodate(X509* cert)
812812
return !CertExpiresWithin(cert, LEAF_VALID_FOR);
813813
}
814814

815+
/**
816+
* Normalizes the specified certificate validity period in seconds to avoid
817+
* issues with 32-bit time_t wrap-around in 2038.
818+
*
819+
* On 64-bit systems this function simply returns the specified value. On 32-bit systems it ensures that
820+
* the current time plus the specified value never overflows the maximum value of time_t. Otherwise, the
821+
* returned value is adjusted so that the sum is exactly the maximum value of time_t.
822+
*
823+
* @param seconds The desired validity period in seconds.
824+
* @returns The normalized validity period in seconds.
825+
*/
826+
long NormalizeCertValidFor(long seconds)
827+
{
828+
if constexpr (sizeof(time_t) > sizeof(int32_t)) {
829+
return seconds;
830+
}
831+
832+
constexpr time_t maxTimeT = static_cast<time_t>(std::numeric_limits<int32_t>::max());
833+
if (auto rem = maxTimeT - time(nullptr); rem < seconds) {
834+
return rem;
835+
}
836+
return seconds;
837+
}
838+
815839
String CertificateToString(X509* cert)
816840
{
817841
BIO *mem = BIO_new(BIO_s_mem());

‎lib/base/tlsutility.hpp‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,8 @@ Shared<boost::asio::ssl::context>::Ptr SetupSslContext(String certPath, String k
5454

5555
String GetCertificateCN(const std::shared_ptr<X509>& certificate);
5656
std::shared_ptr<X509> GetX509Certificate(const String& pemfile);
57-
int MakeX509CSR(const String& cn, const String& keyfile, const String& csrfile = String(), const String& certfile = String(), int validFor = LEAF_VALID_FOR, bool ca = false);
58-
std::shared_ptr<X509> CreateCert(EVP_PKEY *pubkey, X509_NAME *subject, X509_NAME *issuer, EVP_PKEY *cakey, int validFor, bool ca);
57+
int MakeX509CSR(const String& cn, const String& keyfile, const String& csrfile = String(), const String& certfile = String(), long validFor = LEAF_VALID_FOR, bool ca = false);
58+
std::shared_ptr<X509> CreateCert(EVP_PKEY *pubkey, X509_NAME *subject, X509_NAME *issuer, EVP_PKEY *cakey, long validFor, bool ca);
5959

6060
String GetIcingaCADir();
6161
String CertificateToString(X509* cert);
@@ -66,10 +66,11 @@ inline String CertificateToString(const std::shared_ptr<X509>& cert)
6666
}
6767

6868
std::shared_ptr<X509> StringToCertificate(const String& cert);
69-
std::shared_ptr<X509> CreateCertIcingaCA(EVP_PKEY *pubkey, X509_NAME *subject, int validFor, bool ca = false);
70-
std::shared_ptr<X509> CreateCertIcingaCA(const std::shared_ptr<X509>& cert, int validFor = LEAF_VALID_FOR);
69+
std::shared_ptr<X509> CreateCertIcingaCA(EVP_PKEY *pubkey, X509_NAME *subject, long validFor, bool ca = false);
70+
std::shared_ptr<X509> CreateCertIcingaCA(const std::shared_ptr<X509>& cert, long validFor = LEAF_VALID_FOR);
7171
bool IsCertUptodate(const std::shared_ptr<X509>& cert);
7272
bool IsCaUptodate(X509* cert);
73+
long NormalizeCertValidFor(long seconds);
7374

7475
String PBKDF2_SHA1(const String& password, const String& salt, int iterations);
7576
String PBKDF2_SHA256(const String& password, const String& salt, int iterations);

‎test/base-tlsutility.cpp‎

Lines changed: 32 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -70,26 +70,6 @@ static std::shared_ptr<EVP_PKEY> GetEVP_PKEY(const String& keyfile)
7070
return std::shared_ptr<EVP_PKEY>(pkey, EVP_PKEY_free);
7171
}
7272

73-
/**
74-
* Returns a time_t value that is the current time plus the specified number of seconds,
75-
* but capped to max time_t on 32-bit systems to avoid the 2038 problem.
76-
*
77-
* @param timestamp The number of seconds to add to the current time.
78-
* @return The resulting time_t value, capped to max time_t on 32-bit systems.
79-
*/
80-
time_t ValidCertValidity(long timestamp)
81-
{
82-
time_t t = time(nullptr);
83-
if constexpr (sizeof(t) > sizeof(int32_t)) {
84-
t = t + timestamp;
85-
} else {
86-
// On 32-bit systems time_t is 32-bit and thus limited to 2038.
87-
// So, make sure t+timestamp is never greater than max time_t.
88-
t = std::min(t + timestamp, std::numeric_limits<time_t>::max());
89-
}
90-
return t;
91-
}
92-
9373
/**
9474
* Creates a new X509 certificate signed by the Icinga CA based on an existing CA certificate.
9575
*
@@ -111,30 +91,31 @@ BOOST_FIXTURE_TEST_CASE(create_verify_ca, CertificateFixture)
11191
auto cacert(GetX509Certificate(m_CaDir.string()+"/ca.crt"));
11292
if constexpr (OPENSSL_VERSION_NUMBER >= 0x10100000L) {
11393
// OpenSSL 1.1.x provides https://www.openssl.org/docs/man1.1.0/man3/X509_check_ca.html
114-
BOOST_CHECK_EQUAL(true, IsCa(cacert));
94+
BOOST_CHECK(IsCa(cacert));
11595
} else {
11696
BOOST_CHECK_THROW(IsCa(cacert), std::invalid_argument);
11797
}
118-
BOOST_CHECK_EQUAL(true, VerifyCertificate(cacert, cacert, String())); // Self-signed CA!
119-
BOOST_CHECK_EQUAL(true, IsCaUptodate(cacert.get())); // Is CA up-to-date after its creation?
98+
BOOST_CHECK(VerifyCertificate(cacert, cacert, String())); // Self-signed CA!
99+
BOOST_CHECK(IsCaUptodate(cacert.get())); // Is CA up-to-date after its creation?
120100

121-
auto caValidUntil = ValidCertValidity(ROOT_VALID_FOR);
122-
// Due to processing time the expiration date might be off by a few seconds,
123-
// so we just check whether it is less than the expected value and not exactly equal.
124-
BOOST_CHECK_EQUAL(-1, X509_cmp_time(X509_get_notAfter(cacert.get()), &caValidUntil));
101+
time_t caValidUntil = time(nullptr) + NormalizeCertValidFor(ROOT_VALID_FOR);
102+
// On 32-bit systems time_t might be a 32-bit integer, so the value overflows for ROOT_VALID_FOR.
103+
// In this case, the expiration date is probably set to the maximum possible value time_t can hold,
104+
// so must be the exact same ts as caValidUntil. So, on either platform it must be <= caValidUntil.
105+
BOOST_CHECK_LE(X509_cmp_time(X509_get_notAfter(cacert.get()), &caValidUntil), 0);
125106

126107
// Set the CA certificate to expire in 100 days, i.e. less than the LEAF_VALID_FOR threshold of 397 days.
127-
BOOST_CHECK(X509_gmtime_adj(X509_get_notAfter(cacert.get()), 60*60*24*100));
128-
BOOST_CHECK_EQUAL(false, IsCaUptodate(cacert.get())); // Is CA outdated now?
108+
BOOST_CHECK(X509_gmtime_adj(X509_get_notAfter(cacert.get()), NormalizeCertValidFor(60*60*24*100)));
109+
BOOST_CHECK(!IsCaUptodate(cacert.get())); // Is CA outdated now?
129110

130-
BOOST_CHECK(X509_gmtime_adj(X509_get_notAfter(cacert.get()), 60*60*24*397));
111+
BOOST_CHECK(X509_gmtime_adj(X509_get_notAfter(cacert.get()), NormalizeCertValidFor(60*60*24*397)));
131112
// Even if the CA is going to expire at exactly the same time as the LEAF_VALID_FOR threshold,
132113
// it is still considered to be outdated, so IsCaUptodate() should return false.
133-
BOOST_CHECK_EQUAL(false, IsCaUptodate(cacert.get()));
114+
BOOST_CHECK(!IsCaUptodate(cacert.get()));
134115

135116
// Reset the CA expiration date to the original value, i.e. 15 years.
136-
BOOST_CHECK(X509_gmtime_adj(X509_get_notAfter(cacert.get()), ROOT_VALID_FOR));
137-
BOOST_CHECK_EQUAL(true, IsCaUptodate(cacert.get()));
117+
BOOST_CHECK(X509_gmtime_adj(X509_get_notAfter(cacert.get()), NormalizeCertValidFor(ROOT_VALID_FOR)));
118+
BOOST_CHECK(IsCaUptodate(cacert.get()));
138119
}
139120

140121
BOOST_FIXTURE_TEST_CASE(create_verify_leaf_certs, CertificateFixture)
@@ -144,53 +125,53 @@ BOOST_FIXTURE_TEST_CASE(create_verify_leaf_certs, CertificateFixture)
144125

145126
auto caprivatekey(GetEVP_PKEY(caDir+"/ca.key"));
146127
auto cacert(GetX509Certificate(caDir+"/ca.crt"));
147-
BOOST_CHECK_EQUAL(true, IsCaUptodate(cacert.get()));
128+
BOOST_CHECK(IsCaUptodate(cacert.get()));
148129
BOOST_CHECK_EQUAL(1, X509_verify(cacert.get(), caprivatekey.get())); // 1 == equal, 0 == unequal, -1 == error
149130

150131
auto certInfo = CertificateFixture::EnsureCertFor("example.com", true); // Generates example.com.{key,csr,crt} files.
151132

152133
auto cert(GetX509Certificate(certInfo.crtFile));
153134
if constexpr (OPENSSL_VERSION_NUMBER >= 0x10100000L) {
154-
BOOST_CHECK_EQUAL(false, IsCa(cert));
135+
BOOST_CHECK(!IsCa(cert));
155136
} else {
156137
BOOST_CHECK_THROW(IsCa(cert), std::invalid_argument);
157138
}
158-
BOOST_CHECK_EQUAL(true, IsCertUptodate(cert)); // Is leaf up-to-date after its creation?
159-
BOOST_CHECK_EQUAL(true, VerifyCertificate(cacert, cert, String())); // Signed by our CA?
139+
BOOST_CHECK(IsCertUptodate(cert)); // Is leaf up-to-date after its creation?
140+
BOOST_CHECK(VerifyCertificate(cacert, cert, String())); // Signed by our CA?
160141

161-
auto certValidUntil = ValidCertValidity(LEAF_VALID_FOR);
142+
time_t certValidUntil = time(nullptr) + NormalizeCertValidFor(LEAF_VALID_FOR);
162143
// Due to processing time the expiration date might be off by a few seconds,
163144
// so we just check whether it is less than the expected value and not exactly equal.
164-
BOOST_CHECK_EQUAL(-1, X509_cmp_time(X509_get_notAfter(cert.get()), &certValidUntil));
145+
BOOST_CHECK_LE(X509_cmp_time(X509_get_notAfter(cert.get()), &certValidUntil), 0);
165146

166147
// Set the certificate to expire in 20 days, i.e. less than the RENEW_THRESHOLD of 30 days.
167-
BOOST_CHECK(X509_gmtime_adj(X509_get_notAfter(cert.get()), 60*60*24*20));
168-
BOOST_CHECK_EQUAL(false, IsCertUptodate(cert));
148+
BOOST_CHECK(X509_gmtime_adj(X509_get_notAfter(cert.get()), NormalizeCertValidFor(60*60*24*20)));
149+
BOOST_CHECK(!IsCertUptodate(cert));
169150

170151
// Check whether expired certificates are correctly detected and verification fails.
171152
PkiUtility::SignCsr(certInfo.csrFile, certInfo.crtFile, 60*60*24*-10); // Expire 10 days ago!
172153
cert = GetX509Certificate(certInfo.crtFile);
173-
certValidUntil = ValidCertValidity(60*60*24*-10);
154+
certValidUntil = time(nullptr) + NormalizeCertValidFor(60*60*24*-10);
174155
BOOST_CHECK_EQUAL(-1, X509_cmp_time(X509_get_notAfter(cert.get()), &certValidUntil)); // Is certificate indeed expired?
175-
BOOST_CHECK_EQUAL(false, IsCertUptodate(cert)); // It's already expired, so definitely not up-to-date.
156+
BOOST_CHECK(!IsCertUptodate(cert)); // It's already expired, so definitely not up-to-date.
176157
BOOST_CHECK_THROW(VerifyCertificate(cacert, cert, String()), openssl_error);
177158

178159
certInfo = CertificateFixture::EnsureCertFor("example.com", true);
179160
cert = GetX509Certificate(certInfo.crtFile);
180161
// Set the certificate validity start date to 2016, all certificates created before 2017 are considered outdated.
181162
BOOST_CHECK(X509_gmtime_adj(X509_get_notBefore(cert.get()), -(time(nullptr)-l_2016)));
182-
BOOST_CHECK_EQUAL(false, IsCertUptodate(cert));
163+
BOOST_CHECK(!IsCertUptodate(cert));
183164
// ... but verification should still work, as the certificate is still valid.
184-
BOOST_CHECK_EQUAL(true, VerifyCertificate(cacert, cert, String()));
165+
BOOST_CHECK(VerifyCertificate(cacert, cert, String()));
185166

186167
// Reset the certificate validity start date to the least acceptable value, i.e. 2017.
187168
BOOST_CHECK(X509_gmtime_adj(X509_get_notBefore(cert.get()), -(time(nullptr)-l_2017)));
188-
BOOST_CHECK_EQUAL(true, IsCertUptodate(cert));
189-
BOOST_CHECK_EQUAL(true, VerifyCertificate(cacert, cert, String()));
169+
BOOST_CHECK(IsCertUptodate(cert));
170+
BOOST_CHECK(VerifyCertificate(cacert, cert, String()));
190171

191172
cacert = NewCertFromExisting(cacert, 60*60*24*-10, true); // Expire the CA 10 days ago.
192173
BOOST_CHECK_EQUAL(1, X509_verify(cacert.get(), caprivatekey.get())); // 1 == equal, 0 == unequal, -1 == error
193-
BOOST_CHECK_EQUAL(false, IsCaUptodate(cacert.get()));
174+
BOOST_CHECK(!IsCaUptodate(cacert.get()));
194175
BOOST_CHECK_THROW(VerifyCertificate(cacert, cert, String()), openssl_error); // Signature failure!
195176

196177
PkiUtility::SignCsr(certInfo.csrFile, certInfo.crtFile); // Resign the CSR with the (now expired) CA.
@@ -202,9 +183,9 @@ BOOST_FIXTURE_TEST_CASE(create_verify_leaf_certs, CertificateFixture)
202183
auto newCACert = NewCertFromExisting(cacert, ROOT_VALID_FOR, true);
203184
BOOST_REQUIRE(newCACert);
204185
BOOST_CHECK_EQUAL(1, X509_verify(newCACert.get(), caprivatekey.get())); // 1 == equal, 0 == unequal, -1 == error
205-
BOOST_CHECK_EQUAL(true, IsCaUptodate(newCACert.get()));
206-
BOOST_CHECK_EQUAL(true, VerifyCertificate(newCACert, newCACert, String()));
207-
BOOST_CHECK_EQUAL(true, VerifyCertificate(newCACert, cert, String()));
186+
BOOST_CHECK(IsCaUptodate(newCACert.get()));
187+
BOOST_CHECK(VerifyCertificate(newCACert, newCACert, String()));
188+
BOOST_CHECK(VerifyCertificate(newCACert, cert, String()));
208189
//... but verifying the new CA with the old CA or vice versa should fail.
209190
BOOST_CHECK_THROW(VerifyCertificate(cacert, newCACert, String()), openssl_error);
210191
BOOST_CHECK_THROW(VerifyCertificate(newCACert, cacert, String()), openssl_error);

0 commit comments

Comments
 (0)