Re: [PATCH 2/2] services: Add tlsdate-service.

[Chris Marusich]

Hi ng0!

Neat, I never knew about tlsdate until now!

ng0 <address@hidden> writes:

> * gnu/services/networking.scm (<tlsdate-configuration>): New record type.
> (%tlsdate-accounts): New variables.
> (tlsdate-shepherd-service): New procedure.
> (tlsdate-service-type): New variable.
> * doc/guix.texi (Networking Services): Document it.
> ---
>  doc/guix.texi               | 32 +++++++++++++++++
>  gnu/services/networking.scm | 84 
> ++++++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 115 insertions(+), 1 deletion(-)
>
> diff --git a/doc/guix.texi b/doc/guix.texi
> index 4d7f96d90..f6efd040d 100644
> --- a/doc/guix.texi
> +++ b/doc/guix.texi
> @@ -8594,6 +8594,38 @@ make an initial adjustment of more than 1,000 seconds.
>  List of host names used as the default NTP servers.
>  @end defvr
>  
> address@hidden tlsdate
> address@hidden {Scheme Procedure} tlsdate-service [#:config 
> (tlsdate-configuration)]
> +
> +Return a service that runs @command{tlsdate}, a simple TCP based time 
> service.
> +The daemon will synchronize the system clock with a server of your
> +choice via TCP at boot.

When you said TCP, did you mean TLS?

Are we running the daemon, or are we just running the tlsdate command
once at boot?

> +The optional @var{config} argument should be a
> address@hidden<tlsdate-configuration>} object, by default it syncs the time 
> with gnu.org.
> +
> address@hidden deffn
> +
> address@hidden {Data Type} tlsdate-configuration
> +Data type representing the configuration of tlsdate.
> +
> address@hidden @asis
> address@hidden @code{package} (default: @var{tlsdate})
> +Package object of the tlsdate time service.
> +
> address@hidden @code{port} (default: @var{'()})
> +Set the port of the remote hostname which should be used.
> +
> address@hidden @code{host} (default: @var{"gnu.org"})
> +Set the remote hostname which will be queried. Defaults to @code{gnu.org}.

I've briefly read about tlsdate.  Looks like it's a hack to sync time
from a "trusted" source using TLS.  Since this trust will vary by user,
wouldn't it be better to leave this field to the discretion of the user?

I think one could also argue that to be good Internet neighbors, we
should avoid putting a default hostname in here, to avoid spamming one
specific host by default with all the tlsdate traffic from this service.

> address@hidden @code{extra-options} (default: @var{'()})
> +Extra options will be passed to @code{tlsdate}, please run
> address@hidden tlsdate} for more information.
> +
> address@hidden table
> address@hidden deftp
> +
>  @cindex Tor
>  @deffn {Scheme Procedure} tor-service address@hidden [#:tor @var{tor}]
>  Return a service to run the @uref{https://torproject.org, Tor} anonymous
> diff --git a/gnu/services/networking.scm b/gnu/services/networking.scm
> index d672ecf68..52a899b8f 100644
> --- a/gnu/services/networking.scm
> +++ b/gnu/services/networking.scm
> @@ -3,6 +3,7 @@
>  ;;; Copyright © 2015 Mark H Weaver <address@hidden>
>  ;;; Copyright © 2016 Efraim Flashner <address@hidden>
>  ;;; Copyright © 2016 John Darrington <address@hidden>
> +;;; Copyright © 2016 ng0 <address@hidden>
>  ;;;
>  ;;; This file is part of GNU Guix.
>  ;;;
> @@ -66,7 +67,13 @@
>              wicd-service
>              network-manager-service
>              connman-service
> -            wpa-supplicant-service-type))
> +            wpa-supplicant-service-type
> +
> +            tlsdate-service
> +            tlsdate-configuration
> +            tlsdate-service-type
> +            tlsdate-configuration?
> +            tlsdate-configuration))
>  
>  ;;; Commentary:
>  ;;;
> @@ -360,6 +367,81 @@ make an initial adjustment of more than 1,000 seconds."
>  
>
>  ;;;
> +;;; tlsdate
> +;;;
> +
> +(define-record-type* <tlsdate-configuration>
> +  tlsdate-configuration make-tlsdate-configuration
> +  tlsdate-configuration?
> +  (package        tlsdate-configuration-package
> +                  (default tlsdate))
> +  (host           tlsdate-configuration-host
> +                  (default "gnu.org"))
> +  (port           tlsdate-configuration-port
> +                  (default #f))
> +  (extra-options  tlsdate-configuration-extra-options
> +                  (default '())))
> +
> +(define %tlsdate-accounts
> +  (list (user-group (name "tlsdate") (system? #t))
> +        (user-account
> +         (name "tlsdate")
> +         (group "tlsdate")
> +         (system? #t)
> +         (comment "tlsdate daemon user")

Is this really a daemon?  It looks like we're just invoking a command
which runs once at boot, but perhaps I'm mistaken.

> +         (home-directory "/var/empty")
> +         (shell (file-append shadow "/sbin/nologin")))))
> +
> +(define tlsdate-shepherd-service
> +  (match-lambda
> +    (($ <tlsdate-configuration>
> +        package host port extra-options)
> +     (let* ((tlsdate (file-append package "/bin/tlsdate"))
> +            (command `(,tlsdate
> +                       "-l" ; leap
> +                       "-t" ; timewarp
> +                       ,@(if host
> +                             `(,(string-append
> +                                 "-H" " " host))
> +                             '())
> +                       ,@(if port
> +                             `(,(string-append
> +                                 "-p" " " (number->string port)))
> +                             '())
> +                       ,@extra-options)))
> +       (list (shepherd-service
> +              ;;(provision '(tlsdate))
> +              (provision '(ntp))
> +              ;; tlsdate needs at least one network interface to be up, 
> hence the
> +              ;; dependency on 'loopback'.
> +              (requirement '(user-processes loopback syslogd))
> +
> +              (start #~(make-forkexec-constructor '#$command

Just so I'm clear here, you've written '#$command in the gexp here
because when command is expanded via ungexp (#$), it will be expanded
into a list, and you intend to pass that list literally to the
make-forkexec-constructor procedure (at the time the gexp is run), which
is why you used quote (').  Right?

> +                                                  #:user "tlsdate"
> +                                                  #:group "tlsdate"))
> +              (stop #~(make-kill-destructor))

Does this "service" actually spawn a long-running process?  If not, will
this destructor actually be used?  If it IS used, will it send a signal
to a non-existent process (or worse, a process that happens to share the
same PID as the one we previously spawned)?  Will Shepherd repeatedly
re-spawn the "service"?  I'm still learning how Shepherd services work,
so if I'm missing something obvious, please let me know.

> +              (documentation "Run the tlsdate service.")))))))
> +
> +(define tlsdate-service-type
> +  (service-type
> +   (name 'tlsdate)
> +   (extensions
> +    (list (service-extension shepherd-root-service-type
> +                             tlsdate-shepherd-service)
> +          (service-extension account-service-type
> +                             (const %tlsdate-accounts))))))
> +
> +(define* (tlsdate-service #:key (config (tlsdate-configuration)))
> +  "Return a service that runs @command{tlsdate}, a simple TCP based
> +time service.

When you said TCP, did you mean TLS?

> +The optional @var{config} argument should be a
> address@hidden<tlsdate-configuration>} object, by default it querries gnu.org
> +for time once at boot."
> +  (service tlsdate-service-type config))

When I talked with davexunit on IRC about doing something with a
side-effect once at boot, he was not in support of the idea [1], and I
agree he was right about that.  Maybe this tlsdate service is a little
different since it's happening once at EVERY boot instead of just the
very first one.  However, if we're going to add a tlsdate service,
wouldn't it make sense to use tlsdated instead of invoking tlsdate once?

Alternatively, if using tlsdated is difficult for some reason, is it
feasible to hold off on adding this service?  It seems to me like you
could accomplish what you need by running tlsdate ad-hoc when you need
it, or perhaps by configuring your own mcron job to run it periodically
(see: (guix) Scheduled Job Execution).  Again, I might be missing
something, so please let me know if that isn't a feasible alternative to
this service you're proposing.

[1] See https://gnunet.org/bot/log/guix/2016-11-30#T1216852 (excerpt follows):

[14:29:15]      <marusich>      davexunit, I see. Cool. Also, is there a way to 
do something "one time at first boot" in a GuixSD system, I wonder?
[14:29:44]      <davexunit>     marusich: no
[14:29:52]      <davexunit>     that goes against everything GuixSD is about
[14:30:08]      <davexunit>     if the only the first generation of a system 
did some side-effect
[14:30:14]      <davexunit>     it would be hard to reproduce elsewhere

-- 
Chris

signature.asc
Description: PGP signature