Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[NTP] Add NTP extended configuration #15058

Merged
merged 31 commits into from
Dec 11, 2023
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
46c61d1
Add NTP YANG model
fastiuk Feb 22, 2023
e0fa6a7
Extend NTP config generation mechanism
fastiuk Feb 22, 2023
235e111
Add NTP YANG nodel tests
fastiuk Feb 22, 2023
4d271f4
Add test for NTP Jinja templates
fastiuk Feb 22, 2023
f4e8278
Add ntpdate package
fastiuk Feb 25, 2023
1171ae4
Fix 'bad' when auth disabled
fastiuk Mar 17, 2023
3f6a693
[NTP] Changed owner for ntp keys config file to root and remove read …
fastiuk Mar 22, 2023
6f46ee7
Fix NTP warnings after restarting the service
fastiuk Apr 5, 2023
ee6ee4d
Add ability to encrypt/decrypt NTP keys
fastiuk May 6, 2023
2cbcae9
Update Configuration reference
fastiuk May 13, 2023
0537a0a
Merge branch 'master' into dev-ntp-configuration
fastiuk Sep 1, 2023
80fa802
Fix NTP configuration template
fastiuk Sep 4, 2023
eac4a88
Fix YANG model description and tests
fastiuk Sep 4, 2023
f721615
Align NTP test according to fixed condition
fastiuk Sep 5, 2023
69742ff
Merge branch 'master' into dev-ntp-configuration
fastiuk Sep 6, 2023
f911928
Merge branch 'master' into dev-ntp-configuration
fastiuk Sep 8, 2023
82152b0
Merge branch 'master' into dev-ntp-configuration
fastiuk Oct 25, 2023
b693790
Merge branch 'master' into dev-ntp-configuration
fastiuk Oct 31, 2023
d9ac8e8
Merge branch 'master' into dev-ntp-configuration
fastiuk Nov 13, 2023
30b1685
Merge branch 'master' into dev-ntp-configuration
fastiuk Nov 15, 2023
63a830a
[submodule] Update submodule sonic-utilities
fastiuk Nov 13, 2023
a4df615
Merge branch 'master' into dev-ntp-configuration
fastiuk Nov 22, 2023
97d19e7
[submodule] Update submodule sonic-utilities
fastiuk Nov 13, 2023
63c3dd0
Allow eth0 to be as source ifc without defining it
fastiuk Nov 24, 2023
5eadcfa
Align NTP keys file for bookworm
fastiuk Nov 25, 2023
99e779d
Remowe the WA for collision between pool and nopeer
fastiuk Nov 25, 2023
f2edb44
Update NTP config test according to new path
fastiuk Nov 25, 2023
64b3040
Merge branch 'master' into dev-ntp-configuration
fastiuk Nov 29, 2023
5a16d6d
Remove ntpsec-ntpdate if-up.d script
fastiuk Nov 30, 2023
403d1da
Merge branch 'master' into dev-ntp-configuration
fastiuk Dec 1, 2023
e2e8d97
Update sample config with NTP config
fastiuk Dec 2, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions files/build_templates/init_cfg.json.j2
Original file line number Diff line number Diff line change
Expand Up @@ -145,5 +145,14 @@
{% endif %}
}
{% endif %}
},
"NTP": {
"global": {
"authentication": "disabled",
"dhcp": "enabled",
"server_role": "disabled",
"admin_state": "enabled",
"vrf": "default"
}
}
}
3 changes: 3 additions & 0 deletions files/build_templates/sonic_debian_extension.j2
Original file line number Diff line number Diff line change
Expand Up @@ -365,10 +365,13 @@ sudo dpkg --root=$FILESYSTEM_ROOT -i $debs_path/flashrom_*.deb
sudo cp -f $IMAGE_CONFIGS/cron.d/* $FILESYSTEM_ROOT/etc/cron.d/

# Copy NTP configuration files and templates
sudo LANG=C DEBIAN_FRONTEND=noninteractive chroot $FILESYSTEM_ROOT \
apt-get -y install ntpdate
sudo cp $IMAGE_CONFIGS/ntp/ntp-config.service $FILESYSTEM_ROOT_USR_LIB_SYSTEMD_SYSTEM
echo "ntp-config.service" | sudo tee -a $GENERATED_SERVICE_FILE
sudo cp $IMAGE_CONFIGS/ntp/ntp-config.sh $FILESYSTEM_ROOT/usr/bin/
sudo cp $IMAGE_CONFIGS/ntp/ntp.conf.j2 $FILESYSTEM_ROOT_USR_SHARE_SONIC_TEMPLATES/
sudo cp $IMAGE_CONFIGS/ntp/ntp.keys.j2 $FILESYSTEM_ROOT_USR_SHARE_SONIC_TEMPLATES/
sudo cp $IMAGE_CONFIGS/ntp/ntp-systemd-wrapper $FILESYSTEM_ROOT/usr/lib/ntp/
sudo cp $IMAGE_CONFIGS/ntp/ntp.service $FILESYSTEM_ROOT_USR_LIB_SYSTEMD_SYSTEM
echo "ntp.service" | sudo tee -a $GENERATED_SERVICE_FILE
Expand Down
100 changes: 0 additions & 100 deletions files/image_config/ntp/ntp

This file was deleted.

4 changes: 4 additions & 0 deletions files/image_config/ntp/ntp-config.sh
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@ function modify_ntp_default
}

sonic-cfggen -d -t /usr/share/sonic/templates/ntp.conf.j2 >/etc/ntp.conf
sonic-cfggen -d -t /usr/share/sonic/templates/ntp.keys.j2 >/etc/ntp.keys

chown root:ntp /etc/ntp.keys
chmod o-r /etc/ntp.keys

get_database_reboot_type
echo "Disabling NTP long jump for reboot type ${reboot_type} ..."
Expand Down
17 changes: 13 additions & 4 deletions files/image_config/ntp/ntp-systemd-wrapper
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ if [ -r /etc/default/ntp ]; then
. /etc/default/ntp
fi

if [ -e /run/ntp.conf.dhcp ]; then
dhcp=$(/usr/local/bin/sonic-cfggen -d -v 'NTP["global"]["dhcp"]' 2> /dev/null)
if [ -e /run/ntp.conf.dhcp ] && [ "$dhcp" = "enabled" ]; then
NTPD_OPTS="$NTPD_OPTS -c /run/ntp.conf.dhcp"
fi

Expand All @@ -27,21 +28,29 @@ fi

(
flock -w 180 9
ntpEnabled=$(/usr/local/bin/sonic-cfggen -d -v 'NTP["global"]["admin_state"]' 2> /dev/null)
if [ "$ntpEnabled" = "disabled" ]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can ExecCondition= in the systemd service file be used to check this condition instead? That way the service status will accurately reflect that a condition check failed.

There also shoudn't be another ntp daemon running at this point of time, so there shouldn't be anything that needs to be stopped.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm. nice idea, I guess it will work. I will try it and let you know the result here

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any update here on whether this worked?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey. I tried it with another daemon and it actually works fine. I tried to rework it here, but it won't work here in a couple of reasons:

  1. It is not a one-line solution and will basically make this implementation more complex.
  2. We are using lock here, which will be much harder to use in the service file.
  3. We need to use ExecStopPost condition to stop the daemon.

Other reasons I don't like this option:

  1. We need to modify ntp service file which is controlled by open source community and not modified by us.
  2. That ntp-systemd-wrapper script was created exactly to customize the control of ntp daemon.

So I would like to avoid of such changes

then
logger -p INFO -t "ntpd" "Stopping NTP daemon"
start-stop-daemon --stop --quiet --oknodo --pidfile $PIDFILE
exit 0
fi

# when mgmt vrf is configured, ntp starts in mgmt vrf by default unless user configures otherwise
vrfEnabled=$(/usr/local/bin/sonic-cfggen -d -v 'MGMT_VRF_CONFIG["vrf_global"]["mgmtVrfEnabled"]' 2> /dev/null)
vrfConfigured=$(/usr/local/bin/sonic-cfggen -d -v 'NTP["global"]["vrf"]' 2> /dev/null)
if [ "$vrfEnabled" = "true" ]
then
if [ "$vrfConfigured" = "default" ]
then
log_daemon_msg "Starting NTP server in default-vrf for default set as NTP vrf" "ntpd"
logger -p INFO -t "ntpd" "Starting NTP server in default-vrf for default set as NTP vrf" "ntpd"
start-stop-daemon --start --quiet --oknodo --pidfile $PIDFILE --startas $DAEMON -- -p $PIDFILE $NTPD_OPTS
else
log_daemon_msg "Starting NTP server in mgmt-vrf" "ntpd"
logger -p INFO -t "ntpd" "Starting NTP server in mgmt-vrf"
ip vrf exec mgmt start-stop-daemon --start --quiet --oknodo --pidfile $PIDFILE --startas $DAEMON -- -p $PIDFILE $NTPD_OPTS
fi
else
log_daemon_msg "Starting NTP server in default-vrf" "ntpd"
logger -p INFO -t "ntpd" "Starting NTP server in default-vrf"
start-stop-daemon --start --quiet --oknodo --pidfile $PIDFILE --startas $DAEMON -- -p $PIDFILE $NTPD_OPTS
fi
) 9>$LOCKFILE
Expand Down
134 changes: 87 additions & 47 deletions files/image_config/ntp/ntp.conf.j2
Original file line number Diff line number Diff line change
@@ -1,45 +1,93 @@
###############################################################################
# Managed by Ansible
# file: ansible/roles/acs/templates/ntp.conf.j2
# This file was AUTOMATICALLY GENERATED. DO NOT MODIFY.
# Controlled by ntp-config.service
###############################################################################

# /etc/ntp.conf, configuration for ntpd; see ntp.conf(5) for help

# To avoid ntpd from panic and exit if the drift between new time and
# current system time is large.
tinker panic 0

driftfile /var/lib/ntp/ntp.drift


# Enable this if you want statistics to be logged.
#statsdir /var/log/ntpstats/

statistics loopstats peerstats clockstats
filegen loopstats file loopstats type day enable
filegen peerstats file peerstats type day enable
filegen clockstats file clockstats type day enable

{# Getting NTP global configuration -#}
{% set global = (NTP | d({})).get('global', {}) -%}

{# Adding NTP servers. We need to know if we have some pools, to set proper
config -#}
{% set is_pools = False %}
{% for server in NTP_SERVER if NTP_SERVER[server].admin_state != 'disabled' and
NTP_SERVER[server].resolve_as and
NTP_SERVER[server].association_type -%}
{% set config = NTP_SERVER[server] -%}
{# Server options -#}
{% set soptions = '' -%}
{# Server access control options -#}
{% set aoptions = '' -%}

{# Authentication key -#}
{% if global.authentication == 'enabled' -%}
{% if config.key -%}
{% set soptions = soptions ~ ' key ' ~ config.key -%}
{% endif -%}
{% endif -%}

{# Aggressive polling -#}
{% if config.iburst -%}
{% set soptions = soptions ~ ' iburst' -%}
{% endif -%}

{# Protocol version -#}
{% if config.version -%}
{% set soptions = soptions ~ ' version ' ~ config.version -%}
{% endif -%}

{# Check if there are any pool configured. BTW it doesn't matter what was
configured as "resolve_as" for pools. If they were configured with FQDN they
must remain like that -#}
{% set config_as = config.resolve_as -%}
{% if config.association_type == 'pool' -%}
{% set is_pools = True -%}
{% set config_as = server -%}
{% else -%}
{% set aoptions = aoptions ~ ' nopeer' -%}
{% endif -%}

{{ config.association_type }} {{ config_as }}{{ soptions }}
{% if global.server_role == 'disabled' %}
restrict {{ config_as }} kod limited nomodify notrap noquery{{ aoptions }}
{% endif %}

# You do need to talk to an NTP server or two (or three).
#server ntp.your-provider.example
{% endfor -%}

# pool.ntp.org maps to about 1000 low-stratum NTP servers. Your server will
# pick a different set every time it starts up. Please consider joining the
# pool: <http://www.pool.ntp.org/join.html>
{% for ntp_server in NTP_SERVER %}
server {{ ntp_server }} iburst
{% set trusted_keys_arr = [] -%}
{% for key in NTP_KEY -%}
{% set keydata = NTP_KEY[key] -%}
{% if keydata.trusted == 'yes' -%}
{% set trusted_keys_arr = trusted_keys_arr.append(key) -%}
{% endif -%}
{% endfor %}

#listen on source interface if configured, else
#only listen on MGMT_INTERFACE, LOOPBACK_INTERFACE ip when MGMT_INTERFACE is not defined, or eth0
# if we don't have both of them (default is to listen on all ip addresses)
{% if global.authentication == 'enabled' %}
keys /etc/ntp.keys
{% if trusted_keys_arr != [] %}
trustedkey {{ trusted_keys_arr|join(' ') }}
{% endif %}
{% endif %}

{# listen on source interface if configured, else only listen on MGMT_INTERFACE,
LOOPBACK_INTERFACE ip when MGMT_INTERFACE is not defined, or eth0 if we don't
have both of them (default is to listen on all ip addresses) -#}
interface ignore wildcard

# set global variable for configured source interface name
# set global boolean to indicate if the ip of the configured source interface is configured
# if the source interface is configured but no ip on that interface, then listen on another
# interface based on existing logic
{# Set interface to listen on set global variable for configured source
interface name set global boolean to indicate if the ip of the configured source
interface is configured if the source interface is configured but no ip on that
interface, then listen on another interface based on existing logic. -#}
fastiuk marked this conversation as resolved.
Show resolved Hide resolved
{%- macro check_ip_on_interface(interface_name, table_name) %}
{%- set ns = namespace(valid_intf = 'false') %}
{%- if table_name %}
Expand All @@ -54,8 +102,8 @@ interface ignore wildcard

{% set ns = namespace(source_intf = "") %}
{% set ns = namespace(source_intf_ip = 'false') %}
{% if (NTP) and (NTP['global']['src_intf']) %}
{% set ns.source_intf = (NTP['global']['src_intf']) %}
{% if global.src_intf %}
fastiuk marked this conversation as resolved.
Show resolved Hide resolved
{% set ns.source_intf = global.src_intf %}
{% if ns.source_intf != "" %}
{% if ns.source_intf == "eth0" %}
{% set ns.source_intf_ip = 'true' %}
Expand Down Expand Up @@ -90,32 +138,24 @@ interface listen eth0
{% endif %}
interface listen 127.0.0.1

# Access control configuration; see /usr/share/doc/ntp-doc/html/accopt.html for
# details. The web page <http://support.ntp.org/bin/view/Support/AccessRestrictions>
# might also be helpful.
#
# Note that "restrict" applies to both servers and clients, so a configuration
# that might be intended to block requests from certain clients could also end
# up blocking replies from your own upstream servers.
{# Access control options -#}
{% set options = '' -%}

{# Allow additional servers mobilization from the pool. Otherwise we don't need
that -#}
{% if is_pools == False -%}
fastiuk marked this conversation as resolved.
Show resolved Hide resolved
{% set options = options ~ ' nopeer' -%}
{% endif -%}
{# Disable NTP server functionality. Should stay on when dhcp is enabled -#}
{# {% if global.server_role == 'disabled' and global.dhcp == 'disabled' -%}
{% set options = options ~ ' ignore' -%}
{% endif -%} #}

# Access control configuration
# By default, exchange time with everybody, but don't allow configuration.
restrict -4 default kod notrap nomodify nopeer noquery
restrict -6 default kod notrap nomodify nopeer noquery
restrict -4 default kod limited notrap nomodify noquery{{ options }}
restrict -6 default kod limited notrap nomodify noquery{{ options }}
fastiuk marked this conversation as resolved.
Show resolved Hide resolved

# Local users may interrogate the ntp server more closely.
restrict 127.0.0.1
restrict ::1

# Clients from this (example!) subnet have unlimited access, but only if
# cryptographically authenticated.
#restrict 192.168.123.0 mask 255.255.255.0 notrust


# If you want to provide time to your local subnet, change the next line.
# (Again, the address is an example only.)
#broadcast 192.168.123.255

# If you want to listen to time broadcasts on your local subnet, de-comment the
# next lines. Please do this only if you trust everybody on the network!
#disable auth
#broadcastclient
18 changes: 18 additions & 0 deletions files/image_config/ntp/ntp.keys.j2
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
###############################################################################
# This file was AUTOMATICALLY GENERATED. DO NOT MODIFY.
# Controlled by ntp-config.service
###############################################################################

{# We can connect only to the servers we trust. Determine those servers -#}
{% set trusted_arr = [] -%}
{% for server in NTP_SERVER if NTP_SERVER[server].trusted == 'yes' and
NTP_SERVER[server].resolve_as -%}
{% set _ = trusted_arr.append(NTP_SERVER[server].resolve_as) -%}
{% endfor -%}

{# Define authentication keys inventory -#}
{% set trusted_str = ' ' ~ trusted_arr|join(',') -%}
{% for keyid in NTP_KEY if NTP_KEY[keyid].type and NTP_KEY[keyid].value %}
{% set keyval = NTP_KEY[keyid].value | b64decode %}
{{ keyid }} {{ NTP_KEY[keyid].type }} {{ keyval }}{{trusted_str}}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please also add an example of ntp.keys where trusted_str is not empty?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I will attach it here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is the example:

###############################################################################
# This file was AUTOMATICALLY GENERATED. DO NOT MODIFY.
# Controlled by ntp-config.service
###############################################################################

1 md5 password1 1.2.3.45,216.239.35.4
2 md5 password2 1.2.3.45,216.239.35.4
3 sha1 password3 1.2.3.45,216.239.35.4

Ip addresses here are trusted servers

{% endfor -%}
Loading