From e27884d1ea7d7076bb854493798266021a07f8db Mon Sep 17 00:00:00 2001 From: wangrong Date: Thu, 23 Apr 2026 15:21:37 +0800 Subject: [PATCH] refactor: improve Polkit authorization using SystemBusNameSubject - Switch from PID-based to bus name-based Polkit authentication for better security (PID could be spoofed in race conditions) - Add frontend authorization check at startup to fail early Task: https://pms.uniontech.com/task-view-388685.html --- src/app/main.cpp | 43 ++++++++++- src/service/bootmakerservice.cpp | 126 ++++++------------------------- src/service/bootmakerservice_p.h | 5 +- 3 files changed, 69 insertions(+), 105 deletions(-) diff --git a/src/app/main.cpp b/src/app/main.cpp index 24a1bcda..ce86b7ad 100644 --- a/src/app/main.cpp +++ b/src/app/main.cpp @@ -1,4 +1,4 @@ -// SPDX-FileCopyrightText: 2017 - 2022 UnionTech Software Technology Co., Ltd. +// SPDX-FileCopyrightText: 2017 - 2026 UnionTech Software Technology Co., Ltd. // // SPDX-License-Identifier: GPL-3.0-only @@ -14,11 +14,26 @@ #include #include #include +#include #if QT_VERSION < QT_VERSION_CHECK(6, 0, 0) #include #endif +#if QT_VERSION < QT_VERSION_CHECK(6, 0, 0) +#if defined (Q_OS_LINUX) +#include +#include +#endif +#else +#if defined (Q_OS_LINUX) +#include +#include +#endif +#endif + +const QString s_PolkitActionCreate = "com.deepin.bootmaker.create"; + DCORE_USE_NAMESPACE DWIDGET_USE_NAMESPACE @@ -53,6 +68,27 @@ static bool switchToRoot(QApplication &app) #endif +// 在前端中预先进行身份验证, 便于流程控制 +bool checkAuthorization() +{ +#if defined (Q_OS_LINUX) + QString busName = QDBusConnection::systemBus().baseService(); + auto authority = PolkitQt1::Authority::instance(); + if (!authority) { + qWarning() << "Failed to get Polkit authority instance"; + return false; + } + PolkitQt1::Authority::Result ret = authority->checkAuthorizationSync( + s_PolkitActionCreate, + PolkitQt1::SystemBusNameSubject(busName), + PolkitQt1::Authority::AllowUserInteraction); + + return PolkitQt1::Authority::Yes == ret; +#else + return true; +#endif +} + int main(int argc, char **argv) { qInfo() << "Starting Boot Maker application"; @@ -78,6 +114,11 @@ int main(int argc, char **argv) // app.setApplicationVersion(DApplication::buildVersion(VERSION)); // app.setTheme("light"); + if (!checkAuthorization()) { + qInfo() << "Authorization failed, exiting"; + return 1; + } + #ifdef Q_OS_MAC qDebug() << "Checking root privileges on macOS"; if (switchToRoot(app)) { diff --git a/src/service/bootmakerservice.cpp b/src/service/bootmakerservice.cpp index 050ef1e7..10a62323 100644 --- a/src/service/bootmakerservice.cpp +++ b/src/service/bootmakerservice.cpp @@ -1,4 +1,4 @@ -// SPDX-FileCopyrightText: 2016 - 2022 UnionTech Software Technology Co., Ltd. +// SPDX-FileCopyrightText: 2016 - 2026 UnionTech Software Technology Co., Ltd. // // SPDX-License-Identifier: GPL-3.0-only @@ -43,17 +43,21 @@ const QString s_PolkitActionReboot = "com.deepin.bootmaker.reboot"; /** @brief Polkit action authorization check. Use com.deepin.bootmaker.policy config file. - Default action id: "com.deepin.bootmaker" @note Available on linux/unix/macos platform. @return check passed. */ -bool checkAuthorization(qint64 pid, const QString &action) +bool checkAuthorization(const QString &busName, const QString &action) { #if defined (Q_OS_LINUX) || defined (Q_OS_UNIX) || defined (Q_OS_MAC) - qDebug() << "Checking authorization for action:" << action << "PID:" << pid; - PolkitQt1::Authority::Result ret = PolkitQt1::Authority::instance()->checkAuthorizationSync( + qDebug() << "Checking authorization for action:" << action << "BusName:" << busName; + auto authority = PolkitQt1::Authority::instance(); + if (!authority) { + qWarning() << "Failed to get Polkit authority instance"; + return false; + } + PolkitQt1::Authority::Result ret = authority->checkAuthorizationSync( action, - PolkitQt1::UnixProcessSubject(pid), + PolkitQt1::SystemBusNameSubject(busName), PolkitQt1::Authority::AllowUserInteraction); if (PolkitQt1::Authority::Yes == ret) { @@ -69,53 +73,6 @@ bool checkAuthorization(qint64 pid, const QString &action) #endif } -#if 0 // Not use now -int getProcIdByExeName(std::string execName) -{ - int pid = -1; - - // Open the /proc directory - DIR *dp = opendir("/proc"); - if (dp != NULL) { - // Enumerate all entries in directory until process found - struct dirent *dirp; - while (pid < 0 && (dirp = readdir(dp))) { - // Skip non-numeric entries - int id = atoi(dirp->d_name); - if (id > 0) { - // Read contents of virtual /proc/{pid}/cmdline file - auto exeSymlinkPath = std::string("/proc/") + dirp->d_name + "/exe"; - char *actualpath = realpath(exeSymlinkPath.c_str(), NULL); - if (actualpath) { - // Compare against requested process name - if (execName == actualpath) { - pid = id; - } - } - } - } - } - - closedir(dp); - - return pid; -} -#endif - -static QString getProcIdExe(qint64 id) -{ - qDebug() << "Getting executable path for process ID:" << id; - QString execName; - if (id > 0) { - // Read contents of virtual /proc/{pid}/cmdline file - QString exeSymlinkPath = QString("/proc/%1/exe").arg(id); - char *actualpath = realpath(exeSymlinkPath.toStdString().c_str(), NULL); - execName = QString(actualpath); - qDebug() << "Process executable path:" << execName; - } - return execName; -} - BootMakerService::BootMakerService(QObject *parent) : QObject(parent), d_ptr(new BootMakerServicePrivate(this)) { @@ -161,7 +118,7 @@ void BootMakerService::Reboot() { Q_D(BootMakerService); qInfo() << "Reboot requested"; - if (checkAuthorization(d->dbusCallerPid(), s_PolkitActionReboot)) { + if (d->checkAuthorization(s_PolkitActionReboot)) { qDebug() << "Reboot authorized, proceeding"; d->bm->reboot(); } else { @@ -173,8 +130,8 @@ void BootMakerService::Start() { Q_D(BootMakerService); qInfo() << "Start requested"; - if (!d->checkCaller()) { - qWarning() << "Start request denied - Invalid caller"; + if (!d->checkAuthorization(s_PolkitActionCreate)) { + qWarning() << "Start request denied - Authorization failed"; return; } @@ -186,8 +143,8 @@ void BootMakerService::Stop() { Q_D(BootMakerService); qInfo() << "Stop requested"; - if (!d->checkCaller()) { - qWarning() << "Stop request denied - Invalid caller"; + if (!d->checkAuthorization(s_PolkitActionCreate)) { + qWarning() << "Stop request denied - Authorization failed"; return; } @@ -203,8 +160,8 @@ QString BootMakerService::DeviceList() { Q_D(BootMakerService); qDebug() << "Device list requested"; - if (!d->checkCaller()) { - qWarning() << "Device list request denied - Invalid caller"; + if (!d->checkAuthorization(s_PolkitActionCreate)) { + qWarning() << "Device list request denied - Authorization failed"; return ""; } return deviceListToJson(d->bm->deviceList()); @@ -215,12 +172,7 @@ bool BootMakerService::Install(const QString &image, const QString &device, cons Q_D(BootMakerService); qInfo() << "Install requested - Image:" << image << "Device:" << device << "Partition:" << partition; - if (!d->checkCaller()) { - qWarning() << "Install request denied - Invalid caller"; - return false; - } - - if (!d->disableCheck && !checkAuthorization(d->dbusCallerPid(), s_PolkitActionCreate)) { + if (!d->checkAuthorization(s_PolkitActionCreate)) { qWarning() << "Install request denied - Authorization failed"; return false; } @@ -237,50 +189,22 @@ bool BootMakerService::CheckFile(const QString &filepath) return d->bm->checkfile(filepath); } -bool BootMakerServicePrivate::checkCaller() +bool BootMakerServicePrivate::checkAuthorization(const QString &action) { if (disableCheck) { - qDebug() << "Caller check disabled"; + qDebug() << "Authorization check disabled"; return true; } Q_Q(BootMakerService); if (!q->calledFromDBus()) { - qWarning() << "Caller check failed - Not called from DBus"; - return false; - } - - qint64 callerPid = dbusCallerPid(); - QString callerExe = getProcIdExe(callerPid); - QString dbmExe = QStandardPaths::findExecutable("deepin-boot-maker", {"/usr/bin"}); - - qDebug() << "Caller check - PID:" << callerPid << "Executable:" << callerExe; - - if (callerExe != dbmExe) { - qWarning() << "Caller not authorized - Invalid executable"; + qWarning() << "Authorization check failed - Not called from DBus"; return false; } - qDebug() << "Caller authorized"; - return true; -} - -/** - @return DBus interface caller pid - If the call is not from dbus (from UT), return 0 - */ -qint64 BootMakerServicePrivate::dbusCallerPid() -{ - Q_Q(BootMakerService); - if (!q->calledFromDBus()) { - qDebug() << "Not called from DBus, returning 0"; - return 0; - } - auto interface = q->connection().interface(); - if (interface) { - return static_cast(interface->servicePid(q->message().service()).value()); - } + QString busName = q->message().service(); + bool ret = ::checkAuthorization(busName, action); + qDebug() << "Authorization check result: " << ret; - qDebug() << "Failed to get DBus caller PID"; - return 0; + return ret; } diff --git a/src/service/bootmakerservice_p.h b/src/service/bootmakerservice_p.h index e097091f..7998d3de 100644 --- a/src/service/bootmakerservice_p.h +++ b/src/service/bootmakerservice_p.h @@ -1,4 +1,4 @@ -// SPDX-FileCopyrightText: 2023 UnionTech Software Technology Co., Ltd. +// SPDX-FileCopyrightText: 2023-2026 UnionTech Software Technology Co., Ltd. // // SPDX-License-Identifier: GPL-3.0-only @@ -17,8 +17,7 @@ class BootMakerServicePrivate { } ~BootMakerServicePrivate() {} - bool checkCaller(); - qint64 dbusCallerPid(); + bool checkAuthorization(const QString &action); bool disableCheck = false; BootMaker *bm = nullptr;