Skip to content
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

[Suggestion]: better error message if sync fails due to time difference (or allow sync with large time difference) #95

Open
1 task done
umlaeute opened this issue Apr 8, 2024 · 5 comments
Assignees
Labels
enhancement New feature or request

Comments

@umlaeute
Copy link

umlaeute commented Apr 8, 2024

Suggestion

so today i've managed to sync my laptop with my PC.
it took a couple of trials though, even though both were on the same local network.

synchronisation failed, and clicking on the error message i got something like:

Packet took too long to reach its destination, please check your network conditions

now the network conditions were fine (i checked and double checked).

eventually i found out that my PC's clock had drifted by 80 seconds or so.
manually syncing the clock withntpdate fixed the problem.

so i think the error message is somewhat misleading: the packet reached the destination in time (ping tells me that the roundtrip trime is between 1..2ms), but the timestamp in the package differed too much from the expected value.

probably the client could send a "ping"-like packet where the server responds with its own timestamp, so a more meaningful error message could be displayed.
(if that's necessary at all; it would of course be nice if the two could sync even if the time is off)

Submission checklist

  • I have specified my suggestion in the issue title
@umlaeute umlaeute added the enhancement New feature or request label Apr 8, 2024
@GleammerRay
Copy link
Collaborator

Hello @umlaeute . Sure thing, I will look into allowing the client to find the server time difference and adjust to it if necessary, but I am not making authentication an optional thing as it provides a nice additional layer of security.

@umlaeute
Copy link
Author

umlaeute commented Apr 8, 2024

but I am not making authentication an optional thing as it provides a nice additional layer of security.

not entirely sure what you mean, but of course, i was not suggesting anything like that :-)

@GleammerRay
Copy link
Collaborator

Apologies, I misread what you wrote earlier. Of course, I believe that it would be pretty trivial to resolve for time difference.

@turtaf
Copy link

turtaf commented Apr 23, 2024

+1
I didn't realize I had no time sync on my server and with the only error message I would not be able to understand what the problem was. Thankfully I found this issue...

@GleammerRay
Copy link
Collaborator

I am planning to implement the use of a cryptographic challenge for the synchronization layer instead of time-based authentication for the next release.

This will allow for better security and better compatibility with systems that use non-matching time zones.

GleammerRay added a commit that referenced this issue Jul 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants