sigma icon indicating copy to clipboard operation
sigma copied to clipboard

The "NewValue" field for Windows Defender should be "New Value"

Open f-block opened this issue 3 years ago • 17 comments

The detection of Windows Defender Exclusions within windows/builtin/windefend/win_defender_exclusions.yml fails since it uses the NewValue field instead of New Value.

f-block avatar Jun 17 '22 10:06 f-block

HI, Yaml don't like space in field name so we have replace it with _ The correct selection is New_Value|contains: '\Microsoft\Windows Defender\Exclusions' Go catch 👍

I have add field to winlogbeat-modules-enabled.yml some time ago Should be the same without ECS

    #
    # Microsoft-Windows-Windows Defender/Operational
    #
    Action_ID: winlog.event_data.Action\ ID
    Action_Name: winlog.event_data.Action\ Name
    Additional_Actions_ID: winlog.event_data.Additional\ Actions\ ID
    Additional_Actions_String: winlog.event_data.Additional\ Actions\ String
    Category_ID: winlog.event_data.Category\ ID
    Category_Name: winlog.event_data.Category\ Name
    Detection_ID: winlog.event_data.Detection\ ID
    Detection_Time: winlog.event_data.Detection\ Time
    Detection_User: winlog.event_data.Detection\ User
    Engine_Version: winlog.event_data.Engine\ Version
    Error_Code: winlog.event_data.Error\ Code
    Error_Description: winlog.event_data.Error\ Description
    Execution_ID: winlog.event_data.Execution\ ID
    Execution_Name: winlog.event_data.Execution\ Name
    FWLink: winlog.event_data.FWLink
    New_Value: winlog.event_data.New\ Value
    Old_Value: winlog.event_data.Old\ Value
    Origin_ID: winlog.event_data.Origin\ ID
    Origin_Name: winlog.event_data.Origin\ Name
    Path: winlog.event_data.Path
    Post_Clean_Status: winlog.event_data.Post\ Clean\ Status
    Pre_Execution_Status: winlog.event_data.Pre\ Execution\ Status
    Process_Name: winlog.event_data.Process\ Name
    Product_Name: winlog.event_data.Product\ Name
    Product_Version: winlog.event_data.Product\ Version
    Remediation_User: winlog.event_data.Remediation\ User
    Security_intelligence_Version: winlog.event_data.Security\ intelligence\ Version
    Severity_ID: winlog.event_data.Severity\ ID
    Severity_Name: winlog.event_data.Severity\ Name
    Source_ID: winlog.event_data.Source\ ID
    Source_Name: winlog.event_data.Source\ Name
    Status_Code: winlog.event_data.Status\ Code
    Status_Description: winlog.event_data.Status\ Description
    Threat_ID: winlog.event_data.Threat\ ID
    Threat_Name: winlog.event_data.Threat\ Name
    Type_ID: winlog.event_data.Type\ ID
    Type_Name: winlog.event_data.Type\ Name

frack113 avatar Jun 17 '22 11:06 frack113

Hi,

just changed the files accordingly but recognized afterwards that, at least for me with es-dsl, escaping the New Value whitespace in New_Value: winlog.event_data.New\ Value doesn't work. So I would add this:

New_Value: winlog.event_data.New Value

?

This works at least in my case.

f-block avatar Jun 17 '22 11:06 f-block

Or maybe putting the value in quotes (if in some cases the white space is a problem for some yaml parsers):

New_Value: 'winlog.event_data.New Value'

?

f-block avatar Jun 17 '22 11:06 f-block

This is a trouble with es-dsl . Field with space must be echape in elastic. When I try with es-qs I get a winlog.event_data.Source\ Name:"AMSI" But with es-dsl I get "winlog.event_data.Source Name": "AMSI" or "winlog.event_data.Source\\ Name": "AMSI"

frack113 avatar Jun 17 '22 14:06 frack113

hm, so is there a quick fix for the es-dsl backend, or is the solution to start with a winlogbeat-esl.yml with a higher order number?

f-block avatar Jun 17 '22 15:06 f-block

Sorry I did not find a quick fix to the es-dsl backend. Hope a pySigma for elastic come fast

frack113 avatar Jun 17 '22 16:06 frack113

Hi! YAML is fine with spaces in field names as long as they are quoted. I would prefer the space instead of creating a special case because pySigma also supports prefixing of fields to solve the event_data thing.

thomaspatzke avatar Jun 17 '22 20:06 thomaspatzke

This is a trouble with es-dsl . Field with space must be echape in elastic.

Never was aware that this is the case for DSL. In query strings this is required because else the whitespace would cause syntactical separation of the field name before the space before the remaining field name part after the space. This shouldn't be necessary for DSL as there is a clear boundary of the field name by the JSON syntax. I can't find anything regarding this for field name in context of DSL.

thomaspatzke avatar Jun 17 '22 21:06 thomaspatzke

I can't find anything regarding this for field name in context of DSL.

As I writte in elasticsearch field name with space are not a good idea . https://github.com/elastic/elasticsearch-dsl-py/issues/721

But You can : https://www.elastic.co/guide/en/elasticsearch/reference/current/query-dsl-query-string-query.html#_field_names first\ name:Alice

frack113 avatar Jun 18 '22 06:06 frack113

In the issue they talk about query strings, where escaping is obviously requires for syntactical reasons. Query DSL is the way to express queries in a structured way and at the bottom of the issue the responder gives even the hint to use match queries for this purpose and not using query strings.

We have three alternatives:

  • plain query strings: escaping required
  • query strings embedded in dsl: escaping required
  • query dsl without query strings: escaping not required because the query is already structured with JSON?

Did the DSL with spaces in field names doesn't worked in a concrete example? Then we could take the escaping logic from the es-qs backend and escape it. But I'm still unsure if this is required or even counterproductive at this point. This should be tried.

thomaspatzke avatar Jun 18 '22 07:06 thomaspatzke

Hi,

DSL had no problems with unescaped spaces. So using any of the following mappings produces the expected results for the rule windows/builtin/windefend/win_defender_exclusions.yml with the es-dsl backend, and the mappings were at least parseable by yq (I've no idea of YAML so not sure what valid syntax is right there):

    New_Value: 'winlog.event_data.New Value'
    New_Value: winlog.event_data.New Value
    New Value: winlog.event_data.New Value
    'New Value': 'winlog.event_data.New Value'
    'New Value': winlog.event_data.New Value

To throw in my two cents: I think the backend should take care of escaping any characters, because it knows best if it is necessary or not, but the mapping should not. This would mean that es-qs should take care of escaping the New Value field. The following mapping would be my suggestion for this case (under the assumption that unquoted spaces are a problem for some YAML parsers):

'New Value': 'winlog.event_data.New Value'

f-block avatar Jun 18 '22 08:06 f-block

I remember well having a discussion about space in field name : https://github.com/SigmaHQ/sigma/pull/2139

frack113 avatar Jun 19 '22 06:06 frack113

I fully agree with @f-block and disagree with the decisions made in #2139 (unfortunately I've missed this discussion) for some reasons:

  • There is no technical reason for forbidding white spaces. It's supported by YAML and its parsers (just verified this for pyYAML, even quoting is not required) and such field names seem also to exist.
  • Introducing special cases requires that these special cases are handled in translation of rules into queries. This means that each backend must have mappings for all existing cases if it doesn't has the same replacement scheme as Sigma by coincidence. Backend developers can't easily determine these fields, as they aren't contained in the Sigma rules. So a directory has to be maintained in the documentation, which seems to be deemed to be incomplete to me. A human reader of a rule can't determine if there was indeed a space in the field name or if it was removed/replaced.
  • Removal of spaces from field names doesn't allows it anymore to define generic handling for field name space handling in backend processing pipelines. Systems that support white spaces (Elastic, Splunk, ...) can simply pass them through, other might drop them or replace them by other characters, which can be reflected in conversion code/config.
  • It can create inconsistencies and naming clashes. The replacement field name might exist in other rules or might be introduced somewhere in the future. This makes it hard or even impossible to define clear mappings for such fields.
  • It also creates a special case for rule writers, which I think will result in errors, too. Some might even just ignore and use white space field names because it simply works for them.

I see more problems introduced by the avoidance of white spaces than solved. Therefore I propose that:

  • Whitespaces in field names are explicitely allowed.
  • The part about this in the specification is removed.

Regarding the <Provider Name=...-thing I'm fine to use Provider_Name. It's not really an existing field name with white spaces, but a compound name created artificially for this purpose. Nevertheless, a mapping is only defined for few backends...this is something that should be fixed, as it currently renders all the rules that contain it useless for most backends.

Lets add @Neo23x0 @phantinuss to the discussion, I think this is quite important to have a consistent scheme here.

thomaspatzke avatar Jun 19 '22 09:06 thomaspatzke

TLDR: once the information (white spaces) is lost, it can be used anymore. So we should have a good reason for dropping it and I don't see it 😉

thomaspatzke avatar Jun 19 '22 10:06 thomaspatzke

I am against New_Value as it collides with the field names that are defined as attributes of XML Tags in WinEventLog (e.g. Provider_Name). IIRC NewValue (stripping of whitespace) was introduced as it was the main way it was used in SigmaHQ at that point and because we thought white space in field names is an issue for at least some back ends. If that is not the case I am not opposed to changing it to "New Value" as I can understand the reasoning and advantage of "keeping it close to the source" which was what we wanted to achieve with "NewValue" anyways.

Regarding the field mapping for Provider_Name: Yes there should be more mappings. I can have a look tomorrow if I can guess what it should be for the other back ends but I guess as it is a special case, that this will be hard without access to the back ends in question.

phantinuss avatar Jun 19 '22 16:06 phantinuss

Space in field name will break some backend output where a space is important (like sqlite or es-qs or splunk). In view of the test and correction work on sigmac, it is better to implement it only on PySigma. With a warning 'PySigma use original field name with space, backend must correctly escape them to the need'

EVTX-ETW-Resources/ETWProvidersManifests/Windows11/21H2/W11_21H2_Pro_20220517_22000.675/WEPExplorer grep -r "New\ Value" | wc -l => 1 (Microsoft-Windows-Windows Defender.xml) grep -r "NewValue" | wc -l => 21

grep -r "Provider\ Name" | wc -l => 0 grep -r "ProviderName" | wc -l => 64

grep -r data\ name=".\ ."\ inType= | uniq -u | wc -l => 1881

frack113 avatar Jun 19 '22 17:06 frack113

IIRC NewValue (stripping of whitespace) was introduced as it was the main way it was used in SigmaHQ at that point and because we thought white space in field names is an issue for at least some back ends.

I don't remember the reason anymore, but I think with the extended possibilities of processing pipelines in pySigma we get the tool to handle this in the conversion process.

Regarding the field mapping for Provider_Name: Yes there should be more mappings. I can have a look tomorrow if I can guess what it should be for the other back ends but I guess as it is a special case, that this will be hard without access to the back ends in question.

I think with the switch to pySigma we're in the comfortable situation that all backends have to be touched anyways to port them from legacy sigmac. So it would be great to have some overview that could be integrated to the backend template and/or documentation to make backend developers aware of this.

Space in field name will break some backend output where a space is important (like sqlite or es-qs or splunk).

While testing today I've already seen that the pySigma Splunk backend generates wrong queries for field names with white spaces. I will add some generic handling possibilities in the backend base to make it easy for backend developers to handle this.

thomaspatzke avatar Jun 19 '22 19:06 thomaspatzke

Jumping in regarding this issue. Just FYI most spaces in the event log raw XML field name (if there are any) are added spaces as the defining ETW manifest does not contain them. Here are a couple of examples

From Defender EID 1150

image

ETW Definition for the same EID

<template tid="task_01150Args">
            <data name="ProductName" inType="win:UnicodeString" />
            <data name="Platformversion" inType="win:UnicodeString" />
            <data name="Unused" inType="win:UnicodeString" />
            <data name="Engineversion" inType="win:UnicodeString" />
            <data name="Securityintelligenceversion" inType="win:UnicodeString" />
</template>

From Defender EID 5007

image

ETW Definition for the same EID

<template tid="task_05007Args">
            <data name="ProductName" inType="win:UnicodeString" />
            <data name="ProductVersion" inType="win:UnicodeString" />
            <data name="OldValue" inType="win:UnicodeString" />
            <data name="NewValue" inType="win:UnicodeString" />
</template>

Most of the event field names do not contain spaces (except for some rare ones). But it's always better to check the field name in the ETW manifest to be 100% sure. In this specific defender case. The space is optional.

nasbench avatar Dec 12 '22 23:12 nasbench

Following an internal discussion, we decided to close this PR for the following reason.

The field in question New Value does have a space if you view it from the event viewer XML format but as explained in my comment above the actual ETW definition (which is the source) doesn't have it. So this seems to be a small issue as it'll depend on from where one will ingest the log event in question to determine if there is a space.

After checking other event logs. It seems (so far) that only the defender event log in the event viewer is using spaces. For this, we choose to ignore this special case due to the reason explained above.

SIGMA is designed to not care actually about the event field and how it's written in the rule. Because at the end of the day during conversion SIGMAC/PySIGMA will use mappings to backends fields. And if those backends choose to have a space or not is not up to us (we just have to comply with the mapping).

The point of the fields being exactly the same as they are in the manifest/log is to ease the writing and understanding of the detection. Since this a "unique" special case. We should treat it as such and avoid spaces.

We will map the defender logs to their respective backends in another PR (at least the missing ones) and it'll look something like this

Thanks for your submission.

nasbench avatar Dec 15 '22 11:12 nasbench