[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-commits
Subject: [ksecrets] /: Testing frame now in place
From: Valentin Rusu <kde () rusu ! info>
Date: 2015-08-04 9:45:52
Message-ID: E1ZMYnA-0000yO-9u () scm ! kde ! org
[Download RAW message or body]
Git commit 2c667bb50ef2898357b614016bbe6e3b84cab263 by Valentin Rusu.
Committed on 04/08/2015 at 09:45.
Pushed by vrusu into branch 'master'.
Testing frame now in place
M +3 -1 autotests/api/CMakeLists.txt
A +20 -0 autotests/api/README.md
M +69 -1 autotests/api/ksecretsservice-test.cpp
M +3 -0 autotests/api/ksecretsservice-test.h
A +9 -0 doc/api/notes.adoc
M +0 -1 src/api/ksecrets/ksecretscollection.cpp
M +0 -1 src/api/ksecrets/ksecretsitem.cpp
A +74 -0 src/runtime/ksecrets-crypt/config.cpp [License: LGPL (v2+)]
M +52 -40 src/runtime/ksecrets-crypt/ksecrets-crypt.c
M +9 -1 src/runtime/ksecrets-crypt/ksecrets-crypt.h
M +1 -1 src/runtime/pam-ksecrets/CMakeLists.txt
http://commits.kde.org/ksecrets/2c667bb50ef2898357b614016bbe6e3b84cab263
diff --git a/autotests/api/CMakeLists.txt b/autotests/api/CMakeLists.txt
index 910a0b8..071cb72 100644
--- a/autotests/api/CMakeLists.txt
+++ b/autotests/api/CMakeLists.txt
@@ -4,6 +4,8 @@ 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-crypt)
if(NOT Qt5Test_FOUND)
@@ -15,6 +17,6 @@ find_package(KF5 REQUIRED CoreAddons)
ecm_add_test(
ksecretsservice-test.cpp
- LINK_LIBRARIES KF5::Secrets Qt5::Test KF5::Secrets KF5::CoreAddons
+ LINK_LIBRARIES KF5::Secrets Qt5::Test KF5::Secrets KF5::CoreAddons ksecrets_crypt \
${LIBGCRYPT_LIBRARIES} keyutils )
diff --git a/autotests/api/README.md b/autotests/api/README.md
new file mode 100644
index 0000000..c510e0a
--- /dev/null
+++ b/autotests/api/README.md
@@ -0,0 +1,20 @@
+= Setting-up KSecrets Autotests
+
+KSecrets service needs decrpytion keys to be present into the current kernel
+keyring. These keys are normally put there by the pam_ksecrets module.
+That module won't be present in a test environment. However, the underlying
+code is able to function into a test environment. Just follow this steps:
+
+. Create a test user into the test system. That's because the tests use
+/etc/passwd information. This test user should also have a valid home
+directory where the CI user could write. For example, let the name of the user
+be "ksstest"
+
+. Define a password for that user. For example, let it be "ksstest".
+
+. Launch the test
+
+ su - ksstest -c "export KSS_TEST_PASSWORD=ksstest && \
$PWD/autotests/api/ksecretsservice-test" +
+. That's it!
+
diff --git a/autotests/api/ksecretsservice-test.cpp b/autotests/api/ksecretsservice-test.cpp
index de29c3e..981967d 100644
--- a/autotests/api/ksecretsservice-test.cpp
+++ b/autotests/api/ksecretsservice-test.cpp
@@ -26,10 +26,17 @@
#include <ksecretsitem.h>
#include <ksecrets-crypt.h>
+#include <QtCore/QtGlobal>
#include <QtTest/QTest>
#include <QDebug>
+#include <ksharedconfig.h>
+#include <kconfiggroup.h>
-QTEST_MAIN(KSecretServiceTest)
+extern "C" {
+#include <keyutils.h>
+}
+
+QTEST_GUILESS_MAIN(KSecretServiceTest)
#define TEST_CREATE_COLLECTION_NAME "kf5-create-collection"
#define TEST_COLLECTION_NAME "kf5-test-collection"
@@ -40,9 +47,19 @@ KSecretServiceTest::KSecretServiceTest(QObject* parent)
}
KSecrets::CollectionPtr collection;
+extern KSharedConfig::Ptr sharedConfig; // defined in the ksecrets_crypt library
void KSecretServiceTest::initTestCase()
{
+ /* This test should not be run under your normal session, as it'll may
+ * interact with an existing KSecrets set-up and you risk corrupting your
+ * main secrets file. On an existing session, this file is already present
+ */
+ QStandardPaths::setTestModeEnabled(true);
+ sharedConfig = KSharedConfig::openConfig(QLatin1String("ksecretsrc"));
+
+ setupPAM();
+
collection = KSecrets::Service::findCollection(
QLatin1String(TEST_CREATE_COLLECTION_NAME),
KSecrets::Service::CreateCollection).result();
@@ -109,11 +126,62 @@ void KSecretServiceTest::testItems()
QCOMPARE(theSecret->value().toString(), NEW_ITEM_VALUE);
}
+
void KSecretServiceTest::cleanupTestCase()
{
if (collection->isValid().result()) {
collection->deleteCollection();
}
+ cleanupPAM();
+
+ QString configPath = QStandardPaths::locate(
+ QStandardPaths::ConfigLocation,
+ sharedConfig->name());
+ qDebug() << "removing config file " << configPath;
+ QFile(configPath).remove();
+}
+
+
+void KSecretServiceTest::setupPAM()
+{
+ const char* KSS_TEST_USER = "USER";
+ QVERIFY(qEnvironmentVariableIsSet(KSS_TEST_USER));
+ QVERIFY(!qEnvironmentVariableIsEmpty(KSS_TEST_USER));
+ auto testUser = qgetenv(KSS_TEST_USER);
+
+ const char* KSS_TEST_PASSWORD = "KSS_TEST_PASSWORD";
+ QVERIFY(qEnvironmentVariableIsSet(KSS_TEST_PASSWORD));
+ QVERIFY(!qEnvironmentVariableIsEmpty(KSS_TEST_PASSWORD));
+ auto testPass = qgetenv(KSS_TEST_PASSWORD);
+
+ const char* KEYNAME_ENCRYPTING = "ksecrets-test:encrypting";
+ const char* KEYNAME_MAC = "ksecrets-test:mac";
+
+ /* second prevention measure - change keyring keynames */
+ sharedConfig->group(QLatin1String("keyring"))
+ .writeEntry("keyname_encrypt", KEYNAME_ENCRYPTING);
+ sharedConfig->group(QLatin1String("keyring"))
+ .writeEntry("keyname_mac", KEYNAME_MAC);
+
+ /* third prevention measure - the encrypting key should not be present
+ * into the kernel keyring */
+ key_serial_t key;
+ key = request_key("user", KEYNAME_ENCRYPTING, 0, KEY_SPEC_SESSION_KEYRING);
+ QVERIFY(-1 == key);
+
+ /* now go setup user's keyring */
+ QVERIFY(kss_set_credentials(testUser.constData(), testPass.constData()));
+
+ // the right keys should be present into the kernel keyring
+ key = request_key("user", KEYNAME_ENCRYPTING, 0, KEY_SPEC_SESSION_KEYRING);
+ QVERIFY(-1 != key);
+ key = request_key("user", KEYNAME_MAC, 0, KEY_SPEC_SESSION_KEYRING);
+ QVERIFY(-1 != key);
+}
+
+void KSecretServiceTest::cleanupPAM()
+{
+ QVERIFY(kss_delete_credentials());
}
#include "ksecretsservice-test.moc"
diff --git a/autotests/api/ksecretsservice-test.h b/autotests/api/ksecretsservice-test.h
index 40222ea..89e4b28 100644
--- a/autotests/api/ksecretsservice-test.h
+++ b/autotests/api/ksecretsservice-test.h
@@ -36,5 +36,8 @@ private Q_SLOTS:
void testItems();
void cleanupTestCase();
+private:
+ void setupPAM();
+ void cleanupPAM();
};
#endif // KSECRETS_TEST_H
diff --git a/doc/api/notes.adoc b/doc/api/notes.adoc
new file mode 100644
index 0000000..d983d2c
--- /dev/null
+++ b/doc/api/notes.adoc
@@ -0,0 +1,9 @@
+== Notes about the KSecrets API
+
+The communication between the API libraray and the ksecretsd should be
+encrypted, to reduce the attack surface, as the dbus traffic can be spied.
+
+The collections are already stored in a encrypted form on disk and they are
+typically handled only by one application. So, instead of decrypting the
+collection in ksecrets then re-encrypt them for dbus forwarding, the API will
+obtain the entire raw-encrypted collection and will handle it directly.
diff --git a/src/api/ksecrets/ksecretscollection.cpp b/src/api/ksecrets/ksecretscollection.cpp
index a579a31..f9b0b8c 100644
--- a/src/api/ksecrets/ksecretscollection.cpp
+++ b/src/api/ksecrets/ksecretscollection.cpp
@@ -21,7 +21,6 @@
#include "ksecretscollection.h"
#include "ksecretscollection_p.h"
-#include "collection_interface.h"
#include <QDateTime>
#include <QtDBus/QDBusPendingReply>
diff --git a/src/api/ksecrets/ksecretsitem.cpp b/src/api/ksecrets/ksecretsitem.cpp
index 008cd9f..d41dd1a 100644
--- a/src/api/ksecrets/ksecretsitem.cpp
+++ b/src/api/ksecrets/ksecretsitem.cpp
@@ -21,7 +21,6 @@
#include "ksecretsitem.h"
#include "ksecretsitem_p.h"
#include "ksecretsserviceitemjobs.h"
-#include "item_interface.h"
#include <QDateTime>
#include <QtConcurrent>
diff --git a/src/runtime/ksecrets-crypt/config.cpp b/src/runtime/ksecrets-crypt/config.cpp
new file mode 100644
index 0000000..e155038
--- /dev/null
+++ b/src/runtime/ksecrets-crypt/config.cpp
@@ -0,0 +1,74 @@
+/*
+ This file is part of the KDE Libraries
+
+ Copyright (C) 2015 Valentin Rusu (valir@kde.org)
+
+ This library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Library General Public
+ License as published by the Free Software Foundation; either
+ version 2 of the License, or (at your option) any later version.
+
+ This library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Library General Public License for more details.
+
+ You should have received a copy of the GNU Library General Public License
+ along with this library; see the file COPYING.LIB. If not, write to
+ the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
+ Boston, MA 02110-1301, USA.
+*/
+
+#include <ksharedconfig.h>
+#include <kconfiggroup.h>
+#include <QtCore/QDir>
+#include <QtCore/QFileInfo>
+#include <QtCore/QLoggingCategory>
+
+QLoggingCategory logCat("pam_ksecrets");
+KSharedConfigPtr sharedConfig;
+
+#define CONFIGNAME "ksecretsrc"
+
+extern "C" {
+const char* prepare_secret_file_location(const char* home_dir)
+{
+ qCDebug(logCat) << "prepare_secret_file_location(" << home_dir << ")";
+ const char* defaultDataPath = QStandardPaths::isTestModeEnabled() ? \
".qttest/ksecrets.data" : ".config/kde.org/ksecrets.data"; + sharedConfig = \
KSharedConfig::openConfig(QLatin1String(CONFIGNAME)); + QString secretsLocation = \
sharedConfig->group(QLatin1String("General")).readEntry("SecretsPath", defaultDataPath); +
+ QDir userDir(home_dir); // reasonably assume user dir always exist
+ QString secretsPath = userDir.absoluteFilePath(secretsLocation);
+ QFileInfo secretsInfo(secretsPath);
+ if (!secretsInfo.exists()) {
+ qCDebug(logCat) << "secrets file not found, attempting parent directory creation";
+ QString basePath = secretsInfo.dir().absolutePath();
+ if (!userDir.mkpath(basePath)) {
+ qCDebug(logCat) << "error attempting path creation: " << basePath << " error=" << \
errno << " (" << strerror(errno) << ")"; + return NULL;
+ }
+ }
+
+ return secretsPath.toUtf8().constData();
+}
+
+const char* get_keyname_encrypting()
+{
+ return sharedConfig
+ ->group(QLatin1String("keyring"))
+ .readEntry("keyname_encrypt", "ksecrets:encrypting")
+ .toUtf8()
+ .constData();
+}
+
+const char* get_keyname_mac()
+{
+ return sharedConfig
+ ->group(QLatin1String("keyring"))
+ .readEntry("keyname_mac", "ksecrets:mac")
+ .toUtf8()
+ .constData();
+}
+}
+
diff --git a/src/runtime/ksecrets-crypt/ksecrets-crypt.c \
b/src/runtime/ksecrets-crypt/ksecrets-crypt.c index ee390cd..0da2007 100644
--- a/src/runtime/ksecrets-crypt/ksecrets-crypt.c
+++ b/src/runtime/ksecrets-crypt/ksecrets-crypt.c
@@ -43,10 +43,10 @@
#define KSECRETS_KEYSIZE 256
#define KSECRETS_ITERATIONS 50000
-#define KSS_KEY_TYPE_ENCRYPT "ksecrets:encrypting"
-#define KSS_KEY_TYPE_MAC "ksecrets:mac"
-
+/* 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();
bool kss_init_gcry()
{
@@ -88,7 +88,7 @@ bool kss_get_salt(const char* username, char** salt)
struct passwd* user_info;
user_info = getpwnam(username);
if (!user_info) {
- syslog(KSS_LOG_ERR, "pam_kwallet: Couldn't get user info (passwd) info");
+ syslog(KSS_LOG_ERR, "pam_kwallet: Couldn't get user %s info", username);
return false;
}
@@ -99,20 +99,21 @@ bool kss_get_salt(const char* username, char** salt)
}
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 */
- const char* dir = dirname((char*)path);
-
*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-%s", path,
- errno, strerror(errno));
+ syslog(KSS_LOG_ERR, "Couldn't open file: %s because: %d (%m)", path, errno);
return false;
}
@@ -120,8 +121,7 @@ bool kss_get_salt(const char* username, char** salt)
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");
+ 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);
@@ -129,8 +129,7 @@ bool kss_get_salt(const char* username, char** salt)
else {
FILE* fd = fopen(path, "r");
if (fd == NULL) {
- syslog(KSS_LOG_ERR, "Couldn't open file: %s because: %d-%s", path,
- errno, strerror(errno));
+ syslog(KSS_LOG_ERR, "Couldn't open file: %s because: %d (%m)", path, errno);
return 1;
}
*salt = (char*)malloc(sizeof(char) * KSECRETS_SALTSIZE);
@@ -146,15 +145,13 @@ bool kss_get_salt(const char* username, char** salt)
return true;
}
-bool kss_derive_keys(const char* user_name, const char* password,
- char* encryption_key, char* mac_key)
+bool kss_derive_keys(const char* user_name, const char* password, char* encryption_key, char* \
mac_key) {
gpg_error_t gcryerr;
syslog(KSS_LOG_INFO, "kss_set_credentials: attempting keys generation");
if (0 == password) {
- syslog(
- KSS_LOG_INFO, "NULL password given. ksecrets will not be available.");
+ syslog(KSS_LOG_INFO, "NULL password given. ksecrets will not be available.");
return false;
}
@@ -168,19 +165,15 @@ bool kss_derive_keys(const char* user_name, const char* password,
/* 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);
+ gcryerr = gcry_kdf_derive(password, strlen(password), GCRY_KDF_ITERSALTED_S2K, \
GCRY_MD_SHA512, salt, 8, KSECRETS_ITERATIONS, 2 * KSECRETS_KEYSIZE, keys); if (gcryerr) {
- syslog(KSS_LOG_ERR, "key derivation failed: code 0x%0x: %s/%s", gcryerr,
- gcry_strsource(gcryerr), gcry_strerror(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);
- syslog(KSS_LOG_INFO,
- "successuflly generated ksecrets keys from user password.");
+ syslog(KSS_LOG_INFO, "successuflly generated ksecrets keys from user password.");
return true;
}
@@ -188,35 +181,30 @@ bool kss_derive_keys(const char* user_name, const char* password,
bool kss_store_keys(const char* encryption_key, const char* mac_key)
{
key_serial_t ks;
- ks = add_key("user", KSS_KEY_TYPE_ENCRYPT, encryption_key, KSECRETS_KEYSIZE,
- KEY_SPEC_SESSION_KEYRING);
+ const char* key_name = get_keyname_encrypting();
+ ks = add_key("user", key_name, encryption_key, KSECRETS_KEYSIZE, \
KEY_SPEC_SESSION_KEYRING); if (-1 == ks) {
- syslog(KSS_LOG_ERR,
- "ksecrets: cannot store encryption key in kernel keyring: errno=%d",
- errno);
+ syslog(KSS_LOG_ERR, "ksecrets: cannot store encryption key in kernel keyring: errno=%d \
(%m)", errno); return false;
}
- syslog(KSS_LOG_DEBUG,
- "ksecrets: encrpyting key now in kernel keyring with id %d", ks);
+ syslog(KSS_LOG_DEBUG, "ksecrets: encrpyting key now in kernel keyring with id %d and desc \
%s", ks, key_name);
- ks = add_key("user", KSS_KEY_TYPE_MAC, mac_key, KSECRETS_KEYSIZE,
- KEY_SPEC_SESSION_KEYRING);
+ key_name = get_keyname_mac();
+ ks = add_key("user", key_name, mac_key, KSECRETS_KEYSIZE, KEY_SPEC_SESSION_KEYRING);
if (-1 == ks) {
- syslog(KSS_LOG_ERR,
- "ksecrets: cannot store mac key in kernel keyring: errno=%d", errno);
+ syslog(KSS_LOG_ERR, "ksecrets: cannot store mac key in kernel keyring: errno=%d (%m)", \
errno); return false;
}
- syslog(KSS_LOG_DEBUG, "ksecrets: mac key now in kernel keyring with id %d",
- ks);
+ syslog(KSS_LOG_DEBUG, "ksecrets: mac key now in kernel keyring with id %d and desc %s", \
ks, key_name); return true;
}
bool kss_keys_already_there()
{
key_serial_t key;
- key = request_key(KSS_KEY_TYPE_ENCRYPT, 0, 0, KEY_SPEC_SESSION_KEYRING);
+ key = request_key("user", get_keyname_encrypting(), 0, KEY_SPEC_SESSION_KEYRING);
if (-1 == key) {
- syslog(KSS_LOG_DEBUG, "request_key failed with errno %d", errno);
+ syslog(KSS_LOG_DEBUG, "request_key failed with errno %d (%m), so assuming ksecrets not \
yet loaded", errno); return false;
}
syslog(KSS_LOG_DEBUG, "ksecrets: keys already in keyring");
@@ -225,6 +213,7 @@ bool kss_keys_already_there()
bool 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;
@@ -239,9 +228,32 @@ bool kss_set_credentials(const char* user_name, const char* password)
return true;
}
-void kss_delete_credentials()
+bool kss_delete_credentials()
{
syslog(KSS_LOG_INFO, "kss_delete_credentials");
+ key_serial_t key;
+ key = request_key("user", get_keyname_encrypting(), 0, KEY_SPEC_SESSION_KEYRING);
+ if (-1 == key) {
+ syslog(KSS_LOG_DEBUG, "request_key failed with errno %d (%m), cannot purge encrypting \
key", errno); + return false;
+ }
+ long res = keyctl(KEYCTL_REVOKE, key);
+ if (-1 == res) {
+ syslog(KSS_LOG_DEBUG, "removing key failed with errno %d (%m), cannot purge encrypting \
key", errno); + return false;
+ }
+
+ key = request_key("user", get_keyname_mac(), 0, KEY_SPEC_SESSION_KEYRING);
+ if (-1 == key) {
+ syslog(KSS_LOG_DEBUG, "request_key failed with errno %d (%m), cannot purge mac key", \
errno); + return false;
+ }
+ res = keyctl(KEYCTL_REVOKE, key);
+ if (-1 == res) {
+ syslog(KSS_LOG_DEBUG, "removing key failed with errno %d (%m), cannot purge mac key", \
errno); + return false;
+ }
+ return true;
}
bool kss_can_change_password()
@@ -256,4 +268,4 @@ bool kss_change_password(const char* new_password)
syslog(LOG_INFO, "kss_change_password");
return true;
}
-
+/* vim:tw=220:ts=4 */
diff --git a/src/runtime/ksecrets-crypt/ksecrets-crypt.h \
b/src/runtime/ksecrets-crypt/ksecrets-crypt.h index 1205c01..8e743d8 100644
--- a/src/runtime/ksecrets-crypt/ksecrets-crypt.h
+++ b/src/runtime/ksecrets-crypt/ksecrets-crypt.h
@@ -23,12 +23,20 @@
#include <stdbool.h>
+#ifdef __cplusplus
+extern "C" {
+#endif
+
bool kss_set_credentials(const char* user_name, const char* password);
-void kss_delete_credentials();
+bool kss_delete_credentials();
bool kss_can_change_password();
bool kss_change_password(const char* newPassword);
+#ifdef __cplusplus
+}
+#endif
+
#endif
diff --git a/src/runtime/pam-ksecrets/CMakeLists.txt b/src/runtime/pam-ksecrets/CMakeLists.txt
index b1e8a50..7c0b71c 100644
--- a/src/runtime/pam-ksecrets/CMakeLists.txt
+++ b/src/runtime/pam-ksecrets/CMakeLists.txt
@@ -17,7 +17,7 @@ INCLUDE_DIRECTORIES(
set(pam_ksecret_SRC
pam_ksecrets.c)
-add_library(pam_ksecrets SHARED ${pam_ksecret_SRC})
+add_library(pam_ksecrets STATIC ${pam_ksecret_SRC})
set_target_properties(pam_ksecrets PROPERTIES PREFIX "")
target_link_libraries(pam_ksecrets
ksecrets_crypt
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic