From 7366d785a62d86de5f7eb7bfab15039492eea9bf Mon Sep 17 00:00:00 2001
From: Kevin Fason <99461833+kevinfason@users.noreply.github.com>
Date: Sat, 18 Feb 2023 23:38:59 -0700
Subject: [PATCH] network time: support pool directive and maxclock; closes
 #5569

Added if then to determine if the GUI-provided server is part of the public NTP pool or not. If the hostname ends in 'pool.ntp.org', it will write the entry to ntpd.conf with 'pool' instead of 'server' for that network server. If not then it will write it as 'server'. The pool directive tells ntpd to treat it differently. a 'server'  host is only looked up at service startup whereas a 'pool' host is monitored and changed if it becomes unresponsive or is determined to be a falseticker among other things. ntpd will also pull several DNS entries for each pool entry so I have a followup change to allow configuration of this setting in the GUI, known as 'maxclock'. It sets how many servers to maintain with a default of 10.

This adds support in the GUI for the maxclock system setting. It is used to tell NTPd how many associations (time servers) to maintain. The default is 10 however an odd number is suggested by ntpd docs to make falseticker detection simpler. This change writes what is in the GUI to ntpd.conf.

With the use of the pool directive, ntpd will use more servers than what is listed on the general page. This setting allows the user to set the max number of associations (time servers) to be maintained. Ntpd will use multiple entries from each pool entry that it maintains. Default is 10 but ntpd docs say to use an odd number to make throwing out falsetickers easier. The used is calculated wierdly from the max with the pool entries. For example with a setting of 10 and using the four default X.opnsense.pool.ntp.org entries it will have 6 associations it maintains instead of the 4 listed in the GUI. I went into more detail in the issue itself.

You can use for example, only 'us.pool.ntp.org' and it will maintain 9 associations from this pool. This means the default install configuration could just be '0.opnsense.pool.ntp.org' or, if possible, setup a 'opnsense.pool.ntp.org' so perhaps some documentation changes are in order as well?

I duplicated how the orphan setting is addressed however I did not know how these settings are maintained in a configuration backup so someone smarter may need to address that if required?
---
 src/etc/inc/plugins.inc.d/ntpd.inc | 97 ++++++++++++++----------------
 src/www/services_ntpd.php          | 16 ++++-
 2 files changed, 61 insertions(+), 52 deletions(-)

diff --git a/src/etc/inc/plugins.inc.d/ntpd.inc b/src/etc/inc/plugins.inc.d/ntpd.inc
index 259e3c813..42a0ca9e7 100644
--- a/src/etc/inc/plugins.inc.d/ntpd.inc
+++ b/src/etc/inc/plugins.inc.d/ntpd.inc
@@ -39,17 +39,17 @@ function ntpd_services()
 {
     global $config;
 
-    $services = array();
+    $services = [];
 
     if (!ntpd_enabled()) {
         return $services;
     }
 
-    $pconfig = array();
+    $pconfig = [];
     $pconfig['name'] = 'ntpd';
     $pconfig['description'] = gettext('Network Time Daemon');
-    $pconfig['php']['restart'] = array('ntpd_configure_do');
-    $pconfig['php']['start'] = array('ntpd_configure_do');
+    $pconfig['php']['restart'] = ['ntpd_configure_do'];
+    $pconfig['php']['start'] = ['ntpd_configure_do'];
     /* the pidfile for ntpd is unreliable - DO NOT USE IT */
 
     if (!empty($config['ntpd']['clientmode'])) {
@@ -63,9 +63,9 @@ function ntpd_services()
 
 function ntpd_syslog()
 {
-    $logfacilities = array();
+    $logfacilities = [];
 
-    $logfacilities['ntpd'] = array('facility' => array('ntp', 'ntpd', 'ntpdate'));
+    $logfacilities['ntpd'] = ['facility' => ['ntp', 'ntpd', 'ntpdate']];
 
     return $logfacilities;
 }
@@ -74,7 +74,7 @@ function ntpd_cron()
 {
     global $config;
 
-    $jobs = array();
+    $jobs = [];
 
     if (ntpd_enabled() && !empty($config['ntpd']['clientmode'])) {
         $jobs[]['autocron'] = ['/usr/local/sbin/pluginctl -s ntpd restart', '0', '2'];
@@ -84,11 +84,11 @@ function ntpd_cron()
 }
 function ntpd_configure()
 {
-    return array(
-        'bootup' => array('ntpd_configure_do'),
-        'local' => array('ntpd_configure_do'),
-        'newwanip' => array('ntpd_configure_do'),
-    );
+    return [
+        'bootup' => ['ntpd_configure_do'],
+        'local' => ['ntpd_configure_do'],
+        'newwanip' => ['ntpd_configure_do'],
+    ];
 }
 
 function ntpd_configure_gps($serialport)
@@ -202,6 +202,16 @@ function ntpd_configure_do($verbose = false)
     }
     $ntpcfg .= "\n";
 
+    /* Add Max Clock Count */
+    $ntpcfg .= "# Max number of associations\n";
+    $ntpcfg .= 'tos maxclock ';
+    if (!empty($config['ntpd']['maxclock'])) {
+        $ntpcfg .= $config['ntpd']['maxclock'];
+    } else {
+        $ntpcfg .= '10';
+    }
+    $ntpcfg .= "\n";
+
     /* Add PPS configuration */
     if (
         !empty($config['ntpd']['pps'])
@@ -317,14 +327,19 @@ function ntpd_configure_do($verbose = false)
     }
     /* End GPS configuration */
 
-    $noselect = isset($config['ntpd']['noselect']) ? explode(' ', $config['ntpd']['noselect']) : array();
-    $prefer = isset($config['ntpd']['prefer']) ? explode(' ', $config['ntpd']['prefer']) : array();
-    $iburst = isset($config['ntpd']['iburst']) ? explode(' ', $config['ntpd']['iburst']) : array();
+    $noselect = isset($config['ntpd']['noselect']) ? explode(' ', $config['ntpd']['noselect']) : [];
+    $prefer = isset($config['ntpd']['prefer']) ? explode(' ', $config['ntpd']['prefer']) : [];
+    $iburst = isset($config['ntpd']['iburst']) ? explode(' ', $config['ntpd']['iburst']) : [];
 
     $ntpcfg .= "\n\n# Upstream Servers\n";
     /* foreach through ntp servers and write out to ntpd.conf */
     foreach (explode(' ', $config['system']['timeservers']) as $ts) {
-        $ntpcfg .= "server {$ts}";
+        /* Determine if Network Time Server is from the NTP Pool or not */
+        if (preg_match("/\.pool\.ntp\.org$/", $ts)) {
+            $ntpcfg .= "pool {$ts}";
+        } else {
+            $ntpcfg .= "server {$ts}";
+        }
         if (in_array($ts, $iburst)) {
             $ntpcfg .= ' iburst';
         }
@@ -367,65 +382,45 @@ function ntpd_configure_do($verbose = false)
     if (!empty($config['ntpd']['logsys'])) {
         $ntpcfg .= ' +sysall';
     }
-    $ntpcfg .= "\n";
-    $ntpcfg .= "driftfile {$driftfile}\n";
+
+    $ntpcfg .= "\ndriftfile {$driftfile}";
+
     /* Access restrictions */
-    $ntpcfg .= 'restrict default';
+    $ntpaccess = '';
     if (empty($config['ntpd']['kod'])) { /*note: this one works backwards */
-        $ntpcfg .= ' kod';
+        $ntpaccess .= ' kod';
     }
     if (empty($config['ntpd']['limited'])) { /*note: this one works backwards */
-        $ntpcfg .= ' limited';
+        $ntpaccess .= ' limited';
     }
     if (empty($config['ntpd']['nomodify'])) { /*note: this one works backwards */
-        $ntpcfg .= ' nomodify';
+        $ntpaccess .= ' nomodify';
     }
     if (!empty($config['ntpd']['noquery'])) {
-        $ntpcfg .= ' noquery';
+        $ntpaccess .= ' noquery';
     }
     if (empty($config['ntpd']['nopeer'])) { /*note: this one works backwards */
-        $ntpcfg .= ' nopeer';
+        $ntpaccess .= ' nopeer';
     }
     if (empty($config['ntpd']['notrap'])) { /*note: this one works backwards */
-        $ntpcfg .= ' notrap';
+        $ntpaccess .= ' notrap';
     }
     if (!empty($config['ntpd']['noserve'])) {
-        $ntpcfg .= ' noserve';
-    }
-    $ntpcfg .= "\nrestrict -6 default";
-    if (empty($config['ntpd']['kod'])) { /*note: this one works backwards */
-        $ntpcfg .= ' kod';
-    }
-    if (empty($config['ntpd']['limited'])) { /*note: this one works backwards */
-        $ntpcfg .= ' limited';
-    }
-    if (empty($config['ntpd']['nomodify'])) { /*note: this one works backwards */
-        $ntpcfg .= ' nomodify';
-    }
-    if (!empty($config['ntpd']['noquery'])) {
-        $ntpcfg .= ' noquery';
-    }
-    if (empty($config['ntpd']['nopeer'])) { /*note: this one works backwards */
-        $ntpcfg .= ' nopeer';
-    }
-    if (!empty($config['ntpd']['noserve'])) {
-        $ntpcfg .= ' noserve';
-    }
-    if (empty($config['ntpd']['notrap'])) { /*note: this one works backwards */
-        $ntpcfg .= ' notrap';
+        $ntpaccess .= ' noserve';
     }
+    $ntpcfg .= "\nrestrict default {$ntpaccess}";
+    $ntpcfg .= "\nrestrict -6 default {$ntpaccess}";
+    $ntpcfg .= "\nrestrict source {$ntpaccess}";
     $ntpcfg .= "\n";
 
     /* A leapseconds file is really only useful if this clock is stratum 1 */
-    $ntpcfg .= "\n";
     if (!empty($config['ntpd']['leapsec'])) {
         $leapsec = base64_decode($config['ntpd']['leapsec']);
         file_put_contents('/var/db/leap-seconds', $leapsec);
         $ntpcfg .= "leapfile /var/db/leap-seconds\n";
     }
 
-
-    $interfaces = array();
+    $interfaces = [];
     if (isset($config['ntpd']['interface'])) {
         $interfaces = explode(',', $config['ntpd']['interface']);
     }
diff --git a/src/www/services_ntpd.php b/src/www/services_ntpd.php
index e3e6752d0..a7594d828 100644
--- a/src/www/services_ntpd.php
+++ b/src/www/services_ntpd.php
@@ -41,11 +41,12 @@ $copy_fields = [
     'clockstats',
     'interface',
     'kod',
-    'limited',
     'leapsec',
+    'limited',
     'logpeer',
     'logsys',
     'loopstats',
+    'maxclock',
     'nomodify',
     'nopeer',
     'noquery',
@@ -89,6 +90,9 @@ if ($_SERVER['REQUEST_METHOD'] === 'GET') {
     if (!empty($pconfig['orphan']) && ($pconfig['orphan'] < 0 || $pconfig['orphan'] > 15 || !is_numeric($pconfig['orphan']))) {
         $input_errors[] = gettext("Orphan mode must be a value between 0..15");
     }
+    if (!empty($pconfig['maxclock']) && (!is_numeric($pconfig['maxclock']) || $pconfig['maxclock'] < 2 || $pconfig['maxclock'] > 99)) {
+        $input_errors[] = gettext('Maxclock value must be a value between 2..99');
+    }
     $prev_opt = !empty($a_ntpd['custom_options']) ? $a_ntpd['custom_options'] : "";
     if ($prev_opt != str_replace("\r\n", "\n", $pconfig['custom_options']) && !userIsAdmin($_SESSION['Username'])) {
         $input_errors[] = gettext('Advanced options may only be edited by system administrators due to the increased possibility of privilege escalation.');
@@ -352,6 +356,16 @@ include("head.inc");
                       </div>
                     </td>
                   </tr>
+                  <tr>
+                    <td><a id="help_for_maxclock" href="#" class="showhelp"><i class="fa fa-info-circle"></i></a> <?=gettext('Maxclock') ?></td>
+                    <td>
+                      <input name="maxclock" type="text" value="<?=$pconfig['maxclock']?>" placeholder="10" />
+                      <div class="hidden" data-for="help_for_maxclock">
+                        <?=gettext("(2-99)");?><br />
+                        <?=gettext("Specify the maximum number of servers retained by the server discovery schemes. The default is 10, which should typically be changed. This should be an odd number (to most effectively outvote falsetickers) typically two or three more than minclock (1), plus the number of pool entries. The pool entries must be added as maxclock, but not minclock, which also counts the pool entries themselves. For example, tos maxclock 11 with four pool lines would keep 7 associations."); ?>
+                      </div>
+                    </td>
+                  </tr>
                   <tr>
                     <td><i class="fa fa-info-circle text-muted"></i> <?=gettext('NTP graphs') ?></td>
                     <td>