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

Handle empty sysmon DNS answer data #35207

Merged
merged 6 commits into from Jun 13, 2023
Merged

Conversation

Technici4n
Copy link
Contributor

@Technici4n Technici4n commented Apr 25, 2023

What does this PR do?

Makes the sysmon pipeline handle DNS records with blank data, for example <Data Name='QueryResults'>type: 33 ;type: 33 ;<some ip v6>;<some ip v4>;</Data>.

Why is it important?

I encountered such data - I am not sure how it was produced, it might have been an issue with the network configuration. Nonetheless, I did not want to drop the records, so I edited the pipeline a bit, and figured this might be useful to others. If you don't want this, it's also fine.

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.

Author's Checklist

How to test this PR locally

Related issues

Use cases

Screenshots

Logs

@Technici4n Technici4n requested a review from a team as a code owner April 25, 2023 10:12
@botelastic botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Apr 25, 2023
@mergify
Copy link
Contributor

mergify bot commented Apr 25, 2023

This pull request does not have a backport label.
If this is a bug or security fix, could you label this PR @Technici4n? 🙏.
For such, you'll need to label your PR with:

  • The upcoming major version of the Elastic Stack
  • The upcoming minor version of the Elastic Stack (if you're not pushing a breaking change)

To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

  • backport-v8./d.0 is the label to automatically backport to the 8./d branch. /d is the digit

@elasticmachine
Copy link
Collaborator

elasticmachine commented Apr 25, 2023

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2023-06-08T21:13:20.215+0000

  • Duration: 42 min 25 sec

Test stats 🧪

Test Results
Failed 0
Passed 336
Skipped 0
Total 336

💚 Flaky test report

Tests succeeded.

🤖 GitHub comments

Expand to view the 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!)

@mergify
Copy link
Contributor

mergify bot commented Apr 25, 2023

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b empty-dns-answer-data upstream/empty-dns-answer-data
git merge upstream/main
git push upstream empty-dns-answer-data

@elasticmachine
Copy link
Collaborator

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

@botelastic botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label May 23, 2023
@efd6
Copy link
Contributor

efd6 commented Jun 5, 2023

@Technici4n Are you able to provide a complete event XML (appropriately cleaned of private data) so that we can construct tests for this addition?

@Technici4n
Copy link
Contributor Author

Hi, here is a cleaned event:

<Event
	xmlns='http://schemas.microsoft.com/win/2004/08/events/event'>
	<System>
		<Provider Name='Microsoft-Windows-Sysmon' Guid='{00000000-0000-0000-0000-000000000000}'/>
		<EventID>22</EventID>
		<Version>5</Version>
		<Level>4</Level>
		<Task>22</Task>
		<Opcode>0</Opcode>
		<Keywords>0x8000000000000000</Keywords>
		<TimeCreated SystemTime='2000-00-00T00:00:00.000Z'/>
		<EventRecordID>1111</EventRecordID>
		<Correlation/>
		<Execution ProcessID='1000' ThreadID='2000'/>
		<Channel>Microsoft-Windows-Sysmon/Operational</Channel>
		<Computer>internal.network.org</Computer>
		<Security UserID='A-0-0-00'/>
	</System>
	<EventData>
		<Data Name='RuleName'>-</Data>
		<Data Name='UtcTime'>2000-00-00T00:00:00.000</Data>
		<Data Name='ProcessGuid'>{00000000-0000-0000-0000-000000000000}</Data>
		<Data Name='ProcessId'>500</Data>
		<Data Name='QueryName'>some.other.domain.com</Data>
		<Data Name='QueryStatus'>0</Data>
		<Data Name='QueryResults'>type:  33 ;type:  33 ;1:2:3::3;1.2.3.3;</Data>
		<Data Name='Image'>C:\Windows\System32\lsass.exe</Data>
		<Data Name='User'>NT AUTHORITY\SYSTEM</Data>
	</EventData>
</Event>

I am not running through winlogbeat directly, but I am reusing its pipelines. This is the error message that I get:
error.message is "Processor \"conditional\" with tag \"\" in pipeline \"winlogbeat-8.8.0-sysmon\" failed with message \"unexpected QueryResult format\""

efd6 added a commit to efd6/integrations that referenced this pull request Jun 7, 2023
efd6 added a commit to efd6/integrations that referenced this pull request Jun 7, 2023
CHANGELOG.next.asciidoc Outdated Show resolved Hide resolved
x-pack/winlogbeat/module/sysmon/ingest/sysmon.yml Outdated Show resolved Hide resolved
@efd6
Copy link
Contributor

efd6 commented Jun 7, 2023

I've added the test case. Would you resolve the conflict and address the comments? Thanks

efd6 added a commit to efd6/integrations that referenced this pull request Jun 8, 2023
@efd6
Copy link
Contributor

efd6 commented Jun 8, 2023

/test

@efd6
Copy link
Contributor

efd6 commented Jun 8, 2023

/test

efd6 added a commit to efd6/integrations that referenced this pull request Jun 9, 2023
Copy link
Contributor

@efd6 efd6 left a comment

Choose a reason for hiding this comment

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

Thanks

@efd6 efd6 added backport-skip Skip notification from the automated backport with mergify 8.9-candidate Winlogbeat labels Jun 13, 2023
@efd6 efd6 merged commit 791fdf8 into elastic:main Jun 13, 2023
15 of 17 checks passed
efd6 added a commit to elastic/integrations that referenced this pull request Jun 13, 2023
sodhikirti07 pushed a commit to elastic/integrations that referenced this pull request Jun 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8.9-candidate backport-skip Skip notification from the automated backport with mergify enhancement Winlogbeat
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants