Warn about sensitive data in environment variables that aren't in Kubernetes secrets

[atombrella]

Description

I was unsuccessfully trying to make a custom rule in Rego to warn about Kubernetes secrets that aren't in a Kubernetes secret.

# ....
        env:
          - name: "PGPASSWORD"
            value: "baz"

Where something like this would be more correct.

# ....
        env:
          - name: PGPASSWORD
            valueFrom:
              secretKeyRef:
                name: schemapass
                key: password

The Rego code that I made an effort to get working:

# METADATA
# title: Environment variables that contain sensitive data should be in a Kubernetes secret
# description: Passwords and tokens should not be stored in clear-text environment variables. They are better stored in a Kubernetes secret.
# schemas:
#   - input: schema["kubernetes"]
# related_resources:
#   - https://kubernetes.io/docs/concepts/configuration/secret/
# custom:
#   id: KUB-REL-ID006
#   avd_id: KUB-REL-ID006
#   severity: CRITICAL
#   input:
#     selector:
#     - type: kubernetes

package user.kubernetes.ID006

import data.lib.kubernetes

import future.keywords.in

# perhaps this could be customizable, as it's impossible to
# generate an exhaustive list of sensitive variable names
banned_env_names := {"PGPASSWORD", "MYSQL_ROOT_PASSWORD", "AWS_SECRET_KEY"}

deny[res] {
    some container in kubernetes.containers[_]
    some env in container.env[_]
    env.name in banned_env_names
    has_inline_value(env)
    msg := kubernetes.format(sprintf("%s %s in %s namespace contains one or more env-variables with sensitive data in cleartext.", [
      lower(kubernetes.kind), kubernetes.name, kubernetes.namespace
    ]))
    res := result.new(msg, input.spec.template.spec)
}

# Check if the environment variable is set using a direct value
has_inline_value(env) {
  env.value != ""
  not env.valueFrom.secretKeyRef
}

You already check for tokens and passwords in cleartext when scanning container/docker images. As noted in the comment, I think it would be incorrect to block environment variables whose names have PASSWORD in them, as I've noticed examples of such variables that are meant to have boolean values etc. I don't know Rego and Trivy internals well enough to determine if it could be a setting with names that definitely should/must be in Kubernetes secrets.

Target

Kubernetes

Scanner

Misconfiguration

[simar7]

@atombrella what's your question?

[atombrella]

@simar7 Basically if this could be something to add as a feature in Trivy? I'd find value in such a check. Leaving sensitive data in clear text in a Helm chart and Kubernetes resource definitions also goes under the umbrella of security.

The learning curve of Rego is a bit steep, and I'm still battling to get the code in the post to match a Kubernetes deployment file with PGPASSWORD set with a direct value, instead of valueFrom.

[simar7]

Ah right - It's interesting. We've thought about this in the past. Here's the PR where we tackled one such situation aquasecurity/trivy-checks#374 - this was in annotations.

The biggest concern with such a check is performance. Checking for arbitrary text values can be quite expensive.

[atombrella]

Thanks for the PR link. It's good with some examples to learn this somewhat tricky language.

One thing to consider with my proposal is if you can feed a setting into the Rego check. For a project, you have a limited set of applications with environment variables that can contain sensitive data. Perhaps that could be a message broker, a database and a cloud provider, so most likely at most 10 variable names to check for.

Could this discussion be turned into an issue?

[simar7]

One thing to consider with my proposal is if you can feed a setting into the Rego check. For a project, you have a limited set of applications with environment variables that can contain sensitive data. Perhaps that could be a message broker, a database and a cloud provider, so most likely at most 10 variable names to check for.

You can already pass in custom data. https://trivy.dev/latest/docs/scanner/misconfiguration/custom/data/

Could this discussion be turned into an issue?

I'm not sure if we should be adding this as a built in check due to the performance limitations mentioned above. This would be a good case to write a custom check that fits your needs. You can pass the data to it using the feature I linked above.

[atombrella]

Thank you! I already have a small private repository with some checks that aren't security related (probes being some of them).

I'd like to make it public when it's less draft-like, and actually has some tests and documentation.