Skip to content

fix: Harden DBus security configuration#188

Merged
deepin-bot[bot] merged 1 commit intolinuxdeepin:release/1071from
itsXuSt:v20
Jan 16, 2026
Merged

fix: Harden DBus security configuration#188
deepin-bot[bot] merged 1 commit intolinuxdeepin:release/1071from
itsXuSt:v20

Conversation

@itsXuSt
Copy link
Contributor

@itsXuSt itsXuSt commented Jan 14, 2026

  • Remove redundant allow own rule from default policy
  • Keep allow own only in root policy for security
  • Remove redundant interface-specific allow rules
  • Simplify configuration while maintaining functionality

Log: Improve DBus security by ensuring only root can own the service

Summary by Sourcery

Tighten DBus policy for com.deepin.diskmanager by removing redundant and insecure default policy rules.

Enhancements:

  • Restrict DBus service ownership to root by removing the default context allow-own rule.
  • Simplify DBus configuration by dropping redundant interface-specific allow rules while retaining destination-based access.

- Remove redundant allow own rule from default policy
- Keep allow own only in root policy for security
- Remove redundant interface-specific allow rules
- Simplify configuration while maintaining functionality

Log: Improve DBus security by ensuring only root can own the service
@sourcery-ai
Copy link

sourcery-ai bot commented Jan 14, 2026

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

This PR tightens the D-Bus security configuration for com.deepin.diskmanager by removing redundant and overly permissive rules from the default policy, ensuring that only the root policy can own the service while keeping method invocation behavior intact.

Sequence diagram for DBus ownership and method calls after policy hardening

sequenceDiagram
  actor RootUser
  actor UnprivilegedClient
  participant DBusDaemon
  participant DiskManagerService

  RootUser->>DBusDaemon: Request own com.deepin.diskmanager
  DBusDaemon->>DBusDaemon: Check RootPolicy allow own
  DBusDaemon-->>RootUser: Ownership granted
  DBusDaemon->>DiskManagerService: Activate service

  UnprivilegedClient->>DBusDaemon: Send method call to com.deepin.diskmanager
  DBusDaemon->>DBusDaemon: Check DefaultPolicy allow send_destination
  DBusDaemon-->>UnprivilegedClient: Policy allows message
  DBusDaemon->>DiskManagerService: Forward method call
  DiskManagerService-->>UnprivilegedClient: Return result

  UnprivilegedClient->>DBusDaemon: Attempt to own com.deepin.diskmanager
  DBusDaemon->>DBusDaemon: Check DefaultPolicy (no allow own)
  DBusDaemon-->>UnprivilegedClient: Ownership denied
Loading

File-Level Changes

Change Details Files
Harden D-Bus policy by removing redundant and permissive rules from the default context while preserving ability to send messages to the service.
  • Removed the allow-own rule for the service name from the default policy so non-root users cannot own the service
  • Retained generic send_destination permission to allow clients to communicate with the service
  • Deleted redundant interface-specific allow rules that duplicated the generic send_destination permission, simplifying the policy
service/assets/data/com.deepin.diskmanager.conf

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey - I've reviewed your changes and they look great!


Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@deepin-ci-robot
Copy link

deepin pr auto review

代码审查报告

1. 语法逻辑

该文件是一个标准的 D-Bus 系统总线配置文件(XML 格式),用于定义 com.deepin.diskmanager 服务的权限策略。从语法上看,XML 结构完整,标签闭合正确,符合 D-Bus 配置文件的规范。

修改内容分析

  • 删除了 <allow own="com.deepin.diskmanager"/>:这意味着移除了任何用户“拥有”(即请求该服务名称)该总线名称的权限。
  • 删除了 针对特定接口(com.deepin.diskmanager, org.freedesktop.DBus.Properties, org.freedesktop.DBus.Introspectable)的显式 <allow send_destination="..."/> 规则。
  • 保留了 <allow send_destination="com.deepin.diskmanager"/>:这是一个通用的发送权限规则,允许任何进程向该目的地发送消息。

潜在逻辑问题

  • 所有权缺失:删除了 <allow own="..."/> 后,如果该服务是以非 root 用户运行,它将无法在系统总线上注册 com.deepin.diskmanager 这个名称。这会导致服务启动失败或无法被调用,除非该服务以 root 身份运行(因为 root 通常拥有默认的 own 权限)或者在其他配置文件中(如 system.conf 或其他更具体的 .conf 文件)赋予了该用户 own 权限。
  • 权限过于宽泛:保留的规则 <allow send_destination="com.deepin.diskmanager"/> 没有限制 send_interface。这实际上允许任何用户向该服务发送任何接口的消息。虽然删除了特定的接口规则,但由于存在这个更宽泛的规则,之前的删除在权限控制上可能没有实际效果,反而让配置看起来不够精确。

2. 代码质量

  • 可读性:修改后的代码更加简洁,减少了重复的 send_destination 声明,这在一定程度上提高了可读性。
  • 规范性:D-Bus 配置文件的规则是自上而下匹配的。目前的配置使用了 policy context="default",这适用于所有未在其他更具体的 policy(如针对特定 user 或 group)中覆盖的连接。

3. 代码性能

  • 配置文件的解析性能差异微乎其微。删除几行规则对 D-Bus 守护进程的运行时性能没有可感知的影响。

4. 代码安全 —— 重点关注

这是本次修改中风险最大的部分。修改极大地放宽了安全限制,或者可能导致服务不可用。

  1. 服务可用性风险

    • 如前所述,如果该服务不是由 root 启动的,且没有其他配置文件允许该服务用户 own 该名称,服务将无法启动。
    • 建议:请确认该服务的运行用户身份。如果是以普通用户运行,必须保留或添加 <allow own="com.deepin.diskmanager"/>,否则服务会崩溃。
  2. 权限提升风险

    • 修改后的配置允许任何人com.deepin.diskmanager 发送任何消息(因为没有限制 send_interface,也没有限制 send_member)。
    • 磁盘管理器通常涉及高权限操作(如挂载、格式化磁盘)。如果该服务后端没有对调用者进行严格的 UID/PID 检查和 Polkit 权限验证,任何本地用户都可以通过 D-Bus 调用危险的方法。
    • 建议:遵循“最小权限原则”。如果不需要所有人访问,应该限制特定的用户或组(例如 <policy group="disk"><policy user="root">)。
    • 建议:如果确实需要允许所有人访问,也应该在服务内部代码逻辑中严格校验每个方法的调用者权限,或者显式列出允许的接口,而不是使用通配符式的 send_destination

改进意见与建议代码

假设该服务需要被桌面环境调用,且需要一定的安全性,建议的改进方案如下:

方案 A:如果服务由 root 运行,且希望限制只有特定组能访问(推荐)

<?xml version="1.0" encoding="UTF-8"?> <!-- 建议添加 XML 声明 -->
<!DOCTYPE busconfig PUBLIC "-//freedesktop//DTD D-BUS Bus Configuration 1.0//EN"
 "http://www.freedesktop.org/standards/dbus/1.0/busconfig.dtd">
<busconfig>
  <!-- 仅允许 root 用户拥有该服务名称 -->
  <policy user="root">
    <allow own="com.deepin.diskmanager"/>
  </policy>

  <!-- 默认策略:拒绝所有连接(可选,取决于系统默认配置,显式拒绝更安全) -->
  <policy context="default">
    <deny send_destination="com.deepin.diskmanager"/>
  </policy>

  <!-- 允许特定组(例如 disk 组或 storage 组)访问 -->
  <policy group="disk">
    <allow send_destination="com.deepin.diskmanager"/>
    <!-- 如果需要,可以显式限制允许调用的接口 -->
    <allow send_destination="com.deepin.diskmanager"
           send_interface="com.deepin.diskmanager"/>
    <allow send_destination="com.deepin.diskmanager"
           send_interface="org.freedesktop.DBus.Properties"/>
    <allow send_destination="com.deepin.diskmanager"
           send_interface="org.freedesktop.DBus.Introspectable"/>
  </policy>
</busconfig>

方案 B:如果必须保持原 diff 的开放策略(任何人可访问),但需修复服务启动问题

如果该服务确实需要被任何用户调用,且服务以特定用户(如 deepin-diskmanager)运行,你需要确保它能注册名称:

<busconfig>
  <!-- 确保服务运行者可以拥有名称 -->
  <policy user="deepin-diskmanager">
    <allow own="com.deepin.diskmanager"/>
  </policy>

  <!-- 允许任何人发送消息 -->
  <policy context="default">
    <allow send_destination="com.deepin.diskmanager"/>
  </policy>
</busconfig>

总结
目前的 diff 修改删除了 own 权限,这极大概率会导致服务无法启动。同时,留下的通配符权限规则虽然方便,但在安全上较为松散。建议根据服务的实际运行身份和所需的安全级别,参考上述方案进行调整。

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: itsXuSt, max-lvs

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

@itsXuSt
Copy link
Contributor Author

itsXuSt commented Jan 16, 2026

/forcemerge

@deepin-bot
Copy link
Contributor

deepin-bot bot commented Jan 16, 2026

This pr force merged! (status: unstable)

@deepin-bot deepin-bot bot merged commit 0a2c5e4 into linuxdeepin:release/1071 Jan 16, 2026
16 of 18 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.

3 participants