|
|
Created:
6 years, 2 months ago by onager Modified:
6 years, 2 months ago Reviewers:
Joachim Metz CC:
jberggren, romaing, kiddi, log2timeline-dev_googlegroups.com, aaronp Visibility:
Public. |
Description[plaso] Added fseventd parser #1467
Patch Set 1 #
Total comments: 61
Patch Set 2 : Changes after review #Patch Set 3 : Changes after review #
Total comments: 23
Patch Set 4 : Changes after review #
Total comments: 70
Patch Set 5 : Changes after review #
Total comments: 12
Patch Set 6 : Changes after review #
Total comments: 10
Patch Set 7 : Changes after review #
MessagesTotal messages: 28
Initial comments https://codereview.appspot.com/340110043/diff/1/plaso/engine/worker.py File plaso/engine/worker.py (left): https://codereview.appspot.com/340110043/diff/1/plaso/engine/worker.py#oldcode73 plaso/engine/worker.py:73: _FSEVENTSD_FILE_RE = re.compile(r'^[0-9a-fA-F]{16}$') side topic: do we want to hash these files? https://codereview.appspot.com/340110043/diff/1/plaso/formatters/fseventsd.py File plaso/formatters/fseventsd.py (right): https://codereview.appspot.com/340110043/diff/1/plaso/formatters/fseventsd.py... plaso/formatters/fseventsd.py:101: mask = event_values['flags'] why is the same value named mask here and flags in event? https://codereview.appspot.com/340110043/diff/1/plaso/lib/definitions.py File plaso/lib/definitions.py (right): https://codereview.appspot.com/340110043/diff/1/plaso/lib/definitions.py#newc... plaso/lib/definitions.py:125: TIME_DESCRIPTION_RECORDED = 'Event Recorded' For my OCD: move 1 line up ;) https://codereview.appspot.com/340110043/diff/1/plaso/parsers/fseventsd.py File plaso/parsers/fseventsd.py (right): https://codereview.appspot.com/340110043/diff/1/plaso/parsers/fseventsd.py#ne... plaso/parsers/fseventsd.py:13: from plaso.lib import errors For my OCD: alphabetical order https://codereview.appspot.com/340110043/diff/1/plaso/parsers/fseventsd.py#ne... plaso/parsers/fseventsd.py:26: offset (int): offset of the record in the fseventsd file. For my OCD: alphabetical order https://codereview.appspot.com/340110043/diff/1/plaso/parsers/fseventsd.py#ne... plaso/parsers/fseventsd.py:28: flags (int): object type and event flags stored in the record. + node_id (preferably node_identifier) https://codereview.appspot.com/340110043/diff/1/plaso/parsers/fseventsd.py#ne... plaso/parsers/fseventsd.py:29: event_id (int): the record event id. id => identifier https://codereview.appspot.com/340110043/diff/1/plaso/parsers/fseventsd.py#ne... plaso/parsers/fseventsd.py:34: def __init__(self): + docstring https://codereview.appspot.com/340110043/diff/1/plaso/parsers/fseventsd.py#ne... plaso/parsers/fseventsd.py:37: self.offset = None For my OCD: alphabetical order https://codereview.appspot.com/340110043/diff/1/plaso/parsers/fseventsd.py#ne... plaso/parsers/fseventsd.py:47: Refer to https://nicoleibrahim.com/apple-fsevents-forensics/ for details. https => http ERR_SSL_PROTOCOL_ERROR https://codereview.appspot.com/340110043/diff/1/plaso/parsers/fseventsd.py#ne... plaso/parsers/fseventsd.py:59: """Parses a SLD header from a stream. SLD header => I assume this would be DLS seeing the byte order hence DLS1 and DLS2 (1 or 2 is the version information) speculation but short for something like: Disk Level Stream https://developer.apple.com/library/content/documentation/Darwin/Conceptual/F... also see reference to dls in logs https://discussions.apple.com/thread/1626732 https://codereview.appspot.com/340110043/diff/1/plaso/parsers/fseventsd.py#ne... plaso/parsers/fseventsd.py:60: + Args https://codereview.appspot.com/340110043/diff/1/plaso/parsers/fseventsd.py#ne... plaso/parsers/fseventsd.py:65: sld_header = self._SLD_HEADER.parse_stream(file_object) "sld_header =" => return https://codereview.appspot.com/340110043/diff/1/plaso/parsers/fseventsd.py#ne... plaso/parsers/fseventsd.py:69: return sld_header merge with line 65 https://codereview.appspot.com/340110043/diff/1/plaso/parsers/fseventsd.py#ne... plaso/parsers/fseventsd.py:75: record (construct.Container): parsed record structure. shouldn't this be construct.Struct ? https://codereview.appspot.com/340110043/diff/1/plaso/parsers/fseventsd.py#ne... plaso/parsers/fseventsd.py:98: + Args https://codereview.appspot.com/340110043/diff/1/plaso/parsers/fseventsd.py#ne... plaso/parsers/fseventsd.py:100: float: posix timestamp, or None if none modification time is available. modification time stored as a POSIX timestamps with # precision, or None if not available. https://codereview.appspot.com/340110043/diff/1/plaso/parsers/fseventsd.py#ne... plaso/parsers/fseventsd.py:124: # https://github.com/log2timeline/dfdatetime/issues/65 is resolved. How is https://github.com/log2timeline/dfdatetime/issues/65 going to help here? What is precision of timestamp? Why not use PosixTimeInMicroseconds? We could also add a PosixTimeWithFractionOfSecond. https://codereview.appspot.com/340110043/diff/1/plaso/parsers/fseventsd.py#ne... plaso/parsers/fseventsd.py:146: The version 1 format was used in Mac OS X 10.5 (Leopard) through macOS 10.12 I opt to stick with MacOS https://codereview.appspot.com/340110043/diff/1/plaso/parsers/fseventsd.py#ne... plaso/parsers/fseventsd.py:156: 'sld_header', sld_header => dsl1_header https://codereview.appspot.com/340110043/diff/1/plaso/parsers/fseventsd.py#ne... plaso/parsers/fseventsd.py:157: construct.Const(construct.String('signature', 4), b'1SLD'), String => Bytes https://codereview.appspot.com/340110043/diff/1/plaso/parsers/fseventsd.py#ne... plaso/parsers/fseventsd.py:158: construct.Bytes('unknown', 4), => construct.Padding(4), at quick glance this looks like a CRC based checksum https://codereview.appspot.com/340110043/diff/1/plaso/parsers/fseventsd.py#ne... plaso/parsers/fseventsd.py:162: 'sld_record', sld_record -> dsl1_record https://codereview.appspot.com/340110043/diff/1/plaso/parsers/fseventsd.py#ne... plaso/parsers/fseventsd.py:164: construct.ULInt32('event_id'), id => identifier https://codereview.appspot.com/340110043/diff/1/plaso/parsers/fseventsd.py#ne... plaso/parsers/fseventsd.py:179: class FseventsdParserV2(BaseFseventsdParser): Why not collapse this into 1 parser and select DLS1_RECORD or DLS2_RECORD based on the version in the signature? https://codereview.appspot.com/340110043/diff/1/tests/formatters/fseventsd.py File tests/formatters/fseventsd.py (right): https://codereview.appspot.com/340110043/diff/1/tests/formatters/fseventsd.py... tests/formatters/fseventsd.py:14: class FirefoxCookieFormatterTest(test_lib.EventFormatterTestCase): FirefoxCookieFormatterTest => FSEventsd... https://codereview.appspot.com/340110043/diff/1/tests/parsers/fseventsd.py File tests/parsers/fseventsd.py (right): https://codereview.appspot.com/340110043/diff/1/tests/parsers/fseventsd.py#ne... tests/parsers/fseventsd.py:7: from tests import test_lib as shared_test_lib fix import order 1. futures 2. python native imports 3. third party 4. plaso 5. tests https://codereview.appspot.com/340110043/diff/1/tests/parsers/fseventsd.py#ne... tests/parsers/fseventsd.py:18: - white line https://codereview.appspot.com/340110043/diff/1/tests/parsers/fseventsd.py#ne... tests/parsers/fseventsd.py:41: self.assertEqual(event.flags, b'\x00\x00\x00\x00\x80\x00\x00\x01') shouldn't this be an integer?
Sign in to reply to this message.
https://codereview.appspot.com/340110043/diff/1/plaso/parsers/fseventsd.py File plaso/parsers/fseventsd.py (right): https://codereview.appspot.com/340110043/diff/1/plaso/parsers/fseventsd.py#ne... plaso/parsers/fseventsd.py:59: """Parses a SLD header from a stream. Maybe it is disk log(ger) stream seeing other fseventsd log messages
Sign in to reply to this message.
https://codereview.appspot.com/340110043/diff/1/plaso/parsers/fseventsd.py File plaso/parsers/fseventsd.py (right): https://codereview.appspot.com/340110043/diff/1/plaso/parsers/fseventsd.py#ne... plaso/parsers/fseventsd.py:162: 'sld_record', I meant dls1_header
Sign in to reply to this message.
Code updated.
Sign in to reply to this message.
Code updated.
Sign in to reply to this message.
https://codereview.appspot.com/340110043/diff/1/plaso/engine/worker.py File plaso/engine/worker.py (left): https://codereview.appspot.com/340110043/diff/1/plaso/engine/worker.py#oldcode73 plaso/engine/worker.py:73: _FSEVENTSD_FILE_RE = re.compile(r'^[0-9a-fA-F]{16}$') On 2018/01/04 20:34:56, Joachim Metz wrote: > side topic: do we want to hash these files? I think we want to hash everything. https://codereview.appspot.com/340110043/diff/1/plaso/formatters/fseventsd.py File plaso/formatters/fseventsd.py (right): https://codereview.appspot.com/340110043/diff/1/plaso/formatters/fseventsd.py... plaso/formatters/fseventsd.py:101: mask = event_values['flags'] On 2018/01/04 20:34:56, Joachim Metz wrote: > why is the same value named mask here and flags in event? Done. https://codereview.appspot.com/340110043/diff/1/plaso/lib/definitions.py File plaso/lib/definitions.py (right): https://codereview.appspot.com/340110043/diff/1/plaso/lib/definitions.py#newc... plaso/lib/definitions.py:125: TIME_DESCRIPTION_RECORDED = 'Event Recorded' On 2018/01/04 20:34:56, Joachim Metz wrote: > For my OCD: move 1 line up ;) Done. https://codereview.appspot.com/340110043/diff/1/plaso/parsers/fseventsd.py File plaso/parsers/fseventsd.py (right): https://codereview.appspot.com/340110043/diff/1/plaso/parsers/fseventsd.py#ne... plaso/parsers/fseventsd.py:13: from plaso.lib import errors On 2018/01/04 20:34:57, Joachim Metz wrote: > For my OCD: alphabetical order Done. https://codereview.appspot.com/340110043/diff/1/plaso/parsers/fseventsd.py#ne... plaso/parsers/fseventsd.py:26: offset (int): offset of the record in the fseventsd file. On 2018/01/04 20:34:56, Joachim Metz wrote: > For my OCD: alphabetical order Done. https://codereview.appspot.com/340110043/diff/1/plaso/parsers/fseventsd.py#ne... plaso/parsers/fseventsd.py:28: flags (int): object type and event flags stored in the record. On 2018/01/04 20:34:56, Joachim Metz wrote: > + node_id (preferably node_identifier) Done. https://codereview.appspot.com/340110043/diff/1/plaso/parsers/fseventsd.py#ne... plaso/parsers/fseventsd.py:29: event_id (int): the record event id. On 2018/01/04 20:34:57, Joachim Metz wrote: > id => identifier Done. https://codereview.appspot.com/340110043/diff/1/plaso/parsers/fseventsd.py#ne... plaso/parsers/fseventsd.py:34: def __init__(self): On 2018/01/04 20:34:57, Joachim Metz wrote: > + docstring Done. https://codereview.appspot.com/340110043/diff/1/plaso/parsers/fseventsd.py#ne... plaso/parsers/fseventsd.py:37: self.offset = None On 2018/01/04 20:34:56, Joachim Metz wrote: > For my OCD: alphabetical order Done. https://codereview.appspot.com/340110043/diff/1/plaso/parsers/fseventsd.py#ne... plaso/parsers/fseventsd.py:47: Refer to https://nicoleibrahim.com/apple-fsevents-forensics/ for details. On 2018/01/04 20:34:57, Joachim Metz wrote: > https => http > > ERR_SSL_PROTOCOL_ERROR Done. https://codereview.appspot.com/340110043/diff/1/plaso/parsers/fseventsd.py#ne... plaso/parsers/fseventsd.py:59: """Parses a SLD header from a stream. On 2018/01/04 20:39:30, Joachim Metz wrote: > Maybe it is disk log(ger) stream seeing other fseventsd log messages Done. https://codereview.appspot.com/340110043/diff/1/plaso/parsers/fseventsd.py#ne... plaso/parsers/fseventsd.py:59: """Parses a SLD header from a stream. On 2018/01/04 20:34:56, Joachim Metz wrote: > SLD header => I assume this would be DLS seeing the byte order > > hence DLS1 and DLS2 (1 or 2 is the version information) > > speculation but short for something like: Disk Level Stream > https://developer.apple.com/library/content/documentation/Darwin/Conceptual/F... > > also see reference to dls in logs https://discussions.apple.com/thread/1626732 Done. https://codereview.appspot.com/340110043/diff/1/plaso/parsers/fseventsd.py#ne... plaso/parsers/fseventsd.py:60: On 2018/01/04 20:34:57, Joachim Metz wrote: > + Args Done. https://codereview.appspot.com/340110043/diff/1/plaso/parsers/fseventsd.py#ne... plaso/parsers/fseventsd.py:65: sld_header = self._SLD_HEADER.parse_stream(file_object) On 2018/01/04 20:34:57, Joachim Metz wrote: > "sld_header =" => return Done. https://codereview.appspot.com/340110043/diff/1/plaso/parsers/fseventsd.py#ne... plaso/parsers/fseventsd.py:75: record (construct.Container): parsed record structure. On 2018/01/04 20:34:56, Joachim Metz wrote: > shouldn't this be construct.Struct ? No, parse_stream returns a container, see https://github.com/construct/construct/blob/stable25/docs_pdfetc/construct.pdf https://codereview.appspot.com/340110043/diff/1/plaso/parsers/fseventsd.py#ne... plaso/parsers/fseventsd.py:98: On 2018/01/04 20:34:56, Joachim Metz wrote: > + Args Done. https://codereview.appspot.com/340110043/diff/1/plaso/parsers/fseventsd.py#ne... plaso/parsers/fseventsd.py:100: float: posix timestamp, or None if none modification time is available. On 2018/01/04 20:34:57, Joachim Metz wrote: > modification time stored as a POSIX timestamps with # precision, or None if not > available. Acknowledged. https://codereview.appspot.com/340110043/diff/1/plaso/parsers/fseventsd.py#ne... plaso/parsers/fseventsd.py:124: # https://github.com/log2timeline/dfdatetime/issues/65 is resolved. On 2018/01/04 20:34:56, Joachim Metz wrote: > How is https://github.com/log2timeline/dfdatetime/issues/65 going to help here? > > What is precision of timestamp? Why not use PosixTimeInMicroseconds? We could > also add a PosixTimeWithFractionOfSecond. This would help because these events occurred in a time span, rather than at a point at time. https://codereview.appspot.com/340110043/diff/1/plaso/parsers/fseventsd.py#ne... plaso/parsers/fseventsd.py:146: The version 1 format was used in Mac OS X 10.5 (Leopard) through macOS 10.12 On 2018/01/04 20:34:57, Joachim Metz wrote: > I opt to stick with MacOS Acknowledged. https://codereview.appspot.com/340110043/diff/1/plaso/parsers/fseventsd.py#ne... plaso/parsers/fseventsd.py:156: 'sld_header', On 2018/01/04 20:34:57, Joachim Metz wrote: > sld_header => dsl1_header Done. https://codereview.appspot.com/340110043/diff/1/plaso/parsers/fseventsd.py#ne... plaso/parsers/fseventsd.py:157: construct.Const(construct.String('signature', 4), b'1SLD'), On 2018/01/04 20:34:56, Joachim Metz wrote: > String => Bytes Done. https://codereview.appspot.com/340110043/diff/1/plaso/parsers/fseventsd.py#ne... plaso/parsers/fseventsd.py:158: construct.Bytes('unknown', 4), On 2018/01/04 20:34:57, Joachim Metz wrote: > => construct.Padding(4), > > at quick glance this looks like a CRC based checksum Done. https://codereview.appspot.com/340110043/diff/1/plaso/parsers/fseventsd.py#ne... plaso/parsers/fseventsd.py:162: 'sld_record', On 2018/01/04 20:34:56, Joachim Metz wrote: > sld_record -> dsl1_record Done. https://codereview.appspot.com/340110043/diff/1/plaso/parsers/fseventsd.py#ne... plaso/parsers/fseventsd.py:162: 'sld_record', On 2018/01/04 20:34:56, Joachim Metz wrote: > sld_record -> dsl1_record Done. https://codereview.appspot.com/340110043/diff/1/plaso/parsers/fseventsd.py#ne... plaso/parsers/fseventsd.py:164: construct.ULInt32('event_id'), On 2018/01/04 20:34:56, Joachim Metz wrote: > id => identifier Done. https://codereview.appspot.com/340110043/diff/1/plaso/parsers/fseventsd.py#ne... plaso/parsers/fseventsd.py:179: class FseventsdParserV2(BaseFseventsdParser): On 2018/01/04 20:34:57, Joachim Metz wrote: > Why not collapse this into 1 parser and select DLS1_RECORD or DLS2_RECORD based > on the version in the signature? Acknowledged. https://codereview.appspot.com/340110043/diff/1/tests/formatters/fseventsd.py File tests/formatters/fseventsd.py (right): https://codereview.appspot.com/340110043/diff/1/tests/formatters/fseventsd.py... tests/formatters/fseventsd.py:14: class FirefoxCookieFormatterTest(test_lib.EventFormatterTestCase): On 2018/01/04 20:34:57, Joachim Metz wrote: > FirefoxCookieFormatterTest => FSEventsd... Done. https://codereview.appspot.com/340110043/diff/1/tests/parsers/fseventsd.py File tests/parsers/fseventsd.py (right): https://codereview.appspot.com/340110043/diff/1/tests/parsers/fseventsd.py#ne... tests/parsers/fseventsd.py:7: from tests import test_lib as shared_test_lib On 2018/01/04 20:34:57, Joachim Metz wrote: > fix import order > > 1. futures > 2. python native imports > 3. third party > 4. plaso > 5. tests Done. https://codereview.appspot.com/340110043/diff/1/tests/parsers/fseventsd.py#ne... tests/parsers/fseventsd.py:18: On 2018/01/04 20:34:57, Joachim Metz wrote: > - white line Done. https://codereview.appspot.com/340110043/diff/1/tests/parsers/fseventsd.py#ne... tests/parsers/fseventsd.py:41: self.assertEqual(event.flags, b'\x00\x00\x00\x00\x80\x00\x00\x01') On 2018/01/04 20:34:57, Joachim Metz wrote: > shouldn't this be an integer? It can be either, but I think a byte string is most direct, given the field is defined as "bytes" in the definition.
Sign in to reply to this message.
https://codereview.appspot.com/340110043/diff/40001/plaso/parsers/fseventsd.py File plaso/parsers/fseventsd.py (right): https://codereview.appspot.com/340110043/diff/40001/plaso/parsers/fseventsd.p... plaso/parsers/fseventsd.py:38: super(FseventsdEventData, self).__init__() + data_type=self.DATA_TYPE https://codereview.appspot.com/340110043/diff/40001/plaso/parsers/fseventsd.p... plaso/parsers/fseventsd.py:53: # The version 1 format was used in Mac OS X 10.5 (Leopard) through macOS 10.12 I opt to move these comments to the docstring. https://codereview.appspot.com/340110043/diff/40001/plaso/parsers/fseventsd.p... plaso/parsers/fseventsd.py:93: - 1x whiteline https://codereview.appspot.com/340110043/diff/40001/plaso/parsers/fseventsd.p... plaso/parsers/fseventsd.py:96: """Parses a DLS header from a stream. stream => file-like object. https://codereview.appspot.com/340110043/diff/40001/plaso/parsers/fseventsd.p... plaso/parsers/fseventsd.py:103: int: the version of the header that was parsed, either 1 or 2. version => format version https://codereview.appspot.com/340110043/diff/40001/plaso/parsers/fseventsd.p... plaso/parsers/fseventsd.py:111: for version, header_type in headers.items(): since _DLS_HEADER_V1 and _DLS_HEADER_V2 are the same structure, why not read the structure and check the signature? This will make the code more straightforward https://codereview.appspot.com/340110043/diff/40001/plaso/parsers/fseventsd.p... plaso/parsers/fseventsd.py:134: # Node identifier is only set in DLS V2 records. if you pass format_version and check format_version >= 2 this comment will become clear from the code. https://codereview.appspot.com/340110043/diff/40001/plaso/parsers/fseventsd.p... plaso/parsers/fseventsd.py:155: dfdatetime.DateTimeValues: time values, or None if not available. time values, or => parent modification time or https://codereview.appspot.com/340110043/diff/40001/plaso/parsers/fseventsd.p... plaso/parsers/fseventsd.py:173: version, header = self._ParseDLSHeader(file_object) version => format_version https://codereview.appspot.com/340110043/diff/40001/plaso/parsers/fseventsd.p... plaso/parsers/fseventsd.py:178: # https://github.com/log2timeline/dfdatetime/issues/65 is resolved. Please explain in the comment that you need timespan support here https://codereview.appspot.com/340110043/diff/40001/tests/parsers/fseventsd.py File tests/parsers/fseventsd.py (right): https://codereview.appspot.com/340110043/diff/40001/tests/parsers/fseventsd.p... tests/parsers/fseventsd.py:75: self.assertEqual(event.timestamp, expected_timestamp) Please add a messages (formatter) test as well
Sign in to reply to this message.
https://codereview.appspot.com/340110043/diff/40001/plaso/parsers/fseventsd.py File plaso/parsers/fseventsd.py (right): https://codereview.appspot.com/340110043/diff/40001/plaso/parsers/fseventsd.p... plaso/parsers/fseventsd.py:38: super(FseventsdEventData, self).__init__() On 2018/01/15 07:33:58, Joachim Metz wrote: > + data_type=self.DATA_TYPE Done. https://codereview.appspot.com/340110043/diff/40001/plaso/parsers/fseventsd.p... plaso/parsers/fseventsd.py:53: # The version 1 format was used in Mac OS X 10.5 (Leopard) through macOS 10.12 On 2018/01/15 07:33:58, Joachim Metz wrote: > I opt to move these comments to the docstring. Acknowledged. https://codereview.appspot.com/340110043/diff/40001/plaso/parsers/fseventsd.p... plaso/parsers/fseventsd.py:93: On 2018/01/15 07:33:58, Joachim Metz wrote: > - 1x whiteline Done. https://codereview.appspot.com/340110043/diff/40001/plaso/parsers/fseventsd.p... plaso/parsers/fseventsd.py:96: """Parses a DLS header from a stream. On 2018/01/15 07:33:58, Joachim Metz wrote: > stream => file-like object. Done. https://codereview.appspot.com/340110043/diff/40001/plaso/parsers/fseventsd.p... plaso/parsers/fseventsd.py:103: int: the version of the header that was parsed, either 1 or 2. On 2018/01/15 07:33:58, Joachim Metz wrote: > version => format version Done. https://codereview.appspot.com/340110043/diff/40001/plaso/parsers/fseventsd.p... plaso/parsers/fseventsd.py:111: for version, header_type in headers.items(): On 2018/01/15 07:33:58, Joachim Metz wrote: > since _DLS_HEADER_V1 and _DLS_HEADER_V2 are the same structure, why not read the > structure and check the signature? > > This will make the code more straightforward Done. https://codereview.appspot.com/340110043/diff/40001/plaso/parsers/fseventsd.p... plaso/parsers/fseventsd.py:134: # Node identifier is only set in DLS V2 records. On 2018/01/15 07:33:58, Joachim Metz wrote: > if you pass format_version and check format_version >= 2 this comment will > become clear from the code. I think this comment would still be desirable https://codereview.appspot.com/340110043/diff/40001/plaso/parsers/fseventsd.p... plaso/parsers/fseventsd.py:155: dfdatetime.DateTimeValues: time values, or None if not available. On 2018/01/15 07:33:58, Joachim Metz wrote: > time values, or => parent modification time or Done. https://codereview.appspot.com/340110043/diff/40001/plaso/parsers/fseventsd.p... plaso/parsers/fseventsd.py:173: version, header = self._ParseDLSHeader(file_object) On 2018/01/15 07:33:58, Joachim Metz wrote: > version => format_version Acknowledged. https://codereview.appspot.com/340110043/diff/40001/plaso/parsers/fseventsd.p... plaso/parsers/fseventsd.py:178: # https://github.com/log2timeline/dfdatetime/issues/65 is resolved. On 2018/01/15 07:33:58, Joachim Metz wrote: > Please explain in the comment that you need timespan support here Done. https://codereview.appspot.com/340110043/diff/40001/tests/parsers/fseventsd.py File tests/parsers/fseventsd.py (right): https://codereview.appspot.com/340110043/diff/40001/tests/parsers/fseventsd.p... tests/parsers/fseventsd.py:75: self.assertEqual(event.timestamp, expected_timestamp) On 2018/01/15 07:33:58, Joachim Metz wrote: > Please add a messages (formatter) test as well Acknowledged.
Sign in to reply to this message.
Code updated.
Sign in to reply to this message.
https://codereview.appspot.com/340110043/diff/40001/plaso/parsers/fseventsd.py File plaso/parsers/fseventsd.py (right): https://codereview.appspot.com/340110043/diff/40001/plaso/parsers/fseventsd.p... plaso/parsers/fseventsd.py:134: # Node identifier is only set in DLS V2 records. If you use something like: if format_version == 2: data.node_identifier = record.node_identifier the comment would not be needed, since it is clear from the code https://codereview.appspot.com/340110043/diff/60001/plaso/engine/worker.py File plaso/engine/worker.py (left): https://codereview.appspot.com/340110043/diff/60001/plaso/engine/worker.py#ol... plaso/engine/worker.py:287: if len(path_segments) == 2 and path_segments[0].lower() == '.fseventsd': What if the fseventsd parser is disabled? https://codereview.appspot.com/340110043/diff/60001/plaso/formatters/fsevents... File plaso/formatters/fseventsd.py (right): https://codereview.appspot.com/340110043/diff/60001/plaso/formatters/fsevents... plaso/formatters/fseventsd.py:25: _OBJECT_TYPE_MASKS = { MASKS or FLAGS? These look more types than either of them https://codereview.appspot.com/340110043/diff/60001/plaso/formatters/fsevents... plaso/formatters/fseventsd.py:31: _EVENT_MASKS = { masks => flags https://codereview.appspot.com/340110043/diff/60001/plaso/formatters/fsevents... plaso/formatters/fseventsd.py:69: mask (int): fsevents record type mask. mask or flags? please fix the terminology used in this file also see: https://en.wikipedia.org/wiki/Mask_(computing) https://codereview.appspot.com/340110043/diff/60001/plaso/parsers/fseventsd.py File plaso/parsers/fseventsd.py (right): https://codereview.appspot.com/340110043/diff/60001/plaso/parsers/fseventsd.p... plaso/parsers/fseventsd.py:27: event_identifier (int): the record event id. id => identifier https://github.com/log2timeline/plaso/wiki/Style-guide#docstrings https://codereview.appspot.com/340110043/diff/60001/plaso/parsers/fseventsd.p... plaso/parsers/fseventsd.py:28: flags (int): object type and event flags stored in the record. please split object type and event flags into 2 different values, they have different meaning. https://codereview.appspot.com/340110043/diff/60001/plaso/parsers/fseventsd.p... plaso/parsers/fseventsd.py:29: node_identifier (str): node identifier stored in the fseventsd record. what is the purpose of this node? https://codereview.appspot.com/340110043/diff/60001/plaso/parsers/fseventsd.p... plaso/parsers/fseventsd.py:30: offset (int): offset of the record in the fseventsd file. -offset already defined by super class https://github.com/log2timeline/plaso/blob/master/plaso/containers/events.py#L31 https://codereview.appspot.com/340110043/diff/60001/plaso/parsers/fseventsd.p... plaso/parsers/fseventsd.py:31: path (str): path recorded in the fseventsd record. path of what? the original file entry? https://codereview.appspot.com/340110043/diff/60001/plaso/parsers/fseventsd.p... plaso/parsers/fseventsd.py:43: self.offset = None -offset already defined by super class https://github.com/log2timeline/plaso/blob/master/plaso/containers/events.py#L31 https://codereview.appspot.com/340110043/diff/60001/plaso/parsers/fseventsd.p... plaso/parsers/fseventsd.py:52: """ + NAME, DESCRIPTION https://codereview.appspot.com/340110043/diff/60001/plaso/parsers/fseventsd.p... plaso/parsers/fseventsd.py:107: logging.debug('Unable to parse DLS header with error: {0:s}'.format( remove this logging. https://codereview.appspot.com/340110043/diff/60001/plaso/parsers/fseventsd.p... plaso/parsers/fseventsd.py:120: data = FseventsdEventData() data => event_data Let's keep this consistent also to more clearly distinguish between event data and other types of data. https://codereview.appspot.com/340110043/diff/60001/plaso/parsers/fseventsd.p... plaso/parsers/fseventsd.py:182: if header.signature == self._DLS_V1_SIGNATURE: Move this outside the loop if header.signature == self._DLS_V1_SIGNATURE: record_structure = self._DLS_RECORD_V1 ... and in the loop do: record_structure.parse_stream(file_object) https://codereview.appspot.com/340110043/diff/60001/plaso/parsers/fseventsd.p... plaso/parsers/fseventsd.py:187: data = self._BuildEventData(record) => event_data https://codereview.appspot.com/340110043/diff/60001/plaso/parsers/presets.py File plaso/parsers/presets.py (right): https://codereview.appspot.com/340110043/diff/60001/plaso/parsers/presets.py#... plaso/parsers/presets.py:35: 'cups_ipp', 'filestat', 'fseventsdv1', 'fseventsdv2', change to 'fseventsd' https://codereview.appspot.com/340110043/diff/60001/tests/formatters/test_lib.py File tests/formatters/test_lib.py (right): https://codereview.appspot.com/340110043/diff/60001/tests/formatters/test_lib... tests/formatters/test_lib.py:27: def _MakeTestEvent(self, event_data): Make => Create keep consistent with the rest of the code base https://codereview.appspot.com/340110043/diff/60001/tests/formatters/test_lib... tests/formatters/test_lib.py:45: event_formatter (EventFormatter): event formatter under test. under test => "to be tested" or "to test with" https://codereview.appspot.com/340110043/diff/60001/tests/formatters/test_lib... tests/formatters/test_lib.py:51: def _TestGetMessages( If you want this make an issue to update the rest of the codebase and update the style guide to use this method instead of the current approach. https://codereview.appspot.com/340110043/diff/60001/tests/formatters/test_lib... tests/formatters/test_lib.py:64: formatter_mediator (FormatterMediator): mediates the interactions between make FormatterMediator optional https://codereview.appspot.com/340110043/diff/60001/tests/parsers/fseventsd.py File tests/parsers/fseventsd.py (right): https://codereview.appspot.com/340110043/diff/60001/tests/parsers/fseventsd.p... tests/parsers/fseventsd.py:23: """Tests the Parse function.""" + on a fseventsd version 1 file. https://codereview.appspot.com/340110043/diff/60001/tests/parsers/fseventsd.p... tests/parsers/fseventsd.py:45: expected_time = os_file_entry.modification_time Please add a comment that the actual time of the file can change over time. https://codereview.appspot.com/340110043/diff/60001/tests/parsers/fseventsd.p... tests/parsers/fseventsd.py:47: self.assertEqual(event.timestamp, expected_timestamp) + test messages? https://codereview.appspot.com/340110043/diff/60001/tests/parsers/fseventsd.p... tests/parsers/fseventsd.py:51: """Tests the Parse function.""" + on a fseventsd version 2 file. https://codereview.appspot.com/340110043/diff/60001/tests/parsers/fseventsd.p... tests/parsers/fseventsd.py:72: os_file_entry = path_spec_resolver.Resolver.OpenFileEntry(os_path_spec) Please add a comment that the actual time of the file can change over time. https://codereview.appspot.com/340110043/diff/60001/tests/parsers/fseventsd.p... tests/parsers/fseventsd.py:75: self.assertEqual(event.timestamp, expected_timestamp) + test messages?
Sign in to reply to this message.
Code updated.
Sign in to reply to this message.
https://codereview.appspot.com/340110043/diff/60001/plaso/engine/worker.py File plaso/engine/worker.py (left): https://codereview.appspot.com/340110043/diff/60001/plaso/engine/worker.py#ol... plaso/engine/worker.py:287: if len(path_segments) == 2 and path_segments[0].lower() == '.fseventsd': On 2018/01/19 05:55:01, Joachim Metz wrote: > What if the fseventsd parser is disabled? Then we treat them like any other file. The only reason these files were excluded was the error messages caused by the incomplete gzip implementation in dfvfs: https://github.com/log2timeline/plaso/issues/218#issuecomment-111308123 I suspect the same is true for the other files that are special-cased here too, but I haven't had a chance to check. https://codereview.appspot.com/340110043/diff/60001/plaso/formatters/fsevents... File plaso/formatters/fseventsd.py (right): https://codereview.appspot.com/340110043/diff/60001/plaso/formatters/fsevents... plaso/formatters/fseventsd.py:25: _OBJECT_TYPE_MASKS = { On 2018/01/19 05:55:01, Joachim Metz wrote: > MASKS or FLAGS? These look more types than either of them These are masks, per https://en.wikipedia.org/wiki/Mask_(computing). Compare with _USN_REASON_FLAGS usage in https://github.com/log2timeline/plaso/blob/master/plaso/formatters/file_syste..., on lines 178 and 240 https://codereview.appspot.com/340110043/diff/60001/plaso/formatters/fsevents... plaso/formatters/fseventsd.py:31: _EVENT_MASKS = { On 2018/01/19 05:55:01, Joachim Metz wrote: > masks => flags These are masks https://codereview.appspot.com/340110043/diff/60001/plaso/formatters/fsevents... plaso/formatters/fseventsd.py:69: mask (int): fsevents record type mask. On 2018/01/19 05:55:01, Joachim Metz wrote: > mask or flags? > > please fix the terminology used in this file also see: > https://en.wikipedia.org/wiki/Mask_(computing) Done. https://codereview.appspot.com/340110043/diff/60001/plaso/parsers/fseventsd.py File plaso/parsers/fseventsd.py (right): https://codereview.appspot.com/340110043/diff/60001/plaso/parsers/fseventsd.p... plaso/parsers/fseventsd.py:27: event_identifier (int): the record event id. On 2018/01/19 05:55:01, Joachim Metz wrote: > id => identifier > > https://github.com/log2timeline/plaso/wiki/Style-guide#docstrings Done. https://codereview.appspot.com/340110043/diff/60001/plaso/parsers/fseventsd.p... plaso/parsers/fseventsd.py:28: flags (int): object type and event flags stored in the record. On 2018/01/19 05:55:02, Joachim Metz wrote: > please split object type and event flags into 2 different values, they have > different meaning. The flags are stored as one field in the record, I can't do this without pulling all the flag-masking code from the formatter into the parser. https://codereview.appspot.com/340110043/diff/60001/plaso/parsers/fseventsd.p... plaso/parsers/fseventsd.py:29: node_identifier (str): node identifier stored in the fseventsd record. On 2018/01/19 05:55:01, Joachim Metz wrote: > what is the purpose of this node? Done. https://codereview.appspot.com/340110043/diff/60001/plaso/parsers/fseventsd.p... plaso/parsers/fseventsd.py:30: offset (int): offset of the record in the fseventsd file. On 2018/01/19 05:55:01, Joachim Metz wrote: > -offset already defined by super class > https://github.com/log2timeline/plaso/blob/master/plaso/containers/events.py#L31 Done. https://codereview.appspot.com/340110043/diff/60001/plaso/parsers/fseventsd.p... plaso/parsers/fseventsd.py:31: path (str): path recorded in the fseventsd record. On 2018/01/19 05:55:01, Joachim Metz wrote: > path of what? the original file entry? This is path of the object that the record is about. Not sure what you're looking for here. https://codereview.appspot.com/340110043/diff/60001/plaso/parsers/fseventsd.p... plaso/parsers/fseventsd.py:43: self.offset = None On 2018/01/19 05:55:01, Joachim Metz wrote: > -offset already defined by super class > https://github.com/log2timeline/plaso/blob/master/plaso/containers/events.py#L31 Done. https://codereview.appspot.com/340110043/diff/60001/plaso/parsers/fseventsd.p... plaso/parsers/fseventsd.py:52: """ On 2018/01/19 05:55:01, Joachim Metz wrote: > + NAME, DESCRIPTION Done. https://codereview.appspot.com/340110043/diff/60001/plaso/parsers/fseventsd.p... plaso/parsers/fseventsd.py:107: logging.debug('Unable to parse DLS header with error: {0:s}'.format( On 2018/01/19 05:55:01, Joachim Metz wrote: > remove this logging. Done. https://codereview.appspot.com/340110043/diff/60001/plaso/parsers/fseventsd.p... plaso/parsers/fseventsd.py:120: data = FseventsdEventData() On 2018/01/19 05:55:01, Joachim Metz wrote: > data => event_data > > Let's keep this consistent also to more clearly distinguish between event data > and other types of data. Done. https://codereview.appspot.com/340110043/diff/60001/plaso/parsers/fseventsd.p... plaso/parsers/fseventsd.py:182: if header.signature == self._DLS_V1_SIGNATURE: On 2018/01/19 05:55:01, Joachim Metz wrote: > Move this outside the loop > > if header.signature == self._DLS_V1_SIGNATURE: > record_structure = self._DLS_RECORD_V1 > ... > > and in the loop do: > record_structure.parse_stream(file_object) That assumes that all headers in the same file are of the same version, which hasn't been tested. https://codereview.appspot.com/340110043/diff/60001/plaso/parsers/fseventsd.p... plaso/parsers/fseventsd.py:187: data = self._BuildEventData(record) On 2018/01/19 05:55:01, Joachim Metz wrote: > => event_data Done. https://codereview.appspot.com/340110043/diff/60001/plaso/parsers/presets.py File plaso/parsers/presets.py (right): https://codereview.appspot.com/340110043/diff/60001/plaso/parsers/presets.py#... plaso/parsers/presets.py:35: 'cups_ipp', 'filestat', 'fseventsdv1', 'fseventsdv2', On 2018/01/19 05:55:02, Joachim Metz wrote: > change to 'fseventsd' Done. https://codereview.appspot.com/340110043/diff/60001/tests/formatters/test_lib.py File tests/formatters/test_lib.py (right): https://codereview.appspot.com/340110043/diff/60001/tests/formatters/test_lib... tests/formatters/test_lib.py:27: def _MakeTestEvent(self, event_data): On 2018/01/19 05:55:02, Joachim Metz wrote: > Make => Create > > keep consistent with the rest of the code base Done. https://codereview.appspot.com/340110043/diff/60001/tests/formatters/test_lib... tests/formatters/test_lib.py:45: event_formatter (EventFormatter): event formatter under test. On 2018/01/19 05:55:02, Joachim Metz wrote: > under test => "to be tested" or "to test with" Leaving this as is, see https://en.wikipedia.org/wiki/System_under_test and https://en.wikipedia.org/wiki/Device_under_test https://codereview.appspot.com/340110043/diff/60001/tests/formatters/test_lib... tests/formatters/test_lib.py:51: def _TestGetMessages( On 2018/01/19 05:55:02, Joachim Metz wrote: > If you want this make an issue to update the rest of the codebase and update the > style guide to use this method instead of the current approach. I'm in two minds about this. Formatters are clearly closely tied to the events produced by the parsers, but tests for formatters would seem to logically live in the formatter tests. There's also the issue of TODO: Add test for GetMessages that we have all over the place. Without methods like this, getting all those TODOs completed isn't likely. Leaving both tests for now. https://codereview.appspot.com/340110043/diff/60001/tests/formatters/test_lib... tests/formatters/test_lib.py:64: formatter_mediator (FormatterMediator): mediates the interactions between On 2018/01/19 05:55:02, Joachim Metz wrote: > make FormatterMediator optional Done. https://codereview.appspot.com/340110043/diff/60001/tests/parsers/fseventsd.py File tests/parsers/fseventsd.py (right): https://codereview.appspot.com/340110043/diff/60001/tests/parsers/fseventsd.p... tests/parsers/fseventsd.py:23: """Tests the Parse function.""" On 2018/01/19 05:55:02, Joachim Metz wrote: > + on a fseventsd version 1 file. Done. https://codereview.appspot.com/340110043/diff/60001/tests/parsers/fseventsd.p... tests/parsers/fseventsd.py:45: expected_time = os_file_entry.modification_time On 2018/01/19 05:55:02, Joachim Metz wrote: > Please add a comment that the actual time of the file can change over time. I'm not sure what you mean here, please explain. https://codereview.appspot.com/340110043/diff/60001/tests/parsers/fseventsd.p... tests/parsers/fseventsd.py:47: self.assertEqual(event.timestamp, expected_timestamp) On 2018/01/19 05:55:02, Joachim Metz wrote: > + test messages? Done. https://codereview.appspot.com/340110043/diff/60001/tests/parsers/fseventsd.p... tests/parsers/fseventsd.py:51: """Tests the Parse function.""" On 2018/01/19 05:55:02, Joachim Metz wrote: > + on a fseventsd version 2 file. Done. https://codereview.appspot.com/340110043/diff/60001/tests/parsers/fseventsd.p... tests/parsers/fseventsd.py:72: os_file_entry = path_spec_resolver.Resolver.OpenFileEntry(os_path_spec) On 2018/01/19 05:55:02, Joachim Metz wrote: > Please add a comment that the actual time of the file can change over time. Acknowledged. https://codereview.appspot.com/340110043/diff/60001/tests/parsers/fseventsd.p... tests/parsers/fseventsd.py:75: self.assertEqual(event.timestamp, expected_timestamp) On 2018/01/19 05:55:02, Joachim Metz wrote: > + test messages? Done.
Sign in to reply to this message.
https://codereview.appspot.com/340110043/diff/60001/plaso/engine/worker.py File plaso/engine/worker.py (left): https://codereview.appspot.com/340110043/diff/60001/plaso/engine/worker.py#ol... plaso/engine/worker.py:287: if len(path_segments) == 2 and path_segments[0].lower() == '.fseventsd': my concern is also does that affect the performance, seeing that now the text parsers will be applied to the gzip compressed files. Maybe the idea in https://github.com/log2timeline/plaso/issues/1659 could provide a solution.
Sign in to reply to this message.
https://codereview.appspot.com/340110043/diff/60001/plaso/engine/worker.py File plaso/engine/worker.py (left): https://codereview.appspot.com/340110043/diff/60001/plaso/engine/worker.py#ol... plaso/engine/worker.py:287: if len(path_segments) == 2 and path_segments[0].lower() == '.fseventsd': On 2018/01/19 09:25:31, Joachim Metz wrote: > my concern is also does that affect the performance, seeing that now the text > parsers will be applied to the gzip compressed files. > > Maybe the idea in https://github.com/log2timeline/plaso/issues/1659 could > provide a solution. Probably, but it's not clear how much effect this will have, and this is also a pretty narrow use case.
Sign in to reply to this message.
https://codereview.appspot.com/340110043/diff/60001/plaso/engine/worker.py File plaso/engine/worker.py (left): https://codereview.appspot.com/340110043/diff/60001/plaso/engine/worker.py#ol... plaso/engine/worker.py:287: if len(path_segments) == 2 and path_segments[0].lower() == '.fseventsd': I opt we test this, but separate from this CL. https://codereview.appspot.com/340110043/diff/60001/plaso/formatters/fsevents... File plaso/formatters/fseventsd.py (right): https://codereview.appspot.com/340110043/diff/60001/plaso/formatters/fsevents... plaso/formatters/fseventsd.py:25: _OBJECT_TYPE_MASKS = { Please change this to types, that is more specific to what they represent. These are stored as bit flags, and a bitmask can be used to ignore a specific part of the full x-bit value. mask = 0x0f flag or type = 0x01 a flag indicates a specific condition e.g. "is signed" these look like types, what if 0x00006000: 'other type' ? https://codereview.appspot.com/340110043/diff/60001/plaso/formatters/fsevents... plaso/formatters/fseventsd.py:31: _EVENT_MASKS = { please change to flags, that is what they represent. https://codereview.appspot.com/340110043/diff/60001/plaso/formatters/fsevents... plaso/formatters/fseventsd.py:69: mask (int): fsevents record type mask. please change as requested https://codereview.appspot.com/340110043/diff/60001/plaso/parsers/fseventsd.py File plaso/parsers/fseventsd.py (right): https://codereview.appspot.com/340110043/diff/60001/plaso/parsers/fseventsd.p... plaso/parsers/fseventsd.py:28: flags (int): object type and event flags stored in the record. that is fine, the parser can actually mask the value 0x000ffff (or equiv) and extract the relevant data. https://codereview.appspot.com/340110043/diff/60001/plaso/parsers/fseventsd.p... plaso/parsers/fseventsd.py:182: if header.signature == self._DLS_V1_SIGNATURE: So the header is stored multiple times? per page? Maybe rename this to page_header to make this more clear. https://codereview.appspot.com/340110043/diff/60001/tests/formatters/test_lib.py File tests/formatters/test_lib.py (right): https://codereview.appspot.com/340110043/diff/60001/tests/formatters/test_lib... tests/formatters/test_lib.py:51: def _TestGetMessages( let's file an issue to fix this adding yet another variation specific to fsevents will not solve this issue. https://codereview.appspot.com/340110043/diff/80001/plaso/formatters/fsevents... File plaso/formatters/fseventsd.py (right): https://codereview.appspot.com/340110043/diff/80001/plaso/formatters/fsevents... plaso/formatters/fseventsd.py:32: 0x00000000: 'None', remove none https://codereview.appspot.com/340110043/diff/80001/plaso/formatters/fsevents... plaso/formatters/fseventsd.py:33: 0x00000002: 'Mount', make these descriptions a bit more indicative of what they mean. Is mounted? Or point to where they are defined. https://codereview.appspot.com/340110043/diff/80001/plaso/parsers/fseventsd.py File plaso/parsers/fseventsd.py (right): https://codereview.appspot.com/340110043/diff/80001/plaso/parsers/fseventsd.p... plaso/parsers/fseventsd.py:22: """Fseventsd event data. MacOS file system event (fseventsd) event data https://codereview.appspot.com/340110043/diff/80001/plaso/parsers/fseventsd.p... plaso/parsers/fseventsd.py:27: node_identifier (int): file system node identifier stored in the fseventsd "stored in the fseventsd record" > related to the file system event.
Sign in to reply to this message.
https://codereview.appspot.com/340110043/diff/80001/plaso/formatters/fsevents... File plaso/formatters/fseventsd.py (right): https://codereview.appspot.com/340110043/diff/80001/plaso/formatters/fsevents... plaso/formatters/fseventsd.py:25: _OBJECT_TYPE_MASKS = { Based on the description http://nicoleibrahim.com/apple-fsevents-forensics/ these are all flags confirmed by https://developer.apple.com/documentation/coreservices/1455361-fseventstreame...
Sign in to reply to this message.
https://codereview.appspot.com/340110043/diff/80001/plaso/parsers/fseventsd.py File plaso/parsers/fseventsd.py (right): https://codereview.appspot.com/340110043/diff/80001/plaso/parsers/fseventsd.p... plaso/parsers/fseventsd.py:26: flags (int): object type and event flags stored in the record. these are all flags https://developer.apple.com/documentation/coreservices/1455361-fseventstreame...
Sign in to reply to this message.
Code updated.
Sign in to reply to this message.
https://codereview.appspot.com/340110043/diff/60001/plaso/engine/worker.py File plaso/engine/worker.py (left): https://codereview.appspot.com/340110043/diff/60001/plaso/engine/worker.py#ol... plaso/engine/worker.py:287: if len(path_segments) == 2 and path_segments[0].lower() == '.fseventsd': On 2018/01/20 16:52:16, Joachim Metz wrote: > I opt we test this, but separate from this CL. Acknowledged. https://codereview.appspot.com/340110043/diff/60001/plaso/formatters/fsevents... File plaso/formatters/fseventsd.py (right): https://codereview.appspot.com/340110043/diff/60001/plaso/formatters/fsevents... plaso/formatters/fseventsd.py:25: _OBJECT_TYPE_MASKS = { On 2018/01/20 16:52:16, Joachim Metz wrote: > Please change this to types, that is more specific to what they represent. These > are stored as bit flags, and a bitmask can be used to ignore a specific part of > the full x-bit value. > > > mask = 0x0f > flag or type = 0x01 > > a flag indicates a specific condition e.g. "is signed" > these look like types, what if 0x00006000: 'other type' ? > I've changed this to flags https://codereview.appspot.com/340110043/diff/60001/plaso/formatters/fsevents... plaso/formatters/fseventsd.py:31: _EVENT_MASKS = { On 2018/01/20 16:52:16, Joachim Metz wrote: > please change to flags, that is what they represent. Done. https://codereview.appspot.com/340110043/diff/60001/plaso/formatters/fsevents... plaso/formatters/fseventsd.py:69: mask (int): fsevents record type mask. On 2018/01/20 16:52:16, Joachim Metz wrote: > please change as requested Done. https://codereview.appspot.com/340110043/diff/60001/plaso/parsers/fseventsd.py File plaso/parsers/fseventsd.py (right): https://codereview.appspot.com/340110043/diff/60001/plaso/parsers/fseventsd.p... plaso/parsers/fseventsd.py:28: flags (int): object type and event flags stored in the record. On 2018/01/20 16:52:16, Joachim Metz wrote: > that is fine, the parser can actually mask the value 0x000ffff (or equiv) and > extract the relevant data. These flags aren't contiguous, and this would be a very complex mask. Changed the docstring per your other comment. https://codereview.appspot.com/340110043/diff/60001/plaso/parsers/fseventsd.p... plaso/parsers/fseventsd.py:182: if header.signature == self._DLS_V1_SIGNATURE: On 2018/01/20 16:52:16, Joachim Metz wrote: > So the header is stored multiple times? per page? > > Maybe rename this to page_header to make this more clear. Done. https://codereview.appspot.com/340110043/diff/60001/tests/formatters/test_lib.py File tests/formatters/test_lib.py (right): https://codereview.appspot.com/340110043/diff/60001/tests/formatters/test_lib... tests/formatters/test_lib.py:51: def _TestGetMessages( On 2018/01/20 16:52:16, Joachim Metz wrote: > let's file an issue to fix this adding yet another variation specific to > fsevents will not solve this issue. Done. https://codereview.appspot.com/340110043/diff/60001/tests/formatters/test_lib... tests/formatters/test_lib.py:64: formatter_mediator (FormatterMediator): mediates the interactions between On 2018/01/19 09:06:39, onager wrote: > On 2018/01/19 05:55:02, Joachim Metz wrote: > > make FormatterMediator optional > > Done. https://github.com/log2timeline/plaso/issues/1669 https://codereview.appspot.com/340110043/diff/80001/plaso/formatters/fsevents... File plaso/formatters/fseventsd.py (right): https://codereview.appspot.com/340110043/diff/80001/plaso/formatters/fsevents... plaso/formatters/fseventsd.py:25: _OBJECT_TYPE_MASKS = { On 2018/01/20 17:28:44, Joachim Metz wrote: > Based on the description http://nicoleibrahim.com/apple-fsevents-forensics/ > these are all flags > > confirmed by > https://developer.apple.com/documentation/coreservices/1455361-fseventstreame... Done. https://codereview.appspot.com/340110043/diff/80001/plaso/formatters/fsevents... plaso/formatters/fseventsd.py:32: 0x00000000: 'None', On 2018/01/20 16:52:17, Joachim Metz wrote: > remove none This is documented here, and seems to be a valid flag: https://developer.apple.com/documentation/coreservices/1455361-fseventstreame... why remove it? https://codereview.appspot.com/340110043/diff/80001/plaso/formatters/fsevents... plaso/formatters/fseventsd.py:33: 0x00000002: 'Mount', On 2018/01/20 16:52:17, Joachim Metz wrote: > make these descriptions a bit more indicative of what they mean. > Is mounted? Or point to where they are defined. Added a link to the Apple documentation for the flags as used in the FSEventsStream API, which are similar. https://codereview.appspot.com/340110043/diff/80001/plaso/parsers/fseventsd.py File plaso/parsers/fseventsd.py (right): https://codereview.appspot.com/340110043/diff/80001/plaso/parsers/fseventsd.p... plaso/parsers/fseventsd.py:22: """Fseventsd event data. On 2018/01/20 16:52:17, Joachim Metz wrote: > MacOS file system event (fseventsd) event data Done. https://codereview.appspot.com/340110043/diff/80001/plaso/parsers/fseventsd.p... plaso/parsers/fseventsd.py:26: flags (int): object type and event flags stored in the record. On 2018/01/20 17:33:07, Joachim Metz wrote: > these are all flags > https://developer.apple.com/documentation/coreservices/1455361-fseventstreame... Not sure if this is the change you wanted here - please have actionable comments to reduce round trips. Also note that the flags in https://developer.apple.com/documentation/coreservices/1455361-fseventstreame... are not the same as the ones stored in the FSEvents file, for example, 0x00100000 is defined as kFSEventStreamEventFlagItemIsHardlink in that documentation, but the value that indicates a record is a hard link is 0x00001000 in the fsevents flags. https://codereview.appspot.com/340110043/diff/80001/plaso/parsers/fseventsd.p... plaso/parsers/fseventsd.py:27: node_identifier (int): file system node identifier stored in the fseventsd On 2018/01/20 16:52:17, Joachim Metz wrote: > "stored in the fseventsd record" > related to the file system event. Done.
Sign in to reply to this message.
one question about the event flags remaining, otherwise nearly there. https://codereview.appspot.com/340110043/diff/60001/plaso/parsers/fseventsd.py File plaso/parsers/fseventsd.py (right): https://codereview.appspot.com/340110043/diff/60001/plaso/parsers/fseventsd.p... plaso/parsers/fseventsd.py:28: flags (int): object type and event flags stored in the record. ack, these are all flags per my other comment. https://codereview.appspot.com/340110043/diff/100001/plaso/formatters/fsevent... File plaso/formatters/fseventsd.py (right): https://codereview.appspot.com/340110043/diff/100001/plaso/formatters/fsevent... plaso/formatters/fseventsd.py:27: # those described in https://developer.apple.com/documentation/coreservices/core_services_enumerat... Please explain what is different, this is unclear to me. IMHO just merge _OBJECT_TYPE_FLAGS and _EVENT_FLAGS and use the names defined by apple developer article is more clear. Also where do the object types come from? http://nicoleibrahim.com/apple-fsevents-forensics/ defines these as flags. It hints at the object types "0x00000001 Item is Folder" though there is no indication if this is deduced from observation or reading the developer documentation. https://codereview.appspot.com/340110043/diff/100001/plaso/parsers/fseventsd.py File plaso/parsers/fseventsd.py (right): https://codereview.appspot.com/340110043/diff/100001/plaso/parsers/fseventsd.... plaso/parsers/fseventsd.py:74: _DLS_HEADER = construct.Struct( _DLS_HEADER => _DLS_PAGE_HEADER
Sign in to reply to this message.
https://codereview.appspot.com/340110043/diff/100001/plaso/parsers/fseventsd.py File plaso/parsers/fseventsd.py (right): https://codereview.appspot.com/340110043/diff/100001/plaso/parsers/fseventsd.... plaso/parsers/fseventsd.py:59: construct.CString('filename'), filename => path https://codereview.appspot.com/340110043/diff/100001/plaso/parsers/fseventsd.... plaso/parsers/fseventsd.py:67: construct.CString('filename'), filename => path
Sign in to reply to this message.
https://codereview.appspot.com/340110043/diff/100001/plaso/formatters/fsevent... File plaso/formatters/fseventsd.py (right): https://codereview.appspot.com/340110043/diff/100001/plaso/formatters/fsevent... plaso/formatters/fseventsd.py:27: # those described in https://developer.apple.com/documentation/coreservices/core_services_enumerat... On 2018/01/21 07:54:47, Joachim Metz wrote: > Please explain what is different, this is unclear to me. > > IMHO just merge _OBJECT_TYPE_FLAGS and _EVENT_FLAGS and use the names defined by > apple developer article is more clear. > > Also where do the object types come from? > http://nicoleibrahim.com/apple-fsevents-forensics/ defines these as flags. It > hints at the object types "0x00000001 Item is Folder" though there is no > indication if this is deduced from observation or reading the developer > documentation. Added some more here. Merging these flags would make the message much more difficult to read, and less clear to the user. They're split like this to make the message string more readable https://codereview.appspot.com/340110043/diff/100001/plaso/parsers/fseventsd.py File plaso/parsers/fseventsd.py (right): https://codereview.appspot.com/340110043/diff/100001/plaso/parsers/fseventsd.... plaso/parsers/fseventsd.py:59: construct.CString('filename'), On 2018/01/21 08:58:10, Joachim Metz wrote: > filename => path Done. https://codereview.appspot.com/340110043/diff/100001/plaso/parsers/fseventsd.... plaso/parsers/fseventsd.py:67: construct.CString('filename'), On 2018/01/21 08:58:10, Joachim Metz wrote: > filename => path Done. https://codereview.appspot.com/340110043/diff/100001/plaso/parsers/fseventsd.... plaso/parsers/fseventsd.py:74: _DLS_HEADER = construct.Struct( On 2018/01/21 07:54:47, Joachim Metz wrote: > _DLS_HEADER => _DLS_PAGE_HEADER Done.
Sign in to reply to this message.
https://codereview.appspot.com/340110043/diff/100001/plaso/formatters/fsevent... File plaso/formatters/fseventsd.py (right): https://codereview.appspot.com/340110043/diff/100001/plaso/formatters/fsevent... plaso/formatters/fseventsd.py:27: # those described in https://developer.apple.com/documentation/coreservices/core_services_enumerat... at the moment I have no idea where these values (originally) come from, I see some references in http://nicoleibrahim.com/apple-fsevents-forensics/ but it seems that these values were determined by observation. Therefore I would recommend not to re-interpret these values ourselves, but keep them as the other tools and make a note to try to determine what this values actually mean. I would also recommend adding a hexadecimal representation of the flags. E.g. Flags: 0x00000003 (IsFolder, IsMounted)
Sign in to reply to this message.
Code updated.
Sign in to reply to this message.
https://codereview.appspot.com/340110043/diff/100001/plaso/formatters/fsevent... File plaso/formatters/fseventsd.py (right): https://codereview.appspot.com/340110043/diff/100001/plaso/formatters/fsevent... plaso/formatters/fseventsd.py:27: # those described in https://developer.apple.com/documentation/coreservices/core_services_enumerat... On 2018/01/24 06:46:20, Joachim Metz wrote: > at the moment I have no idea where these values (originally) come from, I see > some references in http://nicoleibrahim.com/apple-fsevents-forensics/ but it > seems that these values were determined by observation. > > Therefore I would recommend not to re-interpret these values ourselves, but keep > them as the other tools and make a note to try to determine what this values > actually mean. I would also recommend adding a hexadecimal representation of the > flags. E.g. > > > Flags: 0x00000003 (IsFolder, IsMounted) Done, https://github.com/log2timeline/plaso/issues/1683
Sign in to reply to this message.
LGTM, thx for adding this
Sign in to reply to this message.
Changes have been merged with master branch. To close the review and clean up the feature branch you can run: review.py close fsevents
Sign in to reply to this message.
|