From d396e56694fc4fca61260f23509fa4c423ed4071 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andrius=20=C5=A0tikonas?= Date: Sun, 4 Oct 2020 15:20:58 +0100 Subject: [PATCH 01/16] Add support for importing "PartitionTable" of whole device filesystems. --- src/core/partitiontable.cpp | 36 ++++++++++++++-------------- src/jobs/createpartitionjob.cpp | 11 +++++++-- src/jobs/createpartitiontablejob.cpp | 5 +++- 3 files changed, 31 insertions(+), 21 deletions(-) diff --git a/src/core/partitiontable.cpp b/src/core/partitiontable.cpp index f5f2b7a..60ad8bb 100644 --- a/src/core/partitiontable.cpp +++ b/src/core/partitiontable.cpp @@ -1,7 +1,7 @@ /* SPDX-FileCopyrightText: 2008-2012 Volker Lanz SPDX-FileCopyrightText: 2009 Andrew Coles - SPDX-FileCopyrightText: 2013-2019 Andrius Štikonas + SPDX-FileCopyrightText: 2013-2020 Andrius Štikonas SPDX-FileCopyrightText: 2015-2016 Teo Mrnjavac SPDX-FileCopyrightText: 2016 Chantara Tith SPDX-FileCopyrightText: 2018 Caio Jordão Carvalho @@ -439,7 +439,7 @@ void PartitionTable::updateUnallocated(const Device& d) qint64 PartitionTable::defaultFirstUsable(const Device& d, TableType t) { Q_UNUSED(t) - if (d.type() == Device::Type::LVM_Device || d.type() == Device::Type::SoftwareRAID_Device) { + if (d.type() == Device::Type::LVM_Device || d.type() == Device::Type::SoftwareRAID_Device || t == PartitionTable::TableType::none) { return 0; } @@ -462,21 +462,21 @@ static struct { bool isReadOnly; /**< does KDE Partition Manager support this only in read only mode */ PartitionTable::TableType type; /**< enum type */ } tableTypes[] = { - { QLatin1String("aix"), 4, false, true, PartitionTable::aix }, - { QLatin1String("bsd"), 8, false, true, PartitionTable::bsd }, - { QLatin1String("dasd"), 1, false, true, PartitionTable::dasd }, - { QLatin1String("msdos"), 4, true, false, PartitionTable::msdos }, - { QLatin1String("msdos"), 4, true, false, PartitionTable::msdos_sectorbased }, - { QLatin1String("dos"), 4, true, false, PartitionTable::msdos_sectorbased }, - { QLatin1String("dvh"), 16, true, true, PartitionTable::dvh }, - { QLatin1String("gpt"), 128, false, false, PartitionTable::gpt }, - { QLatin1String("loop"), 1, false, true, PartitionTable::loop }, - { QLatin1String("mac"), 0xffff, false, true, PartitionTable::mac }, - { QLatin1String("pc98"), 16, false, true, PartitionTable::pc98 }, - { QLatin1String("amiga"), 128, false, true, PartitionTable::amiga }, - { QLatin1String("sun"), 8, false, true, PartitionTable::sun }, - { QLatin1String("vmd"), 0xffff, false, false, PartitionTable::vmd }, - { QLatin1String("none"), 1, false, false, PartitionTable::none }, + { QLatin1String("aix"), 4, false, true, PartitionTable::TableType::aix }, + { QLatin1String("bsd"), 8, false, true, PartitionTable::TableType::bsd }, + { QLatin1String("dasd"), 1, false, true, PartitionTable::TableType::dasd }, + { QLatin1String("msdos"), 4, true, false, PartitionTable::TableType::msdos }, + { QLatin1String("msdos"), 4, true, false, PartitionTable::TableType::msdos_sectorbased }, + { QLatin1String("dos"), 4, true, false, PartitionTable::TableType::msdos_sectorbased }, + { QLatin1String("dvh"), 16, true, true, PartitionTable::TableType::dvh }, + { QLatin1String("gpt"), 128, false, false, PartitionTable::TableType::gpt }, + { QLatin1String("loop"), 1, false, true, PartitionTable::TableType::loop }, + { QLatin1String("mac"), 0xffff, false, true, PartitionTable::TableType::mac }, + { QLatin1String("pc98"), 16, false, true, PartitionTable::TableType::pc98 }, + { QLatin1String("amiga"), 128, false, true, PartitionTable::TableType::amiga }, + { QLatin1String("sun"), 8, false, true, PartitionTable::TableType::sun }, + { QLatin1String("vmd"), 0xffff, false, false, PartitionTable::TableType::vmd }, + { QLatin1String("none"), 1, false, false, PartitionTable::TableType::none }, }; PartitionTable::TableType PartitionTable::nameToTableType(const QString& n) @@ -485,7 +485,7 @@ PartitionTable::TableType PartitionTable::nameToTableType(const QString& n) if (n == type.name) return type.type; - return PartitionTable::unknownTableType; + return PartitionTable::TableType::unknownTableType; } QString PartitionTable::tableTypeToName(TableType l) diff --git a/src/jobs/createpartitionjob.cpp b/src/jobs/createpartitionjob.cpp index 15c4008..435ff88 100644 --- a/src/jobs/createpartitionjob.cpp +++ b/src/jobs/createpartitionjob.cpp @@ -1,6 +1,6 @@ /* SPDX-FileCopyrightText: 2008-2010 Volker Lanz - SPDX-FileCopyrightText: 2013-2018 Andrius Štikonas + SPDX-FileCopyrightText: 2013-2020 Andrius Štikonas SPDX-FileCopyrightText: 2016 Chantara Tith SPDX-FileCopyrightText: 2018 Caio Jordão Carvalho SPDX-FileCopyrightText: 2019 Yuri Chornoivan @@ -40,9 +40,16 @@ bool CreatePartitionJob::run(Report& parent) Q_ASSERT(partition().devicePath() == device().deviceNode()); bool rval = false; - Report* report = jobStarted(parent); + if (device().partitionTable()->type() == PartitionTable::TableType::none) { + partition().setPartitionPath(device().deviceNode()); + partition().setState(Partition::State::None); + rval = true; + jobFinished(*report, rval); + return rval; + } + if (device().type() == Device::Type::Disk_Device || device().type() == Device::Type::SoftwareRAID_Device) { std::unique_ptr backendDevice = CoreBackendManager::self()->backend()->openDevice(device()); diff --git a/src/jobs/createpartitiontablejob.cpp b/src/jobs/createpartitiontablejob.cpp index bfb18ca..41153b4 100644 --- a/src/jobs/createpartitiontablejob.cpp +++ b/src/jobs/createpartitiontablejob.cpp @@ -1,6 +1,6 @@ /* SPDX-FileCopyrightText: 2008-2010 Volker Lanz - SPDX-FileCopyrightText: 2014-2018 Andrius Štikonas + SPDX-FileCopyrightText: 2014-2020 Andrius Štikonas SPDX-FileCopyrightText: 2016 Chantara Tith SPDX-FileCopyrightText: 2018 Caio Jordão Carvalho @@ -35,6 +35,9 @@ bool CreatePartitionTableJob::run(Report& parent) Report* report = jobStarted(parent); + if (device().partitionTable()->type() == PartitionTable::TableType::none) + return true; + if (device().type() == Device::Type::Disk_Device || device().type() == Device::Type::SoftwareRAID_Device) { std::unique_ptr backendDevice = CoreBackendManager::self()->backend()->openDevice(device()); From 924830ebbd9e22d3b1c88181886b1f609aff7756 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andrius=20=C5=A0tikonas?= Date: Sun, 4 Oct 2020 18:33:58 +0100 Subject: [PATCH 02/16] When falling back to blkid for filesystem type detection, distinguish between FAT16/32. CCBUG: 418253 --- src/plugins/sfdisk/sfdiskbackend.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/plugins/sfdisk/sfdiskbackend.cpp b/src/plugins/sfdisk/sfdiskbackend.cpp index 102cc90..fd059b3 100644 --- a/src/plugins/sfdisk/sfdiskbackend.cpp +++ b/src/plugins/sfdisk/sfdiskbackend.cpp @@ -560,7 +560,7 @@ FileSystem::Type SfdiskBackend::fileSystemNameToType(const QString& name, const else if (name == QStringLiteral("vfat")) { if (version == QStringLiteral("FAT32")) rval = FileSystem::Type::Fat32; - else if (version == QStringLiteral("FAT16")) + else if (version == QStringLiteral("FAT16") || version == QStringLiteral("msdos")) // blkid uses msdos for both FAT16 and FAT12 rval = FileSystem::Type::Fat16; else if (version == QStringLiteral("FAT12")) rval = FileSystem::Type::Fat12; From 4819eec59b96bc73e5444f8bf29365884d647327 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andrius=20=C5=A0tikonas?= Date: Sun, 4 Oct 2020 21:18:08 +0100 Subject: [PATCH 03/16] Allow moving partitions with unknown file system. BUG: 404398 --- src/fs/unknown.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/fs/unknown.cpp b/src/fs/unknown.cpp index 6df09f7..156862c 100644 --- a/src/fs/unknown.cpp +++ b/src/fs/unknown.cpp @@ -12,7 +12,7 @@ namespace FS { -FileSystem::CommandSupportType unknown::m_Move = FileSystem::cmdSupportNone; +FileSystem::CommandSupportType unknown::m_Move = FileSystem::cmdSupportCore; unknown::unknown(qint64 firstsector, qint64 lastsector, qint64 sectorsused, const QString& label, const QVariantMap& features) : FileSystem(firstsector, lastsector, sectorsused, label, features, FileSystem::Type::Unknown) From 7ecdaf3ac8f63de014fce26e95444f4a420ffcb3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andrius=20=C5=A0tikonas?= Date: Sun, 4 Oct 2020 22:21:58 +0100 Subject: [PATCH 04/16] Fix resizing of fat partitions. Workaround a bug in fatresize (or libparted) where resizing to partition length failed. Instead try to resize to length - 1. BUG: 407834 --- src/fs/fat16.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/fs/fat16.cpp b/src/fs/fat16.cpp index e225178..7f79b2c 100644 --- a/src/fs/fat16.cpp +++ b/src/fs/fat16.cpp @@ -81,7 +81,7 @@ bool fat16::create(Report& report, const QString& deviceNode) bool fat16::resize(Report& report, const QString& deviceNode, qint64 length) const { - ExternalCommand cmd(report, QStringLiteral("fatresize"), { QStringLiteral("--verbose"), QStringLiteral("--size"), QString::number(length), deviceNode }); + ExternalCommand cmd(report, QStringLiteral("fatresize"), { QStringLiteral("--verbose"), QStringLiteral("--size"), QString::number(length - 1), deviceNode }); return cmd.run(-1) && cmd.exitCode() == 0; } From 1fb5a123b575e3c1cf4bef14c12ddac1c1b95c7a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andrius=20=C5=A0tikonas?= Date: Mon, 5 Oct 2020 22:00:33 +0100 Subject: [PATCH 05/16] SPDX: Use CC0 for .gitignore. --- .gitignore | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.gitignore b/.gitignore index 39e5dd6..5838c04 100644 --- a/.gitignore +++ b/.gitignore @@ -1,5 +1,5 @@ -# SPDX-FileCopyrightText: 2020 Andrius Štikonas -# SPDX-License-Identifier: MIT +# SPDX-FileCopyrightText: none +# SPDX-License-Identifier: CC0-1.0 build .kdev4/ From 08c1acb6b408c7be3a1778e4de52345f9cb7720e Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Sat, 10 Oct 2020 21:59:57 +0200 Subject: [PATCH 06/16] Missing include (clang/FreeBSD triggers this) --- src/core/fstab.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/core/fstab.cpp b/src/core/fstab.cpp index 2f18200..16b94fc 100644 --- a/src/core/fstab.cpp +++ b/src/core/fstab.cpp @@ -10,6 +10,7 @@ #include "util/report.h" #include +#include #if defined(Q_OS_LINUX) #include From edfabb90dcdefe0065f4557634027675d359917f Mon Sep 17 00:00:00 2001 From: David Edmundson Date: Thu, 8 Oct 2020 02:23:24 +0100 Subject: [PATCH 07/16] WIP: port to polkit --- CMakeLists.txt | 1 + src/util/CMakeLists.txt | 11 +- src/util/externalcommand.cpp | 47 ++---- src/util/externalcommand.h | 9 -- src/util/externalcommandhelper.cpp | 136 ++++++++++++------ src/util/externalcommandhelper.h | 14 +- src/util/org.kde.kpmcore.helperinterface.conf | 2 +- ...org.kde.kpmcore.helperinterface.service.in | 4 + 8 files changed, 123 insertions(+), 101 deletions(-) create mode 100644 src/util/org.kde.kpmcore.helperinterface.service.in diff --git a/CMakeLists.txt b/CMakeLists.txt index 4408e97..48537de 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -39,6 +39,7 @@ include(KDECompilerSettings NO_POLICY_SCOPE) include(FeatureSummary) include(GenerateExportHeader) include(ECMSetupVersion) +include(ECMConfiguredInstall) ecm_setup_version(${VERSION} VARIABLE_PREFIX KPMCORE VERSION_HEADER "${CMAKE_CURRENT_BINARY_DIR}/kpmcore_version.h" diff --git a/src/util/CMakeLists.txt b/src/util/CMakeLists.txt index fab7a70..cb45508 100644 --- a/src/util/CMakeLists.txt +++ b/src/util/CMakeLists.txt @@ -23,6 +23,9 @@ qt5_generate_dbus_interface( OPTIONS -a ) + +find_package(PolkitQt5-1 REQUIRED) + qt5_add_dbus_interface(ApplicationInterface_SRCS ${CMAKE_CURRENT_BINARY_DIR}/${application_interface_xml} externalcommand_interface) qt5_add_dbus_interface(HelperInterface_SRCS ${CMAKE_CURRENT_BINARY_DIR}/${helper_interface_xml} externalcommandhelper_interface) @@ -57,11 +60,15 @@ target_link_libraries(kpmcore_externalcommand Qt5::DBus KF5::AuthCore KF5::I18n + PolkitQt5-1::Core ) -install(TARGETS kpmcore_externalcommand DESTINATION ${KAUTH_HELPER_INSTALL_DIR}) +install(TARGETS kpmcore_externalcommand DESTINATION ${KDE_INSTALL_LIBEXECDIR}) install( FILES util/org.kde.kpmcore.helperinterface.conf DESTINATION ${KDE_INSTALL_DBUSDIR}/system.d ) install( FILES util/org.kde.kpmcore.applicationinterface.conf DESTINATION ${KDE_INSTALL_DBUSDIR}/system.d ) -kauth_install_helper_files(kpmcore_externalcommand org.kde.kpmcore.externalcommand root) kauth_install_actions(org.kde.kpmcore.externalcommand util/org.kde.kpmcore.externalcommand.actions) +ecm_install_configured_files( + INPUT util/org.kde.kpmcore.helperinterface.service.in + DESTINATION ${KDE_INSTALL_DBUSDIR}/system-services +) diff --git a/src/util/externalcommand.cpp b/src/util/externalcommand.cpp index ef3de03..99170a1 100644 --- a/src/util/externalcommand.cpp +++ b/src/util/externalcommand.cpp @@ -49,7 +49,6 @@ struct ExternalCommandPrivate int m_ExitCode; QByteArray m_Output; QByteArray m_Input; - DBusThread *m_thread; QProcess::ProcessChannelMode processChannelMode; }; @@ -71,9 +70,9 @@ ExternalCommand::ExternalCommand(const QString& cmd, const QStringList& args, co d->m_ExitCode = -1; d->m_Output = QByteArray(); - if (!helperStarted) - if(!startHelper()) - Log(Log::Level::error) << xi18nc("@info:status", "Could not obtain administrator privileges."); +// if (!helperStarted) +// if(!startHelper()) +// Log(Log::Level::error) << xi18nc("@info:status", "Could not obtain administrator privileges."); d->processChannelMode = processChannelMode; } @@ -129,6 +128,8 @@ bool ExternalCommand::start(int timeout) if (cmd.isEmpty()) cmd = QStandardPaths::findExecutable(command(), { QStringLiteral("/sbin/"), QStringLiteral("/usr/sbin/"), QStringLiteral("/usr/local/sbin/") }); + qDebug() << "start"; + auto interface = helperInterface(); if (!interface) return false; @@ -231,7 +232,7 @@ OrgKdeKpmcoreExternalcommandInterface* ExternalCommand::helperInterface() return nullptr; } - auto *interface = new org::kde::kpmcore::externalcommand(QStringLiteral("org.kde.kpmcore.externalcommand"), + auto *interface = new org::kde::kpmcore::externalcommand(QStringLiteral("org.kde.kpmcore.helperinterface"), QStringLiteral("/Helper"), QDBusConnection::systemBus(), this); interface->setTimeout(10 * 24 * 3600 * 1000); // 10 days return interface; @@ -355,46 +356,14 @@ bool ExternalCommand::startHelper() exit(0); } - d->m_thread = new DBusThread; - d->m_thread->start(); - - KAuth::Action action = KAuth::Action(QStringLiteral("org.kde.kpmcore.externalcommand.init")); - action.setHelperId(QStringLiteral("org.kde.kpmcore.externalcommand")); - action.setTimeout(10 * 24 * 3600 * 1000); // 10 days - action.setParentWidget(parent); - QVariantMap arguments; - action.setArguments(arguments); - m_job = action.execute(); - m_job->start(); - - // Wait until ExternalCommand Helper is ready (helper sends newData signal just before it enters event loop) - QEventLoop loop; - auto exitLoop = [&] () { loop.exit(); }; - auto conn = QObject::connect(m_job, &KAuth::ExecuteJob::newData, exitLoop); - QObject::connect(m_job, &KJob::finished, [=] () { if(m_job->error()) exitLoop(); } ); - loop.exec(); - QObject::disconnect(conn); - - helperStarted = true; + qDebug() <<"starting helper"; return true; } void ExternalCommand::stopHelper() { - auto *interface = new org::kde::kpmcore::externalcommand(QStringLiteral("org.kde.kpmcore.externalcommand"), + auto *interface = new org::kde::kpmcore::externalcommand(QStringLiteral("org.kde.kpmcore.helperinterface"), QStringLiteral("/Helper"), QDBusConnection::systemBus()); interface->exit(); } - -void DBusThread::run() -{ - if (!QDBusConnection::systemBus().registerService(QStringLiteral("org.kde.kpmcore.applicationinterface")) || - !QDBusConnection::systemBus().registerObject(QStringLiteral("/Application"), this, QDBusConnection::ExportAllSlots)) { - qWarning() << QDBusConnection::systemBus().lastError().message(); - return; - } - - QEventLoop loop; - loop.exec(); -} diff --git a/src/util/externalcommand.h b/src/util/externalcommand.h index 8bbdaa4..02b5537 100644 --- a/src/util/externalcommand.h +++ b/src/util/externalcommand.h @@ -36,15 +36,6 @@ class OrgKdeKpmcoreExternalcommandInterface; struct ExternalCommandPrivate; -class DBusThread : public QThread -{ - Q_OBJECT - // We register on DBus so the helper can monitor us and terminate if we - // terminate. - Q_CLASSINFO("D-Bus Interface", "org.kde.kpmcore.applicationinterface") - void run() override; -}; - /** An external command. Runs an external command as a child process. diff --git a/src/util/externalcommandhelper.cpp b/src/util/externalcommandhelper.cpp index b84fc7c..fe445ae 100644 --- a/src/util/externalcommandhelper.cpp +++ b/src/util/externalcommandhelper.cpp @@ -13,6 +13,7 @@ #include "externalcommand_whitelist.h" #include + #include #include #include @@ -21,6 +22,10 @@ #include #include +#include +#include + +#include /** Initialize ExternalCommandHelper Daemon and prepare DBus interface * @@ -35,52 +40,34 @@ * This helper also starts another DBus interface where it listens to * command execution requests from the application that started the helper. * + * + * DAVE - this all needs updating + * */ -ActionReply ExternalCommandHelper::init(const QVariantMap& args) + +ExternalCommandHelper::ExternalCommandHelper() { - Q_UNUSED(args) - - ActionReply reply; - - if (!QDBusConnection::systemBus().isConnected() || !QDBusConnection::systemBus().registerService(QStringLiteral("org.kde.kpmcore.helperinterface")) || - !QDBusConnection::systemBus().registerObject(QStringLiteral("/Helper"), this, QDBusConnection::ExportAllSlots)) { - qWarning() << QDBusConnection::systemBus().lastError().message(); - reply.addData(QStringLiteral("success"), false); - - // Also end the application loop started by KAuth's main() code. Our loop - // exits when our client disappears. Without client we have no reason to - // live. - qApp->quit(); - - return reply; + if (!QDBusConnection::systemBus().registerObject(QStringLiteral("/Helper"), this, QDBusConnection::ExportAllSlots)) { + ::exit(-1); } - - m_loop = std::make_unique(); - HelperSupport::progressStep(QVariantMap()); - // End the loop and return only once the client is done using us. - auto serviceWatcher = - new QDBusServiceWatcher(QStringLiteral("org.kde.kpmcore.applicationinterface"), - QDBusConnection::systemBus(), - QDBusServiceWatcher::WatchForUnregistration, - this); - connect(serviceWatcher, &QDBusServiceWatcher::serviceUnregistered, - [this]() { - m_loop->exit(); + if (!QDBusConnection::systemBus().registerService(QStringLiteral("org.kde.kpmcore.helperinterface"))) { + ::exit(-1); + } + + // we know this service must be registered already as DBus policy blocks calls from anyone else + m_serviceWatcher = new QDBusServiceWatcher(this); + m_serviceWatcher->setConnection(QDBusConnection ::systemBus()); + m_serviceWatcher->setWatchMode(QDBusServiceWatcher::WatchForUnregistration); + + connect(m_serviceWatcher, &QDBusServiceWatcher::serviceUnregistered, qApp, [this](const QString &service) { + m_serviceWatcher->removeWatchedService(service); + if (m_serviceWatcher->watchedServices().isEmpty()) { + qApp->quit(); + } }); - - m_loop->exec(); - reply.addData(QStringLiteral("success"), true); - - // Also end the application loop started by KAuth's main() code. Our loop - // exits when our client disappears. Without client we have no reason to - // live. - qApp->quit(); - - return reply; } - /** Reads the given number of bytes from the sourceDevice into the given buffer. @param sourceDevice device or file to read from @param buffer buffer to store the bytes read in @@ -148,6 +135,9 @@ bool ExternalCommandHelper::writeData(const QString &targetDevice, const QByteAr */ bool ExternalCommandHelper::createFile(const QString &filePath, const QByteArray& fileContents) { + if (!isCallerAuthorized()) { + return false; + } QFile device(filePath); auto flags = QIODevice::WriteOnly | QIODevice::Unbuffered; @@ -167,6 +157,9 @@ bool ExternalCommandHelper::createFile(const QString &filePath, const QByteArray // If targetDevice is empty then return QByteArray with data that was read from disk. QVariantMap ExternalCommandHelper::copyblocks(const QString& sourceDevice, const qint64 sourceFirstByte, const qint64 sourceLength, const QString& targetDevice, const qint64 targetFirstByte, const qint64 blockSize) { + if (!isCallerAuthorized()) { + return QVariantMap(); + } QVariantMap reply; reply[QStringLiteral("success")] = true; @@ -256,6 +249,9 @@ QVariantMap ExternalCommandHelper::copyblocks(const QString& sourceDevice, const bool ExternalCommandHelper::writeData(const QByteArray& buffer, const QString& targetDevice, const qint64 targetFirstByte) { + if (!isCallerAuthorized()) { + return false; + } // Do not allow using this helper for writing to arbitrary location if ( targetDevice.left(5) != QStringLiteral("/dev/") ) return false; @@ -265,6 +261,9 @@ bool ExternalCommandHelper::writeData(const QByteArray& buffer, const QString& t bool ExternalCommandHelper::createFile(const QByteArray& fileContents, const QString& filePath) { + if (!isCallerAuthorized()) { + return false; + } // Do not allow using this helper for writing to arbitrary location if ( !filePath.contains(QStringLiteral("/etc/fstab")) ) return false; @@ -274,6 +273,9 @@ bool ExternalCommandHelper::createFile(const QByteArray& fileContents, const QSt QVariantMap ExternalCommandHelper::start(const QString& command, const QStringList& arguments, const QByteArray& input, const int processChannelMode) { + if (!isCallerAuthorized()) { + return QVariantMap(); + } QTextCodec::setCodecForLocale(QTextCodec::codecForName("UTF-8")); QVariantMap reply; reply[QStringLiteral("success")] = true; @@ -287,7 +289,7 @@ QVariantMap ExternalCommandHelper::start(const QString& command, const QStringLi QString basename = command.mid(command.lastIndexOf(QLatin1Char('/')) + 1); if (std::find(std::begin(allowedCommands), std::end(allowedCommands), basename) == std::end(allowedCommands)) { qInfo() << command <<" command is not one of the whitelisted command"; - m_loop->exit(); + qApp->quit(); reply[QStringLiteral("success")] = false; return reply; } @@ -309,10 +311,10 @@ QVariantMap ExternalCommandHelper::start(const QString& command, const QStringLi void ExternalCommandHelper::exit() { - m_loop->exit(); - - QDBusConnection::systemBus().unregisterObject(QStringLiteral("/Helper")); - QDBusConnection::systemBus().unregisterService(QStringLiteral("org.kde.kpmcore.helperinterface")); + if (!isCallerAuthorized()) { + return; + } + qApp->quit(); } void ExternalCommandHelper::onReadOutput() @@ -331,4 +333,48 @@ void ExternalCommandHelper::onReadOutput() *report() << QString::fromLocal8Bit(s);*/ } -KAUTH_HELPER_MAIN("org.kde.kpmcore.externalcommand", ExternalCommandHelper) +bool ExternalCommandHelper::isCallerAuthorized() +{ + if (!calledFromDBus()) { + return false; + } + + // track who called into us so we can close when all callers have gone away + // this has to happen before authorisation as anyone could have activated us + m_serviceWatcher->addWatchedService(message().service()); + + PolkitQt1::SystemBusNameSubject subject(message().service()); + PolkitQt1::Authority *authority = PolkitQt1::Authority::instance(); + + PolkitQt1::Authority::Result result; + QEventLoop e; + connect(authority, &PolkitQt1::Authority::checkAuthorizationFinished, [&result, &e](PolkitQt1::Authority::Result _result) { + result = _result; + e.quit(); + }); + + authority->checkAuthorization(QStringLiteral("org.kde.kpmcore.externalcommand.init"), subject, PolkitQt1::Authority::AllowUserInteraction); + e.exec(); + + if (authority->hasError()) { + qDebug() << "Encountered error while checking authorization, error code:" << authority->lastError() << authority->errorDetails(); + authority->clearError(); + } + + switch (result) { + case PolkitQt1::Authority::Yes: + return true; + default: + sendErrorReply(QDBusError::AccessDenied); + return false; + } +} + +int main(int argc, char ** argv) +{ + QCoreApplication app(argc, argv); + ExternalCommandHelper helper; + app.exec(); +} + +#include "externalcommandhelper.moc" diff --git a/src/util/externalcommandhelper.h b/src/util/externalcommandhelper.h index b910cf1..6a452b6 100644 --- a/src/util/externalcommandhelper.h +++ b/src/util/externalcommandhelper.h @@ -18,10 +18,13 @@ #include #include #include +#include + +class QDBusServiceWatcher; using namespace KAuth; -class ExternalCommandHelper : public QObject +class ExternalCommandHelper : public QObject, public QDBusContext { Q_OBJECT Q_CLASSINFO("D-Bus Interface", "org.kde.kpmcore.externalcommand") @@ -31,12 +34,12 @@ Q_SIGNALS: void quit(); public: + ExternalCommandHelper(); bool readData(const QString& sourceDevice, QByteArray& buffer, const qint64 offset, const qint64 size); bool writeData(const QString& targetDevice, const QByteArray& buffer, const qint64 offset); bool createFile(const QString& filePath, const QByteArray& fileContents); public Q_SLOTS: - ActionReply init(const QVariantMap& args); Q_SCRIPTABLE QVariantMap start(const QString& command, const QStringList& arguments, const QByteArray& input, const int processChannelMode); Q_SCRIPTABLE QVariantMap copyblocks(const QString& sourceDevice, const qint64 sourceFirstByte, const qint64 sourceLength, const QString& targetDevice, const qint64 targetFirstByte, const qint64 blockSize); Q_SCRIPTABLE bool writeData(const QByteArray& buffer, const QString& targetDevice, const qint64 targetFirstByte); @@ -44,11 +47,12 @@ public Q_SLOTS: Q_SCRIPTABLE void exit(); private: - void onReadOutput(); - std::unique_ptr m_loop; + bool isCallerAuthorized(); + + void onReadOutput(); QProcess m_cmd; -// QByteArray output; + QDBusServiceWatcher *m_serviceWatcher = nullptr; }; #endif diff --git a/src/util/org.kde.kpmcore.helperinterface.conf b/src/util/org.kde.kpmcore.helperinterface.conf index 990dadf..069015e 100644 --- a/src/util/org.kde.kpmcore.helperinterface.conf +++ b/src/util/org.kde.kpmcore.helperinterface.conf @@ -9,7 +9,7 @@ - diff --git a/src/util/org.kde.kpmcore.helperinterface.service.in b/src/util/org.kde.kpmcore.helperinterface.service.in new file mode 100644 index 0000000..e448fe1 --- /dev/null +++ b/src/util/org.kde.kpmcore.helperinterface.service.in @@ -0,0 +1,4 @@ +[D-BUS Service] +Name=org.kde.kpmcore.helperinterface +Exec=@KDE_INSTALL_FULL_LIBEXECDIR@/kpmcore_externalcommand +User=root From 296f281ffb562c5d8024ff1b6217c0d8dc0aeac4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andrius=20=C5=A0tikonas?= Date: Fri, 9 Oct 2020 20:41:41 +0100 Subject: [PATCH 08/16] WIP: cleanup polkit port: licenses, dead code and kauth removal, add TODO about signals. --- CMakeLists.txt | 1 + src/util/CMakeLists.txt | 1 + src/util/externalcommand.cpp | 38 ++----------------- src/util/externalcommand.h | 8 ---- src/util/externalcommandhelper.cpp | 15 ++++---- src/util/externalcommandhelper.h | 5 +-- src/util/helpers.cpp | 1 + ...org.kde.kpmcore.helperinterface.service.in | 2 + test/helpers.cpp | 4 -- test/helpers.h | 1 - 10 files changed, 18 insertions(+), 58 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 48537de..17a9f9e 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1,6 +1,7 @@ # SPDX-FileCopyrightText: 2008 Volker Lanz # SPDX-FileCopyrightText: 2015 Teo Mrnjavac # SPDX-FileCopyrightText: 2014-2020 Andrius Štikonas +# SPDX-FileCopyrightText: 2020 David Edmundson # SPDX-License-Identifier: GPL-3.0-or-later diff --git a/src/util/CMakeLists.txt b/src/util/CMakeLists.txt index cb45508..535ae3b 100644 --- a/src/util/CMakeLists.txt +++ b/src/util/CMakeLists.txt @@ -5,6 +5,7 @@ # SPDX-FileCopyrightText: 2018 Huzaifa Faruqui # SPDX-FileCopyrightText: 2019 Albert Astals Cid # SPDX-FileCopyrightText: 2019 Antonio Rojas +# SPDX-FileCopyrightText: 2020 David Edmundson # SPDX-License-Identifier: GPL-3.0-or-later diff --git a/src/util/externalcommand.cpp b/src/util/externalcommand.cpp index 99170a1..21e8123 100644 --- a/src/util/externalcommand.cpp +++ b/src/util/externalcommand.cpp @@ -7,6 +7,7 @@ SPDX-FileCopyrightText: 2018 Caio Jordão Carvalho SPDX-FileCopyrightText: 2019 Shubham Jangra SPDX-FileCopyrightText: 2019 Yuri Chornoivan + SPDX-FileCopyrightText: 2020 David Edmundson SPDX-License-Identifier: GPL-3.0-or-later */ @@ -53,7 +54,6 @@ struct ExternalCommandPrivate }; KAuth::ExecuteJob* ExternalCommand::m_job; -bool ExternalCommand::helperStarted = false; QWidget* ExternalCommand::parent; @@ -69,11 +69,6 @@ ExternalCommand::ExternalCommand(const QString& cmd, const QStringList& args, co d->m_Args = args; d->m_ExitCode = -1; d->m_Output = QByteArray(); - -// if (!helperStarted) -// if(!startHelper()) -// Log(Log::Level::error) << xi18nc("@info:status", "Could not obtain administrator privileges."); - d->processChannelMode = processChannelMode; } @@ -128,8 +123,6 @@ bool ExternalCommand::start(int timeout) if (cmd.isEmpty()) cmd = QStandardPaths::findExecutable(command(), { QStringLiteral("/sbin/"), QStringLiteral("/usr/sbin/"), QStringLiteral("/usr/local/sbin/") }); - qDebug() << "start"; - auto interface = helperInterface(); if (!interface) return false; @@ -167,8 +160,9 @@ bool ExternalCommand::copyBlocks(const CopySource& source, CopyTarget& target) const qint64 blockSize = 10 * 1024 * 1024; // number of bytes per block to copy // TODO KF6:Use new signal-slot syntax - connect(m_job, SIGNAL(percent(KJob*, unsigned long)), this, SLOT(emitProgress(KJob*, unsigned long))); - connect(m_job, &KAuth::ExecuteJob::newData, this, &ExternalCommand::emitReport); + // FIXME: port and reenable these signals + //connect(m_job, SIGNAL(percent(KJob*, unsigned long)), this, SLOT(emitProgress(KJob*, unsigned long))); + //connect(m_job, &KAuth::ExecuteJob::newData, this, &ExternalCommand::emitReport); auto interface = helperInterface(); if (!interface) @@ -343,27 +337,3 @@ void ExternalCommand::setExitCode(int i) { d->m_ExitCode = i; } - -bool ExternalCommand::startHelper() -{ - if (!QDBusConnection::systemBus().isConnected()) { - qWarning() << QDBusConnection::systemBus().lastError().message(); - return false; - } - - QDBusInterface iface(QStringLiteral("org.kde.kpmcore.helperinterface"), QStringLiteral("/Helper"), QStringLiteral("org.kde.kpmcore.externalcommand"), QDBusConnection::systemBus()); - if (iface.isValid()) { - exit(0); - } - - qDebug() <<"starting helper"; - return true; -} - -void ExternalCommand::stopHelper() -{ - auto *interface = new org::kde::kpmcore::externalcommand(QStringLiteral("org.kde.kpmcore.helperinterface"), - QStringLiteral("/Helper"), QDBusConnection::systemBus()); - interface->exit(); - -} diff --git a/src/util/externalcommand.h b/src/util/externalcommand.h index 02b5537..224ebb4 100644 --- a/src/util/externalcommand.h +++ b/src/util/externalcommand.h @@ -91,13 +91,6 @@ public: void emitReport(const QVariantMap& report) { Q_EMIT reportSignal(report); } - // KAuth - /**< start ExternalCommand Helper */ - bool startHelper(); - - /**< stop ExternalCommand Helper */ - static void stopHelper(); - /**< Sets a parent widget for the authentication dialog. * @param p parent widget */ @@ -123,7 +116,6 @@ private: // KAuth static KAuth::ExecuteJob *m_job; - static bool helperStarted; static QWidget *parent; }; diff --git a/src/util/externalcommandhelper.cpp b/src/util/externalcommandhelper.cpp index fe445ae..4053d17 100644 --- a/src/util/externalcommandhelper.cpp +++ b/src/util/externalcommandhelper.cpp @@ -5,6 +5,7 @@ SPDX-FileCopyrightText: 2018-2019 Harald Sitter SPDX-FileCopyrightText: 2018 Simon Depiets SPDX-FileCopyrightText: 2019 Shubham Jangra + SPDX-FileCopyrightText: 2020 David Edmundson SPDX-License-Identifier: GPL-3.0-or-later */ @@ -191,7 +192,7 @@ QVariantMap ExternalCommandHelper::copyblocks(const QString& sourceDevice, const sourceLength, readOffset, writeOffset, copyDirection == 1 ? i18nc("direction: left", "left") : i18nc("direction: right", "right")); - HelperSupport::progressStep(report); + //HelperSupport::progressStep(report); bool rval = true; @@ -211,9 +212,9 @@ QVariantMap ExternalCommandHelper::copyblocks(const QString& sourceDevice, const const qint64 mibsPerSec = (blocksCopied * blockSize / 1024 / 1024) / (timer.elapsed() / 1000); const qint64 estSecsLeft = (100 - percent) * timer.elapsed() / percent / 1000; report[QStringLiteral("report")]= xi18nc("@info:progress", "Copying %1 MiB/second, estimated time left: %2", mibsPerSec, QTime(0, 0).addSecs(estSecsLeft).toString()); - HelperSupport::progressStep(report); + //HelperSupport::progressStep(report); } - HelperSupport::progressStep(percent); + //HelperSupport::progressStep(percent); } } @@ -224,7 +225,7 @@ QVariantMap ExternalCommandHelper::copyblocks(const QString& sourceDevice, const const qint64 lastBlockReadOffset = copyDirection > 0 ? readOffset + blockSize * blocksCopied : sourceFirstByte; const qint64 lastBlockWriteOffset = copyDirection > 0 ? writeOffset + blockSize * blocksCopied : targetFirstByte; report[QStringLiteral("report")]= xi18nc("@info:progress", "Copying remainder of block size %1 from %2 to %3.", lastBlock, lastBlockReadOffset, lastBlockWriteOffset); - HelperSupport::progressStep(report); + //HelperSupport::progressStep(report); rval = readData(sourceDevice, buffer, lastBlockReadOffset, lastBlock); if (rval) { @@ -235,13 +236,13 @@ QVariantMap ExternalCommandHelper::copyblocks(const QString& sourceDevice, const } if (rval) { - HelperSupport::progressStep(100); + //HelperSupport::progressStep(100); bytesWritten += buffer.size(); } } report[QStringLiteral("report")] = xi18ncp("@info:progress argument 2 is a string such as 7 bytes (localized accordingly)", "Copying 1 block (%2) finished.", "Copying %1 blocks (%2) finished.", blocksCopied, i18np("1 byte", "%1 bytes", bytesWritten)); - HelperSupport::progressStep(report); + //HelperSupport::progressStep(report); reply[QStringLiteral("success")] = rval; return reply; @@ -377,4 +378,4 @@ int main(int argc, char ** argv) app.exec(); } -#include "externalcommandhelper.moc" +// #include "externalcommandhelper.moc" diff --git a/src/util/externalcommandhelper.h b/src/util/externalcommandhelper.h index 6a452b6..d36447a 100644 --- a/src/util/externalcommandhelper.h +++ b/src/util/externalcommandhelper.h @@ -3,6 +3,7 @@ SPDX-FileCopyrightText: 2018 Huzaifa Faruqui SPDX-FileCopyrightText: 2018 Caio Jordão Carvalho SPDX-FileCopyrightText: 2019 Shubham Jangra + SPDX-FileCopyrightText: 2020 David Edmundson SPDX-License-Identifier: GPL-3.0-or-later */ @@ -13,8 +14,6 @@ #include #include -#include - #include #include #include @@ -22,8 +21,6 @@ class QDBusServiceWatcher; -using namespace KAuth; - class ExternalCommandHelper : public QObject, public QDBusContext { Q_OBJECT diff --git a/src/util/helpers.cpp b/src/util/helpers.cpp index 365327d..a531c0c 100644 --- a/src/util/helpers.cpp +++ b/src/util/helpers.cpp @@ -60,6 +60,7 @@ KAboutData aboutKPMcore() aboutData.addCredit(xi18nc("@info:credit", "Pali Rohár"), i18nc("@info:credit", "UDF support"), QStringLiteral("pali.rohar@gmail.com")); aboutData.addCredit(xi18nc("@info:credit", "Adriaan de Groot"), i18nc("@info:credit", "Calamares maintainer"), QStringLiteral("groot@kde.org")); aboutData.addCredit(xi18nc("@info:credit", "Caio Jordão Carvalho"), i18nc("@info:credit", "Improved SMART support"), QStringLiteral("caiojcarvalho@gmail.com")); + aboutData.addCredit(xi18nc("@info:credit", "David Edmundson"), i18nc("@info:credit", "Port from KAuth to Polkit"), QStringLiteral("kde@davidedmundson.co.uk")); return aboutData; } diff --git a/src/util/org.kde.kpmcore.helperinterface.service.in b/src/util/org.kde.kpmcore.helperinterface.service.in index e448fe1..c3da3f1 100644 --- a/src/util/org.kde.kpmcore.helperinterface.service.in +++ b/src/util/org.kde.kpmcore.helperinterface.service.in @@ -1,3 +1,5 @@ +# SPDX-FileCopyrightText: 2020 David Edmundson +# SPDX-License-Identifier: GPL-3.0-or-later [D-BUS Service] Name=org.kde.kpmcore.helperinterface Exec=@KDE_INSTALL_FULL_LIBEXECDIR@/kpmcore_externalcommand diff --git a/test/helpers.cpp b/test/helpers.cpp index 0187cd8..1bd2f8d 100644 --- a/test/helpers.cpp +++ b/test/helpers.cpp @@ -46,7 +46,3 @@ KPMCoreInitializer::KPMCoreInitializer( const char* backend ) : KPMCoreInitializ { } -KPMCoreInitializer::~KPMCoreInitializer() -{ - ExternalCommand::stopHelper(); -} diff --git a/test/helpers.h b/test/helpers.h index 9e762af..82b1341 100644 --- a/test/helpers.h +++ b/test/helpers.h @@ -20,7 +20,6 @@ public: KPMCoreInitializer(); /// Default backend KPMCoreInitializer( const QString& backend ); /// Use named backend KPMCoreInitializer( const char* backend ); /// Use named backend - ~KPMCoreInitializer(); bool isValid() const { From f646b507aeee1d084da1a948c03241d095eddf98 Mon Sep 17 00:00:00 2001 From: David Edmundson Date: Sun, 11 Oct 2020 21:17:09 +0100 Subject: [PATCH 09/16] Fix a crash. --- src/util/externalcommandhelper.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/externalcommandhelper.cpp b/src/util/externalcommandhelper.cpp index 4053d17..0d99889 100644 --- a/src/util/externalcommandhelper.cpp +++ b/src/util/externalcommandhelper.cpp @@ -349,7 +349,7 @@ bool ExternalCommandHelper::isCallerAuthorized() PolkitQt1::Authority::Result result; QEventLoop e; - connect(authority, &PolkitQt1::Authority::checkAuthorizationFinished, [&result, &e](PolkitQt1::Authority::Result _result) { + connect(authority, &PolkitQt1::Authority::checkAuthorizationFinished, &e, [&e, &result](PolkitQt1::Authority::Result _result) { result = _result; e.quit(); }); From 424c8f0bf0a07e7d01a58e966a54624c611815ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andrius=20=C5=A0tikonas?= Date: Sun, 11 Oct 2020 22:19:14 +0100 Subject: [PATCH 10/16] Update comment describing kpmcore_externalcommandhelper. --- src/util/externalcommandhelper.cpp | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/src/util/externalcommandhelper.cpp b/src/util/externalcommandhelper.cpp index 0d99889..8cede39 100644 --- a/src/util/externalcommandhelper.cpp +++ b/src/util/externalcommandhelper.cpp @@ -30,20 +30,13 @@ /** Initialize ExternalCommandHelper Daemon and prepare DBus interface * - * KAuth helper runs in the background until application exits. - * To avoid forever running helper in case of application crash - * ExternalCommand class opens a DBus service that we monitor for changes. + * This helper runs in the background until all applications using it exit. * If helper is not busy then it exits when the client services gets - * unregistered. Otherwise, - * we wait for the current job to finish before exiting, so even in case - * of main application crash, we do not leave partially moved data. - * - * This helper also starts another DBus interface where it listens to - * command execution requests from the application that started the helper. - * - * - * DAVE - this all needs updating + * unregistered. In case the client crashes, the helper waits + * for the current job to finish before exiting, to avoid leaving partially moved data. * + * This helper starts DBus interface where it listens to command execution requests. + * New clients connecting to the helper have to authenticate using Polkit. */ ExternalCommandHelper::ExternalCommandHelper() @@ -377,5 +370,3 @@ int main(int argc, char ** argv) ExternalCommandHelper helper; app.exec(); } - -// #include "externalcommandhelper.moc" From 89bd4eb6c764480ff5301691cdbb689ffd45fce8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andrius=20=C5=A0tikonas?= Date: Sun, 11 Oct 2020 22:20:54 +0100 Subject: [PATCH 11/16] Store successful polkit authentication requests. We need this to avoid multiple authentication requests when applying long operations. Otherwise, the user will have to authenticate after each job, which can mean up to 4 password dialogs for some longer operations. --- src/util/externalcommandhelper.cpp | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/util/externalcommandhelper.cpp b/src/util/externalcommandhelper.cpp index 8cede39..c874d3b 100644 --- a/src/util/externalcommandhelper.cpp +++ b/src/util/externalcommandhelper.cpp @@ -333,9 +333,11 @@ bool ExternalCommandHelper::isCallerAuthorized() return false; } - // track who called into us so we can close when all callers have gone away - // this has to happen before authorisation as anyone could have activated us - m_serviceWatcher->addWatchedService(message().service()); + // Cache successful authentication requests, so that clients don't need + // to authenticate multiple times during long partitioning operations. + if (m_serviceWatcher->watchedServices().contains(message().service())) { + return true; + } PolkitQt1::SystemBusNameSubject subject(message().service()); PolkitQt1::Authority *authority = PolkitQt1::Authority::instance(); @@ -357,9 +359,13 @@ bool ExternalCommandHelper::isCallerAuthorized() switch (result) { case PolkitQt1::Authority::Yes: + // track who called into us so we can close when all callers have gone away + m_serviceWatcher->addWatchedService(message().service()); return true; default: sendErrorReply(QDBusError::AccessDenied); + if (m_serviceWatcher->watchedServices().isEmpty()) + qApp->quit(); return false; } } From 2dff59260c72582941dbfd4ec52625a0e3ea5ddd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andrius=20=C5=A0tikonas?= Date: Sun, 11 Oct 2020 22:26:04 +0100 Subject: [PATCH 12/16] Remove exit call from the helper. It is no longer used. BUG: 412887 --- src/util/externalcommandhelper.cpp | 8 -------- src/util/externalcommandhelper.h | 1 - 2 files changed, 9 deletions(-) diff --git a/src/util/externalcommandhelper.cpp b/src/util/externalcommandhelper.cpp index c874d3b..e9e9b03 100644 --- a/src/util/externalcommandhelper.cpp +++ b/src/util/externalcommandhelper.cpp @@ -303,14 +303,6 @@ QVariantMap ExternalCommandHelper::start(const QString& command, const QStringLi return reply; } -void ExternalCommandHelper::exit() -{ - if (!isCallerAuthorized()) { - return; - } - qApp->quit(); -} - void ExternalCommandHelper::onReadOutput() { /* const QByteArray s = cmd.readAllStandardOutput(); diff --git a/src/util/externalcommandhelper.h b/src/util/externalcommandhelper.h index d36447a..10ba3f5 100644 --- a/src/util/externalcommandhelper.h +++ b/src/util/externalcommandhelper.h @@ -41,7 +41,6 @@ public Q_SLOTS: Q_SCRIPTABLE QVariantMap copyblocks(const QString& sourceDevice, const qint64 sourceFirstByte, const qint64 sourceLength, const QString& targetDevice, const qint64 targetFirstByte, const qint64 blockSize); Q_SCRIPTABLE bool writeData(const QByteArray& buffer, const QString& targetDevice, const qint64 targetFirstByte); Q_SCRIPTABLE bool createFile(const QByteArray& fileContents, const QString& filePath); - Q_SCRIPTABLE void exit(); private: From a2ee82e0218a9ff59909b7fb1bc70891a58524d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andrius=20=C5=A0tikonas?= Date: Sun, 11 Oct 2020 22:39:47 +0100 Subject: [PATCH 13/16] Simplify createFile function, it was checking for authorization twice. --- src/util/externalcommand.cpp | 4 ++-- src/util/externalcommand.h | 2 +- src/util/externalcommandhelper.cpp | 16 ++++------------ src/util/externalcommandhelper.h | 3 +-- 4 files changed, 8 insertions(+), 17 deletions(-) diff --git a/src/util/externalcommand.cpp b/src/util/externalcommand.cpp index 21e8123..90147c1 100644 --- a/src/util/externalcommand.cpp +++ b/src/util/externalcommand.cpp @@ -209,13 +209,13 @@ bool ExternalCommand::writeData(Report& commandReport, const QByteArray& buffer, return waitForDbusReply(pcall); } -bool ExternalCommand::createFile(const QByteArray& buffer, const QString& deviceNode) +bool ExternalCommand::createFile(const QByteArray& fileContents, const QString& filePath) { auto interface = helperInterface(); if (!interface) return false; - QDBusPendingCall pcall = interface->createFile(buffer, deviceNode); + QDBusPendingCall pcall = interface->createFile(filePath, fileContents); return waitForDbusReply(pcall); } diff --git a/src/util/externalcommand.h b/src/util/externalcommand.h index 224ebb4..05f7dc1 100644 --- a/src/util/externalcommand.h +++ b/src/util/externalcommand.h @@ -57,7 +57,7 @@ public: public: bool copyBlocks(const CopySource& source, CopyTarget& target); bool writeData(Report& commandReport, const QByteArray& buffer, const QString& deviceNode, const quint64 firstByte); // same as copyBlocks but from QByteArray - bool createFile(const QByteArray& buffer, const QString& deviceNode); // similar to writeData but creates a new file + bool createFile(const QByteArray& filePath, const QString& fileContents); // similar to writeData but creates a new file /**< @param cmd the command to run */ void setCommand(const QString& cmd); diff --git a/src/util/externalcommandhelper.cpp b/src/util/externalcommandhelper.cpp index e9e9b03..4b85a5e 100644 --- a/src/util/externalcommandhelper.cpp +++ b/src/util/externalcommandhelper.cpp @@ -132,6 +132,10 @@ bool ExternalCommandHelper::createFile(const QString &filePath, const QByteArray if (!isCallerAuthorized()) { return false; } + // Do not allow using this helper for writing to arbitrary location + if ( !filePath.contains(QStringLiteral("/etc/fstab")) ) + return false; + QFile device(filePath); auto flags = QIODevice::WriteOnly | QIODevice::Unbuffered; @@ -253,18 +257,6 @@ bool ExternalCommandHelper::writeData(const QByteArray& buffer, const QString& t return writeData(targetDevice, buffer, targetFirstByte); } -bool ExternalCommandHelper::createFile(const QByteArray& fileContents, const QString& filePath) -{ - if (!isCallerAuthorized()) { - return false; - } - // Do not allow using this helper for writing to arbitrary location - if ( !filePath.contains(QStringLiteral("/etc/fstab")) ) - return false; - - return createFile(filePath, fileContents); -} - QVariantMap ExternalCommandHelper::start(const QString& command, const QStringList& arguments, const QByteArray& input, const int processChannelMode) { if (!isCallerAuthorized()) { diff --git a/src/util/externalcommandhelper.h b/src/util/externalcommandhelper.h index 10ba3f5..2eb6050 100644 --- a/src/util/externalcommandhelper.h +++ b/src/util/externalcommandhelper.h @@ -34,13 +34,12 @@ public: ExternalCommandHelper(); bool readData(const QString& sourceDevice, QByteArray& buffer, const qint64 offset, const qint64 size); bool writeData(const QString& targetDevice, const QByteArray& buffer, const qint64 offset); - bool createFile(const QString& filePath, const QByteArray& fileContents); public Q_SLOTS: Q_SCRIPTABLE QVariantMap start(const QString& command, const QStringList& arguments, const QByteArray& input, const int processChannelMode); Q_SCRIPTABLE QVariantMap copyblocks(const QString& sourceDevice, const qint64 sourceFirstByte, const qint64 sourceLength, const QString& targetDevice, const qint64 targetFirstByte, const qint64 blockSize); Q_SCRIPTABLE bool writeData(const QByteArray& buffer, const QString& targetDevice, const qint64 targetFirstByte); - Q_SCRIPTABLE bool createFile(const QByteArray& fileContents, const QString& filePath); + Q_SCRIPTABLE bool createFile(const QString& filePath, const QByteArray& fileContents); private: From cd4d9b1985a6837b5a8f74102744c7cc9b7014ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andrius=20=C5=A0tikonas?= Date: Sun, 11 Oct 2020 22:45:13 +0100 Subject: [PATCH 14/16] Rename helper interfaces. --- src/util/externalcommand.cpp | 8 ++++---- src/util/externalcommandhelper.cpp | 8 ++++---- src/util/externalcommandhelper.h | 8 ++++---- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/util/externalcommand.cpp b/src/util/externalcommand.cpp index 90147c1..744af8d 100644 --- a/src/util/externalcommand.cpp +++ b/src/util/externalcommand.cpp @@ -129,7 +129,7 @@ bool ExternalCommand::start(int timeout) bool rval = false; - QDBusPendingCall pcall = interface->start(cmd, args(), d->m_Input, d->processChannelMode); + QDBusPendingCall pcall = interface->RunCommand(cmd, args(), d->m_Input, d->processChannelMode); QDBusPendingCallWatcher *watcher = new QDBusPendingCallWatcher(pcall, this); QEventLoop loop; @@ -168,7 +168,7 @@ bool ExternalCommand::copyBlocks(const CopySource& source, CopyTarget& target) if (!interface) return false; - QDBusPendingCall pcall = interface->copyblocks(source.path(), source.firstByte(), source.length(), + QDBusPendingCall pcall = interface->CopyBlocks(source.path(), source.firstByte(), source.length(), target.path(), target.firstByte(), blockSize); QDBusPendingCallWatcher *watcher = new QDBusPendingCallWatcher(pcall, this); @@ -205,7 +205,7 @@ bool ExternalCommand::writeData(Report& commandReport, const QByteArray& buffer, if (!interface) return false; - QDBusPendingCall pcall = interface->writeData(buffer, deviceNode, firstByte); + QDBusPendingCall pcall = interface->WriteData(buffer, deviceNode, firstByte); return waitForDbusReply(pcall); } @@ -215,7 +215,7 @@ bool ExternalCommand::createFile(const QByteArray& fileContents, const QString& if (!interface) return false; - QDBusPendingCall pcall = interface->createFile(filePath, fileContents); + QDBusPendingCall pcall = interface->CreateFile(filePath, fileContents); return waitForDbusReply(pcall); } diff --git a/src/util/externalcommandhelper.cpp b/src/util/externalcommandhelper.cpp index 4b85a5e..026a07d 100644 --- a/src/util/externalcommandhelper.cpp +++ b/src/util/externalcommandhelper.cpp @@ -127,7 +127,7 @@ bool ExternalCommandHelper::writeData(const QString &targetDevice, const QByteAr @param fileContents the data that we write @return true on success */ -bool ExternalCommandHelper::createFile(const QString &filePath, const QByteArray& fileContents) +bool ExternalCommandHelper::CreateFile(const QString &filePath, const QByteArray& fileContents) { if (!isCallerAuthorized()) { return false; @@ -153,7 +153,7 @@ bool ExternalCommandHelper::createFile(const QString &filePath, const QByteArray } // If targetDevice is empty then return QByteArray with data that was read from disk. -QVariantMap ExternalCommandHelper::copyblocks(const QString& sourceDevice, const qint64 sourceFirstByte, const qint64 sourceLength, const QString& targetDevice, const qint64 targetFirstByte, const qint64 blockSize) +QVariantMap ExternalCommandHelper::CopyBlocks(const QString& sourceDevice, const qint64 sourceFirstByte, const qint64 sourceLength, const QString& targetDevice, const qint64 targetFirstByte, const qint64 blockSize) { if (!isCallerAuthorized()) { return QVariantMap(); @@ -245,7 +245,7 @@ QVariantMap ExternalCommandHelper::copyblocks(const QString& sourceDevice, const return reply; } -bool ExternalCommandHelper::writeData(const QByteArray& buffer, const QString& targetDevice, const qint64 targetFirstByte) +bool ExternalCommandHelper::WriteData(const QByteArray& buffer, const QString& targetDevice, const qint64 targetFirstByte) { if (!isCallerAuthorized()) { return false; @@ -257,7 +257,7 @@ bool ExternalCommandHelper::writeData(const QByteArray& buffer, const QString& t return writeData(targetDevice, buffer, targetFirstByte); } -QVariantMap ExternalCommandHelper::start(const QString& command, const QStringList& arguments, const QByteArray& input, const int processChannelMode) +QVariantMap ExternalCommandHelper::RunCommand(const QString& command, const QStringList& arguments, const QByteArray& input, const int processChannelMode) { if (!isCallerAuthorized()) { return QVariantMap(); diff --git a/src/util/externalcommandhelper.h b/src/util/externalcommandhelper.h index 2eb6050..a71923f 100644 --- a/src/util/externalcommandhelper.h +++ b/src/util/externalcommandhelper.h @@ -36,10 +36,10 @@ public: bool writeData(const QString& targetDevice, const QByteArray& buffer, const qint64 offset); public Q_SLOTS: - Q_SCRIPTABLE QVariantMap start(const QString& command, const QStringList& arguments, const QByteArray& input, const int processChannelMode); - Q_SCRIPTABLE QVariantMap copyblocks(const QString& sourceDevice, const qint64 sourceFirstByte, const qint64 sourceLength, const QString& targetDevice, const qint64 targetFirstByte, const qint64 blockSize); - Q_SCRIPTABLE bool writeData(const QByteArray& buffer, const QString& targetDevice, const qint64 targetFirstByte); - Q_SCRIPTABLE bool createFile(const QString& filePath, const QByteArray& fileContents); + Q_SCRIPTABLE QVariantMap RunCommand(const QString& command, const QStringList& arguments, const QByteArray& input, const int processChannelMode); + Q_SCRIPTABLE QVariantMap CopyBlocks(const QString& sourceDevice, const qint64 sourceFirstByte, const qint64 sourceLength, const QString& targetDevice, const qint64 targetFirstByte, const qint64 blockSize); + Q_SCRIPTABLE bool WriteData(const QByteArray& buffer, const QString& targetDevice, const qint64 targetFirstByte); + Q_SCRIPTABLE bool CreateFile(const QString& filePath, const QByteArray& fileContents); private: From 6cff70567b1907103a15193166132191a74d6063 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andrius=20=C5=A0tikonas?= Date: Sun, 11 Oct 2020 23:46:20 +0100 Subject: [PATCH 15/16] Fix signals between helper and client. --- src/jobs/job.cpp | 4 ++-- src/jobs/job.h | 2 +- src/util/externalcommand.cpp | 8 +++----- src/util/externalcommand.h | 7 +------ src/util/externalcommandhelper.cpp | 25 +++++++++++-------------- src/util/externalcommandhelper.h | 4 ++-- 6 files changed, 20 insertions(+), 30 deletions(-) diff --git a/src/jobs/job.cpp b/src/jobs/job.cpp index f46f409..4e2878b 100644 --- a/src/jobs/job.cpp +++ b/src/jobs/job.cpp @@ -94,9 +94,9 @@ void Job::emitProgress(int i) Q_EMIT progress(i); } -void Job::updateReport(const QVariantMap& reportString) +void Job::updateReport(const QString& report) { - m_Report->line() << reportString[QStringLiteral("report")].toString(); + m_Report->line() << report; } Report* Job::jobStarted(Report& parent) diff --git a/src/jobs/job.h b/src/jobs/job.h index 8e74128..cc32c4d 100644 --- a/src/jobs/job.h +++ b/src/jobs/job.h @@ -71,7 +71,7 @@ public: } void emitProgress(int i); - void updateReport(const QVariantMap& reportString); + void updateReport(const QString& report); protected: bool copyBlocks(Report& report, CopyTarget& target, CopySource& source); diff --git a/src/util/externalcommand.cpp b/src/util/externalcommand.cpp index 744af8d..75e9308 100644 --- a/src/util/externalcommand.cpp +++ b/src/util/externalcommand.cpp @@ -159,15 +159,13 @@ bool ExternalCommand::copyBlocks(const CopySource& source, CopyTarget& target) bool rval = true; const qint64 blockSize = 10 * 1024 * 1024; // number of bytes per block to copy - // TODO KF6:Use new signal-slot syntax - // FIXME: port and reenable these signals - //connect(m_job, SIGNAL(percent(KJob*, unsigned long)), this, SLOT(emitProgress(KJob*, unsigned long))); - //connect(m_job, &KAuth::ExecuteJob::newData, this, &ExternalCommand::emitReport); - auto interface = helperInterface(); if (!interface) return false; + connect(interface, &OrgKdeKpmcoreExternalcommandInterface::progress, this, &ExternalCommand::progress); + connect(interface, &OrgKdeKpmcoreExternalcommandInterface::report, this, &ExternalCommand::reportSignal); + QDBusPendingCall pcall = interface->CopyBlocks(source.path(), source.firstByte(), source.length(), target.path(), target.firstByte(), blockSize); diff --git a/src/util/externalcommand.h b/src/util/externalcommand.h index 05f7dc1..2b3d044 100644 --- a/src/util/externalcommand.h +++ b/src/util/externalcommand.h @@ -89,8 +89,6 @@ public: /**< @return pointer to the Report or nullptr */ Report* report(); - void emitReport(const QVariantMap& report) { Q_EMIT reportSignal(report); } - /**< Sets a parent widget for the authentication dialog. * @param p parent widget */ @@ -100,10 +98,7 @@ public: Q_SIGNALS: void progress(int); - void reportSignal(const QVariantMap&); - -public Q_SLOTS: - void emitProgress(KJob*, unsigned long percent) { Q_EMIT progress(percent); } + void reportSignal(const QString&); private: void setExitCode(int i); diff --git a/src/util/externalcommandhelper.cpp b/src/util/externalcommandhelper.cpp index 026a07d..9c68226 100644 --- a/src/util/externalcommandhelper.cpp +++ b/src/util/externalcommandhelper.cpp @@ -41,7 +41,7 @@ ExternalCommandHelper::ExternalCommandHelper() { - if (!QDBusConnection::systemBus().registerObject(QStringLiteral("/Helper"), this, QDBusConnection::ExportAllSlots)) { + if (!QDBusConnection::systemBus().registerObject(QStringLiteral("/Helper"), this, QDBusConnection::ExportAllSlots | QDBusConnection::ExportAllSignals)) { ::exit(-1); } @@ -183,13 +183,10 @@ QVariantMap ExternalCommandHelper::CopyBlocks(const QString& sourceDevice, const timer.start(); - QVariantMap report; - - report[QStringLiteral("report")] = xi18nc("@info:progress", "Copying %1 blocks (%2 bytes) from %3 to %4, direction: %5.", blocksToCopy, + QString reportText = xi18nc("@info:progress", "Copying %1 blocks (%2 bytes) from %3 to %4, direction: %5.", blocksToCopy, sourceLength, readOffset, writeOffset, copyDirection == 1 ? i18nc("direction: left", "left") : i18nc("direction: right", "right")); - - //HelperSupport::progressStep(report); + Q_EMIT report(reportText); bool rval = true; @@ -208,10 +205,10 @@ QVariantMap ExternalCommandHelper::CopyBlocks(const QString& sourceDevice, const if (percent % 5 == 0 && timer.elapsed() > 1000) { const qint64 mibsPerSec = (blocksCopied * blockSize / 1024 / 1024) / (timer.elapsed() / 1000); const qint64 estSecsLeft = (100 - percent) * timer.elapsed() / percent / 1000; - report[QStringLiteral("report")]= xi18nc("@info:progress", "Copying %1 MiB/second, estimated time left: %2", mibsPerSec, QTime(0, 0).addSecs(estSecsLeft).toString()); - //HelperSupport::progressStep(report); + reportText = xi18nc("@info:progress", "Copying %1 MiB/second, estimated time left: %2", mibsPerSec, QTime(0, 0).addSecs(estSecsLeft).toString()); + Q_EMIT report(reportText); } - //HelperSupport::progressStep(percent); + Q_EMIT progress(percent); } } @@ -221,8 +218,8 @@ QVariantMap ExternalCommandHelper::CopyBlocks(const QString& sourceDevice, const const qint64 lastBlockReadOffset = copyDirection > 0 ? readOffset + blockSize * blocksCopied : sourceFirstByte; const qint64 lastBlockWriteOffset = copyDirection > 0 ? writeOffset + blockSize * blocksCopied : targetFirstByte; - report[QStringLiteral("report")]= xi18nc("@info:progress", "Copying remainder of block size %1 from %2 to %3.", lastBlock, lastBlockReadOffset, lastBlockWriteOffset); - //HelperSupport::progressStep(report); + reportText = xi18nc("@info:progress", "Copying remainder of block size %1 from %2 to %3.", lastBlock, lastBlockReadOffset, lastBlockWriteOffset); + Q_EMIT report(reportText); rval = readData(sourceDevice, buffer, lastBlockReadOffset, lastBlock); if (rval) { @@ -233,13 +230,13 @@ QVariantMap ExternalCommandHelper::CopyBlocks(const QString& sourceDevice, const } if (rval) { - //HelperSupport::progressStep(100); + Q_EMIT progress(100); bytesWritten += buffer.size(); } } - report[QStringLiteral("report")] = xi18ncp("@info:progress argument 2 is a string such as 7 bytes (localized accordingly)", "Copying 1 block (%2) finished.", "Copying %1 blocks (%2) finished.", blocksCopied, i18np("1 byte", "%1 bytes", bytesWritten)); - //HelperSupport::progressStep(report); + reportText = xi18ncp("@info:progress argument 2 is a string such as 7 bytes (localized accordingly)", "Copying 1 block (%2) finished.", "Copying %1 blocks (%2) finished.", blocksCopied, i18np("1 byte", "%1 bytes", bytesWritten)); + Q_EMIT report(reportText); reply[QStringLiteral("success")] = rval; return reply; diff --git a/src/util/externalcommandhelper.h b/src/util/externalcommandhelper.h index a71923f..c86a8d6 100644 --- a/src/util/externalcommandhelper.h +++ b/src/util/externalcommandhelper.h @@ -27,8 +27,8 @@ class ExternalCommandHelper : public QObject, public QDBusContext Q_CLASSINFO("D-Bus Interface", "org.kde.kpmcore.externalcommand") Q_SIGNALS: - void progress(int); - void quit(); + Q_SCRIPTABLE void progress(int); + Q_SCRIPTABLE void report(QString); public: ExternalCommandHelper(); From e085ba9ed9ae7e8840bc475af3989347a87d3207 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andrius=20=C5=A0tikonas?= Date: Mon, 12 Oct 2020 00:18:31 +0100 Subject: [PATCH 16/16] Do not link to KAuth. But there is still a build time dependency on kauth-policy-gen. --- src/CMakeLists.txt | 1 - src/util/externalcommand.cpp | 6 ------ src/util/externalcommand.h | 12 ------------ 3 files changed, 19 deletions(-) diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index ce33f9d..fda245d 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -37,7 +37,6 @@ target_link_libraries( kpmcore PUBLIC KF5::I18n KF5::CoreAddons KF5::WidgetsAddons - KF5::AuthCore ) install(TARGETS kpmcore EXPORT KPMcoreTargets ${INSTALL_TARGETS_DEFAULT_ARGS}) diff --git a/src/util/externalcommand.cpp b/src/util/externalcommand.cpp index 75e9308..54996bb 100644 --- a/src/util/externalcommand.cpp +++ b/src/util/externalcommand.cpp @@ -37,8 +37,6 @@ #include #include #include - -#include #include #include @@ -53,10 +51,6 @@ struct ExternalCommandPrivate QProcess::ProcessChannelMode processChannelMode; }; -KAuth::ExecuteJob* ExternalCommand::m_job; -QWidget* ExternalCommand::parent; - - /** Creates a new ExternalCommand instance without Report. @param cmd the command to run @param args the arguments to pass to the command diff --git a/src/util/externalcommand.h b/src/util/externalcommand.h index 2b3d044..516c160 100644 --- a/src/util/externalcommand.h +++ b/src/util/externalcommand.h @@ -24,8 +24,6 @@ #include -namespace KAuth { class ExecuteJob; } - class KJob; class Report; class CopySource; @@ -89,13 +87,6 @@ public: /**< @return pointer to the Report or nullptr */ Report* report(); - /**< Sets a parent widget for the authentication dialog. - * @param p parent widget - */ - static void setParentWidget(QWidget *p) { - parent = p; - } - Q_SIGNALS: void progress(int); void reportSignal(const QString&); @@ -109,9 +100,6 @@ private: private: std::unique_ptr d; - // KAuth - static KAuth::ExecuteJob *m_job; - static QWidget *parent; }; #endif