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

Fix Schema cache for manifest-free events using EventName #120

Merged
merged 5 commits into from
Feb 19, 2024

Conversation

n4r1b
Copy link
Owner

@n4r1b n4r1b commented Feb 10, 2024

This fix is intended to solve issue #109.

Manifest-free events don't have an EventId nor any other value EVENT_DESCRIPTOR that would allow us to properly differentiate from one another. This is explained in the definition of the EVENT_DESCRIPTOR in evntprov.h

For manifest-free events (i.e. TraceLogging), Event.Id and Event.Version are not useful and should be ignored. Use Event name, level, keyword, and opcode for event filtering and identification.

Hence, to differentiate events this fix adds the event_name member to the SchemaKey. This value will only be obtained for events where the EventId == 0 and one of the elements from ExtendedData of the Record has the type EVENT_HEADER_EXT_TYPE_EVENT_SCHEMA_TL. In those cases we will try to parse the DataPtr to fetch the EventName.

The extended data passed when the type is EVENT_HEADER_EXT_TYPE_EVENT_SCHEMA_TL seems to not be documented but looking into TraceLoggingProvider.h it seems like it represents the data after the _tlg_EVENT_METADATA_PREAMBLE in the structure _tlgEventMetadata_t

struct _tlgEventMetadata_t
{
    UINT8 Type; // = _TlgBlobEvent4
    UCHAR Channel;
    UCHAR Level;
    UCHAR Opcode;
    ULONGLONG Keyword;
#define _tlg_EVENT_METADATA_PREAMBLE 11 // sizeof(Channel + Level + Opcode + Keyword)
    UINT16 RemainingSize; // = sizeof(RemainingSize + Tags + EventName + Fields)
    /*
    UINT8 Tags[]; // 1 or more bytes. Read until you hit a byte with high bit unset.
    char EventName[sizeof("eventName")]; // UTF-8 nul-terminated event name
    for each field {
        char FieldName[sizeof("fieldName")]; // UTF-8 nul-terminated field name
        UINT8 InType; // TlgIn_t
        UINT8 OutType; // TlgOut_t, only present if (InType & Chain) == Chain.
        UINT8 Tags[]; // Only present if OutType is present and (OutType & Chain) == Chain. Read until you hit a byte with high bit unset.
        UINT16 ValueCount;  // Only present if (InType & CountMask) == Ccount.
        UINT16 TypeInfoSize; // Only present if (InType & CountMask) == Custom.
        char TypeInfo[TypeInfoSize]; // Only present if (InType & CountMask) == Custom.
    }
    */
};

Since we only care on the EventName the parsing routine only consider the first three members (RemainingSize, Tags and EventName).

As a side note, this struct sets the type to _TlgBlobEvent4 which is defined in _TlgBlob_t with the comment "// Same as _TlgBlobEvent3 but event id is always 0."

Lastly, this all comes down to _tlgWriteCommon, where we can see how the Id and Version are set to 0.

    ((UINT32*)pDesc)[0] = *p << 24;  p += 1; // Id=0, Version=0, Channel=*p
    ((UINT32*)pDesc)[1] = *(UINT16 UNALIGNED const*)p; p += 2; // Level, Opcode, Task

TODO

  • Write test cases

@matterpreter
Copy link
Contributor

Tested against my provider that emits two different events and this is working great!

@n4r1b n4r1b requested a review from daladim February 12, 2024 16:29
@daladim
Copy link
Collaborator

daladim commented Feb 17, 2024

I'm sorry, I don't have a Windows PC near me anymore. @n4r1b , I'm afraid I won't be able to really review this MR, but I trust your judgement :-)

@daladim
Copy link
Collaborator

daladim commented Feb 17, 2024

Looks like the // TODO: Check we don't overflow the size! is already fixed (as you loop has the n < size condition), isn't it?

@n4r1b
Copy link
Owner Author

n4r1b commented Feb 19, 2024

Looks like the // TODO: Check we don't overflow the size! is already fixed (as you loop has the n < size condition), isn't it?

That's correct! Thanks for pointing it out thou, I'll remove the comment before merging the PR :)

@n4r1b
Copy link
Owner Author

n4r1b commented Feb 19, 2024

I've disable the TraceLogging test due to it being a bit flaky.
I need to debug it locally, the panic is due to the 10s timeout we set for the tests.

I'll merge for now and investigate more.

@n4r1b n4r1b merged commit a82c99a into master Feb 19, 2024
3 checks passed
@n4r1b n4r1b deleted the fix_tlg_multiple_events branch February 19, 2024 18:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants