Skip to content

Conversation

@sanathkumarbs
Copy link

@sanathkumarbs sanathkumarbs commented Nov 3, 2022

Proposed changes

Describe the use case and detail of the change. If this PR addresses an issue on GitHub, make sure to include a link to that issue using one of the supported keywords here in this description (not in the title of the PR).

Fixes/Updates:

  • Parse XML only if we have any XML data
  • Fix log levels for NGINX App Protect Monitoring extension
  • Add more debug logs across tests and extension
  • Fix the log component name for the NGINX App Protect Monitoring extension to nginx-app-protect-monitoring instead of security-events-manager.
  • Removes support for the SeverityLabel, GeoIP, Priority fields in SecurityViolationEvent Proto
  • Use DateTime generated in Metadata as the source for the timestamp
    • Removes use of date_time in logging fields
    • Removes the DateTime field from SecurityViolationEvent Proto
  • Renames the following to be consistent with database column naming on control plane and less confusing:
    • SourceHost -> ServerAddr
    • SourcePort -> ServerPort
    • IPClient -> RemoteAddr
    • DestinationPort -> RemotePort
  • Set the ServerAddr to be the Hostname of the machine, we might want to update this in the future based on customer feedback
  • Updates tests at all the places necessary
  • Use Blocked as a value when the response code is 0 for blocked requests to be more user-friendly
  • Use Violation XML as the source of truth; remove the need of parsing sig_names and sig_ids
  • Remove the use of SigIDs and SigNames from the Proto message and Log Format

Checklist

Before creating a PR, run through this checklist and mark each as complete.

  • I have read the CONTRIBUTING document
  • If applicable, I have added tests that prove my fix is effective or that my feature works
  • If applicable, I have checked that any relevant tests pass after adding my changes
  • I have updated any relevant documentation (README.md)

@github-actions github-actions bot added bug Something isn't working dependencies labels Nov 3, 2022
Copy link

@mohamed-gougam mohamed-gougam left a comment

Choose a reason for hiding this comment

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

I will add changes to this covering b64 encoding for request and URI. @sanathkumarbs is this ok for you?

@sanathkumarbs sanathkumarbs changed the title Draft: fix: fixes various items for security monitor fix: polish and fix various items for security monitor Nov 9, 2022
@sanathkumarbs sanathkumarbs self-assigned this Nov 10, 2022
@sanathkumarbs sanathkumarbs merged commit 4c2a3c3 into main Nov 10, 2022
@oliveromahony oliveromahony deleted the sec-mon-fixes branch November 21, 2022 14:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working dependencies

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants