From 5907debce03abecb8fa799e807dfa123fec116a2 Mon Sep 17 00:00:00 2001 From: Pawel Wieczorek Date: Mon, 6 Oct 2014 19:08:25 +0200 Subject: [PATCH 01/16] Add PolicyBucketId validation This patch introduces mechanism for checking whether new PolicyBucketId contains forbidden characters. Now only alphanumeric characters, hyphen and underscore can be used in PolicyBucketId. InvalidBucketIdException is thrown and OPERATION_NOT_ALLOWED error code is returned otherwise. Change-Id: I48b6e14d20cb62adc17560929055553df0ce1077 --- src/common/exceptions/InvalidBucketIdException.h | 55 ++++++++++++++++++++++++ src/common/types/PolicyBucket.cpp | 34 +++++++++++++++ src/common/types/PolicyBucket.h | 30 ++++++------- src/service/logic/Logic.cpp | 3 ++ test/common/types/policybucket.cpp | 36 ++++++++++++++++ 5 files changed, 140 insertions(+), 18 deletions(-) create mode 100644 src/common/exceptions/InvalidBucketIdException.h diff --git a/src/common/exceptions/InvalidBucketIdException.h b/src/common/exceptions/InvalidBucketIdException.h new file mode 100644 index 0000000..f8772e4 --- /dev/null +++ b/src/common/exceptions/InvalidBucketIdException.h @@ -0,0 +1,55 @@ +/* + * Copyright (c) 2014 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 src/common/exceptions/InvalidBucketIdException.h + * @author Pawel Wieczorek + * @version 0.1 + * @brief This file defines exception thrown when bucket ID contains forbidden characters + */ + +#ifndef SRC_COMMON_EXCEPTIONS_INVALIDBUCKETIDEXCEPTION_H_ +#define SRC_COMMON_EXCEPTIONS_INVALIDBUCKETIDEXCEPTION_H_ + +#include + +#include + +namespace Cynara { + +class InvalidBucketIdException : public Exception { +public: + InvalidBucketIdException(const std::string &bucketId) : m_bucketId(bucketId) {}; + virtual ~InvalidBucketIdException() {}; + + const std::string message(void) const { + if (m_message.empty()) { + m_message = "Bucket ID " + bucketId() + " contains forbidden characters"; + } + return m_message; + } + + const std::string &bucketId(void) const { + return m_bucketId; + } + +private: + mutable std::string m_message; + std::string m_bucketId; +}; + +} /* namespace Cynara */ + +#endif /* SRC_COMMON_EXCEPTIONS_INVALIDBUCKETIDEXCEPTION_H_ */ diff --git a/src/common/types/PolicyBucket.cpp b/src/common/types/PolicyBucket.cpp index f1a45a1..622bc05 100644 --- a/src/common/types/PolicyBucket.cpp +++ b/src/common/types/PolicyBucket.cpp @@ -22,6 +22,7 @@ #include +#include #include #include @@ -29,6 +30,25 @@ namespace Cynara { +const std::string PolicyBucket::m_idSeparators = "-_"; + +PolicyBucket::PolicyBucket(const PolicyBucketId &id, const PolicyResult &defaultPolicy) + : m_defaultPolicy(defaultPolicy), m_id(id) { + idValidator(id); +} + +PolicyBucket::PolicyBucket(const PolicyBucketId &id, const PolicyCollection &policies) + : m_policyCollection(makePolicyMap(policies)), m_defaultPolicy(PredefinedPolicyType::DENY), + m_id(id) { + idValidator(id); +} + +PolicyBucket::PolicyBucket(const PolicyBucketId &id, const PolicyResult &defaultPolicy, + const PolicyCollection &policies) + : m_policyCollection(makePolicyMap(policies)), m_defaultPolicy(defaultPolicy), m_id(id) { + idValidator(id); +} + PolicyBucket PolicyBucket::filtered(const PolicyKey &key) const { PolicyBucket result(m_id + "_filtered"); @@ -78,4 +98,18 @@ PolicyMap PolicyBucket::makePolicyMap(const PolicyCollection &policies) { return result; } +void PolicyBucket::idValidator(const PolicyBucketId &id) { + auto isCharInvalid = [] (char c) { + return !(std::isalnum(c) || isIdSeparator(c)); + }; + + if (id.end() != find_if(id.begin(), id.end(), isCharInvalid)) { + throw InvalidBucketIdException(id); + } +} + +bool PolicyBucket::isIdSeparator(char c) { + return m_idSeparators.end() != find(m_idSeparators.begin(), m_idSeparators.end(), c); +} + } // namespace Cynara diff --git a/src/common/types/PolicyBucket.h b/src/common/types/PolicyBucket.h index a2faf9c..df57d0f 100644 --- a/src/common/types/PolicyBucket.h +++ b/src/common/types/PolicyBucket.h @@ -52,20 +52,12 @@ public: //delete default constructor in order to prevent creation of buckets with no id PolicyBucket() = delete; PolicyBucket(const PolicyBucketId &id, - const PolicyResult &defaultPolicy = PredefinedPolicyType::DENY) - : m_defaultPolicy(defaultPolicy), - m_id(id) {} + const PolicyResult &defaultPolicy = PredefinedPolicyType::DENY); PolicyBucket(const PolicyBucketId &id, - const PolicyCollection &policies) - : m_policyCollection(makePolicyMap(policies)), - m_defaultPolicy(PredefinedPolicyType::DENY), - m_id(id) {} + const PolicyCollection &policies); PolicyBucket(const PolicyBucketId &id, const PolicyResult &defaultPolicy, - const PolicyCollection &policies) - : m_policyCollection(makePolicyMap(policies)), - m_defaultPolicy(defaultPolicy), - m_id(id) {} + const PolicyCollection &policies); PolicyBucket filtered(const PolicyKey &key) const; void insertPolicy(PolicyPtr policy); @@ -76,13 +68,6 @@ public: static PolicyMap makePolicyMap(const PolicyCollection &policies); -private: - PolicyMap m_policyCollection; - PolicyResult m_defaultPolicy; - PolicyBucketId m_id; - -public: - const_policy_iterator begin(void) const { return const_policy_iterator(m_policyCollection.begin()); } @@ -111,6 +96,15 @@ public: void setDefaultPolicy(const PolicyResult &defaultPolicy) { m_defaultPolicy = defaultPolicy; } + +private: + static void idValidator(const PolicyBucketId &id); + static bool isIdSeparator(char c); + + PolicyMap m_policyCollection; + PolicyResult m_defaultPolicy; + PolicyBucketId m_id; + static const std::string m_idSeparators; }; } /* namespace Cynara */ diff --git a/src/service/logic/Logic.cpp b/src/service/logic/Logic.cpp index eeed387..f76ce98 100644 --- a/src/service/logic/Logic.cpp +++ b/src/service/logic/Logic.cpp @@ -26,6 +26,7 @@ #include #include #include +#include #include #include @@ -134,6 +135,8 @@ void Logic::execute(RequestContextPtr context, InsertOrUpdateBucketRequestPtr re code = CodeResponse::Code::FAILED; } catch (const DefaultBucketSetNoneException &ex) { code = CodeResponse::Code::NOT_ALLOWED; + } catch (const InvalidBucketIdException &ex) { + code = CodeResponse::Code::NOT_ALLOWED; } context->returnResponse(context, std::make_shared(code, diff --git a/test/common/types/policybucket.cpp b/test/common/types/policybucket.cpp index 983a040..6c5d7a4 100644 --- a/test/common/types/policybucket.cpp +++ b/test/common/types/policybucket.cpp @@ -27,6 +27,7 @@ #include #include +#include "exceptions/InvalidBucketIdException.h" #include "types/PolicyBucket.h" #include "types/PolicyCollection.h" #include "types/PolicyKey.h" @@ -57,6 +58,15 @@ protected: Policy::simpleWithKey(PolicyKey("*", "*", "p1"), PredefinedPolicyType::ALLOW), Policy::simpleWithKey(PolicyKey("*", "*", "*"), PredefinedPolicyType::ALLOW) }; + + const std::vector goodIds = { + "_goodid", "good_id", "goodid_", "-goodid", "good-id", "goodid-" + }; + + const std::vector badIds = { + "{badid", "bad[id", "badid~", "/badid", "bad*id", "badid|", "badid;", "\tbadid", "badid\n", + " badid", "bad id", "badid " + }; }; TEST_F(PolicyBucketFixture, filtered) { @@ -141,3 +151,29 @@ TEST_F(PolicyBucketFixture, filtered_wildcard_none) { auto filtered = bucket.filtered(PolicyKey("cccc", "uuuu", "pppp")); ASSERT_THAT(filtered, IsEmpty()); } + +/** + * @brief Validate PolicyBucketIds during creation - passing bucket ids + * @test Scenario: + * - Iterate through vector of valid bucket ids and create them normally + * - PolicyBucket constructor should not throw any exception + */ +TEST_F(PolicyBucketFixture, bucket_id_validation_passes) { + for (auto it = goodIds.begin(); it != goodIds.end(); ++it) { + SCOPED_TRACE(*it); + ASSERT_NO_THROW(PolicyBucket(PolicyBucketId(*it))); + } +} + +/** + * @brief Validate PolicyBucketIds during creation - failing bucket ids + * @test Scenario: + * - Iterate through vector of bucket ids containing forbidden characters + * - PolicyBucket constructor should throw an exception every time it is called + */ +TEST_F(PolicyBucketFixture, bucket_id_validation_fails) { + for (auto it = badIds.begin(); it != badIds.end(); ++it) { + SCOPED_TRACE(*it); + ASSERT_THROW(PolicyBucket(PolicyBucketId(*it)), InvalidBucketIdException); + } +} -- 2.7.4 From bf1744e399095b89fb655787414d2d948f413281 Mon Sep 17 00:00:00 2001 From: Lukasz Wojciechowski Date: Fri, 24 Oct 2014 17:52:32 +0200 Subject: [PATCH 02/16] Treat invalid check_id as an error in async cancel If check_id passed to cynara_async_cancel_request() is invalid CYNARA_API_INVALID_PARAM will be returned. Id is invalid when: * was never generated by any previous call to cynara_async_create_request(); * response callback related to this id was already delivered. Change-Id: Iaa05fe71c752aedcb5414d162fc374f37420f36d --- src/client-async/logic/Logic.cpp | 2 +- src/include/cynara-client-async.h | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/client-async/logic/Logic.cpp b/src/client-async/logic/Logic.cpp index 3a92b2c..8babbed 100644 --- a/src/client-async/logic/Logic.cpp +++ b/src/client-async/logic/Logic.cpp @@ -112,7 +112,7 @@ int Logic::cancelRequest(cynara_check_id checkId) { auto it = m_checks.find(checkId); if (it == m_checks.end() || it->second.cancelled()) - return CYNARA_API_SUCCESS; + return CYNARA_API_INVALID_PARAM; m_socketClient->appendRequest(std::make_shared(it->first)); diff --git a/src/include/cynara-client-async.h b/src/include/cynara-client-async.h index 9bed89d..536e5d7 100644 --- a/src/include/cynara-client-async.h +++ b/src/include/cynara-client-async.h @@ -386,6 +386,8 @@ int cynara_async_process(cynara_async *p_cynara); * cynara_status_callback callback may be triggered to be able to send cancel to cynara. * cynara_response_callback callback will be triggered with with * cynara_async_call_cause::CYNARA_CALL_CAUSE_CANCEL as cause param. + * If given id is not valid (was not requested or response callback was already delivered) + * cynara_async_cancel_request() returns CYNARA_API_INVALID_PARAM. * * \par Sync (or) Async: * This is a synchronous API. -- 2.7.4 From f7fc977dc46be0209bc54e9b40e0b3870fd34c39 Mon Sep 17 00:00:00 2001 From: Pawel Wieczorek Date: Wed, 22 Oct 2014 11:51:19 +0200 Subject: [PATCH 03/16] Add migration tool for Cynara's database This patch introduces tool for database migration if newer version of Cynara uses backward incompatible format of storing policies data. Migration tool is also used during installation of Cynara in order to initialize database with minimal contents. Change-Id: I7e6a376dad812c54f45a6a11ca559c97383d453d --- CMakeLists.txt | 6 ++ migration/CMakeLists.txt | 24 +++++ migration/cynara-db-migration.sh | 163 +++++++++++++++++++++++++++++++++ packaging/cynara-db-migration.manifest | 5 + packaging/cynara.spec | 23 +++++ 5 files changed, 221 insertions(+) create mode 100644 migration/CMakeLists.txt create mode 100644 migration/cynara-db-migration.sh create mode 100644 packaging/cynara-db-migration.manifest diff --git a/CMakeLists.txt b/CMakeLists.txt index ba5fe97..1c72d22 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -40,6 +40,11 @@ SET(BIN_INSTALL_DIR CACHE PATH "Binary installation directory") +SET(SBIN_INSTALL_DIR + "${CMAKE_INSTALL_PREFIX}/sbin" + CACHE PATH + "Administrative binary installation directory") + SET(INCLUDE_INSTALL_DIR "${CMAKE_INSTALL_PREFIX}/include" CACHE PATH @@ -92,6 +97,7 @@ SET(TARGET_LIB_CYNARA_STORAGE "cynara-storage") ADD_SUBDIRECTORY(src) ADD_SUBDIRECTORY(pkgconfig) ADD_SUBDIRECTORY(systemd) +ADD_SUBDIRECTORY(migration) IF (BUILD_TESTS) ADD_SUBDIRECTORY(test) diff --git a/migration/CMakeLists.txt b/migration/CMakeLists.txt new file mode 100644 index 0000000..53e10d5 --- /dev/null +++ b/migration/CMakeLists.txt @@ -0,0 +1,24 @@ +# Copyright (c) 2014 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 CMakeLists.txt +# @author Pawel Wieczorek +# @brief CMake for migration tool +# + +INSTALL(FILES + ${CMAKE_BINARY_DIR}/migration/cynara-db-migration.sh + DESTINATION + ${SBIN_INSTALL_DIR}/cynara/ + ) diff --git a/migration/cynara-db-migration.sh b/migration/cynara-db-migration.sh new file mode 100644 index 0000000..0cf5edd --- /dev/null +++ b/migration/cynara-db-migration.sh @@ -0,0 +1,163 @@ +#!/bin/sh +# +# Copyright (c) 2014 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 migration/cynara-db-migration.sh +# @author Pawel Wieczorek +# @brief Migration tool for Cynara's database +# + +##### Constants (these must not be modified by shell) + +STATE_PATH='/var/cynara' +DB_DIR='db' +INDEX_NAME='buckets' +DEFAULT_NAME='_' +DENY_POLICY=';0x0;' + +##### Variables, with default values (optional) + +CYNARA_USER='cynara' +CYNARA_GROUP='cynara' + +SMACK_LABEL='System' + +OLD_VERSION= +NEW_VERSION= + +##### Functions + +parse_opts() { + while getopts ":f:t:u:g:l:" opt; do + case $opt in + f ) + OLD_VERSION=${OPTARG} + ;; + t ) + NEW_VERSION=${OPTARG} + ;; + u ) + CYNARA_USER=${OPTARG} + ;; + g ) + CYNARA_GROUP=${OPTARG} + ;; + l ) + SMACK_LABEL=${OPTARG} + ;; + \? ) + echo "Invalid option: -$OPTARG" >&2 + failure + ;; + : ) + echo "Option -$OPTARG requires an argument." >&2 + failure + ;; + esac + done +} + +create_db() { + if [ -z ${NEW_VERSION} ]; then + failure + fi + + # Create Cynara's database directory: + mkdir -p ${STATE_PATH}/${DB_DIR} + + # Create contents of minimal database: first index file, then default bucket + echo ${DENY_POLICY} > ${STATE_PATH}/${DB_DIR}/${INDEX_NAME} + touch ${STATE_PATH}/${DB_DIR}/${DEFAULT_NAME} + + # Set proper permissions for newly created database + chown -R ${CYNARA_USER}:${CYNARA_GROUP} ${STATE_PATH}/${DB_DIR} + + # Set proper SMACK labels for newly created database + chsmack -a ${SMACK_LABEL} ${STATE_PATH}/${DB_DIR} + chsmack -a ${SMACK_LABEL} ${STATE_PATH}/${DB_DIR}/* +} + +migrate_db() { + if [ -z ${OLD_VERSION} -o -z ${NEW_VERSION} ]; then + failure + fi + + : + # : is a null command, as functions may not be empty. + # Actual body will be added if database structure changes. +} + +remove_db() { + if [ -z ${OLD_VERSION} ]; then + failure + fi + + rm -rf ${STATE_PATH} +} + +usage() { + cat << EOF +Usage: $0 COMMAND OPTIONS + +This script installs, migrates to another version or removes Cynara's policies database + +Commands: + upgrade (up) migrate old data to new version of database structure + install (in) create minimal database + uninstall (rm) remove database entirely + +Options: + -f from Set old version of database (mandatory for upgrade and uninstall) + -t to Set new version of database (mandatory for upgrade and install) + -u user Set database owner (default: cynara) + -g group Set database group (default: cynara) + -l label Set SMACK label for database (default: System) + -h Show this help message +EOF +} + +failure() { + usage + exit 1 +} + +##### Main + +if [ 0 -eq $# ]; then + failure +fi + +case $1 in + "up" | "upgrade" ) + shift $OPTIND + parse_opts "$@" + migrate_db + ;; + "in" | "install" ) + shift $OPTIND + parse_opts "$@" + create_db + ;; + "rm" | "uninstall" ) + shift $OPTIND + parse_opts "$@" + remove_db + ;; + "-h" | "--help" ) + usage + ;; + * ) + failure +esac diff --git a/packaging/cynara-db-migration.manifest b/packaging/cynara-db-migration.manifest new file mode 100644 index 0000000..a76fdba --- /dev/null +++ b/packaging/cynara-db-migration.manifest @@ -0,0 +1,5 @@ + + + + + diff --git a/packaging/cynara.spec b/packaging/cynara.spec index ffcccfb..35ca376 100644 --- a/packaging/cynara.spec +++ b/packaging/cynara.spec @@ -17,10 +17,13 @@ Source1009: libcynara-creds-dbus.manifest Source1010: libcynara-creds-socket.manifest Source1011: libcynara-session.manifest Source1012: libcynara-storage.manifest +Source1013: cynara-db-migration.manifest Requires: default-ac-domains Requires(pre): pwdutils +Requires(pre): cynara-db-migration >= %{version}-%{release} Requires(post): smack Requires(postun): pwdutils +Requires(postun): cynara-db-migration >= %{version}-%{release} BuildRequires: cmake BuildRequires: zip BuildRequires: pkgconfig(libsystemd-daemon) @@ -226,6 +229,13 @@ Requires: cynara = %{version}-%{release} %description -n cynara-devel service (devel version) +####################################################### +%package -n cynara-db-migration +Summary: Migration tools for Cynara's database + +%description -n cynara-db-migration +Migration tools for Cynara's database + %prep %setup -q cp -a %{SOURCE1001} . @@ -240,6 +250,7 @@ cp -a %{SOURCE1009} . cp -a %{SOURCE1010} . cp -a %{SOURCE1011} . cp -a %{SOURCE1012} . +cp -a %{SOURCE1013} . cp -a test/db/db* . %build @@ -287,6 +298,13 @@ if [ $? -eq 1 ]; then useradd -d /var/lib/empty -s /sbin/nologin -r -g %{group_name} %{user_name} > /dev/null 2>&1 fi +if [ $1 -gt 1 ] ; then + OLDVERSION="$(rpm -q --qf '%%{version}' %{name})" + %{_sbindir}/cynara/cynara-db-migration.sh upgrade -f ${OLDVERSION} -t %{version} +else + %{_sbindir}/cynara/cynara-db-migration.sh install -t %{version} +fi + %post ### Add file capabilities if needed ### setcap/getcap binary are useful. To use them you must install libcap and libcap-tools packages @@ -310,6 +328,7 @@ fi %postun if [ $1 = 0 ]; then + %{_sbindir}/cynara/cynara-db-migration.sh uninstall -f %{version} userdel -r %{user_name} > /dev/null 2>&1 groupdel %{user_name} > /dev/null 2>&1 systemctl daemon-reload @@ -515,3 +534,7 @@ fi %{_includedir}/cynara/cynara-session.h %{_libdir}/libcynara-session.so %{_libdir}/pkgconfig/cynara-session.pc + +%files -n cynara-db-migration +%manifest cynara-db-migration.manifest +%attr(744,root,root) %{_sbindir}/cynara/cynara-db-migration.sh -- 2.7.4 From 11ba3c703cfb535b7ec33b7ed23fee7c34fcc374 Mon Sep 17 00:00:00 2001 From: Lukasz Wojciechowski Date: Mon, 3 Nov 2014 08:25:37 +0100 Subject: [PATCH 04/16] Remove visibility attributes from header file Visibility attributes ar not needed in header file. Usage of them by CYNARA_API macro causes also to make internal file attributes.h published. Change-Id: I99bb84d5af96120cdc448e837601cecc05494570 --- src/include/cynara-creds-commons.h | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/include/cynara-creds-commons.h b/src/include/cynara-creds-commons.h index 813c1b9..ee6d2b8 100644 --- a/src/include/cynara-creds-commons.h +++ b/src/include/cynara-creds-commons.h @@ -26,7 +26,6 @@ #ifndef CYNARA_CREDS_COMMONS_H #define CYNARA_CREDS_COMMONS_H -#include #include enum cynara_client_creds { @@ -78,7 +77,6 @@ extern "C" { * CYNARA_API_UNKNOWN_ERROR if there is other error. * */ -CYNARA_API int cynara_creds_get_default_client_method(enum cynara_client_creds *method); /** @@ -115,7 +113,6 @@ int cynara_creds_get_default_client_method(enum cynara_client_creds *method); * CYNARA_API_OUT_OF_MEMORY if there is error in memory allocation. * CYNARA_API_UNKNOWN_ERROR if there is other error. */ -CYNARA_API int cynara_creds_get_default_user_method(enum cynara_user_creds *method); #ifdef __cplusplus -- 2.7.4 From fa40410ab295da5e6847945c75f66b8a6027137d Mon Sep 17 00:00:00 2001 From: Marcin Niesluchowski Date: Tue, 4 Nov 2014 18:09:54 +0100 Subject: [PATCH 05/16] Change Group in spec file Group Security/Access Control has been removed. Cynara current group is Security/Application Privilege. Nonexistent group causes build break. Change-Id: I58d800209cb232e60e60747eb79244fb57c7b977 --- packaging/cynara.spec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packaging/cynara.spec b/packaging/cynara.spec index 35ca376..e390eb7 100644 --- a/packaging/cynara.spec +++ b/packaging/cynara.spec @@ -2,7 +2,7 @@ Name: cynara Summary: Cynara service with client libraries Version: 0.3.0 Release: 1 -Group: Security/Access Control +Group: Security/Application Privilege License: Apache-2.0 Source0: %{name}-%{version}.tar.gz Source1001: cynara.manifest -- 2.7.4 From 0bbfec596d7f4072fab1bc3f29bc413736ec416a Mon Sep 17 00:00:00 2001 From: Adam Malinowski Date: Thu, 6 Nov 2014 14:47:15 +0100 Subject: [PATCH 06/16] Fix build break caused by wrong system group names cynara-rpmlintrc file was added to project in order to ignore errors related to wrong group names. File will be removed when problems with new group names is fixed. Change-Id: Ibd0ee42b707fba059f0172522cba4804c28d2cb5 --- packaging/cynara-rpmlintrc | 1 + packaging/cynara.spec | 1 + 2 files changed, 2 insertions(+) create mode 100644 packaging/cynara-rpmlintrc diff --git a/packaging/cynara-rpmlintrc b/packaging/cynara-rpmlintrc new file mode 100644 index 0000000..1fad581 --- /dev/null +++ b/packaging/cynara-rpmlintrc @@ -0,0 +1 @@ +setBadness('non-standard-group', 0) diff --git a/packaging/cynara.spec b/packaging/cynara.spec index e390eb7..213c94e 100644 --- a/packaging/cynara.spec +++ b/packaging/cynara.spec @@ -5,6 +5,7 @@ Release: 1 Group: Security/Application Privilege License: Apache-2.0 Source0: %{name}-%{version}.tar.gz +Source1000: %{name}-rpmlintrc Source1001: cynara.manifest Source1002: libcynara-client.manifest Source1003: libcynara-client-async.manifest -- 2.7.4 From 626099ee7f86f15d9d1e62826ecca418598e3a6c Mon Sep 17 00:00:00 2001 From: Marcin Niesluchowski Date: Wed, 5 Nov 2014 12:40:35 +0100 Subject: [PATCH 07/16] Fix read errno handling in Socket class Cynara Socket class treats ECONNRESET (socket closed transmiting RST instead of FIN) during read as unknown error. Handle it as disconnection. Change-Id: Iecbfa5c32c7ef8b6b5da97170269aa86e2740c22 --- src/common/sockets/Socket.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/common/sockets/Socket.cpp b/src/common/sockets/Socket.cpp index 0c33a7e..5a57a54 100644 --- a/src/common/sockets/Socket.cpp +++ b/src/common/sockets/Socket.cpp @@ -269,6 +269,9 @@ bool Socket::receiveFromServer(BinaryQueue &queue) { case EWOULDBLOCK: #endif return true; + case ECONNRESET: + LOGW("read returned -1 with ECONNRESET / Connection closed by server."); + return false; default: LOGE("'read' function error [%d] : <%s>", err, strerror(err)); throw UnexpectedErrorException(err, strerror(err)); -- 2.7.4 From aaf46e4b88c42b633e731d526010ebe2a5197b42 Mon Sep 17 00:00:00 2001 From: Rafal Krypa Date: Mon, 10 Nov 2014 13:43:59 +0100 Subject: [PATCH 08/16] Fix invocations of LOG missing format string argument MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit First argument of LOG* macros is passed to sd_journal_print() as format string. In some places these macros were used with no format string at all, simply passing e.what() from an exception. This could lead to a format string vulnerability in the code, potentially allowing arbitrary code execution. This error also caused build break: In file included from /data/src/tizen/cynara/src/client/api/client-api.cpp:27:0: /data/src/tizen/cynara/src/common/exceptions/TryCatch.h: In function ‘int Cynara::tryCatch(const std::function&)’: /data/src/tizen/cynara/src/common/exceptions/TryCatch.h:41:178: error: format not a string literal and no format arguments [-Werror=format-security] LOGE(e.what()); (... and more ...) Change-Id: I1259283cf1bd2fa0fb2d271e38a7b416e17939f7 Signed-off-by: Rafal Krypa --- src/admin/api/admin-api.cpp | 2 +- src/client-async/api/client-async-api.cpp | 4 ++-- src/client/api/client-api.cpp | 2 +- src/common/exceptions/TryCatch.h | 6 +++--- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/admin/api/admin-api.cpp b/src/admin/api/admin-api.cpp index e444b7a..1e73ff7 100644 --- a/src/admin/api/admin-api.cpp +++ b/src/admin/api/admin-api.cpp @@ -197,7 +197,7 @@ int cynara_admin_check(struct cynara_admin *p_cynara_admin, userStr = user; privilegeStr = privilege; } catch (const std::length_error &e) { - LOGE(e.what()); + LOGE("%s", e.what()); return CYNARA_API_INVALID_PARAM; } diff --git a/src/client-async/api/client-async-api.cpp b/src/client-async/api/client-async-api.cpp index d350e10..1e3479e 100644 --- a/src/client-async/api/client-async-api.cpp +++ b/src/client-async/api/client-async-api.cpp @@ -84,7 +84,7 @@ int cynara_async_check_cache(cynara_async *p_cynara, const char *client, const c userStr = user; privilegeStr = privilege; } catch (const std::length_error &e) { - LOGE(e.what()); + LOGE("%s", e.what()); return CYNARA_API_INVALID_PARAM; } return p_cynara->impl->checkCache(clientStr, clientSessionStr, userStr, privilegeStr); @@ -113,7 +113,7 @@ int cynara_async_create_request(cynara_async *p_cynara, const char *client, userStr = user; privilegeStr = privilege; } catch (const std::length_error &e) { - LOGE(e.what()); + LOGE("%s", e.what()); return CYNARA_API_INVALID_PARAM; } cynara_check_id checkId; diff --git a/src/client/api/client-api.cpp b/src/client/api/client-api.cpp index 4eb4625..935c6cd 100644 --- a/src/client/api/client-api.cpp +++ b/src/client/api/client-api.cpp @@ -88,7 +88,7 @@ int cynara_check(cynara *p_cynara, const char *client, const char *client_sessio userStr = user; privilegeStr = privilege; } catch (const std::length_error &e) { - LOGE(e.what()); + LOGE("%s", e.what()); return CYNARA_API_INVALID_PARAM; } return p_cynara->impl->check(clientStr, clientSessionStr, userStr, privilegeStr); diff --git a/src/common/exceptions/TryCatch.h b/src/common/exceptions/TryCatch.h index b1ef172..3964c01 100644 --- a/src/common/exceptions/TryCatch.h +++ b/src/common/exceptions/TryCatch.h @@ -38,13 +38,13 @@ int tryCatch(const std::function &func) { try { return func(); } catch (const std::bad_alloc &e) { - LOGE(e.what()); + LOGE("%s", e.what()); return CYNARA_API_OUT_OF_MEMORY; } catch (const NoMemoryException &e) { - LOGE(e.what()); + LOGE("%s", e.what()); return CYNARA_API_OUT_OF_MEMORY; } catch (const std::exception &e) { - LOGE(e.what()); + LOGE("%s", e.what()); return CYNARA_API_UNKNOWN_ERROR; } catch (...) { LOGE("Unexpected exception"); -- 2.7.4 From 0ded5caa100a6802eb2f3e45e7b2073f0cef95eb Mon Sep 17 00:00:00 2001 From: Pawel Wieczorek Date: Wed, 27 Aug 2014 09:18:02 +0200 Subject: [PATCH 09/16] Implement mechanism assuring integrity of database There is also added mechanism for cleaning up Cynara's database directory upon loading policies to memory. There is added test checking whether mechanism behaves as intended. Change-Id: I926d1aebf394c092e00731b73717e0e1c55bad0c --- packaging/cynara.spec | 3 +- src/storage/CMakeLists.txt | 1 + src/storage/InMemoryStorageBackend.cpp | 46 +++- src/storage/InMemoryStorageBackend.h | 10 +- src/storage/Integrity.cpp | 232 +++++++++++++++++++++ src/storage/Integrity.h | 71 +++++++ test/CMakeLists.txt | 1 + test/db/db6/_ | 0 test/db/db6/_additional | 1 + test/db/db6/_additional~ | 0 test/db/db6/_~ | 0 test/db/db6/buckets | 2 + test/db/db6/buckets~ | 2 + test/db/db6/guard | 0 .../fakeinmemorystoragebackend.h | 2 +- .../inmemeorystoragebackendfixture.h | 2 + .../inmemorystoragebackend.cpp | 38 +++- 17 files changed, 396 insertions(+), 15 deletions(-) create mode 100644 src/storage/Integrity.cpp create mode 100644 src/storage/Integrity.h create mode 100644 test/db/db6/_ create mode 100644 test/db/db6/_additional create mode 100644 test/db/db6/_additional~ create mode 100644 test/db/db6/_~ create mode 100644 test/db/db6/buckets create mode 100644 test/db/db6/buckets~ create mode 100644 test/db/db6/guard diff --git a/packaging/cynara.spec b/packaging/cynara.spec index 213c94e..68d52ce 100644 --- a/packaging/cynara.spec +++ b/packaging/cynara.spec @@ -283,7 +283,7 @@ cp ./conf/creds.conf %{buildroot}/%{conf_path}/creds.conf mkdir -p %{buildroot}/usr/lib/systemd/system/sockets.target.wants mkdir -p %{buildroot}/%{state_path} -mkdir -p %{buildroot}/%{tests_dir} +mkdir -p %{buildroot}/%{tests_dir}/empty_db cp -a db* %{buildroot}/%{tests_dir} ln -s ../cynara.socket %{buildroot}/usr/lib/systemd/system/sockets.target.wants/cynara.socket ln -s ../cynara-admin.socket %{buildroot}/usr/lib/systemd/system/sockets.target.wants/cynara-admin.socket @@ -494,6 +494,7 @@ fi %manifest cynara-tests.manifest %attr(755,root,root) /usr/bin/cynara-tests %attr(755,root,root) %{tests_dir}/db*/* +%dir %attr(755,root,root) %{tests_dir}/empty_db %files -n libcynara-creds-commons %manifest libcynara-creds-commons.manifest diff --git a/src/storage/CMakeLists.txt b/src/storage/CMakeLists.txt index 620dc2d..7bccc8c 100644 --- a/src/storage/CMakeLists.txt +++ b/src/storage/CMakeLists.txt @@ -25,6 +25,7 @@ SET(CYNARA_LIB_CYNARA_STORAGE_PATH ${CYNARA_PATH}/storage) SET(LIB_CYNARA_STORAGE_SOURCES ${CYNARA_LIB_CYNARA_STORAGE_PATH}/BucketDeserializer.cpp ${CYNARA_LIB_CYNARA_STORAGE_PATH}/InMemoryStorageBackend.cpp + ${CYNARA_LIB_CYNARA_STORAGE_PATH}/Integrity.cpp ${CYNARA_LIB_CYNARA_STORAGE_PATH}/Storage.cpp ${CYNARA_LIB_CYNARA_STORAGE_PATH}/StorageDeserializer.cpp ${CYNARA_LIB_CYNARA_STORAGE_PATH}/StorageSerializer.cpp diff --git a/src/storage/InMemoryStorageBackend.cpp b/src/storage/InMemoryStorageBackend.cpp index af18926..a2366e8 100644 --- a/src/storage/InMemoryStorageBackend.cpp +++ b/src/storage/InMemoryStorageBackend.cpp @@ -30,7 +30,6 @@ #include #include #include -#include #include #include @@ -43,6 +42,7 @@ #include #include +#include #include #include @@ -50,17 +50,28 @@ namespace Cynara { -const std::string InMemoryStorageBackend::m_indexFileName = "buckets"; +const std::string InMemoryStorageBackend::m_indexFilename = "buckets"; +const std::string InMemoryStorageBackend::m_backupFilenameSuffix = "~"; +const std::string InMemoryStorageBackend::m_bucketFilenamePrefix = "_"; void InMemoryStorageBackend::load(void) { - std::string indexFilename = m_dbPath + m_indexFileName; + Integrity integrity(m_dbPath, m_indexFilename, m_backupFilenameSuffix, m_bucketFilenamePrefix); + bool isBackupValid = integrity.backupGuardExists(); + std::string bucketSuffix = ""; + std::string indexFilename = m_dbPath + m_indexFilename; + + if (isBackupValid) { + bucketSuffix += m_backupFilenameSuffix; + indexFilename += m_backupFilenameSuffix; + } try { auto indexStream = std::make_shared(); openFileStream(indexStream, indexFilename); StorageDeserializer storageDeserializer(indexStream, - std::bind(&InMemoryStorageBackend::bucketStreamOpener, this, std::placeholders::_1)); + std::bind(&InMemoryStorageBackend::bucketStreamOpener, this, + std::placeholders::_1, bucketSuffix)); storageDeserializer.initBuckets(buckets()); storageDeserializer.loadBuckets(buckets()); @@ -74,6 +85,13 @@ void InMemoryStorageBackend::load(void) { LOGN("Creating defaultBucket."); this->buckets().insert({ defaultPolicyBucketId, PolicyBucket(defaultPolicyBucketId) }); } + + if (isBackupValid) { + integrity.revalidatePrimaryDatabase(buckets()); + } + //in case there were unnecessary files in db directory + integrity.deleteNonIndexedFiles(std::bind(&InMemoryStorageBackend::hasBucket, this, + std::placeholders::_1)); } void InMemoryStorageBackend::save(void) { @@ -90,11 +108,19 @@ void InMemoryStorageBackend::save(void) { } auto indexStream = std::make_shared(); - openDumpFileStream(indexStream, m_dbPath + m_indexFileName); + std::string indexFilename = m_dbPath + m_indexFilename; + openDumpFileStream(indexStream, indexFilename + m_backupFilenameSuffix); StorageSerializer storageSerializer(indexStream); storageSerializer.dump(buckets(), std::bind(&InMemoryStorageBackend::bucketDumpStreamOpener, this, std::placeholders::_1)); + + Integrity integrity(m_dbPath, m_indexFilename, m_backupFilenameSuffix, m_bucketFilenamePrefix); + + integrity.syncDatabase(buckets(), true); + integrity.createBackupGuard(); + integrity.revalidatePrimaryDatabase(buckets()); + //guard is removed during revalidation } PolicyBucket InMemoryStorageBackend::searchDefaultBucket(const PolicyKey &key) { @@ -181,8 +207,9 @@ void InMemoryStorageBackend::openFileStream(std::shared_ptr strea // stream.exceptions(std::ifstream::failbit | std::ifstream::badbit); stream->open(filename); - if (!stream->is_open()) + if (!stream->is_open()) { throw FileNotFoundException(filename); + } } void InMemoryStorageBackend::openDumpFileStream(std::shared_ptr stream, @@ -195,8 +222,8 @@ void InMemoryStorageBackend::openDumpFileStream(std::shared_ptr s } std::shared_ptr InMemoryStorageBackend::bucketStreamOpener( - const PolicyBucketId &bucketId) { - std::string bucketFilename = m_dbPath + "_" + bucketId; + const PolicyBucketId &bucketId, const std::string &filenameSuffix) { + std::string bucketFilename = m_dbPath + m_bucketFilenamePrefix + bucketId + filenameSuffix; auto bucketStream = std::make_shared(); try { openFileStream(bucketStream, bucketFilename); @@ -208,7 +235,8 @@ std::shared_ptr InMemoryStorageBackend::bucketStreamOpener( std::shared_ptr InMemoryStorageBackend::bucketDumpStreamOpener( const PolicyBucketId &bucketId) { - std::string bucketFilename = m_dbPath + "_" + bucketId; + std::string bucketFilename = m_dbPath + m_bucketFilenamePrefix + + bucketId + m_backupFilenameSuffix; auto bucketStream = std::make_shared(); openDumpFileStream(bucketStream, bucketFilename); diff --git a/src/storage/InMemoryStorageBackend.h b/src/storage/InMemoryStorageBackend.h index cc35f95..d03dd6c 100644 --- a/src/storage/InMemoryStorageBackend.h +++ b/src/storage/InMemoryStorageBackend.h @@ -62,15 +62,19 @@ public: protected: InMemoryStorageBackend() {} void openFileStream(std::shared_ptr stream, const std::string &filename); - std::shared_ptr bucketStreamOpener(const PolicyBucketId &bucketId); + std::shared_ptr bucketStreamOpener(const PolicyBucketId &bucketId, + const std::string &fileNameSuffix); - void openDumpFileStream(std::shared_ptr stream, const std::string &filename); + virtual void openDumpFileStream(std::shared_ptr stream, + const std::string &filename); std::shared_ptr bucketDumpStreamOpener(const PolicyBucketId &bucketId); private: std::string m_dbPath; Buckets m_buckets; - static const std::string m_indexFileName; + static const std::string m_indexFilename; + static const std::string m_backupFilenameSuffix; + static const std::string m_bucketFilenamePrefix; protected: virtual Buckets &buckets(void) { diff --git a/src/storage/Integrity.cpp b/src/storage/Integrity.cpp new file mode 100644 index 0000000..8057e9c --- /dev/null +++ b/src/storage/Integrity.cpp @@ -0,0 +1,232 @@ +/* + * Copyright (c) 2014 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 src/storage/Integrity.cpp + * @author Pawel Wieczorek + * @version 0.1 + * @brief Implementation of Cynara::Integrity + */ + +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include +#include + +#include "Integrity.h" + +namespace Cynara { + +const std::string Integrity::m_guardFilename = "guard"; + +bool Integrity::backupGuardExists(void) const { + struct stat buffer; + std::string guardFilename = m_dbPath + m_guardFilename; + + int ret = stat(guardFilename.c_str(), &buffer); + + if (ret == 0) { + return true; + } else { + int err = errno; + if (err != ENOENT) { + LOGE("'stat' function error [%d] : <%s>", err, strerror(err)); + throw UnexpectedErrorException(err, strerror(err)); + } + return false; + } +} + +void Integrity::createBackupGuard(void) const { + syncElement(m_dbPath + m_guardFilename, O_CREAT | O_EXCL | O_WRONLY | O_TRUNC); + syncDirectory(m_dbPath); +} + +void Integrity::syncDatabase(const Buckets &buckets, bool syncBackup) { + std::string suffix = ""; + + if (syncBackup) { + suffix += m_backupFilenameSuffix; + } + + for (const auto &bucketIter : buckets) { + const auto &bucketId = bucketIter.first; + const auto &bucketFilename = m_dbPath + m_bucketFilenamePrefix + bucketId + suffix; + + syncElement(bucketFilename); + } + + syncElement(m_dbPath + m_indexFilename + suffix); + syncDirectory(m_dbPath); +} + +void Integrity::revalidatePrimaryDatabase(const Buckets &buckets) { + createPrimaryHardLinks(buckets); + syncDatabase(buckets, false); + + deleteHardLink(m_dbPath + m_guardFilename); + syncDirectory(m_dbPath); + + deleteBackupHardLinks(buckets); +} + +void Integrity::deleteNonIndexedFiles(BucketPresenceTester tester) { + DIR *dirPtr = nullptr; + struct dirent *direntPtr; + + if ((dirPtr = opendir(m_dbPath.c_str())) == nullptr) { + int err = errno; + LOGE("'opendir' function error [%d] : <%s>", err, strerror(err)); + throw UnexpectedErrorException(err, strerror(err)); + return; + } + + while (errno = 0, (direntPtr = readdir(dirPtr)) != nullptr) { + std::string filename = direntPtr->d_name; + //ignore all special files (working dir, parent dir, index) + if ("." == filename || ".." == filename || "buckets" == filename) { + continue; + } + + std::string bucketId; + auto nameLength = filename.length(); + auto prefixLength = m_bucketFilenamePrefix.length(); + + //remove if it is impossible that it is a bucket file + if (nameLength < prefixLength) { + deleteHardLink(m_dbPath + filename); + continue; + } + + //remove if there is no bucket filename prefix + //0 is returned from string::compare() if strings are equal + if (0 != filename.compare(0, prefixLength, m_bucketFilenamePrefix)) { + deleteHardLink(m_dbPath + filename); + continue; + } + + //remove if bucket is not in index + bucketId = filename.substr(prefixLength); + if (!tester(bucketId)) { + deleteHardLink(m_dbPath + filename); + } + } + + if (errno) { + int err = errno; + LOGE("'readdir' function error [%d] : <%s>", err, strerror(err)); + throw UnexpectedErrorException(err, strerror(err)); + return; + } +} + +void Integrity::syncElement(const std::string &filename, int flags, mode_t mode) { + int fileFd = TEMP_FAILURE_RETRY(open(filename.c_str(), flags, mode)); + + if (fileFd < 0) { + int err = errno; + if (err != EEXIST) { + LOGE("'open' function error [%d] : <%s>", err, strerror(err)); + throw UnexpectedErrorException(err, strerror(err)); + } else { + throw CannotCreateFileException(filename); + } + } + + int ret = fsync(fileFd); + + if (ret < 0) { + int err = errno; + LOGE("'fsync' function error [%d] : <%s>", err, strerror(err)); + throw UnexpectedErrorException(err, strerror(err)); + } + + ret = close(fileFd); + + if (ret < 0) { + int err = errno; + LOGE("'close' function error [%d] : <%s>", err, strerror(err)); + throw UnexpectedErrorException(err, strerror(err)); + } +} + +// from: man 2 fsync +// Calling fsync() does not necessarily ensure that the entry in the directory containing +// the file has also reached disk. For that an explicit fsync() on a file descriptor for +// the directory is also needed. +void Integrity::syncDirectory(const std::string &dirname, mode_t mode) { + syncElement(dirname, O_DIRECTORY, mode); +} + +void Integrity::createPrimaryHardLinks(const Buckets &buckets) { + for (const auto &bucketIter : buckets) { + const auto &bucketId = bucketIter.first; + const auto &bucketFilename = m_dbPath + m_bucketFilenamePrefix + bucketId; + + deleteHardLink(bucketFilename); + createHardLink(bucketFilename + m_backupFilenameSuffix, bucketFilename); + } + + const auto &indexFilename = m_dbPath + m_indexFilename; + + deleteHardLink(indexFilename); + createHardLink(indexFilename + m_backupFilenameSuffix, indexFilename); +} + +void Integrity::deleteBackupHardLinks(const Buckets &buckets) { + for (const auto &bucketIter : buckets) { + const auto &bucketId = bucketIter.first; + const auto &bucketFilename = m_dbPath + m_bucketFilenamePrefix + + bucketId + m_backupFilenameSuffix; + + deleteHardLink(bucketFilename); + } + + deleteHardLink(m_dbPath + m_indexFilename + m_backupFilenameSuffix); +} + +void Integrity::createHardLink(const std::string &oldName, const std::string &newName) { + int ret = link(oldName.c_str(), newName.c_str()); + + if (ret < 0) { + int err = errno; + throw UnexpectedErrorException(err, strerror(err)); + LOGN("Trying to link to non-existent file: <%s>", oldName.c_str()); + } +} + +void Integrity::deleteHardLink(const std::string &filename) { + int ret = unlink(filename.c_str()); + + if (ret < 0) { + int err = errno; + if (err != ENOENT) { + LOGE("'unlink' function error [%d] : <%s>", err, strerror(err)); + throw UnexpectedErrorException(err, strerror(err)); + } else { + LOGN("Trying to unlink non-existent file: <%s>", filename.c_str()); + } + } +} + +} /* namespace Cynara */ diff --git a/src/storage/Integrity.h b/src/storage/Integrity.h new file mode 100644 index 0000000..4b677b0 --- /dev/null +++ b/src/storage/Integrity.h @@ -0,0 +1,71 @@ +/* + * Copyright (c) 2014 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 src/storage/Integrity.h + * @author Pawel Wieczorek + * @version 0.1 + * @brief Headers for Cynara::Integrity + */ + +#ifndef SRC_STORAGE_INTEGRITY_H_ +#define SRC_STORAGE_INTEGRITY_H_ + +#include +#include + +#include + +namespace Cynara { + +class Integrity +{ +public: + typedef std::function BucketPresenceTester; + Integrity(const std::string &path, const std::string &index, const std::string &backupSuffix, + const std::string &bucketPrefix) + : m_dbPath(path), m_indexFilename(index), m_backupFilenameSuffix(backupSuffix), + m_bucketFilenamePrefix(bucketPrefix) { + } + virtual ~Integrity() {}; + + virtual bool backupGuardExists(void) const; + virtual void createBackupGuard(void) const; + virtual void syncDatabase(const Buckets &buckets, bool syncBackup); + virtual void revalidatePrimaryDatabase(const Buckets &buckets); + virtual void deleteNonIndexedFiles(BucketPresenceTester tester); + +protected: + static void syncElement(const std::string &filename, int flags = O_RDONLY, + mode_t mode = S_IRUSR | S_IWUSR); + static void syncDirectory(const std::string &dirname, mode_t mode = S_IRUSR | S_IWUSR); + + void createPrimaryHardLinks(const Buckets &buckets); + void deleteBackupHardLinks(const Buckets &buckets); + + static void createHardLink(const std::string &oldName, const std::string &newName); + static void deleteHardLink(const std::string &filename); + +private: + const std::string m_dbPath; + const std::string m_indexFilename; + const std::string m_backupFilenameSuffix; + const std::string m_bucketFilenamePrefix; + static const std::string m_guardFilename; +}; + +} // namespace Cynara + +#endif /* SRC_STORAGE_INTEGRITY_H_ */ diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index ffddd87..063098f 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -32,6 +32,7 @@ SET(CYNARA_SOURCES_FOR_TESTS ${CYNARA_SRC}/helpers/creds-commons/creds-commons.cpp ${CYNARA_SRC}/storage/BucketDeserializer.cpp ${CYNARA_SRC}/storage/InMemoryStorageBackend.cpp + ${CYNARA_SRC}/storage/Integrity.cpp ${CYNARA_SRC}/storage/Storage.cpp ${CYNARA_SRC}/storage/StorageDeserializer.cpp ${CYNARA_SRC}/storage/StorageSerializer.cpp diff --git a/test/db/db6/_ b/test/db/db6/_ new file mode 100644 index 0000000..e69de29 diff --git a/test/db/db6/_additional b/test/db/db6/_additional new file mode 100644 index 0000000..cbeab67 --- /dev/null +++ b/test/db/db6/_additional @@ -0,0 +1 @@ +client1;user1;privilege1;0x0; diff --git a/test/db/db6/_additional~ b/test/db/db6/_additional~ new file mode 100644 index 0000000..e69de29 diff --git a/test/db/db6/_~ b/test/db/db6/_~ new file mode 100644 index 0000000..e69de29 diff --git a/test/db/db6/buckets b/test/db/db6/buckets new file mode 100644 index 0000000..19d7a93 --- /dev/null +++ b/test/db/db6/buckets @@ -0,0 +1,2 @@ +;0x0 +additional;0xFFFF diff --git a/test/db/db6/buckets~ b/test/db/db6/buckets~ new file mode 100644 index 0000000..19d7a93 --- /dev/null +++ b/test/db/db6/buckets~ @@ -0,0 +1,2 @@ +;0x0 +additional;0xFFFF diff --git a/test/db/db6/guard b/test/db/db6/guard new file mode 100644 index 0000000..e69de29 diff --git a/test/storage/inmemorystoragebackend/fakeinmemorystoragebackend.h b/test/storage/inmemorystoragebackend/fakeinmemorystoragebackend.h index 1c80dea..52b4fe3 100644 --- a/test/storage/inmemorystoragebackend/fakeinmemorystoragebackend.h +++ b/test/storage/inmemorystoragebackend/fakeinmemorystoragebackend.h @@ -29,7 +29,7 @@ class FakeInMemoryStorageBackend : public Cynara::InMemoryStorageBackend { public: using Cynara::InMemoryStorageBackend::InMemoryStorageBackend; - MOCK_METHOD0(buckets, Cynara::Buckets&()); + MOCK_METHOD0(buckets, Cynara::Buckets&(void)); }; diff --git a/test/storage/inmemorystoragebackend/inmemeorystoragebackendfixture.h b/test/storage/inmemorystoragebackend/inmemeorystoragebackendfixture.h index 3f08dea..2ec2920 100644 --- a/test/storage/inmemorystoragebackend/inmemeorystoragebackendfixture.h +++ b/test/storage/inmemorystoragebackend/inmemeorystoragebackendfixture.h @@ -67,6 +67,8 @@ protected: // TODO: consider defaulting accessor with ON_CALL Cynara::Buckets m_buckets; + static const std::string m_indexFileName; + static const std::string m_backupFileNameSuffix; }; diff --git a/test/storage/inmemorystoragebackend/inmemorystoragebackend.cpp b/test/storage/inmemorystoragebackend/inmemorystoragebackend.cpp index 11c0968..c0fd640 100644 --- a/test/storage/inmemorystoragebackend/inmemorystoragebackend.cpp +++ b/test/storage/inmemorystoragebackend/inmemorystoragebackend.cpp @@ -40,6 +40,9 @@ using namespace Cynara; +const std::string InMemeoryStorageBackendFixture::m_indexFileName = "buckets"; +const std::string InMemeoryStorageBackendFixture::m_backupFileNameSuffix = "~"; + TEST_F(InMemeoryStorageBackendFixture, defaultPolicyIsDeny) { using ::testing::ReturnRef; @@ -192,7 +195,7 @@ TEST_F(InMemeoryStorageBackendFixture, deletePolicyFromNonexistentBucket) { // Database dir is empty TEST_F(InMemeoryStorageBackendFixture, load_no_db) { using ::testing::ReturnRef; - auto testDbPath = std::string(CYNARA_TESTS_DIR) + "/db1/"; + auto testDbPath = std::string(CYNARA_TESTS_DIR) + "/empty_db/"; FakeInMemoryStorageBackend backend(testDbPath); EXPECT_CALL(backend, buckets()).WillRepeatedly(ReturnRef(m_buckets)); backend.load(); @@ -252,3 +255,36 @@ TEST_F(InMemeoryStorageBackendFixture, second_bucket_corrupted) { backend.load(); ASSERT_DB_VIRGIN(m_buckets); } + +/** + * @brief Database was corrupted, restore from backup (which is present) + * @test Scenario: + * - There still is guard file in earlier prepared Cynara's policy database directory (db6) + * - Execution of load() should use backup files (present, with different contents than primaries) + * - Loaded database is checked - backup files were empty, so should recently loaded policies + */ +TEST_F(InMemeoryStorageBackendFixture, load_from_backup) { + using ::testing::ReturnRef; + using ::testing::IsEmpty; + using ::testing::InSequence; + + auto testDbPath = std::string(CYNARA_TESTS_DIR) + "/db6/"; + FakeInMemoryStorageBackend backend(testDbPath); + + { + // Calls are expected in a specific order + InSequence s; + EXPECT_CALL(backend, buckets()).WillRepeatedly(ReturnRef(m_buckets)); + backend.load(); + } + + std::vector bucketIds = { "", "additional" }; + for(const auto &bucketId : bucketIds) { + SCOPED_TRACE(bucketId); + const auto bucketIter = m_buckets.find(bucketId); + ASSERT_NE(m_buckets.end(), bucketIter); + + const auto &bucketPolicies = bucketIter->second; + ASSERT_THAT(bucketPolicies, IsEmpty()); + } +} -- 2.7.4 From d1b33eecd1065cf626efc5380dcfc2da6a7c1359 Mon Sep 17 00:00:00 2001 From: Pawel Wieczorek Date: Thu, 13 Nov 2014 12:07:20 +0100 Subject: [PATCH 10/16] Ensure creation of minimal database This patch changes default behaviour of migration tool during package upgrade. Previously, Cynara's state path was left untouched. Now creation of minimal database is ensured. No changes are made if it already existed. Change-Id: I25158aec7d7b436ac1446d43277afe1337bfe4e5 --- migration/cynara-db-migration.sh | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/migration/cynara-db-migration.sh b/migration/cynara-db-migration.sh index 0cf5edd..8cbebbf 100644 --- a/migration/cynara-db-migration.sh +++ b/migration/cynara-db-migration.sh @@ -94,9 +94,10 @@ migrate_db() { failure fi - : - # : is a null command, as functions may not be empty. - # Actual body will be added if database structure changes. + # Create minimal database if there was none: + if [ ! -d "${STATE_PATH}/${DB_DIR}" ]; then + create_db + fi } remove_db() { -- 2.7.4 From 8dd72120e13a4192408b8940b85ef40d980a3fdd Mon Sep 17 00:00:00 2001 From: Zofia Abramowska Date: Thu, 13 Nov 2014 14:25:56 +0100 Subject: [PATCH 11/16] Fix segfault in dump_buckets test Change-Id: If614900c9710dc0600c48622051afb484709155f --- test/storage/serializer/serialize.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/test/storage/serializer/serialize.cpp b/test/storage/serializer/serialize.cpp index cceed78..938283d 100644 --- a/test/storage/serializer/serialize.cpp +++ b/test/storage/serializer/serialize.cpp @@ -48,8 +48,8 @@ public: // Fake StorageSerializer for Cynara::PolicyBucket class FakeStorageSerializer : public Cynara::StorageSerializer { public: - FakeStorageSerializer() : Cynara::StorageSerializer(outStream), - outStream(new std::ostringstream()) {} + FakeStorageSerializer(std::shared_ptr o) : Cynara::StorageSerializer(o), + outStream(o) {} MOCK_METHOD1(dump, void(const Cynara::PolicyBucket &bucket)); std::shared_ptr outStream; }; @@ -82,7 +82,8 @@ TEST_F(StorageSerializerFixture, dump_buckets) { using ::testing::UnorderedElementsAreArray; // Will be returned as serializer for buckets - auto fakeBucketSerializer = std::make_shared(); + auto fakeBucketSerializer = std::make_shared( + std::make_shared()); buckets = { { "bucket1", PolicyBucket("bucket1", PredefinedPolicyType::DENY) }, -- 2.7.4 From 656f1a804ecf28619a0f7ea91ce26b43d27ec075 Mon Sep 17 00:00:00 2001 From: Zofia Abramowska Date: Wed, 12 Nov 2014 16:30:35 +0100 Subject: [PATCH 12/16] Remove unnecessary Protocol shared pointers ProtocolFramePtr and ProtocolFrameHeaderPtr was used unnecessary (no shared ownership required). Change-Id: I71d4bf797450a46bc35e2321ff8d01a6508bcf88 --- src/common/protocol/ProtocolAdmin.cpp | 76 ++++++++++++------------- src/common/protocol/ProtocolClient.cpp | 26 ++++----- src/common/protocol/ProtocolFrame.cpp | 4 +- src/common/protocol/ProtocolFrame.h | 8 +-- src/common/protocol/ProtocolFrameHeader.h | 2 - src/common/protocol/ProtocolFrameSerializer.cpp | 16 +++--- src/common/protocol/ProtocolFrameSerializer.h | 4 +- 7 files changed, 66 insertions(+), 70 deletions(-) diff --git a/src/common/protocol/ProtocolAdmin.cpp b/src/common/protocol/ProtocolAdmin.cpp index 7d7d0f9..ca17e34 100644 --- a/src/common/protocol/ProtocolAdmin.cpp +++ b/src/common/protocol/ProtocolAdmin.cpp @@ -228,14 +228,14 @@ void ProtocolAdmin::execute(RequestContextPtr context, AdminCheckRequestPtr requ request->key().user().value().c_str(), request->key().privilege().value().c_str(), request->startBucket().c_str(), request->recursive()); - ProtocolFramePtr frame = ProtocolFrameSerializer::startSerialization(request->sequenceNumber()); + ProtocolFrame frame = ProtocolFrameSerializer::startSerialization(request->sequenceNumber()); - ProtocolSerialization::serialize(*frame, OpAdminCheckRequest); - ProtocolSerialization::serialize(*frame, request->key().client().value()); - ProtocolSerialization::serialize(*frame, request->key().user().value()); - ProtocolSerialization::serialize(*frame, request->key().privilege().value()); - ProtocolSerialization::serialize(*frame, request->startBucket()); - ProtocolSerialization::serialize(*frame, request->recursive()); + ProtocolSerialization::serialize(frame, OpAdminCheckRequest); + ProtocolSerialization::serialize(frame, request->key().client().value()); + ProtocolSerialization::serialize(frame, request->key().user().value()); + ProtocolSerialization::serialize(frame, request->key().privilege().value()); + ProtocolSerialization::serialize(frame, request->startBucket()); + ProtocolSerialization::serialize(frame, request->recursive()); ProtocolFrameSerializer::finishSerialization(frame, context->responseQueue()); } @@ -246,12 +246,12 @@ void ProtocolAdmin::execute(RequestContextPtr context, InsertOrUpdateBucketReque request->bucketId().c_str(), request->result().policyType(), request->result().metadata().c_str()); - ProtocolFramePtr frame = ProtocolFrameSerializer::startSerialization(request->sequenceNumber()); + ProtocolFrame frame = ProtocolFrameSerializer::startSerialization(request->sequenceNumber()); - ProtocolSerialization::serialize(*frame, OpInsertOrUpdateBucket); - ProtocolSerialization::serialize(*frame, request->bucketId()); - ProtocolSerialization::serialize(*frame, request->result().policyType()); - ProtocolSerialization::serialize(*frame, request->result().metadata()); + ProtocolSerialization::serialize(frame, OpInsertOrUpdateBucket); + ProtocolSerialization::serialize(frame, request->bucketId()); + ProtocolSerialization::serialize(frame, request->result().policyType()); + ProtocolSerialization::serialize(frame, request->result().metadata()); ProtocolFrameSerializer::finishSerialization(frame, context->responseQueue()); } @@ -260,10 +260,10 @@ void ProtocolAdmin::execute(RequestContextPtr context, RemoveBucketRequestPtr re LOGD("Serializing RemoveBucketRequest: sequenceNumber [%" PRIu16 "], bucketId <%s>", request->sequenceNumber(), request->bucketId().c_str()); - ProtocolFramePtr frame = ProtocolFrameSerializer::startSerialization(request->sequenceNumber()); + ProtocolFrame frame = ProtocolFrameSerializer::startSerialization(request->sequenceNumber()); - ProtocolSerialization::serialize(*frame, OpRemoveBucket); - ProtocolSerialization::serialize(*frame, request->bucketId()); + ProtocolSerialization::serialize(frame, OpRemoveBucket); + ProtocolSerialization::serialize(frame, request->bucketId()); ProtocolFrameSerializer::finishSerialization(frame, context->responseQueue()); } @@ -273,39 +273,39 @@ void ProtocolAdmin::execute(RequestContextPtr context, SetPoliciesRequestPtr req "insertOrUpdate count [%zu], remove count [%zu]", request->sequenceNumber(), request->policiesToBeInsertedOrUpdated().size(), request->policiesToBeRemoved().size()); - ProtocolFramePtr frame = + ProtocolFrame frame = ProtocolFrameSerializer::startSerialization(request->sequenceNumber()); - ProtocolSerialization::serialize(*frame, OpSetPolicies); + ProtocolSerialization::serialize(frame, OpSetPolicies); - ProtocolSerialization::serialize(*frame, + ProtocolSerialization::serialize(frame, static_cast(request->policiesToBeInsertedOrUpdated().size())); for (auto policyBucket : request->policiesToBeInsertedOrUpdated()) { - ProtocolSerialization::serialize(*frame, policyBucket.first); - ProtocolSerialization::serialize(*frame, + ProtocolSerialization::serialize(frame, policyBucket.first); + ProtocolSerialization::serialize(frame, static_cast(policyBucket.second.size())); for (auto policy : policyBucket.second) { // PolicyKey - ProtocolSerialization::serialize(*frame, policy.key().client().value()); - ProtocolSerialization::serialize(*frame, policy.key().user().value()); - ProtocolSerialization::serialize(*frame, policy.key().privilege().value()); + ProtocolSerialization::serialize(frame, policy.key().client().value()); + ProtocolSerialization::serialize(frame, policy.key().user().value()); + ProtocolSerialization::serialize(frame, policy.key().privilege().value()); // PolicyResult - ProtocolSerialization::serialize(*frame, policy.result().policyType()); - ProtocolSerialization::serialize(*frame, policy.result().metadata()); + ProtocolSerialization::serialize(frame, policy.result().policyType()); + ProtocolSerialization::serialize(frame, policy.result().metadata()); } } - ProtocolSerialization::serialize(*frame, + ProtocolSerialization::serialize(frame, static_cast(request->policiesToBeRemoved().size())); for (auto policyBucket : request->policiesToBeRemoved()) { - ProtocolSerialization::serialize(*frame, policyBucket.first); - ProtocolSerialization::serialize(*frame, + ProtocolSerialization::serialize(frame, policyBucket.first); + ProtocolSerialization::serialize(frame, static_cast(policyBucket.second.size())); for (auto policyKey : policyBucket.second) { // PolicyKey - ProtocolSerialization::serialize(*frame, policyKey.client().value()); - ProtocolSerialization::serialize(*frame, policyKey.user().value()); - ProtocolSerialization::serialize(*frame, policyKey.privilege().value()); + ProtocolSerialization::serialize(frame, policyKey.client().value()); + ProtocolSerialization::serialize(frame, policyKey.user().value()); + ProtocolSerialization::serialize(frame, policyKey.privilege().value()); } } @@ -318,12 +318,12 @@ void ProtocolAdmin::execute(RequestContextPtr context, CheckResponsePtr response response->sequenceNumber(), response->m_resultRef.policyType(), response->m_resultRef.metadata().c_str()); - ProtocolFramePtr frame = ProtocolFrameSerializer::startSerialization( + ProtocolFrame frame = ProtocolFrameSerializer::startSerialization( response->sequenceNumber()); - ProtocolSerialization::serialize(*frame, OpCheckPolicyResponse); - ProtocolSerialization::serialize(*frame, response->m_resultRef.policyType()); - ProtocolSerialization::serialize(*frame, response->m_resultRef.metadata()); + ProtocolSerialization::serialize(frame, OpCheckPolicyResponse); + ProtocolSerialization::serialize(frame, response->m_resultRef.policyType()); + ProtocolSerialization::serialize(frame, response->m_resultRef.metadata()); ProtocolFrameSerializer::finishSerialization(frame, context->responseQueue()); } @@ -332,11 +332,11 @@ void ProtocolAdmin::execute(RequestContextPtr context, CodeResponsePtr response) LOGD("Serializing CodeResponse: op [%" PRIu8 "], sequenceNumber [%" PRIu16 "], " "code [%" PRIu16 "]", OpCodeResponse, response->sequenceNumber(), response->m_code); - ProtocolFramePtr frame = ProtocolFrameSerializer::startSerialization( + ProtocolFrame frame = ProtocolFrameSerializer::startSerialization( response->sequenceNumber()); - ProtocolSerialization::serialize(*frame, OpCodeResponse); - ProtocolSerialization::serialize(*frame, static_cast(response->m_code)); + ProtocolSerialization::serialize(frame, OpCodeResponse); + ProtocolSerialization::serialize(frame, static_cast(response->m_code)); ProtocolFrameSerializer::finishSerialization(frame, context->responseQueue()); } diff --git a/src/common/protocol/ProtocolClient.cpp b/src/common/protocol/ProtocolClient.cpp index 94811dd..c643789 100644 --- a/src/common/protocol/ProtocolClient.cpp +++ b/src/common/protocol/ProtocolClient.cpp @@ -141,52 +141,52 @@ ResponsePtr ProtocolClient::extractResponseFromBuffer(BinaryQueue &bufferQueue) } void ProtocolClient::execute(RequestContextPtr context, CancelRequestPtr request) { - ProtocolFramePtr frame = ProtocolFrameSerializer::startSerialization(request->sequenceNumber()); + ProtocolFrame frame = ProtocolFrameSerializer::startSerialization(request->sequenceNumber()); LOGD("Serializing CancelRequest op [%" PRIu8 "]", OpCancelRequest); - ProtocolSerialization::serialize(*frame, OpCancelRequest); + ProtocolSerialization::serialize(frame, OpCancelRequest); ProtocolFrameSerializer::finishSerialization(frame, context->responseQueue()); } void ProtocolClient::execute(RequestContextPtr context, CheckRequestPtr request) { - ProtocolFramePtr frame = ProtocolFrameSerializer::startSerialization(request->sequenceNumber()); + ProtocolFrame frame = ProtocolFrameSerializer::startSerialization(request->sequenceNumber()); LOGD("Serializing CheckRequest: client <%s>, user <%s>, privilege <%s>", request->key().client().value().c_str(), request->key().user().value().c_str(), request->key().privilege().value().c_str()); - ProtocolSerialization::serialize(*frame, OpCheckPolicyRequest); - ProtocolSerialization::serialize(*frame, request->key().client().value()); - ProtocolSerialization::serialize(*frame, request->key().user().value()); - ProtocolSerialization::serialize(*frame, request->key().privilege().value()); + ProtocolSerialization::serialize(frame, OpCheckPolicyRequest); + ProtocolSerialization::serialize(frame, request->key().client().value()); + ProtocolSerialization::serialize(frame, request->key().user().value()); + ProtocolSerialization::serialize(frame, request->key().privilege().value()); ProtocolFrameSerializer::finishSerialization(frame, context->responseQueue()); } void ProtocolClient::execute(RequestContextPtr context, CancelResponsePtr response) { - ProtocolFramePtr frame = ProtocolFrameSerializer::startSerialization( + ProtocolFrame frame = ProtocolFrameSerializer::startSerialization( response->sequenceNumber()); LOGD("Serializing CancelResponse: op [%" PRIu8 "]", OpCancelResponse); - ProtocolSerialization::serialize(*frame, OpCancelResponse); + ProtocolSerialization::serialize(frame, OpCancelResponse); ProtocolFrameSerializer::finishSerialization(frame, context->responseQueue()); } void ProtocolClient::execute(RequestContextPtr context, CheckResponsePtr response) { - ProtocolFramePtr frame = ProtocolFrameSerializer::startSerialization( + ProtocolFrame frame = ProtocolFrameSerializer::startSerialization( response->sequenceNumber()); LOGD("Serializing CheckResponse: op [%" PRIu8 "], policyType [%" PRIu16 "], metadata <%s>", OpCheckPolicyResponse, response->m_resultRef.policyType(), response->m_resultRef.metadata().c_str()); - ProtocolSerialization::serialize(*frame, OpCheckPolicyResponse); - ProtocolSerialization::serialize(*frame, response->m_resultRef.policyType()); - ProtocolSerialization::serialize(*frame, response->m_resultRef.metadata()); + ProtocolSerialization::serialize(frame, OpCheckPolicyResponse); + ProtocolSerialization::serialize(frame, response->m_resultRef.policyType()); + ProtocolSerialization::serialize(frame, response->m_resultRef.metadata()); ProtocolFrameSerializer::finishSerialization(frame, context->responseQueue()); } diff --git a/src/common/protocol/ProtocolFrame.cpp b/src/common/protocol/ProtocolFrame.cpp index 560217e..736be30 100644 --- a/src/common/protocol/ProtocolFrame.cpp +++ b/src/common/protocol/ProtocolFrame.cpp @@ -28,7 +28,7 @@ namespace Cynara { -ProtocolFrame::ProtocolFrame(ProtocolFrameHeaderPtr frameHeader, BinaryQueuePtr data) : +ProtocolFrame::ProtocolFrame(ProtocolFrameHeader frameHeader, BinaryQueuePtr data) : m_frameHeader(frameHeader), m_frameBodyContent(data) { } @@ -38,7 +38,7 @@ void ProtocolFrame::read(size_t num, void *bytes) { void ProtocolFrame::write(size_t num, const void *bytes) { m_frameBodyContent->appendCopy(bytes, num); - m_frameHeader->increaseFrameLength(num); + m_frameHeader.increaseFrameLength(num); } } /* namespace Cynara */ diff --git a/src/common/protocol/ProtocolFrame.h b/src/common/protocol/ProtocolFrame.h index 3d85cc0..73f22e9 100644 --- a/src/common/protocol/ProtocolFrame.h +++ b/src/common/protocol/ProtocolFrame.h @@ -39,10 +39,10 @@ class ProtocolFrameSerializer; class ProtocolFrame: public IStream { public: - ProtocolFrame(ProtocolFrameHeaderPtr frameHeader, BinaryQueuePtr headerContent); + ProtocolFrame(ProtocolFrameHeader frameHeader, BinaryQueuePtr headerContent); virtual ~ProtocolFrame() {}; - ProtocolFrameHeaderPtr frameHeader(void) { + ProtocolFrameHeader &frameHeader(void) { return m_frameHeader; } @@ -50,7 +50,7 @@ public: virtual void write(size_t num, const void *bytes); private: - ProtocolFrameHeaderPtr m_frameHeader; + ProtocolFrameHeader m_frameHeader; BinaryQueuePtr m_frameBodyContent; BinaryQueue &bodyContent(void) { @@ -60,8 +60,6 @@ private: friend class ProtocolFrameSerializer; }; -typedef std::shared_ptr ProtocolFramePtr; - } /* namespace Cynara */ #endif /* SRC_COMMON_PROTOCOL_PROTOCOLFRAME_H_ */ diff --git a/src/common/protocol/ProtocolFrameHeader.h b/src/common/protocol/ProtocolFrameHeader.h index ff92bf9..7b85465 100644 --- a/src/common/protocol/ProtocolFrameHeader.h +++ b/src/common/protocol/ProtocolFrameHeader.h @@ -107,8 +107,6 @@ private: friend class ProtocolFrameSerializer; }; -typedef std::shared_ptr ProtocolFrameHeaderPtr; - } /* namespace Cynara */ #endif /* SRC_COMMON_PROTOCOL_PROTOCOLFRAMEHEADER_H_ */ diff --git a/src/common/protocol/ProtocolFrameSerializer.cpp b/src/common/protocol/ProtocolFrameSerializer.cpp index 88a07d3..0599f02 100644 --- a/src/common/protocol/ProtocolFrameSerializer.cpp +++ b/src/common/protocol/ProtocolFrameSerializer.cpp @@ -65,19 +65,19 @@ void ProtocolFrameSerializer::deserializeHeader(ProtocolFrameHeader &frameHeader } } -ProtocolFramePtr ProtocolFrameSerializer::startSerialization(ProtocolFrameSequenceNumber sequenceNumber) { +ProtocolFrame ProtocolFrameSerializer::startSerialization(ProtocolFrameSequenceNumber sequenceNumber) { LOGD("Serialization started"); BinaryQueuePtr headerQueue = std::make_shared(); BinaryQueuePtr bodyQueue = std::make_shared(); - ProtocolFrameHeaderPtr header = std::make_shared(headerQueue); - header->setSequenceNumber(sequenceNumber); - header->increaseFrameLength(ProtocolFrameHeader::frameHeaderLength()); - return std::make_shared(header, bodyQueue); + ProtocolFrameHeader header(headerQueue); + header.setSequenceNumber(sequenceNumber); + header.increaseFrameLength(ProtocolFrameHeader::frameHeaderLength()); + return ProtocolFrame(header, bodyQueue); } -void ProtocolFrameSerializer::finishSerialization(ProtocolFramePtr frame, BinaryQueue &data) { - ProtocolFrameHeader &frameHeader = *frame->frameHeader(); +void ProtocolFrameSerializer::finishSerialization(ProtocolFrame &frame, BinaryQueue &data) { + ProtocolFrameHeader &frameHeader = frame.frameHeader(); ProtocolSerialization::serializeNoSize(frameHeader, ProtocolFrameHeader::m_signature); ProtocolSerialization::serialize(frameHeader, frameHeader.m_frameLength); ProtocolSerialization::serialize(frameHeader, frameHeader.m_sequenceNumber); @@ -87,7 +87,7 @@ void ProtocolFrameSerializer::finishSerialization(ProtocolFramePtr frame, Binary (int)frameHeader.m_sequenceNumber); data.appendMoveFrom(frameHeader.headerContent()); - data.appendMoveFrom(frame->bodyContent()); + data.appendMoveFrom(frame.bodyContent()); } } /* namespace Cynara */ diff --git a/src/common/protocol/ProtocolFrameSerializer.h b/src/common/protocol/ProtocolFrameSerializer.h index 8961e14..f2e201e 100644 --- a/src/common/protocol/ProtocolFrameSerializer.h +++ b/src/common/protocol/ProtocolFrameSerializer.h @@ -34,8 +34,8 @@ class ProtocolFrameSerializer { public: static void deserializeHeader(ProtocolFrameHeader &frameHeader, BinaryQueue &data); - static ProtocolFramePtr startSerialization(ProtocolFrameSequenceNumber sequenceNumber); - static void finishSerialization(ProtocolFramePtr frame, BinaryQueue &data); + static ProtocolFrame startSerialization(ProtocolFrameSequenceNumber sequenceNumber); + static void finishSerialization(ProtocolFrame &frame, BinaryQueue &data); }; } /* namespace Cynara */ -- 2.7.4 From 481d3058557b53ed807fce6ba6412da683e0f392 Mon Sep 17 00:00:00 2001 From: Zofia Abramowska Date: Wed, 12 Nov 2014 17:07:55 +0100 Subject: [PATCH 13/16] Reorganize ProtocolAdmin and ProtocolClient Private methods of ProtocolAdmin and ProtocolClient lost an argument, which was used only to pass member value from the same class. Change-Id: I5657d38cf9ccd47892082479eeae92d62f894227 --- src/common/protocol/ProtocolAdmin.cpp | 90 +++++++++++++++++----------------- src/common/protocol/ProtocolAdmin.h | 12 ++--- src/common/protocol/ProtocolClient.cpp | 34 ++++++------- src/common/protocol/ProtocolClient.h | 8 +-- 4 files changed, 73 insertions(+), 71 deletions(-) diff --git a/src/common/protocol/ProtocolAdmin.cpp b/src/common/protocol/ProtocolAdmin.cpp index ca17e34..2d4c879 100644 --- a/src/common/protocol/ProtocolAdmin.cpp +++ b/src/common/protocol/ProtocolAdmin.cpp @@ -50,53 +50,54 @@ ProtocolPtr ProtocolAdmin::clone(void) { return std::make_shared(); } -RequestPtr ProtocolAdmin::deserializeAdminCheckRequest(ProtocolFrameHeader &frame) { +RequestPtr ProtocolAdmin::deserializeAdminCheckRequest(void) { std::string clientId, userId, privilegeId; PolicyBucketId startBucket; bool recursive; - ProtocolDeserialization::deserialize(frame, clientId); - ProtocolDeserialization::deserialize(frame, userId); - ProtocolDeserialization::deserialize(frame, privilegeId); - ProtocolDeserialization::deserialize(frame, startBucket); - ProtocolDeserialization::deserialize(frame, recursive); + ProtocolDeserialization::deserialize(m_frameHeader, clientId); + ProtocolDeserialization::deserialize(m_frameHeader, userId); + ProtocolDeserialization::deserialize(m_frameHeader, privilegeId); + ProtocolDeserialization::deserialize(m_frameHeader, startBucket); + ProtocolDeserialization::deserialize(m_frameHeader, recursive); LOGD("Deserialized AdminCheckRequest: clientId <%s>, userId <%s>, privilegeId <%s>, " "startBucket <%s>, recursive [%d]", clientId.c_str(), userId.c_str(), privilegeId.c_str(), startBucket.c_str(), recursive); return std::make_shared(PolicyKey(clientId, userId, privilegeId), - startBucket, recursive, frame.sequenceNumber()); + startBucket, recursive, + m_frameHeader.sequenceNumber()); } -RequestPtr ProtocolAdmin::deserializeInsertOrUpdateBucketRequest(ProtocolFrameHeader &frame) { +RequestPtr ProtocolAdmin::deserializeInsertOrUpdateBucketRequest(void) { PolicyBucketId policyBucketId; PolicyType policyType; PolicyResult::PolicyMetadata policyMetaData; - ProtocolDeserialization::deserialize(frame, policyBucketId); - ProtocolDeserialization::deserialize(frame, policyType); - ProtocolDeserialization::deserialize(frame, policyMetaData); + ProtocolDeserialization::deserialize(m_frameHeader, policyBucketId); + ProtocolDeserialization::deserialize(m_frameHeader, policyType); + ProtocolDeserialization::deserialize(m_frameHeader, policyMetaData); LOGD("Deserialized InsertOrUpdateBucketRequest: bucketId <%s>, " "result.type [%" PRIu16 "], result.meta <%s>", policyBucketId.c_str(), policyType, policyMetaData.c_str()); return std::make_shared(policyBucketId, - PolicyResult(policyType, policyMetaData), frame.sequenceNumber()); + PolicyResult(policyType, policyMetaData), m_frameHeader.sequenceNumber()); } -RequestPtr ProtocolAdmin::deserializeRemoveBucketRequest(ProtocolFrameHeader &frame) { +RequestPtr ProtocolAdmin::deserializeRemoveBucketRequest(void) { PolicyBucketId policyBucketId; - ProtocolDeserialization::deserialize(frame, policyBucketId); + ProtocolDeserialization::deserialize(m_frameHeader, policyBucketId); LOGD("Deserialized RemoveBucketRequest: bucketId <%s>", policyBucketId.c_str()); - return std::make_shared(policyBucketId, frame.sequenceNumber()); + return std::make_shared(policyBucketId, m_frameHeader.sequenceNumber()); } -RequestPtr ProtocolAdmin::deserializeSetPoliciesRequest(ProtocolFrameHeader &frame) { +RequestPtr ProtocolAdmin::deserializeSetPoliciesRequest(void) { ProtocolFrameFieldsCount toBeInsertedOrUpdatedCount, toBeRemovedCount; ProtocolFrameFieldsCount policyCount; PolicyKeyFeature::ValueType clientId, user, privilege; @@ -105,19 +106,19 @@ RequestPtr ProtocolAdmin::deserializeSetPoliciesRequest(ProtocolFrameHeader &fra std::map> toBeInsertedOrUpdatedPolicies; std::map> toBeRemovedPolicies; - ProtocolDeserialization::deserialize(frame, toBeInsertedOrUpdatedCount); + ProtocolDeserialization::deserialize(m_frameHeader, toBeInsertedOrUpdatedCount); for (ProtocolFrameFieldsCount b = 0; b < toBeInsertedOrUpdatedCount; ++b) { PolicyBucketId policyBucketId; - ProtocolDeserialization::deserialize(frame, policyBucketId); - ProtocolDeserialization::deserialize(frame, policyCount); + ProtocolDeserialization::deserialize(m_frameHeader, policyBucketId); + ProtocolDeserialization::deserialize(m_frameHeader, policyCount); for (ProtocolFrameFieldsCount p = 0; p < policyCount; ++p) { // PolicyKey - ProtocolDeserialization::deserialize(frame, clientId); - ProtocolDeserialization::deserialize(frame, user); - ProtocolDeserialization::deserialize(frame, privilege); + ProtocolDeserialization::deserialize(m_frameHeader, clientId); + ProtocolDeserialization::deserialize(m_frameHeader, user); + ProtocolDeserialization::deserialize(m_frameHeader, privilege); // PolicyResult - ProtocolDeserialization::deserialize(frame, policyType); - ProtocolDeserialization::deserialize(frame, metadata); + ProtocolDeserialization::deserialize(m_frameHeader, policyType); + ProtocolDeserialization::deserialize(m_frameHeader, metadata); toBeInsertedOrUpdatedPolicies[policyBucketId].push_back( Policy(PolicyKey(clientId, user, privilege), @@ -125,16 +126,16 @@ RequestPtr ProtocolAdmin::deserializeSetPoliciesRequest(ProtocolFrameHeader &fra } } - ProtocolDeserialization::deserialize(frame, toBeRemovedCount); + ProtocolDeserialization::deserialize(m_frameHeader, toBeRemovedCount); for (ProtocolFrameFieldsCount b = 0; b < toBeRemovedCount; ++b) { PolicyBucketId policyBucketId; - ProtocolDeserialization::deserialize(frame, policyBucketId); - ProtocolDeserialization::deserialize(frame, policyCount); + ProtocolDeserialization::deserialize(m_frameHeader, policyBucketId); + ProtocolDeserialization::deserialize(m_frameHeader, policyCount); for (ProtocolFrameFieldsCount p = 0; p < policyCount; ++p) { // PolicyKey - ProtocolDeserialization::deserialize(frame, clientId); - ProtocolDeserialization::deserialize(frame, user); - ProtocolDeserialization::deserialize(frame, privilege); + ProtocolDeserialization::deserialize(m_frameHeader, clientId); + ProtocolDeserialization::deserialize(m_frameHeader, user); + ProtocolDeserialization::deserialize(m_frameHeader, privilege); toBeRemovedPolicies[policyBucketId].push_back(PolicyKey(clientId, user, privilege)); } @@ -144,7 +145,8 @@ RequestPtr ProtocolAdmin::deserializeSetPoliciesRequest(ProtocolFrameHeader &fra "remove count [%" PRIu16 "]", toBeInsertedOrUpdatedCount, toBeRemovedCount); return std::make_shared(toBeInsertedOrUpdatedPolicies, - toBeRemovedPolicies, frame.sequenceNumber()); + toBeRemovedPolicies, + m_frameHeader.sequenceNumber()); } RequestPtr ProtocolAdmin::extractRequestFromBuffer(BinaryQueue &bufferQueue) { @@ -158,13 +160,13 @@ RequestPtr ProtocolAdmin::extractRequestFromBuffer(BinaryQueue &bufferQueue) { LOGD("Deserialized opCode [%" PRIu8 "]", opCode); switch (opCode) { case OpAdminCheckRequest: - return deserializeAdminCheckRequest(m_frameHeader); + return deserializeAdminCheckRequest(); case OpInsertOrUpdateBucket: - return deserializeInsertOrUpdateBucketRequest(m_frameHeader); + return deserializeInsertOrUpdateBucketRequest(); case OpRemoveBucket: - return deserializeRemoveBucketRequest(m_frameHeader); + return deserializeRemoveBucketRequest(); case OpSetPolicies: - return deserializeSetPoliciesRequest(m_frameHeader); + return deserializeSetPoliciesRequest(); default: throw InvalidProtocolException(InvalidProtocolException::WrongOpCode); break; @@ -174,29 +176,29 @@ RequestPtr ProtocolAdmin::extractRequestFromBuffer(BinaryQueue &bufferQueue) { return nullptr; } -ResponsePtr ProtocolAdmin::deserializeCheckResponse(ProtocolFrameHeader &frame) { +ResponsePtr ProtocolAdmin::deserializeCheckResponse(void) { PolicyType result; PolicyResult::PolicyMetadata additionalInfo; - ProtocolDeserialization::deserialize(frame, result); - ProtocolDeserialization::deserialize(frame, additionalInfo); + ProtocolDeserialization::deserialize(m_frameHeader, result); + ProtocolDeserialization::deserialize(m_frameHeader, additionalInfo); const PolicyResult policyResult(result, additionalInfo); LOGD("Deserialized CheckResponse: result [%" PRIu16 "], metadata <%s>", policyResult.policyType(), policyResult.metadata().c_str()); - return std::make_shared(policyResult, frame.sequenceNumber()); + return std::make_shared(policyResult, m_frameHeader.sequenceNumber()); } -ResponsePtr ProtocolAdmin::deserializeCodeResponse(ProtocolFrameHeader &frame) { +ResponsePtr ProtocolAdmin::deserializeCodeResponse(void) { ProtocolResponseCode responseCode; - ProtocolDeserialization::deserialize(frame, responseCode); + ProtocolDeserialization::deserialize(m_frameHeader, responseCode); LOGD("Deserialized CodeResponse: code [%" PRIu16 "], ", responseCode); return std::make_shared(static_cast(responseCode), - frame.sequenceNumber()); + m_frameHeader.sequenceNumber()); } ResponsePtr ProtocolAdmin::extractResponseFromBuffer(BinaryQueue &bufferQueue) { @@ -210,9 +212,9 @@ ResponsePtr ProtocolAdmin::extractResponseFromBuffer(BinaryQueue &bufferQueue) { LOGD("Deserialized opCode [%" PRIu8 "]", opCode); switch (opCode) { case OpCheckPolicyResponse: - return deserializeCheckResponse(m_frameHeader); + return deserializeCheckResponse(); case OpCodeResponse: - return deserializeCodeResponse(m_frameHeader); + return deserializeCodeResponse(); default: throw InvalidProtocolException(InvalidProtocolException::WrongOpCode); break; diff --git a/src/common/protocol/ProtocolAdmin.h b/src/common/protocol/ProtocolAdmin.h index 2d1e702..3f7fcc9 100644 --- a/src/common/protocol/ProtocolAdmin.h +++ b/src/common/protocol/ProtocolAdmin.h @@ -47,13 +47,13 @@ public: virtual void execute(RequestContextPtr context, CodeResponsePtr response); private: - RequestPtr deserializeAdminCheckRequest(ProtocolFrameHeader &frame); - RequestPtr deserializeInsertOrUpdateBucketRequest(ProtocolFrameHeader &frame); - RequestPtr deserializeRemoveBucketRequest(ProtocolFrameHeader &frame); - RequestPtr deserializeSetPoliciesRequest(ProtocolFrameHeader &frame); + RequestPtr deserializeAdminCheckRequest(void); + RequestPtr deserializeInsertOrUpdateBucketRequest(void); + RequestPtr deserializeRemoveBucketRequest(void); + RequestPtr deserializeSetPoliciesRequest(void); - ResponsePtr deserializeCheckResponse(ProtocolFrameHeader &frame); - ResponsePtr deserializeCodeResponse(ProtocolFrameHeader &frame); + ResponsePtr deserializeCheckResponse(void); + ResponsePtr deserializeCodeResponse(void); }; } // namespace Cynara diff --git a/src/common/protocol/ProtocolClient.cpp b/src/common/protocol/ProtocolClient.cpp index c643789..c130809 100644 --- a/src/common/protocol/ProtocolClient.cpp +++ b/src/common/protocol/ProtocolClient.cpp @@ -54,23 +54,23 @@ ProtocolPtr ProtocolClient::clone(void) { return std::make_shared(); } -RequestPtr ProtocolClient::deserializeCancelRequest(ProtocolFrameHeader &frame) { +RequestPtr ProtocolClient::deserializeCancelRequest(void) { LOGD("Deserialized CancelRequest"); - return std::make_shared(frame.sequenceNumber()); + return std::make_shared(m_frameHeader.sequenceNumber()); } -RequestPtr ProtocolClient::deserializeCheckRequest(ProtocolFrameHeader &frame) { +RequestPtr ProtocolClient::deserializeCheckRequest(void) { std::string clientId, userId, privilegeId; - ProtocolDeserialization::deserialize(frame, clientId); - ProtocolDeserialization::deserialize(frame, userId); - ProtocolDeserialization::deserialize(frame, privilegeId); + ProtocolDeserialization::deserialize(m_frameHeader, clientId); + ProtocolDeserialization::deserialize(m_frameHeader, userId); + ProtocolDeserialization::deserialize(m_frameHeader, privilegeId); LOGD("Deserialized CheckRequest: client <%s>, user <%s>, privilege <%s>", clientId.c_str(), userId.c_str(), privilegeId.c_str()); return std::make_shared(PolicyKey(clientId, userId, privilegeId), - frame.sequenceNumber()); + m_frameHeader.sequenceNumber()); } RequestPtr ProtocolClient::extractRequestFromBuffer(BinaryQueue &bufferQueue) { @@ -85,9 +85,9 @@ RequestPtr ProtocolClient::extractRequestFromBuffer(BinaryQueue &bufferQueue) { LOGD("Deserialized opCode [%" PRIu8 "]", opCode); switch (opCode) { case OpCheckPolicyRequest: - return deserializeCheckRequest(m_frameHeader); + return deserializeCheckRequest(); case OpCancelRequest: - return deserializeCancelRequest(m_frameHeader); + return deserializeCancelRequest(); default: throw InvalidProtocolException(InvalidProtocolException::WrongOpCode); break; @@ -97,24 +97,24 @@ RequestPtr ProtocolClient::extractRequestFromBuffer(BinaryQueue &bufferQueue) { return nullptr; } -ResponsePtr ProtocolClient::deserializeCancelResponse(ProtocolFrameHeader &frame) { +ResponsePtr ProtocolClient::deserializeCancelResponse(void) { LOGD("Deserialized CancelResponse"); - return std::make_shared(frame.sequenceNumber()); + return std::make_shared(m_frameHeader.sequenceNumber()); } -ResponsePtr ProtocolClient::deserializeCheckResponse(ProtocolFrameHeader &frame) { +ResponsePtr ProtocolClient::deserializeCheckResponse(void) { PolicyType result; PolicyResult::PolicyMetadata additionalInfo; - ProtocolDeserialization::deserialize(frame, result); - ProtocolDeserialization::deserialize(frame, additionalInfo); + ProtocolDeserialization::deserialize(m_frameHeader, result); + ProtocolDeserialization::deserialize(m_frameHeader, additionalInfo); const PolicyResult policyResult(result, additionalInfo); LOGD("Deserialized CheckResponse: result [%" PRIu16 "], metadata <%s>", policyResult.policyType(), policyResult.metadata().c_str()); - return std::make_shared(policyResult, frame.sequenceNumber()); + return std::make_shared(policyResult, m_frameHeader.sequenceNumber()); } ResponsePtr ProtocolClient::extractResponseFromBuffer(BinaryQueue &bufferQueue) { @@ -128,9 +128,9 @@ ResponsePtr ProtocolClient::extractResponseFromBuffer(BinaryQueue &bufferQueue) LOGD("Deserialized opCode [%" PRIu8 "]", opCode); switch (opCode) { case OpCheckPolicyResponse: - return deserializeCheckResponse(m_frameHeader); + return deserializeCheckResponse(); case OpCancelResponse: - return deserializeCancelResponse(m_frameHeader); + return deserializeCancelResponse(); default: throw InvalidProtocolException(InvalidProtocolException::WrongOpCode); break; diff --git a/src/common/protocol/ProtocolClient.h b/src/common/protocol/ProtocolClient.h index 1bb1b6a..dc72e7a 100644 --- a/src/common/protocol/ProtocolClient.h +++ b/src/common/protocol/ProtocolClient.h @@ -48,11 +48,11 @@ public: virtual void execute(RequestContextPtr context, CheckResponsePtr response); private: - RequestPtr deserializeCancelRequest(ProtocolFrameHeader &frame); - RequestPtr deserializeCheckRequest(ProtocolFrameHeader &frame); + RequestPtr deserializeCancelRequest(void); + RequestPtr deserializeCheckRequest(void); - ResponsePtr deserializeCancelResponse(ProtocolFrameHeader &frame); - ResponsePtr deserializeCheckResponse(ProtocolFrameHeader &frame); + ResponsePtr deserializeCancelResponse(void); + ResponsePtr deserializeCheckResponse(void); }; } // namespace Cynara -- 2.7.4 From cb00a4e9ea320a3f74cb28b06e30bb7b5e43d613 Mon Sep 17 00:00:00 2001 From: Zofia Abramowska Date: Wed, 12 Nov 2014 16:36:52 +0100 Subject: [PATCH 14/16] Remove dangerous reference RequestContext contained reference to an external BinaryQueue. One problem was, BBQ was held inside vector (so practically any operation on vector made this object out-of-date), second problem was, RequestContext was passed to other classes inside shared_ptr, so owner of this bbq looses control other its reference. Moreover, soon RequestContext will be held pending (e.g. when waiting for external Agent to return answer) inside cynara logic, so BBQ stored inside RequestContext needs to be alive as long as corresponding connection is opened. Not more, not less. Change-Id: I79c9eb9b5e74927bd7bb159da01fae23612ca83e --- src/client-async/sockets/SocketClientAsync.cpp | 8 +-- src/client-async/sockets/SocketClientAsync.h | 4 +- src/common/containers/BinaryQueue.h | 67 ++++++++++++------------- src/common/exceptions/ContextErrorException.h | 42 ++++++++++++++++ src/common/protocol/Protocol.h | 4 +- src/common/protocol/ProtocolAdmin.cpp | 16 +++--- src/common/protocol/ProtocolAdmin.h | 4 +- src/common/protocol/ProtocolClient.cpp | 12 ++--- src/common/protocol/ProtocolClient.h | 4 +- src/common/protocol/ProtocolFrameSerializer.cpp | 8 +-- src/common/protocol/ProtocolFrameSerializer.h | 2 +- src/common/protocol/ProtocolSignal.cpp | 8 +-- src/common/protocol/ProtocolSignal.h | 4 +- src/common/request/RequestContext.h | 12 +++-- src/common/sockets/SocketClient.cpp | 6 ++- src/common/sockets/SocketClient.h | 4 +- src/service/sockets/Descriptor.cpp | 24 ++++++--- src/service/sockets/Descriptor.h | 26 +++++----- 18 files changed, 158 insertions(+), 97 deletions(-) create mode 100644 src/common/exceptions/ContextErrorException.h diff --git a/src/client-async/sockets/SocketClientAsync.cpp b/src/client-async/sockets/SocketClientAsync.cpp index e3ffbd4..148b13a 100644 --- a/src/client-async/sockets/SocketClientAsync.cpp +++ b/src/client-async/sockets/SocketClientAsync.cpp @@ -30,6 +30,8 @@ namespace Cynara { SocketClientAsync::SocketClientAsync(const std::string &socketPath, ProtocolPtr protocol) : m_socket(socketPath, 0), m_protocol(protocol) { + m_readQueue = std::make_shared(); + m_writeQueue = std::make_shared(); } Socket::ConnectionStatus SocketClientAsync::connect(void) { @@ -54,15 +56,15 @@ void SocketClientAsync::appendRequest(RequestPtr request) { } bool SocketClientAsync::isDataToSend(void) { - return m_socket.isDataToSend() || !m_writeQueue.empty(); + return m_socket.isDataToSend() || !m_writeQueue->empty(); } Socket::SendStatus SocketClientAsync::sendToCynara(void) { - return m_socket.sendToServer(m_writeQueue); + return m_socket.sendToServer(*m_writeQueue); } bool SocketClientAsync::receiveFromCynara(void) { - return m_socket.receiveFromServer(m_readQueue); + return m_socket.receiveFromServer(*m_readQueue); } ResponsePtr SocketClientAsync::getResponse(void) { diff --git a/src/client-async/sockets/SocketClientAsync.h b/src/client-async/sockets/SocketClientAsync.h index 5fb0543..eaacf84 100644 --- a/src/client-async/sockets/SocketClientAsync.h +++ b/src/client-async/sockets/SocketClientAsync.h @@ -56,8 +56,8 @@ public: private: Socket m_socket; ProtocolPtr m_protocol; - BinaryQueue m_readQueue; - BinaryQueue m_writeQueue; + BinaryQueuePtr m_readQueue; + BinaryQueuePtr m_writeQueue; }; } // namespace Cynara diff --git a/src/common/containers/BinaryQueue.h b/src/common/containers/BinaryQueue.h index f69f786..12b2e8a 100644 --- a/src/common/containers/BinaryQueue.h +++ b/src/common/containers/BinaryQueue.h @@ -33,52 +33,20 @@ namespace Cynara { */ class BinaryQueue; typedef std::shared_ptr BinaryQueuePtr; +typedef std::weak_ptr BinaryQueueWeakPtr; /** * Binary stream implemented as constant size bucket list * * @todo Add optimized implementation for FlattenConsume */ -class BinaryQueue -{ - public: +class BinaryQueue { +public: typedef void (*BufferDeleter)(const void *buffer, size_t bufferSize, void *userParam); static void bufferDeleterFree(const void *buffer, size_t bufferSize, void *userParam); - - private: - struct Bucket - { - const void *buffer; - const void *ptr; - size_t size; - size_t left; - - BufferDeleter deleter; - void *param; - - Bucket(const void *buffer, - size_t bufferSize, - BufferDeleter deleter, - void *userParam); - ~Bucket(); - // make it noncopyable - Bucket(const Bucket &) = delete; - const Bucket &operator=(const Bucket &) = delete; - // make it nonmoveable - Bucket(Bucket &&) = delete; - Bucket &operator=(Bucket &&) = delete; - }; - - typedef std::list BucketList; - BucketList m_buckets; - size_t m_size; - - static void deleteBucket(Bucket *bucket); - - public: /** * Construct empty binary queue */ @@ -239,6 +207,35 @@ class BinaryQueue * is larger than available bytes in binary queue */ void flattenConsume(void *buffer, size_t bufferSize); + +private: + struct Bucket { + const void *buffer; + const void *ptr; + size_t size; + size_t left; + + BufferDeleter deleter; + void *param; + + Bucket(const void *buffer, + size_t bufferSize, + BufferDeleter deleter, + void *userParam); + ~Bucket(); + // make it noncopyable + Bucket(const Bucket &) = delete; + const Bucket &operator=(const Bucket &) = delete; + // make it nonmoveable + Bucket(Bucket &&) = delete; + Bucket &operator=(Bucket &&) = delete; + }; + + typedef std::list BucketList; + BucketList m_buckets; + size_t m_size; + + static void deleteBucket(Bucket *bucket); }; } // namespace Cynara diff --git a/src/common/exceptions/ContextErrorException.h b/src/common/exceptions/ContextErrorException.h new file mode 100644 index 0000000..771065a --- /dev/null +++ b/src/common/exceptions/ContextErrorException.h @@ -0,0 +1,42 @@ +/* + * Copyright (c) 2014 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 src/common/exceptions/ContextErrorException.h + * @author Zofia Abramowska + * @version 1.0 + * @brief Implementation of ContextErrorException.h + */ + +#ifndef SRC_COMMON_EXCEPTIONS_CONTEXTERROREXCEPTION_H_ +#define SRC_COMMON_EXCEPTIONS_CONTEXTERROREXCEPTION_H_ + +#include "Exception.h" + +namespace Cynara { + +class ContextErrorException : public Exception { +public: + ContextErrorException() = default; + virtual ~ContextErrorException() {}; + + virtual const std::string message(void) const { + return "ContextErrorException"; + } +}; + +} /* namespace Cynara */ + +#endif /* SRC_COMMON_EXCEPTIONS_CONTEXTERROREXCEPTION_H_ */ diff --git a/src/common/protocol/Protocol.h b/src/common/protocol/Protocol.h index b98f145..63ee24d 100644 --- a/src/common/protocol/Protocol.h +++ b/src/common/protocol/Protocol.h @@ -44,8 +44,8 @@ public: virtual ProtocolPtr clone(void) = 0; - virtual RequestPtr extractRequestFromBuffer(BinaryQueue &bufferQueue) = 0; - virtual ResponsePtr extractResponseFromBuffer(BinaryQueue &bufferQueue) = 0; + virtual RequestPtr extractRequestFromBuffer(BinaryQueuePtr bufferQueue) = 0; + virtual ResponsePtr extractResponseFromBuffer(BinaryQueuePtr bufferQueue) = 0; ProtocolFrameHeader &frameHeader(void) { return m_frameHeader; diff --git a/src/common/protocol/ProtocolAdmin.cpp b/src/common/protocol/ProtocolAdmin.cpp index 2d4c879..8d792f9 100644 --- a/src/common/protocol/ProtocolAdmin.cpp +++ b/src/common/protocol/ProtocolAdmin.cpp @@ -149,7 +149,7 @@ RequestPtr ProtocolAdmin::deserializeSetPoliciesRequest(void) { m_frameHeader.sequenceNumber()); } -RequestPtr ProtocolAdmin::extractRequestFromBuffer(BinaryQueue &bufferQueue) { +RequestPtr ProtocolAdmin::extractRequestFromBuffer(BinaryQueuePtr bufferQueue) { ProtocolFrameSerializer::deserializeHeader(m_frameHeader, bufferQueue); if (m_frameHeader.isFrameComplete()) { @@ -201,7 +201,7 @@ ResponsePtr ProtocolAdmin::deserializeCodeResponse(void) { m_frameHeader.sequenceNumber()); } -ResponsePtr ProtocolAdmin::extractResponseFromBuffer(BinaryQueue &bufferQueue) { +ResponsePtr ProtocolAdmin::extractResponseFromBuffer(BinaryQueuePtr bufferQueue) { ProtocolFrameSerializer::deserializeHeader(m_frameHeader, bufferQueue); if (m_frameHeader.isFrameComplete()) { @@ -239,7 +239,7 @@ void ProtocolAdmin::execute(RequestContextPtr context, AdminCheckRequestPtr requ ProtocolSerialization::serialize(frame, request->startBucket()); ProtocolSerialization::serialize(frame, request->recursive()); - ProtocolFrameSerializer::finishSerialization(frame, context->responseQueue()); + ProtocolFrameSerializer::finishSerialization(frame, *(context->responseQueue())); } void ProtocolAdmin::execute(RequestContextPtr context, InsertOrUpdateBucketRequestPtr request) { @@ -255,7 +255,7 @@ void ProtocolAdmin::execute(RequestContextPtr context, InsertOrUpdateBucketReque ProtocolSerialization::serialize(frame, request->result().policyType()); ProtocolSerialization::serialize(frame, request->result().metadata()); - ProtocolFrameSerializer::finishSerialization(frame, context->responseQueue()); + ProtocolFrameSerializer::finishSerialization(frame, *(context->responseQueue())); } void ProtocolAdmin::execute(RequestContextPtr context, RemoveBucketRequestPtr request) { @@ -267,7 +267,7 @@ void ProtocolAdmin::execute(RequestContextPtr context, RemoveBucketRequestPtr re ProtocolSerialization::serialize(frame, OpRemoveBucket); ProtocolSerialization::serialize(frame, request->bucketId()); - ProtocolFrameSerializer::finishSerialization(frame, context->responseQueue()); + ProtocolFrameSerializer::finishSerialization(frame, *(context->responseQueue())); } void ProtocolAdmin::execute(RequestContextPtr context, SetPoliciesRequestPtr request) { @@ -311,7 +311,7 @@ void ProtocolAdmin::execute(RequestContextPtr context, SetPoliciesRequestPtr req } } - ProtocolFrameSerializer::finishSerialization(frame, context->responseQueue()); + ProtocolFrameSerializer::finishSerialization(frame, *(context->responseQueue())); } void ProtocolAdmin::execute(RequestContextPtr context, CheckResponsePtr response) { @@ -327,7 +327,7 @@ void ProtocolAdmin::execute(RequestContextPtr context, CheckResponsePtr response ProtocolSerialization::serialize(frame, response->m_resultRef.policyType()); ProtocolSerialization::serialize(frame, response->m_resultRef.metadata()); - ProtocolFrameSerializer::finishSerialization(frame, context->responseQueue()); + ProtocolFrameSerializer::finishSerialization(frame, *(context->responseQueue())); } void ProtocolAdmin::execute(RequestContextPtr context, CodeResponsePtr response) { @@ -340,7 +340,7 @@ void ProtocolAdmin::execute(RequestContextPtr context, CodeResponsePtr response) ProtocolSerialization::serialize(frame, OpCodeResponse); ProtocolSerialization::serialize(frame, static_cast(response->m_code)); - ProtocolFrameSerializer::finishSerialization(frame, context->responseQueue()); + ProtocolFrameSerializer::finishSerialization(frame, *(context->responseQueue())); } } // namespace Cynara diff --git a/src/common/protocol/ProtocolAdmin.h b/src/common/protocol/ProtocolAdmin.h index 3f7fcc9..9d2c6ac 100644 --- a/src/common/protocol/ProtocolAdmin.h +++ b/src/common/protocol/ProtocolAdmin.h @@ -35,8 +35,8 @@ public: virtual ProtocolPtr clone(void); - virtual RequestPtr extractRequestFromBuffer(BinaryQueue &bufferQueue); - virtual ResponsePtr extractResponseFromBuffer(BinaryQueue &bufferQueue); + virtual RequestPtr extractRequestFromBuffer(BinaryQueuePtr bufferQueue); + virtual ResponsePtr extractResponseFromBuffer(BinaryQueuePtr bufferQueue); virtual void execute(RequestContextPtr context, AdminCheckRequestPtr request); virtual void execute(RequestContextPtr context, InsertOrUpdateBucketRequestPtr request); diff --git a/src/common/protocol/ProtocolClient.cpp b/src/common/protocol/ProtocolClient.cpp index c130809..3f13653 100644 --- a/src/common/protocol/ProtocolClient.cpp +++ b/src/common/protocol/ProtocolClient.cpp @@ -73,7 +73,7 @@ RequestPtr ProtocolClient::deserializeCheckRequest(void) { m_frameHeader.sequenceNumber()); } -RequestPtr ProtocolClient::extractRequestFromBuffer(BinaryQueue &bufferQueue) { +RequestPtr ProtocolClient::extractRequestFromBuffer(BinaryQueuePtr bufferQueue) { ProtocolFrameSerializer::deserializeHeader(m_frameHeader, bufferQueue); if (m_frameHeader.isFrameComplete()) { @@ -117,7 +117,7 @@ ResponsePtr ProtocolClient::deserializeCheckResponse(void) { return std::make_shared(policyResult, m_frameHeader.sequenceNumber()); } -ResponsePtr ProtocolClient::extractResponseFromBuffer(BinaryQueue &bufferQueue) { +ResponsePtr ProtocolClient::extractResponseFromBuffer(BinaryQueuePtr bufferQueue) { ProtocolFrameSerializer::deserializeHeader(m_frameHeader, bufferQueue); if (m_frameHeader.isFrameComplete()) { @@ -147,7 +147,7 @@ void ProtocolClient::execute(RequestContextPtr context, CancelRequestPtr request ProtocolSerialization::serialize(frame, OpCancelRequest); - ProtocolFrameSerializer::finishSerialization(frame, context->responseQueue()); + ProtocolFrameSerializer::finishSerialization(frame, *(context->responseQueue())); } void ProtocolClient::execute(RequestContextPtr context, CheckRequestPtr request) { @@ -162,7 +162,7 @@ void ProtocolClient::execute(RequestContextPtr context, CheckRequestPtr request) ProtocolSerialization::serialize(frame, request->key().user().value()); ProtocolSerialization::serialize(frame, request->key().privilege().value()); - ProtocolFrameSerializer::finishSerialization(frame, context->responseQueue()); + ProtocolFrameSerializer::finishSerialization(frame, *(context->responseQueue())); } void ProtocolClient::execute(RequestContextPtr context, CancelResponsePtr response) { @@ -173,7 +173,7 @@ void ProtocolClient::execute(RequestContextPtr context, CancelResponsePtr respon ProtocolSerialization::serialize(frame, OpCancelResponse); - ProtocolFrameSerializer::finishSerialization(frame, context->responseQueue()); + ProtocolFrameSerializer::finishSerialization(frame, *(context->responseQueue())); } void ProtocolClient::execute(RequestContextPtr context, CheckResponsePtr response) { @@ -188,7 +188,7 @@ void ProtocolClient::execute(RequestContextPtr context, CheckResponsePtr respons ProtocolSerialization::serialize(frame, response->m_resultRef.policyType()); ProtocolSerialization::serialize(frame, response->m_resultRef.metadata()); - ProtocolFrameSerializer::finishSerialization(frame, context->responseQueue()); + ProtocolFrameSerializer::finishSerialization(frame, *(context->responseQueue())); } } // namespace Cynara diff --git a/src/common/protocol/ProtocolClient.h b/src/common/protocol/ProtocolClient.h index dc72e7a..a1c5110 100644 --- a/src/common/protocol/ProtocolClient.h +++ b/src/common/protocol/ProtocolClient.h @@ -38,8 +38,8 @@ public: virtual ProtocolPtr clone(void); - virtual RequestPtr extractRequestFromBuffer(BinaryQueue &bufferQueue); - virtual ResponsePtr extractResponseFromBuffer(BinaryQueue &bufferQueue); + virtual RequestPtr extractRequestFromBuffer(BinaryQueuePtr bufferQueue); + virtual ResponsePtr extractResponseFromBuffer(BinaryQueuePtr bufferQueue); virtual void execute(RequestContextPtr context, CancelRequestPtr request); virtual void execute(RequestContextPtr context, CheckRequestPtr request); diff --git a/src/common/protocol/ProtocolFrameSerializer.cpp b/src/common/protocol/ProtocolFrameSerializer.cpp index 0599f02..68c497c 100644 --- a/src/common/protocol/ProtocolFrameSerializer.cpp +++ b/src/common/protocol/ProtocolFrameSerializer.cpp @@ -31,15 +31,15 @@ namespace Cynara { void ProtocolFrameSerializer::deserializeHeader(ProtocolFrameHeader &frameHeader, - BinaryQueue &data) { + BinaryQueuePtr data) { if (!frameHeader.isHeaderComplete()) { - if ((data.size() < ProtocolFrameHeader::frameHeaderLength())) { + if ((data->size() < ProtocolFrameHeader::frameHeaderLength())) { return; } LOGD("Deserializing frameHeader"); - frameHeader.setHeaderContent(BinaryQueuePtr(&data, [=] (BinaryQueue *) {})); + frameHeader.setHeaderContent(data); ProtocolFrameSignature signature; ProtocolDeserialization::deserialize(frameHeader, frameHeader.m_signature.length(), @@ -60,7 +60,7 @@ void ProtocolFrameSerializer::deserializeHeader(ProtocolFrameHeader &frameHeader frameHeader.setHeaderComplete(); } - if (data.size() >= (frameHeader.frameLength() - ProtocolFrameHeader::frameHeaderLength())) { + if (data->size() >= (frameHeader.frameLength() - ProtocolFrameHeader::frameHeaderLength())) { frameHeader.setBodyComplete(); } } diff --git a/src/common/protocol/ProtocolFrameSerializer.h b/src/common/protocol/ProtocolFrameSerializer.h index f2e201e..d7d6c15 100644 --- a/src/common/protocol/ProtocolFrameSerializer.h +++ b/src/common/protocol/ProtocolFrameSerializer.h @@ -33,7 +33,7 @@ namespace Cynara { class ProtocolFrameSerializer { public: - static void deserializeHeader(ProtocolFrameHeader &frameHeader, BinaryQueue &data); + static void deserializeHeader(ProtocolFrameHeader &frameHeader, BinaryQueuePtr data); static ProtocolFrame startSerialization(ProtocolFrameSequenceNumber sequenceNumber); static void finishSerialization(ProtocolFrame &frame, BinaryQueue &data); }; diff --git a/src/common/protocol/ProtocolSignal.cpp b/src/common/protocol/ProtocolSignal.cpp index 963ee9a..d04dbbe 100644 --- a/src/common/protocol/ProtocolSignal.cpp +++ b/src/common/protocol/ProtocolSignal.cpp @@ -46,17 +46,17 @@ ProtocolPtr ProtocolSignal::clone(void) { return std::make_shared(); } -RequestPtr ProtocolSignal::extractRequestFromBuffer(BinaryQueue &bufferQueue) { - if (bufferQueue.size() >= sizeof(struct signalfd_siginfo)) { +RequestPtr ProtocolSignal::extractRequestFromBuffer(BinaryQueuePtr bufferQueue) { + if (bufferQueue->size() >= sizeof(struct signalfd_siginfo)) { struct signalfd_siginfo sigInfo; - bufferQueue.flattenConsume(&sigInfo, sizeof(sigInfo)); + bufferQueue->flattenConsume(&sigInfo, sizeof(sigInfo)); return std::make_shared(sigInfo); } return nullptr; } -ResponsePtr ProtocolSignal::extractResponseFromBuffer(BinaryQueue &bufferQueue UNUSED) { +ResponsePtr ProtocolSignal::extractResponseFromBuffer(BinaryQueuePtr bufferQueue UNUSED) { throw NotImplementedException(); } diff --git a/src/common/protocol/ProtocolSignal.h b/src/common/protocol/ProtocolSignal.h index e2142da..6c544c4 100644 --- a/src/common/protocol/ProtocolSignal.h +++ b/src/common/protocol/ProtocolSignal.h @@ -36,8 +36,8 @@ public: virtual ProtocolPtr clone(void); - virtual RequestPtr extractRequestFromBuffer(BinaryQueue &bufferQueue); - virtual ResponsePtr extractResponseFromBuffer(BinaryQueue &bufferQueue); + virtual RequestPtr extractRequestFromBuffer(BinaryQueuePtr bufferQueue); + virtual ResponsePtr extractResponseFromBuffer(BinaryQueuePtr bufferQueue); virtual void execute(RequestContextPtr context, SignalRequestPtr request); }; diff --git a/src/common/request/RequestContext.h b/src/common/request/RequestContext.h index 33b6ef4..9c98bef 100644 --- a/src/common/request/RequestContext.h +++ b/src/common/request/RequestContext.h @@ -26,6 +26,7 @@ #include #include +#include #include #include #include @@ -36,10 +37,10 @@ namespace Cynara { class RequestContext { private: ResponseTakerWeakPtr m_responseTaker; - BinaryQueue &m_responseQueue; + BinaryQueueWeakPtr m_responseQueue; public: - RequestContext(ResponseTakerPtr responseTaker, BinaryQueue &responseQueue) + RequestContext(ResponseTakerPtr responseTaker, BinaryQueuePtr responseQueue) : m_responseTaker(responseTaker), m_responseQueue(responseQueue) { } @@ -49,8 +50,11 @@ public: response->execute(response, taker, self); } - BinaryQueue &responseQueue(void) const { - return m_responseQueue; + BinaryQueuePtr responseQueue(void) const { + auto bbqPtr = m_responseQueue.lock(); + if (bbqPtr) + return bbqPtr; + throw ContextErrorException(); } }; diff --git a/src/common/sockets/SocketClient.cpp b/src/common/sockets/SocketClient.cpp index 6db91d6..b3ce8ad 100644 --- a/src/common/sockets/SocketClient.cpp +++ b/src/common/sockets/SocketClient.cpp @@ -37,6 +37,8 @@ namespace Cynara { SocketClient::SocketClient(const std::string &socketPath, ProtocolPtr protocol) : m_socket(socketPath), m_protocol(protocol) { + m_writeQueue = std::make_shared(); + m_readQueue = std::make_shared(); } bool SocketClient::connect(void) { @@ -64,14 +66,14 @@ ResponsePtr SocketClient::askCynaraServer(RequestPtr request) { request->execute(request, m_protocol, context); //send request to cynara - if (m_socket.sendToServer(m_writeQueue) == Socket::SendStatus::CONNECTION_LOST) { + if (m_socket.sendToServer(*m_writeQueue) == Socket::SendStatus::CONNECTION_LOST) { LOGW("Disconnected while sending request to Cynara."); return nullptr; } // receive response from cynara while (true) { - if (!m_socket.receiveFromServer(m_readQueue)) { + if (!m_socket.receiveFromServer(*m_readQueue)) { LOGW("Disconnected while receiving response from Cynara."); return nullptr; } diff --git a/src/common/sockets/SocketClient.h b/src/common/sockets/SocketClient.h index 9f9737f..c313b13 100644 --- a/src/common/sockets/SocketClient.h +++ b/src/common/sockets/SocketClient.h @@ -41,8 +41,8 @@ class SocketClient { private: Socket m_socket; ProtocolPtr m_protocol; - BinaryQueue m_readQueue; - BinaryQueue m_writeQueue; + BinaryQueuePtr m_readQueue; + BinaryQueuePtr m_writeQueue; public: SocketClient(const std::string &socketPath, ProtocolPtr protocol); diff --git a/src/service/sockets/Descriptor.cpp b/src/service/sockets/Descriptor.cpp index 0e55e4b..b2e9459 100644 --- a/src/service/sockets/Descriptor.cpp +++ b/src/service/sockets/Descriptor.cpp @@ -27,8 +27,17 @@ namespace Cynara { Descriptor::Descriptor() : m_listen(false), m_used(false), m_client(false), m_protocol(nullptr) { } +void Descriptor::checkQueues(void) { + if (!m_writeQueue) + m_writeQueue = std::make_shared(); + if (!m_readQueue) + m_readQueue = std::make_shared(); +} + bool Descriptor::hasDataToWrite(void) const { - return !(m_writeQueue.empty() && m_writeBuffer.empty()); + if (m_writeQueue) + return !(m_writeQueue->empty() && m_writeBuffer.empty()); + return false; } ResponseTakerPtr Descriptor::responseTaker(void) const { @@ -36,19 +45,22 @@ ResponseTakerPtr Descriptor::responseTaker(void) const { } void Descriptor::pushReadBuffer(const RawBuffer &readbuffer) { - m_readQueue.appendCopy(readbuffer.data(), readbuffer.size()); + checkQueues(); + m_readQueue->appendCopy(readbuffer.data(), readbuffer.size()); } RequestPtr Descriptor::extractRequest(void) { + checkQueues(); return m_protocol->extractRequestFromBuffer(m_readQueue); } RawBuffer &Descriptor::prepareWriteBuffer(void) { - size_t queuedDataSize = m_writeQueue.size(); + checkQueues(); + size_t queuedDataSize = m_writeQueue->size(); size_t bufferDataSize = m_writeBuffer.size(); m_writeBuffer.resize(queuedDataSize + bufferDataSize); - m_writeQueue.flattenConsume(m_writeBuffer.data() + bufferDataSize, queuedDataSize); + m_writeQueue->flattenConsume(m_writeBuffer.data() + bufferDataSize, queuedDataSize); return m_writeBuffer; } @@ -57,8 +69,8 @@ void Descriptor::clear(void) { m_listen = false; m_used = false; m_client = false; - m_readQueue.clear(); - m_writeQueue.clear(); + m_readQueue.reset(); + m_writeQueue.reset(); m_writeBuffer.clear(); m_protocol.reset(); } diff --git a/src/service/sockets/Descriptor.h b/src/service/sockets/Descriptor.h index 0f42a36..5d77028 100644 --- a/src/service/sockets/Descriptor.h +++ b/src/service/sockets/Descriptor.h @@ -34,17 +34,6 @@ namespace Cynara { class Descriptor { -private: - bool m_listen; - bool m_used; - bool m_client; - - BinaryQueue m_readQueue; - BinaryQueue m_writeQueue; - RawBuffer m_writeBuffer; - - ProtocolPtr m_protocol; - public: Descriptor(); @@ -68,7 +57,7 @@ public: ResponseTakerPtr responseTaker(void) const; - BinaryQueue &writeQueue(void) { + BinaryQueuePtr writeQueue(void) { return m_writeQueue; } @@ -94,6 +83,19 @@ public: RawBuffer &prepareWriteBuffer(void); void clear(void); + +private: + bool m_listen; + bool m_used; + bool m_client; + + BinaryQueuePtr m_readQueue; + BinaryQueuePtr m_writeQueue; + RawBuffer m_writeBuffer; + + ProtocolPtr m_protocol; + + void checkQueues(void); }; } // namespace Cynara -- 2.7.4 From 86891d2550fcec655be8a7d4d53037a626c42b2d Mon Sep 17 00:00:00 2001 From: Pawel Wieczorek Date: Fri, 14 Nov 2014 13:04:19 +0100 Subject: [PATCH 15/16] Disallow adding valid and invalid policies at once Storage::insertPolicies() now cares, if all buckets exist before it makes any change in database (in memory as well as in storage). No changes are made if any part of request contains invalid parameters. Change-Id: Ia8d180c7af88bd945dca22f2a4a41b049fdb4c33 --- src/storage/Storage.cpp | 6 ++++++ test/storage/storage/policies.cpp | 32 ++++++++++++++++++++++++++------ 2 files changed, 32 insertions(+), 6 deletions(-) diff --git a/src/storage/Storage.cpp b/src/storage/Storage.cpp index 39793e2..aa9bc93 100644 --- a/src/storage/Storage.cpp +++ b/src/storage/Storage.cpp @@ -101,7 +101,13 @@ void Storage::insertPolicies(const std::map> // TODO: Rewrite, when transactions are supported // Check if all of buckets exist for (const auto &group : policiesByBucketId) { + const auto &bucketId = group.first; const auto &policies = group.second; + + if (m_backend.hasBucket(bucketId) == false) { + throw BucketNotExistsException(bucketId); + } + std::for_each(policies.cbegin(), policies.cend(), pointedBucketExists); } diff --git a/test/storage/storage/policies.cpp b/test/storage/storage/policies.cpp index 0166ff5..8d72dde 100644 --- a/test/storage/storage/policies.cpp +++ b/test/storage/storage/policies.cpp @@ -86,8 +86,12 @@ TEST(storage, insertPolicies) { FakeStorageBackend backend; Storage storage(backend); - PolicyBucketId testBucket1 = "test-bucket-1"; - PolicyBucketId testBucket2 = "test-bucket-2"; + std::vector testBuckets = { + PolicyBucketId("test-bucket-1"), + PolicyBucketId("test-bucket-2"), + }; + + PolicyResult defaultPolicy(PredefinedPolicyType::DENY); typedef std::pair> BucketPolicyPair; @@ -96,18 +100,26 @@ TEST(storage, insertPolicies) { }; std::map> policiesToInsert = { - BucketPolicyPair(testBucket1, { + BucketPolicyPair(testBuckets[0], { createPolicy("1", ALLOW), createPolicy("2", DENY), createPolicy("3", DENY) }), - BucketPolicyPair(testBucket2, { - createPolicy("4", { BUCKET, testBucket1 }), + BucketPolicyPair(testBuckets[1], { + createPolicy("4", { BUCKET, testBuckets[0] }), createPolicy("5", PredefinedPolicyType::ALLOW) }) }; - EXPECT_CALL(backend, hasBucket(testBucket1)).WillOnce(Return(true)); + for (const auto &bucket: testBuckets) { + EXPECT_CALL(backend, hasBucket(bucket)).WillOnce(Return(false)); + EXPECT_CALL(backend, createBucket(bucket, defaultPolicy)).WillOnce(Return()); + + storage.addOrUpdateBucket(bucket, defaultPolicy); + } + + EXPECT_CALL(backend, hasBucket(testBuckets[0])).WillRepeatedly(Return(true)); + EXPECT_CALL(backend, hasBucket(testBuckets[1])).WillOnce(Return(true)); for (const auto &group : policiesToInsert) { const auto &bucketId = group.first; @@ -130,6 +142,8 @@ TEST(storage, insertPointingToNonexistentBucket) { PolicyBucketId testBucketId = "test-bucket-1"; PolicyBucketId nonexistentBucketId = "nonexistent"; + PolicyResult defaultPolicy(PredefinedPolicyType::DENY); + typedef std::pair> BucketPolicyPair; auto createPolicy = [] (const std::string &keySuffix, const PolicyResult &result) -> Policy { @@ -143,6 +157,12 @@ TEST(storage, insertPointingToNonexistentBucket) { }), }; + EXPECT_CALL(backend, hasBucket(testBucketId)).WillOnce(Return(false)); + EXPECT_CALL(backend, createBucket(testBucketId, defaultPolicy)).WillOnce(Return()); + + storage.addOrUpdateBucket(testBucketId, defaultPolicy); + + EXPECT_CALL(backend, hasBucket(testBucketId)).WillOnce(Return(true)); EXPECT_CALL(backend, hasBucket(nonexistentBucketId)).WillOnce(Return(false)); ASSERT_THROW(storage.insertPolicies(policiesToInsert), BucketNotExistsException); -- 2.7.4 From 6013044e77d40a59b49043e82e47332cd69b6d3d Mon Sep 17 00:00:00 2001 From: Zofia Abramowska Date: Fri, 14 Nov 2014 14:29:31 +0100 Subject: [PATCH 16/16] Add context invalidation mechanism RequestTaker gets RequestContext, which might be processed in another event loop. During this loop socket associated with this context might get closed, so class holding this context needs notification. Change-Id: I77dee05b84a987e444f4ec71e87bcb867682768b --- src/common/request/RequestTaker.cpp | 4 ++++ src/common/request/RequestTaker.h | 2 ++ src/service/logic/Logic.cpp | 4 ++++ src/service/logic/Logic.h | 2 ++ src/service/sockets/SocketManager.cpp | 5 ++++- 5 files changed, 16 insertions(+), 1 deletion(-) diff --git a/src/common/request/RequestTaker.cpp b/src/common/request/RequestTaker.cpp index c87a9a4..a4d7c2d 100644 --- a/src/common/request/RequestTaker.cpp +++ b/src/common/request/RequestTaker.cpp @@ -59,4 +59,8 @@ void RequestTaker::execute(RequestContextPtr context UNUSED, SignalRequestPtr re throw NotImplementedException(); } +void RequestTaker::contextClosed(RequestContextPtr context UNUSED) { + throw NotImplementedException(); +} + } // namespace Cynara diff --git a/src/common/request/RequestTaker.h b/src/common/request/RequestTaker.h index b1d3466..522d8bb 100644 --- a/src/common/request/RequestTaker.h +++ b/src/common/request/RequestTaker.h @@ -39,6 +39,8 @@ public: virtual void execute(RequestContextPtr context, RemoveBucketRequestPtr request); virtual void execute(RequestContextPtr context, SetPoliciesRequestPtr request); virtual void execute(RequestContextPtr context, SignalRequestPtr request); + + virtual void contextClosed(RequestContextPtr context); }; } // namespace Cynara diff --git a/src/service/logic/Logic.cpp b/src/service/logic/Logic.cpp index f76ce98..ca56d64 100644 --- a/src/service/logic/Logic.cpp +++ b/src/service/logic/Logic.cpp @@ -174,6 +174,10 @@ void Logic::execute(RequestContextPtr context, SetPoliciesRequestPtr request) { request->sequenceNumber())); } +void Logic::contextClosed(RequestContextPtr context UNUSED) { + //We don't care now, but we will +} + void Logic::onPoliciesChanged(void) { m_storage->save(); m_socketManager->disconnectAllClients(); diff --git a/src/service/logic/Logic.h b/src/service/logic/Logic.h index 3c434a5..e74cffe 100644 --- a/src/service/logic/Logic.h +++ b/src/service/logic/Logic.h @@ -64,6 +64,8 @@ public: virtual void execute(RequestContextPtr context, SetPoliciesRequestPtr request); virtual void execute(RequestContextPtr context, SignalRequestPtr request); + virtual void contextClosed(RequestContextPtr context); + private: PluginManagerPtr m_pluginManager; StoragePtr m_storage; diff --git a/src/service/sockets/SocketManager.cpp b/src/service/sockets/SocketManager.cpp index cb3c9b4..c9e1692 100644 --- a/src/service/sockets/SocketManager.cpp +++ b/src/service/sockets/SocketManager.cpp @@ -210,9 +210,12 @@ void SocketManager::readyForAccept(int fd) { void SocketManager::closeSocket(int fd) { LOGD("SocketManger closeSocket fd [%d] start", fd); + Descriptor &desc = m_fds[fd]; + requestTaker()->contextClosed(std::make_shared(nullptr, + desc.writeQueue())); removeReadSocket(fd); removeWriteSocket(fd); - m_fds[fd].clear(); + desc.clear(); close(fd); LOGD("SocketManger closeSocket fd [%d] done", fd); } -- 2.7.4