Skip to content

Add audit action to the policy verdict log#11843

Merged
aanm merged 1 commit intocilium:masterfrom
ap4y:policy-verdict-log-audit
Jun 4, 2020
Merged

Add audit action to the policy verdict log#11843
aanm merged 1 commit intocilium:masterfrom
ap4y:policy-verdict-log-audit

Conversation

@ap4y
Copy link
Copy Markdown
Contributor

@ap4y ap4y commented Jun 2, 2020

The audit mode may overwrite policy related verdict and in this
situation it's unclear from the policy verdict log that this
happened. This commit adds a new audit bit to the policy log's struct
and adds a new policy log action 'audit' based on that flag. Related
policy log calls were updated to contain audit flag.

fixes #11778

@ap4y ap4y requested a review from a team June 2, 2020 23:53
@ap4y ap4y requested a review from a team as a code owner June 2, 2020 23:53
@maintainer-s-little-helper
Copy link
Copy Markdown

Please set the appropriate release note label.

@coveralls
Copy link
Copy Markdown

coveralls commented Jun 3, 2020

Coverage Status

Coverage increased (+0.005%) to 36.938% when pulling 0f8abe0 on ap4y:policy-verdict-log-audit into 8ba1f84 on cilium:master.

@ap4y ap4y force-pushed the policy-verdict-log-audit branch from 1b6c730 to dcf892b Compare June 3, 2020 00:41
@ap4y ap4y requested a review from a team as a code owner June 3, 2020 00:41
@joestringer
Copy link
Copy Markdown
Member

test-me-please

@joestringer joestringer added release-note/misc This PR makes changes that have no direct user impact. needs-backport/1.8 and removed dont-merge/needs-release-note labels Jun 3, 2020
Copy link
Copy Markdown
Member

@joestringer joestringer left a comment

Choose a reason for hiding this comment

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

Minor spelling nit but otherwise LGTM.

I suspect we might be able to hide the POLICY_AUDIT_MODE check inside the policy_can_xxx() functions by passing in &audited but it's probably a little more explicit the current way.

Comment thread pkg/monitor/datapath_policy.go Outdated
Comment thread bpf/bpf_host.c Outdated
The audit mode may overwrite policy related verdict and in this
situation it's unclear from the policy verdict log that this
happened. This commit adds a new audit bit to the policy log's struct
and adds a new policy log action 'audit' based on that flag. Related
policy log calls were updated to contain audit flag.

Signed-off-by: Arthur Evstifeev <aevstifeev@gitlab.com>
@ap4y ap4y force-pushed the policy-verdict-log-audit branch from dcf892b to 0f8abe0 Compare June 3, 2020 22:59
@joestringer
Copy link
Copy Markdown
Member

joestringer commented Jun 3, 2020

@joestringer joestringer requested a review from pchaigno June 4, 2020 01:25
@joestringer
Copy link
Copy Markdown
Member

retest-runtime

@joestringer
Copy link
Copy Markdown
Member

retest-net-next

@joestringer
Copy link
Copy Markdown
Member

retest-4.9

Copy link
Copy Markdown
Member

@nebril nebril left a comment

Choose a reason for hiding this comment

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

CI bits LGTM

@maintainer-s-little-helper maintainer-s-little-helper Bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jun 4, 2020
@aanm aanm merged commit b8f78b3 into cilium:master Jun 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Policy audit mode represents policy deny verdicts as "action allow match none"

8 participants