rules: the required_engine_version is now on by default

[fntlnz]

Co-Authored-By: Leonardo Di Donato leodidonato@gmail.com
Signed-off-by: Lorenzo Fontana lo@linux.com

What type of PR is this?
/kind feature

If contributing rules or changes to rules, please make sure to also uncomment one of the following line:

/kind rule-update

Any specific area of the project related to this PR?

/area rules

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #1272

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

update: the required_engine_version is now on by default 
rule(Container Drift Detected (open+create)): specify that rule is only enabled with engine 6
rule(Create Disallowed Pod): required_engine_version 5
rule(Create Privileged Pod): required_engine_version 5
rule(Create Sensitive Mount Pod): required_engine_version 5
rule(Create HostNetwork Pod): required_engine_version 5
rule(Pod Created in Kube Namespace): required_engine_version 5
rule(ClusterRole With Wildcard Created): required_engine_version 5
rule(ClusterRole With Write Privileges Created): required_engine_version 5
rule(ClusterRole With Pod Exec Created): required_engine_version 5

[leodido]

/milestone 0.24.0

[fntlnz]

Now that we are scoping the engine version we probably need to do a review and add this to older rules that were subject to an engine version increase too.

[poiana]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please assign fntlnz
You can assign the PR to them by writing /assign @fntlnz in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[fntlnz]

While working on this @leodido and I also took a stub and went back in time to update the minimum engine version in all the rules we have.

This is Falco 0.20.0 (engine version 5) if you try to use a rule of the current master (engine version 6)

root@9e5db6db2068:/# falco -r /rules/falco_rules.yaml  -r /rules/k8s_audit_rules.yaml 
2020-06-23T10:11:15+0000: Falco initialized with configuration file /etc/falco/falco.yaml
2020-06-23T10:11:15+0000: Loading rules from file /rules/falco_rules.yaml:
2020-06-23T10:11:15+0000: Runtime error: Rules require engine version 6, but engine version is 5
---
- rule: Container Drift Detected (open+create)
  desc: New executable created in a container due to open+create
  required_engine_version: 6
  condition: >
    evt.type in (open,openat,creat) and
    evt.is_open_exec=true and
    container and
    not runc_writing_exec_fifo and
    not runc_writing_var_lib_docker and
    evt.rawres>=0
  output: Drift detected (open+create), new executable created in a container (user=%user.name command=%proc.cmdline filename=%evt.arg.filename name=%evt.arg.name mode=%evt.arg.mode event=%evt.type)
  priority: ERROR
# Application rules have moved to application_rules.yaml. Please look
# there if you want to enable them by adding to
# falco_rules.local.yaml.
---. Exiting.
root@9e5db6db2068:/# ^C
root@9e5db6db2068:/# falco --version                                                  
Falco version: 0.20.0+d77080a

This is Falco 0.15.0 (engine version 3) if you try to load a rule with engine version 5.

root@c19b49f0bec2:/# falco -r /rules/falco_rules.yaml -r /rules/k8s_audit_rules.yaml 
2020-06-23T10:15:14+0000: Falco initialized with configuration file /etc/falco/falco.yaml
2020-06-23T10:15:14+0000: Loading rules from file /rules/falco_rules.yaml:
2020-06-23T10:15:15+0000: Loading rules from file /rules/k8s_audit_rules.yaml:
2020-06-23T10:15:15+0000: Runtime error: Error loading rules: [string "-- Copyright (C) 2016-2018 Draios Inc dba Sys..."]:222: Rules require engine version 5, but engine version is 3. Exiting.

[leogr]

Overall SGTM, but not yet sure anything is working as expected.

I have tested this rule files with the experimental event-generator test which requires the gRPC bidi, thus I had to:

• disable the Container Drift Detected (open+create) since the gRPC bidi branch is still on engine version 5
• add kubernetes-admin to the allowed_k8s_users since events were generate with that user within my installation (Kind)

That being said some k8saudit tests did not pass:

INFO sleep for 1s                                  action=k8saudit.K8SServiceaccountCreated
INFO create k8s resource                           action=k8saudit.K8SServiceaccountCreated kind=ServiceAccount name=vanilla-serviceaccount
INFO create k8s resource                           action=k8saudit.K8SServiceaccountCreated kind=Role name=vanilla-role
INFO create k8s resource                           action=k8saudit.K8SServiceaccountCreated kind=RoleBinding name=vanilla-role-binding
INFO test passed                                   action=k8saudit.K8SServiceaccountCreated rule="K8s Serviceaccount Created" source=K8S_AUDIT
INFO sleep for 1s                                  action=k8saudit.K8SServiceCreated
INFO create k8s resource                           action=k8saudit.K8SServiceCreated kind=Service name=vanilla-service
INFO test passed                                   action=k8saudit.K8SServiceCreated rule="K8s Service Created" source=K8S_AUDIT
INFO sleep for 1s                                  action=k8saudit.ClusterRoleWithWritePrivilegesCreated
INFO create k8s resource                           action=k8saudit.ClusterRoleWithWritePrivilegesCreated kind=Role name=write-privileges-role
WARN context deadline exceeded                    
INFO sleep for 1s                                  action=k8saudit.CreateSensitiveMountPod
INFO create k8s resource                           action=k8saudit.CreateSensitiveMountPod kind=Deployment name=sensitive-mount-deployment
WARN rpc error: code = DeadlineExceeded desc = context deadline exceeded 
INFO sleep for 1s                                  action=k8saudit.CreatePrivilegedPod
INFO create k8s resource                           action=k8saudit.CreatePrivilegedPod kind=Deployment name=privileged-deployment
WARN context deadline exceeded                    
WARN action not enabled                            action=k8saudit.CreateDisallowedPod
INFO sleep for 1s                                  action=k8saudit.CreateHostNetworkPod
INFO create k8s resource                           action=k8saudit.CreateHostNetworkPod kind=Deployment name=hostnetwork-deployment
WARN context deadline exceeded                    
INFO sleep for 1s                                  action=k8saudit.CreateNodePortService
INFO create k8s resource                           action=k8saudit.CreateNodePortService kind=Service name=nodeport-service
INFO test passed                                   action=k8saudit.CreateNodePortService rule="Create NodePort Service" source=K8S_AUDIT
INFO sleep for 1s                                  action=k8saudit.ClusterRoleWithPodExecCreated
INFO create k8s resource                           action=k8saudit.ClusterRoleWithPodExecCreated kind=Role name=pod-exec-role
WARN context deadline exceeded                    
INFO sleep for 1s                                  action=k8saudit.CreateModifyConfigmapWithPrivateCredentials
INFO create k8s resource                           action=k8saudit.CreateModifyConfigmapWithPrivateCredentials kind=ConfigMap name=private-creds-configmap
INFO test passed                                   action=k8saudit.CreateModifyConfigmapWithPrivateCredentials rule="Create/Modify Configmap With Private Credentials" source=K8S_AUDIT
INFO sleep for 1s                                  action=k8saudit.K8SConfigMapCreated
INFO create k8s resource                           action=k8saudit.K8SConfigMapCreated kind=ConfigMap name=vanilla-configmap
INFO test passed                                   action=k8saudit.K8SConfigMapCreated rule="K8s ConfigMap Created" source=K8S_AUDIT
INFO sleep for 1s                                  action=k8saudit.K8SDeploymentCreated
INFO create k8s resource                           action=k8saudit.K8SDeploymentCreated kind=Deployment name=vanilla-deployment
INFO test passed                                   action=k8saudit.K8SDeploymentCreated rule="K8s Deployment Created" source=K8S_AUDIT
INFO sleep for 1s                                  action=k8saudit.ClusterRoleWithWildcardCreated
INFO create k8s resource                           action=k8saudit.ClusterRoleWithWildcardCreated kind=Role name=wildcard-resources-role
WARN rpc error: code = DeadlineExceeded desc = Deadline Exceeded 

N.b.: "context deadline exceeded" means the rule was not triggered within 2 mins.

On the other hand, all syscall tests passed.

Please, do not take these results as definitive since the tester is still experimental and I have to double-check results again. I will investigate more soon.

[fntlnz]

/hold

There is something going on with the integration tests.
Looks like required_engine_version changes the tests behavior in unexpected ways. @leodido and I tested locally and looks like it complains about loading the driver (which should not be necessary at all here)

[Kaizhe]

Does it mean we have a global #- required_engine_version: 2 and required_engine_version per rule?

[fntlnz]

Yes @Kaizhe - every rule inherits the global requirements while specific rules can specify an higher requirement if they contain a field that was added on a specific engine version.

[leogr]

I might have found the problem with the integration test. I'm working on it.
/assign

[leogr]

cc @leodido @fntlnz
This PR still needs some rework, moreover, I can confirm #1315 affects this PR, so I propose to reschedule for the 0.25.0 milestone

[leodido]

/milestone 0.25.0

[leogr]

Rebased on the current master. Working on it.
/assign

[leogr]

I can confirm that integration tests are not passing for the reason explained here 👉 #1315

I'm investigating and trying to fix this problem.

Please keep on
/hold

[leogr]

Yes @Kaizhe - every rule inherits the global requirements while specific rules can specify an higher requirement if they contain a field that was added on a specific engine version.

Unfortunately, that is not true. I have discovered that rule loader supports required_engine_version at document level only.

It was not immediately apparent because when the required_engine_version property is found in any object, the loader just overrides the engine version of the context (i.e., the YAML document) with no errors nor warnings. Moreover, any object that contains required_engine_version is not further inspected; thus, if a rule contains required_engine_version, it will just not loaded (same behavior in case of list and macro as well).

I've formatted the Lua code to make the if/elseif chain (that implements the above behavior) clearer.

For these reasons, I've converted the required_engine_version of this PR into comments by 490a67c

Although this PR does not introduce significant changes eventually, IMHO it worths merging this anyway since that comments and code formatting will help folks to understand how to use rules properly).

cc @fntlnz

[leogr]

Rebased again to solve conflicts with the master branch.

I believe this is ready to be reviewed now.

/hold cancel

[leodido]

/poiana

[poiana]

@leodido: I'm Poiana, I stop the drama!

In response to this:

/poiana

Instructions 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/test-infra repository.

[leogr]

Hey @fntlnz

I'd propose to close this PR since it cannot fix the original problem and it just introduces some comment and indentation (and because of that, it's conflicting every time a rule file is changed).

WDYT?
cc @leodido

[leogr]

Hey @fntlnz

I'd propose to close this PR since it cannot fix the original problem and it just introduces some comment and indentation (and because of that, it's conflicting every time a rule file is changed).

WDYT?
cc @leodido

Just proposing again to close this, since #1381 got merged.
/hold

[fntlnz]

/close

[poiana]

@fntlnz: Closed this PR.

In response to this:

/close

Instructions 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/test-infra repository.