sdwdate and sdwdate-gui development thread

Patrick · August 20, 2015, 7:33pm

Did you “git fetch” beforehand?

troubadour · August 21, 2015, 10:55pm

Yes.

Patrick · August 22, 2015, 12:22am

Worked for me this way.

If not…

Should show.

commit b66ca854ea1d2be60b738582723acc849921c830
Author: Patrick Schleizer <adrelanos@riseup.net>
Date:   Wed Aug 19 23:37:06 2015 +0000

    workaround for 'dh_installinit should run systemd-tmpfiles if a /usr/lib/tmpfiles.d/ snippet gets shipped for systemd-only packages also' - http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=795519

troubadour · August 23, 2015, 6:56pm

Done, thanks.

Some commits.

sdwdate
Create a success file along first_success. Checked every cycle, it allows to show the proper icon and message when sdwdate is restarted “fresh”. first_success is created on the first cycle after boot.
sdwdate-gui
After confusing myself with tmpfiles.d and sudoers.d, sdwdate-gui is run as user sdwdate. The sdwdate tmpfiles.d can be reused and more important, to remove /var/run/sdwdate/success, “sudo rm” has to be allowed passwordless, and it seems safe to restrict it to sdwdate.

Patrick · August 23, 2015, 8:36pm

Running sdwdate-gui as user sdwdate is an interesting idea.

In that case, we have to add Pre-Depends in sdwdate-gui’s debian/control.

Pre-Depends: sdwdate

Because sdwdate keeps care of creating the user sdwdate and the folder /var/run/sdwdate.

[hr]

So at the moment we have sdwdate/sdwdate.conf at python · troubadoour/sdwdate · GitHub

d /var/run/sdwdate 0775 sdwdate sdwdate

vs

github.com

troubadoour/sdwdate-gui/blob/master/usr/lib/tmpfiles.d/sdwdate-gui.conf

## Copyright (C) 2012 - 2018 ENCRYPTED SUPPORT LP <adrelanos@riseup.net>
## See the file COPYING for copying conditions.

d /run/sdwdate-gui 0775 sdwdate-gui sdwdate-gui
f /run/sdwdate-gui/anon-status 0644 sdwdate-gui sdwdate-gui

d /var/run/sdwdate 0775 sdwdate-gui sdwdate-gui

I guess that’s a conflict and going to break.

As per http://www.freedesktop.org/software/systemd/man/tmpfiles.d.html:

#Type Path        Mode UID  GID  Age Argument

So it cannot be both. So sdwdate-gui/sdwdate-gui.conf at master · troubadoour/sdwdate-gui · GitHub should be removed.

When sdwdate-gui runs as user sdwdate, then it does neither require sudo nor sudoers.d exceptions, because user sdwdate is allowed to create/delete any file inside /var/run/sdwdate so or so. Just make sure when manually running sdwdate-gui to start it as user sdwdate using “sudo -u sdwdate sdwdate-gui”.

troubadour · August 23, 2015, 9:26pm

In that case, we have to add Pre-Depends in sdwdate-gui's debian/control.

Done.

So it cannot be both. So https://github.com/troubadoour/sdwdate-gui/blob/master/usr/lib/tmpfiles.d/sdwdate-gui.conf should be removed.

My mistake, I thought it was removed in a previous commit. Done.

When sdwdate-gui runs as user sdwdate, then it does neither require sudo nor sudoers.d exceptions, because user sdwdate is allowed to create/delete any file inside /var/run/sdwdate so or so. Just make sure when manually running sdwdate-gui to start it as user sdwdate using "sudo -u sdwdate sdwdate-gui".

I did test it earlier. Removing any exception in /etc//sudoers.d/sdwdate-gui breaks it, even [code]ALL ALL=NOPASSWD: /usr/sbin/service sdwdate *[/code] which is declared in /etc/sudoers.d/sdwdate. Still breaking now.

troubadour · August 23, 2015, 9:36pm

Fixed a lintian warning (stronger-dependency-implies-weaker).

Patrick · August 23, 2015, 11:09pm

Yes, the “sudo service” commands will still require sudoers.d exceptions. My mistake pointing this out earlier. Just “rm” should work without sudo.

On a second thought, maybe I was wrong about Pre-Depends. Even if sdwdate is just a normal Depends. There should be no way for sdwdate-gui to start before sdwdate has also been installed, because sdwdate-gui is not automatically started during package installation [such as though a systemd service]. It’s only automatically started a next [or first] boot or manually after installation.

troubadour · August 28, 2015, 7:16pm

Yes, removed the sdwdate dependencies in sdwdate-gui. Being started in /etc/xdg/autostart, If swdate is not installed or not running, an error icon is displayed in system tray, reporting so.

Some commits.

sdwdate-gui:

open sdwdate log. It opens a konsole with command “tail -f -n 100 /var/log/sdwdate.log”. That’s open to discussion.

sdwdate:

timesanitycheck. A separate module, check at the beginning of every sdwdate_loop cycle. If slow or fast, report and restart a normal sleep period. No retry.

Note: expiration time is hardcoded in the script. If necessary, we can recreate /etc/default/sdwdate (it’s no longer timesantycheck).

Patrick · August 29, 2015, 2:29am

Hardcoded is fine.

This is expected. You probably mean “greater than expiration timestamp”.

Patrick · August 29, 2015, 10:16am

eaxh -> each
teturned -> returned

[hr]

I think this can be refactored a bit.

            if sdwdate.success:
                if sdwdate.set_new_time():
                    sdwdate.add_subtract_nanoseconds()
                    sdwdate.run_sclockadj()
            else:
                if not sdwdate.first_success:
                    f = open(sdwdate.first_success_path, 'w')
                    f.close()
                f = open(sdwdate.success_path, 'w')
                f.close()

                if sdwdate.set_new_time():
                    sdwdate.add_subtract_nanoseconds()
                    sdwdate.set_time_using_date()

            message = sdwdate.message + '\nSleeping.'
            sdwdate.write_status(icon, message)
            sleep_time = 10

->

        if status == 'success':
            sdwdate.build_median()
            sdwdate.add_subtract_nanoseconds()
            if sdwdate.set_new_time():
                if sdwdate.success:
                    sdwdate.run_sclockadj()
                else:
                    sdwdate.set_time_using_date()
                    if not sdwdate.first_success:
                        f = open(sdwdate.first_success_path, 'w')
                        f.close()

            f = open(sdwdate.success_path, 'w')
            f.close()

Running sdwdate.set_time_using_date() earlier so the status files get the right timestamp. (And because in corner cases it could fai: hdd failure, damaged date binary.)

And putting
f = open(sdwdate.success_path, ‘w’)
f.close()
outside the if. Should also happen in case of sclockadj.

Theoretically… Unsure… Also
if not sdwdate.first_success:
f = open(sdwdate.first_success_path, ‘w’)
f.close()
could be put outside the if. Does not matter as long as date vs sclockadj is not configurable. But somehow looks more correct?

[hr]

Can you also run timesanitycheck as a check for results from remote?

Patrick · August 29, 2015, 11:01am

Extraneous space.

sdwdate: Please remove usr/share/icons/anon-icon-pack folder.
sdwdate-gui: Wrong icon path. It’s just inside usr/share/icons folder. Misses sub folder. Move to usr/share/icons/sdwdate-gui. (Right in the code, but wrong location in the source folder.)
“No internet. Fetching remote times…” -> ‘No internet’ sounds like no internet access is available at all for anything. We need a more causal mental connection. “Outgoing internet access is still restricted because initial time fetching is not done yet. Fetching remote times…” Or something like that.
I like the log viewer, this encourages scrutiny.

Patrick · August 29, 2015, 11:20am

Expiration timestamp is much too young. In 2014. You probably left that in for testing, I guess.

If timesanitycheck before running sdwdate [not after fetching remotes] failed, the messages:
"The clock is %s. Current time “%s” is less than the build timestamp “%s”’ or
"The clock is %s. Current time “%s” is greater than the expriration timestamp"
are technically correct, but does not provide any insight to the user on what the cause could be [bios clock failure, host clock attack succeeded, user set clock wrong on host for some reason either on purpose or by mistake] or how to fix the situation [power off, fix host clock, power on again or manually set clock in VM + restart tor, most likely].

Unsure, what about a right click “manually set the time” menu?

troubadour · September 2, 2015, 8:28pm

Some of the messages were written on the corner of a table, without much care. We’ll have to improve the verbose. The messages should end in a yaml file eventually, same mechanics as whonix-setup-wizard. sdwdate could pass short messages to sdwate-gui, which would translate them into something unserstandable. Looks like a good approach to me, because it would wipe the code from most of the text, and for possible future translations, whatever we might use (yaml or gettext), it would be ready.

I’ll fix the other mistakes. In the meantime, have replaced the default left click message by a simple gui that mimics (roughly) the original. It updates itself on status changes. The text is copyable but it has to be closed by the user. It’s basic in the view of keeping it as little intrusive as possible.

troubadour · September 3, 2015, 4:02am

Forget the last commit. One piece is miising and the message is popping on every status change.

troubadour · September 6, 2015, 3:41pm

A digest of the latest commits.

sdwdate

fixed typos.
fixed expiration times (was a test value left).
HTML provisionally in sdwdate, because the new popup message requires it.
removed usr/share/icons/anon-icon-pack folder.
changed icon folder in source.
improved main loop as per Whonix Forum. first_succes check is outside “if sdwdate.set_new_time()”.

[code] if status == ‘success’:
sdwdate.build_median()
sdwdate.add_subtract_nanoseconds()
if sdwdate.set_new_time():
if sdwdate.success:
sdwdate.run_sclockadj()
else:
sdwdate.set_time_using_date()
if not sdwdate.first_success:
f = open(sdwdate.first_success_path, ‘w’)
f.close()

        f = open(sdwdate.success_path, 'w')
        f.close()[/code]

Much better, I must admit.

sdwdate-gui. The last three commits are relevant.

the gui message is stripped from anything fancy, only the message in richtext format, copyable. It is shown/hidden by left mouse action, updated if status change while it’s open.
all the icons from sdwdate and sdwadte-gui are in /usr/share/icons/sdwdate-gui.

If timesanitycheck before running sdwdate [not after fetching remotes] failed, the messages: "The clock is %s. Current time "%s" is less than the build timestamp "%s"' or "The clock is %s. Current time "%s" is greater than the expriration timestamp" are technically correct, but does not provide any insight to the user on what the cause could be [bios clock failure, host clock attack succeeded, user set clock wrong on host for some reason either on purpose or by mistake] or how to fix the situation [power off, fix host clock, power on again or manually set clock in VM + restart tor, most likely].

Yes. We could run timesanitycheck after fetching remotes too, although the likeliness of having three remotes failing the check is quite low.

Unsure, what about a right click "manually set the time" menu?

Will look into it. If it'feasible, why not?

troubadour · September 7, 2015, 7:54pm

Probably not. The messages have to be written to the log too. If it’s still technically possible to fetch the messages and strip the HTML before writing them, It does not look the correct way for a daemon to depend on an external file for its logging.

Patrick · September 7, 2015, 10:33pm

calcuations → calculations

Should we use sclockadj_debug_helper?

Probably no need. The permission issues are gone for a while now. The maintainer script / command line parameters have been sorted out. So that comment can be deleted. Won't hurt though keeping the commented out sclockadj debug helper code in for some more time. (Should be logged rather than echo'ed.)

[hr]

Looks good.

The window title for the left click action however “show_message” is not good.

I don’t see a problem with depending on python-guimessages or having an external file containing the messages. The status file could contain content with full html, which is stripped before adding that to the log. However, I think for now translations are low priority. Generating quite some effort accepting those. I guess we would need more contributors. Or more time.

Yes. We could run timesanitycheck after fetching remotes too, although the likeliness of having three remotes failing the check is quite low.

Yes, three servers would have to freak out and report bogus results. Filtering out every single bogus result however seems worthwhile.

Patrick · September 8, 2015, 2:53pm

Please git cherry-pick the following commit in the python branch.

disable systemd-timesyncd by default as it conflicts with sdwdate - https://phabricator.whonix.org/T336

https://github.com/Whonix/sdwdate/commit/83ef0068cb7c0287d6f2d6feab2d8d4961f8a260

troubadour · September 10, 2015, 8:13pm

Please git cherry-pick the following commit in the python branch.

Done.

The window title for the left click action however "show_message" is not good.

Now the window is a full GUI (with a proper title). I was reluctant to do it that way because it becomes resizable, it can be mximized / minmized, bur on the whole, it looks better.

Have added the url comments in the log. url comments in log · troubadoour/sdwdate@64b8685 · GitHub
Will add human readable time beside every unixtime entry.