From: Krzysztof Jackiewicz Date: Mon, 3 Apr 2023 15:46:07 +0000 (+0200) Subject: Fix svace/coverity issues X-Git-Tag: accepted/tizen/unified/20230406.165733~1 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=55ac958364fb4a335d0089804571d66e7f1d2f7d;p=platform%2Fcore%2Fsecurity%2Fkey-manager.git Fix svace/coverity issues Change-Id: I681fd80cddf5f56bc99b35546940e111d29a5311 --- diff --git a/doc/examples/key_derive.cpp b/doc/examples/key_derive.cpp index 79c65070..39f2706e 100644 --- a/doc/examples/key_derive.cpp +++ b/doc/examples/key_derive.cpp @@ -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(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(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] diff --git a/src/manager/client-async/descriptor-set.cpp b/src/manager/client-async/descriptor-set.cpp index fdee29db..316ea09f 100644 --- a/src/manager/client-async/descriptor-set.cpp +++ b/src/manager/client-async/descriptor-set.cpp @@ -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(); diff --git a/src/manager/client-capi/ckmc-manager.cpp b/src/manager/client-capi/ckmc-manager.cpp index 083ee287..24aa8cba 100644 --- a/src/manager/client-capi/ckmc-manager.cpp +++ b/src/manager/client-capi/ckmc-manager.cpp @@ -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, diff --git a/src/manager/client/client-manager-impl.cpp b/src/manager/client/client-manager-impl.cpp index 8628dafe..b7881971 100644 --- a/src/manager/client/client-manager-impl.cpp +++ b/src/manager/client/client-manager-impl.cpp @@ -801,7 +801,7 @@ int Manager::Impl::exportWrappedKey(const CryptoAlgorithm ¶ms, 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 ¶ms, 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; diff --git a/src/manager/main/socket-manager.cpp b/src/manager/main/socket-manager.cpp index a7ea5b47..0d6d26b1 100644 --- a/src/manager/main/socket-manager.cpp +++ b/src/manager/main/socket-manager.cpp @@ -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 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(pqTimeout - currentTime).count() : + 0; ptrTimeout->tv_usec = 0; } diff --git a/src/manager/main/socket-manager.h b/src/manager/main/socket-manager.h index 39ac3e72..1c7e295c 100644 --- a/src/manager/main/socket-manager.h +++ b/src/manager/main/socket-manager.h @@ -33,7 +33,7 @@ #include #include #include -#include +#include #include @@ -114,7 +114,7 @@ protected: InterfaceID interfaceID; GenericSocketService *service; - time_t timeout; + std::chrono::time_point 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) {} diff --git a/src/manager/service/ckm-logic.cpp b/src/manager/service/ckm-logic.cpp index de74c470..5366ca96 100644 --- a/src/manager/service/ckm-logic.cpp +++ b/src/manager/service/ckm-logic.cpp @@ -1101,7 +1101,7 @@ std::tuple 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)); diff --git a/src/manager/service/key-provider.cpp b/src/manager/service/key-provider.cpp index 24c48aaf..cf15ed8f 100644 --- a/src/manager/service/key-provider.cpp +++ b/src/manager/service/key-provider.cpp @@ -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!"); diff --git a/src/manager/service/key-provider.h b/src/manager/service/key-provider.h index 1d6a6c20..2d283c62 100644 --- a/src/manager/service/key-provider.h +++ b/src/manager/service/key-provider.h @@ -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. diff --git a/unit-tests/test_db_crypto.cpp b/unit-tests/test_db_crypto.cpp index b73e7624..ab2ce10a 100644 --- a/unit-tests/test_db_crypto.cpp +++ b/unit-tests/test_db_crypto.cpp @@ -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(); diff --git a/unit-tests/test_descriptor-set.cpp b/unit-tests/test_descriptor-set.cpp index 83b6b653..5aad04b0 100644 --- a/unit-tests/test_descriptor-set.cpp +++ b/unit-tests/test_descriptor-set.cpp @@ -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); }); });