feat: 加固未鉴权的 system 级 dbus 接口（v20 安全整改合入 v25） - #1197
Open
fly602 wants to merge 1 commit into
Open
Conversation
|
[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. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Reviewer's GuideAdds polkit-based authorization checks to previously unauthenticated system-level D-Bus setters in power, input devices (touchpad), and system daemon power control, plus corresponding polkit action definitions, following existing airplane_mode1 patterns without touching generated bindings. Sequence diagram for polkit authorization on system D-Bus setterssequenceDiagram
actor SessionDaemon
participant Power1Manager as org.deepin.dde.Power1.Manager
participant InputDevicesTouchpad as org.deepin.dde.InputDevices1.Touchpad
participant SystemDaemon as org.deepin.dde.Daemon1
participant PolicykitAuthority as org.freedesktop.policykit1.Authority
SessionDaemon->>Power1Manager: SetTlpMode(sender, mode)
Power1Manager->>PolicykitAuthority: checkAuthorization(actionSetTlpMode, sender)
PolicykitAuthority-->>Power1Manager: CheckAuthorization
alt authorized
Power1Manager->>Power1Manager: setTlpMode(mode)
else not authorized
Power1Manager-->>SessionDaemon: dbus.Error("not authorized")
end
SessionDaemon->>InputDevicesTouchpad: SetTouchpadEnable(sender, enabled)
InputDevicesTouchpad->>PolicykitAuthority: checkAuthorization(actionSetTouchpadEnable, sender)
PolicykitAuthority-->>InputDevicesTouchpad: CheckAuthorization
alt authorized
InputDevicesTouchpad->>InputDevicesTouchpad: setTouchpadEnable(enabled)
else not authorized
InputDevicesTouchpad-->>SessionDaemon: dbus.Error("not authorized")
end
SessionDaemon->>SystemDaemon: SetIdleState(sender, state)
SystemDaemon->>PolicykitAuthority: checkAuth(actionSetIdleState, sender)
PolicykitAuthority-->>SystemDaemon: CheckAuthorization
alt authorized
SystemDaemon->>SystemDaemon: setState(idleStatePath, state)
else not authorized
SystemDaemon-->>SessionDaemon: dbus.Error("not authorized")
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The
checkAuthorizationhelper is duplicated in multiple packages with identical logic; consider extracting a shared helper (or reusing the existingcheckAuthwhere possible) to avoid divergence in future changes. - Both
checkAuthorizationhelpers calldbus.SystemBus()on every invocation; if these setters are called frequently, it may be worth reusing an existing bus connection or caching the authority to avoid repeated setup overhead.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `checkAuthorization` helper is duplicated in multiple packages with identical logic; consider extracting a shared helper (or reusing the existing `checkAuth` where possible) to avoid divergence in future changes.
- Both `checkAuthorization` helpers call `dbus.SystemBus()` on every invocation; if these setters are called frequently, it may be worth reusing an existing bus connection or caching the authority to avoid repeated setup overhead.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
fly602
force-pushed
the
agent/developer/1b88c2b8
branch
from
August 7, 2026 07:11
052b723 to
d59d092
Compare
1. Port the v20 security hardening to v25 by adding polkit authorization (sender first param + checkAuthorization/checkAuth + .policy action with allow_active:yes/allow_inactive:no/ allow_any:no) to the system-level D-Bus methods that previously lacked authentication, mirroring the existing system/airplane_mode1. 2. system/power1 (org.deepin.dde.Power1): SetShortIdleState and SetTlpMode gain a sender + checkAuthorization guard; add org.deepin.dde.power.set-short-idle-state and set-tlp-mode actions to misc/polkit-action/org.deepin.dde.power.policy. 3. system/inputdevices1 (org.deepin.dde.InputDevices1.Touchpad): SetTouchpadEnable gains sender + checkAuthorization (helper added to inputdevices_ifc.go); add a new misc/polkit-action/org.deepin.dde.inputdevices.policy with the set-touchpad-enable action. 4. bin/dde-system-daemon (org.deepin.dde.Daemon1): SetIdleState and SetScreenState gain sender + the existing checkAuth helper; add org.deepin.dde.daemon.set-idle-state and set-screen-state actions to org.deepin.dde.daemon.system.policy.in (regenerated by the build via ts_to_policy). 5. Add misc/polkit-rules/org.deepin.dde.power.rules and org.deepin.dde.inputdevices.rules to allowlist root internal callers (subject.user === "root"), because root has no active local session and the new .policy defaults would otherwise silently deny dde-system-daemon->Power1.SetShortIdleState and keyevent1->InputDevices1.SetTouchpadEnable, breaking short-idle power management and the keyboard touchpad toggle. 6. system/airplane_mode1 is already compliant, no change. Adding sender does not change the on-wire signature (dbusutil-gen and the runtime dispatcher skip dbus.Sender), so exported_methods_auto.go and the go-dbus-factory bindings are unchanged; active local user callers stay covered by allow_active:yes with no new dialog. Log: Harden previously unauthenticated system-level D-Bus interfaces with polkit authorization. Influence: 1. Verify short-idle power management (wifi on/off, power mode switching) still works from the session UI with no polkit dialog. 2. Verify the keyboard touchpad on/off shortcut still toggles the touchpad enable state. 3. Verify setting TLP/power-save mode from the UI works with no polkit prompt. 4. Confirm root internal callers (dde-system-daemon -> SetShortIdleState, keyevent1 -> SetTouchpadEnable) are allowed by the new .rules and not silently denied. 5. Confirm a non-session external caller is now denied by polkit on SetShortIdleState, SetTlpMode, SetTouchpadEnable, SetIdleState and SetScreenState. feat: 加固未鉴权的 system 级 dbus 接口 1. 将 v20 安全整改合入 v25：为原先未鉴权的 system 级 D-Bus 方法补齐 polkit 鉴权（sender 首参 + checkAuthorization/checkAuth + .policy 动作，allow_active:yes/allow_inactive:no/allow_any:no），参照仓内 system/airplane_mode1 既有做法。 2. system/power1（org.deepin.dde.Power1）：SetShortIdleState、 SetTlpMode 增加 sender + checkAuthorization；在 misc/polkit-action/org.deepin.dde.power.policy 新增 org.deepin.dde.power.set-short-idle-state 与 set-tlp-mode 动作。 3. system/inputdevices1（org.deepin.dde.InputDevices1.Touchpad）： SetTouchpadEnable 增加 sender + checkAuthorization（helper 加到 inputdevices_ifc.go）；新增 misc/polkit-action/org.deepin.dde.inputdevices.policy，含 set-touchpad-enable 动作。 4. bin/dde-system-daemon（org.deepin.dde.Daemon1）：SetIdleState、 SetScreenState 增加 sender 并复用本仓既有 checkAuth；在 org.deepin.dde.daemon.system.policy.in 新增 org.deepin.dde.daemon.set-idle-state 与 set-screen-state 动作 （构建期 ts_to_policy 重新生成 .policy）。 5. 新增 misc/polkit-rules/org.deepin.dde.power.rules 与 org.deepin.dde.inputdevices.rules，对 root 内部调用方 （subject.user === "root"）放行，避免 root 无活跃本地会话时被新 .policy 默认值静默拒绝，导致短 idle 电源管理 （dde-system-daemon->Power1.SetShortIdleState）与键盘触摸板开关 （keyevent1->InputDevices1.SetTouchpadEnable）失效。 6. system/airplane_mode1 已合规，无改动。新增 sender 不改变线上签名 （dbusutil-gen 与运行时派发均跳过 dbus.Sender），故 exported_methods_auto.go 与 go-dbus-factory 绑定不变；活跃本地 用户调用方仍由 allow_active:yes 静默放行，不新增用户侧鉴权弹窗。 Log: 加固原先未鉴权的 system 级 D-Bus 接口，补齐 polkit 鉴权。 Influence: 1. 验证从会话 UI 触发的短 idle 电源管理（wifi 开关、电源模式切换）仍正常，无 polkit 弹窗。 2. 验证键盘触摸板开/关快捷键仍能切换触摸板使能状态。 3. 验证从 UI 设置 TLP/节能模式不弹 polkit 提示。 4. 确认 root 内部调用方（dde-system-daemon->SetShortIdleState、 keyevent1->SetTouchpadEnable）被新 .rules 放行，不被静默拒绝。 5. 确认非会话的外部调用方在 SetShortIdleState、SetTlpMode、 SetTouchpadEnable、SetIdleState、SetScreenState 上被 polkit 拒绝。 PMS: TASK-393313
fly602
force-pushed
the
agent/developer/1b88c2b8
branch
from
August 7, 2026 07:28
d59d092 to
57cacf6
Compare
deepin pr auto review★ 总体评分：60分■ 【总体评价】
■ 【详细分析】
■ 【改进建议代码示例】 diff --git a/misc/polkit-action/org.deepin.dde.daemon.system.policy.in b/misc/polkit-action/org.deepin.dde.daemon.system.policy.in
index bda2ac11b..123456789 100644
--- a/misc/polkit-action/org.deepin.dde.daemon.system.policy.in
+++ b/misc/polkit-action/org.deepin.dde.daemon.system.policy.in
@@ -31,7 +31,7 @@
<message>Authentication is required to set the short idle state</message>
<defaults>
<allow_any>no</allow_any>
- <allow_inactive>no</allow_inactive>
- <allow_active>yes</allow_active>
+ <allow_inactive>auth_admin_keep</allow_inactive>
+ <allow_active>auth_admin_keep</allow_active>
</defaults>
</action>
<action id="org.deepin.dde.daemon.set-screen-state">
@@ -39,8 +39,8 @@
<message>Authentication is required to set the screen idle state</message>
<defaults>
<allow_any>no</allow_any>
- <allow_inactive>no</allow_inactive>
- <allow_active>yes</allow_active>
+ <allow_inactive>auth_admin_keep</allow_inactive>
+ <allow_active>auth_admin_keep</allow_active>
</defaults>
</action>
diff --git a/misc/polkit-rules/org.deepin.dde.inputdevices.rules b/misc/polkit-rules/org.deepin.dde.inputdevices.rules
index a026c88b1..000000000 100644
--- a/misc/polkit-rules/org.deepin.dde.inputdevices.rules
+++ b/misc/polkit-rules/org.deepin.dde.inputdevices.rules
@@ -1,12 +0,0 @@
-polkit.addRule(function(action, subject) {
- // Allow root internal callers (e.g. keyevent1 running inside
- // dde-system-daemon invoking org.deepin.dde.InputDevices1.Touchpad
- // .SetTouchpadEnable over the system bus) to toggle the touchpad.
- // Root has no active local session, so the allow_active:yes default in
- // the .policy does not match and would otherwise fall back to
- // allow_any:no and silently deny the call.
- if (action.id === "org.deepin.dde.inputdevices.set-touchpad-enable" &&
- subject.user === "root") {
- return polkit.Result.YES;
- }
-});
diff --git a/system/power1/manager_ifc.go b/system/power1/manager_ifc.go
index 9ff425ee7..123456789 100644
--- a/system/power1/manager_ifc.go
+++ b/system/power1/manager_ifc.go
@@ -125,7 +125,9 @@ func (m *Manager) SetShortIdleState(sender dbus.Sender, state bool) *dbus.Error
logger.Warningf("checkAuthorization failed, err: %v, actionId=%v", err, actionSetShortIdleState)
return dbusutil.ToError(err)
}
- m.setShortIdleState(state)
- return nil
+ err = m.setShortIdleState(state)
+ if err != nil {
+ return dbusutil.ToError(err)
+ }
+ return nil
} |
|
@fly602: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
背景
将 v20 上的 dde dbus 安全整改合入 v25（DDE-63）。对 v25 中未鉴权的 system 级 D-Bus 接口按 v25 polkit 风格加固：
sender dbus.Sender首参 +checkAuthorization+.policy动作（allow_active: yes、allow_inactive: no、allow_any: no），参照本仓已合规的system/airplane_mode1。改动内容
system/power1（org.deepin.dde.Power1）
manager_ifc.go：SetShortIdleState、SetTlpMode增加sender dbus.Sender首参与checkAuthorization鉴权；新增同名包级checkAuthorizationhelper（与airplane_mode1同形）。misc/polkit-action/org.deepin.dde.power.policy：新增动作org.deepin.dde.power.set-short-idle-state、org.deepin.dde.power.set-tlp-mode。system/inputdevices1（org.deepin.dde.InputDevices1.Touchpad）
touchpad.go：SetTouchpadEnable增加sender+checkAuthorization。inputdevices_ifc.go：新增包级checkAuthorizationhelper。daemon.go：新增动作 id 常量。misc/polkit-action/org.deepin.dde.inputdevices.policy：新建（set-touchpad-enable动作）。该文件命中.gitignore的*.policy规则，按本仓airplane/bluetooth/display/power等纯源.policy的既有做法git add -f纳入版本管理。bin/dde-system-daemon（org.deepin.dde.Daemon1）
power.go：SetIdleState、SetScreenState增加sender首参，复用本仓既有checkAuth（wallpaper.go）。misc/polkit-action/org.deepin.dde.daemon.system.policy.in：新增org.deepin.dde.daemon.set-idle-state、org.deepin.dde.daemon.set-screen-state动作（构建期ts_to_policy重新生成.policy，与既有enable/disable-readonly-protection同流程）。system/airplane_mode1
Enable/EnableWifi/EnableBluetooth均带sender+checkAuthorization+org.deepin.dde.airplane.policy），仅复核，无改动。关于生成文件 / 调用方 / go-dbus-factory
exported_methods_auto.go未改动：dbusutil-gen与运行时 dispatcher 均跳过dbus.Sender（见go-lib/dbusutil/_tool/dbusutil-gen/exported_methods.go跳过dbus.Sender、dbusutil.go运行时同样跳过），Fn为interface{}，加sender不改变em输出。已提交的power1/exported_methods_auto.go为旧生成器（无排序）产物，若用现网生成器重跑只会触发与鉴权无关的方法重排，为避免无关 churn 故未重跑。sender不进入线上签名/内省（AirplaneMode.xml中Enable仅含enabled一参即为例证），Power.xml/Touchpad.xml/auto.go仍有效。.rules放行：SetShortIdleState由bin/dde-system-daemon（root）调用 → polkit 对 root 默认放行；SetIdleState/SetScreenState由session/power1、keybinding1（dde-session-daemon，活跃本地用户）经系统总线调用 →allow_active: yes静默放行；SetTouchpadEnable由会话侧inputdevices1（活跃本地用户）与system/keyevent1（root）调用 → 均放行。SetLEDEnabled在 v25 已移除，跳过；RegisterAgent弹窗为跨仓库问题（调用点lastore1/agent.go，弹窗由 lastore-daemon 侧策略触发），不在本 PR 范围。约束遵守
org.deepin.dde.*、sender首参、polkit 三件套。Summary by Sourcery
Harden previously unauthenticated system-level D-Bus interfaces for power, input devices, and system daemon idle/screen state by integrating v25-style polkit authorization and sender-based checks.
New Features:
Enhancements:
Build: