feat: add allowcaller package for security-loader caller allowlist - #119
Open
fly602 wants to merge 1 commit into
Open
feat: add allowcaller package for security-loader caller allowlist#119fly602 wants to merge 1 commit into
fly602 wants to merge 1 commit into
Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
|
[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 a new reusable allowcaller package implementing the deepin-security-loader caller allowlist (registry plus client handshake) and associated tests, refactoring and unifying prior duplicated implementations. Sequence diagram for allowcaller Handshake with deepin-security-loadersequenceDiagram
actor CallerProcess
participant Handshake as allowcaller.Handshake
participant DBus as dbus.SystemBus
participant LoaderReq as security_loader_fd1
participant LoaderResp as security_loader_fd2
CallerProcess->>Handshake: Handshake(args, destinations)
Handshake->>Handshake: parseLoaderArgs(args)
alt [not launched by loader or parse error]
Handshake-->>CallerProcess: cleanedArgs, loaded=false / error
else [launched by loader]
Handshake->>DBus: SystemBus()
DBus-->>Handshake: conn
Handshake->>DBus: conn.Names()
DBus-->>Handshake: uniqueName
Handshake->>LoaderReq: validateLoaderFD(fd1, O_WRONLY)
Handshake->>LoaderResp: validateLoaderFD(fd2, O_RDONLY)
Handshake->>LoaderReq: Write(request JSON)
Handshake->>LoaderReq: Close()
par async read with timeout
Handshake->>LoaderResp: readResponse(fd2)
LoaderResp-->>Handshake: response bytes
end
Handshake->>Handshake: json.Unmarshal(response)
alt [response.Result == true]
Handshake-->>CallerProcess: cleanedArgs, loaded=true, nil
else [authorization failed]
Handshake-->>CallerProcess: cleanedArgs, loaded=true, error
end
end
Sequence diagram for Registry AddCaller authorization and persistencesequenceDiagram
actor SecurityLoader
participant Registry as allowcaller.Registry
participant Bus as busService
participant FS as Filesystem
SecurityLoader->>Registry: AddCaller(scope, sender, uniqueName)
Registry->>Bus: NameHasOwner(uniqueName)
Bus-->>Registry: hasOwner
alt [caller has owner]
Registry->>Registry: authorizeRegistrar(sender, uniqueName)
Registry->>Bus: GetConnUID(sender)
Bus-->>Registry: senderUID
alt [senderUID != 0]
Registry->>Bus: GetConnGroups(sender)
Bus-->>Registry: groups
Registry->>Bus: GetConnUID(uniqueName)
Bus-->>Registry: targetUID
Registry->>Bus: GetConnPID(sender)
Bus-->>Registry: senderPID
Registry->>Bus: GetConnPID(uniqueName)
Bus-->>Registry: targetPID
Registry->>Registry: isProcessDescendant(targetPID, senderPID, processParent)
end
Registry->>Registry: saveLocked()
Registry->>FS: writeState(persistedState)
FS-->>Registry: ok
Registry-->>SecurityLoader: nil
else [no owner]
Registry-->>SecurityLoader: error
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
fly602
force-pushed
the
agent/developer/1b88c2b8
branch
2 times, most recently
from
August 7, 2026 09:53
efb57b8 to
f237f22
Compare
1. Add a reusable allowcaller package implementing the deepin-security-loader caller-allowlist mechanism for D-Bus services that are launched by deepin-security-loader. 2. registry.go: a multi-scope Registry keeps the exact system-bus unique names vouched for by deepin-security-loader for each protected D-Bus object (scope). Protected methods call Authorize(scope, sender) and reject callers whose unique name was not registered via AddCaller. State is persisted scoped to the current system-bus lifetime (BusID) and entries auto-remove on a NameOwnerChanged watch; root is trusted and registrars are validated by the deepin-daemon group, same uid and process descendant checks. 3. handshake.go: Handshake consumes the --fd1/--fd2 arguments injected by deepin-security-loader and registers the process system-bus unique name for the requested destinations, returning cleanedArgs and a loaded flag the caller must honour to avoid starting without authorization. 4. The package is scope-name agnostic (the v25 D-Bus names live in the consuming projects) and avoids a circular go-dbus-factory dependency by using raw D-Bus match rules plus dbusutil.SignalLoop for the NameOwnerChanged watch. 5. This unifies and parameterizes the v20 dde-daemon securityloader/allowcaller.go and dde-api locale-helper/allow_caller.go so both consumers share one tested implementation. 6. Add unit tests (registry_test.go + handshake_test.go, 21 cases) covering authorization, persistence, cross-scope isolation, registrar validation, descendant checks, fd parse/validate and response limiting; online coverage 56.4%. The known non-blocking test-only -race item in TestReadResponseLimit is left for a follow-up; the Stage 2 end-to-end handshake field-name check against the real deepin-security-loader binary is tracked as a Stage 2 prerequisite by the reviewer. Log: Add a reusable allowcaller package implementing the deepin-security-loader caller-allowlist and handshake for D-Bus services. Influence: 1. Verify go test ./allowcaller/... passes (21 cases, coverage 56.4%) in a clean environment. 2. Confirm go build ./allowcaller/ and go vet ./allowcaller/ are clean. 3. Confirm no existing go-lib consumer breaks: this is a pure new package with no go.mod or existing-file change. 4. Confirm the package avoids a circular go-dbus-factory dependency (raw D-Bus match rules + dbusutil.SignalLoop). feat: 新增 allowcaller 包实现 loader 调用者白名单 1. 新增可复用的 allowcaller 包，实现 deepin-security-loader 的 调用者白名单机制，供由 deepin-security-loader 启动的 D-Bus 服务使用。 2. registry.go：多 scope 的 Registry 记录由 deepin-security-loader 担保的每个受保护 D-Bus 对象（scope）的 精确系统总线唯一名。受保护方法调用 Authorize(scope, sender)， 拒绝未通过 AddCaller 注册的调用方。状态按当前系统总线生命周期 （BusID）持久化，并在 NameOwnerChanged 监听中自动清理条目；root 信任，注册方按 deepin-daemon 组、同 uid 及进程后代校验。 3. handshake.go：Handshake 消费 deepin-security-loader 注入的 --fd1/--fd2 参数，为请求的 destinations 注册进程系统总线唯一名， 返回 cleanedArgs 与 loaded 标志，调用方必须据此处理以避免在无 鉴权情况下启动。 4. 本包与 scope 名无关（v25 的 D-Bus 名由消费方项目定义），并通过 裸 D-Bus match rule 加 dbusutil.SignalLoop 做 NameOwnerChanged 监听，避免对 go-dbus-factory 的循环依赖。 5. 将 v20 dde-daemon 的 securityloader/allowcaller.go 与 dde-api 的 locale-helper/allow_caller.go 合并并参数化，使两个消费方 共用同一份经过测试的实现。 6. 新增单元测试（registry_test.go + handshake_test.go，21 用例）， 覆盖鉴权、持久化、跨 scope 隔离、注册方校验、后代校验、fd 解析/ 校验与响应限制，在线覆盖率 56.4%。已知非阻塞的仅测试代码 -race 项（TestReadResponseLimit）留作后续修复；Stage 2 对真实 deepin-security-loader 二进制的握手字段名端到端核对由审核员列为 Stage 2 前置项。 Log: 新增可复用的 allowcaller 包，实现 deepin-security-loader 的 调用者白名单与握手，供 D-Bus 服务使用。 Influence: 1. 验证 go test ./allowcaller/... 通过（21 用例，覆盖率 56.4%）。 2. 确认 go build ./allowcaller/ 与 go vet ./allowcaller/ 干净。 3. 确认不破坏既有 go-lib 消费方：本 PR 为纯新增包，无 go.mod 或既有文件改动。 4. 确认本包避免对 go-dbus-factory 的循环依赖（裸 D-Bus match rule + dbusutil.SignalLoop）。 PMS: TASK-393313
fly602
force-pushed
the
agent/developer/1b88c2b8
branch
from
August 7, 2026 10:26
f237f22 to
9eb4680
Compare
deepin pr auto review★ 总体评分：60分■ 【总体评价】
■ 【详细分析】
■ 【改进建议代码示例】 // registry.go - 修复锁管理导致的死锁风险
func (r *Registry) AddCaller(scope string, sender dbus.Sender, uniqueName string) (err error) {
if r == nil {
return errors.New("allow-caller registry is nil")
}
if scope == "" {
return errors.New("scope is empty")
}
if sender == "" {
return errors.New("D-Bus sender is empty")
}
if !strings.HasPrefix(uniqueName, ":") {
return fmt.Errorf("invalid D-Bus unique name %q", uniqueName)
}
hasOwner, err := r.service.NameHasOwner(uniqueName)
if err != nil {
return fmt.Errorf("check D-Bus owner %q failed: %w", uniqueName, err)
}
if !hasOwner {
return fmt.Errorf("D-Bus caller %q has no owner", uniqueName)
}
if err := r.authorizeRegistrar(sender, uniqueName); err != nil {
return err
}
r.persistMu.Lock()
defer r.persistMu.Unlock()
// 提取为内部函数，确保 panic 时 mu 也能被释放
err = r.addCallerLocked(scope, uniqueName)
if err != nil {
return err
}
logger.Infof("registered allow-caller %s for %s", uniqueName, scope)
return nil
}
func (r *Registry) addCallerLocked(scope string, uniqueName string) (err error) {
r.mu.Lock()
defer r.mu.Unlock()
callers := r.callers[scope]
if callers == nil {
callers = make(map[string]struct{})
r.callers[scope] = callers
}
if _, exists := callers[uniqueName]; exists {
return nil
}
callers[uniqueName] = struct{}{}
if err := r.saveLocked(); err != nil {
delete(r.callers[scope], uniqueName)
if len(r.callers[scope]) == 0 {
delete(r.callers, scope)
}
return err
}
return nil
}
// registry.go - 修复凭证混用导致的权限绕过风险
func (s serviceBus) GetConnGroups(name string) ([]uint32, error) {
pid, err := s.GetConnPID(name)
if err != nil {
return nil, err
}
// 防御性检查：如果进程有 CAP_SETGID，拒绝信任其 /proc 中的动态组信息
// 防止进程在建立 D-Bus 连接后动态加入特权组绕过校验
hasSetGID, err := hasCapability(pid, "CAP_SETGID")
if err == nil && hasSetGID {
return nil, fmt.Errorf("caller pid %d has CAP_SETGID, cannot trust dynamic groups", pid)
}
return getProcessGroups(pid)
}
// hasCapability 检查进程是否拥有指定 capability
func hasCapability(pid uint32, capName string) (bool, error) {
content, err := os.ReadFile(fmt.Sprintf("/proc/%d/status", pid))
if err != nil {
return false, err
}
for _, line := range strings.Split(string(content), "\n") {
if strings.HasPrefix(line, "CapEff:") {
fields := strings.Fields(line)
if len(fields) != 2 {
return false, nil
}
capVal, err := strconv.ParseUint(fields[1], 16, 64)
if err != nil {
return false, nil
}
// CAP_SETGID 的 bit 位为 6
if capName == "CAP_SETGID" && (capVal&(1<<6)) != 0 {
return true, nil
}
return false, nil
}
}
return false, 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.
背景
本 PR 是 DDE-63（v20 dde dbus 安全整改合入 v25）的 基础层（Stage 1 第 2 步）：抽取
go-lib/allowcaller公共包，供 dde-daemon 与 dde-api 共用，消除 v20 中 dde-daemonsecurityloader/allowcaller.go（~505 行）与 dde-apilocale-helper/allow_caller.go（~447 行）的重复实现（~450 行）。v25 的安全整改方向已由架构师基于实际 v20 diff 重新定稿：将原先未鉴权/用 polkit 的 system 级接口改为 deepin-security-loader 的调用者白名单（AllowCaller）机制。本 PR 提供该机制的公共实现。
前提已确认：
deepin-security-loader外部包在 V25 存在（UOS Desktop 25 Professional，apt-cache search命中，版本1.0.3-1，已安装，二进制/usr/bin/deepin-security-loader+...-exec，用法deepin-security-loader --group ... -- <exec> ...，经--fd1/--fd2注入握手 fd）。改动
新增
allowcaller包（4 文件，1345 行）：registry.go：Registry多 scope 调用者白名单。NewRegistry(service *dbusutil.Service, stateFile string) *Registry— 生产工厂：包装dbusutil.Service、加载持久化状态、注册NameOwnerChanged监听自动清理 vanished 名。AddCaller(scope, sender, uniqueName)/Authorize(scope, sender)/RemoveCaller(uniqueName)/Close()。deepin-daemon、与目标同 UID、且目标进程为注册方进程的后代（进程父子链校验，防越权注册）。BusID（系统总线生命周期）隔离，重启后旧条目忽略；原子写（tmp + rename，0600）。IsProcessDescendant导出 helper；内部getProcessGroups/getProcessParentPID读/proc。handshake.go：Handshake(args, destinations) (cleanedArgs, loaded, err)— 调用方侧，消费--fd1/--fd2、向 loader 注册自身系统总线 unique name；含 fd 管道校验、响应大小限制（5 MiB）、5s 超时。registry_test.go/handshake_test.go：移植 v20 测试，用 fake bus 覆盖鉴权、持久化、跨 scope 隔离、并发注册、注册方校验、后代校验、状态总线隔离、fd 解析/校验/响应限制等。设计要点
org.deepin.dde.*DBus 名由消费方 dde-daemon 的securityloaderwrapper 定义），多 scope 设计让单进程多接口共用一个 Registry/状态文件，dde-api 单接口也兼容。go-dbus-factory/org.freedesktop.dbus（ofdbus）监听NameOwnerChanged，但 go-dbus-factory 依赖 go-lib，go-lib 不能反向依赖 go-dbus-factory。本包改用裸 D-Bus match rule（conn.BusObject().Call("org.freedesktop.DBus.AddMatch", 0, rule)）+dbusutil.SignalLoop.AddHandler，零新外部依赖（仅用已在 go.mod 的godbus/dbus/v5与同模块dbusutil/log）。验证
gofmt（已格式化）与gofmt -e解析校验。go test因 go.mod 中既有无关依赖（kingpin 的传递依赖alecthomas/template）下载超时而无法本地跑全，但本包自身代码无编译错误（go build ./allowcaller/对本包通过；测试用 fake bus，不依赖真实 D-Bus）。请协助在依赖齐全环境跑go test ./allowcaller/。后续
本 PR 为 Stage 1 基础。待合并后，dde-daemon 将新增
securityloader/wrapper（基于本包，定义 v25 scope 常量 + 握手 destination），并按架构师方案重做 PR #1197（polkit → Authorize、删 SetShortIdleState 等）；dde-api LocaleHelper 同理接入。Summary by Sourcery
Introduce a shared allowcaller package providing a persistent, multi-scope D-Bus caller allowlist and security-loader handshake utilities for reuse across services.
New Features:
Tests: