[Winlogbeat] Add support for custom XML queries #29330

taylor-swanson · 2021-12-07T21:28:12Z

What does this PR do?

Added new configuration field (xml_query) to support custom XML queries
This new configuration item will conflict with existing simple
query configuration items (ignore_older, event_id, level, provider)
Validator has been updated to check for key conflicts and XML syntax,
but does not check for correctness of XML schema.

Why is it important?

The existing filtering options for Winlogbeat don't provide enough control when trying to filter out unwanted events. A user may want to include multiple channels in the same query or filter out certain SIDs. Custom XML queries provide a very powerful mechanism for fine tuning what events should be searched.

Checklist

My code follows the style guidelines of this project
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
I have made corresponding change to the default configuration files
I have added tests that prove my fix is effective or that my feature works
I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

How to test this PR locally

A quick way to generate XML queries is to create a custom view in Event Viewer and export the XML. This can be pasted as-is into a winlogbeat config:

winlogbeat.event_logs:
  - name: dhcp-server
    xml_query: >
      <QueryList>
        <Query Id="0" Path="DhcpAdminEvents">
          <Select Path="DhcpAdminEvents">*</Select>
          <Select Path="Microsoft-Windows-Dhcp-Server/FilterNotifications">*</Select>
          <Select Path="Microsoft-Windows-Dhcp-Server/Operational">*</Select>
        </Query>
      </QueryList>

The bookmark should be written to the registry file:

- name: microsoft-dhcp
  record_number: 10
  timestamp: 2021-12-06T20:40:54.198229Z
  bookmark: "<BookmarkList>\r\n  <Bookmark Channel='DhcpAdminEvents' RecordId='0'/>\r\n
    \ <Bookmark Channel='Microsoft-Windows-Dhcp-Server/FilterNotifications' RecordId='0'/>\r\n
    \ <Bookmark Channel='Microsoft-Windows-Dhcp-Server/Operational' RecordId='10'
    IsCurrent='true'/>\r\n</BookmarkList>"

There should be one bookmark per channel at minimum. I believe this is being generated by Windows, so in theory it should always do the right thing here, regardless of how complicated the query becomes. The record_number at the top-level registry entry is irrelevant here, since winlogbeat will always select the bookmark value(s) first. The record_number is only used in cases where custom queries are NOT given and no bookmark was produced previously (I've never seen this occur, so I'm not sure under what circumstances this will happen).

Related issues

Closes [Winlogbeat] XML filtering #1054

- Added new configuration field (xml_query) to support custom XML queries - This new configuration item will conflict with existing simple query configuration items (ignore_older, event_id, level, provider) - Validator has been updated to check for key conflicts and XML syntax, but does not check for correctness of XML schema.

mergify · 2021-12-07T21:28:27Z

This pull request does not have a backport label. Could you fix it @taylor-swanson? 🙏
To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

backport-v./d./d./d is the label to automatically backport to the 7./d branch. /d is the digit

NOTE: backport-skip has been added to this pull request.

elasticmachine · 2021-12-07T22:30:00Z

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

Start Time: 2021-12-15T15:06:26.533+0000
Duration: 64 min 1 sec
Commit: 4ac59de

Test stats 🧪

Test	Results
Failed	0
Passed	2161
Skipped	26
Total	2187

💚 Flaky test report

Tests succeeded.

🤖 GitHub comments

To re-run your PR in the CI, just comment with:

/test : Re-trigger the build.
/package : Generate the packages and run the E2E tests.
/beats-tester : Run the installation tests with beats-tester.
run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

taylor-swanson · 2021-12-09T14:07:40Z

/test

taylor-swanson · 2021-12-09T14:26:11Z

run elasticsearch-ci/docs

elasticmachine · 2021-12-13T15:34:33Z

Pinging @elastic/security-external-integrations (Team:Security-External Integrations)

andrewkroh

This part looks good. Can you please add some tests for this. We should have something added to the system tests (https://github.com/elastic/beats/blob/master/winlogbeat/tests/system/test_wineventlog.py) and I imagine there's a Go test that can be added that using xml_query. And a config validation test to check the assertions on the config.

There's a wineventlog-experimental reader that I expect needs to be updated as well. Check https://github.com/elastic/beats/blob/master/winlogbeat/eventlog/wineventlog_experimental.go#L47.

winlogbeat/docs/winlogbeat-options.asciidoc

andrewkroh · 2021-12-13T18:15:13Z

There should be one bookmark per channel at minimum.

Per event log reader rather than "channel" is how I would describe it.

The record_number is only used in cases where custom queries are NOT given and no bookmark was produced previously (I've never seen this occur, so I'm not sure under what circumstances this will happen).

record_number is legacy. It was kept to help users migrating from an older version of the registry file that did not contain bookmark. That was a very long time ago so that field and the associated logic could be removed now.

- Removed redundant ID assignment - Modified wineventlog_experimental to support custom XML queries - Added unit tests for config validation - Added unit/system test for XML query runner

winlogbeat/eventlog/wineventlog.go

winlogbeat/eventlog/wineventlog_experimental.go

efd6 · 2021-12-14T23:56:28Z

winlogbeat/eventlog/wineventlog_experimental.go

+		queryLog := c.Name
+		if info, err := os.Stat(c.Name); err == nil && info.Mode().IsRegular() {


I know that this is pre-existing, but is it ever likely that a user will specify a file log destination with a file scheme?

I'm not entirely sure, perhaps someone else can elaborate on that more (@andrewkroh ?). We do support providing a filename and reading from it, that much I do know.

In the docs we say they need to pass an absolute path so I don't think users will pass a file URI.

To read events from an archived .evtx file you can specify the name as the absolute path (it cannot be relative) to the file.

https://www.elastic.co/guide/en/beats/winlogbeat/7.16/configuration-winlogbeat-options.html#configuration-winlogbeat-options-event_logs-name

andrewkroh

LGTM

- Added new configuration field (xml_query) to support custom XML queries - This new configuration item will conflict with existing simple query configuration items (ignore_older, event_id, level, provider) - Validator has been updated to check for key conflicts and XML syntax, but does not check for correctness of XML schema. - Added unit tests for config validation - Added unit/system test for XML query runner (cherry picked from commit b5e9414)

- Added new configuration field (xml_query) to support custom XML queries - This new configuration item will conflict with existing simple query configuration items (ignore_older, event_id, level, provider) - Validator has been updated to check for key conflicts and XML syntax, but does not check for correctness of XML schema. - Added unit tests for config validation - Added unit/system test for XML query runner (cherry picked from commit b5e9414) Co-authored-by: Taylor Swanson <90622908+taylor-swanson@users.noreply.github.com>

taylor-swanson added enhancement Winlogbeat Team:Security-External Integrations labels Dec 7, 2021

taylor-swanson self-assigned this Dec 7, 2021

botelastic bot added needs_team Indicates that the issue/PR needs a Team:* label and removed needs_team Indicates that the issue/PR needs a Team:* label labels Dec 7, 2021

mergify bot added the backport-skip Skip notification from the automated backport with mergify label Dec 7, 2021

taylor-swanson added backport-v7.16.0 Automated backport with mergify backport-v8.0.0 Automated backport with mergify and removed backport-skip Skip notification from the automated backport with mergify labels Dec 9, 2021

taylor-swanson added 3 commits December 9, 2021 08:33

Update changelog with PR number

ece5a93

Add ID key

5f72d28

Remove unneeded channelName variable and improve docs

4ee6d1f

taylor-swanson marked this pull request as ready for review December 13, 2021 15:34

taylor-swanson requested a review from a team as a code owner December 13, 2021 15:34

andrewkroh reviewed Dec 13, 2021

View reviewed changes

winlogbeat/docs/winlogbeat-options.asciidoc Outdated Show resolved Hide resolved

winlogbeat/docs/winlogbeat-options.asciidoc Outdated Show resolved Hide resolved

taylor-swanson added 2 commits December 14, 2021 08:34

Doc improvements

b24cc7a

Review comments and tests

c95a65a

- Removed redundant ID assignment - Modified wineventlog_experimental to support custom XML queries - Added unit tests for config validation - Added unit/system test for XML query runner

efd6 reviewed Dec 14, 2021

View reviewed changes

Review comments

4ac59de

andrewkroh approved these changes Dec 15, 2021

View reviewed changes

taylor-swanson merged commit b5e9414 into elastic:master Dec 16, 2021

taylor-swanson deleted the xml-query branch December 16, 2021 15:13

mergify bot mentioned this pull request Dec 16, 2021

[7.17](backport #29330) [Winlogbeat] Add support for custom XML queries #29474

Merged

mergify bot mentioned this pull request Dec 16, 2021

[8.0](backport #29330) [Winlogbeat] Add support for custom XML queries #29475

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Winlogbeat] Add support for custom XML queries #29330

[Winlogbeat] Add support for custom XML queries #29330

taylor-swanson commented Dec 7, 2021 •

edited

Loading

mergify bot commented Dec 7, 2021

elasticmachine commented Dec 7, 2021 •

edited by jenkins-beats-ci bot

Loading

Build stats

Test stats 🧪

taylor-swanson commented Dec 9, 2021

taylor-swanson commented Dec 9, 2021

elasticmachine commented Dec 13, 2021

andrewkroh left a comment

andrewkroh commented Dec 13, 2021

efd6 Dec 14, 2021

taylor-swanson Dec 15, 2021

andrewkroh Dec 15, 2021

andrewkroh left a comment

		queryLog := c.Name
		if info, err := os.Stat(c.Name); err == nil && info.Mode().IsRegular() {

[Winlogbeat] Add support for custom XML queries #29330

[Winlogbeat] Add support for custom XML queries #29330

Conversation

taylor-swanson commented Dec 7, 2021 • edited Loading

What does this PR do?

Why is it important?

Checklist

How to test this PR locally

Related issues

mergify bot commented Dec 7, 2021

elasticmachine commented Dec 7, 2021 • edited by jenkins-beats-ci bot Loading

💚 Build Succeeded

Build stats

Test stats 🧪

💚 Flaky test report

🤖 GitHub comments

taylor-swanson commented Dec 9, 2021

taylor-swanson commented Dec 9, 2021

elasticmachine commented Dec 13, 2021

andrewkroh left a comment

Choose a reason for hiding this comment

andrewkroh commented Dec 13, 2021

efd6 Dec 14, 2021

Choose a reason for hiding this comment

taylor-swanson Dec 15, 2021

Choose a reason for hiding this comment

andrewkroh Dec 15, 2021

Choose a reason for hiding this comment

andrewkroh left a comment

Choose a reason for hiding this comment

taylor-swanson commented Dec 7, 2021 •

edited

Loading

elasticmachine commented Dec 7, 2021 •

edited by jenkins-beats-ci bot

Loading