Skip to content

fix(accounts1): authorize group info D-Bus methods to prevent info leak - #1196

Open
mhduiy wants to merge 1 commit into
masterfrom
bugfix-370877
Open

fix(accounts1): authorize group info D-Bus methods to prevent info leak#1196
mhduiy wants to merge 1 commit into
masterfrom
bugfix-370877

Conversation

@mhduiy

@mhduiy mhduiy commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

fix(accounts1): authorize group info D-Bus methods to prevent info leak

  1. Add PolicyKit authorization (org.deepin.dde.accounts.user-administration)
    to the GetGroups method before returning all system group names;
  2. Add the same authorization to the GetGroupInfoByName method before
    returning a group's full info, which includes its member list;
  3. Switch internal callers (NewManager init and the
    CreateGroup/DeleteGroup/ModifyGroup property refresh) to the
    data-layer users.GetAllGroups() so already-authenticated paths do
    not re-authorize;
    Log: Authorize the accounts group-info reading D-Bus methods to block
    unauthorized disclosure of system group names and group membership.
    Influence: Prevents unprivileged local users from enumerating system
    groups and reading privileged group (sudo/wheel/root) member lists.

fix(accounts1): 为组信息 D-Bus 方法补充鉴权以防信息泄露

  1. 为 GetGroups 方法在返回全部系统组名前补充 PolicyKit 鉴权
    (org.deepin.dde.accounts.user-administration);
  2. 为 GetGroupInfoByName 方法在返回组完整信息(含组成员名单)前补充
    同一鉴权;
  3. 将内部调用方(NewManager 初始化及 CreateGroup/DeleteGroup/ModifyGroup
    刷新属性)改为直接调用数据层 users.GetAllGroups(),避免在已鉴权路径上
    重复鉴权;
    Log: 为 accounts 组信息读取类 D-Bus 方法补充管理员鉴权,阻止未授权读取
    系统组名与组成员名单。
    PMS: BUG-370877
    Influence: 防止本地非特权用户经 D-Bus 枚举系统组并读取特权组
    (sudo/wheel/root)成员名单。

Change-Id: I09dee0d436f331df8cd3da55d2e783652b65321f

Summary by Sourcery

Authorize group information D-Bus methods and avoid redundant authorization for internal group list refreshes.

Bug Fixes:

  • Protect GetGroups and GetGroupInfoByName D-Bus methods with authorization checks to prevent unauthorized disclosure of system group names and membership.

Enhancements:

  • Use the data-layer users.GetAllGroups for internal group list initialization and refresh to bypass redundant authorization in trusted code paths.

1. Add PolicyKit authorization (org.deepin.dde.accounts.user-administration)
   to the GetGroups method before returning all system group names;
2. Add the same authorization to the GetGroupInfoByName method before
   returning a group's full info, which includes its member list;
3. Switch internal callers (NewManager init and the
   CreateGroup/DeleteGroup/ModifyGroup property refresh) to the
   data-layer users.GetAllGroups() so already-authenticated paths do
   not re-authorize;
Log: Authorize the accounts group-info reading D-Bus methods to block
unauthorized disclosure of system group names and group membership.
Influence: Prevents unprivileged local users from enumerating system
groups and reading privileged group (sudo/wheel/root) member lists.

fix(accounts1): 为组信息 D-Bus 方法补充鉴权以防信息泄露

1. 为 GetGroups 方法在返回全部系统组名前补充 PolicyKit 鉴权
   (org.deepin.dde.accounts.user-administration);
2. 为 GetGroupInfoByName 方法在返回组完整信息(含组成员名单)前补充
   同一鉴权;
3. 将内部调用方(NewManager 初始化及 CreateGroup/DeleteGroup/ModifyGroup
   刷新属性)改为直接调用数据层 users.GetAllGroups(),避免在已鉴权路径上
   重复鉴权;
Log: 为 accounts 组信息读取类 D-Bus 方法补充管理员鉴权,阻止未授权读取
系统组名与组成员名单。
PMS: BUG-370877
Influence: 防止本地非特权用户经 D-Bus 枚举系统组并读取特权组
(sudo/wheel/root)成员名单。

Change-Id: I09dee0d436f331df8cd3da55d2e783652b65321f

@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 @mhduiy, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@sourcery-ai

sourcery-ai Bot commented Aug 6, 2026

Copy link
Copy Markdown
Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Adds PolicyKit authorization checks to group information D-Bus methods and updates internal call sites to use the lower-level users.GetAllGroups() helper instead of reusing the now-authorized D-Bus API.

Sequence diagram for authorized group info D-Bus access

sequenceDiagram
  actor DbusClient
  participant Manager
  participant PolicyKit as checkAuth
  participant Users as users
  participant DbusUtil as dbusutil

  DbusClient->>Manager: GetGroups(sender)
  Manager->>PolicyKit: checkAuth(sender)
  alt authorized
    PolicyKit-->>Manager: nil
    Manager->>Users: GetAllGroups()
    Users-->>Manager: groups, err
    Manager->>DbusUtil: ToError(err)
    Manager-->>DbusClient: groups, busErr
  else access denied
    PolicyKit-->>Manager: error
    Manager->>DbusUtil: ToError(error)
    Manager-->>DbusClient: nil, busErr
  end
Loading

File-Level Changes

Change Details Files
Authorize D-Bus methods that expose group information to prevent unauthorized enumeration of system groups and their membership.
  • Updated GetGroups D-Bus method signature to accept a dbus.Sender and perform m.checkAuth before returning group names.
  • Updated GetGroupInfoByName D-Bus method signature to accept a dbus.Sender and perform m.checkAuth before returning detailed group info including members.
  • On authorization failure in these methods, log a debug message and return a D-Bus error via dbusutil.ToError.
accounts1/manager_ifc.go
Avoid re-authorizing when refreshing group list in already-authenticated code paths by using the data-layer helper directly.
  • Switched CreateGroup, DeleteGroup, and ModifyGroup to refresh the group list using users.GetAllGroups() instead of calling the GetGroups D-Bus method.
  • Updated NewManager initialization to populate GroupList using users.GetAllGroups() instead of GetGroups, keeping startup behavior but bypassing D-Bus-level authorization for internal use.
accounts1/manager_ifc.go
accounts1/manager.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

@deepin-ci-robot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: mhduiy

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

@deepin-ci-robot

Copy link
Copy Markdown

deepin pr auto review

★ 总体评分:100分

■ 【总体评价】

代码修复了DBus接口未授权访问漏洞,逻辑严谨且实现正确
代码完全符合安全规范与逻辑要求,无任何扣分项

■ 【详细分析】

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

修改了GetGroups和GetGroupInfoByName的函数签名增加sender参数,并将内部调用替换为users.GetAllGroups,所有调用点均正确适配,无编译错误或逻辑瑕疵
潜在问题:无
建议:无

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

代码精准地分离了DBus对外接口的鉴权逻辑与内部底层调用逻辑,避免了在内部调用时触发不必要的鉴权检查,符合职责单一原则
潜在问题:无
建议:无

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

内部调用直接走底层users.GetAllGroups,避免了经过DBus层带来的额外序列化与反序列化开销,性能有所改善
潜在问题:无
建议:无

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

漏洞对比统计:新增漏洞 0 个,减少漏洞 0 个,持平 0 个
本次提交成功修复了原有的未授权信息泄露风险,通过增加checkAuth校验确保只有授权的DBus客户端才能获取组信息,内部调用绕过鉴权不暴露给外部,未引入新漏洞
建议:建议后续对其他类似的DBus接口进行排查,确保均具备完善的权限校验机制

■ 【改进建议代码示例】

// 当前代码实现已足够优秀,无需额外修改,此处展示当前最佳实践供参考
func (m *Manager) GetGroups(sender dbus.Sender) (groups []string, busErr *dbus.Error) {
	if err := m.checkAuth(sender); err != nil {
		logger.Debug("[GetGroups] access denied:", err)
		return nil, dbusutil.ToError(err)
	}
	groups, err := users.GetAllGroups()
	return groups, dbusutil.ToError(err)
}

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