unofficial mirror of guix-patches@gnu.org 
 help / color / mirror / code / Atom feed
* [bug#29483] [PATCH] services: Add openntpd service.
@ 2017-11-28  9:04 Efraim Flashner
  2017-12-01 10:19 ` Ludovic Courtès
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Efraim Flashner @ 2017-11-28  9:04 UTC (permalink / raw)
  To: 29483; +Cc: Efraim Flashner

* gnu/packages/ntp.scm (openntpd)[arguments]: Add 'configure-flags to
set openntpd daemon's user and protected path. Add a custom phase to not
try to create said directory at install time.
* gnu/services/networking.scm (<openntpd-configuration>): New record type.
(openntpd-shepherd-service, openntpd-service-activation): New procedures.
(openntpd-service-type): New variable.
* doc/guix.texi (Networking Services): Add openntpd documentation.
---
 doc/guix.texi               | 11 ++++++
 gnu/packages/ntp.scm        | 12 ++++++
 gnu/services/networking.scm | 92 ++++++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 114 insertions(+), 1 deletion(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 2a6825682..f0a7dd958 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -10498,6 +10498,17 @@ make an initial adjustment of more than 1,000 seconds.
 List of host names used as the default NTP servers.
 @end defvr
 
+@cindex Openntpd
+@deffn {Scheme Procedure} openntpd-service [#:openntpd @var{openntpd}] @
+  [#:servers @var{%ntp-servers}] @
+  [#:allow-large-adjustment? #f]
+Return a service that runs the daemon from @var{openntpd}, the
+@uref{http://www.openntpd.org, OpenNTPD package}.  The daemon will
+keep the system clock synchronized with that of @var{servers}.
+@var{allow-large-adjustment?} determines whether @command{ntpd} is allowed to
+make an initial adjustment of more than 180 seconds."
+@end deffn
+
 @cindex inetd
 @deffn {Scheme variable} inetd-service-type
 This service runs the @command{inetd} (@pxref{inetd invocation,,,
diff --git a/gnu/packages/ntp.scm b/gnu/packages/ntp.scm
index d270f513d..619b9f998 100644
--- a/gnu/packages/ntp.scm
+++ b/gnu/packages/ntp.scm
@@ -107,6 +107,18 @@ computers over a network.")
                (base32
                 "0fn12i4kzsi0zkr4qp3dp9bycmirnfapajqvdfx02zhr4hanj0kv"))))
     (build-system gnu-build-system)
+    (arguments
+     '(#:configure-flags '("--with-privsep-user=ntpd"
+                           "--with-privsep-path=/var/lib/openntpd"
+                           "--localstatedir=/var/lib/openntpd")
+       #:phases
+       (modify-phases %standard-phases
+         (add-after 'unpack 'modify-install-locations
+           (lambda _
+             ;; Don't try to create /var/lib/openntpd/run or /var/lib/openntpd/db
+             (substitute* "src/Makefile.in"
+               (("DESTDIR\\)\\$\\(localstatedir") "TMPDIR"))
+             #t)))))
     (inputs
      `(("libressl" ,libressl))) ; enable TLS time constraints. See ntpd.conf(5).
     (home-page "http://www.openntpd.org/")
diff --git a/gnu/services/networking.scm b/gnu/services/networking.scm
index b0c23aafc..82762738f 100644
--- a/gnu/services/networking.scm
+++ b/gnu/services/networking.scm
@@ -1,7 +1,7 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2013, 2014, 2015, 2016, 2017 Ludovic Courtès <ludo@gnu.org>
 ;;; Copyright © 2015 Mark H Weaver <mhw@netris.org>
-;;; Copyright © 2016 Efraim Flashner <efraim@flashner.co.il>
+;;; Copyright © 2016, 2017 Efraim Flashner <efraim@flashner.co.il>
 ;;; Copyright © 2016 John Darrington <jmd@gnu.org>
 ;;; Copyright © 2017 Clément Lassieur <clement@lassieur.org>
 ;;; Copyright © 2017 Thomas Danckaert <post@thomasdanckaert.be>
@@ -62,6 +62,11 @@
             ntp-service
             ntp-service-type
 
+            openntpd-configuration
+            openntpd-configuration?
+            openntpd-service
+            openntpd-service-type
+
             inetd-configuration
             inetd-entry
             inetd-service-type
@@ -447,6 +452,91 @@ make an initial adjustment of more than 1,000 seconds."
                               (allow-large-adjustment?
                                allow-large-adjustment?))))
 
+(define-record-type* <openntpd-configuration>
+  openntpd-configuration make-openntpd-configuration
+  openntpd-configuration?
+  (openntpd                openntpd-configuration-openntpd
+                           (default openntpd))
+  (servers                 openntpd-configuration-servers)
+  (allow-large-adjustment? openntpd-allow-large-adjustment?
+                           (default #f))) ; upstream default
+
+(define openntpd-shepherd-service
+  (match-lambda
+    (($ <openntpd-configuration> openntpd servers allow-large-adjustment?)
+     (let ()
+       (define config
+         (string-append (string-join (map (cut string-append "server " <>)
+                                          servers)
+                                     "\n")
+                        "
+# Only listen on localhost
+listen on 127.0.0.1
+listen on ::1
+
+# Query the 'Date' from trusted HTTPS servers via TLS.
+constraint from www.gnu.org\n"))
+
+       (define ntpd.conf
+         (plain-file "ntpd.conf" config))
+
+       (list (shepherd-service
+              (provision '(openntpd))
+              (documentation "Run the Network Time Protocol (NTP) daemon.")
+              (requirement '(user-processes networking))
+              (start #~(make-forkexec-constructor
+                        (list (string-append #$openntpd "/sbin/ntpd")
+                              "-f" #$ntpd.conf
+                              #$@(if allow-large-adjustment?
+                                     '("-s")
+                                     '()))))
+              (stop #~(make-kill-destructor))))))))
+
+(define (openntpd-service-activation config)
+  "Return the activation gexp for CONFIG."
+  (with-imported-modules '((guix build utils))
+    #~(begin
+        (use-modules (guix build utils))
+        (define %user
+          (getpw "ntpd"))
+
+        (let ((directory "/var/lib/openntpd"))
+          (mkdir-p directory)
+          ;; and for the socket
+          (mkdir-p (string-append directory "/db"))
+          (mkdir-p (string-append directory "/run"))
+          (chown directory (passwd:uid %user) (passwd:gid %user))
+          (chmod directory #o755)))))
+
+(define openntpd-service-type
+  (service-type (name 'openntpd)
+                (extensions
+                 (list (service-extension shepherd-root-service-type
+                                          openntpd-shepherd-service)
+                       (service-extension account-service-type
+                                          (const %ntp-accounts))
+                       (service-extension activation-service-type
+                                          openntpd-service-activation)))
+                (description
+                 "Run the @command{ntpd}, the Network Time Protocol (NTP)
+daemon of the @uref{http://www.ntp.org, Network Time Foundation}, as
+implemented by OpenNTPD.  The daemon will keep the system clock synchronized
+with that of the given servers.")))
+
+(define* (openntpd-service #:key (openntpd openntpd)
+                           (servers %ntp-servers)
+                           allow-large-adjustment?)
+  "Return a service that runs the daemon from @var{openntpd}, the
+@uref{http://www.openntpd.org, OpenNTPD package}.  The daemon will
+keep the system clock synchronized with that of @var{servers}.
+@var{allow-large-adjustment?} determines whether @command{ntpd} is allowed to
+make an initial adjustment of more than 180 seconds."
+  (service openntpd-service-type
+           (openntpd-configuration (openntpd openntpd)
+                              (servers servers)
+                              (allow-large-adjustment?
+                               allow-large-adjustment?))))
+
 \f
 ;;;
 ;;; Inetd.
-- 
2.15.0

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [bug#29483] [PATCH] services: Add openntpd service.
  2017-11-28  9:04 [bug#29483] [PATCH] services: Add openntpd service Efraim Flashner
@ 2017-12-01 10:19 ` Ludovic Courtès
  2018-01-11 21:44   ` Ludovic Courtès
  2018-01-19 23:52   ` Ludovic Courtès
  2017-12-03 19:24 ` Marius Bakke
  2018-03-05  9:32 ` bug#29483: " Efraim Flashner
  2 siblings, 2 replies; 10+ messages in thread
From: Ludovic Courtès @ 2017-12-01 10:19 UTC (permalink / raw)
  To: Efraim Flashner; +Cc: 29483

Hello!

Efraim Flashner <efraim@flashner.co.il> skribis:

> * gnu/packages/ntp.scm (openntpd)[arguments]: Add 'configure-flags to
> set openntpd daemon's user and protected path. Add a custom phase to not
> try to create said directory at install time.
> * gnu/services/networking.scm (<openntpd-configuration>): New record type.
> (openntpd-shepherd-service, openntpd-service-activation): New procedures.
> (openntpd-service-type): New variable.
> * doc/guix.texi (Networking Services): Add openntpd documentation.

Nice!

> +@cindex Openntpd

“OpenNTPD” maybe?  Or all lower case?

> +@deffn {Scheme Procedure} openntpd-service [#:openntpd @var{openntpd}] @
> +  [#:servers @var{%ntp-servers}] @
> +  [#:allow-large-adjustment? #f]
> +Return a service that runs the daemon from @var{openntpd}, the
> +@uref{http://www.openntpd.org, OpenNTPD package}.  The daemon will
> +keep the system clock synchronized with that of @var{servers}.
> +@var{allow-large-adjustment?} determines whether @command{ntpd} is allowed to
> +make an initial adjustment of more than 180 seconds."
> +@end deffn

The convention now is to expose and document the configuration record
type and the service type, and to not provide a “foo-service” procedure.

Could you adjust accordingly?

> +(define-record-type* <openntpd-configuration>
> +  openntpd-configuration make-openntpd-configuration
> +  openntpd-configuration?
> +  (openntpd                openntpd-configuration-openntpd
> +                           (default openntpd))
> +  (servers                 openntpd-configuration-servers)

Probably with: (default %ntp-servers).

> +# Only listen on localhost
> +listen on 127.0.0.1
> +listen on ::1
> +
> +# Query the 'Date' from trusted HTTPS servers via TLS.
> +constraint from www.gnu.org\n"))

It would be nice to make that constraint server configurable too (not a
blocker though).

> +       (list (shepherd-service
> +              (provision '(openntpd))

Perhaps we should make that ‘ntpd’ so that it conflicts with the other
ntpd.

> +(define openntpd-service-type
> +  (service-type (name 'openntpd)
> +                (extensions
> +                 (list (service-extension shepherd-root-service-type
> +                                          openntpd-shepherd-service)
> +                       (service-extension account-service-type
> +                                          (const %ntp-accounts))

Are you sure that it uses those accounts?

> +                 "Run the @command{ntpd}, the Network Time Protocol (NTP)
> +daemon of the @uref{http://www.ntp.org, Network Time Foundation}, as
          ^---- remove -------------------------------------------^
> +implemented by OpenNTPD.  The daemon will keep the system clock synchronized
> +with that of the given servers.")))
> +
> +(define* (openntpd-service #:key (openntpd openntpd)
> +                           (servers %ntp-servers)
> +                           allow-large-adjustment?)

Remove.

Could you send an updated patch?

Thank you!

Ludo’.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [bug#29483] [PATCH] services: Add openntpd service.
  2017-11-28  9:04 [bug#29483] [PATCH] services: Add openntpd service Efraim Flashner
  2017-12-01 10:19 ` Ludovic Courtès
@ 2017-12-03 19:24 ` Marius Bakke
  2018-03-05  9:32 ` bug#29483: " Efraim Flashner
  2 siblings, 0 replies; 10+ messages in thread
From: Marius Bakke @ 2017-12-03 19:24 UTC (permalink / raw)
  To: Efraim Flashner, 29483

[-- Attachment #1: Type: text/plain, Size: 8708 bytes --]

Efraim Flashner <efraim@flashner.co.il> writes:

> * gnu/packages/ntp.scm (openntpd)[arguments]: Add 'configure-flags to
> set openntpd daemon's user and protected path. Add a custom phase to not
> try to create said directory at install time.
> * gnu/services/networking.scm (<openntpd-configuration>): New record type.
> (openntpd-shepherd-service, openntpd-service-activation): New procedures.
> (openntpd-service-type): New variable.
> * doc/guix.texi (Networking Services): Add openntpd documentation.
> ---
>  doc/guix.texi               | 11 ++++++
>  gnu/packages/ntp.scm        | 12 ++++++
>  gnu/services/networking.scm | 92 ++++++++++++++++++++++++++++++++++++++++++++-
>  3 files changed, 114 insertions(+), 1 deletion(-)
>
> diff --git a/doc/guix.texi b/doc/guix.texi
> index 2a6825682..f0a7dd958 100644
> --- a/doc/guix.texi
> +++ b/doc/guix.texi
> @@ -10498,6 +10498,17 @@ make an initial adjustment of more than 1,000 seconds.
>  List of host names used as the default NTP servers.
>  @end defvr
>  
> +@cindex Openntpd
> +@deffn {Scheme Procedure} openntpd-service [#:openntpd @var{openntpd}] @
> +  [#:servers @var{%ntp-servers}] @
> +  [#:allow-large-adjustment? #f]
> +Return a service that runs the daemon from @var{openntpd}, the
> +@uref{http://www.openntpd.org, OpenNTPD package}.  The daemon will
> +keep the system clock synchronized with that of @var{servers}.
> +@var{allow-large-adjustment?} determines whether @command{ntpd} is allowed to
> +make an initial adjustment of more than 180 seconds."
> +@end deffn
> +
>  @cindex inetd
>  @deffn {Scheme variable} inetd-service-type
>  This service runs the @command{inetd} (@pxref{inetd invocation,,,
> diff --git a/gnu/packages/ntp.scm b/gnu/packages/ntp.scm
> index d270f513d..619b9f998 100644
> --- a/gnu/packages/ntp.scm
> +++ b/gnu/packages/ntp.scm
> @@ -107,6 +107,18 @@ computers over a network.")
>                 (base32
>                  "0fn12i4kzsi0zkr4qp3dp9bycmirnfapajqvdfx02zhr4hanj0kv"))))
>      (build-system gnu-build-system)
> +    (arguments
> +     '(#:configure-flags '("--with-privsep-user=ntpd"
> +                           "--with-privsep-path=/var/lib/openntpd"
> +                           "--localstatedir=/var/lib/openntpd")

Do we have to change localstatedir?  Would it work to create
/var/run/ntpd.sock and chown it?  Or is this the common way of
deployment?  No strong opinion though.

> +       #:phases
> +       (modify-phases %standard-phases
> +         (add-after 'unpack 'modify-install-locations
> +           (lambda _
> +             ;; Don't try to create /var/lib/openntpd/run or /var/lib/openntpd/db
> +             (substitute* "src/Makefile.in"
> +               (("DESTDIR\\)\\$\\(localstatedir") "TMPDIR"))
> +             #t)))))
>      (inputs
>       `(("libressl" ,libressl))) ; enable TLS time constraints. See ntpd.conf(5).
>      (home-page "http://www.openntpd.org/")
> diff --git a/gnu/services/networking.scm b/gnu/services/networking.scm
> index b0c23aafc..82762738f 100644
> --- a/gnu/services/networking.scm
> +++ b/gnu/services/networking.scm
> @@ -1,7 +1,7 @@
>  ;;; GNU Guix --- Functional package management for GNU
>  ;;; Copyright © 2013, 2014, 2015, 2016, 2017 Ludovic Courtès <ludo@gnu.org>
>  ;;; Copyright © 2015 Mark H Weaver <mhw@netris.org>
> -;;; Copyright © 2016 Efraim Flashner <efraim@flashner.co.il>
> +;;; Copyright © 2016, 2017 Efraim Flashner <efraim@flashner.co.il>
>  ;;; Copyright © 2016 John Darrington <jmd@gnu.org>
>  ;;; Copyright © 2017 Clément Lassieur <clement@lassieur.org>
>  ;;; Copyright © 2017 Thomas Danckaert <post@thomasdanckaert.be>
> @@ -62,6 +62,11 @@
>              ntp-service
>              ntp-service-type
>  
> +            openntpd-configuration
> +            openntpd-configuration?
> +            openntpd-service
> +            openntpd-service-type
> +
>              inetd-configuration
>              inetd-entry
>              inetd-service-type
> @@ -447,6 +452,91 @@ make an initial adjustment of more than 1,000 seconds."
>                                (allow-large-adjustment?
>                                 allow-large-adjustment?))))
>  
> +(define-record-type* <openntpd-configuration>
> +  openntpd-configuration make-openntpd-configuration
> +  openntpd-configuration?
> +  (openntpd                openntpd-configuration-openntpd
> +                           (default openntpd))
> +  (servers                 openntpd-configuration-servers)
> +  (allow-large-adjustment? openntpd-allow-large-adjustment?
> +                           (default #f))) ; upstream default
> +
> +(define openntpd-shepherd-service
> +  (match-lambda
> +    (($ <openntpd-configuration> openntpd servers allow-large-adjustment?)
> +     (let ()
> +       (define config
> +         (string-append (string-join (map (cut string-append "server " <>)
> +                                          servers)
> +                                     "\n")
> +                        "
> +# Only listen on localhost
> +listen on 127.0.0.1
> +listen on ::1
> +
> +# Query the 'Date' from trusted HTTPS servers via TLS.
> +constraint from www.gnu.org\n"))

It would be good if these options are configurable.  A user may want to
use a different constraint server, or none at all, and maybe also expose
this as an SNTP service.  IIRC constraints can also be specified
multiple times, so maybe add #:listen-on and #:constraints ?

It would also be great to have a system test that at least verifies that
the default configuration is okay.  Testing NTP functionality may be
trickier.

> +
> +       (define ntpd.conf
> +         (plain-file "ntpd.conf" config))
> +
> +       (list (shepherd-service
> +              (provision '(openntpd))
> +              (documentation "Run the Network Time Protocol (NTP) daemon.")
> +              (requirement '(user-processes networking))
> +              (start #~(make-forkexec-constructor
> +                        (list (string-append #$openntpd "/sbin/ntpd")
> +                              "-f" #$ntpd.conf
> +                              #$@(if allow-large-adjustment?
> +                                     '("-s")
> +                                     '()))))
> +              (stop #~(make-kill-destructor))))))))
> +
> +(define (openntpd-service-activation config)
> +  "Return the activation gexp for CONFIG."
> +  (with-imported-modules '((guix build utils))
> +    #~(begin
> +        (use-modules (guix build utils))
> +        (define %user
> +          (getpw "ntpd"))
> +
> +        (let ((directory "/var/lib/openntpd"))
> +          (mkdir-p directory)
> +          ;; and for the socket
> +          (mkdir-p (string-append directory "/db"))
> +          (mkdir-p (string-append directory "/run"))
> +          (chown directory (passwd:uid %user) (passwd:gid %user))
> +          (chmod directory #o755)))))
> +
> +(define openntpd-service-type
> +  (service-type (name 'openntpd)
> +                (extensions
> +                 (list (service-extension shepherd-root-service-type
> +                                          openntpd-shepherd-service)
> +                       (service-extension account-service-type
> +                                          (const %ntp-accounts))
> +                       (service-extension activation-service-type
> +                                          openntpd-service-activation)))
> +                (description
> +                 "Run the @command{ntpd}, the Network Time Protocol (NTP)
> +daemon of the @uref{http://www.ntp.org, Network Time Foundation}, as
> +implemented by OpenNTPD.  The daemon will keep the system clock synchronized
> +with that of the given servers.")))
> +
> +(define* (openntpd-service #:key (openntpd openntpd)
> +                           (servers %ntp-servers)
> +                           allow-large-adjustment?)
> +  "Return a service that runs the daemon from @var{openntpd}, the
> +@uref{http://www.openntpd.org, OpenNTPD package}.  The daemon will
> +keep the system clock synchronized with that of @var{servers}.
> +@var{allow-large-adjustment?} determines whether @command{ntpd} is allowed to
> +make an initial adjustment of more than 180 seconds."
> +  (service openntpd-service-type
> +           (openntpd-configuration (openntpd openntpd)
> +                              (servers servers)
> +                              (allow-large-adjustment?
> +                               allow-large-adjustment?))))
> +
>  \f
>  ;;;
>  ;;; Inetd.
> -- 
> 2.15.0

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [bug#29483] [PATCH] services: Add openntpd service.
  2017-12-01 10:19 ` Ludovic Courtès
@ 2018-01-11 21:44   ` Ludovic Courtès
  2018-01-19 23:52   ` Ludovic Courtès
  1 sibling, 0 replies; 10+ messages in thread
From: Ludovic Courtès @ 2018-01-11 21:44 UTC (permalink / raw)
  To: Efraim Flashner; +Cc: 29483

Ping!

Let’s not let bitdust settle on this patch!

Ludo’.

ludo@gnu.org (Ludovic Courtès) skribis:

> Hello!
>
> Efraim Flashner <efraim@flashner.co.il> skribis:
>
>> * gnu/packages/ntp.scm (openntpd)[arguments]: Add 'configure-flags to
>> set openntpd daemon's user and protected path. Add a custom phase to not
>> try to create said directory at install time.
>> * gnu/services/networking.scm (<openntpd-configuration>): New record type.
>> (openntpd-shepherd-service, openntpd-service-activation): New procedures.
>> (openntpd-service-type): New variable.
>> * doc/guix.texi (Networking Services): Add openntpd documentation.
>
> Nice!
>
>> +@cindex Openntpd
>
> “OpenNTPD” maybe?  Or all lower case?
>
>> +@deffn {Scheme Procedure} openntpd-service [#:openntpd @var{openntpd}] @
>> +  [#:servers @var{%ntp-servers}] @
>> +  [#:allow-large-adjustment? #f]
>> +Return a service that runs the daemon from @var{openntpd}, the
>> +@uref{http://www.openntpd.org, OpenNTPD package}.  The daemon will
>> +keep the system clock synchronized with that of @var{servers}.
>> +@var{allow-large-adjustment?} determines whether @command{ntpd} is allowed to
>> +make an initial adjustment of more than 180 seconds."
>> +@end deffn
>
> The convention now is to expose and document the configuration record
> type and the service type, and to not provide a “foo-service” procedure.
>
> Could you adjust accordingly?
>
>> +(define-record-type* <openntpd-configuration>
>> +  openntpd-configuration make-openntpd-configuration
>> +  openntpd-configuration?
>> +  (openntpd                openntpd-configuration-openntpd
>> +                           (default openntpd))
>> +  (servers                 openntpd-configuration-servers)
>
> Probably with: (default %ntp-servers).
>
>> +# Only listen on localhost
>> +listen on 127.0.0.1
>> +listen on ::1
>> +
>> +# Query the 'Date' from trusted HTTPS servers via TLS.
>> +constraint from www.gnu.org\n"))
>
> It would be nice to make that constraint server configurable too (not a
> blocker though).
>
>> +       (list (shepherd-service
>> +              (provision '(openntpd))
>
> Perhaps we should make that ‘ntpd’ so that it conflicts with the other
> ntpd.
>
>> +(define openntpd-service-type
>> +  (service-type (name 'openntpd)
>> +                (extensions
>> +                 (list (service-extension shepherd-root-service-type
>> +                                          openntpd-shepherd-service)
>> +                       (service-extension account-service-type
>> +                                          (const %ntp-accounts))
>
> Are you sure that it uses those accounts?
>
>> +                 "Run the @command{ntpd}, the Network Time Protocol (NTP)
>> +daemon of the @uref{http://www.ntp.org, Network Time Foundation}, as
>           ^---- remove -------------------------------------------^
>> +implemented by OpenNTPD.  The daemon will keep the system clock synchronized
>> +with that of the given servers.")))
>> +
>> +(define* (openntpd-service #:key (openntpd openntpd)
>> +                           (servers %ntp-servers)
>> +                           allow-large-adjustment?)
>
> Remove.
>
> Could you send an updated patch?
>
> Thank you!
>
> Ludo’.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [bug#29483] [PATCH] services: Add openntpd service.
  2017-12-01 10:19 ` Ludovic Courtès
  2018-01-11 21:44   ` Ludovic Courtès
@ 2018-01-19 23:52   ` Ludovic Courtès
       [not found]     ` <20180129192423.GC17751@macbook41>
  1 sibling, 1 reply; 10+ messages in thread
From: Ludovic Courtès @ 2018-01-19 23:52 UTC (permalink / raw)
  To: Efraim Flashner; +Cc: 29483

Ping again!  :-)

ludo@gnu.org (Ludovic Courtès) skribis:

> Hello!
>
> Efraim Flashner <efraim@flashner.co.il> skribis:
>
>> * gnu/packages/ntp.scm (openntpd)[arguments]: Add 'configure-flags to
>> set openntpd daemon's user and protected path. Add a custom phase to not
>> try to create said directory at install time.
>> * gnu/services/networking.scm (<openntpd-configuration>): New record type.
>> (openntpd-shepherd-service, openntpd-service-activation): New procedures.
>> (openntpd-service-type): New variable.
>> * doc/guix.texi (Networking Services): Add openntpd documentation.
>
> Nice!
>
>> +@cindex Openntpd
>
> “OpenNTPD” maybe?  Or all lower case?
>
>> +@deffn {Scheme Procedure} openntpd-service [#:openntpd @var{openntpd}] @
>> +  [#:servers @var{%ntp-servers}] @
>> +  [#:allow-large-adjustment? #f]
>> +Return a service that runs the daemon from @var{openntpd}, the
>> +@uref{http://www.openntpd.org, OpenNTPD package}.  The daemon will
>> +keep the system clock synchronized with that of @var{servers}.
>> +@var{allow-large-adjustment?} determines whether @command{ntpd} is allowed to
>> +make an initial adjustment of more than 180 seconds."
>> +@end deffn
>
> The convention now is to expose and document the configuration record
> type and the service type, and to not provide a “foo-service” procedure.
>
> Could you adjust accordingly?
>
>> +(define-record-type* <openntpd-configuration>
>> +  openntpd-configuration make-openntpd-configuration
>> +  openntpd-configuration?
>> +  (openntpd                openntpd-configuration-openntpd
>> +                           (default openntpd))
>> +  (servers                 openntpd-configuration-servers)
>
> Probably with: (default %ntp-servers).
>
>> +# Only listen on localhost
>> +listen on 127.0.0.1
>> +listen on ::1
>> +
>> +# Query the 'Date' from trusted HTTPS servers via TLS.
>> +constraint from www.gnu.org\n"))
>
> It would be nice to make that constraint server configurable too (not a
> blocker though).
>
>> +       (list (shepherd-service
>> +              (provision '(openntpd))
>
> Perhaps we should make that ‘ntpd’ so that it conflicts with the other
> ntpd.
>
>> +(define openntpd-service-type
>> +  (service-type (name 'openntpd)
>> +                (extensions
>> +                 (list (service-extension shepherd-root-service-type
>> +                                          openntpd-shepherd-service)
>> +                       (service-extension account-service-type
>> +                                          (const %ntp-accounts))
>
> Are you sure that it uses those accounts?
>
>> +                 "Run the @command{ntpd}, the Network Time Protocol (NTP)
>> +daemon of the @uref{http://www.ntp.org, Network Time Foundation}, as
>           ^---- remove -------------------------------------------^
>> +implemented by OpenNTPD.  The daemon will keep the system clock synchronized
>> +with that of the given servers.")))
>> +
>> +(define* (openntpd-service #:key (openntpd openntpd)
>> +                           (servers %ntp-servers)
>> +                           allow-large-adjustment?)
>
> Remove.
>
> Could you send an updated patch?
>
> Thank you!
>
> Ludo’.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [bug#29483] [PATCH] services: Add openntpd service.
       [not found]       ` <871shzbgoz.fsf@gnu.org>
@ 2018-03-02 14:02         ` Efraim Flashner
  2018-03-02 16:46           ` Ludovic Courtès
  0 siblings, 1 reply; 10+ messages in thread
From: Efraim Flashner @ 2018-03-02 14:02 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: 29483


[-- Attachment #1.1: Type: text/plain, Size: 2417 bytes --]

On Mon, Feb 05, 2018 at 04:26:52PM +0100, Ludovic Courtès wrote:
> Heya Efraim,
> 
> > +(define openntpd-shepherd-service
> > +  (match-lambda
> > +    (($ <openntpd-configuration> openntpd openntpd-listen-on
> > +        openntpd-query-from openntpd-sensor openntpd-server
> > +        openntpd-servers openntpd-constraint-from
> > +        openntpd-constraints-from allow-large-adjustment?)
> 
> This is error prone (you could be matching the wrong fields), could you
> change that to ‘match-record’?
> 

I think this is the only thing left over. I compared my fields to
murmur, and for murmur we're looking at true/false or a single value.
Other than 'openntpd' and 'allow-large-adjustment?' each are lists
because they can all be lists, and I didn't want to make the logic phase
of generating the config file to be immensely long.

currently:

(match-lambda
  (($ <openntpd-configuration> openntpd openntpd-listen-on
      openntpd-query-from openntpd-sensor openntpd-server
      openntpd-servers openntpd-constraint-from
      openntpd-constraints-from allow-large-adjustment?)
   (let ()
     (define config
       (string-join
         (filter-map (lambda (field value)
                (string-join
                  (map (cut string-append field <> "\n")
                       value)))
              '("listen on " "query from " "sensor " "server " "servers "
                "constraint from ")
              (list openntpd-listen-on openntpd-query-from openntpd-sensor
                    openntpd-server openntpd-servers openntpd-constraint-from))
         ;; The 'constraints from' field needs to be enclosed in double quotes.
         (string-join
           (map (cut string-append "constraints from \"" <> "\"\n")
                openntpd-constraints-from))))

Other wise I suppose I'd be looking more at [the following] for most fields:

(match-record
...
  (if (not (null-list? openntpd-listen-on))
    (lambda (value)
      (string-append "listen on " value "\n")
     value)
     '())
...

currently to use the defaults I have
  (service openntpd-service-type (openntpd-configuration))
which obviously isn't ideal.
    

-- 
Efraim Flashner   <efraim@flashner.co.il>   אפרים פלשנר
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

[-- Attachment #1.2: 0001-services-Add-openntpd-service.patch --]
[-- Type: text/plain, Size: 11277 bytes --]

From 7db57baceadf509407068cd969d24f107ee8a027 Mon Sep 17 00:00:00 2001
From: Efraim Flashner <efraim@flashner.co.il>
Date: Tue, 28 Nov 2017 10:19:11 +0200
Subject: [PATCH] services: Add openntpd service.

* gnu/packages/ntp.scm (openntpd)[arguments]: Add 'configure-flags to
set openntpd daemon's user and localstatedir. Add a custom phase to not
try to create said directory at install time.
* gnu/services/networking.scm (<openntpd-configuration>): New record type.
(openntpd-shepherd-service, openntpd-service-activation): New procedures.
(openntpd-service-type): New variable.
* doc/guix.texi (Networking Services): Add openntpd documentation.
---
 doc/guix.texi               | 55 ++++++++++++++++++++++++-
 gnu/packages/ntp.scm        | 13 +++++-
 gnu/services/networking.scm | 98 ++++++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 163 insertions(+), 3 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index f9d7e13e2..100b23215 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -24,7 +24,7 @@ Copyright @copyright{} 2015, 2016, 2017 Leo Famulari@*
 Copyright @copyright{} 2015, 2016, 2017, 2018 Ricardo Wurmus@*
 Copyright @copyright{} 2016 Ben Woodcroft@*
 Copyright @copyright{} 2016, 2017 Chris Marusich@*
-Copyright @copyright{} 2016, 2017 Efraim Flashner@*
+Copyright @copyright{} 2016, 2017, 2018 Efraim Flashner@*
 Copyright @copyright{} 2016 John Darrington@*
 Copyright @copyright{} 2016, 2017 ng0@*
 Copyright @copyright{} 2016, 2017 Jan Nieuwenhuizen@*
@@ -10718,6 +10718,59 @@ make an initial adjustment of more than 1,000 seconds.
 List of host names used as the default NTP servers.
 @end defvr
 
+@cindex OpenNTPD
+@deffn {Scheme Procedure} openntpd-service-type
+Run the @command{ntpd}, the Network Time Protocol (NTP) daemon, as implemented
+by @uref{http://www.openntpd.org, OpenNTPD}.  The daemon will keep the system
+clock synchronized with that of the given servers.
+
+@example
+(service
+ openntpd-service-type
+ (openntpd-configuration
+  (listen-on '("127.0.0.1" "::1"))
+  (sensor '("udcf0 correction 70000"))
+  (constraint-from '("www.gnu.org"))
+  (constraints-from '("https://www.google.com/"))
+  (allow-large-adjustment? #t)))
+
+@end example
+@end deffn
+
+@deftp {Data Type} openntpd-configuration
+@table @asis
+@item @code{openntpd} (default: @code{(file-append openntpd "/sbin/ntpd")})
+The openntpd executable to use.
+@item @code{listen-on} (default: @code{'("127.0.0.1" "::1")})
+A list of local IP addresses or hostnames the ntpd daemon should listen on.
+@item @code{query-from} (default: @code{'()})
+A list of local IP address the ntpd daemon should use for outgoing queries.
+@item @code{sensor} (default: @code{'()})
+Specify a list of timedelta sensor devices ntpd should use.  @code{ntpd}
+will listen to each sensor that acutally exists and ignore non-existant ones.
+See @uref{https://man.openbsd.org/ntpd.conf, upstream documentation} for more
+information.
+@item @code{server} (default: @var{%ntp-servers})
+Specify a list of IP addresses or hostnames of NTP servers to synchronize to.
+@item @code{servers} (default: @code{'()})
+Specify a list of IP addresses or hostnames of NTP pools to synchronize to.
+@item @code{constraint-from} (default: @code{'()})
+@code{ntpd} can be configured to query the ‘Date’ from trusted HTTPS servers via TLS.
+This time information is not used for precision but acts as an authenticated
+constraint, thereby reducing the impact of unauthenticated NTP
+man-in-the-middle attacks.
+Specify a list of URLs, IP addresses or hostnames of HTTPS servers to provide
+a constraint.
+@item @code{constraints-from} (default: @code{'()})
+As with constraint from, specify a list of URLs, IP addresses or hostnames of
+HTTPS servers to provide a constraint.  Should the hostname resolve to multiple
+IP addresses, @code{ntpd} will calculate a median constraint from all of them.
+@item @code{allow-large-adjustment?} (default: @code{#f})
+Determines if @code{ntpd} is allowed to make an initial adjustment of more
+than 180 seconds.
+@end table
+@end deftp
+
 @cindex inetd
 @deffn {Scheme variable} inetd-service-type
 This service runs the @command{inetd} (@pxref{inetd invocation,,,
diff --git a/gnu/packages/ntp.scm b/gnu/packages/ntp.scm
index d270f513d..1c3b8cd31 100644
--- a/gnu/packages/ntp.scm
+++ b/gnu/packages/ntp.scm
@@ -3,7 +3,7 @@
 ;;; Copyright © 2014, 2015 Mark H Weaver <mhw@netris.org>
 ;;; Copyright © 2015 Taylan Ulrich Bayırlı/Kammer <taylanbayirli@gmail.com>
 ;;; Copyright © 2015 Ludovic Courtès <ludo@gnu.org>
-;;; Copyright © 2016, 2017 Efraim Flashner <efraim@flashner.co.il>
+;;; Copyright © 2016, 2017, 2018 Efraim Flashner <efraim@flashner.co.il>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -107,6 +107,17 @@ computers over a network.")
                (base32
                 "0fn12i4kzsi0zkr4qp3dp9bycmirnfapajqvdfx02zhr4hanj0kv"))))
     (build-system gnu-build-system)
+    (arguments
+     '(#:configure-flags '("--with-privsep-user=ntpd"
+                           "--localstatedir=/var")
+       #:phases
+       (modify-phases %standard-phases
+         (add-after 'unpack 'modify-install-locations
+           (lambda _
+             ;; Don't try to create /var/run or /var/db
+             (substitute* "src/Makefile.in"
+               (("DESTDIR\\)\\$\\(localstatedir") "TMPDIR"))
+             #t)))))
     (inputs
      `(("libressl" ,libressl))) ; enable TLS time constraints. See ntpd.conf(5).
     (home-page "http://www.openntpd.org/")
diff --git a/gnu/services/networking.scm b/gnu/services/networking.scm
index 5ba3c5eed..c59ff85bc 100644
--- a/gnu/services/networking.scm
+++ b/gnu/services/networking.scm
@@ -1,7 +1,7 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2013, 2014, 2015, 2016, 2017 Ludovic Courtès <ludo@gnu.org>
 ;;; Copyright © 2015 Mark H Weaver <mhw@netris.org>
-;;; Copyright © 2016 Efraim Flashner <efraim@flashner.co.il>
+;;; Copyright © 2016, 2018 Efraim Flashner <efraim@flashner.co.il>
 ;;; Copyright © 2016 John Darrington <jmd@gnu.org>
 ;;; Copyright © 2017 Clément Lassieur <clement@lassieur.org>
 ;;; Copyright © 2017 Thomas Danckaert <post@thomasdanckaert.be>
@@ -64,6 +64,10 @@
             ntp-service
             ntp-service-type
 
+            openntpd-configuration
+            openntpd-configuration?
+            openntpd-service-type
+
             inetd-configuration
             inetd-entry
             inetd-service-type
@@ -446,6 +450,98 @@ make an initial adjustment of more than 1,000 seconds."
                               (allow-large-adjustment?
                                allow-large-adjustment?))))
 
+(define-record-type* <openntpd-configuration>
+  openntpd-configuration make-openntpd-configuration
+  openntpd-configuration?
+  (openntpd                openntpd-configuration-openntpd
+                           (default openntpd))
+  (listen-on               openntpd-listen-on
+                           (default '("127.0.0.1"
+                                      "::1")))
+  (query-from              openntpd-query-from
+                           (default '()))
+  (sensor                  openntpd-sensor
+                           (default '()))
+  (server                  openntpd-server
+                           (default %ntp-servers))
+  (servers                 openntpd-servers
+                           (default '()))
+  (constraint-from         openntpd-constraint-from
+                           (default '()))
+  (constraints-from        openntpd-constriants-from
+                           (default '()))
+  (allow-large-adjustment? openntpd-allow-large-adjustment?
+                           (default #f))) ; upstream default
+
+(define openntpd-shepherd-service
+  (match-lambda
+    (($ <openntpd-configuration> openntpd openntpd-listen-on
+        openntpd-query-from openntpd-sensor openntpd-server
+        openntpd-servers openntpd-constraint-from
+        openntpd-constraints-from allow-large-adjustment?)
+     (let ()
+       (define config
+         (string-join
+           (filter-map (lambda (field value)
+                  (string-join
+                    (map (cut string-append field <> "\n")
+                         value)))
+                '("listen on " "query from " "sensor " "server " "servers "
+                  "constraint from ")
+                (list openntpd-listen-on openntpd-query-from openntpd-sensor
+                      openntpd-server openntpd-servers openntpd-constraint-from))
+           ;; The 'constraints from' field needs to be enclosed in double quotes.
+           (string-join
+             (map (cut string-append "constraints from \"" <> "\"\n")
+                  openntpd-constraints-from))))
+
+       (define ntpd.conf
+         (plain-file "ntpd.conf" config))
+
+       (list (shepherd-service
+              (provision '(ntpd))
+              (documentation "Run the Network Time Protocol (NTP) daemon.")
+              (requirement '(user-processes networking))
+              (start #~(make-forkexec-constructor
+                        (list (string-append #$openntpd "/sbin/ntpd")
+                              "-f" #$ntpd.conf
+                              "-d" ;; don't daemonize
+                              #$@(if allow-large-adjustment?
+                                     '("-s")
+                                     '()))
+                        ;; When ntpd is daemonized it repeatedly tries to respawn
+                        ;; while running, leading shepherd to disable it.  To
+                        ;; prevent spamming stderr, redirect output to logfile.
+                        #:log-file "/var/log/ntpd"))
+              (stop #~(make-kill-destructor))))))))
+
+(define (openntpd-service-activation config)
+  "Return the activation gexp for CONFIG."
+  (with-imported-modules '((guix build utils))
+    #~(begin
+        (use-modules (guix build utils))
+
+        (mkdir-p "/var/db")
+        (mkdir-p "/var/run")
+        (unless (file-exists? "/var/db/ntpd.drift")
+          (with-output-to-file "/var/db/ntpd.drift"
+                               (lambda _
+                                 (format #t "0.0")))))))
+
+(define openntpd-service-type
+  (service-type (name 'openntpd)
+                (extensions
+                 (list (service-extension shepherd-root-service-type
+                                          openntpd-shepherd-service)
+                       (service-extension account-service-type
+                                          (const %ntp-accounts))
+                       (service-extension activation-service-type
+                                          openntpd-service-activation)))
+                (description
+                 "Run the @command{ntpd}, the Network Time Protocol (NTP)
+daemon, as implemented by @uref{http://www.openntpd.org, OpenNTPD}.  The
+daemon will keep the system clock synchronized with that of the given servers.")))
+
 \f
 ;;;
 ;;; Inetd.
-- 
2.16.2


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [bug#29483] [PATCH] services: Add openntpd service.
  2018-03-02 14:02         ` Efraim Flashner
@ 2018-03-02 16:46           ` Ludovic Courtès
  2018-03-04 18:02             ` Efraim Flashner
  0 siblings, 1 reply; 10+ messages in thread
From: Ludovic Courtès @ 2018-03-02 16:46 UTC (permalink / raw)
  To: Efraim Flashner; +Cc: 29483

Hello,

Efraim Flashner <efraim@flashner.co.il> skribis:

> On Mon, Feb 05, 2018 at 04:26:52PM +0100, Ludovic Courtès wrote:
>> Heya Efraim,
>> 
>> > +(define openntpd-shepherd-service
>> > +  (match-lambda
>> > +    (($ <openntpd-configuration> openntpd openntpd-listen-on
>> > +        openntpd-query-from openntpd-sensor openntpd-server
>> > +        openntpd-servers openntpd-constraint-from
>> > +        openntpd-constraints-from allow-large-adjustment?)
>> 
>> This is error prone (you could be matching the wrong fields), could you
>> change that to ‘match-record’?
>> 
>
> I think this is the only thing left over.

To be clear, the switch from ‘match-lambda’ to ‘match-record’ should be
entirely mechanical.  The above snippet would become:

  (define (openntpd-shepherd-service config)
    (match-record config <openntpd-configuration>
      (openntpd openntpd-listen-on
       openntpd-query-from openntpd-sensor openntpd-server
       openntpd-servers openntpd-constraint-from
       openntpd-constraints-from allow-large-adjustment?)
      …))

That’s all I was suggesting.  The body of that procedure can remain
unchanged.

Does that make sense?

Thanks,
Ludo’.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [bug#29483] [PATCH] services: Add openntpd service.
  2018-03-02 16:46           ` Ludovic Courtès
@ 2018-03-04 18:02             ` Efraim Flashner
  2018-03-04 22:21               ` Ludovic Courtès
  0 siblings, 1 reply; 10+ messages in thread
From: Efraim Flashner @ 2018-03-04 18:02 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: 29483


[-- Attachment #1.1: Type: text/plain, Size: 1903 bytes --]

On Fri, Mar 02, 2018 at 05:46:30PM +0100, Ludovic Courtès wrote:
> Hello,
> 
> Efraim Flashner <efraim@flashner.co.il> skribis:
> 
> > On Mon, Feb 05, 2018 at 04:26:52PM +0100, Ludovic Courtès wrote:
> >> Heya Efraim,
> >> 
> >> > +(define openntpd-shepherd-service
> >> > +  (match-lambda
> >> > +    (($ <openntpd-configuration> openntpd openntpd-listen-on
> >> > +        openntpd-query-from openntpd-sensor openntpd-server
> >> > +        openntpd-servers openntpd-constraint-from
> >> > +        openntpd-constraints-from allow-large-adjustment?)
> >> 
> >> This is error prone (you could be matching the wrong fields), could you
> >> change that to ‘match-record’?
> >> 
> >
> > I think this is the only thing left over.
> 
> To be clear, the switch from ‘match-lambda’ to ‘match-record’ should be
> entirely mechanical.  The above snippet would become:
> 
>   (define (openntpd-shepherd-service config)
>     (match-record config <openntpd-configuration>
>       (openntpd openntpd-listen-on
>        openntpd-query-from openntpd-sensor openntpd-server
>        openntpd-servers openntpd-constraint-from
>        openntpd-constraints-from allow-large-adjustment?)
>       …))
> 
> That’s all I was suggesting.  The body of that procedure can remain
> unchanged.
> 
> Does that make sense?
> 

Yes, that does make sense. Switching helped me find that I mistakenly
used openntpd-<var> when it should've just been <var>, so I've fixed
that. I've also added a default value field, like the ones that were
recently added for the SQL services, and I tested that it worked with
(service openntpd-service-type) in my test config.

-- 
Efraim Flashner   <efraim@flashner.co.il>   אפרים פלשנר
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

[-- Attachment #1.2: 0001-services-Add-openntpd-service.patch --]
[-- Type: text/plain, Size: 11155 bytes --]

From 0c4d07cce671ad9131416f51098082286f241046 Mon Sep 17 00:00:00 2001
From: Efraim Flashner <efraim@flashner.co.il>
Date: Tue, 28 Nov 2017 10:19:11 +0200
Subject: [PATCH] services: Add openntpd service.

* gnu/packages/ntp.scm (openntpd)[arguments]: Add 'configure-flags to
set openntpd daemon's user and localstatedir. Add a custom phase to not
try to create said directory at install time.
* gnu/services/networking.scm (<openntpd-configuration>): New record type.
(openntpd-shepherd-service, openntpd-service-activation): New procedures.
(openntpd-service-type): New variable.
* doc/guix.texi (Networking Services): Add openntpd documentation.
---
 doc/guix.texi               |  55 +++++++++++++++++++++++-
 gnu/packages/ntp.scm        |  13 +++++-
 gnu/services/networking.scm | 102 +++++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 167 insertions(+), 3 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 057272df4..60703875f 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -24,7 +24,7 @@ Copyright @copyright{} 2015, 2016, 2017 Leo Famulari@*
 Copyright @copyright{} 2015, 2016, 2017, 2018 Ricardo Wurmus@*
 Copyright @copyright{} 2016 Ben Woodcroft@*
 Copyright @copyright{} 2016, 2017 Chris Marusich@*
-Copyright @copyright{} 2016, 2017 Efraim Flashner@*
+Copyright @copyright{} 2016, 2017, 2018 Efraim Flashner@*
 Copyright @copyright{} 2016 John Darrington@*
 Copyright @copyright{} 2016, 2017 ng0@*
 Copyright @copyright{} 2016, 2017 Jan Nieuwenhuizen@*
@@ -10767,6 +10767,59 @@ make an initial adjustment of more than 1,000 seconds.
 List of host names used as the default NTP servers.
 @end defvr
 
+@cindex OpenNTPD
+@deffn {Scheme Procedure} openntpd-service-type
+Run the @command{ntpd}, the Network Time Protocol (NTP) daemon, as implemented
+by @uref{http://www.openntpd.org, OpenNTPD}.  The daemon will keep the system
+clock synchronized with that of the given servers.
+
+@example
+(service
+ openntpd-service-type
+ (openntpd-configuration
+  (listen-on '("127.0.0.1" "::1"))
+  (sensor '("udcf0 correction 70000"))
+  (constraint-from '("www.gnu.org"))
+  (constraints-from '("https://www.google.com/"))
+  (allow-large-adjustment? #t)))
+
+@end example
+@end deffn
+
+@deftp {Data Type} openntpd-configuration
+@table @asis
+@item @code{openntpd} (default: @code{(file-append openntpd "/sbin/ntpd")})
+The openntpd executable to use.
+@item @code{listen-on} (default: @code{'("127.0.0.1" "::1")})
+A list of local IP addresses or hostnames the ntpd daemon should listen on.
+@item @code{query-from} (default: @code{'()})
+A list of local IP address the ntpd daemon should use for outgoing queries.
+@item @code{sensor} (default: @code{'()})
+Specify a list of timedelta sensor devices ntpd should use.  @code{ntpd}
+will listen to each sensor that acutally exists and ignore non-existant ones.
+See @uref{https://man.openbsd.org/ntpd.conf, upstream documentation} for more
+information.
+@item @code{server} (default: @var{%ntp-servers})
+Specify a list of IP addresses or hostnames of NTP servers to synchronize to.
+@item @code{servers} (default: @code{'()})
+Specify a list of IP addresses or hostnames of NTP pools to synchronize to.
+@item @code{constraint-from} (default: @code{'()})
+@code{ntpd} can be configured to query the ‘Date’ from trusted HTTPS servers via TLS.
+This time information is not used for precision but acts as an authenticated
+constraint, thereby reducing the impact of unauthenticated NTP
+man-in-the-middle attacks.
+Specify a list of URLs, IP addresses or hostnames of HTTPS servers to provide
+a constraint.
+@item @code{constraints-from} (default: @code{'()})
+As with constraint from, specify a list of URLs, IP addresses or hostnames of
+HTTPS servers to provide a constraint.  Should the hostname resolve to multiple
+IP addresses, @code{ntpd} will calculate a median constraint from all of them.
+@item @code{allow-large-adjustment?} (default: @code{#f})
+Determines if @code{ntpd} is allowed to make an initial adjustment of more
+than 180 seconds.
+@end table
+@end deftp
+
 @cindex inetd
 @deffn {Scheme variable} inetd-service-type
 This service runs the @command{inetd} (@pxref{inetd invocation,,,
diff --git a/gnu/packages/ntp.scm b/gnu/packages/ntp.scm
index d270f513d..1c3b8cd31 100644
--- a/gnu/packages/ntp.scm
+++ b/gnu/packages/ntp.scm
@@ -3,7 +3,7 @@
 ;;; Copyright © 2014, 2015 Mark H Weaver <mhw@netris.org>
 ;;; Copyright © 2015 Taylan Ulrich Bayırlı/Kammer <taylanbayirli@gmail.com>
 ;;; Copyright © 2015 Ludovic Courtès <ludo@gnu.org>
-;;; Copyright © 2016, 2017 Efraim Flashner <efraim@flashner.co.il>
+;;; Copyright © 2016, 2017, 2018 Efraim Flashner <efraim@flashner.co.il>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -107,6 +107,17 @@ computers over a network.")
                (base32
                 "0fn12i4kzsi0zkr4qp3dp9bycmirnfapajqvdfx02zhr4hanj0kv"))))
     (build-system gnu-build-system)
+    (arguments
+     '(#:configure-flags '("--with-privsep-user=ntpd"
+                           "--localstatedir=/var")
+       #:phases
+       (modify-phases %standard-phases
+         (add-after 'unpack 'modify-install-locations
+           (lambda _
+             ;; Don't try to create /var/run or /var/db
+             (substitute* "src/Makefile.in"
+               (("DESTDIR\\)\\$\\(localstatedir") "TMPDIR"))
+             #t)))))
     (inputs
      `(("libressl" ,libressl))) ; enable TLS time constraints. See ntpd.conf(5).
     (home-page "http://www.openntpd.org/")
diff --git a/gnu/services/networking.scm b/gnu/services/networking.scm
index 5ba3c5eed..7aadde3af 100644
--- a/gnu/services/networking.scm
+++ b/gnu/services/networking.scm
@@ -1,7 +1,7 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2013, 2014, 2015, 2016, 2017 Ludovic Courtès <ludo@gnu.org>
 ;;; Copyright © 2015 Mark H Weaver <mhw@netris.org>
-;;; Copyright © 2016 Efraim Flashner <efraim@flashner.co.il>
+;;; Copyright © 2016, 2018 Efraim Flashner <efraim@flashner.co.il>
 ;;; Copyright © 2016 John Darrington <jmd@gnu.org>
 ;;; Copyright © 2017 Clément Lassieur <clement@lassieur.org>
 ;;; Copyright © 2017 Thomas Danckaert <post@thomasdanckaert.be>
@@ -64,6 +64,10 @@
             ntp-service
             ntp-service-type
 
+            openntpd-configuration
+            openntpd-configuration?
+            openntpd-service-type
+
             inetd-configuration
             inetd-entry
             inetd-service-type
@@ -447,6 +451,102 @@ make an initial adjustment of more than 1,000 seconds."
                                allow-large-adjustment?))))
 
 \f
+;;;
+;;; OpenNTPD.
+;;;
+
+(define-record-type* <openntpd-configuration>
+  openntpd-configuration make-openntpd-configuration
+  openntpd-configuration?
+  (openntpd                openntpd-configuration-openntpd
+                           (default openntpd))
+  (listen-on               openntpd-listen-on
+                           (default '("127.0.0.1"
+                                      "::1")))
+  (query-from              openntpd-query-from
+                           (default '()))
+  (sensor                  openntpd-sensor
+                           (default '()))
+  (server                  openntpd-server
+                           (default %ntp-servers))
+  (servers                 openntpd-servers
+                           (default '()))
+  (constraint-from         openntpd-constraint-from
+                           (default '()))
+  (constraints-from        openntpd-constriants-from
+                           (default '()))
+  (allow-large-adjustment? openntpd-allow-large-adjustment?
+                           (default #f))) ; upstream default
+
+(define (openntpd-shepherd-service config)
+  (match-record config <openntpd-configuration>
+    (openntpd listen-on query-from sensor server servers constraint-from
+              constraints-from allow-large-adjustment?)
+    (let ()
+      (define config
+        (string-join
+          (filter-map
+            (lambda (field value)
+              (string-join
+                (map (cut string-append field <> "\n")
+                     value)))
+            '("listen on " "query from " "sensor " "server " "servers "
+              "constraint from ")
+            (list listen-on query-from sensor server servers constraint-from))
+          ;; The 'constraints from' field needs to be enclosed in double quotes.
+          (string-join
+            (map (cut string-append "constraints from \"" <> "\"\n")
+                 constraints-from))))
+
+      (define ntpd.conf
+        (plain-file "ntpd.conf" config))
+
+      (list (shepherd-service
+              (provision '(ntpd))
+              (documentation "Run the Network Time Protocol (NTP) daemon.")
+              (requirement '(user-processes networking))
+              (start #~(make-forkexec-constructor
+                         (list (string-append #$openntpd "/sbin/ntpd")
+                               "-f" #$ntpd.conf
+                               "-d" ;; don't daemonize
+                               #$@(if allow-large-adjustment?
+                                    '("-s")
+                                    '()))
+                         ;; When ntpd is daemonized it repeatedly tries to respawn
+                         ;; while running, leading shepherd to disable it.  To
+                         ;; prevent spamming stderr, redirect output to logfile.
+                         #:log-file "/var/log/ntpd"))
+              (stop #~(make-kill-destructor)))))))
+
+(define (openntpd-service-activation config)
+  "Return the activation gexp for CONFIG."
+  (with-imported-modules '((guix build utils))
+    #~(begin
+        (use-modules (guix build utils))
+
+        (mkdir-p "/var/db")
+        (mkdir-p "/var/run")
+        (unless (file-exists? "/var/db/ntpd.drift")
+          (with-output-to-file "/var/db/ntpd.drift"
+                               (lambda _
+                                 (format #t "0.0")))))))
+
+(define openntpd-service-type
+  (service-type (name 'openntpd)
+                (extensions
+                 (list (service-extension shepherd-root-service-type
+                                          openntpd-shepherd-service)
+                       (service-extension account-service-type
+                                          (const %ntp-accounts))
+                       (service-extension activation-service-type
+                                          openntpd-service-activation)))
+                (default-value (openntpd-configuration))
+                (description
+                 "Run the @command{ntpd}, the Network Time Protocol (NTP)
+daemon, as implemented by @uref{http://www.openntpd.org, OpenNTPD}.  The
+daemon will keep the system clock synchronized with that of the given servers.")))
+
+\f
 ;;;
 ;;; Inetd.
 ;;;
-- 
2.16.2


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [bug#29483] [PATCH] services: Add openntpd service.
  2018-03-04 18:02             ` Efraim Flashner
@ 2018-03-04 22:21               ` Ludovic Courtès
  0 siblings, 0 replies; 10+ messages in thread
From: Ludovic Courtès @ 2018-03-04 22:21 UTC (permalink / raw)
  To: Efraim Flashner; +Cc: 29483

Hello,

Efraim Flashner <efraim@flashner.co.il> skribis:

> On Fri, Mar 02, 2018 at 05:46:30PM +0100, Ludovic Courtès wrote:

[...]

>> To be clear, the switch from ‘match-lambda’ to ‘match-record’ should be
>> entirely mechanical.  The above snippet would become:
>> 
>>   (define (openntpd-shepherd-service config)
>>     (match-record config <openntpd-configuration>
>>       (openntpd openntpd-listen-on
>>        openntpd-query-from openntpd-sensor openntpd-server
>>        openntpd-servers openntpd-constraint-from
>>        openntpd-constraints-from allow-large-adjustment?)
>>       …))
>> 
>> That’s all I was suggesting.  The body of that procedure can remain
>> unchanged.
>> 
>> Does that make sense?
>> 
>
> Yes, that does make sense. Switching helped me find that I mistakenly
> used openntpd-<var> when it should've just been <var>, so I've fixed
> that. I've also added a default value field, like the ones that were
> recently added for the SQL services, and I tested that it worked with
> (service openntpd-service-type) in my test config.

Awesome.

> From 0c4d07cce671ad9131416f51098082286f241046 Mon Sep 17 00:00:00 2001
> From: Efraim Flashner <efraim@flashner.co.il>
> Date: Tue, 28 Nov 2017 10:19:11 +0200
> Subject: [PATCH] services: Add openntpd service.
>
> * gnu/packages/ntp.scm (openntpd)[arguments]: Add 'configure-flags to
> set openntpd daemon's user and localstatedir. Add a custom phase to not
> try to create said directory at install time.
> * gnu/services/networking.scm (<openntpd-configuration>): New record type.
> (openntpd-shepherd-service, openntpd-service-activation): New procedures.
> (openntpd-service-type): New variable.
> * doc/guix.texi (Networking Services): Add openntpd documentation.

[...]

> +  (constraints-from        openntpd-constriants-from
                                             ^^
Typo.  :-)

Got for it!

Thank you,
Ludo’.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#29483: [PATCH] services: Add openntpd service.
  2017-11-28  9:04 [bug#29483] [PATCH] services: Add openntpd service Efraim Flashner
  2017-12-01 10:19 ` Ludovic Courtès
  2017-12-03 19:24 ` Marius Bakke
@ 2018-03-05  9:32 ` Efraim Flashner
  2 siblings, 0 replies; 10+ messages in thread
From: Efraim Flashner @ 2018-03-05  9:32 UTC (permalink / raw)
  To: 29483-done

[-- Attachment #1: Type: text/plain, Size: 219 bytes --]

done

-- 
Efraim Flashner   <efraim@flashner.co.il>   אפרים פלשנר
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2018-03-05  9:33 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-11-28  9:04 [bug#29483] [PATCH] services: Add openntpd service Efraim Flashner
2017-12-01 10:19 ` Ludovic Courtès
2018-01-11 21:44   ` Ludovic Courtès
2018-01-19 23:52   ` Ludovic Courtès
     [not found]     ` <20180129192423.GC17751@macbook41>
     [not found]       ` <871shzbgoz.fsf@gnu.org>
2018-03-02 14:02         ` Efraim Flashner
2018-03-02 16:46           ` Ludovic Courtès
2018-03-04 18:02             ` Efraim Flashner
2018-03-04 22:21               ` Ludovic Courtès
2017-12-03 19:24 ` Marius Bakke
2018-03-05  9:32 ` bug#29483: " Efraim Flashner

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).