-
Notifications
You must be signed in to change notification settings - Fork 334
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
Refactored setupapi parser #2717 #2718
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2718 +/- ##
==========================================
+ Coverage 86.21% 86.28% +0.07%
==========================================
Files 481 487 +6
Lines 32788 33062 +274
==========================================
+ Hits 28268 28529 +261
- Misses 4520 4533 +13
Continue to review full report at Codecov.
|
plaso/parsers/setupapi.py
Outdated
structure (pyparsing.ParseResults): structure of tokens derived from | ||
log entry. | ||
time_structure (pyparsing.ParseResults): structure of tokens derived from | ||
a setupapi time stamp. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
time stamp => date and time value
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The setupapi documents call this field a timestamp: https://docs.microsoft.com/en-us/windows-hardware/drivers/install/format-of-a-text-log-section-header
Clarified this.
plaso/parsers/setupapi.py
Outdated
|
||
Returns: | ||
dfdatetime.TimeElements: date and time extracted from the value or None | ||
if the value does not represent a valid string. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a valid string => a valid date and time value
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
plaso/parsers/setupapi.py
Outdated
|
||
Returns: | ||
bool: True if this is the correct parser, False otherwise. | ||
""" | ||
try: | ||
structure = self._SETUPAPI_LINE.parseString(lines) | ||
_ = self._LOG_HEADER_START.parseString(line) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove '_ =', it is not needed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
plaso/parsers/winfirewall.py
Outdated
@@ -224,7 +224,7 @@ def VerifyStructure(self, parser_mediator, line): | |||
""" | |||
# TODO: Examine other versions of the file format and if this parser should | |||
# support them. | |||
return line == '#Version: 1.5' | |||
return '#Version: 1.5' in line |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why change this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because of the whitespace matching. Changed this to just have the newline.
tests/parsers/setupapi.py
Outdated
@@ -101,8 +101,8 @@ def testParseSetupLog(self): | |||
|
|||
self.CheckTimestamp(event.timestamp, '2015-11-22 17:53:29.305000') | |||
|
|||
expected_message = ('Setup Plug and Play Device Install - START') | |||
expected_short_message = ('START - Setup Plug and Play Device Install') | |||
expected_message = ('Setup Plug and Play Device Install') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove bounding parenthesis
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please address comments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Should we add this to the win_gen preset?
@js-forensic - good idea, I thought this had already been done |
plaso/parsers/winfirewall.py
Outdated
@@ -224,7 +224,8 @@ def VerifyStructure(self, parser_mediator, line): | |||
""" | |||
# TODO: Examine other versions of the file format and if this parser should | |||
# support them. | |||
return line == '#Version: 1.5' | |||
stripped_line = line.strip() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
strip() => rstrip()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please make sure that the parser does not use state from a previous file it parsed
tox tests are failing:
I'll restart them, no luck, seems to be an issue with a dependency #2727 |
Ping for @joachimmetz |
plaso/parsers/setupapi.py
Outdated
|
||
def _ParseRecordLogline(self, parser_mediator, structure): | ||
"""Parses a logline record structure and produces events. | ||
def _BuildDatetime(self, time_structure): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change name of this method to _GetDateTime
or equiv to keep it consistent with other parsers
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some small remaining questions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Also changed the EncodedTextReader to not strip every line and updated parsers that relied on this behavior.
There's also more changes that I'd like make to the parser, but I've tried to keep changes to the tests minimal in this first round.