Skip to content

fix(security): add caller verification for more Manager and Updater methods - #457

Open
qiuzhiqian wants to merge 1 commit into
masterfrom
fix-dbus-policy
Open

fix(security): add caller verification for more Manager and Updater methods#457
qiuzhiqian wants to merge 1 commit into
masterfrom
fix-dbus-policy

Conversation

@qiuzhiqian

@qiuzhiqian qiuzhiqian commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Add sender parameter and checkInvokePermission calls to the following D-Bus methods that were missing authorization checks:

Manager: GetArchivesInfo, PackageExists, PackageInstallable, GetUpdateLogs, GetHistoryLogs, PackagesSize, PackagesDownloadSize, RegisterAgent, SetAutoClean, UnRegisterAgent, QueryAllSizeWithSource, PowerOff, CanRollback

Updater: GetCheckIntervalAndTime, ListMirrorSources, SetDeliveryDownloadSpeedLimit, SetDeliveryUploadSpeedLimit

Bug: https://pms.uniontech.com/bug-view-371795.html

Summary by Sourcery

Enforce caller authorization for additional Manager and Updater D-Bus methods to close missing security checks.

Bug Fixes:

  • Add caller verification to Manager methods handling archive info, package queries, logs, size calculations, agent registration, auto-clean configuration, aggregate size queries, power operations, and rollback capability checks.
  • Require caller authorization for Updater methods related to update check scheduling, mirror source listing, and delivery speed limit configuration.

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

Sorry @qiuzhiqian, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@github-actions

Copy link
Copy Markdown

CLA Assistant Lite bot:
提交邮箱中包含我们的合作伙伴,但您似乎并非合作伙伴的成员或对接人,请联系相关对接人将您添加至组织之中,或由其重新发起 Pull Request。
The commit email domain belongs to one of our partners, but it seems you are not yet a member of the current organization, please contact the contact person to add you to the organization or let them submit the Pull Request.

xml seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You can retrigger this bot by commenting recheck in this Pull Request

@sourcery-ai

sourcery-ai Bot commented Jul 29, 2026

Copy link
Copy Markdown

Reviewer's Guide

Adds D-Bus caller verification to several Manager and Updater methods by threading a sender parameter through their signatures and invoking existing permission checks before performing privileged operations.

Sequence diagram for Manager methods with caller verification

sequenceDiagram
    actor Client
    participant DBus
    participant Manager

    Client->>DBus: GetArchivesInfo(sender)
    DBus->>Manager: GetArchivesInfo(sender)
    Manager->>Manager: checkInvokePermission(sender)
    alt [permission denied]
        Manager-->>DBus: dbusutil.ToError(err)
        DBus-->>Client: error
    else [permission granted]
        Manager->>Manager: getArchiveInfo()
        Manager-->>DBus: info, nil
        DBus-->>Client: info
    end
Loading

Sequence diagram for Updater methods with caller verification

sequenceDiagram
    actor Client
    participant DBus
    participant Updater
    participant Manager

    Client->>DBus: SetDeliveryDownloadSpeedLimit(sender, limitConfig)
    DBus->>Updater: SetDeliveryDownloadSpeedLimit(sender, limitConfig)
    Updater->>Manager: checkInvokePermission(sender)
    alt [permission denied]
        Updater-->>DBus: dbusutil.ToError(err)
        DBus-->>Client: error
    else [permission granted]
        Updater->>Updater: json.Unmarshal(limitConfig, speedLimitConfig)
        Updater-->>DBus: nil
        DBus-->>Client: ok
    end
Loading

File-Level Changes

Change Details Files
Add sender argument and permission checks to previously unguarded Manager D-Bus methods.
  • Updated multiple Manager interface methods to accept a dbus.Sender parameter instead of relying on implicit caller context.
  • Inserted checkInvokePermission(sender) calls at the start of methods, returning dbusutil.ToError on failure.
  • Ensured DelayAutoQuit and other existing logic remain intact while being executed only after permission validation where appropriate.
src/lastore-daemon/manager_ifc.go
Add sender-based permission checks to Updater D-Bus methods that read or modify update configuration and delivery limits.
  • Extended several Updater methods to accept a dbus.Sender parameter and wired them to use the Manager's checkInvokePermission for authorization.
  • Guarded configuration read methods (e.g., GetCheckIntervalAndTime, ListMirrorSources) with permission checks before accessing internal state.
  • Protected delivery speed limit mutators with permission checks to prevent unauthorized configuration changes.
src/lastore-daemon/updater_ifc.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

…ethods

Add sender parameter and checkInvokePermission calls to the following
D-Bus methods that were missing authorization checks:

Manager: GetArchivesInfo, PackageExists, PackageInstallable,
GetUpdateLogs, GetHistoryLogs, PackagesSize, PackagesDownloadSize,
SetAutoClean, QueryAllSizeWithSource, PowerOff, CanRollback

Updater: GetCheckIntervalAndTime, ListMirrorSources,
SetDeliveryDownloadSpeedLimit, SetDeliveryUploadSpeedLimit

Bug: https://pms.uniontech.com/bug-view-371795.html
@deepin-ci-robot

Copy link
Copy Markdown

deepin pr auto review

★ 总体评分:100分

■ 【总体评价】

代码通过为D-Bus导出方法统一添加sender参数和权限校验,有效修复了越权访问安全漏洞
逻辑正确且符合D-Bus规范,代码模式高度一致,无性能损耗,安全修复彻底

■ 【详细分析】

  • 1.语法逻辑(完全正确)✓

代码在manager_ifc.goupdater_ifc.go中为多个方法添加了sender dbus.Sender作为首个参数,符合D-Bus库的反射注入规范,调用者信息会被自动填充。在方法内部调用m.checkInvokePermission(sender)u.manager.checkInvokePermission(sender)进行鉴权,返回值处理正确。
潜在问题:无
建议:无需修改

  • 2.代码质量(良好)✓

修改模式高度统一,所有未受保护的D-Bus方法均添加了一致的鉴权逻辑,错误处理使用统一的dbusutil.ToError(err)转换,代码可读性强。
潜在问题:鉴权代码存在一定重复,但受限于Go D-Bus库的方法导出机制,这种重复是合理的
建议:保持现状即可,若未来有更多方法需要鉴权,可考虑通过代码生成器自动注入鉴权代码以减少手工维护成本

  • 3.代码性能(无性能问题)✓

新增的checkInvokePermission调用通常涉及获取调用者凭证或检查Polkit策略,属于轻量级系统调用,对D-Bus方法调用的整体性能影响可忽略不计。
潜在问题:无
建议:确保checkInvokePermission内部实现避免复杂的阻塞操作或频繁的磁盘I/O

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

漏洞对比统计:新增漏洞 0 个,减少漏洞 1 个,持平 0 个
本次代码修改是对D-Bus接口越权访问漏洞的修复,通过在执行敏感操作前校验调用者权限,有效防止了未授权应用调用系统级操作。修改本身未引入新的安全漏洞。

  • 安全漏洞1(无):本次修改未引入安全漏洞,且成功修复了原有的D-Bus接口越权访问漏洞——非常重要

  • 建议:确保checkInvokePermission函数内部实现了严格的Polkit策略校验或UID匹配逻辑,防止权限校验被绕过

■ 【改进建议代码示例】

// 当前代码已正确修复安全漏洞,无需额外修改。以下为最佳实践示例,确保权限校验的健壮性:

// 文件:src/lastore-daemon/manager_ifc.go
func (m *Manager) GetArchivesInfo(sender dbus.Sender) (info string, busErr *dbus.Error) {
	m.service.DelayAutoQuit()
	if err := m.checkInvokePermission(sender); err != nil {
		return "", dbusutil.ToError(err)
	}
	info, err := getArchiveInfo()
	if err != nil {
		return "", dbusutil.ToError(err)
	}
	return info, nil
}

// 确保在 manager.go 中 checkInvokePermission 实现了严格的 Polkit 校验
// func (m *Manager) checkInvokePermission(sender dbus.Sender) error {
//     // 获取调用者 PID 和 UID
//     pid, err := m.service.GetConnPID(string(sender))
//     if err != nil {
//         return err
//     }
//     // 调用 Polkit 进行授权检查
//     // ...
// }

@deepin-ci-robot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: qiuzhiqian, zhaohuiw42

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

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