From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:470:142:3::10]:48431) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1i67wb-00053s-Fm for guix-patches@gnu.org; Fri, 06 Sep 2019 02:42:07 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1i67wY-000334-U0 for guix-patches@gnu.org; Fri, 06 Sep 2019 02:42:05 -0400 Received: from debbugs.gnu.org ([209.51.188.43]:55765) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1i67wY-00032h-KK for guix-patches@gnu.org; Fri, 06 Sep 2019 02:42:02 -0400 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1i67wY-0003JH-G8 for guix-patches@gnu.org; Fri, 06 Sep 2019 02:42:02 -0400 Subject: [bug#37295] [PATCHv3] services: ntp: Support different NTP server types and options. Resent-Message-ID: From: Maxim Cournoyer References: <8736hd1sfb.fsf@x200.i-did-not-set--mail-host-address--so-tickle-me> <87ef0wzz3j.fsf_-_@gmail.com> <20190905070943.GT13917@E5400> Date: Tue, 03 Sep 2019 17:04:27 +0900 In-Reply-To: <20190905070943.GT13917@E5400> (Efraim Flashner's message of "Thu, 5 Sep 2019 10:09:43 +0300") Message-ID: <87k1apx0tw.fsf_-_@gmail.com> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-patches-bounces+kyle=kyleam.com@gnu.org Sender: "Guix-patches" To: Efraim Flashner Cc: 37295@debbugs.gnu.org --=-=-= Content-Type: text/plain Hello Efraim, Efraim Flashner writes: > Can you check how this affects the openntpd service? It currently also > uses %ntp-servers I had overlooked this important detail; thanks for pointing it out! I made the simplest change possible, by introducing a %openntpd-servers variable that holds a list of addresses (strings), which is defined as (map ntp-server-address %ntp-servers). This variable is used as the new default value for the "servers" field of the openntpd-configuration record. See the attached patch for details. --=-=-= Content-Type: text/x-patch; charset=utf-8 Content-Disposition: attachment; filename=0001-services-ntp-Support-different-NTP-server-types-and-.patch Content-Transfer-Encoding: quoted-printable >From 98d246ba9c0119e6a2441c635193cc34b7218b4e Mon Sep 17 00:00:00 2001 From: Maxim Cournoyer Date: Tue, 3 Sep 2019 10:14:59 +0900 Subject: [PATCH] services: ntp: Support different NTP server types and options. * gnu/services/networking.scm (ntp-server-types): New enum. (): New record type. (ntp-server->string): New procedure. (%ntp-servers): Define in terms of records. Use the first entrypoint server as a pool instead of a list of static servers. This is m= ore resilient since a new server of the pool can be interrogated on every request. Add the 'iburst' options. (ntp-configuration-servers): Define a custom accessor that warns but honors the now deprecated server format. (): Use it. (%openntpd-servers): New variable, (): Use it, as a pool ('servers' field) instead of a regular server. * tests/networking.scm: New file. * Makefile.am (SCM_TESTS): Register it. * doc/guix.texi: Update documentation. --- Makefile.am | 1 + doc/guix.texi | 40 +++++++++++-- gnu/services/networking.scm | 108 ++++++++++++++++++++++++++++++------ tests/networking.scm | 50 +++++++++++++++++ 4 files changed, 177 insertions(+), 22 deletions(-) create mode 100644 tests/networking.scm diff --git a/Makefile.am b/Makefile.am index fa6bf8fe80..32d518acbd 100644 --- a/Makefile.am +++ b/Makefile.am @@ -399,6 +399,7 @@ SCM_TESTS =3D \ tests/modules.scm \ tests/monads.scm \ tests/nar.scm \ + tests/networking.scm \ tests/opam.scm \ tests/packages.scm \ tests/pack.scm \ diff --git a/doc/guix.texi b/doc/guix.texi index 9de0957d14..12bb1c18e2 100644 --- a/doc/guix.texi +++ b/doc/guix.texi @@ -12988,8 +12988,9 @@ This is the data type for the NTP service configura= tion. =20 @table @asis @item @code{servers} (default: @code{%ntp-servers}) -This is the list of servers (host names) with which @command{ntpd} will be -synchronized. +This is the list of servers (@code{} records) with which +@command{ntpd} will be synchronized. See the @code{ntp-server} data type +definition below. =20 @item @code{allow-large-adjustment?} (default: @code{#t}) This determines whether @command{ntpd} is allowed to make an initial @@ -13005,6 +13006,32 @@ List of host names used as the default NTP servers= . These are servers of the @uref{https://www.ntppool.org/en/, NTP Pool Project}. @end defvr =20 +@deftp {Data Type} ntp-server +The data type representing the configuration of a NTP server. + +@table @asis +@item @code{type} (default: @code{'server}) +The type of the NTP server, given as a symbol. One of @code{'pool}, +@code{'server}, @code{'peer}, @code{'broadcast} or @code{'manycastclient}. + +@item @code{address} +The address of the server, as a string. + +@item @code{options} +NTPD options to use with that specific server, given as a list of option n= ames +and/or of option names and values tuples. The following example define a s= erver +to use with the options @option{iburst} and @option{prefer}, as well as +@option{version} 3 and a @option{maxpoll} time of 16 seconds. + +@example +(ntp-server + (type 'server) + (address "some.ntp.server.org") + (options `(iburst (version 3) (maxpoll 16) prefer)))) +@end example +@end table +@end deftp + @cindex OpenNTPD @deffn {Scheme Procedure} openntpd-service-type Run the @command{ntpd}, the Network Time Protocol (NTP) daemon, as impleme= nted @@ -13024,6 +13051,11 @@ clock synchronized with that of the given servers. @end example @end deffn =20 +@defvr {Scheme Variable} %openntpd-servers +This variable is a list of the server addresses defined in +@var{%ntp-servers}. +@end defvr + @deftp {Data Type} openntpd-configuration @table @asis @item @code{openntpd} (default: @code{(file-append openntpd "/sbin/ntpd")}) @@ -13037,9 +13069,9 @@ Specify a list of timedelta sensor devices ntpd sho= uld use. @code{ntpd} will listen to each sensor that actually exists and ignore non-existent on= es. See @uref{https://man.openbsd.org/ntpd.conf, upstream documentation} for m= ore information. -@item @code{server} (default: @var{%ntp-servers}) +@item @code{server} (default: @code{'()}) Specify a list of IP addresses or hostnames of NTP servers to synchronize = to. -@item @code{servers} (default: @code{'()}) +@item @code{servers} (default: @var{%openntp-servers}) 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 =E2=80=98Date=E2=80=99 from tru= sted HTTPS servers via TLS. diff --git a/gnu/services/networking.scm b/gnu/services/networking.scm index 13a5c6c98d..c45bfcdad9 100644 --- a/gnu/services/networking.scm +++ b/gnu/services/networking.scm @@ -51,6 +51,7 @@ #:use-module (guix records) #:use-module (guix modules) #:use-module (guix deprecation) + #:use-module (rnrs enums) #:use-module (srfi srfi-1) #:use-module (srfi srfi-9) #:use-module (srfi srfi-26) @@ -72,13 +73,22 @@ dhcpd-configuration-pid-file dhcpd-configuration-interfaces =20 - %ntp-servers - ntp-configuration ntp-configuration? + ntp-configuration-ntp + ntp-configuration-servers + ntp-allow-large-adjustment? + + %ntp-servers + ntp-server + ntp-server-type + ntp-server-address + ntp-server-options + ntp-service ntp-service-type =20 + %openntpd-servers openntpd-configuration openntpd-configuration? openntpd-service-type @@ -292,31 +302,87 @@ Protocol (DHCP) client, on all the non-loopback netwo= rk interfaces." (list (service-extension shepherd-root-service-type dhcpd-shepherd-ser= vice) (service-extension activation-service-type dhcpd-activation))))) =20 -(define %ntp-servers - ;; Default set of NTP servers. These URLs are managed by the NTP Pool pr= oject. - ;; Within Guix, Leo Famulari is the administrative c= ontact - ;; for this NTP pool "zone". - '("0.guix.pool.ntp.org" - "1.guix.pool.ntp.org" - "2.guix.pool.ntp.org" - "3.guix.pool.ntp.org")) - ;;; ;;; NTP. ;;; =20 -;; TODO: Export. +(define ntp-server-types (make-enumeration + '(pool + server + peer + broadcast + manycastclient))) + +(define-record-type* + ntp-server make-ntp-server + ntp-server? + ;; The type can be one of the symbols of the NTP-SERVER-TYPE? enumeratio= n. + (type ntp-server-type + (default 'server)) + (address ntp-server-address) ; a string + ;; The list of options can contain single option names or tuples in the = form + ;; '(name value). + (options ntp-server-options + (default '()))) + +(define (ntp-server->string ntp-server) + ;; Serialize the NTP server object as a string, ready to use in the NTP + ;; configuration file. + (define (flatten lst) + (reverse + (let loop ((x lst) + (res '())) + (if (list? x) + (fold loop res x) + (cons (format #f "~s" x) res))))) + + (match ntp-server + (($ type address options) + ;; XXX: It'd be neater if fields were validated at the syntax level (= for + ;; static ones at least). Perhaps the Guix record type could support= a + ;; predicate property on a field? + (unless (enum-set-member? type ntp-server-types) + (error "Invalid NTP server type" type)) + (string-join (cons* (symbol->string type) + address + (flatten options)))))) + +(define %ntp-servers + ;; Default set of NTP servers. These URLs are managed by the NTP Pool pr= oject. + ;; Within Guix, Leo Famulari is the administrative c= ontact + ;; for this NTP pool "zone". + (list + (ntp-server + (type 'pool) + (address "0.guix.pool.ntp.org") + (options '("iburst"))))) ;as recommended in the ntpd man= ual + (define-record-type* ntp-configuration make-ntp-configuration ntp-configuration? (ntp ntp-configuration-ntp (default ntp)) - (servers ntp-configuration-servers + (servers %ntp-configuration-servers ;list of objects (default %ntp-servers)) (allow-large-adjustment? ntp-allow-large-adjustment? (default #t))) ;as recommended in the ntpd manu= al =20 +(define (ntp-configuration-servers ntp-configuration) + ;; A wrapper to support the deprecated form of this field. + (let ((ntp-servers (%ntp-configuration-servers ntp-configuration))) + (match ntp-servers + (((? string?) (? string?) ...) + (format (current-error-port) "warning: Defining NTP servers as stri= ngs is \ +deprecated. Please use records instead.\n") + (map (lambda (addr) + (ntp-server + (type 'server) + (address addr) + (options '()))) ntp-servers)) + ((($ ) ($ ) ...) + ntp-servers)))) + (define ntp-shepherd-service (match-lambda (($ ntp servers allow-large-adjustment?) @@ -324,8 +390,7 @@ Protocol (DHCP) client, on all the non-loopback network= interfaces." ;; TODO: Add authentication support. (define config (string-append "driftfile /var/run/ntpd/ntp.drift\n" - (string-join (map (cut string-append "server " <>) - servers) + (string-join (map ntp-server->string servers) "\n") " # Disable status queries as a workaround for CVE-2013-5211: @@ -335,7 +400,11 @@ restrict -6 default kod nomodify notrap nopeer noquery= limited =20 # Yet, allow use of the local 'ntpq'. restrict 127.0.0.1 -restrict -6 ::1\n")) +restrict -6 ::1 + +# This is required to use servers from a pool directive when using the 'no= peer' +# option by default, as documented in the 'ntp.conf' manual. +restrict source notrap nomodify noquery\n")) =20 (define ntpd.conf (plain-file "ntpd.conf" config)) @@ -409,6 +478,9 @@ make an initial adjustment of more than 1,000 seconds." ;;; OpenNTPD. ;;; =20 +(define %openntpd-servers + (map ntp-server-address %ntp-servers)) + (define-record-type* openntpd-configuration make-openntpd-configuration openntpd-configuration? @@ -422,9 +494,9 @@ make an initial adjustment of more than 1,000 seconds." (sensor openntpd-sensor (default '())) (server openntpd-server - (default %ntp-servers)) - (servers openntpd-servers (default '())) + (servers openntpd-servers + (default %openntpd-servers)) (constraint-from openntpd-constraint-from (default '())) (constraints-from openntpd-constraints-from diff --git a/tests/networking.scm b/tests/networking.scm new file mode 100644 index 0000000000..001d7df74d --- /dev/null +++ b/tests/networking.scm @@ -0,0 +1,50 @@ +;;; GNU Guix --- Functional package management for GNU +;;; Copyright =C2=A9 2019 Maxim Cournoyer +;;; +;;; This file is part of GNU Guix. +;;; +;;; GNU Guix is free software; you can redistribute it and/or modify it +;;; under the terms of the GNU General Public License as published by +;;; the Free Software Foundation; either version 3 of the License, or (at +;;; your option) any later version. +;;; +;;; GNU Guix is distributed in the hope that it will be useful, but +;;; WITHOUT ANY WARRANTY; without even the implied warranty of +;;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +;;; GNU General Public License for more details. +;;; +;;; You should have received a copy of the GNU General Public License +;;; along with GNU Guix. If not, see . + +(define-module (tests networking) + #:use-module (gnu services networking) + #:use-module (srfi srfi-64)) + +;;; Tests for the (gnu services networking) module. + +(define ntp-server->string (@@ (gnu services networking) ntp-server->strin= g)) + +(define %ntp-server-sample + (ntp-server + (type 'server) + (address "some.ntp.server.org") + (options `(iburst (version 3) (maxpoll 16) prefer)))) + +(test-begin "networking") + +(test-equal "ntp-server->string" + (ntp-server->string %ntp-server-sample) + "server some.ntp.server.org iburst version 3 maxpoll 16 prefer") + +(test-equal "ntp configuration servers deprecated form" + (ntp-configuration-servers + (ntp-configuration + (servers (list (ntp-server + (type 'server) + (address "example.pool.ntp.org") + (options '())))))) + (ntp-configuration-servers + (ntp-configuration + (servers (list "example.pool.ntp.org"))))) + +(test-end "networking") --=20 2.23.0 --=-=-= Content-Type: text/plain We could also overhaul the code of the openntpd-service-type so that it'd be possible to provide different server types and options like for the 'ntp-service-type', but I'm not sure it's worth it, given that OpenNTPD is more spartan than NTP (it only supports two server types ('server' vs 'servers'), already captured as fields in its configuration record; and its server directives only support one option, 'weight', which can currently be included in the server string if desired). However, I noticed that the configuration file produced by the openntpd service, while valid, is not very clean. For the documented following openntpd-service-type definition: --8<---------------cut here---------------start------------->8--- (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))) --8<---------------cut here---------------end--------------->8--- The following configuration file is generated: --8<---------------cut here---------------start------------->8--- listen on 127.0.0.1 listen on ::1 constraints from "https://www.google.com/" constraints from "https://www.google.com/" sensor udcf0 correction 70000 constraints from "https://www.google.com/" constraints from "https://www.google.com/" servers 0.guix.pool.ntp.org constraints from "https://www.google.com/" constraint from www.gnu.org --8<---------------cut here---------------end--------------->8--- This is reproducible when testing without my changes (with the difference that multiple servers were used instead of a single "pool" entry point). I've opened an issue so that this issue can be tracked separately here: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=37318. Thank you! Maxim --=-=-=--