From b1db3456d1814ebe10b3c3613f28a81a1579949c Mon Sep 17 00:00:00 2001 From: Patrick Vacek Date: Tue, 11 Aug 2020 15:34:31 +0200 Subject: [PATCH 01/10] Allow re-importing the client TLS certifcate and key. We already allow it for the root CA. Signed-off-by: Patrick Vacek --- src/libaktualizr/storage/invstorage.cc | 9 ++++----- src/libaktualizr/storage/storage_common_test.cc | 9 +++++---- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/libaktualizr/storage/invstorage.cc b/src/libaktualizr/storage/invstorage.cc index 1969c56192..96a229972c 100644 --- a/src/libaktualizr/storage/invstorage.cc +++ b/src/libaktualizr/storage/invstorage.cc @@ -86,13 +86,12 @@ void INvStorage::importInstalledVersions(const boost::filesystem::path& base_pat void INvStorage::importData(const ImportConfig& import_config) { importPrimaryKeys(import_config.base_path, import_config.uptane_public_key_path, import_config.uptane_private_key_path); - // root CA certificate can be updated importUpdateSimple(import_config.base_path, &INvStorage::storeTlsCa, &INvStorage::loadTlsCa, import_config.tls_cacert_path); - importSimple(import_config.base_path, &INvStorage::storeTlsCert, &INvStorage::loadTlsCert, - import_config.tls_clientcert_path); - importSimple(import_config.base_path, &INvStorage::storeTlsPkey, &INvStorage::loadTlsPkey, - import_config.tls_pkey_path); + importUpdateSimple(import_config.base_path, &INvStorage::storeTlsCert, &INvStorage::loadTlsCert, + import_config.tls_clientcert_path); + importUpdateSimple(import_config.base_path, &INvStorage::storeTlsPkey, &INvStorage::loadTlsPkey, + import_config.tls_pkey_path); importInstalledVersions(import_config.base_path); } diff --git a/src/libaktualizr/storage/storage_common_test.cc b/src/libaktualizr/storage/storage_common_test.cc index dabf8954a9..1f657b84b3 100644 --- a/src/libaktualizr/storage/storage_common_test.cc +++ b/src/libaktualizr/storage/storage_common_test.cc @@ -526,7 +526,8 @@ TEST(storage, load_store_secondary_info) { EXPECT_EQ(sec_infos[0].extra, "data1"); } -/* Import keys and credentials from file into storage. */ +/* Import keys and credentials from file into storage. + * Re-import updated credentials from file into storage. */ TEST(storage, import_data) { TemporaryDirectory temp_dir; std::unique_ptr storage = Storage(temp_dir.Path()); @@ -592,12 +593,12 @@ TEST(storage, import_data) { EXPECT_TRUE(storage->loadTlsCert(&tls_cert)); EXPECT_TRUE(storage->loadTlsPkey(&tls_pkey)); - // only root cert is being updated + // All TLS objects should be updated: EXPECT_EQ(primary_private, "uptane_private_1"); EXPECT_EQ(primary_public, "uptane_public_1"); EXPECT_EQ(tls_ca, "tls_cacert_2"); - EXPECT_EQ(tls_cert, "tls_cert_1"); - EXPECT_EQ(tls_pkey, "tls_pkey_1"); + EXPECT_EQ(tls_cert, "tls_cert_2"); + EXPECT_EQ(tls_pkey, "tls_pkey_2"); } #ifndef __NO_MAIN__ From 4858c9affc98c3d0112502ed20e1ff3e01a188ef Mon Sep 17 00:00:00 2001 From: Patrick Vacek Date: Thu, 13 Aug 2020 15:14:22 +0200 Subject: [PATCH 02/10] Print more import details to the log at debug level. Signed-off-by: Patrick Vacek --- src/libaktualizr/storage/invstorage.cc | 19 ++++++++++++------- src/libaktualizr/storage/invstorage.h | 4 ++-- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/src/libaktualizr/storage/invstorage.cc b/src/libaktualizr/storage/invstorage.cc index 96a229972c..059af544a5 100644 --- a/src/libaktualizr/storage/invstorage.cc +++ b/src/libaktualizr/storage/invstorage.cc @@ -8,20 +8,22 @@ #include "utilities/utils.h" void INvStorage::importSimple(const boost::filesystem::path& base_path, store_data_t store_func, load_data_t load_func, - const utils::BasedPath& imported_data_path) { + const utils::BasedPath& imported_data_path, const std::string& data_name) { if (!(this->*load_func)(nullptr) && !imported_data_path.empty()) { boost::filesystem::path abs_path = imported_data_path.get(base_path); if (!boost::filesystem::exists(abs_path)) { - LOG_ERROR << "Couldn't import data: " << abs_path << " doesn't exist."; + LOG_ERROR << "Couldn't import " << data_name << ": " << abs_path << " doesn't exist."; return; } std::string content = Utils::readFile(abs_path.string()); (this->*store_func)(content); + LOG_DEBUG << "Successfully imported " << data_name << " from " << abs_path; } } void INvStorage::importUpdateSimple(const boost::filesystem::path& base_path, store_data_t store_func, - load_data_t load_func, const utils::BasedPath& imported_data_path) { + load_data_t load_func, const utils::BasedPath& imported_data_path, + const std::string& data_name) { std::string prev_content; std::string content; bool update = false; @@ -37,13 +39,14 @@ void INvStorage::importUpdateSimple(const boost::filesystem::path& base_path, st if (update && !imported_data_path.empty()) { boost::filesystem::path abs_path = imported_data_path.get(base_path); if (!boost::filesystem::exists(abs_path)) { - LOG_ERROR << "Couldn't import data: " << abs_path << " doesn't exist."; + LOG_ERROR << "Couldn't import " << data_name << ": " << abs_path << " doesn't exist."; return; } if (content.empty()) { content = Utils::readFile(abs_path.string()); } (this->*store_func)(content); + LOG_DEBUG << "Successfully imported " << data_name << " from " << abs_path; } } @@ -65,6 +68,7 @@ void INvStorage::importPrimaryKeys(const boost::filesystem::path& base_path, con const std::string pub_content = Utils::readFile(pubkey_abs_path.string()); const std::string priv_content = Utils::readFile(privkey_abs_path.string()); storePrimaryKeys(pub_content, priv_content); + LOG_DEBUG << "Successfully imported Uptane keys from " << pubkey_abs_path << " and " << privkey_abs_path; } void INvStorage::importInstalledVersions(const boost::filesystem::path& base_path) { @@ -80,6 +84,7 @@ void INvStorage::importInstalledVersions(const boost::filesystem::path& base_pat // installed versions in legacy fs storage are all for primary savePrimaryInstalledVersion(installed_versions[current_index], InstalledVersionUpdateMode::kCurrent); boost::filesystem::remove(file_path); + LOG_DEBUG << "Successfully imported installed versions from " << file_path; } } @@ -87,11 +92,11 @@ void INvStorage::importData(const ImportConfig& import_config) { importPrimaryKeys(import_config.base_path, import_config.uptane_public_key_path, import_config.uptane_private_key_path); importUpdateSimple(import_config.base_path, &INvStorage::storeTlsCa, &INvStorage::loadTlsCa, - import_config.tls_cacert_path); + import_config.tls_cacert_path, "server CA certificate"); importUpdateSimple(import_config.base_path, &INvStorage::storeTlsCert, &INvStorage::loadTlsCert, - import_config.tls_clientcert_path); + import_config.tls_clientcert_path, "client certificate"); importUpdateSimple(import_config.base_path, &INvStorage::storeTlsPkey, &INvStorage::loadTlsPkey, - import_config.tls_pkey_path); + import_config.tls_pkey_path, "client TLS key"); importInstalledVersions(import_config.base_path); } diff --git a/src/libaktualizr/storage/invstorage.h b/src/libaktualizr/storage/invstorage.h index 67c4b292f7..becd899af6 100644 --- a/src/libaktualizr/storage/invstorage.h +++ b/src/libaktualizr/storage/invstorage.h @@ -167,9 +167,9 @@ class INvStorage { private: void importSimple(const boost::filesystem::path& base_path, store_data_t store_func, load_data_t load_func, - const utils::BasedPath& imported_data_path); + const utils::BasedPath& imported_data_path, const std::string& data_name); void importUpdateSimple(const boost::filesystem::path& base_path, store_data_t store_func, load_data_t load_func, - const utils::BasedPath& imported_data_path); + const utils::BasedPath& imported_data_path, const std::string& data_name); void importPrimaryKeys(const boost::filesystem::path& base_path, const utils::BasedPath& import_pubkey_path, const utils::BasedPath& import_privkey_path); From d587efafaf673abe1b9790502be362a86e49cc4e Mon Sep 17 00:00:00 2001 From: Patrick Vacek Date: Thu, 13 Aug 2020 15:14:53 +0200 Subject: [PATCH 03/10] cert-provider: Allow specifying device ID even without a fleet CA. This enables the shared credential hack to help test cert replacement. Note that the relevant parameter is --certificate-cn because that's how it's used for the proper fleet CA method. Signed-off-by: Patrick Vacek --- src/cert_provider/main.cc | 40 +++++++++++++++++++-------------------- 1 file changed, 19 insertions(+), 21 deletions(-) diff --git a/src/cert_provider/main.cc b/src/cert_provider/main.cc index 3c7038d8a4..aaaab777a0 100644 --- a/src/cert_provider/main.cc +++ b/src/cert_provider/main.cc @@ -45,12 +45,12 @@ bpo::variables_map parseOptions(int argc, char** argv) { ("certificate-c", bpo::value(), "value for C field in certificate subject name") ("certificate-st", bpo::value(), "value for ST field in certificate subject name") ("certificate-o", bpo::value(), "value for O field in certificate subject name") - ("certificate-cn", bpo::value(), "value for CN field in certificate subject name") + ("certificate-cn", bpo::value(), "value for CN field in certificate subject name (used for device ID)") ("target,t", bpo::value(), "target device to scp credentials to (or [user@]host)") ("port,p", bpo::value(), "target port") - ("directory,d", bpo::value(), "directory on target to write credentials to (conflicts with -config)") - ("root-ca,r", "provide root CA") - ("server-url,u", "provide server url file") + ("directory,d", bpo::value(), "directory on target to write credentials to (conflicts with --config)") + ("root-ca,r", "provide root CA certificate") + ("server-url,u", "provide server URL file") ("local,l", bpo::value(), "local directory to write credentials to") ("config,g", bpo::value >()->composing(), "configuration file or directory from which to get file names") ("skip-checks,s", "skip strict host key checking for ssh/scp commands"); @@ -95,7 +95,7 @@ bpo::variables_map parseOptions(int argc, char** argv) { return false; \ } bool generateAndSign(const std::string& cacert_path, const std::string& capkey_path, std::string* pkey, - std::string* cert, const bpo::variables_map& commandline_map) { + std::string* cert, const bpo::variables_map& commandline_map, const std::string& newcert_cn) { int rsa_bits = 2048; if (commandline_map.count("bits") != 0) { rsa_bits = (commandline_map["bits"].as()); @@ -137,17 +137,6 @@ bool generateAndSign(const std::string& cacert_path, const std::string& capkey_p } }; - std::string newcert_cn; - if (commandline_map.count("certificate-cn") != 0) { - newcert_cn = (commandline_map["certificate-cn"].as()); - if (newcert_cn.empty()) { - std::cerr << "Common name (--certificate-cn) can't be empty" << std::endl; - return false; - } - } else { - newcert_cn = Utils::genPrettyName(); - } - // read CA certificate std::string cacert_contents = Utils::readFile(cacert_path); StructGuard bio_in_cacert(BIO_new_mem_buf(cacert_contents.c_str(), static_cast(cacert_contents.size())), @@ -406,6 +395,18 @@ int main(int argc, char* argv[]) { serverUrl = Bootstrap::readServerUrl(credentials_path); } + std::string device_id; + if (commandline_map.count("certificate-cn") != 0) { + device_id = (commandline_map["certificate-cn"].as()); + if (device_id.empty()) { + std::cerr << "Common name (device ID, --certificate-cn) can't be empty" << std::endl; + return EXIT_FAILURE; + } + } else { + device_id = Utils::genPrettyName(); + std::cout << "Random device ID is " << device_id << "\n"; + } + boost::filesystem::path directory = "/var/sota/import"; utils::BasedPath pkey_file = utils::BasedPath("pkey.pem"); utils::BasedPath cert_file = utils::BasedPath("client.pem"); @@ -458,10 +459,6 @@ int main(int argc, char* argv[]) { std::string ca; if (fleet_ca_path.empty()) { // no fleet CA => provision with shared credentials - - std::string device_id = Utils::genPrettyName(); - std::cout << "Random device ID is " << device_id << "\n"; - Bootstrap boot(credentials_path, ""); HttpClient http; Json::Value data; @@ -490,7 +487,8 @@ int main(int argc, char* argv[]) { return EXIT_FAILURE; } } else { // fleet CA set => generate and sign a new certificate - if (!generateAndSign(fleet_ca_path.native(), fleet_ca_key_path.native(), &pkey, &cert, commandline_map)) { + if (!generateAndSign(fleet_ca_path.native(), fleet_ca_key_path.native(), &pkey, &cert, commandline_map, + device_id)) { return EXIT_FAILURE; } From 090677d2dc1ab2159801398c1d7fcb9a58e39867 Mon Sep 17 00:00:00 2001 From: Patrick Vacek Date: Tue, 18 Aug 2020 18:21:18 +0200 Subject: [PATCH 04/10] Remove redundant common name parsing code. This logic is now only in once place (in the Crypto class) and the KeyManager function just uses that directly. Signed-off-by: Patrick Vacek --- src/libaktualizr/crypto/crypto.cc | 10 +++++----- src/libaktualizr/crypto/crypto.h | 2 +- src/libaktualizr/crypto/crypto_test.cc | 3 +-- src/libaktualizr/crypto/keymanager.cc | 17 ++--------------- 4 files changed, 9 insertions(+), 23 deletions(-) diff --git a/src/libaktualizr/crypto/crypto.cc b/src/libaktualizr/crypto/crypto.cc index 83275579a1..f70b858dc1 100644 --- a/src/libaktualizr/crypto/crypto.cc +++ b/src/libaktualizr/crypto/crypto.cc @@ -313,21 +313,20 @@ bool Crypto::parseP12(BIO *p12_bio, const std::string &p12_password, std::string return true; } -bool Crypto::extractSubjectCN(const std::string &cert, std::string *cn) { +std::string Crypto::extractSubjectCN(const std::string &cert) { StructGuard bio(BIO_new_mem_buf(const_cast(cert.c_str()), static_cast(cert.size())), BIO_vfree); StructGuard x(PEM_read_bio_X509(bio.get(), nullptr, nullptr, nullptr), X509_free); if (x == nullptr) { - return false; + throw std::runtime_error("Could not parse certificate"); } int len = X509_NAME_get_text_by_NID(X509_get_subject_name(x.get()), NID_commonName, nullptr, 0); if (len < 0) { - return false; + throw std::runtime_error("Could not get CN from certificate"); } boost::scoped_array buf(new char[len + 1]); X509_NAME_get_text_by_NID(X509_get_subject_name(x.get()), NID_commonName, buf.get(), len + 1); - *cn = std::string(buf.get()); - return true; + return std::string(buf.get()); } StructGuard Crypto::generateRSAKeyPairEVP(KeyType key_type) { @@ -447,6 +446,7 @@ bool Crypto::IsRsaKeyType(KeyType type) { return false; } } + KeyType Crypto::IdentifyRSAKeyType(const std::string &public_key_pem) { StructGuard bufio(BIO_new_mem_buf(reinterpret_cast(public_key_pem.c_str()), static_cast(public_key_pem.length())), diff --git a/src/libaktualizr/crypto/crypto.h b/src/libaktualizr/crypto/crypto.h index 3d145afbf0..6e5c206d1b 100644 --- a/src/libaktualizr/crypto/crypto.h +++ b/src/libaktualizr/crypto/crypto.h @@ -80,7 +80,7 @@ class Crypto { static std::string ED25519Sign(const std::string &private_key, const std::string &message); static bool parseP12(BIO *p12_bio, const std::string &p12_password, std::string *out_pkey, std::string *out_cert, std::string *out_ca); - static bool extractSubjectCN(const std::string &cert, std::string *cn); + static std::string extractSubjectCN(const std::string &cert); static StructGuard generateRSAKeyPairEVP(KeyType key_type); static bool generateRSAKeyPair(KeyType key_type, std::string *public_key, std::string *private_key); static bool generateEDKeyPair(std::string *public_key, std::string *private_key); diff --git a/src/libaktualizr/crypto/crypto_test.cc b/src/libaktualizr/crypto/crypto_test.cc index 1541952824..110164e660 100644 --- a/src/libaktualizr/crypto/crypto_test.cc +++ b/src/libaktualizr/crypto/crypto_test.cc @@ -101,8 +101,7 @@ TEST(crypto, certificate_pkcs11) { EXPECT_TRUE(res); if (!res) return; - std::string device_name; - EXPECT_TRUE(Crypto::extractSubjectCN(cert, &device_name)); + const std::string device_name = Crypto::extractSubjectCN(cert); EXPECT_EQ(device_name, "cc34f7f3-481d-443b-bceb-e838a36a2d1f"); } #endif diff --git a/src/libaktualizr/crypto/keymanager.cc b/src/libaktualizr/crypto/keymanager.cc index d84ae5941f..6f8a5a207c 100644 --- a/src/libaktualizr/crypto/keymanager.cc +++ b/src/libaktualizr/crypto/keymanager.cc @@ -162,7 +162,7 @@ std::string KeyManager::getCa() const { } std::string KeyManager::getCN() const { - std::string not_found_cert_message = "Certificate is not found, can't extract device_id"; + const std::string not_found_cert_message = "Certificate is not found, can't extract device_id"; std::string cert; if (config_.tls_cert_source == CryptoSource::kFile) { if (!backend_->loadTlsCert(&cert)) { @@ -177,20 +177,7 @@ std::string KeyManager::getCN() const { } } - StructGuard bio(BIO_new_mem_buf(const_cast(cert.c_str()), static_cast(cert.size())), BIO_vfree); - StructGuard x(PEM_read_bio_X509(bio.get(), nullptr, nullptr, nullptr), X509_free); - if (x == nullptr) { - throw std::runtime_error("Could not parse certificate"); - } - - int len = X509_NAME_get_text_by_NID(X509_get_subject_name(x.get()), NID_commonName, nullptr, 0); - if (len < 0) { - throw std::runtime_error("Could not get CN from certificate"); - } - boost::scoped_array buf(new char[len + 1]); - X509_NAME_get_text_by_NID(X509_get_subject_name(x.get()), NID_commonName, buf.get(), len + 1); - std::string cn(buf.get()); - return cn; + return Crypto::extractSubjectCN(cert); } void KeyManager::getCertInfo(std::string *subject, std::string *issuer, std::string *not_before, From a20ac4c0255c0c98813562b27b2d2cdbf700de38 Mon Sep 17 00:00:00 2001 From: Patrick Vacek Date: Wed, 19 Aug 2020 14:01:45 +0200 Subject: [PATCH 05/10] Move cert generation code to Crypto class. Also refactor to use RSA_generate_key_ex instead of the deprecated RSA_generate_key. Signed-off-by: Patrick Vacek --- src/cert_provider/CMakeLists.txt | 11 +- src/cert_provider/main.cc | 218 ++++++------------------------ src/libaktualizr/crypto/crypto.cc | 143 ++++++++++++++++++++ src/libaktualizr/crypto/crypto.h | 4 + 4 files changed, 191 insertions(+), 185 deletions(-) diff --git a/src/cert_provider/CMakeLists.txt b/src/cert_provider/CMakeLists.txt index e90c4a2ccb..6e429b424b 100644 --- a/src/cert_provider/CMakeLists.txt +++ b/src/cert_provider/CMakeLists.txt @@ -1,5 +1,7 @@ +set(CERT_PROVIDER_SRC main.cc) + # set the name of the executable -add_executable(aktualizr-cert-provider main.cc +add_executable(aktualizr-cert-provider ${CERT_PROVIDER_SRC} $ $ $ @@ -9,8 +11,6 @@ add_executable(aktualizr-cert-provider main.cc $ $) -set_source_files_properties(main.cc PROPERTIES COMPILE_FLAGS -Wno-deprecated-declarations) - target_link_libraries(aktualizr-cert-provider ${CMAKE_THREAD_LIBS_INIT} ${Boost_SYSTEM_LIBRARIES} @@ -23,8 +23,6 @@ target_link_libraries(aktualizr-cert-provider ${sodium_LIBRARY_RELEASE} ) -aktualizr_source_file_checks(main.cc) - add_dependencies(build_tests aktualizr-cert-provider) install(TARGETS aktualizr-cert-provider RUNTIME DESTINATION bin COMPONENT garage_deploy) @@ -46,8 +44,7 @@ add_test(NAME aktualizr-cert-provider-option-version COMMAND aktualizr-cert-provider --version) set_tests_properties(aktualizr-cert-provider-option-version PROPERTIES PASS_REGULAR_EXPRESSION "Current aktualizr-cert-provider version is: ${AKTUALIZR_VERSION}") -aktualizr_source_file_checks(${AKTUALIZR_CERT_PROVIDER_SRC} - ${AKTUALIZR_CERT_HEADERS} +aktualizr_source_file_checks(${CERT_PROVIDER_SRC} cert_provider_shared_cred_test.cc cert_provider_test.cc cert_provider_test.h) diff --git a/src/cert_provider/main.cc b/src/cert_provider/main.cc index aaaab777a0..30290d1c11 100644 --- a/src/cert_provider/main.cc +++ b/src/cert_provider/main.cc @@ -1,9 +1,7 @@ #include #include -#include #include #include -#include #include #include @@ -88,179 +86,6 @@ bpo::variables_map parseOptions(int argc, char** argv) { return vm; } -// NOLINTNEXTLINE(cppcoreguidelines-macro-usage) -#define SSL_ERROR(description) \ - { \ - std::cerr << (description) << ERR_error_string(ERR_get_error(), nullptr) << std::endl; \ - return false; \ - } -bool generateAndSign(const std::string& cacert_path, const std::string& capkey_path, std::string* pkey, - std::string* cert, const bpo::variables_map& commandline_map, const std::string& newcert_cn) { - int rsa_bits = 2048; - if (commandline_map.count("bits") != 0) { - rsa_bits = (commandline_map["bits"].as()); - if (rsa_bits < 31) { // sic! - std::cerr << "RSA key size can't be smaller than 31 bits" << std::endl; - return false; - } - } - - int cert_days = 365; - if (commandline_map.count("days") != 0) { - cert_days = (commandline_map["days"].as()); - } - - std::string newcert_c; - if (commandline_map.count("certificate-c") != 0) { - newcert_c = (commandline_map["certificate-c"].as()); - if (newcert_c.length() != 2) { - std::cerr << "Country code (--certificate-c) should be 2 characters long" << std::endl; - return false; - } - }; - - std::string newcert_st; - if (commandline_map.count("certificate-st") != 0) { - newcert_st = (commandline_map["certificate-st"].as()); - if (newcert_st.empty()) { - std::cerr << "State name (--certificate-st) can't be empty" << std::endl; - return false; - } - }; - - std::string newcert_o; - if (commandline_map.count("certificate-o") != 0) { - newcert_o = (commandline_map["certificate-o"].as()); - if (newcert_o.empty()) { - std::cerr << "Organization name (--certificate-o) can't be empty" << std::endl; - return false; - } - }; - - // read CA certificate - std::string cacert_contents = Utils::readFile(cacert_path); - StructGuard bio_in_cacert(BIO_new_mem_buf(cacert_contents.c_str(), static_cast(cacert_contents.size())), - BIO_free_all); - StructGuard ca_certificate(PEM_read_bio_X509(bio_in_cacert.get(), nullptr, nullptr, nullptr), X509_free); - if (ca_certificate.get() == nullptr) { - std::cerr << "Reading CA certificate failed.\n"; - return false; - } - - // read CA private key - std::string capkey_contents = Utils::readFile(capkey_path); - StructGuard bio_in_capkey(BIO_new_mem_buf(capkey_contents.c_str(), static_cast(capkey_contents.size())), - BIO_free_all); - StructGuard ca_privkey(PEM_read_bio_PrivateKey(bio_in_capkey.get(), nullptr, nullptr, nullptr), - EVP_PKEY_free); - if (ca_privkey.get() == nullptr) SSL_ERROR("PEM_read_bio_PrivateKey failed: "); - - // create certificate - StructGuard certificate(X509_new(), X509_free); - if (certificate.get() == nullptr) SSL_ERROR("X509_new failed: "); - - X509_set_version(certificate.get(), 2); // X509v3 - - { - std::random_device urandom; - std::uniform_int_distribution<> serial_dist(0, (1UL << 20) - 1); - ASN1_INTEGER_set(X509_get_serialNumber(certificate.get()), serial_dist(urandom)); - } - - // create and set certificate subject name - StructGuard subj(X509_NAME_new(), X509_NAME_free); - if (subj.get() == nullptr) SSL_ERROR("X509_NAME_new failed: "); - - if (!newcert_c.empty()) { - if (X509_NAME_add_entry_by_txt(subj.get(), "C", MBSTRING_ASC, - reinterpret_cast(newcert_c.c_str()), -1, -1, 0) == 0) - SSL_ERROR("X509_NAME_add_entry_by_txt failed: "); - } - - if (!newcert_st.empty()) { - if (X509_NAME_add_entry_by_txt(subj.get(), "ST", MBSTRING_ASC, - reinterpret_cast(newcert_st.c_str()), -1, -1, 0) == 0) - SSL_ERROR("X509_NAME_add_entry_by_txt failed: "); - } - - if (!newcert_o.empty()) { - if (X509_NAME_add_entry_by_txt(subj.get(), "O", MBSTRING_ASC, - reinterpret_cast(newcert_o.c_str()), -1, -1, 0) == 0) - SSL_ERROR("X509_NAME_add_entry_by_txt failed: "); - } - - assert(!newcert_cn.empty()); - if (X509_NAME_add_entry_by_txt(subj.get(), "CN", MBSTRING_ASC, - reinterpret_cast(newcert_cn.c_str()), -1, -1, 0) == 0) - SSL_ERROR("X509_NAME_add_entry_by_txt failed: "); - - if (X509_set_subject_name(certificate.get(), subj.get()) == 0) SSL_ERROR("X509_set_subject_name failed: "); - - // set issuer name - X509_NAME* ca_subj = X509_get_subject_name(ca_certificate.get()); - if (ca_subj == nullptr) SSL_ERROR("X509_get_subject_name failed: "); - - if (X509_set_issuer_name(certificate.get(), ca_subj) == 0) SSL_ERROR("X509_set_issuer_name failed: "); - - // create and set key - - // freed by owner EVP_PKEY - RSA* certificate_rsa = RSA_generate_key(rsa_bits, RSA_F4, nullptr, nullptr); - if (certificate_rsa == nullptr) SSL_ERROR("RSA_generate_key failed: "); - - StructGuard certificate_pkey(EVP_PKEY_new(), EVP_PKEY_free); - if (certificate_pkey.get() == nullptr) SSL_ERROR("EVP_PKEY_new failed: "); - - if (!EVP_PKEY_assign_RSA(certificate_pkey.get(), certificate_rsa)) // NOLINT - SSL_ERROR("EVP_PKEY_assign_RSA failed: "); - - if (X509_set_pubkey(certificate.get(), certificate_pkey.get()) == 0) SSL_ERROR("X509_set_pubkey failed: "); - - // set validity period - if (X509_gmtime_adj(X509_get_notBefore(certificate.get()), 0) == nullptr) SSL_ERROR("X509_gmtime_adj failed: "); - - if (X509_gmtime_adj(X509_get_notAfter(certificate.get()), 60L * 60L * 24L * cert_days) == nullptr) - SSL_ERROR("X509_gmtime_adj failed: "); - - // sign - const EVP_MD* cert_digest = EVP_sha256(); - if (X509_sign(certificate.get(), ca_privkey.get(), cert_digest) == 0) SSL_ERROR("X509_sign failed: "); - - // serialize private key - char* privkey_buf; - StructGuard privkey_file(BIO_new(BIO_s_mem()), BIO_vfree); - if (privkey_file == nullptr) { - std::cerr << "Error opening memstream" << std::endl; - return false; - } - int ret = PEM_write_bio_RSAPrivateKey(privkey_file.get(), certificate_rsa, nullptr, nullptr, 0, nullptr, nullptr); - if (ret == 0) { - std::cerr << "PEM_write_RSAPrivateKey" << std::endl; - return false; - } - // NOLINTNEXTLINE(cppcoreguidelines-pro-type-cstyle-cast) - auto privkey_len = BIO_get_mem_data(privkey_file.get(), &privkey_buf); - *pkey = std::string(privkey_buf, static_cast(privkey_len)); - - // serialize certificate - char* cert_buf; - StructGuard cert_file(BIO_new(BIO_s_mem()), BIO_vfree); - if (cert_file == nullptr) { - std::cerr << "Error opening memstream" << std::endl; - return false; - } - ret = PEM_write_bio_X509(cert_file.get(), certificate.get()); - if (ret == 0) { - std::cerr << "PEM_write_X509" << std::endl; - return false; - } - // NOLINTNEXTLINE(cppcoreguidelines-pro-type-cstyle-cast) - auto cert_len = BIO_get_mem_data(cert_file.get(), &cert_buf); - *cert = std::string(cert_buf, static_cast(cert_len)); - - return true; -} - class SSHRunner { public: SSHRunner(std::string target, const bool skip_checks, const int port = 22) @@ -367,7 +192,7 @@ int main(int argc, char* argv[]) { if (fleet_ca_path.empty() != fleet_ca_key_path.empty()) { std::cerr << "fleet-ca and fleet-ca-key options should be used together" << std::endl; - return 1; + return EXIT_FAILURE; } if (!commandline_map["directory"].empty() && !commandline_map["config"].empty()) { @@ -487,8 +312,45 @@ int main(int argc, char* argv[]) { return EXIT_FAILURE; } } else { // fleet CA set => generate and sign a new certificate - if (!generateAndSign(fleet_ca_path.native(), fleet_ca_key_path.native(), &pkey, &cert, commandline_map, - device_id)) { + int rsa_bits = 2048; + if (commandline_map.count("bits") != 0) { + rsa_bits = (commandline_map["bits"].as()); + } + + int cert_days = 365; + if (commandline_map.count("days") != 0) { + cert_days = (commandline_map["days"].as()); + } + + std::string newcert_c; + if (commandline_map.count("certificate-c") != 0) { + newcert_c = (commandline_map["certificate-c"].as()); + if (newcert_c.length() != 2) { + std::cerr << "Country code (--certificate-c) should be 2 characters long" << std::endl; + return EXIT_FAILURE; + } + } + + std::string newcert_st; + if (commandline_map.count("certificate-st") != 0) { + newcert_st = (commandline_map["certificate-st"].as()); + if (newcert_st.empty()) { + std::cerr << "State name (--certificate-st) can't be empty" << std::endl; + return EXIT_FAILURE; + } + } + + std::string newcert_o; + if (commandline_map.count("certificate-o") != 0) { + newcert_o = (commandline_map["certificate-o"].as()); + if (newcert_o.empty()) { + std::cerr << "Organization name (--certificate-o) can't be empty" << std::endl; + return EXIT_FAILURE; + } + } + + if (!Crypto::generateAndSignCert(fleet_ca_path.native(), fleet_ca_key_path.native(), &pkey, &cert, rsa_bits, + cert_days, newcert_c, newcert_st, newcert_o, device_id)) { return EXIT_FAILURE; } diff --git a/src/libaktualizr/crypto/crypto.cc b/src/libaktualizr/crypto/crypto.cc index f70b858dc1..2875e7d7e3 100644 --- a/src/libaktualizr/crypto/crypto.cc +++ b/src/libaktualizr/crypto/crypto.cc @@ -2,6 +2,7 @@ #include #include +#include #include #include @@ -477,6 +478,148 @@ KeyType Crypto::IdentifyRSAKeyType(const std::string &public_key_pem) { return KeyType::kUnknown; } } +// NOLINTNEXTLINE(cppcoreguidelines-macro-usage) +#define SSL_ERROR(description) \ + { \ + std::cerr << (description) << ERR_error_string(ERR_get_error(), nullptr) << std::endl; \ + return false; \ + } +bool Crypto::generateAndSignCert(const std::string &cacert_path, const std::string &capkey_path, std::string *pkey, + std::string *cert, const int rsa_bits, const int cert_days, const std::string &cert_c, + const std::string &cert_st, const std::string &cert_o, const std::string &cert_cn) { + if (rsa_bits < 31) { // sic! + std::cerr << "RSA key size can't be smaller than 31 bits" << std::endl; + return false; + } + + // read CA certificate + std::string cacert_contents = Utils::readFile(cacert_path); + StructGuard bio_in_cacert(BIO_new_mem_buf(cacert_contents.c_str(), static_cast(cacert_contents.size())), + BIO_free_all); + StructGuard ca_certificate(PEM_read_bio_X509(bio_in_cacert.get(), nullptr, nullptr, nullptr), X509_free); + if (ca_certificate.get() == nullptr) { + std::cerr << "Reading CA certificate failed.\n"; + return false; + } + + // read CA private key + std::string capkey_contents = Utils::readFile(capkey_path); + StructGuard bio_in_capkey(BIO_new_mem_buf(capkey_contents.c_str(), static_cast(capkey_contents.size())), + BIO_free_all); + StructGuard ca_privkey(PEM_read_bio_PrivateKey(bio_in_capkey.get(), nullptr, nullptr, nullptr), + EVP_PKEY_free); + if (ca_privkey.get() == nullptr) SSL_ERROR("PEM_read_bio_PrivateKey failed: "); + + // create certificate + StructGuard certificate(X509_new(), X509_free); + if (certificate.get() == nullptr) SSL_ERROR("X509_new failed: "); + + X509_set_version(certificate.get(), 2); // X509v3 + + { + std::random_device urandom; + std::uniform_int_distribution<> serial_dist(0, (1UL << 20) - 1); + ASN1_INTEGER_set(X509_get_serialNumber(certificate.get()), serial_dist(urandom)); + } + + // create and set certificate subject name + StructGuard subj(X509_NAME_new(), X509_NAME_free); + if (subj.get() == nullptr) SSL_ERROR("X509_NAME_new failed: "); + + if (!cert_c.empty()) { + if (X509_NAME_add_entry_by_txt(subj.get(), "C", MBSTRING_ASC, + reinterpret_cast(cert_c.c_str()), -1, -1, 0) == 0) + SSL_ERROR("X509_NAME_add_entry_by_txt failed: "); + } + + if (!cert_st.empty()) { + if (X509_NAME_add_entry_by_txt(subj.get(), "ST", MBSTRING_ASC, + reinterpret_cast(cert_st.c_str()), -1, -1, 0) == 0) + SSL_ERROR("X509_NAME_add_entry_by_txt failed: "); + } + + if (!cert_o.empty()) { + if (X509_NAME_add_entry_by_txt(subj.get(), "O", MBSTRING_ASC, + reinterpret_cast(cert_o.c_str()), -1, -1, 0) == 0) + SSL_ERROR("X509_NAME_add_entry_by_txt failed: "); + } + + assert(!cert_cn.empty()); + if (X509_NAME_add_entry_by_txt(subj.get(), "CN", MBSTRING_ASC, + reinterpret_cast(cert_cn.c_str()), -1, -1, 0) == 0) + SSL_ERROR("X509_NAME_add_entry_by_txt failed: "); + + if (X509_set_subject_name(certificate.get(), subj.get()) == 0) SSL_ERROR("X509_set_subject_name failed: "); + + // set issuer name + X509_NAME *ca_subj = X509_get_subject_name(ca_certificate.get()); + if (ca_subj == nullptr) SSL_ERROR("X509_get_subject_name failed: "); + + if (X509_set_issuer_name(certificate.get(), ca_subj) == 0) SSL_ERROR("X509_set_issuer_name failed: "); + + // create and set key (would be nice to reuse generateRSAKeyPairEVP but the + // complications with reusing certificate_rsa below make that hard). + + StructGuard bne(BN_new(), BN_free); + if (BN_set_word(bne.get(), RSA_F4) != 1) SSL_ERROR("BN_set_word failed: "); + + // freed by owner EVP_PKEY + RSA *certificate_rsa = RSA_new(); + if (RSA_generate_key_ex(certificate_rsa, rsa_bits, bne.get(), nullptr) != 1) + SSL_ERROR("RSA_generate_key_ex failed: "); + + StructGuard certificate_pkey(EVP_PKEY_new(), EVP_PKEY_free); + if (certificate_pkey.get() == nullptr) SSL_ERROR("EVP_PKEY_new failed: "); + + if (!EVP_PKEY_assign_RSA(certificate_pkey.get(), certificate_rsa)) // NOLINT + SSL_ERROR("EVP_PKEY_assign_RSA failed: "); + + if (X509_set_pubkey(certificate.get(), certificate_pkey.get()) == 0) SSL_ERROR("X509_set_pubkey failed: "); + + // set validity period + if (X509_gmtime_adj(X509_get_notBefore(certificate.get()), 0) == nullptr) SSL_ERROR("X509_gmtime_adj failed: "); + + if (X509_gmtime_adj(X509_get_notAfter(certificate.get()), 60L * 60L * 24L * cert_days) == nullptr) + SSL_ERROR("X509_gmtime_adj failed: "); + + // sign + const EVP_MD *cert_digest = EVP_sha256(); + if (X509_sign(certificate.get(), ca_privkey.get(), cert_digest) == 0) SSL_ERROR("X509_sign failed: "); + + // serialize private key + char *privkey_buf; + StructGuard privkey_file(BIO_new(BIO_s_mem()), BIO_vfree); + if (privkey_file == nullptr) { + std::cerr << "Error opening memstream" << std::endl; + return false; + } + int ret = PEM_write_bio_RSAPrivateKey(privkey_file.get(), certificate_rsa, nullptr, nullptr, 0, nullptr, nullptr); + if (ret == 0) { + std::cerr << "PEM_write_RSAPrivateKey" << std::endl; + return false; + } + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-cstyle-cast) + auto privkey_len = BIO_get_mem_data(privkey_file.get(), &privkey_buf); + *pkey = std::string(privkey_buf, static_cast(privkey_len)); + + // serialize certificate + char *cert_buf; + StructGuard cert_file(BIO_new(BIO_s_mem()), BIO_vfree); + if (cert_file == nullptr) { + std::cerr << "Error opening memstream" << std::endl; + return false; + } + ret = PEM_write_bio_X509(cert_file.get(), certificate.get()); + if (ret == 0) { + std::cerr << "PEM_write_X509" << std::endl; + return false; + } + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-cstyle-cast) + auto cert_len = BIO_get_mem_data(cert_file.get(), &cert_buf); + *cert = std::string(cert_buf, static_cast(cert_len)); + + return true; +} MultiPartHasher::Ptr MultiPartHasher::create(Hash::Type hash_type) { switch (hash_type) { diff --git a/src/libaktualizr/crypto/crypto.h b/src/libaktualizr/crypto/crypto.h index 6e5c206d1b..1ad2e5bef4 100644 --- a/src/libaktualizr/crypto/crypto.h +++ b/src/libaktualizr/crypto/crypto.h @@ -91,6 +91,10 @@ class Crypto { static bool IsRsaKeyType(KeyType type); static KeyType IdentifyRSAKeyType(const std::string &public_key_pem); + + static bool generateAndSignCert(const std::string &cacert_path, const std::string &capkey_path, std::string *pkey, + std::string *cert, const int rsa_bits, const int cert_days, const std::string &cert_c, + const std::string &cert_st, const std::string &cert_o, const std::string &cert_cn); }; #endif // CRYPTO_H_ From 9e86af0ab65896c15d877fe7ad21f5f30f519063 Mon Sep 17 00:00:00 2001 From: Patrick Vacek Date: Wed, 19 Aug 2020 16:29:23 +0200 Subject: [PATCH 06/10] Standardize StorageCommon test names. Signed-off-by: Patrick Vacek --- src/libaktualizr/storage/CMakeLists.txt | 2 +- .../storage/storage_common_test.cc | 27 +++++++++---------- 2 files changed, 14 insertions(+), 15 deletions(-) diff --git a/src/libaktualizr/storage/CMakeLists.txt b/src/libaktualizr/storage/CMakeLists.txt index e5a422d6b1..7b47bcb4e9 100644 --- a/src/libaktualizr/storage/CMakeLists.txt +++ b/src/libaktualizr/storage/CMakeLists.txt @@ -23,7 +23,7 @@ if(STORAGE_TYPE STREQUAL "sqlite") add_aktualizr_test(NAME sql_utils SOURCES sql_utils_test.cc PROJECT_WORKING_DIRECTORY) add_aktualizr_test(NAME sqlstorage SOURCES sqlstorage_test.cc ARGS ${CMAKE_CURRENT_SOURCE_DIR}/test) list(REMOVE_ITEM TEST_SOURCES sql_schemas.cc) - add_aktualizr_test(NAME storage SOURCES storage_common_test.cc PROJECT_WORKING_DIRECTORY) + add_aktualizr_test(NAME storage_common SOURCES storage_common_test.cc PROJECT_WORKING_DIRECTORY) add_test(NAME test_schema_migration COMMAND ${CMAKE_CURRENT_SOURCE_DIR}/schema_migration_test.sh ${PROJECT_SOURCE_DIR}/config/sql) diff --git a/src/libaktualizr/storage/storage_common_test.cc b/src/libaktualizr/storage/storage_common_test.cc index 1f657b84b3..432830c33c 100644 --- a/src/libaktualizr/storage/storage_common_test.cc +++ b/src/libaktualizr/storage/storage_common_test.cc @@ -6,7 +6,6 @@ #include #include "libaktualizr/types.h" -#include "logging/logging.h" #include "storage/sqlstorage.h" #include "utilities/utils.h" @@ -37,7 +36,7 @@ StorageConfig MakeConfig(StorageType type, const boost::filesystem::path &storag } /* Load and store Primary keys. */ -TEST(storage, load_store_primary_keys) { +TEST(StorageCommon, LoadStorePrimaryKeys) { TemporaryDirectory temp_dir; std::unique_ptr storage = Storage(temp_dir.Path()); @@ -55,7 +54,7 @@ TEST(storage, load_store_primary_keys) { } /* Load and store TLS credentials. */ -TEST(storage, load_store_tls) { +TEST(StorageCommon, LoadStoreTls) { TemporaryDirectory temp_dir; std::unique_ptr storage = Storage(temp_dir.Path()); @@ -75,7 +74,7 @@ TEST(storage, load_store_tls) { } /* Load and store Uptane metadata. */ -TEST(storage, load_store_metadata) { +TEST(StorageCommon, LoadStoreMetadata) { TemporaryDirectory temp_dir; std::unique_ptr storage = Storage(temp_dir.Path()); @@ -174,7 +173,7 @@ TEST(storage, load_store_metadata) { } /* Load and store Uptane roots. */ -TEST(storage, load_store_root) { +TEST(StorageCommon, LoadStoreRoot) { TemporaryDirectory temp_dir; std::unique_ptr storage = Storage(temp_dir.Path()); @@ -210,7 +209,7 @@ TEST(storage, load_store_root) { } /* Load and store the device ID. */ -TEST(storage, load_store_deviceid) { +TEST(StorageCommon, LoadStoreDeviceId) { TemporaryDirectory temp_dir; std::unique_ptr storage = Storage(temp_dir.Path()); @@ -229,7 +228,7 @@ TEST(storage, load_store_deviceid) { /* Load and store ECU serials. * Preserve ECU ordering between store and load calls. */ -TEST(storage, load_store_ecu_serials) { +TEST(StorageCommon, LoadStoreEcuSerials) { TemporaryDirectory temp_dir; std::unique_ptr storage = Storage(temp_dir.Path()); @@ -249,7 +248,7 @@ TEST(storage, load_store_ecu_serials) { } /* Load and store a list of misconfigured ECUs. */ -TEST(storage, load_store_misconfigured_ecus) { +TEST(StorageCommon, LoadStoreMisconfiguredEcus) { TemporaryDirectory temp_dir; std::unique_ptr storage = Storage(temp_dir.Path()); @@ -271,7 +270,7 @@ TEST(storage, load_store_misconfigured_ecus) { } /* Load and store a flag indicating successful registration. */ -TEST(storage, load_store_ecu_registered) { +TEST(StorageCommon, LoadStoreEcuRegistered) { TemporaryDirectory temp_dir; std::unique_ptr storage = Storage(temp_dir.Path()); @@ -287,7 +286,7 @@ TEST(storage, load_store_ecu_registered) { } /* Load and store installed versions. */ -TEST(storage, load_store_installed_versions) { +TEST(StorageCommon, LoadStoreInstalledVersions) { TemporaryDirectory temp_dir; std::unique_ptr storage = Storage(temp_dir.Path()); @@ -423,7 +422,7 @@ TEST(storage, load_store_installed_versions) { * Load and store an ECU installation result in an SQL database. * Load and store a device installation result in an SQL database. */ -TEST(storage, load_store_installation_results) { +TEST(StorageCommon, LoadStoreInstallationResults) { TemporaryDirectory temp_dir; std::unique_ptr storage = Storage(temp_dir.Path()); @@ -466,7 +465,7 @@ TEST(storage, load_store_installation_results) { "This call will return a negative value since the installation report was cleaned!")); } -TEST(storage, downloaded_files_info) { +TEST(StorageCommon, DownloadedFilesInfo) { TemporaryDirectory temp_dir; std::unique_ptr storage = Storage(temp_dir.Path()); @@ -486,7 +485,7 @@ TEST(storage, downloaded_files_info) { ASSERT_EQ(names.at(0), "target2"); } -TEST(storage, load_store_secondary_info) { +TEST(StorageCommon, LoadStoreSecondaryInfo) { TemporaryDirectory temp_dir; std::unique_ptr storage = Storage(temp_dir.Path()); @@ -528,7 +527,7 @@ TEST(storage, load_store_secondary_info) { /* Import keys and credentials from file into storage. * Re-import updated credentials from file into storage. */ -TEST(storage, import_data) { +TEST(StorageImport, ImportData) { TemporaryDirectory temp_dir; std::unique_ptr storage = Storage(temp_dir.Path()); boost::filesystem::create_directories(temp_dir / "import"); From c43736e0c56e52c9ee44156acd57b80eea01db54 Mon Sep 17 00:00:00 2001 From: Patrick Vacek Date: Fri, 21 Aug 2020 11:45:13 +0200 Subject: [PATCH 07/10] Refactor cert generation, signing, and serialization into a separate functions. This allows for much easier reuse of the specific parts. Also switch to exceptions instead of printing and returning false. Signed-off-by: Patrick Vacek --- src/cert_provider/main.cc | 8 +- src/libaktualizr/crypto/crypto.cc | 185 ++++++++++++++++++------------ src/libaktualizr/crypto/crypto.h | 8 +- 3 files changed, 123 insertions(+), 78 deletions(-) diff --git a/src/cert_provider/main.cc b/src/cert_provider/main.cc index 30290d1c11..cc5b3c2b14 100644 --- a/src/cert_provider/main.cc +++ b/src/cert_provider/main.cc @@ -349,10 +349,10 @@ int main(int argc, char* argv[]) { } } - if (!Crypto::generateAndSignCert(fleet_ca_path.native(), fleet_ca_key_path.native(), &pkey, &cert, rsa_bits, - cert_days, newcert_c, newcert_st, newcert_o, device_id)) { - return EXIT_FAILURE; - } + StructGuard certificate = + Crypto::generateCert(rsa_bits, cert_days, newcert_c, newcert_st, newcert_o, device_id); + Crypto::signCert(fleet_ca_path.native(), fleet_ca_key_path.native(), certificate.get()); + Crypto::serializeCert(&pkey, &cert, certificate.get()); if (provide_ca) { // Read server root CA from server_ca.pem in archive if found (to support diff --git a/src/libaktualizr/crypto/crypto.cc b/src/libaktualizr/crypto/crypto.cc index 2875e7d7e3..d4c3286957 100644 --- a/src/libaktualizr/crypto/crypto.cc +++ b/src/libaktualizr/crypto/crypto.cc @@ -478,41 +478,19 @@ KeyType Crypto::IdentifyRSAKeyType(const std::string &public_key_pem) { return KeyType::kUnknown; } } -// NOLINTNEXTLINE(cppcoreguidelines-macro-usage) -#define SSL_ERROR(description) \ - { \ - std::cerr << (description) << ERR_error_string(ERR_get_error(), nullptr) << std::endl; \ - return false; \ - } -bool Crypto::generateAndSignCert(const std::string &cacert_path, const std::string &capkey_path, std::string *pkey, - std::string *cert, const int rsa_bits, const int cert_days, const std::string &cert_c, - const std::string &cert_st, const std::string &cert_o, const std::string &cert_cn) { - if (rsa_bits < 31) { // sic! - std::cerr << "RSA key size can't be smaller than 31 bits" << std::endl; - return false; - } - // read CA certificate - std::string cacert_contents = Utils::readFile(cacert_path); - StructGuard bio_in_cacert(BIO_new_mem_buf(cacert_contents.c_str(), static_cast(cacert_contents.size())), - BIO_free_all); - StructGuard ca_certificate(PEM_read_bio_X509(bio_in_cacert.get(), nullptr, nullptr, nullptr), X509_free); - if (ca_certificate.get() == nullptr) { - std::cerr << "Reading CA certificate failed.\n"; - return false; +StructGuard Crypto::generateCert(const int rsa_bits, const int cert_days, const std::string &cert_c, + const std::string &cert_st, const std::string &cert_o, + const std::string &cert_cn) { + if (rsa_bits < 31) { // sic! + throw std::runtime_error("RSA key size can't be smaller than 31 bits"); } - // read CA private key - std::string capkey_contents = Utils::readFile(capkey_path); - StructGuard bio_in_capkey(BIO_new_mem_buf(capkey_contents.c_str(), static_cast(capkey_contents.size())), - BIO_free_all); - StructGuard ca_privkey(PEM_read_bio_PrivateKey(bio_in_capkey.get(), nullptr, nullptr, nullptr), - EVP_PKEY_free); - if (ca_privkey.get() == nullptr) SSL_ERROR("PEM_read_bio_PrivateKey failed: "); - // create certificate StructGuard certificate(X509_new(), X509_free); - if (certificate.get() == nullptr) SSL_ERROR("X509_new failed: "); + if (certificate.get() == nullptr) { + throw std::runtime_error(std::string("X509_new failed: ") + ERR_error_string(ERR_get_error(), nullptr)); + } X509_set_version(certificate.get(), 2); // X509v3 @@ -522,81 +500,150 @@ bool Crypto::generateAndSignCert(const std::string &cacert_path, const std::stri ASN1_INTEGER_set(X509_get_serialNumber(certificate.get()), serial_dist(urandom)); } - // create and set certificate subject name + // create and set certificate subject name StructGuard subj(X509_NAME_new(), X509_NAME_free); - if (subj.get() == nullptr) SSL_ERROR("X509_NAME_new failed: "); + if (subj.get() == nullptr) { + throw std::runtime_error(std::string("X509_NAME_new failed: ") + ERR_error_string(ERR_get_error(), nullptr)); + } if (!cert_c.empty()) { if (X509_NAME_add_entry_by_txt(subj.get(), "C", MBSTRING_ASC, - reinterpret_cast(cert_c.c_str()), -1, -1, 0) == 0) - SSL_ERROR("X509_NAME_add_entry_by_txt failed: "); + reinterpret_cast(cert_c.c_str()), -1, -1, 0) == 0) { + throw std::runtime_error(std::string("X509_NAME_add_entry_by_txt failed: ") + + ERR_error_string(ERR_get_error(), nullptr)); + } } if (!cert_st.empty()) { if (X509_NAME_add_entry_by_txt(subj.get(), "ST", MBSTRING_ASC, - reinterpret_cast(cert_st.c_str()), -1, -1, 0) == 0) - SSL_ERROR("X509_NAME_add_entry_by_txt failed: "); + reinterpret_cast(cert_st.c_str()), -1, -1, 0) == 0) { + throw std::runtime_error(std::string("X509_NAME_add_entry_by_txt failed: ") + + ERR_error_string(ERR_get_error(), nullptr)); + } } if (!cert_o.empty()) { if (X509_NAME_add_entry_by_txt(subj.get(), "O", MBSTRING_ASC, - reinterpret_cast(cert_o.c_str()), -1, -1, 0) == 0) - SSL_ERROR("X509_NAME_add_entry_by_txt failed: "); + reinterpret_cast(cert_o.c_str()), -1, -1, 0) == 0) { + throw std::runtime_error(std::string("X509_NAME_add_entry_by_txt failed: ") + + ERR_error_string(ERR_get_error(), nullptr)); + } } assert(!cert_cn.empty()); if (X509_NAME_add_entry_by_txt(subj.get(), "CN", MBSTRING_ASC, - reinterpret_cast(cert_cn.c_str()), -1, -1, 0) == 0) - SSL_ERROR("X509_NAME_add_entry_by_txt failed: "); - - if (X509_set_subject_name(certificate.get(), subj.get()) == 0) SSL_ERROR("X509_set_subject_name failed: "); - - // set issuer name - X509_NAME *ca_subj = X509_get_subject_name(ca_certificate.get()); - if (ca_subj == nullptr) SSL_ERROR("X509_get_subject_name failed: "); + reinterpret_cast(cert_cn.c_str()), -1, -1, 0) == 0) { + throw std::runtime_error(std::string("X509_NAME_add_entry_by_txt failed: ") + + ERR_error_string(ERR_get_error(), nullptr)); + } - if (X509_set_issuer_name(certificate.get(), ca_subj) == 0) SSL_ERROR("X509_set_issuer_name failed: "); + if (X509_set_subject_name(certificate.get(), subj.get()) == 0) { + throw std::runtime_error(std::string("X509_set_subject_name failed: ") + + ERR_error_string(ERR_get_error(), nullptr)); + } // create and set key (would be nice to reuse generateRSAKeyPairEVP but the // complications with reusing certificate_rsa below make that hard). StructGuard bne(BN_new(), BN_free); - if (BN_set_word(bne.get(), RSA_F4) != 1) SSL_ERROR("BN_set_word failed: "); + if (BN_set_word(bne.get(), RSA_F4) != 1) { + throw std::runtime_error(std::string("BN_set_word failed: ") + ERR_error_string(ERR_get_error(), nullptr)); + } // freed by owner EVP_PKEY RSA *certificate_rsa = RSA_new(); - if (RSA_generate_key_ex(certificate_rsa, rsa_bits, bne.get(), nullptr) != 1) - SSL_ERROR("RSA_generate_key_ex failed: "); + if (RSA_generate_key_ex(certificate_rsa, rsa_bits, bne.get(), nullptr) != 1) { + throw std::runtime_error(std::string("RSA_generate_key_ex failed: ") + ERR_error_string(ERR_get_error(), nullptr)); + } StructGuard certificate_pkey(EVP_PKEY_new(), EVP_PKEY_free); - if (certificate_pkey.get() == nullptr) SSL_ERROR("EVP_PKEY_new failed: "); + if (certificate_pkey.get() == nullptr) { + throw std::runtime_error(std::string("EVP_PKEY_new failed: ") + ERR_error_string(ERR_get_error(), nullptr)); + } - if (!EVP_PKEY_assign_RSA(certificate_pkey.get(), certificate_rsa)) // NOLINT - SSL_ERROR("EVP_PKEY_assign_RSA failed: "); + if (!EVP_PKEY_assign_RSA(certificate_pkey.get(), certificate_rsa)) { // NOLINT + throw std::runtime_error(std::string("EVP_PKEY_assign_RSA failed: ") + ERR_error_string(ERR_get_error(), nullptr)); + } - if (X509_set_pubkey(certificate.get(), certificate_pkey.get()) == 0) SSL_ERROR("X509_set_pubkey failed: "); + if (X509_set_pubkey(certificate.get(), certificate_pkey.get()) == 0) { + throw std::runtime_error(std::string("X509_set_pubkey failed: ") + ERR_error_string(ERR_get_error(), nullptr)); + } // set validity period - if (X509_gmtime_adj(X509_get_notBefore(certificate.get()), 0) == nullptr) SSL_ERROR("X509_gmtime_adj failed: "); + if (X509_gmtime_adj(X509_get_notBefore(certificate.get()), 0) == nullptr) { + throw std::runtime_error(std::string("X509_gmtime_adj failed: ") + ERR_error_string(ERR_get_error(), nullptr)); + } - if (X509_gmtime_adj(X509_get_notAfter(certificate.get()), 60L * 60L * 24L * cert_days) == nullptr) - SSL_ERROR("X509_gmtime_adj failed: "); + if (X509_gmtime_adj(X509_get_notAfter(certificate.get()), 60L * 60L * 24L * cert_days) == nullptr) { + throw std::runtime_error(std::string("X509_gmtime_adj failed: ") + ERR_error_string(ERR_get_error(), nullptr)); + } - // sign + return certificate; +} + +void Crypto::signCert(const std::string &cacert_path, const std::string &capkey_path, X509 *const certificate) { + // read CA certificate + std::string cacert_contents = Utils::readFile(cacert_path); + StructGuard bio_in_cacert(BIO_new_mem_buf(cacert_contents.c_str(), static_cast(cacert_contents.size())), + BIO_free_all); + StructGuard ca_certificate(PEM_read_bio_X509(bio_in_cacert.get(), nullptr, nullptr, nullptr), X509_free); + if (ca_certificate.get() == nullptr) { + throw std::runtime_error(std::string("Reading CA certificate failed: ") + + ERR_error_string(ERR_get_error(), nullptr)); + } + + // read CA private key + std::string capkey_contents = Utils::readFile(capkey_path); + StructGuard bio_in_capkey(BIO_new_mem_buf(capkey_contents.c_str(), static_cast(capkey_contents.size())), + BIO_free_all); + StructGuard ca_privkey(PEM_read_bio_PrivateKey(bio_in_capkey.get(), nullptr, nullptr, nullptr), + EVP_PKEY_free); + if (ca_privkey.get() == nullptr) { + throw std::runtime_error(std::string("PEM_read_bio_PrivateKey failed: ") + + ERR_error_string(ERR_get_error(), nullptr)); + } + + // set issuer name + X509_NAME *ca_subj = X509_get_subject_name(ca_certificate.get()); + if (ca_subj == nullptr) { + throw std::runtime_error(std::string("X509_get_subject_name failed: ") + + ERR_error_string(ERR_get_error(), nullptr)); + } + + if (X509_set_issuer_name(certificate, ca_subj) == 0) { + throw std::runtime_error(std::string("X509_set_issuer_name failed: ") + ERR_error_string(ERR_get_error(), nullptr)); + } + + // sign const EVP_MD *cert_digest = EVP_sha256(); - if (X509_sign(certificate.get(), ca_privkey.get(), cert_digest) == 0) SSL_ERROR("X509_sign failed: "); + if (X509_sign(certificate, ca_privkey.get(), cert_digest) == 0) { + throw std::runtime_error(std::string("X509_sign failed: ") + ERR_error_string(ERR_get_error(), nullptr)); + } +} +void Crypto::serializeCert(std::string *pkey, std::string *cert, X509 *const certificate) { // serialize private key char *privkey_buf; StructGuard privkey_file(BIO_new(BIO_s_mem()), BIO_vfree); if (privkey_file == nullptr) { - std::cerr << "Error opening memstream" << std::endl; - return false; + throw std::runtime_error(std::string("BIO_new failed: ") + ERR_error_string(ERR_get_error(), nullptr)); + } + + StructGuard certificate_pkey(X509_get_pubkey(certificate), EVP_PKEY_free); + if (certificate_pkey == nullptr) { + throw std::runtime_error(std::string("X509_get_pubkey failed: ") + ERR_error_string(ERR_get_error(), nullptr)); } - int ret = PEM_write_bio_RSAPrivateKey(privkey_file.get(), certificate_rsa, nullptr, nullptr, 0, nullptr, nullptr); + + StructGuard certificate_rsa(EVP_PKEY_get1_RSA(certificate_pkey.get()), RSA_free); + if (certificate_rsa == nullptr) { + throw std::runtime_error(std::string("EVP_PKEY_get1_RSA failed: ") + ERR_error_string(ERR_get_error(), nullptr)); + } + + int ret = + PEM_write_bio_RSAPrivateKey(privkey_file.get(), certificate_rsa.get(), nullptr, nullptr, 0, nullptr, nullptr); if (ret == 0) { - std::cerr << "PEM_write_RSAPrivateKey" << std::endl; - return false; + throw std::runtime_error(std::string("PEM_write_RSAPrivateKey failed: ") + + ERR_error_string(ERR_get_error(), nullptr)); } // NOLINTNEXTLINE(cppcoreguidelines-pro-type-cstyle-cast) auto privkey_len = BIO_get_mem_data(privkey_file.get(), &privkey_buf); @@ -606,19 +653,15 @@ bool Crypto::generateAndSignCert(const std::string &cacert_path, const std::stri char *cert_buf; StructGuard cert_file(BIO_new(BIO_s_mem()), BIO_vfree); if (cert_file == nullptr) { - std::cerr << "Error opening memstream" << std::endl; - return false; + throw std::runtime_error(std::string("BIO_new failed: ") + ERR_error_string(ERR_get_error(), nullptr)); } - ret = PEM_write_bio_X509(cert_file.get(), certificate.get()); + ret = PEM_write_bio_X509(cert_file.get(), certificate); if (ret == 0) { - std::cerr << "PEM_write_X509" << std::endl; - return false; + throw std::runtime_error(std::string("PEM_write_bio_X509 failed: ") + ERR_error_string(ERR_get_error(), nullptr)); } // NOLINTNEXTLINE(cppcoreguidelines-pro-type-cstyle-cast) auto cert_len = BIO_get_mem_data(cert_file.get(), &cert_buf); *cert = std::string(cert_buf, static_cast(cert_len)); - - return true; } MultiPartHasher::Ptr MultiPartHasher::create(Hash::Type hash_type) { diff --git a/src/libaktualizr/crypto/crypto.h b/src/libaktualizr/crypto/crypto.h index 1ad2e5bef4..ffeb8bc293 100644 --- a/src/libaktualizr/crypto/crypto.h +++ b/src/libaktualizr/crypto/crypto.h @@ -92,9 +92,11 @@ class Crypto { static bool IsRsaKeyType(KeyType type); static KeyType IdentifyRSAKeyType(const std::string &public_key_pem); - static bool generateAndSignCert(const std::string &cacert_path, const std::string &capkey_path, std::string *pkey, - std::string *cert, const int rsa_bits, const int cert_days, const std::string &cert_c, - const std::string &cert_st, const std::string &cert_o, const std::string &cert_cn); + static StructGuard generateCert(const int rsa_bits, const int cert_days, const std::string &cert_c, + const std::string &cert_st, const std::string &cert_o, + const std::string &cert_cn); + static void signCert(const std::string &cacert_path, const std::string &capkey_path, X509 *const certificate); + static void serializeCert(std::string *pkey, std::string *cert, X509 *const certificate); }; #endif // CRYPTO_H_ From a3bcf8242bee500207751124a4d4cd8ae50da5d5 Mon Sep 17 00:00:00 2001 From: Patrick Vacek Date: Fri, 21 Aug 2020 15:00:27 +0200 Subject: [PATCH 08/10] Add option to self-sign a generated certificate. This should only be used for testing, so it prints a message to tell you that. Signed-off-by: Patrick Vacek --- src/libaktualizr/crypto/crypto.cc | 11 ++++++++++- src/libaktualizr/crypto/crypto.h | 2 +- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/src/libaktualizr/crypto/crypto.cc b/src/libaktualizr/crypto/crypto.cc index d4c3286957..9b811d41cd 100644 --- a/src/libaktualizr/crypto/crypto.cc +++ b/src/libaktualizr/crypto/crypto.cc @@ -481,7 +481,7 @@ KeyType Crypto::IdentifyRSAKeyType(const std::string &public_key_pem) { StructGuard Crypto::generateCert(const int rsa_bits, const int cert_days, const std::string &cert_c, const std::string &cert_st, const std::string &cert_o, - const std::string &cert_cn) { + const std::string &cert_cn, bool self_sign) { if (rsa_bits < 31) { // sic! throw std::runtime_error("RSA key size can't be smaller than 31 bits"); } @@ -578,6 +578,15 @@ StructGuard Crypto::generateCert(const int rsa_bits, const int cert_days, throw std::runtime_error(std::string("X509_gmtime_adj failed: ") + ERR_error_string(ERR_get_error(), nullptr)); } + // self-sign + if (self_sign) { + const EVP_MD *cert_digest = EVP_sha256(); + if (X509_sign(certificate.get(), certificate_pkey.get(), cert_digest) == 0) { + throw std::runtime_error(std::string("X509_sign failed: ") + ERR_error_string(ERR_get_error(), nullptr)); + } + LOG_INFO << "Successfully self-signed the generated certificate. This should not be used in production!"; + } + return certificate; } diff --git a/src/libaktualizr/crypto/crypto.h b/src/libaktualizr/crypto/crypto.h index ffeb8bc293..5f923ec6c7 100644 --- a/src/libaktualizr/crypto/crypto.h +++ b/src/libaktualizr/crypto/crypto.h @@ -94,7 +94,7 @@ class Crypto { static StructGuard generateCert(const int rsa_bits, const int cert_days, const std::string &cert_c, const std::string &cert_st, const std::string &cert_o, - const std::string &cert_cn); + const std::string &cert_cn, bool self_sign = false); static void signCert(const std::string &cacert_path, const std::string &capkey_path, X509 *const certificate); static void serializeCert(std::string *pkey, std::string *cert, X509 *const certificate); }; From 7e71c3088b40b8488ed3cd26dcc4a469a1141707 Mon Sep 17 00:00:00 2001 From: Patrick Vacek Date: Fri, 21 Aug 2020 15:00:56 +0200 Subject: [PATCH 09/10] Only re-import the client cert if the device ID is the same. Expand the test to check the negative case (with a different device ID) as well. Signed-off-by: Patrick Vacek --- src/libaktualizr/storage/invstorage.cc | 44 ++++++++++- src/libaktualizr/storage/invstorage.h | 1 + .../storage/storage_common_test.cc | 73 ++++++++++++++++--- 3 files changed, 103 insertions(+), 15 deletions(-) diff --git a/src/libaktualizr/storage/invstorage.cc b/src/libaktualizr/storage/invstorage.cc index 059af544a5..68014fa3db 100644 --- a/src/libaktualizr/storage/invstorage.cc +++ b/src/libaktualizr/storage/invstorage.cc @@ -50,6 +50,46 @@ void INvStorage::importUpdateSimple(const boost::filesystem::path& base_path, st } } +void INvStorage::importUpdateCertificate(const boost::filesystem::path& base_path, + const utils::BasedPath& imported_data_path) { + std::string prev_content; + std::string content; + bool update = false; + if (!loadTlsCert(&prev_content)) { + update = true; + } else if (!imported_data_path.empty()) { + content = Utils::readFile(imported_data_path.get(base_path).string()); + if (Crypto::sha256digest(content) != Crypto::sha256digest(prev_content)) { + update = true; + } + } + + if (update && !imported_data_path.empty()) { + boost::filesystem::path abs_path = imported_data_path.get(base_path); + if (!boost::filesystem::exists(abs_path)) { + LOG_ERROR << "Couldn't import client certificate: " << abs_path << " doesn't exist."; + return; + } + if (content.empty()) { + content = Utils::readFile(abs_path.string()); + } + + // Make sure the device ID of the new cert hasn't changed. + const std::string new_device_id = Crypto::extractSubjectCN(content); + std::string old_device_id; + if (!loadDeviceId(&old_device_id)) { + LOG_DEBUG << "Unable to load previous device ID."; + } else if (new_device_id != old_device_id) { + throw std::runtime_error(std::string("Certificate at ") + abs_path.string() + " has a device ID of " + + new_device_id + " but the device currently is identified as " + old_device_id + + ". Changing device ID is not supported!"); + } + + storeTlsCert(content); + LOG_DEBUG << "Successfully imported client certificate from " << abs_path; + } +} + void INvStorage::importPrimaryKeys(const boost::filesystem::path& base_path, const utils::BasedPath& import_pubkey_path, const utils::BasedPath& import_privkey_path) { if (loadPrimaryKeys(nullptr, nullptr) || import_pubkey_path.empty() || import_privkey_path.empty()) { @@ -91,13 +131,11 @@ void INvStorage::importInstalledVersions(const boost::filesystem::path& base_pat void INvStorage::importData(const ImportConfig& import_config) { importPrimaryKeys(import_config.base_path, import_config.uptane_public_key_path, import_config.uptane_private_key_path); + importUpdateCertificate(import_config.base_path, import_config.tls_clientcert_path); importUpdateSimple(import_config.base_path, &INvStorage::storeTlsCa, &INvStorage::loadTlsCa, import_config.tls_cacert_path, "server CA certificate"); - importUpdateSimple(import_config.base_path, &INvStorage::storeTlsCert, &INvStorage::loadTlsCert, - import_config.tls_clientcert_path, "client certificate"); importUpdateSimple(import_config.base_path, &INvStorage::storeTlsPkey, &INvStorage::loadTlsPkey, import_config.tls_pkey_path, "client TLS key"); - importInstalledVersions(import_config.base_path); } diff --git a/src/libaktualizr/storage/invstorage.h b/src/libaktualizr/storage/invstorage.h index becd899af6..dee8a81166 100644 --- a/src/libaktualizr/storage/invstorage.h +++ b/src/libaktualizr/storage/invstorage.h @@ -170,6 +170,7 @@ class INvStorage { const utils::BasedPath& imported_data_path, const std::string& data_name); void importUpdateSimple(const boost::filesystem::path& base_path, store_data_t store_func, load_data_t load_func, const utils::BasedPath& imported_data_path, const std::string& data_name); + void importUpdateCertificate(const boost::filesystem::path& base_path, const utils::BasedPath& imported_data_path); void importPrimaryKeys(const boost::filesystem::path& base_path, const utils::BasedPath& import_pubkey_path, const utils::BasedPath& import_privkey_path); diff --git a/src/libaktualizr/storage/storage_common_test.cc b/src/libaktualizr/storage/storage_common_test.cc index 432830c33c..a7f5c90e4c 100644 --- a/src/libaktualizr/storage/storage_common_test.cc +++ b/src/libaktualizr/storage/storage_common_test.cc @@ -526,7 +526,8 @@ TEST(StorageCommon, LoadStoreSecondaryInfo) { } /* Import keys and credentials from file into storage. - * Re-import updated credentials from file into storage. */ + * Re-import updated credentials from file into storage. + * Reject new certificate with a different device ID. */ TEST(StorageImport, ImportData) { TemporaryDirectory temp_dir; std::unique_ptr storage = Storage(temp_dir.Path()); @@ -540,15 +541,21 @@ TEST(StorageImport, ImportData) { import_config.tls_clientcert_path = utils::BasedPath("cert"); import_config.tls_pkey_path = utils::BasedPath("pkey"); + std::string tls_cert_in1; + std::string tls_pkey_in1; + const std::string device_id1 = "test_id1"; + StructGuard certificate1 = Crypto::generateCert(1024, 365, "", "", "", device_id1, true); + Crypto::serializeCert(&tls_pkey_in1, &tls_cert_in1, certificate1.get()); + Utils::writeFile(import_config.uptane_private_key_path.get(import_config.base_path).string(), std::string("uptane_private_1")); Utils::writeFile(import_config.uptane_public_key_path.get(import_config.base_path).string(), std::string("uptane_public_1")); Utils::writeFile(import_config.tls_cacert_path.get(import_config.base_path).string(), std::string("tls_cacert_1")); - Utils::writeFile(import_config.tls_clientcert_path.get(import_config.base_path).string(), std::string("tls_cert_1")); - Utils::writeFile(import_config.tls_pkey_path.get(import_config.base_path).string(), std::string("tls_pkey_1")); + Utils::writeFile(import_config.tls_clientcert_path.get(import_config.base_path).string(), tls_cert_in1); + Utils::writeFile(import_config.tls_pkey_path.get(import_config.base_path).string(), tls_pkey_in1); - // Initially the storage is empty + // Initially the storage is empty. EXPECT_FALSE(storage->loadPrimaryPublic(nullptr)); EXPECT_FALSE(storage->loadPrimaryPrivate(nullptr)); EXPECT_FALSE(storage->loadTlsCa(nullptr)); @@ -556,6 +563,8 @@ TEST(StorageImport, ImportData) { EXPECT_FALSE(storage->loadTlsPkey(nullptr)); storage->importData(import_config); + // Set the device ID to simulate initialization with the given certificate. + storage->storeDeviceId(device_id1); std::string primary_public; std::string primary_private; @@ -563,7 +572,7 @@ TEST(StorageImport, ImportData) { std::string tls_cert; std::string tls_pkey; - // the data has been imported + // Verify that the data has been imported. EXPECT_TRUE(storage->loadPrimaryPublic(&primary_public)); EXPECT_TRUE(storage->loadPrimaryPrivate(&primary_private)); EXPECT_TRUE(storage->loadTlsCa(&tls_ca)); @@ -573,16 +582,56 @@ TEST(StorageImport, ImportData) { EXPECT_EQ(primary_private, "uptane_private_1"); EXPECT_EQ(primary_public, "uptane_public_1"); EXPECT_EQ(tls_ca, "tls_cacert_1"); - EXPECT_EQ(tls_cert, "tls_cert_1"); - EXPECT_EQ(tls_pkey, "tls_pkey_1"); + EXPECT_EQ(tls_cert, tls_cert_in1); + EXPECT_EQ(tls_pkey, tls_pkey_in1); + + // Create second TLS cert/key (with a different device ID) and other dummy + // files. + std::string tls_cert_in2; + std::string tls_pkey_in2; + const std::string device_id2 = "test_id2"; + StructGuard certificate2 = Crypto::generateCert(1024, 365, "", "", "", device_id2, true); + Crypto::serializeCert(&tls_pkey_in2, &tls_cert_in2, certificate2.get()); + EXPECT_NE(tls_cert_in1, tls_cert_in2); + EXPECT_NE(tls_pkey_in1, tls_pkey_in2); Utils::writeFile(import_config.uptane_private_key_path.get(import_config.base_path).string(), std::string("uptane_private_2")); Utils::writeFile(import_config.uptane_public_key_path.get(import_config.base_path).string(), std::string("uptane_public_2")); Utils::writeFile(import_config.tls_cacert_path.get(import_config.base_path).string(), std::string("tls_cacert_2")); - Utils::writeFile(import_config.tls_clientcert_path.get(import_config.base_path).string(), std::string("tls_cert_2")); - Utils::writeFile(import_config.tls_pkey_path.get(import_config.base_path).string(), std::string("tls_pkey_2")); + Utils::writeFile(import_config.tls_clientcert_path.get(import_config.base_path).string(), tls_cert_in2); + Utils::writeFile(import_config.tls_pkey_path.get(import_config.base_path).string(), tls_pkey_in2); + + // Attempt to re-import, but expect failure because the TLS cert's device ID + // changed. + EXPECT_THROW(storage->importData(import_config), std::runtime_error); + + EXPECT_TRUE(storage->loadPrimaryPublic(&primary_public)); + EXPECT_TRUE(storage->loadPrimaryPrivate(&primary_private)); + EXPECT_TRUE(storage->loadTlsCa(&tls_ca)); + EXPECT_TRUE(storage->loadTlsCert(&tls_cert)); + EXPECT_TRUE(storage->loadTlsPkey(&tls_pkey)); + + // Nothing should be updated. Uptane keys cannot be updated, and the TLS + // credentials should have failed. + EXPECT_EQ(primary_private, "uptane_private_1"); + EXPECT_EQ(primary_public, "uptane_public_1"); + EXPECT_EQ(tls_ca, "tls_cacert_1"); + EXPECT_EQ(tls_cert, tls_cert_in1); + EXPECT_EQ(tls_pkey, tls_pkey_in1); + + // Create third TLS cert/key (with the same device ID as the first) and other + // dummy files. + std::string tls_cert_in3; + std::string tls_pkey_in3; + StructGuard certificate3 = Crypto::generateCert(1024, 365, "", "", "", device_id1, true); + Crypto::serializeCert(&tls_pkey_in3, &tls_cert_in3, certificate3.get()); + EXPECT_NE(tls_cert_in1, tls_cert_in3); + EXPECT_NE(tls_pkey_in1, tls_pkey_in3); + + Utils::writeFile(import_config.tls_clientcert_path.get(import_config.base_path).string(), tls_cert_in3); + Utils::writeFile(import_config.tls_pkey_path.get(import_config.base_path).string(), tls_pkey_in3); storage->importData(import_config); @@ -592,12 +641,12 @@ TEST(StorageImport, ImportData) { EXPECT_TRUE(storage->loadTlsCert(&tls_cert)); EXPECT_TRUE(storage->loadTlsPkey(&tls_pkey)); - // All TLS objects should be updated: + // All TLS objects should be updated. EXPECT_EQ(primary_private, "uptane_private_1"); EXPECT_EQ(primary_public, "uptane_public_1"); EXPECT_EQ(tls_ca, "tls_cacert_2"); - EXPECT_EQ(tls_cert, "tls_cert_2"); - EXPECT_EQ(tls_pkey, "tls_pkey_2"); + EXPECT_EQ(tls_cert, tls_cert_in3); + EXPECT_EQ(tls_pkey, tls_pkey_in3); } #ifndef __NO_MAIN__ From dcad65bb348c4d75ed6164a1b6c076f5d6aa437c Mon Sep 17 00:00:00 2001 From: Patrick Vacek Date: Fri, 21 Aug 2020 15:20:08 +0200 Subject: [PATCH 10/10] Reuse Crypto::generateRSAKeyPairEVP() in generateCert(). Signed-off-by: Patrick Vacek --- src/libaktualizr/crypto/crypto.cc | 62 ++++++++++++------------------- src/libaktualizr/crypto/crypto.h | 1 + 2 files changed, 25 insertions(+), 38 deletions(-) diff --git a/src/libaktualizr/crypto/crypto.cc b/src/libaktualizr/crypto/crypto.cc index 9b811d41cd..147497c03c 100644 --- a/src/libaktualizr/crypto/crypto.cc +++ b/src/libaktualizr/crypto/crypto.cc @@ -346,32 +346,42 @@ StructGuard Crypto::generateRSAKeyPairEVP(KeyType key_type) { return {nullptr, EVP_PKEY_free}; } - int ret; + return Crypto::generateRSAKeyPairEVP(bits); +} - ret = RAND_status(); +StructGuard Crypto::generateRSAKeyPairEVP(const int bits) { + if (bits < 31) { // sic! + throw std::runtime_error("RSA key size can't be smaller than 31 bits"); + } + + int ret = RAND_status(); if (ret != 1) { /* random generator has NOT been seeded with enough data */ ret = RAND_poll(); if (ret != 1) { /* seed data was NOT generated */ - return {nullptr, EVP_PKEY_free}; + throw std::runtime_error("Random generator has not been sufficiently seeded."); } } + /* exponent - RSA_F4 is defined as 0x10001L */ StructGuard bne(BN_new(), BN_free); - ret = BN_set_word(bne.get(), RSA_F4); - if (ret != 1) { - return {nullptr, EVP_PKEY_free}; + if (BN_set_word(bne.get(), RSA_F4) != 1) { + throw std::runtime_error(std::string("BN_set_word failed: ") + ERR_error_string(ERR_get_error(), nullptr)); } + StructGuard rsa(RSA_new(), RSA_free); - ret = RSA_generate_key_ex(rsa.get(), bits, /* number of bits for the key - 2048 is a sensible value */ - bne.get(), /* exponent - RSA_F4 is defined as 0x10001L */ - nullptr); /* callback argument - not needed in this case */ - if (ret != 1) { - return {nullptr, EVP_PKEY_free}; + if (RSA_generate_key_ex(rsa.get(), bits, bne.get(), nullptr) != 1) { + throw std::runtime_error(std::string("RSA_generate_key_ex failed: ") + ERR_error_string(ERR_get_error(), nullptr)); } StructGuard pkey(EVP_PKEY_new(), EVP_PKEY_free); + if (pkey.get() == nullptr) { + throw std::runtime_error(std::string("EVP_PKEY_new failed: ") + ERR_error_string(ERR_get_error(), nullptr)); + } + // release the rsa pointer here, pkey is the new owner - EVP_PKEY_assign_RSA(pkey.get(), rsa.release()); // NOLINT + if (!EVP_PKEY_assign_RSA(pkey.get(), rsa.release())) { // NOLINT(cppcoreguidelines-pro-type-cstyle-cast) + throw std::runtime_error(std::string("EVP_PKEY_assign_RSA failed: ") + ERR_error_string(ERR_get_error(), nullptr)); + } return pkey; } @@ -482,10 +492,6 @@ KeyType Crypto::IdentifyRSAKeyType(const std::string &public_key_pem) { StructGuard Crypto::generateCert(const int rsa_bits, const int cert_days, const std::string &cert_c, const std::string &cert_st, const std::string &cert_o, const std::string &cert_cn, bool self_sign) { - if (rsa_bits < 31) { // sic! - throw std::runtime_error("RSA key size can't be smaller than 31 bits"); - } - // create certificate StructGuard certificate(X509_new(), X509_free); if (certificate.get() == nullptr) { @@ -542,28 +548,8 @@ StructGuard Crypto::generateCert(const int rsa_bits, const int cert_days, ERR_error_string(ERR_get_error(), nullptr)); } - // create and set key (would be nice to reuse generateRSAKeyPairEVP but the - // complications with reusing certificate_rsa below make that hard). - - StructGuard bne(BN_new(), BN_free); - if (BN_set_word(bne.get(), RSA_F4) != 1) { - throw std::runtime_error(std::string("BN_set_word failed: ") + ERR_error_string(ERR_get_error(), nullptr)); - } - - // freed by owner EVP_PKEY - RSA *certificate_rsa = RSA_new(); - if (RSA_generate_key_ex(certificate_rsa, rsa_bits, bne.get(), nullptr) != 1) { - throw std::runtime_error(std::string("RSA_generate_key_ex failed: ") + ERR_error_string(ERR_get_error(), nullptr)); - } - - StructGuard certificate_pkey(EVP_PKEY_new(), EVP_PKEY_free); - if (certificate_pkey.get() == nullptr) { - throw std::runtime_error(std::string("EVP_PKEY_new failed: ") + ERR_error_string(ERR_get_error(), nullptr)); - } - - if (!EVP_PKEY_assign_RSA(certificate_pkey.get(), certificate_rsa)) { // NOLINT - throw std::runtime_error(std::string("EVP_PKEY_assign_RSA failed: ") + ERR_error_string(ERR_get_error(), nullptr)); - } + // create and set key. + StructGuard certificate_pkey(Crypto::generateRSAKeyPairEVP(rsa_bits)); if (X509_set_pubkey(certificate.get(), certificate_pkey.get()) == 0) { throw std::runtime_error(std::string("X509_set_pubkey failed: ") + ERR_error_string(ERR_get_error(), nullptr)); diff --git a/src/libaktualizr/crypto/crypto.h b/src/libaktualizr/crypto/crypto.h index 5f923ec6c7..61e628895e 100644 --- a/src/libaktualizr/crypto/crypto.h +++ b/src/libaktualizr/crypto/crypto.h @@ -82,6 +82,7 @@ class Crypto { std::string *out_ca); static std::string extractSubjectCN(const std::string &cert); static StructGuard generateRSAKeyPairEVP(KeyType key_type); + static StructGuard generateRSAKeyPairEVP(const int bits); static bool generateRSAKeyPair(KeyType key_type, std::string *public_key, std::string *private_key); static bool generateEDKeyPair(std::string *public_key, std::string *private_key); static bool generateKeyPair(KeyType key_type, std::string *public_key, std::string *private_key);