feat: integrate deepin-security-loader for privileged service access#3286
Conversation
There was a problem hiding this comment.
Sorry @xionglinlin, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
b51f14b to
e7c5030
Compare
deepin pr auto review你好!我是CodeGeeX。我已仔细审查了你提供的Git Diff。本次修改的核心目的是将 整体设计思路清晰,但代码在安全性、逻辑严谨性、系统服务配置和性能方面存在一些需要改进的地方。以下是详细的审查意见: 一、 代码安全1. 文件描述符(FD)未关闭导致资源泄露与潜在安全风险
2. JSON配置文件解析缺乏完整性校验
3. 包装脚本中的能力检查可能被绕过
二、 语法与逻辑1. Systemd服务类型配置冲突(严重)
2. 日志初始化顺序错误
3. 去重逻辑性能低下
三、 代码性能1. JSON文件读取的内存分配
四、 代码质量与规范1. 版权年份错误
2. CMake安装脚本冗余
3. 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 |
|
[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. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
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
e7c5030 to
556a1fe
Compare
|
TAG Bot New tag: 6.1.92 |
|
/forcemerge |
|
This pr force merged! (status: behind) |
dde-control-center-loader-wrapper) to launch control center via deepin-security-loaderdeepin-security-loader --group deepin-daemonto obtain authorization for protected D-Bus services/usr/libexec/deepin/and the wrapper to/usr/bin/dde-control-centerSecurityLoaderHelperclass to handle handshake with the loader via passed file descriptors/usr/share/dde- control-center/permission-interfaces/dbustoforkingto accommodate the wrapper launchmain.cppfor better startup debuggingdeepin-security-loaderpackage in debian controlLog: Improved security by using deepin-security-loader for D-Bus authorization
Influence:
dde-control-centercommandgtk-launch org.deepin.dde.control- center/usr/share/ dde-control-center/permission-interfaces/dde-control-center -dand check it registers on D-Bus asorg.deepin.dde.ControlCenter1feat: 集成deepin-security-loader以访问特权服务
deepin-security-loader --group deepin-daemon启动,获 取受保护D-Bus服务的授权/usr/libexec/deepin/,包装脚本安装到/usr/ bin/dde-control-centerSecurityLoaderHelper类,通过传入的文件描述符处理与加载器的握手/usr/share/dde-control-center/permission-interfaces/目录加载权限 配置文件dbus改为forking以适配包装脚本启动main.cpp中,便于启动调试deepin-security-loader包依赖Log: 通过deepin-security-loader提升D-Bus授权安全性
Influence:
dde-control-center命令正常启动gtk-launch org.deepin.dde.control-center/usr/share/dde-control-center/permission- interfaces/中添加/删除JSON文件dde-control-center -d,检查是否在D-Bus上注册为org.deepin.dde.ControlCenter1PMS: TASK-390841
Change-Id: I05848626cd62c47a560aa06f2ba0f64aa2293b9d