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

Added the security_control profile entries. #894

Closed
wants to merge 3 commits into from

Conversation

pagbabian-splunk
Copy link
Contributor

Related Issue:

Description of changes:

Added the security_control profile to the class.

@pagbabian-splunk pagbabian-splunk added enhancement New feature or request findings Issues related to Findings Category non_breaking Non Breaking, backwards compatible changes labels Dec 18, 2023
Copy link
Contributor

@floydtree floydtree left a comment

Choose a reason for hiding this comment

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

The only issue I see here is, attacks being added at multiple levels in the class. Attacks is already baked in all the findings classes via finding_info.attacks. Which attacks should one populate?

@pagbabian-splunk
Copy link
Contributor Author

I suggest we move attacks from the finding_info object into the main class, as attacks should remain in security_control and it should be more prominent in the event. malware is often detected along with attacks and without the security_control, the detection_finding class is missing malware which I would guess will be more than half of the detections.

@floydtree
Copy link
Contributor

malware is indeed a part of detection finding class. You might have missed it due to filters or something similar.

attacks was added to finding_info to allow for an array of finding_Info to be used in the Incidents class which can represent more than 1 finding. To keep the correlation of the attacks and the finding in question, tying them up together is essential.

@pagbabian-splunk
Copy link
Contributor Author

Yes, no problem with malware: I only bring it up to say that attacks should be at the same level as malware, i.e. at the class level, and not in finding_info. That way, the profile can overlay without redundancy.

@pagbabian-splunk
Copy link
Contributor Author

pagbabian-splunk commented Dec 19, 2023

There is an additional problem I discovered: because detection_finding builds in malware in the class, it becomes suppressed when the $include for security_control.json is added. With the System classes that build in the Host profile, all of the Host attributes are natively in the class, and the profile declaration is there for query consistency.

Not so with detection_finding and security_control: the entire profile is not natively in the class, hence when the profile is added, malware disappears until the profile is selected.

Therefore, we need to decide whether any of the other profile attributes (e.g. action_id or disposition_id are needed) otherwise the class cannot accommodate the profile. If we don't include the profile, we can of course still add some of the profile's attributes, but the class would not be a security control. If we do want the profile's attributes, then malware (and attacks within finding_info IMO) should not be natively in the class, but delegated to the profile.

@floydtree
Copy link
Contributor

Yes, no problem with malware: I only bring it up to say that attacks should be at the same level as malware, i.e. at the class level, and not in finding_info. That way, the profile can overlay without redundancy.

Right, but as I mentioned earlier - attacks was added to finding_info to allow for an array of finding_Info to be used in the Incidents class to help represent more than 1 finding. To keep the correlation of the attacks and the finding in question, tying them up together is essential.

There is an additional problem I discovered: because detection_finding builds in malware in the class, it becomes suppressed when the $include for security_control.json is added. With the System classes that build in the Host profile, all of the Host attributes are natively in the class, and the profile declaration is there for query consistency.
Not so with detection_finding and security_control: the entire profile is not natively in the class, hence when the profile is added, malware disappears until the profile is selected.

There's way to overcome this. If you add "profile": null as an extra key while adding an attribute to a class definition, you'll get the desired behavior.

Therefore, we need to decide whether any of the other profile attributes (e.g. action_id or disposition_id are needed) otherwise the class cannot accommodate the profile. If we don't include the profile, we can of course still add some of the profile's attributes, but the class would not be a security control. If we do want the profile's attributes, then malware (and attacks within finding_info IMO) should not be natively in the class, but delegated to the profile.

Attacks details can, and are indeed present in a finding even if the finding is not referring to a malware detection. So tying malware and attacks together doesn't help the situation. attacks should exist without malware, and I explained why attacks is in finding_info above.

I think we either have to accept the redundancy for attacks (perhaps with a better description in the sec control profile) or not have sec control profile in the detections class. I would most definitely vote for having the said redundancy.

@pagbabian-splunk
Copy link
Contributor Author

finding_Info to be used in the Incidents class to help represent more than 1 finding. To keep the correlation of the attacks and the finding in question, tying them up together is essential.

I see - I don't have visibility into how the findings group envisioned the transition to Incident, but now I understand that finding_info is the vehicle for grouping findings within an incident. However, I would then want malware to also be in the finding_info object. I understand than ATT&CK can exist without malware being found, but when malware is found in a finding, the same would hold true for having the information directly in the incident.

…rity_control suppressing the built-in attribute.

Signed-off-by: Paul Agbabian <[email protected]>
@pagbabian-splunk
Copy link
Contributor Author

The only issue I see here is, attacks being added at multiple levels in the class. Attacks is already baked in all the findings classes via finding_info.attacks. Which attacks should one populate?

I will create a new PR that moves malware into finding_info for consistency, (per this week's call), and add description information about how to populate. E.g. Incidents will also benefit from having malware in the finding_info array, and while compliance_finding and vulnerability_finding don't care about malware, they also don't care about analytics or kill_chain so the object is more utilitarian than precise across the finding classes.

@pagbabian-splunk pagbabian-splunk added the v1.1.0 Changes marked for v1.1.0 of OCSF label Dec 20, 2023
@floydtree
Copy link
Contributor

I will create a new PR that moves malware into finding_info for consistency, (per this week's call), and add description information about how to populate. E.g. Incidents will also benefit from having malware in the finding_info array, and while compliance_finding and vulnerability_finding don't care about malware, they also don't care about analytics or kill_chain so the object is more utilitarian than precise across the finding classes.

On second thought, I see a concern with this approach - if we add malware to finding_info object then why not other "crucial" attributes in the detection finding class? Why not evidences, vulnerability etc? I think adding malware to finding_info leads to an anti-pattern when compared to the rest of the class. Also, what are we to benefit by moving malware from base to the finding_info?

Further, do we want to have even more redundancy in the class when the security_control profile is applied to the class? (only attacks vs both malware, attacks being duplicated in finding_info)?

@pagbabian-splunk
Copy link
Contributor Author

True but we already have Attacks there in finding_info so that an array of findings can be held by an incident and attacks available without referencing to the finding events. Why wouldn't I want to see the malware from those findings as well. On the contrary for me this would be more rather than less consistent. Found malware is as important as MITRE tags.

@floydtree
Copy link
Contributor

floydtree commented Dec 20, 2023

Why wouldn't I want to see the malware from those findings as well

As noted earlier, this holds true for other attributes as well.

@pagbabian-splunk
Copy link
Contributor Author

pagbabian-splunk commented Dec 20, 2023

Yes, I think we should do an analysis and make sure we have the right rationale for where things sit. Given the reuse of the same finding_info in Incident (planned), this case is a bit unique. E.g. we generally have the rating attributes such as risk_level and impact_score in the class, while we have frameworks like kill_chain and attacks in finding_info.

We have evidences in the class but related_events in finding_info. There is an argument that outside information (evidences) and outside references (related_events) are separated. Would I want only one when looking at the Incident?

Given the sharing of finding_info with vulnerability_finding we have vulnerability_details in both classes, but kill_chain which isn't likely populated for vulnerabilities in the shared finding_info.

I agree we don't want everything in the object, but malware is different than a rating like impact_score. Also, the security_control profile pairs the attacks with the malware and we would be splitting them if we didn't keep them paired in both places (i.e. your original point - we'd be adding attacks in both placesbutmalware` only in the class).

Therefore, I would either move attacks to the class and not have duplication when the profile is applied, or keep them paired in finding_info and document that the class level should be the attributes that are populated, so that a query against the profile consistently returns attacks and/or malware and duplicated in finding_info so it can be easily referenced in Incident.

One more point on this: we have two perspectives here: one from the security control detection, where a verdict of some kind (disposition here) but also action was done. The other is the analyst (SIEM let's say) that is NOT a control, does NOT Allow or Deny the action. Hence, when detection_finding is sent from the product to the SIEM, it likely will impose the control. When a SIEM analysis evaluates activities to produce a finding, it does not Allow or Deny as an action and would not impose the control.

I bring this in for the rationale that we need dispositions, or verdicts to use UDM terminology, without having to apply the security_control profile as the result of a SIEM or other analysis.

@pagbabian-splunk
Copy link
Contributor Author

Closing this PR in favor of #906

@pagbabian-splunk
Copy link
Contributor Author

Closing this PR in favor of #906

@floydtree floydtree deleted the ctrl_findings branch January 4, 2024 17:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

2 participants