[plaso] Add parser and formatter for Trend Micro antivirus.

[plaso] Add parser and formatter for Trend Micro antivirus.

[ep, 2018-02-09 16:54:35]

There are several mistakes that I would like to correct here. Please disregard
until the next patch set is pushed. Thanks, and sorry for the noise!

[ep, 2018-02-12 09:25:14]

Code updated.

[onager, 2018-02-14 16:10:34]

https://codereview.appspot.com/335560043/diff/20001/plaso/formatters/trendmic...
File plaso/formatters/trendmicroav.py (right):

https://codereview.appspot.com/335560043/diff/20001/plaso/formatters/trendmic...
plaso/formatters/trendmicroav.py:20: '-> {action}',
Using "->" is a little weird, please use a : instead, for better consistency.

https://codereview.appspot.com/335560043/diff/20001/plaso/parsers/dsv_parser.py
File plaso/parsers/dsv_parser.py (right):

https://codereview.appspot.com/335560043/diff/20001/plaso/parsers/dsv_parser....
plaso/parsers/dsv_parser.py:86: def _CreateDictReader(self, parser_mediator,
line_reader):
Please rebase this on the current master

https://codereview.appspot.com/335560043/diff/20001/plaso/parsers/trendmicroa...
File plaso/parsers/trendmicroav.py (right):

https://codereview.appspot.com/335560043/diff/20001/plaso/parsers/trendmicroa...
plaso/parsers/trendmicroav.py:20: _SCAN_RESULTS = {
Please move these to the formatter.

https://codereview.appspot.com/335560043/diff/20001/plaso/parsers/trendmicroa...
plaso/parsers/trendmicroav.py:27: 6: "Falure (move)",
Failure

https://codereview.appspot.com/335560043/diff/20001/plaso/parsers/trendmicroa...
plaso/parsers/trendmicroav.py:40: _SCAN_TYPES = {
Also in the formatter.

https://codereview.appspot.com/335560043/diff/20001/plaso/parsers/trendmicroa...
plaso/parsers/trendmicroav.py:57: trigger_location (str): trigger location.
Trigger location and username aren't set in init - please remove them from the
attributes, or set them in init.

https://codereview.appspot.com/335560043/diff/20001/plaso/parsers/trendmicroa...
plaso/parsers/trendmicroav.py:76: DELIMITER = '<;>'
+blank line after docstring

https://codereview.appspot.com/335560043/diff/20001/plaso/parsers/trendmicroa...
plaso/parsers/trendmicroav.py:79: MIN_COLUMNS = None
This doesn't work, the DSVParser checks the numbers of columns match
https://github.com/log2timeline/plaso/blob/d64d622e91a892296989645de871de1679...

https://codereview.appspot.com/335560043/diff/20001/plaso/parsers/trendmicroa...
plaso/parsers/trendmicroav.py:82: """Iterate over the log lines and provide a
reader for the values.
Please re-write this docstring:

1) Add a Yields: section
2) Move the format description to the class docstring
3) The single line docstring should be active "Iterates..."

https://codereview.appspot.com/335560043/diff/20001/plaso/parsers/trendmicroa...
plaso/parsers/trendmicroav.py:95: values =
line.decode(self._encoding).strip().split(self.DELIMITER)
Break this down into multiple lines to make debugging easier:

eg.

try:
  line = line.decode(...)
except Unicode...

lines = line.strip(...)
values = lines.split(..)

https://codereview.appspot.com/335560043/diff/20001/plaso/parsers/trendmicroa...
plaso/parsers/trendmicroav.py:96: except UnicodeDecodeError, exception:
UnicodeDecodeError as exception

https://codereview.appspot.com/335560043/diff/20001/plaso/parsers/trendmicroa...
plaso/parsers/trendmicroav.py:132: else:
Remove the else, and dedent this block.

https://codereview.appspot.com/335560043/diff/20001/plaso/parsers/trendmicroa...
plaso/parsers/trendmicroav.py:151: The date value is an 8-character string in
the YYYYMMDD format.
Move these descriptions to the argument docstrings.

https://codereview.appspot.com/335560043/diff/20001/plaso/parsers/trendmicroa...
plaso/parsers/trendmicroav.py:157: timestamp (str): Unix time in seconds since
the epoch.
timestamps isn't an argument to this function

https://codereview.appspot.com/335560043/diff/20001/plaso/parsers/trendmicroa...
plaso/parsers/trendmicroav.py:178: return timelib.Timestamp.FromTimeParts(
Use dfdatetime instead, this function is on the way out:
https://codereview.appspot.com/332630043/

https://codereview.appspot.com/335560043/diff/20001/plaso/parsers/trendmicroa...
plaso/parsers/trendmicroav.py:193: kwargs['encoding'] = 'cp1252'
+docstring

https://codereview.appspot.com/335560043/diff/20001/tests/parsers/trendmicroa...
File tests/parsers/trendmicroav.py (right):

https://codereview.appspot.com/335560043/diff/20001/tests/parsers/trendmicroa...
tests/parsers/trendmicroav.py:34: expected_timestamp =
timelib.Timestamp.CopyFromString(
Please use the newer checktimestamp method.

[ep, 2018-03-05 16:13:11]

Code updated.

[ep, 2018-03-05 16:14:01]

https://codereview.appspot.com/335560043/diff/20001/plaso/formatters/trendmic...
File plaso/formatters/trendmicroav.py (right):

https://codereview.appspot.com/335560043/diff/20001/plaso/formatters/trendmic...
plaso/formatters/trendmicroav.py:20: '-> {action}',
On 2018/02/14 16:10:33, onager wrote:
> Using "->" is a little weird, please use a : instead, for better consistency.

Done.

https://codereview.appspot.com/335560043/diff/20001/plaso/parsers/dsv_parser.py
File plaso/parsers/dsv_parser.py (right):

https://codereview.appspot.com/335560043/diff/20001/plaso/parsers/dsv_parser....
plaso/parsers/dsv_parser.py:86: def _CreateDictReader(self, parser_mediator,
line_reader):
On 2018/02/14 16:10:33, onager wrote:
> Please rebase this on the current master

Done.

https://codereview.appspot.com/335560043/diff/20001/plaso/parsers/trendmicroa...
File plaso/parsers/trendmicroav.py (right):

https://codereview.appspot.com/335560043/diff/20001/plaso/parsers/trendmicroa...
plaso/parsers/trendmicroav.py:20: _SCAN_RESULTS = {
On 2018/02/14 16:10:33, onager wrote:
> Please move these to the formatter.

Done.

https://codereview.appspot.com/335560043/diff/20001/plaso/parsers/trendmicroa...
plaso/parsers/trendmicroav.py:27: 6: "Falure (move)",
On 2018/02/14 16:10:33, onager wrote:
> Failure

Done.

https://codereview.appspot.com/335560043/diff/20001/plaso/parsers/trendmicroa...
plaso/parsers/trendmicroav.py:40: _SCAN_TYPES = {
On 2018/02/14 16:10:33, onager wrote:
> Also in the formatter.

Done.

https://codereview.appspot.com/335560043/diff/20001/plaso/parsers/trendmicroa...
plaso/parsers/trendmicroav.py:57: trigger_location (str): trigger location.
On 2018/02/14 16:10:34, onager wrote:
> Trigger location and username aren't set in init - please remove them from the
> attributes, or set them in init.

Done.

https://codereview.appspot.com/335560043/diff/20001/plaso/parsers/trendmicroa...
plaso/parsers/trendmicroav.py:76: DELIMITER = '<;>'
On 2018/02/14 16:10:34, onager wrote:
> +blank line after docstring

Done.

https://codereview.appspot.com/335560043/diff/20001/plaso/parsers/trendmicroa...
plaso/parsers/trendmicroav.py:79: MIN_COLUMNS = None
On 2018/02/14 16:10:33, onager wrote:
> This doesn't work, the DSVParser checks the numbers of columns match
>
https://github.com/log2timeline/plaso/blob/d64d622e91a892296989645de871de1679...
> 

I believe this is OK; if there are less columns than those in COLUMNS, the zip()
in _CreateDictReader() will fill them with None.

https://codereview.appspot.com/335560043/diff/20001/plaso/parsers/trendmicroa...
plaso/parsers/trendmicroav.py:82: """Iterate over the log lines and provide a
reader for the values.
On 2018/02/14 16:10:34, onager wrote:
> Please re-write this docstring:
> 
> 1) Add a Yields: section
> 2) Move the format description to the class docstring
> 3) The single line docstring should be active "Iterates..."

Done.

https://codereview.appspot.com/335560043/diff/20001/plaso/parsers/trendmicroa...
plaso/parsers/trendmicroav.py:95: values =
line.decode(self._encoding).strip().split(self.DELIMITER)
On 2018/02/14 16:10:33, onager wrote:
> Break this down into multiple lines to make debugging easier:
> 
> eg.
> 
> try:
>   line = line.decode(...)
> except Unicode...
> 
> lines = line.strip(...)
> values = lines.split(..)

Done.

https://codereview.appspot.com/335560043/diff/20001/plaso/parsers/trendmicroa...
plaso/parsers/trendmicroav.py:96: except UnicodeDecodeError, exception:
On 2018/02/14 16:10:33, onager wrote:
> UnicodeDecodeError as exception

Done.

https://codereview.appspot.com/335560043/diff/20001/plaso/parsers/trendmicroa...
plaso/parsers/trendmicroav.py:132: else:
On 2018/02/14 16:10:33, onager wrote:
> Remove the else, and dedent this block.

Done.

https://codereview.appspot.com/335560043/diff/20001/plaso/parsers/trendmicroa...
plaso/parsers/trendmicroav.py:151: The date value is an 8-character string in
the YYYYMMDD format.
On 2018/02/14 16:10:33, onager wrote:
> Move these descriptions to the argument docstrings.

Done.

https://codereview.appspot.com/335560043/diff/20001/plaso/parsers/trendmicroa...
plaso/parsers/trendmicroav.py:157: timestamp (str): Unix time in seconds since
the epoch.
On 2018/02/14 16:10:33, onager wrote:
> timestamps isn't an argument to this function

Done.

https://codereview.appspot.com/335560043/diff/20001/plaso/parsers/trendmicroa...
plaso/parsers/trendmicroav.py:178: return timelib.Timestamp.FromTimeParts(
On 2018/02/14 16:10:33, onager wrote:
> Use dfdatetime instead, this function is on the way out:
> https://codereview.appspot.com/332630043/

Done.

https://codereview.appspot.com/335560043/diff/20001/plaso/parsers/trendmicroa...
plaso/parsers/trendmicroav.py:193: kwargs['encoding'] = 'cp1252'
On 2018/02/14 16:10:33, onager wrote:
> +docstring

Done.

https://codereview.appspot.com/335560043/diff/20001/tests/parsers/trendmicroa...
File tests/parsers/trendmicroav.py (right):

https://codereview.appspot.com/335560043/diff/20001/tests/parsers/trendmicroa...
tests/parsers/trendmicroav.py:34: expected_timestamp =
timelib.Timestamp.CopyFromString(
On 2018/02/14 16:10:34, onager wrote:
> Please use the newer checktimestamp method.

Done.

[onager, 2018-03-08 10:59:12]

Just a few nits left.

https://codereview.appspot.com/335560043/diff/40001/plaso/parsers/trendmicroa...
File plaso/parsers/trendmicroav.py (right):

https://codereview.appspot.com/335560043/diff/40001/plaso/parsers/trendmicroa...
plaso/parsers/trendmicroav.py:20: from plaso.formatters import trendmicroav as
formatter
Don't import the formatter here, let it expand the values read from the file.

https://codereview.appspot.com/335560043/diff/40001/plaso/parsers/trendmicroa...
plaso/parsers/trendmicroav.py:132: timestamp (str): Unix time in seconds since
the epoch.
Remove the timestamp and timezone docstrings - these args no longer exist

https://codereview.appspot.com/335560043/diff/40001/plaso/parsers/trendmicroa...
plaso/parsers/trendmicroav.py:139: int: a timestamp integer containing the
number of micro seconds since
This now returns a TimeElements, not an int.

https://codereview.appspot.com/335560043/diff/40001/plaso/parsers/trendmicroa...
plaso/parsers/trendmicroav.py:143: TimestampError: if the timestamp is badly
formed or unable to transfer
There's no timestamp to be badly formed any more.

https://codereview.appspot.com/335560043/diff/40001/plaso/parsers/trendmicroa...
plaso/parsers/trendmicroav.py:148: return None
The docstring seems to suggest this should raise - please harmonize the
docstring and the behavior here.

https://codereview.appspot.com/335560043/diff/40001/plaso/parsers/trendmicroa...
plaso/parsers/trendmicroav.py:207: event_data.action =
formatter.SCAN_RESULTS[int(row['action'])]
Don't do this - store the action as an int in the event, then expand it in the
formatter, when the event is being exported.

https://codereview.appspot.com/335560043/diff/40001/plaso/parsers/trendmicroa...
plaso/parsers/trendmicroav.py:242: if action not in formatter.SCAN_RESULTS:
OK, I can see why you might want to import the formatter here. This is better
than duplicated the valid keys I think.

[ep, 2018-03-08 13:39:28]

Code updated.

[ep, 2018-03-08 13:40:13]

https://codereview.appspot.com/335560043/diff/40001/plaso/parsers/trendmicroa...
File plaso/parsers/trendmicroav.py (right):

https://codereview.appspot.com/335560043/diff/40001/plaso/parsers/trendmicroa...
plaso/parsers/trendmicroav.py:20: from plaso.formatters import trendmicroav as
formatter
On 2018/03/08 10:59:12, onager wrote:
> Don't import the formatter here, let it expand the values read from the file.

(keeping it as per your note later on in the review.)

https://codereview.appspot.com/335560043/diff/40001/plaso/parsers/trendmicroa...
plaso/parsers/trendmicroav.py:132: timestamp (str): Unix time in seconds since
the epoch.
On 2018/03/08 10:59:11, onager wrote:
> Remove the timestamp and timezone docstrings - these args no longer exist

Done.

https://codereview.appspot.com/335560043/diff/40001/plaso/parsers/trendmicroa...
plaso/parsers/trendmicroav.py:139: int: a timestamp integer containing the
number of micro seconds since
On 2018/03/08 10:59:12, onager wrote:
> This now returns a TimeElements, not an int.

Done.

https://codereview.appspot.com/335560043/diff/40001/plaso/parsers/trendmicroa...
plaso/parsers/trendmicroav.py:143: TimestampError: if the timestamp is badly
formed or unable to transfer
On 2018/03/08 10:59:11, onager wrote:
> There's no timestamp to be badly formed any more.

Done (and replaced with ValueError, which can crop up during the str->int
conversions or in dfdatetime. If it is raised, it's caught by the calling
function.)

https://codereview.appspot.com/335560043/diff/40001/plaso/parsers/trendmicroa...
plaso/parsers/trendmicroav.py:148: return None
On 2018/03/08 10:59:12, onager wrote:
> The docstring seems to suggest this should raise - please harmonize the
> docstring and the behavior here.

Done.

https://codereview.appspot.com/335560043/diff/40001/plaso/parsers/trendmicroa...
plaso/parsers/trendmicroav.py:207: event_data.action =
formatter.SCAN_RESULTS[int(row['action'])]
On 2018/03/08 10:59:11, onager wrote:
> Don't do this - store the action as an int in the event, then expand it in the
> formatter, when the event is being exported.

Done.

https://codereview.appspot.com/335560043/diff/40001/plaso/parsers/trendmicroa...
plaso/parsers/trendmicroav.py:242: if action not in formatter.SCAN_RESULTS:
On 2018/03/08 10:59:11, onager wrote:
> OK, I can see why you might want to import the formatter here. This is better
> than duplicated the valid keys I think.

Acknowledged.

[onager, 2018-03-08 14:46:21]

https://codereview.appspot.com/335560043/diff/60001/plaso/formatters/trendmic...
File plaso/formatters/trendmicroav.py (right):

https://codereview.appspot.com/335560043/diff/60001/plaso/formatters/trendmic...
plaso/formatters/trendmicroav.py:69: def _ConditionalFormatMessages(self,
event_values):
Rather than overriding _ConditionalFormatMessages, move this to GetMessages
instead, like the other formatters do, just so this is a little more consistent.


Can you also open an issue to create a formatter superclass for expanding
values, so we can generalize this as discussed.

[ep, 2018-03-08 15:01:20]

Code updated.

[ep, 2018-03-08 15:01:53]

https://codereview.appspot.com/335560043/diff/60001/plaso/formatters/trendmic...
File plaso/formatters/trendmicroav.py (right):

https://codereview.appspot.com/335560043/diff/60001/plaso/formatters/trendmic...
plaso/formatters/trendmicroav.py:69: def _ConditionalFormatMessages(self,
event_values):
On 2018/03/08 14:46:21, onager wrote:
> Rather than overriding _ConditionalFormatMessages, move this to GetMessages
> instead, like the other formatters do, just so this is a little more
consistent.
> 
> 
> Can you also open an issue to create a formatter superclass for expanding
> values, so we can generalize this as discussed.

Done.

[ep, 2018-03-08 15:02:17]

On 2018/03/08 15:01:53, ep wrote:

> > Can you also open an issue to create a formatter superclass for expanding
> > values, so we can generalize this as discussed.
> 
> Done.

(Issue #1744, for the record.)

[onager, 2018-03-08 15:05:35]

LGTM, I'll merge this

[onager, 2018-03-08 15:12:02]

Changes have been merged with master branch. To close the review and clean up
the feature branch you can run: review.py close trendmicro

[Joachim Metz, 2018-03-23 07:05:00]

this was merged please close