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 Te= st) = 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 kse= crets_crypt ${LIBGCRYPT_LIBRARIES} keyutils + LINK_LIBRARIES KF5::Secrets Qt5::Test KF5::Secrets KF5::CoreAddons kse= crets_store ${LIBGCRYPT_LIBRARIES} keyutils ) = diff --git a/autotests/api/ksecretsservice-test.cpp b/autotests/api/ksecret= sservice-test.cpp index efa0788..c16c424 100644 --- a/autotests/api/ksecretsservice-test.cpp +++ b/autotests/api/ksecretsservice-test.cpp @@ -24,7 +24,7 @@ #include #include #include -#include +#include = #include #include @@ -47,7 +47,7 @@ KSecretServiceTest::KSecretServiceTest(QObject* parent) } = KSecrets::CollectionPtr collection; -extern KSharedConfig::Ptr sharedConfig; // defined in the ksecrets_crypt l= ibrary +KSharedConfig::Ptr sharedConfig; = void KSecretServiceTest::initTestCase() { diff --git a/autotests/ksecrets_store/ksecrets_store_test.cpp b/autotests/k= secrets_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 +#include #include #include = @@ -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 =3D backend.setup(test_file_path, "test"); } = void KSecretServiceStoreTest::cleanupTestCase() @@ -46,7 +49,8 @@ void KSecretServiceStoreTest::cleanupTestCase() void KSecretServiceStoreTest::testOpen() { KSecretsStore backend; - auto openfut =3D backend.open(test_file_path, true); + auto setupfut =3D backend.setup(test_file_path, "test"); + auto openfut =3D backend.open(true); auto openres =3D openfut.get(); - QVERIFY(openres.status_ =3D=3D KSecretsStore::OpenStatus::Good); + QVERIFY(openres.status_ =3D=3D KSecretsStore::StoreStatus::Good); } diff --git a/src/api/ksecrets/ksecretsitem.cpp b/src/api/ksecrets/ksecretsi= tem.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-c= rypt/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 +#include = #include #include @@ -80,9 +80,9 @@ const char* get_keyname_mac() bool get_salt(const char* path, char* buf, size_t len) { bool res=3D false; - KSecretsBackend backend; - auto openfut =3D backend.open(path); - if (openfut.get().status_ =3D=3D KSecretsBackend::OpenStatus::Good) { + KSecretsStore store; + auto openfut =3D store.open(path); + if (openfut.get().status_ =3D=3D KSecretsStore::OpenStatus::Good) { = } return res; diff --git a/src/runtime/ksecrets_store/CMakeLists.txt b/src/runtime/ksecre= ts_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}/KF5Sec= retsStoreConfigVersion.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/ksec= rets_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 #include @@ -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 =3D gcry_control(GCRYCTL_INIT_SECMEM, 32768, 0); if (gcryerr !=3D 0) { - syslog(KSS_LOG_ERR, "ksecrets: cannot get secure memory: %d", gcry= err); - 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 cr= eate - * 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 =3D=3D username) { - syslog(KSS_LOG_ERR, "no username given, salt could not been retrie= ved"); - return false; - } - - struct passwd* user_info; - user_info =3D getpwnam(username); - if (!user_info) { - syslog(KSS_LOG_ERR, "pam_kwallet: Couldn't get user %s info", user= name); - return false; - } - - struct stat info; - if (stat(user_info->pw_dir, &info) !=3D 0) { - syslog(KSS_LOG_ERR, "pam_kwallet: Cannot stat user directory"); - return false; - } - - const char* path =3D prepare_secret_file_location(user_info->pw_dir); - if (NULL =3D=3D path) { - syslog(KSS_LOG_ERR, "pam_kwallet: Cannot prepare secret file locat= ion"); - return false; - } - - *salt =3D NULL; - if (stat(path, &info) !=3D 0 || info.st_size =3D=3D 0) { - unlink(path); /* in case the file already exists and it has size o= f 0 */ - - *salt =3D gcry_random_bytes(KSECRETS_SALTSIZE, GCRY_STRONG_RANDOM); - FILE* fd =3D fopen(path, "w"); - - /* If the file can't be created */ - if (fd =3D=3D 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) =3D=3D -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", p= ath); - } - else { - FILE* fd =3D fopen(path, "r"); - if (fd =3D=3D NULL) { - syslog(KSS_LOG_ERR, "Couldn't open file: %s because: %d (%m)",= path, errno); - return 1; - } - *salt =3D (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 =3D=3D NULL) { - syslog(KSS_LOG_ERR, "ksecrets: Couldn't create or read the salt fi= le"); - return false; - } - return true; -} - -bool kss_derive_keys(const char* user_name, const char* password, char* en= cryption_key, char* mac_key) +int kss_derive_keys(const char* salt, const char* password, char* encrypti= on_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 =3D 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 =3D gcry_kdf_derive(password, strlen(password), GCRY_KDF_ITERS= ALTED_S2K, GCRY_MD_SHA512, salt, 8, KSECRETS_ITERATIONS, 2 * KSECRETS_KEYSI= ZE, keys); + char keys[2 * keySize]; + gcryerr =3D gcry_kdf_derive(password, strlen(password), GCRY_KDF_ITERS= ALTED_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", gc= ryerr, 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 p= assword."); = 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 =3D get_keyname_encrypting(); - ks =3D add_key("user", key_name, encryption_key, KSECRETS_KEYSIZE, KEY= _SPEC_SESSION_KEYRING); + ks =3D add_key("user", key_name, encryption_key, keySize, KEY_SPEC_SES= SION_KEYRING); if (-1 =3D=3D ks) { syslog(KSS_LOG_ERR, "ksecrets: cannot store encryption key in kern= el keyring: errno=3D%d (%m)", errno); return false; @@ -191,7 +106,7 @@ bool kss_store_keys(const char* encryption_key, const c= har* mac_key) syslog(KSS_LOG_DEBUG, "ksecrets: encrpyting key now in kernel keyring = with id %d and desc %s", ks, key_name); = key_name =3D get_keyname_mac(); - ks =3D add_key("user", key_name, mac_key, KSECRETS_KEYSIZE, KEY_SPEC_S= ESSION_KEYRING); + ks =3D add_key("user", key_name, mac_key, keySize, KEY_SPEC_SESSION_KE= YRING); if (-1 =3D=3D ks) { syslog(KSS_LOG_ERR, "ksecrets: cannot store mac key in kernel keyr= ing: errno=3D%d (%m)", errno); return false; @@ -200,7 +115,7 @@ bool kss_store_keys(const char* encryption_key, const c= har* mac_key) return true; } = -bool kss_keys_already_there() +int kss_keys_already_there() { key_serial_t key; key =3D request_key("user", get_keyname_encrypting(), 0, KEY_SPEC_SESS= ION_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/ksec= rets_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 +#include = #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/ks= ecrets_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 #include +#include #include +#include #include #include +#include #define GCRPYT_NO_DEPRECATED #include = +#define KSECRETS_SALTSIZE 56 +#define KSECRETS_KEYSIZE 256 +#define KSS_LOG_ERR (LOG_AUTH | LOG_ERR) + +const char* keyNameEncrypting =3D nullptr; +const char* keyNameMac =3D nullptr; + +extern "C" { +bool kss_init_gcry(); +bool kss_derive_keys(const char* salt, const char* password, char* encrypt= ion_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_ =3D KSecretsStore::OpenStatus::NotYetOpened; + status_ =3D KSecretsStore::StoreStatus::JustCreated; } = KSecretsStore::KSecretsStore() @@ -43,52 +61,66 @@ KSecretsStore::KSecretsStore() = KSecretsStore::~KSecretsStore() =3D default; = -std::future KSecretsStore::open(std::string&& p= ath, bool readonly /* =3Dtrue */) noexcept +std::future KSecretsStore::setup(const char* p= ath, const char* password, const char* keyNameEcrypting, const char* keyNam= eMac) { // sanity checks - if (path.empty()) { - return std::async(std::launch::deferred, []() { return OpenResult{= OpenStatus::NoPathGiven, 0 }; }); + if (d->status_ !=3D StoreStatus::JustCreated) { + return std::async(std::launch::deferred, []() { return SetupResult= { StoreStatus::IncorrectState, -1 }; }); + } + if (path =3D=3D nullptr || strlen(path) =3D=3D 0) { + return std::async(std::launch::deferred, []() { return SetupResult= { StoreStatus::NoPathGiven, 0 }; }); } = bool shouldCreateFile =3D false; struct stat buf; - if (stat(path.c_str(), &buf) !=3D 0) { + if (stat(path, &buf) !=3D 0) { auto err =3D errno; if (ENOENT =3D=3D err) { shouldCreateFile =3D true; } else { - return std::async(std::launch::deferred, [err]() { return Open= Result{ OpenStatus::SystemError, errno }; }); + return std::async(std::launch::deferred, [err]() { return Setu= pResult{ StoreStatus::SystemError, errno }; }); } } else { if (buf.st_size =3D=3D 0) { - unlink(path.c_str()); + unlink(path); shouldCreateFile =3D true; // recreate if empty file was found } } = - // now we can proceed + ::keyNameEncrypting =3D keyNameEcrypting; + ::keyNameMac =3D keyNameMac; + auto localThis =3D this; - if (!readonly) { - return std::async( - std::launch::async, [localThis, path, shouldCreateFile]() { re= turn localThis->d->createFileIfNeededThenDo(path, shouldCreateFile, std::bi= nd(&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 =3D path; + std::string pwd =3D password; + return std::async(std::launch::async, [localThis, filePath, shouldCrea= teFile, pwd]() { return localThis->d->setup(filePath, shouldCreateFile, pwd= ); }); } = -KSecretsStore::OpenResult KSecretsStorePrivate::createFileIfNeededThenDo(c= onst 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 =3D createFile(path); - if (createRes !=3D 0) { - return setOpenStatus({ KSecretsStore::OpenStatus::SystemError,= createRes }); + auto createres =3D createFile(path); + if (createres !=3D 0) { + return setStoreStatus(KSecretsStore::SetupResult({ KSecretsSto= re::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_ =3D path; + return setStoreStatus(KSecretsStore::SetupResult({ KSecretsStore::Stor= eStatus::SetupDone, 0 })); } = char fileMagic[] =3D { 'k', 's', 'e', 'c', 'r', 'e', 't', 's' }; @@ -114,42 +146,63 @@ int KSecretsStorePrivate::createFile(const std::strin= g& 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::strin= g& path) +std::future KSecretsStore::open(bool readonly /= * =3Dtrue */) noexcept { - file_ =3D fopen(path.c_str(), "w+"); - if (file_ =3D=3D nullptr) { - return { KSecretsStore::OpenStatus::SystemError, errno }; + // check the state first + if (d->status_ =3D=3D StoreStatus::JustCreated) { + return std::async(std::launch::deferred, []() { return OpenResult{= StoreStatus::SetupShouldBeCalledFirst, -1 }; }); + } + if (d->status_ !=3D StoreStatus::SetupDone) { + // open could not be called two times or perhaps the setup call le= ft 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 =3D 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(KSecretsStor= e::OpenResult openStatus) -{ - openStatus_ =3D openStatus; - return openStatus; -} +const char* KSecretsStore::salt() const { return d->salt(); } = -KSecretsStore::OpenResult KSecretsStorePrivate::open(const std::string& pa= th) +const char* KSecretsStorePrivate::salt() const { return fileHead_.salt; } + +KSecretsStore::OpenResult KSecretsStorePrivate::open(bool lockFile) { - if (file_ =3D=3D nullptr) { - file_ =3D fopen(path.c_str(), "r"); + using OpenResult =3D KSecretsStore::OpenResult; + secretsFile_.file_ =3D ::open(secretsFile_.filePath_.c_str(), O_DSYNC = | O_NOATIME | O_NOFOLLOW); + if (secretsFile_.file_ =3D=3D -1) { + return setStoreStatus(OpenResult({ KSecretsStore::StoreStatus::Can= notOpenFile, errno })); } - if (file_ =3D=3D nullptr) { - return setOpenStatus({ KSecretsStore::OpenStatus::SystemError, err= no }); + if (lockFile) { + if (flock(secretsFile_.file_, LOCK_EX) =3D=3D -1) { + return KSecretsStore::OpenResult{ KSecretsStore::StoreStatus::= CannotLockFile, errno }; + } + secretsFile_.locked_ =3D true; } - if (fread(&fileHead_, sizeof(fileHead_), 1, file_) !=3D sizeof(fileHea= d_)) { - return setOpenStatus({ KSecretsStore::OpenStatus::SystemError, fer= ror(file_) }); + auto r =3D read(secretsFile_.file_, &fileHead_, sizeof(fileHead_)); + if (r =3D=3D -1) { + return setStoreStatus(OpenResult({ KSecretsStore::StoreStatus::Can= notReadFile, errno })); } - if (memcmp(fileHead_.magic, fileMagic, fileMagicLen) !=3D 0) { - return setOpenStatus({ KSecretsStore::OpenStatus::InvalidFile, 0 }= ); + if ((size_t)r < sizeof(fileHead_) || memcmp(fileHead_.magic, fileMagic= , fileMagicLen) !=3D 0) { + return setStoreStatus(OpenResult({ KSecretsStore::StoreStatus::Inv= alidFile, -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_ !=3D -1) { + auto r =3D close(file_); + if (r =3D=3D -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 c= orrup because of the previous error"); + } + file_ =3D -1; + } } = KSecretsStore::CollectionNames KSecretsStore::dirCollections() const noexc= ept @@ -158,13 +211,13 @@ KSecretsStore::CollectionNames KSecretsStore::dirColl= ections() 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*) co= nst noexcept { // TODO return CollectionPtr(); @@ -176,7 +229,7 @@ bool KSecretsStore::deleteCollection(CollectionPtr) noe= xcept 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/ksec= rets_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 +#include = #include #include @@ -45,7 +45,7 @@ class KSecretsStorePrivate; * with each API call. * That guarantees client applications that edits are always synced to dis= k/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 It= ems into it. * The Item class holds, sure enough, the secret value but also let applic= ations associate * the secret value with metadata, such as the label or other custom prope= rties. @@ -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 effic= ient than + * initializing en empty than setting the label, eventually the pa= rameters and + * the value. The returned item may still be modified, keep in min= d 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 us= ing * 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 oth= er * methods of this API. * @@ -181,28 +186,56 @@ public: KSecretsStore(const KSecretsStore&) =3D 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 ksec= rets file? - FileLocked, + CannotOpenFile, + CannotLockFile, + CannotReadFile, SystemError }; + + struct SetupResult { + StoreStatus status_; + int errno_; + operator bool() const { return status_ =3D=3D StoreStatus::SetupDo= ne; } + }; + + /* + * 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 setup(const char* path, const char* password,= const char* keyNameEcrypting =3D "ksecrets:encrypting", const char* keyNam= eMac =3D "ksecrets:mac"); + struct OpenResult { - OpenStatus status_; + StoreStatus status_; int errno_; + operator bool() const { return status_ =3D=3D 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 sp= eed - * things-up + * things-up and avoid placing a lock on the file + * + * @return OpenResult whose operator bool could be used to check the e= rror condition */ - std::future open(const char* path, bool readOnly =3D true)= noexcept; + std::future open(bool readOnly =3D true) noexcept; = constexpr static auto SALT_SIZE =3D 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/ks= ecrets_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() =3D 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 =3D std::function; - 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 = =3D=3D openStatus_.status_; } - + template S setStoreStatus(S s) + { + status_ =3D s.status_; + return s; + } + bool isOpen() const noexcept { return KSecretsStore::StoreStatus::Good= =3D=3D 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-ksec= rets/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-ksec= rets/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 + #include #include #include #include = -#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 {