Addressed a few issues with recent Script Activity PR.

[davemcatcisco]

Related Issue:

#1197

Description of changes:

As described in the issue but to recap:

• Added an is_script_truncated is_script_content_truncated flag.
• Added an entry to the type_id enum in the script object.
• Improved descriptions in previous PR as requested at the time.

[irakledibm]

I would suggest to add "is_truncated" instead of "is_script_truncated" to a dictionary. Then this attribute can be used in multiple classes/objects to indicate truncation not only in scripts but also in payloads, logs, command lines etc.

[davemcatcisco]

I would suggest to add "is_truncated" instead of "is_script_truncated" to a dictionary. Then this attribute can be used in multiple classes/objects to indicate truncation not only in scripts but also in payloads, logs, command lines etc.

Thanks for the suggestion. This was also suggested during an internal discussion on this issue. I can see the usefulness of a general purpose attribute like is_truncated, and I actually searched the dictionary for something like it before I started this PR because I assumed it must exist already! However, I feel that the problem with is_truncated is that it can be ambiguous in certain contexts, even in the context where it is used here.

First I should clarify that the attribute I'm adding here is actually is_script_content_truncated and not is_script_truncated as I wrote above. I think this proposed name conveys that the value of the script_content attribute is truncated. If the attribute was named is_truncated as you suggest, I think the most likely (mis)interpretation is that the original script as observed by the security product was truncated.

But maybe I'm overthinking it and I don't have a very strong opinion either way. If other reviewers think that a generic is_truncated makes more sense, I'll happily switch to that. Please let me know here if you agree/disagree with my expanded reasoning.

[floydtree]

This is an interesting topic to discuss. Note that, an attribute called is_attribute_truncated was recently added to the framework. Check #1172 for reference.

However, I think the specificity is quite important for this attribute. As exemplified in this PR's case, what is being truncated is quite important. Also, as evident here, we cannot always rely on the context provided by the corresponding object as it may not be sufficient to help distinguish.

We might risk creating a bunch of is_***_truncated attributes, but if the specificity is important then I think we should. Whichever path we end up choosing, we must be consistent throughout the schema, i.e. we'll need to change is_attribute_truncated to its more specific version if we go the route of specific truncated attributes.

Thoughts? @pagbabian-splunk @davemcatcisco @irakledibm

[irakledibm]

I think that it is a fundamental problem with OCSF. An attributes defined in a dictionary should represent a common generic values, such as is_truncated with associated common properties, such as boolean_t, is_array etc. Event/Object/Profile attributes should point to those dictionary attributes, however they may have an alias, such as is_script_content_truncated. This way a dictionary will not blow out of proportions and still address different use cases.

[pagbabian-splunk]

Agree about aliases. OCSF could easily add the meta schema alias which I’ve always wished for. But an alias for a consumer or a data table is a different column name and if it always collapsed to its root name it would defeat the purpose and lose the specific name. The system consuming would need to do a translation (like Splunk does for its aliases at read time). In the end a particular OCSF event is a static entity. An attribute without its surrounding context will be as specific as its name implies. A consumer would need a more specific predicate that includes the class name or event type. We don't serialize object names in events so object context is lost. Some further thought - this is particularly a problem with `boolean` attributes, since the values are limited and themselve generic. When we have enums that are "See specific usage" the values are specific to the class or object, and at least in a query can be specific in most cases. For `string` types less so but their values can have great variance. We just have been reviewing another generic `boolean`, `is_deleted` which isn't too different from `is_truncated`. We aren't very consistent in other words. The problem is more acute when within objects, as both `is_deleted` (File) and `is_truncated` (Script) would have been. Paul (from my mobile device)

…

On Fri, Oct 11, 2024 at 8:52 AM irakledibm ***@***.***> wrote: I think that it is a fundamental problem with OCSF. An attributes defined in a dictionary should represent a common generic values, such as is_truncated with associated common properties, such as boolean_t, is_array etc. Event/Object/Profile attributes should point to those dictionary attributes, however they may have an alias, such as is_script_content_truncated. This way a dictionary will not blow out of proportions and still address different use cases. — Reply to this email directly, view it on GitHub <#1198 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AS5LBZQARB3BXW3TZMFEH43Z27X4VAVCNFSM6AAAAABPSB4OKOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIMBXGY4TENRYGU> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[pagbabian-splunk]

One thought is to distinguish between classes and objects, such that within objects, the object name is carried, while within classes, the generic name is used (because the event is always of a specific type_id). Don't know what that would mean for the current state of things though.

[pagbabian-splunk]

Did a quick check: almost all of our boolean attributes are within objects, only 6 are in classes. Most of the names are relatively generic: is_renewal (what is renewed?); is_managed (what is managed?); is_personal (what is personal); etc. Some are more easily put into context, e.g. is_mfa.

[davemcatcisco]

Great discussion! But do we have a conclusion? Do I change is_script_content_truncated to a generic is_truncated?

FWIW, I still think that in this case, an is_truncated attribute on a script object is potentially confusing. The reason, as I've mentioned above, is that the flag is intended to denote that the script_content attribute is truncated - not that the underlying script itself was truncated.

[davemcatcisco]

Just to make a more general comment in the spirit of the discussion, was consideration ever given to supporting attributes that are scoped to a specific object (or local to that object if that phraseology is clearer)? I think this would avoid a lot of the issues we see with naming things.

[pagbabian-splunk]

I think we would need to think about how to do that, likely we would need to change the framework as today all attributes need to be pre-defined in the dictionary. In fact, the way we make them local is by embedding the scope into the name, as we are trying to do here.

Given that this attribute, is_script_content_truncated is even more specific than at the object level, having an object scope still wouldn't have conveyed the intent. Therefore, let's go with what you have. We may still need an is_truncated in the future for something, similar to the recent is_deleted on the File object.

Note that from a self-documenting standpoint, is_deleted on the File object implies the file is deleted, while is_truncated on the Script object would imply the script is deleted. In this case, the representation of the script content is deleted.

[davemcatcisco]

Attn reviewers (@Aniak5, @zschmerber, @pagbabian-splunk, @floydtree, @mikeradka): As just discussed on the weekly call, the discussion in this PR is regarded as finalised, and this PR can now be merged once it has the necessary approval. I have just fixed conflicts that had arisen. Thanking you in advance.