Skip to content

Revert "feat: 集成 security-loader 加强安全" 及相关提交 — 回退安全启动方案 - #1215

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

Revert "feat: 集成 security-loader 加强安全" 及相关提交 — 回退安全启动方案#1215
fly602 wants to merge 1 commit into
linuxdeepin:masterfrom
fly602:master

Conversation

@fly602

@fly602 fly602 commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

回退 security-loader 安全启动方案及其相关提交,包括:

  • 集成 security-loader 及 AllowCaller 权限控制
  • 输入设备、飞行模式、LocaleHelper、lastore Agent 等接口的鉴权改造
  • polkit 提权回退机制
  • 启动流程修复及 authorization 修复

原因

该方案影响范围过广,涉及 dde-session-daemon、dde-system-daemon、 langselector、polkit 策略、systemd 服务等多处改动,且与 V25 项目
当前架构存在不匹配之处,不适合在此阶段落地。如果上层应用适配会
影响其项目架构。

后续计划

需重新评估安全方案的整体设计,制定更契合 V25 架构的分阶段实施方案。

Summary by Sourcery

Revert the broad security-loader integration and restore the previous daemon startup, D-Bus authorization, power control, and packaging behavior.

Enhancements:

  • Remove the security-loader startup handshake, caller registry, and related authorization hooks from session and system daemon interfaces.
  • Restore direct D-Bus method signatures and service access for power, airplane mode, and input-device operations.
  • Simplify power short-idle state management and retain the power daemon interface for controlling it.
  • Restore the standard daemon installation layout and remove the security-loader packaging dependency and supporting wrappers.

Build:

  • Remove security-loader-specific packaging and installation configuration while restoring the standard daemon library layout.

Deployment:

  • Remove security-loader-related service and policy artifacts, including the hardware-control policy and loader wrappers.

Tests:

  • Remove security-loader and short-idle concurrency tests that covered the reverted implementation.

Chores:

  • Delete the securityloader package and associated language-selector security integration.

@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 @fly602, 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 19, 2026

Copy link
Copy Markdown

Reviewer's Guide

This PR removes the security-loader–based secure startup scheme and reverts related authorization and installation changes, restoring simpler direct Polkit usage, DBus policies, and daemon wiring while keeping the newer short-idle and power-management behavior functional via direct power1 integration.

Sequence diagram for SetIdleState with direct power1 integration

sequenceDiagram
  actor Client
  participant SystemDaemon as dde_system_daemon.Daemon
  participant Power as power1.Manager
  participant KernelFile as idle_state_file

  Client->>SystemDaemon: SetIdleState(state bool)
  SystemDaemon->>SystemDaemon: setState(file, state)
  SystemDaemon->>Power: ShortIdleState().Get(0)
  Power-->>SystemDaemon: shortIdleState bool, error
  alt shortIdleState != state
    SystemDaemon->>Power: SetShortIdleState(state)
  else shortIdleState == state
    SystemDaemon->>SystemDaemon: return errors.New("Short idle state not exchange.")
  end
  SystemDaemon->>KernelFile: ioutil.ReadFile(file)
  KernelFile-->>SystemDaemon: content []byte
  SystemDaemon->>KernelFile: ioutil.WriteFile(file, newContent, 0644)
  KernelFile-->>SystemDaemon: write result
  SystemDaemon-->>Client: dbus.Error (nil or error)
Loading

File-Level Changes

Change Details Files
Simplify system power daemon manager lifecycle and short-idle wiring, removing internal lock-based indirection.
  • Remove Daemon.manager mutex and associated ShortIdleState accessors from the power daemon
  • Bind dbus property change handlers directly to Daemon.manager instead of a local variable
  • Simplify Stop logic to operate directly on Daemon.manager batteries and destruction
system/power1/daemon.go
Revert security-loader driven short-idle control from dde-system-daemon and rewire to call power1 DBus API directly while simplifying idle state file handling.
  • Remove loader/securityloader integration and shortIdleController abstraction from system daemon power code
  • Introduce systemPower client field in Daemon and use it to query and set ShortIdleState via DBus
  • Inline authorization-free SetIdleState/SetScreenState methods and collapse writeStateFile into setState using ioutil for IO
bin/dde-system-daemon/power.go
bin/dde-system-daemon/main.go
Replace security-loader based airplane-mode authorization with direct Polkit checks and remove AllowCaller plumbing.
  • Drop AllowCallerRegistry from Manager and its SetAllowCaller/authorize helpers
  • Add checkAuthorization helper using org.freedesktop.policykit1 Authority.CheckAuthorization
  • Use checkAuthorization in Enable/EnableWifi/EnableBluetooth and update rfkill control logic accordingly
system/airplane_mode1/manager.go
system/airplane_mode1/exported_methods_auto.go
misc/conf/org.deepin.dde.AirplaneMode1.conf
misc/polkit-action/org.deepin.dde.airplane.policy
Simplify power manager short-idle state handling, removing concurrency primitives and security-loader AllowCaller integration while exposing a DBus method to drive short idle.
  • Remove AllowCallerRegistry and related SetAllowCaller from Manager
  • Remove RWMutex protection around ShortIdleState and shortIdleEnable; use plain booleans
  • Change setShortIdleState from returning error to void and adjust callers to rely on ShortIdleState directly
  • Add SetShortIdleState DBus method and export it in auto-generated methods
  • Update logging and power mode behavior to reference ShortIdleState directly
system/power1/manager.go
system/power1/manager_ifc.go
system/power1/exported_methods_auto.go
system/power1/manager_powersave.go
system/power1/daemon.go
bin/dde-system-daemon/power.go
Remove security-loader based authorization and AllowCaller handling from input devices, simplifying touchpad control to a direct DBus call.
  • Drop AllowCallerRegistry from InputDevices and Touchpad structures
  • Remove SetAllowCaller DBus method from InputDevices and touchpad securityloader-based auth logic
  • Adjust newTouchpad construction and exported methods to use plain Touchpad.SetTouchpadEnable
system/inputdevices1/inputdevices.go
system/inputdevices1/inputdevices_ifc.go
system/inputdevices1/touchpad.go
system/inputdevices1/exported_methods_auto.go
misc/conf/org.deepin.dde.InputDevices1.conf
Remove security-loader handshake and wrappers from session daemon and langselector, restoring direct startup and standard installation layout.
  • Delete securityloader handshake from dde-session-daemon main and its loader wrapper scripts
  • Remove langselector securityloader integration and wrappers
  • Adjust Makefile to install all binaries and service-trigger JSONs into /usr/lib/deepin-daemon instead of DAEMON_LIBDIR indirection
  • Drop deepin-security-loader dependency from packaging and undo DAEMON_LIBDIR overrides in RPM spec
bin/dde-session-daemon/main.go
bin/langselector/main.go
Makefile
rpm/dde-daemon.spec
misc/scripts/dde-session-daemon-loader-wrapper
misc/scripts/langselector-loader-wrapper
misc/systemd/services/user/org.dde.session.Daemon1.service
Clean up DBus configuration and polkit policy files to remove AllowCaller-specific privileges and security-loader coupling for Daemon and Power services.
  • Remove SetAllowCaller call permissions from org.deepin.dde.Daemon1, Power1, InputDevices1, and AirplaneMode1 DBus configs
  • Delete hardware-control polkit action and securityloader-specific policy definitions
  • Adjust systemd and polkit wiring to match the reverted, simpler daemon model
misc/conf/org.deepin.dde.Daemon1.conf
misc/conf/org.deepin.dde.Power1.conf
misc/conf/org.deepin.dde.InputDevices1.conf
misc/conf/org.deepin.dde.AirplaneMode1.conf
misc/polkit-action/org.deepin.dde.hardware-control.policy.in
misc/polkit-action/org.deepin.dde.power.policy
misc/systemd/services/system/dde-system-daemon.service
Remove securityloader package and its tests completely from the codebase.
  • Delete AllowCaller registry implementation and tests
  • Delete handshake and polkit helper implementations and tests
  • Remove langselector1/securityloader helper
securityloader/allowcaller.go
securityloader/allowcaller_test.go
securityloader/handshake.go
securityloader/handshake_test.go
securityloader/polkit.go
langselector1/securityloader.go
Drop short-idle related tests that depended on removed locking semantics.
  • Remove tests verifying shortIdleEnable concurrent access and disabled behavior of setShortIdleState
  • Keep remaining power1 tests for DBus path validity
system/power1/power_test.go
bin/dde-system-daemon/power_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

@deepin-ci-robot

Copy link
Copy Markdown

deepin pr auto review

★ 总体评分:20分

■ 【总体评价】

代码移除了security-loader机制并回退至标准polkit授权,但多个DBus方法在移除鉴权时未补充替代授权检查,导致任意本地用户可无授权修改系统电源状态和内核文件
逻辑层面修复了短idle状态判断bug,但因引入3个高危权限绕过漏洞和并发竞态条件,安全风险极高

■ 【详细分析】

  • 1.语法逻辑(存在错误)✕
    system/power1/manager.go中移除了ShortIdleStateMushortIdleEnableMu互斥锁,但ShortIdleState字段在setShortIdleStatedoSetModetime.AfterFunc回调、updatePowerMode等多goroutine中并发读写,构成数据竞争。system/power1/daemon.go移除了managerMu锁,Start/Stop与DBus方法调用之间无同步保护。bin/dde-system-daemon/power.gosetState移除了idleStateMu锁,并发调用会导致内核文件写入交错。setShortIdleStateif !m.ShortIdleState的修复是正确的,因为此时m.ShortIdleState已被赋值为state
    潜在问题:并发数据竞争可能导致短idle状态与电源模式不一致;daemon.goStartd.manager赋值与DBus导出之间无屏障,外部调用可能访问未完全初始化的manager
    建议:对ShortIdleStateshortIdleEnable恢复互斥锁保护或使用atomic.Bool;在Start完成DBus导出后再设置d.manager
  • 2.代码质量(较差)✕
    bin/dde-system-daemon/power.goos.ReadFile/os.WriteFile回退为ioutil.ReadFile/ioutil.WriteFile,而ioutil自Go 1.16已废弃。删除了power_test.gopower1/power_test.go中的全部测试用例,包括并发安全测试TestShortIdleEnableConcurrentAccess,在移除互斥锁的同时删除了验证并发安全的测试。setStatelogger.Infof("##### setState...")保留了调试风格的日志输出。systemPowerSetShortIdleState方法在d.systemPower为nil时静默返回,无错误上报。
    潜在问题:废弃API使用会在新Go版本中产生告警;测试覆盖严重退化;调试日志残留
    建议:恢复使用os.ReadFile/os.WriteFile;保留或重写并发测试用例;清理调试日志格式
  • 3.代码性能(无性能问题)✓
    移除security-loader的handshake流程减少了进程启动时的IPC开销,DBus代理调用替代内部模块接口查找的性能差异可忽略。airplane_mode1/manager.gocheckAuthorization每次调用都新建dbus.SystemBus()连接,但这是原有模式,非本次引入的退化。
    建议:可复用mgr.service.Conn()替代每次新建系统总线连接
  • 4.代码安全(存在 6 个安全漏洞)✕
    漏洞对比统计:新增漏洞 6 个,减少漏洞 0 个,持平 0 个
    移除security-loader鉴权机制后,多个DBus导出方法未补充polkit授权检查,且polkit策略被削弱,导致任意本地用户可无授权执行特权操作

  • 安全漏洞1(高危):权限绕过 在 bin/dde-system-daemon/power.goSetIdleStateSetScreenState 方法中,移除了sender参数和authorize调用,任何本地DBus客户端可直接调用这两个方法向内核状态文件(/sys/路径)写入任意值,可导致系统不稳定或拒绝服务 ——非常重要

  • 安全漏洞2(高危):权限绕过 在 system/power1/manager_ifc.goSetShortIdleState 方法中,新增的DBus导出方法无任何授权检查,任意本地用户可触发电源模式切换和WiFi状态变更(通过deepin-power-control idle wifi命令),影响系统电源管理和网络连接 ——非常重要

  • 安全漏洞3(高危):权限绕过 在 system/power1/manager_ifc.goSetTlpMode 方法中,移除了sender参数和polkit授权检查(原action: org.deepin.dde.power.doAction),任意本地用户可直接修改TLP电源配置模式 ——非常重要

  • 安全漏洞4(中危):权限绕过 在 system/inputdevices1/touchpad.goSetTouchpadEnable 方法中,移除了sender参数和polkit授权检查(原action: org.deepin.dde.inputdevices.set-touchpad),任意本地用户可远程禁用其他用户的触摸板 ——非常重要

  • 安全漏洞5(中危):策略降级 在 misc/polkit-action/org.deepin.dde.airplane.policy 中,将allow_activeauth_admin_keep修改为yes,活跃用户切换飞行模式无需任何认证,可被恶意利用中断网络连接 ——非常重要

  • 安全漏洞6(中危):并发竞态 在 system/power1/manager.go 中,移除ShortIdleStateMu互斥锁后,ShortIdleState字段在DBus方法SetShortIdleState、信号回调ConnectChanged、定时器time.AfterFunc中并发读写,可能导致电源模式状态不一致,在极端情况下使系统进入异常电源状态 ——非常重要

  • 建议:为SetIdleStateSetScreenStateSetShortIdleStateSetTlpModeSetTouchpadEnable恢复polkit授权检查;恢复airplane模式polkit策略为auth_admin_keep;对ShortIdleState恢复互斥锁保护

■ 【改进建议代码示例】

// system/power1/manager_ifc.go - 恢复polkit授权检查
package power

import (
	dbus "github.com/godbus/dbus/v5"
	polkit "github.com/linuxdeepin/go-dbus-factory/system/org.freedesktop.policykit1"
	"github.com/linuxdeepin/go-lib/dbusutil"
)

func (m *Manager) checkAuthorization(actionId string, sender dbus.Sender) error {
	authority := polkit.NewAuthority(m.service.Conn())
	subject := polkit.MakeSubject(polkit.SubjectKindSystemBusName)
	subject.SetDetail("name", string(sender))
	ret, err := authority.CheckAuthorization(0, subject, actionId,
		nil, polkit.CheckAuthorizationFlagsAllowUserInteraction, "")
	if err != nil {
		return err
	}
	if !ret.IsAuthorized {
		return dbusutil.ToError(nil)
	}
	return nil
}

func (m *Manager) SetTlpMode(sender dbus.Sender, mode string) *dbus.Error {
	if err := m.checkAuthorization("org.deepin.dde.power.doAction", sender); err != nil {
		logger.Warningf("SetTlpMode authorization failed: %q", err.Error())
		return dbusutil.ToError(err)
	}
	logger.Info("SetTlpMode : ", mode)
	return dbusutil.ToError(m.setTlpMode(mode))
}

func (m *Manager) SetShortIdleState(sender dbus.Sender, state bool) *dbus.Error {
	if err := m.checkAuthorization("org.deepin.dde.daemon.set-idle-state", sender); err != nil {
		logger.Warningf("SetShortIdleState authorization failed: %q", err.Error())
		return dbusutil.ToError(err)
	}
	logger.Info(" SetShortIdleState : ", state)
	m.setShortIdleState(state)
	return nil
}
// system/power1/manager.go - 恢复互斥锁保护
type Manager struct {
	// ... 其他字段
	ShortIdleState   bool
	ShortIdleStateMu sync.RWMutex
	shortIdleEnableMu sync.RWMutex
	shortIdleEnable   bool
}

func (m *Manager) setShortIdleState(state bool) {
	logger.Info(" setShortIdleState state : ", state)
	m.shortIdleEnableMu.RLock()
	enabled := m.shortIdleEnable
	m.shortIdleEnableMu.RUnlock()
	if !enabled {
		logger.Info("System not open dsg of shortIdleEnable.")
		return
	}

	m.ShortIdleStateMu.Lock()
	defer m.ShortIdleStateMu.Unlock()

	if m.ShortIdleState != state {
		m.ShortIdleState = state
		// ... 其余逻辑保持不变
	}
	// 使用 m.ShortIdleState 进行判断(已修复的bug)
	if !m.ShortIdleState {
		// 退出短idle逻辑
	}
}

zccrs
zccrs previously approved these changes Aug 19, 2026
回退 security-loader 安全启动方案及其相关提交,包括:

- 集成 security-loader 及 AllowCaller 权限控制
- 输入设备、飞行模式、LocaleHelper、lastore Agent 等接口的鉴权改造
- polkit 提权回退机制
- 启动流程修复及 authorization 修复

原因

该方案影响范围过广,涉及 dde-session-daemon、dde-system-daemon、
langselector、polkit 策略、systemd 服务等多处改动,且与 V25 项目
当前架构存在不匹配之处,不适合在此阶段落地。如果上层应用适配会
影响其项目架构。

后续计划

需重新评估安全方案的整体设计,制定更契合 V25 架构的分阶段实施方案。
@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

1 similar comment
@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

@fly602 fly602 closed this Aug 19, 2026
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