-
Notifications
You must be signed in to change notification settings - Fork 53
Bloodhound changes #665
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
base: develop
Are you sure you want to change the base?
Bloodhound changes #665
Conversation
66e17a2 to
cc2e11b
Compare
|
Q - could you explain on the failed tests in |
|
@ytsssun The fails are expected. I did not set the image to be L2 CIS compliant, I only wanted to validate that the overrides were being properly read. |
|
The formatting for this line is wonky in plain text output: Ideally we'd format it more like: |
cc2e11b to
0dd21ce
Compare
|
Added updated code, will rebuild and run a test image to show the new output next |
0dd21ce to
c34ff53
Compare
|
Updated test output has been added |
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.
This is very close, but it seems like it'd be nice if the implementation respected the status value provided in the override file when rendering the report.
| { | ||
| "reason": "Test requires iptables FORWARD chain configuration that conflicts with Kubernetes networking architecture. Kubernetes manages its own iptables rules through kube-proxy and CNI plugins.", | ||
| "reference": "https://github.com/bottlerocket-os/bottlerocket/issues/540", | ||
| "status": "SKIP" | ||
| } |
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.
Do we also need an override for the IPv6 check, 3.4.2.1?
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.
Per #540 3.4.2.1 actually does pass if properly configured, as we only seem to override the IPv4 rules.
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.
Per #540 3.4.2.1 actually does pass if properly configured, as we only seem to override the IPv4 rules.
But does the node actually work properly if using IPv6 for the overlay network, with forwarding disabled? I'm not sure why that would be true for IPv6 but not true for IPv4.
c34ff53 to
0ade908
Compare
Add override system to skip specific compliance tests through JSON configuration files. Enables variants to skip incompatible tests while maintaining compliance checking. Signed-off-by: Richard Kelly <[email protected]>
Replace ManualChecker with actual K8S04021000Checker implementation and add override to pass on Bottlerocket. Signed-off-by: Richard Kelly <[email protected]>
Remove TLS_RSA_WITH_AES_128_GCM_SHA256 and TLS_RSA_WITH_AES_256_GCM_SHA384 from allowed cipher suites per latest Kubernetes CIS benchmark. Signed-off-by: Richard Kelly <[email protected]>
0ade908 to
cba0a94
Compare
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.
How are exceptions shown in the JSON output?
| %package -n %{_cross_os}bloodhound-k8s-overrides | ||
| Summary: CIS Test overrides for Kubernetes variants | ||
| Requires: (%{_cross_os}bloodhound and %{_cross_os}variant-runtime(k8s)) | ||
| %description -n %{_cross_os}bloodhound-k8s-overrides | ||
| %{summary}. |
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.
nit: can just add these into the existing bloodhound-k8s subpackage which has the same requirements
The added test overrides do not directly flatten into the previous json output structure. In order to maintain the same json structure we require a custom serializer Signed-off-by: Richard Kelly <[email protected]>
Issue number:
Closes #540
Description of changes:
Adds the ability for bloodhound test overrides to exist, with reasoning, and optional messaging in the case of skipped tests. Additionally, overrides 2 tests:
Finally, we've removed the RSA ciphers no longer considered secure by the latest kubernetes CIS benchmark from k8s 04021200
Testing done:
Run tests:
Check to ensure overrides are on kubernetes variants, but not ECS variants:
json output sample:
Terms of contribution:
By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.