-
Notifications
You must be signed in to change notification settings - Fork 106
Security monitoring feature parity with Agent V2 #1463
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
base: main
Are you sure you want to change the base?
Security monitoring feature parity with Agent V2 #1463
Conversation
|
✅ All required contributors have signed the F5 CLA for this PR. Thank you! |
b3af22a to
5271a0f
Compare
…asic struct.
This has been done to allow for management-plane can reference it as a contract with backward/forward compatibility
… the logic in line with agent v2
… adds additional assertions and validations to ensure of expected final output
cbf8c1d to
36b2d35
Compare
| if i >= len(fieldOrder) { | ||
| break | ||
| } | ||
| fieldValueMap[fieldOrder[i]] = strings.TrimSpace(field) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we prebuild this map? If we expect to be high-volume, rebuilding the map log parse is probably not needed?
|
|
||
| parts := strings.Split(value, ",") | ||
|
|
||
| var trimmedParts []string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit, you can preallocate with length
trimmedParts := make([]string, 0, len(parts))
| // Remove the "ASM:" prefix if present so we only process the values | ||
| message = strings.TrimPrefix(message, "ASM:") | ||
|
|
||
| fields := strings.Split(message, ",") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
did we explore the golang csv parser? https://pkg.go.dev/encoding/csv
0f31873 to
e08cb5d
Compare
|
Hi Kunal, thanks for the PR! My only comment is that this implementation converts App Protect security violations into a custom/proprietary format that essentially uses OTel as a pipe. It does the job, no doubt. Taking a step back, one of the key reasons we have OTel in NGINX Agent V3 is to enable customers to view their own telemetry. The goal is to able users to customize Agent's OTel collector and send telemetry to their own, perhaps private collectors. Or they have a plan with Datadog or some cloud provider. If the security violations don't follow the OTel standard for log processing, then it makes it that much harder (if not impossible) for the customer to view the security violations on their own systems. This is a key feature of Agent V3 and something we strive to maintain. |
Proposed changes
Purpose of this PR is to over the gaps between the current custom
securityviolationsprocessorin agent V3, with what level of parsing and transformations we had in agent V2 security-monitoring processor.Additionally, a protobuf schema is defined to serve as contract between the agent and the downstream consumer.
Commit 1:
The
securityviolationsprocessornow processes NGINX App Protect WAF syslog messages, and transforms them intoSecurityViolationEventprotobuf messages. This protobuf definition replaces the existing struct definition in/internalfolder. This was done to allow management-plane to import this schema as a contract for handling security violations.Commit 2:
Additionally, added the following capabilities to the parsing the details extraction from raw violations, to bring the feature in parity with Agent V2 implementation:
Commit 3:
These changes were thoroughly tested with addition of
/testdatain Agent V2 implementation (https://github.com/nginx/agent/tree/dev-v2/src/extensions/nginx-app-protect/monitoring/processor/testdata) and additional variety of violations, to ensure robust coverage.Commit 4:
Added NAP V5 support: Lookup for docker0 interface IP, which is then utilized to validate for configured syslog IP in NGINX Config.
Commit 5:
Added architecture diagram to describe the custom securityviolations processor.
Agent Config Modifications for Test
Testing
Violations Triggered
Expected Violations: VIOL_FILETYPE, VIOL_HTTP_PROTOCOL, VIOL_BOT_CLIENT
Output Payload:
Expected Violations: VIOL_METHOD, VIOL_HTTP_PROTOCOL, VIOL_BOT_CLIENT
Output Payload:
Expected Violations: VIOL_ATTACK_SIGNATURE, VIOL_HTTP_PROTOCOL, VIOL_BOT_CLIENT, VIOL_URL_METACHAR, VIOL_RATING_THREAT
Expected Signature IDs ": 200000099, 200000093
Output Payload:
Checklist
Before creating a PR, run through this checklist and mark each as complete.
CONTRIBUTINGdocumentmake install-toolsand have attached any dependency changes to this pull requestREADME.md)