-
Notifications
You must be signed in to change notification settings - Fork 178
fix: [disk-encrypt] Can not decrypt disk by TPM. #2849
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
fix: [disk-encrypt] Can not decrypt disk by TPM. #2849
Conversation
Warning
|
Note
详情{
"debian/control": [
{
"line": "Homepage: http://www.deepin.org",
"line_number": 52,
"rule": "S35",
"reason": "Url link | 6fe814dfb7"
}
]
} |
Reviewer's GuideThis PR refactors TPM-based disk encryption to support dynamic PCR parameters and configurable algorithms, replacing hard-coded defaults and ini-based persistence with JSON-based tokens and DConfig overrides. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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 @GongHeng2017 - I've reviewed your changes - here's some feedback:
- The new config_utils functions mix deleteLater() and manual delete for DConfig – please standardize on a single ownership model or smart pointer to avoid inconsistent object lifetimes.
- There’s a lot of repeated tpm_utils::isSupportAlgoByTPM calls in tpmSupportInterAlgo/tpmSupportSMAlgo – consider abstracting that pattern to reduce duplication and simplify future maintenance.
- In getPassphraseFromTPM, when JSON fields are missing, it would be helpful to log exactly which keys are absent to aid debugging rather than a single generic message.
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟢 Testing: all looks good
- 🟡 Complexity: 1 issue found
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
src/plugins/filemanager/dfmplugin-disk-encrypt-entry/dfmplugin_disk_encrypt_global.h
Outdated
Show resolved
Hide resolved
src/plugins/filemanager/dfmplugin-disk-encrypt-entry/utils/encryptutils.h
Show resolved
Hide resolved
-- The libutpm2 not install, so add the depands. -- Set the pcr to 0,7. -- Optimizing the algo of obtaining for tpm. Log: fix issue Bug: https://pms.uniontech.com/bug-view-316269.html
7a42370
to
8fe2c0e
Compare
Warning
|
Note
详情{
"debian/control": [
{
"line": "Homepage: http://www.deepin.org",
"line_number": 52,
"rule": "S35",
"reason": "Url link | 6fe814dfb7"
}
]
} |
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: GongHeng2017, Johnson-zs, liujianqiang-niu The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/forcemerge |
This pr force merged! (status: behind) |
-- The libutpm2 not install, so add the depands.
-- Set the pcr to 0,7.
-- Optimizing the algo of obtaining for tpm.
Log: fix issue
Bug: https://pms.uniontech.com/bug-view-316269.html
Summary by Sourcery
Enable TPM-based disk decryption by properly managing PCR settings and algorithm parameters, with optional sourcing from DConfig to fix decryption failures.
New Features:
Bug Fixes:
Enhancements: