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

ka.useragent field #709

Merged
merged 3 commits into from
Jul 8, 2019
Merged

ka.useragent field #709

merged 3 commits into from
Jul 8, 2019

Conversation

fntlnz
Copy link
Contributor

@fntlnz fntlnz commented Jul 8, 2019

What type of PR is this?

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

/kind feature
/kind cleanup

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

Any specific area of the project related to this PR?

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

/area engine

What this PR does / why we need it:

  • Adds the ka.useragent field as alias of jevt.value[/userAgent]
  • Doing this also required another commit to do the necessary formatting according to the new .clang-format

Which issue(s) this PR fixes:

Fixes #698

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

fields: kubernetes audit ka.useragent alias

@fntlnz
Copy link
Contributor Author

fntlnz commented Jul 8, 2019

To test this, this rule can be used:

- rule: Only allow hyperkube as useragent
  desc: Test doing an useragent check for k8s audit
  condition: ka.useragent != "hyperkube"
  priority: ERROR
  output: "unexpected client useragent: useragent=%ka.useragent " 
  source: k8s_audit

Here is a detection example:

Events detected: 434
Rule counts by severity:
   ERROR: 434
Triggered rules by rule name:
   Only allow hyperkube as useragent: 434
Syscall event drop monitoring:
   - event drop detected: 44 occurrences
   - num times actions taken: 11

@fntlnz fntlnz changed the title Feat/ka useragent field ka.useragent field Jul 8, 2019
fntlnz and others added 3 commits July 8, 2019 12:52
Signed-off-by: Lorenzo Fontana <lo@linux.com>

Co-authored-by: Leonardo Di Donato <leodidonato@gmail.com>
Signed-off-by: Lorenzo Fontana <lo@linux.com>

Co-authored-by: Leonardo Di Donato <leodidonato@gmail.com>
Signed-off-by: Lorenzo Fontana <lo@linux.com>

Co-authored-by: Leonardo Di Donato <leodidonato@gmail.com>
@fntlnz
Copy link
Contributor Author

fntlnz commented Jul 8, 2019

/assign @leodido

@poiana
Copy link

poiana commented Jul 8, 2019

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 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

@poiana poiana added the lgtm label Jul 8, 2019
@poiana
Copy link

poiana commented Jul 8, 2019

LGTM label has been added.

Git tree hash: 8dc8e963bf40037050801d27534d2dce0a4dfbfd

@poiana poiana added the approved label Jul 8, 2019
@leodido leodido merged commit affb108 into dev Jul 8, 2019
@poiana poiana deleted the feat/ka-useragent-field branch July 8, 2019 15:40
@@ -24,4 +24,4 @@ limitations under the License.
// This is the result of running "falco --list -N | sha256sum" and
// represents the fields supported by this version of falco. It's used
// at build time to detect a changed set of fields.
#define FALCO_FIELDS_CHECKSUM "9b5557ec8f16f5606a1544573b152d211d5212f653ee039146836a17266ff449"
#define FALCO_FIELDS_CHECKSUM "ceb069d9f9b2d4ebcc5de39bddc53b7af2e6b8f072edc293668fd6ac4e532413"
Copy link
Contributor

Choose a reason for hiding this comment

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

When you update the checksum, that almost always means new fields were added to either sysdig (for syscall events) or falco (for k8s audit events). When that happens, you should also bump the FALCO_ENGINE_VERSION define on line 22. That allows us to know the times at which the engine was upgraded to have new fields. When you actually use those fields, you should update the rules file that uses them to have a - required_engine_version: NNN where NNN is the current value of FALCO_ENGINE_VERSION.

Copy link
Contributor Author

@fntlnz fntlnz Jul 8, 2019

Choose a reason for hiding this comment

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

good to know, is this documented anywhere or should we do it? @mstemm

Copy link
Contributor

Choose a reason for hiding this comment

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

From a user perspective, there are some notes at https://falco.org/docs/rules/#versioning. Not much for developer specific notes, though.

@mstemm
Copy link
Contributor

mstemm commented Jul 8, 2019

In the future, I think it would be a good idea to separate whitespace/formatting changes from other changes. It's hard to tell which changes are simply syntax and which change behavior.

(It is a separate commit, which is very handy, but I think even as a separate PR is better, as it's easier to identify from the PR name).

@fntlnz
Copy link
Contributor Author

fntlnz commented Jul 8, 2019

@mstemm We did the changes in two separate commits, however I agree that doing in two different PRs would've been even better!

@leodido leodido mentioned this pull request Jul 8, 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.

Add a new field ka.useragent
4 participants