remove year 2038 check
https://gerrit.openbmc-project.xyz/c/openbmc/bmcweb/+/49188 resolves
the year 2038 problem in BMCWeb. There's no need to check it in
cert-manager anymore. However, the current Cert interface can't take
certificate whose NotBefore is before the Unix Epoch given the
timestamp is uint64_t. So this change adds the check to return
errors in this case.
This change also fixed the existing issue of setting unix epoch.
TESTED: unit tests + QEMU
1. added a cert that's valid from 1970/01/01 to 9999/12/31 into
unit tests
2. tested the dbus properties in QEMU after installing the above
cert;
```
.ValidNotAfter property t 253402300799 emits-change writable
.ValidNotBefore property t 0 emits-change writable
```
This is expected.
Signed-off-by: Nan Zhou <nanzhoumails@gmail.com>
Change-Id: Idc6b7721fc84b6b9022467e6b0c9e1984f682912
diff --git a/certificate.cpp b/certificate.cpp
index 8bdd858..de01f4c 100644
--- a/certificate.cpp
+++ b/certificate.cpp
@@ -348,7 +348,7 @@
elog<InvalidCertificate>(Reason("Certificate validation failed"));
}
- validateCertificateExpiryDate(cert);
+ validateCertificateStartDate(cert);
// Verify that the certificate can be used in a TLS context
const SSL_METHOD* method = TLS_method();
@@ -426,28 +426,25 @@
}
}
-void Certificate::validateCertificateExpiryDate(const X509_Ptr& cert)
+// Checks that notBefore is not earlier than the unix epoch given that
+// the corresponding DBus interface is uint64_t.
+void Certificate::validateCertificateStartDate(const X509_Ptr& cert)
{
int days = 0;
int secs = 0;
ASN1_TIME_ptr epoch(ASN1_TIME_new(), ASN1_STRING_free);
- // Set time to 12:00am GMT, Jan 1 1970
- ASN1_TIME_set_string(epoch.get(), "700101120000Z");
+ // Set time to 00:00am GMT, Jan 1 1970; format: YYYYMMDDHHMMSSZ
+ ASN1_TIME_set_string(epoch.get(), "19700101000000Z");
- ASN1_TIME* notAfter = X509_get_notAfter(cert.get());
- ASN1_TIME_diff(&days, &secs, epoch.get(), notAfter);
+ ASN1_TIME* notBefore = X509_get_notBefore(cert.get());
+ ASN1_TIME_diff(&days, &secs, epoch.get(), notBefore);
- static const int dayToSeconds = 24 * 60 * 60;
-
- // TODO #issue15 - allow only upto year 2038 which time_t supports for now
- // time_t is defined as int32 so any expiry date greater than 2038 will
- // cause the time_t variable overflow resulting in -ve number.
- if (days > (INT_MAX - secs) / dayToSeconds)
+ if (days < 0 || secs < 0)
{
- log<level::ERR>("Certificate expiry date is beyond year 2038",
- entry("DAYS=%d", days));
- elog<InvalidCertificate>(Reason("Expiry date should be below 2038"));
+ log<level::ERR>("Certificate valid date starts before the Unix Epoch");
+ elog<InvalidCertificate>(
+ Reason("NotBefore should after 19700101000000Z"));
}
}
@@ -560,10 +557,10 @@
int secs = 0;
ASN1_TIME_ptr epoch(ASN1_TIME_new(), ASN1_STRING_free);
- // Set time to 12:00am GMT, Jan 1 1970
- ASN1_TIME_set_string(epoch.get(), "700101120000Z");
+ // Set time to 00:00am GMT, Jan 1 1970; format: YYYYMMDDHHMMSSZ
+ ASN1_TIME_set_string(epoch.get(), "19700101000000Z");
- static const uint32_t dayToSeconds = 24 * 60 * 60;
+ static const uint64_t dayToSeconds = 24 * 60 * 60;
ASN1_TIME* notAfter = X509_get_notAfter(cert.get());
ASN1_TIME_diff(&days, &secs, epoch.get(), notAfter);
CertificateIface::validNotAfter((days * dayToSeconds) + secs);
diff --git a/certificate.hpp b/certificate.hpp
index 2a09e69..27ac488 100644
--- a/certificate.hpp
+++ b/certificate.hpp
@@ -120,16 +120,16 @@
private:
/**
- * @brief Return error if ceritificate expiry date is gt 2038
+ * @brief Return error if ceritificate NotBefore date is lt 1970
*
- * Parse the certificate and return error if certificate expiry date
- * is gt 2038.
+ * Parse the certificate and return error if certificate NotBefore date
+ * is lt 1970.
*
* @param[in] cert Reference to certificate object uploaded
*
* @return void
*/
- void validateCertificateExpiryDate(const X509_Ptr& cert);
+ void validateCertificateStartDate(const X509_Ptr& cert);
/**
* @brief Populate certificate properties by parsing given certificate file
diff --git a/test/certs_manager_test.cpp b/test/certs_manager_test.cpp
index 4ffa667..c8f0719 100644
--- a/test/certs_manager_test.cpp
+++ b/test/certs_manager_test.cpp
@@ -56,6 +56,7 @@
fs::remove(certificateFile);
fs::remove(CSRFile);
fs::remove(privateKeyFile);
+ fs::remove_all("demoCA");
}
void createNewCertificate(bool setNewCertId = false)
@@ -65,7 +66,7 @@
privateKeyFile = "privkey.pem";
rsaPrivateKeyFilePath = certDir + "/.rsaprivkey.pem";
std::string cmd = "openssl req -x509 -sha256 -newkey rsa:2048 ";
- cmd += "-keyout cert.pem -out cert.pem -days 3650 -nodes";
+ cmd += "-keyout cert.pem -out cert.pem -days 365000 -nodes";
cmd += " -subj /O=openbmc-project.xyz/CN=localhost";
if (setNewCertId)
@@ -80,6 +81,38 @@
}
}
+ void createNeverExpiredRootCertificate()
+ {
+ // remove the old cert
+ fs::remove(certificateFile);
+
+ // The following routines create a cert that has NotBefore
+ // set to 1970/01/01 and NotAfter set to 9999/12/31 via the
+ // OpenSSL CA application.
+ certificateFile = "cert.pem";
+ ASSERT_EQ(std::system("mkdir -p demoCA"), 0);
+ ASSERT_EQ(std::system("mkdir -p demoCA/private/"), 0);
+ ASSERT_EQ(std::system("mkdir -p demoCA/newcerts/"), 0);
+ ASSERT_EQ(std::system("touch demoCA/index.txt"), 0);
+ ASSERT_EQ(std::system("echo 1000 > demoCA/serial"), 0);
+ ASSERT_EQ(
+ std::system(
+ "openssl req -x509 -sha256 -newkey rsa:2048 -keyout "
+ "demoCA/private/cakey.pem -out demoCA/cacert.pem -nodes "
+ "-subj /O=openbmc-project.xyz/C=US/ST=CA/CN=localhost-ca"),
+ 0);
+ ASSERT_EQ(std::system(
+ "openssl req -new -newkey rsa:2048 -nodes -keyout "
+ "demoCA/server.key -out demoCA/server.csr -subj "
+ "/O=openbmc-project.xyz/C=US/ST=CA/CN=localhost-server"),
+ 0);
+ ASSERT_EQ(
+ std::system(
+ "openssl ca -batch -startdate 19700101000000Z -enddate "
+ "99991231235959Z -out cert.pem -infiles demoCA/server.csr"),
+ 0);
+ }
+
bool compareFiles(const std::string& file1, const std::string& file2)
{
std::ifstream f1(file1, std::ifstream::binary | std::ifstream::ate);
@@ -242,12 +275,57 @@
Manager manager(bus, event, objPath.c_str(), type, std::move(unit),
std::move(certDir));
MainApp mainApp(&manager);
+ // install the default certificate that's valid from today to 100 years
+ // later
mainApp.install(certificateFile);
std::vector<std::unique_ptr<Certificate>>& certs =
manager.getCertificates();
- EXPECT_FALSE(certs.empty());
+ ASSERT_EQ(certs.size(), 1);
+ // check some attributes as well
+ EXPECT_EQ(certs.front()->validNotAfter() - certs.front()->validNotBefore(),
+ 365000ULL * 24 * 3600);
+ EXPECT_EQ(certs.front()->subject(), "O=openbmc-project.xyz,CN=localhost");
+ EXPECT_EQ(certs.front()->issuer(), "O=openbmc-project.xyz,CN=localhost");
+
+ std::string verifyPath =
+ verifyDir + "/" + getCertSubjectNameHash(certificateFile) + ".0";
+
+ // Check that certificate has been created at installation directory
+ EXPECT_FALSE(fs::is_empty(verifyDir));
+ EXPECT_TRUE(fs::exists(verifyPath));
+
+ // Check that installed cert is identical to input one
+ EXPECT_TRUE(compareFiles(certificateFile, verifyPath));
+}
+
+/** @brief Check if storage install routine is invoked for storage setup
+ */
+TEST_F(TestCertificates, InvokeAuthorityInstallNeverExpiredRootCert)
+{
+ std::string endpoint("ldap");
+ std::string unit("");
+ std::string type("authority");
+ std::string verifyDir(certDir);
+ UnitsToRestart verifyUnit(unit);
+ auto objPath = std::string(OBJPATH) + '/' + type + '/' + endpoint;
+ auto event = sdeventplus::Event::get_default();
+ // Attach the bus to sd_event to service user requests
+ bus.attach_event(event.get(), SD_EVENT_PRIORITY_NORMAL);
+ Manager manager(bus, event, objPath.c_str(), type, std::move(unit),
+ std::move(certDir));
+ MainApp mainApp(&manager);
+
+ // install the certificate that's valid from the Unix Epoch to Dec 31, 9999
+ createNeverExpiredRootCertificate();
+ mainApp.install(certificateFile);
+
+ std::vector<std::unique_ptr<Certificate>>& certs =
+ manager.getCertificates();
+
+ EXPECT_EQ(certs.front()->validNotBefore(), 0);
+ EXPECT_EQ(certs.front()->validNotAfter(), 253402300799ULL);
std::string verifyPath =
verifyDir + "/" + getCertSubjectNameHash(certificateFile) + ".0";