[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-commits
Subject: [ksecrets] /: Refactoring after the code review
From: Valentin Rusu <kde () rusu ! info>
Date: 2015-08-12 11:26:27
Message-ID: E1ZPUAt-0007H7-L1 () scm ! kde ! org
[Download RAW message or body]
Git commit a250017bb30db279b5e9a24d3843bbd91005fd61 by Valentin Rusu.
Committed on 12/08/2015 at 11:25.
Pushed by vrusu into branch 'master'.
Refactoring after the code review
Also added and implemented the setup method. More tests should now be added.
M +5 -2 autotests/api/CMakeLists.txt
M +2 -2 autotests/api/ksecretsservice-test.cpp
M +8 -4 autotests/ksecrets_store/ksecrets_store_test.cpp
M +1 -1 src/api/ksecrets/ksecretsitem.cpp
M +0 -1 src/runtime/CMakeLists.txt
M +4 -4 src/runtime/ksecrets-crypt/config.cpp
M +5 -4 src/runtime/ksecrets_store/CMakeLists.txt
R +24 -116 src/runtime/ksecrets_store/ksecrets_credentials.c [from: \
src/runtime/ksecrets_store/ksecrets_crypt.c - 053% similarity] R +7 -7 \
src/runtime/ksecrets_store/ksecrets_credentials.h [from: \
src/runtime/ksecrets_store/ksecrets_crypt.h - 077% similarity] M +102 -49 \
src/runtime/ksecrets_store/ksecrets_store.cpp M +50 -18 \
src/runtime/ksecrets_store/ksecrets_store.h M +19 -8 \
src/runtime/ksecrets_store/ksecrets_store_p.h M +4 -4 \
src/runtime/pam-ksecrets/CMakeLists.txt M +9 -3 src/runtime/pam-ksecrets/pam_ksecrets.c
http://commits.kde.org/ksecrets/a250017bb30db279b5e9a24d3843bbd91005fd61
diff --git a/autotests/api/CMakeLists.txt b/autotests/api/CMakeLists.txt
index 071cb72..6dc2343 100644
--- a/autotests/api/CMakeLists.txt
+++ b/autotests/api/CMakeLists.txt
@@ -6,7 +6,10 @@ find_package(Qt5 ${REQUIRED_QT_VERSION} CONFIG REQUIRED Test)
find_package(LibGcrypt 1.6.0 REQUIRED)
-include_directories(${CMAKE_SOURCE_DIR}/src/runtime/ksecrets-crypt)
+include_directories(
+ ${CMAKE_SOURCE_DIR}/src/runtime/ksecrets_store
+ ${CMAKE_BINARY_DIR}/src/runtime/ksecrets_store
+ )
if(NOT Qt5Test_FOUND)
message(STATUS "Qt5Test not found, autotests will not be built.")
@@ -17,6 +20,6 @@ find_package(KF5 REQUIRED CoreAddons)
ecm_add_test(
ksecretsservice-test.cpp
- LINK_LIBRARIES KF5::Secrets Qt5::Test KF5::Secrets KF5::CoreAddons ksecrets_crypt \
${LIBGCRYPT_LIBRARIES} keyutils + LINK_LIBRARIES KF5::Secrets Qt5::Test KF5::Secrets \
KF5::CoreAddons ksecrets_store ${LIBGCRYPT_LIBRARIES} keyutils )
diff --git a/autotests/api/ksecretsservice-test.cpp b/autotests/api/ksecretsservice-test.cpp
index efa0788..c16c424 100644
--- a/autotests/api/ksecretsservice-test.cpp
+++ b/autotests/api/ksecretsservice-test.cpp
@@ -24,7 +24,7 @@
#include <ksecretscollection.h>
#include <ksecretsvalue.h>
#include <ksecretsitem.h>
-#include <ksecrets-crypt.h>
+#include <ksecrets_credentials.h>
#include <QtCore/QtGlobal>
#include <QtTest/QTest>
@@ -47,7 +47,7 @@ KSecretServiceTest::KSecretServiceTest(QObject* parent)
}
KSecrets::CollectionPtr collection;
-extern KSharedConfig::Ptr sharedConfig; // defined in the ksecrets_crypt library
+KSharedConfig::Ptr sharedConfig;
void KSecretServiceTest::initTestCase()
{
diff --git a/autotests/ksecrets_store/ksecrets_store_test.cpp \
b/autotests/ksecrets_store/ksecrets_store_test.cpp index ba70646..353f10a 100644
--- a/autotests/ksecrets_store/ksecrets_store_test.cpp
+++ b/autotests/ksecrets_store/ksecrets_store_test.cpp
@@ -19,9 +19,9 @@
Boston, MA 02110-1301, USA.
*/
-#include "ksecrets_backend_test.h"
+#include "ksecrets_store_test.h"
-#include <ksecrets_backend.h>
+#include <ksecrets_store.h>
#include <QtTest/QtTest>
#include <QtCore/QDir>
@@ -36,6 +36,9 @@ KSecretServiceStoreTest::KSecretServiceStoreTest(QObject* parent)
void KSecretServiceStoreTest::initTestCase()
{
+ // create a test file here and performe the real open test below
+ KSecretsStore backend;
+ auto setupfut = backend.setup(test_file_path, "test");
}
void KSecretServiceStoreTest::cleanupTestCase()
@@ -46,7 +49,8 @@ void KSecretServiceStoreTest::cleanupTestCase()
void KSecretServiceStoreTest::testOpen()
{
KSecretsStore backend;
- auto openfut = backend.open(test_file_path, true);
+ auto setupfut = backend.setup(test_file_path, "test");
+ auto openfut = backend.open(true);
auto openres = openfut.get();
- QVERIFY(openres.status_ == KSecretsStore::OpenStatus::Good);
+ QVERIFY(openres.status_ == KSecretsStore::StoreStatus::Good);
}
diff --git a/src/api/ksecrets/ksecretsitem.cpp b/src/api/ksecrets/ksecretsitem.cpp
index e8e039f..8eda750 100644
--- a/src/api/ksecrets/ksecretsitem.cpp
+++ b/src/api/ksecrets/ksecretsitem.cpp
@@ -32,7 +32,7 @@ SecretItem::SecretItem()
}
SecretItem::SecretItem(const SecretItem& that)
- d(that.d)
+ : d(that.d)
{
}
diff --git a/src/runtime/CMakeLists.txt b/src/runtime/CMakeLists.txt
index d26d1bb..01eb2e5 100644
--- a/src/runtime/CMakeLists.txt
+++ b/src/runtime/CMakeLists.txt
@@ -1,4 +1,3 @@
-add_subdirectory(ksecrets-crypt)
add_subdirectory(pam-ksecrets)
add_subdirectory(ksecrets_store)
# add_subdirectory(ksecrets)
diff --git a/src/runtime/ksecrets-crypt/config.cpp b/src/runtime/ksecrets-crypt/config.cpp
index 771254f..a711c6c 100644
--- a/src/runtime/ksecrets-crypt/config.cpp
+++ b/src/runtime/ksecrets-crypt/config.cpp
@@ -19,7 +19,7 @@
Boston, MA 02110-1301, USA.
*/
-#include <ksecrets_backend.h>
+#include <ksecrets_store.h>
#include <ksharedconfig.h>
#include <kconfiggroup.h>
@@ -80,9 +80,9 @@ const char* get_keyname_mac()
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) {
+ KSecretsStore store;
+ auto openfut = store.open(path);
+ if (openfut.get().status_ == KSecretsStore::OpenStatus::Good) {
}
return res;
diff --git a/src/runtime/ksecrets_store/CMakeLists.txt \
b/src/runtime/ksecrets_store/CMakeLists.txt index dcee020..82b5ad1 100644
--- a/src/runtime/ksecrets_store/CMakeLists.txt
+++ b/src/runtime/ksecrets_store/CMakeLists.txt
@@ -9,14 +9,15 @@ ecm_setup_version(${KF5_VERSION} VARIABLE_PREFIX KSECRETS_BACKEND
PACKAGE_VERSION_FILE \
"${CMAKE_CURRENT_BINARY_DIR}/KF5SecretsStoreConfigVersion.cmake")
set(ksecrets_store_SRC
- ksecrets_crypt.c
- ksecrets_store.cpp)
+ ksecrets_credentials.c
+ ksecrets_store.cpp)
add_library(ksecrets_store SHARED ${ksecrets_store_SRC})
generate_export_header(ksecrets_store BASE_NAME ksecrets_store)
target_link_libraries(ksecrets_store
- pthread
- ${LIBGCRYPT_LIBRARIES})
+ pthread
+ keyutils
+ ${LIBGCRYPT_LIBRARIES})
target_compile_features(ksecrets_store PRIVATE cxx_range_for)
set_target_properties(ksecrets_store PROPERTIES PREFIX "")
diff --git a/src/runtime/ksecrets_store/ksecrets_crypt.c \
b/src/runtime/ksecrets_store/ksecrets_credentials.c similarity index 53%
rename from src/runtime/ksecrets_store/ksecrets_crypt.c
rename to src/runtime/ksecrets_store/ksecrets_credentials.c
index 959765f..155796a 100644
--- a/src/runtime/ksecrets_store/ksecrets_crypt.c
+++ b/src/runtime/ksecrets_store/ksecrets_credentials.c
@@ -17,7 +17,7 @@
* the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
* Boston, MA 02110-1301, USA.
*/
-#include "ksecrets_crypt.h"
+#include "ksecrets_credentials.h"
#include <unistd.h>
#include <errno.h>
@@ -39,114 +39,37 @@
#define KSS_LOG_INFO (LOG_AUTH | LOG_INFO)
#define KSS_LOG_ERR (LOG_AUTH | LOG_ERR)
-#define KSECRETS_SALTSIZE 56
-#define KSECRETS_KEYSIZE 256
#define KSECRETS_ITERATIONS 50000
/* these functions are implemented in config.cpp next to this file */
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()
+#define false 0
+#define true 1
+
+int kss_init_gcry()
{
- syslog(KSS_LOG_DEBUG, "setting-up grypt library");
+ syslog(KSS_LOG_DEBUG, "ksecrets: setting-up grypt library");
if (!gcry_check_version(GCRYPT_REQUIRED_VERSION)) {
- syslog(KSS_LOG_ERR, "kwalletd: libcrypt version is too old");
- return false;
+ syslog(KSS_LOG_ERR, "ksecrets_store: libcrypt version is too old");
+ return 0;
}
gcry_error_t gcryerr;
gcryerr = gcry_control(GCRYCTL_INIT_SECMEM, 32768, 0);
if (gcryerr != 0) {
- syslog(KSS_LOG_ERR, "ksecrets: cannot get secure memory: %d", gcryerr);
- return false;
+ syslog(KSS_LOG_ERR, "ksecrets_store: cannot get secure memory: %d", gcryerr);
+ return 0;
}
gcry_control(GCRYCTL_INITIALIZATION_FINISHED, 0);
syslog(KSS_LOG_DEBUG, "gcrypt library now set-up");
- return true;
+ return 1;
}
-/**
- * This function reads the crypting salt from the main ksecrets data file
- * If the file is not present, then it is created. This case should happen
- * when user opens the session for the first time. That is possible because
- * ksecrets file format is SALT-IV-CIPHERTEXT-MAC. So in our case we'll create
- * file containing the SALT. It'll be extended by the ksecrets daemon upon
- * first usage.
- *
- * NOTE This function has code from pam-kwallet
- */
-bool kss_get_salt(const char* username, char** salt)
-{
- if (0 == username) {
- syslog(KSS_LOG_ERR, "no username given, salt could not been retrieved");
- return false;
- }
-
- struct passwd* user_info;
- user_info = getpwnam(username);
- if (!user_info) {
- syslog(KSS_LOG_ERR, "pam_kwallet: Couldn't get user %s info", username);
- return false;
- }
-
- struct stat info;
- if (stat(user_info->pw_dir, &info) != 0) {
- syslog(KSS_LOG_ERR, "pam_kwallet: Cannot stat user directory");
- return false;
- }
-
- const char* path = prepare_secret_file_location(user_info->pw_dir);
- if (NULL == path) {
- syslog(KSS_LOG_ERR, "pam_kwallet: Cannot prepare secret file location");
- return false;
- }
-
- *salt = NULL;
- if (stat(path, &info) != 0 || info.st_size == 0) {
- unlink(path); /* in case the file already exists and it has size of 0 */
-
- *salt = gcry_random_bytes(KSECRETS_SALTSIZE, GCRY_STRONG_RANDOM);
- FILE* fd = fopen(path, "w");
-
- /* If the file can't be created */
- if (fd == NULL) {
- syslog(KSS_LOG_ERR, "Couldn't open file: %s because: %d (%m)", path, errno);
- return false;
- }
-
- fwrite(*salt, KSECRETS_SALTSIZE, 1, fd);
- fclose(fd);
-
- if (chown(path, user_info->pw_uid, user_info->pw_gid) == -1) {
- syslog(KSS_LOG_ERR, "Couldn't change ownership of the created salt file");
- return false;
- }
- syslog(KSS_LOG_INFO, "ksecrets: created secrets file path : %s", path);
- }
- else {
- FILE* fd = fopen(path, "r");
- if (fd == NULL) {
- syslog(KSS_LOG_ERR, "Couldn't open file: %s because: %d (%m)", path, errno);
- return 1;
- }
- *salt = (char*)malloc(sizeof(char) * KSECRETS_SALTSIZE);
- memset(*salt, '\0', KSECRETS_SALTSIZE);
- fread(*salt, KSECRETS_SALTSIZE, 1, fd);
- fclose(fd);
- syslog(KSS_LOG_INFO, "ksecrets: read salt from secrets file : %s", path);
- }
- if (salt == NULL) {
- syslog(KSS_LOG_ERR, "ksecrets: Couldn't create or read the salt file");
- return false;
- }
- return true;
-}
-
-bool kss_derive_keys(const char* user_name, const char* password, char* encryption_key, char* \
mac_key) +int kss_derive_keys(const char* salt, const char* password, char* encryption_key, \
char* mac_key, size_t keySize) {
gpg_error_t gcryerr;
@@ -156,34 +79,26 @@ bool kss_derive_keys(const char* user_name, const char* password, char* \
encrypti return false;
}
- if (!kss_init_gcry())
- return false;
-
- char* salt;
- salt = 0;
- if (!kss_get_salt(user_name, &salt))
- return false;
-
/* generate both encryption and MAC key in one go */
- char keys[2 * KSECRETS_KEYSIZE];
- gcryerr = gcry_kdf_derive(password, strlen(password), GCRY_KDF_ITERSALTED_S2K, \
GCRY_MD_SHA512, salt, 8, KSECRETS_ITERATIONS, 2 * KSECRETS_KEYSIZE, keys); + char keys[2 * \
keySize]; + gcryerr = gcry_kdf_derive(password, strlen(password), GCRY_KDF_ITERSALTED_S2K, \
GCRY_MD_SHA512, salt, 8, KSECRETS_ITERATIONS, 2 * keySize, keys); if (gcryerr) {
syslog(KSS_LOG_ERR, "key derivation failed: code 0x%0x: %s/%s", gcryerr, \
gcry_strsource(gcryerr), gcry_strerror(gcryerr)); return false;
}
- memcpy(encryption_key, keys, KSECRETS_KEYSIZE);
- memcpy(mac_key, keys + KSECRETS_KEYSIZE, KSECRETS_KEYSIZE);
+ memcpy(encryption_key, keys, keySize);
+ memcpy(mac_key, keys + keySize, keySize);
syslog(KSS_LOG_INFO, "successuflly generated ksecrets keys from user password.");
return true;
}
-bool kss_store_keys(const char* encryption_key, const char* mac_key)
+int kss_store_keys(const char* encryption_key, const char* mac_key, size_t keySize)
{
key_serial_t ks;
const char* key_name = get_keyname_encrypting();
- ks = add_key("user", key_name, encryption_key, KSECRETS_KEYSIZE, \
KEY_SPEC_SESSION_KEYRING); + ks = add_key("user", key_name, encryption_key, keySize, \
KEY_SPEC_SESSION_KEYRING); if (-1 == ks) {
syslog(KSS_LOG_ERR, "ksecrets: cannot store encryption key in kernel keyring: errno=%d \
(%m)", errno); return false;
@@ -191,7 +106,7 @@ bool kss_store_keys(const char* encryption_key, const char* mac_key)
syslog(KSS_LOG_DEBUG, "ksecrets: encrpyting key now in kernel keyring with id %d and desc \
%s", ks, key_name);
key_name = get_keyname_mac();
- ks = add_key("user", key_name, mac_key, KSECRETS_KEYSIZE, KEY_SPEC_SESSION_KEYRING);
+ ks = add_key("user", key_name, mac_key, keySize, KEY_SPEC_SESSION_KEYRING);
if (-1 == ks) {
syslog(KSS_LOG_ERR, "ksecrets: cannot store mac key in kernel keyring: errno=%d (%m)", \
errno); return false;
@@ -200,7 +115,7 @@ bool kss_store_keys(const char* encryption_key, const char* mac_key)
return true;
}
-bool kss_keys_already_there()
+int kss_keys_already_there()
{
key_serial_t key;
key = request_key("user", get_keyname_encrypting(), 0, KEY_SPEC_SESSION_KEYRING);
@@ -212,24 +127,17 @@ bool kss_keys_already_there()
return true;
}
-bool kss_set_credentials(const char* user_name, const char* password)
+int kss_set_credentials(const char* user_name, const char* password)
{
syslog(KSS_LOG_DEBUG, "kss_set_credentials for %s", user_name);
if (kss_keys_already_there())
return true;
- char encryption_key[KSECRETS_KEYSIZE];
- char mac_key[KSECRETS_KEYSIZE];
- if (!kss_derive_keys(user_name, password, encryption_key, mac_key))
- return false;
-
- if (!kss_store_keys(encryption_key, mac_key))
- return false;
return true;
}
-bool kss_delete_credentials()
+int kss_delete_credentials()
{
syslog(KSS_LOG_INFO, "kss_delete_credentials");
key_serial_t key;
@@ -257,14 +165,14 @@ bool kss_delete_credentials()
return true;
}
-bool kss_can_change_password()
+int kss_can_change_password()
{
/* nothing to do for the moment */
syslog(KSS_LOG_INFO, "kss_can_change_password");
return true;
}
-bool kss_change_password(const char* new_password)
+int kss_change_password(const char* new_password)
{
syslog(LOG_INFO, "kss_change_password");
return true;
diff --git a/src/runtime/ksecrets_store/ksecrets_crypt.h \
b/src/runtime/ksecrets_store/ksecrets_credentials.h similarity index 77%
rename from src/runtime/ksecrets_store/ksecrets_crypt.h
rename to src/runtime/ksecrets_store/ksecrets_credentials.h
index 8e743d8..0242a24 100644
--- a/src/runtime/ksecrets_store/ksecrets_crypt.h
+++ b/src/runtime/ksecrets_store/ksecrets_credentials.h
@@ -18,22 +18,22 @@
the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
Boston, MA 02110-1301, USA.
*/
-#ifndef KSECRETSCRYPT_H
-#define KSECRETSCRYPT_H
+#ifndef KSECRETS_CREDENTIALS_H
+#define KSECRETS_CREDENTIALS_H
-#include <stdbool.h>
+#include <ksecrets_store_export.h>
#ifdef __cplusplus
extern "C" {
#endif
-bool kss_set_credentials(const char* user_name, const char* password);
+int kss_set_credentials(const char* user_name, const char* password);
-bool kss_delete_credentials();
+int kss_delete_credentials();
-bool kss_can_change_password();
+int kss_can_change_password();
-bool kss_change_password(const char* newPassword);
+int kss_change_password(const char* newPassword);
#ifdef __cplusplus
}
diff --git a/src/runtime/ksecrets_store/ksecrets_store.cpp \
b/src/runtime/ksecrets_store/ksecrets_store.cpp index 775f896..df43001 100644
--- a/src/runtime/ksecrets_store/ksecrets_store.cpp
+++ b/src/runtime/ksecrets_store/ksecrets_store.cpp
@@ -24,16 +24,34 @@
#include <future>
#include <thread>
+#include <sys/types.h>
#include <sys/stat.h>
+#include <sys/file.h>
#include <unistd.h>
#include <stdio.h>
+#include <syslog.h>
#define GCRPYT_NO_DEPRECATED
#include <gcrypt.h>
+#define KSECRETS_SALTSIZE 56
+#define KSECRETS_KEYSIZE 256
+#define KSS_LOG_ERR (LOG_AUTH | LOG_ERR)
+
+const char* keyNameEncrypting = nullptr;
+const char* keyNameMac = nullptr;
+
+extern "C" {
+bool kss_init_gcry();
+bool kss_derive_keys(const char* salt, const char* password, char* encryption_key, char* \
mac_key, size_t); +bool kss_store_keys(const char* encryption_key, const char* mac_key, size_t \
keySize); +const char* get_keyname_encrypting() { return keyNameEncrypting; }
+const char* get_keyname_mac() { return keyNameMac; }
+}
+
KSecretsStorePrivate::KSecretsStorePrivate(KSecretsStore* b)
: b_(b)
{
- openStatus_.status_ = KSecretsStore::OpenStatus::NotYetOpened;
+ status_ = KSecretsStore::StoreStatus::JustCreated;
}
KSecretsStore::KSecretsStore()
@@ -43,52 +61,66 @@ KSecretsStore::KSecretsStore()
KSecretsStore::~KSecretsStore() = default;
-std::future<KSecretsStore::OpenResult> KSecretsStore::open(std::string&& path, bool readonly \
/* =true */) noexcept +std::future<KSecretsStore::SetupResult> KSecretsStore::setup(const char* \
path, const char* password, const char* keyNameEcrypting, const char* keyNameMac) {
// sanity checks
- if (path.empty()) {
- return std::async(std::launch::deferred, []() { return OpenResult{ \
OpenStatus::NoPathGiven, 0 }; }); + if (d->status_ != StoreStatus::JustCreated) {
+ 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 }; }); }
bool shouldCreateFile = false;
struct stat buf;
- if (stat(path.c_str(), &buf) != 0) {
+ if (stat(path, &buf) != 0) {
auto err = errno;
if (ENOENT == err) {
shouldCreateFile = true;
}
else {
- return std::async(std::launch::deferred, [err]() { return OpenResult{ \
OpenStatus::SystemError, errno }; }); + return std::async(std::launch::deferred, \
[err]() { return SetupResult{ StoreStatus::SystemError, errno }; }); }
}
else {
if (buf.st_size == 0) {
- unlink(path.c_str());
+ unlink(path);
shouldCreateFile = true; // recreate if empty file was found
}
}
- // now we can proceed
+ ::keyNameEncrypting = keyNameEcrypting;
+ ::keyNameMac = keyNameMac;
+
auto localThis = this;
- if (!readonly) {
- return std::async(
- std::launch::async, [localThis, path, shouldCreateFile]() { return \
localThis->d->createFileIfNeededThenDo(path, shouldCreateFile, \
std::bind(&KSecretsStorePrivate::lock_open, localThis->d.get(), path)); });
- }
- else {
- return std::async(
- std::launch::deferred, [localThis, path, shouldCreateFile]() { return \
localThis->d->createFileIfNeededThenDo(path, shouldCreateFile, \
std::bind(&KSecretsStorePrivate::open, localThis->d.get(), path)); });
- }
+ 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); }); }
-KSecretsStore::OpenResult KSecretsStorePrivate::createFileIfNeededThenDo(const std::string& \
path, bool shouldCreateFile, open_action action) +KSecretsStore::SetupResult \
KSecretsStorePrivate::setup(const std::string& path, bool shouldCreateFile, const std::string& \
password) {
if (shouldCreateFile) {
- auto createRes = createFile(path);
- if (createRes != 0) {
- return setOpenStatus({ KSecretsStore::OpenStatus::SystemError, createRes });
+ auto createres = createFile(path);
+ if (createres != 0) {
+ return setStoreStatus(KSecretsStore::SetupResult({ \
KSecretsStore::StoreStatus::SystemError, createres })); }
}
- return action(path);
+ if (!kss_init_gcry()) {
+ return setStoreStatus(KSecretsStore::SetupResult({ \
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 })); + }
+
+ if (!kss_store_keys(encryption_key, mac_key, KSECRETS_KEYSIZE)) {
+ return setStoreStatus(KSecretsStore::SetupResult({ \
KSecretsStore::StoreStatus::CannotStoreKeys, -1 })); + }
+ secretsFile_.filePath_ = path;
+ return setStoreStatus(KSecretsStore::SetupResult({ KSecretsStore::StoreStatus::SetupDone, \
0 })); }
char fileMagic[] = { 'k', 's', 'e', 'c', 'r', 'e', 't', 's' };
@@ -114,42 +146,63 @@ int KSecretsStorePrivate::createFile(const std::string& path)
return res;
}
-const char* KSecretsStore::salt() const { return d->salt(); }
-
-const char* KSecretsStorePrivate::salt() const { return fileHead_.salt; }
-
-KSecretsStore::OpenResult KSecretsStorePrivate::lock_open(const std::string& path)
+std::future<KSecretsStore::OpenResult> KSecretsStore::open(bool readonly /* =true */) noexcept
{
- file_ = fopen(path.c_str(), "w+");
- if (file_ == nullptr) {
- return { KSecretsStore::OpenStatus::SystemError, errno };
+ // 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); });
}
- // TODO perform the lock here
- return open(path);
}
-KSecretsStore::OpenResult KSecretsStorePrivate::setOpenStatus(KSecretsStore::OpenResult \
openStatus)
-{
- openStatus_ = openStatus;
- return openStatus;
-}
+const char* KSecretsStore::salt() const { return d->salt(); }
-KSecretsStore::OpenResult KSecretsStorePrivate::open(const std::string& path)
+const char* KSecretsStorePrivate::salt() const { return fileHead_.salt; }
+
+KSecretsStore::OpenResult KSecretsStorePrivate::open(bool lockFile)
{
- if (file_ == nullptr) {
- file_ = fopen(path.c_str(), "r");
+ using OpenResult = KSecretsStore::OpenResult;
+ 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 (file_ == nullptr) {
- return setOpenStatus({ KSecretsStore::OpenStatus::SystemError, errno });
+ if (lockFile) {
+ if (flock(secretsFile_.file_, LOCK_EX) == -1) {
+ return KSecretsStore::OpenResult{ KSecretsStore::StoreStatus::CannotLockFile, \
errno }; + }
+ secretsFile_.locked_ = true;
}
- if (fread(&fileHead_, sizeof(fileHead_), 1, file_) != sizeof(fileHead_)) {
- return setOpenStatus({ KSecretsStore::OpenStatus::SystemError, ferror(file_) });
+ auto r = read(secretsFile_.file_, &fileHead_, sizeof(fileHead_));
+ if (r == -1) {
+ return setStoreStatus(OpenResult({ KSecretsStore::StoreStatus::CannotReadFile, errno \
})); }
- if (memcmp(fileHead_.magic, fileMagic, fileMagicLen) != 0) {
- return setOpenStatus({ KSecretsStore::OpenStatus::InvalidFile, 0 });
+ if ((size_t)r < sizeof(fileHead_) || memcmp(fileHead_.magic, fileMagic, fileMagicLen) != \
0) { + return setStoreStatus(OpenResult({ KSecretsStore::StoreStatus::InvalidFile, -1 \
})); }
// decrypting will occur upon collection request
- return setOpenStatus({ KSecretsStore::OpenStatus::Good, 0 });
+ 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::CollectionNames KSecretsStore::dirCollections() const noexcept
@@ -158,13 +211,13 @@ KSecretsStore::CollectionNames KSecretsStore::dirCollections() const \
noexcept return CollectionNames();
}
-KSecretsStore::CollectionPtr KSecretsStore::createCollection(std::string&&) noexcept
+KSecretsStore::CollectionPtr KSecretsStore::createCollection(const char*) noexcept
{
// TODO
return CollectionPtr();
}
-KSecretsStore::CollectionPtr KSecretsStore::readCollection(std::string&&) const noexcept
+KSecretsStore::CollectionPtr KSecretsStore::readCollection(const char*) const noexcept
{
// TODO
return CollectionPtr();
@@ -176,7 +229,7 @@ bool KSecretsStore::deleteCollection(CollectionPtr) noexcept
return false;
}
-bool KSecretsStore::deleteCollection(std::string&&) noexcept
+bool KSecretsStore::deleteCollection(const char*) noexcept
{
// TODO
return false;
diff --git a/src/runtime/ksecrets_store/ksecrets_store.h \
b/src/runtime/ksecrets_store/ksecrets_store.h index b7d0440..bb4f394 100644
--- a/src/runtime/ksecrets_store/ksecrets_store.h
+++ b/src/runtime/ksecrets_store/ksecrets_store.h
@@ -18,10 +18,10 @@
the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
Boston, MA 02110-1301, USA.
*/
-#ifndef KSECRETS_BACKEND_H
-#define KSECRETS_BACKEND_H
+#ifndef KSECRETS_STORE_H
+#define KSECRETS_STORE_H
-#include <ksecrets_backend_export.h>
+#include <ksecrets_store_export.h>
#include <memory>
#include <ctime>
@@ -45,7 +45,7 @@ class KSecretsStorePrivate;
* 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.
+ * The API calls are organized in classes, following the structure of data in the store.
* 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.
@@ -69,7 +69,7 @@ class KSecretsStorePrivate;
* By providing a class, one could use local variables and the class
* would be destroyed, releasing the file, upon block exit.
*/
-class KSECRETS_BACKEND_EXPORT KSecretsStore {
+class KSECRETS_STORE_EXPORT KSecretsStore {
class ItemPrivate;
class CollectionPrivate;
@@ -144,6 +144,11 @@ public:
ItemList searchItems(const char*, AttributesMap&&) noexcept;
/**
+ * Creates an item in the collection in one go. This is more efficient than
+ * initializing en empty than setting the label, eventually the parameters and
+ * the value. The returned item may still be modified, keep in mind that each
+ * method call will trigger an store file update.
+ *
* @return ItemPtr which can be empty if creating the item was not
* possible. So please check it via it's operator bool() before using
* it.
@@ -171,7 +176,7 @@ public:
/*
* Default constructor.
*
- * This constructor only initializes the backend class. You should call
+ * This constructor only initializes the store class. You should call
* the open() method right after the initialization and before any other
* methods of this API.
*
@@ -181,28 +186,56 @@ public:
KSecretsStore(const KSecretsStore&) = delete;
virtual ~KSecretsStore();
- enum class OpenStatus {
- NotYetOpened,
+ enum class StoreStatus {
Good,
+ JustCreated,
+ SetupShouldBeCalledFirst,
+ IncorrectState,
+ CannotInitGcrypt,
+ CannotDeriveKeys,
+ CannotStoreKeys,
+ SetupDone,
+ SetupError,
NoPathGiven,
InvalidFile, // the file format was not recognized. Is this a ksecrets file?
- FileLocked,
+ CannotOpenFile,
+ CannotLockFile,
+ CannotReadFile,
SystemError
};
+
+ struct SetupResult {
+ StoreStatus status_;
+ int errno_;
+ operator bool() const { return status_ == StoreStatus::SetupDone; }
+ };
+
+ /*
+ * 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.
+ *
+ * @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 {
- OpenStatus status_;
+ StoreStatus status_;
int errno_;
+ operator bool() const { return status_ == StoreStatus::Good; }
};
/**
- * 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 path full path to the store file to handle
* @parameter readOnly set it to true if you only intend reading to speed
- * things-up
+ * things-up and avoid placing a lock on the file
+ *
+ * @return OpenResult whose operator bool could be used to check the error condition
*/
- std::future<OpenResult> open(const char* path, bool readOnly = true) noexcept;
+ std::future<OpenResult> open(bool readOnly = true) noexcept;
constexpr static auto SALT_SIZE = 56;
/**
@@ -214,9 +247,8 @@ public:
CollectionNames dirCollections() const noexcept;
/*
* @return CollectionPtr which can empty if the call did not succeed.
- *Please check that with operator bool().
+ * Please check that with operator bool().
* If it fails, have you already called open()?
- *
*/
CollectionPtr createCollection(const char*) noexcept;
/*
diff --git a/src/runtime/ksecrets_store/ksecrets_store_p.h \
b/src/runtime/ksecrets_store/ksecrets_store_p.h index 25f2de0..4ec75c8 100644
--- a/src/runtime/ksecrets_store/ksecrets_store_p.h
+++ b/src/runtime/ksecrets_store/ksecrets_store_p.h
@@ -58,10 +58,9 @@ public:
KSecretsStorePrivate() = delete;
explicit KSecretsStorePrivate(KSecretsStore*);
- KSecretsStore::OpenResult lock_open(const std::string&);
- KSecretsStore::OpenResult open(const std::string&);
+ 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::OpenResult createFileIfNeededThenDo(const std::string&, bool, open_action);
int createFile(const std::string&);
const char* salt() const;
@@ -72,13 +71,25 @@ public:
char iv[IV_SIZE];
};
- KSecretsStore::OpenResult setOpenStatus(KSecretsStore::OpenResult);
- bool isOpen() const noexcept { return KSecretsStore::OpenStatus::Good == \
openStatus_.status_; }
-
+ template <typename S> S setStoreStatus(S s)
+ {
+ status_ = s.status_;
+ return s;
+ }
+ bool isOpen() const noexcept { return KSecretsStore::StoreStatus::Good == status_; }
+
+ struct SecretsFile {
+ SecretsFile()
+ : file_(-1), locked_(false) {};
+ ~SecretsFile();
+ std::string filePath_;
+ int file_;
+ bool locked_;
+ };
KSecretsStore* b_;
- FILE* file_;
+ SecretsFile secretsFile_;
FileHeadStruct fileHead_;
- KSecretsStore::OpenResult openStatus_;
+ KSecretsStore::StoreStatus status_;
};
#endif
diff --git a/src/runtime/pam-ksecrets/CMakeLists.txt b/src/runtime/pam-ksecrets/CMakeLists.txt
index 9804532..8bdcdf4 100644
--- a/src/runtime/pam-ksecrets/CMakeLists.txt
+++ b/src/runtime/pam-ksecrets/CMakeLists.txt
@@ -11,8 +11,8 @@ INCLUDE_DIRECTORIES(
${PAM_INCLUDE_DIR}
${CMAKE_BINARY_DIR}
${CMAKE_CURRENT_BINARY_DIR}
- ${CMAKE_CURRENT_SOURCE_DIR}/../ksecrets_backend
- ${CMAKE_CURRENT_BINARY_DIR}/../ksecrets_backend
+ ${CMAKE_CURRENT_SOURCE_DIR}/../ksecrets_store
+ ${CMAKE_CURRENT_BINARY_DIR}/../ksecrets_store
)
set(pam_ksecret_SRC
@@ -21,10 +21,10 @@ set(pam_ksecret_SRC
add_library(pam_ksecrets STATIC ${pam_ksecret_SRC})
set_target_properties(pam_ksecrets PROPERTIES PREFIX "")
target_link_libraries(pam_ksecrets
- ksecrets_backend
+ ksecrets_store
${LIBGCRYPT_LIBRARIES}
${PAM_LIBRARIES}
- keyutils)
+ )
install(TARGETS pam_ksecrets DESTINATION /${CMAKE_INSTALL_LIBDIR}/security)
diff --git a/src/runtime/pam-ksecrets/pam_ksecrets.c b/src/runtime/pam-ksecrets/pam_ksecrets.c
index 144e22c..eaf8f21 100644
--- a/src/runtime/pam-ksecrets/pam_ksecrets.c
+++ b/src/runtime/pam-ksecrets/pam_ksecrets.c
@@ -3,17 +3,23 @@
#define PAM_SM_SESSION
#define PAM_SM_PASSWORD
+#include <ksecrets_credentials.h>
+
#include <syslog.h>
#include <security/pam_ext.h>
#include <security/pam_appl.h>
#include <security/pam_modules.h>
-#include "ksecrets-crypt.h"
-
#define UNUSED(x) (void)(x)
const char* password;
+/* these extern functions are implemented in ksecrets_store_bridge.cpp */
+extern int kss_set_credentials(const char*, const char*);
+extern int kss_delete_credentials();
+extern int kss_can_change_password();
+extern int kss_change_password(const char*);
+
PAM_EXTERN int pam_sm_authenticate(
pam_handle_t* pamh, int flags, int argc, const char** argv)
{
@@ -93,7 +99,7 @@ PAM_EXTERN int pam_sm_chauthtok(
if (flags & PAM_PRELIM_CHECK) {
pam_syslog(pamh, LOG_INFO, "pam_sm_chauthtok preliminary check");
- if (kss_can_change_password(pamh)) {
+ if (kss_can_change_password()) {
return PAM_SUCCESS;
}
else {
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic