Skip to content

feat: 添加D-Bus调用者白名单鉴权机制,org.deepin.dde.LocaleHelper1设置语言 - #197

Open
fly602 wants to merge 1 commit into
linuxdeepin:masterfrom
fly602:master
Open

feat: 添加D-Bus调用者白名单鉴权机制,org.deepin.dde.LocaleHelper1设置语言#197
fly602 wants to merge 1 commit into
linuxdeepin:masterfrom
fly602:master

Conversation

@fly602

@fly602 fly602 commented Aug 11, 2026

Copy link
Copy Markdown
Contributor
  1. 新增 allowCallerRegistry 模块实现调用者白名单注册与鉴权功能
  2. 注册逻辑强制校验发起方权限仅允许 root 或 deepin-daemon 组进程注册其子 孙进程
  3. 在 SetLocale 和 GenerateLocale 接口中集成白名单校验通过的进程可跳过后 续鉴权
  4. 实现白名单状态通过 JSON 文件持久化并在重启时加载同时比对 BusID 防止跨 生命周期恢复
  5. 监听 D-Bus NameOwnerChanged 信号实现连接断开时自动清理白名单条目
  6. 将 Polkit 策略由 auth_admin_keep 改为 yes 将权限控制重心转移至服务端 白名单
  7. 更新 D-Bus 总线配置严格限制 SetAllowCaller 方法仅对特权用户和组开放

Influence:

  1. 验证 deepin-daemon 组内的父进程能否成功通过 SetAllowCaller 注册其子 进程
  2. 验证已注册的子进程调用 SetLocale 和 GenerateLocale 时无需 Polkit 弹窗 且功能正常
  3. 验证未在白名单中的普通进程调用上述方法时被正确拦截并返回错误
  4. 测试非法注册请求的拒绝情况包括非特权组进程注册非子孙进程注册以及跨 UID 注册
  5. 验证被注册的子进程异常退出后白名单是否能够自动清除对应条目
  6. 验证 locale-helper 服务重启后白名单状态的正确恢复以及不同 BusID 下的 隔离失效
  7. 验证 root 用户在注册和调用时的豁免逻辑是否正常工作

Task: https://pms.uniontech.com/task-view-393313.html

Summary by Sourcery

Introduce a D-Bus allow-caller whitelist mechanism for locale-helper and integrate it into locale-changing methods, shifting primary authorization from Polkit to a server-side registry.

New Features:

  • Add a SetAllowCaller D-Bus method and underlying allow-caller registry to let privileged processes register descendant callers that can invoke locale-helper without further prompts.

Enhancements:

  • Update SetLocale and GenerateLocale to first consult the allow-caller registry and fall back to Polkit only when the registry is not enabled.
  • Persist allow-caller state across service restarts with bus ID validation and automatic cleanup on D-Bus NameOwnerChanged signals.
  • Restrict SetAllowCaller access in the D-Bus bus configuration to root and the deepin-daemon group, tightening server-side access control.

Tests:

  • Add comprehensive unit tests for the allow-caller registry, including process ancestry checks, group membership validation, persistence/load behavior, and authorization edge cases.

1. 新增 allowCallerRegistry 模块实现调用者白名单注册与鉴权功能
2. 注册逻辑强制校验发起方权限仅允许 root 或 deepin-daemon 组进程注册其子
孙进程
3. 在 SetLocale 和 GenerateLocale 接口中集成白名单校验通过的进程可跳过后
续鉴权
4. 实现白名单状态通过 JSON 文件持久化并在重启时加载同时比对 BusID 防止跨
生命周期恢复
5. 监听 D-Bus NameOwnerChanged 信号实现连接断开时自动清理白名单条目
6. 将 Polkit 策略由 auth_admin_keep 改为 yes 将权限控制重心转移至服务端
白名单
7. 更新 D-Bus 总线配置严格限制 SetAllowCaller 方法仅对特权用户和组开放

Influence:
1. 验证 deepin-daemon 组内的父进程能否成功通过 SetAllowCaller 注册其子
进程
2. 验证已注册的子进程调用 SetLocale 和 GenerateLocale 时无需 Polkit 弹窗
且功能正常
3. 验证未在白名单中的普通进程调用上述方法时被正确拦截并返回错误
4. 测试非法注册请求的拒绝情况包括非特权组进程注册非子孙进程注册以及跨
UID 注册
5. 验证被注册的子进程异常退出后白名单是否能够自动清除对应条目
6. 验证 locale-helper 服务重启后白名单状态的正确恢复以及不同 BusID 下的
隔离失效
7. 验证 root 用户在注册和调用时的豁免逻辑是否正常工作

Task: https://pms.uniontech.com/task-view-393313.html
@deepin-ci-robot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: fly602

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

@sourcery-ai

sourcery-ai Bot commented Aug 11, 2026

Copy link
Copy Markdown

Reviewer's Guide

Implements a D-Bus caller allowlist mechanism for locale-helper, wiring it into SetLocale/GenerateLocale authorization, persisting allowlisted callers across restarts, and tightening D-Bus/Polkit policy to shift trust to the server-side allowlist.

Sequence diagram for allow-caller registration and locale setting authorization

sequenceDiagram
    actor PrivilegedCaller
    participant LocaleHelper as Helper
    participant AllowRegistry as allowCallerRegistry
    participant Polkit as polkit.Authority

    PrivilegedCaller->>LocaleHelper: SetAllowCaller(uniqueName)
    LocaleHelper->>AllowRegistry: addCaller(sender, uniqueName)
    AllowRegistry-->>LocaleHelper: [caller registered]
    LocaleHelper-->>PrivilegedCaller: dbusutil.ToError(nil)

    loop later
        actor AllowedCaller
        AllowedCaller->>LocaleHelper: SetLocale(locale)
        LocaleHelper->>AllowRegistry: authorize(sender)
        alt allowlist hit
            AllowRegistry-->>LocaleHelper: nil
            LocaleHelper-->>AllowedCaller: [locale set]
        else allowlist not enabled
            AllowRegistry-->>LocaleHelper: errAllowCallerNotEnabled
            LocaleHelper->>Polkit: checkAuth(sender)
            alt polkit authorized
                Polkit-->>LocaleHelper: (true, nil)
                LocaleHelper-->>AllowedCaller: [locale set]
            else polkit denied
                Polkit-->>LocaleHelper: (false or error)
                LocaleHelper-->>AllowedCaller: dbusutil.ToError(errAuthFailed)
            end
        end
    end
Loading

File-Level Changes

Change Details Files
Introduce an allow-caller registry that tracks and authorizes privileged child processes, with persistence and cleanup on D-Bus disconnect.
  • Add allowCallerRegistry implementation that validates registrars by UID, group membership, and process ancestry via /proc
  • Persist allowlisted callers to a JSON state file keyed by system bus ID and reload them on service startup filtering out stale entries
  • Subscribe to org.freedesktop.DBus NameOwnerChanged to automatically remove allowlisted entries when their connection disappears
  • Provide helper functions for group lookup, process group/parent inspection, and bus abstraction
locale-helper/allow_caller.go
Integrate the allow-caller registry into the locale-helper service lifecycle and D-Bus methods, adding SetAllowCaller and conditional Polkit fallback.
  • Extend Helper struct to hold an allowCallerRegistry instance and initialize it from main(), including signal-loop setup and deferred close
  • Add SetAllowCaller D-Bus method that registers allowed callers and logs rejected attempts
  • Replace direct checkAuth calls in SetLocale/generateLocale with authorizeOrPolkit, which consults the registry first then falls back to Polkit when not enabled
locale-helper/main.go
locale-helper/ifc.go
locale-helper/exported_methods_auto.go
Tighten D-Bus bus configuration and adjust client-facing policy to restrict who can register allowlisted callers while keeping other methods accessible.
  • Update org.deepin.dde.LocaleHelper1.conf to allow SetAllowCaller only for root user and deepin-daemon group and deny it for the default catch-all policy
  • Ensure other standard D-Bus interfaces remain accessible to all users
misc/conf/org.deepin.dde.LocaleHelper1.conf
Add comprehensive unit tests for the allow-caller registry behavior, including authorization rules, persistence, and edge conditions.
  • Introduce a mock allowCallerBus to simulate D-Bus properties in tests
  • Cover registrar authorization scenarios: root bypass, privileged group membership, UID ownership, and descendant process checks
  • Test registry behavior for add/remove/authorize, persistence save/load with bus ID matching, stale callers, and error paths
  • Add OS-integration smoke test for group ID lookup
locale-helper/allow_caller_test.go

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

@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.

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
Copy Markdown

deepin pr auto review

★ 总体评分:90分

■ 【总体评价】

代码实现了基于D-Bus的白名单调用者鉴权机制,逻辑严密且安全防护到位
代码质量高且包含完整测试,但因错误处理策略可能影响回退逻辑扣10分

■ 【详细分析】

  • 1.语法逻辑(基本正确)✓

代码整体逻辑正确,allowCallerRegistry 的实现完整,锁的使用规范,isProcessDescendant 包含了环检测。在 ifc.goauthorizeOrPolkit 函数中,当 authorize 返回非 errAllowCallerNotEnabled 错误时,直接返回 false, err,这会导致鉴权失败而非回退到 Polkit,虽然可能是预期行为,但在某些异常情况下可能阻断正常用户的回退路径。
潜在问题:authorizeOrPolkit 对非预期错误的处理过于严格,可能导致服务异常时无法回退
建议:考虑在 authorize 发生系统级错误(如 D-Bus 连接问题)时也回退到 Polkit,或者明确区分错误类型

  • 2.代码质量(优秀)✓

代码结构清晰,职责划分明确,allow_caller.go 中的接口抽象(allowCallerBus)便于测试。注释详尽,解释了设计意图(如 errAllowCallerNotEnabled 的用途)。测试覆盖率高,涵盖了各种边界条件如空指针、权限校验、持久化回滚等。
潜在问题:无
建议:无

  • 3.代码性能(高效)✓

使用 sync.RWMutex 优化读多写少场景,snapshotCallersLocked 在持锁状态下快速生成快照。isProcessDescendant 使用循环而非递归,避免了栈溢出风险,且有环检测机制。持久化使用临时文件+Rename的原子操作,性能良好。
潜在问题:无
建议:无

  • 4.代码安全(存在0个安全漏洞)✓

漏洞对比统计:新增漏洞 0 个,减少漏洞 0 个,持平 0 个
代码实现了多层安全防护:D-Bus 策略限制 SetAllowCaller 仅 root 和 deepin-daemon 组可调用;authorizeRegistrar 严格校验调用者身份(特权组、UID 一致性、进程父子关系),防止越权注册;持久化文件权限设置为 0600 且目录权限 0700,防止信息泄露;isProcessDescendant 防止了进程树遍历中的死循环。整体安全设计严密,无攻击面暴露。

  • 建议:无

■ 【改进建议代码示例】

// ifc.go 中的 authorizeOrPolkit 可以优化错误处理逻辑
func (h *Helper) authorizeOrPolkit(sender dbus.Sender) (bool, error) {
    err := h.allowCallers.authorize(sender)
    if err == nil {
        return true, nil
    }
    // 如果是未启用白名单或系统级错误,回退到 Polkit
    if errors.Is(err, errAllowCallerNotEnabled) || isSystemError(err) {
        // Not launched via deepin-security-loader or system error: fall back to Polkit.
        return h.checkAuth(sender)
    }
    return false, err
}

// 辅助函数判断是否为系统级错误(可根据实际情况扩展)
func isSystemError(err error) bool {
    // 例如 D-Bus 连接错误等
    return strings.Contains(err.Error(), "D-Bus")
}

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.

2 participants