Skip to content

Commit

Permalink
winlogbeat: fix testing on windows 11 and re-enable (#32519)
Browse files Browse the repository at this point in the history
* fix invalid write
* remove lint and provide errno for failure
* squelch error when template not available
* enable windows-11 testing
  • Loading branch information
efd6 authored Jul 27, 2022
1 parent 8123052 commit fe37716
Show file tree
Hide file tree
Showing 14 changed files with 73 additions and 74 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.next.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,8 @@ https://github.com/elastic/beats/compare/v8.2.0\...main[Check the HEAD diff]

- Powershell: Fix processing of parameter details. {pull}31833[31833]
- Security: Fix processing of sidlist, access list and access mask. {pull}31833[31833]
- Fix fatal invalid memory write on Windows 11. {issue}32469[32469] {pull}32519[32519]
- Fix handling of event formatting when no metadata is available on Windows 11. {issue}32468[32468] {pull}32519[32519]

*Functionbeat*

Expand Down
11 changes: 5 additions & 6 deletions winlogbeat/Jenkinsfile.yml
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,11 @@ stages:
platforms: ## override default labels in this specific stage.
- "windows-2012-r2"
stage: extended_win
# See https://github.com/elastic/beats/issues/32468
# windows-11:
# mage: "mage build unitTest"
# platforms: ## override default labels in this specific stage.
# - "windows-11"
# stage: extended_win
windows-11:
mage: "mage build unitTest"
platforms: ## override default labels in this specific stage.
- "windows-11"
stage: extended_win
windows-10:
mage: "mage build unitTest"
platforms: ## override default labels in this specific stage.
Expand Down
8 changes: 4 additions & 4 deletions winlogbeat/sys/wineventlog/bookmark.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,9 @@
package wineventlog

import (
"fmt"
"syscall"

"github.com/pkg/errors"
"golang.org/x/sys/windows"

"github.com/elastic/beats/v7/winlogbeat/sys"
Expand All @@ -42,8 +42,8 @@ func (b Bookmark) XML() (string, error) {
var bufferUsed uint32

err := _EvtRender(NilHandle, EvtHandle(b), EvtRenderBookmark, 0, nil, &bufferUsed, nil)
if err != nil && err != windows.ERROR_INSUFFICIENT_BUFFER {
return "", errors.Wrap(err, "failed to determine necessary buffer size for EvtRender")
if err != nil && err != windows.ERROR_INSUFFICIENT_BUFFER { //nolint:errorlint // Bad linter! This is always errno or nil.
return "", fmt.Errorf("failed to determine necessary buffer size for EvtRender: %w", err)
}

bb := sys.NewPooledByteBuffer()
Expand All @@ -52,7 +52,7 @@ func (b Bookmark) XML() (string, error) {

err = _EvtRender(NilHandle, EvtHandle(b), EvtRenderBookmark, uint32(bb.Len()), bb.PtrAt(0), &bufferUsed, nil)
if err != nil {
return "", errors.Wrap(err, "failed to render bookmark XML")
return "", fmt.Errorf("failed to render bookmark XML: %w", err)
}

return sys.UTF16BytesToString(bb.Bytes())
Expand Down
6 changes: 5 additions & 1 deletion winlogbeat/sys/wineventlog/format_message.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,9 +87,13 @@ func evtFormatMessage(metadataHandle EvtHandle, eventHandle EvtHandle, messageID
// Get a buffer from the pool and adjust its length.
bb := sys.NewPooledByteBuffer()
defer bb.Free()
// The documentation for EventFormatMessage specifies that the buffer is
// requested "in characters", and the buffer itself is LPWSTR, meaning the
// characters are WCHAR so double the value.
// https://docs.microsoft.com/en-us/windows/win32/api/winevt/nf-winevt-evtformatmessage
bb.Reserve(int(bufferUsed * 2))

err = _EvtFormatMessage(metadataHandle, eventHandle, messageID, valuesCount, valuesPtr, messageFlag, uint32(bb.Len()), bb.PtrAt(0), &bufferUsed)
err = _EvtFormatMessage(metadataHandle, eventHandle, messageID, valuesCount, valuesPtr, messageFlag, bufferUsed, bb.PtrAt(0), &bufferUsed)
switch err { //nolint:errorlint // This is an errno or nil.
case nil: // OK

Expand Down
12 changes: 5 additions & 7 deletions winlogbeat/sys/wineventlog/iterator.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,10 @@
package wineventlog

import (
"errors"
"fmt"
"sync"

"github.com/pkg/errors"
"golang.org/x/sys/windows"
)

Expand Down Expand Up @@ -148,7 +149,7 @@ func (itr *EventIterator) moreHandles() bool {
var numReturned uint32

err := itr.evtNext(itr.subscription, batchSize, &itr.handles[0], 0, 0, &numReturned)
switch err {
switch err { //nolint:errorlint // Bad linter! This is always errno or nil.
case nil:
itr.lastErr = nil
itr.active = itr.handles[:numReturned]
Expand All @@ -159,18 +160,15 @@ func (itr *EventIterator) moreHandles() bool {
itr.subscription.Close()
itr.subscription, err = itr.subscriptionFactory()
if err != nil {
itr.lastErr = errors.Wrap(err, "failed in EvtNext while trying to "+
"recover from RPC_S_INVALID_BOUND error")
itr.lastErr = fmt.Errorf("failed in EvtNext while trying to recover from RPC_S_INVALID_BOUND error: %w", err)
return false
}

// Reduce batch size and try again.
batchSize = batchSize / 2
continue
} else {
itr.lastErr = errors.Wrap(err, "failed in EvtNext (try "+
"reducing the batch size or providing a subscription "+
"factory for automatic recovery)")
itr.lastErr = fmt.Errorf("failed in EvtNext (try reducing the batch size or providing a subscription factory for automatic recovery): %w", err)
}
default:
itr.lastErr = err
Expand Down
2 changes: 1 addition & 1 deletion winlogbeat/sys/wineventlog/iterator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ import (
)

func TestEventIterator(t *testing.T) {
logp.TestingSetup()
logp.TestingSetup() //nolint:errcheck // Not needed.

writer, tearDown := createLog(t)
defer tearDown()
Expand Down
10 changes: 4 additions & 6 deletions winlogbeat/sys/wineventlog/metadata_store.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,12 @@
package wineventlog

import (
"fmt"
"strconv"
"strings"
"sync"
"text/template"

"github.com/pkg/errors"
"go.uber.org/multierr"

"github.com/elastic/beats/v7/winlogbeat/sys"
Expand Down Expand Up @@ -299,7 +299,7 @@ func newEventMetadataFromEventHandle(publisher *PublisherMetadata, eventHandle E
// publisher metadata is unavailable or is out of sync with the events.
event, err := winevent.UnmarshalXML([]byte(xml))
if err != nil {
return nil, errors.Wrap(err, "failed to unmarshal XML")
return nil, fmt.Errorf("failed to unmarshal XML: %w", err)
}

em := &EventMetadata{
Expand Down Expand Up @@ -404,8 +404,7 @@ func (em *EventMetadata) initEventMessage(itr *EventMetadataIterator, publisher

msg, err := getMessageString(publisher, NilHandle, messageID, templateInserts.Slice())
if err != nil {
return errors.Wrapf(err, "failed to get message string using message "+
"ID %v for for event ID %v", messageID, em.EventID)
return fmt.Errorf("failed to get message string using message ID %v for for event ID %v: %w", messageID, em.EventID, err)
}

return em.setMessage(msg)
Expand All @@ -419,8 +418,7 @@ func (em *EventMetadata) setMessage(msg string) error {
Delims(leftTemplateDelim, rightTemplateDelim).
Funcs(eventMessageTemplateFuncs).Parse(msg)
if err != nil {
return errors.Wrapf(err, "failed to parse message template for "+
"event ID %v (template='%v')", em.EventID, msg)
return fmt.Errorf("failed to parse message template for event ID %v (template='%v'): %w", em.EventID, msg, err)
}

// One node means there were no parameters so this will optimize that case
Expand Down
2 changes: 1 addition & 1 deletion winlogbeat/sys/wineventlog/metadata_store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import (
)

func TestPublisherMetadataStore(t *testing.T) {
logp.TestingSetup()
logp.TestingSetup() //nolint:errcheck // Not needed.

s, err := NewPublisherMetadataStore(
NilHandle,
Expand Down
52 changes: 28 additions & 24 deletions winlogbeat/sys/wineventlog/publisher_metadata.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,10 @@
package wineventlog

import (
"fmt"
"os"
"syscall"

"github.com/pkg/errors"
"go.uber.org/multierr"
"golang.org/x/sys/windows"
)
Expand Down Expand Up @@ -59,7 +59,7 @@ func NewPublisherMetadata(session EvtHandle, name string) (*PublisherMetadata, e

handle, err := _EvtOpenPublisherMetadata(session, publisherName, logFile, 0, 0)
if err != nil {
return nil, errors.Wrap(err, "failed in EvtOpenPublisherMetadata")
return nil, fmt.Errorf("failed in EvtOpenPublisherMetadata: %w", err)
}

return &PublisherMetadata{
Expand All @@ -79,7 +79,7 @@ func (m *PublisherMetadata) stringProperty(propertyID EvtPublisherMetadataProper
case nil:
return "", nil
default:
return "", errors.Errorf("unexpected data type: %T", v)
return "", fmt.Errorf("unexpected data type: %T", v)
}
}

Expand All @@ -94,7 +94,7 @@ func (m *PublisherMetadata) PublisherGUID() (windows.GUID, error) {
case nil:
return windows.GUID{}, nil
default:
return windows.GUID{}, errors.Errorf("unexpected data type: %T", v)
return windows.GUID{}, fmt.Errorf("unexpected data type: %T", v)
}
}

Expand Down Expand Up @@ -172,20 +172,20 @@ func NewMetadataKeywords(publisherMetadataHandle EvtHandle) ([]MetadataKeyword,

arrayHandle, ok := v.(EvtObjectArrayPropertyHandle)
if !ok {
return nil, errors.Errorf("unexpected handle type: %T", v)
return nil, fmt.Errorf("unexpected handle type: %T", v)
}
defer arrayHandle.Close()

arrayLen, err := EvtGetObjectArraySize(arrayHandle)
if err != nil {
return nil, errors.Wrap(err, "failed to get keyword array length")
return nil, fmt.Errorf("failed to get keyword array length: %w", err)
}

var values []MetadataKeyword
for i := uint32(0); i < arrayLen; i++ {
md, err := NewMetadataKeyword(publisherMetadataHandle, arrayHandle, i)
if err != nil {
return nil, errors.Wrapf(err, "failed to get keyword at array index %v", i)
return nil, fmt.Errorf("failed to get keyword at array index %v: %w", i, err)
}

values = append(values, *md)
Expand Down Expand Up @@ -245,20 +245,20 @@ func NewMetadataOpcodes(publisherMetadataHandle EvtHandle) ([]MetadataOpcode, er

arrayHandle, ok := v.(EvtObjectArrayPropertyHandle)
if !ok {
return nil, errors.Errorf("unexpected handle type: %T", v)
return nil, fmt.Errorf("unexpected handle type: %T", v)
}
defer arrayHandle.Close()

arrayLen, err := EvtGetObjectArraySize(arrayHandle)
if err != nil {
return nil, errors.Wrap(err, "failed to get opcode array length")
return nil, fmt.Errorf("failed to get opcode array length: %w", err)
}

var values []MetadataOpcode
for i := uint32(0); i < arrayLen; i++ {
md, err := NewMetadataOpcode(publisherMetadataHandle, arrayHandle, i)
if err != nil {
return nil, errors.Wrapf(err, "failed to get opcode at array index %v", i)
return nil, fmt.Errorf("failed to get opcode at array index %v: %w", i, err)
}

values = append(values, *md)
Expand Down Expand Up @@ -318,20 +318,20 @@ func NewMetadataLevels(publisherMetadataHandle EvtHandle) ([]MetadataLevel, erro

arrayHandle, ok := v.(EvtObjectArrayPropertyHandle)
if !ok {
return nil, errors.Errorf("unexpected handle type: %T", v)
return nil, fmt.Errorf("unexpected handle type: %T", v)
}
defer arrayHandle.Close()

arrayLen, err := EvtGetObjectArraySize(arrayHandle)
if err != nil {
return nil, errors.Wrap(err, "failed to get level array length")
return nil, fmt.Errorf("failed to get level array length: %w", err)
}

var values []MetadataLevel
for i := uint32(0); i < arrayLen; i++ {
md, err := NewMetadataLevel(publisherMetadataHandle, arrayHandle, i)
if err != nil {
return nil, errors.Wrapf(err, "failed to get level at array index %v", i)
return nil, fmt.Errorf("failed to get level at array index %v: %w", i, err)
}

values = append(values, *md)
Expand Down Expand Up @@ -392,20 +392,20 @@ func NewMetadataTasks(publisherMetadataHandle EvtHandle) ([]MetadataTask, error)

arrayHandle, ok := v.(EvtObjectArrayPropertyHandle)
if !ok {
return nil, errors.Errorf("unexpected handle type: %T", v)
return nil, fmt.Errorf("unexpected handle type: %T", v)
}
defer arrayHandle.Close()

arrayLen, err := EvtGetObjectArraySize(arrayHandle)
if err != nil {
return nil, errors.Wrap(err, "failed to get task array length")
return nil, fmt.Errorf("failed to get task array length: %w", err)
}

var values []MetadataTask
for i := uint32(0); i < arrayLen; i++ {
md, err := NewMetadataTask(publisherMetadataHandle, arrayHandle, i)
if err != nil {
return nil, errors.Wrapf(err, "failed to get task at array index %v", i)
return nil, fmt.Errorf("failed to get task at array index %v: %w", i, err)
}

values = append(values, *md)
Expand Down Expand Up @@ -473,20 +473,20 @@ func NewMetadataChannels(publisherMetadataHandle EvtHandle) ([]MetadataChannel,

arrayHandle, ok := v.(EvtObjectArrayPropertyHandle)
if !ok {
return nil, errors.Errorf("unexpected handle type: %T", v)
return nil, fmt.Errorf("unexpected handle type: %T", v)
}
defer arrayHandle.Close()

arrayLen, err := EvtGetObjectArraySize(arrayHandle)
if err != nil {
return nil, errors.Wrap(err, "failed to get task array length")
return nil, fmt.Errorf("failed to get task array length: %w", err)
}

var values []MetadataChannel
for i := uint32(0); i < arrayLen; i++ {
md, err := NewMetadataChannel(publisherMetadataHandle, arrayHandle, i)
if err != nil {
return nil, errors.Wrapf(err, "failed to get task at array index %v", i)
return nil, fmt.Errorf("failed to get task at array index %v: %w", i, err)
}

values = append(values, *md)
Expand Down Expand Up @@ -548,8 +548,8 @@ type EventMetadataIterator struct {

func NewEventMetadataIterator(publisher *PublisherMetadata) (*EventMetadataIterator, error) {
eventMetadataEnumHandle, err := _EvtOpenEventMetadataEnum(publisher.Handle, 0)
if err != nil {
return nil, errors.Wrap(err, "failed to open event metadata enumerator with EvtOpenEventMetadataEnum")
if err != nil && err != windows.ERROR_FILE_NOT_FOUND { //nolint:errorlint // Bad linter! This is always errno or nil.
return nil, fmt.Errorf("failed to open event metadata enumerator with EvtOpenEventMetadataEnum: %w (%#[1]v)", err)
}

return &EventMetadataIterator{
Expand All @@ -569,14 +569,18 @@ func (itr *EventMetadataIterator) Close() error {
// no more items or an error occurred. You should call Err() to check for an
// error.
func (itr *EventMetadataIterator) Next() bool {
if itr.eventMetadataEnumHandle == 0 {
// This is only the case when we could not find the event metadata file.
return false
}
// Close existing handle.
itr.currentEvent.Close()

var err error
itr.currentEvent, err = _EvtNextEventMetadata(itr.eventMetadataEnumHandle, 0)
if err != nil {
if err != windows.ERROR_NO_MORE_ITEMS {
itr.lastErr = errors.Wrap(err, "failed advancing to next event metadata handle")
if err != windows.ERROR_NO_MORE_ITEMS { //nolint:errorlint // Bad linter! This is always errno or nil.
itr.lastErr = fmt.Errorf("failed advancing to next event metadata handle: %w", err)
}
return false
}
Expand All @@ -589,7 +593,7 @@ func (itr *EventMetadataIterator) Err() error {
}

func typeCastError(expected, got interface{}) error {
return errors.Errorf("wrong type for property. expected:%T got:%T", expected, got)
return fmt.Errorf("wrong type for property. expected:%T got:%T", expected, got)
}

func (itr *EventMetadataIterator) uint32Property(propertyID EvtEventMetadataPropertyID) (uint32, error) {
Expand Down
1 change: 0 additions & 1 deletion winlogbeat/sys/wineventlog/renderer.go
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,6 @@ func (r *Renderer) renderSystem(handle EvtHandle, event *winevent.Event) error {
continue
}

//nolint:errcheck // Bad linter!
switch property {
case EvtSystemProviderName:
event.Provider.Name = data.(string)
Expand Down
Loading

0 comments on commit fe37716

Please sign in to comment.