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