[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-commits
Subject:    [ksecrets] /: Further development of open logic
From:       Valentin Rusu <kde () rusu ! info>
Date:       2015-08-11 8:01:18
Message-ID: E1ZP4Uo-0008VW-1r () scm ! kde ! org
[Download RAW message or body]

Git commit ce0335264cbe7f96c761a9cccff551f26d6087e3 by Valentin Rusu.
Committed on 09/08/2015 at 08:17.
Pushed by vrusu into branch 'master'.

Further development of open logic

OK we reached a point where a circular reference would occur between
ksecrets-crypt and ksecrets_backend, so now stop and prepare to merge
ksecrets-crypt into ksecrets_backend

M  +1    -1    autotests/CMakeLists.txt
M  +4    -2    doc/pam-ksecrets/notes.adoc
M  +2    -1    src/api/ksecrets/ksecretsitem.h
M  +3    -0    src/runtime/ksecrets-crypt/CMakeLists.txt
M  +17   -0    src/runtime/ksecrets-crypt/config.cpp
M  +3    -1    src/runtime/ksecrets-crypt/ksecrets-crypt.c
M  +11   -2    src/runtime/ksecrets_backend/CMakeLists.txt
M  +119  -18   src/runtime/ksecrets_backend/ksecrets_backend.cpp
M  +48   -39   src/runtime/ksecrets_backend/ksecrets_backend.h
M  +20   -0    src/runtime/ksecrets_backend/ksecrets_backend_p.h

http://commits.kde.org/ksecrets/ce0335264cbe7f96c761a9cccff551f26d6087e3

diff --git a/autotests/CMakeLists.txt b/autotests/CMakeLists.txt
index 50a3315..e359f58 100644
--- a/autotests/CMakeLists.txt
+++ b/autotests/CMakeLists.txt
@@ -1,2 +1,2 @@
 add_subdirectory(api)
-
+add_subdirectory(ksecrets_backend)
diff --git a/doc/pam-ksecrets/notes.adoc b/doc/pam-ksecrets/notes.adoc
index d15dd86..4ac6024 100644
--- a/doc/pam-ksecrets/notes.adoc
+++ b/doc/pam-ksecrets/notes.adoc
@@ -67,6 +67,8 @@ This situation of a user session being penetrated is not worst that one on a \
com  
 Finally, the point of ksecrets is to securely store items on disk and avoid unauthorized \
applications from reading/getting them. All security measures in ksecrets should be put in \
place according to this goal.  
-= References =
- http://stackoverflow.com/questions/14548748/encrypting-a-file-from-a-password-using-libgcrypt
+= References = 
 
+  http://stackoverflow.com/questions/14548748/encrypting-a-file-from-a-password-using-libgcrypt
 +
+  https://panthema.net/2008/0714-cryptography-speedtest-comparison/
diff --git a/src/api/ksecrets/ksecretsitem.h b/src/api/ksecrets/ksecretsitem.h
index 47eb6ed..d0e6fc6 100644
--- a/src/api/ksecrets/ksecretsitem.h
+++ b/src/api/ksecrets/ksecretsitem.h
@@ -118,7 +118,8 @@ class KSECRETS_EXPORT SecretItem : public QSharedData {
 
   QSharedDataPointer<SecretItemPrivate> d;
 };
-};
+
+}
 
 Q_DECLARE_METATYPE(KSecrets::SecretItem)
 
diff --git a/src/runtime/ksecrets-crypt/CMakeLists.txt \
b/src/runtime/ksecrets-crypt/CMakeLists.txt index 7d87137..2268625 100644
--- a/src/runtime/ksecrets-crypt/CMakeLists.txt
+++ b/src/runtime/ksecrets-crypt/CMakeLists.txt
@@ -10,6 +10,9 @@ find_package(LibGcrypt 1.6.0 REQUIRED)
 set(KF5_DEP_VERSION "5.12.0")
 find_package(KF5Config ${KF5_DEP_VERSION} REQUIRED)
 
+include_directories(${CMAKE_CURRENT_SOURCE_DIR}/../ksecrets_backend)
+include_directories(${CMAKE_CURRENT_BINARY_DIR}/../ksecrets_backend)
+
 remove_definitions(-DQT_NO_CAST_FROM_ASCII)
 set(ksecrets_crypt_SRC
   ksecrets-crypt.c
diff --git a/src/runtime/ksecrets-crypt/config.cpp b/src/runtime/ksecrets-crypt/config.cpp
index e155038..771254f 100644
--- a/src/runtime/ksecrets-crypt/config.cpp
+++ b/src/runtime/ksecrets-crypt/config.cpp
@@ -19,6 +19,8 @@
     Boston, MA 02110-1301, USA.
 */
 
+#include <ksecrets_backend.h>
+
 #include <ksharedconfig.h>
 #include <kconfiggroup.h>
 #include <QtCore/QDir>
@@ -31,6 +33,10 @@ KSharedConfigPtr sharedConfig;
 #define CONFIGNAME "ksecretsrc"
 
 extern "C" {
+/*
+ * @note even if you could use QDir::home() to retrieve user's home directory, this function
+ * may be called in contexts where current user information is not yet available
+ */
 const char* prepare_secret_file_location(const char* home_dir)
 {
     qCDebug(logCat) << "prepare_secret_file_location(" << home_dir << ")";
@@ -70,5 +76,16 @@ const char* get_keyname_mac()
         .toUtf8()
         .constData();
 }
+
+bool get_salt(const char* path, char* buf, size_t len)
+{
+    bool res= false;
+    KSecretsBackend backend;
+    auto openfut = backend.open(path);
+    if (openfut.get().status_ == KSecretsBackend::OpenStatus::Good) {
+
+    }
+    return res;
+}
 }
 
diff --git a/src/runtime/ksecrets-crypt/ksecrets-crypt.c \
b/src/runtime/ksecrets-crypt/ksecrets-crypt.c index 0da2007..b0af79c 100644
--- a/src/runtime/ksecrets-crypt/ksecrets-crypt.c
+++ b/src/runtime/ksecrets-crypt/ksecrets-crypt.c
@@ -47,6 +47,7 @@
 extern const char* prepare_secret_file_location(const char*);
 extern const char* get_keyname_encrypting();
 extern const char* get_keyname_mac();
+extern bool get_salt(const char*, char*, size_t);
 
 bool kss_init_gcry()
 {
@@ -268,4 +269,5 @@ bool kss_change_password(const char* new_password)
     syslog(LOG_INFO, "kss_change_password");
     return true;
 }
-/* vim:tw=220:ts=4 */
+/* vim: tw=220 ts=4
+*/
diff --git a/src/runtime/ksecrets_backend/CMakeLists.txt \
b/src/runtime/ksecrets_backend/CMakeLists.txt index 9c9c806..a852068 100644
--- a/src/runtime/ksecrets_backend/CMakeLists.txt
+++ b/src/runtime/ksecrets_backend/CMakeLists.txt
@@ -1,12 +1,21 @@
 
-project(ksecrets_backend)
+set(CMAKE_MODULE_PATH "${CMAKE_SOURCE_DIR}/cmake" )
+find_package(LibGcrypt 1.6.0 REQUIRED)
+
 cmake_minimum_required(VERSION 3.2.2)
 
+ecm_setup_version(${KF5_VERSION} VARIABLE_PREFIX KSECRETS_BACKEND
+                  VERSION_HEADER "${CMAKE_CURRENT_BINARY_DIR}/ksecrets_backend_version.h"
+                  PACKAGE_VERSION_FILE \
"${CMAKE_CURRENT_BINARY_DIR}/KF5SecretsBackendConfigVersion.cmake") +
 set(ksecrets_backend_SRC
    ksecrets_backend.cpp)
 
 add_library(ksecrets_backend SHARED ${ksecrets_backend_SRC})
-target_link_libraries(ksecrets_backend pthread)
+generate_export_header(ksecrets_backend BASE_NAME ksecrets_backend)
+target_link_libraries(ksecrets_backend
+   pthread
+   ${LIBGCRYPT_LIBRARIES})
 target_compile_features(ksecrets_backend PRIVATE cxx_range_for)
 set_target_properties(ksecrets_backend PROPERTIES PREFIX "")
 
diff --git a/src/runtime/ksecrets_backend/ksecrets_backend.cpp \
b/src/runtime/ksecrets_backend/ksecrets_backend.cpp index 876614e..3395f64 100644
--- a/src/runtime/ksecrets_backend/ksecrets_backend.cpp
+++ b/src/runtime/ksecrets_backend/ksecrets_backend.cpp
@@ -25,10 +25,15 @@
 #include <future>
 #include <thread>
 #include <sys/stat.h>
+#include <unistd.h>
+#include <stdio.h>
+#define GCRPYT_NO_DEPRECATED
+#include <gcrypt.h>
 
 KSecretsBackendPrivate::KSecretsBackendPrivate(KSecretsBackend* b)
     : b_(b)
 {
+    openStatus_.status_ = KSecretsBackend::OpenStatus::NotYetOpened;
 }
 
 KSecretsBackend::KSecretsBackend()
@@ -38,46 +43,142 @@ KSecretsBackend::KSecretsBackend()
 
 KSecretsBackend::~KSecretsBackend() = default;
 
-std::future<KSecretsBackend::OpenResult> KSecretsBackend::open(
-    std::string&& path, bool readonly /* =true */) noexcept
+std::future<KSecretsBackend::OpenResult> KSecretsBackend::open(std::string&& path, bool \
readonly /* =true */) noexcept  {
     // sanity checks
     if (path.empty()) {
-        return std::async(std::launch::deferred, []() {
-            return OpenResult{ OpenResult::OpenStatus::NoPathGiven, 0 };
-        });
+        return std::async(std::launch::deferred, []() { return OpenResult{ \
OpenStatus::NoPathGiven, 0 }; });  }
 
+    bool shouldCreateFile = false;
     struct stat buf;
     if (stat(path.c_str(), &buf) != 0) {
         auto err = errno;
-        return std::async(std::launch::deferred, [err]() {
-            return OpenResult{ OpenResult::OpenStatus::SystemError, errno };
-        });
+        if (ENOENT == err) {
+            shouldCreateFile = true;
+        }
+        else {
+            return std::async(std::launch::deferred, [err]() { return OpenResult{ \
OpenStatus::SystemError, errno }; }); +        }
+    }
+    else {
+        if (buf.st_size == 0) {
+            unlink(path.c_str());
+            shouldCreateFile = true; // recreate if empty file was found
+        }
     }
 
     // now we can proceed
     auto localThis = this;
     if (!readonly) {
-        return std::async(std::launch::async,
-            [localThis, path]() { return localThis->d->lock_open(path); });
+        return std::async(
+            std::launch::async, [localThis, path, shouldCreateFile]() { return \
localThis->d->createFileIfNeededThenDo(path, shouldCreateFile, \
std::bind(&KSecretsBackendPrivate::lock_open, localThis->d.get(), path)); });  }
     else {
-        return std::async(std::launch::deferred,
-            [localThis, path]() { return localThis->d->open(path); });
+        return std::async(
+            std::launch::deferred, [localThis, path, shouldCreateFile]() { return \
localThis->d->createFileIfNeededThenDo(path, shouldCreateFile, \
std::bind(&KSecretsBackendPrivate::open, localThis->d.get(), path)); }); +    }
+}
+
+KSecretsBackend::OpenResult KSecretsBackendPrivate::createFileIfNeededThenDo(const \
std::string& path, bool shouldCreateFile, open_action action) +{
+    if (shouldCreateFile) {
+        auto createRes = createFile(path);
+        if (createRes != 0) {
+            return setOpenStatus({ KSecretsBackend::OpenStatus::SystemError, createRes });
+        }
+    }
+    return action(path);
+}
+
+char fileMagic[] = { 'k', 's', 'e', 'c', 'r', 'e', 't', 's' };
+constexpr auto fileMagicLen = sizeof(fileMagic) / sizeof(fileMagic[0]);
+
+int KSecretsBackendPrivate::createFile(const std::string& path)
+{
+    FILE* f = fopen(path.c_str(), "w");
+    if (f == nullptr) {
+        return errno;
+    }
+
+    FileHeadStruct emptyFileData;
+    memcpy(emptyFileData.magic, fileMagic, fileMagicLen);
+    gcry_randomize(emptyFileData.salt, KSecretsBackend::SALT_SIZE, GCRY_STRONG_RANDOM);
+    gcry_randomize(emptyFileData.iv, IV_SIZE, GCRY_STRONG_RANDOM);
+
+    int res = 0;
+    if (fwrite(&emptyFileData, sizeof(emptyFileData), 1, f) != sizeof(emptyFileData)) {
+        res = -1; // FIXME is errno available here?
+    }
+    fclose(f);
+    return res;
+}
+
+const char* KSecretsBackend::salt() const { return d->salt(); }
+
+const char* KSecretsBackendPrivate::salt() const { return fileHead_.salt; }
+
+KSecretsBackend::OpenResult KSecretsBackendPrivate::lock_open(const std::string& path)
+{
+    file_ = fopen(path.c_str(), "w+");
+    if (file_ == nullptr) {
+        return { KSecretsBackend::OpenStatus::SystemError, errno };
+    }
+    // TODO perform the lock here
+    return open(path);
+}
+
+KSecretsBackend::OpenResult KSecretsBackendPrivate::setOpenStatus(KSecretsBackend::OpenResult \
openStatus) +{
+    openStatus_ = openStatus;
+    return openStatus;
+}
+
+KSecretsBackend::OpenResult KSecretsBackendPrivate::open(const std::string& path)
+{
+    if (file_ == nullptr) {
+        file_ = fopen(path.c_str(), "r");
     }
+    if (file_ == nullptr) {
+        return setOpenStatus({ KSecretsBackend::OpenStatus::SystemError, errno });
+    }
+    if (fread(&fileHead_, sizeof(fileHead_), 1, file_) != sizeof(fileHead_)) {
+        return setOpenStatus({ KSecretsBackend::OpenStatus::SystemError, ferror(file_) });
+    }
+    if (memcmp(fileHead_.magic, fileMagic, fileMagicLen) != 0) {
+        return setOpenStatus({ KSecretsBackend::OpenStatus::InvalidFile, 0 });
+    }
+    // decrypting will occur upon collection request
+    return setOpenStatus({ KSecretsBackend::OpenStatus::Good, 0 });
+}
+
+KSecretsBackend::CollectionNames KSecretsBackend::dirCollections() const noexcept
+{
+    // TODO
+    return CollectionNames();
+}
+
+KSecretsBackend::CollectionPtr KSecretsBackend::createCollection(std::string&&) noexcept
+{
+    // TODO
+    return CollectionPtr();
+}
+
+KSecretsBackend::CollectionPtr KSecretsBackend::readCollection(std::string&&) const noexcept
+{
+    // TODO
+    return CollectionPtr();
 }
 
-KSecretsBackend::OpenResult KSecretsBackendPrivate::lock_open(
-    const std::string& path)
+bool KSecretsBackend::deleteCollection(CollectionPtr) noexcept
 {
     // TODO
-    return { KSecretsBackend::OpenResult::OpenStatus::Good, 0};
+    return false;
 }
 
-KSecretsBackend::OpenResult KSecretsBackendPrivate::open(
-    const std::string& path)
+bool KSecretsBackend::deleteCollection(std::string&&) noexcept
 {
     // TODO
-    return { KSecretsBackend::OpenResult::OpenStatus::Good, 0};
+    return false;
 }
+// vim: tw=220:ts=4
diff --git a/src/runtime/ksecrets_backend/ksecrets_backend.h \
b/src/runtime/ksecrets_backend/ksecrets_backend.h index fa6b9ef..59ee479 100644
--- a/src/runtime/ksecrets_backend/ksecrets_backend.h
+++ b/src/runtime/ksecrets_backend/ksecrets_backend.h
@@ -21,10 +21,13 @@
 #ifndef KSECRETS_BACKEND_H
 #define KSECRETS_BACKEND_H
 
+#include <ksecrets_backend_export.h>
+
 #include <memory>
 #include <ctime>
 #include <map>
 #include <vector>
+#include <array>
 #include <future>
 
 class KSecretsBackendPrivate;
@@ -34,39 +37,29 @@ class KSecretsBackendPrivate;
  *
  * This class would store the secrets into an underlying custom formated file.
  *
- * Each API call is stateless. That is, the secrets file will always be left
- *in a consistent
- * state between calls. So, even if your application crashes, the file won't
- *get corrupted.
- * FIXME is that OK? Further tests should be confirm if a background sync
- *should be introduced
- *       in order to get operations faster. However, today computing power do
- *not justify
- *       Ã  priori optimizations, so this first version would modify the file
- *with each API call.
- *       That guarantee client applications that edits are always synced to
- *disk/storage.
+ * Each API call is stateless. That is, the secrets file will always be left in a consistent
+ * state between calls. So, even if your application crashes, the file won't get corrupted.
+ *   FIXME is that OK? Further tests should be confirm if a background sync
+ *   should be introduced in order to get operations faster. However, today computing power do
+ *   not justify à priori optimizations, so this first version would modify the file
+ *   with each API call.
+ * That guarantees client applications that edits are always synced to disk/storage.
  *
- * The API calls are organized in classes, following the structure of data in
- *the backend.
- * Applications will first work with a Collection, the search or insert Items
- *into it.
- * The Item class holds, sure enough, the secret value but also let
- *applications associate
- * the secret value with metadata, such as the label or other custom
- *properties.
+ * The API calls are organized in classes, following the structure of data in the backend.
+ * Applications will first work with a Collection, the search or insert Items into it.
+ * The Item class holds, sure enough, the secret value but also let applications associate
+ * the secret value with metadata, such as the label or other custom properties.
  *
  * Before using a collection, the application should open it.
  * Upon opening, it's possible to indicate if readonly mode is possible.
  *
- * When opening without readonly flag, then the file is exclusively locked.
- *The lock is
- * released when the class is destroyed. You should keep the file locked as
- *shortly as
- * possible, in order to avoid deadlocks between applications that also need
- *to read the
+ * When opening without readonly flag, then the file is exclusively locked. The lock is
+ * released when the class is destroyed. You should keep the file locked as shortly as
+ * possible, in order to avoid deadlocks between applications that also need to read the
  * secrets. For more information @see open().
  *
+ * The data are encrypted using libgcypt and the algorythm Twofish which is the fasted for \
this library. + *
  * TODO give here a code example once the API stabilizes
  *
  * @note All const members in this interface are thread-safe.
@@ -76,7 +69,7 @@ class KSecretsBackendPrivate;
  *       By providing a class, one could use local variables and the class
  *       would be destroyed, releasing the file, upon block exit.
  */
-class KSecretsBackend {
+class KSECRETS_BACKEND_EXPORT KSecretsBackend {
     class ItemPrivate;
     class CollectionPrivate;
 
@@ -187,8 +180,7 @@ public:
          * possible. So please check if via it's operator bool() before using
          * it.
          */
-        ItemPtr createItem(
-            std::string&& label, AttributesMap&&, ItemValue&&) noexcept;
+        ItemPtr createItem(std::string&& label, AttributesMap&&, ItemValue&&) noexcept;
         /*
          * Convenience method for creating items without supplemental
          * attributes.
@@ -221,21 +213,37 @@ public:
     KSecretsBackend(const KSecretsBackend&) = delete;
     virtual ~KSecretsBackend();
 
+    enum class OpenStatus {
+        NotYetOpened,
+        Good,
+        NoPathGiven,
+        InvalidFile, // the file format was not recognized. Is this a ksecrets file?
+        FileLocked,
+        SystemError
+    };
     struct OpenResult {
-        enum class OpenStatus {
-            Good,
-            NoPathGiven,
-            FileLocked,
-            SystemError // @see
-        };
-
         OpenStatus status_;
         int errno_;
     };
 
-    std::future<OpenResult> open(
-        std::string&&, bool readOnly = true) noexcept;
-    std::vector<std::string> dirCollections() noexcept;
+    /**
+     * Open the designated file. Creates file if not found, regardless
+     * of the readOnly parameter value.
+     *
+     * @parameter path full path to the backend file to handle
+     * @parameter readOnly set it to true if you only intend reading to speed
+     * things-up
+     */
+    std::future<OpenResult> open(std::string&& path, bool readOnly = true) noexcept;
+
+    constexpr static auto SALT_SIZE = 56;
+    /**
+     * @return pointer to the salt structure inside the internal structure of this object. The \
buffer has SALT_SIZE length. +     */
+    const char* salt() const;
+
+    using CollectionNames = std::vector<std::string>;
+    CollectionNames dirCollections() const noexcept;
     /*
      * @return CollectionPtr which can empty if the call did not succeed.
      *Please check that with operator bool().
@@ -258,3 +266,4 @@ private:
 };
 
 #endif
+// vim: tw=220:ts=4
diff --git a/src/runtime/ksecrets_backend/ksecrets_backend_p.h \
b/src/runtime/ksecrets_backend/ksecrets_backend_p.h index 156a9f1..a1c65eb 100644
--- a/src/runtime/ksecrets_backend/ksecrets_backend_p.h
+++ b/src/runtime/ksecrets_backend/ksecrets_backend_p.h
@@ -30,8 +30,28 @@ public:
 
     KSecretsBackend::OpenResult lock_open(const std::string&);
     KSecretsBackend::OpenResult open(const std::string&);
+    using open_action
+        = std::function<KSecretsBackend::OpenResult(const std::string&)>;
+    KSecretsBackend::OpenResult createFileIfNeededThenDo(
+        const std::string&, bool, open_action);
+    int createFile(const std::string&);
+    const char* salt() const;
+
+    constexpr static auto IV_SIZE = 32;
+    struct FileHeadStruct {
+        char magic[9];
+        char salt[KSecretsBackend::SALT_SIZE];
+        char iv[IV_SIZE];
+    };
+
+    KSecretsBackend::OpenResult setOpenStatus(KSecretsBackend::OpenResult);
+    bool isOpen() const noexcept { return KSecretsBackend::OpenStatus::Good == \
openStatus_.status_; }  
     KSecretsBackend* b_;
+    FILE* file_;
+    FileHeadStruct fileHead_;
+    KSecretsBackend::OpenResult openStatus_;
 };
 
 #endif
+// vim: tw=220:ts=4


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic