Skip to content
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

Context sensitive exceptions #545

Open
mykter opened this issue Apr 20, 2021 · 5 comments
Open

Context sensitive exceptions #545

mykter opened this issue Apr 20, 2021 · 5 comments
Labels
enhancement New feature or request exceptions Relates to conftest exceptions

Comments

@mykter
Copy link

mykter commented Apr 20, 2021

If I could go back in time I'd add to the discussion in #315, however..

We currently have a mechanism for suppressing named rules for an entire input file (or, as noted in that issue, the entire input when using --combine).

It would be nice to be able to add exceptions for specific matches on the input. Extending the deployment from the example:

apiVersion: apps/v1
kind: Deployment
metadata:
  name: mydep
spec:
  template:
    spec:
      containers:
      - name: web
        image: nginx
        ports:
        - containerPort: 8080
      - name: host-agent
        image: host-agent

Here the exception I want to be able to express is "mydep can run host-agent as root".
I don't want to say "mydep can run any containers as root", nor do I want to say "any deployment that includes the host-agent can run any container as root". As far as I can tell, those are the two closest options that are currently supported.

I'm not sure how this could be implemented.

Perhaps it could pair nicely with structured errors, so given:

violation_noservice[{"msg": msg, "details":{"name":name}}] {
  ...
  msg = sprintf("Found service %s but services are not allowed", [name])
}

we can somehow match against details->name?

Or alternatively you could do a textual match against a violation's msg, and then you can ensure that whatever contextual information you need is included in the msg. A little fragile, but perhaps straightforward. That's essentially the workaround I'm adopting - don't use the built-in exceptions, instead output all violations to json and then filter out any with msgs matching a list of exceptions.

I can't think of a way to express it more directly, i.e. tell conftest to ignore the no_service rule for mydep's host-agent container. If there is though that would be ideal, as it avoids depending on the rule to output the data you need, instead you can just define it directly in terms of the input.

@jpreese jpreese added the exceptions Relates to conftest exceptions label Apr 21, 2021
@jpreese
Copy link
Member

jpreese commented Apr 25, 2021

If I understand what you're trying to accomplish correctly, this should already be possible. When you create an exception, the body of that exception is what is matched on. e.g. for "mydep can run host-agent as root"

Full example:

apiVersion: apps/v1
kind: Deployment
metadata:
  name: mydep
spec:
  template:
    spec:
      containers:
      - name: web
        image: nginx
        ports:
        - containerPort: 8080
      - name: host-agent
        image: host-agent
---
apiVersion: apps/v1
kind: Deployment
metadata:
  name: not-mydep
spec:
  template:
    spec:
      containers:
      - name: web
        image: nginx
        ports:
        - containerPort: 8080
      - name: host-agent
        image: host-agent
package main

deny_host_agent_root[msg] {
    input.kind == "Deployment"
    not input.spec.securityContext.runAsNonRoot

    msg := "cannot run as root"
}

# Here the exception I want to be able to express is "mydep can run host-agent as root".
exception[rules] {
  input.metadata.name == "mydep"
  input.spec.template.spec.containers[_].name == "host-agent"

  rules = ["host_agent_root"]
}
conftest test -p policy.rego dep.yaml

@mykter
Copy link
Author

mykter commented Apr 27, 2021

Thanks for the complete example, here's a version that shows what I meant.

package main

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

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


# Here the exception I want to be able to express is "mydep can run host-agent as root".
exception[rules] {
  input.metadata.name == "mydep"
  input.spec.template.spec.containers[_].name == "host-agent"

  rules = ["root"]
}
apiVersion: apps/v1
kind: Deployment
metadata:
  name: mydep
spec:
  template:
    spec:
      containers: 
      - name: web
        image: nginx
        ports:
        - containerPort: 8080 
        securityContext:
          runAsNonRoot: false
      - name: host-agent
        image: host-agent
---
apiVersion: apps/v1
kind: Deployment
metadata:
  name: not-mydep
spec:
  template:
    spec:
      containers:
      - name: web
        image: nginx
        ports:
        - containerPort: 8080
        securityContext:
          runAsNonRoot: true
      - name: host-agent
        image: host-agent
        securityContext:
          runAsNonRoot: true
$ conftest test -p policy.rego dep.yaml
EXCP - dep.yaml - main - data.main.exception[_][_] == "root"

2 tests, 1 passed, 0 warnings, 0 failures, 1 exception

The issue shown here is that the exception intends to only allow mydep to run host-agent as root, but it actually allows my-dep to run any container as root - in the example it's running the web container as root, as well as host-agent.

I don't see a way in the current implementation that supports having a generic rule like "must specify runAsNonRoot=true" whilst having a targetted exception like this.

@mykter mykter closed this as completed Apr 27, 2021
@mykter mykter reopened this Apr 27, 2021
@jpreese
Copy link
Member

jpreese commented Apr 29, 2021

Awesome, thanks for the clarification. I see now. This feels like a duplicate of #453

@mykter
Copy link
Author

mykter commented Apr 30, 2021

Perhaps it is, I struggled to follow 453 tbh. Definitely in the same ballpark.

@moredhel
Copy link
Contributor

I've been having the same issue, specifically when trying to apply exceptions to specific resources in a terraform file.

I had a look at how we could extend conftest to support this use-case. I also put together a quick & dirty proof of concept to see what was possible.

This can set the stage for further discussion & refinement on the feature.

#584

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request exceptions Relates to conftest exceptions
Projects
None yet
Development

No branches or pull requests

3 participants