controls/cis_debian13: Complete and correct section 1 controls - #14806
controls/cis_debian13: Complete and correct section 1 controls#14806israel-villar wants to merge 1 commit into
Conversation
|
Hi @israel-villar. Thanks for your PR. I'm waiting for a ComplianceAsCode member to verify that this patch is reasonable to test. If it is, they should reply with Tip We noticed you've done this a few times! Consider joining the org to skip this step and gain Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
rrskris
left a comment
There was a problem hiding this comment.
Reviewed and verified this locally rather than just reading the diff:
- All 57 rule references resolve — every rule id added in this PR exists in
linux_os/guide/on current master (checked mechanically against the tree). - Control file consistency: 83 controls after the patch, no duplicate ids, every
status: automatedcontrol hasrules:, all level names valid. ./build_product debian13succeeds on this branch (fedora:41 container, clean
tree) — and sincecis_level1_server.profileselectscis_debian13:all:l1_server,
the L1 rule references were resolved by a real build, not just eyeballed.- Nice catches worth calling out:
related_rules:→rules:on 1.1.1.8 (the former
is not selected into the profile, so squashfs was silently unchecked), and the
disable_prelink→package_prelink_removedswap.
One coordination question rather than a review point: we're looking to help fill the
remaining cis_debian13.yml sections and noticed feat/debian13-cis-sections234 on
your fork — to avoid duplicating your work, would it help if we took section 5.1
(SSH server configuration) and/or section 7 (system maintenance)? Happy to
follow the structure you've established here. (Context: we build Debian hardening
pipelines on top of ComplianceAsCode at
StratumOSS, so Debian 13 coverage is
something we can test against real builds.)
Hi @rrskris. Thank you for such a thorough review. On the structure: sections 2–4 are in a separate branch/PR precisely because section 1 was already merged into master before I started — keeping the new work isolated made the diff easier to review. On coordination:
One other area I think would be valuable to work on together: Automatus testing for Debian 13. The existing automatus-debian12.yaml workflow and Dockerfiles/test_suite-debian12 would serve as a very close starting point — the changes should be minimal (product name, image base). Since you're already running builds against real Debian 13 systems, you'd be well positioned to help validate that. Happy to open a draft PR with the initial files if that sounds useful. |
|
Thanks @israel-villar, that all makes sense — and glad section 1 is already in. Yes to Automatus for Debian 13 — that's a great fit for us since we're already running real Debian 13 builds, so we can validate the rule tests against an actual target rather than just wiring the workflow. Happy to own it: I'll put together And yes — please do open the sections 6/7 PR when you're ready; I'm glad to review there too. |
|
@rrskris I guess I can merge this for you after you agree, cause I think you don't have merge rights in this repo. Correct? Do I understand well that the next step is to rebase this PR upon the part where Debian Automatus is used in the CI? |
|
Thanks a lot @vojtapolasek. Yeah, I don't have merge rights in this repo. My piece of work was the Automatus Debian 13 workflow in #14885, which jan-cerny already reviewed and merged, so there's nothing of mine to merge here. Now that the Debian 13 Automatus job is on master (#14885), rebasing this PR onto current master is exactly the right next step. CI will actually run these section-1 rules against a real debian:13 target (bash + ansible remediation), which is the whole reason for wiring it up. @israel-villar, since it's your PR you'd drive the rebase, I'd be happy to help validate on my side if useful (I can run the affected rules locally against a debian:13 container, which is how I checked the workflow itself). |
Verified all section 1 controls against CIS Debian Linux 13 Benchmark v1.0.0 PDF and applied the following changes on top of upstream: Corrections to existing controls: - 1.1.1.6: fix title (overlayfs -> overlay) - 1.1.1.7: fix related_rules -> rules - 1.1.2.1.1: fix title (add 'tmpfs or' to partition description) - 1.2.1.2: convert pending to automated (apt_disable_weak_dependencies) - 1.2.1.3-1.2.1.9: convert pending to automated with apt file/dir rules - 1.2.1.4: fix title (add missing '(Automated)' suffix) - 1.3.1.1: fix title (AppArmor packages) - 1.3.1.2: fix title (AppArmor is enabled) - 1.3.1.4: convert pending to automated (apparmor sysctl rule) - 1.5.6: replace disable_prelink with package_prelink_removed - 1.5.7: convert pending to automated (service_apport_disabled) New controls (not yet in upstream): - 1.6.2-1.6.6: banner content and access controls - 1.7.1-1.7.11: GDM/display manager controls Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
febc45a to
d958dc9
Compare
|
Thanks @rrskris and @vojtapolasek — I've rebased this branch onto current master. The rebase produced several conflicts in section 1, which I resolved control-by-control:
Thanks @rrskris,. Now that the Debian 13 Automatus workflow (#14885) is on master, this should get real CI coverage against a debian:13 target. |
|
Re-verified the rebased branch locally (same method as my earlier review):
From my side this is good to go. @vojtapolasek |
|
@rrskris @vojtapolasek Thanks again for the thorough re-verification! Since this has been confirmed "good to go", is there anything else you need from me to help move this toward merge — e.g. should I rebase again against current master, or is it just waiting on maintainer bandwidth at this point? Happy to help however's useful. |
|
@israel-villar Thanks for the ping, and sorry to have left you waiting on this one. I need to walk back part of what I said earlier. My Jul 18 re-verification was against a master that has since moved a long way, and it doesn't carry forward the way I implied. To answer your actual question directly: yes, a rebase is needed — the branch is currently 98 commits behind master ( None of this is on you. What master still genuinely lacks, where this PR is still right:
Two more are worth separating out, not because they're wrong but because master has since taken a different position and a maintainer should get to rule on them:
Minor: 1.3.1.1 and 1.3.1.2 read correctly on master now, so those two title fixes can come out. If it's useful, the path I'd suggest is a rebase onto current master with the scope cut to the six items above, and 1.2.1.2 and 1.7.2 called out explicitly in the description. That turns a 50-day-old diff into a small, easy-to-approve one, and it keeps this thread and its history rather than starting over. @jan-cerny — when you have a moment, could you drop an Once it's rebased I'm glad to re-verify, same method as before. With the Automatus job now on master these rules will actually be exercised against a real |
|
I think ok-to-test is not needed. That enables testing on Openshift and this PR is about Debian. Or am I missing something? |
Verified all section 1 controls against CIS Debian Linux 13 Benchmark v1.0.0 PDF and applied the following changes on top of upstream:
Corrections to existing controls:
New controls (not yet in upstream):
Description:
Rationale:
Review Hints: