Skip to content

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

Closed
fly602 wants to merge 9 commits into
linuxdeepin:masterfrom
fly602:master
Closed

Revert "feat: 集成 security-loader 加强安全" 及相关提交 — 回退安全启动方案#1214
fly602 wants to merge 9 commits 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 security-loader startup and authorization rollout to return the daemons to their previous architecture and deployment model.

Bug Fixes:
- Restore the existing power-management and idle-state behavior after removing the security-loader integration.

Enhancements:
- Revert security-loader handshakes, AllowCaller registries, and related per-interface authorization changes across session, system, input-device, airplane-mode, and language-selector services.
- Restore the previous D-Bus APIs and service startup behavior while retaining direct authorization for airplane-mode operations.
- Remove the security-loader package, wrappers, hardware-control policy, and associated authorization plumbing.
- Restore the prior daemon installation layout and packaging dependencies.

Build:
- Restore the standard daemon installation paths and simplify policy generation configuration.

Deployment:
- Remove security-loader runtime packaging and loader wrapper deployment from RPM and service installation.

Tests:
- Remove tests covering the reverted security-loader and concurrent short-idle state behavior.

Chores:
- Clean up reverted security-related policy and generated interface changes.

@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

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

Copy link
Copy Markdown

Reviewer's Guide

Reverts the security-loader based secure startup integration and AllowCaller-based authorization, simplifying power, airplane mode, input devices, and system/session daemon flows back to polkit/DBus-based access control and the previous install layout, while keeping short-idle and power features functional under the older model.

Sequence diagram for AirplaneMode authorization reverted to direct polkit

sequenceDiagram
    actor Client
    participant AirplaneModeManager as AirplaneMode_Manager
    participant SystemBus as DBus_SystemBus
    participant PolkitAuthority as Polkit_Authority

    Client->>AirplaneModeManager: Enable(sender, enableAirplaneMode)
    AirplaneModeManager->>AirplaneModeManager: checkAuthorization(actionId, string(sender))
    AirplaneModeManager->>SystemBus: SystemBus()
    SystemBus-->>AirplaneModeManager: systemBus
    AirplaneModeManager->>PolkitAuthority: NewAuthority(systemBus)
    AirplaneModeManager->>PolkitAuthority: CheckAuthorization(actionId, subject)
    PolkitAuthority-->>AirplaneModeManager: AuthorizationResult
    alt authorized
        AirplaneModeManager->>AirplaneModeManager: block(rfkillTypeAll, enableAirplaneMode)
        AirplaneModeManager-->>Client: *dbus.Error(nil)
    else not authorized
        AirplaneModeManager-->>Client: *dbus.Error("not authorized")
    end
Loading

Sequence diagram for system idle state handling via systemPower

sequenceDiagram
    actor Client
    participant SystemDaemon as DdeSystemDaemon
    participant SystemPower as SystemPower_Power
    participant IdleFile as KernelIdleFile

    Client->>SystemDaemon: SetIdleState(state)
    SystemDaemon->>SystemDaemon: setState(idleStatePath, state)
    SystemDaemon->>SystemPower: ShortIdleState().Get(0)
    SystemPower-->>SystemDaemon: shortIdleState
    alt shortIdleState == state
        SystemDaemon-->>Client: *dbus.Error("Short idle state not exchange.")
    else shortIdleState != state
        alt file == idleStatePath
            SystemDaemon->>SystemDaemon: systemPowerSetShortIdleState(state)
            SystemDaemon->>SystemPower: SetShortIdleState(0, state)
            SystemPower-->>SystemDaemon: error or nil
        end
        SystemDaemon->>IdleFile: ReadFile(file)
        IdleFile-->>SystemDaemon: content
        SystemDaemon->>IdleFile: WriteFile(file, newContent)
        IdleFile-->>SystemDaemon: ok
        SystemDaemon-->>Client: *dbus.Error(nil)
    end
Loading

File-Level Changes

Change Details Files
Remove security-loader integration and AllowCaller authorization paths from system and session daemons.
  • Delete securityloader package and its tests, wrappers, and handshake logic
  • Strip securityloader usage from dde-session-daemon and dde-system-daemon main, including AllowCallerRegistry wiring
  • Remove SetAllowCaller exported methods and D-Bus policy rules tied to AllowCaller for Daemon1, Power1, InputDevices1, and AirplaneMode1
bin/dde-session-daemon/main.go
bin/dde-system-daemon/main.go
bin/dde-system-daemon/exported_methods_auto.go
system/power1/exported_methods_auto.go
system/inputdevices1/exported_methods_auto.go
system/airplane_mode1/exported_methods_auto.go
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/scripts/dde-session-daemon-loader-wrapper
misc/scripts/langselector-loader-wrapper
securityloader/*
Rework airplane mode authorization to use direct polkit authority checks instead of security-loader.
  • Remove Manager.allowCallers and SetAllowCaller from airplane_mode1
  • Introduce checkAuthorization helper using org.freedesktop.policykit1 Authority.CheckAuthorization
  • Update Enable/EnableWifi/EnableBluetooth to call checkAuthorization based on dbus.Sender
system/airplane_mode1/manager.go
system/airplane_mode1/exported_methods_auto.go
misc/polkit-action/org.deepin.dde.airplane.policy
Simplify power module manager concurrency and short idle handling, and rewire system-daemon to use the power1 D-Bus interface directly.
  • Drop Daemon.managerMu lock and ShortIdleState/shortIdleEnable mutexes in Manager, using plain fields
  • Move short idle control from dde-system-daemon through power1 D-Bus (systemPower ShortIdleState/SetShortIdleState) instead of loader-based interface
  • Remove securityloader-based authorization and AllowCaller from power manager, and add a SetShortIdleState D-Bus method
  • Adjust short idle logic to be non-error-returning and based on the ShortIdleState field
  • Remove power_test short-idle concurrency tests and loader-based short idle controller tests
system/power1/daemon.go
system/power1/manager.go
system/power1/manager_powersave.go
system/power1/manager_ifc.go
system/power1/exported_methods_auto.go
system/power1/power_test.go
bin/dde-system-daemon/power.go
bin/dde-system-daemon/power_test.go
misc/polkit-action/org.deepin.dde.power.policy
Remove security-loader integration from input devices and langselector, falling back to simpler D-Bus methods without AllowCaller.
  • Drop InputDevices.allowCallers and SetAllowCaller, and stop wiring securityloader.DefaultRegistry
  • Change Touchpad constructor to not take AllowCallerRegistry and remove polkit-based authorization via securityloader
  • Simplify SetTouchpadEnable to directly call internal setter without authorization middleware
  • Remove langselector securityloader handshake helper and its use in langselector main
system/inputdevices1/inputdevices.go
system/inputdevices1/inputdevices_ifc.go
system/inputdevices1/touchpad.go
system/inputdevices1/exported_methods_auto.go
bin/langselector/main.go
langselector1/securityloader.go
Revert install-time layout and RPM packaging changes that were made to support security-loader and libexec relocation.
  • Remove DAEMON_LIBDIR indirection and install session/langselector binaries back under /usr/lib/deepin-daemon instead of libexec wrappers
  • Drop installation of loader wrappers and service-trigger under DAEMON_LIBDIR; use PREFIX/lib/deepin-daemon paths again
  • Adjust RPM spec Makefile patching section to only rewrite lib->libexec paths, no DAEMON_LIBDIR-specific overrides, and fix dde-lockservice ExecStart path
Makefile
rpm/dde-daemon.spec
misc/systemd/services/system/dde-system-daemon.service
misc/systemd/services/user/org.dde.session.Daemon1.service
misc/scripts/dde-lock.sh
misc/scripts/dde-shutdown.sh
misc/service-trigger/*.json

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

★ 总体评分:10分

■ 【总体评价】

代码实现了安全授权机制的大规模回退与电源状态管理的重构,但引入了致命的逻辑错误和极其严重的权限绕过漏洞
逻辑错误导致屏幕状态可能无法正确写入,且移除全部鉴权导致任意本地用户可控制系统电源与硬件状态,按规则强制降至10分

■ 【详细分析】

  • 1.语法逻辑 存在致命错误 ✕

bin/dde-system-daemon/power.gosetState 函数中,当传入的 fileidleScreenStatePath(屏幕状态)时,函数依然会去获取电源的 shortIdleState 进行比较。如果获取到的短空闲状态恰好与目标屏幕状态布尔值相同,函数会直接返回 errors.New("Short idle state not exchange.") 错误,从而跳过后续的内核文件写入逻辑,导致屏幕状态无法被正确设置。此外,在 system/power1/daemon.go 中移除了 managerMu 读写锁,而在 system/power1/manager.go 中移除了 ShortIdleStateMushortIdleEnableMu,导致 Start/Stop 并发访问 d.manager,以及 setShortIdleStatedoSetMode 并发读写 ShortIdleState 时存在严重的数据竞争。
潜在问题:屏幕状态设置失效引发系统界面卡死或无法休眠;并发读写导致进程崩溃或状态错乱
建议:在 setState 函数开头增加对 file 类型的判断,仅当 file == d.idleStatePath 时才获取和比较 shortIdleState;恢复 daemon.gomanager.go 中关键状态变量的读写锁保护

  • 2.代码质量 存在严重问题 ✕

本次变更删除了 bin/dde-system-daemon/power_test.gosystem/power1/power_test.go 中的所有并发与状态测试用例,且未补充任何针对新 D-Bus 调用链路的单元测试,导致核心逻辑无测试覆盖。在 bin/dde-system-daemon/power.go 中,将原本使用的 os.ReadFileos.WriteFile 降级为已废弃的 ioutil.ReadFileioutil.WriteFile。代码中残留大量明显的临时调试日志,如 logger.Infof("##### setState shortIdleState : %v, state : %v", shortIdleState, state)。在 system/airplane_mode1/manager.go 中新增的 checkAuthorization 函数没有复用现有的 D-Bus 连接,而是每次调用都重新获取系统总线连接。
潜在问题:废弃 API 在未来 Go 版本中面临移除风险;调试日志污染生产环境日志;重复建立 D-Bus 连接增加代码复杂度与资源消耗
建议:将 ioutil 替换回 os 包对应函数;清理带有 ##### 等明显调试特征的日志;重构 checkAuthorization 使其接受 *dbus.Conn 参数以复用连接

  • 3.代码性能 存在严重性能问题 ✕

system/airplane_mode1/manager.go 新增的 checkAuthorization 函数中,每次鉴权操作都调用 dbus.SystemBus() 获取连接并实例化 polkit.NewAuthority。虽然 godbus 底层对共享连接有一定处理,但在高频调用场景(如频繁切换飞行模式或蓝牙状态)下,这种写法会产生不必要的对象分配与潜在的总线轮询开销。此外,将原本的进程内模块调用 (shortIdleController) 改为 D-Bus 远程调用 (org.deepin.dde.Power1),虽然属于架构调整,但在高频率的 idle 状态切换时引入了额外的 IPC 序列化与上下文切换成本。
潜在问题:高频鉴权调用时的性能损耗;D-Bus IPC 增加状态同步延迟
建议:将 checkAuthorization 改为结构体方法,在 Manager 初始化时缓存 polkit.Authority 对象;评估 D-Bus 调用频率,必要时引入本地缓存或防抖机制

  • 4.代码安全 存在 6 个安全漏洞(严重4个,高危1个,中危1个) ✕

漏洞对比统计:新增漏洞 6 个,减少漏洞 0 个,持平 0 个
本次变更大规模移除了 securityloader 机制及 Polkit 前置鉴权,导致多个涉及系统核心状态与硬件控制的 D-Bus 接口完全暴露,任何本地普通用户均可直接调用这些接口篡改系统电源策略、触发内核状态变更或禁用硬件设备,攻击面极广。

  • 安全漏洞1(严重):[权限绕过] 在 bin/dde-system-daemon/power.goSetIdleStateSetScreenState 函数中,完全移除了 sender 参数及相关的 Polkit 鉴权逻辑。攻击者可通过 dbus-sendgdbus 命令直接调用这两个方法,向 /sys/power/ 下的内核文件写入任意状态,可导致系统拒绝服务(如强制保持唤醒阻止休眠)或意外挂起。 ——非常重要

  • 安全漏洞2(严重):[权限绕过] 在 system/power1/manager_ifc.goSetTlpMode 函数中,移除了 sender 参数和 securityloader.AuthorizeWithPolkit 鉴权。任意用户可调用此接口将系统电源模式强制设置为性能模式或节能模式,可能导致笔记本过热损坏或系统性能被恶意降级。 ——非常重要

  • 安全漏洞3(严重):[权限绕过] 在 system/inputdevices1/touchpad.goSetTouchpadEnable 函数中,移除了 sender 参数和 Polkit 鉴权。任意用户可调用此接口禁用触摸板,在无外接鼠标的笔记本上构成拒绝服务攻击,严重影响用户正常使用。 ——非常重要

  • 安全漏洞4(严重):[权限绕过] 在 bin/dde-system-daemon/power.gosetState 函数中,通过 D-Bus 调用 systemPower.SetShortIdleState 时,由于 system/power1/manager_ifc.go 中新增的 SetShortIdleState 方法未做任何鉴权,且 setState 本身也无鉴权,形成完整的无鉴权调用链。攻击者可利用此链路操纵系统的短空闲电源策略。 ——非常重要

  • 安全漏洞5(高危):[权限配置错误] 在 misc/polkit-action/org.deepin.dde.airplane.policy 中,将飞行模式的 allow_active 策略从 auth_admin_keep 降级为 yes。结合 system/airplane_mode1/manager.go 中恢复的纯 Polkit 鉴权逻辑,这意味着任何处于活动会话的普通用户都可以无需输入密码直接开启飞行模式,可被用于恶意切断目标主机的所有网络连接。 ——非常重要

  • 安全漏洞6(中危):[不安全的默认配置] 在 system/power1/manager_ifc.go 中新增导出的 SetShortIdleState D-Bus 方法,且在 misc/conf/org.deepin.dde.Power1.conf 的默认策略中未对该方法进行显式的 deny 规则限制,完全依赖应用层缺失的鉴权,违反了最小权限原则的纵深防御配置。 ——非常重要

  • 建议:立即恢复 SetIdleStateSetScreenStateSetTlpModeSetTouchpadEnableSetShortIdleState 的 Polkit 鉴权逻辑,复用 system/airplane_mode1/manager.go 中的 checkAuthorization 模式并传入对应的 actionId;将 org.deepin.dde.airplane.policyallow_active 恢复为 auth_admin_keep;在 D-Bus 配置文件中对敏感方法添加默认拒绝策略

■ 【改进建议代码示例】

// bin/dde-system-daemon/power.go
package main

import (
	"errors"
	"fmt"
	"io/ioutil"
	"strconv"

	"github.com/godbus/dbus/v5"
	systemPower "github.com/linuxdeepin/go-dbus-factory/system/org.deepin.dde.power1"
	polkit "github.com/linuxdeepin/go-dbus-factory/system/org.freedesktop.policykit1"
	"github.com/linuxdeepin/go-lib/dbusutil"
	"github.com/linuxdeepin/go-lib/utils"
)

func (d *Daemon) systemPowerSetShortIdleState(state bool) {
	logger.Info("systemPowerSetShortIdleState : ", state)
	if d.systemPower != nil {
		err := d.systemPower.SetShortIdleState(0, state)
		if err != nil {
			logger.Warning("failed to SetShortIdleState, err : ", err)
		}
	}
}

func (d *Daemon) setState(file string, state bool) error {
	// 修复逻辑错误:仅在处理 idleStatePath 时检查短空闲状态
	if file == d.idleStatePath {
		shortIdleState, err := d.systemPower.ShortIdleState().Get(0)
		if err != nil {
			logger.Warning("Get systemPower.ShortIdleState err :", err)
		} else {
			logger.Infof("setState shortIdleState : %v, state : %v", shortIdleState, state)
			if shortIdleState == state {
				logger.Info("shortIdleState is same with state : ", state)
				return errors.New("short idle state not exchange")
			}
			d.systemPowerSetShortIdleState(state)
		}
	}

	if !utils.IsFileExist(file) {
		err := fmt.Errorf("%s not found", file)
		logger.Warning(err)
		return err
	}

	content, err := ioutil.ReadFile(file)
	if err != nil {
		logger.Errorf("Failed to read file %s: %v", file, err)
		return err
	}

	contentStr := strings.TrimSpace(string(content))
	currentValue, err := strconv.Atoi(contentStr)
	if err != nil {
		logger.Errorf("Failed to parse content %s: %v", contentStr, err)
		return err
	}

	newValue := 0
	if state {
		newValue = 1
	}
	if currentValue == newValue {
		logger.Infof("State %s is already %v", file, state)
		return nil
	}

	newContent := strconv.Itoa(newValue)
	err = ioutil.WriteFile(file, []byte(newContent), 0644)
	if err != nil {
		logger.Errorf("Failed to write file %s: %v", file, err)
		return err
	}
	return nil
}

// checkPolkitAuth 通用的 Polkit 鉴权函数
func (d *Daemon) checkPolkitAuth(sender dbus.Sender, actionID string) error {
	authority := polkit.NewAuthority(d.service.Conn())
	subject := polkit.MakeSubject(polkit.SubjectKindSystemBusName)
	subject.SetDetail("name", string(sender))

	result, err := authority.CheckAuthorization(0, subject, actionID,
		nil, polkit.CheckAuthorizationFlagsAllowUserInteraction, "")
	if err != nil {
		return fmt.Errorf("polkit check failed: %w", err)
	}
	if !result.IsAuthorized {
		return errors.New("access denied by polkit")
	}
	return nil
}

// 修复安全漏洞:恢复 SetIdleState 的 sender 鉴权
func (d *Daemon) SetIdleState(sender dbus.Sender, state bool) *dbus.Error {
	if err := d.checkPolkitAuth(sender, "org.deepin.dde.daemon.set-idle-state"); err != nil {
		logger.Warningf("SetIdleState authorization failed: %v", err)
		return dbusutil.ToError(err)
	}
	logger.Infof("SetIdleState %s try set state: %v", d.idleStatePath, state)
	return dbusutil.ToError(d.setState(d.idleStatePath, state))
}

// 修复安全漏洞:恢复 SetScreenState 的 sender 鉴权
func (d *Daemon) SetScreenState(sender dbus.Sender, state bool) *dbus.Error {
	if err := d.checkPolkitAuth(sender, "org.deepin.dde.daemon.set-screen-state"); err != nil {
		logger.Warningf("SetScreenState authorization failed: %v", err)
		return dbusutil.ToError(err)
	}
	logger.Infof("SetScreenState %s try set state: %v", d.idleScreenStatePath, state)
	return dbusutil.ToError(d.setState(d.idleScreenStatePath, state))
}

@deepin-ci-robot

Copy link
Copy Markdown

@fly602: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
github-pr-review-ci cbcdf1e link true /test github-pr-review-ci

Full PR test history. Your PR dashboard.

Details

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

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

2 participants