Scrap Public Key Cryptography code based on QCA as QDbus is secure enough
Summary: QDbus already provides enough security to the calls made by the application to the helper. Hence no need to encrypt, sign the requests of the application and verify their integrity at the helper side. Reviewers: stikonas, cjlcarvalho Reviewed By: stikonas Subscribers: #kde_partition_manager Tags: #kde_partition_manager Differential Revision: https://phabricator.kde.org/D21275
This commit is contained in:
Shubham Jangra
parent
6f8e6403f8
commit
a7532d4041
|
|
@ -42,8 +42,6 @@
|
|||
#include <QThread>
|
||||
#include <QVariant>
|
||||
|
||||
#include <QtCrypto>
|
||||
|
||||
#include <KAuth>
|
||||
#include <KJob>
|
||||
#include <KLocalizedString>
|
||||
|
|
@ -61,8 +59,6 @@ struct ExternalCommandPrivate
|
|||
};
|
||||
|
||||
KAuth::ExecuteJob* ExternalCommand::m_job;
|
||||
QCA::PrivateKey* ExternalCommand::privateKey;
|
||||
QCA::Initializer* ExternalCommand::init;
|
||||
bool ExternalCommand::helperStarted = false;
|
||||
QWidget* ExternalCommand::parent;
|
||||
|
||||
|
|
@ -106,13 +102,16 @@ ExternalCommand::ExternalCommand(Report& report, const QString& cmd, const QStri
|
|||
|
||||
ExternalCommand::~ExternalCommand()
|
||||
{
|
||||
|
||||
}
|
||||
|
||||
// void ExternalCommand::setup()
|
||||
// {
|
||||
// connect(this, qOverload<int, QProcess::ExitStatus>(&QProcess::finished), this, &ExternalCommand::onFinished);
|
||||
// connect(this, &ExternalCommand::readyReadStandardOutput, this, &ExternalCommand::onReadOutput);
|
||||
// }
|
||||
/*
|
||||
void ExternalCommand::setup()
|
||||
{
|
||||
connect(this, qOverload<int, QProcess::ExitStatus>(&QProcess::finished), this, &ExternalCommand::onFinished);
|
||||
connect(this, &ExternalCommand::readyReadStandardOutput, this, &ExternalCommand::onReadOutput);
|
||||
}
|
||||
*/
|
||||
|
||||
/** Executes the external command.
|
||||
@param timeout timeout to wait for the process to start
|
||||
|
|
@ -146,19 +145,8 @@ bool ExternalCommand::start(int timeout)
|
|||
interface->setTimeout(10 * 24 * 3600 * 1000); // 10 days
|
||||
|
||||
bool rval = false;
|
||||
QByteArray request;
|
||||
const quint64 nonce = interface->getNonce();
|
||||
request.setNum(nonce);
|
||||
request.append(cmd.toUtf8());
|
||||
for (const auto &argument : qAsConst(d->m_Args))
|
||||
request.append(argument.toUtf8());
|
||||
request.append(d->m_Input);
|
||||
request.append(d->processChannelMode);
|
||||
|
||||
QByteArray hash = QCryptographicHash::hash(request, QCryptographicHash::Sha512);
|
||||
|
||||
QDBusPendingCall pcall = interface->start(privateKey->signMessage(hash, QCA::EMSA3_Raw),
|
||||
nonce, cmd, args(), d->m_Input, d->processChannelMode);
|
||||
QDBusPendingCall pcall = interface->start(cmd, args(), d->m_Input, d->processChannelMode);
|
||||
|
||||
QDBusPendingCallWatcher *watcher = new QDBusPendingCallWatcher(pcall, this);
|
||||
QEventLoop loop;
|
||||
|
|
@ -200,22 +188,9 @@ bool ExternalCommand::copyBlocks(const CopySource& source, CopyTarget& target)
|
|||
auto *interface = new org::kde::kpmcore::externalcommand(QStringLiteral("org.kde.kpmcore.externalcommand"),
|
||||
QStringLiteral("/Helper"), QDBusConnection::systemBus(), this);
|
||||
interface->setTimeout(10 * 24 * 3600 * 1000); // 10 days
|
||||
QByteArray request;
|
||||
|
||||
const quint64 nonce = interface->getNonce();
|
||||
request.setNum(nonce);
|
||||
request.append(source.path().toUtf8());
|
||||
request.append(QByteArray::number(source.firstByte()));
|
||||
request.append(QByteArray::number(source.length()));
|
||||
request.append(target.path().toUtf8());
|
||||
request.append(QByteArray::number(target.firstByte()));
|
||||
request.append(QByteArray::number(blockSize));
|
||||
|
||||
QByteArray hash = QCryptographicHash::hash(request, QCryptographicHash::Sha512);
|
||||
|
||||
QDBusPendingCall pcall = interface->copyblocks(privateKey->signMessage(hash, QCA::EMSA3_Raw), nonce,
|
||||
source.path(), source.firstByte(), source.length(),
|
||||
target.path(), target.firstByte(), blockSize);
|
||||
QDBusPendingCall pcall = interface->copyblocks(source.path(), source.firstByte(), source.length(),
|
||||
target.path(), target.firstByte(), blockSize);
|
||||
|
||||
QDBusPendingCallWatcher *watcher = new QDBusPendingCallWatcher(pcall, this);
|
||||
QEventLoop loop;
|
||||
|
|
@ -257,18 +232,8 @@ bool ExternalCommand::writeData(Report& commandReport, const QByteArray& buffer,
|
|||
auto *interface = new org::kde::kpmcore::externalcommand(QStringLiteral("org.kde.kpmcore.externalcommand"),
|
||||
QStringLiteral("/Helper"), QDBusConnection::systemBus(), this);
|
||||
interface->setTimeout(10 * 24 * 3600 * 1000); // 10 days
|
||||
QByteArray request;
|
||||
|
||||
const quint64 nonce = interface->getNonce();
|
||||
request.setNum(nonce);
|
||||
request.append(buffer);
|
||||
request.append(deviceNode.toUtf8());
|
||||
request.append(QByteArray::number(firstByte));
|
||||
|
||||
QByteArray hash = QCryptographicHash::hash(request, QCryptographicHash::Sha512);
|
||||
|
||||
QDBusPendingCall pcall = interface->writeData(privateKey->signMessage(hash, QCA::EMSA3_Raw), nonce,
|
||||
buffer, deviceNode, firstByte);
|
||||
|
||||
QDBusPendingCall pcall = interface->writeData(buffer, deviceNode, firstByte);
|
||||
|
||||
QDBusPendingCallWatcher *watcher = new QDBusPendingCallWatcher(pcall, this);
|
||||
QEventLoop loop;
|
||||
|
|
@ -389,33 +354,11 @@ bool ExternalCommand::startHelper()
|
|||
d->m_thread = new DBusThread;
|
||||
d->m_thread->start();
|
||||
|
||||
init = new QCA::Initializer;
|
||||
// Generate RSA key pair for signing external command requests
|
||||
if (!QCA::isSupported("pkey") || !QCA::PKey::supportedIOTypes().contains(QCA::PKey::RSA)) {
|
||||
qCritical() << xi18n("QCA does not support RSA.");
|
||||
return false;
|
||||
}
|
||||
|
||||
privateKey = new QCA::PrivateKey;
|
||||
*privateKey = QCA::KeyGenerator().createRSA(4096);
|
||||
if(privateKey->isNull()) {
|
||||
qCritical() << xi18n("Failed to make private RSA key.");
|
||||
return false;
|
||||
}
|
||||
|
||||
if (!privateKey->canSign()) {
|
||||
qCritical() << xi18n("Generated key cannot be used for signatures.");
|
||||
return false;
|
||||
}
|
||||
|
||||
QCA::PublicKey pubkey = privateKey->toPublicKey();
|
||||
|
||||
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;
|
||||
arguments.insert(QStringLiteral("pubkey"), pubkey.toDER());
|
||||
action.setArguments(arguments);
|
||||
m_job = action.execute();
|
||||
m_job->start();
|
||||
|
|
@ -435,38 +378,9 @@ bool ExternalCommand::startHelper()
|
|||
void ExternalCommand::stopHelper()
|
||||
{
|
||||
auto *interface = new org::kde::kpmcore::externalcommand(QStringLiteral("org.kde.kpmcore.externalcommand"),
|
||||
QStringLiteral("/Helper"), QDBusConnection::systemBus());
|
||||
QByteArray request;
|
||||
const quint64 nonce = interface->getNonce();
|
||||
request.setNum(nonce);
|
||||
QByteArray hash = QCryptographicHash::hash(request, QCryptographicHash::Sha512);
|
||||
interface->exit(privateKey->signMessage(hash, QCA::EMSA3_Raw), nonce);
|
||||
QStringLiteral("/Helper"), QDBusConnection::systemBus());
|
||||
interface->exit();
|
||||
|
||||
delete privateKey;
|
||||
delete init;
|
||||
}
|
||||
|
||||
quint64 ExternalCommand::getNonce(QDBusInterface& iface)
|
||||
{
|
||||
QDBusPendingCall pcall = iface.asyncCall(QStringLiteral("getNonce"));
|
||||
QDBusPendingCallWatcher *watcher = new QDBusPendingCallWatcher(pcall);
|
||||
QEventLoop loop;
|
||||
quint64 rval = 0;
|
||||
|
||||
auto exitLoop = [&] (QDBusPendingCallWatcher *watcher) {
|
||||
loop.exit();
|
||||
|
||||
if (watcher->isError())
|
||||
qWarning() << watcher->error();
|
||||
else {
|
||||
QDBusPendingReply<quint64> reply = *watcher;
|
||||
rval = reply;
|
||||
}
|
||||
};
|
||||
|
||||
connect(watcher, &QDBusPendingCallWatcher::finished, exitLoop);
|
||||
loop.exec();
|
||||
return rval;
|
||||
}
|
||||
|
||||
void DBusThread::run()
|
||||
|
|
|
|||
|
|
@ -31,13 +31,14 @@
|
|||
|
||||
#include <memory>
|
||||
|
||||
class KJob;
|
||||
namespace KAuth { class ExecuteJob; }
|
||||
namespace QCA { class PrivateKey; class Initializer; }
|
||||
|
||||
class KJob;
|
||||
class Report;
|
||||
class CopySource;
|
||||
class CopyTarget;
|
||||
class QDBusInterface;
|
||||
|
||||
struct ExternalCommandPrivate;
|
||||
|
||||
class DBusThread : public QThread
|
||||
|
|
@ -126,18 +127,13 @@ public Q_SLOTS:
|
|||
|
||||
private:
|
||||
void setExitCode(int i);
|
||||
|
||||
void onReadOutput();
|
||||
static quint64 getNonce(QDBusInterface& iface);
|
||||
|
||||
private:
|
||||
std::unique_ptr<ExternalCommandPrivate> d;
|
||||
|
||||
// KAuth
|
||||
static quint64 m_Nonce;
|
||||
static KAuth::ExecuteJob *m_job;
|
||||
static QCA::Initializer *init;
|
||||
static QCA::PrivateKey *privateKey;
|
||||
static bool helperStarted;
|
||||
static QWidget *parent;
|
||||
};
|
||||
|
|
|
|||
|
|
@ -39,11 +39,12 @@
|
|||
*
|
||||
* This helper also starts another DBus interface where it listens to
|
||||
* command execution requests from the application that started the helper.
|
||||
* These requests are validated using public key cryptography, to prevent
|
||||
* other unprivileged applications from gaining root privileges.
|
||||
*
|
||||
*/
|
||||
ActionReply ExternalCommandHelper::init(const QVariantMap& args)
|
||||
{
|
||||
Q_UNUSED(args)
|
||||
|
||||
ActionReply reply;
|
||||
|
||||
if (!QDBusConnection::systemBus().isConnected() || !QDBusConnection::systemBus().registerService(QStringLiteral("org.kde.kpmcore.helperinterface")) ||
|
||||
|
|
@ -52,9 +53,7 @@ ActionReply ExternalCommandHelper::init(const QVariantMap& args)
|
|||
reply.addData(QStringLiteral("success"), false);
|
||||
return reply;
|
||||
}
|
||||
|
||||
m_publicKey = QCA::PublicKey::fromDER(args[QStringLiteral("pubkey")].toByteArray());
|
||||
|
||||
|
||||
m_loop = std::make_unique<QEventLoop>();
|
||||
HelperSupport::progressStep(QVariantMap());
|
||||
|
||||
|
|
@ -75,16 +74,6 @@ ActionReply ExternalCommandHelper::init(const QVariantMap& args)
|
|||
return reply;
|
||||
}
|
||||
|
||||
/** Generates cryptographic nonce
|
||||
* @return nonce
|
||||
*/
|
||||
quint64 ExternalCommandHelper::getNonce()
|
||||
{
|
||||
const quint64 nonce = m_Generator.generate();
|
||||
m_Nonces.insert(nonce);
|
||||
return nonce;
|
||||
}
|
||||
|
||||
/** 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
|
||||
|
|
@ -145,35 +134,11 @@ bool ExternalCommandHelper::writeData(const QString &targetDevice, const QByteAr
|
|||
}
|
||||
|
||||
// If targetDevice is empty then return QByteArray with data that was read from disk.
|
||||
QVariantMap ExternalCommandHelper::copyblocks(const QByteArray& signature, const quint64 nonce, 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)
|
||||
{
|
||||
QVariantMap reply;
|
||||
reply[QStringLiteral("success")] = true;
|
||||
|
||||
if (m_Nonces.find(nonce) != m_Nonces.end())
|
||||
m_Nonces.erase( nonce );
|
||||
else {
|
||||
reply[QStringLiteral("success")] = false;
|
||||
return reply;
|
||||
}
|
||||
|
||||
QByteArray request;
|
||||
|
||||
request.setNum(nonce);
|
||||
request.append(sourceDevice.toUtf8());
|
||||
request.append(QByteArray::number(sourceFirstByte));
|
||||
request.append(QByteArray::number(sourceLength));
|
||||
request.append(targetDevice.toUtf8());
|
||||
request.append(QByteArray::number(targetFirstByte));
|
||||
request.append(QByteArray::number(blockSize));
|
||||
|
||||
QByteArray hash = QCryptographicHash::hash(request, QCryptographicHash::Sha512);
|
||||
if (!m_publicKey.verifyMessage(hash, signature, QCA::EMSA3_Raw)) {
|
||||
qCritical() << xi18n("Invalid cryptographic signature");
|
||||
reply[QStringLiteral("success")] = false;
|
||||
return reply;
|
||||
}
|
||||
|
||||
const qint64 blocksToCopy = sourceLength / blockSize;
|
||||
qint64 readOffset = sourceFirstByte;
|
||||
qint64 writeOffset = targetFirstByte;
|
||||
|
|
@ -258,65 +223,27 @@ QVariantMap ExternalCommandHelper::copyblocks(const QByteArray& signature, const
|
|||
return reply;
|
||||
}
|
||||
|
||||
bool ExternalCommandHelper::writeData(const QByteArray& signature, const quint64 nonce, const QByteArray& buffer, const QString& targetDevice, const qint64 targetFirstByte)
|
||||
bool ExternalCommandHelper::writeData(const QByteArray& buffer, const QString& targetDevice, const qint64 targetFirstByte)
|
||||
{
|
||||
if (m_Nonces.find(nonce) != m_Nonces.end())
|
||||
m_Nonces.erase( nonce );
|
||||
else
|
||||
return false;
|
||||
|
||||
QByteArray request;
|
||||
request.setNum(nonce);
|
||||
request.append(buffer);
|
||||
request.append(targetDevice.toUtf8());
|
||||
request.append(QByteArray::number(targetFirstByte));
|
||||
|
||||
// Do not allow using this helper for writing to arbitrary location
|
||||
if ( targetDevice.left(5) != QStringLiteral("/dev/") && !targetDevice.contains(QStringLiteral("/etc/fstab")))
|
||||
return false;
|
||||
|
||||
QByteArray hash = QCryptographicHash::hash(request, QCryptographicHash::Sha512);
|
||||
if (!m_publicKey.verifyMessage(hash, signature, QCA::EMSA3_Raw)) {
|
||||
qCritical() << xi18n("Invalid cryptographic signature");
|
||||
return false;
|
||||
}
|
||||
|
||||
return writeData(targetDevice, buffer, targetFirstByte);
|
||||
}
|
||||
|
||||
|
||||
QVariantMap ExternalCommandHelper::start(const QByteArray& signature, const quint64 nonce, const QString& command, const QStringList& arguments, const QByteArray& input, const int processChannelMode)
|
||||
QVariantMap ExternalCommandHelper::start(const QString& command, const QStringList& arguments, const QByteArray& input, const int processChannelMode)
|
||||
{
|
||||
QTextCodec::setCodecForLocale(QTextCodec::codecForName("UTF-8"));
|
||||
QVariantMap reply;
|
||||
reply[QStringLiteral("success")] = true;
|
||||
|
||||
if (m_Nonces.find(nonce) != m_Nonces.end())
|
||||
m_Nonces.erase( nonce );
|
||||
else {
|
||||
reply[QStringLiteral("success")] = false;
|
||||
return reply;
|
||||
}
|
||||
|
||||
if (command.isEmpty()) {
|
||||
reply[QStringLiteral("success")] = false;
|
||||
return reply;
|
||||
}
|
||||
|
||||
QByteArray request;
|
||||
request.setNum(nonce);
|
||||
request.append(command.toUtf8());
|
||||
for (const auto &argument : arguments)
|
||||
request.append(argument.toUtf8());
|
||||
request.append(input);
|
||||
request.append(processChannelMode);
|
||||
QByteArray hash = QCryptographicHash::hash(request, QCryptographicHash::Sha512);
|
||||
if (!m_publicKey.verifyMessage(hash, signature, QCA::EMSA3_Raw)) {
|
||||
qCritical() << xi18n("Invalid cryptographic signature");
|
||||
reply[QStringLiteral("success")] = false;
|
||||
return reply;
|
||||
}
|
||||
|
||||
// Compare with command whitelist
|
||||
QString basename = command.mid(command.lastIndexOf(QLatin1Char('/')) + 1);
|
||||
if (std::find(std::begin(allowedCommands), std::end(allowedCommands), basename) == std::end(allowedCommands)) {
|
||||
|
|
@ -326,7 +253,7 @@ QVariantMap ExternalCommandHelper::start(const QByteArray& signature, const quin
|
|||
return reply;
|
||||
}
|
||||
|
||||
// connect(&cmd, &QProcess::readyReadStandardOutput, this, &ExternalCommandHelper::onReadOutput);
|
||||
// connect(&cmd, &QProcess::readyReadStandardOutput, this, &ExternalCommandHelper::onReadOutput);
|
||||
|
||||
m_cmd.setEnvironment( { QStringLiteral("LVM_SUPPRESS_FD_WARNINGS=1") } );
|
||||
m_cmd.setProcessChannelMode(static_cast<QProcess::ProcessChannelMode>(processChannelMode));
|
||||
|
|
@ -341,19 +268,8 @@ QVariantMap ExternalCommandHelper::start(const QByteArray& signature, const quin
|
|||
return reply;
|
||||
}
|
||||
|
||||
void ExternalCommandHelper::exit(const QByteArray& signature, const quint64 nonce)
|
||||
void ExternalCommandHelper::exit()
|
||||
{
|
||||
QByteArray request;
|
||||
if (m_Nonces.find(nonce) == m_Nonces.end())
|
||||
return;
|
||||
|
||||
request.setNum(nonce);
|
||||
QByteArray hash = QCryptographicHash::hash(request, QCryptographicHash::Sha512);
|
||||
if (!m_publicKey.verifyMessage(hash, signature, QCA::EMSA3_Raw)) {
|
||||
qCritical() << xi18n("Invalid cryptographic signature");
|
||||
return;
|
||||
}
|
||||
|
||||
m_loop->exit();
|
||||
|
||||
QDBusConnection::systemBus().unregisterObject(QStringLiteral("/Helper"));
|
||||
|
|
@ -362,18 +278,18 @@ void ExternalCommandHelper::exit(const QByteArray& signature, const quint64 nonc
|
|||
|
||||
void ExternalCommandHelper::onReadOutput()
|
||||
{
|
||||
// const QByteArray s = cmd.readAllStandardOutput();
|
||||
/* const QByteArray s = cmd.readAllStandardOutput();
|
||||
|
||||
// if(output.length() > 10*1024*1024) { // prevent memory overflow for badly corrupted file systems
|
||||
// if (report())
|
||||
// report()->line() << xi18nc("@info:status", "(Command is printing too much output)");
|
||||
// return;
|
||||
// }
|
||||
if(output.length() > 10*1024*1024) { // prevent memory overflow for badly corrupted file systems
|
||||
if (report())
|
||||
report()->line() << xi18nc("@info:status", "(Command is printing too much output)");
|
||||
return;
|
||||
}
|
||||
|
||||
// output += s;
|
||||
output += s;
|
||||
|
||||
// if (report())
|
||||
// *report() << QString::fromLocal8Bit(s);
|
||||
if (report())
|
||||
*report() << QString::fromLocal8Bit(s);*/
|
||||
}
|
||||
|
||||
KAUTH_HELPER_MAIN("org.kde.kpmcore.externalcommand", ExternalCommandHelper)
|
||||
|
|
|
|||
|
|
@ -24,12 +24,9 @@
|
|||
#include <KAuth>
|
||||
|
||||
#include <QEventLoop>
|
||||
#include <QRandomGenerator64>
|
||||
#include <QString>
|
||||
#include <QProcess>
|
||||
|
||||
#include <QtCrypto>
|
||||
|
||||
using namespace KAuth;
|
||||
|
||||
class ExternalCommandHelper : public QObject
|
||||
|
|
@ -47,23 +44,17 @@ public:
|
|||
|
||||
public Q_SLOTS:
|
||||
ActionReply init(const QVariantMap& args);
|
||||
Q_SCRIPTABLE quint64 getNonce();
|
||||
Q_SCRIPTABLE QVariantMap start(const QByteArray& signature, const quint64 nonce, const QString& command, const QStringList& arguments, const QByteArray& input, const int processChannelMode);
|
||||
Q_SCRIPTABLE QVariantMap copyblocks(const QByteArray& signature, const quint64 nonce, const QString& sourceDevice, const qint64 sourceFirstByte, const qint64 sourceLength, const QString& targetDevice, const qint64 targetFirstByte, const qint64 blockSize);
|
||||
Q_SCRIPTABLE bool writeData(const QByteArray& signature, const quint64 nonce, const QByteArray& buffer, const QString& targetDevice, const qint64 targetFirstByte);
|
||||
Q_SCRIPTABLE void exit(const QByteArray& signature, const quint64 nonce);
|
||||
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 void exit();
|
||||
|
||||
private:
|
||||
void onReadOutput();
|
||||
|
||||
std::unique_ptr<QEventLoop> m_loop;
|
||||
QCA::Initializer initializer;
|
||||
QCA::PublicKey m_publicKey;
|
||||
QRandomGenerator64 m_Generator;
|
||||
std::unordered_set<quint64> m_Nonces;
|
||||
QProcess m_cmd;
|
||||
|
||||
// QByteArray output;
|
||||
// QByteArray output;
|
||||
};
|
||||
|
||||
#endif
|
||||
|
|
|
|||
Loading…
Reference in New Issue