Bug 210245 - [PATCH] Update etc/ntp.conf to eliminate failure modes and reflect current ntpd functionality
Summary: [PATCH] Update etc/ntp.conf to eliminate failure modes and reflect current nt...
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: conf (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Many People
Assignee: Ian Lepore
URL:
Keywords: easy, patch
Depends on:
Blocks:
 
Reported: 2016-06-13 03:40 UTC by Paul J Murphy
Modified: 2018-07-04 14:19 UTC (History)
2 users (show)

See Also:
paul: mfc-stable10?
paul: mfc-stable9?


Attachments
Patch for base/head/etc/ntp.conf (revision 301726) (3.59 KB, patch)
2016-06-13 03:40 UTC, Paul J Murphy
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Paul J Murphy 2016-06-13 03:40:43 UTC
Created attachment 171362 [details]
Patch for base/head/etc/ntp.conf (revision 301726)

head/etc/ntp.conf is significantly outdated in a number of ways, and can result in a system with unsynchronised time or time synchronised to a bad/false source.  This has the potential to escalate into a security failure (probably fail-safe, but with an outage) issue for Kerberos, DNSSEC, amongst other things.  The most significant issue is that it uses configuration which the official NTP documentation describes as "suboptimal but works with older versions" (http://doc.ntp.org/current-stable/discover.html#pool).

The impact of this issue is deceptively serious, as it gives a significant probability of the NTP service becoming entirely non-functional rising with uptime, and a higher probability of a single failure resulting in false time.  The NTP Pool is constantly in flux, with servers entering and leaving the pool.  With FreeBSD's current default configuration of just 3 statically configured random servers, there is a reasonable chance that all 3 randomly picked servers could vanish over extended uptime.  The current configuration will only recover from vanishing pool servers on service restart or system reboot.  Additionally, with only 3 servers configured, losing a single server is a critical issue, as ntpd can no longer properly identify a server gradually drifting away from true time (it will recognise that the 2 remaining servers do not agree, but has lost a very significant portion of its ability to reliably identify the "false ticker").

NTP 4.2.7p22 (2010/04/02) added a new "pool" configuration command to address these issues.  Using a "pool ... preempt" configuration causes ntpd to dynamically manage the pool servers without any operator intervention or service restarts.  When a server goes bad or stops responding, this new configuration causes it to automatically be removed from ntpd's peers/associations, and it will automatically try to discover new servers from the configured pools (via new DNS lookups) to maintain the set of servers between "minclock" (default 3) and "maxclock" (default 10).  This functionality has been in the NTP source for quite some time now, and seems to be both functional and stable.

This patch also removes the obsolete and unnecessary "restrict -6" syntax which ceased to be relevant quite some time ago, and has no business being anywhere in a default configuration for NTP 4.2.8.

This bug does duplicate bug #201803, but that bug failed to get any long term attention, and I don't have permission to update the impacted version to CURRENT.  I believe that there are some significant issues which can be very easily and quickly mitigated here, and that this should be addressed for 11.0 release (and seriously considered for 9.x and 10.x errata).  I am creating a new bug for this so that it is visible as a 11.0 / CURRENT issue, and not lost in the noise of old dusty bugs.

This patch improves on the patch in the previous bug in two significant ways: 1) it includes the "preempt" option to fully enable the automatic association management, and 2) it includes 3.freebsd.pool.ntp.org to provide the full breadth of the pool to the automation (it is designed to deal with "too many" and keep the best, discard the worst).  It also includes a couple of commented examples of the minimal configuration for automated pool usage.
Comment 1 Paul J Murphy 2016-06-13 16:46:28 UTC
Some additional motivation / justification for this modernisation.  The IETF NTP WG's best current practice draft discourages the use of 3 and even 4 sources.  It is defining best current practice to use the automatic "pool" configuration command (or equivalent), with more than 4 sources (4 instances of the command results in more than 4 sources).

Network Time Protocol Best Current Practices <https://tools.ietf.org/html/draft-reilly-ntp-bcp-02#section-4.1>

This might still be at the draft stage, but this particular issue is one where it is entirely safe to be ahead of the final standard / recommendation.
Comment 2 Ian Lepore freebsd_committer freebsd_triage 2018-06-23 02:21:07 UTC
I just stumbled across this; I updated ntp.conf to use the pool keyword last year, without realizing this PR was here.  I didn't realize until finding this that the -4/-6 flags had become meaningless in ntp.conf.  I've opened a phabricator review to make that part of your changes to ntp.conf

It turns out that it is also no longer necessary to use the 'preempt' keyword on pool statements. Associations mobilized via pools are automatically preemptable in exactly the same way as those mobilized via manycast.  I verified that our current ntp.conf pool statement behaves this way by adding ipfw rules to block packets from all the servers shown by ntpq -p.  As they became unreachable the associations were dropped and new ones were mobilized to keep at least the number of good peers set by 'tos minclock'.

https://reviews.freebsd.org/D15974
Comment 3 commit-hook freebsd_committer freebsd_triage 2018-06-24 03:29:32 UTC
A commit references this bug:

Author: ian
Date: Sun Jun 24 03:29:01 UTC 2018
New revision: 335595
URL: https://svnweb.freebsd.org/changeset/base/335595

Log:
  Modernize usage of "restrict" keyword in ntp.conf

  It is no longer necessary to specify a -4/-6 flag on any ntp.conf
  keyword.  The address type is inferred from the address itself as
  necessary.  "restrict default" statements always apply to both address
  families regardless of any -4/-6 flag that may be present.

  So this change just tidies up our default config by removing the redundant
  restrict -6 statement and comment, and by removing the -6 flag from the
  restrict keyword that allows access from localhost.

  This change was inspired by the patches provided in PRs 201803 and 210245,
  and included some contrib/ntp code inspection to verify that the -4/-6
  keywords are basically no-ops in all contexts now.

  PR:		201803 210245
  Differential Revision:	https://reviews.freebsd.org/D15974

Changes:
  head/etc/ntp.conf
Comment 4 commit-hook freebsd_committer freebsd_triage 2018-07-04 14:05:17 UTC
A commit references this bug:

Author: ian
Date: Wed Jul  4 14:04:23 UTC 2018
New revision: 335949
URL: https://svnweb.freebsd.org/changeset/base/335949

Log:
  MFC r335595-r335596

  r335595:
  Modernize usage of "restrict" keyword in ntp.conf

  It is no longer necessary to specify a -4/-6 flag on any ntp.conf
  keyword.  The address type is inferred from the address itself as
  necessary.  "restrict default" statements always apply to both address
  families regardless of any -4/-6 flag that may be present.

  So this change just tidies up our default config by removing the redundant
  restrict -6 statement and comment, and by removing the -6 flag from the
  restrict keyword that allows access from localhost.

  This change was inspired by the patches provided in PRs 201803 and 210245,
  and included some contrib/ntp code inspection to verify that the -4/-6
  keywords are basically no-ops in all contexts now.

  PR:		201803 210245
  Differential Revision:	https://reviews.freebsd.org/D15974

  r335596:
  Fix a comment; the ntp leaplist file is updated periodically, but not weekly
  (it's only updated when a check shows it's within 30 days of expiring).

  PR:		207138

Changes:
_U  stable/11/
  stable/11/etc/ntp.conf
Comment 5 commit-hook freebsd_committer freebsd_triage 2018-07-04 14:11:31 UTC
A commit references this bug:

Author: ian
Date: Wed Jul  4 14:10:36 UTC 2018
New revision: 335950
URL: https://svnweb.freebsd.org/changeset/base/335950

Log:
  MFC r335595-r335596

  r335595:
  Modernize usage of "restrict" keyword in ntp.conf

  It is no longer necessary to specify a -4/-6 flag on any ntp.conf
  keyword.  The address type is inferred from the address itself as
  necessary.  "restrict default" statements always apply to both address
  families regardless of any -4/-6 flag that may be present.

  So this change just tidies up our default config by removing the redundant
  restrict -6 statement and comment, and by removing the -6 flag from the
  restrict keyword that allows access from localhost.

  This change was inspired by the patches provided in PRs 201803 and 210245,
  and included some contrib/ntp code inspection to verify that the -4/-6
  keywords are basically no-ops in all contexts now.

  PR:		201803 210245
  Differential Revision:	https://reviews.freebsd.org/D15974

  r335596:
  Fix a comment; the ntp leaplist file is updated periodically, but not weekly
  (it's only updated when a check shows it's within 30 days of expiring).

  PR:		207138

Changes:
_U  stable/10/
  stable/10/etc/ntp.conf