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 <a.zdyb@partner.samsung.com>
Signed-off-by: Pawel Wieczorek <p.wieczorek2@samsung.com>
Change-Id: Ia943f77a2a0c85f394c40dd10333a73df4d0c96a

diff --git a/packaging/cynara.spec b/packaging/cynara.spec
index 677142b..f4e8d1a 100644 (file)
--- 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 (file)
--- 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 <string>
 
+#include <exceptions/DatabaseException.h>
+
 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 (file)
--- a/src/service/storage/InMemoryStorageBackend.cpp
+++ b/src/service/storage/InMemoryStorageBackend.cpp
@@ -35,6 +35,7 @@
 #include <log/log.h>
 #include <exceptions/BucketNotExistsException.h>
 #include <exceptions/CannotCreateFileException.h>
+#include <exceptions/DatabaseException.h>
 #include <exceptions/FileNotFoundException.h>
 #include <exceptions/UnexpectedErrorException.h>
 #include <types/PolicyBucket.h>
@@ -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 (file)
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 (file)
index 0000000..e69de29

diff --git a/test/db/db3/buckets b/test/db/db3/buckets
new file mode 100644 (file)
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 (file)
index 0000000..e69de29

diff --git a/test/db/db4/_additional b/test/db/db4/_additional
new file mode 100644 (file)
index 0000000..e69de29

diff --git a/test/db/db4/buckets b/test/db/db4/buckets
new file mode 100644 (file)
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 (file)
index 0000000..e69de29

diff --git a/test/db/db5/_additional b/test/db/db5/_additional
new file mode 100644 (file)
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 (file)
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 (file)
--- a/test/storage/inmemorystoragebackend/inmemeorystoragebackendfixture.h
+++ b/test/storage/inmemorystoragebackend/inmemeorystoragebackendfixture.h
@@ -24,6 +24,7 @@
 #define INMEMEORYSTORAGEBACKENDFIXTURE_H_
 
 #include <gmock/gmock.h>
+#include <gtest/gtest.h>
 
 #include <types/PolicyBucket.h>
 #include <types/PolicyCollection.h>
@@ -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 (file)
--- a/test/storage/inmemorystoragebackend/inmemorystoragebackend.cpp
+++ b/test/storage/inmemorystoragebackend/inmemorystoragebackend.cpp
@@ -23,8 +23,10 @@
 #include <gmock/gmock.h>
 #include <gtest/gtest.h>
 
-#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<std::string> 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);
+}