-
Notifications
You must be signed in to change notification settings - Fork 40
chore: harden service security #588
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
- Fix CMake variable name from CVERSION to VERSION in debian/rules - Enhance deepin-devicecontrol service with comprehensive sandboxing: - Apply strict resource limits (2G memory, IO weight 200) - Enable filesystem protection (ProtectSystem, ProtectHome, PrivateTmp) - Restrict executable paths and set write permissions selectively - Add security restrictions (NoNewPrivileges, MemoryDenyWriteExecute) - Define specific accessible and inaccessible system paths - Set capability bounding set and ambient capabilities - Adjust scheduling priority and OOM score Log: harden service security.
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThis PR hardens the deepin-devicecontrol systemd service using stricter sandboxing and resource constraints, and fixes a CMake-related variable name in debian packaging, along with a minor .gitignore update. 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 - I've left some high level feedback:
- For the new systemd hardening options (memory limit, IO weight, OOMScoreAdjust, etc.), consider adding brief inline comments in the unit file explaining the rationale for the chosen values so future maintainers know why these specific thresholds were selected.
- Double-check whether any binaries or scripts the service might legitimately spawn outside the explicitly allowed executable paths are now blocked; if so, it may be safer to group such paths (e.g., a dedicated helper directory) rather than enumerating individual binaries to ease future changes.
- Given the aggressive filesystem and namespace restrictions (ProtectSystem, ProtectHome, PrivateTmp, InaccessiblePaths, etc.), validate that all required runtime write locations are explicitly covered by ReadWritePaths or similar directives to avoid subtle failures when the service writes logs, state, or sockets.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- For the new systemd hardening options (memory limit, IO weight, OOMScoreAdjust, etc.), consider adding brief inline comments in the unit file explaining the rationale for the chosen values so future maintainers know why these specific thresholds were selected.
- Double-check whether any binaries or scripts the service might legitimately spawn outside the explicitly allowed executable paths are now blocked; if so, it may be safer to group such paths (e.g., a dedicated helper directory) rather than enumerating individual binaries to ease future changes.
- Given the aggressive filesystem and namespace restrictions (ProtectSystem, ProtectHome, PrivateTmp, InaccessiblePaths, etc.), validate that all required runtime write locations are explicitly covered by ReadWritePaths or similar directives to avoid subtle failures when the service writes logs, state, or sockets.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
deepin pr auto review我来帮你分析这个diff的修改内容,主要涉及三个方面:
安全改进:
性能优化:
权限管理:
建议改进:
总体来说,这些修改提高了系统的安全性和稳定性,是一个很好的安全加固方案。建议在部署前进行充分测试,确保所有功能正常工作。 |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: lzwind, re2zero 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 |
|
/merge |
|
This pr cannot be merged! (status: unstable) |
|
/forcemerge |
|
This pr force merged! (status: unstable) |
Log: harden service security.
Summary by Sourcery
Harden the deepin-devicecontrol service security and correct a Debian packaging variable name.
Enhancements:
Build:
Chores: