-
Notifications
You must be signed in to change notification settings - Fork 130
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 reg_key and reg_value attributes to Evidence Artifacts object. #1078
Conversation
Just an off-the-cuff thought here - I admittedly haven't spun this up yet on a server to play around with. But it may be possible to have the Windows Extension simply append the |
If you can figure out how to do that, please let me know because I can't. I tried something like that first and got verification errors, though I might have been doing something wrong. I then looked at how the problem was solved for other extension attributes, saw the example from the Linux extension, and figured it made sense - for consistency if nothing else - to do it the same way. |
I don't mind checking into it from my end a little. I think our Splunk extension accomplishes that somewhere within its schema definitions. Will get back to you on it once I get a chance to dig in! |
Hi @davemcatcisco , I had a chance to look into this, and here is one way I was able to make this work by altering the Windows extension rather than the core schema:
{
"caption": "Evidence Artifacts",
"description": "A collection of evidence artifacts associated to the activity/activities that triggered a security detection.",
"extends": "evidences",
"attributes": {
"reg_key": {
"requirement": "recommended"
},
"reg_value": {
"requirement": "recommended"
}
}
}
|
Hi @mikeradka. Thanks for looking at this. I'm a bit confused though. My proposal also just changes the Windows extension and doesn't touch the core schema. The only differences I can see in what you did and what I did are that (a) I've also included {
"caption": "Windows Evidence Artifacts",
"description": "Extends the evidences object to add Windows specific fields",
"extends": "evidences",
"attributes": {
"reg_key": {
"description": "Describes details about the registry key that triggered the detection.",
"requirement": "recommended"
},
"reg_value": {
"description": "Describes details about the registry value that triggered the detection.",
"requirement": "recommended"
}
},
"constraints": {
"at_least_one": [
"reg_key",
"reg_value"
]
}
} |
Ah, I see @davemcatcisco . For some reason I had an impression the Core object was modified. I think what you have is okay, with one follow up question - are those two constraints added to the existing core constraints, or do they replace them? I would imagine they are simply added as two additional constraints. If you can confirm that is the case, then this looks good to me. |
This is interesting. When I spin up ocsf-server with my modified schema, I see the following. It has correctly picked up the two new attributes but it is continuing to use the |
I am glad you saw this, @davemcatcisco! I witnessed the same yesterday but didn't get the chance just yet to post about it. I showed @rmouritzen-splunk, in a sync yesterday. Schema-wise, I think your changes look good, but we should probably get this addressed on the server end. I also noticed that the caption didn't change, but perhaps that is by design? |
@mikeradka - Are you referring to the caption in the first column of the table in my screenshot, i.e. "Registry Key" and "Registry Value"? That is how I think it should be because the pre-existing attributes there just say "File", "Process", etc. |
No, I was referring to this one:
Notice that it doesn't prepend I think that is okay - I wouldn't want the entire |
Signed-off-by: Dave McCormack <[email protected]>
Signed-off-by: Dave McCormack <[email protected]>
@mikeradka , @floydtree , @Aniak5 , @mikeradka , @zschmerber - I've just updated this as discussed on the weekly call 10 mins ago, i.e. it is now exactly as requested. I'd appreciate your approval and merge if possible. Thanks. |
Approved. For the long term, we should address the constraints overlap. For tracking the constraints behavior and validation, i've added this PR as a reference to @rmouritzen-splunk 's server issue regarding constraints, here: ocsf/ocsf-server#91 |
Thanks, all! |
#### Related Issue: [#1124](#1124) #### Description of changes: - Added the pre-existing `job` attribute to the `Evidence Artifacts` object. - Adjusted the `at_least_one` constraint in the object to include `job`. Note that this approach is the same as that taken to fix other gaps in the `Evidence Artifacts` object, e.g. PR #1078. Signed-off-by: Dave McCormack <[email protected]> Co-authored-by: Rajas <[email protected]>
Related Issue:
#1077
Description of changes:
The requirement simply stated in the issue linked above was to add the pre-existing
reg_key
andreg_value
attributes to theEvidence Artifacts
object.However, because these attributes are Windows extension attributes rather than core attributes, it is necessary to extend the
Evidence Artifacts
object asWindows Evidence Artifacts
and then add the attributes to this object.Note that this approach follows the pattern that was previously used to add Linux-specific attributes to the
Process
object.