sliver icon indicating copy to clipboard operation
sliver copied to clipboard

Incorrect "Next Check-In" when target system clock is not up to date

Open mrThe opened this issue 2 years ago • 7 comments

Describe the bug Minor issue when target clock is not up to date. In beacons list in column "Next Check-In" it will show weird things like "-47m35.154999998s"

To Reproduce Steps to reproduce the behavior:

  1. Run sliver
  2. Generate beacon
  3. Run beacon on host with not synced time, in my case, it's off by around 40 minutes
  4. See error

Expected behavior Don't rely on target datetime, and calculate next checkin using server time, so it will be correct.

Screenshots image

Sliver: [*] Client v1.5.12 - aacf2c16ac00e4609231f5faee588bdb9ea9c532 - linux/amd64 Compiled at 2022-04-20 20:06:30 +0000 UTC Compiled with go version go1.18 linux/amd64

[*] Server v1.5.12 - aacf2c16ac00e4609231f5faee588bdb9ea9c532 - linux/amd64 Compiled at 2022-04-20 20:06:30 +0000 UTC

mrThe avatar May 22 '22 15:05 mrThe

Matching clocks are currently a requirement of certain protocols, but in some cases we could implement something to detect if the clocks are far out of sync

moloch-- avatar May 23 '22 15:05 moloch--

Well, i guess after #685 there is another issue for that case.. image :)

Beacon works as expected tho.

UPDATE: Looks like this fails only if we are using fresh version of server and an old beacon. For newer ones everyting looks more promising:

image (there is a few minutes between beacons listings)

mrThe avatar May 30 '22 15:05 mrThe

Yea, i think @RafBishopFox pushed this change.

moloch-- avatar May 30 '22 17:05 moloch--

I am not sure how to preserve backwards compatibility in this case. There is not enough information in the beacon registration message to differentiate an "old" beacon from a "new" one.

Previously, the beacon registration message included a Unix timestamp that represented it was scheduled to check in next. The calculation happened on the implant side and that is what resulted in the issue you saw. My PR changed things so that the implant reported how many seconds until its next check in. This made implants generated since the change unaffected by an implant with an out of sync clock on the implant side.

For backwards compatibility, we could see if the next check in number provided by the implant makes sense given the interval and the jitter. If the check in number is somewhere between interval and interval plus jitter seconds, then it is probably a new implant. Otherwise it is "old". This will not fix the issue you reported for old implants though since there is not enough information in the beacon registration message to know if the implant's clock is skewed. For implants made after the PR, the check in time should be more accurate.

@mrThe - What was the beacon you generated to see the behavior you saw in your update? I did not see any of my test implants with the same last check in time and next check in time.

RafBishopFox avatar May 31 '22 13:05 RafBishopFox

I think the slight backwards incompatibility is fine so long as it's not crashing.

moloch-- avatar May 31 '22 16:05 moloch--

Yeah - this is only affecting the display of the beacon time, not the actual beacon itself.

RafBishopFox avatar May 31 '22 16:05 RafBishopFox

@RafBishopFox It was just a regular beacon for x64 linux, with all default settings, except for mtls connection address ofc. Generated on version mentioned in first message.

Well, technically i don't think that back compatibility is such a big deal, since it doesn't break anything critical. Also, the beacon from the second screenshot are still running, and it works as expected, showing the right time, so i guess it's safe to close this issue.

mrThe avatar May 31 '22 16:05 mrThe