Skip to content

feat: integrate security loader into session startup - #224

Merged
fly602 merged 1 commit into
linuxdeepin:masterfrom
fly602:master
Aug 18, 2026
Merged

feat: integrate security loader into session startup#224
fly602 merged 1 commit into
linuxdeepin:masterfrom
fly602:master

Conversation

@fly602

@fly602 fly602 commented Aug 17, 2026

Copy link
Copy Markdown
Contributor
  1. Launch managed sessions through the security loader wrapper.
  2. Validate loader pipes and authorize Power1 system bus access.
  3. Update service startup, package dependencies, and helper tests.

Log: Secure Power1 access during managed session startup

Influence:

  1. Verify startup succeeds with the security loader configured.
  2. Verify fallback startup when the security loader is unavailable.
  3. Run securityloaderhelper_test and exercise power mode recovery.

feat: 集成会话启动安全加载器

  1. 通过安全加载器包装脚本启动受管理的会话进程。
  2. 校验加载器管道并授权访问 Power1 系统总线服务。
  3. 更新服务启动方式、软件包依赖及辅助函数测试。

Log: 受管理会话启动时安全访问 Power1 服务

Influence:

  1. 验证已配置安全加载器时会话可正常启动。
  2. 验证安全加载器不可用时回退启动正常。
  3. 运行 securityloaderhelper_test 并验证电源模式恢复。

PMS: TASK-393313

Summary by Sourcery

Secure managed session startup by routing it through the security loader while preserving a validated direct-start fallback.

New Features:

  • Integrate managed session startup with the security loader and authorize Power1 system-bus access before serving the session.

Bug Fixes:

  • Reject malformed loader arguments, invalid pipes, failed handshakes, and unsafe loader responses during startup.

Enhancements:

  • Support direct startup without the loader while providing bounded, validated loader communication and clearer authorization failures.

Build:

  • Install the session executable and loader wrapper in the deepin libexec directory and enable CTest integration.

Deployment:

  • Update the session manager service and package dependencies to launch sessions through the security-loader wrapper.

Tests:

  • Add security loader helper tests covering argument parsing, authorization request construction, successful handshakes, and denied responses.

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

Copy link
Copy Markdown

Reviewer's Guide

Integrates a security loader handshake into dde-session startup, routes system bus access (especially Power1) through a validated loader pipe mechanism, and adds build/test infrastructure and packaging for the loader-based startup wrapper.

Sequence diagram for security loader handshake during session startup

sequenceDiagram
    actor DisplayManager
    participant dde_session_loader_wrapper
    participant dde_session_main as dde-session(main)
    participant securityloaderhelper
    participant SecurityLoader
    participant Power1 as org.deepin.dde.Power1
    participant SystemBus as systemBusConnection

    DisplayManager->>dde_session_loader_wrapper: start session
    dde_session_loader_wrapper->>dde_session_main: exec dde-session --fd1 --fd2

    dde_session_main->>securityloaderhelper: parseSecurityLoaderFds(hasFd1, fd1, hasFd2, fd2, error)
    dde_session_main->>SystemBus: systemBusConnection()
    SystemBus-->>dde_session_main: baseService (uniqueName)

    alt loaderInfo.loaded
        dde_session_main->>securityloaderhelper: authorizePowerCaller(loaderInfo, uniqueName, error)
        securityloaderhelper->>securityloaderhelper: buildPowerAuthorizationRequest(uniqueName)
        securityloaderhelper->>SecurityLoader: writeRequest(requestFd, request)
        SecurityLoader-->>securityloaderhelper: response on responseFd
        securityloaderhelper->>securityloaderhelper: readResponse(responseFd)
        securityloaderhelper-->>dde_session_main: authorization result
        alt authorization success
            dde_session_main->>SystemBus: use systemBusConnection for Power1
            dde_session_main->>Power1: SetTlpMode / Get(TlpMode)
            dde_session_main-->>DisplayManager: session running
        else authorization failure
            dde_session_main-->>DisplayManager: exit(EXIT_FAILURE)
        end
    else loaderInfo not loaded
        dde_session_main->>SystemBus: use systemBusConnection for Power1
        dde_session_main->>Power1: SetTlpMode / Get(TlpMode)
        dde_session_main-->>DisplayManager: session running
    end
Loading

Architecture diagram for loader-based dde-session startup and Power1 access

flowchart LR
    DM["DisplayManager / Systemd user service"]
    W["dde-session-loader-wrapper (bin/dde-session)"]
    L["deepin-security-loader"]
    S["dde-session (libexec/deepin/dde-session)"]
    B["systemBusConnection (QDBus system bus)"]
    P["org.deepin.dde.Power1"]

    DM --> W
    W -->|passes --fd1/--fd2 pipes| S
    S -->|security loader request/response| L
    S -->|DBus calls| B
    B --> P
Loading

File-Level Changes

Change Details Files
Introduce security loader handshake utilities and use them during session startup to authorize access to org.deepin.dde.Power1 on the system bus.
  • Add SecurityLoaderInfo struct and helper API to parse loader FDs, build a JSON authorization request, and perform a handshake over POSIX pipes with timeout and size checks.
  • Implement non-blocking, poll-based read/write for loader pipes with robust error handling and validation of FIFO type and access mode.
  • Expose a cached system bus connection helper and use it to get the session’s unique name and to perform the handshake before creating the Session object.
src/dde-session/securityloaderhelper.h
src/dde-session/securityloaderhelper.cpp
src/dde-session/main.cpp
Wire the security loader helpers into existing D-Bus clients so Power1 and login1 use the shared system bus connection.
  • Replace direct QDBusConnection::systemBus() usage with systemBusConnection() in SessionManager’s login1 proxies.
  • Switch Power1-related QDBusInterface construction to use the shared system bus connection helper.
  • Ensure all system bus interactions relevant to power management align with the loader-authorized connection.
src/dde-session/impl/sessionmanager.cpp
src/dde-session/securityloaderhelper.h
src/dde-session/securityloaderhelper.cpp
Add a dedicated test binary for the security loader helpers and enable CTest-based testing.
  • Enable CTest in the top-level CMake and conditionally build a securityloaderhelper_test executable.
  • Implement a test program that validates FD parsing edge cases, request JSON structure, and a full handshake path using local pipes.
  • Register the test with CTest so it can be run as part of the standard test suite.
CMakeLists.txt
src/dde-session/CMakeLists.txt
src/dde-session/securityloaderhelper_test.cpp
Change how dde-session is installed and introduce a loader-based wrapper as the user-facing entry point.
  • Install the dde-session binary into libexec instead of the standard bindir.
  • Install a new dde-session-loader-wrapper script into bindir, renaming it to dde-session to act as the managed session entrypoint.
  • Adjust build artifacts so systemd and users launch the wrapper while the real session manager lives in libexec.
src/dde-session/CMakeLists.txt
misc/dde-session-loader-wrapper
Update distribution packaging to depend on the deepin-security-loader and align with the new startup path.
  • Add deepin-security-loader to Arch PKGBUILD runtime dependencies.
  • Keep Debian and systemd units aligned with the new session startup model (no explicit code-level changes shown in diff but headers added).
  • Ensure package metadata reflects that session startup now requires the security loader in normal operation.
archlinux/PKGBUILD
debian/control
systemd/dde-session-manager.service.in

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

@fly602
fly602 force-pushed the master branch 8 times, most recently from ad7b673 to 180deb8 Compare August 17, 2026 07:39
Comment thread archlinux/PKGBUILD Outdated
Comment thread src/dde-session/impl/sessionmanager.cpp Outdated
Comment thread src/dde-session/CMakeLists.txt
@fly602
fly602 force-pushed the master branch 4 times, most recently from 1651730 to aa80112 Compare August 18, 2026 01:28
@fly602

fly602 commented Aug 18, 2026

Copy link
Copy Markdown
Contributor Author

/test github-pr-review-ci

1. Launch managed sessions through the security loader wrapper.
2. Validate loader pipes and authorize Power1 system bus access.
3. Update service startup, package dependencies, and helper tests.

Log: Secure Power1 access during managed session startup

Influence:
1. Verify startup succeeds with the security loader configured.
2. Verify fallback startup when the security loader is unavailable.
3. Run securityloaderhelper_test and exercise power mode recovery.

feat: 集成会话启动安全加载器

1. 通过安全加载器包装脚本启动受管理的会话进程。
2. 校验加载器管道并授权访问 Power1 系统总线服务。
3. 更新服务启动方式、软件包依赖及辅助函数测试。

Log: 受管理会话启动时安全访问 Power1 服务

Influence:
1. 验证已配置安全加载器时会话可正常启动。
2. 验证安全加载器不可用时回退启动正常。
3. 运行 securityloaderhelper_test 并验证电源模式恢复。

PMS: TASK-393313
@deepin-ci-robot

Copy link
Copy Markdown

deepin pr auto review

★ 总体评分:95分

■ 【总体评价】

代码实现了与安全加载器的握手鉴权机制,整体逻辑严谨且错误处理完善
逻辑正确且具备良好的输入验证与资源管理,因部分文件描述符管理细节扣5分

■ 【详细分析】

  • 1.语法逻辑(基本正确)✓

authorizePowerCaller 函数在 securityloaderhelper.cpp 中正确处理了管道文件描述符的校验与读写超时。但在 parseSecurityLoaderFds 函数中,当 requestFd == responseFd 时,仅重置了 fd 为 -1,未将 info.loaded 设为 false,虽然后续 authorizePowerCaller 能正确拦截,但状态语义不够清晰。
潜在问题:info.loaded 状态与实际可用描述符不同步,可能增加后续维护理解成本
建议:在 parseSecurityLoaderFds 检测到相同描述符时,将 info.loaded 置为 false

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

代码结构清晰,函数职责单一,注释详尽。dde-session-loader-wrapper.in 脚本正确使用了 set -u 并规范了 PATH,参数传递使用了 "$@" 防止分词。错误消息处理统一通过 setErrorMessage 函数管理。
潜在问题:securityloaderhelper.cpp 中手动管理文件描述符关闭的代码重复度较高
建议:可以考虑使用 RAII 机制或封装一个 fd 管理类来自动释放资源

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

writeRequestreadResponse 使用了非阻塞 I/O 配合 poll 机制,并带有超时控制,避免了死锁和无限等待。读取响应时设置了 MaxResponseSize 限制,防止内存耗尽。
建议:保持现有的非阻塞 I/O 模式

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

漏洞对比统计:新增漏洞 0 个,减少漏洞 0 个,持平 0 个
代码对传入的文件描述符进行了严格校验,包括数值范围、是否为管道以及访问权限模式。对安全加载器的响应进行了大小限制和 JSON 格式校验,并对返回的错误消息进行了不可打印字符过滤,防止日志注入。Wrapper 脚本未使用不安全的 eval,参数传递安全。

  • 建议:继续保持严格的输入验证和输出过滤策略

■ 【改进建议代码示例】

SecurityLoaderInfo parseSecurityLoaderFds(bool hasRequestFd,
                                          const QString &requestFd,
                                          bool hasResponseFd,
                                          const QString &responseFd,
                                          QString *errorMessage)
{
    SecurityLoaderInfo info;
    info.loaded = hasRequestFd || hasResponseFd;
    if (!info.loaded)
        return info;
    if (!hasRequestFd || !hasResponseFd) {
        setErrorMessage(errorMessage, QStringLiteral("security loader requires both --fd1 and --fd2"));
        return info;
    }
    if (!parseFd(requestFd, &info.requestFd) || !parseFd(responseFd, &info.responseFd)) {
        setErrorMessage(errorMessage, QStringLiteral("security loader file descriptors are invalid"));
        info.requestFd = -1;
        info.responseFd = -1;
        info.loaded = false; // 增加状态同步
        return info;
    }
    if (info.requestFd == info.responseFd) {
        setErrorMessage(errorMessage, QStringLiteral("security loader file descriptors must be distinct"));
        closeFd(info.requestFd, nullptr);
        info.requestFd = -1;
        info.responseFd = -1;
        info.loaded = false; // 增加状态同步
    }
    return info;
}

@fly602
fly602 requested a review from yixinshark August 18, 2026 02:25
@deepin-ci-robot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: fly602, yixinshark

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 merged commit 8b39a25 into linuxdeepin:master Aug 18, 2026
16 checks passed
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.

4 participants