Xymon Mailing List Archive search

Patch: ntpdate option -p deprecated

8 messages in this thread

list Roland Rosenfeld · Sun, 11 Feb 2024 15:14:25 +0100 ·
Hi!

In Debian ntpdate from ntpsec package throws a warning that "-p is no
longer supported".  So it may be a good idea to remove the "-p 1" from
NTPDATEOPTS="-u -q -p 1"                        # Standard options to ntpdate
in xymond/etcfiles/xymonserver.cfg.DIST.

The attached patch solves this issue.

Greetings
Roland
-------------- next part --------------
Description: ntpdate obsoleted the "-p" option
Author: Carsten Leonhardt <user-37849fef6fe0@xymon.invalid>
Bug-Debian: https://bugs.debian.org/1057044
Forwarded: no
Last-Update: 2023-11-28

--- a/xymond/etcfiles/xymonserver.cfg.DIST
+++ b/xymond/etcfiles/xymonserver.cfg.DIST
@@ -133,7 +133,7 @@
 FPING="@FPING@"					# Path and options for the ping program.
 FPINGOPTS="-Ae"					# Standard options to fping/xymonping
 NTPDATE="ntpdate"				# Path to the 'ntpdate' program
-NTPDATEOPTS="-u -q -p 1"			# Standard options to ntpdate
+NTPDATEOPTS="-u -q"				# Standard options to ntpdate
 TRACEROUTE="traceroute"                         # How to do traceroute on failing ping tests. Requires "trace" in hosts.cfg .
 TRACEROUTEOPTS="-n -q 2 -w 2 -m 15"		# Standard options to traceroute
 XYMONROUTERTEXT="router"			# What to call a failing intermediate network device.
list Brian Scott · Mon, 12 Feb 2024 18:32:18 +1100 ·
The -p 1 option is still desirable for standard ntpd. The default is to do 4 probes which is overkill for xymon.

I have just checked a fresh install of ntpsec from source and can't see ntpdate being installed. Found this on the ntpsec site under Security Improvements:

  * The deprecated and vulnerability-prone ntpdate program has been
    replaced with a shell wrapper around ntpdig. Its -e and -p options
    are not implemented. It is no longer documented, but can be found in
    the attic/ directory of the source distribution.

I notice that in the script the behaviour when it finds the -p option is to simply warn and ignore it:

p) echo "ntpdate: -p is no longer supported." >&2;;

So it's just a neatness thing. The script is undocumented (debian may have pinched their man page from standard ntp) and unlikely to ever be updated.

Not sure where this leaves us. I know others had similar problems when distros moved to chrony for ntp. Maybe we need a more generalised approach to different ntp implementations going forward.

Cheers,

Brian Scott
quoted from Roland Rosenfeld

On 12/2/2024 1:14 am, Roland Rosenfeld wrote:
Hi!

In Debian ntpdate from ntpsec package throws a warning that "-p is no
longer supported".  So it may be a good idea to remove the "-p 1" from
NTPDATEOPTS="-u -q -p 1"                        # Standard options to ntpdate
in xymond/etcfiles/xymonserver.cfg.DIST.

The attached patch solves this issue.

Greetings
Roland

list Roland Rosenfeld · Mon, 12 Feb 2024 10:08:09 +0100 ·
Hi Brian!
quoted from Brian Scott

On Mon, 12 Feb 2024, Brian Scott wrote:
The -p 1 option is still desirable for standard ntpd. The default is
to do 4 probes which is overkill for xymon.
You are right, but I'm not sure, whether there are still current
systems using the original ntpd.
quoted from Brian Scott
I have just checked a fresh install of ntpsec from source and can't see
ntpdate being installed. Found this on the ntpsec site under Security
Improvements:

 * The deprecated and vulnerability-prone ntpdate program has been
   replaced with a shell wrapper around ntpdig. Its -e and -p options
   are not implemented. It is no longer documented, but can be found in
   the attic/ directory of the source distribution.

I notice that in the script the behaviour when it finds the -p option is to
simply warn and ignore it:

p) echo "ntpdate: -p is no longer supported." >&2;;

So it's just a neatness thing. The script is undocumented (debian may have
pinched their man page from standard ntp) and unlikely to ever be updated.

Not sure where this leaves us. I know others had similar problems when
distros moved to chrony for ntp. Maybe we need a more generalised approach
to different ntp implementations going forward.
Maybe the best way for ntpsec users is to replace the ntpdate call
with a final ntpdig call (which saves one shell script call).
As far as I can see "ntpdate -u -q foo" with ntpsec-ntpdate wrapper
simply calls "ntpdig -t 1 foo", so why not directly use this?

The attached rewritten patch implements this.

Greetings
Roland
Attachments (1)
list Brian Scott · Tue, 13 Feb 2024 00:06:11 +1100 ·
quoted from Roland Rosenfeld
On 12/2/2024 8:08 pm, Roland Rosenfeld wrote:
Hi Brian!

On Mon, 12 Feb 2024, Brian Scott wrote:
The -p 1 option is still desirable for standard ntpd. The default is
to do 4 probes which is overkill for xymon.
You are right, but I'm not sure, whether there are still current
systems using the original ntpd.
FreeBSD includes it as part of the base system. I monitor quite a few 
FreeBSD systems.
quoted from Roland Rosenfeld
I have just checked a fresh install of ntpsec from source and can't see
ntpdate being installed. Found this on the ntpsec site under Security
Improvements:

  * The deprecated and vulnerability-prone ntpdate program has been
    replaced with a shell wrapper around ntpdig. Its -e and -p options
    are not implemented. It is no longer documented, but can be found in
    the attic/ directory of the source distribution.

I notice that in the script the behaviour when it finds the -p option is to
simply warn and ignore it:

p) echo "ntpdate: -p is no longer supported." >&2;;

So it's just a neatness thing. The script is undocumented (debian may have
pinched their man page from standard ntp) and unlikely to ever be updated.

Not sure where this leaves us. I know others had similar problems when
distros moved to chrony for ntp. Maybe we need a more generalised approach
to different ntp implementations going forward.
Maybe the best way for ntpsec users is to replace the ntpdate call
with a final ntpdig call (which saves one shell script call).
As far as I can see "ntpdate -u -q foo" with ntpsec-ntpdate wrapper
simply calls "ntpdig -t 1 foo", so why not directly use this?
Agree 100%

i think using a discouraged wrapper like this makes no sense.

However, we still need some way to deal with the cases where the default 
is still the old ntp.
The attached rewritten patch implements this.
Would you mind if I tried to generalise your patch to automatically 
detect which version to use and pick the correct options accordingly? A 
quick look at the code suggests changes to configure.server, a new 
script under build/ to detect which type of ntp is installed, and 
possibly changes to the various build/Makefile.* to set a per-OS 
default. Also I notice that the defaults when nothing is set in 
xymonserver.cfg are established in lib/environ.c but they would be quite 
messy to tinker with.
Greetings
Roland
Cheers,

Brian
list Jeremy Laidman · Tue, 13 Feb 2024 09:05:57 +1100 ·
quoted from Brian Scott
On Tue, 13 Feb 2024 at 00:07, Brian Scott <user-b09d5329b577@xymon.invalid> wrote:
i think using a discouraged wrapper like this makes no sense.

However, we still need some way to deal with the cases where the default
is still the old ntp.
The attached rewritten patch implements this.
Would you mind if I tried to generalise your patch to automatically
detect which version to use and pick the correct options accordingly? A
quick look at the code suggests changes to configure.server, a new
script under build/ to detect which type of ntp is installed, and
possibly changes to the various build/Makefile.* to set a per-OS
default. Also I notice that the defaults when nothing is set in
xymonserver.cfg are established in lib/environ.c but they would be quite
messy to tinker with.
I don't think this should be done at compile/build time. The build server
might not use the same NTP package, or even the same version of distro, or
the same distro, or even the same OS.

The xymonserver.cfg config file has NTPDATE and NTPDATEOPTS values that can
be modified by the sysadmin, to use ntpdig, chronyd, or whatever is
installed onto the Xymon server at the time. If ntpdate is deprecated,
perhaps these variables should now be adjusted to reflect a more commonly
installed NTP client.

I'm not sure if the Xymon server does anything with the results of the
ntpdate query. If it does, then we need to be mindful that the parser might
not recognise the format of ntpdig output. In which case, a wrapper might
actually be the best solution. Perhaps the Xymon source should provide a
few different wrapper scripts called ntpdate-chrony, ntpdate-ntpsec, etc,
and the sysadmin can simply adjust NTPDATE to use the appropriate wrapper
script.

J
list Japheth Cleaver · Mon, 12 Feb 2024 14:36:48 -0800 ·
quoted from Jeremy Laidman
On Mon, February 12, 2024 14:05, Jeremy Laidman wrote:
On Tue, 13 Feb 2024 at 00:07, Brian Scott <user-b09d5329b577@xymon.invalid> wrote:
i think using a discouraged wrapper like this makes no sense.

However, we still need some way to deal with the cases where the default
is still the old ntp.
The attached rewritten patch implements this.
Would you mind if I tried to generalise your patch to automatically
detect which version to use and pick the correct options accordingly? A
quick look at the code suggests changes to configure.server, a new
script under build/ to detect which type of ntp is installed, and
possibly changes to the various build/Makefile.* to set a per-OS
default. Also I notice that the defaults when nothing is set in
xymonserver.cfg are established in lib/environ.c but they would be quite
messy to tinker with.
I don't think this should be done at compile/build time. The build server
might not use the same NTP package, or even the same version of distro, or
the same distro, or even the same OS.

The xymonserver.cfg config file has NTPDATE and NTPDATEOPTS values that
can
be modified by the sysadmin, to use ntpdig, chronyd, or whatever is
installed onto the Xymon server at the time. If ntpdate is deprecated,
perhaps these variables should now be adjusted to reflect a more commonly
installed NTP client.
It probably warrants a re-think of how we're doing ntp checks overall, but
I agree that for the moment this probably should be handled at the
distribution packaging level via environment variables.
quoted from Jeremy Laidman
I'm not sure if the Xymon server does anything with the results of the
ntpdate query. If it does, then we need to be mindful that the parser
might
not recognise the format of ntpdig output. In which case, a wrapper might
actually be the best solution. Perhaps the Xymon source should provide a
few different wrapper scripts called ntpdate-chrony, ntpdate-ntpsec, etc,
and the sysadmin can simply adjust NTPDATE to use the appropriate wrapper
script.
IIRC chrony *should* work correct, but I don't know that ntpdig has been
tested here.

The general process has been hacking the cl to present the same data so
the xymond_client parser doesn't have to care, but with all the changes
with ntp client-side over the years this might be one to clean up for
sure. Especially with it already being special-cased.

-jc
list Brian Scott · Tue, 13 Feb 2024 10:57:38 +1100 ·
quoted from Jeremy Laidman
On 13/2/2024 9:05 am, Jeremy Laidman wrote:
On Tue, 13 Feb 2024 at 00:07, Brian Scott <user-b09d5329b577@xymon.invalid> wrote:


    i think using a discouraged wrapper like this makes no sense.

    However, we still need some way to deal with the cases where the
    default
    is still the old ntp.
The attached rewritten patch implements this.
    Would you mind if I tried to generalise your patch to automatically
    detect which version to use and pick the correct options
    accordingly? A
    quick look at the code suggests changes to configure.server, a new
    script under build/ to detect which type of ntp is installed, and
    possibly changes to the various build/Makefile.* to set a per-OS
    default. Also I notice that the defaults when nothing is set in
    xymonserver.cfg are established in lib/environ.c but they would be
    quite
    messy to tinker with.


I don't think this should be done at compile/build time. The build server might not use the same NTP package, or even the same version of distro, or the same distro, or even the same OS.

The xymonserver.cfg config file has NTPDATE and NTPDATEOPTS values that can be modified by the sysadmin, to use ntpdig, chronyd, or whatever is installed onto the Xymon server at the time. If ntpdate is deprecated, perhaps these variables should now be adjusted to reflect a more commonly installed NTP client.

I'm not sure if the Xymon server does anything with the results of the ntpdate query. If it does, then we need to be mindful that the parser might not recognise the format of ntpdig output. In which case, a wrapper might actually be the best solution. Perhaps the Xymon source should provide a few different wrapper scripts called ntpdate-chrony, ntpdate-ntpsec, etc, and the sysadmin can simply adjust NTPDATE to use the appropriate wrapper script.
I think Xymon only cares about the success of the command (i.e. is the ntp* server running). ntpdig and ntpdate produce quite different output (I don't have a copy of chronyd around to check). All Xymon does is run the command and displays the output; no parsing is needed. This simply answers the question "Is an ntp service running?" and shouldn't be regarded as proper test of the ntp service ("Is it providing sane answers?", "Is it synced to a reasonable stratum level?", "Is it stable?", ...). Time offset (the only thing reliably given by the various commands) is better handled (but less accurately) by the normal Xymon process reading the client data since that doesn't require the client to be an ntp server.

I think the question what a 'proper' ntp test does doesn't matter until somebody with spare time writes a more complete and cross platform test then posts it on xymonton.

Cheers,

Brian

(BTW: ntpdig supports the -p option and it seems to work well with it - the problem is the wrapper script that doesn't pass it on)
list Jeremy Laidman · Tue, 13 Feb 2024 12:23:43 +1100 ·
quoted from Brian Scott
On Tue, 13 Feb 2024 at 10:57, Brian Scott <user-b09d5329b577@xymon.invalid> wrote:
I think Xymon only cares about the success of the command (i.e. is the
ntp* server running). ntpdig and ntpdate produce quite different output (I
don't have a copy of chronyd around to check). All Xymon does is run the
command and displays the output; no parsing is needed. This simply answers
the question "Is an ntp service running?" and shouldn't be regarded as
proper test of the ntp service ("Is it providing sane answers?", "Is it
synced to a reasonable stratum level?", "Is it stable?", ...). Time offset
(the only thing reliably given by the various commands) is better handled
(but less accurately) by the normal Xymon process reading the client data
since that doesn't require the client to be an ntp server.
That's a good point. At first glance at the code, the contrary seems to be
the case. The parser specifically looks for the string "no server suitable
for synchronization" in the output of the $NTPDATE command, and it sets a
result variable based on this. However, almost the very next line of code
executed resets this result variable to zero (and hence discards the result
of the string match), and then sets the variable based on the return code
of the $NTPDATE command. So any command will work as long as the return
code depends on a good response from the NTP server.

Cheers
Jeremy