Fix svace/coverity issues 15/290815/3
authorKrzysztof Jackiewicz <k.jackiewicz@samsung.com>
Mon, 3 Apr 2023 15:46:07 +0000 (17:46 +0200)
committerKrzysztof Jackiewicz <k.jackiewicz@samsung.com>
Tue, 4 Apr 2023 14:06:26 +0000 (16:06 +0200)
Change-Id: I681fd80cddf5f56bc99b35546940e111d29a5311

doc/examples/key_derive.cpp
src/manager/client-async/descriptor-set.cpp
src/manager/client-capi/ckmc-manager.cpp
src/manager/client/client-manager-impl.cpp
src/manager/main/socket-manager.cpp
src/manager/main/socket-manager.h
src/manager/service/ckm-logic.cpp
src/manager/service/key-provider.cpp
src/manager/service/key-provider.h
unit-tests/test_db_crypto.cpp
unit-tests/test_descriptor-set.cpp

index 79c6507..39f2706 100644 (file)
@@ -28,6 +28,14 @@ int key_derive(const ckmc_raw_buffer_s* peers_public)
        const char* const OURS_PRV_ALIAS = "ours_private";
        const char* const OURS_PUB_ALIAS = "ours_public";
 
+       char label[] = "label";
+       char context[] = "context";
+
+       ckmc_param_list_h ecdh_params = nullptr;
+       ckmc_param_list_h kbkdf_params = nullptr;
+       ckmc_raw_buffer_s* label_buf = nullptr;
+       ckmc_raw_buffer_s* context_buf = nullptr;
+
        ckmc_policy_s unexportable { nullptr, false };
        ckmc_policy_s exportable { nullptr, true };
 
@@ -41,84 +49,77 @@ int key_derive(const ckmc_raw_buffer_s* peers_public)
                return -1;
 
        // set ECDH params
-       ckmc_param_list_h ecdh_params;
        ret = ckmc_param_list_new(&ecdh_params);
        if (ret != CKMC_ERROR_NONE)
-               return -1;
+               goto exit;
 
        ret = ckmc_param_list_set_integer(ecdh_params, CKMC_PARAM_ALGO_TYPE, CKMC_ALGO_ECDH);
        if (ret != CKMC_ERROR_NONE)
-               return -1;
+               goto exit;
 
        ret = ckmc_param_list_set_buffer(ecdh_params, CKMC_PARAM_ECDH_PUBKEY, peers_public);
        if (ret != CKMC_ERROR_NONE)
-               return -1;
+               goto exit;
 
        // derive shared secret
        ret = ckmc_key_derive(ecdh_params, OURS_PRV_ALIAS, nullptr, SECRET_ALIAS, unexportable);
        if (ret != CKMC_ERROR_NONE)
-               return -1;
+               goto exit;
 
        // set KBKDF params
-       ckmc_param_list_h kbkdf_params;
        ret = ckmc_param_list_new(&kbkdf_params);
        if (ret != CKMC_ERROR_NONE)
-               return -1;
+               goto exit;
 
        ret = ckmc_param_list_set_integer(kbkdf_params, CKMC_PARAM_ALGO_TYPE, CKMC_ALGO_KBKDF);
        if (ret != CKMC_ERROR_NONE)
-               return -1;
+               goto exit;
 
        ret = ckmc_param_list_set_integer(kbkdf_params, CKMC_PARAM_KDF_PRF, CKMC_KDF_PRF_HMAC_SHA256);
        if (ret != CKMC_ERROR_NONE)
-               return -1;
+               goto exit;
 
        ret = ckmc_param_list_set_integer(kbkdf_params, CKMC_PARAM_KBKDF_MODE, CKMC_KBKDF_MODE_COUNTER);
        if (ret != CKMC_ERROR_NONE)
-               return -1;
+               goto exit;
 
        ret = ckmc_param_list_set_integer(kbkdf_params,
                                          CKMC_PARAM_KBKDF_COUNTER_LOCATION,
                                          CKMC_KBKDF_COUNTER_BEFORE_FIXED);
        if (ret != CKMC_ERROR_NONE)
-               return -1;
+               goto exit;
 
-       char label[] = "label";
-       ckmc_raw_buffer_s* label_buf = nullptr;
        ret = ckmc_buffer_new(reinterpret_cast<unsigned char*>(label), strlen(label), &label_buf);
        if (ret != CKMC_ERROR_NONE)
-               return -1;
+               goto exit;
 
        ret = ckmc_param_list_set_buffer(kbkdf_params, CKMC_PARAM_KBKDF_LABEL, label_buf);
-       ckmc_buffer_free(label_buf);
        if (ret != CKMC_ERROR_NONE)
-               return -1;
+               goto exit;
 
-       char context[] = "context";
-       ckmc_raw_buffer_s* context_buf = nullptr;
        ret = ckmc_buffer_new(reinterpret_cast<unsigned char*>(context), strlen(context), &context_buf);
        if (ret != CKMC_ERROR_NONE)
-               return -1;
+               goto exit;
 
        ret = ckmc_param_list_set_buffer(kbkdf_params, CKMC_PARAM_KBKDF_CONTEXT, context_buf);
-       ckmc_buffer_free(context_buf);
        if (ret != CKMC_ERROR_NONE)
-               return -1;
+               goto exit;
 
        ret = ckmc_param_list_set_integer(kbkdf_params, CKMC_PARAM_KDF_LEN, 32);
        if (ret != CKMC_ERROR_NONE)
-               return -1;
+               goto exit;
 
        // derive symmetric key
        ret = ckmc_key_derive(kbkdf_params, SECRET_ALIAS, nullptr, KEY_ALIAS, unexportable);
-       if (ret != CKMC_ERROR_NONE)
-               return -1;
-
-       // remove shared secret
-       ret = ckmc_remove_key(SECRET_ALIAS);
-       if (ret != CKMC_ERROR_NONE)
-               return -1;
 
-       return 0;
+exit:
+       ckmc_remove_alias(OURS_PRV_ALIAS);
+       ckmc_remove_alias(OURS_PUB_ALIAS);
+       ckmc_param_list_free(ecdh_params);
+       ckmc_buffer_free(label_buf);
+       ckmc_buffer_free(context_buf);
+       ckmc_param_list_free(kbkdf_params);
+       ckmc_remove_alias(SECRET_ALIAS);
+       return ret;
 }
 //! [KEY_DERIVE example]
index fdee29d..316ea09 100644 (file)
@@ -40,7 +40,7 @@ DescriptorSet::~DescriptorSet()
 
 void DescriptorSet::purge()
 {
-       for (auto it : m_descriptors)
+       for (auto& it : m_descriptors)
                close(it.first);
 
        m_descriptors.clear();
index 083ee28..24aa8cb 100644 (file)
@@ -1072,7 +1072,7 @@ int ckmc_export_wrapped_key(const ckmc_param_list_h params,
        int ret = 0;
        ckmc_key_s *wrapped_key = nullptr;
        CKM::RawBuffer wrappedKey;
-       CKM::KeyType keyType;
+       CKM::KeyType keyType = CKM::KeyType::KEY_NONE;
        auto mgr = CKM::Manager::create();
 
        ret = to_ckmc_error(mgr->exportWrappedKey(*ca,
index 8628daf..b788197 100644 (file)
@@ -801,7 +801,7 @@ int Manager::Impl::exportWrappedKey(const CryptoAlgorithm &params,
 
        AliasSupport wrapping_helper(wrappingKeyAlias);
        AliasSupport wrapped_helper(wrappedKeyAlias);
-       DataType *dataTypeKey = nullptr;
+       DataType dataTypeKey;
 
        int retCode = Request(*this,
                LogicCommand::EXPORT_WRAPPED_KEY,
@@ -818,9 +818,9 @@ int Manager::Impl::exportWrappedKey(const CryptoAlgorithm &params,
        if (retCode != CKM_API_SUCCESS)
                return retCode;
 
-       if (dataTypeKey->isSKey()) {
+       if (dataTypeKey.isSKey()) {
                keyType = KeyType::KEY_AES;
-       } else if (dataTypeKey->isKeyPrivate()) {
+       } else if (dataTypeKey.isKeyPrivate()) {
                keyType = KeyType::KEY_RSA_PRIVATE;
        } else {
                return CKM_API_ERROR_INVALID_FORMAT;
index a7ea5b4..0d6d26b 100644 (file)
@@ -51,9 +51,9 @@
 namespace {
 
 #ifdef OVERRIDE_SOCKET_TIMEOUT
-const time_t SOCKET_TIMEOUT = OVERRIDE_SOCKET_TIMEOUT;
+const std::chrono::seconds SOCKET_TIMEOUT(OVERRIDE_SOCKET_TIMEOUT);
 #else // OVERRIDE_SOCKET_TIMEOUT
-const time_t SOCKET_TIMEOUT = 1000;
+const std::chrono::seconds SOCKET_TIMEOUT(1000);
 #endif // OVERRIDE_SOCKET_TIMEOUT
 
 int getCredentialsFromSocket(int sock, CKM::Credentials &cred)
@@ -76,14 +76,8 @@ int getCredentialsFromSocket(int sock, CKM::Credentials &cred)
        return 0;
 }
 
-time_t monotonicNow() {
-       struct timespec now;
-       if (clock_gettime(CLOCK_MONOTONIC_RAW, &now) == -1) {
-               int err = errno;
-               LogError("Can't access monotonic clock, error: " <<  CKM::GetErrnoString(err));
-               return 0;
-       }
-       return now.tv_sec;
+std::chrono::time_point<std::chrono::steady_clock> monotonicNow() {
+       return std::chrono::steady_clock::now();
 }
 
 } // namespace anonymous
@@ -445,12 +439,14 @@ void SocketManager::MainLoop()
                        LogDebug("No usable timeout found.");
                        ptrTimeout = NULL; // select will wait without timeout
                } else {
-                       time_t currentTime = monotonicNow();
+                       auto currentTime = monotonicNow();
                        auto &pqTimeout = m_socketDescriptionVector[m_timeoutQueue.front()].timeout;
 
                        // 0 means that select won't block and socket will be closed ;-)
                        ptrTimeout->tv_sec =
-                               currentTime < pqTimeout ? pqTimeout - currentTime : 0;
+                               currentTime < pqTimeout ?
+                               std::chrono::duration_cast<std::chrono::seconds>(pqTimeout - currentTime).count() :
+                               0;
                        ptrTimeout->tv_usec = 0;
                }
 
index 39ac3e7..1c7e295 100644 (file)
@@ -33,7 +33,7 @@
 #include <mutex>
 #include <thread>
 #include <functional>
-#include <ctime>
+#include <chrono>
 
 #include <dpl/exception.h>
 
@@ -114,7 +114,7 @@ protected:
 
                InterfaceID interfaceID;
                GenericSocketService *service;
-               time_t timeout;
+               std::chrono::time_point<std::chrono::steady_clock> timeout;
                RawBuffer rawBuffer;
                int counter;
                std::string cynaraPrivilege;
@@ -124,7 +124,6 @@ protected:
                SocketDescription() :
                        interfaceID(-1),
                        service(nullptr),
-                       timeout(0),
                        counter(0),
                        m_flags(0) {}
 
index de74c47..5366ca9 100644 (file)
@@ -1101,7 +1101,7 @@ std::tuple<CKMLogic::DBOperation, PermissionMask, int> CKMLogic::beginAndGetPerm
        const Name &name,
        const ClientId &owner)
 {
-       PermissionMask permission;
+       PermissionMask permission = Permission::NONE;
        auto [dbOp, retCode] = begin(cred, name, owner);
        if (retCode == CKM_API_SUCCESS)
                permission = toPermissionMask(dbOp.database().getPermissionRow(name, owner, cred.client));
index 24c48aa..cf15ed8 100644 (file)
@@ -281,12 +281,12 @@ KeyProvider::KeyProvider(KeyProvider &&second)
        second.m_domainKEK = NULL;
 }
 
-bool KeyProvider::isInitialized()
+bool KeyProvider::isInitialized() const
 {
        return m_isInitialized;
 }
 
-RawBuffer KeyProvider::getPureDomainKEK()
+RawBuffer KeyProvider::getPureDomainKEK() const
 {
        if (!m_isInitialized)
                ThrowErr(Exc::InternalError, "Object not initialized!");
@@ -295,7 +295,7 @@ RawBuffer KeyProvider::getPureDomainKEK()
        return RawBuffer(m_domainKEK->key, m_domainKEK->key + m_domainKEK->info.keyLength);
 }
 
-RawBuffer KeyProvider::getWrappedDomainKEK(const Password &password)
+RawBuffer KeyProvider::getWrappedDomainKEK(const Password &password) const
 {
        if (!m_isInitialized)
                ThrowErr(Exc::InternalError, "Object not initialized!");
@@ -303,7 +303,7 @@ RawBuffer KeyProvider::getWrappedDomainKEK(const Password &password)
        return wrapDomainKEK(*m_domainKEK, password);
 }
 
-RawBuffer KeyProvider::getPureDEK(const RawBuffer &wrappedDEKbuffer)
+RawBuffer KeyProvider::getPureDEK(const RawBuffer &wrappedDEKbuffer) const
 {
        if (!m_isInitialized)
                ThrowErr(Exc::InternalError, "Object not initialized!");
@@ -335,7 +335,7 @@ RawBuffer KeyProvider::getPureDEK(const RawBuffer &wrappedDEKbuffer)
        return RawBuffer(DEK.key, DEK.key + DEK.info.keyLength);
 }
 
-RawBuffer KeyProvider::generateDEK(const std::string &client)
+RawBuffer KeyProvider::generateDEK(const std::string &client) const
 {
        if (!m_isInitialized)
                ThrowErr(Exc::InternalError, "Object not initialized!");
index 1d6a6c2..2d283c6 100644 (file)
@@ -42,7 +42,7 @@ struct KeyInfo {
 
 struct DomainKEKInfo : KeyInfo {
        DomainKEKInfo() = default;
-       DomainKEKInfo(const KeyInfo& info) : KeyInfo(info) {}
+       explicit DomainKEKInfo(const KeyInfo& info) : KeyInfo(info) {}
        uint32_t version;
        uint32_t backend;
 };
@@ -109,23 +109,23 @@ public:
        KeyProvider &operator=(const KeyProvider &) = delete;
        KeyProvider &operator=(KeyProvider &&);
 
-       bool isInitialized();
+       bool isInitialized() const;
 
        // Returns Key used to decrypt database.
-       RawBuffer getPureDomainKEK();
+       RawBuffer getPureDomainKEK() const;
 
        // Returns Key in form used to store key in file
-       RawBuffer getWrappedDomainKEK(const Password &password);
+       RawBuffer getWrappedDomainKEK(const Password &password) const;
 
        // Unwraps (decrypts) a DEK using a key derived from DomainKEK and data stored in wrapped key
        // info. It returns the DEK in unencrypted form.
-       RawBuffer getPureDEK(const RawBuffer &wrappedDEKbuffer);
+       RawBuffer getPureDEK(const RawBuffer &wrappedDEKbuffer) const;
 
        // Generates a random DEK and encrypts it using a key derived from DomainKEK and custom client
        // string (not to be confused with ClientId). The function returns  the DEK in wrapped
        // (encrypted) form. The function is used to produce a key for database encryption as well as
        // application keys.
-       RawBuffer generateDEK(const std::string &client);
+       RawBuffer generateDEK(const std::string &client) const;
 
        // First run of application for some user. DomainKEK was not created yet. We must create one.
        // This key will be used to encrypt user database.
index b73e762..ab2ce10 100644 (file)
@@ -425,7 +425,7 @@ void verifyDBisValid(DBFixture &fixture)
                                                           DataType::BINARY_DATA));
                BOOST_REQUIRE((migration_names / migration_owners) == ret_list.size());
 
-               for (auto it : ret_list)
+               for (auto& it : ret_list)
                        BOOST_REQUIRE(it.first == current_owner);
 
                ret_list.clear();
index 83b6b65..5aad04b 100644 (file)
@@ -261,10 +261,10 @@ POSITIVE_TEST_CASE(T090_WriteAfterRead)
                readFd(desc, fd[0], revents);
 
                descriptors.remove(desc);
-               descriptors.add(fd2[1], POLLOUT, [&](int desc, short revents) {
+               descriptors.add(fd2[1], POLLOUT, [&](int desc2, short revents) {
                        callback2 = true;
-                       writeFd(desc, fd2[1], revents);
-                       descriptors.remove(desc);
+                       writeFd(desc2, fd2[1], revents);
+                       descriptors.remove(desc2);
                });
        });