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

Proposal add psp rules support #825

Merged
merged 2 commits into from
Oct 14, 2019
Merged

Proposal add psp rules support #825

merged 2 commits into from
Oct 14, 2019

Conversation

mstemm
Copy link
Contributor

@mstemm mstemm commented Sep 10, 2019

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 flaky-test

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 engine

/area rules

/area deployment

/area integrations

/area examples

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

NONE

Separate from the PR that actually makes the changes.

Signed-off-by: Mark Stemm <mark.stemm@gmail.com>

### Goals

Transparently read a candidate PSP into an equivalent set of falco rules that can look for the conditions in the PSP.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we have more technical detail here? Are we generating Falco rules as a file? as a pipe? as shared memory?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PSP is converted into a set of falco rules which can be either saved as a file for later use/inspection, or loaded directly so they they can monitor system calls and k8s audit activity.


### Use cases

You'll be able to run falco with a `--psp` argument that provides a single PSP Yaml file. Falco will automatcially convert the PSP into an equivalent set of falco rules, load the rules, and then run with the loaded rules. You can optionally provide a `--psp_save=<path>` command line option to save the converted rules to a file.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Arguments to the CLI are effectively an API

My only suggestion here would be that we start using some sort of prefix to denote this is Kubernetes specific.

For example:

--k8s-pod-security-policy
--k8s-psp
--enable-kubernetes-pod-security-policy

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like the falco daemon cli to also do cli related tasks, this should be done in falcoctl in my opinion

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer having a single program handle both the steps of conversion and loading the generated rules, as I feel that the fewer steps involved, the easier it is to get set up and running.


## Summary

We want to make it easier for K8s Cluster Operators to Author Pod Security Policies by providing a way to read a PSP, convert it to a set of falco rules, and then run falco with those rules.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes! This would be amazing for the project! Strong +1 from me

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where is the convertion done? In a cli ? In falco itself?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's in falco. Happy to discuss whether the conversion occurs in falco or outside--I lean towards in falco as it involves fewer moving parts, but obviously there are other opinions.

Remember, though, that the bulk of the work is in implementing the rules that result from the PSP, which requires new operators and fields to work on audit logs.

Copy link
Contributor

@fntlnz fntlnz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand why this is proposed to be in the falco code base.

From my understanding this is a meechanism to create falco rules based on kubernetes PSPs.

I would prefer to have an external tool to generate the rules and falco just use it without changing the falco binary to support new flags and adding code into it for this.


## Summary

We want to make it easier for K8s Cluster Operators to Author Pod Security Policies by providing a way to read a PSP, convert it to a set of falco rules, and then run falco with those rules.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where is the convertion done? In a cli ? In falco itself?

proposals/20190909-psp-rules-support.md Show resolved Hide resolved

### Use cases

You'll be able to run falco with a `--psp` argument that provides a single PSP Yaml file. Falco will automatcially convert the PSP into an equivalent set of falco rules, load the rules, and then run with the loaded rules. You can optionally provide a `--psp_save=<path>` command line option to save the converted rules to a file.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like the falco daemon cli to also do cli related tasks, this should be done in falcoctl in my opinion


### Diagrams

No diagrams yet.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will be helpful

proposals/20190909-psp-rules-support.md Show resolved Hide resolved
@leodido leodido added this to the 0.18.0 milestone Sep 25, 2019
Copy link
Member

@leodido leodido left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!

Just a bunch of typos and fixes I am going to fix before merging it.

proposals/20190909-psp-rules-support.md Outdated Show resolved Hide resolved
proposals/20190909-psp-rules-support.md Outdated Show resolved Hide resolved
proposals/20190909-psp-rules-support.md Outdated Show resolved Hide resolved
proposals/20190909-psp-rules-support.md Show resolved Hide resolved
proposals/20190909-psp-rules-support.md Outdated Show resolved Hide resolved
Co-authored-by: Lorenzo Fontana <lo@linux.com>
Signed-off-by: Leonardo Di Donato <leodidonato@gmail.com>
@poiana
Copy link

poiana commented Sep 26, 2019

LGTM label has been added.

Git tree hash: ccdf8cfc14e5cfa01753653c8e7e348e742d2979

@poiana
Copy link

poiana commented Sep 26, 2019

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: fntlnz, leodido

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:

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

@leodido leodido modified the milestones: 0.18.0, 0.19.0 Oct 3, 2019
@fntlnz
Copy link
Contributor

fntlnz commented Oct 7, 2019

/area proposals

@leodido leodido merged commit 6019320 into dev Oct 14, 2019
@poiana poiana deleted the proposal-add-psp-rules-support branch October 14, 2019 11:59
@fntlnz fntlnz modified the milestones: 0.19.0, 0.18.0 Oct 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants