From 4798ce0b0bb57b74b687089dfca22c7fa9a3bb55 Mon Sep 17 00:00:00 2001 From: Krzysztof Jackiewicz Date: Fri, 17 Jul 2020 22:32:21 +0200 Subject: [PATCH 01/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 02/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 03/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 04/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 From c806f3ad70a2c694abb0e2bc5e6b695417416b48 Mon Sep 17 00:00:00 2001 From: Mateusz Cegielka Date: Tue, 4 Aug 2020 15:29:11 +0200 Subject: [PATCH 05/16] Change safe-buffer test structure test_safe-buffer.cpp contains tests that ensure std::vector fails to erase possibly confidential memory when its destructor is called, which try to make sure the SafeBuffer testing method is valid. Since the SafeBuffer test results may be completely wrong if these tests fail, it would be better to merge them into one test to avoid misleading results. I have merged the 4 tests into a single test and added some comments. Change-Id: I9d58a7a3942a0318c0fa96047a1bdb7e708a69d4 --- unit-tests/test_safe-buffer.cpp | 31 +++++++------------------------ 1 file changed, 7 insertions(+), 24 deletions(-) diff --git a/unit-tests/test_safe-buffer.cpp b/unit-tests/test_safe-buffer.cpp index ff9dccd..582bbbd 100644 --- a/unit-tests/test_safe-buffer.cpp +++ b/unit-tests/test_safe-buffer.cpp @@ -83,35 +83,18 @@ BOOST_AUTO_TEST_SUITE(SAFE_BUFFER_TEST) // Tests for SafeBuffer. Checks if memory occupied by the buffer is wiped after it's deleted. -POSITIVE_TEST_CASE(SafeBufferTest_uc_control_group) +POSITIVE_TEST_CASE(SafeBufferTest) { - size_t cnt = buffer_erase_test>(); - - BOOST_REQUIRE_MESSAGE(cnt > LEN / 2, + // Run a control group to check if this kind of test can even work. + BOOST_REQUIRE_MESSAGE(buffer_erase_test>() > LEN / 2, "Less than 1/2 of data matches the original."); -} - -POSITIVE_TEST_CASE(SafeBufferTest_item_control_group) -{ - size_t cnt = buffer_erase_test>(); - - BOOST_REQUIRE_MESSAGE(cnt > LEN / 2, + BOOST_REQUIRE_MESSAGE(buffer_erase_test>() > LEN / 2, "Less than 1/2 of data matches the original."); -} -POSITIVE_TEST_CASE(SafeBufferTest_uc) -{ - size_t cnt = buffer_erase_test(); - - BOOST_REQUIRE_MESSAGE(cnt <= LEN / 10, + // Actually check whether SafeBuffer erases memory after destruction. + BOOST_REQUIRE_MESSAGE(buffer_erase_test() <= LEN / 10, "More than 1/10 of data matches the original."); -} - -POSITIVE_TEST_CASE(SafeBufferTest_item) -{ - size_t cnt = buffer_erase_test::Type>(); - - BOOST_REQUIRE_MESSAGE(cnt <= LEN / 10, + BOOST_REQUIRE_MESSAGE(buffer_erase_test::Type>() <= LEN / 10, "More than 1/10 of data matches the original."); } -- 2.7.4 From 5ce2a43ca9f0ffd18f9b7768ec644d13f38c3605 Mon Sep 17 00:00:00 2001 From: Mateusz Cegielka Date: Mon, 20 Jul 2020 17:59:15 +0200 Subject: [PATCH 06/16] Remove CryptoExt class in favor of friendship In the ckm_db_tool CLI helper project, CKMLogicExt and CryptoExt classes are responsible for breaking encapsulation of CKMLogic and Crypto classes. However, code used for extracting a Crypto member and casting it to the CryptoExt type is repeated two times (soon three), and rather dangerous. This refactor makes CKMLogicExt a friend of the Crypto class. This makes it possible to implement additional methods directly in CKMLogicExt without doing dangerous slicing object casts. Change-Id: Ice7261b76f46f9a6206f7ae1faded1f3d8e359cb --- misc/ckm_db_tool/CMakeLists.txt | 1 - misc/ckm_db_tool/ckm-logic-ext.cpp | 52 +++++++++++----------------- misc/ckm_db_tool/db-crypto-ext.cpp | 71 -------------------------------------- misc/ckm_db_tool/db-crypto-ext.h | 41 ---------------------- src/manager/service/db-crypto.h | 6 ++++ 5 files changed, 26 insertions(+), 145 deletions(-) delete mode 100644 misc/ckm_db_tool/db-crypto-ext.cpp delete mode 100644 misc/ckm_db_tool/db-crypto-ext.h diff --git a/misc/ckm_db_tool/CMakeLists.txt b/misc/ckm_db_tool/CMakeLists.txt index 117cc80..021b9c5 100644 --- a/misc/ckm_db_tool/CMakeLists.txt +++ b/misc/ckm_db_tool/CMakeLists.txt @@ -18,7 +18,6 @@ INCLUDE_DIRECTORIES( ) SET(CKM_DB_TOOLS_SOURCES - ${CMAKE_CURRENT_SOURCE_DIR}/db-crypto-ext.cpp ${CMAKE_CURRENT_SOURCE_DIR}/ckm-logic-ext.cpp ${CMAKE_CURRENT_SOURCE_DIR}/db-wrapper.cpp ${KEY_MANAGER_PATH}/common/ckm-error.cpp diff --git a/misc/ckm_db_tool/ckm-logic-ext.cpp b/misc/ckm_db_tool/ckm-logic-ext.cpp index 2eba078..3b035fc 100644 --- a/misc/ckm_db_tool/ckm-logic-ext.cpp +++ b/misc/ckm_db_tool/ckm-logic-ext.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2000 - 2017 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. @@ -21,45 +21,33 @@ #include "ckm-logic-ext.h" -#include "db-crypto-ext.h" +#include + +namespace { + +const char *DB_CMD_OBJECT_SELECT = "SELECT * FROM [join_name_object_tables];"; + +} // anonymous namespace namespace CKM { -DB::SqlConnection::Output CKMLogicExt::Execute(uid_t user, - const std::string &cmd) +DB::SqlConnection::Output CKMLogicExt::Execute(uid_t user, const std::string &cmd) { - /* - * We need to access to DB::Crypto::m_connection to call Execute() on it. We don't want to mess - * with DB::Crypto too much so adding a friend and extending public interface was not an option. - * That's why we need a derived class DB::CryptoExt. m_userDataMap must be left unchanged after - * this operation but DB::Crypto can't be copied. According to C++ standard static casting - * DB::Crypto pointer to DB::CryptoExt pointer is UB. Therefore DB::Crypto is temporarily moved - * into DB::CryptoExt and moved back to m_userDataMap after the call to Execute(). - */ - DB::CryptoExt db(std::move(m_userDataMap[user].database)); - - try { - DB::SqlConnection::Output output = db.Execute(cmd); - m_userDataMap[user].database = std::move(*static_cast(&db)); - return output; - } catch (const DB::SqlConnection::Exception::Base &e) { - m_userDataMap[user].database = std::move(*static_cast(&db)); - throw; - } + DB::SqlConnection::Output out; + m_userDataMap[user].database.m_connection->ExecCommand(&out, "%s", cmd.c_str()); + return out; } DB::RowVector CKMLogicExt::getRows(uid_t user) { - DB::CryptoExt db(std::move(m_userDataMap[user].database)); - - try { - DB::RowVector output = db.getRows(); - m_userDataMap[user].database = std::move(*static_cast(&db)); - return output; - } catch (const DB::SqlConnection::Exception::Base &e) { - m_userDataMap[user].database = std::move(*static_cast(&db)); - throw; - } + DB::Crypto& database = m_userDataMap[user].database; + DB::SqlConnection::DataCommandUniquePtr selectCommand = + database.m_connection->PrepareDataCommand(DB_CMD_OBJECT_SELECT); + + DB::RowVector rows; + while (selectCommand->Step()) + rows.push_back(database.getRow(selectCommand)); + return rows; } void CKMLogicExt::saveRow(uid_t user, const DB::Row &row) diff --git a/misc/ckm_db_tool/db-crypto-ext.cpp b/misc/ckm_db_tool/db-crypto-ext.cpp deleted file mode 100644 index df28078..0000000 --- a/misc/ckm_db_tool/db-crypto-ext.cpp +++ /dev/null @@ -1,71 +0,0 @@ -/* - * Copyright (c) 2014-2017 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 db-crypto-ext.cpp - * @author Krzysztof Jackiewicz (k.jackiewicz@samsung.com) - * @version 1.0 - * @brief Limited implementation of encrypted db access layer - */ - -#include "db-crypto-ext.h" - -#include - -namespace CKM { -namespace DB { - -const char *DB_CMD_OBJECT_SELECT = - "SELECT * FROM [join_name_object_tables];"; - -SqlConnection::Output CryptoExt::Execute(const std::string &cmd) -{ - SqlConnection::Output out; - - if (!m_connection) { - ThrowMsg(SqlConnection::Exception::ConnectionBroken, - "Not connected to database"); - } - - m_connection->ExecCommand(&out, "%s", cmd.c_str()); - return out; -} - -RowVector CryptoExt::getRows() -{ - try { - RowVector output; - SqlConnection::DataCommandUniquePtr selectCommand = - m_connection->PrepareDataCommand(DB_CMD_OBJECT_SELECT); - - while (selectCommand->Step()) { - // extract data - output.push_back(getRow(selectCommand)); - } - - return output; - } catch (const SqlConnection::Exception::InvalidColumn &) { - LogError("Select statement invalid column error"); - } catch (const SqlConnection::Exception::SyntaxError &) { - LogError("Couldn't prepare select statement"); - } catch (const SqlConnection::Exception::InternalError &) { - LogError("Couldn't execute select statement"); - } - - ThrowErr(Exc::DatabaseFailed, "Couldn't get row from database"); -} - -} // namespace DB -} // namespace CKM diff --git a/misc/ckm_db_tool/db-crypto-ext.h b/misc/ckm_db_tool/db-crypto-ext.h deleted file mode 100644 index bbf7476..0000000 --- a/misc/ckm_db_tool/db-crypto-ext.h +++ /dev/null @@ -1,41 +0,0 @@ -/* - * Copyright (c) 2015-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 db-crypto-ext.h - * @author Krzysztof Jackiewicz (k.jackiewicz@samsung.com) - * @version 1.0 - * @brief Header of encrypted db access layer - */ - -#pragma once - -#include -#include -#include -#include - -namespace CKM { -namespace DB { -struct CryptoExt : public Crypto { - explicit CryptoExt(Crypto orig) : Crypto(std::move(orig)) {} - - SqlConnection::Output Execute(const std::string &cmd); - RowVector getRows(); -}; - -} // namespace DB -} // namespace CKM - diff --git a/src/manager/service/db-crypto.h b/src/manager/service/db-crypto.h index 3c3c206..6fd3a39 100644 --- a/src/manager/service/db-crypto.h +++ b/src/manager/service/db-crypto.h @@ -37,6 +37,9 @@ #pragma GCC diagnostic warning "-Wdeprecated-declarations" namespace CKM { + +class CKMLogicExt; + namespace DB { class Crypto { public: @@ -202,6 +205,8 @@ protected: private: bool m_inUserTransaction; + friend CKMLogicExt; + void resetDB(); void initDatabase(); void createDBSchema(); @@ -288,6 +293,7 @@ public: }; } // namespace DB + } // namespace CKM #pragma GCC diagnostic pop -- 2.7.4 From 9ea674f06681e732d98be7b433ce8db7a9ee7b05 Mon Sep 17 00:00:00 2001 From: Mateusz Cegielka Date: Wed, 22 Jul 2020 11:23:37 +0200 Subject: [PATCH 07/16] Add consistent error messages to ckm_db_tool ckm_db_tool is a set of CLI utilities for debugging key-manager. Various displayed error messages are inconsistent, do not display all available information, and contain minor gramatical errors. Also, new interactive features are planned to be introduced, which will require reading and writing more information to the terminal. A simple helper functions has been created for displaying error, warning and info messages. All error messages have been changed to use them, received grammar fixes, started displaying APICodeToString result when possible, and rewritten to follow a consistent style. Finally, warning and askPassword functions were implemented to prepare for next patches. Change-Id: Ifd0608637f3f4ef3ce31c2fe7c79074da9a93bbb --- misc/ckm_db_tool/CMakeLists.txt | 2 ++ misc/ckm_db_tool/ckm_db_merge.cpp | 24 +++++++------ misc/ckm_db_tool/ckm_db_tool.cpp | 27 +++++++------- misc/ckm_db_tool/db-wrapper.cpp | 9 ++--- misc/ckm_db_tool/ui.cpp | 74 +++++++++++++++++++++++++++++++++++++++ misc/ckm_db_tool/ui.h | 40 +++++++++++++++++++++ 6 files changed, 149 insertions(+), 27 deletions(-) create mode 100644 misc/ckm_db_tool/ui.cpp create mode 100644 misc/ckm_db_tool/ui.h diff --git a/misc/ckm_db_tool/CMakeLists.txt b/misc/ckm_db_tool/CMakeLists.txt index 021b9c5..db6a133 100644 --- a/misc/ckm_db_tool/CMakeLists.txt +++ b/misc/ckm_db_tool/CMakeLists.txt @@ -20,11 +20,13 @@ INCLUDE_DIRECTORIES( SET(CKM_DB_TOOLS_SOURCES ${CMAKE_CURRENT_SOURCE_DIR}/ckm-logic-ext.cpp ${CMAKE_CURRENT_SOURCE_DIR}/db-wrapper.cpp + ${CMAKE_CURRENT_SOURCE_DIR}/ui.cpp ${KEY_MANAGER_PATH}/common/ckm-error.cpp ${KEY_MANAGER_PATH}/crypto/platform/decider.cpp ${KEY_MANAGER_PATH}/crypto/sw-backend/internals.cpp ${KEY_MANAGER_PATH}/crypto/sw-backend/obj.cpp ${KEY_MANAGER_PATH}/crypto/sw-backend/store.cpp + ${KEY_MANAGER_PATH}/dpl/core/src/colors.cpp ${KEY_MANAGER_PATH}/dpl/db/src/naive_synchronization_object.cpp ${KEY_MANAGER_PATH}/dpl/db/src/sql_connection.cpp ${KEY_MANAGER_PATH}/initial-values/BufferHandler.cpp diff --git a/misc/ckm_db_tool/ckm_db_merge.cpp b/misc/ckm_db_tool/ckm_db_merge.cpp index 5248b4c..44e4226 100644 --- a/misc/ckm_db_tool/ckm_db_merge.cpp +++ b/misc/ckm_db_tool/ckm_db_merge.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2000 - 2017 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. @@ -29,6 +29,7 @@ #include #include "db-wrapper.h" +#include "ui.h" using namespace std; using namespace CKM; @@ -86,11 +87,11 @@ int mergeDatabase( int ret; if (CKM_API_SUCCESS != (ret = source.unlock())) { // this should migrate database to newest version - cerr << "Unlocking source database failed: " << ret << endl; + UI::error() << "unlocking source database failed: " << APICodeToString(ret) << endl; return 1; } if (CKM_API_SUCCESS != (ret = target.unlock())) { // this should migrate database to newest version - cerr << "Unlocking target database failed: " << ret << endl; + UI::error() << "unlocking target database failed: " << APICodeToString(ret) << endl; return 1; } @@ -106,7 +107,9 @@ int mergeDatabase( try { target.saveRow(e); } catch (const CKM::DB::SqlConnection::Exception::Base &e) { - cerr << "Sql exception. Migration failed or object already exist in database: " << e.DumpToString() << endl; + UI::error() + << "sql exception, migration failed or object already exists in the database: " + << e.DumpToString() << endl; } } } @@ -172,21 +175,22 @@ int main(int argc, char *argv[]) { } if (uid1 == uid2) { - cerr << "Source database and target database must be different!" << endl; + UI::error() << "source and target databases must be different" << endl; return 1; } return mergeDatabase(uid1, password1, uid2, password2, filters); } catch (const CKM::DB::SqlConnection::Exception::InvalidColumn &e) { - cerr << "Invalid Column exception was catched. Probably migration failed. " << e.DumpToString() << endl; + UI::error() << "invalid column exception, probably a failed migration: " << e.DumpToString() + << endl; } catch (const invalid_argument &e) { - cerr << "Argument could not be converted" << endl; + UI::error() << "argument could not be converted: " << e.what() << endl; } catch (const out_of_range &e) { - cerr << "Out of range" << endl; + UI::error() << "argument out of range: " << e.what() << endl; } catch (const exception &e) { - cerr << "Unexpected error: " << e.what() << endl; + UI::error() << "unexpected exception: " << e.what() << endl; } catch (...) { - cerr << "Unknown exception" << endl; + UI::error() << "unknown exception" << endl; } return 1; diff --git a/misc/ckm_db_tool/ckm_db_tool.cpp b/misc/ckm_db_tool/ckm_db_tool.cpp index e1a4fa6..6c31dc7 100644 --- a/misc/ckm_db_tool/ckm_db_tool.cpp +++ b/misc/ckm_db_tool/ckm_db_tool.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2000 - 2017 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. @@ -27,6 +27,7 @@ #include #include "db-wrapper.h" +#include "ui.h" using namespace std; using namespace CKM; @@ -104,19 +105,19 @@ int main(int argc, char *argv[]) int retCode; if (CKM_API_SUCCESS != (retCode = dbw.unlock())) { - cerr << "Unlocking database failed: " << APICodeToString(retCode) << endl; + UI::error() << "unlocking database failed: " << APICodeToString(retCode) << endl; return -1; } - cout << "Database unlocked" << endl; + UI::info() << "database unlocked" << endl; while (true) { string cmd; if (argcmd.empty()) { - cout << "> "; + cmd = UI::promptLine(">"); - if (!getline(cin, cmd)) { + if (!std::cin) { cout << "exit" << endl; break; // EOF } @@ -139,17 +140,17 @@ int main(int argc, char *argv[]) } dbw.lock(); - cout << "Database locked" << endl; + UI::info() << "database locked" << endl; return 0; - } catch (const std::invalid_argument &e) { - cerr << "Argument could not be converted" << endl; - } catch (const std::out_of_range &e) { - cerr << "Out of range" << endl; - } catch (const std::exception &e) { - cerr << "Exception: " << e.what() << endl; + } catch (const invalid_argument &e) { + UI::error() << "argument could not be converted: " << e.what() << endl; + } catch (const out_of_range &e) { + UI::error() << "argument out of range: " << e.what() << endl; + } catch (const exception &e) { + UI::error() << "unexpected error: " << e.what() << endl; } catch (...) { - cerr << "Unexpected exception occurred" << endl; + UI::error() << "unknown exception" << endl; } return -1; } diff --git a/misc/ckm_db_tool/db-wrapper.cpp b/misc/ckm_db_tool/db-wrapper.cpp index aa09810..0d9ba91 100644 --- a/misc/ckm_db_tool/db-wrapper.cpp +++ b/misc/ckm_db_tool/db-wrapper.cpp @@ -20,6 +20,7 @@ * @version 1.0 */ #include "db-wrapper.h" +#include "ui.h" #include #include @@ -99,13 +100,13 @@ void DbWrapper::process(const std::string &acmd) displayRow(row, trim); } } catch (const DB::SqlConnection::Exception::Base &e) { - std::cerr << e.GetMessage() << std::endl; + UI::error() << "sql exception: " << e.GetMessage() << std::endl; } catch (const Exc::Exception &e) { - std::cerr << e.message() << std::endl; + UI::error() << "unexpected exception: " << e.message() << std::endl; } catch (const std::exception &e) { - std::cerr << e.what() << std::endl; + UI::error() << "unexpected exception: " << e.what() << std::endl; } catch (...) { - std::cerr << "Unexpected exception occurred" << std::endl; + UI::error() << "unknown exception" << std::endl; } } diff --git a/misc/ckm_db_tool/ui.cpp b/misc/ckm_db_tool/ui.cpp new file mode 100644 index 0000000..b3f9470 --- /dev/null +++ b/misc/ckm_db_tool/ui.cpp @@ -0,0 +1,74 @@ +/* + * Copyright (c) 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. + * 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 ui.cpp + * @author Mateusz Cegielka (m.cegielka@samsung.com) + * @version 1.0 + */ + +#include +#include "ui.h" + +#include + +namespace CKM { + +namespace UI { + +std::ostream& info() +{ + return std::cerr + << Colors::Text::BOLD_GREEN_BEGIN << "info:" + << Colors::Text::BOLD_GREEN_END << " "; +} + +std::ostream& warning() +{ + return std::cerr + << Colors::Text::BOLD_YELLOW_BEGIN << "warning:" + << Colors::Text::BOLD_YELLOW_END << " "; +} + +std::ostream& error() +{ + return std::cerr + << Colors::Text::BOLD_RED_BEGIN << "error:" + << Colors::Text::BOLD_RED_END << " "; +} + +std::string promptLine(const std::string &prompt) +{ + std::cerr + << Colors::Text::BOLD_WHITE_BEGIN << prompt + << Colors::Text::BOLD_WHITE_END << " "; + + std::string buf; + std::getline(std::cin, buf); + return buf; +} + +std::string promptPassword(const std::string &prompt) +{ + std::string colorPrompt + = Colors::Text::BOLD_WHITE_BEGIN + prompt + + Colors::Text::BOLD_WHITE_END + " "; + char *password = getpass(colorPrompt.c_str()); + return password ? password : ""; +} + +} // namespace UI + +} // namespace CKM diff --git a/misc/ckm_db_tool/ui.h b/misc/ckm_db_tool/ui.h new file mode 100644 index 0000000..a0236df --- /dev/null +++ b/misc/ckm_db_tool/ui.h @@ -0,0 +1,40 @@ +/* + * Copyright (c) 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. + * 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 ui.h + * @author Mateusz Cegielka (m.cegielka@samsung.com) + * @version 1.0 + */ + +#pragma once + +#include +#include + +namespace CKM { + +namespace UI { + +std::ostream& info(); +std::ostream& warning(); +std::ostream& error(); + +std::string promptLine(const std::string &prompt); +std::string promptPassword(const std::string &prompt); + +} // namespace UI + +} // namespace CKM -- 2.7.4 From 831d82cd099fb16f6c65c84527447867137a6f89 Mon Sep 17 00:00:00 2001 From: Mateusz Cegielka Date: Fri, 17 Jul 2020 17:30:16 +0200 Subject: [PATCH 08/16] Add automatic data decryption to ckm_db_tool The key manager stores key data in an encrypted database. The project also contains a ckm_db_tool CLI utility, which decrypts the database and launches an interactive SQL shell. However, inside the decrypted database, the data column is still encrypted with application-specific keys. This is inconvenient during debugging, as there is no easy way to see the data. Also, decrypting some objects' data may require object-specific passwords. This patch adds a --decrypt flag, which automatically decrypts contents of any column called "data" in all SQL query results. Additionally, if decryption of an object requires a password, it prompts the user to enter the password and uses it to decrypt the object's data. The implementation finds "data", "dataType" and "idx" columns in the output, assumes they come from the "objects" table, and uses the three values to fetch and decrypt object data with existing CKM APIs. All rows are prefixed with a message detailing whether the decryption was successful. Change-Id: I01462c5d3b24a0d7a2fea92446c4e46949b1b4f4 --- misc/ckm_db_tool/CMakeLists.txt | 1 + misc/ckm_db_tool/ckm-logic-ext.cpp | 28 ++++++++ misc/ckm_db_tool/ckm-logic-ext.h | 17 ++++- misc/ckm_db_tool/ckm_db_tool.cpp | 18 +++-- misc/ckm_db_tool/db-wrapper.cpp | 21 ++++-- misc/ckm_db_tool/db-wrapper.h | 7 +- misc/ckm_db_tool/decrypt.cpp | 136 +++++++++++++++++++++++++++++++++++++ misc/ckm_db_tool/decrypt.h | 53 +++++++++++++++ src/manager/service/ckm-logic.h | 2 + 9 files changed, 269 insertions(+), 14 deletions(-) create mode 100644 misc/ckm_db_tool/decrypt.cpp create mode 100644 misc/ckm_db_tool/decrypt.h diff --git a/misc/ckm_db_tool/CMakeLists.txt b/misc/ckm_db_tool/CMakeLists.txt index db6a133..c4d9c15 100644 --- a/misc/ckm_db_tool/CMakeLists.txt +++ b/misc/ckm_db_tool/CMakeLists.txt @@ -21,6 +21,7 @@ SET(CKM_DB_TOOLS_SOURCES ${CMAKE_CURRENT_SOURCE_DIR}/ckm-logic-ext.cpp ${CMAKE_CURRENT_SOURCE_DIR}/db-wrapper.cpp ${CMAKE_CURRENT_SOURCE_DIR}/ui.cpp + ${CMAKE_CURRENT_SOURCE_DIR}/decrypt.cpp ${KEY_MANAGER_PATH}/common/ckm-error.cpp ${KEY_MANAGER_PATH}/crypto/platform/decider.cpp ${KEY_MANAGER_PATH}/crypto/sw-backend/internals.cpp diff --git a/misc/ckm_db_tool/ckm-logic-ext.cpp b/misc/ckm_db_tool/ckm-logic-ext.cpp index 3b035fc..2cb55d1 100644 --- a/misc/ckm_db_tool/ckm-logic-ext.cpp +++ b/misc/ckm_db_tool/ckm-logic-ext.cpp @@ -27,6 +27,8 @@ namespace { const char *DB_CMD_OBJECT_SELECT = "SELECT * FROM [join_name_object_tables];"; +const char *DB_CMD_NAME_SELECT_BY_IDX = "SELECT name, label FROM NAMES WHERE idx = ?001;"; + } // anonymous namespace namespace CKM { @@ -55,6 +57,32 @@ void CKMLogicExt::saveRow(uid_t user, const DB::Row &row) m_userDataMap[user].database.saveRow(row); } +NameEntry CKMLogicExt::getNameByIdx(uid_t user, int idx) +{ + DB::SqlConnection::DataCommandUniquePtr selectCommand = + m_userDataMap[user].database.m_connection->PrepareDataCommand(DB_CMD_NAME_SELECT_BY_IDX); + selectCommand->BindInteger(1, idx); + + if (!selectCommand->Step()) + ThrowErr(Exc::DatabaseFailed, "getting name from database failed"); + + NameEntry name; + name.name = selectCommand->GetColumnString(0); + name.owner = selectCommand->GetColumnString(1); + return name; +} + +int CKMLogicExt::readDataHelperProtected( + const Credentials &cred, + DataType dataType, + const Name &name, + const ClientId &explicitOwner, + const Password &password, + Crypto::GObjUPtr &obj) +{ + return readDataHelper(false, cred, dataType, name, explicitOwner, password, obj); +} + } // namespace CKM diff --git a/misc/ckm_db_tool/ckm-logic-ext.h b/misc/ckm_db_tool/ckm-logic-ext.h index c988179..4ee1d77 100644 --- a/misc/ckm_db_tool/ckm-logic-ext.h +++ b/misc/ckm_db_tool/ckm-logic-ext.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2000 - 2017 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. @@ -26,14 +26,29 @@ #include namespace CKM { + +struct NameEntry { + Name name; + ClientId owner; +}; + struct CKMLogicExt : public CKMLogic { CKMLogicExt() : m_systemDbUnlocked(false) {} DB::SqlConnection::Output Execute(uid_t user, const std::string &cmd); DB::RowVector getRows(uid_t user); void saveRow(uid_t user, const DB::Row &row); + NameEntry getNameByIdx(uid_t user, int idx); + int readDataHelperProtected( + const Credentials &cred, + DataType dataType, + const Name &name, + const ClientId &explicitOwner, + const Password &password, + Crypto::GObjUPtr &obj); private: bool m_systemDbUnlocked; }; + } // namespace CKM diff --git a/misc/ckm_db_tool/ckm_db_tool.cpp b/misc/ckm_db_tool/ckm_db_tool.cpp index 6c31dc7..1846b62 100644 --- a/misc/ckm_db_tool/ckm_db_tool.cpp +++ b/misc/ckm_db_tool/ckm_db_tool.cpp @@ -40,6 +40,7 @@ void usage() cout << " -u, --uid UID User id as in /ckm/db- - default value 0\n"; cout << " -p, --pass PASSWORD Password used for database encryption. For system database (uid < 5000) no password should be used.\n"; cout << " -c, --cmd SQLCOMMAND Sqlite3 command to execute on database. If command not provided tool will enter interactive mode.\n"; + cout << " --decrypt Enables automatic decryption of the data column.\n"; cout << " -h, --help Shows this help.\n"; cout << "Example: Open database for user 5000 and select all data from table names\n"; cout << " cmd_db_tool -u 5000 -p P45W0RD \"select * from names\"\n"; @@ -65,15 +66,17 @@ int main(int argc, char *argv[]) uid_t uid = 0; Password pass; std::string argcmd; + bool shouldDecrypt = false; while(1) { int option_index = 0; static struct option long_options[] = { - {"uid", required_argument, 0, 'u'}, - {"cmd", required_argument, 0, 'c'}, - {"pass", required_argument, 0, 'p'}, - {"help", no_argument, 0, 'h'}, - {0, 0, 0, 0 } + {"uid", required_argument, 0, 'u'}, + {"cmd", required_argument, 0, 'c'}, + {"pass", required_argument, 0, 'p'}, + {"decrypt", no_argument, 0, 'd'}, + {"help", no_argument, 0, 'h'}, + {0, 0, 0, 0 } }; int c = getopt_long(argc, argv, "u:c:p:h", long_options, &option_index); @@ -97,6 +100,9 @@ int main(int argc, char *argv[]) case 'p': pass = optarg; break; + case 'd': + shouldDecrypt = true; + break; } } @@ -133,7 +139,7 @@ int main(int argc, char *argv[]) continue; } - dbw.process(cmd); + dbw.process(cmd, shouldDecrypt); if (!argcmd.empty()) break; diff --git a/misc/ckm_db_tool/db-wrapper.cpp b/misc/ckm_db_tool/db-wrapper.cpp index 0d9ba91..4f9498b 100644 --- a/misc/ckm_db_tool/db-wrapper.cpp +++ b/misc/ckm_db_tool/db-wrapper.cpp @@ -24,6 +24,7 @@ #include #include +#include #include @@ -74,7 +75,7 @@ void DbWrapper::lock() m_logic.lockUserKey(m_uid); } -void DbWrapper::process(const std::string &acmd) +void DbWrapper::process(const std::string &acmd, bool shouldDecrypt) { try { std::string cmd = acmd; @@ -93,11 +94,16 @@ void DbWrapper::process(const std::string &acmd) if (output.GetNames().empty()) return; - displayRow(output.GetNames(), trim); + Decrypt decryptState(output.GetNames(), m_logic, m_uid); + Decrypt *decrypt = nullptr; + if (shouldDecrypt && decryptState.canDecryptQuery()) + decrypt = &decryptState; + + displayRow(output.GetNames(), trim, nullptr); std::cout << "--------------------------" << std::endl; for (const auto &row : output.GetValues()) { - displayRow(row, trim); + displayRow(row, trim, decrypt); } } catch (const DB::SqlConnection::Exception::Base &e) { UI::error() << "sql exception: " << e.GetMessage() << std::endl; @@ -110,11 +116,18 @@ void DbWrapper::process(const std::string &acmd) } } -void DbWrapper::displayRow(const DB::SqlConnection::Output::Row &row, bool trim) +void DbWrapper::displayRow(const DB::SqlConnection::Output::Row &row, bool trim, Decrypt *decrypt) { + std::pair decrypted; + if (decrypt) + decrypted = decrypt->tryDecrypt(row); + for (auto it = row.begin(); it != row.end(); it++) { std::string col = *it; + if (decrypted.second && it - row.begin() == decrypt->getDataColumnIndex()) + col = decrypted.first; + /* * Convert unprintable data to base64. Note that a row may contain an incomplete data * since it holds a binary data as a string. diff --git a/misc/ckm_db_tool/db-wrapper.h b/misc/ckm_db_tool/db-wrapper.h index 70aa3cf..bfc5e98 100644 --- a/misc/ckm_db_tool/db-wrapper.h +++ b/misc/ckm_db_tool/db-wrapper.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2000 - 2017 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. @@ -29,6 +29,7 @@ #include #include #include "ckm-logic-ext.h" +#include "decrypt.h" namespace CKM { @@ -38,12 +39,12 @@ public: int unlock(); void lock(); - void process(const std::string &cmd); + void process(const std::string &cmd, bool shouldDecrypt); DB::RowVector getRows(); void saveRow(const DB::Row &row); private: - void displayRow(const DB::SqlConnection::Output::Row &row, bool trim); + void displayRow(const DB::SqlConnection::Output::Row &row, bool trim, Decrypt *decrypt); uid_t m_uid; Password m_pw; diff --git a/misc/ckm_db_tool/decrypt.cpp b/misc/ckm_db_tool/decrypt.cpp new file mode 100644 index 0000000..6187b25 --- /dev/null +++ b/misc/ckm_db_tool/decrypt.cpp @@ -0,0 +1,136 @@ +/* + * Copyright (c) 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. + * 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 decrypt.cpp + * @author Mateusz Cegielka (m.cegielka@samsung.com) + * @version 1.0 + */ + +#include "decrypt.h" +#include "ui.h" + +#include +#include +#include + +#include +#include + +#include + +namespace { + +int findColumnByName(const std::string &name, const CKM::Decrypt::Row &columns) +{ + auto column = std::find(columns.begin(), columns.end(), name); + if (column == columns.end()) + return -1; + + return column - columns.begin(); +} + +std::string fmtObject(const CKM::NameEntry &name) +{ + return "'" + name.name + "'"; +} + +CKM::Password askObjectPassword(const std::string &displayName) +{ + std::string password = CKM::UI::promptPassword( + "enter password for object " + displayName + ":"); + return CKM::Password(password.begin(), password.end()); +} + +} // anonymous namespace + +namespace CKM { + +Decrypt::Decrypt(const Row &columns, CKMLogicExt &logic, uid_t uid) + : m_logic(logic), + m_uid(uid), + m_columnData(-1), + m_columnDataType(-1), + m_columnIdx(-1) +{ + m_columnData = findColumnByName("data", columns); + if (m_columnData == -1) + return; + m_columnDataType = findColumnByName("dataType", columns); + m_columnIdx = findColumnByName("idx", columns); + if (m_columnDataType == -1 || m_columnIdx == -1) + UI::warning() << "unable to decrypt, missing dataType or idx columns" << std::endl; +} + +bool Decrypt::canDecryptQuery() const +{ + return m_columnData != -1 && m_columnDataType != -1 && m_columnIdx != -1; +} + +std::pair Decrypt::tryDecrypt(const Row &row) const +{ + NameEntry nameEntry = getNameEntry(row); + std::string name = fmtObject(nameEntry); + + try { + return {decrypt(row, "", nameEntry), true}; + } catch (const Exc::AuthenticationFailed &) { + } + + while (true) { + Password password = askObjectPassword(name); + if (password.empty()) { + UI::warning() << "skipped decrypting object " << name << std::endl; + return {"", false}; + } + + try { + return {decrypt(row, password, nameEntry), true}; + } catch (const Exc::AuthenticationFailed &) { + UI::error() << "authentication failed for object " << name << std::endl; + } + } +} + +int Decrypt::getDataColumnIndex() const +{ + return m_columnData; +} + +NameEntry Decrypt::getNameEntry(const Row &row) const +{ + int idx = std::stoi(row[m_columnIdx]); + + return m_logic.getNameByIdx(m_uid, idx); +} + +std::string Decrypt::decrypt(const Row &row, const Password &password, + const NameEntry& nameEntry) const +{ + DataType dataType = static_cast(std::stoi(row[m_columnDataType])); + Credentials credentials(m_uid, nameEntry.owner); + Crypto::GObjUPtr obj; + + int ret = m_logic.readDataHelperProtected(credentials, dataType, nameEntry.name, + nameEntry.owner, password, obj); + if (ret != CKM_API_SUCCESS) + throw std::logic_error("reading row data failed"); + + RawBuffer decrypted = obj->getBinary(); + UI::info() << "decrypted object " << fmtObject(nameEntry) << std::endl; + return std::string(decrypted.begin(), decrypted.end()); +} + +} // namespace CKM diff --git a/misc/ckm_db_tool/decrypt.h b/misc/ckm_db_tool/decrypt.h new file mode 100644 index 0000000..644b694 --- /dev/null +++ b/misc/ckm_db_tool/decrypt.h @@ -0,0 +1,53 @@ +/* + * Copyright (c) 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. + * 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 decrypt.h + * @author Mateusz Cegielka (m.cegielka@samsung.com) + * @version 1.0 + */ + +#pragma once + +#include +#include + +#include "ckm-logic-ext.h" +#include + +namespace CKM { + +class Decrypt { +public: + using Row = DB::SqlConnection::Output::Row; + + Decrypt(const Row &columns, CKMLogicExt &logic, uid_t uid); + + bool canDecryptQuery() const; + std::pair tryDecrypt(const Row &row) const; + int getDataColumnIndex() const; + +private: + NameEntry getNameEntry(const Row &row) const; + std::string decrypt(const Row &row, const Password &password, const NameEntry& name) const; + + CKMLogicExt &m_logic; + uid_t m_uid; + int m_columnData; + int m_columnDataType; + int m_columnIdx; +}; + +} // namespace CKM diff --git a/src/manager/service/ckm-logic.h b/src/manager/service/ckm-logic.h index b1c87cb..c832edc 100644 --- a/src/manager/service/ckm-logic.h +++ b/src/manager/service/ckm-logic.h @@ -321,6 +321,7 @@ private: DB::Row row, const Password &password); +protected: int readDataHelper( bool exportFlag, const Credentials &cred, @@ -330,6 +331,7 @@ private: const Password &password, Crypto::GObjUPtr &obj); +private: int readDataHelper( bool exportFlag, const Credentials &cred, -- 2.7.4 From 8e9da70e3c15bddc9831702dbd0e97dfe19d2790 Mon Sep 17 00:00:00 2001 From: Mateusz Cegielka Date: Mon, 10 Aug 2020 13:24:30 +0200 Subject: [PATCH 09/16] Optimize message serialization There is a MessageBuffer class, which stores a list of byte slices as std::list> and can be used for serializing structs. Every member calls a Write method, which creates a new vector and appends it to the list. After the list is built, a vector with an exactly right size is allocated and the data is copied. Also, the class contains unnecessary mutable state, because the logic is shared with streaming deserialization. I have replaced the serialization methods with a single function, which serializes all objects twice. The first pass ignores the data and only computes the message size, which allows the second pass that actually writes the data to only use a single allocation. The new interface is also simpler and more robust. Change-Id: I6091b71083997faf9302ad8549ade467deb23a58 --- .../client-async/client-manager-async-impl.cpp | 10 +- .../client-async/client-manager-async-impl.h | 4 +- src/manager/client/client-control.cpp | 66 +++---- src/manager/client/client-manager-impl.cpp | 216 ++++++++++----------- src/manager/common/message-buffer.cpp | 45 +++-- src/manager/common/message-buffer.h | 50 +++-- src/manager/crypto/sw-backend/store.cpp | 4 +- src/manager/crypto/tz-backend/store.cpp | 6 +- src/manager/service/ckm-logic.cpp | 62 ++---- src/manager/service/ckm-service.cpp | 2 +- src/manager/service/encryption-service.cpp | 2 +- src/manager/service/ocsp-logic.cpp | 2 +- unit-tests/test_data-type.cpp | 7 +- unit-tests/test_pkcs12.cpp | 4 +- unit-tests/test_serialization.cpp | 9 +- 15 files changed, 247 insertions(+), 242 deletions(-) diff --git a/src/manager/client-async/client-manager-async-impl.cpp b/src/manager/client-async/client-manager-async-impl.cpp index 19479f0..ea5aa54 100644 --- a/src/manager/client-async/client-manager-async-impl.cpp +++ b/src/manager/client-async/client-manager-async-impl.cpp @@ -252,9 +252,9 @@ void ManagerAsync::Impl::ocspCheck(const ObserverPtr &observer, } m_counter++; - auto send = MessageBuffer::Serialize(m_counter, rawCertChain); + auto send = SerializeMessage(m_counter, rawCertChain); - thread()->sendMessage(AsyncRequest(observer, SERVICE_SOCKET_OCSP, send.Pop(), + thread()->sendMessage(AsyncRequest(observer, SERVICE_SOCKET_OCSP, std::move(send), m_counter, 0)); }, [&observer](int error) { observer->ReceivedError(error); @@ -402,11 +402,11 @@ void ManagerAsync::Impl::crypt( auto command = static_cast(encryption ? EncryptionCommand::ENCRYPT : EncryptionCommand::DECRYPT); - auto send = MessageBuffer::Serialize(command, m_counter, cas, - helper.getName(), helper.getOwner(), password, input); + auto send = SerializeMessage(command, m_counter, cas, helper.getName(), helper.getOwner(), + password, input); thread()->sendMessage(AsyncRequest(observer, SERVICE_SOCKET_ENCRYPTION, - send.Pop(), m_counter, command)); + std::move(send), m_counter, command)); }, [&observer](int error) { observer->ReceivedError(error); }); diff --git a/src/manager/client-async/client-manager-async-impl.h b/src/manager/client-async/client-manager-async-impl.h index 6fca459..1a836d7 100644 --- a/src/manager/client-async/client-manager-async-impl.h +++ b/src/manager/client-async/client-manager-async-impl.h @@ -160,10 +160,10 @@ private: { m_counter++; // yes, it changes m_counter argument passed in args - auto send = MessageBuffer::Serialize(command, args...); + auto send = SerializeMessage(command, args...); thread()->sendMessage(AsyncRequest(observer, SERVICE_SOCKET_CKM_STORAGE, - send.Pop(), + std::move(send), m_counter, command)); } diff --git a/src/manager/client/client-control.cpp b/src/manager/client/client-control.cpp index ed59582..d0a4d18 100644 --- a/src/manager/client/client-control.cpp +++ b/src/manager/client/client-control.cpp @@ -46,12 +46,11 @@ public: return CKM_API_ERROR_INPUT_PARAM; MessageBuffer recv; - auto send = MessageBuffer::Serialize(static_cast - (ControlCommand::UNLOCK_USER_KEY), - user, - password); + auto send = SerializeMessage(static_cast(ControlCommand::UNLOCK_USER_KEY), + user, + password); - int retCode = m_controlConnection.processRequest(send.Pop(), recv); + int retCode = m_controlConnection.processRequest(send, recv); if (CKM_API_SUCCESS != retCode) return retCode; @@ -71,10 +70,9 @@ public: return CKM_API_ERROR_INPUT_PARAM; MessageBuffer recv; - auto send = MessageBuffer::Serialize(static_cast - (ControlCommand::LOCK_USER_KEY), user); + auto send = SerializeMessage(static_cast(ControlCommand::LOCK_USER_KEY), user); - int retCode = m_controlConnection.processRequest(send.Pop(), recv); + int retCode = m_controlConnection.processRequest(send, recv); if (CKM_API_SUCCESS != retCode) return retCode; @@ -94,10 +92,9 @@ public: return CKM_API_ERROR_INPUT_PARAM; MessageBuffer recv; - auto send = MessageBuffer::Serialize(static_cast - (ControlCommand::REMOVE_USER_DATA), user); + auto send = SerializeMessage(static_cast(ControlCommand::REMOVE_USER_DATA), user); - int retCode = m_controlConnection.processRequest(send.Pop(), recv); + int retCode = m_controlConnection.processRequest(send, recv); if (CKM_API_SUCCESS != retCode) return retCode; @@ -118,13 +115,12 @@ public: return CKM_API_ERROR_INPUT_PARAM; MessageBuffer recv; - auto send = MessageBuffer::Serialize( - static_cast(ControlCommand::CHANGE_USER_PASSWORD), - user, - oldPassword, - newPassword); + auto send = SerializeMessage(static_cast(ControlCommand::CHANGE_USER_PASSWORD), + user, + oldPassword, + newPassword); - int retCode = m_controlConnection.processRequest(send.Pop(), recv); + int retCode = m_controlConnection.processRequest(send, recv); if (CKM_API_SUCCESS != retCode) return retCode; @@ -144,12 +140,11 @@ public: return CKM_API_ERROR_INPUT_PARAM; MessageBuffer recv; - auto send = MessageBuffer::Serialize( - static_cast(ControlCommand::RESET_USER_PASSWORD), - user, - newPassword); + auto send = SerializeMessage(static_cast(ControlCommand::RESET_USER_PASSWORD), + user, + newPassword); - int retCode = m_controlConnection.processRequest(send.Pop(), recv); + int retCode = m_controlConnection.processRequest(send, recv); if (CKM_API_SUCCESS != retCode) return retCode; @@ -169,10 +164,9 @@ public: return CKM_API_ERROR_INPUT_PARAM; MessageBuffer recv; - auto send = MessageBuffer::Serialize(static_cast - (ControlCommand::REMOVE_APP_DATA), owner); + auto send = SerializeMessage(static_cast(ControlCommand::REMOVE_APP_DATA), owner); - int retCode = m_controlConnection.processRequest(send.Pop(), recv); + int retCode = m_controlConnection.processRequest(send, recv); if (CKM_API_SUCCESS != retCode) return retCode; @@ -189,10 +183,9 @@ public: EXCEPTION_GUARD_START_CPPAPI MessageBuffer recv; - auto send = MessageBuffer::Serialize(static_cast - (ControlCommand::UPDATE_CC_MODE)); + auto send = SerializeMessage(static_cast(ControlCommand::UPDATE_CC_MODE)); - int retCode = m_controlConnection.processRequest(send.Pop(), recv); + int retCode = m_controlConnection.processRequest(send, recv); if (CKM_API_SUCCESS != retCode) return retCode; @@ -213,15 +206,14 @@ public: MessageBuffer recv; AliasSupport helper(alias); - auto send = MessageBuffer::Serialize(static_cast - (ControlCommand::SET_PERMISSION), - static_cast(user), - helper.getName(), - helper.getOwner(), - accessor, - permissionMask); - - int retCode = m_controlConnection.processRequest(send.Pop(), recv); + auto send = SerializeMessage(static_cast(ControlCommand::SET_PERMISSION), + static_cast(user), + helper.getName(), + helper.getOwner(), + accessor, + permissionMask); + + int retCode = m_controlConnection.processRequest(send, recv); if (CKM_API_SUCCESS != retCode) return retCode; diff --git a/src/manager/client/client-manager-impl.cpp b/src/manager/client/client-manager-impl.cpp index 9eecd5a..3bbdcc3 100644 --- a/src/manager/client/client-manager-impl.cpp +++ b/src/manager/client/client-manager-impl.cpp @@ -49,14 +49,14 @@ int getCertChain( EXCEPTION_GUARD_START_CPPAPI MessageBuffer recv; - auto send = MessageBuffer::Serialize(static_cast(command), - msgId, - certificate->getDER(), - untrustedVector, - trustedVector, - useTrustedSystemCertificates); + auto send = SerializeMessage(static_cast(command), + msgId, + certificate->getDER(), + untrustedVector, + trustedVector, + useTrustedSystemCertificates); - int retCode = serviceConnection.processRequest(send.Pop(), recv); + int retCode = serviceConnection.processRequest(send, recv); if (CKM_API_SUCCESS != retCode) return retCode; @@ -111,15 +111,15 @@ int Manager::Impl::saveBinaryData( MessageBuffer recv; AliasSupport helper(alias); - auto send = MessageBuffer::Serialize(static_cast(LogicCommand::SAVE), - msgId, - dataType, - helper.getName(), - helper.getOwner(), - rawData, - PolicySerializable(policy)); + auto send = SerializeMessage(static_cast(LogicCommand::SAVE), + msgId, + dataType, + helper.getName(), + helper.getOwner(), + rawData, + PolicySerializable(policy)); - int retCode = m_storageConnection.processRequest(send.Pop(), recv); + int retCode = m_storageConnection.processRequest(send, recv); if (CKM_API_SUCCESS != retCode) return retCode; @@ -186,16 +186,15 @@ int Manager::Impl::savePKCS12( MessageBuffer recv; AliasSupport helper(alias); - auto send = MessageBuffer::Serialize(static_cast - (LogicCommand::SAVE_PKCS12), - msgId, - helper.getName(), - helper.getOwner(), - PKCS12Serializable(*pkcs.get()), - PolicySerializable(keyPolicy), - PolicySerializable(certPolicy)); + auto send = SerializeMessage(static_cast(LogicCommand::SAVE_PKCS12), + msgId, + helper.getName(), + helper.getOwner(), + PKCS12Serializable(*pkcs.get()), + PolicySerializable(keyPolicy), + PolicySerializable(certPolicy)); - int retCode = m_storageConnection.processRequest(send.Pop(), recv); + int retCode = m_storageConnection.processRequest(send, recv); if (CKM_API_SUCCESS != retCode) return retCode; @@ -228,14 +227,14 @@ int Manager::Impl::getPKCS12(const Alias &alias, const Password &keyPass, MessageBuffer recv; AliasSupport helper(alias); - auto send = MessageBuffer::Serialize(static_cast(LogicCommand::GET_PKCS12), - msgId, - helper.getName(), - helper.getOwner(), - keyPass, - certPass); + auto send = SerializeMessage(static_cast(LogicCommand::GET_PKCS12), + msgId, + helper.getName(), + helper.getOwner(), + keyPass, + certPass); - int retCode = m_storageConnection.processRequest(send.Pop(), recv); + int retCode = m_storageConnection.processRequest(send, recv); if (CKM_API_SUCCESS != retCode) return retCode; @@ -266,12 +265,12 @@ int Manager::Impl::removeAlias(const Alias &alias) MessageBuffer recv; AliasSupport helper(alias); - auto send = MessageBuffer::Serialize(static_cast(LogicCommand::REMOVE), - msgId, - helper.getName(), - helper.getOwner()); + auto send = SerializeMessage(static_cast(LogicCommand::REMOVE), + msgId, + helper.getName(), + helper.getOwner()); - int retCode = m_storageConnection.processRequest(send.Pop(), recv); + int retCode = m_storageConnection.processRequest(send, recv); if (CKM_API_SUCCESS != retCode) return retCode; @@ -303,14 +302,14 @@ int Manager::Impl::getBinaryData( MessageBuffer recv; AliasSupport helper(alias); - auto send = MessageBuffer::Serialize(static_cast(LogicCommand::GET), - msgId, - sendDataType, - helper.getName(), - helper.getOwner(), - password); + auto send = SerializeMessage(static_cast(LogicCommand::GET), + msgId, + sendDataType, + helper.getName(), + helper.getOwner(), + password); - int retCode = m_storageConnection.processRequest(send.Pop(), recv); + int retCode = m_storageConnection.processRequest(send, recv); if (CKM_API_SUCCESS != retCode) return retCode; @@ -339,13 +338,13 @@ int Manager::Impl::getBinaryDataEncryptionStatus(const DataType sendDataType, MessageBuffer recv; AliasSupport helper(alias); - auto send = MessageBuffer::Serialize(static_cast(LogicCommand::GET_PROTECTION_STATUS), - msgId, - sendDataType, - helper.getName(), - helper.getOwner()); + auto send = SerializeMessage(static_cast(LogicCommand::GET_PROTECTION_STATUS), + msgId, + sendDataType, + helper.getName(), + helper.getOwner()); - int retCode = m_storageConnection.processRequest(send.Pop(), recv); + int retCode = m_storageConnection.processRequest(send, recv); if (CKM_API_SUCCESS != retCode) return retCode; @@ -456,11 +455,9 @@ int Manager::Impl::getBinaryDataAliasVectorHelper(DataType dataType, int msgId = ++m_counter; MessageBuffer recv; - auto send = MessageBuffer::Serialize(static_cast(LogicCommand::GET_LIST), - msgId, - dataType); + auto send = SerializeMessage(static_cast(LogicCommand::GET_LIST), msgId, dataType); - int retCode = m_storageConnection.processRequest(send.Pop(), recv); + int retCode = m_storageConnection.processRequest(send, recv); if (retCode != CKM_API_SUCCESS) return retCode; @@ -611,15 +608,14 @@ int Manager::Impl::createKeyAES( MessageBuffer recv; AliasSupport aliasHelper(keyAlias); - auto send = MessageBuffer::Serialize(static_cast - (LogicCommand::CREATE_KEY_AES), - msgId, - static_cast(size), - PolicySerializable(policyKey), - aliasHelper.getName(), - aliasHelper.getOwner()); + auto send = SerializeMessage(static_cast(LogicCommand::CREATE_KEY_AES), + msgId, + static_cast(size), + PolicySerializable(policyKey), + aliasHelper.getName(), + aliasHelper.getOwner()); - int retCode = m_storageConnection.processRequest(send.Pop(), recv); + int retCode = m_storageConnection.processRequest(send, recv); if (CKM_API_SUCCESS != retCode) return retCode; @@ -678,18 +674,17 @@ int Manager::Impl::createKeyPair( MessageBuffer recv; AliasSupport privateHelper(privateKeyAlias); AliasSupport publicHelper(publicKeyAlias); - auto send = MessageBuffer::Serialize(static_cast - (LogicCommand::CREATE_KEY_PAIR), - msgId, - CryptoAlgorithmSerializable(keyGenAlgorithm), - PolicySerializable(policyPrivateKey), - PolicySerializable(policyPublicKey), - privateHelper.getName(), - privateHelper.getOwner(), - publicHelper.getName(), - publicHelper.getOwner()); - - int retCode = m_storageConnection.processRequest(send.Pop(), recv); + auto send = SerializeMessage(static_cast(LogicCommand::CREATE_KEY_PAIR), + msgId, + CryptoAlgorithmSerializable(keyGenAlgorithm), + PolicySerializable(policyPrivateKey), + PolicySerializable(policyPublicKey), + privateHelper.getName(), + privateHelper.getOwner(), + publicHelper.getName(), + publicHelper.getOwner()); + + int retCode = m_storageConnection.processRequest(send, recv); if (CKM_API_SUCCESS != retCode) return retCode; @@ -788,16 +783,15 @@ int Manager::Impl::createSignature( MessageBuffer recv; AliasSupport helper(privateKeyAlias); - auto send = MessageBuffer::Serialize(static_cast - (LogicCommand::CREATE_SIGNATURE), - msgId, - helper.getName(), - helper.getOwner(), - password, - message, - CryptoAlgorithmSerializable(cAlgorithm)); + auto send = SerializeMessage(static_cast(LogicCommand::CREATE_SIGNATURE), + msgId, + helper.getName(), + helper.getOwner(), + password, + message, + CryptoAlgorithmSerializable(cAlgorithm)); - int retCode = m_storageConnection.processRequest(send.Pop(), recv); + int retCode = m_storageConnection.processRequest(send, recv); if (CKM_API_SUCCESS != retCode) return retCode; @@ -826,17 +820,16 @@ int Manager::Impl::verifySignature( MessageBuffer recv; AliasSupport helper(publicKeyOrCertAlias); - auto send = MessageBuffer::Serialize(static_cast - (LogicCommand::VERIFY_SIGNATURE), - msgId, - helper.getName(), - helper.getOwner(), - password, - message, - signature, - CryptoAlgorithmSerializable(cAlg)); - - int retCode = m_storageConnection.processRequest(send.Pop(), recv); + auto send = SerializeMessage(static_cast(LogicCommand::VERIFY_SIGNATURE), + msgId, + helper.getName(), + helper.getOwner(), + password, + message, + signature, + CryptoAlgorithmSerializable(cAlg)); + + int retCode = m_storageConnection.processRequest(send, recv); if (CKM_API_SUCCESS != retCode) return retCode; @@ -871,9 +864,9 @@ int Manager::Impl::ocspCheck(const CertificateShPtrVector &certChain, rawCertChain.push_back(e->getDER()); } - auto send = MessageBuffer::Serialize(msgId, rawCertChain); + auto send = SerializeMessage(msgId, rawCertChain); - int retCode = m_ocspConnection.processRequest(send.Pop(), recv); + int retCode = m_ocspConnection.processRequest(send, recv); if (CKM_API_SUCCESS != retCode) return retCode; @@ -899,15 +892,14 @@ int Manager::Impl::setPermission(const Alias &alias, MessageBuffer recv; AliasSupport helper(alias); - auto send = MessageBuffer::Serialize(static_cast - (LogicCommand::SET_PERMISSION), - msgId, - helper.getName(), - helper.getOwner(), - accessor, - permissionMask); + auto send = SerializeMessage(static_cast(LogicCommand::SET_PERMISSION), + msgId, + helper.getName(), + helper.getOwner(), + accessor, + permissionMask); - int retCode = m_storageConnection.processRequest(send.Pop(), recv); + int retCode = m_storageConnection.processRequest(send, recv); if (CKM_API_SUCCESS != retCode) return retCode; @@ -937,15 +929,15 @@ int Manager::Impl::crypt(EncryptionCommand command, MessageBuffer recv; AliasSupport helper(keyAlias); CryptoAlgorithmSerializable cas(algo); - auto send = MessageBuffer::Serialize(static_cast(command), - msgId, - cas, - helper.getName(), - helper.getOwner(), - password, - input); - - int retCode = m_encryptionConnection.processRequest(send.Pop(), recv); + auto send = SerializeMessage(static_cast(command), + msgId, + cas, + helper.getName(), + helper.getOwner(), + password, + input); + + int retCode = m_encryptionConnection.processRequest(send, recv); if (CKM_API_SUCCESS != retCode) return retCode; diff --git a/src/manager/common/message-buffer.cpp b/src/manager/common/message-buffer.cpp index 0aa2d11..ef1ceb2 100644 --- a/src/manager/common/message-buffer.cpp +++ b/src/manager/common/message-buffer.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2000-2019 Samsung Electronics Co., Ltd. All rights reserved + * Copyright (c) 2000-2020 Samsung Electronics Co., Ltd. All rights reserved * * Contact: Dongsun Lee * @@ -25,6 +25,7 @@ #include +#include #include namespace CKM { @@ -34,16 +35,6 @@ void MessageBuffer::Push(const RawBuffer &data) m_buffer.AppendCopy(&data[0], data.size()); } -RawBuffer MessageBuffer::Pop() -{ - size_t size = m_buffer.Size(); - RawBuffer buffer; - buffer.resize(size + sizeof(size_t)); - memcpy(&buffer[0], &size, sizeof(size_t)); - m_buffer.FlattenConsume(&buffer[sizeof(size_t)], size); - return buffer; -} - bool MessageBuffer::Ready() { CountBytesLeft(); @@ -71,9 +62,37 @@ void MessageBuffer::Read(size_t num, void *bytes) m_bytesLeft -= num; } -void MessageBuffer::Write(size_t num, const void *bytes) +void MessageBuffer::Write(size_t, const void *) +{ + ThrowErr(Exc::InternalError, "unexpected IStream::Write call in MessageBuffer"); +} + +void MessageSerializer::Sizer::Read(size_t, void*) +{ + ThrowErr(Exc::InternalError, "unexpected IStream::Read call in SerializeMessage"); +} + +void MessageSerializer::Sizer::Write(size_t num, const void*) +{ + m_bytes += num; +} + +MessageSerializer::Writer::Writer(size_t size) +{ + m_buffer.reserve(sizeof(size_t) + size); + Serializer::Serialize(*this, size); +} + +void MessageSerializer::Writer::Read(size_t, void*) +{ + ThrowErr(Exc::InternalError, "unexpected IStream::Read call in SerializeMessage"); +} + +void MessageSerializer::Writer::Write(size_t num, const void* bytes) { - m_buffer.AppendCopy(bytes, num); + assert(m_buffer.size() + num <= m_buffer.capacity()); + m_buffer.resize(m_buffer.size() + num); + memcpy(&m_buffer[m_buffer.size() - num], bytes, num); } } // namespace CKM diff --git a/src/manager/common/message-buffer.h b/src/manager/common/message-buffer.h index 7266d12..6f0572a 100644 --- a/src/manager/common/message-buffer.h +++ b/src/manager/common/message-buffer.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2000-2019 Samsung Electronics Co., Ltd. All rights reserved + * Copyright (c) 2000-2020 Samsung Electronics Co., Ltd. All rights reserved * * Contact: Dongsun Lee * @@ -49,23 +49,12 @@ public: void Push(const RawBuffer &data); - RawBuffer Pop(); - bool Ready(); virtual void Read(size_t num, void *bytes); virtual void Write(size_t num, const void *bytes); - // generic serialization - template - static MessageBuffer Serialize(const Args &... args) - { - MessageBuffer buffer; - Serializer::Serialize(buffer, args...); - return buffer; - } - // generic deserialization template void Deserialize(Args &... args) @@ -89,6 +78,43 @@ protected: CKM::BinaryQueue m_buffer; }; +class MessageSerializer final { + struct COMMON_API Sizer : IStream { + void Read(size_t num, void* bytes) override; + void Write(size_t num, const void* bytes) override; + + size_t m_bytes = 0; + }; + + struct COMMON_API Writer : IStream { + Writer(size_t size); + + void Read(size_t num, void* bytes) override; + void Write(size_t num, const void* bytes) override; + + RawBuffer m_buffer; + }; + + MessageSerializer() = delete; + +public: + template + static RawBuffer Run(const T&... args) + { + Sizer sizer; + Serializer::Serialize(sizer, args...); + Writer writer(sizer.m_bytes); + Serializer::Serialize(writer, args...); + return std::move(writer.m_buffer); + } +}; + +template +RawBuffer SerializeMessage(const T&... args) +{ + return MessageSerializer::Run(args...); +} + } // namespace CKM #endif // _CENT_KEY_MNG_SOCKET_BUFFER_ diff --git a/src/manager/crypto/sw-backend/store.cpp b/src/manager/crypto/sw-backend/store.cpp index 98ae2bf..08891ed 100644 --- a/src/manager/crypto/sw-backend/store.cpp +++ b/src/manager/crypto/sw-backend/store.cpp @@ -136,11 +136,11 @@ RawBuffer pack(const RawBuffer &data, const Password &pass) // serialization exceptions will be catched as CKM::Exception and will cause // CKM_API_ERROR_SERVER_ERROR - packed = MessageBuffer::Serialize(ret.first, iv, ret.second).Pop(); + packed = SerializeMessage(ret.first, iv, ret.second); } // encryption scheme + internal buffer - return MessageBuffer::Serialize(scheme, packed).Pop(); + return SerializeMessage(scheme, packed); } } // namespace anonymous diff --git a/src/manager/crypto/tz-backend/store.cpp b/src/manager/crypto/tz-backend/store.cpp index 56ca699..3183937 100644 --- a/src/manager/crypto/tz-backend/store.cpp +++ b/src/manager/crypto/tz-backend/store.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2015 - 2019 Samsung Electronics Co., Ltd All Rights Reserved + * Copyright (c) 2015 - 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. @@ -79,9 +79,9 @@ RawBuffer pack(const RawBuffer &keyId, const Password &pwd, int scheme = pwd.empty() ? EncryptionScheme::NONE : EncryptionScheme::PASSWORD; if (scheme == EncryptionScheme::PASSWORD) { - return MessageBuffer::Serialize(scheme, keyId, iv, tag).Pop(); + return SerializeMessage(scheme, keyId, iv, tag); } else { - return MessageBuffer::Serialize(scheme, keyId).Pop(); + return SerializeMessage(scheme, keyId); } } diff --git a/src/manager/service/ckm-logic.cpp b/src/manager/service/ckm-logic.cpp index 867b527..925a00e 100644 --- a/src/manager/service/ckm-logic.cpp +++ b/src/manager/service/ckm-logic.cpp @@ -214,13 +214,13 @@ RawBuffer CKMLogic::unlockUserKey(uid_t user, const Password &password) else // do not allow lock/unlock operations for system users retCode = CKM_API_ERROR_INPUT_PARAM; - return MessageBuffer::Serialize(retCode).Pop(); + return SerializeMessage(retCode); } RawBuffer CKMLogic::updateCCMode() { m_accessControl.updateCCMode(); - return MessageBuffer::Serialize(CKM_API_SUCCESS).Pop(); + return SerializeMessage(CKM_API_SUCCESS); } RawBuffer CKMLogic::lockUserKey(uid_t user) @@ -232,7 +232,7 @@ RawBuffer CKMLogic::lockUserKey(uid_t user) else // do not allow lock/unlock operations for system users retCode = CKM_API_ERROR_INPUT_PARAM; - return MessageBuffer::Serialize(retCode).Pop(); + return SerializeMessage(retCode); } RawBuffer CKMLogic::removeUserData(uid_t user) @@ -246,7 +246,7 @@ RawBuffer CKMLogic::removeUserData(uid_t user) ? CKM_API_ERROR_FILE_SYSTEM : CKM_API_SUCCESS; - return MessageBuffer::Serialize(retCode).Pop(); + return SerializeMessage(retCode); } int CKMLogic::changeUserPasswordHelper(uid_t user, @@ -279,7 +279,7 @@ RawBuffer CKMLogic::changeUserPassword( retCode = CKM_API_ERROR_SERVER_ERROR; } - return MessageBuffer::Serialize(retCode).Pop(); + return SerializeMessage(retCode); } int CKMLogic::resetUserPasswordHelper( @@ -321,7 +321,7 @@ RawBuffer CKMLogic::resetUserPassword( retCode = CKM_API_ERROR_SERVER_ERROR; } - return MessageBuffer::Serialize(retCode).Pop(); + return SerializeMessage(retCode); } RawBuffer CKMLogic::removeApplicationData(const ClientId &owner) @@ -352,7 +352,7 @@ RawBuffer CKMLogic::removeApplicationData(const ClientId &owner) retCode = CKM_API_ERROR_SERVER_ERROR; } - return MessageBuffer::Serialize(retCode).Pop(); + return SerializeMessage(retCode); } int CKMLogic::checkSaveConditions( @@ -523,8 +523,7 @@ RawBuffer CKMLogic::saveData( const PolicySerializable &policy) { int retCode = verifyAndSaveDataHelper(cred, name, explicitOwner, data, policy); - auto response = MessageBuffer::Serialize(msgId, retCode, data.type); - return response.Pop(); + return SerializeMessage(msgId, retCode, data.type); } int CKMLogic::extractPKCS12Data( @@ -608,8 +607,7 @@ RawBuffer CKMLogic::savePKCS12( retCode = CKM_API_ERROR_SERVER_ERROR; } - auto response = MessageBuffer::Serialize(msgId, retCode); - return response.Pop(); + return SerializeMessage(msgId, retCode); } @@ -691,8 +689,7 @@ RawBuffer CKMLogic::removeData( retCode = CKM_API_ERROR_DB_ERROR; } - auto response = MessageBuffer::Serialize(msgId, retCode); - return response.Pop(); + return SerializeMessage(msgId, retCode); } int CKMLogic::readSingleRow(const Name &name, @@ -965,11 +962,7 @@ RawBuffer CKMLogic::getData( if (CKM_API_SUCCESS != retCode) rowData.clear(); - auto response = MessageBuffer::Serialize(msgId, - retCode, - objDataType, - rowData); - return response.Pop(); + return SerializeMessage(msgId, retCode, objDataType, rowData); } RawBuffer CKMLogic::getDataProtectionStatus( @@ -1001,11 +994,7 @@ RawBuffer CKMLogic::getDataProtectionStatus( retCode = CKM_API_SUCCESS; } - auto response = MessageBuffer::Serialize(msgId, - retCode, - objDataType, - status); - return response.Pop(); + return SerializeMessage(msgId, retCode, objDataType, status); } int CKMLogic::getPKCS12Helper( @@ -1094,8 +1083,7 @@ RawBuffer CKMLogic::getPKCS12( retCode = CKM_API_ERROR_SERVER_ERROR; } - auto response = MessageBuffer::Serialize(msgId, retCode, output); - return response.Pop(); + return SerializeMessage(msgId, retCode, output); } int CKMLogic::getDataListHelper(const Credentials &cred, @@ -1179,11 +1167,7 @@ RawBuffer CKMLogic::getDataList( userVector.end()); } - auto response = MessageBuffer::Serialize(msgId, - retCode, - dataType, - ownerNameVector); - return response.Pop(); + return SerializeMessage(msgId, retCode, dataType, ownerNameVector); } int CKMLogic::importInitialData( @@ -1475,7 +1459,7 @@ RawBuffer CKMLogic::createKeyPair( retCode = CKM_API_ERROR_SERVER_ERROR; } - return MessageBuffer::Serialize(msgId, retCode).Pop(); + return SerializeMessage(msgId, retCode); } RawBuffer CKMLogic::createKeyAES( @@ -1500,7 +1484,7 @@ RawBuffer CKMLogic::createKeyAES( retCode = CKM_API_ERROR_SERVER_ERROR; } - return MessageBuffer::Serialize(msgId, retCode).Pop(); + return SerializeMessage(msgId, retCode); } int CKMLogic::readCertificateHelper( @@ -1663,8 +1647,7 @@ RawBuffer CKMLogic::getCertificateChain( LogError("Unknown error."); } - auto response = MessageBuffer::Serialize(msgId, retCode, chainRawVector); - return response.Pop(); + return SerializeMessage(msgId, retCode, chainRawVector); } RawBuffer CKMLogic::getCertificateChain( @@ -1695,8 +1678,7 @@ RawBuffer CKMLogic::getCertificateChain( LogError("Unknown error."); } - auto response = MessageBuffer::Serialize(msgId, retCode, chainRawVector); - return response.Pop(); + return SerializeMessage(msgId, retCode, chainRawVector); } RawBuffer CKMLogic::createSignature( @@ -1729,8 +1711,7 @@ RawBuffer CKMLogic::createSignature( retCode = CKM_API_ERROR_SERVER_ERROR; } - auto response = MessageBuffer::Serialize(msgId, retCode, signature); - return response.Pop(); + return SerializeMessage(msgId, retCode, signature); } RawBuffer CKMLogic::verifySignature( @@ -1766,8 +1747,7 @@ RawBuffer CKMLogic::verifySignature( retCode = CKM_API_ERROR_SERVER_ERROR; } - auto response = MessageBuffer::Serialize(msgId, retCode); - return response.Pop(); + return SerializeMessage(msgId, retCode); } int CKMLogic::setPermissionHelper( @@ -1837,7 +1817,7 @@ RawBuffer CKMLogic::setPermission( retCode = CKM_API_ERROR_DB_ERROR; } - return MessageBuffer::Serialize(msgID, retCode).Pop(); + return SerializeMessage(msgID, retCode); } int CKMLogic::loadAppKey(UserData &handle, const ClientId &owner) diff --git a/src/manager/service/ckm-service.cpp b/src/manager/service/ckm-service.cpp index 3e32c2c..a558ead 100644 --- a/src/manager/service/ckm-service.cpp +++ b/src/manager/service/ckm-service.cpp @@ -205,7 +205,7 @@ RawBuffer CKMService::ProcessControl(MessageBuffer &buffer, bool allowed) if (!allowed) { LogError("Access denied!"); - return MessageBuffer::Serialize(CKM_API_ERROR_ACCESS_DENIED).Pop(); + return SerializeMessage(CKM_API_ERROR_ACCESS_DENIED); } return logicFunc(); diff --git a/src/manager/service/encryption-service.cpp b/src/manager/service/encryption-service.cpp index bc4aa2b..843c3f3 100644 --- a/src/manager/service/encryption-service.cpp +++ b/src/manager/service/encryption-service.cpp @@ -47,7 +47,7 @@ void EncryptionService::RespondToClient(const CryptoRequest &request, const RawBuffer &data) { try { - RawBuffer response = MessageBuffer::Serialize(request.msgId, retCode, data).Pop(); + RawBuffer response = SerializeMessage(request.msgId, retCode, data); m_serviceManager->Write(request.conn, response); } catch (...) { LogError("Failed to send response to the client"); diff --git a/src/manager/service/ocsp-logic.cpp b/src/manager/service/ocsp-logic.cpp index 9861dc0..051ba23 100644 --- a/src/manager/service/ocsp-logic.cpp +++ b/src/manager/service/ocsp-logic.cpp @@ -113,7 +113,7 @@ RawBuffer OCSPLogic::ocspCheck(int msgId, const RawBufferVector &rawChain, if (retCode == CKM_API_SUCCESS) ocspStatus = ocsp.verify(certChain); - return MessageBuffer::Serialize(msgId, retCode, ocspStatus).Pop(); + return SerializeMessage(msgId, retCode, ocspStatus); } } // namespace CKM diff --git a/unit-tests/test_data-type.cpp b/unit-tests/test_data-type.cpp index 0d2ec24..a237d6a 100644 --- a/unit-tests/test_data-type.cpp +++ b/unit-tests/test_data-type.cpp @@ -44,9 +44,9 @@ NEGATIVE_TEST_CASE(CONSTRUCTOR) NEGATIVE_TEST_CASE(SERIALIZATION) { MessageBuffer msg; - msg.Push(MessageBuffer::Serialize(static_cast(DataType::DB_FIRST - 1)).Pop()); + msg.Push(SerializeMessage(static_cast(DataType::DB_FIRST - 1))); BOOST_REQUIRE_THROW(DataType{msg}, Exc::InputParam); - msg.Push(MessageBuffer::Serialize(static_cast(DataType::DB_LAST + 1)).Pop()); + msg.Push(SerializeMessage(static_cast(DataType::DB_LAST + 1))); BOOST_REQUIRE_THROW(DataType{msg}, Exc::InputParam); } @@ -56,8 +56,7 @@ POSITIVE_TEST_CASE(SERIALIZATION) MessageBuffer msg; DataType dt; - BOOST_REQUIRE_NO_THROW(DataType(typeToCheck).Serialize(msg)); - msg.Push(msg.Pop()); + BOOST_REQUIRE_NO_THROW(msg.Push(SerializeMessage(DataType(typeToCheck)))); BOOST_REQUIRE_NO_THROW(dt = DataType{msg}); BOOST_REQUIRE(dt == typeToCheck); }; diff --git a/unit-tests/test_pkcs12.cpp b/unit-tests/test_pkcs12.cpp index bb7b3fe..4a68f3e 100644 --- a/unit-tests/test_pkcs12.cpp +++ b/unit-tests/test_pkcs12.cpp @@ -135,7 +135,7 @@ POSITIVE_TEST_CASE(pkcs12Serializable) auto checkPkcs = [&](const PKCS12Serializable& ps) { MessageBuffer msg; - msg.Push(MessageBuffer::Serialize(ps).Pop()); + msg.Push(SerializeMessage(ps)); PKCS12Serializable deserialized(msg); BOOST_REQUIRE(!deserialized.empty()); @@ -171,7 +171,7 @@ NEGATIVE_TEST_CASE(pkcs12Serializable) { auto checkEmptiness = [](const PKCS12Serializable& ps) { MessageBuffer msg; - msg.Push(MessageBuffer::Serialize(ps).Pop()); + msg.Push(SerializeMessage(ps)); PKCS12Serializable deserialized(msg); BOOST_REQUIRE(deserialized.empty()); }; diff --git a/unit-tests/test_serialization.cpp b/unit-tests/test_serialization.cpp index 8b7152e..b9b9eed 100644 --- a/unit-tests/test_serialization.cpp +++ b/unit-tests/test_serialization.cpp @@ -98,8 +98,7 @@ POSITIVE_TEST_CASE(Serialization_CryptoAlgorithm) CryptoAlgorithmSerializable input(ca); CryptoAlgorithmSerializable output; - auto msg = MessageBuffer::Serialize(input); - RawBuffer buffer = msg.Pop(); + RawBuffer buffer = SerializeMessage(input); MessageBuffer resp; resp.Push(buffer); resp.Deserialize(output); @@ -122,8 +121,7 @@ NEGATIVE_TEST_CASE(Serialization_CryptoAlgorithm) CryptoAlgorithmSerializable input(ca); CryptoAlgorithmSerializable output; - auto msg = MessageBuffer::Serialize(input); - RawBuffer buffer = msg.Pop(); + RawBuffer buffer = SerializeMessage(input); MessageBuffer resp; resp.Push(buffer); resp.Deserialize(output); @@ -153,8 +151,7 @@ NEGATIVE_TEST_CASE(Serialization_CryptoAlgorithm_wrong_name) CryptoAlgorithmSerializable input(ca); CryptoAlgorithmSerializable output; - auto msg = MessageBuffer::Serialize(input); - RawBuffer buffer = msg.Pop(); + RawBuffer buffer = SerializeMessage(input); MessageBuffer resp; resp.Push(buffer); BOOST_REQUIRE_THROW(resp.Deserialize(output), -- 2.7.4 From 39b2753ce5be83421d819cc0a60b9bda628291f7 Mon Sep 17 00:00:00 2001 From: Mateusz Cegielka Date: Mon, 10 Aug 2020 13:24:35 +0200 Subject: [PATCH 10/16] Optimize, fix memory zeroing and refactor BinaryQueue BinaryQueue is a class responsible for buffering data received from sockets before deserialization, vendored from DPL. It stores the received data as a list of blocks, which is probably the optimal approach given the constraints of the services framework here. However, its implementation is a little inefficient and incorrect: - Stores data in std::vector instead of RawBuffer. Because of that, any piece of data that passes through a socket may live in memory much longer than it should. - Erases elements from the front of a std::vector. This means all the other elements need to be shifted, which could even result in quadratic complexity given large enough socket reads and small enough messages. - Always copies incoming data. This means all of incoming traffic has to be copied one more time than it needs to. I have fixed the first issue in the obvious way. To fix the second issue, I have added a new member that tracks how many bytes have been read from the first bucket in the queue, which makes physically erasing elements from the vector unnecessary. Lastly, I changed the push signature from taking a pointer and a size to taking a RawBuffer&&, which eliminated some copies and made the remaining ones more explicit. Change-Id: I36932d5492815e38bf1cdab249327d26c9805ac6 --- misc/ckm_db_tool/db-wrapper.cpp | 3 +- src/manager/client-async/service.cpp | 5 +- src/manager/client/client-common.cpp | 5 +- src/manager/common/message-buffer.cpp | 6 +- src/manager/common/message-buffer.h | 4 +- src/manager/crypto/sw-backend/store.cpp | 4 +- src/manager/crypto/tz-backend/store.cpp | 4 +- src/manager/dpl/core/include/dpl/binary_queue.h | 68 +++++----------------- src/manager/dpl/core/src/binary_queue.cpp | 75 ++++++++++--------------- src/manager/main/thread-service.cpp | 4 +- src/manager/service/encryption-service.cpp | 2 +- unit-tests/test_binary-queue.cpp | 68 ++++++++++------------ unit-tests/test_serialization.cpp | 9 +-- 13 files changed, 95 insertions(+), 162 deletions(-) diff --git a/misc/ckm_db_tool/db-wrapper.cpp b/misc/ckm_db_tool/db-wrapper.cpp index 4f9498b..427fce1 100644 --- a/misc/ckm_db_tool/db-wrapper.cpp +++ b/misc/ckm_db_tool/db-wrapper.cpp @@ -59,9 +59,8 @@ int DbWrapper::unlock() return m_logic.unlockSystemDB(); int retCode; - RawBuffer ret = m_logic.unlockUserKey(m_uid, m_pw); MessageBuffer buff; - buff.Push(ret); + buff.Push(m_logic.unlockUserKey(m_uid, m_pw)); buff.Deserialize(retCode); return retCode; } diff --git a/src/manager/client-async/service.cpp b/src/manager/client-async/service.cpp index cd7cd19..ceeeeae 100644 --- a/src/manager/client-async/service.cpp +++ b/src/manager/client-async/service.cpp @@ -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. @@ -183,8 +183,7 @@ void Service::receiveData() if (!m_responseBuffer) m_responseBuffer.reset(new MessageBuffer()); - RawBuffer raw(buffer, buffer + temp); - m_responseBuffer->Push(raw); + m_responseBuffer->Push(RawBuffer(buffer, buffer + temp)); // parse while you can while (m_responseBuffer->Ready()) { diff --git a/src/manager/client/client-common.cpp b/src/manager/client/client-common.cpp index bb9ad4d..f440b95 100644 --- a/src/manager/client/client-common.cpp +++ b/src/manager/client/client-common.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2000 - 2014 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. @@ -308,8 +308,7 @@ int ServiceConnection::receive(CKM::MessageBuffer &recv_buf) break; } - CKM::RawBuffer raw(buffer, buffer + temp); - recv_buf.Push(raw); + recv_buf.Push(CKM::RawBuffer(buffer, buffer + temp)); } while (!recv_buf.Ready()); if (ec != CKM_API_SUCCESS) diff --git a/src/manager/common/message-buffer.cpp b/src/manager/common/message-buffer.cpp index ef1ceb2..237f716 100644 --- a/src/manager/common/message-buffer.cpp +++ b/src/manager/common/message-buffer.cpp @@ -30,9 +30,9 @@ namespace CKM { -void MessageBuffer::Push(const RawBuffer &data) +void MessageBuffer::Push(RawBuffer &&data) { - m_buffer.AppendCopy(&data[0], data.size()); + m_buffer.Push(std::move(data)); } bool MessageBuffer::Ready() @@ -58,7 +58,7 @@ void MessageBuffer::Read(size_t num, void *bytes) Throw(Exception::OutOfData); } - m_buffer.FlattenConsume(bytes, num); + m_buffer.Pop(bytes, num); m_bytesLeft -= num; } diff --git a/src/manager/common/message-buffer.h b/src/manager/common/message-buffer.h index 6f0572a..14e4c3f 100644 --- a/src/manager/common/message-buffer.h +++ b/src/manager/common/message-buffer.h @@ -47,7 +47,7 @@ public: MessageBuffer(MessageBuffer &&) = default; MessageBuffer &operator=(MessageBuffer &&) = default; - void Push(const RawBuffer &data); + void Push(RawBuffer &&data); bool Ready(); @@ -71,7 +71,7 @@ protected: if (m_buffer.Size() < sizeof(size_t)) return; // we cannot count m_bytesLeft because buffer is too small - m_buffer.FlattenConsume(&m_bytesLeft, sizeof(size_t)); + m_buffer.Pop(&m_bytesLeft, sizeof(size_t)); } size_t m_bytesLeft; diff --git a/src/manager/crypto/sw-backend/store.cpp b/src/manager/crypto/sw-backend/store.cpp index 08891ed..5c846ed 100644 --- a/src/manager/crypto/sw-backend/store.cpp +++ b/src/manager/crypto/sw-backend/store.cpp @@ -75,7 +75,7 @@ RawBuffer passwordToKey(const Password &password, const RawBuffer &salt, RawBuffer unpack(const RawBuffer &packed, const Password &pass) { MessageBuffer buffer; - buffer.Push(packed); + buffer.Push(RawBuffer(packed)); int encryptionScheme = 0; RawBuffer data; buffer.Deserialize(encryptionScheme, data); @@ -84,7 +84,7 @@ RawBuffer unpack(const RawBuffer &packed, const Password &pass) return data; MessageBuffer internalBuffer; - internalBuffer.Push(data); + internalBuffer.Push(std::move(data)); RawBuffer encrypted; RawBuffer iv; RawBuffer tag; diff --git a/src/manager/crypto/tz-backend/store.cpp b/src/manager/crypto/tz-backend/store.cpp index 3183937..ba7de98 100644 --- a/src/manager/crypto/tz-backend/store.cpp +++ b/src/manager/crypto/tz-backend/store.cpp @@ -47,7 +47,7 @@ void unpack(const RawBuffer &packed, RawBuffer &tag) { MessageBuffer buffer; - buffer.Push(packed); + buffer.Push(RawBuffer(packed)); buffer.Deserialize(scheme); @@ -61,7 +61,7 @@ void unpack(const RawBuffer &packed, RawBuffer unpackData(const RawBuffer &packed) { MessageBuffer buffer; - buffer.Push(packed); + buffer.Push(RawBuffer(packed)); int scheme; buffer.Deserialize(scheme); diff --git a/src/manager/dpl/core/include/dpl/binary_queue.h b/src/manager/dpl/core/include/dpl/binary_queue.h index 404a80c..99a03ef 100644 --- a/src/manager/dpl/core/include/dpl/binary_queue.h +++ b/src/manager/dpl/core/include/dpl/binary_queue.h @@ -19,19 +19,18 @@ * @version 1.0 * @brief This file is the header file of binary queue */ -#ifndef CENT_KEY_BINARY_QUEUE_H -#define CENT_KEY_BINARY_QUEUE_H +#pragma once + +#include #include -#include -#include #include +#include +#include + namespace CKM { -/** - * Binary stream implemented as constant size bucket list - */ class COMMON_API BinaryQueue final { public: class Exception { @@ -40,59 +39,22 @@ public: DECLARE_EXCEPTION_TYPE(Base, OutOfData) }; -private: - typedef std::vector Bucket; - - typedef std::list BucketList; - BucketList m_buckets; - size_t m_size; - -public: - /** - * Construct empty binary queue - */ BinaryQueue(); - BinaryQueue(const BinaryQueue &other) = delete; - - /** - * Construct binary queue by moving data from other binary queue - * - * @param[in] other Other binary queue to move from - */ BinaryQueue(BinaryQueue &&) = default; - /** - * Append copy of @a bufferSize bytes from memory pointed by @a buffer - * to the end of binary queue. Uses default deleter based on free. - * - * @return none - * @param[in] buffer Pointer to buffer to copy data from - * @param[in] bufferSize Number of bytes to copy - * @exception std::bad_alloc Cannot allocate memory to hold additional data - */ - void AppendCopy(const void *buffer, size_t bufferSize); + void Push(RawBuffer&& buffer); + + void Pop(void* buffer, size_t bufferSize); - /** - * Retrieve total size of all data contained in binary queue - * - * @return Number of bytes in binary queue - */ size_t Size() const; - /** - * Retrieve @a bufferSize bytes from beginning of binary queue, copy them - * to user supplied buffer, and remove from binary queue - * - * @return none - * @param[in] buffer Pointer to user buffer to receive bytes - * @param[in] bufferSize Size of user buffer pointed by @a buffer - * @exception BinaryQueue::Exception::OutOfData Number of bytes to flatten - * is larger than available bytes in binary queue - */ - void FlattenConsume(void *buffer, size_t bufferSize); +private: + // TODO: The default choice is std::deque which is generally faster, but in this use case it's + // very likely that this queue will only ever contain 1 element, and std::deque may be slower, + // so I have kept this like before. The optimal approach would be to use deque+SSO, probably. + std::queue> m_buckets; + size_t m_size, m_offset; }; } // namespace CKM - -#endif // CENT_KEY_BINARY_QUEUE_H diff --git a/src/manager/dpl/core/src/binary_queue.cpp b/src/manager/dpl/core/src/binary_queue.cpp index ac8db27..ee6606d 100644 --- a/src/manager/dpl/core/src/binary_queue.cpp +++ b/src/manager/dpl/core/src/binary_queue.cpp @@ -19,72 +19,59 @@ * @version 1.0 * @brief This file is the implementation file of binary queue */ + #include -#include + #include +#include #include namespace CKM { -BinaryQueue::BinaryQueue() : - m_size(0) -{} -void BinaryQueue::AppendCopy(const void *buffer, size_t bufferSize) +BinaryQueue::BinaryQueue() + : m_size(0) + , m_offset(0) { - if (bufferSize == 0) - return; - - assert(buffer != NULL); - - m_buckets.emplace_back(bufferSize); - memcpy(m_buckets.back().data(), buffer, bufferSize); - m_size += bufferSize; } -size_t BinaryQueue::Size() const +void BinaryQueue::Push(RawBuffer&& buffer) { - return m_size; + if (buffer.empty()) + return; + + m_size += buffer.size(); + m_buckets.push(std::move(buffer)); } -void BinaryQueue::FlattenConsume(void *buffer, size_t bufferSize) +void BinaryQueue::Pop(void* buffer, size_t bufferSize) { - // Check parameters if (bufferSize == 0) return; - - assert(buffer != NULL); - if (bufferSize > m_size) Throw(Exception::OutOfData); - size_t bytesLeft = bufferSize; - void *ptr = buffer; - assert(!m_buckets.empty()); - - // Flatten data - do { - auto& bucket = m_buckets.front(); + assert(buffer != NULL); - // Get consume size - size_t bucketSize = bucket.size(); - size_t count = std::min(bytesLeft, bucketSize); + while (bufferSize > 0) { + RawBuffer& bucket = m_buckets.front(); + size_t toPop = std::min(bucket.size() - m_offset, bufferSize); - // Copy data to user pointer - memcpy(ptr, bucket.data(), count); + memcpy(buffer, bucket.data() + m_offset, toPop); + buffer = static_cast(buffer) + toPop; + bufferSize -= toPop; + m_size -= toPop; - // consume - if (count == bucketSize) { - m_buckets.pop_front(); - } else { - bucket.erase(bucket.begin(), bucket.begin() + count); - assert(count == bytesLeft); - break; - } + if (toPop == bucket.size() - m_offset) { + m_buckets.pop(); + m_offset = 0; + } else + m_offset += toPop; + } +} - bytesLeft -= count; - ptr = static_cast(ptr) + count; - } while (bytesLeft); - m_size -= bufferSize; +size_t BinaryQueue::Size() const +{ + return m_size; } } // namespace CKM diff --git a/src/manager/main/thread-service.cpp b/src/manager/main/thread-service.cpp index ef51289..e89efd3 100644 --- a/src/manager/main/thread-service.cpp +++ b/src/manager/main/thread-service.cpp @@ -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. @@ -46,7 +46,7 @@ void ThreadService::Handle(const WriteEvent &) void ThreadService::Handle(const ReadEvent &event) { auto &info = m_connectionInfoMap[event.connectionID.counter]; - info.buffer.Push(event.rawBuffer); + info.buffer.Push(RawBuffer(event.rawBuffer)); if (!info.buffer.Ready()) return; diff --git a/src/manager/service/encryption-service.cpp b/src/manager/service/encryption-service.cpp index 843c3f3..96e7f14 100644 --- a/src/manager/service/encryption-service.cpp +++ b/src/manager/service/encryption-service.cpp @@ -142,7 +142,7 @@ void EncryptionService::ProcessEncryption(const ConnectionID &conn, void EncryptionService::CustomHandle(const ReadEvent &event) { auto &info = m_connectionInfoMap[event.connectionID.counter]; - info.buffer.Push(event.rawBuffer); + info.buffer.Push(RawBuffer(event.rawBuffer)); while (ProcessOne(event.connectionID, info, true)); } diff --git a/unit-tests/test_binary-queue.cpp b/unit-tests/test_binary-queue.cpp index 779eab4..7a4b23f 100644 --- a/unit-tests/test_binary-queue.cpp +++ b/unit-tests/test_binary-queue.cpp @@ -22,79 +22,69 @@ using namespace CKM; namespace { -constexpr unsigned char buf[] = {0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07}; -constexpr size_t bufSize = sizeof(buf); +const RawBuffer BUF = {0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07}; } // namespace anonymous BOOST_AUTO_TEST_SUITE(BINARY_QUEUE_TEST) -POSITIVE_TEST_CASE(append_copy) +POSITIVE_TEST_CASE(push) { BinaryQueue bq; BOOST_REQUIRE(bq.Size() == 0); - bq.AppendCopy(buf, bufSize); + bq.Push(RawBuffer(BUF)); + BOOST_REQUIRE(bq.Size() == BUF.size()); - BOOST_REQUIRE(bq.Size() == bufSize); - - bq.AppendCopy(buf, bufSize); - - BOOST_REQUIRE(bq.Size() == 2 * bufSize); + bq.Push(RawBuffer(BUF)); + BOOST_REQUIRE(bq.Size() == 2 * BUF.size()); } -NEGATIVE_TEST_CASE(append_copy) +NEGATIVE_TEST_CASE(push) { BinaryQueue bq; - bq.AppendCopy(buf, 0); - + bq.Push(RawBuffer()); BOOST_REQUIRE(bq.Size() == 0); } -POSITIVE_TEST_CASE(flatten_consume) +POSITIVE_TEST_CASE(pop) { - BinaryQueue bq; + const size_t PART1_SIZE = BUF.size() / 2; - constexpr size_t part1Size = bufSize / 2; - - bq.AppendCopy(buf, part1Size); - bq.AppendCopy(buf + part1Size, bufSize - part1Size); - - char out[3 * bufSize / 4]; - bq.FlattenConsume(out, sizeof(out)); - BOOST_REQUIRE(memcmp(buf, out, sizeof(out)) == 0); + BinaryQueue bq; - constexpr size_t remainingSize = bufSize - sizeof(out); - BOOST_REQUIRE(bq.Size() == remainingSize); + bq.Push(RawBuffer(BUF.begin(), BUF.begin() + PART1_SIZE)); + bq.Push(RawBuffer(BUF.begin() + PART1_SIZE, BUF.end())); - bq.FlattenConsume(out, remainingSize); + RawBuffer out(3 * BUF.size() / 4); + bq.Pop(out.data(), out.size()); + BOOST_REQUIRE(std::equal(out.begin(), out.end(), BUF.begin())); - BOOST_REQUIRE(memcmp(buf + sizeof(out), out, remainingSize) == 0); - BOOST_REQUIRE(memcmp(buf + remainingSize, - out + remainingSize, - sizeof(out) - remainingSize) == 0); + const size_t REST_SIZE = BUF.size() - out.size(); + BOOST_REQUIRE(bq.Size() == REST_SIZE); + bq.Pop(out.data(), REST_SIZE); + BOOST_REQUIRE(std::equal(out.begin(), out.begin() + REST_SIZE, BUF.begin() + out.size())); + BOOST_REQUIRE(std::equal(out.begin() + REST_SIZE, out.end(), BUF.begin() + REST_SIZE)); BOOST_REQUIRE(bq.Size() == 0); } -NEGATIVE_TEST_CASE(flatten_consume) +NEGATIVE_TEST_CASE(pop) { BinaryQueue bq; char out; - BOOST_REQUIRE_THROW(bq.FlattenConsume(&out, sizeof(out)), - BinaryQueue::Exception::OutOfData); + BOOST_REQUIRE_THROW(bq.Pop(&out, sizeof(out)), BinaryQueue::Exception::OutOfData); - bq.AppendCopy(buf, bufSize); + bq.Push(RawBuffer(BUF)); - bq.FlattenConsume(&out, 0); - BOOST_REQUIRE(bq.Size() == bufSize); + bq.Pop(&out, 0); + BOOST_REQUIRE(bq.Size() == BUF.size()); - char out2[bufSize + 1]; - BOOST_REQUIRE_THROW(bq.FlattenConsume(out2, sizeof(out2)), - BinaryQueue::Exception::OutOfData); - BOOST_REQUIRE(bq.Size() == bufSize); + RawBuffer out2(BUF.size() + 1); + BOOST_REQUIRE_THROW(bq.Pop(out2.data(), out2.size()), BinaryQueue::Exception::OutOfData); + BOOST_REQUIRE(bq.Size() == BUF.size()); } BOOST_AUTO_TEST_SUITE_END() diff --git a/unit-tests/test_serialization.cpp b/unit-tests/test_serialization.cpp index b9b9eed..c825acd 100644 --- a/unit-tests/test_serialization.cpp +++ b/unit-tests/test_serialization.cpp @@ -98,9 +98,8 @@ POSITIVE_TEST_CASE(Serialization_CryptoAlgorithm) CryptoAlgorithmSerializable input(ca); CryptoAlgorithmSerializable output; - RawBuffer buffer = SerializeMessage(input); MessageBuffer resp; - resp.Push(buffer); + resp.Push(SerializeMessage(input)); resp.Deserialize(output); checkIntParam(output, ParamName::ALGO_TYPE, @@ -121,9 +120,8 @@ NEGATIVE_TEST_CASE(Serialization_CryptoAlgorithm) CryptoAlgorithmSerializable input(ca); CryptoAlgorithmSerializable output; - RawBuffer buffer = SerializeMessage(input); MessageBuffer resp; - resp.Push(buffer); + resp.Push(SerializeMessage(input)); resp.Deserialize(output); // wrong type @@ -151,9 +149,8 @@ NEGATIVE_TEST_CASE(Serialization_CryptoAlgorithm_wrong_name) CryptoAlgorithmSerializable input(ca); CryptoAlgorithmSerializable output; - RawBuffer buffer = SerializeMessage(input); MessageBuffer resp; - resp.Push(buffer); + resp.Push(SerializeMessage(input)); BOOST_REQUIRE_THROW(resp.Deserialize(output), CryptoAlgorithmSerializable::UnsupportedParam); } -- 2.7.4 From 198def3c4d5e0cb8ad1e85fab43caf557dab9d10 Mon Sep 17 00:00:00 2001 From: Mateusz Cegielka Date: Fri, 31 Jul 2020 13:12:52 +0200 Subject: [PATCH 11/16] Refactor base64 implementation This codebase contains two slightly different base64 encoding wrappers, both using low-level OpenSSL BIO API. The wrappers provide access to a streaming interface, despite the fact that this property is not used anywhere. To handle errors, the wrappers sometimes use exceptions and sometimes return codes. To implement this, a stateful class was used, and these four facts resulted in needlessly verbose code. I have merged the two implementations and simplified them to two free functions. The encode function now uses higher-level OpenSSL EVP API, and the decode function was refactored. Change-Id: I5016723158321d0c1aa10810aa9067cd2249f38e --- common/base64_generic.cpp | 66 +++++++ common/base64_generic.h | 61 +++++++ misc/ckm_db_tool/CMakeLists.txt | 1 + misc/ckm_db_tool/db-wrapper.cpp | 11 +- misc/ckm_initial_values/CMakeLists.txt | 4 +- misc/ckm_initial_values/base64.cpp | 205 ---------------------- misc/ckm_initial_values/base64.h | 65 ------- misc/ckm_initial_values/main.cpp | 10 +- src/CMakeLists.txt | 1 + src/manager/CMakeLists.txt | 2 +- src/manager/common/base64.cpp | 208 ----------------------- src/manager/common/base64.h | 68 +++----- src/manager/common/certificate-impl.cpp | 15 +- src/manager/initial-values/BufferHandler.cpp | 38 +++-- src/manager/initial-values/InitialValuesFile.cpp | 15 +- src/manager/service/crypto-logic.cpp | 22 +-- unit-tests/CMakeLists.txt | 2 +- unit-tests/test_base64.cpp | 55 +----- unit-tests/test_crypto-logic.cpp | 13 +- 19 files changed, 211 insertions(+), 651 deletions(-) create mode 100644 common/base64_generic.cpp create mode 100644 common/base64_generic.h delete mode 100644 misc/ckm_initial_values/base64.cpp delete mode 100644 misc/ckm_initial_values/base64.h delete mode 100644 src/manager/common/base64.cpp diff --git a/common/base64_generic.cpp b/common/base64_generic.cpp new file mode 100644 index 0000000..5787904 --- /dev/null +++ b/common/base64_generic.cpp @@ -0,0 +1,66 @@ +/* + * Copyright (c) 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. + * 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 + +#include +#include +#include +#include + +#include +#include + +namespace { + +bool isBase64Char(char c) +{ + return isalnum(c) || c == '+' || c == '/' || c == '='; +} + +std::unique_ptr makeBioPtr(BIO* ptr) +{ + if (!ptr) + throw std::bad_alloc(); + return {ptr, &BIO_free}; +} + +} // anonymous namespace + +int base64EncodeGenericImpl(const void* input, size_t inputLen, void* output) +{ + return EVP_EncodeBlock(reinterpret_cast(output), + reinterpret_cast(input), inputLen); +} + +int base64DecodeGenericImpl(const void* input, size_t inputLen, void* output, size_t outputLen) +{ + if (inputLen == 0) + return 0; + if (!std::all_of(static_cast(input), static_cast(input) + inputLen, isBase64Char)) + throw std::invalid_argument("decoding base64 failed, unexpected characters"); + + auto inputBio = makeBioPtr(BIO_new_mem_buf(input, inputLen)); + auto base64Bio = makeBioPtr(BIO_new(BIO_f_base64())); + BIO_set_flags(base64Bio.get(), BIO_FLAGS_BASE64_NO_NL); + BIO_push(base64Bio.get(), inputBio.get()); + + int length = BIO_read(base64Bio.get(), output, outputLen); + if (length < 0) + throw std::invalid_argument("decoding base64 failed, openssl bio api error"); + + return length; +} diff --git a/common/base64_generic.h b/common/base64_generic.h new file mode 100644 index 0000000..6f890b8 --- /dev/null +++ b/common/base64_generic.h @@ -0,0 +1,61 @@ +/* + * Copyright (c) 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. + * 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. + */ + +#pragma once + +#include + +#include + +COMMON_API +int base64EncodeGenericImpl(const void* input, size_t inputLen, void* output); + +COMMON_API +int base64DecodeGenericImpl(const void* input, size_t inputLen, void* output, size_t outputLen); + +template +Output base64EncodeGeneric(const Input& input) +{ + // For each 3 input bytes (rounded up), 4 encoded bytes and a null terminator. + Output output((input.size() + 2) / 3 * 4 + 1, '\0'); + + static_assert(sizeof(input[0]) == 1); + static_assert(sizeof(output[0]) == 1); + int length = base64EncodeGenericImpl(input.data(), input.size(), &output[0]); + + output.resize(length); + return output; +} + +// Due to backwards compatibility, this function contains a few quirks. All '\n' newlines will be +// removed, other non-base64 characters will throw an exception, and base64 must be implemented +// using the OpenSSL BIO API. The last fact also implies that this function can accept padding in +// the middle of text, or too much padding at the end. +template +Output base64DecodeGeneric(Input input) +{ + input.erase(std::remove(input.begin(), input.end(), '\n'), input.end()); + + // For each 4 encoded bytes (rounded up), 3 decoded bytes. + Output output((input.size() + 3) / 4 * 3, '\0'); + + static_assert(sizeof(input[0]) == 1); + static_assert(sizeof(output[0]) == 1); + size_t length = base64DecodeGenericImpl(input.data(), input.size(), &output[0], output.size()); + + output.resize(length); + return output; +} diff --git a/misc/ckm_db_tool/CMakeLists.txt b/misc/ckm_db_tool/CMakeLists.txt index c4d9c15..a8ccca7 100644 --- a/misc/ckm_db_tool/CMakeLists.txt +++ b/misc/ckm_db_tool/CMakeLists.txt @@ -7,6 +7,7 @@ FIND_PACKAGE(Threads REQUIRED) INCLUDE_DIRECTORIES( ${KEY_MANAGER_DEP_INCLUDE_DIRS} ${CMAKE_CURRENT_SOURCE_DIR} + ${PROJECT_SOURCE_DIR}/common ${KEY_MANAGER_PATH}/main ${KEY_MANAGER_PATH}/common ${KEY_MANAGER_PATH}/service diff --git a/misc/ckm_db_tool/db-wrapper.cpp b/misc/ckm_db_tool/db-wrapper.cpp index 427fce1..b64a27c 100644 --- a/misc/ckm_db_tool/db-wrapper.cpp +++ b/misc/ckm_db_tool/db-wrapper.cpp @@ -133,15 +133,8 @@ void DbWrapper::displayRow(const DB::SqlConnection::Output::Row &row, bool trim, */ if (std::any_of(col.begin(), col.end(), - [](const char c){ return isprint(c) + iscntrl(c) == 0; })) { - Base64Encoder b64enc; - RawBuffer tmp(col.begin(), col.end()); - b64enc.append(tmp); - b64enc.finalize(); - col = "b64:"; - RawBuffer encoded = b64enc.get(); - std::copy(encoded.begin(), encoded.end(), std::back_inserter(col)); - } + [](const char c){ return isprint(c) + iscntrl(c) == 0; })) + col = "b64:" + CKM::base64Encode(col); if (trim && col.size() > MAX_LEN) { col.resize(MAX_LEN); diff --git a/misc/ckm_initial_values/CMakeLists.txt b/misc/ckm_initial_values/CMakeLists.txt index 4d2b792..4673a18 100644 --- a/misc/ckm_initial_values/CMakeLists.txt +++ b/misc/ckm_initial_values/CMakeLists.txt @@ -21,11 +21,13 @@ PKG_CHECK_MODULES(CKM_INITIAL_VALUES_DEP INCLUDE_DIRECTORIES( ${CKM_INITIAL_VALUES_DEP_INCLUDE_DIRS} + ${PROJECT_SOURCE_DIR}/common + ${PROJECT_SOURCE_DIR}/src/manager/common ) SET(CKM_INITIAL_VALUES_SOURCES + ${PROJECT_SOURCE_DIR}/common/base64_generic.cpp ${CMAKE_CURRENT_SOURCE_DIR}/main.cpp - ${CMAKE_CURRENT_SOURCE_DIR}/base64.cpp ) LINK_DIRECTORIES(${CKM_INITIAL_VALUES_DEP_LIBRARY_DIRS}) diff --git a/misc/ckm_initial_values/base64.cpp b/misc/ckm_initial_values/base64.cpp deleted file mode 100644 index 1c0d497..0000000 --- a/misc/ckm_initial_values/base64.cpp +++ /dev/null @@ -1,205 +0,0 @@ -/* - * Copyright (c) 2011 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. - */ -#include "base64.h" - -#include -#include - -#include -#include -#include -#include - - -namespace CKM { - -Base64Encoder::Base64Encoder() : - m_b64(0), - m_bmem(0), - m_finalized(false) -{ -} - -void Base64Encoder::append(const RawBuffer &data) -{ - if (m_finalized) { - throw std::logic_error("Already finalized"); - } - - if (!m_b64) - reset(); - - BIO_write(m_b64, data.data(), data.size()); -} - -void Base64Encoder::finalize() -{ - if (m_finalized) { - throw std::logic_error("Already finalized."); - } - - m_finalized = true; - (void)BIO_flush(m_b64); -} - -RawBuffer Base64Encoder::get() -{ - if (!m_finalized) { - throw std::logic_error("Not finalized"); - } - - BUF_MEM *bptr = nullptr; - BIO_get_mem_ptr(m_b64, &bptr); - - if (!bptr) { - throw std::logic_error("Bio internal error"); - } - - if (bptr->length > 0) - return RawBuffer(bptr->data, bptr->data + bptr->length); - - return RawBuffer(); -} - -void Base64Encoder::reset() -{ - m_finalized = false; - BIO_free_all(m_b64); - m_b64 = BIO_new(BIO_f_base64()); - m_bmem = BIO_new(BIO_s_mem()); - - if (!m_b64 || !m_bmem) { - throw std::logic_error("Error during allocation memory in BIO"); - } - - BIO_set_flags(m_b64, BIO_FLAGS_BASE64_NO_NL); - m_b64 = BIO_push(m_b64, m_bmem); -} - -Base64Encoder::~Base64Encoder() -{ - BIO_free_all(m_b64); -} - -Base64Decoder::Base64Decoder() : - m_finalized(false) -{ -} - -void Base64Decoder::append(const RawBuffer &data) -{ - if (m_finalized) { - throw std::logic_error("Already finalized."); - } - - std::copy(data.begin(), data.end(), std::back_inserter(m_input)); -} - -static bool whiteCharacter(char a) -{ - return a == '\n'; -} - -bool Base64Decoder::finalize() -{ - if (m_finalized) { - throw std::logic_error("Already finalized."); - } - - m_finalized = true; - - m_input.erase(std::remove_if(m_input.begin(), - m_input.end(), - whiteCharacter), - m_input.end()); - - for (size_t i = 0; i < m_input.size(); ++i) { - if (isalnum(m_input[i]) - || m_input[i] == '+' - || m_input[i] == '/' - || m_input[i] == '=') - continue; - - return false; - } - - BIO *b64, *bmem; - size_t len = m_input.size(); - - RawBuffer buffer(len); - - if (!buffer.data()) { - throw std::logic_error("Error in malloc."); - } - - memset(buffer.data(), 0, buffer.size()); - b64 = BIO_new(BIO_f_base64()); - - if (!b64) { - throw std::logic_error("Couldn't create BIO object."); - } - - BIO_set_flags(b64, BIO_FLAGS_BASE64_NO_NL); - RawBuffer tmp(m_input); - m_input.clear(); - - bmem = BIO_new_mem_buf(tmp.data(), len); - - if (!bmem) { - BIO_free(b64); - throw std::logic_error("Internal error in BIO"); - } - - bmem = BIO_push(b64, bmem); - - if (!bmem) { - BIO_free(b64); - throw std::logic_error("Internal error in BIO"); - } - - int readlen = BIO_read(bmem, buffer.data(), buffer.size()); - m_output.clear(); - - bool status = true; - - if (readlen > 0) { - buffer.resize(readlen); - m_output = std::move(buffer); - } else { - status = false; - } - - BIO_free_all(bmem); - return status; -} - -RawBuffer Base64Decoder::get() const -{ - if (!m_finalized) { - throw std::logic_error("Not finalized"); - } - - return m_output; -} - -void Base64Decoder::reset() -{ - m_finalized = false; - m_input.clear(); - m_output.clear(); -} - -} // namespace CKM diff --git a/misc/ckm_initial_values/base64.h b/misc/ckm_initial_values/base64.h deleted file mode 100644 index c9085bf..0000000 --- a/misc/ckm_initial_values/base64.h +++ /dev/null @@ -1,65 +0,0 @@ -/* - * Copyright (c) 2011 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. - */ -#ifndef _BASE64_H_ -#define _BASE64_H_ - -#include -#include - -typedef std::vector RawBuffer; - -struct bio_st; -typedef bio_st BIO; - -namespace CKM { - -class Base64Encoder { -public: - Base64Encoder(); - void append(const RawBuffer &data); - void finalize(); - RawBuffer get(); - void reset(); - ~Base64Encoder(); - -private: - BIO *m_b64; - BIO *m_bmem; - bool m_finalized; -}; - -class Base64Decoder { -public: - Base64Decoder(); - void append(const RawBuffer &data); - - /* - * Function will return false when BIO_read fails - * (for example: when string was not in base64 format). - */ - bool finalize(); - RawBuffer get() const; - void reset(); - ~Base64Decoder() {} - -private: - RawBuffer m_input; - RawBuffer m_output; - bool m_finalized; -}; -} // namespace CKM - -#endif diff --git a/misc/ckm_initial_values/main.cpp b/misc/ckm_initial_values/main.cpp index c5d9e7d..eccb805 100644 --- a/misc/ckm_initial_values/main.cpp +++ b/misc/ckm_initial_values/main.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2018-2019 Samsung Electronics Co., Ltd. All rights reserved + * Copyright (c) 2018-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. @@ -41,7 +41,7 @@ #include #include -#include "base64.h" +#include typedef std::vector Buffer; typedef std::istreambuf_iterator InputIterator; @@ -89,11 +89,7 @@ const std::unordered_map FORMAT = { std::string base64(const Buffer& data) { try { - CKM::Base64Encoder encoder; - encoder.append(data); - encoder.finalize(); - auto result = encoder.get(); - return std::string(result.begin(), result.end()); + return base64EncodeGeneric(data); } catch (std::exception &e) { std::cerr << "Error: " << e.what() << std::endl; } diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 88f8a37..2c06a86 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -65,6 +65,7 @@ INCLUDE_DIRECTORIES(SYSTEM INCLUDE_DIRECTORIES( ${KEY_MANAGER_SRC_PATH}/include + ${PROJECT_SOURCE_DIR}/common ${KEY_MANAGER_PATH}/main ${KEY_MANAGER_PATH}/common ${KEY_MANAGER_PATH}/service diff --git a/src/manager/CMakeLists.txt b/src/manager/CMakeLists.txt index cbf39b3..c614ab4 100644 --- a/src/manager/CMakeLists.txt +++ b/src/manager/CMakeLists.txt @@ -12,8 +12,8 @@ SET(KEY_MANAGER_COMMON_VERSION ${KEY_MANAGER_COMMON_VERSION_MAJOR}.0.1) SET(COMMON_PATH ${PROJECT_SOURCE_DIR}/src/manager) SET(COMMON_SOURCES + ${PROJECT_SOURCE_DIR}/common/base64_generic.cpp ${COMMON_PATH}/common/algo-param.cpp - ${COMMON_PATH}/common/base64.cpp ${COMMON_PATH}/common/data-type.cpp ${COMMON_PATH}/common/openssl-error-handler.cpp ${COMMON_PATH}/common/exception.cpp diff --git a/src/manager/common/base64.cpp b/src/manager/common/base64.cpp deleted file mode 100644 index 091ddfd..0000000 --- a/src/manager/common/base64.cpp +++ /dev/null @@ -1,208 +0,0 @@ -/* - * Copyright (c) 2011 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. - */ -#include -#include - -#include -#include -#include -#include - -#include - -#include -#include - -namespace CKM { - -Base64Encoder::Base64Encoder() : - m_b64(0), - m_bmem(0), - m_finalized(false) -{ -} - -void Base64Encoder::append(const RawBuffer &data) -{ - if (m_finalized) { - ThrowErr(Exc::InternalError, "Already finalized"); - } - - if (!m_b64) - reset(); - - BIO_write(m_b64, data.data(), data.size()); -} - -void Base64Encoder::finalize() -{ - if (m_finalized) { - ThrowErr(Exc::InternalError, "Already finalized."); - } - - m_finalized = true; - (void)BIO_flush(m_b64); -} - -RawBuffer Base64Encoder::get() -{ - if (!m_finalized) { - ThrowErr(Exc::InternalError, "Not finalized"); - } - - BUF_MEM *bptr = nullptr; - BIO_get_mem_ptr(m_b64, &bptr); - - if (!bptr) { - ThrowErr(Exc::InternalError, "Bio internal error"); - } - - if (bptr->length > 0) - return RawBuffer(bptr->data, bptr->data + bptr->length); - - return RawBuffer(); -} - -void Base64Encoder::reset() -{ - m_finalized = false; - BIO_free_all(m_b64); - m_b64 = BIO_new(BIO_f_base64()); - m_bmem = BIO_new(BIO_s_mem()); - - if (!m_b64 || !m_bmem) { - ThrowErr(Exc::InternalError, "Error during allocation memory in BIO"); - } - - BIO_set_flags(m_b64, BIO_FLAGS_BASE64_NO_NL); - m_b64 = BIO_push(m_b64, m_bmem); -} - -Base64Encoder::~Base64Encoder() -{ - BIO_free_all(m_b64); -} - -Base64Decoder::Base64Decoder() : - m_finalized(false) -{ -} - -void Base64Decoder::append(const RawBuffer &data) -{ - if (m_finalized) { - ThrowErr(Exc::InternalError, "Already finalized."); - } - - std::copy(data.begin(), data.end(), std::back_inserter(m_input)); -} - -static bool whiteCharacter(char a) -{ - return a == '\n'; -} - -bool Base64Decoder::finalize() -{ - if (m_finalized) { - ThrowErr(Exc::InternalError, "Already finalized."); - } - - m_finalized = true; - - m_input.erase(std::remove_if(m_input.begin(), - m_input.end(), - whiteCharacter), - m_input.end()); - - for (size_t i = 0; i < m_input.size(); ++i) { - if (isalnum(m_input[i]) - || m_input[i] == '+' - || m_input[i] == '/' - || m_input[i] == '=') - continue; - - LogError("Base64 input contains illegal chars: " << m_input[i]); - return false; - } - - BIO *b64, *bmem; - size_t len = m_input.size(); - - RawBuffer buffer(len); - - if (!buffer.data()) { - ThrowErr(Exc::InternalError, "Error in malloc."); - } - - memset(buffer.data(), 0, buffer.size()); - b64 = BIO_new(BIO_f_base64()); - - if (!b64) { - ThrowErr(Exc::InternalError, "Couldn't create BIO object."); - } - - BIO_set_flags(b64, BIO_FLAGS_BASE64_NO_NL); - RawBuffer tmp(m_input); - m_input.clear(); - - bmem = BIO_new_mem_buf(tmp.data(), len); - - if (!bmem) { - BIO_free(b64); - ThrowErr(Exc::InternalError, "Internal error in BIO"); - } - - bmem = BIO_push(b64, bmem); - - if (!bmem) { - BIO_free(b64); - ThrowErr(Exc::InternalError, "Internal error in BIO"); - } - - int readlen = BIO_read(bmem, buffer.data(), buffer.size()); - m_output.clear(); - - bool status = true; - - if (readlen > 0) { - buffer.resize(readlen); - m_output = std::move(buffer); - } else { - status = false; - } - - BIO_free_all(bmem); - return status; -} - -RawBuffer Base64Decoder::get() const -{ - if (!m_finalized) { - ThrowErr(Exc::InternalError, "Not finalized"); - } - - return m_output; -} - -void Base64Decoder::reset() -{ - m_finalized = false; - m_input.clear(); - m_output.clear(); -} - -} // namespace CKM diff --git a/src/manager/common/base64.h b/src/manager/common/base64.h index f73266e..a028661 100644 --- a/src/manager/common/base64.h +++ b/src/manager/common/base64.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2011 Samsung Electronics Co., Ltd All Rights Reserved + * Copyright (c) 2011 - 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. @@ -13,58 +13,30 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -#ifndef _BASE64_H_ -#define _BASE64_H_ -#include +#pragma once -#include -#include -#include +#include +#include -struct bio_st; -typedef bio_st BIO; +#include namespace CKM { -class COMMON_API Base64Encoder { -public: - NONCOPYABLE(Base64Encoder) +template +Output base64Encode(const Input& input) +{ + return base64EncodeGeneric(input); +} + +template +Output base64Decode(const Input& input) +{ + try { + return base64DecodeGeneric(input); + } catch (const std::invalid_argument& e) { + ThrowErr(Exc::InternalError, e.what()); + } +} - Base64Encoder(); - void append(const RawBuffer &data); - void finalize(); - RawBuffer get(); - void reset(); - ~Base64Encoder(); - -private: - BIO *m_b64; - BIO *m_bmem; - bool m_finalized; -}; - -class COMMON_API Base64Decoder { -public: - NONCOPYABLE(Base64Decoder) - - Base64Decoder(); - void append(const RawBuffer &data); - - /* - * Function will return false when BIO_read fails - * (for example: when string was not in base64 format). - */ - bool finalize(); - RawBuffer get() const; - void reset(); - ~Base64Decoder() {} - -private: - RawBuffer m_input; - RawBuffer m_output; - bool m_finalized; -}; } // namespace CKM - -#endif diff --git a/src/manager/common/certificate-impl.cpp b/src/manager/common/certificate-impl.cpp index f81d44a..71b02eb 100644 --- a/src/manager/common/certificate-impl.cpp +++ b/src/manager/common/certificate-impl.cpp @@ -19,6 +19,7 @@ * @brief Certificate implementation. */ #include +#include #include #include @@ -28,7 +29,8 @@ #include #include -#include +#include +#include namespace CKM { @@ -38,11 +40,12 @@ CertificateImpl::CertificateImpl(const RawBuffer &der, DataFormat format) LogDebug("Certificate to parse. Size: " << der.size()); if (DataFormat::FORM_DER_BASE64 == format) { - Base64Decoder base64; - base64.reset(); - base64.append(der); - base64.finalize(); - auto tmp = base64.get(); + RawBuffer tmp; + try { + tmp = base64DecodeGeneric(der); + } catch (const std::invalid_argument&) { + LogError("Certificate in FORM_DER_BASE64 format is not valid base64."); + } auto ptr = reinterpret_cast(tmp.data()); auto size = static_cast(tmp.size()); m_x509 = d2i_X509(nullptr, &ptr, size); diff --git a/src/manager/initial-values/BufferHandler.cpp b/src/manager/initial-values/BufferHandler.cpp index 5917844..404ec58 100644 --- a/src/manager/initial-values/BufferHandler.cpp +++ b/src/manager/initial-values/BufferHandler.cpp @@ -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. @@ -23,9 +23,12 @@ #include #include #include +#include #include #include -#include +#include +#include +#include namespace { const char *const XML_ATTR_IV = "IV"; @@ -42,20 +45,20 @@ void BufferHandler::Start(const XML::Parser::Attributes &attr) { if (attr.find(XML_ATTR_IV) != attr.end()) { std::string IVstring = attr.at(XML_ATTR_IV); - Base64Decoder base64; - base64.reset(); - base64.append(RawBuffer(IVstring.begin(), IVstring.end())); - base64.finalize(); - m_encryptionParams.iv = base64.get(); + try { + m_encryptionParams.iv = base64DecodeGeneric(IVstring); + } catch (const std::invalid_argument&) { + LogError("Encryption param IV is not valid base64."); + } } if (attr.find(XML_ATTR_TAG) != attr.end()) { std::string tag = attr.at(XML_ATTR_TAG); - Base64Decoder base64; - base64.reset(); - base64.append(RawBuffer(tag.begin(), tag.end())); - base64.finalize(); - m_encryptionParams.tag = base64.get(); + try { + m_encryptionParams.tag = base64DecodeGeneric(tag); + } catch (const std::invalid_argument&) { + LogError("Encryption param tag is not valid base64."); + } } } @@ -84,11 +87,12 @@ void BufferHandler::End() case ENCRYPTED: { std::string trimmed = XML::trimEachLine(std::string(m_data.begin(), m_data.end())); - Base64Decoder base64; - base64.reset(); - base64.append(RawBuffer(trimmed.begin(), trimmed.end())); - base64.finalize(); - m_data = base64.get(); + try { + m_data = base64DecodeGeneric(trimmed); + } catch (const std::invalid_argument&) { + LogError("Data is not valid base64."); + m_data.clear(); + } break; } diff --git a/src/manager/initial-values/InitialValuesFile.cpp b/src/manager/initial-values/InitialValuesFile.cpp index 8108af1..6655825 100644 --- a/src/manager/initial-values/InitialValuesFile.cpp +++ b/src/manager/initial-values/InitialValuesFile.cpp @@ -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. @@ -21,6 +21,7 @@ */ #include +#include #include #include #include @@ -30,6 +31,7 @@ #include #include #include +#include #include namespace { @@ -253,11 +255,12 @@ void InitialValuesFile::EncryptionKeyHandler::End() { std::string trimmed = XML::trimEachLine(std::string(m_encryptedKey.begin(), m_encryptedKey.end())); - Base64Decoder base64; - base64.reset(); - base64.append(RawBuffer(trimmed.begin(), trimmed.end())); - base64.finalize(); - m_encryptedKey = base64.get(); + try { + m_encryptedKey = base64DecodeGeneric(trimmed); + } catch (const std::invalid_argument&) { + LogError("Encrypted key is not valid base64."); + m_encryptedKey.clear(); + } }; CKM::RawBuffer InitialValuesFile::EncryptionKeyHandler::getEncryptedKey() const diff --git a/src/manager/service/crypto-logic.cpp b/src/manager/service/crypto-logic.cpp index 6de54dc..2dcf715 100644 --- a/src/manager/service/crypto-logic.cpp +++ b/src/manager/service/crypto-logic.cpp @@ -220,34 +220,20 @@ void CryptoLogic::decryptRow(const Password &password, DB::Row &row) void CryptoLogic::encBase64(RawBuffer &data) { - Base64Encoder benc; - RawBuffer encdata; - - benc.append(data); - benc.finalize(); - encdata = benc.get(); + RawBuffer encdata = base64Encode(data); if (encdata.size() == 0) - ThrowErr(Exc::InternalError, "Base64Encoder returned empty data."); + ThrowErr(Exc::InternalError, "base64Encode returned empty data."); data = std::move(encdata); } void CryptoLogic::decBase64(RawBuffer &data) { - Base64Decoder bdec; - RawBuffer decdata; - - bdec.reset(); - bdec.append(data); - - if (!bdec.finalize()) - ThrowErr(Exc::InternalError, "Failed in Base64Decoder.finalize."); - - decdata = bdec.get(); + RawBuffer decdata = base64Decode(data); if (decdata.size() == 0) - ThrowErr(Exc::InternalError, "Base64Decoder returned empty data."); + ThrowErr(Exc::InternalError, "base64Decode returned empty data."); data = std::move(decdata); } diff --git a/unit-tests/CMakeLists.txt b/unit-tests/CMakeLists.txt index 8478846..bc3e2bb 100644 --- a/unit-tests/CMakeLists.txt +++ b/unit-tests/CMakeLists.txt @@ -66,6 +66,7 @@ INCLUDE_DIRECTORIES( LINK_DIRECTORIES(${KEY_MANAGER_DEP_LIBRARY_DIRS}) SET(UNIT_TESTS_SOURCES + ${PROJECT_SOURCE_DIR}/common/base64_generic.cpp ${PROJECT_SOURCE_DIR}/common/colour_log_formatter.cpp ${PROJECT_SOURCE_DIR}/common/DBFixture.cpp ${PROJECT_SOURCE_DIR}/common/test_common.cpp @@ -98,7 +99,6 @@ SET(UNIT_TESTS_SOURCES ${MANAGER_PATH}/client-async/descriptor-set.cpp ${MANAGER_PATH}/common/algo-param.cpp - ${MANAGER_PATH}/common/base64.cpp ${MANAGER_PATH}/common/certificate-impl.cpp ${MANAGER_PATH}/common/ckm-zero-memory.cpp ${MANAGER_PATH}/common/data-type.cpp diff --git a/unit-tests/test_base64.cpp b/unit-tests/test_base64.cpp index 6a75d19..ad34421 100644 --- a/unit-tests/test_base64.cpp +++ b/unit-tests/test_base64.cpp @@ -27,8 +27,8 @@ #include #include -using CKM::Base64Encoder; -using CKM::Base64Decoder; +using CKM::base64Encode; +using CKM::base64Decode; using CKM::RawBuffer; namespace { @@ -50,22 +50,12 @@ BOOST_AUTO_TEST_SUITE(BASE64_TEST) POSITIVE_TEST_CASE(ENCODE_DECODE) { /* try encode */ - Base64Encoder encoder; - BOOST_REQUIRE_NO_THROW(encoder.append(rawbuf)); - BOOST_REQUIRE_NO_THROW(encoder.finalize()); - RawBuffer encdata; - BOOST_REQUIRE_NO_THROW(encdata = encoder.get()); - BOOST_REQUIRE_NO_THROW(encoder.reset()); + BOOST_REQUIRE_NO_THROW(encdata = base64Encode(rawbuf)); /* try decode */ - Base64Decoder decoder; - BOOST_REQUIRE_NO_THROW(decoder.append(encdata)); - BOOST_REQUIRE_NO_THROW(decoder.finalize()); - RawBuffer decdata; - BOOST_REQUIRE_NO_THROW(decdata = decoder.get()); - BOOST_REQUIRE_NO_THROW(decoder.reset()); + BOOST_REQUIRE_NO_THROW(decdata = base64Decode(encdata)); /* compare with orig data */ BOOST_REQUIRE_MESSAGE( @@ -74,44 +64,9 @@ POSITIVE_TEST_CASE(ENCODE_DECODE) "Original data and encoded-decoded data is different!"); } -NEGATIVE_TEST_CASE(THROW_SOMETHING) -{ - /* encode data */ - Base64Encoder encoder; - BOOST_REQUIRE_THROW(encoder.get(), CKM::Exc::InternalError); - - BOOST_REQUIRE_NO_THROW(encoder.append(rawbuf)); - BOOST_REQUIRE_NO_THROW(encoder.finalize()); - - BOOST_REQUIRE_THROW(encoder.append(rawbuf), - CKM::Exc::InternalError); - BOOST_REQUIRE_THROW(encoder.finalize(), - CKM::Exc::InternalError); - - RawBuffer encdata; - BOOST_REQUIRE_NO_THROW(encdata = encoder.get()); - - /* decode data */ - Base64Decoder decoder; - BOOST_REQUIRE_THROW(decoder.get(), CKM::Exc::InternalError); - - BOOST_REQUIRE_NO_THROW(decoder.append(encdata)); - BOOST_REQUIRE_NO_THROW(decoder.finalize()); - - BOOST_REQUIRE_THROW(decoder.append(encdata), - CKM::Exc::InternalError); - BOOST_REQUIRE_THROW(decoder.finalize(), - CKM::Exc::InternalError); - - RawBuffer decdata; - BOOST_REQUIRE_NO_THROW(decdata = decoder.get()); -} - NEGATIVE_TEST_CASE(ILLEGAL_DATA) { - Base64Decoder decoder; - BOOST_REQUIRE_NO_THROW(decoder.append(rawbuf)); - BOOST_REQUIRE(!decoder.finalize()); + BOOST_REQUIRE_THROW(base64Decode(rawbuf), CKM::Exc::InternalError); } BOOST_AUTO_TEST_SUITE_END() diff --git a/unit-tests/test_crypto-logic.cpp b/unit-tests/test_crypto-logic.cpp index 907c766..c06e2a7 100644 --- a/unit-tests/test_crypto-logic.cpp +++ b/unit-tests/test_crypto-logic.cpp @@ -37,18 +37,13 @@ const auto TEST_DATA = createRandom(10); void changeBase64(RawBuffer& data) { - auto b64 = [](auto&& coder, RawBuffer& data){ - coder.append(data); - coder.finalize(); - data = coder.get(); - - BOOST_REQUIRE(!data.empty()); - }; - b64(Base64Decoder(), data); + data = base64Decode(data); + BOOST_REQUIRE(!data.empty()); ++data[0]; - b64(Base64Encoder(), data); + data = base64Encode(data); + BOOST_REQUIRE(!data.empty()); } } // namespace anonymous -- 2.7.4 From 57127f9d95de2e7148295ca969935015542a6c5d Mon Sep 17 00:00:00 2001 From: Konrad Lipinski Date: Mon, 14 Sep 2020 15:10:02 +0200 Subject: [PATCH 12/16] Prevent some CAPI exception leakage Change-Id: Ic9fb8985f6052479e7c9c6e24cf24607f34e3526 --- src/manager/client-capi/ckmc-control.cpp | 34 +++++++++++++++++++++++++++++++- src/manager/client-capi/ckmc-type.cpp | 22 ++++++++++++++++++++- 2 files changed, 54 insertions(+), 2 deletions(-) diff --git a/src/manager/client-capi/ckmc-control.cpp b/src/manager/client-capi/ckmc-control.cpp index 2ee1438..66e1c31 100644 --- a/src/manager/client-capi/ckmc-control.cpp +++ b/src/manager/client-capi/ckmc-control.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2000 - 2014 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. @@ -39,50 +39,72 @@ CKM::Password _toPasswordStr(const char *str) KEY_MANAGER_CAPI int ckmc_unlock_user_key(uid_t user, const char *password) { + EXCEPTION_GUARD_START_CAPI + auto control = CKM::Control::create(); int ret = control->unlockUserKey(user, _toPasswordStr(password)); return to_ckmc_error(ret); + + EXCEPTION_GUARD_END } KEY_MANAGER_CAPI int ckmc_lock_user_key(uid_t user) { + EXCEPTION_GUARD_START_CAPI + auto control = CKM::Control::create(); int ret = control->lockUserKey(user); return to_ckmc_error(ret); + + EXCEPTION_GUARD_END } KEY_MANAGER_CAPI int ckmc_remove_user_data(uid_t user) { + EXCEPTION_GUARD_START_CAPI + auto control = CKM::Control::create(); int ret = control->removeUserData(user); return to_ckmc_error(ret); + + EXCEPTION_GUARD_END } KEY_MANAGER_CAPI int ckmc_change_user_password(uid_t user, const char *oldPassword, const char *newPassword) { + EXCEPTION_GUARD_START_CAPI + auto control = CKM::Control::create(); int ret = control->changeUserPassword(user, _toPasswordStr(oldPassword), _toPasswordStr(newPassword)); return to_ckmc_error(ret); + + EXCEPTION_GUARD_END } KEY_MANAGER_CAPI int ckmc_reset_user_password(uid_t user, const char *newPassword) { + EXCEPTION_GUARD_START_CAPI + auto control = CKM::Control::create(); int ret = control->resetUserPassword(user, _toPasswordStr(newPassword)); return to_ckmc_error(ret); + + EXCEPTION_GUARD_END } KEY_MANAGER_CAPI int ckmc_allow_access_by_adm(uid_t user, const char *owner, const char *alias, const char *accessor, ckmc_access_right_e granted) { + EXCEPTION_GUARD_START_CAPI + LogWarning("DEPRECATION WARNING: " << __func__ << "() is deprecated and will be " "removed from next release. Use ckmc_set_permission_by_adm() instead."); @@ -99,24 +121,32 @@ int ckmc_allow_access_by_adm(uid_t user, const char *owner, const char *alias, return ckmc_set_permission_by_adm(user, CKM::AliasSupport::merge(CKM::ClientId(owner), CKM::Name(alias)).c_str(), accessor, permissionMask); + + EXCEPTION_GUARD_END } KEY_MANAGER_CAPI int ckmc_set_permission_by_adm(uid_t user, const char *alias, const char *accessor, int permissions) { + EXCEPTION_GUARD_START_CAPI + if (!alias || !accessor) return CKMC_ERROR_INVALID_PARAMETER; auto control = CKM::Control::create(); return to_ckmc_error(control->setPermission(user, alias, accessor, permissions)); + + EXCEPTION_GUARD_END } KEY_MANAGER_CAPI int ckmc_deny_access_by_adm(uid_t user, const char *owner, const char *alias, const char *accessor) { + EXCEPTION_GUARD_START_CAPI + LogWarning("DEPRECATION WARNING: " << __func__ << "() is deprecated and will be " "removed from next release. Use ckmc_set_permission_by_adm() instead."); @@ -130,4 +160,6 @@ int ckmc_deny_access_by_adm(uid_t user, const char *owner, const char *alias, CKM::AliasSupport::merge(CKM::ClientId(owner), CKM::Name(alias)).c_str(), accessor, CKM::Permission::NONE)); + + EXCEPTION_GUARD_END } diff --git a/src/manager/client-capi/ckmc-type.cpp b/src/manager/client-capi/ckmc-type.cpp index 3e1ca0a..f79c2e1 100644 --- a/src/manager/client-capi/ckmc-type.cpp +++ b/src/manager/client-capi/ckmc-type.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2014 - 2020 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. @@ -239,6 +239,8 @@ 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) { + EXCEPTION_GUARD_START_CAPI + FILE *fp = fopen(file_path, "r"); if (fp == NULL) @@ -266,6 +268,8 @@ int ckmc_load_cert_from_file(const char *file_path, ckmc_cert_s **cert) X509_free(pcert); return ret; + + EXCEPTION_GUARD_END } KEY_MANAGER_CAPI @@ -312,6 +316,8 @@ int ckmc_load_from_pkcs12_file(const char *file_path, const char *passphrase, ckmc_key_s **private_key, ckmc_cert_s **ckmcert, ckmc_cert_list_s **ca_cert_list) { + EXCEPTION_GUARD_START_CAPI + class Pkcs12Converter { private: FILE *fp_in; @@ -496,6 +502,8 @@ int ckmc_load_from_pkcs12_file(const char *file_path, const char *passphrase, *ca_cert_list = converter.retCaCertList; return CKMC_ERROR_NONE; + + EXCEPTION_GUARD_END } KEY_MANAGER_CAPI @@ -708,12 +716,16 @@ int ckmc_param_list_set_integer(ckmc_param_list_h params, ckmc_param_name_e name, uint64_t value) { + EXCEPTION_GUARD_START_CAPI + if (!params) return CKMC_ERROR_INVALID_PARAMETER; CKM::CryptoAlgorithm *algo = reinterpret_cast(params); bool ret = algo->setParam(static_cast(name), value); return (ret ? CKMC_ERROR_NONE : CKMC_ERROR_INVALID_PARAMETER); + + EXCEPTION_GUARD_END } KEY_MANAGER_CAPI @@ -721,6 +733,8 @@ int ckmc_param_list_set_buffer(ckmc_param_list_h params, ckmc_param_name_e name, const ckmc_raw_buffer_s *buffer) { + EXCEPTION_GUARD_START_CAPI + if (!params || !buffer || !buffer->data || buffer->size == 0) return CKMC_ERROR_INVALID_PARAMETER; @@ -728,6 +742,8 @@ int ckmc_param_list_set_buffer(ckmc_param_list_h params, CKM::RawBuffer b(buffer->data, buffer->data + buffer->size); bool ret = algo->setParam(static_cast(name), b); return (ret ? CKMC_ERROR_NONE : CKMC_ERROR_INVALID_PARAMETER); + + EXCEPTION_GUARD_END } KEY_MANAGER_CAPI @@ -752,6 +768,8 @@ int ckmc_param_list_get_buffer(ckmc_param_list_h params, ckmc_param_name_e name, ckmc_raw_buffer_s **ppbuffer) { + EXCEPTION_GUARD_START_CAPI + if (!params || !ppbuffer || *ppbuffer) return CKMC_ERROR_INVALID_PARAMETER; @@ -763,6 +781,8 @@ int ckmc_param_list_get_buffer(ckmc_param_list_h params, return CKMC_ERROR_INVALID_PARAMETER; return ckmc_buffer_new(value.data(), value.size(), ppbuffer); + + EXCEPTION_GUARD_END } KEY_MANAGER_CAPI -- 2.7.4 From ebd52dd8a2616b8752099d547903c966366a3552 Mon Sep 17 00:00:00 2001 From: Konrad Lipinski Date: Thu, 27 Aug 2020 15:08:47 +0200 Subject: [PATCH 13/16] Deduplicate ckmc_get_*_alias_list Change-Id: I0d2906da9ee277ff77787a4d5fe8945b46be4557 --- src/manager/client-capi/ckmc-manager.cpp | 161 +++++++++---------------------- 1 file changed, 45 insertions(+), 116 deletions(-) diff --git a/src/manager/client-capi/ckmc-manager.cpp b/src/manager/client-capi/ckmc-manager.cpp index 970fed5..39bb854 100644 --- a/src/manager/client-capi/ckmc-manager.cpp +++ b/src/manager/client-capi/ckmc-manager.cpp @@ -283,6 +283,48 @@ int ckmc_generic_get_alias_info_list_helper(ckmc_alias_info_list_s **alias_info_ EXCEPTION_GUARD_END } +int get_alias_list(ckmc_alias_list_s **alias_list, int (CKM::Manager::*getAliasVector)(CKM::AliasVector&)) +{ + EXCEPTION_GUARD_START_CAPI + + if (alias_list == nullptr) + return CKMC_ERROR_INVALID_PARAMETER; + + CKM::AliasVector aliasVector; + int ret; + auto mgr = CKM::Manager::create(); + + if ((ret = ((*mgr).*getAliasVector)(aliasVector)) != CKM_API_SUCCESS) + return to_ckmc_error(ret); + + ckmc_alias_list_s *start = nullptr; + ckmc_alias_list_s *plist = nullptr; + + for (const auto &it : aliasVector) { + char *alias = strndup(it.c_str(), it.size()); + + ret = ckmc_alias_list_add(plist, alias, &plist); + + if (ret != CKMC_ERROR_NONE) { + free(alias); + ckmc_alias_list_all_free(start); + return ret; + } + + if (start == nullptr) + start = plist; + } + + *alias_list = start; // according to documentation in header + + if (plist == nullptr) // if the alias_list size is zero + return CKMC_ERROR_DB_ALIAS_UNKNOWN; + + return CKMC_ERROR_NONE; + + EXCEPTION_GUARD_END +} + } // namespace anonymous KEY_MANAGER_CAPI @@ -358,48 +400,9 @@ int ckmc_get_key(const char *alias, const char *password, ckmc_key_s **key) KEY_MANAGER_CAPI int ckmc_get_key_alias_list(ckmc_alias_list_s **alias_list) { - EXCEPTION_GUARD_START_CAPI - - int ret; - - if (alias_list == nullptr) - return CKMC_ERROR_INVALID_PARAMETER; - - CKM::AliasVector aliasVector; - auto mgr = CKM::Manager::create(); - - if ((ret = mgr->getKeyAliasVector(aliasVector)) != CKM_API_SUCCESS) - return to_ckmc_error(ret); - - ckmc_alias_list_s *start = nullptr; - ckmc_alias_list_s *plist = nullptr; - - for (const auto &it : aliasVector) { - char *alias = strndup(it.c_str(), it.size()); - - ret = ckmc_alias_list_add(plist, alias, &plist); - - if (ret != CKMC_ERROR_NONE) { - free(alias); - ckmc_alias_list_all_free(start); - return ret; - } - - if (start == nullptr) - start = plist; - } - - *alias_list = start; // according to documentation in header - - if (plist == nullptr) // if the alias_list size is zero - return CKMC_ERROR_DB_ALIAS_UNKNOWN; - - return CKMC_ERROR_NONE; - - EXCEPTION_GUARD_END + return get_alias_list(alias_list, &CKM::Manager::getKeyAliasVector); } - KEY_MANAGER_CAPI int ckmc_get_key_alias_info_list(ckmc_alias_info_list_s **alias_info_list) { @@ -462,44 +465,7 @@ int ckmc_get_cert(const char *alias, const char *password, ckmc_cert_s **cert) KEY_MANAGER_CAPI int ckmc_get_cert_alias_list(ckmc_alias_list_s **alias_list) { - EXCEPTION_GUARD_START_CAPI - - if (alias_list == nullptr) - return CKMC_ERROR_INVALID_PARAMETER; - - CKM::AliasVector aliasVector; - int ret; - auto mgr = CKM::Manager::create(); - - if ((ret = mgr->getCertificateAliasVector(aliasVector)) != CKM_API_SUCCESS) - return to_ckmc_error(ret); - - ckmc_alias_list_s *start = nullptr; - ckmc_alias_list_s *plist = nullptr; - - for (const auto &it : aliasVector) { - char *alias = strndup(it.c_str(), it.size()); - - ret = ckmc_alias_list_add(plist, alias, &plist); - - if (ret != CKMC_ERROR_NONE) { - free(alias); - ckmc_alias_list_all_free(start); - return ret; - } - - if (start == nullptr) - start = plist; - } - - *alias_list = start; // according to documentation from header - - if (plist == nullptr) // if the alias_list size is zero - return CKMC_ERROR_DB_ALIAS_UNKNOWN; - - return CKMC_ERROR_NONE; - - EXCEPTION_GUARD_END + return get_alias_list(alias_list, &CKM::Manager::getCertificateAliasVector); } KEY_MANAGER_CAPI @@ -659,44 +625,7 @@ int ckmc_get_data(const char *alias, const char *password, KEY_MANAGER_CAPI int ckmc_get_data_alias_list(ckmc_alias_list_s **alias_list) { - EXCEPTION_GUARD_START_CAPI - - if (alias_list == nullptr) - return CKMC_ERROR_INVALID_PARAMETER; - - int ret; - CKM::AliasVector aliasVector; - auto mgr = CKM::Manager::create(); - - if ((ret = mgr->getDataAliasVector(aliasVector)) != CKM_API_SUCCESS) - return to_ckmc_error(ret); - - ckmc_alias_list_s *start = nullptr; - ckmc_alias_list_s *plist = nullptr; - - for (const auto &it : aliasVector) { - char *alias = strndup(it.c_str(), it.size()); - - ret = ckmc_alias_list_add(plist, alias, &plist); - - if (ret != CKMC_ERROR_NONE) { - free(alias); - ckmc_alias_list_all_free(start); - return ret; - } - - if (start == nullptr) - start = plist; - } - - *alias_list = start; // according to documentation in header - - if (plist == nullptr) // if the alias_list size is zero - return CKMC_ERROR_DB_ALIAS_UNKNOWN; - - return CKMC_ERROR_NONE; - - EXCEPTION_GUARD_END + return get_alias_list(alias_list, &CKM::Manager::getDataAliasVector); } KEY_MANAGER_CAPI -- 2.7.4 From 292e5c8683b3a4aa85fd810b81f546e96cb00c64 Mon Sep 17 00:00:00 2001 From: Konrad Lipinski Date: Thu, 27 Aug 2020 16:03:41 +0200 Subject: [PATCH 14/16] Deduplicate sw backend keyPair creation Change-Id: Iff7d579d02e54e841140ba419aa6fffd19086dd3 --- src/manager/crypto/sw-backend/internals.cpp | 124 ++++++++++------------------ 1 file changed, 45 insertions(+), 79 deletions(-) diff --git a/src/manager/crypto/sw-backend/internals.cpp b/src/manager/crypto/sw-backend/internals.cpp index 493d586..a2a9054 100644 --- a/src/manager/crypto/sw-backend/internals.cpp +++ b/src/manager/crypto/sw-backend/internals.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2015 - 2020 Samsung Electronics Co., Ltd All Rights Reserved + * Copyright (c) 2015-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. @@ -380,84 +380,83 @@ int getRsaPadding(const RSAPaddingAlgorithm padAlgo) } } -DataPair createKeyPairRSA(const int size) +EvpPkeyCtxUPtr newCtx(int id) { - EvpPkeyUPtr pkey; - - // validateParams should prevent it - assert(size == 1024 || size == 2048 || size == 4096); + if (auto ctx = EVP_PKEY_CTX_new_id(id, NULL)) + return EvpPkeyCtxUPtr(ctx, EVP_PKEY_CTX_free); - EvpPkeyCtxUPtr ctx(EVP_PKEY_CTX_new_id(EVP_PKEY_RSA, NULL), EVP_PKEY_CTX_free); - - if (!ctx) - ThrowErr(Exc::Crypto::InternalError, - "Error in EVP_PKEY_CTX_new_id function !!"); + ThrowErr(Exc::Crypto::InternalError, "Error in EVP_PKEY_CTX_new_id function"); +} - OPENSSL_ERROR_HANDLE(EVP_PKEY_keygen_init(ctx.get())); +EvpPkeyCtxUPtr newCtx(EVP_PKEY *pkey) +{ + if (auto ctx = EVP_PKEY_CTX_new(pkey, NULL)) + return EvpPkeyCtxUPtr(ctx, EVP_PKEY_CTX_free); - OPENSSL_ERROR_HANDLE(EVP_PKEY_CTX_set_rsa_keygen_bits(ctx.get(), size)); + ThrowErr(Exc::Crypto::InternalError, "Error in EVP_PKEY_CTX_new function"); +} +DataPair keyPair(const EvpPkeyCtxUPtr &ctx, KeyType prv, KeyType pub) +{ EVP_PKEY *pkeyTmp = NULL; OPENSSL_ERROR_HANDLE(EVP_PKEY_keygen(ctx.get(), &pkeyTmp)); - pkey = EvpPkeyUPtr(pkeyTmp, EVP_PKEY_free); + auto pkey = EvpPkeyUPtr(pkeyTmp, EVP_PKEY_free); return std::make_pair( - {DataType(KeyType::KEY_RSA_PRIVATE), i2d(i2d_PrivateKey_bio, pkey.get())}, - {DataType(KeyType::KEY_RSA_PUBLIC), i2d(i2d_PUBKEY_bio, pkey.get())}); + {DataType(prv), i2d(i2d_PrivateKey_bio, pkey.get())}, + {DataType(pub), i2d(i2d_PUBKEY_bio, pkey.get())}); } -DataPair createKeyPairDSA(const int size) +DataPair createKeyPairRSA(const int size) { - EvpPkeyUPtr pkey; - EvpPkeyUPtr pparam; - // validateParams should prevent it - assert(size == 1024 || size == 2048 || size == 3072 || size == 4096); + assert(size == 1024 || size == 2048 || size == 4096); - /* Create the context for generating the parameters */ - EvpPkeyCtxUPtr pctx(EVP_PKEY_CTX_new_id(EVP_PKEY_DSA, NULL), EVP_PKEY_CTX_free); + auto ctx = newCtx(EVP_PKEY_RSA); - if (!pctx) - ThrowErr(Exc::Crypto::InternalError, "Error in EVP_PKEY_CTX_new_id function"); + OPENSSL_ERROR_HANDLE(EVP_PKEY_keygen_init(ctx.get())); - OPENSSL_ERROR_HANDLE(EVP_PKEY_paramgen_init(pctx.get())); + OPENSSL_ERROR_HANDLE(EVP_PKEY_CTX_set_rsa_keygen_bits(ctx.get(), size)); - OPENSSL_ERROR_HANDLE(EVP_PKEY_CTX_set_dsa_paramgen_bits(pctx.get(), size)); + return keyPair(ctx, KeyType::KEY_RSA_PRIVATE, KeyType::KEY_RSA_PUBLIC); +} +DataPair paramgenKeyPair(const EvpPkeyCtxUPtr &pctx, KeyType prv, KeyType pub) +{ /* Generate parameters */ EVP_PKEY *pparamTmp = NULL; - OPENSSL_ERROR_HANDLE(EVP_PKEY_paramgen(pctx.get(), &pparamTmp)); - pparam = EvpPkeyUPtr(pparamTmp, EVP_PKEY_free); + auto pparam = EvpPkeyUPtr(pparamTmp, EVP_PKEY_free); // Start to generate key - EvpPkeyCtxUPtr kctx(EVP_PKEY_CTX_new(pparam.get(), NULL), EVP_PKEY_CTX_free); - - if (!kctx) - ThrowErr(Exc::Crypto::InternalError, "Error in EVP_PKEY_CTX_new function"); + auto kctx = newCtx(pparam.get()); OPENSSL_ERROR_HANDLE(EVP_PKEY_keygen_init(kctx.get())); - /* Generate the key */ - EVP_PKEY *pkeyTmp = NULL; + return keyPair(kctx, prv, pub); +} - OPENSSL_ERROR_HANDLE(EVP_PKEY_keygen(kctx.get(), &pkeyTmp)); +DataPair createKeyPairDSA(const int size) +{ + // validateParams should prevent it + assert(size == 1024 || size == 2048 || size == 3072 || size == 4096); - pkey = EvpPkeyUPtr(pkeyTmp, EVP_PKEY_free); + /* Create the context for generating the parameters */ + auto pctx = newCtx(EVP_PKEY_DSA); - return std::make_pair( - {DataType(KeyType::KEY_DSA_PRIVATE), i2d(i2d_PrivateKey_bio, pkey.get())}, - {DataType(KeyType::KEY_DSA_PUBLIC), i2d(i2d_PUBKEY_bio, pkey.get())}); + OPENSSL_ERROR_HANDLE(EVP_PKEY_paramgen_init(pctx.get())); + + OPENSSL_ERROR_HANDLE(EVP_PKEY_CTX_set_dsa_paramgen_bits(pctx.get(), size)); + + return paramgenKeyPair(pctx, KeyType::KEY_DSA_PRIVATE, KeyType::KEY_DSA_PUBLIC); } DataPair createKeyPairECDSA(ElipticCurve type) { int ecCurve; - EvpPkeyUPtr pkey; - EvpPkeyUPtr pparam; switch (type) { default: @@ -476,40 +475,13 @@ DataPair createKeyPairECDSA(ElipticCurve type) } /* Create the context for generating the parameters */ - EvpPkeyCtxUPtr pctx(EVP_PKEY_CTX_new_id(EVP_PKEY_EC, NULL), EVP_PKEY_CTX_free); - - if (!pctx) - ThrowErr(Exc::Crypto::InternalError, "Error in EVP_PKEY_CTX_new_id function"); + auto pctx = newCtx(EVP_PKEY_EC); OPENSSL_ERROR_HANDLE(EVP_PKEY_paramgen_init(pctx.get())); OPENSSL_ERROR_HANDLE(EVP_PKEY_CTX_set_ec_paramgen_curve_nid(pctx.get(), ecCurve)); - /* Generate parameters */ - EVP_PKEY *pparamTmp = NULL; - - OPENSSL_ERROR_HANDLE(EVP_PKEY_paramgen(pctx.get(), &pparamTmp)); - - pparam = EvpPkeyUPtr(pparamTmp, EVP_PKEY_free); - - // Start to generate key - EvpPkeyCtxUPtr kctx(EVP_PKEY_CTX_new(pparam.get(), NULL), EVP_PKEY_CTX_free); - - if (!kctx) - ThrowErr(Exc::Crypto::InternalError, "Error in EVP_PKEY_CTX_new function"); - - OPENSSL_ERROR_HANDLE(EVP_PKEY_keygen_init(kctx.get())); - - /* Generate the key */ - EVP_PKEY *pkeyTmp = NULL; - - OPENSSL_ERROR_HANDLE(EVP_PKEY_keygen(kctx.get(), &pkeyTmp)); - - pkey = EvpPkeyUPtr(pkeyTmp, EVP_PKEY_free); - - return std::make_pair( - {DataType(KeyType::KEY_ECDSA_PRIVATE), i2d(i2d_PrivateKey_bio, pkey.get())}, - {DataType(KeyType::KEY_ECDSA_PUBLIC), i2d(i2d_PUBKEY_bio, pkey.get())}); + return paramgenKeyPair(pctx, KeyType::KEY_ECDSA_PRIVATE, KeyType::KEY_ECDSA_PUBLIC); } Data createKeyAES(const int sizeBits) @@ -534,10 +506,7 @@ RawBuffer signMessage(EVP_PKEY *privKey, if (EVP_PKEY_type(EVP_PKEY_id(privKey)) != EVP_PKEY_RSA) ThrowErr(Exc::Crypto::InputParam, "Only RSA supports no hash option"); - EvpPkeyCtxUPtr pctx(EVP_PKEY_CTX_new(privKey, NULL), EVP_PKEY_CTX_free); - - if (!pctx.get()) - ThrowErr(Exc::Crypto::InternalError, "Error in EVP_PKEY_CTX_new function"); + auto pctx = newCtx(privKey); OPENSSL_ERROR_HANDLE(EVP_PKEY_sign_init(pctx.get())); @@ -609,10 +578,7 @@ int verifyMessage(EVP_PKEY *pubKey, if (EVP_PKEY_type(EVP_PKEY_id(pubKey)) != EVP_PKEY_RSA) ThrowErr(Exc::Crypto::InputParam, "Only RSA supports no hash option"); - EvpPkeyCtxUPtr pctx(EVP_PKEY_CTX_new(pubKey, NULL), EVP_PKEY_CTX_free); - - if (!pctx.get()) - ThrowErr(Exc::Crypto::InternalError, "Error in EVP_PKEY_CTX_new function"); + auto pctx = newCtx(pubKey); OPENSSL_ERROR_HANDLE(EVP_PKEY_verify_init(pctx.get())); -- 2.7.4 From 8e882b88ee667a2532d7d8dc50a0a495ad931fd4 Mon Sep 17 00:00:00 2001 From: Konrad Lipinski Date: Thu, 27 Aug 2020 17:21:41 +0200 Subject: [PATCH 15/16] Deduplicate client manager impl deserialization Change-Id: I9205aac1c97dd1d9a4f16caffdd24e6e7b1f2b85 --- src/manager/client/client-manager-impl.cpp | 110 ++++++----------------------- 1 file changed, 22 insertions(+), 88 deletions(-) diff --git a/src/manager/client/client-manager-impl.cpp b/src/manager/client/client-manager-impl.cpp index 3bbdcc3..3a2994a 100644 --- a/src/manager/client/client-manager-impl.cpp +++ b/src/manager/client/client-manager-impl.cpp @@ -1,4 +1,4 @@ -/* Copyright (c) 2014 - 2020 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,6 +35,14 @@ namespace CKM { namespace { +template +int deserialize(const int msgId, MessageBuffer &recv, T&&...t) +{ + int retMsgId, retCode; + recv.Deserialize(retMsgId, retCode, std::forward(t)...); + return msgId != retMsgId ? CKM_API_ERROR_UNKNOWN : retCode; +} + template int getCertChain( ServiceConnection &serviceConnection, @@ -61,12 +69,8 @@ int getCertChain( if (CKM_API_SUCCESS != retCode) return retCode; - int retMsgId; RawBufferVector rawBufferVector; - recv.Deserialize(retMsgId, retCode, rawBufferVector); - - if (msgId != retMsgId) - return CKM_API_ERROR_UNKNOWN; + retCode = deserialize(msgId, recv, rawBufferVector); if (retCode != CKM_API_SUCCESS) return retCode; @@ -124,14 +128,8 @@ int Manager::Impl::saveBinaryData( if (CKM_API_SUCCESS != retCode) return retCode; - int retMsgId; int opType; - recv.Deserialize(retMsgId, retCode, opType); - - if (retMsgId != msgId) - return CKM_API_ERROR_UNKNOWN; - - return retCode; + return deserialize(msgId, recv, opType); EXCEPTION_GUARD_END } @@ -199,13 +197,7 @@ int Manager::Impl::savePKCS12( if (CKM_API_SUCCESS != retCode) return retCode; - int retMsgId; - recv.Deserialize(retMsgId, retCode); - - if (retMsgId != msgId) - return CKM_API_ERROR_UNKNOWN; - - return retCode; + return deserialize(msgId, recv); EXCEPTION_GUARD_END } @@ -275,13 +267,7 @@ int Manager::Impl::removeAlias(const Alias &alias) if (CKM_API_SUCCESS != retCode) return retCode; - int retMsgId; - recv.Deserialize(retMsgId, retCode); - - if (retMsgId != msgId) - return CKM_API_ERROR_UNKNOWN; - - return retCode; + return deserialize(msgId, recv); EXCEPTION_GUARD_END } @@ -314,13 +300,7 @@ int Manager::Impl::getBinaryData( if (CKM_API_SUCCESS != retCode) return retCode; - int retMsgId; - recv.Deserialize(retMsgId, retCode, recvDataType, rawData); - - if (retMsgId != msgId) - return CKM_API_ERROR_UNKNOWN; - - return retCode; + return deserialize(msgId, recv, recvDataType, rawData); EXCEPTION_GUARD_END } @@ -349,13 +329,9 @@ int Manager::Impl::getBinaryDataEncryptionStatus(const DataType sendDataType, if (CKM_API_SUCCESS != retCode) return retCode; - int retMsgId; DataType tmpDataType; bool passwordProtectionStatus; - recv.Deserialize(retMsgId, retCode, tmpDataType, passwordProtectionStatus); - - if (retMsgId != msgId) - return CKM_API_ERROR_UNKNOWN; + retCode = deserialize(msgId, recv, tmpDataType, passwordProtectionStatus); if (retCode != CKM_API_SUCCESS) { return retCode; @@ -620,13 +596,7 @@ int Manager::Impl::createKeyAES( if (CKM_API_SUCCESS != retCode) return retCode; - int retMsgId; - recv.Deserialize(retMsgId, retCode); - - if (retMsgId != msgId) - return CKM_API_ERROR_UNKNOWN; - - return retCode; + return deserialize(msgId, recv); EXCEPTION_GUARD_END } @@ -689,13 +659,7 @@ int Manager::Impl::createKeyPair( if (CKM_API_SUCCESS != retCode) return retCode; - int retMsgId; - recv.Deserialize(retMsgId, retCode); - - if (retMsgId != msgId) - return CKM_API_ERROR_UNKNOWN; - - return retCode; + return deserialize(msgId, recv); EXCEPTION_GUARD_END } @@ -796,13 +760,7 @@ int Manager::Impl::createSignature( if (CKM_API_SUCCESS != retCode) return retCode; - int retMsgId; - recv.Deserialize(retMsgId, retCode, signature); - - if (retMsgId != msgId) - return CKM_API_ERROR_UNKNOWN; - - return retCode; + return deserialize(msgId, recv, signature); EXCEPTION_GUARD_END } @@ -834,13 +792,7 @@ int Manager::Impl::verifySignature( if (CKM_API_SUCCESS != retCode) return retCode; - int retMsgId; - recv.Deserialize(retMsgId, retCode); - - if (retMsgId != msgId) - return CKM_API_ERROR_UNKNOWN; - - return retCode; + return deserialize(msgId, recv); EXCEPTION_GUARD_END } @@ -871,13 +823,7 @@ int Manager::Impl::ocspCheck(const CertificateShPtrVector &certChain, if (CKM_API_SUCCESS != retCode) return retCode; - int retMsgId; - recv.Deserialize(retMsgId, retCode, ocspStatus); - - if (msgId != retMsgId) - return CKM_API_ERROR_UNKNOWN; - - return retCode; + return deserialize(msgId, recv, ocspStatus); EXCEPTION_GUARD_END } @@ -904,13 +850,7 @@ int Manager::Impl::setPermission(const Alias &alias, if (CKM_API_SUCCESS != retCode) return retCode; - int retMsgId; - recv.Deserialize(retMsgId, retCode); - - if (msgId != retMsgId) - return CKM_API_ERROR_UNKNOWN; - - return retCode; + return deserialize(msgId, recv); EXCEPTION_GUARD_END } @@ -942,13 +882,7 @@ int Manager::Impl::crypt(EncryptionCommand command, if (CKM_API_SUCCESS != retCode) return retCode; - int retMsgId; - recv.Deserialize(retMsgId, retCode, output); - - if (msgId != retMsgId) - return CKM_API_ERROR_UNKNOWN; - - return retCode; + return deserialize(msgId, recv, output); EXCEPTION_GUARD_END } -- 2.7.4 From b11be04b10c8fe53826f2c0d1cccf789a17b7955 Mon Sep 17 00:00:00 2001 From: Konrad Lipinski Date: Wed, 16 Sep 2020 12:56:09 +0200 Subject: [PATCH 16/16] Forward retCode in alias vector getters Change-Id: I16c94d941ed145fa93de359327bc6c8717578d89 --- src/manager/client/client-manager-impl.cpp | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/manager/client/client-manager-impl.cpp b/src/manager/client/client-manager-impl.cpp index 3a2994a..8e3c86d 100644 --- a/src/manager/client/client-manager-impl.cpp +++ b/src/manager/client/client-manager-impl.cpp @@ -442,10 +442,7 @@ int Manager::Impl::getBinaryDataAliasVectorHelper(DataType dataType, DataType tmpDataType; recv.Deserialize(retMsgId, retCode, tmpDataType, ownerNameVector); - if (retMsgId != msgId) - return CKM_API_ERROR_UNKNOWN; - - return CKM_API_SUCCESS; + return retMsgId != msgId ? CKM_API_ERROR_UNKNOWN : retCode; } int Manager::Impl::getBinaryDataAliasVector(DataType dataType, -- 2.7.4