Winlogbeat: Fix compatibility problems with newer OS versions in CI by adriansr · Pull Request #22362 · elastic/beats

adriansr · 2020-11-02T20:46:53Z

This PR includes a collection of fixes for different issues found in Winlogbeat after enabling CI testing for previously untested versions of Windows.

Two bugs have been fixed (only affected 32bit binaries):

Bad EVT_VARIANT struct definition, caused errors formatting events.
The first field of the structure, a union, didn't have the right size when compiled for 32-bits, causing "invalid argument" errors when the struct was passed to an API. Also, some EVT_VARIANT types weren't being handled properly.
Failed to seek to a saved state in an .evtx file.
This is caused by a helper tool in Go (mkwinsyscall) not generating correct syscall calls under GOARCH=386 (or any 32-bit arch). This only affects functions that receive a 64-bit parameter, in the case of Beats, only EvtSeek is affected. (Bug report).

The rest are changes to tests due to inconsistent behaviors of different Windows versions:

Remove formatting assertion in TestFormatMessage
Under most Windows versions, GUID values cannot be substituted with string values. However, this is not the case under at least Win7 32bit. Just disable the testing assertion as it has no impact.
Disable TestFormatMessage/no_metadata tests
Under Windows 10, event formatting is not failing with an error even though not metadata is being passed.
Check for errors on publishing events to an event log
Under Windows 7 and Windows 10, writes to a newly created event log fail occasionally. It seems that there is a delay between when an event log is created and publishing events to it is allowed.
Sysmon: Ignore winlog.user.* fields during test
For some reason, the following fields:
- winlog.user.type
- winlog.user.name
- winlog.user.domain
are not populated by the Sysmon module under Windows 7 32-bit. I couldn't reproduce outside of CI, where it's failing to lookup SID S-1-5-18.

Fixes #19798
Fixes #19829
Fixes #22046
Fixes #22302

elasticmachine · 2020-11-03T14:05:26Z

💚 Flaky test report

Tests succeeded.

Expand to view the summary

Test stats 🧪

Test	Results
Failed	0
Passed	2192
Skipped	24
Total	2216

elasticmachine · 2020-11-03T14:05:29Z

💚 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

Build Cause: [Pull request #22362 updated]
Start Time: 2020-11-10T16:59:39.157+0000
Duration: 32 min 56 sec

Test stats 🧪

Test	Results
Failed	0
Passed	2192
Skipped	24
Total	2216

adriansr · 2020-11-03T19:38:28Z

jenkins, test this

The first field of the structure, a union, didn't have the proper size when compiled for 32-bits, causing "invalid argument" errors when the struct was passed to an API. Also, some EVT_VARIANT types weren't being handled properly.

Under most Windows versions, GUID values cannot be substituted with string values. However, this is not the case under at least Win7 32bit. Just disable the testing assertion as it has no impact.

Go's mkwinsyscall is generating syscall invocations that don't work properly under GOARCH=386 (or any 32-bit arch). This only affects functions that receive a 64-bit parameter, in this case only EvtSeek is affected. This replaces `mkwinsyscall` with a patched one that adds some workaround code.

Under Windows 10, event formatting is not failing with an error even though not metadata is being passed.

Under Windows 7 and Windows 10, writes to a newly created event log fail occasionally. It seems that there is a delay between when an event log is created and publishing events to it is allowed. This commit updates the tests to retry on failure when creating an event log for testing.

For some reason, the following fields: - winlog.user.type - winlog.user.name - winlog.user.domain are not populated by the Sysmon module under Windows 7 32-bit. I couldn't reproduce outside of CI, where it's failing to lookup SID `S-1-5-18`: ``` testing_windows.go:69: Expected and actual are different: --- Expected +++ Actual @@ -47,6 +47,3 @@ "user": { - "domain": "NT AUTHORITY", - "identifier": "S-1-5-18", - "name": "SYSTEM", - "type": "Well Known Group" + "identifier": "S-1-5-18" }, ``` This commit updates the test to ignore those fields and removes them from the golden files.

Under Windows 10, writing large events (31800 bytes) results in an empty event log. Seems that the events are being silently dropped due to being too big.

elasticmachine · 2020-11-10T17:40:28Z

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

andrewkroh

LGTM. Thank you for tackling this.

andrewkroh · 2020-11-10T18:00:19Z

winlogbeat/eventlog/wineventlog_test.go

+func safeWriteEvent(t testing.TB, log *eventlog.Log, etype uint16, eid uint32, msgs []string) {
+	deadline := time.Now().Add(time.Second * 10)
+	for {
+		err := log.Report(etype, eid, msgs)


Curious what the story is behind this. What kind of temporary errors were you seeing?

You get an access denied error if you try to publish too shortly after creating the event log

…lastic#22362) * Fix EVT_VARIANT struct definition for GOARCH=386 The first field of the structure, a union, didn't have the proper size when compiled for 32-bits, causing "invalid argument" errors when the struct was passed to an API. Also, some EVT_VARIANT types weren't being handled properly. * Remove formatting assertion in TestFormatMessage Under most Windows versions, GUID values cannot be substituted with string values. However, this is not the case under at least Win7 32bit. Just disable the testing assertion as it has no impact. * Update generated syscalls to avoid bug under 32-bit Go's mkwinsyscall is generating syscall invocations that don't work properly under GOARCH=386 (or any 32-bit arch). This only affects functions that receive a 64-bit parameter, in this case only EvtSeek is affected. This replaces `mkwinsyscall` with a patched one that adds some workaround code. * Enable all windows tests * Disable TestFormatMessage/no_metadata tests Under Windows 10, event formatting is not failing with an error even though not metadata is being passed. * Testing: Error check on publishing events to an event log Under Windows 7 and Windows 10, writes to a newly created event log fail occasionally. It seems that there is a delay between when an event log is created and publishing events to it is allowed. This commit updates the tests to retry on failure when creating an event log for testing. * Sysmon: Ignore winlog.user.* fields during test For some reason, the following fields: - winlog.user.type - winlog.user.name - winlog.user.domain are not populated by the Sysmon module under Windows 7 32-bit. I couldn't reproduce outside of CI, where it's failing to lookup SID `S-1-5-18`: ``` testing_windows.go:69: Expected and actual are different: --- Expected +++ Actual @@ -47,6 +47,3 @@ "user": { - "domain": "NT AUTHORITY", - "identifier": "S-1-5-18", - "name": "SYSTEM", - "type": "Well Known Group" + "identifier": "S-1-5-18" }, ``` This commit updates the test to ignore those fields and removes them from the golden files. * Testing: limit event size in testWindowsEventLog Under Windows 10, writing large events (31800 bytes) results in an empty event log. Seems that the events are being silently dropped due to being too big. (cherry picked from commit d97fe08)

…22362) (#22727) * Fix EVT_VARIANT struct definition for GOARCH=386 The first field of the structure, a union, didn't have the proper size when compiled for 32-bits, causing "invalid argument" errors when the struct was passed to an API. Also, some EVT_VARIANT types weren't being handled properly. * Remove formatting assertion in TestFormatMessage Under most Windows versions, GUID values cannot be substituted with string values. However, this is not the case under at least Win7 32bit. Just disable the testing assertion as it has no impact. * Update generated syscalls to avoid bug under 32-bit Go's mkwinsyscall is generating syscall invocations that don't work properly under GOARCH=386 (or any 32-bit arch). This only affects functions that receive a 64-bit parameter, in this case only EvtSeek is affected. This replaces `mkwinsyscall` with a patched one that adds some workaround code. * Enable all windows tests * Disable TestFormatMessage/no_metadata tests Under Windows 10, event formatting is not failing with an error even though not metadata is being passed. * Testing: Error check on publishing events to an event log Under Windows 7 and Windows 10, writes to a newly created event log fail occasionally. It seems that there is a delay between when an event log is created and publishing events to it is allowed. This commit updates the tests to retry on failure when creating an event log for testing. * Sysmon: Ignore winlog.user.* fields during test For some reason, the following fields: - winlog.user.type - winlog.user.name - winlog.user.domain are not populated by the Sysmon module under Windows 7 32-bit. I couldn't reproduce outside of CI, where it's failing to lookup SID `S-1-5-18`: ``` testing_windows.go:69: Expected and actual are different: --- Expected +++ Actual @@ -47,6 +47,3 @@ "user": { - "domain": "NT AUTHORITY", - "identifier": "S-1-5-18", - "name": "SYSTEM", - "type": "Well Known Group" + "identifier": "S-1-5-18" }, ``` This commit updates the test to ignore those fields and removes them from the golden files. * Testing: limit event size in testWindowsEventLog Under Windows 10, writing large events (31800 bytes) results in an empty event log. Seems that the events are being silently dropped due to being too big. (cherry picked from commit d97fe08)

…22362) (#22728) * Fix EVT_VARIANT struct definition for GOARCH=386 The first field of the structure, a union, didn't have the proper size when compiled for 32-bits, causing "invalid argument" errors when the struct was passed to an API. Also, some EVT_VARIANT types weren't being handled properly. * Remove formatting assertion in TestFormatMessage Under most Windows versions, GUID values cannot be substituted with string values. However, this is not the case under at least Win7 32bit. Just disable the testing assertion as it has no impact. * Update generated syscalls to avoid bug under 32-bit Go's mkwinsyscall is generating syscall invocations that don't work properly under GOARCH=386 (or any 32-bit arch). This only affects functions that receive a 64-bit parameter, in this case only EvtSeek is affected. This replaces `mkwinsyscall` with a patched one that adds some workaround code. * Enable all windows tests * Disable TestFormatMessage/no_metadata tests Under Windows 10, event formatting is not failing with an error even though not metadata is being passed. * Testing: Error check on publishing events to an event log Under Windows 7 and Windows 10, writes to a newly created event log fail occasionally. It seems that there is a delay between when an event log is created and publishing events to it is allowed. This commit updates the tests to retry on failure when creating an event log for testing. * Sysmon: Ignore winlog.user.* fields during test For some reason, the following fields: - winlog.user.type - winlog.user.name - winlog.user.domain are not populated by the Sysmon module under Windows 7 32-bit. I couldn't reproduce outside of CI, where it's failing to lookup SID `S-1-5-18`: ``` testing_windows.go:69: Expected and actual are different: --- Expected +++ Actual @@ -47,6 +47,3 @@ "user": { - "domain": "NT AUTHORITY", - "identifier": "S-1-5-18", - "name": "SYSTEM", - "type": "Well Known Group" + "identifier": "S-1-5-18" }, ``` This commit updates the test to ignore those fields and removes them from the golden files. * Testing: limit event size in testWindowsEventLog Under Windows 10, writing large events (31800 bytes) results in an empty event log. Seems that the events are being silently dropped due to being too big. (cherry picked from commit d97fe08)

botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Nov 2, 2020

adriansr added the Team:Security-External Integrations label Nov 2, 2020

botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Nov 2, 2020

adriansr force-pushed the wb_32bit_wineventlog branch from 5c6b19e to 833b5d0 Compare November 3, 2020 13:23

adriansr force-pushed the wb_32bit_wineventlog branch 3 times, most recently from a1e53f2 to 50b5421 Compare November 10, 2020 16:13

adriansr added 8 commits November 10, 2020 17:58

Fix EVT_VARIANT struct definition for GOARCH=386

ae9b3c8

The first field of the structure, a union, didn't have the proper size when compiled for 32-bits, causing "invalid argument" errors when the struct was passed to an API. Also, some EVT_VARIANT types weren't being handled properly.

Remove formatting assertion in TestFormatMessage

57fc64c

Under most Windows versions, GUID values cannot be substituted with string values. However, this is not the case under at least Win7 32bit. Just disable the testing assertion as it has no impact.

Enable all windows tests

68c2637

Disable TestFormatMessage/no_metadata tests

3e350c7

Under Windows 10, event formatting is not failing with an error even though not metadata is being passed.

Testing: limit event size in testWindowsEventLog

6834315

Under Windows 10, writing large events (31800 bytes) results in an empty event log. Seems that the events are being silently dropped due to being too big.

adriansr force-pushed the wb_32bit_wineventlog branch from 50b5421 to 6834315 Compare November 10, 2020 16:58

adriansr changed the title ~~WIP: Winlogbeat 32-bit new eventlog~~ Winlogbeat: Fix compatibility problems with newer OS versions in the support matrix Nov 10, 2020

adriansr changed the title ~~Winlogbeat: Fix compatibility problems with newer OS versions in the support matrix~~ Winlogbeat: Fix compatibility problems with newer OS versions in CI Nov 10, 2020

adriansr marked this pull request as ready for review November 10, 2020 17:40

adriansr requested a review from a team as a code owner November 10, 2020 17:40

adriansr requested review from a team and removed request for a team November 10, 2020 17:40

adriansr added needs_backport PR is waiting to be backported to other branches. review labels Nov 10, 2020

andrewkroh approved these changes Nov 10, 2020

View reviewed changes

adriansr merged commit d97fe08 into elastic:master Nov 11, 2020

adriansr mentioned this pull request Nov 24, 2020

Cherry-pick #22362 to 7.x: Winlogbeat: Fix compatibility problems with newer OS versions in CI #22727

Merged

adriansr added v7.11.0 and removed needs_backport PR is waiting to be backported to other branches. labels Nov 24, 2020

adriansr mentioned this pull request Nov 24, 2020

Cherry-pick #22362 to 7.10: Winlogbeat: Fix compatibility problems with newer OS versions in CI #22728

Merged

adriansr added the v7.10.1 label Nov 24, 2020

andrewkroh mentioned this pull request Mar 18, 2021

[Winlogbeat] Retry ReportEvent in tests #24645

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Winlogbeat: Fix compatibility problems with newer OS versions in CI#22362

Winlogbeat: Fix compatibility problems with newer OS versions in CI#22362
adriansr merged 8 commits intoelastic:masterfrom
adriansr:wb_32bit_wineventlog

adriansr commented Nov 2, 2020 •

edited

Loading

Uh oh!

elasticmachine commented Nov 3, 2020 •

edited by jenkins-beats-ci bot

Loading

Test stats 🧪

Uh oh!

elasticmachine commented Nov 3, 2020 •

edited by jenkins-beats-ci bot

Loading

Build stats

Test stats 🧪

Uh oh!

adriansr commented Nov 3, 2020

Uh oh!

elasticmachine commented Nov 10, 2020

Uh oh!

andrewkroh left a comment

Uh oh!

andrewkroh Nov 10, 2020

Uh oh!

adriansr Nov 10, 2020 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

adriansr commented Nov 2, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elasticmachine commented Nov 3, 2020 • edited by jenkins-beats-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

💚 Flaky test report

Test stats 🧪

Uh oh!

elasticmachine commented Nov 3, 2020 • edited by jenkins-beats-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

💚 Build Succeeded

Build stats

Test stats 🧪

Uh oh!

adriansr commented Nov 3, 2020

Uh oh!

elasticmachine commented Nov 10, 2020

Uh oh!

andrewkroh left a comment

Choose a reason for hiding this comment

Uh oh!

andrewkroh Nov 10, 2020

Choose a reason for hiding this comment

Uh oh!

adriansr Nov 10, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

adriansr commented Nov 2, 2020 •

edited

Loading

elasticmachine commented Nov 3, 2020 •

edited by jenkins-beats-ci bot

Loading

elasticmachine commented Nov 3, 2020 •

edited by jenkins-beats-ci bot

Loading

adriansr Nov 10, 2020 •

edited

Loading