From aeea9c49b37ce32e35ffb6b594ed82654c9c9eab Mon Sep 17 00:00:00 2001 From: Adrian Serrano Date: Thu, 21 Mar 2019 04:05:05 +0100 Subject: [PATCH 1/4] Fix Winlogbeat escaping CRLF sequences Previous fix (#11006) made Winlogbeat escape CRLF control characters which are expected in Windows event logs. Fixes #11328 --- winlogbeat/sys/event_test.go | 21 ++++++----------- winlogbeat/sys/xmlreader.go | 2 +- winlogbeat/tests/system/test_eventlogging.py | 24 ++++++++++++++++++++ winlogbeat/tests/system/test_wineventlog.py | 24 ++++++++++++++++++++ 4 files changed, 56 insertions(+), 15 deletions(-) diff --git a/winlogbeat/sys/event_test.go b/winlogbeat/sys/event_test.go index c77c05c2ea3..2c04fce23a4 100644 --- a/winlogbeat/sys/event_test.go +++ b/winlogbeat/sys/event_test.go @@ -21,6 +21,7 @@ import ( "encoding/json" "encoding/xml" "fmt" + "strings" "testing" "time" @@ -168,21 +169,13 @@ func TestXML(t *testing.T) { } } +// Tests that control characters other than CR and LF are escaped +// when the event is decoded. func TestInvalidXML(t *testing.T) { - eventXML := fmt.Sprintf(` - - - - {00000000-0000-0000-0000-000000000000} - じゃあ宇宙カウボーイ。。。%s - - - -`, "\x1b") - _, err := UnmarshalEventXML([]byte(eventXML)) - if !assert.NoError(t, err) { - assert.Equal(t, err.Error(), "XML syntax error on line 6: illegal character code U+001B") - } + evXML := strings.Replace(allXML, "%1", " \n\x1b", -1) + ev, err := UnmarshalEventXML([]byte(evXML)) + assert.Equal(t, nil, err) + assert.Equal(t, "Creating WSMan shell on server with ResourceUri: \r\n\\u001b", ev.Message) } func BenchmarkXMLUnmarshal(b *testing.B) { diff --git a/winlogbeat/sys/xmlreader.go b/winlogbeat/sys/xmlreader.go index 8eb23e1ce14..ba51c90dcd5 100644 --- a/winlogbeat/sys/xmlreader.go +++ b/winlogbeat/sys/xmlreader.go @@ -58,7 +58,7 @@ func (r *xmlSafeReader) Read(d []byte) (n int, err error) { } for i := 0; i < len(r.buf); { code, size := utf8.DecodeRune(r.buf[i:]) - if unicode.IsControl(code) { + if !unicode.IsSpace(code) && unicode.IsControl(code) { n = copy(d, r.buf[:i]) r.buf = r.buf[n+1:] r.code = []byte(fmt.Sprintf("\\u%04x", code)) diff --git a/winlogbeat/tests/system/test_eventlogging.py b/winlogbeat/tests/system/test_eventlogging.py index 455d154ecc0..f3c5cfd02d8 100644 --- a/winlogbeat/tests/system/test_eventlogging.py +++ b/winlogbeat/tests/system/test_eventlogging.py @@ -234,3 +234,27 @@ def test_processors(self): evts = self.read_events(config) self.assertTrue(len(evts), 1) self.assertNotIn("message", evts[0]) + + def test_multiline_events(self): + """ + eventlogging - Event with newlines and control characters + """ + msg = """ +A trusted logon process has been registered with the Local Security Authority. +This logon process will be trusted to submit logon requests. + +Subject: + +Security ID: SYSTEM +Account Name: MS4\x1e$ +Account Domain: WORKGROUP +Logon ID: 0x3e7 +Logon Process Name: IKE""" + self.write_event_log(msg) + evts = self.read_events() + self.assertTrue(len(evts), 1) + self.assertEqual(unicode(self.api), evts[0]["winlog.api"], evts[0]) + self.assertNotIn("event.original", evts[0], evts[0]) + self.assertIn("message", evts[0], evts[0]) + self.assertNotIn("\\u000a", evts[0]["message"]) + self.assertEqual(unicode(msg), evts[0]["message"].decode('unicode-escape'), evts[0]) diff --git a/winlogbeat/tests/system/test_wineventlog.py b/winlogbeat/tests/system/test_wineventlog.py index df649fd1453..39b596f72c4 100644 --- a/winlogbeat/tests/system/test_wineventlog.py +++ b/winlogbeat/tests/system/test_wineventlog.py @@ -388,3 +388,27 @@ def test_processors(self): evts = self.read_events(config) self.assertTrue(len(evts), 1) self.assertNotIn("message", evts[0]) + + def test_multiline_events(self): + """ + wineventlog - Event with newlines and control characters + """ + msg = """ +A trusted logon process has been registered with the Local Security Authority. +This logon process will be trusted to submit logon requests. + +Subject: + +Security ID: SYSTEM +Account Name: MS4\x1e$ +Account Domain: WORKGROUP +Logon ID: 0x3e7 +Logon Process Name: IKE""" + self.write_event_log(msg) + evts = self.read_events() + self.assertTrue(len(evts), 1) + self.assertEqual(unicode(self.api), evts[0]["winlog.api"], evts[0]) + self.assertNotIn("event.original", evts[0], evts[0]) + self.assertIn("message", evts[0], evts[0]) + self.assertNotIn("\\u000a", evts[0]["message"]) + self.assertEqual(unicode(msg), evts[0]["message"].decode('unicode-escape'), evts[0]) From 59d4dd9eff9df23b0963e0dbc5576c6d65501e36 Mon Sep 17 00:00:00 2001 From: Adrian Serrano Date: Thu, 21 Mar 2019 04:46:34 +0100 Subject: [PATCH 2/4] Add tag character to unit test --- winlogbeat/sys/event_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/winlogbeat/sys/event_test.go b/winlogbeat/sys/event_test.go index 2c04fce23a4..c9c1af3a756 100644 --- a/winlogbeat/sys/event_test.go +++ b/winlogbeat/sys/event_test.go @@ -172,10 +172,10 @@ func TestXML(t *testing.T) { // Tests that control characters other than CR and LF are escaped // when the event is decoded. func TestInvalidXML(t *testing.T) { - evXML := strings.Replace(allXML, "%1", " \n\x1b", -1) + evXML := strings.Replace(allXML, "%1", "\t \n\x1b", -1) ev, err := UnmarshalEventXML([]byte(evXML)) assert.Equal(t, nil, err) - assert.Equal(t, "Creating WSMan shell on server with ResourceUri: \r\n\\u001b", ev.Message) + assert.Equal(t, "Creating WSMan shell on server with ResourceUri: \t\r\n\\u001b", ev.Message) } func BenchmarkXMLUnmarshal(b *testing.B) { From c43651eee4f300a407e8d5c09ff619fa8f23bc8e Mon Sep 17 00:00:00 2001 From: Adrian Serrano Date: Thu, 21 Mar 2019 05:10:09 +0100 Subject: [PATCH 3/4] Make assertions clearer --- winlogbeat/tests/system/test_eventlogging.py | 8 ++++---- winlogbeat/tests/system/test_wineventlog.py | 10 +++++----- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/winlogbeat/tests/system/test_eventlogging.py b/winlogbeat/tests/system/test_eventlogging.py index f3c5cfd02d8..e772b5b9ceb 100644 --- a/winlogbeat/tests/system/test_eventlogging.py +++ b/winlogbeat/tests/system/test_eventlogging.py @@ -254,7 +254,7 @@ def test_multiline_events(self): evts = self.read_events() self.assertTrue(len(evts), 1) self.assertEqual(unicode(self.api), evts[0]["winlog.api"], evts[0]) - self.assertNotIn("event.original", evts[0], evts[0]) - self.assertIn("message", evts[0], evts[0]) - self.assertNotIn("\\u000a", evts[0]["message"]) - self.assertEqual(unicode(msg), evts[0]["message"].decode('unicode-escape'), evts[0]) + self.assertNotIn("event.original", evts[0], msg=evts[0]) + self.assertIn("message", evts[0], msg=evts[0]) + self.assertNotIn("\\u000a", evts[0]["message"], msg=evts[0]) + self.assertEqual(unicode(msg), evts[0]["message"].decode('unicode-escape'), msg=evts[0]) diff --git a/winlogbeat/tests/system/test_wineventlog.py b/winlogbeat/tests/system/test_wineventlog.py index 39b596f72c4..61649c86507 100644 --- a/winlogbeat/tests/system/test_wineventlog.py +++ b/winlogbeat/tests/system/test_wineventlog.py @@ -407,8 +407,8 @@ def test_multiline_events(self): self.write_event_log(msg) evts = self.read_events() self.assertTrue(len(evts), 1) - self.assertEqual(unicode(self.api), evts[0]["winlog.api"], evts[0]) - self.assertNotIn("event.original", evts[0], evts[0]) - self.assertIn("message", evts[0], evts[0]) - self.assertNotIn("\\u000a", evts[0]["message"]) - self.assertEqual(unicode(msg), evts[0]["message"].decode('unicode-escape'), evts[0]) + self.assertEqual(unicode(self.api), evts[0]["winlog.api"], msg=evts[0]) + self.assertNotIn("event.original", evts[0], msg=evts[0]) + self.assertIn("message", evts[0], msg=evts[0]) + self.assertNotIn("\\u000a", evts[0]["message"], msg=evts[0]) + self.assertEqual(unicode(msg), evts[0]["message"].decode('unicode-escape'), msg=evts[0]) From a0b8e13c98dc1db801ba75b42f49c78e64c09950 Mon Sep 17 00:00:00 2001 From: Adrian Serrano Date: Thu, 21 Mar 2019 15:00:16 +0100 Subject: [PATCH 4/4] Update CHANGELOG --- CHANGELOG.next.asciidoc | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.next.asciidoc b/CHANGELOG.next.asciidoc index 0c835775af5..a21c9a46ef4 100644 --- a/CHANGELOG.next.asciidoc +++ b/CHANGELOG.next.asciidoc @@ -222,6 +222,7 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d *Winlogbeat* - Prevent Winlogbeat from dropping events with invalid XML. {pull}11006{11006} +- Fix Winlogbeat escaping CR, LF and TAB characters. {issue}11328[11328] {pull}11357[11357] *Functionbeat*