Skip to content

chore: enhance D-Bus service security configuration#104

Merged
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
itsXuSt:v25
Jan 16, 2026
Merged

chore: enhance D-Bus service security configuration#104
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
itsXuSt:v25

Conversation

@itsXuSt
Copy link
Contributor

@itsXuSt itsXuSt commented Jan 14, 2026

  • Remove allow own from default policy, restrict to root only
  • Remove redundant interface-specific allow rules
  • Simplify configuration while maintaining functionality

Log: Tighten D-Bus permissions by restricting service ownership to root user

Summary by Sourcery

Tighten D-Bus configuration for the com.deepin.bootmaker service to reduce unnecessary permissions while preserving method access.

Enhancements:

  • Restrict D-Bus service ownership so it is no longer generally allowed in the default policy.
  • Simplify D-Bus policy by removing redundant interface-specific allow rules while retaining destination-based access.

- Remove allow own from default policy, restrict to root only
- Remove redundant interface-specific allow rules
- Simplify configuration while maintaining functionality

Log: Tighten D-Bus permissions by restricting service ownership to root user
@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 policy for com.deepin.bootmaker by removing broad ownership and redundant interface-specific permissions, leaving a simpler configuration that still allows clients to send messages to the service while restricting bus name ownership to root (configured elsewhere).

Sequence diagram for D-Bus message flow under tightened policy

sequenceDiagram
  actor ClientApp
  participant SystemBus
  participant BootmakerService

  Note over SystemBus,BootmakerService: BootmakerService owns com.deepin.bootmaker (root-only)

  ClientApp->>SystemBus: Method call
  Note over ClientApp,SystemBus: send_destination=com.deepin.bootmaker
  SystemBus->>BootmakerService: Deliver method call
  BootmakerService-->>SystemBus: Reply
  SystemBus-->>ClientApp: Return result

  Note over ClientApp,SystemBus: Client cannot own com.deepin.bootmaker (allow own removed)
Loading

File-Level Changes

Change Details Files
Tighten and simplify D-Bus policy for com.deepin.bootmaker while preserving required client access.
  • Remove the default policy rule that allowed any user to own the com.deepin.bootmaker bus name, so ownership can be restricted to root via other policy context
  • Keep the generic send_destination permission for com.deepin.bootmaker so clients can still send messages to the service
  • Delete redundant allow rules that duplicated the generic send_destination for specific interfaces (service’s own interface, org.freedesktop.DBus.Properties, and org.freedesktop.DBus.Introspectable) to reduce configuration complexity
src/service/data/com.deepin.bootmaker.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 left some high level feedback:

  • The description mentions restricting service ownership to root, but this diff only removes the <allow own> rule without adding a root-only policy block; verify that an explicit root-only allow own is defined elsewhere or update the config here to reflect that intent.
  • Since the interface-specific allow rules were removed as redundant, consider adding a brief inline comment near the remaining send_destination rule to clarify that it intentionally covers all interfaces for this service to avoid future reintroduction of per-interface rules.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The description mentions restricting service ownership to root, but this diff only removes the `<allow own>` rule without adding a root-only policy block; verify that an explicit root-only `allow own` is defined elsewhere or update the config here to reflect that intent.
- Since the interface-specific `allow` rules were removed as redundant, consider adding a brief inline comment near the remaining `send_destination` rule to clarify that it intentionally covers all interfaces for this service to avoid future reintroduction of per-interface rules.

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

这段代码是对 D-Bus 系统服务配置文件 com.deepin.bootmaker.conf 的修改。这个文件通常位于 /usr/share/dbus-1/system.d//etc/dbus-1/system.d/ 目录下,用于控制谁可以访问该服务。

以下是对该 diff 的详细审查意见,分为语法逻辑、代码质量、代码性能和代码安全四个方面:

1. 语法逻辑

  • 审查结果:通过
  • 分析:XML 语法结构正确。修改移除了 <allow own="com.deepin.bootmaker"/> 以及一系列特定的 <allow send_destination="..."/> 规则,只保留了一条通用的 send_destination 规则。从 XML 解析的角度来看,这是合法的。

2. 代码质量

  • 审查结果:良好(简化了配置)
  • 分析
    • 简化:原配置中显式列出了 com.deepin.bootmakerorg.freedesktop.DBus.Propertiesorg.freedesktop.DBus.Introspectable 接口。修改后的配置使用了更通用的规则 <allow send_destination="com.deepin.bootmaker"/>
    • 可读性:修改后的配置更简洁,减少了冗余代码。如果目的是允许任何用户向该服务发送消息,这种写法是标准的。

3. 代码性能

  • 审查结果:无显著影响
  • 分析:D-Bus 守护进程在解析配置文件时,这些规则会被加载到内存中。虽然减少了几个 XML 标签,但这对于系统启动时间或运行时性能的影响微乎其微,可以忽略不计。

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

  • 审查结果:存在潜在风险,需确认业务逻辑
  • 分析
    • 移除 own 权限
      • 修改前:包含 <allow own="com.deepin.bootmaker"/>。这允许服务名称的所有者(通常是 root 启动的服务进程)在总线上注册这个名字。
      • 修改后:移除了该行。
      • 风险:如果该服务是由系统服务(systemd)启动的,通常不需要在 system.d 的配置文件中显式声明 own 权限,因为 D-Bus 系统总线通常默认允许 root 用户拥有服务名。但是,如果该服务尝试降权运行(例如以特定用户身份运行),移除此行可能会导致服务无法注册名称,从而启动失败。请确认该服务是否仍以 root 身份运行。
    • 放宽接口访问限制
      • 修改前:显式指定了允许访问的接口(com.deepin.bootmaker, Properties, Introspectable)。这是一种白名单机制,更加安全。
      • 修改后:仅保留了 <allow send_destination="com.deepin.bootmaker"/>。这意味着任何用户都可以向该服务发送任何接口的消息(只要目标服务实现了该接口)。
      • 风险:这显著扩大了攻击面。如果 com.deepin.bootmaker 服务中存在某些未加固的内部方法或敏感接口,恶意用户现在可以直接调用它们。原配置通过限制接口名提供了一层额外的防御。

改进建议

  1. 确认服务运行身份
    如果该服务不是以 root 身份运行,或者为了配置的明确性,建议恢复 <allow own="com.deepin.bootmaker"/> 这一行,以确保服务能够成功申请总线名称。

  2. 收紧安全策略(推荐)
    虽然简化了配置,但移除接口限制降低了安全性。建议采用折中方案:保留 own 权限(如果需要),并在 default 策略中明确允许常用的标准接口,同时限制核心业务接口的访问(如果有特定安全需求)。

    建议的修改版本(更安全且明确):

    <policy context="default">
      <!-- 确保服务能拥有该名称 -->
      <allow own="com.deepin.bootmaker"/>
    
      <!-- 允许发送消息到该服务 -->
      <allow send_destination="com.deepin.bootmaker"/>
    
      <!-- 明确允许标准接口(通常无害) -->
      <allow send_destination="com.deepin.bootmaker"
             send_interface="org.freedesktop.DBus.Introspectable"/>
      <allow send_destination="com.deepin.bootmaker"
             send_interface="org.freedesktop.DBus.Properties"/>
             
      <!-- 注意:这里没有显式限制 com.deepin.bootmaker 接口,
           因此上面的 send_destination 规则已经隐式允许了它。
           如果你想严格限制,可以移除通用的 send_destination,
           只保留下面这一行(但这取决于你是否需要调用其他接口) -->
    </policy>

    总结
    如果这是一个内部工具且运行在受信任的环境(如个人电脑的 Live 系统),当前的修改是可以接受的。但如果这是一个长期运行在多用户系统上的后台服务,建议恢复 <allow 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

/merge

@deepin-bot
Copy link
Contributor

deepin-bot bot commented Jan 16, 2026

This pr cannot be merged! (status: unstable)

@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 918cc10 into linuxdeepin:master 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