From 1c3bee02b5b66ebd389eec9b4fe35c760c170ec9 Mon Sep 17 00:00:00 2001 From: Aleksander Zdyb Date: Tue, 26 Aug 2014 10:41:33 +0200 Subject: [PATCH] Fix startup with half-populated db In some cases, Cynara could start with half-populated database. As this is potential security issue, we now make sure, that in case of any error, Cynara will start with empty database and return DENY for every request. There are added tests revealing these potential issues. Some test cases depend on specific state of Cynara's policy database directory. These are now provided in cynara-tests package and placed in /usr/share/cynara/tests/ during installation. Test execution does not affect real database -- it uses above tests path only, accessible by non-root users as well. Signed-off-by: Aleksander Zdyb Signed-off-by: Pawel Wieczorek Change-Id: Ia943f77a2a0c85f394c40dd10333a73df4d0c96a --- packaging/cynara.spec | 8 ++- .../exceptions/BucketRecordCorruptedException.h | 6 +- src/service/storage/InMemoryStorageBackend.cpp | 11 ++-- test/db/db2/buckets | 1 + test/db/db3/_ | 0 test/db/db3/buckets | 1 + test/db/db4/_ | 0 test/db/db4/_additional | 0 test/db/db4/buckets | 2 + test/db/db5/_ | 0 test/db/db5/_additional | 1 + test/db/db5/buckets | 2 + .../inmemeorystoragebackendfixture.h | 11 ++++ .../inmemorystoragebackend.cpp | 68 +++++++++++++++++++++- 14 files changed, 102 insertions(+), 9 deletions(-) create mode 100644 test/db/db2/buckets create mode 100644 test/db/db3/_ create mode 100644 test/db/db3/buckets create mode 100644 test/db/db4/_ create mode 100644 test/db/db4/_additional create mode 100644 test/db/db4/buckets create mode 100644 test/db/db5/_ create mode 100644 test/db/db5/_additional create mode 100644 test/db/db5/buckets diff --git a/packaging/cynara.spec b/packaging/cynara.spec index 677142b..f4e8d1a 100644 --- a/packaging/cynara.spec +++ b/packaging/cynara.spec @@ -29,6 +29,7 @@ BuildRequires: pkgconfig(libsystemd-journal) %global group_name %{name} %global state_path %{_localstatedir}/%{name}/ +%global tests_dir %{_datarootdir}/%{name}/tests %global build_type %{?build_type:%build_type}%{!?build_type:RELEASE} @@ -195,6 +196,7 @@ cp -a %{SOURCE1007} . cp -a %{SOURCE1008} . cp -a %{SOURCE1009} . cp -a %{SOURCE1010} . +cp -a test/db/db* . %build %if 0%{?sec_build_binary_debug_enable} @@ -205,7 +207,8 @@ export CXXFLAGS="$CXXFLAGS -DTIZEN_DEBUG_ENABLE" export CXXFLAGS="$CXXFLAGS -Wp,-U_FORTIFY_SOURCE" %endif -export CXXFLAGS="$CXXFLAGS -DCYNARA_STATE_PATH=\\\"%{state_path}\\\"" +export CXXFLAGS="$CXXFLAGS -DCYNARA_STATE_PATH=\\\"%{state_path}\\\" \ + -DCYNARA_TESTS_DIR=\\\"%{tests_dir}\\\"" export LDFLAGS+="-Wl,--rpath=%{_libdir}" %cmake . \ @@ -221,6 +224,8 @@ rm -rf %{buildroot} mkdir -p %{buildroot}/usr/lib/systemd/system/sockets.target.wants mkdir -p %{buildroot}/%{state_path} +mkdir -p %{buildroot}/%{tests_dir} +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 @@ -380,6 +385,7 @@ fi %files -n cynara-tests %manifest cynara-tests.manifest %attr(755,root,root) /usr/bin/cynara-tests +%attr(755,root,root) %{tests_dir}/db*/* %files -n libcynara-creds-commons %manifest libcynara-creds-commons.manifest diff --git a/src/common/exceptions/BucketRecordCorruptedException.h b/src/common/exceptions/BucketRecordCorruptedException.h index 8152a77..335fbc9 100644 --- a/src/common/exceptions/BucketRecordCorruptedException.h +++ b/src/common/exceptions/BucketRecordCorruptedException.h @@ -22,13 +22,13 @@ #ifndef SRC_COMMON_EXCEPTIONS_BUCKETRECORDCORRUPTEDEXCEPTION_H_ #define SRC_COMMON_EXCEPTIONS_BUCKETRECORDCORRUPTEDEXCEPTION_H_ -#include "Exception.h" - #include +#include + namespace Cynara { -class BucketRecordCorruptedException : public Exception { +class BucketRecordCorruptedException : public DatabaseException { public: BucketRecordCorruptedException(void) = delete; virtual ~BucketRecordCorruptedException() noexcept {}; diff --git a/src/service/storage/InMemoryStorageBackend.cpp b/src/service/storage/InMemoryStorageBackend.cpp index 9fee197..20c16cf 100644 --- a/src/service/storage/InMemoryStorageBackend.cpp +++ b/src/service/storage/InMemoryStorageBackend.cpp @@ -35,6 +35,7 @@ #include #include #include +#include #include #include #include @@ -63,13 +64,15 @@ void InMemoryStorageBackend::load(void) { storageDeserializer.initBuckets(buckets()); storageDeserializer.loadBuckets(buckets()); - } catch (const FileNotFoundException &) { - LOGE("Reading cynara database failed."); + } catch (const DatabaseException &) { + LOGC("Reading cynara database failed."); + buckets().clear(); + // TODO: Implement emergency mode toggle } if (!hasBucket(defaultPolicyBucketId)) { - LOGN("Creating defaultBucket."); - this->buckets().insert({ defaultPolicyBucketId, PolicyBucket() }); + LOGN("Creating defaultBucket."); + this->buckets().insert({ defaultPolicyBucketId, PolicyBucket() }); } } diff --git a/test/db/db2/buckets b/test/db/db2/buckets new file mode 100644 index 0000000..b6d719f --- /dev/null +++ b/test/db/db2/buckets @@ -0,0 +1 @@ +;0x0 diff --git a/test/db/db3/_ b/test/db/db3/_ new file mode 100644 index 0000000..e69de29 diff --git a/test/db/db3/buckets b/test/db/db3/buckets new file mode 100644 index 0000000..b6d719f --- /dev/null +++ b/test/db/db3/buckets @@ -0,0 +1 @@ +;0x0 diff --git a/test/db/db4/_ b/test/db/db4/_ new file mode 100644 index 0000000..e69de29 diff --git a/test/db/db4/_additional b/test/db/db4/_additional new file mode 100644 index 0000000..e69de29 diff --git a/test/db/db4/buckets b/test/db/db4/buckets new file mode 100644 index 0000000..19d7a93 --- /dev/null +++ b/test/db/db4/buckets @@ -0,0 +1,2 @@ +;0x0 +additional;0xFFFF diff --git a/test/db/db5/_ b/test/db/db5/_ new file mode 100644 index 0000000..e69de29 diff --git a/test/db/db5/_additional b/test/db/db5/_additional new file mode 100644 index 0000000..d30fa55 --- /dev/null +++ b/test/db/db5/_additional @@ -0,0 +1 @@ +INVALID \ No newline at end of file diff --git a/test/db/db5/buckets b/test/db/db5/buckets new file mode 100644 index 0000000..19d7a93 --- /dev/null +++ b/test/db/db5/buckets @@ -0,0 +1,2 @@ +;0x0 +additional;0xFFFF diff --git a/test/storage/inmemorystoragebackend/inmemeorystoragebackendfixture.h b/test/storage/inmemorystoragebackend/inmemeorystoragebackendfixture.h index 2c6e593..298732c 100644 --- a/test/storage/inmemorystoragebackend/inmemeorystoragebackendfixture.h +++ b/test/storage/inmemorystoragebackend/inmemeorystoragebackendfixture.h @@ -24,6 +24,7 @@ #define INMEMEORYSTORAGEBACKENDFIXTURE_H_ #include +#include #include #include @@ -52,6 +53,16 @@ protected: } } + static void ASSERT_DB_VIRGIN(Cynara::Buckets &buckets) { + using ::testing::IsEmpty; + ASSERT_EQ(1, buckets.size()); + auto defaultBucketIter = buckets.find(Cynara::defaultPolicyBucketId); + ASSERT_NE(buckets.end(), defaultBucketIter); + auto &defaultBucket = defaultBucketIter->second; + ASSERT_THAT(defaultBucket, IsEmpty()); + ASSERT_EQ(Cynara::PredefinedPolicyType::DENY, defaultBucket.defaultPolicy()); + } + virtual ~InMemeoryStorageBackendFixture() {} // TODO: consider defaulting accessor with ON_CALL diff --git a/test/storage/inmemorystoragebackend/inmemorystoragebackend.cpp b/test/storage/inmemorystoragebackend/inmemorystoragebackend.cpp index 6e84475..e81af8f 100644 --- a/test/storage/inmemorystoragebackend/inmemorystoragebackend.cpp +++ b/test/storage/inmemorystoragebackend/inmemorystoragebackend.cpp @@ -23,8 +23,10 @@ #include #include -#include "exceptions/DefaultBucketDeletionException.h" #include "exceptions/BucketNotExistsException.h" +#include "exceptions/BucketDeserializationException.h" +#include "exceptions/DefaultBucketDeletionException.h" +#include "exceptions/FileNotFoundException.h" #include "storage/InMemoryStorageBackend.h" #include "storage/StorageBackend.h" #include "types/PolicyCollection.h" @@ -186,3 +188,67 @@ TEST_F(InMemeoryStorageBackendFixture, deletePolicyFromNonexistentBucket) { EXPECT_THROW(backend.deletePolicy("non-existent", Helpers::generatePolicyKey()), BucketNotExistsException); } + +// Database dir is empty +TEST_F(InMemeoryStorageBackendFixture, load_no_db) { + using ::testing::ReturnRef; + auto testDbPath = std::string(CYNARA_TESTS_DIR) + "/db1/"; + FakeInMemoryStorageBackend backend(testDbPath); + EXPECT_CALL(backend, buckets()).WillRepeatedly(ReturnRef(m_buckets)); + backend.load(); + ASSERT_DB_VIRGIN(m_buckets); +} + +// Database dir contains index with default bucket, but no file for this bucket +TEST_F(InMemeoryStorageBackendFixture, load_no_default) { + using ::testing::ReturnRef; + auto testDbPath = std::string(CYNARA_TESTS_DIR) + "/db2/"; + FakeInMemoryStorageBackend backend(testDbPath); + EXPECT_CALL(backend, buckets()).WillRepeatedly(ReturnRef(m_buckets)); + backend.load(); + ASSERT_DB_VIRGIN(m_buckets); +} + +// Database contains index with default bucket and an empty bucket file +TEST_F(InMemeoryStorageBackendFixture, load_default_only) { + using ::testing::ReturnRef; + auto testDbPath = std::string(CYNARA_TESTS_DIR) + "/db3/"; + FakeInMemoryStorageBackend backend(testDbPath); + EXPECT_CALL(backend, buckets()).WillRepeatedly(ReturnRef(m_buckets)); + backend.load(); + ASSERT_DB_VIRGIN(m_buckets); +} + +// Database contains index with default bucket and an additional bucket +// There are files for both buckets present +TEST_F(InMemeoryStorageBackendFixture, load_2_buckets) { + using ::testing::ReturnRef; + using ::testing::IsEmpty; + + auto testDbPath = std::string(CYNARA_TESTS_DIR) + "/db4/"; + + FakeInMemoryStorageBackend backend(testDbPath); + 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()); + } +} + +// Database contains index with 2 buckets; 1st bucket is valid, but second is corrupted +TEST_F(InMemeoryStorageBackendFixture, second_bucket_corrupted) { + using ::testing::ReturnRef; + auto testDbPath = std::string(CYNARA_TESTS_DIR) + "/db5/"; + FakeInMemoryStorageBackend backend(testDbPath); + EXPECT_CALL(backend, buckets()).WillRepeatedly(ReturnRef(m_buckets)); + backend.load(); + ASSERT_DB_VIRGIN(m_buckets); +} -- 2.7.4