diff options
author | Christian Kieschnick <christian.kieschnick@hicknhack-software.com> | 2019-01-03 19:50:36 +0300 |
---|---|---|
committer | Christian Kieschnick <christian.kieschnick@hicknhack-software.com> | 2019-01-03 19:50:36 +0300 |
commit | 2e183888256d51796f3e2fef933509f62aad231a (patch) | |
tree | bcd96810f72efc02668105e59c013d88e7c1d5bb | |
parent | d4c391deb2a714fd9cc733b9c91c8fb698679702 (diff) |
Fixed bug in FileWatcher, improved unsafe sharing
BulkFileWatcher didn't recognize added files in observed directory
Improved UI for unsecured sharing only (hide certificate ui, added
warnings and adjusted indicators)
-rw-r--r-- | src/core/FileWatcher.cpp | 38 | ||||
-rw-r--r-- | src/gui/FileDialog.cpp | 4 | ||||
-rw-r--r-- | src/keeshare/DatabaseSettingsWidgetKeeShare.cpp | 2 | ||||
-rw-r--r-- | src/keeshare/KeeShare.cpp | 24 | ||||
-rw-r--r-- | src/keeshare/KeeShare.h | 1 | ||||
-rw-r--r-- | src/keeshare/KeeShareSettings.cpp | 46 | ||||
-rw-r--r-- | src/keeshare/KeeShareSettings.h | 32 | ||||
-rw-r--r-- | src/keeshare/SettingsWidgetKeeShare.cpp | 39 | ||||
-rw-r--r-- | src/keeshare/ShareObserver.cpp | 179 | ||||
-rw-r--r-- | src/keeshare/group/EditGroupWidgetKeeShare.cpp | 57 | ||||
-rw-r--r-- | src/keeshare/group/EditGroupWidgetKeeShare.h | 2 | ||||
-rw-r--r-- | tests/TestSharing.cpp | 65 |
12 files changed, 321 insertions, 168 deletions
diff --git a/src/core/FileWatcher.cpp b/src/core/FileWatcher.cpp index ac44174bd..4873adbf4 100644 --- a/src/core/FileWatcher.cpp +++ b/src/core/FileWatcher.cpp @@ -182,28 +182,28 @@ void BulkFileWatcher::handleFileChanged(const QString& path) void BulkFileWatcher::handleDirectoryChanged(const QString& path) { qDebug("Directory changed %s", qPrintable(path)); - const QFileInfo directory(path); - const QMap<QString, bool>& watchedFiles = m_watchedFilesInDirectory[directory.absolutePath()]; - for (const QString& file : watchedFiles.keys()) { - const QFileInfo info(file); - const bool existed = watchedFiles[info.absoluteFilePath()]; - if (!info.exists() && existed) { - qDebug("Remove watch file %s", qPrintable(info.absoluteFilePath())); - m_fileWatcher.removePath(info.absolutePath()); - emit fileRemoved(info.absoluteFilePath()); + const QFileInfo directoryInfo(path); + const QMap<QString, bool>& watchedFiles = m_watchedFilesInDirectory[directoryInfo.absoluteFilePath()]; + for (const QString& filename : watchedFiles.keys()) { + const QFileInfo fileInfo(filename); + const bool existed = watchedFiles[fileInfo.absoluteFilePath()]; + if (!fileInfo.exists() && existed) { + qDebug("Remove watch file %s", qPrintable(fileInfo.absoluteFilePath())); + m_fileWatcher.removePath(fileInfo.absolutePath()); + emit fileRemoved(fileInfo.absoluteFilePath()); } - if (!existed && info.exists()) { - qDebug("Add watch file %s", qPrintable(info.absoluteFilePath())); - m_fileWatcher.addPath(info.absolutePath()); - emit fileCreated(info.absoluteFilePath()); + if (!existed && fileInfo.exists()) { + qDebug("Add watch file %s", qPrintable(fileInfo.absoluteFilePath())); + m_fileWatcher.addPath(fileInfo.absolutePath()); + emit fileCreated(fileInfo.absoluteFilePath()); } - if (existed && info.exists()) { - qDebug("Refresh watch file %s", qPrintable(info.absoluteFilePath())); - m_fileWatcher.removePath(info.absolutePath()); - m_fileWatcher.addPath(info.absolutePath()); - emit fileChanged(info.absoluteFilePath()); + if (existed && fileInfo.exists()) { + qDebug("Refresh watch file %s", qPrintable(fileInfo.absoluteFilePath())); + m_fileWatcher.removePath(fileInfo.absolutePath()); + m_fileWatcher.addPath(fileInfo.absolutePath()); + emit fileChanged(fileInfo.absoluteFilePath()); } - m_watchedFilesInDirectory[info.absolutePath()][info.absoluteFilePath()] = info.exists(); + m_watchedFilesInDirectory[fileInfo.absolutePath()][fileInfo.absoluteFilePath()] = fileInfo.exists(); } } diff --git a/src/gui/FileDialog.cpp b/src/gui/FileDialog.cpp index 77c08e3eb..a39ee9e6b 100644 --- a/src/gui/FileDialog.cpp +++ b/src/gui/FileDialog.cpp @@ -114,7 +114,9 @@ QString FileDialog::getFileName(QWidget* parent, dialog.selectFile(defaultName); } dialog.setOptions(options); - dialog.setDefaultSuffix(defaultExtension); + if (!defaultExtension.isEmpty()) { + dialog.setDefaultSuffix(defaultExtension); + } dialog.setLabelText(QFileDialog::Accept, QFileDialog::tr("Select")); QStringList results; if (dialog.exec()) { diff --git a/src/keeshare/DatabaseSettingsWidgetKeeShare.cpp b/src/keeshare/DatabaseSettingsWidgetKeeShare.cpp index cb24c6ed5..6180c184e 100644 --- a/src/keeshare/DatabaseSettingsWidgetKeeShare.cpp +++ b/src/keeshare/DatabaseSettingsWidgetKeeShare.cpp @@ -57,7 +57,7 @@ void DatabaseSettingsWidgetKeeShare::loadSettings(QSharedPointer<Database> db) QStringList hierarchy = group->hierarchy(); hierarchy.removeFirst(); QList<QStandardItem*> row = QList<QStandardItem*>(); - row << new QStandardItem(hierarchy.join(" > ")); + row << new QStandardItem(hierarchy.join(tr(" > ", "Breadcrumb separator"))); row << new QStandardItem(KeeShare::referenceTypeLabel(reference)); row << new QStandardItem(reference.path); m_referencesModel->appendRow(row); diff --git a/src/keeshare/KeeShare.cpp b/src/keeshare/KeeShare.cpp index e552fa9c4..3e15d2137 100644 --- a/src/keeshare/KeeShare.cpp +++ b/src/keeshare/KeeShare.cpp @@ -125,16 +125,30 @@ void KeeShare::setReferenceTo(Group* group, const KeeShareSettings::Reference& r customData->set(KeeShare_Reference, encoded); } +bool KeeShare::isEnabled(const Group* group) +{ + const auto reference = KeeShare::referenceOf(group); +#if !defined(WITH_XC_KEESHARE_SECURE) + if (reference.path.endsWith(secureContainerFileType(), Qt::CaseInsensitive)){ + return false; + } +#endif +#if !defined(WITH_XC_KEESHARE_INSECURE) + if (reference.path.endsWith(insecureContainerFileType(), Qt::CaseInsensitive)){ + return false; + } +#endif + const auto active = KeeShare::active(); + return (reference.isImporting() && active.in) || (reference.isExporting() && active.out); +} + QPixmap KeeShare::indicatorBadge(const Group* group, QPixmap pixmap) { if (!isShared(group)) { return pixmap; } - const auto reference = KeeShare::referenceOf(group); - const auto active = KeeShare::active(); - const bool enabled = (reference.isImporting() && active.in) || (reference.isExporting() && active.out); - const QPixmap badge = enabled ? databaseIcons()->iconPixmap(DatabaseIcons::SharedIconIndex) - : databaseIcons()->iconPixmap(DatabaseIcons::UnsharedIconIndex); + const QPixmap badge = isEnabled(group) ? databaseIcons()->iconPixmap(DatabaseIcons::SharedIconIndex) + : databaseIcons()->iconPixmap(DatabaseIcons::UnsharedIconIndex); QImage canvas = pixmap.toImage(); const QRectF target(canvas.width() * 0.4, canvas.height() * 0.4, canvas.width() * 0.6, canvas.height() * 0.6); QPainter painter(&canvas); diff --git a/src/keeshare/KeeShare.h b/src/keeshare/KeeShare.h index a0a7d836e..3692361ec 100644 --- a/src/keeshare/KeeShare.h +++ b/src/keeshare/KeeShare.h @@ -41,6 +41,7 @@ public: static QPixmap indicatorBadge(const Group* group, QPixmap pixmap); static bool isShared(const Group* group); + static bool isEnabled(const Group *group); static KeeShareSettings::Own own(); static KeeShareSettings::Active active(); diff --git a/src/keeshare/KeeShareSettings.cpp b/src/keeshare/KeeShareSettings.cpp index a1fcfac37..47ad76df6 100644 --- a/src/keeshare/KeeShareSettings.cpp +++ b/src/keeshare/KeeShareSettings.cpp @@ -32,10 +32,9 @@ namespace KeeShareSettings { namespace { - Certificate packCertificate(const OpenSSHKey& key, bool verified, const QString& signer) + Certificate packCertificate(const OpenSSHKey& key, const QString& signer) { KeeShareSettings::Certificate extracted; - extracted.trusted = verified; extracted.signer = signer; Q_ASSERT(key.type() == "ssh-rsa"); extracted.key = OpenSSHKey::serializeToBinary(OpenSSHKey::Public, key); @@ -108,9 +107,6 @@ namespace KeeShareSettings writer.writeStartElement("Signer"); writer.writeCharacters(certificate.signer); writer.writeEndElement(); - writer.writeStartElement("Trusted"); - writer.writeCharacters(certificate.trusted ? "True" : "False"); - writer.writeEndElement(); writer.writeStartElement("Key"); writer.writeCharacters(certificate.key.toBase64()); writer.writeEndElement(); @@ -118,7 +114,7 @@ namespace KeeShareSettings bool Certificate::operator==(const Certificate& other) const { - return trusted == other.trusted && key == other.key && signer == other.signer; + return key == other.key && signer == other.signer; } bool Certificate::operator!=(const Certificate& other) const @@ -128,7 +124,7 @@ namespace KeeShareSettings bool Certificate::isNull() const { - return !trusted && key.isEmpty() && signer.isEmpty(); + return key.isEmpty() && signer.isEmpty(); } QString Certificate::fingerprint() const @@ -158,8 +154,6 @@ namespace KeeShareSettings while (!reader.error() && reader.readNextStartElement()) { if (reader.name() == "Signer") { certificate.signer = reader.readElementText(); - } else if (reader.name() == "Trusted") { - certificate.trusted = reader.readElementText() == "True"; } else if (reader.name() == "Key") { certificate.key = QByteArray::fromBase64(reader.readElementText().toLatin1()); } @@ -217,7 +211,7 @@ namespace KeeShareSettings Own own; own.key = packKey(key); const QString name = qgetenv("USER"); // + "@" + QHostInfo::localHostName(); - own.certificate = packCertificate(key, true, name); + own.certificate = packCertificate(key, name); return own; } @@ -301,13 +295,39 @@ namespace KeeShareSettings return own; } + bool ScopedCertificate::operator==(const ScopedCertificate &other) const + { + return trusted == other.trusted && path == other.path && certificate == other.certificate; + } + + bool ScopedCertificate::operator!=(const ScopedCertificate &other) const + { + return !operator==(other); + } + + void ScopedCertificate::serialize(QXmlStreamWriter& writer, const ScopedCertificate& scopedCertificate) + { + writer.writeAttribute("Path", scopedCertificate.path); + writer.writeAttribute("Trusted", scopedCertificate.trusted ? "True" : "False"); + Certificate::serialize(writer, scopedCertificate.certificate); + } + + ScopedCertificate ScopedCertificate::deserialize(QXmlStreamReader &reader) + { + ScopedCertificate scopedCertificate; + scopedCertificate.path = reader.attributes().value("Path").toString(); + scopedCertificate.trusted = reader.attributes().value("Trusted") == "True"; + scopedCertificate.certificate = Certificate::deserialize(reader); + return scopedCertificate; + } + QString Foreign::serialize(const Foreign& foreign) { return xmlSerialize([&](QXmlStreamWriter& writer) { writer.writeStartElement("Foreign"); - for (const Certificate& certificate : foreign.certificates) { + for (const ScopedCertificate& scopedCertificate : foreign.certificates) { writer.writeStartElement("Certificate"); - Certificate::serialize(writer, certificate); + ScopedCertificate::serialize(writer, scopedCertificate); writer.writeEndElement(); } writer.writeEndElement(); @@ -322,7 +342,7 @@ namespace KeeShareSettings if (reader.name() == "Foreign") { while (!reader.error() && reader.readNextStartElement()) { if (reader.name() == "Certificate") { - foreign.certificates << Certificate::deserialize(reader); + foreign.certificates << ScopedCertificate::deserialize(reader); } else { ::qWarning() << "Unknown Cerificates element" << reader.name(); reader.skipCurrentElement(); diff --git a/src/keeshare/KeeShareSettings.h b/src/keeshare/KeeShareSettings.h index 94d67d1ca..a6522a5b4 100644 --- a/src/keeshare/KeeShareSettings.h +++ b/src/keeshare/KeeShareSettings.h @@ -33,16 +33,10 @@ namespace KeeShareSettings { QByteArray key; QString signer; - bool trusted; bool operator==(const Certificate& other) const; bool operator!=(const Certificate& other) const; - Certificate() - : trusted(false) - { - } - bool isNull() const; QString fingerprint() const; QString publicKey() const; @@ -71,11 +65,13 @@ namespace KeeShareSettings { bool in; bool out; + Active() : in(false) , out(false) { } + bool isNull() const { return !in && !out; @@ -92,6 +88,7 @@ namespace KeeShareSettings bool operator==(const Own& other) const; bool operator!=(const Own& other) const; + bool isNull() const { return key.isNull() && certificate.isNull(); @@ -102,14 +99,25 @@ namespace KeeShareSettings static Own generate(); }; - struct Foreign + struct ScopedCertificate { - QList<Certificate> certificates; + QString path; + Certificate certificate; + bool trusted; - bool isNull() const - { - return certificates.isEmpty(); - } + bool operator==(const ScopedCertificate& other) const; + bool operator!=(const ScopedCertificate& other) const; + + bool isUnknown() const { return certificate.isNull(); } + bool isKnown() const { return !certificate.isNull(); } + + static void serialize(QXmlStreamWriter& writer, const ScopedCertificate& certificate); + static ScopedCertificate deserialize(QXmlStreamReader& reader); + }; + + struct Foreign + { + QList<ScopedCertificate> certificates; static QString serialize(const Foreign& foreign); static Foreign deserialize(const QString& raw); diff --git a/src/keeshare/SettingsWidgetKeeShare.cpp b/src/keeshare/SettingsWidgetKeeShare.cpp index 86b9e8c87..2b5ba3e3c 100644 --- a/src/keeshare/SettingsWidgetKeeShare.cpp +++ b/src/keeshare/SettingsWidgetKeeShare.cpp @@ -34,6 +34,11 @@ SettingsWidgetKeeShare::SettingsWidgetKeeShare(QWidget* parent) { m_ui->setupUi(this); +#if !defined(WITH_XC_KEESHARE_SECURE) + // Setting does not help the user of Version without secure export + m_ui->ownCertificateGroupBox->setVisible(false); +#endif + connect(m_ui->ownCertificateSignerEdit, SIGNAL(textChanged(QString)), SLOT(setVerificationExporter(QString))); connect(m_ui->generateOwnCerticateButton, SIGNAL(clicked(bool)), SLOT(generateCertificate())); @@ -65,15 +70,25 @@ void SettingsWidgetKeeShare::loadSettings() void SettingsWidgetKeeShare::updateForeignCertificates() { m_importedCertificateModel.reset(new QStandardItemModel()); - m_importedCertificateModel->setHorizontalHeaderLabels( - QStringList() << tr("Signer") << tr("Status") << tr("Fingerprint") << tr("Certificate")); - - for (const KeeShareSettings::Certificate& certificate : m_foreign.certificates) { - QStandardItem* signer = new QStandardItem(certificate.signer); - QStandardItem* verified = new QStandardItem(certificate.trusted ? tr("trusted") : tr("untrusted")); - QStandardItem* fingerprint = new QStandardItem(certificate.fingerprint()); - QStandardItem* key = new QStandardItem(certificate.publicKey()); - m_importedCertificateModel->appendRow(QList<QStandardItem*>() << signer << verified << fingerprint << key); + m_importedCertificateModel->setHorizontalHeaderLabels(QStringList() << tr("Path") << tr("Status") +#if defined(WITH_XC_KEESHARE_SECURE) + << tr("Signer") << tr("Fingerprint") << tr("Certificate") +#endif + ); + + for (const auto& scopedCertificate : m_foreign.certificates) { + const auto items = QList<QStandardItem*>() + << new QStandardItem(scopedCertificate.path) + << new QStandardItem(scopedCertificate.trusted ? tr("Trusted") : tr("Untrusted")) +#if defined(WITH_XC_KEESHARE_SECURE) + << new QStandardItem(scopedCertificate.isKnown() + ? scopedCertificate.certificate.signer + : tr("Unknown")) + << new QStandardItem(scopedCertificate.certificate.fingerprint()) + << new QStandardItem(scopedCertificate.certificate.publicKey()) +#endif + ; + m_importedCertificateModel->appendRow(items); } m_ui->importedCertificateTableView->setModel(m_importedCertificateModel.data()); @@ -124,7 +139,7 @@ void SettingsWidgetKeeShare::importCertificate() } const auto filetype = tr("key.share", "Filetype for KeeShare key"); const auto filters = QString("%1 (*." + filetype + ");;%2 (*)").arg(tr("KeeShare key file"), tr("All files")); - QString filename = fileDialog()->getOpenFileName(this, tr("Select path"), defaultDirPath, filters, nullptr, 0); + QString filename = fileDialog()->getOpenFileName(this, tr("Select path"), defaultDirPath, filters, nullptr, QFileDialog::Options(0)); if (filename.isEmpty()) { return; } @@ -161,7 +176,7 @@ void SettingsWidgetKeeShare::exportCertificate() const auto filetype = tr("key.share", "Filetype for KeeShare key"); const auto filters = QString("%1 (*." + filetype + ");;%2 (*)").arg(tr("KeeShare key file"), tr("All files")); QString filename = tr("%1.%2", "Template for KeeShare key file").arg(m_own.certificate.signer).arg(filetype); - filename = fileDialog()->getSaveFileName(this, tr("Select path"), defaultDirPath, filters, nullptr, 0, filetype, filename); + filename = fileDialog()->getSaveFileName(this, tr("Select path"), defaultDirPath, filters, nullptr, QFileDialog::Options(0), filetype, filename); if (filename.isEmpty()) { return; } @@ -198,7 +213,7 @@ void SettingsWidgetKeeShare::untrustSelectedCertificates() void SettingsWidgetKeeShare::removeSelectedCertificates() { - QList<KeeShareSettings::Certificate> certificates = m_foreign.certificates; + auto certificates = m_foreign.certificates; const auto* selectionModel = m_ui->importedCertificateTableView->selectionModel(); Q_ASSERT(selectionModel); for (const auto& index : selectionModel->selectedRows()) { diff --git a/src/keeshare/ShareObserver.cpp b/src/keeshare/ShareObserver.cpp index d26c68dcf..2f98b1bcd 100644 --- a/src/keeshare/ShareObserver.cpp +++ b/src/keeshare/ShareObserver.cpp @@ -56,12 +56,12 @@ static const QString KeeShare_Container("container.share.kdbx"); enum Trust { - None, Invalid, - Single, - Lasting, - Known, - Own + Own, + UntrustedForever, + UntrustedOnce, + TrustedOnce, + TrustedForever, }; bool isOfExportType(const QFileInfo &fileInfo, const QString type) @@ -72,17 +72,12 @@ bool isOfExportType(const QFileInfo &fileInfo, const QString type) QPair<Trust, KeeShareSettings::Certificate> check(QByteArray& data, const KeeShareSettings::Reference& reference, const KeeShareSettings::Certificate& ownCertificate, - const QList<KeeShareSettings::Certificate>& knownCertificates, + const QList<KeeShareSettings::ScopedCertificate>& knownCertificates, const KeeShareSettings::Sign& sign) { - if (sign.signature.isEmpty()) { - for (const auto& certificate : knownCertificates) { - if (certificate.key == certificate.key && certificate.trusted) { - return {Known, certificate}; - } - } - } - else { + KeeShareSettings::Certificate certificate; + if (!sign.signature.isEmpty()) { + certificate = sign.certificate; auto key = sign.certificate.sshKey(); key.openKey(QString()); const Signature signer; @@ -92,45 +87,61 @@ QPair<Trust, KeeShareSettings::Certificate> check(QByteArray& data, } if (ownCertificate.key == sign.certificate.key) { - return {Own, ownCertificate}; + return {Own, ownCertificate }; } - - for (const auto& certificate : knownCertificates) { - if (certificate.key == certificate.key && certificate.trusted) { - return {Known, certificate}; - } + } + enum Scope { Invalid, Global, Local }; + Scope scope = Invalid; + bool trusted = false; + for (const auto& scopedCertificate : knownCertificates) { + if (scopedCertificate.certificate.key == certificate.key && scopedCertificate.path == reference.path) { + // Global scope is overwritten by local scope + scope = Global; + trusted = scopedCertificate.trusted; } + if (scopedCertificate.certificate.key == certificate.key && scopedCertificate.path == reference.path) { + scope = Local; + trusted = scopedCertificate.trusted; + break; + } + } + if (scope != Invalid){ + // we introduce now scopes if there is a global + return {trusted ? TrustedForever : UntrustedForever, certificate}; } QMessageBox warning; - KeeShareSettings::Certificate certificate; if (sign.signature.isEmpty()){ warning.setIcon(QMessageBox::Warning); - warning.setWindowTitle(ShareObserver::tr("Untrustworthy container without signature")); - warning.setText(ShareObserver::tr("We cannot verify the source of the shared container because it is not signed. Do you really want to import %1?").arg(reference.path)); - certificate = KeeShareSettings::Certificate(); + warning.setWindowTitle(ShareObserver::tr("Import from container without signature")); + warning.setText(ShareObserver::tr("We cannot verify the source of the shared container because it is not signed. Do you really want to import from %1?") + .arg(reference.path)); } else { warning.setIcon(QMessageBox::Question); - warning.setWindowTitle(ShareObserver::tr("Import from untrustworthy certificate for sharing container")); - warning.setText(ShareObserver::tr("Do you want to trust %1 with the fingerprint of %2") - .arg(sign.certificate.signer) - .arg(sign.certificate.fingerprint())); - certificate = sign.certificate; - } - auto once = warning.addButton(ShareObserver::tr("Only this time"), QMessageBox::ButtonRole::YesRole); - auto always = warning.addButton(ShareObserver::tr("Always"), QMessageBox::ButtonRole::YesRole); - auto abort = warning.addButton(ShareObserver::tr("No"), QMessageBox::ButtonRole::NoRole); - warning.setDefaultButton(abort); + warning.setWindowTitle(ShareObserver::tr("Import from container with certificate")); + warning.setText(ShareObserver::tr("Do you want to trust %1 with the fingerprint of %2 from %3") + .arg(certificate.signer, certificate.fingerprint(), reference.path)); + } + auto untrustedOnce = warning.addButton(ShareObserver::tr("Not this time"), QMessageBox::ButtonRole::NoRole); + auto untrustedForever = warning.addButton(ShareObserver::tr("Never"), QMessageBox::ButtonRole::NoRole); + auto trustedForever = warning.addButton(ShareObserver::tr("Always"), QMessageBox::ButtonRole::YesRole); + auto trustedOnce = warning.addButton(ShareObserver::tr("Just this time"), QMessageBox::ButtonRole::YesRole); + warning.setDefaultButton(untrustedOnce); warning.exec(); - if (warning.clickedButton() == once){ - return {Single, certificate}; + if (warning.clickedButton() == trustedForever){ + return {TrustedForever, certificate }; } - if (warning.clickedButton() == always){ - return {Lasting, certificate}; + if (warning.clickedButton() == trustedOnce){ + return {TrustedOnce, certificate}; } - qWarning("Prevented import due to untrusted certificate of %s", qPrintable(sign.certificate.signer)); - return {None, certificate}; + if (warning.clickedButton() == untrustedOnce){ + return {UntrustedOnce, certificate }; + } + if (warning.clickedButton() == untrustedForever){ + return {UntrustedForever, certificate }; + } + return {UntrustedOnce, certificate }; } } // End Namespace @@ -287,7 +298,7 @@ ShareObserver::Result ShareObserver::importSecureContainerInto(const KeeShareSet { #if !defined(WITH_XC_KEESHARE_SECURE) Q_UNUSED(targetGroup); - return { reference.path, Result::Error, tr("Secured share container are not supported") }; + return { reference.path, Result::Warning, tr("Secured share container are not supported - import prevented") }; #else QuaZip zip(reference.path); if (!zip.open(QuaZip::mdUnzip)) { @@ -332,34 +343,45 @@ ShareObserver::Result ShareObserver::importSecureContainerInto(const KeeShareSet auto foreign = KeeShare::foreign(); auto own = KeeShare::own(); - auto trusted = check(payload, reference, own.certificate, foreign.certificates, sign); - switch (trusted.first) { - case None: - qWarning("Prevent untrusted import"); - return {reference.path, Result::Warning, tr("Untrusted import prevented")}; - + auto trust = check(payload, reference, own.certificate, foreign.certificates, sign); + switch (trust.first) { case Invalid: - qCritical("Prevent untrusted import"); + qWarning("Prevent untrusted import"); return {reference.path, Result::Error, tr("Untrusted import prevented")}; - case Known: - case Lasting: { + case UntrustedForever: + case TrustedForever: { bool found = false; - for (KeeShareSettings::Certificate& knownCertificate : foreign.certificates) { - if (knownCertificate.key == trusted.second.key) { - knownCertificate.signer = trusted.second.signer; - knownCertificate.trusted = true; + bool trusted = trust.first == TrustedForever; + for (KeeShareSettings::ScopedCertificate& scopedCertificate : foreign.certificates) { + if (scopedCertificate.certificate.key == trust.second.key && scopedCertificate.path == reference.path) { + scopedCertificate.certificate.signer = trust.second.signer; + scopedCertificate.path = reference.path; + scopedCertificate.trusted = trusted; found = true; } } if (!found) { - foreign.certificates << trusted.second; + foreign.certificates << KeeShareSettings::ScopedCertificate{ reference.path, trust.second, trusted}; // we need to update with the new signer KeeShare::setForeign(foreign); } + if (trusted) { + qDebug("Synchronize %s %s with %s", + qPrintable(reference.path), + qPrintable(targetGroup->name()), + qPrintable(sourceDb->rootGroup()->name())); + Merger merger(sourceDb->rootGroup(), targetGroup); + merger.setForcedMergeMode(Group::Synchronize); + const bool changed = merger.merge(); + if (changed) { + return {reference.path, Result::Success, tr("Successful secured import")}; + } + } + // Silent ignore of untrusted import or unchanging import + return {}; } - [[fallthrough]]; - case Single: + case TrustedOnce: case Own: { qDebug("Synchronize %s %s with %s", qPrintable(reference.path), @@ -384,7 +406,7 @@ ShareObserver::Result ShareObserver::importInsecureContainerInto(const KeeShareS { #if !defined(WITH_XC_KEESHARE_INSECURE) Q_UNUSED(targetGroup); - return {reference.path, Result::Error, tr("Insecured share container are not supported")}; + return {reference.path, Result::Warning, tr("Insecured share container are not supported - import prevented")}; #else QFile file(reference.path); if (!file.open(QIODevice::ReadOnly)){ @@ -408,26 +430,41 @@ ShareObserver::Result ShareObserver::importInsecureContainerInto(const KeeShareS auto foreign = KeeShare::foreign(); auto own = KeeShare::own(); static KeeShareSettings::Sign sign; // invalid sign - auto trusted = check(payload, reference, own.certificate, foreign.certificates, sign); - switch(trusted.first) { - case Known: - case Lasting: { + auto trust = check(payload, reference, own.certificate, foreign.certificates, sign); + switch(trust.first) { + case UntrustedForever: + case TrustedForever: { bool found = false; - for (KeeShareSettings::Certificate& knownCertificate : foreign.certificates) { - if (knownCertificate.key == trusted.second.key) { - knownCertificate.signer = trusted.second.signer; - knownCertificate.trusted = true; + bool trusted = trust.first == TrustedForever; + for (KeeShareSettings::ScopedCertificate& scopedCertificate : foreign.certificates) { + if (scopedCertificate.certificate.key == trust.second.key && scopedCertificate.path == reference.path) { + scopedCertificate.certificate.signer = trust.second.signer; + scopedCertificate.path = reference.path; + scopedCertificate.trusted = trusted; found = true; } } if (!found) { - foreign.certificates << trusted.second; + foreign.certificates << KeeShareSettings::ScopedCertificate{ reference.path, trust.second, trusted}; // we need to update with the new signer KeeShare::setForeign(foreign); } + if (trusted) { + qDebug("Synchronize %s %s with %s", + qPrintable(reference.path), + qPrintable(targetGroup->name()), + qPrintable(sourceDb->rootGroup()->name())); + Merger merger(sourceDb->rootGroup(), targetGroup); + merger.setForcedMergeMode(Group::Synchronize); + const bool changed = merger.merge(); + if (changed) { + return {reference.path, Result::Success, tr("Successful secured import")}; + } + } + return {}; } - [[fallthrough]]; - case Single: { + + case TrustedOnce: { qDebug("Synchronize %s %s with %s", qPrintable(reference.path), qPrintable(targetGroup->name()), @@ -571,7 +608,7 @@ ShareObserver::Result ShareObserver::exportIntoReferenceSecureContainer(const Ke { #if !defined(WITH_XC_KEESHARE_SECURE) Q_UNUSED(targetDb); - return {reference.path, Result::Error, tr("Overwriting secured share container is not supported")}; + return {reference.path, Result::Warning, tr("Overwriting secured share container is not supported - export prevented")}; #else QByteArray bytes; { @@ -637,7 +674,7 @@ ShareObserver::Result ShareObserver::exportIntoReferenceInsecureContainer(const { #if !defined(WITH_XC_KEESHARE_INSECURE) Q_UNUSED(targetDb); - return {reference.path, Result::Error, tr("Overwriting secured share container is not supported")}; + return {reference.path, Result::Warning, tr("Overwriting secured share container is not supported - export prevented")}; #else QFile file(reference.path); const bool fileOpened = file.open(QIODevice::WriteOnly); diff --git a/src/keeshare/group/EditGroupWidgetKeeShare.cpp b/src/keeshare/group/EditGroupWidgetKeeShare.cpp index 2881a6630..76ae6b122 100644 --- a/src/keeshare/group/EditGroupWidgetKeeShare.cpp +++ b/src/keeshare/group/EditGroupWidgetKeeShare.cpp @@ -49,8 +49,8 @@ EditGroupWidgetKeeShare::EditGroupWidgetKeeShare(QWidget* parent) connect(m_ui->togglePasswordGeneratorButton, SIGNAL(toggled(bool)), SLOT(togglePasswordGeneratorButton(bool))); connect(m_ui->passwordEdit, SIGNAL(textChanged(QString)), SLOT(selectPassword())); connect(m_ui->passwordGenerator, SIGNAL(appliedPassword(QString)), SLOT(setGeneratedPassword(QString))); - connect(m_ui->pathEdit, SIGNAL(textChanged(QString)), SLOT(setPath(QString))); - connect(m_ui->pathSelectionButton, SIGNAL(pressed()), SLOT(selectPath())); + connect(m_ui->pathEdit, SIGNAL(editingFinished()), SLOT(selectPath())); + connect(m_ui->pathSelectionButton, SIGNAL(pressed()), SLOT(launchPathSelectionDialog())); connect(m_ui->typeComboBox, SIGNAL(currentIndexChanged(int)), SLOT(selectType())); connect(KeeShare::instance(), SIGNAL(activeChanged()), SLOT(showSharingState())); @@ -102,15 +102,40 @@ void EditGroupWidgetKeeShare::showSharingState() if (!m_temporaryGroup) { return; } + + auto supportedExtensions = QStringList(); +#if defined(WITH_XC_KEESHARE_INSECURE) + supportedExtensions << KeeShare::insecureContainerFileType(); +#endif +#if defined(WITH_XC_KEESHARE_SECURE) + supportedExtensions << KeeShare::secureContainerFileType(); +#endif + const auto reference = KeeShare::referenceOf(m_temporaryGroup); + if (!reference.path.isEmpty()) { + bool supported = false; + for(const auto &extension : supportedExtensions){ + if (reference.path.endsWith(extension, Qt::CaseInsensitive)){ + supported = true; + break; + } + } + if (!supported) { + m_ui->messageWidget->showMessage(tr("Your KeePassXC version does not support sharing your container type. Please use %1.").arg(supportedExtensions.join(", ")), MessageWidget::Warning); + return; + } + } const auto active = KeeShare::active(); if (!active.in && !active.out) { m_ui->messageWidget->showMessage(tr("Database sharing is disabled"), MessageWidget::Information); + return; } if (active.in && !active.out) { m_ui->messageWidget->showMessage(tr("Database export is disabled"), MessageWidget::Information); + return; } if (!active.in && active.out) { m_ui->messageWidget->showMessage(tr("Database import is disabled"), MessageWidget::Information); + return; } } @@ -149,17 +174,17 @@ void EditGroupWidgetKeeShare::setGeneratedPassword(const QString& password) m_ui->togglePasswordGeneratorButton->setChecked(false); } -void EditGroupWidgetKeeShare::setPath(const QString& path) +void EditGroupWidgetKeeShare::selectPath() { if (!m_temporaryGroup) { return; } auto reference = KeeShare::referenceOf(m_temporaryGroup); - reference.path = path; + reference.path = m_ui->pathEdit->text(); KeeShare::setReferenceTo(m_temporaryGroup, reference); } -void EditGroupWidgetKeeShare::selectPath() +void EditGroupWidgetKeeShare::launchPathSelectionDialog() { if (!m_temporaryGroup) { return; @@ -171,20 +196,28 @@ void EditGroupWidgetKeeShare::selectPath() } auto reference = KeeShare::referenceOf(m_temporaryGroup); QString defaultFiletype = ""; + auto supportedExtensions = QStringList(); + auto unsupportedExtensions = QStringList(); auto knownFilters = QStringList() << QString("%1 (*)").arg("All files"); #if defined(WITH_XC_KEESHARE_INSECURE) - defaultFiletype = KeeShare::secureContainerFileType(); + defaultFiletype = KeeShare::insecureContainerFileType(); + supportedExtensions << KeeShare::insecureContainerFileType(); knownFilters.prepend(QString("%1 (*.%2)").arg(tr("KeeShare insecure container"), KeeShare::insecureContainerFileType())); +#else + unsupportedExtensions << KeeShare::insecureContainerFileType(); #endif #if defined(WITH_XC_KEESHARE_SECURE) defaultFiletype = KeeShare::secureContainerFileType(); + supportedExtensions << KeeShare::secureContainerFileType(); knownFilters.prepend(QString("%1 (*.%2)").arg(tr("KeeShare secure container"), KeeShare::secureContainerFileType())); +#else + unsupportedExtensions << KeeShare::secureContainerFileType(); #endif const auto filters = knownFilters.join(";;"); auto filename = reference.path; if (filename.isEmpty()) { - filename = tr("%1.%2", "Template for KeeShare container").arg(m_temporaryGroup->name()).arg(defaultFiletype); + filename = m_temporaryGroup->name(); } switch (reference.type) { case KeeShareSettings::ImportFrom: @@ -206,6 +239,16 @@ void EditGroupWidgetKeeShare::selectPath() if (filename.isEmpty()) { return; } + bool validFilename = false; + for(const auto& extension : supportedExtensions + unsupportedExtensions){ + if (filename.endsWith(extension, Qt::CaseInsensitive)) { + validFilename = true; + break; + } + } + if (!validFilename){ + filename += (!filename.endsWith(".") ? "." : "") + defaultFiletype; + } m_ui->pathEdit->setText(filename); config()->set("KeeShare/LastShareDir", QFileInfo(filename).absolutePath()); diff --git a/src/keeshare/group/EditGroupWidgetKeeShare.h b/src/keeshare/group/EditGroupWidgetKeeShare.h index b01bada44..140f13c86 100644 --- a/src/keeshare/group/EditGroupWidgetKeeShare.h +++ b/src/keeshare/group/EditGroupWidgetKeeShare.h @@ -46,8 +46,8 @@ private slots: void update(); void selectType(); void selectPassword(); + void launchPathSelectionDialog(); void selectPath(); - void setPath(const QString& path); void setGeneratedPassword(const QString& password); void togglePasswordGeneratorButton(bool checked); diff --git a/tests/TestSharing.cpp b/tests/TestSharing.cpp index f6500ca8b..78d8c1802 100644 --- a/tests/TestSharing.cpp +++ b/tests/TestSharing.cpp @@ -41,7 +41,8 @@ QTEST_GUILESS_MAIN(TestSharing) Q_DECLARE_METATYPE(KeeShareSettings::Type) Q_DECLARE_METATYPE(KeeShareSettings::Key) Q_DECLARE_METATYPE(KeeShareSettings::Certificate) -Q_DECLARE_METATYPE(QList<KeeShareSettings::Certificate>) +Q_DECLARE_METATYPE(KeeShareSettings::ScopedCertificate) +Q_DECLARE_METATYPE(QList<KeeShareSettings::ScopedCertificate>) void TestSharing::initTestCase() { @@ -127,9 +128,9 @@ void TestSharing::testNullObjects() QVERIFY(xmlActive.isNull()); const KeeShareSettings::Foreign foreign; - QVERIFY(foreign.isNull()); + QVERIFY(foreign.certificates.isEmpty()); const KeeShareSettings::Foreign xmlForeign = KeeShareSettings::Foreign::deserialize(empty); - QVERIFY(xmlForeign.isNull()); + QVERIFY(xmlForeign.certificates.isEmpty()); const KeeShareSettings::Reference reference; QVERIFY(reference.isNull()); @@ -141,28 +142,33 @@ void TestSharing::testCertificateSerialization() { QFETCH(bool, trusted); const OpenSSHKey& key = stubkey(); - KeeShareSettings::Certificate original; - original.key = OpenSSHKey::serializeToBinary(OpenSSHKey::Public, key); - original.signer = "Some <!> &#_\"\" weird string"; + KeeShareSettings::ScopedCertificate original; + original.path = "/path"; + original.certificate = KeeShareSettings::Certificate + { + OpenSSHKey::serializeToBinary(OpenSSHKey::Public, key), + "Some <!> &#_\"\" weird string" + }; original.trusted = trusted; QString buffer; QXmlStreamWriter writer(&buffer); writer.writeStartDocument(); writer.writeStartElement("Certificate"); - KeeShareSettings::Certificate::serialize(writer, original); + KeeShareSettings::ScopedCertificate::serialize(writer, original); writer.writeEndElement(); writer.writeEndDocument(); QXmlStreamReader reader(buffer); reader.readNextStartElement(); QVERIFY(reader.name() == "Certificate"); - KeeShareSettings::Certificate restored = KeeShareSettings::Certificate::deserialize(reader); + KeeShareSettings::ScopedCertificate restored = KeeShareSettings::ScopedCertificate::deserialize(reader); - QCOMPARE(restored.key, original.key); - QCOMPARE(restored.signer, original.signer); + QCOMPARE(restored.certificate.key, original.certificate.key); + QCOMPARE(restored.certificate.signer, original.certificate.signer); QCOMPARE(restored.trusted, original.trusted); + QCOMPARE(restored.path, original.path); - QCOMPARE(restored.sshKey().publicParts(), key.publicParts()); + QCOMPARE(restored.certificate.sshKey().publicParts(), key.publicParts()); } void TestSharing::testCertificateSerialization_data() @@ -234,7 +240,7 @@ void TestSharing::testSettingsSerialization() QFETCH(bool, exporting); QFETCH(KeeShareSettings::Certificate, ownCertificate); QFETCH(KeeShareSettings::Key, ownKey); - QFETCH(QList<KeeShareSettings::Certificate>, foreignCertificates); + QFETCH(QList<KeeShareSettings::ScopedCertificate>, foreignCertificates); KeeShareSettings::Own originalOwn; KeeShareSettings::Foreign originalForeign; @@ -257,41 +263,48 @@ void TestSharing::testSettingsSerialization() QCOMPARE(restoredActive.in, importing); QCOMPARE(restoredActive.out, exporting); QCOMPARE(restoredOwn.certificate.key, ownCertificate.key); - QCOMPARE(restoredOwn.certificate.trusted, ownCertificate.trusted); QCOMPARE(restoredOwn.key.key, ownKey.key); QCOMPARE(restoredForeign.certificates.count(), foreignCertificates.count()); for (int i = 0; i < foreignCertificates.count(); ++i) { - QCOMPARE(restoredForeign.certificates[i].key, foreignCertificates[i].key); + QCOMPARE(restoredForeign.certificates[i].certificate.key, foreignCertificates[i].certificate.key); } } void TestSharing::testSettingsSerialization_data() { const OpenSSHKey& sshKey0 = stubkey(0); - KeeShareSettings::Certificate certificate0; - certificate0.key = OpenSSHKey::serializeToBinary(OpenSSHKey::Public, sshKey0); - certificate0.signer = "Some <!> &#_\"\" weird string"; + KeeShareSettings::ScopedCertificate certificate0; + certificate0.path = "/path/0"; + certificate0.certificate = KeeShareSettings::Certificate + { + OpenSSHKey::serializeToBinary(OpenSSHKey::Public, sshKey0), + "Some <!> &#_\"\" weird string" + }; certificate0.trusted = true; KeeShareSettings::Key key0; key0.key = OpenSSHKey::serializeToBinary(OpenSSHKey::Private, sshKey0); const OpenSSHKey& sshKey1 = stubkey(1); - KeeShareSettings::Certificate certificate1; - certificate1.key = OpenSSHKey::serializeToBinary(OpenSSHKey::Public, sshKey1); - certificate1.signer = "Another "; - certificate1.trusted = true; + KeeShareSettings::ScopedCertificate certificate1; + certificate1.path = "/path/1"; + certificate1.certificate = KeeShareSettings::Certificate + { + OpenSSHKey::serializeToBinary(OpenSSHKey::Public, sshKey1), + "Another " + }; + certificate1.trusted = false; QTest::addColumn<bool>("importing"); QTest::addColumn<bool>("exporting"); QTest::addColumn<KeeShareSettings::Certificate>("ownCertificate"); QTest::addColumn<KeeShareSettings::Key>("ownKey"); QTest::addColumn<QList<KeeShareSettings::Certificate>>("foreignCertificates"); - QTest::newRow("1") << false << false << KeeShareSettings::Certificate() << KeeShareSettings::Key() << QList<KeeShareSettings::Certificate>(); - QTest::newRow("2") << true << false << KeeShareSettings::Certificate() << KeeShareSettings::Key() << QList<KeeShareSettings::Certificate>(); - QTest::newRow("3") << true << true << KeeShareSettings::Certificate() << KeeShareSettings::Key() << QList<KeeShareSettings::Certificate>({ certificate0, certificate1 }); - QTest::newRow("4") << false << true << certificate0 << key0 << QList<KeeShareSettings::Certificate>(); - QTest::newRow("5") << false << false << certificate0 << key0 << QList<KeeShareSettings::Certificate>({ certificate1 }); + QTest::newRow("1") << false << false << KeeShareSettings::Certificate() << KeeShareSettings::Key() << QList<KeeShareSettings::ScopedCertificate>(); + QTest::newRow("2") << true << false << KeeShareSettings::Certificate() << KeeShareSettings::Key() << QList<KeeShareSettings::ScopedCertificate>(); + QTest::newRow("3") << true << true << KeeShareSettings::Certificate() << KeeShareSettings::Key() << QList<KeeShareSettings::ScopedCertificate>({ certificate0, certificate1 }); + QTest::newRow("4") << false << true << certificate0 << key0 << QList<KeeShareSettings::ScopedCertificate>(); + QTest::newRow("5") << false << false << certificate0 << key0 << QList<KeeShareSettings::ScopedCertificate>({ certificate1 }); } const OpenSSHKey& TestSharing::stubkey(int index) |