Add new rule that detect retrieving environment variables from /proc files

[hi120ki]

Signed-off-by: Hi120ki [email protected]

What type of PR is this?

Uncomment one (or more) /kind <> lines:

/kind bug

/kind cleanup

/kind design

/kind documentation

/kind failing-test

/kind feature

/kind release

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

/kind rule-update

/kind rule-create

Any specific area of the project related to this PR?

Uncomment one (or more) /area <> lines:

/area build

/area engine

/area rules

/area tests

/area proposals

/area CI

What this PR does / why we need it:

These days, it's common to pass a secret to an application running on a container via an environment variable. Secrets store very sensitive information such as cloud credentials and JWT secretkeys, so it is important to detect when environment variables have been retrieved by an attack.

And if the application has SSRF or path traversal vulnerabilities, the attacker can steal the environment variables of the application process by reading /proc/self/environ and /proc/1/environ under the /proc directory.

https://linux.die.net/man/5/proc

For example, SSRF:

curl -X POST -d "url=file:///proc/self/environ" https://example.com/ssrf
curl -X POST -d "url=file:///proc/1/environ" https://example.com/ssrf

Path traversal:

curl --path-as-is https://example.com/path/../../proc/self/environ
curl --path-as-is https://example.com/path/../../proc/1/environ

In MITRE ATT&CK, this technique is categorised as T1552 : Unsecured Credentials, and Sub-technique T1552.001 : Unsecured Credentials: Credentials In Files, and Detection is DS0022 - File - File Access, and component is T1003.007 OS Credential Dumping: Proc Filesystem.

I propose the following as a rule to detect environment variable reading from files under the /proc directory.

- list: proc_environ_file_names
  items: [/proc/self/environ, /proc/1/environ]

- macro: proc_environ_files
  condition: >
    fd.name in (proc_environ_file_names)

- rule: Read environment variable from /proc files
  desc: An attempt to read process environment variables from /proc files
  condition: >
    container and open_read and proc_environ_files
  enabled: true
  output: >
    Environment variables were retrieved from /proc files (user=%user.name user_loginuid=%user.loginuid program=%proc.name
    command=%proc.cmdline file=%fd.name parent=%proc.pname gparent=%proc.aname[2] ggparent=%proc.aname[3] gggparent=%proc.aname[4] container_id=%container.id image=%container.image.repository)
  priority: WARNING
  tags: [filesystem, mitre_credential_access, mitre_discovery]

Adding a host to the detection target causes false positives by systemd, so limited to container.

I'm waiting for your comments on detecting reading these files and my proposed rules.

Thank you.

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

rule(list: known_binaries_to_read_environment_variables_from_proc_files): add new list
rule(Read environment variable from /proc files): add rule to detect an attempt to read process environment variables from /proc files

[poiana]

Welcome @hi120ki! It looks like this is your first PR to falcosecurity/falco 🎉

[darryk10]

Hi @hi120ki, thanks for contributing first of all :)
Amazing rule and I agree with you on the security use case.
I also do like the container scoping to avoid the noise since we need to rely on open_read events.
Just wondering if we can open the rule to hosts, just whitelisting systemd as you mentioned, or you feel it might introduce too much default noise.
I'm going to test the rule as well to further reduce the noise if possible :)
Thanks

[jasondellaluce]

/ok-to-test

[jasondellaluce]

/milestone 0.33.0

[hi120ki]

@darryk10 @jasondellaluce I update my proposed rule to expand host.

- rule: Read environment variable from /proc files
  desc: An attempt to read process environment variables from /proc files
  condition: >
    open_read and (fd.name glob /proc/*/environ)
    and not proc.name in (systemctl, systemd-detect-, cloud-id)
  enabled: true
  output: >
    Environment variables were retrieved from /proc files (user=%user.name user_loginuid=%user.loginuid program=%proc.name
    command=%proc.cmdline file=%fd.name parent=%proc.pname gparent=%proc.aname[2] ggparent=%proc.aname[3] gggparent=%proc.aname[4] container_id=%container.id image=%container.image.repository)
  priority: WARNING
  tags: [filesystem, mitre_credential_access, mitre_discovery]

Host machine has many processes, and there is many /proc/*/environ files.(* is process id).
So, updated rule uses use glob instead of in and enables to detect any process's environ files.

Then, in host machine some system process such as systemd uses /proc/*/environ files, and I added allowlist into rule.
If you find other system processes, cloud you add to allowlist?

[hi120ki]

@darryk10 @jasondellaluce I tryed to apply my rule to my testbed, there are so many false positive detection from host machine. (For example, systemd-tmpfile systemd-user-se systemd-update-)
Of cource, it is nice to add all systemd and other system-level process binary to allowlist if we can, but systemd updates fast, and it is impossible to maintain this allowlist.

So, we should restrict this rule only container. I'm waiting for your comments.

[darryk10]

Hi @hi120ki, thanks for the further test. I've been testing the host condition and I agree the rule generates too noise. Even though would be good to have a role covering host as well, I would suggest to keep this use case for container only to reduce the noise as you suggested in the PR.
Regarding the condition for container, instead, it's very accurate and very low false positive. In addition I observed good true positives on malwares and bad activities so definitely a rule we want in our ruleset.
Happy to approve the PR once reverted on container use case only.
Really appreciate your work and looking forward to receiving new rules from you :)

[hi120ki]

@darryk10 Hi, I run this rule on my testbed, and found one false positive detection in scsi_id proc.
This proc runs by gcp-compute-persistent-disk-csi-driver, it is common in GKE cluster.
I added this into allowlist, could you review it?

[darryk10]

@hi120ki I think you also need to specify the change log in the PR otherwise the PR isn't mergeable.

[jasondellaluce]

/approve

[Kaizhe]

to optimize rule checking performance, it is good to put open_read in the beginning of the rule.

[hi120ki]

Thank you! I put open_read in the beginning of the rule.

[poiana]

LGTM label has been added.

Git tree hash: 36807142c07ff0a54c78bd2f0180c69e8a4cd8ca

[jasondellaluce]

/approve

[poiana]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: darryk10, hi120ki, jasondellaluce, leogr

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

The pull request process is described here

Needs approval from an approver in each of these files:

• rules/OWNERS [jasondellaluce,leogr]

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

[leogr]

Closing and reopening to trigger the CI
/close

[poiana]

@leogr: Closed this PR.

In response to this:

Closing and reopening to trigger the CI
/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.

[leogr]

/reopen

[poiana]

@leogr: Reopened this PR.

In response to this:

/reopen

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]

/hold

for the CI to complete

[leogr]

/hold cancel