NTP should always be allowed from localhost and bindaddress/binddevice can only exist once
In progress, NormalPublicBUG
Actions

Assigned To

None

Authored By

	Apachez
	Oct 28 2023, 3:01 AM

Description

This is a spinoff of task https://vyos.dev/T5691

The issue in T5691 was that firewall rules to allow for traffic to/from localhost are missing by default in VyOS and have to be added manually.

That is handled in task https://vyos.dev/T5509

This task is regarding to add localhost by default as allowed source to speak to chronyd (the current NTP daemon in VyOS).

But also that it seems according to https://manpages.debian.org/bookworm/chrony/chrony.conf.5.en.html that both bindaddress and binddevice can only be specified once.

The documentation isnt clear about what will happen if both are specified but you can only have one bindaddress and one binddevice, not multiple of each.

Generally speaking binddevice is the better option since that will also work if the IP-address of the interface is set dynamically.

Suggestion is to modify the allow and listen sections of /usr/share/vyos/templates/chrony/chrony.conf.j2 into this:

# Allowed clients configuration
# Localhost shall always be allowed
allow 127.0.0.1
{% if allow_client.address is vyos_defined %}
{%     for address in allow_client.address %}
allow {{ address }}
{%     endfor %}
{% endif %}

# NTP should only listen on configured address
{% if listen_address is vyos_defined %}
bindaddress {{ listen_address }}
{% endif %}
{% if interface is vyos_defined %}
binddevice {{ interface }}
{% endif %}

Details

Difficulty level: Unknown (require assessment)
Version: VyOS 1.5-rolling-202310240118
Why the issue appeared?: Will be filled on close
Is it a breaking change?: Unspecified (possibly destroys the router)
Issue type: Improvement (missing useful functionality)

Related Objects

Mentioned In: T5691: `show ntp` not working
Mentioned Here: T6123: Limit NTP allow-client config to internal addresses by default
T5509: Add capability to add firewall rules similar to CoPP through VyOS configuration
T5691: `show ntp` not working

Event Timeline

Apachez created this task.Oct 28 2023, 3:01 AM

Apachez mentioned this in T5691: `show ntp` not working.

Apachez claimed this task.Oct 28 2023, 3:04 AM

This task is regarding to add localhost by default as allowed source to speak to chronyd (the current NTP daemon in VyOS).

This is already the case by default with Chrony. It doesn't seem like there's anything to do here.

Turns out that the output of bindaddress will be broken unless put in a loop even if a single entry the only allowed entry.

bindaddress ['192.168.1.2']

Below fixes that (basically the original design for listen section but with some cleanups):

# Allowed clients configuration
# Localhost shall always be allowed
allow 127.0.0.1
{% if allow_client.address is vyos_defined %}
{%     for address in allow_client.address %}
allow {{ address }}
{%     endfor %}
{% endif %}

# NTP should only listen on configured address
{% if listen_address is vyos_defined %}
{%     for address in listen_address %}
bindaddress {{ address }}
{%     endfor %}
{% endif %}
{% if interface is vyos_defined %}
binddevice {{ interface }}
{% endif %}

With above the following VyOS config:

set service ntp allow-client address '192.168.0.0/16'
set service ntp interface 'eth1'
set service ntp listen-address '192.168.1.2'
set service ntp server 194.58.200.20 prefer
set service ntp vrf 'INTERNET'

Will result in following /run/chrony/chrony.conf file:

### Autogenerated by ntp.py ###

# This would step the system clock if the adjustment is larger than 0.1 seconds,
# but only in the first three clock updates.
makestep 1.0 3

# The rtcsync directive enables a mode where the system time is periodically
# copied to the RTC and chronyd does not try to track its drift. This directive
# cannot be used with the rtcfile directive. On Linux, the RTC copy is performed
# by the kernel every 11 minutes.
rtcsync

# This directive specifies the maximum amount of memory that chronyd is allowed
# to allocate for logging of client accesses and the state that chronyd as an
# NTP server needs to support the interleaved mode for its clients.
clientloglimit 1048576

driftfile /run/chrony/drift
dumpdir /run/chrony
ntsdumpdir /run/chrony
pidfile /run/chrony/chrony.pid

# Determine when will the next leap second occur and what is the current offset
leapsectz right/UTC

user _chrony

# NTP servers to reach out to
server 194.58.200.20 iburst   prefer

# Allowed clients configuration
# Localhost shall always be allowed
allow 127.0.0.1
allow 192.168.0.0/16

# NTP should only listen on configured address
bindaddress 192.168.1.2
binddevice eth1

Output of show ntp:

vyos@vyos:~$ show ntp
                             .- Number of sample points in measurement set.
                            /    .- Number of residual runs with same sign.
                           |    /    .- Length of measurement set (time).
                           |   |    /      .- Est. clock freq error (ppm).
                           |   |   |      /           .- Est. error in freq.
                           |   |   |     |           /         .- Est. offset.
                           |   |   |     |          |          |   On the -.
                           |   |   |     |          |          |   samples. \
                           |   |   |     |          |          |             |
Name/IP Address            NP  NR  Span  Frequency  Freq Skew  Offset  Std Dev
==============================================================================
ntp.netnod.se               4   4     6     +0.000     88.405     +0ns    11us

Output of sudo chronyc tracking:

vyos@vyos:~$ sudo chronyc tracking
Reference ID    : C23AC814 (194.58.200.20)
Stratum         : 2
Ref time (UTC)  : Sat Oct 28 03:23:32 2023
System time     : 0.000033702 seconds fast of NTP time
Last offset     : +0.000051510 seconds
RMS offset      : 0.000051510 seconds
Frequency       : 1.053 ppm fast
Residual freq   : +0.001 ppm
Skew            : 4.716 ppm
Root delay      : 0.001273694 seconds
Root dispersion : 0.000146958 seconds
Update interval : 64.7 seconds
Leap status     : Normal

Output from a different host connected to VyOS using nmap to verify that VyOS will serve NTP-clients:

# nmap -sU -p 123 --script ntp-info 192.168.1.2
Starting Nmap 7.94SVN ( https://nmap.org ) at 2023-10-28 05:24 CEST
Nmap scan report for 192.168.1.2
Host is up (0.00080s latency).

PORT    STATE SERVICE
123/udp open  ntp
| ntp-info: 
|_  receive time stamp: 2023-10-28T03:24:43
MAC Address: 08:00:27:CC:6B:0A (Oracle VirtualBox virtual NIC)

Nmap done: 1 IP address (1 host up) scanned in 23.47 seconds

What kind of cleanup are you talking about?

VyOS already prevents more than one listen-address statement per IPv4/IPv6.
Chrony by default allows localhost to port 323 for sending commands (from chronyc).
Allow statements are for NTP client connections, not for chronyc connections.

Original template /usr/share/vyos/templates/chrony/chrony.conf.j2 (just the allow and listen sections):

# Allowed clients configuration                                                                                                                                                    
{% if allow_client.address is vyos_defined %}                                                                                                                                      
{%     for address in allow_client.address %}                                                                                                                                      
allow {{ address }}                                                                                                                                                                
{%     endfor %}                                                                                                                                                                   
{% else %}                                   
deny all                                     
{% endif %}                                  
                                             
{% if listen_address is vyos_defined or interface is vyos_defined %}
# NTP should listen on configured addresses only                    
{%     if listen_address is vyos_defined %}                         
{%         for address in listen_address %}                         
bindaddress {{ address }}                                           
{%         endfor %}                                                
{%     endif %}                                                     
{%     if interface is vyos_defined %}                              
binddevice {{ interface }}                                          
{%     endif %}                                 
{% endif %}

Suggested changes:

# Allowed clients configuration                                                                                                                                                    
# Localhost shall always be allowed                                                                                                                                                
allow 127.0.0.1                                                                                                                                                                    
{% if allow_client.address is vyos_defined %}                                                                                                                                      
{%     for address in allow_client.address %}
allow {{ address }}                          
{%     endfor %}                             
{% endif %}                                  
                                             
# NTP should only listen on configured address
{% if listen_address is vyos_defined %}       
{%     for address in listen_address %}       
bindaddress {{ address }}                     
{%     endfor %}                              
{% endif %}                                   
{% if interface is vyos_defined %}            
binddevice {{ interface }}                    
{% endif %}

Instead of "deny all" if no allow-clients are configured then localhost is always allowed. Can be handy when using containers and other if needed to sync to localhost for whatever reason (if the use of RTC isnt enough).

Removed the redundant check for the same variables twice in the listen block. This will also be more visible in the resulting /run/chrony/chrony.conf if bindaddress or binddevice are set since the listen header will always be visible.

pasik added a subscriber: pasik.Oct 28 2023, 9:25 AM

Instead of "deny all" if no allow-clients are configured then localhost is always allowed. Can be handy when using containers and other if needed to sync to localhost for whatever reason (if the use of RTC isnt enough).

AFAIK containers are not seen as localhost, are they?

@Apachez Are you working on it?

changes_chrony_conf_j2.png (558×998 px, 45 KB)

I gave it a go due to similarities between this and https://vyos.dev/T6123.

I added several addresses to chrony.conf.j2 in vyos1-x, rebuilt the vyos1-x package and integrated into a build of current following Apachez' guide here: https://forum.vyos.io/t/how-to-properly-build-and-smoketest-vyos/12198/2

Result (see screenshot) is that the allow clients are still being limited to 0.0.0.0/0 and ::/0 and no additional addresses are being pulled in.

I've spend considerable time looking at chrony and service ntp code trying to understand how 0.0.0.0/0 and ::/0 are being assigned but I can't find it. I'm baffled.

Is the config.boot.default file linked below used to load the initial configuration? If so, how do we go from it as is to having allow clients 0.0.0.0/0 and ::/0 added? Should we edit config.boot.default to include the additional allow from's rather than relying on chrony.conf to set defaults?

https://github.com/vyos/vyos-build/blob/f32d9fa78b8ccb3d0fb48d6c811ae57e8ed93015/data/live-build-config/includes.chroot/opt/vyatta/etc/config.boot.default

Removed assignee for now in case somebody else wants to fix this?

Giggum attached a referenced file: F4264753: changes_chrony_conf_j2.png. (Show Details)Wed, Apr 10, 12:40 PM

Giggum attached a referenced file: F4264754: show_conf.png. (Show Details)

Editing config.boot.default has addressed this. Pull request opened for comments/integration here: https://github.com/vyos/vyos-build/pull/559

Will a migrationsscript be included so that users who used the default of:

ntp {
    allow-client {
        address "0.0.0.0/0"
        address "::/0"
    }

would have that adjusted into this?

ntp {
    allow-client {
        address 127.0.0.0/8
        address 169.254.0.0/16
        address 10.0.0.0/8
        address 172.16.0.0/12
        address 192.168.0.0/16
        address ::1/128
        address fe80::/10
        address fc00::/7
    }

Otherwise this fix will only be valid for new deployments while current installations (who used the default) wont be fixed and still being exposed.

	F4264754: show_conf.png
	Wed, Apr 10, 2:08 AM

NTP should always be allowed from localhost and bindaddress/binddevice can only exist onceIn progress, NormalPublicBUGActions

Description

Details

Related Objects

Event Timeline

NTP should always be allowed from localhost and bindaddress/binddevice can only exist once
In progress, NormalPublicBUG
Actions