[New Rules] Api server rules #12
Conversation
General repo struct refactor
| # Ensure that the --audit-log-maxsize argument is set to 100 or as appropriate (Automated) | ||
| finding = result { | ||
| command_args := data_adapter.command_args | ||
| rule_evaluation = common.arg_at_least(command_args, "--audit-log-maxsize", 100) |
There was a problem hiding this comment.
maybe it is better to have this split into 3 utils?
get arg value
to number
and compare the number?
There was a problem hiding this comment.
lets think more about use cases we would commonly use in the future.. what makes sense more..
so i was thinking maybe that would be a better approach for us, by having smaller responsibility for common func
| # Ensure that the --insecure-port argument is set to 0 (Automated) | ||
| finding = result { | ||
| command_args := data_adapter.command_args | ||
| rule_evaluation := common.array_contains(command_args, "--insecure-port=0") |
There was a problem hiding this comment.
what would the the default here for example?
There was a problem hiding this comment.
I have an issue that it is too hard to understand from the code. I know I have a test to check. but it is not the ideal situation
| # Ensure that the --kubelet-https argument is set to true (Automated) | ||
| finding = result { | ||
| command_args := data_adapter.command_args | ||
| rule_evaluation = common.array_contains(command_args, "--kubelet-https=false") == false # if not set to false explictly -> rule pass |
There was a problem hiding this comment.
I think it would be better that our rules would check for the ok mode. like whitelisting all the ok modes.
instead of checking for the not ok mode.
it would make reading rules and the way of writing them easier
wdyt?
There was a problem hiding this comment.
I just picked how raspbernetes did that. And I really like their approach
you can easily understand from the code if you are looking for a requiredValue or deniedValue
so it is another alternative
There was a problem hiding this comment.
lets talk about this
|
@oren-zohar I need you to go over default values more carefully I would suggest first to check the tests are well defined before doing any fixes. The issue starts when a test doesn't validate the needed requirements |
# Conflicts: # compliance/cis_k8s/rules/cis_1_2_2/rule.rego # compliance/cis_k8s/rules/cis_1_2_2/test.rego # compliance/lib/common.rego # compliance/lib/data_adapter.rego
|
|
||
| rule_evaluation { | ||
| # Verify that the --disable-admission-plugins argument is set to a value that does not include NamespaceLifecycle. | ||
| common.array_contains(command_args, "--disable-admission-plugins=") | ||
| command_args["--disable-admission-plugins"] |
There was a problem hiding this comment.
out of curiosity, how does it checks that it is about api-server, and not something else?
How rule_evaluation is not always false?
There was a problem hiding this comment.
(add todo if you're going to impl. in the polishing)
There was a problem hiding this comment.
it can only access command_args if it the relevant command, since:
csp-security-policies/compliance/lib/data_adapter.rego
Lines 53 to 56 in a41fa43
| } | ||
|
|
||
| test_pass { | ||
| test.assert_pass(finding) with input as rule_input("api_server", "--enable-admission-plugins=LimitRanger,PodSecurityPolicy") |
compliance/lib/common.rego
Outdated
| } else = false { | ||
| true | ||
| } | ||
|
|
||
| # checks if a argument is set to greater value then minimum | ||
| arg_at_least(arguments, key, minimum) { | ||
| value := get_arg_value(arguments, key) | ||
| greater_or_equal(arguments, key, minimum) { |
There was a problem hiding this comment.
That is better than before but we lost the context. The we check argument value in a map.
Honestly this is a too complex method
And if the value is not the arguments you should check at all.. right? instead of returning false..
How about we would have math.gte ? that checks what you need?
gte is a known acronym for greater than equal to - https://www.google.co.il/search?q=greater+than+equal+gte
| # Ensure that the --insecure-bind-address argument is not set (Automated) | ||
| finding = result { | ||
| command_args := data_adapter.api_server_command_args | ||
| rule_evaluation := common.contains_key(command_args, "--insecure-bind-address") == false |
There was a problem hiding this comment.
| rule_evaluation := common.contains_key(command_args, "--insecure-bind-address") == false | |
| rule_evaluation := assert.false(common.contains_key(command_args, "--insecure-bind-address")) |
| default rule_evaluation = false | ||
| rule_evaluation { | ||
| value := command_args["--request-timeout"] | ||
| common.duration_gte(value, 61) |
| # Ensure that the --etcd-certfile and --etcd-keyfile arguments are set as appropriate (Automated) | ||
| command_args := data_adapter.api_server_command_args | ||
|
|
||
| rule_evaluation { |
There was a problem hiding this comment.
use default instead of else = false
| # Ensure that the --tls-cert-file and --tls-private-key-file arguments are set as appropriate (Automated) | ||
| command_args := data_adapter.api_server_command_args | ||
|
|
||
| rule_evaluation { |
There was a problem hiding this comment.
use default instead of else = false
| command_args := data_adapter.api_server_command_args | ||
|
|
||
| rule_evaluation { | ||
| command_args["--kubelet-client-certificate"] |
There was a problem hiding this comment.
use default instead of else = false
| import data.lib.test | ||
|
|
||
| test_violation { | ||
| test.assert_fail(finding) with input as rule_input("api_server", "--authorization-mode=AlwaysAllow") |
There was a problem hiding this comment.
add another test of "--authorization-mode=RBAC,AlwaysAllow"
compliance/lib/common.rego
Outdated
| to_number(value) >= minimum | ||
| } else = false { | ||
| true | ||
| } | ||
|
|
||
| # checks if duration is greater or equal to a value in seconds | ||
| # duration: string (https://pkg.go.dev/time#ParseDuration) | ||
| duration_gte(duration, seconds) { |
There was a problem hiding this comment.
instead of seconds - duration as well
compliance/lib/common.rego
Outdated
| @@ -1,5 +1,7 @@ | |||
| package compliance.lib.common | |||
|
|
|||
| NS_FACTOR = 1000000000 # seconds to nano seconds factor | |||
elastic#12 ---NOTE--- This is an imported commit, it was initially committed to the csp-security-policies repo which was then merged into cloudbeat. See: elastic/cloudbeat#1405
added some new common functions
changed data_adapter to split process arguments to an array
new rules: