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

The new Detection Finding event class #877

Merged
merged 41 commits into from
Dec 7, 2023

Conversation

floydtree
Copy link
Contributor

@floydtree floydtree commented Dec 1, 2023

Related Issue: #789

This class has been created based on a series of discussions in the Findings group. Relevant info can be found in the discussion linked above.

Description of changes:

  1. Adding a new event class detection_finding
  2. Adding a new object evidences to account for various evidence artifacts presented in a detection/alert type finding.
  3. Updating description of the base finding class to make attributes specific to the findings category.

Follow-up:

  1. Add Security Control profile once ready - Update the security_control profile to include access control check semantics, and firewall profile semantics. #851
  2. Add elaborate descriptions to impact, risk, confidence family of attributes and their enumerations. (fyi @irakledibm @zschmerber)

floydtree and others added 30 commits November 8, 2023 16:28
Signed-off-by: Rajas <rajaspa@amazon.com>
Signed-off-by: Rajas <rajaspa@amazon.com>
Signed-off-by: Rajas <rajaspa@amazon.com>
Signed-off-by: Rajas <rajaspa@amazon.com>
Signed-off-by: Rajas <rajaspa@amazon.com>
Signed-off-by: Michael Radka <mradka@splunk.com>
Signed-off-by: Michael Radka <mradka@splunk.com>
Signed-off-by: Michael Radka <mradka@splunk.com>
Signed-off-by: Michael Radka <mradka@splunk.com>
Signed-off-by: Michael Radka <mradka@splunk.com>
Signed-off-by: Rajas <rajaspa@amazon.com>
Signed-off-by: Rajas <rajaspa@amazon.com>
Signed-off-by: Rajas <rajaspa@amazon.com>
Signed-off-by: Rajas <rajaspa@amazon.com>
Signed-off-by: Rajas <rajaspa@amazon.com>
Signed-off-by: Rajas <rajaspa@amazon.com>
Signed-off-by: Rajas <rajaspa@amazon.com>
Signed-off-by: Rajas <rajaspa@amazon.com>
Signed-off-by: Rajas <rajaspa@amazon.com>
Signed-off-by: Rajas <rajaspa@amazon.com>
Signed-off-by: Rajas <rajaspa@amazon.com>
Signed-off-by: Rajas <rajaspa@amazon.com>
@floydtree floydtree added non_breaking Non Breaking, backwards compatible changes description_updates Issues related to missing/incorrect/lacking descriptions of attributes labels Dec 1, 2023
@floydtree floydtree self-assigned this Dec 1, 2023
Copy link
Contributor

@irakledibm irakledibm left a comment

Choose a reason for hiding this comment

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

looks great

events/findings/detection_finding.json Outdated Show resolved Hide resolved
events/findings/detection_finding.json Show resolved Hide resolved
Signed-off-by: Rajas <rajaspa@amazon.com>
Copy link
Contributor

@zschmerber zschmerber left a comment

Choose a reason for hiding this comment

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

looks good and worked in my test.

@pagbabian-splunk
Copy link
Contributor

Do we intend for detection_finding to be a set of lifecycle events? If so, would all of the other attributes be carried with each stage, e.g. New -> In Progress -> Resolved? Carrying all of it might be tricky for the event producer if the attributes are not available at each step in a stateless system. Alternatively, the finding can be aggregated into an Incident class (draft proposed), which refers to the finding during its lifecycle events, rather than replicates all of the attributes at each step.

That said, there is value in having a lifecycle due to triage of a finding before it becomes an incident, e.g. Suppressed, or Resolved as a False Positive.

Lastly, there is a case to be made for a Benign Positive - that is, it is an expected finding due to some IT process going on, etc. It could simply be another detail of Resolved, except that the description of Resolved says it was remediated, which indicates that something was needed to be done.

@zschmerber
Copy link
Contributor

I am of the opinion that detection, vulnerability and compliance findings are to be immutable in the Activity ID they display. If there is a change upstream a second log displaying a new Activity ID with new timestamp can relay that information. Only the Incident finding class should have that mutability option.

@floydtree
Copy link
Contributor Author

floydtree commented Dec 7, 2023

Do we intend for detection_finding to be a set of lifecycle events? If so, would all of the other attributes be carried with each stage, e.g. New -> In Progress -> Resolved? Carrying all of it might be tricky for the event producer if the attributes are not available at each step in a stateless system. Alternatively, the finding can be aggregated into an Incident class (draft proposed), which refers to the finding during its lifecycle events, rather than replicates all of the attributes at each step.
That said, there is value in having a lifecycle due to triage of a finding before it becomes an incident, e.g. Suppressed, or Resolved as a False Positive.

Yes, for the very reason you highlighted, findings need to have lifecycle status that can be assigned during initial triage before it is classified as an incident.

Lastly, there is a case to be made for a Benign Positive - that is, it is an expected finding due to some IT process going on, etc. It could simply be another detail of Resolved, except that the description of Resolved says it was remediated, which indicates that something was needed to be done.

Agreed, I did push a commit to address that.

@pagbabian-splunk
Copy link
Contributor

One more small point: I assume a finding can be the result of analysis of other findings. Not just activities.

In finding_info we have related_events described as: "Describes events related to a finding or detection as identified by the security product."

I think it would be good to call out that the events can be other findings. E.g. the related_analytics could be used to aggregate other findings, say that shared the same MITRE techniques, or shared the same device, IP etc.

This would just be for instruction in docs.

@pagbabian-splunk pagbabian-splunk merged commit c71a190 into ocsf:main Dec 7, 2023
2 checks passed
@floydtree floydtree deleted the detections branch December 7, 2023 20:54
@floydtree floydtree added the v1.1.0 Changes marked for v1.1.0 of OCSF label Jan 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
description_updates Issues related to missing/incorrect/lacking descriptions of attributes enhancement New feature or request findings Issues related to Findings Category non_breaking Non Breaking, backwards compatible changes v1.1.0 Changes marked for v1.1.0 of OCSF
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants