-
Notifications
You must be signed in to change notification settings - Fork 109
feat: support configurable password encryption algorithms #1005
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
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. 检查配置无效算法时的系统行为
Reviewer's GuideImplements 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 flowsequenceDiagram
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)
Updated class diagram for password encryption configuration and APIsclassDiagram
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
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
deepin pr auto review这份代码主要实现了密码加密算法的可配置化功能,允许通过 dconfig 动态配置密码加密算法(支持 sm3, yescrypt, sha512, sha256)。以下是对代码的详细审查和改进建议: 1. 语法与逻辑Go 部分 (
|
There was a problem hiding this 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.PasswdAlgoDefaultis updated from the DConfig signal callback and read fromEncodePasswd/CryptUserPasswordwithout synchronization, which can cause data races; consider guarding it with a mutex or usingatomic.Value. - In
initAccountDSettings, assigningusers.PasswdAlgoDefault, _ = m.getDConfigPasswdEncryptionAlgorithm()will overwrite the compiled-in default with an empty string on error; it might be safer to only assign whenerr == nil && alg != ""so the default is preserved. - The algorithm name comparison in
mkpasswd_with_algois 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>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| var PasswdAlgoDefault = "sm3" | ||
|
|
||
| func EncodePasswd(words string) string { | ||
| return CryptUserPassword(words, PasswdAlgoDefault) |
There was a problem hiding this comment.
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.
|
[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. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
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:
Log: Added configurable password encryption algorithm support
Influence:
feat: 支持可配置的密码加密算法
通过 dconfig 设置添加了对可配置密码加密算法的支持。系统现在支持多种加密
算法,包括 SM3、yescrypt、SHA512 和 SHA256。默认算法设置为 SM3,但可以通 过配置进行更改。
主要变更包括:
Log: 新增可配置密码加密算法支持
PMS: BUG-346165
Influence:
Summary by Sourcery
Add configurable password encryption algorithms and wire them to account configuration with dynamic updates and safe fallback behavior.
New Features:
Enhancements: