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

Support L7 Network Policy Logging #4625

Merged
merged 1 commit into from Mar 17, 2023
Merged

Support L7 Network Policy Logging #4625

merged 1 commit into from Mar 17, 2023

Conversation

qiyueyao
Copy link
Contributor

@qiyueyao qiyueyao commented Feb 14, 2023

Fix inaccurate Antrea Native Policy logging, where the L7NP will be logged as Redirect not Allow. Update UT to cover more cases.

Add event log file for Suricata at /var/log/antrea/networkpolicy/l7engine. Due to the limitation of available Suricata options, currently eve.log logs all the alerts (reject&pass) regardless of enableLogging, event_type: "alert". Meanwhile packets are only logged if enableLogging: true, "event_type: "packet".

@codecov
Copy link

codecov bot commented Feb 14, 2023

Codecov Report

Merging #4625 (f09f658) into main (bd683b0) will decrease coverage by 1.21%.
The diff coverage is 68.88%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #4625      +/-   ##
==========================================
- Coverage   68.39%   67.19%   -1.21%     
==========================================
  Files         400      418      +18     
  Lines       58298    62823    +4525     
==========================================
+ Hits        39872    42212    +2340     
- Misses      15656    17679    +2023     
- Partials     2770     2932     +162     
Flag Coverage Δ *Carryforward flag
e2e-tests 38.34% <ø> (+<0.01%) ⬆️ Carriedforward from bd683b0
integration-tests 34.46% <0.00%> (-0.09%) ⬇️
kind-e2e-tests 47.16% <68.88%> (+0.09%) ⬆️
unit-tests 57.92% <ø> (+<0.01%) ⬆️ Carriedforward from bd683b0

*This pull request uses carry forward flags. Click here to find out more.

Impacted Files Coverage Δ
...ntroller/networkpolicy/networkpolicy_controller.go 73.14% <0.00%> (+0.95%) ⬆️
pkg/agent/openflow/pipeline.go 89.95% <0.00%> (+0.06%) ⬆️
...kg/agent/controller/networkpolicy/audit_logging.go 83.09% <33.33%> (-3.37%) ⬇️
...nt/controller/networkpolicy/l7engine/reconciler.go 71.21% <87.50%> (+0.72%) ⬆️
pkg/agent/config/node_config.go 78.72% <0.00%> (-17.28%) ⬇️
pkg/agent/multicluster/mc_route_controller.go 45.58% <0.00%> (-9.98%) ⬇️
pkg/agent/proxy/topology.go 72.72% <0.00%> (-9.10%) ⬇️
...lticluster/commonarea/resourceimport_controller.go 71.94% <0.00%> (-8.28%) ⬇️
...icluster/controllers/multicluster/common/helper.go 85.05% <0.00%> (-7.93%) ⬇️
...llers/multicluster/member_clusterset_controller.go 70.38% <0.00%> (-6.87%) ⬇️
... and 55 more

@qiyueyao qiyueyao marked this pull request as ready for review February 23, 2023 18:26
@vicky-liu vicky-liu added this to the Antrea v1.11 release milestone Mar 2, 2023
pkg/agent/controller/networkpolicy/audit_logging_test.go Outdated Show resolved Hide resolved
rulesData := bytes.NewBuffer(nil)
sid := 1

var tagKeyword string
if enableLogging {
tagKeyword = " tag: session, 30, seconds;"
Copy link
Member

Choose a reason for hiding this comment

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

I didn't find doc about these keywords, could you share a link and add comment to help understand?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Suricata lacks the documentation for "tag", which is an open ticket. Looking into the codebase, it's handled in their file detect-engine-tag. Also Snort has a similar functionality, doc.

Copy link
Member

Choose a reason for hiding this comment

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

could you add this information to code comment? otherwise people who looks at this will have the same question.

pkg/agent/controller/networkpolicy/l7engine/reconciler.go Outdated Show resolved Hide resolved
@qiyueyao qiyueyao force-pushed the l7-logging branch 3 times, most recently from 09a9fc0 to 5c70bd2 Compare March 9, 2023 14:17
@qiyueyao qiyueyao force-pushed the l7-logging branch 3 times, most recently from 4fe41a6 to a9dc3f7 Compare March 13, 2023 22:42
Copy link
Contributor

@hongliangl hongliangl left a comment

Choose a reason for hiding this comment

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

Not finished yet, will add other comments later.

docs/antrea-l7-network-policy.md Outdated Show resolved Hide resolved
docs/antrea-l7-network-policy.md Outdated Show resolved Hide resolved
pkg/agent/openflow/pipeline.go Outdated Show resolved Hide resolved
pkg/agent/openflow/fields.go Outdated Show resolved Hide resolved
pkg/agent/controller/networkpolicy/audit_logging.go Outdated Show resolved Hide resolved
pkg/agent/controller/networkpolicy/audit_logging.go Outdated Show resolved Hide resolved
pkg/agent/openflow/fields.go Outdated Show resolved Hide resolved
pkg/agent/controller/networkpolicy/audit_logging.go Outdated Show resolved Hide resolved

Layer 7 traffic that matches the NetworkPolicy will be logged in an event
triggered log file (`/var/log/antrea/networkpolicy/l7engine/eve-YEAR-MONTH-DAY.json`).
The event type for this log is `alert`. If `enableLogging` is set for the rule,
Copy link
Member

Choose a reason for hiding this comment

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

You mean the dropped request will be logged regardless of whether enableLogging is enabled? Is it configurable?

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 will be logged regardless of enableLogging. Based on my investigation of Suricata doc, there is no workaround to configure this. It is either log all events or no log at all, only the packets can be configured with enableLogging, but they can only be logged in the same file. I also asked in the Suricata community, this doesn't seem a high priority feature request, since we have postprocess tooling.

rulesData := bytes.NewBuffer(nil)
sid := 1

var tagKeyword string
if enableLogging {
tagKeyword = " tag: session, 30, seconds;"
Copy link
Member

Choose a reason for hiding this comment

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

could you add this information to code comment? otherwise people who looks at this will have the same question.

Copy link
Contributor

@hongliangl hongliangl left a comment

Choose a reason for hiding this comment

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

LGTM overall, just some small suggestions.

pkg/agent/openflow/pipeline.go Outdated Show resolved Hide resolved
pkg/agent/controller/networkpolicy/audit_logging.go Outdated Show resolved Hide resolved
Antrea-native policy now supports layer 7 NetworkPolicy. To provide
more information for users, logging for this feature is introduced.

Antrea-native policy is not accurate enough in reporting packet status
before sending to l7 engine. Logs are fixed to reflect "Redirect" action.
Audit logging UT are updated to cover more cases.

L7 engine provides its own logs. Currently, Suricata is used as L7 engine.
Configuration is updated to generate two log files, fast.log and eve.json
Both files locates at /var/log/antrea/networkpolicy/. Documentation is updated.

Signed-off-by: Qiyue Yao <yaoq@vmware.com>
Copy link
Member

@tnqn tnqn left a comment

Choose a reason for hiding this comment

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

LGTM

@tnqn
Copy link
Member

tnqn commented Mar 17, 2023

/test-all

1 similar comment
@tnqn
Copy link
Member

tnqn commented Mar 17, 2023

/test-all

@tnqn tnqn merged commit 2ee4e3d into antrea-io:main Mar 17, 2023
@qiyueyao qiyueyao deleted the l7-logging branch March 21, 2023 05:10
jainpulkit22 pushed a commit to urharshitha/antrea that referenced this pull request Apr 28, 2023
Antrea-native policy now supports layer 7 NetworkPolicy. To provide
more information for users, logging for this feature is introduced.

Antrea-native policy is not accurate enough in reporting packet status
before sending to l7 engine. Logs are fixed to reflect "Redirect" action.
Audit logging UT are updated to cover more cases.

L7 engine provides its own logs. Currently, Suricata is used as L7 engine.
Configuration is updated to generate two log files, fast.log and eve.json
Both files locates at /var/log/antrea/networkpolicy/. Documentation is updated.

Signed-off-by: Qiyue Yao <yaoq@vmware.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants