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

List:       kde-commits
Subject:    [ksecrets] src/runtime/ksecrets_store: Refactoring file related code in its class
From:       Valentin Rusu <kde () rusu ! info>
Date:       2015-08-15 14:17:46
Message-ID: E1ZQcHK-0008TQ-Sk () scm ! kde ! org
[Download RAW message or body]

Git commit 5b330d3190e1cad11949883618ec4b3007bcd0f5 by Valentin Rusu.
Committed on 14/08/2015 at 10:52.
Pushed by vrusu into branch 'master'.

Refactoring file related code in its class

M  +1    -0    src/runtime/ksecrets_store/CMakeLists.txt
A  +107  -0    src/runtime/ksecrets_store/ksecrets_file.cpp     [License: LGPL (v2+)]
A  +56   -0    src/runtime/ksecrets_store/ksecrets_file.h     [License: LGPL (v2+)]
M  +8    -43   src/runtime/ksecrets_store/ksecrets_store.cpp
M  +2    -20   src/runtime/ksecrets_store/ksecrets_store_p.h

http://commits.kde.org/ksecrets/5b330d3190e1cad11949883618ec4b3007bcd0f5

diff --git a/src/runtime/ksecrets_store/CMakeLists.txt \
b/src/runtime/ksecrets_store/CMakeLists.txt index 688f065..5805ce7 100644
--- a/src/runtime/ksecrets_store/CMakeLists.txt
+++ b/src/runtime/ksecrets_store/CMakeLists.txt
@@ -9,6 +9,7 @@ ecm_setup_version(${KF5_VERSION} VARIABLE_PREFIX KSECRETS_BACKEND
                   PACKAGE_VERSION_FILE \
"${CMAKE_CURRENT_BINARY_DIR}/KF5SecretsStoreConfigVersion.cmake")  
 set(ksecrets_store_SRC
+    ksecrets_file.cpp
     ksecrets_crypt.cpp
     ksecrets_credentials.cpp
     ksecrets_store.cpp)
diff --git a/src/runtime/ksecrets_store/ksecrets_file.cpp \
b/src/runtime/ksecrets_store/ksecrets_file.cpp new file mode 100644
index 0000000..dd173a6
--- /dev/null
+++ b/src/runtime/ksecrets_store/ksecrets_file.cpp
@@ -0,0 +1,107 @@
+/*
+    This file is part of the KDE Libraries
+
+    Copyright (C) 2015 Valentin Rusu (valir@kde.org)
+
+    This library is free software; you can redistribute it and/or
+    modify it under the terms of the GNU Library General Public
+    License as published by the Free Software Foundation; either
+    version 2 of the License, or (at your option) any later version.
+
+    This library is distributed in the hope that it will be useful,
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+    Library General Public License for more details.
+
+    You should have received a copy of the GNU Library General Public License
+    along with this library; see the file COPYING.LIB. If not, write to
+    the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
+    Boston, MA 02110-1301, USA.
+*/
+
+#include "ksecrets_file.h"
+#include "defines.h"
+
+#include <unistd.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <sys/file.h>
+#include <string.h>
+#define GCRPYT_NO_DEPRECATED
+#include <gcrypt.h>
+
+char fileMagic[] = { 'k', 's', 'e', 'c', 'r', 'e', 't', 's' };
+constexpr auto fileMagicLen = sizeof(fileMagic) / sizeof(fileMagic[0]);
+
+KSecretsFile::KSecretsFile()
+    : file_(-1)
+    , locked_(false)
+{
+    memset(&fileHead_, 0, sizeof(fileHead_));
+}
+
+KSecretsFile::~KSecretsFile()
+{
+    if (file_ != -1) {
+        auto r = close(file_);
+        if (r == -1) {
+            syslog(KSS_LOG_ERR, "ksecrets: system return erro upon secrets "
+                                "file close: %d (%m)",
+                errno);
+            syslog(KSS_LOG_ERR, "ksecrets: the secrets file might now be "
+                                "corrup because of the previous error");
+        }
+        file_ = -1;
+    }
+}
+
+int KSecretsFile::create(const std::string& path)
+{
+    FILE* f = fopen(path.c_str(), "w");
+    if (f == nullptr) {
+        return errno;
+    }
+
+    FileHeadStruct emptyFileData;
+    memcpy(emptyFileData.magic, fileMagic, fileMagicLen);
+
+    // FIXME should we put this kind of call in gcrypt-dedicated file?
+    gcry_randomize(emptyFileData.salt, SALT_SIZE, GCRY_STRONG_RANDOM);
+    gcry_randomize(emptyFileData.iv, IV_SIZE, GCRY_STRONG_RANDOM);
+
+    int res = 0;
+    if (fwrite(&emptyFileData, 1, sizeof(emptyFileData), f) != sizeof(emptyFileData)) {
+        res = ferror(f);
+    }
+    fclose(f);
+    return res;
+}
+
+void KSecretsFile::setup(const std::string& path, bool readOnly)
+{
+    filePath_ = path;
+    readOnly_ = readOnly;
+}
+
+bool KSecretsFile::open()
+{
+    file_ = ::open(filePath_.c_str(), O_DSYNC | O_NOATIME | O_NOFOLLOW);
+    return file_ != -1;
+}
+
+bool KSecretsFile::lock()
+{
+    return flock(file_, LOCK_EX) != -1;
+    locked_ = true;
+}
+
+bool KSecretsFile::readHeader() { return read(file_, &fileHead_, sizeof(fileHead_)) == \
sizeof(fileHead_); } +
+bool KSecretsFile::checkMagic()
+{
+    if (memcmp(fileHead_.magic, fileMagic, fileMagicLen) != 0) {
+        return false;
+    }
+    return true;
+}
+// vim: tw=220:ts=4
diff --git a/src/runtime/ksecrets_store/ksecrets_file.h \
b/src/runtime/ksecrets_store/ksecrets_file.h new file mode 100644
index 0000000..1a2212d
--- /dev/null
+++ b/src/runtime/ksecrets_store/ksecrets_file.h
@@ -0,0 +1,56 @@
+/*
+    This file is part of the KDE Libraries
+
+    Copyright (C) 2015 Valentin Rusu (valir@kde.org)
+
+    This library is free software; you can redistribute it and/or
+    modify it under the terms of the GNU Library General Public
+    License as published by the Free Software Foundation; either
+    version 2 of the License, or (at your option) any later version.
+
+    This library is distributed in the hope that it will be useful,
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+    Library General Public License for more details.
+
+    You should have received a copy of the GNU Library General Public License
+    along with this library; see the file COPYING.LIB. If not, write to
+    the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
+    Boston, MA 02110-1301, USA.
+*/
+#ifndef KSECRETS_FILE_H
+#define KSECRETS_FILE_H
+
+#include <memory>
+
+class KSecretsFile {
+public:
+    KSecretsFile();
+    ~KSecretsFile();
+
+    constexpr static auto IV_SIZE = 32;
+    constexpr static auto SALT_SIZE = 56;
+    struct FileHeadStruct {
+        char magic[9];
+        char salt[SALT_SIZE];
+        char iv[IV_SIZE];
+    };
+
+    int create(const std::string &path);
+    void setup(const std::string &path, bool readOnly) noexcept;
+    bool open() noexcept;
+    bool lock() noexcept;
+    bool readHeader() noexcept;
+    bool checkMagic() noexcept;
+    const char *salt() const noexcept { return fileHead_.salt; }
+
+private:
+    std::string filePath_;
+    int file_;
+    bool locked_;
+    bool readOnly_;
+    FileHeadStruct fileHead_;
+};
+
+#endif
+// vim: tw=220:ts=4
diff --git a/src/runtime/ksecrets_store/ksecrets_store.cpp \
b/src/runtime/ksecrets_store/ksecrets_store.cpp index 4f8dde9..9fcd5b1 100644
--- a/src/runtime/ksecrets_store/ksecrets_store.cpp
+++ b/src/runtime/ksecrets_store/ksecrets_store.cpp
@@ -26,7 +26,6 @@
 #include <thread>
 #include <sys/types.h>
 #include <sys/stat.h>
-#include <sys/file.h>
 #include <unistd.h>
 #include <stdio.h>
 #include <syslog.h>
@@ -50,7 +49,6 @@ KSecretsStorePrivate::KSecretsStorePrivate(KSecretsStore* b)
     : b_(b)
 {
     status_ = KSecretsStore::StoreStatus::JustCreated;
-    memset(&fileHead_, 0, sizeof(fileHead_));
 }
 
 KSecretsStore::KSecretsStore()
@@ -102,8 +100,7 @@ KSecretsStore::SetupResult KSecretsStorePrivate::setup(const std::string& \
                path,
             return setStoreStatus(KSecretsStore::SetupResult({ \
KSecretsStore::StoreStatus::SystemError, createres }));  }
     }
-    secretsFile_.filePath_ = path;
-    secretsFile_.readOnly_ = readOnly;
+    secretsFile_.setup(path, readOnly);
     return open(!readOnly);
 }
 
@@ -136,69 +133,37 @@ KSecretsStore::CredentialsResult \
                KSecretsStorePrivate::setCredentials(const std:
     return setStoreStatus(Result({ KSecretsStore::StoreStatus::CredentialsSet, 0 }));
 }
 
-char fileMagic[] = { 'k', 's', 'e', 'c', 'r', 'e', 't', 's' };
-constexpr auto fileMagicLen = sizeof(fileMagic) / sizeof(fileMagic[0]);
-
 int KSecretsStorePrivate::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, SALT_SIZE, GCRY_STRONG_RANDOM);
-    gcry_randomize(emptyFileData.iv, IV_SIZE, GCRY_STRONG_RANDOM);
-
-    int res = 0;
-    if (fwrite(&emptyFileData, 1, sizeof(emptyFileData), f) != sizeof(emptyFileData)) {
-        res = ferror(f);
-    }
-    fclose(f);
-    return res;
+    return secretsFile_.create(path);
 }
 
 bool KSecretsStore::isGood() const noexcept { return d->status_ == StoreStatus::Good; }
 
-const char* KSecretsStorePrivate::salt() const { return fileHead_.salt; }
+const char* KSecretsStorePrivate::salt() const { return secretsFile_.salt(); }
 
 KSecretsStore::SetupResult KSecretsStorePrivate::open(bool lockFile)
 {
     using OpenResult = KSecretsStore::SetupResult;
-    secretsFile_.file_ = ::open(secretsFile_.filePath_.c_str(), O_DSYNC | O_NOATIME | \
                O_NOFOLLOW);
-    if (secretsFile_.file_ == -1) {
+    if (!secretsFile_.open()) {
         return setStoreStatus(OpenResult({ KSecretsStore::StoreStatus::CannotOpenFile, errno \
}));  }
     if (lockFile) {
-        if (flock(secretsFile_.file_, LOCK_EX) == -1) {
+        if (!secretsFile_.lock()) {
             return setStoreStatus(OpenResult({ KSecretsStore::StoreStatus::CannotLockFile, \
errno }));  }
-        secretsFile_.locked_ = true;
     }
-    auto r = read(secretsFile_.file_, &fileHead_, sizeof(fileHead_));
-    if (r == -1) {
+    if (!secretsFile_.readHeader()) {
         return setStoreStatus(OpenResult({ KSecretsStore::StoreStatus::CannotReadFile, errno \
}));  }
-    if ((size_t)r < sizeof(fileHead_) || memcmp(fileHead_.magic, fileMagic, fileMagicLen) != \
0) { +    if (!secretsFile_.checkMagic()) {
         return setStoreStatus(OpenResult({ KSecretsStore::StoreStatus::InvalidFile, -1 }));
     }
+    // TODO add here MAC integrity check
     // decrypting will occur upon collection request
     return setStoreStatus(OpenResult({ KSecretsStore::StoreStatus::Good, 0 }));
 }
 
-KSecretsStorePrivate::SecretsFile::~SecretsFile()
-{
-    if (file_ != -1) {
-        auto r = close(file_);
-        if (r == -1) {
-            syslog(KSS_LOG_ERR, "ksecrets: system return erro upon secrets file close: %d \
                (%m)", errno);
-            syslog(KSS_LOG_ERR, "ksecrets: the secrets file might now be corrup because of the \
                previous error");
-        }
-        file_ = -1;
-    }
-}
-
 KSecretsStore::DirCollectionsResult KSecretsStore::dirCollections() const noexcept
 {
     // TODO
diff --git a/src/runtime/ksecrets_store/ksecrets_store_p.h \
b/src/runtime/ksecrets_store/ksecrets_store_p.h index 8a8fa1a..a7b10fd 100644
--- a/src/runtime/ksecrets_store/ksecrets_store_p.h
+++ b/src/runtime/ksecrets_store/ksecrets_store_p.h
@@ -22,6 +22,7 @@
 #define KSECRETSBACKEND_P_H
 
 #include "ksecrets_store.h"
+#include "ksecrets_file.h"
 
 class TimeStamped {
 
@@ -64,14 +65,6 @@ public:
     int createFile(const std::string&);
     const char* salt() const;
 
-    constexpr static auto IV_SIZE = 32;
-    constexpr static auto SALT_SIZE = 56;
-    struct FileHeadStruct {
-        char magic[9];
-        char salt[SALT_SIZE];
-        char iv[IV_SIZE];
-    };
-
     template <typename S> S setStoreStatus(S s)
     {
         status_ = s.status_;
@@ -79,19 +72,8 @@ public:
     }
     bool isOpen() const noexcept { return KSecretsStore::StoreStatus::Good == status_; }
 
-    struct SecretsFile {
-        SecretsFile()
-            : file_(-1)
-            , locked_(false){};
-        ~SecretsFile();
-        std::string filePath_;
-        int file_;
-        bool locked_;
-        bool readOnly_;
-    };
     KSecretsStore* b_;
-    SecretsFile secretsFile_;
-    FileHeadStruct fileHead_;
+    KSecretsFile secretsFile_;
     KSecretsStore::StoreStatus status_;
 };
 


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

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