Skip to content

feat: integrate deepin-security-loader for privileged service access#3286

Merged
deepin-bot[bot] merged 1 commit into
linuxdeepin:masterfrom
xionglinlin:feat/security-loader
Jun 11, 2026
Merged

feat: integrate deepin-security-loader for privileged service access#3286
deepin-bot[bot] merged 1 commit into
linuxdeepin:masterfrom
xionglinlin:feat/security-loader

Conversation

@xionglinlin

Copy link
Copy Markdown
Contributor
  1. Add shell wrapper (dde-control-center-loader-wrapper) to launch control center via deepin-security-loader
  2. The wrapper checks if security loader is available and has proper capabilities (cap_setgid)
  3. If available, it launches using deepin-security-loader --group deepin-daemon to obtain authorization for protected D-Bus services
  4. If unavailable, it falls back to direct launch without the loader
  5. Install the real binary to /usr/libexec/deepin/ and the wrapper to /usr/bin/dde-control-center
  6. Add SecurityLoaderHelper class to handle handshake with the loader via passed file descriptors
  7. Load permission configuration from JSON files in /usr/share/dde- control-center/permission-interfaces/
  8. During handshake, send D-Bus unique name and list of authorized interfaces to the loader
  9. Update systemd service type from dbus to forking to accommodate the wrapper launch
  10. Move log initialization earlier in main.cpp for better startup debugging
  11. Add dependency on deepin-security-loader package in debian control
  12. Install libexec files via debian install rules

Log: Improved security by using deepin-security-loader for D-Bus authorization

Influence:

  1. Verify control center launches correctly via dde-control-center command
  2. Test desktop file execution: gtk-launch org.deepin.dde.control- center
  3. Verify wrapper falls back gracefully when deepin-security-loader is not installed
  4. Test with deepin-security-loader present: check systemd journal for logging messages
  5. Verify D-Bus service authorization works by calling protected interfaces (e.g., Accounts, NetworkManager)
  6. Test permission config loading: add/remove JSON files in /usr/share/ dde-control-center/permission-interfaces/
  7. Verify systemd service starts correctly with Type=forking
  8. Test daemon mode: dde-control-center -d and check it registers on D-Bus as org.deepin.dde.ControlCenter1
  9. Validate that file descriptors (fd1, fd2) are properly passed by the loader
  10. Check for regression: verify all control center features still work without security loader

feat: 集成deepin-security-loader以访问特权服务

  1. 添加shell包装脚本(dde-control-center-loader-wrapper),通过deepin- security-loader启动控制中心
  2. 包装脚本检查安全加载器是否可用且具有cap_setgid能力
  3. 如果可用,通过deepin-security-loader --group deepin-daemon启动,获 取受保护D-Bus服务的授权
  4. 如果不可用,回退到直接启动,不使用加载器
  5. 将实际二进制文件安装到/usr/libexec/deepin/,包装脚本安装到/usr/ bin/dde-control-center
  6. 添加SecurityLoaderHelper类,通过传入的文件描述符处理与加载器的握手
  7. /usr/share/dde-control-center/permission-interfaces/目录加载权限 配置文件
  8. 握手期间,发送D-Bus唯一名称和已授权接口列表给加载器
  9. 将systemd服务类型从dbus改为forking以适配包装脚本启动
  10. 将日志初始化提前到main.cpp中,便于启动调试
  11. 在debian控制文件中添加deepin-security-loader包依赖
  12. 通过debian安装规则安装libexec文件

Log: 通过deepin-security-loader提升D-Bus授权安全性

Influence:

  1. 验证控制中心通过dde-control-center命令正常启动
  2. 测试桌面文件执行:gtk-launch org.deepin.dde.control-center
  3. 验证未安装deepin-security-loader时包装脚本正常回退
  4. 测试deepin-security-loader存在时,检查systemd日志中的输出信息
  5. 验证D-Bus服务授权正常工作,调用受保护接口(如Accounts、 NetworkManager)
  6. 测试权限配置加载:在/usr/share/dde-control-center/permission- interfaces/中添加/删除JSON文件
  7. 验证systemd服务以Type=forking正确启动
  8. 测试守护进程模式:dde-control-center -d,检查是否在D-Bus上注册为 org.deepin.dde.ControlCenter1
  9. 验证文件描述符(fd1, fd2)是否正确由加载器传递
  10. 检查回归:验证无安全加载器时控制中心所有功能仍正常工作

PMS: TASK-390841
Change-Id: I05848626cd62c47a560aa06f2ba0f64aa2293b9d

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @xionglinlin, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@xionglinlin xionglinlin force-pushed the feat/security-loader branch from b51f14b to e7c5030 Compare June 10, 2026 02:43
@deepin-ci-robot

Copy link
Copy Markdown

deepin pr auto review

你好!我是CodeGeeX。我已仔细审查了你提供的Git Diff。本次修改的核心目的是将dde-control-center的主二进制文件移至libexec目录,并通过引入deepin-security-loader包装脚本和C++辅助类,实现在启动时通过文件描述符(FD)握手来获取受保护系统服务的免Polkit授权。

整体设计思路清晰,但代码在安全性、逻辑严谨性、系统服务配置和性能方面存在一些需要改进的地方。以下是详细的审查意见:

一、 代码安全

1. 文件描述符(FD)未关闭导致资源泄露与潜在安全风险

  • 问题:在securityloaderhelper.cppperformHandshake方法中,通过QFile::open(fd, ...)打开了fd1fd2,但在close()后,底层的文件描述符并未被关闭(QFile默认在销毁时不关闭原生FD)。此外,如果握手失败或程序异常,这些FD将一直保持打开状态,可能导致资源泄露和潜在的安全隐患。
  • 改进:在握手完成后(无论成功与否),必须显式关闭底层文件描述符。
    // 在 performHandshake 函数末尾或退出前添加:
    if (fd1 >= 0) close(fd1);
    if (fd2 >= 0) close(fd2);

2. JSON配置文件解析缺乏完整性校验

  • 问题appendDest方法只校验了DbusNameDbusPathDbusInterface是否为空,但没有校验它们是否符合D-Bus的命名规范(如:以反斜杠开头、包含点号等)。恶意的或格式错误的配置文件可能导致程序在D-Bus通信时出现未定义行为。
  • 改进:增加基本的格式校验,例如检查DbusPath是否以/开头。

3. 包装脚本中的能力检查可能被绕过

  • 问题dde-control-center-loader-wrapper中通过getcap ... | grep -q cap_setgid来检查deepin-security-loader-exec是否具备特定的能力。如果系统未安装getcap(属于libcap2-bin包),或者输出格式发生变化,这个检查会静默失败并直接Fallback。
  • 改进:在Debian的control文件中,建议将libcap2-bin添加为RecommendsDepends;同时在脚本中增加对getcap命令是否存在的检查。

二、 语法与逻辑

1. Systemd服务类型配置冲突(严重)

  • 问题:在dde-control-center.service.in中,将Type=dbus修改为了Type=forking。但是,主程序dde-control-center在以-d(后台模式)启动时,如果通过包装脚本被deepin-security-loader拉起,其fork行为可能不符合Type=forking的预期(systemd期望主进程退出且子进程成为守护进程)。更严重的是,服务仍然保留了BusName=org.deepin.dde.ControlCenter1,在systemd中,BusName属性是专属于Type=dbus的,与Type=forking混用会导致systemd无法正确追踪服务状态,可能造成服务启动超时或被误杀。
  • 改进
    • 如果必须用Type=forking,请删除BusName,并确保主程序在fork后正确写出PID文件(PIDFile=)。
    • 更好的方案是:保持Type=dbus,让deepin-security-loader--dbus参数启动或在C++代码中确保不干扰D-Bus名称的注册时序。

2. 日志初始化顺序错误

  • 问题:在main.cpp中,将DLogManager的初始化代码移到了命令行解析(parser.process)和SecurityLoaderHelper::doSecurityLoader之前。但是,DLogManager::registerJournalAppender()等函数可能依赖于QCoreApplication已经实例化。在当前代码位置,app已经创建,所以不会崩溃,但SecurityLoaderHelper中输出的日志(如qCInfo(secLoader))将无法被后续注册的File Appender记录,因为握手发生在日志系统完全初始化之前。
  • 改进:将日志初始化代码移回原位(在parser.process之后),或者将其移至doSecurityLoader调用之前,确保握手过程中的日志能被正确持久化。

3. 去重逻辑性能低下

  • 问题appendDest方法中使用QJsonArray进行遍历去重,时间复杂度为O(N^2)。如果配置文件较多,D-Bus接口列表较长,会导致启动速度变慢。
  • 改进:使用QSetQHash进行去重。将DbusName + DbusPath + DbusInterface拼接成字符串作为Key进行查重。

三、 代码性能

1. JSON文件读取的内存分配

  • 问题parseJsonFile中使用file.readAll()一次性读取整个文件,如果配置文件异常庞大,会导致内存峰值。
  • 改进:对于配置文件来说,通常很小,readAll()是可以接受的。但更稳健的做法是使用QJsonDocument::fromDevice(&file)直接从设备解析,避免额外的QByteArray拷贝。

四、 代码质量与规范

1. 版权年份错误

  • 问题:新增的securityloaderhelper.hsecurityloaderhelper.cpp中,版权声明为2026年,这显然是笔误。
  • 改进:修改为当前年份(如2024)。

2. CMake安装脚本冗余

  • 问题CMakeLists.txt中添加了configure_file将wrapper脚本复制到构建目录用于"本地测试",但这在发布构建中是不必要的,且增加了维护成本。
  • 改进:如果仅为了安装,可以移除configure_file,直接在install(PROGRAMS ...)中引用源码目录的文件即可。

3. Shell脚本的健壮性

  • 问题dde-control-center-loader-wrapper中使用了#!/bin/bash,对于简单的逻辑,依赖bash可能不符合Debian的极简规范。
  • 改进:建议改为#!/bin/sh,并确保语法兼容POSIX Shell。

🌟 修改建议代码示例

1. securityloaderhelper.cpp (修复FD泄露与性能问题)

bool SecurityLoaderHelper::performHandshake(int fd1, int fd2)
{
    // ... 前置检查 ...

    // 使用 QHash 去重优化
    // (需在类中添加 QHash<QString, bool> m_destKeys; 成员)
    
    QFile fd1File;
    if (!fd1File.open(fd1, QIODevice::WriteOnly)) {
        qCWarning(secLoader) << "Cannot open fd1 for writing";
        close(fd1); close(fd2); // 出错时关闭FD
        return false;
    }
    fd1File.write(jsonData);
    fd1File.close();

    QFile fd2File;
    if (!fd2File.open(fd2, QIODevice::ReadOnly)) {
        qCWarning(secLoader) << "Cannot open fd2 for reading";
        close(fd1); close(fd2); // 出错时关闭FD
        return false;
    }

    QByteArray response = fd2File.readAll();
    fd2File.close();

    // 【关键修复】:显式关闭底层文件描述符,防止资源泄露
    close(fd1);
    close(fd2);

    // ... 后续JSON解析逻辑 ...
}

void SecurityLoaderHelper::appendDest(const QJsonObject &dest)
{
    const QString dbusName = dest["DbusName"].toString();
    const QString dbusPath = dest["DbusPath"].toString();
    const QString dbusInterface = dest["DbusInterface"].toString();

    if (dbusName.isEmpty() || dbusPath.isEmpty() || dbusInterface.isEmpty()) {
        qCWarning(secLoader) << "Skip invalid D-Bus destination:" << dest;
        return;
    }

    // 增加基本的D-Bus格式校验
    if (!dbusPath.startsWith('/')) {
        qCWarning(secLoader) << "Skip invalid D-Bus path (must start with /):" << dbusPath;
        return;
    }

    // 使用Hash优化查重性能
    QString uniqueKey = QString("%1;%2;%3").arg(dbusName, dbusPath, dbusInterface);
    if (m_destKeys.contains(uniqueKey)) {
        return;
    }
    m_destKeys.insert(uniqueKey, true);

    m_destList.append(dest);
    qCInfo(secLoader) << "  Added:" << dbusName << dbusPath << dbusInterface;
}

2. dde-control-center-loader-wrapper (增强健壮性)

#!/bin/sh
# dde-control-center wrapper for deepin-security-loader

REAL_BINARY="/usr/libexec/deepin/dde-control-center"
LOADER="/usr/bin/deepin-security-loader"
LOADER_EXEC="/usr/bin/deepin-security-loader-exec"

log_to_journal() {
    logger -t dde-control-center -p user.info "$1"
}

log_to_journal "dde-control-center launched with args: $*"

# 检查 getcap 命令是否存在,且 loader 具备 cap_setgid
if [ -x "$LOADER" ] && [ -x "$LOADER_EXEC" ] && command -v getcap >/dev/null 2>&1; then
    if getcap "$LOADER_EXEC" 2>/dev/null | grep -q cap_setgid; then
        log_to_journal "Using deepin-security-loader with authorization groups"
        exec "$LOADER" --group deepin-daemon -- "$REAL_BINARY" "$@"
    fi
fi

log_to_journal "Fallback: launching directly without security loader"
exec "$REAL_BINARY" "$@"

3. systemd service (修复逻辑冲突)

[Service]
# 如果保留 forking,必须删除 BusName,并确保程序后台化逻辑正确
Type=forking
# BusName=org.deepin.dde.ControlCenter1  <-- 必须删除此行
ExecStart=@CMAKE_INSTALL_FULL_BINDIR@/dde-control-center -d
Slice=app.slice
# 建议添加 PIDFile 以防 systemd 无法追踪主进程
# PIDFile=/run/dde-control-center.pid

@deepin-ci-robot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: caixr23, mhduiy, xionglinlin

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

1. Add shell wrapper (`dde-control-center-loader-wrapper`) to launch
control center via deepin-security-loader
2. The wrapper checks if security loader is available and has proper
capabilities (cap_setgid)
3. If available, it launches using `deepin-security-loader --group
deepin-daemon` to obtain authorization for protected D-Bus services
4. If unavailable, it falls back to direct launch without the loader
5. Install the real binary to `/usr/libexec/deepin/` and the wrapper to
`/usr/bin/dde-control-center`
6. Add `SecurityLoaderHelper` class to handle handshake with the loader
via passed file descriptors
7. Load permission configuration from JSON files in `/usr/share/dde-
control-center/permission-interfaces/`
8. During handshake, send D-Bus unique name and list of authorized
interfaces to the loader
9. Update systemd service type from `dbus` to `forking` to accommodate
the wrapper launch
10. Move log initialization earlier in `main.cpp` for better startup
debugging
11. Add dependency on `deepin-security-loader` package in debian control
12. Install libexec files via debian install rules

Log: Improved security by using deepin-security-loader for D-Bus
authorization

Influence:
1. Verify control center launches correctly via `dde-control-center`
command
2. Test desktop file execution: `gtk-launch org.deepin.dde.control-
center`
3. Verify wrapper falls back gracefully when deepin-security-loader is
not installed
4. Test with deepin-security-loader present: check systemd journal for
logging messages
5. Verify D-Bus service authorization works by calling protected
interfaces (e.g., Accounts, NetworkManager)
6. Test permission config loading: add/remove JSON files in `/usr/share/
dde-control-center/permission-interfaces/`
7. Verify systemd service starts correctly with Type=forking
8. Test daemon mode: `dde-control-center -d` and check it registers on
D-Bus as `org.deepin.dde.ControlCenter1`
9. Validate that file descriptors (fd1, fd2) are properly passed by
the loader
10. Check for regression: verify all control center features still work
without security loader

feat: 集成deepin-security-loader以访问特权服务

1. 添加shell包装脚本(dde-control-center-loader-wrapper),通过deepin-
security-loader启动控制中心
2. 包装脚本检查安全加载器是否可用且具有cap_setgid能力
3. 如果可用,通过`deepin-security-loader --group deepin-daemon`启动,获
取受保护D-Bus服务的授权
4. 如果不可用,回退到直接启动,不使用加载器
5. 将实际二进制文件安装到`/usr/libexec/deepin/`,包装脚本安装到`/usr/
bin/dde-control-center`
6. 添加`SecurityLoaderHelper`类,通过传入的文件描述符处理与加载器的握手
7. 从`/usr/share/dde-control-center/permission-interfaces/`目录加载权限
配置文件
8. 握手期间,发送D-Bus唯一名称和已授权接口列表给加载器
9. 将systemd服务类型从`dbus`改为`forking`以适配包装脚本启动
10. 将日志初始化提前到`main.cpp`中,便于启动调试
11. 在debian控制文件中添加`deepin-security-loader`包依赖
12. 通过debian安装规则安装libexec文件

Log: 通过deepin-security-loader提升D-Bus授权安全性

Influence:
1. 验证控制中心通过`dde-control-center`命令正常启动
2. 测试桌面文件执行:`gtk-launch org.deepin.dde.control-center`
3. 验证未安装deepin-security-loader时包装脚本正常回退
4. 测试deepin-security-loader存在时,检查systemd日志中的输出信息
5. 验证D-Bus服务授权正常工作,调用受保护接口(如Accounts、
NetworkManager)
6. 测试权限配置加载:在`/usr/share/dde-control-center/permission-
interfaces/`中添加/删除JSON文件
7. 验证systemd服务以Type=forking正确启动
8. 测试守护进程模式:`dde-control-center -d`,检查是否在D-Bus上注册为
`org.deepin.dde.ControlCenter1`
9. 验证文件描述符(fd1, fd2)是否正确由加载器传递
10. 检查回归:验证无安全加载器时控制中心所有功能仍正常工作

PMS: TASK-390841
Change-Id: I05848626cd62c47a560aa06f2ba0f64aa2293b9d
@xionglinlin xionglinlin force-pushed the feat/security-loader branch from e7c5030 to 556a1fe Compare June 11, 2026 11:09
@deepin-bot

deepin-bot Bot commented Jun 11, 2026

Copy link
Copy Markdown

TAG Bot

New tag: 6.1.92
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #3292

@xionglinlin

Copy link
Copy Markdown
Contributor Author

/forcemerge

@deepin-bot

deepin-bot Bot commented Jun 11, 2026

Copy link
Copy Markdown

This pr force merged! (status: behind)

@deepin-bot deepin-bot Bot merged commit c1e4161 into linuxdeepin:master Jun 11, 2026
15 of 19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants