Proposal: fine-grained policy-excludes

[moredhel]

This is a proposal for implementing fine-grained excludes.

This proposal adds a new excludes type to the conftest tool which allows for more fine-grained exclusion of policies. I have also attached a sample implementation (linked here) that can already be played with & serve as a reference.

Issue

Currently conftest is limited to only being able to 'disable' a policy rule over a whole file. This means that it's not possible to have a policy be applied to a subset of elements within a file, it's all or nothing.

This is naturally an issue, especially for projects which are wanting to adopt conftest incrementally into an already existing project. In this case it is difficult to apply a fine-grained policy enforcement which blocks newly added policy-violating definitions into an already existing file.

While there are workarounds, it is difficult to use them and requires modifying the deny rules directly (meaning they won't show up as being explicit exceptions).

Proposal

The linked Proposal includes an extension to conftest with a new keyword exclude. This can be used to attach specific exclusions to a policy. Below is a quick sample:

package main

deny_root[result] {
	input.kind == "Deployment"
	c = input.spec.template.spec.containers[_]
	not c.securityContext.runAsNonRoot

	# "msg" is used to set the message that conftest will output
        # all other keys are used to match against the exclusion
        #
        # "result" can still be a plain string for backwards compatibility
	result := {
		"container": c.name,
		"deployment": input.metadata.name,
		"msg": sprintf("container %s in deployment %s doesn't set runAsNonRoot", [c.name, input.metadata.name]),
	}
}

# here we are adding specific exceptions.
root_exceptions = [{"deployment": "mydep", "containers": ["host-agent"]}]

# The exception we want to be able to express is "mydep can run container 'host-agent' as root", but no other containers (even in the same pod) are allowed to run as root.
exclude_root[attrs] {
	deployment := input.metadata.name
	container := input.spec.template.spec.containers[_].name
	exception := root_exceptions[_]

	deployment == exception.deployment
	container == exception.containers[_]

        here we can return a list of attributes which we would like to match & exclude on.
	attrs = [{"container": container, "deployment": deployment}]
}

First off, the current behaviour is preserved, so no policies will be affected by this change.

There are a few changes to how we need to write our rego.

1. When returning from the deny/warn policy, an object can be returned instead (as specified in the above example)
2. The exclusion & the deny rules must have a matching suffix (ie. deny_root & exclude_root).
3. the return of the exclude is matched against the attributes of the deny rule (rather than on the name of the rule)

With these changes, we are able to write more flexible exclusion rules with as much flexibility in our exclusion logic as we would like.

Actions

• Are there any blocking issues that should be addressed? If not, then I would appreciate an ok & I will proceed with documentation & flesh out the testing in preparation for merging.

References

• Original Issue
• Proposed Pull Request

[rdsubhas]

Contextual exclusions will greatly improve quality of policies. Thank you for kickstarting this and for the PoC PR! I would like to offer one additional viewpoint, if I may...

A lot of the cumbersomeness with contextual exclusions – is passing the context. The actual deny or warn rule has the full localized context, and somehow bits & pieces of this have to be passed on to evaluate exclusions. This adds an additional layer of connection/indirection, which is hard to enforce.

What if we never have to pass on the context, and the exclusion can be done inline – right where the context is? i.e. What if exclusion is not a separate construct, but a function. For example:

deny_root[result] {
	input.kind == "Deployment"

	exclude(input.name == "mydep")
	# ^^^^^^^^^^^^ let's use context right here

	c = input.spec.template.spec.containers[_]
	not c.securityContext.runAsNonRoot

	exclude(c.name == "host-agent")
	# ^^^^^^^^^^^^ let's use context right here

	result := sprintf("container %s in deployment %s doesn't set runAsNonRoot", [c.name, input.metadata.name])
	# ^^^^^^^^^^^^ normal sprintf result, no need to pass around context
}

Would be awesome if the exclude could be caught & the context made a note of without breaking the flow – i.e. the exclude function doesn't do anything, but the rule is allowed to continue executing so that the reason is captured, and then at the end of evaluation the rule is marked as excluded with it's reason intact. Or maybe the excludes have to be moved to the bottom to have the reason already present in the context like:

deny_root[result] {
	input.kind == "Deployment"
	c = input.spec.template.spec.containers[_]
	not c.securityContext.runAsNonRoot
	result := sprintf("container %s in deployment %s doesn't set runAsNonRoot", [c.name, input.metadata.name])

	exclude(c.name == "host-agent")
	exclude(input.name == "mydep")
}

[moredhel]

Hi @rdsubhas,

Thanks for the detailed feedback! I appreciate it.

I see the appeal of this solution, as you say having the full context makes it considerably easier to make sense of what is going on & help reduce the chance of an error.

My proposal definitely includes additional complexity & possibility for misconfiguration (Although the current exclude functionality still has this issue).

One issue I see with embedding the exclude logic in-line into the policy rules is that they are no longer as re-usable. Take for example department A who develops a rule:

deny_root[result] {
	input.kind == "Deployment"
	c = input.spec.template.spec.containers[_]
	not c.securityContext.runAsNonRoot
	result := sprintf("container %s in deployment %s doesn't set runAsNonRoot", [c.name, input.metadata.name])

	exclude(c.name == "host-agent")
}

If department B would like to re-use the above, it would need to be copied & pasted (in the case they would like to extend/remove the exclusions.

---

You do bring forward a good point though & I think as rego policies get more complicated more engineering practices will need to be applied (such as refactoring most functionality into functions).

## ------ Helpers -----
deployment_containers[container] {
    container := input.spec.template.spec.containers[_]
}

is_deployment {
   input.kind == "Deployment"
}

deployment_containers_root[result] {
     container := deployment_containers[_]
     # conditions
     not c.securityContext.runAsNonRoot

     result := {
        "msg": sprintf("container %s in deployment %s doesn't set runAsNonRoot", [container.name, input.metadata.name]),
        "meta": container
    }
}

# ----- Policy ---------

deny_root[result] {
     is_deployment
     msg := deployment_containers_root
     result := msg[_]
}

exclude_root[result] {
   value := deny_root[_]
   input.metadata.name == "my-dep"
   value.meta.name == "host-agent"
   result := value
}

It's always going to be difficult to add exclusions to certain policies.

Let me know what you think of using the output of the deny_root as the input into the exclude_root rule. This means the it's much easier to know that the inputs are known to be failing.

[rd-zahari-aleksiev]

I think this is more relevant than ever , if you're using --combine, the exception is not only for the whole file, but the whole setup of files and workaround in the rules itself is just that, a workaround, the EXCP is very much needed, because it is a form of documenting your exceptions.