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

New class for OS patching using kb_articles object #746

Merged
merged 5 commits into from
Nov 21, 2023

Conversation

jasonbreimer
Copy link
Contributor

Related Issue:

Resulting from Vulnerability Findings work.

Description of changes:

Add a new class to Discovery for the install state of an OS patch. This new class includes the Host profile for Device as well as the kb_articles object.

Please comment on any additions/changes.

@jasonbreimer jasonbreimer added enhancement New feature or request discovery Issues related to Discovery Category labels Aug 29, 2023
@rroupski rroupski added the non_breaking Non Breaking, backwards compatible changes label Aug 30, 2023
Copy link
Contributor

@rroupski rroupski left a comment

Choose a reason for hiding this comment

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

The class does not define new activity_id values. Just to check, are the default activity_id values, as shown below, good for this class?

    "activity_id": {
      "enum": {
        "1": {
          "caption": "Log",
          "description": "The discovered information is via a log."
        },
        "2": {
          "caption": "Collect",
          "description": "The discovered information is via a collection process."
        }
      }
    }

@jp-harvey
Copy link
Contributor

@jasonbreimer would this be Windows specific?

@pagbabian-splunk
Copy link
Contributor

I'm assuming that the primary attributes come from the Operating System object within Device: sp_name, sp_ver, version. These are all optional attributes. @rroupski is there a way for the enclosing class to add a constraint to the inner attributes? e.g.
"constraints": { "at_least_one": { "device.os.sp_name", "device.os.sp_ver", "device.os.version" } }

@rroupski
Copy link
Contributor

@rroupski is there a way for the enclosing class to add a constraint to the inner attributes? e.g.
"constraints": { "at_least_one": { "device.os.sp_name", "device.os.sp_ver", "device.os.version" } }

Yes, we could add this constrain.

@jasonbreimer
Copy link
Contributor Author

The class does not define new activity_id values. Just to check, are the default activity_id values, as shown below, good for this class?

    "activity_id": {
      "enum": {
        "1": {
          "caption": "Log",
          "description": "The discovered information is via a log."
        },
        "2": {
          "caption": "Collect",
          "description": "The discovered information is via a collection process."
        }
      }
    }

Yes, like other data in this category we go out and query/poll this data. I think "Collect" is fine.

@jasonbreimer
Copy link
Contributor Author

@jasonbreimer would this be Windows specific?

Hey JP, picking up on this one again.... no this would be OS agnostic. I modeled it on data from both linux distros and windows. There are attributes that belong to windows. I don't think anybody else use "bulletin". However I think that's the only one. The other attributes are generic to OS.

@jasonbreimer
Copy link
Contributor Author

device.os.version

No problem to add constraints. I followed guidance and I think this is the syntax/structure:

"constraints": {
"at_least_one": [
"sp_name",
"sp_ver",
"version"
]
}
}

@jp-harvey
Copy link
Contributor

Hey JP, picking up on this one again.... no this would be OS agnostic. I modeled it on data from both linux distros and windows. There are attributes that belong to windows. I don't think anybody else use "bulletin". However I think that's the only one. The other attributes are generic to OS.

No worries, do you think the kb_article sounds very Windows-centric? Is there a more generic name we can use?

@jasonbreimer
Copy link
Contributor Author

Hey JP, picking up on this one again.... no this would be OS agnostic. I modeled it on data from both linux distros and windows. There are attributes that belong to windows. I don't think anybody else use "bulletin". However I think that's the only one. The other attributes are generic to OS.

No worries, do you think the kb_article sounds very Windows-centric? Is there a more generic name we can use?

Hello JP,
That kb_article name is an existing object, there's also a dictionary item which is an array of kb_article. The term "knowledge base" seems to be an ok-ish term from our discussions. I think we dug into this name while reworking Findings. I recall we talked about other terms but none really matched all the vendors perfectly. Like Canonical is weird. While it certainly has windows baggage it could take a CESA, RHBA, ALSA type values. I use this term in our product and we are able to populate across all the vendors.
I would agree its not perfect, but it seems to be a generally acceptable term and there really isn't anything better. Happy to discuss other terminology if you have any ideas though.

Based on PR discussion, add constraints for device.

Signed-off-by: Jason Reimer <jason.reimer@tanium.com>
Signed-off-by: Jason Reimer <jason.reimer@tanium.com>
@jasonbreimer
Copy link
Contributor Author

I'm assuming that the primary attributes come from the Operating System object within Device: sp_name, sp_ver, version. These are all optional attributes. @rroupski is there a way for the enclosing class to add a constraint to the inner attributes? e.g. "constraints": { "at_least_one": { "device.os.sp_name", "device.os.sp_ver", "device.os.version" } }

Hey @pagbabian-splunk, added the constraints you and Roumen referenced! Does the cover the change you would like? Let me know and I will proceed with converting this from a draft PR. Thank you!

@mikeradka
Copy link
Contributor

This is great @jasonbreimer
Just to capture something we talked about in the 11/9/23 findings call - is there a potential to capture OS patching state within the Device Config State class in order to mitigate class bloat/overlap? The rationale was that part of a device's configuration state is the state of its patches.

I could ultimately either way, just wanted to track the idea we discussed today in case you wanted to look a little deeper.

@pagbabian-splunk
Copy link
Contributor

I'm assuming that the primary attributes come from the Operating System object within Device: sp_name, sp_ver, version. These are all optional attributes. @rroupski is there a way for the enclosing class to add a constraint to the inner attributes? e.g. "constraints": { "at_least_one": { "device.os.sp_name", "device.os.sp_ver", "device.os.version" } }

Hey @pagbabian-splunk, added the constraints you and Roumen referenced! Does the cover the change you would like? Let me know and I will proceed with converting this from a draft PR. Thank you!

Yes it does Jason. However if only device.os.version is populated, would we know what the actual service pack associated with the patch is? Likely at least for Windows. However if only the device.os.sp_name is populated, we might not. Do we actually want them all populated?

Nevertheless, I think with the kb_article and the OS version and patch, it would cover things.

@mikeradka mikeradka self-requested a review November 14, 2023 18:14
@jasonbreimer jasonbreimer marked this pull request as ready for review November 19, 2023 23:15
@floydtree floydtree merged commit 64d994f into ocsf:main Nov 21, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discovery Issues related to Discovery Category enhancement New feature or request non_breaking Non Breaking, backwards compatible changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants