From 1a1ff5d523b7386f02e78b392a6d0410c832c2fb Mon Sep 17 00:00:00 2001 From: Krzysztof Jackiewicz Date: Mon, 6 Jul 2020 12:55:32 +0200 Subject: [PATCH 01/16] Remove unused DB::Crypto methods Change-Id: Ie9f54b02736f1eebd72a496f87e250bbdd48b7aa --- src/manager/service/db-crypto.cpp | 26 -------------------------- src/manager/service/db-crypto.h | 12 ------------ 2 files changed, 38 deletions(-) diff --git a/src/manager/service/db-crypto.cpp b/src/manager/service/db-crypto.cpp index f7c09c1..23b7d1d 100644 --- a/src/manager/service/db-crypto.cpp +++ b/src/manager/service/db-crypto.cpp @@ -259,32 +259,6 @@ Crypto &Crypto::operator=(Crypto &&other) return *this; } -void Crypto::createTable(const char *create_cmd, const char *table_name) -{ - try { - m_connection->ExecCommand(create_cmd); - } catch (const SqlConnection::Exception::SyntaxError &) { - LogError("Couldn't create table : " << table_name << "!"); - throw; - } catch (const SqlConnection::Exception::InternalError &) { - LogError("Sqlite got into infinite busy state"); - throw; - } -} - -void Crypto::createView(const char *create_cmd) -{ - try { - m_connection->ExecCommand(create_cmd); - } catch (const SqlConnection::Exception::SyntaxError &) { - LogError("Couldn't create view!"); - throw; - } catch (const SqlConnection::Exception::InternalError &) { - LogError("Sqlite got into infinite busy state"); - throw; - } -} - bool Crypto::getDBVersion(int &schemaVersion) { Transaction transaction(this); diff --git a/src/manager/service/db-crypto.h b/src/manager/service/db-crypto.h index 6372320..642505c 100644 --- a/src/manager/service/db-crypto.h +++ b/src/manager/service/db-crypto.h @@ -127,11 +127,6 @@ public: const ClientId &owner, const ClientId &accessor) const; - // transactions - int beginTransaction(); - int commitTransaction(); - int rollbackTransaction(); - class Transaction { public: explicit Transaction(Crypto *db) : m_db(db), m_inTransaction(false) @@ -223,13 +218,6 @@ private: ScriptOptional getScript(const std::string &scriptName) const; ScriptOptional getMigrationScript(int db_version) const; - void createTable( - const char *create_cmd, - const char *table_name); - - void createView( - const char *create_cmd); - class SchemaInfo { public: explicit SchemaInfo(SqlConnection *connection) : m_connection(connection) {} -- 2.7.4 From 24100cd7d0f450f4e6f9ed9524ed60dbf1f2f448 Mon Sep 17 00:00:00 2001 From: Krzysztof Jackiewicz Date: Tue, 14 Jul 2020 16:35:48 +0200 Subject: [PATCH 02/16] Return if there are no rows to save Before this change, an attempt to save an empty list of objects would populate the NAME and PERMISSIONS table but insert no objects into the OBJECTS table. Change-Id: I08a2b68831ed51564e43ef4a01fca28d2c789641 --- src/manager/service/db-crypto.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/manager/service/db-crypto.cpp b/src/manager/service/db-crypto.cpp index 23b7d1d..c115d05 100644 --- a/src/manager/service/db-crypto.cpp +++ b/src/manager/service/db-crypto.cpp @@ -393,6 +393,9 @@ bool Crypto::isNameOwnerPresent(const Name &name, const ClientId &owner) const void Crypto::saveRows(const Name &name, const ClientId &owner, const RowVector &rows) { + if (rows.empty()) + return; + try { // transaction is present in the layer above NameTable nameTable(m_connection.get()); -- 2.7.4 From bbeb804bdc61b4979a3d3c115467623352601bba Mon Sep 17 00:00:00 2001 From: Krzysztof Jackiewicz Date: Tue, 7 Jul 2020 09:14:35 +0200 Subject: [PATCH 03/16] Improve DB::Crypto code coverage Change-Id: I0fcb65833641ef75ab2af3c265e15df4d45231b6 --- unit-tests/DBFixture.cpp | 10 ++ unit-tests/DBFixture.h | 1 + unit-tests/test_db_crypto.cpp | 271 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 282 insertions(+) diff --git a/unit-tests/DBFixture.cpp b/unit-tests/DBFixture.cpp index da73dfe..3f8c3cc 100644 --- a/unit-tests/DBFixture.cpp +++ b/unit-tests/DBFixture.cpp @@ -185,6 +185,16 @@ DB::Row DBFixture::create_default_row(const Name &name, return row; } +DB::Row DBFixture::create_default_binary_row() +{ + DB::Row row = create_default_row(m_default_name, m_default_owner, DataType::BINARY_DATA); + row.data = RawBuffer(100, 20); + row.dataSize = row.data.size(); + row.tag = RawBuffer(AES_GCM_TAG_SIZE, 1); + + return row; +} + void DBFixture::compare_row(const DB::Row &lhs, const DB::Row &rhs) { BOOST_CHECK_MESSAGE(lhs.name == rhs.name, diff --git a/unit-tests/DBFixture.h b/unit-tests/DBFixture.h index fd2381c..fb036b6 100644 --- a/unit-tests/DBFixture.h +++ b/unit-tests/DBFixture.h @@ -45,6 +45,7 @@ public: static CKM::DB::Row create_default_row(const CKM::Name &name, const CKM::ClientId &owner, CKM::DataType type = CKM::DataType::BINARY_DATA); + static CKM::DB::Row create_default_binary_row(); static void compare_row(const CKM::DB::Row &lhs, const CKM::DB::Row &rhs); // ::::::::::::::::::::::::: time measurement ::::::::::::::::::::::::: diff --git a/unit-tests/test_db_crypto.cpp b/unit-tests/test_db_crypto.cpp index 5ed8d5e..1317561 100644 --- a/unit-tests/test_db_crypto.cpp +++ b/unit-tests/test_db_crypto.cpp @@ -41,6 +41,17 @@ const unsigned int c_num_names = 500; const unsigned int c_num_names_add_test = 5000; const unsigned int c_names_per_owner = 15; +void addRow(DB::RowVector& rows, DataType::Type type) +{ + DB::Row row = DBFixture::create_default_row(DBFixture::m_default_name, + DBFixture::m_default_owner, + type); + row.data = RawBuffer(100, 20); + row.dataSize = row.data.size(); + row.tag = RawBuffer(AES_GCM_TAG_SIZE, 1); + rows.push_back(std::move(row)); +}; + } // namespace anonymous BOOST_FIXTURE_TEST_SUITE(DBCRYPTO_TEST, DBFixture) @@ -109,6 +120,266 @@ POSITIVE_TEST_CASE(DBtestBackend) check_DB_integrity(rowPattern); } +POSITIVE_TEST_CASE(DBtestMove) +{ + struct TestCrypto : public DB::Crypto { + explicit TestCrypto(DB::Crypto orig) : DB::Crypto(std::move(orig)) {} + + bool empty() const { return !m_connection; } + }; + + TestCrypto tmp(std::move(m_db)); + BOOST_REQUIRE(!tmp.empty()); + + BOOST_REQUIRE_NO_THROW(tmp = std::move(tmp)); + BOOST_REQUIRE(!tmp.empty()); + + m_db = std::move(tmp); + BOOST_REQUIRE(tmp.empty()); +} + +POSITIVE_TEST_CASE(DBtestSaveRows) +{ + DB::RowVector rows; + + addRow(rows, DataType::KEY_RSA_PRIVATE); + addRow(rows, DataType::KEY_RSA_PUBLIC); + addRow(rows, DataType::CERTIFICATE); + addRow(rows, DataType::CHAIN_CERT_0); + addRow(rows, DataType::CHAIN_CERT_1); + addRow(rows, DataType::CHAIN_CERT_2); + + BOOST_REQUIRE_NO_THROW(m_db.saveRows(m_default_name, m_default_owner, rows)); + + BOOST_REQUIRE(m_db.isNameOwnerPresent(m_default_name, m_default_owner)); + + rows.clear(); + BOOST_REQUIRE_NO_THROW(m_db.getRows(m_default_name, + m_default_owner, + DataType::KEY_RSA_PUBLIC, + DataType::KEY_RSA_PRIVATE, + rows)); + BOOST_REQUIRE(rows.size() == 2); + BOOST_REQUIRE(rows[0].dataType.isKey() && rows[1].dataType.isKey()); + + rows.clear(); + BOOST_REQUIRE_NO_THROW(m_db.getRows(m_default_name, + m_default_owner, + DataType::KEY_DSA_PRIVATE, + rows)); + BOOST_REQUIRE(rows.empty()); +} + +NEGATIVE_TEST_CASE(DBtestSaveRowsDuplicated) +{ + DB::RowVector rows; + + addRow(rows, DataType::KEY_RSA_PRIVATE); + addRow(rows, DataType::KEY_RSA_PUBLIC); + addRow(rows, DataType::CERTIFICATE); + addRow(rows, DataType::CHAIN_CERT_0); + addRow(rows, DataType::CHAIN_CERT_1); + addRow(rows, DataType::CHAIN_CERT_1); + + // duplicated type + DB::Crypto::Transaction transaction(&m_db); + BOOST_REQUIRE_THROW(m_db.saveRows(m_default_name, m_default_owner, rows), + Exc::DatabaseFailed); + transaction.rollback(); + + BOOST_REQUIRE(!m_db.isNameOwnerPresent(m_default_name, m_default_owner)); +} + +NEGATIVE_TEST_CASE(DBtestSaveRowsEmpty) +{ + BOOST_REQUIRE_NO_THROW(m_db.saveRows(m_default_name, m_default_owner, DB::RowVector())); + + BOOST_REQUIRE(!m_db.isNameOwnerPresent(m_default_name, m_default_owner)); +} + +NEGATIVE_TEST_CASE(DBtestSaveRowDuplicated) +{ + DB::Row row = create_default_binary_row(); + + BOOST_REQUIRE_NO_THROW(m_db.saveRow(row)); + BOOST_REQUIRE_THROW(m_db.saveRow(row), Exc::DatabaseFailed); +} + +NEGATIVE_TEST_CASE(DBtestGetRowsNonExisting) +{ + DB::RowVector rows; + BOOST_REQUIRE_NO_THROW(m_db.getRows(m_default_name, + m_default_owner, + DataType::DB_FIRST, + DataType::DB_LAST, + rows)); + BOOST_REQUIRE(rows.empty()); +} + +POSITIVE_TEST_CASE(DBtestUpdateRow) +{ + DB::Row row = create_default_binary_row(); + + BOOST_REQUIRE_NO_THROW(m_db.saveRow(row)); + + row.dataSize--; + row.data.resize(row.dataSize); + BOOST_REQUIRE_NO_THROW(m_db.updateRow(row)); + + DB::Crypto::RowOptional rowOpt; + BOOST_REQUIRE_NO_THROW(rowOpt = m_db.getRow(row.name, row.owner, DataType::BINARY_DATA)); + BOOST_REQUIRE(rowOpt); + BOOST_REQUIRE(rowOpt->dataSize == row.dataSize); + BOOST_REQUIRE(rowOpt->data == row.data); +} + +NEGATIVE_TEST_CASE(DBtestUpdateRowNotExisting) +{ + DB::Row row = create_default_binary_row(); + BOOST_REQUIRE_NO_THROW(m_db.updateRow(row)); + BOOST_REQUIRE(!m_db.isNameOwnerPresent(row.name, row.owner)); + + BOOST_REQUIRE_NO_THROW(m_db.saveRow(row)); + + row.name = "non-existent name"; + BOOST_REQUIRE_NO_THROW(m_db.updateRow(row)); + BOOST_REQUIRE(!m_db.isNameOwnerPresent(row.name, row.owner)); +} + +POSITIVE_TEST_CASE(DBtestDeleteRow) +{ + DB::Row row = create_default_binary_row(); + BOOST_REQUIRE_NO_THROW(m_db.saveRow(row)); + BOOST_REQUIRE(m_db.isNameOwnerPresent(row.name, row.owner)); + BOOST_REQUIRE(m_db.deleteRow(row.name, row.owner)); + BOOST_REQUIRE(!m_db.isNameOwnerPresent(row.name, row.owner)); +} + +NEGATIVE_TEST_CASE(DBtestDeleteRowNotExisting) +{ + DB::Row row = create_default_binary_row(); + BOOST_REQUIRE(!m_db.isNameOwnerPresent(row.name, row.owner)); + BOOST_REQUIRE(!m_db.deleteRow(row.name, row.owner)); +} + +POSITIVE_TEST_CASE(DBtestIsNameOwnerPresent) +{ + BOOST_REQUIRE(!m_db.isNameOwnerPresent(m_default_name, m_default_owner)); + + insert_row(m_default_name, m_default_owner); + + BOOST_REQUIRE(m_db.isNameOwnerPresent(m_default_name, m_default_owner)); + BOOST_REQUIRE(!m_db.isNameOwnerPresent("non-existent name", m_default_owner)); + BOOST_REQUIRE(!m_db.isNameOwnerPresent(m_default_name, "non-existent owner")); + BOOST_REQUIRE(!m_db.isNameOwnerPresent("non-existent name", "non-existent owner")); + BOOST_REQUIRE(!m_db.isNameOwnerPresent("", "")); + + delete_row(m_default_name, m_default_owner); + + BOOST_REQUIRE(!m_db.isNameOwnerPresent(m_default_name, m_default_owner)); +} + +NEGATIVE_TEST_CASE(DBtestIsNameOwnerPresentSqlInjectionAttempt) +{ + insert_row(m_default_name, m_default_owner); + + BOOST_REQUIRE(!m_db.isNameOwnerPresent("name'; DROP TABLE NAMES; --", m_default_owner)); + BOOST_REQUIRE(!m_db.isNameOwnerPresent(m_default_name, "owner'; DROP TABLE NAMES; --")); + BOOST_REQUIRE(m_db.isNameOwnerPresent(m_default_name, m_default_owner)); +} + +POSITIVE_TEST_CASE(DBtestPermissions) +{ + constexpr char OTHER_OWNER[] = "other owner"; + insert_row(m_default_name, m_default_owner); + + auto checkPermission = [&](const char* accessor, PermissionMask perm) + { + PermissionMaskOptional maskOpt; + BOOST_REQUIRE_NO_THROW(maskOpt = m_db.getPermissionRow(m_default_name, + m_default_owner, + accessor)); + BOOST_REQUIRE(!!maskOpt == (perm != Permission::NONE)); + if (perm != Permission::NONE) + BOOST_REQUIRE(*maskOpt == perm); + + }; + + checkPermission(m_default_owner, Permission::READ | Permission::REMOVE); + + checkPermission(OTHER_OWNER, Permission::NONE); + + BOOST_REQUIRE_NO_THROW(m_db.setPermission(m_default_name, + m_default_owner, + OTHER_OWNER, + Permission::READ)); + + checkPermission(OTHER_OWNER, Permission::READ); + + BOOST_REQUIRE_NO_THROW(m_db.setPermission(m_default_name, + m_default_owner, + OTHER_OWNER, + Permission::NONE)); + + checkPermission(OTHER_OWNER, Permission::NONE); +} + +NEGATIVE_TEST_CASE(DBtestPermissionsNonExisting) +{ + PermissionMaskOptional maskOpt; + BOOST_REQUIRE_NO_THROW(maskOpt = m_db.getPermissionRow(m_default_name, + m_default_owner, + m_default_owner)); + BOOST_REQUIRE(!maskOpt); +} + +POSITIVE_TEST_CASE(DBtestClientKey) +{ + DB::Crypto::RawBufferOptional keyOpt; + RawBuffer key = createRandom(16); + + BOOST_REQUIRE_NO_THROW(keyOpt = m_db.getKey(m_default_owner)); + BOOST_REQUIRE(!keyOpt); + + BOOST_REQUIRE_NO_THROW(m_db.saveKey(m_default_owner, key)); + + BOOST_REQUIRE_NO_THROW(keyOpt = m_db.getKey(m_default_owner)); + BOOST_REQUIRE(keyOpt); + BOOST_REQUIRE(*keyOpt == key); + + constexpr size_t NAME_CNT = 4; + constexpr const char* NAMES[NAME_CNT] = { "name1", "name2", "name3", "name4" }; + for (auto name : NAMES) + insert_row(name, m_default_owner); + + OwnerNameVector ownerNameVector; + BOOST_REQUIRE_NO_THROW(m_db.listNames(m_default_owner, ownerNameVector, DataType::BINARY_DATA)); + BOOST_REQUIRE(ownerNameVector.size() == NAME_CNT); + + BOOST_REQUIRE_NO_THROW(m_db.deleteKey(m_default_owner)); + + ownerNameVector.clear(); + BOOST_REQUIRE_NO_THROW(m_db.listNames(m_default_owner, ownerNameVector, DataType::BINARY_DATA)); + BOOST_REQUIRE(ownerNameVector.empty()); + + BOOST_REQUIRE_NO_THROW(keyOpt = m_db.getKey(m_default_owner)); + BOOST_REQUIRE(!keyOpt); +} + +NEGATIVE_TEST_CASE(DBtestClientKey) +{ + DB::Crypto::RawBufferOptional keyOpt; + RawBuffer key = createRandom(16); + + BOOST_REQUIRE_NO_THROW(m_db.deleteKey(m_default_owner)); + + BOOST_REQUIRE_NO_THROW(keyOpt = m_db.getKey(m_default_owner)); + BOOST_REQUIRE(!keyOpt); + + BOOST_REQUIRE_NO_THROW(m_db.saveKey(m_default_owner, key)); + BOOST_REQUIRE_THROW(m_db.saveKey(m_default_owner, key), Exc::DatabaseFailed); +} + BOOST_AUTO_TEST_SUITE_END() -- 2.7.4 From 6d929a05c7f265a330c5d9d12946fadccfd4d38f Mon Sep 17 00:00:00 2001 From: Krzysztof Jackiewicz Date: Tue, 14 Jul 2020 17:31:44 +0200 Subject: [PATCH 04/16] Remove unused SqlConnection::DataCommand::Reset Change-Id: Ib4279ccd14c6066efc980ec00bd63e76b699ca6a --- src/manager/dpl/db/include/dpl/db/sql_connection.h | 6 ------ src/manager/dpl/db/src/sql_connection.cpp | 15 --------------- 2 files changed, 21 deletions(-) diff --git a/src/manager/dpl/db/include/dpl/db/sql_connection.h b/src/manager/dpl/db/include/dpl/db/sql_connection.h index 063e05b..4ac3d5f 100644 --- a/src/manager/dpl/db/include/dpl/db/sql_connection.h +++ b/src/manager/dpl/db/include/dpl/db/sql_connection.h @@ -261,12 +261,6 @@ public: bool Step(); /** - * Reset prepared statement's arguments - * All parameters will become null - */ - void Reset(); - - /** * Checks whether column value is null * * @throw Exception::InvalidColumn diff --git a/src/manager/dpl/db/src/sql_connection.cpp b/src/manager/dpl/db/src/sql_connection.cpp index 3aa5b72..42744e3 100644 --- a/src/manager/dpl/db/src/sql_connection.cpp +++ b/src/manager/dpl/db/src/sql_connection.cpp @@ -378,21 +378,6 @@ bool SqlConnection::DataCommand::Step() ThrowMsg(Exception::InternalError, "sqlite permanently busy"); } -void SqlConnection::DataCommand::Reset() -{ - /* - * According to: - * http://www.sqllite.org/c3ref/stmt.html - * - * if last sqlite3_step command on this stmt returned an error, - * then sqlite3_reset will return that error, althought it is not an error. - * So sqlite3_reset allways succedes. - */ - sqlite3_reset(m_stmt); - - LogPedantic("SQL data command reset"); -} - void SqlConnection::DataCommand::CheckColumnIndex( SqlConnection::ColumnIndex column) { -- 2.7.4 From e03be7b827f3ff1fe5055af825e56e09063504fb Mon Sep 17 00:00:00 2001 From: Krzysztof Jackiewicz Date: Tue, 14 Jul 2020 17:32:32 +0200 Subject: [PATCH 05/16] Make BeginTransaction exclusive and use it Change-Id: Ie37fb0a36c25079eadab374093065f1e466d22f9 --- src/manager/dpl/db/src/sql_connection.cpp | 2 +- src/manager/service/db-crypto.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/manager/dpl/db/src/sql_connection.cpp b/src/manager/dpl/db/src/sql_connection.cpp index 42744e3..7a0664e 100644 --- a/src/manager/dpl/db/src/sql_connection.cpp +++ b/src/manager/dpl/db/src/sql_connection.cpp @@ -986,7 +986,7 @@ void SqlConnection::TurnOnForeignKeys() void SqlConnection::BeginTransaction() { - ExecCommand("BEGIN;"); + ExecCommand("BEGIN EXCLUSIVE;"); } void SqlConnection::RollbackTransaction() diff --git a/src/manager/service/db-crypto.h b/src/manager/service/db-crypto.h index 642505c..3c3c206 100644 --- a/src/manager/service/db-crypto.h +++ b/src/manager/service/db-crypto.h @@ -133,7 +133,7 @@ public: { if (!m_db->m_inUserTransaction) { try { - m_db->m_connection->ExecCommand("BEGIN EXCLUSIVE"); + m_db->m_connection->BeginTransaction(); m_db->m_inUserTransaction = true; m_inTransaction = true; } catch (const SqlConnection::Exception::InternalError &) { -- 2.7.4 From eb78be94becd970d2b1e97772012a861f787242c Mon Sep 17 00:00:00 2001 From: Krzysztof Jackiewicz Date: Tue, 14 Jul 2020 18:21:34 +0200 Subject: [PATCH 06/16] Get rid of the openssl 1.0.2 specific code Also move entropy initialization to key-manager-main.cpp where it is used. Change-Id: I187c76565b3864b6042a31a6eb71ac5921dc1ffd --- misc/encryption_scheme/scheme-test.cpp | 2 - src/manager/CMakeLists.txt | 1 - src/manager/client-capi/ckmc-type.cpp | 7 +- src/manager/client/client-manager-impl.cpp | 2 - src/manager/common/crypto-init.cpp | 210 --------------------------- src/manager/common/crypto-init.h | 48 ------ src/manager/common/openssl-error-handler.cpp | 11 -- src/manager/common/pkcs12-impl.cpp | 4 - src/manager/crypto/sw-backend/internals.cpp | 12 -- src/manager/crypto/tz-backend/internals.cpp | 6 - src/manager/main/key-manager-main.cpp | 37 ++++- src/manager/main/service-thread.h | 7 +- unit-tests/CMakeLists.txt | 1 - 13 files changed, 34 insertions(+), 314 deletions(-) delete mode 100644 src/manager/common/crypto-init.cpp delete mode 100644 src/manager/common/crypto-init.h diff --git a/misc/encryption_scheme/scheme-test.cpp b/misc/encryption_scheme/scheme-test.cpp index 4aa19f1..70d5e8d 100644 --- a/misc/encryption_scheme/scheme-test.cpp +++ b/misc/encryption_scheme/scheme-test.cpp @@ -40,7 +40,6 @@ #include #include #include -#include #include #include "smack-access.h" @@ -402,7 +401,6 @@ SchemeTest::SchemeTest() : m_userChanged(false), m_directAccessEnabled(false) { m_control = Control::create(); m_mgr = Manager::create(); - initOpenSsl(); SmackAccess sa; sa.add("System", LABEL, "rwx"); diff --git a/src/manager/CMakeLists.txt b/src/manager/CMakeLists.txt index 742d905..cbf39b3 100644 --- a/src/manager/CMakeLists.txt +++ b/src/manager/CMakeLists.txt @@ -14,7 +14,6 @@ SET(COMMON_PATH ${PROJECT_SOURCE_DIR}/src/manager) SET(COMMON_SOURCES ${COMMON_PATH}/common/algo-param.cpp ${COMMON_PATH}/common/base64.cpp - ${COMMON_PATH}/common/crypto-init.cpp ${COMMON_PATH}/common/data-type.cpp ${COMMON_PATH}/common/openssl-error-handler.cpp ${COMMON_PATH}/common/exception.cpp diff --git a/src/manager/client-capi/ckmc-type.cpp b/src/manager/client-capi/ckmc-type.cpp index 9409025..3e1ca0a 100644 --- a/src/manager/client-capi/ckmc-type.cpp +++ b/src/manager/client-capi/ckmc-type.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2000-2019 Samsung Electronics Co., Ltd. All rights reserved + * Copyright (c) 2014 - 2020 Samsung Electronics Co., Ltd. All rights reserved * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -35,7 +35,6 @@ #include #include #include -#include #include #include @@ -240,8 +239,6 @@ int ckmc_cert_new(unsigned char *raw_cert, size_t cert_size, KEY_MANAGER_CAPI int ckmc_load_cert_from_file(const char *file_path, ckmc_cert_s **cert) { - CKM::initOpenSslOnce(); - FILE *fp = fopen(file_path, "r"); if (fp == NULL) @@ -478,8 +475,6 @@ int ckmc_load_from_pkcs12_file(const char *file_path, const char *passphrase, LogWarning("DEPRECATION WARNING: " << __func__ << "() is deprecated and will be " "removed from next release. Use ckmc_pkcs12_load() instead."); - CKM::initOpenSslOnce(); - int ret = CKMC_ERROR_NONE; Pkcs12Converter converter; diff --git a/src/manager/client/client-manager-impl.cpp b/src/manager/client/client-manager-impl.cpp index d5b2df5..9eecd5a 100644 --- a/src/manager/client/client-manager-impl.cpp +++ b/src/manager/client/client-manager-impl.cpp @@ -23,7 +23,6 @@ #include #include -#include #include #include #include @@ -94,7 +93,6 @@ Manager::Impl::Impl() m_ocspConnection(SERVICE_SOCKET_OCSP), m_encryptionConnection(SERVICE_SOCKET_ENCRYPTION) { - initOpenSslOnce(); } diff --git a/src/manager/common/crypto-init.cpp b/src/manager/common/crypto-init.cpp deleted file mode 100644 index 7fa6671..0000000 --- a/src/manager/common/crypto-init.cpp +++ /dev/null @@ -1,210 +0,0 @@ -/* - * Copyright (c) 2000 - 2019 Samsung Electronics Co., Ltd All Rights Reserved - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License - */ -/* - * @file crypto-init.cpp - * @author Maciej Karpiuk (m.karpiuk2@samsung.com) - * @version 1.0 - */ - -#include "crypto-init.h" - -#include -#include -#include -#include -#include - -#include -#include -#include -#include -#include -#include - -#include - -namespace CKM { -namespace { - -const char *DEV_HW_RANDOM_FILE = "/dev/hwrng"; -const char *DEV_URANDOM_FILE = "/dev/urandom"; -const size_t RANDOM_BUFFER_LEN = 32; - -void initializeEntropy() // entropy sources - /dev/random,/dev/urandom(Default) -{ - int ret = 0; - - std::ifstream ifile(DEV_HW_RANDOM_FILE); - - if (ifile.is_open()) - ret = RAND_load_file(DEV_HW_RANDOM_FILE, RANDOM_BUFFER_LEN); - - if (ret != RANDOM_BUFFER_LEN) { - LogWarning("Error in HW_RAND file load"); - ret = RAND_load_file(DEV_URANDOM_FILE, RANDOM_BUFFER_LEN); - - if (ret != RANDOM_BUFFER_LEN) - LogError("Error in U_RAND_file_load"); - } -} - -#if OPENSSL_VERSION_NUMBER < 0x10100000L -std::mutex *g_mutexes = NULL; - -void lockingCallback(int mode, int type, const char *, int) -{ - if (!g_mutexes) { - LogError("Openssl mutexes do not exist"); - return; - } - - if (mode & CRYPTO_LOCK) - g_mutexes[type].lock(); - else if (mode & CRYPTO_UNLOCK) - g_mutexes[type].unlock(); -} - -unsigned long threadIdCallback() -{ - std::hash hasher; - return hasher(std::this_thread::get_id()); -} - -void opensslInstallLocks() -{ - g_mutexes = new std::mutex[CRYPTO_num_locks()]; - - CRYPTO_set_id_callback(threadIdCallback); - CRYPTO_set_locking_callback(lockingCallback); -} - -void opensslUninstallLocks() -{ - CRYPTO_set_id_callback(NULL); - CRYPTO_set_locking_callback(NULL); - - delete[] g_mutexes; - g_mutexes = NULL; -} - -void initOpenSsl(bool isLib) -{ - /* - * Initialize libcrypto (add all algorithms, digests & ciphers) - * It also does the stuff from SSL_library_init() except for ssl_load_ciphers() - */ - OpenSSL_add_all_algorithms(); // Can be optimized by using EVP_add_cipher instead - - if (isLib) - return; - - // below initializes only for executable client. (key-manager daemon) - - initializeEntropy(); - - /* - * Initialize libssl (OCSP uses it) - * SSL_library_init() == OpenSSL_add_ssl_algorithms() - * It always returns 1 - */ - SSL_library_init(); - - // load default configuration (/etc/ssl/openssl.cnf) - OPENSSL_config(NULL); - // Loads all error strings (crypto and ssl) - SSL_load_error_strings(); - - // Install locks for multithreading support - opensslInstallLocks(); -} - -std::mutex cryptoInitMutex; - -void initOpenSslAndDetach(); - -typedef void(*initFnPtr)(); - -// has to be atomic as storing function pointer is not an atomic operation on armv7l -std::atomic initFn(&initOpenSslAndDetach); - -void initEmpty() {} - -// this function will be called only once by initOpenSslOnce for library client -void initOpenSslAndDetach() -{ - // DCLP - std::lock_guard lock(cryptoInitMutex); - - /* - * We don't care about memory ordering here. Current thread will order it - * correctly and for other threads only store matters. Also only one thread - * can be here at once because of lock. - */ - if (initFn.load(std::memory_order_relaxed) != &initEmpty) { - initOpenSsl(true); - - /* - * Synchronizes with load. Everything that happened before this store in - * this thread is visible to everything that happens after load in another - * thread. We switch to an empty function here. - */ - initFn.store(&initEmpty, std::memory_order_release); - } -} -#endif -} // namespace anonymous - -#if OPENSSL_VERSION_NUMBER < 0x10100000L -void initOpenSsl() -{ - initOpenSsl(false); -} - -void deinitOpenSsl() -{ - opensslUninstallLocks(); - CONF_modules_free(); // cleanup of OPENSSL_config - EVP_cleanup(); // cleanup of OpenSSL_add_all_algorithms - ERR_free_strings(); //cleanup of SSL_load_error_strings - deinitOpenSslThread(); -} - -void deinitOpenSslThread() -{ - CRYPTO_cleanup_all_ex_data(); - ERR_remove_thread_state(NULL); -} - -void initOpenSslOnce() -{ - /* - * Synchronizes with store. Everything that happened before store in another - * thread will be visible in this thread after load. - */ - initFn.load(std::memory_order_acquire)(); -} -#else -void initOpenSsl() -{ - initializeEntropy(); -} - -void deinitOpenSsl() {} -void deinitOpenSslThread() {} -void initOpenSslOnce() {} -#endif - -} // namespace CKM diff --git a/src/manager/common/crypto-init.h b/src/manager/common/crypto-init.h deleted file mode 100644 index d8abeca..0000000 --- a/src/manager/common/crypto-init.h +++ /dev/null @@ -1,48 +0,0 @@ -/* Copyright (c) 2000 - 2013 Samsung Electronics Co., Ltd All Rights Reserved - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License - * - * - * @file crypto-init.h - * @author Bartlomiej Grzelewski (b.grzelewski@samsung.com) - * @version 1.0 - * @brief Crypto module implementation. - */ -#pragma once - -#include - -namespace CKM { -// Remarks! -// These functions are used carefully depending on library / executable client. -// -// Init/deinit locking functions are only available for executable client -// (it's key-manager daemon) -// -// For library client, locking functions are not supported because it can make -// undefined behavior(usually segmentation fault) when the client is used as -// plugin(dynamic loaded) because there's probability of openssl's locking function -// being init/deinit on multiple plugins. - -// Must be called once manually because it'll handle openssl locking functions. -// Only for server. -COMMON_API void initOpenSsl(); -COMMON_API void deinitOpenSsl(); -// deinit for every service thread on server. -COMMON_API void deinitOpenSslThread(); - -// init for client or common libraries. -// It'll only do OpenSSL_add_all_algorithms -COMMON_API void initOpenSslOnce(); - -} // namespace CKM diff --git a/src/manager/common/openssl-error-handler.cpp b/src/manager/common/openssl-error-handler.cpp index 60f5e45..9d2bf37 100644 --- a/src/manager/common/openssl-error-handler.cpp +++ b/src/manager/common/openssl-error-handler.cpp @@ -88,19 +88,12 @@ void errorHandle(const char *file, int line, const char *function, int openssl_r /* known errors */ switch (err) { -#if OPENSSL_VERSION_NUMBER > 0x10100000L case ERR_PACK(ERR_LIB_RSA, RSA_F_RSA_OSSL_PRIVATE_DECRYPT, RSA_R_DATA_GREATER_THAN_MOD_LEN): case ERR_PACK(ERR_LIB_RSA, RSA_F_RSA_OSSL_PUBLIC_DECRYPT, RSA_R_DATA_GREATER_THAN_MOD_LEN): case ERR_PACK(ERR_LIB_RSA, RSA_F_RSA_OSSL_PUBLIC_ENCRYPT, RSA_R_DATA_TOO_LARGE_FOR_MODULUS): case ERR_PACK(ERR_LIB_RSA, RSA_F_RSA_OSSL_PUBLIC_DECRYPT, RSA_R_DATA_TOO_LARGE_FOR_MODULUS): case ERR_PACK(ERR_LIB_RSA, RSA_F_RSA_OSSL_PRIVATE_ENCRYPT, RSA_R_DATA_TOO_LARGE_FOR_MODULUS): case ERR_PACK(ERR_LIB_RSA, RSA_F_RSA_OSSL_PRIVATE_DECRYPT, RSA_R_DATA_TOO_LARGE_FOR_MODULUS): -#else /* OPENSSL_VERSION_NUMBER > 0x10100000L */ - case ERR_PACK(ERR_LIB_RSA, RSA_F_PKEY_RSA_CTRL, RSA_R_INVALID_KEYBITS): - case ERR_PACK(ERR_LIB_RSA, RSA_F_RSA_EAY_PRIVATE_DECRYPT, RSA_R_DATA_TOO_LARGE_FOR_MODULUS): - case ERR_PACK(ERR_LIB_RSA, RSA_F_RSA_EAY_PRIVATE_DECRYPT, RSA_R_DATA_GREATER_THAN_MOD_LEN): - case ERR_PACK(ERR_LIB_RSA, RSA_F_RSA_EAY_PUBLIC_DECRYPT, RSA_R_DATA_GREATER_THAN_MOD_LEN): -#endif /* OPENSSL_VERSION_NUMBER > 0x10100000L */ case ERR_PACK(ERR_LIB_RSA, RSA_F_PKEY_RSA_CTRL, RSA_R_KEY_SIZE_TOO_SMALL): case ERR_PACK(ERR_LIB_RSA, RSA_F_RSA_OSSL_PRIVATE_DECRYPT, RSA_R_MISSING_PRIVATE_KEY): case ERR_PACK(ERR_LIB_RSA, RSA_F_RSA_OSSL_PRIVATE_ENCRYPT, RSA_R_MISSING_PRIVATE_KEY): @@ -131,10 +124,6 @@ void errorHandle(const char *file, int line, const char *function, int openssl_r case ERR_PACK(ERR_LIB_X509, X509_F_X509_VERIFY_CERT, ERR_R_SHOULD_NOT_HAVE_BEEN_CALLED): case ERR_PACK(ERR_LIB_EVP, EVP_F_EVP_PKEY_VERIFY, EVP_R_OPERATION_NOT_SUPPORTED_FOR_THIS_KEYTYPE): case ERR_PACK(ERR_LIB_EVP, EVP_F_EVP_PKEY_VERIFY, EVP_R_OPERATON_NOT_INITIALIZED): -#if OPENSSL_VERSION_NUMBER < 0x10100000L - case ERR_PACK(ERR_LIB_EVP, EVP_F_EVP_VERIFYFINAL, EVP_R_WRONG_PUBLIC_KEY_TYPE): - case ERR_PACK(ERR_LIB_EVP, EVP_F_EVP_VERIFYFINAL, EVP_R_NO_VERIFY_FUNCTION_CONFIGURED): -#endif ret = CKM_API_ERROR_VERIFICATION_FAILED; break; } diff --git a/src/manager/common/pkcs12-impl.cpp b/src/manager/common/pkcs12-impl.cpp index 17f60a1..814781b 100644 --- a/src/manager/common/pkcs12-impl.cpp +++ b/src/manager/common/pkcs12-impl.cpp @@ -25,7 +25,6 @@ #include -#include #include #include @@ -73,9 +72,6 @@ PKCS12Impl::PKCS12Impl(const RawBuffer &buffer, const Password &password) return; } - // needed if parsing is done before manager initialization - initOpenSslOnce(); - if (!PKCS12_verify_mac(pkcs12, password.c_str(), password.size())) { LogDebug("Pkcs12 verify failed. Wrong password"); return; diff --git a/src/manager/crypto/sw-backend/internals.cpp b/src/manager/crypto/sw-backend/internals.cpp index 9a13c03..493d586 100644 --- a/src/manager/crypto/sw-backend/internals.cpp +++ b/src/manager/crypto/sw-backend/internals.cpp @@ -570,15 +570,9 @@ RawBuffer digestSignMessage(EVP_PKEY *privKey, EVP_PKEY_CTX *pctx = NULL; // Create the Message Digest Context -#if OPENSSL_VERSION_NUMBER < 0x10100000L - EvpMdCtxUPtr mdctx(EVP_MD_CTX_create(), EVP_MD_CTX_destroy); - if (!mdctx.get()) - ThrowErr(Exc::Crypto::InternalError, "Error in EVP_MD_CTX_create function"); -#else EvpMdCtxUPtr mdctx(EVP_MD_CTX_new(), EVP_MD_CTX_free); if (!mdctx.get()) ThrowErr(Exc::Crypto::InternalError, "Error in EVP_MD_CTX_new function"); -#endif OPENSSL_ERROR_HANDLE(EVP_DigestSignInit(mdctx.get(), &pctx, md_algo, NULL, privKey)); @@ -643,15 +637,9 @@ int digestVerifyMessage(EVP_PKEY *pubKey, EVP_PKEY_CTX *pctx = NULL; // Create the Message Digest Context -#if OPENSSL_VERSION_NUMBER < 0x10100000L - EvpMdCtxUPtr mdctx(EVP_MD_CTX_create(), EVP_MD_CTX_destroy); - if (!mdctx.get()) - ThrowErr(Exc::Crypto::InternalError, "Error in EVP_MD_CTX_create function"); -#else EvpMdCtxUPtr mdctx(EVP_MD_CTX_new(), EVP_MD_CTX_free); if (!mdctx.get()) ThrowErr(Exc::Crypto::InternalError, "Error in EVP_MD_CTX_new function"); -#endif OPENSSL_ERROR_HANDLE(EVP_DigestVerifyInit(mdctx.get(), &pctx, md_algo, NULL, pubKey)); diff --git a/src/manager/crypto/tz-backend/internals.cpp b/src/manager/crypto/tz-backend/internals.cpp index b5a3dc1..9d8efe9 100644 --- a/src/manager/crypto/tz-backend/internals.cpp +++ b/src/manager/crypto/tz-backend/internals.cpp @@ -81,17 +81,11 @@ void generateDSAParams(const int sizeBits, CKM::RawBuffer &prime, // at this stage dsa->p, dsa->q & dsa->r should contain our params // extract them into buffers -#if OPENSSL_VERSION_NUMBER < 0x10100000L - prime = extractBignumData(dsa->p); - subprime = extractBignumData(dsa->q); - base = extractBignumData(dsa->g); -#else const BIGNUM *p, *q, *g; DSA_get0_pqg(dsa.get(), &p, &q, &g); prime = extractBignumData(p); subprime = extractBignumData(q); base = extractBignumData(g); -#endif } tz_data_type toTzDataType(const CKM::DataType dataType) { diff --git a/src/manager/main/key-manager-main.cpp b/src/manager/main/key-manager-main.cpp index 47b5c34..bb867b7 100644 --- a/src/manager/main/key-manager-main.cpp +++ b/src/manager/main/key-manager-main.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2000 - 2014 Samsung Electronics Co., Ltd All Rights Reserved + * Copyright (c) 2014 - 2020 Samsung Electronics Co., Ltd All Rights Reserved * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -22,6 +22,10 @@ #include #include +#include + +#include + #include #include @@ -31,11 +35,36 @@ #include #include #include -#include #include #include +namespace { + +const char *DEV_HW_RANDOM_FILE = "/dev/hwrng"; +const char *DEV_URANDOM_FILE = "/dev/urandom"; +const size_t RANDOM_BUFFER_LEN = 32; + +void initializeEntropy() // entropy sources - /dev/random,/dev/urandom(Default) +{ + int ret = 0; + + std::ifstream ifile(DEV_HW_RANDOM_FILE); + + if (ifile.is_open()) + ret = RAND_load_file(DEV_HW_RANDOM_FILE, RANDOM_BUFFER_LEN); + + if (ret != RANDOM_BUFFER_LEN) { + LogWarning("Error in HW_RAND file load"); + ret = RAND_load_file(DEV_URANDOM_FILE, RANDOM_BUFFER_LEN); + + if (ret != RANDOM_BUFFER_LEN) + LogError("Error in U_RAND_file_load"); + } +} + +} // anonymous namespace + #define REGISTER_SOCKET_SERVICE(manager, service) \ registerSocketService(manager, #service) @@ -91,7 +120,7 @@ int main(void) LogInfo("Init external libraries SKMM and openssl"); - CKM::initOpenSsl(); + initializeEntropy(); CKM::KeyProvider::initializeLibrary(); @@ -110,8 +139,6 @@ int main(void) // Manager has been destroyed and we may close external libraries. LogInfo("Deinit SKMM and openssl"); CKM::KeyProvider::closeLibrary(); - - CKM::deinitOpenSsl(); } catch (const std::runtime_error &e) { LogError(e.what()); } diff --git a/src/manager/main/service-thread.h b/src/manager/main/service-thread.h index dd11452..ac7ea75 100644 --- a/src/manager/main/service-thread.h +++ b/src/manager/main/service-thread.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2000-2019 Samsung Electronics Co., Ltd. All rights reserved + * Copyright (c) 2014 - 2020 Samsung Electronics Co., Ltd. All rights reserved * * Contact: Dongsun Lee * @@ -34,8 +34,6 @@ #include #include -#include - #include #include @@ -102,9 +100,6 @@ protected: static void ThreadLoopStatic(ServiceThread *ptr) { ptr->ThreadLoop(); - - // cleanup openssl in every thread - deinitOpenSslThread(); } void ThreadLoop() diff --git a/unit-tests/CMakeLists.txt b/unit-tests/CMakeLists.txt index 35c05d8..087717f 100644 --- a/unit-tests/CMakeLists.txt +++ b/unit-tests/CMakeLists.txt @@ -101,7 +101,6 @@ SET(UNIT_TESTS_SOURCES ${MANAGER_PATH}/common/base64.cpp ${MANAGER_PATH}/common/certificate-impl.cpp ${MANAGER_PATH}/common/ckm-zero-memory.cpp - ${MANAGER_PATH}/common/crypto-init.cpp ${MANAGER_PATH}/common/data-type.cpp ${MANAGER_PATH}/common/exception.cpp ${MANAGER_PATH}/common/key-impl.cpp -- 2.7.4 From 641b0b4d0315b3a1fae67e4ad75805741ece4b5d Mon Sep 17 00:00:00 2001 From: Krzysztof Jackiewicz Date: Wed, 15 Jul 2020 16:57:51 +0200 Subject: [PATCH 07/16] Improve DB::Crypto negative test ratio Redundant positive tests removed. Negative constructor tests added. Change-Id: Ic1c2d30d4121c4e901485cae63cb7a203865af7d --- unit-tests/test_db_crypto.cpp | 72 +++++++++++++++++++++++-------------------- 1 file changed, 39 insertions(+), 33 deletions(-) diff --git a/unit-tests/test_db_crypto.cpp b/unit-tests/test_db_crypto.cpp index 1317561..2fb5182 100644 --- a/unit-tests/test_db_crypto.cpp +++ b/unit-tests/test_db_crypto.cpp @@ -63,28 +63,13 @@ POSITIVE_TEST_CASE(DBtestSimple) rowPattern.tag = RawBuffer(AES_GCM_TAG_SIZE, 1); check_DB_integrity(rowPattern); -} -POSITIVE_TEST_CASE(DBtestBIG) -{ - DB::Row rowPattern = create_default_row(); + rowPattern.data = createBigBlob(4096); rowPattern.dataSize = rowPattern.data.size(); - rowPattern.tag = RawBuffer(AES_GCM_TAG_SIZE, 1); check_DB_integrity(rowPattern); } -POSITIVE_TEST_CASE(DBtestGlobal) -{ - DB::Row rowPattern = create_default_row(); - rowPattern.data = RawBuffer(1024, 2); - rowPattern.dataSize = rowPattern.data.size(); - rowPattern.tag = RawBuffer(AES_GCM_TAG_SIZE, 1); - - BOOST_REQUIRE_NO_THROW(m_db.saveRow(rowPattern)); - DB::Row name_duplicate = rowPattern; - rowPattern.owner = rowPattern.owner + "1"; -} POSITIVE_TEST_CASE(DBtestTransaction) { DB::Row rowPattern = create_default_row(); @@ -103,23 +88,6 @@ POSITIVE_TEST_CASE(DBtestTransaction) BOOST_CHECK_MESSAGE(!row_optional, "Row still present after rollback"); } -POSITIVE_TEST_CASE(DBtestBackend) -{ - DB::Row rowPattern = create_default_row(); - rowPattern.data = RawBuffer(32, 1); - rowPattern.dataSize = rowPattern.data.size(); - rowPattern.tag = RawBuffer(AES_GCM_TAG_SIZE, 1); - - rowPattern.backendId = CryptoBackend::OpenSSL; - check_DB_integrity(rowPattern); - - rowPattern.backendId = CryptoBackend::TrustZone; - check_DB_integrity(rowPattern); - - rowPattern.backendId = CryptoBackend::None; - check_DB_integrity(rowPattern); -} - POSITIVE_TEST_CASE(DBtestMove) { struct TestCrypto : public DB::Crypto { @@ -383,6 +351,44 @@ NEGATIVE_TEST_CASE(DBtestClientKey) BOOST_AUTO_TEST_SUITE_END() +BOOST_AUTO_TEST_SUITE(DBCRYPTO_CTOR_TEST) + +NEGATIVE_TEST_CASE(DBtestCryptoWrongPasswordLegacy) +{ + BOOST_REQUIRE_THROW(DB::Crypto(DB_TEST_DIR "/testme_ver3.db", "", CKM::RawBuffer(16)), + Exc::DatabaseFailed); +} + +NEGATIVE_TEST_CASE(DBtestCryptoWrongPasswordCurrent) +{ + BOOST_REQUIRE_THROW(DB::Crypto("", DB_TEST_DIR "/testme0_ver4.db", CKM::RawBuffer(16)), + Exc::DatabaseFailed); +} + +NEGATIVE_TEST_CASE(DBtestCryptoNonExistingBoth) +{ + BOOST_REQUIRE_THROW(DB::Crypto("/not/existing.db", "/not/existing.db", defaultPass), + Exc::DatabaseFailed); +} + +NEGATIVE_TEST_CASE(DBtestCryptoNonExistingCurrent) +{ + BOOST_REQUIRE_THROW(DB::Crypto(DB_TEST_DIR "/testme_ver3.db", "/not/existing.db", defaultPass), + Exc::DatabaseFailed); +} + +NEGATIVE_TEST_CASE(DBtestCryptoNotDatabaseLegacy) +{ + BOOST_REQUIRE_THROW(DB::Crypto("/usr/bin/yes", "", defaultPass), Exc::DatabaseFailed); +} + +NEGATIVE_TEST_CASE(DBtestCryptoNotDatabaseCurrent) +{ + BOOST_REQUIRE_THROW(DB::Crypto("", "/usr/bin/yes", defaultPass), Exc::DatabaseFailed); +} + +BOOST_AUTO_TEST_SUITE_END() + BOOST_FIXTURE_TEST_SUITE(DBCRYPTO_PERF_TEST, DBFixture) -- 2.7.4 From 88a82d64a66c2205cac8f79bd767798618aa1057 Mon Sep 17 00:00:00 2001 From: Krzysztof Jackiewicz Date: Wed, 15 Jul 2020 21:25:26 +0200 Subject: [PATCH 08/16] Move db perf tests to a separate exec Performance tests are not unit tests and do not improve code coverage. Also they are all "positive". This commit moves them to a separate binary. Also fixed performance calculation and few other minor issues. Code slightly refactored. Change-Id: Ifcf2463be28001a0e88e5127dd95ee081771382a --- {unit-tests => common}/DBFixture.cpp | 35 +---- {unit-tests => common}/DBFixture.h | 14 +- {unit-tests => common}/test_common.cpp | 0 {unit-tests => common}/test_common.h | 0 misc/CMakeLists.txt | 3 +- misc/db_perf/CMakeLists.txt | 54 ++++++++ misc/db_perf/test_db_perf.cpp | 229 +++++++++++++++++++++++++++++++++ packaging/key-manager.spec | 1 + unit-tests/CMakeLists.txt | 4 +- unit-tests/test_db_crypto.cpp | 152 +--------------------- 10 files changed, 295 insertions(+), 197 deletions(-) rename {unit-tests => common}/DBFixture.cpp (85%) rename {unit-tests => common}/DBFixture.h (85%) rename {unit-tests => common}/test_common.cpp (100%) rename {unit-tests => common}/test_common.h (100%) create mode 100644 misc/db_perf/CMakeLists.txt create mode 100644 misc/db_perf/test_db_perf.cpp diff --git a/unit-tests/DBFixture.cpp b/common/DBFixture.cpp similarity index 85% rename from unit-tests/DBFixture.cpp rename to common/DBFixture.cpp index 3f8c3cc..ee85abc 100644 --- a/unit-tests/DBFixture.cpp +++ b/common/DBFixture.cpp @@ -21,13 +21,11 @@ * @brief */ #include -#include -#include +#include #include #include using namespace CKM; -using namespace std::chrono; static void copyFile(const char *src, const char *dst) { @@ -66,9 +64,6 @@ DBFixture::DBFixture(const char *legacy_db_fname, const char *db_fname, DBCrypto void DBFixture::init(DBCryptoThrows dbCryptoThrows) { - high_resolution_clock::time_point srand_feed = high_resolution_clock::now(); - srand(srand_feed.time_since_epoch().count()); - switch (dbCryptoThrows) { case DBCryptoThrows::yes: BOOST_REQUIRE_THROW(m_db = DB::Crypto(m_crypto_legacy_db_fname, m_crypto_db_fname, defaultPass), Exc::DatabaseFailed); @@ -79,31 +74,6 @@ void DBFixture::init(DBCryptoThrows dbCryptoThrows) } } -double DBFixture::performance_get_time_elapsed_ms() -{ - return duration_cast(m_end_time - m_start_time).count(); -} - -void DBFixture::performance_start(const char *operation_name) -{ - m_operation = std::string(operation_name ? operation_name : "unknown"); - BOOST_TEST_MESSAGE("\t running " << m_operation << - " performance test..."); - m_start_time = high_resolution_clock::now(); -} - -void DBFixture::performance_stop(long num_operations_performed) -{ - m_end_time = high_resolution_clock::now(); - double time_elapsed_ms = performance_get_time_elapsed_ms(); - BOOST_TEST_MESSAGE("\t time elapsed: " << time_elapsed_ms << - "[ms], number of " << m_operation << ": " << num_operations_performed); - - if (num_operations_performed > 0) - BOOST_TEST_MESSAGE("\t average time per " << m_operation << ": " << - time_elapsed_ms / num_operations_performed << "[ms]"); -} - void DBFixture::generate_name(unsigned int id, Name &output) { std::stringstream ss; @@ -118,8 +88,7 @@ void DBFixture::generate_owner(unsigned int id, ClientId &output) output = ss.str(); } -void DBFixture::generate_perf_DB(unsigned int num_name, - unsigned int names_per_owner) +void DBFixture::generate_db(unsigned int num_name, unsigned int names_per_owner) { // to speed up data creation - cache the row DB::Row rowPattern = create_default_row(DataType::BINARY_DATA); diff --git a/unit-tests/DBFixture.h b/common/DBFixture.h similarity index 85% rename from unit-tests/DBFixture.h rename to common/DBFixture.h index fb036b6..2fd022a 100644 --- a/unit-tests/DBFixture.h +++ b/common/DBFixture.h @@ -22,10 +22,11 @@ */ #pragma once -#include #include #include -#include +#include +#include +#include class DBFixture { public: @@ -48,12 +49,8 @@ public: static CKM::DB::Row create_default_binary_row(); static void compare_row(const CKM::DB::Row &lhs, const CKM::DB::Row &rhs); - // ::::::::::::::::::::::::: time measurement ::::::::::::::::::::::::: - void performance_start(const char *operation_name); - void performance_stop(long num_operations_performed); - // ::::::::::::::::::::::::: DB ::::::::::::::::::::::::: - void generate_perf_DB(unsigned int num_name, unsigned int names_per_owner); + void generate_db(unsigned int num_name, unsigned int names_per_owner); long add_full_access_rights(unsigned int num_name, unsigned int num_names_per_owner); void check_DB_integrity(const CKM::DB::Row &rowPattern); @@ -69,11 +66,8 @@ public: private: void init(DBCryptoThrows dbCryptoThrows); - double performance_get_time_elapsed_ms(); static void unlinkDb(); constexpr static const char *m_crypto_legacy_db_fname = "/tmp/testme.db"; constexpr static const char *m_crypto_db_fname = "/tmp/testme0.db"; - std::string m_operation; - std::chrono::high_resolution_clock::time_point m_start_time, m_end_time; }; diff --git a/unit-tests/test_common.cpp b/common/test_common.cpp similarity index 100% rename from unit-tests/test_common.cpp rename to common/test_common.cpp diff --git a/unit-tests/test_common.h b/common/test_common.h similarity index 100% rename from unit-tests/test_common.h rename to common/test_common.h diff --git a/misc/CMakeLists.txt b/misc/CMakeLists.txt index 1dcd73f..b149a7e 100644 --- a/misc/CMakeLists.txt +++ b/misc/CMakeLists.txt @@ -47,4 +47,5 @@ INSTALL(TARGETS ${CKM_TOOL} ) ADD_SUBDIRECTORY(ckm_db_tool) ADD_SUBDIRECTORY(ckm_initial_values) -ADD_SUBDIRECTORY(encryption_scheme) \ No newline at end of file +ADD_SUBDIRECTORY(encryption_scheme) +ADD_SUBDIRECTORY(db_perf) \ No newline at end of file diff --git a/misc/db_perf/CMakeLists.txt b/misc/db_perf/CMakeLists.txt new file mode 100644 index 0000000..672a079 --- /dev/null +++ b/misc/db_perf/CMakeLists.txt @@ -0,0 +1,54 @@ +SET(DB_TEST_DIR ${UNIT_TESTS_DIR}/db) + +ADD_DEFINITIONS("-DBOOST_TEST_DYN_LINK") +ADD_DEFINITIONS("-DDB_TEST_DIR=\"${DB_TEST_DIR}\"") + +SET(MANAGER_PATH ${PROJECT_SOURCE_DIR}/src/manager) + +INCLUDE_DIRECTORIES(SYSTEM ${KEY_MANAGER_DEP_INCLUDE_DIRS}) + +INCLUDE_DIRECTORIES( + ${MANAGER_PATH}/dpl/db/include + ${MANAGER_PATH}/dpl/core/include + ${MANAGER_PATH}/dpl/log/include + ${MANAGER_PATH}/service + ${MANAGER_PATH}/common + ${PROJECT_SOURCE_DIR}/src/include + ${PROJECT_SOURCE_DIR}/common + ${CMAKE_CURRENT_SOURCE_DIR} +) + +SET(DB_PERF_SOURCES + ${CMAKE_CURRENT_SOURCE_DIR}/test_db_perf.cpp + + ${PROJECT_SOURCE_DIR}/common/colour_log_formatter.cpp + ${PROJECT_SOURCE_DIR}/common/DBFixture.cpp + ${PROJECT_SOURCE_DIR}/common/test_common.cpp + + ${MANAGER_PATH}/dpl/core/src/colors.cpp + ${MANAGER_PATH}/dpl/db/src/naive_synchronization_object.cpp + ${MANAGER_PATH}/dpl/db/src/sql_connection.cpp + ${MANAGER_PATH}/service/db-crypto.cpp + ${MANAGER_PATH}/service/file-lock.cpp + ${MANAGER_PATH}/service/file-system.cpp + ${MANAGER_PATH}/service/for-each-file.cpp + ${MANAGER_PATH}/service/key-provider.cpp +) + +SET(TARGET_CKM_DB_PERF "ckm_db_perf") + +ADD_EXECUTABLE( + ${TARGET_CKM_DB_PERF} + ${DB_PERF_SOURCES} +) + +TARGET_LINK_LIBRARIES( + ${TARGET_CKM_DB_PERF} + ${TARGET_KEY_MANAGER_COMMON} + ${KEY_MANAGER_DEP_LIBRARIES} + ${KM_LINK_EXTRA_DEPS} + boost_unit_test_framework + -ldl +) + +INSTALL(TARGETS ${TARGET_CKM_DB_PERF} DESTINATION bin) diff --git a/misc/db_perf/test_db_perf.cpp b/misc/db_perf/test_db_perf.cpp new file mode 100644 index 0000000..a7a6780 --- /dev/null +++ b/misc/db_perf/test_db_perf.cpp @@ -0,0 +1,229 @@ +/* + * Copyright (c) 2020 Samsung Electronics Co., Ltd. All rights reserved + * + * Contact: Dongsun Lee + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License + */ +#include + +#define BOOST_TEST_MODULE KEY_MANAGER_DB_PREF +#include +#include +#include + +#include +#include +#include +#include + + +using namespace CKM; + +namespace { +constexpr unsigned OWNERS = 30; +constexpr unsigned NAMES_PER_OWNER = 15; +constexpr unsigned NAMES = OWNERS * NAMES_PER_OWNER; + +struct TestConfig { + TestConfig() + { + boost::unit_test::unit_test_log.set_threshold_level( + boost::unit_test::log_test_units); + boost::unit_test::results_reporter::set_level(boost::unit_test::SHORT_REPORT); + boost::unit_test::unit_test_log.set_formatter(new CKM::colour_log_formatter); + } + ~TestConfig() + { + } +}; + +struct LogSetup { + LogSetup() + { + SetupClientLogSystem(); + Singleton::Instance().SetTag("CKM_DB_PERF_TESTS"); + } + ~LogSetup() {} +}; + +using namespace std::chrono; + +class Perf { +public: + Perf(const char *operation_name, long iterations = 0) { + m_operation = std::string(operation_name ? operation_name : "unknown"); + BOOST_TEST_MESSAGE("\t running " << m_operation << + " performance test..."); + m_start_time = high_resolution_clock::now(); + m_iterations = iterations; + } + + ~Perf() { + auto end_time = high_resolution_clock::now(); + double time_elapsed_ms = duration_cast(end_time - m_start_time).count(); + BOOST_TEST_MESSAGE("\t time elapsed: " << time_elapsed_ms << + "[ms], number of " << m_operation << ": " << m_iterations); + + if (m_iterations > 0) + BOOST_TEST_MESSAGE("\t average time per " << m_operation << ": " << + time_elapsed_ms / m_iterations << "[ms]"); + } + + void setIterations(long iterations) { + m_iterations = iterations; + } + +private: + std::string m_operation; + std::chrono::high_resolution_clock::time_point m_start_time; + long m_iterations; +}; + +} + +BOOST_GLOBAL_FIXTURE(TestConfig); +BOOST_GLOBAL_FIXTURE(LogSetup); + +BOOST_FIXTURE_TEST_SUITE(DBCRYPTO_PERF_TEST, DBFixture) + +BOOST_AUTO_TEST_CASE(DBperfAddNames) +{ + constexpr unsigned ITERATIONS = 1000; + + // actual test + Perf perf("saveRow", ITERATIONS); + { + generate_db(ITERATIONS, NAMES_PER_OWNER); + } +} + +BOOST_AUTO_TEST_CASE(DBperfLookupAliasByOwner) +{ + constexpr unsigned ITERATIONS = 1000; + + // prepare data + generate_db(NAMES, NAMES_PER_OWNER); + + Name name; + ClientId owner; + + // actual test - successful lookup + Perf perf("getRow", ITERATIONS * NAMES_PER_OWNER); + + unsigned seed = 0; + + for (unsigned t = 0; t < ITERATIONS; t++) { + int owner_num = rand_r(&seed) % OWNERS; + generate_owner(owner_num, owner); + + unsigned start_name = owner_num * NAMES_PER_OWNER; + + for (unsigned name_num = start_name; + name_num < (start_name + NAMES_PER_OWNER); name_num++) { + generate_name(name_num, name); + read_row_expect_success(name, owner); + + } + } +} + +BOOST_AUTO_TEST_CASE(DBperfLookupAliasRandomOwnership) +{ + constexpr unsigned ITERATIONS = 10000; + + // prepare data + generate_db(NAMES, NAMES_PER_OWNER); + + Name name; + ClientId owner; + + // actual test - random lookup + Perf perf("getRow", ITERATIONS); + unsigned seed = 0; + + for (unsigned t = 0; t < ITERATIONS; t++) { + int name_idx = rand_r(&seed) % NAMES; + generate_name(name_idx, name); + generate_owner(name_idx / NAMES_PER_OWNER, owner); + + read_row_expect_success(name, owner); + } +} + +BOOST_AUTO_TEST_CASE(DBperfAddPermissions) +{ + constexpr unsigned ITERATIONS = 200; + + // prepare data + generate_db(ITERATIONS, NAMES_PER_OWNER); + + // actual test - add access rights + Perf perf("setPermission"); + perf.setIterations(add_full_access_rights(ITERATIONS, NAMES_PER_OWNER)); +} + +BOOST_AUTO_TEST_CASE(DBperfAliasRemovalWithFullPermissions) +{ + // prepare data + generate_db(NAMES, NAMES_PER_OWNER); + add_full_access_rights(NAMES, NAMES_PER_OWNER); + + // actual test - random lookup + Name name; + ClientId owner; + { + Perf perf("deleteRow", NAMES); + + for (unsigned t = 0; t < NAMES; t++) { + generate_name(t, name); + generate_owner(t / NAMES_PER_OWNER, owner); + + BOOST_REQUIRE_NO_THROW(m_db.deleteRow(name, owner)); + } + } + + // verify everything has been removed + for (unsigned l = 0; l < OWNERS; l++) { + generate_owner(l, owner); + OwnerNameVector expect_no_data; + BOOST_REQUIRE_NO_THROW(m_db.listNames(owner, expect_no_data, + DataType::BINARY_DATA)); + BOOST_REQUIRE(0 == expect_no_data.size()); + } +} + +BOOST_AUTO_TEST_CASE(DBperfGetAliasList) +{ + constexpr unsigned ITERATIONS = 1000; + + // prepare data + generate_db(NAMES, NAMES_PER_OWNER); + add_full_access_rights(NAMES, NAMES_PER_OWNER); + + ClientId owner; + unsigned seed = 0; + + // actual test - random lookup + Perf perf("listNames", ITERATIONS); + + for (unsigned t = 0; t < ITERATIONS; t++) { + OwnerNameVector ret_list; + generate_owner(rand_r(&seed) % OWNERS, owner); + + BOOST_REQUIRE_NO_THROW(m_db.listNames(owner, ret_list, DataType::BINARY_DATA)); + BOOST_REQUIRE(NAMES == ret_list.size()); + ret_list.clear(); + } +} +BOOST_AUTO_TEST_SUITE_END() diff --git a/packaging/key-manager.spec b/packaging/key-manager.spec index 5ec0938..d69de9e 100644 --- a/packaging/key-manager.spec +++ b/packaging/key-manager.spec @@ -391,6 +391,7 @@ fi %{bin_dir}/ckm_db_tool %{bin_dir}/ckm_db_merge %{bin_dir}/ckm_generate_db +%{bin_dir}/ckm_db_perf %misc_dir #################### ! %{coverage_only} ####################################### diff --git a/unit-tests/CMakeLists.txt b/unit-tests/CMakeLists.txt index 087717f..8478846 100644 --- a/unit-tests/CMakeLists.txt +++ b/unit-tests/CMakeLists.txt @@ -67,14 +67,14 @@ LINK_DIRECTORIES(${KEY_MANAGER_DEP_LIBRARY_DIRS}) SET(UNIT_TESTS_SOURCES ${PROJECT_SOURCE_DIR}/common/colour_log_formatter.cpp - ${CMAKE_CURRENT_SOURCE_DIR}/DBFixture.cpp + ${PROJECT_SOURCE_DIR}/common/DBFixture.cpp + ${PROJECT_SOURCE_DIR}/common/test_common.cpp ${CMAKE_CURRENT_SOURCE_DIR}/main.cpp ${CMAKE_CURRENT_SOURCE_DIR}/test_async-observer.cpp ${CMAKE_CURRENT_SOURCE_DIR}/test_base64.cpp ${CMAKE_CURRENT_SOURCE_DIR}/test_binary-queue.cpp ${CMAKE_CURRENT_SOURCE_DIR}/test_certificate.cpp ${CMAKE_CURRENT_SOURCE_DIR}/test_comm-manager.cpp - ${CMAKE_CURRENT_SOURCE_DIR}/test_common.cpp ${CMAKE_CURRENT_SOURCE_DIR}/test_crypto-logic.cpp ${CMAKE_CURRENT_SOURCE_DIR}/test_data-type.cpp ${CMAKE_CURRENT_SOURCE_DIR}/test_db_crypto.cpp diff --git a/unit-tests/test_db_crypto.cpp b/unit-tests/test_db_crypto.cpp index 2fb5182..b73e762 100644 --- a/unit-tests/test_db_crypto.cpp +++ b/unit-tests/test_db_crypto.cpp @@ -21,25 +21,13 @@ * @brief */ #include -#include -#include -#include #include -#include -#include #include #include using namespace CKM; namespace { -const int restricted_local = 1; -const int restricted_global = 0; - -const unsigned int c_test_retries = 1000; -const unsigned int c_num_names = 500; -const unsigned int c_num_names_add_test = 5000; -const unsigned int c_names_per_owner = 15; void addRow(DB::RowVector& rows, DataType::Type type) { @@ -390,144 +378,6 @@ NEGATIVE_TEST_CASE(DBtestCryptoNotDatabaseCurrent) BOOST_AUTO_TEST_SUITE_END() -BOOST_FIXTURE_TEST_SUITE(DBCRYPTO_PERF_TEST, DBFixture) - -POSITIVE_TEST_CASE(DBperfAddNames) -{ - // actual test - performance_start("saveRow"); - - { - generate_perf_DB(c_num_names_add_test, c_names_per_owner); - } - - performance_stop(c_num_names_add_test); -} - -POSITIVE_TEST_CASE(DBperfLookupAliasByOwner) -{ - // prepare data - generate_perf_DB(c_num_names, c_names_per_owner); - - unsigned int num_owners = c_num_names / c_names_per_owner; - Name name; - ClientId owner; - - // actual test - successful lookup - performance_start("getRow"); - - for (unsigned int t = 0; t < c_test_retries; t++) { - int owner_num = rand_r(&t) % num_owners; - generate_owner(owner_num, owner); - - unsigned int start_name = owner_num * c_names_per_owner; - - for (unsigned int name_num = start_name; - name_num < (start_name + c_names_per_owner); name_num++) { - generate_name(name_num, name); - read_row_expect_success(name, owner); - } - } - - performance_stop(c_test_retries * c_num_names); -} - -// TODO this test makes no sense. Rewrite it. -POSITIVE_TEST_CASE(DBperfLookupAliasRandomOwnershipNoPermissions) -{ - // prepare data - generate_perf_DB(c_num_names, c_names_per_owner); - - Name name; - ClientId owner; - //ClientId smack_label; - //unsigned int num_owners = c_num_names / c_names_per_owner; - - // actual test - random lookup - performance_start("getRow"); - - for (unsigned int t = 0; t < c_test_retries; t++) { - int name_idx = rand_r(&t) % c_num_names; - generate_name(name_idx, name); - generate_owner(name_idx / c_names_per_owner, owner); - //generate_owner(rand_r(&t) % num_owners, smack_label); - - // do not care of result - m_db.getRow(name, owner, DataType::BINARY_DATA); - } - - performance_stop(c_test_retries * c_num_names); -} - -POSITIVE_TEST_CASE(DBperfAddPermissions) -{ - // prepare data - generate_perf_DB(c_num_names, c_names_per_owner); - - // actual test - add access rights - performance_start("setPermission"); - long iterations = add_full_access_rights(c_num_names, c_names_per_owner); - performance_stop(iterations); -} - -POSITIVE_TEST_CASE(DBperfAliasRemoval) -{ - // prepare data - generate_perf_DB(c_num_names, c_names_per_owner); - add_full_access_rights(c_num_names, c_names_per_owner); - - // actual test - random lookup - performance_start("deleteRow"); - Name name; - ClientId owner; - - for (unsigned int t = 0; t < c_num_names; t++) { - generate_name(t, name); - generate_owner(t / c_names_per_owner, owner); - - BOOST_REQUIRE_NO_THROW(m_db.deleteRow(name, owner)); - } - - performance_stop(c_num_names); - - // verify everything has been removed - unsigned int num_owners = c_num_names / c_names_per_owner; - - for (unsigned int l = 0; l < num_owners; l++) { - generate_owner(l, owner); - OwnerNameVector expect_no_data; - BOOST_REQUIRE_NO_THROW(m_db.listNames(owner, expect_no_data, - DataType::BINARY_DATA)); - BOOST_REQUIRE(0 == expect_no_data.size()); - } -} - -POSITIVE_TEST_CASE(DBperfGetAliasList) -{ - // prepare data - generate_perf_DB(c_num_names, c_names_per_owner); - add_full_access_rights(c_num_names, c_names_per_owner); - - unsigned int num_owners = c_num_names / c_names_per_owner; - ClientId owner; - - // actual test - random lookup - performance_start("listNames"); - - for (unsigned int t = 0; t < (c_test_retries / num_owners); t++) { - OwnerNameVector ret_list; - generate_owner(rand_r(&t) % num_owners, owner); - - BOOST_REQUIRE_NO_THROW(m_db.listNames(owner, ret_list, DataType::BINARY_DATA)); - BOOST_REQUIRE(c_num_names == ret_list.size()); - ret_list.clear(); - } - - performance_stop(c_test_retries / num_owners); -} -BOOST_AUTO_TEST_SUITE_END() - - BOOST_AUTO_TEST_SUITE(DBCRYPTO_MIGRATION_TEST) namespace { const unsigned migration_names = 16107; @@ -641,7 +491,7 @@ POSITIVE_TEST_CASE(DBMigrationDBCurrent) currentDB.generate_owner(migration_reference_owner_idx, reference_owner); { - currentDB.generate_perf_DB(migration_names, migration_names / migration_owners); + currentDB.generate_db(migration_names, migration_names / migration_owners); // only the reference owner has access to the other owners' elements for (unsigned int l = 0; l < migration_owners; l++) { -- 2.7.4 From 69b23b422c6cd388dcd64fd69b74d1f172d1bc20 Mon Sep 17 00:00:00 2001 From: Krzysztof Jackiewicz Date: Fri, 17 Jul 2020 12:12:29 +0200 Subject: [PATCH 09/16] Add negative DescriptorSet tests Change-Id: Idfb7dcd64c17aab418380a8fdb5b807a67710239 --- unit-tests/test_descriptor-set.cpp | 78 +++++++++++++++++++++++++++++++++++++- 1 file changed, 77 insertions(+), 1 deletion(-) diff --git a/unit-tests/test_descriptor-set.cpp b/unit-tests/test_descriptor-set.cpp index 1215639..fb985bb 100644 --- a/unit-tests/test_descriptor-set.cpp +++ b/unit-tests/test_descriptor-set.cpp @@ -22,9 +22,14 @@ #include #include #include +#include +#include +#include +#include #include #include +#include #include #include @@ -168,7 +173,7 @@ POSITIVE_TEST_CASE(T050_DoubleAdd) } /* - * Add pipe[0] descriptor and wait. Callback should not be called. Instead the 8s timeout should + * Add pipe[0] descriptor and wait. Callback should not be called. Instead the 1s timeout should * occur and a proper exception should be thrown. */ NEGATIVE_TEST_CASE(T060_Timeout) @@ -282,4 +287,75 @@ POSITIVE_TEST_CASE(T090_WriteAfterRead) BOOST_REQUIRE_MESSAGE(callback2, "Second callback not called"); } +/* + * Add negative descriptor and wait. Callback should not be called. Instead the 1s timeout + * should occur and a proper exception should be thrown. + */ +NEGATIVE_TEST_CASE(T100_AddNegativeFd) +{ + DescriptorSet descriptors; + descriptors.add(-1, POLLALL, unexpectedCallback); + + BOOST_REQUIRE_THROW(descriptors.wait(POLL_TIMEOUT_SHORT), DescriptorSet::Timeout); +} + +/* + * Cross the descriptor limit which should lead to EINVAL. Utilize unused descriptors only. Callback + * should not be called. + */ +NEGATIVE_TEST_CASE(T110_DescriptorLimit) +{ + DescriptorSet descriptors; + + struct rlimit limit; + BOOST_REQUIRE(getrlimit(RLIMIT_NOFILE, &limit) == 0); + + int fd = 3; + for(rlim_t i = 0; i <= limit.rlim_cur; ++fd) { + int ret = fcntl(fd, F_GETFD); + int err = errno; + BOOST_REQUIRE(ret != -1 || err == EBADF); + if (ret == -1) { + descriptors.add(fd, POLLALL, unexpectedCallback); + ++i; + } + } + + BOOST_REQUIRE_THROW(descriptors.wait(POLL_TIMEOUT_SHORT), DescriptorSet::InternalError); +} + +/* + * Not opened descriptor should trigger a callback with error. + */ +NEGATIVE_TEST_CASE(T120_NotOpenedDescriptor) +{ + DescriptorSet descriptors; + + bool callbackCalled = false; + auto callback = [&](int, short revents) { + callbackCalled = true; + BOOST_REQUIRE(revents == POLLNVAL); + }; + descriptors.add(42, POLLALL, callback); + + BOOST_REQUIRE_NO_THROW(descriptors.wait(POLL_TIMEOUT)); + BOOST_REQUIRE(callbackCalled); +} + +/* + * Throwing callback. The exception should be propagated outside of DescriptorSet::wait(). + */ +NEGATIVE_TEST_CASE(T130_ThrowingCallback) +{ + DescriptorSet descriptors; + + auto throwingCallback = [&](int, short) { + throw std::runtime_error(""); + }; + descriptors.add(42, POLLALL, throwingCallback); + + BOOST_REQUIRE_THROW(descriptors.wait(POLL_TIMEOUT), std::runtime_error); +} + + BOOST_AUTO_TEST_SUITE_END() -- 2.7.4 From 8bd6ef2e565b8c0bb624ffe1cc46a7f5d934d7f1 Mon Sep 17 00:00:00 2001 From: Krzysztof Jackiewicz Date: Fri, 17 Jul 2020 13:52:17 +0200 Subject: [PATCH 10/16] Exception tests refactoring * Positive tests merged into one. * Macros replaced with templates. * Missing exceptions added. Change-Id: Ia2da4262e874119a70940c1005d7c018aea9641b --- unit-tests/test_exception.cpp | 91 +++++++++++++++++++------------------------ 1 file changed, 39 insertions(+), 52 deletions(-) diff --git a/unit-tests/test_exception.cpp b/unit-tests/test_exception.cpp index cac3ec2..50e7d73 100644 --- a/unit-tests/test_exception.cpp +++ b/unit-tests/test_exception.cpp @@ -20,69 +20,56 @@ using namespace CKM; -#define CHECK_EXCEPTION(exception, ec, isDebug) \ -do { \ - const std::string errmsg = "test exception string"; \ - BOOST_REQUIRE_THROW(ThrowErr(exception, errmsg), exception); \ - try { \ - ThrowErr(exception, errmsg); \ - } catch (const exception &e) { \ - checkExceptionInternal(e, ec, isDebug ? std::string() : errmsg); \ - } \ -} while (false) - namespace { -void checkExceptionInternal(const Exc::Exception &e, int ec, const std::string &msg) -{ - BOOST_REQUIRE_MESSAGE(e.error() == ec, - "ec(" << ec << ") not matched(" << e.error() << ")"); - - BOOST_REQUIRE_MESSAGE(msg == e.what(), - "msg(" << msg << ") isn't matched(" << e.what() << ")"); - - BOOST_REQUIRE_MESSAGE(e.message().find(msg) != std::string::npos, - "msg(" << msg << ") isn't contained from message(" - << e.message() << ")"); -} - -} // namespace anonymous - -BOOST_AUTO_TEST_SUITE(EXCEPTION_TEST) - -POSITIVE_TEST_CASE(internal_error) -{ - CHECK_EXCEPTION(Exc::InternalError, CKM_API_ERROR_SERVER_ERROR, false); -} - -POSITIVE_TEST_CASE(database_locked) -{ - CHECK_EXCEPTION(Exc::DatabaseLocked, CKM_API_ERROR_DB_LOCKED, false); -} - -POSITIVE_TEST_CASE(database_failed) +template +void verifyCatched(const Exc::DefineException& e, + const std::string errmsg) { - CHECK_EXCEPTION(Exc::DatabaseFailed, CKM_API_ERROR_DB_ERROR, false); + BOOST_REQUIRE_MESSAGE(e.error() == Error, + "ec(" << Error << ") not matched(" << e.error() << ")"); + + if (IsDebug) { + BOOST_REQUIRE_MESSAGE(std::string() == e.what(), + "expected empty e.what(), got(" << e.what() << ")"); + } else { + BOOST_REQUIRE_MESSAGE(errmsg == e.what(), + "msg(" << errmsg << ") isn't matched(" << e.what() << ")"); + + BOOST_REQUIRE_MESSAGE(e.message().find(errmsg) != std::string::npos, + "msg(" << errmsg << ") isn't contained from message(" + << e.message() << ")"); + } } -POSITIVE_TEST_CASE(filesystem_failed) +template +void checkException() { - CHECK_EXCEPTION(Exc::FileSystemFailed, CKM_API_ERROR_FILE_SYSTEM, false); + const std::string errmsg = "test exception string"; + try { + ThrowErr(Exc, errmsg); + BOOST_FAIL("No exception thrown"); + } catch (const Exc &e) { + verifyCatched(e ,errmsg); + } } -POSITIVE_TEST_CASE(inputparam) -{ - CHECK_EXCEPTION(Exc::InputParam, CKM_API_ERROR_INPUT_PARAM, true); -} +} // namespace anonymous -POSITIVE_TEST_CASE(authentication_failed) -{ - CHECK_EXCEPTION(Exc::AuthenticationFailed, CKM_API_ERROR_AUTHENTICATION_FAILED, true); -} +BOOST_AUTO_TEST_SUITE(EXCEPTION_TEST) -POSITIVE_TEST_CASE(transaction_failed) +POSITIVE_TEST_CASE(check_exceptions) { - CHECK_EXCEPTION(Exc::TransactionFailed, CKM_API_ERROR_DB_ERROR, false); + checkException(); + checkException(); + checkException(); + checkException(); + checkException(); + checkException(); + checkException(); + checkException(); + checkException(); + checkException(); } BOOST_AUTO_TEST_SUITE_END() -- 2.7.4 From 089729412d38dc682a90338676f8e8654566d73a Mon Sep 17 00:00:00 2001 From: Krzysztof Jackiewicz Date: Fri, 17 Jul 2020 14:11:56 +0200 Subject: [PATCH 11/16] Add negative forEachFile tests Change-Id: Ic4869009234676967e3571868ad2fa1e1d950c6a --- unit-tests/test_for-each-file.cpp | 27 ++++++++++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/unit-tests/test_for-each-file.cpp b/unit-tests/test_for-each-file.cpp index 6a7954f..7de5ec3 100644 --- a/unit-tests/test_for-each-file.cpp +++ b/unit-tests/test_for-each-file.cpp @@ -18,16 +18,25 @@ * @author Kyungwook Tak (k.tak@samsung.com) * @version 1.0 */ -#include +#include +#include #include #include -#include -#include +#include +#include using namespace CKM; +namespace { +void unexpectedCallback(const std::string&) +{ + BOOST_FAIL("Unexpected callback"); +} + +} // anonymous namespace + BOOST_AUTO_TEST_SUITE(TRAVERSE_DIR_TEST) POSITIVE_TEST_CASE(T010_check_prefix) @@ -45,4 +54,16 @@ POSITIVE_TEST_CASE(T010_check_prefix) "files num in traverse dir should be 10"); } +NEGATIVE_TEST_CASE(T020_not_existing_dir) +{ + BOOST_REQUIRE_THROW(forEachFile("/not/existing/dir", unexpectedCallback), + Exc::FileSystemFailed); +} + +NEGATIVE_TEST_CASE(T030_not_a_directory) +{ + BOOST_REQUIRE_THROW(forEachFile(DB_TEST_DIR "/traverse/res-1", unexpectedCallback), + Exc::FileSystemFailed); +} + BOOST_AUTO_TEST_SUITE_END() -- 2.7.4 From a12f1444697169f9fac905a2615c3fd5fb5de02f Mon Sep 17 00:00:00 2001 From: Krzysztof Jackiewicz Date: Fri, 17 Jul 2020 16:57:46 +0200 Subject: [PATCH 12/16] Remove KeyProvider lib initialization from tests It's a NOOP on tizen.org. Change-Id: I915bba5e55a6f21925c363687b1990d24bf2f2cf --- unit-tests/main.cpp | 24 ------------------------ unit-tests/test_key-provider.cpp | 20 -------------------- 2 files changed, 44 deletions(-) diff --git a/unit-tests/main.cpp b/unit-tests/main.cpp index 0a2c09d..9c8c753 100644 --- a/unit-tests/main.cpp +++ b/unit-tests/main.cpp @@ -20,7 +20,6 @@ */ #include -#include #define BOOST_TEST_MODULE KEY_MANAGER_UNIT_TESTS #include @@ -45,28 +44,6 @@ struct TestConfig { } }; -bool isLibInitialized = false; - -struct KeyProviderLib { - KeyProviderLib() - { - try { - CKM::KeyProvider::initializeLibrary(); - isLibInitialized = true; - } catch (const CKM::Exc::Exception &) { - std::cout << "Library initialization failed!" << std::endl; - } - } - ~KeyProviderLib() - { - try { - CKM::KeyProvider::closeLibrary(); - } catch (const CKM::Exc::Exception &) { - std::cout << "Library deinitialization failed!" << std::endl; - } - } -}; - struct LogSetup { LogSetup() { @@ -76,7 +53,6 @@ struct LogSetup { ~LogSetup() {} }; -BOOST_GLOBAL_FIXTURE(KeyProviderLib); BOOST_GLOBAL_FIXTURE(TestConfig); BOOST_GLOBAL_FIXTURE(LogSetup); diff --git a/unit-tests/test_key-provider.cpp b/unit-tests/test_key-provider.cpp index deeb1aa..d324f52 100644 --- a/unit-tests/test_key-provider.cpp +++ b/unit-tests/test_key-provider.cpp @@ -36,13 +36,9 @@ const std::string USERNAME_LONG = "SOFTWARE_CENTER_SYSTEM_SW_LAB"; const std::string CLIENT_ID_1 = "SAMPLE_CLIENT_ID_1"; const std::string CLIENT_ID_2 = "SAMPLE_CLIENT_ID_2"; -extern bool isLibInitialized; - BOOST_AUTO_TEST_SUITE(KEY_PROVIDER_TEST) POSITIVE_TEST_CASE(KeyDomainKEK) { - BOOST_REQUIRE_MESSAGE(isLibInitialized, - "Library is not initialized!"); CKM::KeyProvider keyProvider; CKM::RawBuffer rb_test; BOOST_REQUIRE_NO_THROW(rb_test = @@ -54,8 +50,6 @@ POSITIVE_TEST_CASE(KeyDomainKEK) NEGATIVE_TEST_CASE(KeyDomainKekInvalidPassword) { - BOOST_REQUIRE_MESSAGE(isLibInitialized, - "Library is not initialized!"); CKM::KeyProvider keyProvider; CKM::RawBuffer rb_test; BOOST_REQUIRE_NO_THROW(rb_test = @@ -68,8 +62,6 @@ NEGATIVE_TEST_CASE(KeyDomainKekInvalidPassword) POSITIVE_TEST_CASE(KeygetPureDomainKEK) { - BOOST_REQUIRE_MESSAGE(isLibInitialized, - "Library is not initialized!"); CKM::KeyProvider keyProvider; CKM::RawBuffer rb_test; BOOST_REQUIRE_NO_THROW(rb_test = @@ -82,8 +74,6 @@ POSITIVE_TEST_CASE(KeygetPureDomainKEK) POSITIVE_TEST_CASE(KeyGetWrappedDomainKEK) { - BOOST_REQUIRE_MESSAGE(isLibInitialized, - "Library is not initialized!"); CKM::KeyProvider keyProvider; CKM::RawBuffer rb_test; BOOST_REQUIRE_NO_THROW(rb_test = @@ -96,8 +86,6 @@ POSITIVE_TEST_CASE(KeyGetWrappedDomainKEK) POSITIVE_TEST_CASE(KeyGenerateDEK) { - BOOST_REQUIRE_MESSAGE(isLibInitialized, - "Library is not initialized!"); CKM::KeyProvider keyProvider; CKM::RawBuffer rb_test; CKM::RawBuffer rb_DEK1; @@ -111,8 +99,6 @@ POSITIVE_TEST_CASE(KeyGenerateDEK) POSITIVE_TEST_CASE(KeyGetPureDEK) { - BOOST_REQUIRE_MESSAGE(isLibInitialized, - "Library is not initialized!"); CKM::KeyProvider keyProvider; CKM::RawBuffer rb_pureDEK1; CKM::RawBuffer rb_DEK1; @@ -128,8 +114,6 @@ POSITIVE_TEST_CASE(KeyGetPureDEK) POSITIVE_TEST_CASE(KeyReencrypt) { - BOOST_REQUIRE_MESSAGE(isLibInitialized, - "Library is not initialized!"); CKM::RawBuffer rb_test; BOOST_REQUIRE_NO_THROW(rb_test = CKM::KeyProvider::generateDomainKEK(USERNAME_LONG, PASSWORD)); @@ -139,8 +123,6 @@ POSITIVE_TEST_CASE(KeyReencrypt) NEGATIVE_TEST_CASE(KeyReencrypt_incorrect_password) { - BOOST_REQUIRE_MESSAGE(isLibInitialized, - "Library is not initialized!"); CKM::RawBuffer rb_test; BOOST_REQUIRE_NO_THROW(rb_test = CKM::KeyProvider::generateDomainKEK(USERNAME_LONG, PASSWORD)); @@ -151,8 +133,6 @@ NEGATIVE_TEST_CASE(KeyReencrypt_incorrect_password) POSITIVE_TEST_CASE(KeyGetPureDEK_after_reencrypt) { - BOOST_REQUIRE_MESSAGE(isLibInitialized, - "Library is not initialized!"); CKM::KeyProvider keyProvider; CKM::RawBuffer rb_DEK1; CKM::RawBuffer rb_test; -- 2.7.4 From 4798ce0b0bb57b74b687089dfca22c7fa9a3bb55 Mon Sep 17 00:00:00 2001 From: Krzysztof Jackiewicz Date: Fri, 17 Jul 2020 22:32:21 +0200 Subject: [PATCH 13/16] Improve KeyProvider tests More negative tests added. Existing tests refactored and fixed where necessary. Redundant check removed from KeyProvider ctor. Change-Id: I5210c0f4c79851543c0f9dcb532a30aa7dc8168f --- src/manager/service/key-provider.cpp | 3 - unit-tests/test_key-provider.cpp | 345 ++++++++++++++++++++++------------- 2 files changed, 217 insertions(+), 131 deletions(-) diff --git a/src/manager/service/key-provider.cpp b/src/manager/service/key-provider.cpp index 6049acb..6ba0bb8 100644 --- a/src/manager/service/key-provider.cpp +++ b/src/manager/service/key-provider.cpp @@ -313,9 +313,6 @@ KeyProvider::KeyProvider( m_domainKEK(new KeyAndInfoContainer()), m_isInitialized(true) { - if (!m_isInitialized) - ThrowErr(Exc::InternalError, "Object not initialized!. Should not happened"); - if (domainKEKInWrapForm.size() != sizeof(WrappedKeyAndInfo)) { LogError("input size:" << domainKEKInWrapForm.size() << " Expected: " << sizeof(WrappedKeyAndInfo)); diff --git a/unit-tests/test_key-provider.cpp b/unit-tests/test_key-provider.cpp index d324f52..c5eb60d 100644 --- a/unit-tests/test_key-provider.cpp +++ b/unit-tests/test_key-provider.cpp @@ -27,197 +27,286 @@ #include #include -const CKM::Password PASSWORD = "12345TIZEN12345AAAAAAAAA"; -const CKM::Password INCORRECT_PASSWORD = "AAAAAAAAAAAAAAAAAAAAA"; -const CKM::Password NEW_PASSWORD = "NEW12345TIZEN12345NEW"; +using namespace CKM; -const std::string USERNAME_SHORT = "AB"; -const std::string USERNAME_LONG = "SOFTWARE_CENTER_SYSTEM_SW_LAB"; -const std::string CLIENT_ID_1 = "SAMPLE_CLIENT_ID_1"; -const std::string CLIENT_ID_2 = "SAMPLE_CLIENT_ID_2"; +namespace { +const Password PASSWORD = "12345TIZEN12345AAAAAAAAA"; +const Password INCORRECT_PASSWORD = "AAAAAAAAAAAAAAAAAAAAA"; +const Password NEW_PASSWORD = "NEW12345TIZEN12345NEW"; + +const std::string USERNAME = "SOFTWARE_CENTER_SYSTEM_SW_LAB"; +const std::string CLIENT_ID = "SAMPLE_CLIENT_ID_1"; + +RawBuffer makeDefaultWrappedDomainKEK() +{ + RawBuffer wdkek; + BOOST_REQUIRE_NO_THROW(wdkek = KeyProvider::generateDomainKEK(USERNAME, PASSWORD)); + BOOST_REQUIRE(!wdkek.empty()); + return wdkek; +} + +KeyProvider makeDefaultKeyProvider() +{ + KeyProvider kp; + RawBuffer wdkek = makeDefaultWrappedDomainKEK(); + + BOOST_REQUIRE_NO_THROW(kp = KeyProvider(wdkek, PASSWORD)); + BOOST_REQUIRE_MESSAGE(kp.isInitialized(), "KeyProvider created, but uninitialized"); + return kp; +} + +} // anonymous namespace BOOST_AUTO_TEST_SUITE(KEY_PROVIDER_TEST) -POSITIVE_TEST_CASE(KeyDomainKEK) + +NEGATIVE_TEST_CASE(KeyProvider_wrong_size) +{ + RawBuffer wdkek = makeDefaultWrappedDomainKEK(); + + wdkek.push_back(0); + BOOST_REQUIRE_THROW(KeyProvider(wdkek, PASSWORD), Exc::InternalError); + + wdkek.pop_back(); + wdkek.pop_back(); + BOOST_REQUIRE_THROW(KeyProvider(wdkek, PASSWORD), Exc::InternalError); +} + +NEGATIVE_TEST_CASE(KeyProvider_garbage) { - CKM::KeyProvider keyProvider; - CKM::RawBuffer rb_test; - BOOST_REQUIRE_NO_THROW(rb_test = - CKM::KeyProvider::generateDomainKEK(USERNAME_LONG, PASSWORD)); - BOOST_REQUIRE_NO_THROW(keyProvider = CKM::KeyProvider(rb_test, PASSWORD)); - BOOST_REQUIRE_MESSAGE(keyProvider.isInitialized(), - "KeyProvider created, but uninitialized"); + BOOST_REQUIRE_THROW(KeyProvider(RawBuffer(sizeof(WrappedKeyAndInfo)), PASSWORD), + Exc::AuthenticationFailed); } -NEGATIVE_TEST_CASE(KeyDomainKekInvalidPassword) +NEGATIVE_TEST_CASE(KeyDomainKek_invalid_password) { - CKM::KeyProvider keyProvider; - CKM::RawBuffer rb_test; - BOOST_REQUIRE_NO_THROW(rb_test = - CKM::KeyProvider::generateDomainKEK(USERNAME_LONG, PASSWORD)); - BOOST_REQUIRE_THROW(keyProvider = CKM::KeyProvider(rb_test, INCORRECT_PASSWORD), - CKM::Exc::AuthenticationFailed); - BOOST_REQUIRE_MESSAGE(!keyProvider.isInitialized(), - "KeyProvider not created, but initialized"); + KeyProvider kp; + RawBuffer wdkek = makeDefaultWrappedDomainKEK(); + + BOOST_REQUIRE_THROW(kp = KeyProvider(wdkek, INCORRECT_PASSWORD), Exc::AuthenticationFailed); + BOOST_REQUIRE_MESSAGE(!kp.isInitialized(), "KeyProvider not created, but initialized"); } POSITIVE_TEST_CASE(KeygetPureDomainKEK) { - CKM::KeyProvider keyProvider; - CKM::RawBuffer rb_test; - BOOST_REQUIRE_NO_THROW(rb_test = - CKM::KeyProvider::generateDomainKEK(USERNAME_LONG, PASSWORD)); - BOOST_REQUIRE_NO_THROW(keyProvider = CKM::KeyProvider(rb_test, PASSWORD)); - BOOST_REQUIRE_MESSAGE(keyProvider.isInitialized(), - "KeyProvider created, but uninitialized"); - BOOST_REQUIRE_NO_THROW(rb_test = keyProvider.getPureDomainKEK()); + KeyProvider kp = makeDefaultKeyProvider(); + RawBuffer dkek; + + BOOST_REQUIRE_NO_THROW(dkek = kp.getPureDomainKEK()); + BOOST_REQUIRE(dkek.size() <= MAX_KEY_SIZE); +} + +NEGATIVE_TEST_CASE(KeygetPureDomainKEK_uninitialized) +{ + KeyProvider kp; + + BOOST_REQUIRE_THROW(kp.getPureDomainKEK(), Exc::InternalError); } POSITIVE_TEST_CASE(KeyGetWrappedDomainKEK) { - CKM::KeyProvider keyProvider; - CKM::RawBuffer rb_test; - BOOST_REQUIRE_NO_THROW(rb_test = - CKM::KeyProvider::generateDomainKEK(USERNAME_LONG, PASSWORD)); - BOOST_REQUIRE_NO_THROW(keyProvider = CKM::KeyProvider(rb_test, PASSWORD)); - BOOST_REQUIRE_MESSAGE(keyProvider.isInitialized(), - "KeyProvider created, but uninitialized"); - BOOST_REQUIRE_NO_THROW(rb_test = keyProvider.getWrappedDomainKEK(PASSWORD)); + KeyProvider kp = makeDefaultKeyProvider(); + RawBuffer wdkek; + + BOOST_REQUIRE_NO_THROW(wdkek = kp.getWrappedDomainKEK("whatever")); + BOOST_REQUIRE(!wdkek.empty()); +} + +NEGATIVE_TEST_CASE(KeyGetWrappedDomainKEK_uninitialized) +{ + KeyProvider kp; + BOOST_REQUIRE_THROW(kp.getWrappedDomainKEK("whatever"), Exc::InternalError); } POSITIVE_TEST_CASE(KeyGenerateDEK) { - CKM::KeyProvider keyProvider; - CKM::RawBuffer rb_test; - CKM::RawBuffer rb_DEK1; - BOOST_REQUIRE_NO_THROW(rb_test = - CKM::KeyProvider::generateDomainKEK(USERNAME_LONG, PASSWORD)); - BOOST_REQUIRE_NO_THROW(keyProvider = CKM::KeyProvider(rb_test, PASSWORD)); - BOOST_REQUIRE_MESSAGE(keyProvider.isInitialized(), - "KeyProvider created, but uninitialized"); - BOOST_REQUIRE_NO_THROW(rb_DEK1 = keyProvider.generateDEK(CLIENT_ID_1)); + KeyProvider kp = makeDefaultKeyProvider(); + RawBuffer wdek; + + BOOST_REQUIRE_NO_THROW(wdek = kp.generateDEK(CLIENT_ID)); + BOOST_REQUIRE(!wdek.empty()); +} + +NEGATIVE_TEST_CASE(KeyGenerateDEK_uninitialized) +{ + KeyProvider kp; + + BOOST_REQUIRE_THROW(kp.generateDEK(CLIENT_ID), Exc::InternalError); } POSITIVE_TEST_CASE(KeyGetPureDEK) { - CKM::KeyProvider keyProvider; - CKM::RawBuffer rb_pureDEK1; - CKM::RawBuffer rb_DEK1; - CKM::RawBuffer rb_test; - BOOST_REQUIRE_NO_THROW(rb_test = - CKM::KeyProvider::generateDomainKEK(USERNAME_LONG, PASSWORD)); - BOOST_REQUIRE_NO_THROW(keyProvider = CKM::KeyProvider(rb_test, PASSWORD)); - BOOST_REQUIRE_MESSAGE(keyProvider.isInitialized(), - "KeyProvider created, but uninitialized"); - BOOST_REQUIRE_NO_THROW(rb_DEK1 = keyProvider.generateDEK(CLIENT_ID_1)); - BOOST_REQUIRE_NO_THROW(rb_pureDEK1 = keyProvider.getPureDEK(rb_DEK1)); + KeyProvider kp = makeDefaultKeyProvider(); + RawBuffer wdek, dek; + + BOOST_REQUIRE_NO_THROW(wdek = kp.generateDEK(CLIENT_ID)); + BOOST_REQUIRE(!wdek.empty()); + + BOOST_REQUIRE_NO_THROW(dek = kp.getPureDEK(wdek)); + BOOST_REQUIRE(dek.size() <= MAX_KEY_SIZE); +} + +NEGATIVE_TEST_CASE(KeyGetPureDEK_uninitialized) +{ + KeyProvider kp; + RawBuffer wdek(sizeof(WrappedKeyAndInfo)); + + BOOST_REQUIRE_THROW(kp.getPureDEK(wdek), Exc::InternalError); +} + +NEGATIVE_TEST_CASE(KeyGetPureDEK_wrong_size) +{ + KeyProvider kp = makeDefaultKeyProvider(); + RawBuffer wdek; + + BOOST_REQUIRE_NO_THROW(wdek = kp.generateDEK(CLIENT_ID)); + + wdek.push_back(0); + BOOST_REQUIRE_THROW(kp.getPureDEK(wdek), Exc::InternalError); + + wdek.pop_back(); + wdek.pop_back(); + BOOST_REQUIRE_THROW(kp.getPureDEK(wdek), Exc::InternalError); +} + +NEGATIVE_TEST_CASE(KeyGetPureDEK_garbage) +{ + KeyProvider kp = makeDefaultKeyProvider(); + RawBuffer wdek(sizeof(WrappedKeyAndInfo)); + + BOOST_REQUIRE_THROW(kp.getPureDEK(wdek), Exc::InternalError); } POSITIVE_TEST_CASE(KeyReencrypt) { - CKM::RawBuffer rb_test; - BOOST_REQUIRE_NO_THROW(rb_test = - CKM::KeyProvider::generateDomainKEK(USERNAME_LONG, PASSWORD)); - BOOST_REQUIRE_NO_THROW(CKM::KeyProvider::reencrypt(rb_test, PASSWORD, - NEW_PASSWORD)); + RawBuffer wdkek = makeDefaultWrappedDomainKEK(); + RawBuffer wdkek2; + + BOOST_REQUIRE_NO_THROW(wdkek2 = KeyProvider::reencrypt(wdkek, PASSWORD, NEW_PASSWORD)); + BOOST_REQUIRE(!wdkek2.empty()); } NEGATIVE_TEST_CASE(KeyReencrypt_incorrect_password) { - CKM::RawBuffer rb_test; - BOOST_REQUIRE_NO_THROW(rb_test = - CKM::KeyProvider::generateDomainKEK(USERNAME_LONG, PASSWORD)); - BOOST_REQUIRE_THROW((rb_test = CKM::KeyProvider::reencrypt(rb_test, - INCORRECT_PASSWORD, - NEW_PASSWORD)), CKM::Exc::AuthenticationFailed); + RawBuffer wdkek = makeDefaultWrappedDomainKEK(); + BOOST_REQUIRE_THROW(KeyProvider::reencrypt(wdkek, INCORRECT_PASSWORD, NEW_PASSWORD), + Exc::AuthenticationFailed); +} + +NEGATIVE_TEST_CASE(KeyReencrypt_wrong_size) +{ + RawBuffer wdkek = makeDefaultWrappedDomainKEK(); + + wdkek.push_back(0); + BOOST_REQUIRE_THROW(KeyProvider::reencrypt(wdkek, PASSWORD, NEW_PASSWORD), Exc::InternalError); + + wdkek.pop_back(); + wdkek.pop_back(); + BOOST_REQUIRE_THROW(KeyProvider::reencrypt(wdkek, PASSWORD, NEW_PASSWORD), Exc::InternalError); } POSITIVE_TEST_CASE(KeyGetPureDEK_after_reencrypt) { - CKM::KeyProvider keyProvider; - CKM::RawBuffer rb_DEK1; - CKM::RawBuffer rb_test; - BOOST_REQUIRE_NO_THROW(rb_test = - CKM::KeyProvider::generateDomainKEK(USERNAME_LONG, PASSWORD)); - BOOST_REQUIRE_NO_THROW(keyProvider = CKM::KeyProvider(rb_test, PASSWORD)); - BOOST_REQUIRE_NO_THROW(rb_DEK1 = keyProvider.generateDEK(CLIENT_ID_1)); - BOOST_REQUIRE_NO_THROW(keyProvider.getPureDEK(rb_DEK1)); + KeyProvider kp; + RawBuffer wdek, dek, wdkek2; + RawBuffer wdkek = makeDefaultWrappedDomainKEK(); + + BOOST_REQUIRE_NO_THROW(wdkek2 = KeyProvider::reencrypt(wdkek, PASSWORD, NEW_PASSWORD)); + BOOST_REQUIRE(!wdkek2.empty()); + + BOOST_REQUIRE_NO_THROW(kp = KeyProvider(wdkek2, NEW_PASSWORD)); + + BOOST_REQUIRE_NO_THROW(wdek = kp.generateDEK(CLIENT_ID)); + BOOST_REQUIRE(!wdek.empty()); + + BOOST_REQUIRE_NO_THROW(dek = kp.getPureDEK(wdek)); + BOOST_REQUIRE(dek.size() <= MAX_KEY_SIZE); } POSITIVE_TEST_CASE(wrapped_container) { - CKM::WrappedKeyAndInfoContainer wrappedContainer; + WrappedKeyAndInfoContainer wkic; auto salt = createRandom(20); - BOOST_REQUIRE_NO_THROW(wrappedContainer.setKeyInfoSalt(salt.data(), salt.size())); - BOOST_REQUIRE_NO_THROW(wrappedContainer.setKeyInfoClient("key_info_client")); - - CKM::WrappedKeyAndInfoContainer wrappedContainer2; - BOOST_REQUIRE_NO_THROW( - wrappedContainer2.setKeyInfo(&wrappedContainer.getWrappedKeyAndInfo().keyInfo)); - - BOOST_REQUIRE( - wrappedContainer.getWrappedKeyAndInfo().keyInfo.keyLength == - wrappedContainer2.getWrappedKeyAndInfo().keyInfo.keyLength); - BOOST_REQUIRE(memcmp( - wrappedContainer.getWrappedKeyAndInfo().keyInfo.salt, - wrappedContainer2.getWrappedKeyAndInfo().keyInfo.salt, - sizeof(wrappedContainer.getWrappedKeyAndInfo().keyInfo.salt)) == 0); - BOOST_REQUIRE(memcmp( - wrappedContainer.getWrappedKeyAndInfo().keyInfo.client, - wrappedContainer2.getWrappedKeyAndInfo().keyInfo.client, - sizeof(wrappedContainer.getWrappedKeyAndInfo().keyInfo.client)) == 0); - - CKM::WrappedKeyAndInfo wrapped3; - wrapped3.keyInfo.keyLength = MAX_WRAPPED_KEY_SIZE; - BOOST_REQUIRE_NO_THROW(CKM::WrappedKeyAndInfoContainer wrappedContainer3( - reinterpret_cast(&wrapped3))); + BOOST_REQUIRE_NO_THROW(wkic.setKeyInfoSalt(salt.data(), salt.size())); + BOOST_REQUIRE_NO_THROW(wkic.setKeyInfoClient("key_info_client")); + + WrappedKeyAndInfoContainer wkic2; + BOOST_REQUIRE_NO_THROW(wkic2.setKeyInfo(&wkic.getWrappedKeyAndInfo().keyInfo)); + + BOOST_REQUIRE(wkic.getWrappedKeyAndInfo().keyInfo.keyLength == + wkic2.getWrappedKeyAndInfo().keyInfo.keyLength); + BOOST_REQUIRE(memcmp(wkic.getWrappedKeyAndInfo().keyInfo.salt, + wkic2.getWrappedKeyAndInfo().keyInfo.salt, + sizeof(wkic.getWrappedKeyAndInfo().keyInfo.salt)) == 0); + BOOST_REQUIRE(memcmp(wkic.getWrappedKeyAndInfo().keyInfo.client, + wkic2.getWrappedKeyAndInfo().keyInfo.client, + sizeof(wkic.getWrappedKeyAndInfo().keyInfo.client)) == 0); + + WrappedKeyAndInfo wki; + wki.keyInfo.keyLength = MAX_WRAPPED_KEY_SIZE; + BOOST_REQUIRE_NO_THROW(WrappedKeyAndInfoContainer(reinterpret_cast(&wki))); } NEGATIVE_TEST_CASE(wrapped_container) { - CKM::WrappedKeyAndInfoContainer wrappedContainer; + WrappedKeyAndInfoContainer wkic; - BOOST_REQUIRE_THROW(wrappedContainer.setKeyInfoClient("key_info_client_waaaaay_too_long"), - CKM::Exc::InternalError); + BOOST_REQUIRE_THROW(wkic.setKeyInfoClient("key_info_client_waaaaay_too_long"), + Exc::InternalError); - CKM::WrappedKeyAndInfo wrapped3; + WrappedKeyAndInfo wki; + wki.keyInfo.keyLength = MAX_WRAPPED_KEY_SIZE + 1; + BOOST_REQUIRE_THROW(WrappedKeyAndInfoContainer(reinterpret_cast(&wki)), + Exc::InternalError); - wrapped3.keyInfo.keyLength = MAX_WRAPPED_KEY_SIZE + 1; - BOOST_REQUIRE_THROW(CKM::WrappedKeyAndInfoContainer wrappedContainer3( - reinterpret_cast(&wrapped3)), - CKM::Exc::InternalError); - - // missing NULL termination in wrapped4.keyInfo.client - CKM::WrappedKeyAndInfo wrapped4; - memset(&wrapped4, 0x01, sizeof(CKM::WrappedKeyAndInfo)); - BOOST_REQUIRE_THROW(CKM::WrappedKeyAndInfoContainer wrappedContainer3( - reinterpret_cast(&wrapped4)), - CKM::Exc::InternalError); + // missing NULL termination in wki2.keyInfo.client + WrappedKeyAndInfo wki2; + memset(&wki2, 0x01, sizeof(WrappedKeyAndInfo)); + BOOST_REQUIRE_THROW(WrappedKeyAndInfoContainer(reinterpret_cast(&wki2)), + Exc::InternalError); } POSITIVE_TEST_CASE(container) { - CKM::KeyAndInfoContainer container; - BOOST_REQUIRE_NO_THROW(container.setKeyInfoKeyLength(10)); + KeyAndInfoContainer kic; + BOOST_REQUIRE_NO_THROW(kic.setKeyInfoKeyLength(10)); - CKM::KeyAndInfoContainer container2; - BOOST_REQUIRE_NO_THROW(container2.setKeyInfo(&container.getKeyAndInfo().keyInfo)); + KeyAndInfoContainer kic2; + BOOST_REQUIRE_NO_THROW(kic2.setKeyInfo(&kic.getKeyAndInfo().keyInfo)); - BOOST_REQUIRE( - container.getKeyAndInfo().keyInfo.keyLength == - container2.getKeyAndInfo().keyInfo.keyLength); + BOOST_REQUIRE(kic.getKeyAndInfo().keyInfo.keyLength == kic2.getKeyAndInfo().keyInfo.keyLength); } POSITIVE_TEST_CASE(moves) { - CKM::KeyProvider provider; + KeyProvider kp = makeDefaultKeyProvider(); + RawBuffer dkek; + + BOOST_REQUIRE_NO_THROW(dkek = kp.getPureDomainKEK()); + BOOST_REQUIRE(dkek.size() <= MAX_KEY_SIZE); try { - CKM::KeyProvider provider2(std::move(provider)); - CKM::KeyProvider provider3 = std::move(provider2); + KeyProvider kp2(std::move(kp)); + KeyProvider kp3 = std::move(kp2); + + BOOST_REQUIRE(!kp.isInitialized()); + BOOST_REQUIRE(!kp2.isInitialized()); + BOOST_REQUIRE(kp3.isInitialized()); + + RawBuffer dkek3; + BOOST_REQUIRE_NO_THROW(dkek3 = kp3.getPureDomainKEK()); + BOOST_REQUIRE(dkek == dkek3); } catch (...) { BOOST_REQUIRE_MESSAGE(false, "Unknown exception on moving KeyProvider"); } } +NEGATIVE_TEST_CASE(moves) +{ + KeyProvider kp = makeDefaultKeyProvider(); + KeyProvider kp2(std::move(kp)); + + BOOST_REQUIRE_THROW(kp.getPureDomainKEK(), Exc::InternalError); +} + BOOST_AUTO_TEST_SUITE_END() -- 2.7.4 From d94d7c7786b694752b1ab7f77551338a5dca1baa Mon Sep 17 00:00:00 2001 From: Mateusz Cegielka Date: Tue, 4 Aug 2020 13:07:54 +0200 Subject: [PATCH 14/16] Remove unused Stringify macro variants Stringify is a helper macro used for formatting variadic arguments to a string in error messages. The code also contains unused StringifyAvoid, StringifyDebug and StringifyError macros. I have removed the unused macros and their tests. Change-Id: I08d00480a2e6ba73ba1a6c573c7afc4fccc36500 --- src/manager/common/stringify.h | 11 +---------- unit-tests/test_stringify.cpp | 16 +--------------- 2 files changed, 2 insertions(+), 25 deletions(-) diff --git a/src/manager/common/stringify.h b/src/manager/common/stringify.h index c8b32d2..300b1ea 100644 --- a/src/manager/common/stringify.h +++ b/src/manager/common/stringify.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2000 - 2015 Samsung Electronics Co., Ltd All Rights Reserved + * Copyright (c) 2000 - 2020 Samsung Electronics Co., Ltd All Rights Reserved * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -69,12 +69,3 @@ #define Stringify(...) \ (static_cast(std::ostringstream() \ STRINGIFY_(PP_NARG(__VA_ARGS__), __VA_ARGS__))).str() - -#define StringifyAvoid(...) std::string() -#define StringifyError(...) Stringify(__VA_ARGS__) - -#ifdef DEBUG -#define StringifyDebug(...) Stringify(__VA_ARGS__) -#else -#define StringifyDebug(...) std::string() -#endif diff --git a/unit-tests/test_stringify.cpp b/unit-tests/test_stringify.cpp index 0fd3727..abf18e2 100644 --- a/unit-tests/test_stringify.cpp +++ b/unit-tests/test_stringify.cpp @@ -20,25 +20,11 @@ BOOST_AUTO_TEST_SUITE(STRINGIFY_TEST) -POSITIVE_TEST_CASE(stringify_default) +POSITIVE_TEST_CASE(stringify) { BOOST_REQUIRE(Stringify("a", "b", "c") == "abc"); BOOST_REQUIRE(Stringify(std::string("a"), "b", "c") == "abc"); BOOST_REQUIRE(Stringify().empty()); } -POSITIVE_TEST_CASE(stringify_avoid) -{ - BOOST_REQUIRE(StringifyAvoid("a", "b", "c").empty()); - BOOST_REQUIRE(StringifyAvoid(std::string("a"), "b", "c").empty()); - BOOST_REQUIRE(StringifyAvoid().empty()); -} - -POSITIVE_TEST_CASE(stringify_error) -{ - BOOST_REQUIRE(StringifyError("a", "b", "c") == "abc"); - BOOST_REQUIRE(StringifyError(std::string("a"), "b", "c") == "abc"); - BOOST_REQUIRE(StringifyError().empty()); -} - BOOST_AUTO_TEST_SUITE_END() -- 2.7.4 From a5e89f2a4b857b42eeba4adb79d2ed3a3f0e75b6 Mon Sep 17 00:00:00 2001 From: Mateusz Cegielka Date: Tue, 4 Aug 2020 13:13:11 +0200 Subject: [PATCH 15/16] Remove most CommunicationManager tests CommunicationManager is a class responsible for adding std::functions to a std::vector, and calling all of them with an argument (this takes 4 lines of actual logic). However, it has 7 redundant tests, including a randomized stress test and some interesting helper classes. I have reduced this number to 2 simple tests, testing basic and exception-related behavior. Change-Id: Ie8ce196df1f0e2a1c280c7aad4bd36c5911a6ada --- unit-tests/test_comm-manager.cpp | 186 +++++---------------------------------- 1 file changed, 21 insertions(+), 165 deletions(-) diff --git a/unit-tests/test_comm-manager.cpp b/unit-tests/test_comm-manager.cpp index 961451c..138de34 100644 --- a/unit-tests/test_comm-manager.cpp +++ b/unit-tests/test_comm-manager.cpp @@ -19,195 +19,51 @@ * @version 1.0 */ -#include -#include #include -#include -#include -#include -#include - -namespace { -struct MessageA { - explicit MessageA(int ai) : i(ai) {} - int i; -}; - -struct MessageB { - explicit MessageB(char ac) : c(ac) {} - char c; -}; - -struct MessageC { - explicit MessageC(const std::string &astr) : str(astr) {} - std::string str; -}; -struct Listener { - Listener() : i(0) {} - - void Handle(const MessageA &msg) - { - i = msg.i; - } - - void Handle(const MessageC &msg) - { - str = msg.str; - } - - int i; - std::string str; -}; +#include -} // namespace anonymous +#include -BOOST_AUTO_TEST_SUITE(MESSAGE_MANAGER_TEST) +BOOST_AUTO_TEST_SUITE(COMMUNICATION_MANAGER_TEST) -POSITIVE_TEST_CASE(TMM_0010_NoListener) +POSITIVE_TEST_CASE(basic) { - CKM::CommunicationManager mgr; - BOOST_REQUIRE_MESSAGE(0 == mgr.SendMessage(MessageA(22)), - "There should be no listener."); -} + CKM::CommunicationManager mgr; -POSITIVE_TEST_CASE(TMM_0020_Basic) -{ - CKM::CommunicationManager mgr; - int received = 0; - mgr.Register([&](const MessageA & msg) { - received = msg.i; - }); - BOOST_REQUIRE_MESSAGE(1 == mgr.SendMessage(MessageA(4)), "No listener found"); - BOOST_REQUIRE_MESSAGE(received != 0, "Message not received"); - BOOST_REQUIRE_MESSAGE(received == 4, "Wrong message received i=" << received); -} + BOOST_REQUIRE_MESSAGE(mgr.SendMessage(22) == 0, "There should be no listener."); -POSITIVE_TEST_CASE(TMM_0030_MultipleMessages) -{ - CKM::CommunicationManager mgr; int reci = 0; char recc = 0; - mgr.Register([&](const MessageA & msg) { - reci = msg.i; + mgr.Register([&](const int& msg) { + reci = msg; }); - mgr.Register([&](const MessageB & msg) { - recc = msg.c; + mgr.Register([&](const char& msg) { + recc = msg; }); - BOOST_REQUIRE_MESSAGE(1 == mgr.SendMessage(MessageB('c')), "No listener found"); + + BOOST_REQUIRE_MESSAGE(mgr.SendMessage('c') == 1, "No listener found"); BOOST_REQUIRE_MESSAGE(reci == 0, "Unexpected message received"); BOOST_REQUIRE_MESSAGE(recc != 0, "Message not received"); BOOST_REQUIRE_MESSAGE(recc == 'c', "Wrong message received c=" << recc); - BOOST_REQUIRE_MESSAGE(1 == mgr.SendMessage(MessageA(42)), "No listener found"); + BOOST_REQUIRE_MESSAGE(mgr.SendMessage(42) == 1, "No listener found"); BOOST_REQUIRE_MESSAGE(reci != 0, "Message not received"); BOOST_REQUIRE_MESSAGE(reci == 42, "Wrong message received i=" << reci); BOOST_REQUIRE_MESSAGE(recc == 'c', "Previous message overwritten c=" << recc); -} -POSITIVE_TEST_CASE(TMM_0040_Listener) -{ - CKM::CommunicationManager mgr; - Listener l; - mgr.Register([&](const MessageC & msg) { - l.Handle(msg); - }); - mgr.Register([&](const MessageA & msg) { - l.Handle(msg); - }); - - BOOST_REQUIRE_MESSAGE(1 == mgr.SendMessage(MessageC("lorem ipsum")), - "No listener found"); - BOOST_REQUIRE_MESSAGE(l.i == 0, "Unexpected message received"); - BOOST_REQUIRE_MESSAGE(!l.str.empty(), "Message not received"); - BOOST_REQUIRE_MESSAGE(l.str == "lorem ipsum", - "Wrong message received c=" << l.str); - - BOOST_REQUIRE_MESSAGE(1 == mgr.SendMessage(MessageA(3)), "No listener found"); - BOOST_REQUIRE_MESSAGE(l.i != 0, "Message not received"); - BOOST_REQUIRE_MESSAGE(l.i == 3, "Wrong message received i=" << l.i); - BOOST_REQUIRE_MESSAGE(l.str == "lorem ipsum", - "Previous message overwritten str=" << l.str); -} - -POSITIVE_TEST_CASE(TMM_0050_2Listeners) -{ - CKM::CommunicationManager mgr; - bool called[2]; - called[0] = false; - called[1] = false; - mgr.Register([&](const MessageA & msg) { - BOOST_REQUIRE_MESSAGE(msg.i == 5, "Unexpected message received i=" << msg.i); - called[0] = true; - }); - mgr.Register([&](const MessageA & msg) { - BOOST_REQUIRE_MESSAGE(msg.i == 5, "Unexpected message received i=" << msg.i); - called[1] = true; + int reci2 = 0; + mgr.Register([&](const int& msg) { + reci2 = msg; }); - BOOST_REQUIRE_MESSAGE(2 == mgr.SendMessage(MessageA(5)), "No listener found"); - BOOST_REQUIRE_MESSAGE(called[0], "First listener not called"); - BOOST_REQUIRE_MESSAGE(called[1], "Second listener not called"); -} - -POSITIVE_TEST_CASE(TMM_0060_Stress) -{ - CKM::CommunicationManager mgr; - - std::default_random_engine generator( - std::chrono::system_clock::now().time_since_epoch().count()); - std::uniform_int_distribution message_dist(0, 2); - std::uniform_int_distribution count_dist(1, 10); - - size_t a = 0; - size_t b = 0; - size_t c = 0; - mgr.Register([&](const MessageA & msg) { - BOOST_REQUIRE_MESSAGE(msg.i == 42, "Wrong message: " << msg.i); - a++; - }); - mgr.Register([&](const MessageB & msg) { - BOOST_REQUIRE_MESSAGE(msg.c == 'c', "Wrong message: " << msg.c); - b++; - }); - mgr.Register([&](const MessageC & msg) { - BOOST_REQUIRE_MESSAGE(msg.str == "lorem ipsum", "Wrong message: " << msg.str); - c++; - }); - - for (size_t i = 0; i < 1000; i++) { - size_t cnt = count_dist(generator); - - for (size_t s = 0; s < cnt; s++) { - switch (message_dist(generator)) { - case 0: - BOOST_REQUIRE_MESSAGE(1 == mgr.SendMessage(MessageA(42)), "No listener found"); - a--; - break; - - case 1: - BOOST_REQUIRE_MESSAGE(1 == mgr.SendMessage(MessageB('c')), "No listener found"); - b--; - break; - - case 2: - BOOST_REQUIRE_MESSAGE(1 == mgr.SendMessage(MessageC("lorem ipsum")), - "No listener found"); - c--; - break; - - default: - BOOST_FAIL("Unexpected message type"); - } - } - } - - BOOST_REQUIRE_MESSAGE(a == 0, "Unexpected number of MessageA: " << a); - BOOST_REQUIRE_MESSAGE(b == 0, "Unexpected number of MessageB: " << b); - BOOST_REQUIRE_MESSAGE(c == 0, "Unexpected number of MessageC: " << c); + BOOST_REQUIRE_MESSAGE(mgr.SendMessage(1234) == 2, "Some listeners not found"); + BOOST_REQUIRE_MESSAGE(reci == 1234, "First receiver not called"); + BOOST_REQUIRE_MESSAGE(reci2 == 1234, "Second receiver not called"); + BOOST_REQUIRE_MESSAGE(recc == 'c', "Previous message overwritten c=" << recc); } -NEGATIVE_TEST_CASE(TMM_0070_ThrowingListener) +NEGATIVE_TEST_CASE(throwing_listener) { CKM::CommunicationManager mgr; -- 2.7.4 From ccf06cea1135370c2fdf4b9ecf32c82997a6d8de Mon Sep 17 00:00:00 2001 From: Mateusz Cegielka Date: Fri, 31 Jul 2020 16:13:33 +0200 Subject: [PATCH 16/16] Move Token and CryptoBackend to common Both Token and CryptoBackend are small types used on the server, both in the crypto and the service modules. They are defined in the service module, and crypto submodules have to include these headers. Other than that, the crypto module is not aware of the service module, and creating an unnecessary cyclic dependency here shows up in static analysis. Since they are minor types which don't contain any logic and are used in different contexts in different modules, I have moved them to the src/manager/common directory. Change-Id: Ifd55ec97173b6e99c9c2fec154803dccfa48a1ae --- src/manager/{service => common}/crypto-backend.h | 0 src/manager/{service => common}/token.h | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename src/manager/{service => common}/crypto-backend.h (100%) rename src/manager/{service => common}/token.h (100%) diff --git a/src/manager/service/crypto-backend.h b/src/manager/common/crypto-backend.h similarity index 100% rename from src/manager/service/crypto-backend.h rename to src/manager/common/crypto-backend.h diff --git a/src/manager/service/token.h b/src/manager/common/token.h similarity index 100% rename from src/manager/service/token.h rename to src/manager/common/token.h -- 2.7.4