NetworkConfiguration / dhcpcd

DHCP / IPv4LL / IPv6RA / DHCPv6 client.
https://roy.marples.name/projects/dhcpcd
BSD 2-Clause "Simplified" License
337 stars 109 forks source link

Take OPTION_NTP_SERVER into account in ntp hook #183

Closed PKizzle closed 1 year ago

PKizzle commented 1 year ago

DHCPv6 offers two options to specify time servers:

Right now dhcpcd only takes option 31 into account. However, my router only provides option 56 if requested using option dhcp6_ntp_server in dhcpcd.conf. Hence, the ntp hook shall be changed to also consider new_dhcp6_ntp_server_addr or any of the other suboptions if provided. https://github.com/NetworkConfiguration/dhcpcd/blob/1dc0ddcd24a647c560a4fb7d0f10a9fbf0ee5695/hooks/50-ntp.conf#L134

Furthermore, RFC 5908 states that OPTION_SNTP_SERVERS is deprecated and OPTION_NTP_SERVER shall be used instead.

rsmarples commented 1 year ago

@PKizzle can you test the dhcp6-ntp branch please? My local testing might not be enough here.

PKizzle commented 1 year ago

Thank you for the quick adjustments. I will have a look at it once I get the Debian build pipeline to work 😅

PKizzle commented 1 year ago

Debian's dhcpcd version is using custom hooks that differ greatly from the original ones. So in my case the IPv6 address of the ntp server is correctly written to /var/run/dhcpcd/hook-state/ntp.conf/eth0.dhcp6 but chrony is not updated with that information. I receive the following error message:

/usr/lib/dhcpcd/dhcpcd-hooks/50-ntp.conf: cannot create /etc/chrony/chrony.conf: Read-only file system 

Taking a look at Debian's custom hooks they use a custom chrony-helper script located in /usr/libexec/chrony.

Edit: The dhcpcd user which Debian uses to run all of the hooks does not have write permission for any of the NTP config files. Hence the hook fails when trying to do so.

PKizzle commented 1 year ago

I have seen that all suboptions of OPTION_NTP_SERVER are taken into account except for NTP_SUBOPTION_MC_ADDR which is also formatted as an IPv6 address. Is there a particular reason to ignore it? Also, according to RFC 5908 OPTION_NTP_SERVER can appear multiple times and contain different NTP servers. Are all NTP servers taken into account and written to /var/run/dhcpcd/hook-state/ntp.conf/eth0.dhcp6 or only the first one?

PKizzle commented 1 year ago

https://github.com/NetworkConfiguration/dhcpcd/blob/5f6f61cbe3edfc8313d7db63d6ecf1b08d7232f1/hooks/50-ntp.conf#L139 I guess this line has been added for debugging purposes but I never received any usable output from it. Isn't this variable an array as well? So the current implementation would only output the first element...

rsmarples commented 1 year ago

https://github.com/NetworkConfiguration/dhcpcd/blob/5f6f61cbe3edfc8313d7db63d6ecf1b08d7232f1/hooks/50-ntp.conf#L139 I guess this line has been added for debugging purposes but I never received any usable output from it. Isn't this variable an array as well? So the current implementation would only output the first element...

It's now removed.

rsmarples commented 1 year ago

I have seen that all suboptions of OPTION_NTP_SERVER are taken into account except for NTP_SUBOPTION_MC_ADDR which is also formatted as an IPv6 address. Is there a particular reason to ignore it? Also, according to RFC 5908 OPTION_NTP_SERVER can appear multiple times and contain different NTP servers. Are all NTP servers taken into account and written to /var/run/dhcpcd/hook-state/ntp.conf/eth0.dhcp6 or only the first one?

I can easilly take it into account, I just don't now if the NTP clients support mulitcast or not and if they need any special directive. Hence I left it out for the time being.

rsmarples commented 1 year ago

Debian's dhcpcd version is using custom hooks that differ greatly from the original ones. So in my case the IPv6 address of the ntp server is correctly written to /var/run/dhcpcd/hook-state/ntp.conf/eth0.dhcp6 but chrony is not updated with that information. I receive the following error message:

/usr/lib/dhcpcd/dhcpcd-hooks/50-ntp.conf: cannot create /etc/chrony/chrony.conf: Read-only file system 

Taking a look at Debian's custom hooks they use a custom chrony-helper script located in /usr/libexec/chrony.

Edit: The dhcpcd user which Debian uses to run all of the hooks does not have write permission for any of the NTP config files. Hence the hook fails when trying to do so.

While I do try and support what I can, it's the defaults per package. So if debian chrony differs from upstream then that's a debian problem, not mine. Also, I don't support the Debian hooks. They make zero interest to work with me so I return the same favour.

PKizzle commented 1 year ago

I can easilly take it into account, I just don't now if the NTP clients support mulitcast or not and if they need any special directive. Hence I left it out for the time being.

It seems like not all clients support it. For example chrony is unable to use any specified multicast servers. Maybe a log message can be added that the option was detected but not used for the aforementioned reason?

PKizzle commented 1 year ago

While I do try and support what I can, it's the defaults per package. So if debian chrony differs from upstream then that's a debian problem, not mine. Also, I don't support the Debian hooks. They make zero interest to work with me so I return the same favour.

That's sad to hear. Actually the Debian version of chrony added the separate chrony-helper script with the only purpose of adjusting the chrony configuration based on dhcp information. I will try to make your hooks somehow work with Debian's messed up dhcpcd version (without the helper script) and report the results.

Edit: While my outdated dhcpcd version that is currently available for stable Debian versions uses the root user for execution the newer ones create a separate dhcpcd user, which explains why chrony.conf can not be accessed. For testing your changes I will go for the easy fix and let dhcpcd again run with full root permissions.

rsmarples commented 1 year ago

Edit: While my outdated dhcpcd version that is currently available for stable Debian versions uses the root user for execution the newer ones create a separate dhcpcd user, which explains why chrony.conf can not be accessed. For testing your changes I will go for the easy fix and let dhcpcd again run with full root permissions.

The dhcpcd hook script are run by the only root process dhcpcd keeps, so can access chrony.conf just fine.

PKizzle commented 1 year ago

I finally found the time to get the latest dhcpcd version running on Debian. After several configuration changes chrony.conf is now updated correctly. The only thing I noticed that does not work as expected is that I only receive an IPv6 address of the NTP server even though also an IPv4 address is provided by the router.

rsmarples commented 1 year ago

Does dhcpcd -U report the IPv4 address?

PKizzle commented 1 year ago

I just deleted all temporary files in /var/lib/dhcpcd and after restarting dhcpcd I finally have an IPv4 and IPv6 NTP server address in /run/dhcpcd/hook-state/ntp.conf/... as well as /etc/chrony/chrony.conf. Thank you again for your support!

Btw my changes to the source code in order to make it run on Debian seem to have broken the dhcpcd -U output 😅

reason=CARRIER
interface=eth0
protocol=link
if_configured=true
ifcarrier=up
ifmetric=1002
ifwireless=0
ifflags=69699
ifmtu=1500
NTP_CONF=/etc/chrony/chrony.conf

dhcpcd_readdump1: Invalid argument

Edit: And I was also able to fix that issue. Now I can see the NTP server beeing detected correctly 😄