Skip to content

Conversation

@fly602
Copy link
Contributor

@fly602 fly602 commented Jan 28, 2026

Added support for configurable password encryption algorithms through dconfig settings. The system now supports multiple encryption algorithms including SM3, yescrypt, SHA512, and SHA256. The default algorithm is set to SM3 but can be changed via configuration.

Key changes include:

  1. Added dconfig key for password encryption algorithm selection
  2. Implemented algorithm lookup and fallback mechanism in C code
  3. Created new function for password encryption with algorithm parameter
  4. Added configuration change monitoring to update algorithm dynamically
  5. Maintained backward compatibility with existing password encoding

Log: Added configurable password encryption algorithm support

Influence:

  1. Test password creation with different algorithm configurations
  2. Verify algorithm fallback when configured algorithm fails
  3. Test configuration change detection and algorithm switching
  4. Verify backward compatibility with existing passwords
  5. Test all supported algorithms: SM3, yescrypt, SHA512, SHA256
  6. Check system behavior when invalid algorithm is configured

feat: 支持可配置的密码加密算法

通过 dconfig 设置添加了对可配置密码加密算法的支持。系统现在支持多种加密
算法,包括 SM3、yescrypt、SHA512 和 SHA256。默认算法设置为 SM3,但可以通 过配置进行更改。

主要变更包括:

  1. 添加了用于密码加密算法选择的 dconfig 键
  2. 在 C 代码中实现了算法查找和回退机制
  3. 创建了带算法参数的新密码加密函数
  4. 添加了配置变更监控以动态更新算法
  5. 保持与现有密码编码的向后兼容性

Log: 新增可配置密码加密算法支持
PMS: BUG-346165
Influence:

  1. 测试使用不同算法配置的密码创建功能
  2. 验证配置算法失败时的回退机制
  3. 测试配置变更检测和算法切换功能
  4. 验证与现有密码的向后兼容性
  5. 测试所有支持的算法:SM3、yescrypt、SHA512、SHA256
  6. 检查配置无效算法时的系统行为

Summary by Sourcery

Add configurable password encryption algorithms and wire them to account configuration with dynamic updates and safe fallback behavior.

New Features:

  • Support selecting the password encryption algorithm (SM3, yescrypt, SHA512, SHA256) via dconfig for account passwords.
  • Expose a Go-level password encryption API that accepts an explicit algorithm while preserving a default choice from configuration.

Enhancements:

  • Refactor password hashing logic in C to use a table of supported algorithms with a generic helper and fallback across algorithms when encryption fails.
  • Initialize and track the default password encryption algorithm in the account manager, updating it in response to configuration change signals.

Added support for configurable password encryption algorithms through
dconfig settings. The system now supports multiple encryption algorithms
including SM3, yescrypt, SHA512, and SHA256. The default algorithm is
set to SM3 but can be changed via configuration.

Key changes include:
1. Added dconfig key for password encryption algorithm selection
2. Implemented algorithm lookup and fallback mechanism in C code
3. Created new function for password encryption with algorithm parameter
4. Added configuration change monitoring to update algorithm dynamically
5. Maintained backward compatibility with existing password encoding

Log: Added configurable password encryption algorithm support

Influence:
1. Test password creation with different algorithm configurations
2. Verify algorithm fallback when configured algorithm fails
3. Test configuration change detection and algorithm switching
4. Verify backward compatibility with existing passwords
5. Test all supported algorithms: SM3, yescrypt, SHA512, SHA256
6. Check system behavior when invalid algorithm is configured

feat: 支持可配置的密码加密算法

通过 dconfig 设置添加了对可配置密码加密算法的支持。系统现在支持多种加密
算法,包括 SM3、yescrypt、SHA512 和 SHA256。默认算法设置为 SM3,但可以通
过配置进行更改。

主要变更包括:
1. 添加了用于密码加密算法选择的 dconfig 键
2. 在 C 代码中实现了算法查找和回退机制
3. 创建了带算法参数的新密码加密函数
4. 添加了配置变更监控以动态更新算法
5. 保持与现有密码编码的向后兼容性

Log: 新增可配置密码加密算法支持
PMS: BUG-346165
Influence:
1. 测试使用不同算法配置的密码创建功能
2. 验证配置算法失败时的回退机制
3. 测试配置变更检测和算法切换功能
4. 验证与现有密码的向后兼容性
5. 测试所有支持的算法:SM3、yescrypt、SHA512、SHA256
6. 检查配置无效算法时的系统行为
@sourcery-ai
Copy link

sourcery-ai bot commented Jan 28, 2026

Reviewer's Guide

Implements configurable password encryption algorithms by wiring a dconfig-driven default algorithm from Go into a new C password hashing API that supports multiple crypt prefixes and fallback behavior, while keeping a simple EncodePasswd entrypoint and backward-compatible defaults.

Sequence diagram for configurable password encryption flow

sequenceDiagram
    actor Caller
    participant Manager
    participant DConfig as DConfig_dsAccount
    participant Users as users_package
    participant CPasswd as C_passwd_lib

    %% Initialization: load default algorithm from dconfig
    Caller->>Manager: initAccountDSettings()
    Manager->>DConfig: NewApp(dsettingsAppID)
    Manager->>DConfig: Value(0, keyPasswordEncryptionAlgorithm)
    DConfig-->>Manager: algoVar (string)
    Manager->>Manager: getDConfigPasswdEncryptionAlgorithm()
    Manager-->>Manager: alg string or error
    Manager->>Users: set PasswdAlgoDefault = alg (or keep default "sm3")

    %% Runtime: dconfig change monitoring
    DConfig-)Manager: ValueChanged(keyPasswordEncryptionAlgorithm)
    Manager->>Manager: getDConfigPasswdEncryptionAlgorithm()
    Manager-->>Manager: new alg string or error
    Manager->>Users: update PasswdAlgoDefault

    %% Password encoding using current algorithm
    Caller->>Users: EncodePasswd(words)
    Users->>Users: CryptUserPassword(words, PasswdAlgoDefault)
    Users->>CPasswd: mkpasswd_with_algo(words, algo)
    CPasswd-->>Users: hashed_password or NULL
    Users-->>Caller: hashed_password (or empty string on failure)
Loading

Updated class diagram for password encryption configuration and APIs

classDiagram
    class Manager {
        +ConfigManager cfgManager
        +dsettings dsAccount
        +bool IsTerminalLocked
        +initAccountDSettings()
        +getDConfigPasswdEncryptionAlgorithm() string
    }

    class users_package {
        <<module>>
        +string PasswdAlgoDefault
        +EncodePasswd(words string) string
        +CryptUserPassword(words string, algo string) string
    }

    class PasswordAlgorithm {
        +const char* name
        +const char* prefix
    }

    class C_passwd_lib {
        <<C_module>>
        +PasswordAlgorithm SUPPORTED_ALGORITHMS[]
        +int DEFAULT_ALGORITHM
        +char* try_encrypt_password(const char* words, const char* prefix)
        +char* mkpasswd_with_algo(const char* words, const char* algo)
    }

    class PasswordAlgoConstants {
        <<C_header>>
        +PASSWD_ALGO_SHA512 : char*
        +PASSWD_ALGO_SHA256 : char*
        +PASSWD_ALGO_YESCRYPT : char*
        +PASSWD_ALGO_SM3 : char*
    }

    Manager --> users_package : configures PasswdAlgoDefault
    users_package --> C_passwd_lib : calls mkpasswd_with_algo
    C_passwd_lib o-- PasswordAlgorithm : uses
    PasswordAlgoConstants ..> C_passwd_lib : provides identifiers
Loading

File-Level Changes

Change Details Files
Introduce multi-algorithm password hashing in C with prefix-based selection and fallback behavior.
  • Replace the old mkpasswd implementation with a new mkpasswd_with_algo function that takes an algorithm name and uses a static table of supported algorithms mapped to crypt() prefixes (SM3, yescrypt, SHA512, SHA256).
  • Add a helper try_encrypt_password that generates a salt via crypt_gensalt_rn, validates it, and calls crypt() to hash the password, returning NULL on failure to signal fallback.
  • Implement a selection flow that chooses the requested algorithm if known, otherwise falls back to a default (SM3), then iterates through remaining supported algorithms as fallbacks if hashing with the primary algorithm fails.
  • Define a DEFAULT_ALGORITHM index and a CRYPT_GENSALT_OUTPUT_SIZE guard to ensure sufficient buffer space for salt generation.
accounts1/users/passwd.c
Expose algorithm-aware password hashing to Go and preserve a simple default-based encoding API.
  • Add a global PasswdAlgoDefault variable initialized to "sm3" for the default algorithm within the Go users package.
  • Refactor EncodePasswd to delegate to a new CryptUserPassword(words, algo) function, keeping the original call sites simple and algorithm-agnostic.
  • Implement CryptUserPassword to marshal Go strings to C, call mkpasswd_with_algo, handle NULL results safely, and convert the C string back to Go.
  • Update the C header to declare mkpasswd_with_algo and define string constants for supported algorithms (SM3, yescrypt, SHA512, SHA256) for potential reuse and clarity.
accounts1/users/passwd.go
accounts1/users/passwd.h
Integrate dconfig-driven algorithm configuration with runtime updates into the manager.
  • Introduce a new dconfig key name constant keyPasswordEncryptionAlgorithm in the Manager config constants.
  • Add getDConfigPasswdEncryptionAlgorithm on Manager to read the algorithm string from the dconfig backend, with type-checking and error reporting.
  • Initialize users.PasswdAlgoDefault from dconfig during initAccountDSettings, before using password encoding, falling back silently on errors.
  • Extend dconfig signal handling to listen for value changes on the passwordEncryptionAlgorithm key and update users.PasswdAlgoDefault at runtime while logging the change for diagnostics.
accounts1/manager.go
Register the new password encryption algorithm setting in the dconfig schema.
  • Update the org.deepin.dde.daemon.account dconfig JSON schema to add a passwordEncryptionAlgorithm key so it can be managed via system configuration tooling.
misc/dsg-configs/org.deepin.dde.daemon.account.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

deepin pr auto review

这份代码主要实现了密码加密算法的可配置化功能,允许通过 dconfig 动态配置密码加密算法(支持 sm3, yescrypt, sha512, sha256)。以下是对代码的详细审查和改进建议:

1. 语法与逻辑

Go 部分 (manager.go)

  • 改进点 1:错误处理
    initAccountDSettings 函数中:

    users.PasswdAlgoDefault, _ = m.getDConfigPasswdEncryptionAlgorithm()

    使用 _ 忽略了错误。如果读取配置失败,users.PasswdAlgoDefault 将保持为空字符串(或之前的值,取决于初始化)。虽然 C 代码中有默认算法回退机制,但在 Go 层面明确错误或设置一个显式的默认值会更安全。
    建议

    algo, err := m.getDConfigPasswdEncryptionAlgorithm()
    if err != nil {
        logger.Warning("failed to get password encryption algorithm from dconfig, err:", err)
        // 可以在这里设置一个默认值,或者依赖 C 层的默认值
        // users.PasswdAlgoDefault = "sm3"
    } else {
        users.PasswdAlgoDefault = algo
    }
  • 改进点 2:并发安全
    users.PasswdAlgoDefault 是一个包级别的全局变量。在 ConnectValueChanged 的回调中被修改,而在 EncodePasswd 中被读取。这存在并发读写(Data Race)的风险。
    建议:使用 sync/atomic 或者互斥锁来保护这个变量,或者确保调用 EncodePasswd 的逻辑是单线程的(但这通常很难保证)。

    import "sync/atomic"
    // ...
    var passwdAlgoDefault atomic.Value // 存储类型应为 string
    
    // 读取时
    algo := passwdAlgoDefault.Load().(string)
    if algo == "" {
        algo = "sm3"
    }
    // 写入时
    passwdAlgoDefault.Store(newAlgo)

C 部分 (passwd.c)

  • 改进点 3:crypt 函数的线程安全性
    crypt_r 是线程安全的,但 crypt 通常不是。代码中调用了 crypt(words, setting)。如果在多线程环境下(Go 调用 C 代码通常是多线程的),这可能导致数据竞争或崩溃。
    建议:检查 crypt 的实现,如果环境支持,优先使用 crypt_r。或者确保 Go 调用端加锁(Go 代码中已有 wLocker sync.Mutex,需确认是否覆盖了此调用路径)。

  • 改进点 4:内存管理
    try_encrypt_password 函数中:

    password = crypt(words, setting);
    // ...
    return password;

    crypt 返回的指针通常指向静态内存区,或者在某些实现中需要由调用者释放(取决于具体 libc 实现)。直接返回这个指针并在 Go 中通过 C.GoString 读取通常是可行的,但如果 crypt 的实现依赖内部静态缓冲区,后续调用会覆盖之前的结果。
    建议:确认当前环境 crypt 的行为。如果可能,尽量在 C 层将结果复制到堆内存中返回,或者确保 Go 层立即读取并转换为 Go 字符串,不持有 C 指针。

2. 代码质量

  • 改进点 5:日志级别
    manager.go 中:

    logger.Warning("password encrypt algo changed:", users.PasswdAlgoDefault)

    管理员修改加密算法是一个重要的安全事件,但在配置变更时使用 Warning 级别可能不太合适(这并非系统异常)。如果是调试目的,建议使用 Debug;如果是审计目的,建议使用 Info 或专门的审计日志。

  • 改进点 6:变量命名
    passwd.c 中的 algoVar (Go 中) 和 alg (Go 中) 命名略不一致。C 代码中的 words 作为密码的参数名,不如 password 直观。

3. 代码性能

  • 改进点 7:C 字符串转换开销
    CryptUserPassword 函数每次调用都会进行两次 C.CString(密码和算法名)和一次 C.GoString。这涉及内存分配和拷贝。
    建议:由于密码加密是 CPU 密集型操作,内存分配的开销相对较小,但如果是高频调用场景,可以优化。不过考虑到密码修改/验证不是高频操作,当前实现尚可接受。

4. 代码安全

  • 改进点 8:密码算法回退逻辑
    mkpasswd_with_algo 中,如果配置的算法(例如 "sm3")执行失败(try_encrypt_password 返回 NULL),代码会遍历 SUPPORTED_ALGORITHMS 尝试其他算法:

    for (i = 0; SUPPORTED_ALGORITHMS[i].name != NULL; i++) {
        // ...
        password = try_encrypt_password(words, SUPPORTED_ALGORITHMS[i].prefix);
        if (password != NULL) {
            return password; // 返回了非预期的算法加密结果
        }
    }

    严重安全问题:如果管理员强制要求使用高强度的 yescrypt,但由于系统库不支持导致失败,代码回退到了强度较低的 sha512sha256。这违背了安全策略的意图。
    建议:如果指定的算法失败,应该直接返回错误(NULL),而不是静默降级。Go 层捕获到空字符串后应报错,而不是使用弱密码哈希。

  • 改进点 9:配置文件权限
    org.deepin.dde.daemon.account.json 中配置项 passwordEncryptionAlgorithm 设置为 "permissions": "readonly"。这是好的做法,防止普通用户篡改算法。

  • 改进点 10:随机数生成
    虽然新代码使用了 crypt_gensalt_rn 来生成盐值,这比旧代码手动生成随机数要好得多。但需确保链接的 libc 版本足够新,支持 crypt_gensalt_rn 及相关算法(如 yescrypt)。

总结建议

  1. 修复安全回退逻辑:C 代码中严禁在指定算法失败时自动降级到其他算法,应直接报错。
  2. Go 层并发控制:对 PasswdAlgoDefault 的读写增加并发保护。
  3. 错误处理:在 Go 初始化时不要忽略读取配置的错误。
  4. 日志审计:调整日志级别,确保关键配置变更可被追踪。
  5. 线程安全:确认 crypt 函数在多线程下的安全性,必要时使用 crypt_r 或 Go 层锁。

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've found 1 issue, and left some high level feedback:

  • The global users.PasswdAlgoDefault is updated from the DConfig signal callback and read from EncodePasswd/CryptUserPassword without synchronization, which can cause data races; consider guarding it with a mutex or using atomic.Value.
  • In initAccountDSettings, assigning users.PasswdAlgoDefault, _ = m.getDConfigPasswdEncryptionAlgorithm() will overwrite the compiled-in default with an empty string on error; it might be safer to only assign when err == nil && alg != "" so the default is preserved.
  • The algorithm name comparison in mkpasswd_with_algo is case-sensitive and expects specific lowercase strings (e.g. "sm3", "yescrypt"); consider normalizing or validating the configured value so small differences in configuration (e.g. "SM3") don't silently fall back to the default.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The global `users.PasswdAlgoDefault` is updated from the DConfig signal callback and read from `EncodePasswd`/`CryptUserPassword` without synchronization, which can cause data races; consider guarding it with a mutex or using `atomic.Value`.
- In `initAccountDSettings`, assigning `users.PasswdAlgoDefault, _ = m.getDConfigPasswdEncryptionAlgorithm()` will overwrite the compiled-in default with an empty string on error; it might be safer to only assign when `err == nil && alg != ""` so the default is preserved.
- The algorithm name comparison in `mkpasswd_with_algo` is case-sensitive and expects specific lowercase strings (e.g. "sm3", "yescrypt"); consider normalizing or validating the configured value so small differences in configuration (e.g. "SM3") don't silently fall back to the default.

## Individual Comments

### Comment 1
<location> `accounts1/users/passwd.go:35-38` </location>
<code_context>
 	wLocker sync.Mutex
 )

+var PasswdAlgoDefault = "sm3"
+
 func EncodePasswd(words string) string {
+	return CryptUserPassword(words, PasswdAlgoDefault)
+}
+
</code_context>

<issue_to_address>
**issue (bug_risk):** Global PasswdAlgoDefault can be written and read concurrently, causing a data race

This variable is updated from the DConfig signal callback in manager.go and read from EncodePasswd without any synchronization. Please protect it with a mutex, use atomic.Value, or funnel updates through a single owning goroutine so all reads are race-free and see a consistent value.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment on lines +35 to +38
var PasswdAlgoDefault = "sm3"

func EncodePasswd(words string) string {
return CryptUserPassword(words, PasswdAlgoDefault)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (bug_risk): Global PasswdAlgoDefault can be written and read concurrently, causing a data race

This variable is updated from the DConfig signal callback in manager.go and read from EncodePasswd without any synchronization. Please protect it with a mutex, use atomic.Value, or funnel updates through a single owning goroutine so all reads are race-free and see a consistent value.

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

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

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 1faa497 into linuxdeepin:master Jan 28, 2026
17 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.

3 participants