Skip to content

chore: enhance D-Bus service security configuration#105

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

chore: enhance D-Bus service security configuration#105
deepin-bot[bot] merged 1 commit intolinuxdeepin:release/eaglefrom
itsXuSt:v20

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

Enhancements:

  • Restrict D-Bus service ownership for com.deepin.bootmaker by removing default allow-own permissions and redundant interface-specific rules.

 - 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 configuration for the com.deepin.bootmaker service by removing default ownership permissions and redundant interface-specific allow rules, restricting service ownership to root while preserving method invocation capabilities for clients.

Sequence diagram for D-Bus name ownership with restricted root-only policy

sequenceDiagram
    actor RootProcess
    actor NonRootProcess
    participant DBusDaemon
    participant BootmakerService

    RootProcess->>DBusDaemon: RequestName(com.deepin.bootmaker)
    DBusDaemon-->>RootProcess: Grant ownership (allowed by root-specific policy)
    RootProcess->>BootmakerService: Start service under owned name

    NonRootProcess->>DBusDaemon: RequestName(com.deepin.bootmaker)
    DBusDaemon-->>NonRootProcess: Deny ownership (no default allow own)

    NonRootProcess->>DBusDaemon: MethodCall(send_destination=com.deepin.bootmaker)
    DBusDaemon-->>BootmakerService: Forward method call (allowed by send_destination rule)
    BootmakerService-->>NonRootProcess: Method return
Loading

File-Level Changes

Change Details Files
Tighten D-Bus service policy by restricting bus name ownership and simplifying allow rules.
  • Remove the ability for non-root users to own the com.deepin.bootmaker D-Bus name from the default policy context
  • Keep send_destination permission so any client can still invoke methods on the com.deepin.bootmaker service
  • Remove redundant interface-qualified allow rules for com.deepin.bootmaker, org.freedesktop.DBus.Properties, and org.freedesktop.DBus.Introspectable to rely on the general destination rule instead
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:

  • If the intent is to restrict service ownership to root, consider adding (or pointing to) an explicit <policy user="root"> <allow own="com.deepin.bootmaker"/> </policy> rule in this file or a referenced config to avoid the service failing to acquire its bus name.
  • Given the stated goal of tightening D-Bus permissions, you may want to revisit the broad <allow send_destination="com.deepin.bootmaker"/> in the default policy, as it still allows any user to invoke methods on the service despite the stricter ownership rule.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- If the intent is to restrict service ownership to root, consider adding (or pointing to) an explicit `<policy user="root"> <allow own="com.deepin.bootmaker"/> </policy>` rule in this file or a referenced config to avoid the service failing to acquire its bus name.
- Given the stated goal of tightening D-Bus permissions, you may want to revisit the broad `<allow send_destination="com.deepin.bootmaker"/>` in the default policy, as it still allows any user to invoke methods on the service despite the stricter ownership rule.

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

审查意见:DBUS 策略配置变更分析

这份 diff 展示了对 com.deepin.bootmaker 服务的 DBus 系统总线配置文件的修改。主要是删除了部分 <allow> 规则。以下是从语法逻辑、代码质量、性能和安全四个维度的详细审查和改进建议:

1. 语法逻辑

  • 现状:修改后的 XML 结构是合法的。移除了多余的 <allow own="..."/> 和具体的接口限制规则,保留了 <policy context="default"> 下的 <allow send_destination="com.deepin.bootmaker"/>
  • 分析:在 DBus 策略中,<allow send_destination="..."> 如果不指定 send_interface,意味着允许向该目的地发送任何接口的消息。因此,被删除的那些具体指定接口(如 org.freedesktop.DBus.Properties)的规则在逻辑上是冗余的,因为它们已经被更宽泛的规则所覆盖。删除它们在逻辑上是成立的,简化了配置。

2. 代码质量

  • 现状:配置文件变得更加简洁,减少了重复代码。
  • 分析:通过删除冗余规则,提高了配置文件的可读性和维护性。这符合“最小化配置”的原则,避免了规则堆砌。

3. 代码性能

  • 现状:DBus 守护进程在解析配置时,需要遍历规则树。
  • 分析:减少规则数量可以略微减少 DBus 守护进程在匹配消息时的检查次数。虽然对于单个服务来说性能提升微乎其微,但从系统整体角度看,精简规则总是有益的。

4. 代码安全 —— 重点风险提示

这里存在一个潜在的安全逻辑变更,需要根据实际业务场景严格评估:

  • 被删除的规则<allow own="com.deepin.bootmaker"/>

    • 含义:这条规则控制谁可以拥有(注册)这个服务名
    • 影响:删除此规则意味着,不再允许默认上下文(context="default",通常是任何用户或进程)注册名为 com.deepin.bootmaker 的服务。
    • 安全性评估
      • 正面:这提高了安全性。如果没有此规则,只有特定的用户(例如 root 或在 <policy user="..."> 中明确配置的用户)才能启动并注册该服务。这可以防止恶意用户冒充 bootmaker 服务。
      • 风险:如果该服务原本设计为由普通用户启动,或者没有在其他地方(如 <policy at_console="true"><policy user="...">)配置 own 权限,那么删除此规则后,服务将无法启动,或者启动后无法在总线上注册名字,导致功能失效。
  • 保留的规则<allow send_destination="com.deepin.bootmaker"/>

    • 含义:允许任何客户端向该服务发送消息。
    • 安全性评估:这是一个非常宽泛的权限。它允许任何客户端调用该服务的任何方法。如果 bootmaker 服务包含敏感操作(如修改磁盘、写入镜像),这种配置可能存在风险。
    • 改进建议:建议遵循“最小权限原则”。如果该服务仅供系统自身或特定应用调用,应该限制 send_interfacemin_uid/max_uid,而不是对所有 default 上下文开放。

改进建议

  1. 确认服务启动身份

    • 必须确认 com.deepin.bootmaker 服务是以什么用户身份运行的。
    • 如果是以 root 运行,删除 <allow own="..."> 是正确的,因为通常 <policy user="root"> 默认拥有所有权限(或者需要显式配置)。
    • 如果是以普通用户运行,请确保在其他 <policy> 块中(如 <policy user="bootmaker_user"><policy group="bootmaker_group">)添加了 <allow own="com.deepin.bootmaker"/>,否则服务将无法工作。
  2. 细化接口访问权限(安全加固)

    • 目前的配置允许任何人向该服务发送任何消息。建议将:
      <allow send_destination="com.deepin.bootmaker"/>
    • 修改为只暴露必要的接口,例如:
      <allow send_destination="com.deepin.bootmaker"
             send_interface="com.deepin.bootmaker.Control"/>
      <allow send_destination="com.deepin.bootmaker"
             send_interface="org.freedesktop.DBus.Properties"/>
    • 或者限制只能由特定用户发送消息:
      <policy user="root">
          <allow send_destination="com.deepin.bootmaker"/>
      </policy>

总结

这个 diff 在逻辑上是合理的(去除了冗余规则),在性能上有微弱优化,但在安全上引入了显著的变化。

  • 关键点:删除 <allow own="com.deepin.bootmaker"/> 限制了服务的注册权,这通常是好的安全实践,但前提是你已经在别处赋予了正确的运行用户注册权限。
  • 遗留问题:保留的 <allow send_destination="..."> 过于宽泛,建议进一步收紧接口访问权限以防止未授权调用。

@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 5d6c2f5 into linuxdeepin:release/eagle Jan 16, 2026
15 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