[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-commits
Subject: [ksecrets] /: Backend setup now works
From: Valentin Rusu <kde () rusu ! info>
Date: 2015-08-13 8:41:35
Message-ID: E1ZPo4t-0002gq-QH () scm ! kde ! org
[Download RAW message or body]
Git commit fff18872cc08cb8bb9f8e1a0db61c70ca4b1d29b by Valentin Rusu.
Committed on 13/08/2015 at 08:34.
Pushed by vrusu into branch 'master'.
Backend setup now works
M +1 -3 autotests/api/CMakeLists.txt
M +4 -1 autotests/ksecrets_store/CMakeLists.txt
M +18 -7 autotests/ksecrets_store/ksecrets_store_test.cpp
M +6 -2 src/runtime/ksecrets_store/ksecrets_credentials.cpp
M +43 -39 src/runtime/ksecrets_store/ksecrets_store.cpp
M +31 -22 src/runtime/ksecrets_store/ksecrets_store.h
M +6 -4 src/runtime/ksecrets_store/ksecrets_store_p.h
http://commits.kde.org/ksecrets/fff18872cc08cb8bb9f8e1a0db61c70ca4b1d29b
diff --git a/autotests/api/CMakeLists.txt b/autotests/api/CMakeLists.txt
index 6dc2343..9cc791c 100644
--- a/autotests/api/CMakeLists.txt
+++ b/autotests/api/CMakeLists.txt
@@ -4,8 +4,6 @@ include(ECMAddTests)
find_package(Qt5 ${REQUIRED_QT_VERSION} CONFIG REQUIRED Test)
-find_package(LibGcrypt 1.6.0 REQUIRED)
-
include_directories(
${CMAKE_SOURCE_DIR}/src/runtime/ksecrets_store
${CMAKE_BINARY_DIR}/src/runtime/ksecrets_store
@@ -20,6 +18,6 @@ find_package(KF5 REQUIRED CoreAddons)
ecm_add_test(
ksecretsservice-test.cpp
- LINK_LIBRARIES KF5::Secrets Qt5::Test KF5::Secrets KF5::CoreAddons ksecrets_store \
${LIBGCRYPT_LIBRARIES} keyutils + LINK_LIBRARIES Qt5::Test KF5::Secrets KF5::CoreAddons \
ksecrets_store keyutils )
diff --git a/autotests/ksecrets_store/CMakeLists.txt b/autotests/ksecrets_store/CMakeLists.txt
index facb282..b0f916b 100644
--- a/autotests/ksecrets_store/CMakeLists.txt
+++ b/autotests/ksecrets_store/CMakeLists.txt
@@ -11,8 +11,11 @@ if(NOT Qt5Test_FOUND)
return()
endif()
+find_package(KF5CoreAddons ${KF5_DEP_VERSION} REQUIRED)
+find_package(KF5Config ${KF5_DEP_VERSION} REQUIRED)
+
ecm_add_test(
ksecrets_store_test.cpp
- LINK_LIBRARIES Qt5::Test ksecrets_store
+ LINK_LIBRARIES Qt5::Test KF5::CoreAddons KF5::ConfigCore ksecrets_store
)
diff --git a/autotests/ksecrets_store/ksecrets_store_test.cpp \
b/autotests/ksecrets_store/ksecrets_store_test.cpp index 353f10a..9465e63 100644
--- a/autotests/ksecrets_store/ksecrets_store_test.cpp
+++ b/autotests/ksecrets_store/ksecrets_store_test.cpp
@@ -24,10 +24,13 @@
#include <ksecrets_store.h>
#include <QtTest/QtTest>
#include <QtCore/QDir>
+#include <ksharedconfig.h>
+#include <kconfiggroup.h>
QTEST_GUILESS_MAIN(KSecretServiceStoreTest);
-const char* test_file_path = "~/.qttest/ksecretsbackend-test.dat";
+KSharedConfig::Ptr sharedConfig;
+QString secretsFilePath;
KSecretServiceStoreTest::KSecretServiceStoreTest(QObject* parent)
: QObject(parent)
@@ -36,21 +39,29 @@ KSecretServiceStoreTest::KSecretServiceStoreTest(QObject* parent)
void KSecretServiceStoreTest::initTestCase()
{
+ QStandardPaths::setTestModeEnabled(true);
+ sharedConfig = KSharedConfig::openConfig(QLatin1String("ksecretsrc"));
+
+ secretsFilePath = QStandardPaths::writableLocation(QStandardPaths::DataLocation);
+ QVERIFY(QDir::home().mkpath(secretsFilePath));
+ secretsFilePath += QLatin1Literal("/ksecrets-test.data");
+ qDebug() << "secrets store path: " << secretsFilePath;
// create a test file here and performe the real open test below
KSecretsStore backend;
- auto setupfut = backend.setup(test_file_path, "test");
+ auto setupfut = backend.setup(secretsFilePath.toLocal8Bit().constData(), false);
+ QVERIFY(setupfut.get());
+ auto credfut = backend.setCredentials("test", "ksecrets-test:crypt", "ksecrets-test:mac");
+ QVERIFY(credfut.get());
}
void KSecretServiceStoreTest::cleanupTestCase()
{
- QDir::home().remove(QLatin1Literal(test_file_path));
+ QDir::home().remove(secretsFilePath);
}
void KSecretServiceStoreTest::testOpen()
{
KSecretsStore backend;
- auto setupfut = backend.setup(test_file_path, "test");
- auto openfut = backend.open(true);
- auto openres = openfut.get();
- QVERIFY(openres.status_ == KSecretsStore::StoreStatus::Good);
+ auto setupfut = backend.setup(secretsFilePath.toLocal8Bit().constData());
+ QVERIFY(setupfut.get());
}
diff --git a/src/runtime/ksecrets_store/ksecrets_credentials.cpp \
b/src/runtime/ksecrets_store/ksecrets_credentials.cpp index 96e4107..b59352e 100644
--- a/src/runtime/ksecrets_store/ksecrets_credentials.cpp
+++ b/src/runtime/ksecrets_store/ksecrets_credentials.cpp
@@ -142,9 +142,13 @@ int KSECRETS_STORE_EXPORT kss_set_credentials(const char* user_name, const \
char* return TRUE;
KSecretsStore secretsStore;
- auto setupres = secretsStore.setup(path, password);
+ auto setupres = secretsStore.setup(path);
+ if (!setupres.get()) {
+ return FALSE;
+ }
- return setupres.get() ? 1 : 0;
+ auto credres = secretsStore.setCredentials(password);
+ return credres.get() ? TRUE: FALSE;
}
extern "C"
diff --git a/src/runtime/ksecrets_store/ksecrets_store.cpp \
b/src/runtime/ksecrets_store/ksecrets_store.cpp index df43001..8da20a4 100644
--- a/src/runtime/ksecrets_store/ksecrets_store.cpp
+++ b/src/runtime/ksecrets_store/ksecrets_store.cpp
@@ -52,6 +52,7 @@ KSecretsStorePrivate::KSecretsStorePrivate(KSecretsStore* b)
: b_(b)
{
status_ = KSecretsStore::StoreStatus::JustCreated;
+ memset(&fileHead_, 0, sizeof(fileHead_));
}
KSecretsStore::KSecretsStore()
@@ -61,11 +62,11 @@ KSecretsStore::KSecretsStore()
KSecretsStore::~KSecretsStore() = default;
-std::future<KSecretsStore::SetupResult> KSecretsStore::setup(const char* path, const char* \
password, const char* keyNameEcrypting, const char* keyNameMac) \
+std::future<KSecretsStore::SetupResult> KSecretsStore::setup(const char* path, bool readOnly \
/* = true */) {
// sanity checks
if (d->status_ != StoreStatus::JustCreated) {
- return std::async(std::launch::deferred, []() { return SetupResult{ \
StoreStatus::IncorrectState, -1 }; }); + return std::async(std::launch::deferred, []() { \
return SetupResult({ StoreStatus::IncorrectState, -1 }); }); }
if (path == nullptr || strlen(path) == 0) {
return std::async(std::launch::deferred, []() { return SetupResult{ \
StoreStatus::NoPathGiven, 0 }; }); @@ -75,7 +76,8 @@ std::future<KSecretsStore::SetupResult> \
KSecretsStore::setup(const char* path, c struct stat buf;
if (stat(path, &buf) != 0) {
auto err = errno;
- if (ENOENT == err) {
+ // when asking read only access, creating file is not possible
+ if (ENOENT == err && !readOnly) {
shouldCreateFile = true;
}
else {
@@ -89,16 +91,12 @@ std::future<KSecretsStore::SetupResult> KSecretsStore::setup(const char* \
path, c }
}
- ::keyNameEncrypting = keyNameEcrypting;
- ::keyNameMac = keyNameMac;
-
auto localThis = this;
std::string filePath = path;
- std::string pwd = password;
- return std::async(std::launch::async, [localThis, filePath, shouldCreateFile, pwd]() { \
return localThis->d->setup(filePath, shouldCreateFile, pwd); }); + return \
std::async(std::launch::async, [localThis, filePath, shouldCreateFile, readOnly]() { return \
localThis->d->setup(filePath, shouldCreateFile, readOnly); }); }
-KSecretsStore::SetupResult KSecretsStorePrivate::setup(const std::string& path, bool \
shouldCreateFile, const std::string& password) +KSecretsStore::SetupResult \
KSecretsStorePrivate::setup(const std::string& path, bool shouldCreateFile, bool readOnly) {
if (shouldCreateFile) {
auto createres = createFile(path);
@@ -106,21 +104,38 @@ KSecretsStore::SetupResult KSecretsStorePrivate::setup(const std::string& \
path,
return setStoreStatus(KSecretsStore::SetupResult({ \
KSecretsStore::StoreStatus::SystemError, createres })); }
}
+ secretsFile_.filePath_ = path;
+ secretsFile_.readOnly_ = readOnly;
+ return open(!readOnly);
+}
+
+std::future<KSecretsStore::CredentialsResult> KSecretsStore::setCredentials(const char* \
password, const char* keyNameEncrypting, const char* keyNameMac) +{
+ ::keyNameEncrypting = keyNameEncrypting;
+ ::keyNameMac = keyNameMac;
+
+ std::string pwd = password;
+ auto localThis = this;
+ return std::async(std::launch::async, [localThis, pwd]() { return \
localThis->d->setCredentials(pwd); }); +}
+
+KSecretsStore::CredentialsResult KSecretsStorePrivate::setCredentials(const std::string& \
password) +{
+ using Result = KSecretsStore::CredentialsResult;
if (!kss_init_gcry()) {
- return setStoreStatus(KSecretsStore::SetupResult({ \
KSecretsStore::StoreStatus::CannotInitGcrypt, -1 })); + return setStoreStatus(Result({ \
KSecretsStore::StoreStatus::CannotInitGcrypt, -1 })); }
// FIXME this should be adjusted on platforms where kernel keyring is not available and \
store the keys elsewhere char encryption_key[KSECRETS_KEYSIZE];
char mac_key[KSECRETS_KEYSIZE];
if (!kss_derive_keys(salt(), password.c_str(), encryption_key, mac_key, KSECRETS_KEYSIZE)) \
{
- return setStoreStatus(KSecretsStore::SetupResult({ \
KSecretsStore::StoreStatus::CannotDeriveKeys, -1 })); + return setStoreStatus(Result({ \
KSecretsStore::StoreStatus::CannotDeriveKeys, -1 })); }
if (!kss_store_keys(encryption_key, mac_key, KSECRETS_KEYSIZE)) {
- return setStoreStatus(KSecretsStore::SetupResult({ \
KSecretsStore::StoreStatus::CannotStoreKeys, -1 })); + return setStoreStatus(Result({ \
KSecretsStore::StoreStatus::CannotStoreKeys, errno })); }
- secretsFile_.filePath_ = path;
- return setStoreStatus(KSecretsStore::SetupResult({ KSecretsStore::StoreStatus::SetupDone, \
0 })); + return setStoreStatus(Result({ KSecretsStore::StoreStatus::CredentialsSet, 0 }));
}
char fileMagic[] = { 'k', 's', 'e', 'c', 'r', 'e', 't', 's' };
@@ -139,47 +154,35 @@ int KSecretsStorePrivate::createFile(const std::string& path)
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?
+ if (fwrite(&emptyFileData, 1, sizeof(emptyFileData), f) != sizeof(emptyFileData)) {
+ res = ferror(f);
}
fclose(f);
return res;
}
-std::future<KSecretsStore::OpenResult> KSecretsStore::open(bool readonly /* =true */) noexcept
+bool KSecretsStore::isGood() const noexcept { return d->status_ == StoreStatus::Good; }
+
+const char* KSecretsStore::salt() const
{
- // check the state first
- if (d->status_ == StoreStatus::JustCreated) {
- return std::async(std::launch::deferred, []() { return OpenResult{ \
StoreStatus::SetupShouldBeCalledFirst, -1 }; });
- }
- if (d->status_ != StoreStatus::SetupDone) {
- // open could not be called two times or perhaps the setup call left this store in an \
invalid state because of some error
- return std::async(std::launch::deferred, []() { return OpenResult{ \
StoreStatus::IncorrectState, -1 }; });
- }
- // now we can proceed
- auto localThis = this;
- if (!readonly) {
- return std::async([localThis]() { return localThis->d->open(true); });
- }
- else {
- return std::async([localThis]() { return localThis->d->open(false); });
- }
+ if (isGood())
+ return d->salt();
+ else
+ return nullptr;
}
-const char* KSecretsStore::salt() const { return d->salt(); }
-
const char* KSecretsStorePrivate::salt() const { return fileHead_.salt; }
-KSecretsStore::OpenResult KSecretsStorePrivate::open(bool lockFile)
+KSecretsStore::SetupResult KSecretsStorePrivate::open(bool lockFile)
{
- using OpenResult = KSecretsStore::OpenResult;
+ using OpenResult = KSecretsStore::SetupResult;
secretsFile_.file_ = ::open(secretsFile_.filePath_.c_str(), O_DSYNC | O_NOATIME | \
O_NOFOLLOW); if (secretsFile_.file_ == -1) {
return setStoreStatus(OpenResult({ KSecretsStore::StoreStatus::CannotOpenFile, errno \
})); }
if (lockFile) {
if (flock(secretsFile_.file_, LOCK_EX) == -1) {
- return KSecretsStore::OpenResult{ KSecretsStore::StoreStatus::CannotLockFile, \
errno }; + return setStoreStatus(OpenResult({ \
KSecretsStore::StoreStatus::CannotLockFile, errno })); }
secretsFile_.locked_ = true;
}
@@ -194,7 +197,8 @@ KSecretsStore::OpenResult KSecretsStorePrivate::open(bool lockFile)
return setStoreStatus(OpenResult({ KSecretsStore::StoreStatus::Good, 0 }));
}
-KSecretsStorePrivate::SecretsFile::~SecretsFile(){
+KSecretsStorePrivate::SecretsFile::~SecretsFile()
+{
if (file_ != -1) {
auto r = close(file_);
if (r == -1) {
diff --git a/src/runtime/ksecrets_store/ksecrets_store.h \
b/src/runtime/ksecrets_store/ksecrets_store.h index bb4f394..4d3e5b2 100644
--- a/src/runtime/ksecrets_store/ksecrets_store.h
+++ b/src/runtime/ksecrets_store/ksecrets_store.h
@@ -190,11 +190,11 @@ public:
Good,
JustCreated,
SetupShouldBeCalledFirst,
+ CredentialsSet,
IncorrectState,
CannotInitGcrypt,
CannotDeriveKeys,
CannotStoreKeys,
- SetupDone,
SetupError,
NoPathGiven,
InvalidFile, // the file format was not recognized. Is this a ksecrets file?
@@ -204,38 +204,47 @@ public:
SystemError
};
- struct SetupResult {
+ template <StoreStatus G>
+ struct OpResult {
StoreStatus status_;
int errno_;
- operator bool() const { return status_ == StoreStatus::SetupDone; }
+ operator bool() const { return status_ == G; }
};
+ using SetupResult = OpResult<StoreStatus::Good>;
+ // struct SetupResult {
+ // StoreStatus status_;
+ // int errno_;
+ // operator bool() const { return status_ == StoreStatus::Good; }
+ // };
+
/*
- * Before opening, the store must be setup, that is, it must know the
- * file path and the user's password. This method is typically called
- * from the pam module but it can also be called from another program
- * after prompting user for the password, for example.
- *
- * This call creates the file if it's not found.
+ * Before usage, the store must be setup, that is, it must know its file path.
+ * This call creates the file if it's not found and the readOnly flag is set to false.
+ * The file is not created when the readOnly flag is set to false in order to prevent
+ * unintended side effects. If the file don't exists, the SetupResult.statu_ is set
+ * to StoreStatus::SystemError and errno should be 2
*
* @return SetupResult whose operator bool could be used to check the error condition
*/
- std::future<SetupResult> setup(const char* path, const char* password, const char* \
keyNameEcrypting = "ksecrets:encrypting", const char* keyNameMac = \
"ksecrets:mac");
-
- struct OpenResult {
- StoreStatus status_;
- int errno_;
- operator bool() const { return status_ == StoreStatus::Good; }
- };
-
+ std::future<SetupResult> setup(const char* path, bool readOnly =true);
+
+ using CredentialsResult = OpResult<StoreStatus::CredentialsSet>;
+ // struct CredentialsResult {
+ // StoreStatus status_;
+ // int errno_;
+ // operator bool() const { return status_ == StoreStatus::Good; }
+ // };
/**
- * @parameter path full path to the store file to handle
- * @parameter readOnly set it to true if you only intend reading to speed
- * things-up and avoid placing a lock on the file
+ * Set the system-wide credentials for the secrets store
*
- * @return OpenResult whose operator bool could be used to check the error condition
+ * This method is typically called from the pam module but it can also be
+ * called from another program after prompting user for the password, for example.
+ * Client applications only need to call this once per user session.
*/
- std::future<OpenResult> open(bool readOnly = true) noexcept;
+ std::future<CredentialsResult> setCredentials(const char* password = nullptr, const char* \
keyNameEcrypting = "ksecrets:encrypting", const char* keyNameMac = "ksecrets:mac"); +
+ bool isGood() const noexcept;
constexpr static auto SALT_SIZE = 56;
/**
diff --git a/src/runtime/ksecrets_store/ksecrets_store_p.h \
b/src/runtime/ksecrets_store/ksecrets_store_p.h index 4ec75c8..270763d 100644
--- a/src/runtime/ksecrets_store/ksecrets_store_p.h
+++ b/src/runtime/ksecrets_store/ksecrets_store_p.h
@@ -58,9 +58,9 @@ public:
KSecretsStorePrivate() = delete;
explicit KSecretsStorePrivate(KSecretsStore*);
- KSecretsStore::SetupResult setup(const std::string& path, bool, const std::string&);
- KSecretsStore::OpenResult open(bool lock);
- using open_action = std::function<KSecretsStore::OpenResult(const std::string&)>;
+ KSecretsStore::SetupResult setup(const std::string& path, bool, bool);
+ KSecretsStore::CredentialsResult setCredentials(const std::string&);
+ KSecretsStore::SetupResult open(bool);
int createFile(const std::string&);
const char* salt() const;
@@ -80,11 +80,13 @@ public:
struct SecretsFile {
SecretsFile()
- : file_(-1), locked_(false) {};
+ : file_(-1)
+ , locked_(false){};
~SecretsFile();
std::string filePath_;
int file_;
bool locked_;
+ bool readOnly_;
};
KSecretsStore* b_;
SecretsFile secretsFile_;
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic