From a7e7f7fdd2a9d38921476d57f305c9cd4459a556 Mon Sep 17 00:00:00 2001 From: Christian Mollekopf Date: Mon, 18 Sep 2017 11:40:41 +0200 Subject: Avoid storing the password in the configuration The password (or any other secret), is now cached in the client process (in-memory only), and delivered to the resource via command. The resource avoids doing any operations against the source until the secret is available. --- common/CMakeLists.txt | 3 +++ common/commands.h | 1 + common/commands/secret.fbs | 7 +++++ common/domain/applicationdomaintype.h | 1 + common/genericresource.cpp | 7 +++++ common/genericresource.h | 2 ++ common/listener.cpp | 12 +++++++++ common/resource.cpp | 5 ++++ common/resource.h | 2 ++ common/resourceaccess.cpp | 22 +++++++++++++++ common/resourceaccess.h | 6 +++++ common/secretstore.cpp | 51 +++++++++++++++++++++++++++++++++++ common/secretstore.h | 50 ++++++++++++++++++++++++++++++++++ common/synchronizer.cpp | 18 +++++++++++++ common/synchronizer.h | 5 ++++ 15 files changed, 192 insertions(+) create mode 100644 common/commands/secret.fbs create mode 100644 common/secretstore.cpp create mode 100644 common/secretstore.h (limited to 'common') diff --git a/common/CMakeLists.txt b/common/CMakeLists.txt index 8421fc2..b0e0d04 100644 --- a/common/CMakeLists.txt +++ b/common/CMakeLists.txt @@ -33,6 +33,7 @@ set(storage_LIBS lmdb) set(command_SRCS store.cpp + secretstore.cpp notifier.cpp resourcecontrol.cpp modelresult.cpp @@ -97,6 +98,7 @@ generate_flatbuffers( commands/revisionreplayed commands/inspection commands/flush + commands/secret domain/contact domain/addressbook domain/event @@ -144,6 +146,7 @@ install(FILES test.h log.h flush.h + secretstore.h ${CMAKE_CURRENT_BINARY_DIR}/sink_export.h DESTINATION ${INCLUDE_INSTALL_DIR}/${PROJECT_NAME} COMPONENT Devel ) diff --git a/common/commands.h b/common/commands.h index 1548eac..8ced268 100644 --- a/common/commands.h +++ b/common/commands.h @@ -48,6 +48,7 @@ enum CommandIds InspectionCommand, RemoveFromDiskCommand, FlushCommand, + SecretCommand, CustomCommand = 0xffff }; diff --git a/common/commands/secret.fbs b/common/commands/secret.fbs new file mode 100644 index 0000000..5bb2246 --- /dev/null +++ b/common/commands/secret.fbs @@ -0,0 +1,7 @@ +namespace Sink.Commands; + +table Secret { + secret: string; +} + +root_type Secret; diff --git a/common/domain/applicationdomaintype.h b/common/domain/applicationdomaintype.h index f7fd07e..de2b5c9 100644 --- a/common/domain/applicationdomaintype.h +++ b/common/domain/applicationdomaintype.h @@ -102,6 +102,7 @@ enum SINK_EXPORT ErrorCode { ConfigurationError, TransmissionError, ConnectionLostError, + MissingCredentialsError }; enum SINK_EXPORT SuccessCode { diff --git a/common/genericresource.cpp b/common/genericresource.cpp index 7178b3d..b6a8f62 100644 --- a/common/genericresource.cpp +++ b/common/genericresource.cpp @@ -45,6 +45,13 @@ GenericResource::~GenericResource() { } +void GenericResource::setSecret(const QString &s) +{ + if (mSynchronizer) { + mSynchronizer->setSecret(s); + } +} + void GenericResource::setupPreprocessors(const QByteArray &type, const QVector &preprocessors) { mPipeline->setPreprocessors(type, preprocessors); diff --git a/common/genericresource.h b/common/genericresource.h index bffc697..4e86b75 100644 --- a/common/genericresource.h +++ b/common/genericresource.h @@ -47,6 +47,8 @@ public: static void removeFromDisk(const QByteArray &instanceIdentifier); static qint64 diskUsage(const QByteArray &instanceIdentifier); + virtual void setSecret(const QString &s) Q_DECL_OVERRIDE; + //TODO Remove this API, it's only used in tests KAsync::Job synchronizeWithSource(const Sink::QueryBase &query); //TODO Remove this API, it's only used in tests diff --git a/common/listener.cpp b/common/listener.cpp index c9fd9d3..c6747cd 100644 --- a/common/listener.cpp +++ b/common/listener.cpp @@ -33,6 +33,7 @@ #include "common/revisionupdate_generated.h" #include "common/notification_generated.h" #include "common/revisionreplayed_generated.h" +#include "common/secret_generated.h" #include #include @@ -240,6 +241,17 @@ void Listener::processCommand(int commandId, uint messageId, const QByteArray &c } break; } + case Sink::Commands::SecretCommand: { + flatbuffers::Verifier verifier((const uint8_t *)commandBuffer.constData(), commandBuffer.size()); + if (Sink::Commands::VerifySecretBuffer(verifier)) { + auto buffer = Sink::Commands::GetSecret(commandBuffer.constData()); + loadResource().setSecret(QString{buffer->secret()->c_str()}); + } else { + SinkWarning() << "received invalid command"; + } + break; + } + case Sink::Commands::SynchronizeCommand: case Sink::Commands::InspectionCommand: case Sink::Commands::DeleteEntityCommand: diff --git a/common/resource.cpp b/common/resource.cpp index 804f0bf..78cc51b 100644 --- a/common/resource.cpp +++ b/common/resource.cpp @@ -51,6 +51,11 @@ void Resource::setLowerBoundRevision(qint64 revision) Q_UNUSED(revision) } +void Resource::setSecret(const QString &s) +{ + Q_UNUSED(s) +} + class ResourceFactory::Private { diff --git a/common/resource.h b/common/resource.h index a50ffb5..938f267 100644 --- a/common/resource.h +++ b/common/resource.h @@ -47,6 +47,8 @@ public: */ virtual void setLowerBoundRevision(qint64 revision); + virtual void setSecret(const QString &s); + signals: void revisionUpdated(qint64); void notify(Notification); diff --git a/common/resourceaccess.cpp b/common/resourceaccess.cpp index 35fa46c..5ed3609 100644 --- a/common/resourceaccess.cpp +++ b/common/resourceaccess.cpp @@ -32,9 +32,11 @@ #include "common/revisionreplayed_generated.h" #include "common/inspection_generated.h" #include "common/flush_generated.h" +#include "common/secret_generated.h" #include "common/entitybuffer.h" #include "common/bufferutils.h" #include "common/test.h" +#include "common/secretstore.h" #include "log.h" #include @@ -234,6 +236,12 @@ ResourceAccess::ResourceAccess(const QByteArray &resourceInstanceIdentifier, con { mResourceStatus = Sink::ApplicationDomain::NoStatus; SinkTrace() << "Starting access"; + + QObject::connect(&SecretStore::instance(), &SecretStore::secretAvailable, this, [this] (const QByteArray &resourceId) { + if (resourceId == d->resourceInstanceIdentifier) { + sendSecret(SecretStore::instance().resourceSecret(d->resourceInstanceIdentifier)).exec(); + } + }); } ResourceAccess::~ResourceAccess() @@ -387,6 +395,15 @@ KAsync::Job ResourceAccess::sendFlushCommand(int flushType, const QByteArr return sendCommand(Sink::Commands::FlushCommand, fbb); } +KAsync::Job ResourceAccess::sendSecret(const QString &secret) +{ + flatbuffers::FlatBufferBuilder fbb; + auto s = fbb.CreateString(secret.toStdString()); + auto location = Sink::Commands::CreateSecret(fbb, s); + Sink::Commands::FinishSecretBuffer(fbb, location); + return sendCommand(Sink::Commands::SecretCommand, fbb); +} + void ResourceAccess::open() { if (d->socket && d->socket->isValid()) { @@ -483,6 +500,11 @@ void ResourceAccess::connected() Commands::write(d->socket.data(), ++d->messageId, Commands::HandshakeCommand, fbb); } + auto secret = SecretStore::instance().resourceSecret(d->resourceInstanceIdentifier); + if (!secret.isEmpty()) { + sendSecret(secret).exec(); + } + // Reenqueue pending commands, we failed to send them processPendingCommandQueue(); // Send queued commands diff --git a/common/resourceaccess.h b/common/resourceaccess.h index b6a0b34..890cc6d 100644 --- a/common/resourceaccess.h +++ b/common/resourceaccess.h @@ -80,6 +80,11 @@ public: return KAsync::null(); } + virtual KAsync::Job sendSecret(const QString &secret) + { + return KAsync::null(); + } + int getResourceStatus() const { return mResourceStatus; @@ -121,6 +126,7 @@ public: KAsync::Job sendInspectionCommand(int inspectionType,const QByteArray &inspectionId, const QByteArray &domainType, const QByteArray &entityId, const QByteArray &property, const QVariant &expecedValue) Q_DECL_OVERRIDE; KAsync::Job sendFlushCommand(int flushType, const QByteArray &flushId) Q_DECL_OVERRIDE; + KAsync::Job sendSecret(const QString &secret) Q_DECL_OVERRIDE; /** * Tries to connect to server, and returns a connected socket on success. */ diff --git a/common/secretstore.cpp b/common/secretstore.cpp new file mode 100644 index 0000000..39d5206 --- /dev/null +++ b/common/secretstore.cpp @@ -0,0 +1,51 @@ +/* + * Copyright (C) 2017 Christian Mollekopf + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) version 3, or any + * later version accepted by the membership of KDE e.V. (or its + * successor approved by the membership of KDE e.V.), which shall + * act as a proxy defined in Section 6 of version 3 of the license. + * + * 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 + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see . + */ + +#include "secretstore.h" + +#include +#include +#include +#include + +using namespace Sink; + +QMutex SecretStore::sMutex; + +SecretStore &SecretStore::instance() +{ + static SecretStore s; + return s; +} + +void SecretStore::insert(const QByteArray &resourceId, const QString &secret) +{ + QMutexLocker locker{&sMutex}; + mCache.insert(resourceId, secret); + locker.unlock(); + emit secretAvailable(resourceId); +} + +QString SecretStore::resourceSecret(const QByteArray &resourceId) +{ + QMutexLocker locker{&sMutex}; + return mCache.value(resourceId); +} + diff --git a/common/secretstore.h b/common/secretstore.h new file mode 100644 index 0000000..04fdaba --- /dev/null +++ b/common/secretstore.h @@ -0,0 +1,50 @@ +/* + * Copyright (C) 2017 Christian Mollekopf + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) version 3, or any + * later version accepted by the membership of KDE e.V. (or its + * successor approved by the membership of KDE e.V.), which shall + * act as a proxy defined in Section 6 of version 3 of the license. + * + * 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 + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see . + */ + +#pragma once + +#include "sink_export.h" + +#include +#include +#include +#include + +namespace Sink { + +class SINK_EXPORT SecretStore : public QObject +{ + Q_OBJECT +public: + static SecretStore &instance(); + + void insert(const QByteArray &resourceId, const QString &secret); + QString resourceSecret(const QByteArray &resourceId); + +Q_SIGNALS: + void secretAvailable(const QByteArray &resourceId); + +private: + QMap mCache; + static QMutex sMutex; +}; + +} + diff --git a/common/synchronizer.cpp b/common/synchronizer.cpp index 959cf48..4a0ac46 100644 --- a/common/synchronizer.cpp +++ b/common/synchronizer.cpp @@ -49,6 +49,20 @@ Synchronizer::~Synchronizer() } +void Synchronizer::setSecret(const QString &s) +{ + mSecret = s; + + if (!mSyncRequestQueue.isEmpty()) { + processSyncQueue().exec(); + } +} + +QString Synchronizer::secret() const +{ + return mSecret; +} + void Synchronizer::setup(const std::function &enqueueCommandCallback, MessageQueue &mq) { mEnqueue = enqueueCommandCallback; @@ -474,6 +488,10 @@ void Synchronizer::setBusy(bool busy, const QString &reason, const QByteArray re KAsync::Job Synchronizer::processSyncQueue() { + if (secret().isEmpty()) { + SinkLogCtx(mLogCtx) << "Secret not available but required."; + return KAsync::null(); + } if (mSyncRequestQueue.isEmpty()) { SinkLogCtx(mLogCtx) << "All requests processed."; return KAsync::null(); diff --git a/common/synchronizer.h b/common/synchronizer.h index cc082be..bd03ba3 100644 --- a/common/synchronizer.h +++ b/common/synchronizer.h @@ -60,6 +60,8 @@ public: bool allChangesReplayed() Q_DECL_OVERRIDE; void flushComplete(const QByteArray &flushId); + void setSecret(const QString &s); + signals: void notify(Notification); @@ -78,6 +80,8 @@ protected: virtual KAsync::Job replay(const Sink::ApplicationDomain::Mail &, Sink::Operation, const QByteArray &oldRemoteId, const QList &); virtual KAsync::Job replay(const Sink::ApplicationDomain::Folder &, Sink::Operation, const QByteArray &oldRemoteId, const QList &); protected: + QString secret() const; + ///Calls the callback to enqueue the command void enqueueCommand(int commandId, const QByteArray &data); @@ -224,6 +228,7 @@ private: MessageQueue *mMessageQueue; bool mSyncInProgress; QMultiHash mPendingSyncRequests; + QString mSecret; }; } -- cgit v1.2.3