-
Notifications
You must be signed in to change notification settings - Fork 89
Adjust teamviewer plugin to account for daylight savings time in timestamps #1614
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
Changes from 7 commits
1e13071
a7cdb7d
334d7fa
1d7e4c5
3562f9f
2d708e1
5e46152
f92238d
3c2e678
8734848
475f91d
5a29da7
c442096
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -132,17 +132,29 @@ def logs(self) -> Iterator[RemoteAccessLogRecord]: | |||||||||||||||||
| logfile = self.target.fs.path(logfile) | ||||||||||||||||||
|
|
||||||||||||||||||
| start_date = None | ||||||||||||||||||
| prev_timestamp = None | ||||||||||||||||||
| fold = 0 | ||||||||||||||||||
| for line in logfile.open("rt", errors="replace"): | ||||||||||||||||||
| if not (line := line.strip()) or line.startswith("# "): | ||||||||||||||||||
| continue | ||||||||||||||||||
|
|
||||||||||||||||||
| if line.startswith("Start:"): | ||||||||||||||||||
| try: | ||||||||||||||||||
| start_date = parse_start(line) | ||||||||||||||||||
| fold = 0 | ||||||||||||||||||
| except Exception as e: | ||||||||||||||||||
| self.target.log.warning("Failed to parse Start message %r in %s", line, logfile) | ||||||||||||||||||
| self.target.log.debug("", exc_info=e) | ||||||||||||||||||
|
|
||||||||||||||||||
| if start_date is None: | ||||||||||||||||||
| continue | ||||||||||||||||||
|
|
||||||||||||||||||
| # See whether the utcoffset with the two different timezones are the same | ||||||||||||||||||
| target_start_date = start_date.replace(tzinfo=target_tz) | ||||||||||||||||||
| if target_start_date.utcoffset() == start_date.utcoffset(): | ||||||||||||||||||
| # Adjust the start_date so it uses the timezone known to target throughtout | ||||||||||||||||||
Miauwkeru marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||||||||||||||
| start_date = target_start_date | ||||||||||||||||||
|
|
||||||||||||||||||
| continue | ||||||||||||||||||
|
|
||||||||||||||||||
| if not (match := RE_LOG.search(line)): | ||||||||||||||||||
|
|
@@ -178,14 +190,23 @@ def logs(self) -> Iterator[RemoteAccessLogRecord]: | |||||||||||||||||
| time += ".000000" | ||||||||||||||||||
|
|
||||||||||||||||||
| try: | ||||||||||||||||||
| timestamp = datetime.strptime(f"{date} {time}", "%Y/%m/%d %H:%M:%S.%f").replace( | ||||||||||||||||||
| tzinfo=start_date.tzinfo if start_date else target_tz | ||||||||||||||||||
| ) | ||||||||||||||||||
| tz_info = start_date.tzinfo if start_date else target_tz | ||||||||||||||||||
| timestamp = datetime.strptime(f"{date} {time}", "%Y/%m/%d %H:%M:%S.%f").replace(tzinfo=tz_info) | ||||||||||||||||||
| except Exception as e: | ||||||||||||||||||
| self.target.log.warning("Unable to parse timestamp %r in file %s", line, logfile) | ||||||||||||||||||
| self.target.log.debug("", exc_info=e) | ||||||||||||||||||
| timestamp = 0 | ||||||||||||||||||
|
||||||||||||||||||
| except Exception as e: | |
| self.target.log.warning("Unable to parse timestamp %r in file %s", line, logfile) | |
| self.target.log.debug("", exc_info=e) | |
| timestamp = 0 | |
| except Exception as e: | |
| self.target.log.warning("Unable to parse timestamp %r in file %s", line, logfile) | |
| self.target.log.debug("", exc_info=e) | |
| continue |
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.
I was also confused about that, as it would return 1 January 1970, but didn't touch it as it is how it worked before.
This does mean we won't be able to get a record if we couldn't parse the timestamp. Tho, it is still available in the logging.
With this patch, other logic can also be updated
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.
I think having a log line without a timestamp is preferred over ignoring the log line altogether, as it could hold forensic 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 only times I have seen this code path occurring is specifically with the following entries after Start: which occurs when the teamviewer service gets started:
Start: 2026/03/05 13:33:56.539 (UTC+5:30)
Version: 15.74.6 (64-bit)
Version short hash: e34788d05e8
ID: 0
Loglevel: Critical
License: 0
IC: -2056635597
CPU: Intel64 Family 6 Model 141 Stepping 1, GenuineIntel
CPU extensions: k0
OS: Win_10.0.26100_W (64-bit)
IP: 127.0.0.1
MID: 0x525400fad56f_1da8405dc678cec_983722041
MIDv: 0
Proxy-Settings: Type=1 IP= User=
IE: 11.1.26100.0
AppPath: C:\Program Files\TeamViewer\TeamViewer_Service.exe
UserAccount: SYSTEM
Server-Environment: Global
All following entries have a timestamp until another Start: gets is found.
It could be a good compromise to use timestamp=start_time here instead, as those entries belong to start_time
In the worst case, the entries won't have any timestamp then
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.
Or we could store the last timestamp we encountered and use that one instead.
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.
as we already store the previous timestamp for overlap detection that should be fine
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.
@JSCU-CNI I changed it to use the previous timestamp. Note that this path is only reached when the timestamp contains a number that is invalid for a timestamp. For example:
2025/10/26 02:50:90.300
If the time regex itself doesn't match, for example if the timestamp contained a letter instead of a number, it already gets ignored
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.
Assuming we have the target timezone self.target.datetime.tzinfo,
what if we renormalize every timestamp, instead of detecting deltas:
naive_dt = datetime.strptime(f"{date} {time}", "%Y/%m/%d %H:%M:%S.%f")
timestamp = naive_dt.replace(tzinfo=target_tz)
if prev_timestamp and prev_timestamp > timestamp:
timestamp = timestamp.replace(fold=1)
prev_timestamp = timestamp
Would that not also help with "spring forward" events?
In other words, why not rely on the target_tz calendar rules?
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.
I didn't know that the fold was used for this. I did notice when working on it that the fold doesn't get passed to flow.record. I made this PR for it fox-it/flow.record#217
Uh oh!
There was an error while loading. Please reload this page.