| Message ID | 1463677071-17121-1-git-send-email-dedeckeh@gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
Hi Hans, staged into https://git.lede-project.org/?p=lede/jow/staging.git A few improvement ideas though; - maybe we can make the jsonfilter dependency conditionally depending on the ntpd applet - maybe pipe the ubus interface status dump directly to jsonfilter - is there any reason to not make "use_dhcp" the default? ~ Jo On 05/19/2016 06:57 PM, Hans Dedecker wrote: > The busybox ntpd utility currently uses ntp servers specified in uci. > This patch allows the ntpd utility to use NTP servers received via DHCP(v6) > Following uci parameters have been added: > use_dhcp : enables NTP server config via DHCP(v6) > dhcp_interface : use NTP servers received only on the specified DHCP(v6) interfaces; if empty all interfaces are considered > > Signed-off-by: Hans Dedecker <dedeckeh@gmail.com> > --- > > The patch is based on a previous discussion held on the OpenWRT-devel mailing list > (https://lists.openwrt.org/pipermail/openwrt-devel/2016-January/039081.html) as per Felix's > comments this solution is based on procd interface service triggers > > package/utils/busybox/Makefile | 2 +- > package/utils/busybox/files/sysntpd | 43 ++++++++++++++++++++++++++++++++----- > 2 files changed, 39 insertions(+), 6 deletions(-) > > diff --git a/package/utils/busybox/Makefile b/package/utils/busybox/Makefile > index 24c064c..24e0e11 100644 > --- a/package/utils/busybox/Makefile > +++ b/package/utils/busybox/Makefile > @@ -42,7 +42,7 @@ define Package/busybox > MAINTAINER:=Felix Fietkau <nbd@openwrt.org> > TITLE:=Core utilities for embedded Linux > URL:=http://busybox.net/ > - DEPENDS:=+BUSYBOX_USE_LIBRPC:librpc +BUSYBOX_CONFIG_PAM:libpam > + DEPENDS:=+BUSYBOX_USE_LIBRPC:librpc +BUSYBOX_CONFIG_PAM:libpam +jsonfilter > MENU:=1 > endef > > diff --git a/package/utils/busybox/files/sysntpd b/package/utils/busybox/files/sysntpd > index f73bb83..5c663d7 100755 > --- a/package/utils/busybox/files/sysntpd > +++ b/package/utils/busybox/files/sysntpd > @@ -7,13 +7,35 @@ USE_PROCD=1 > PROG=/usr/sbin/ntpd > HOTPLUG_SCRIPT=/usr/sbin/ntpd-hotplug > > +get_dhcp_ntp_servers() { > + local interfaces="$1" > + local filter="*" > + local network_dump interface ntpservers ntpserver > + > + network_dump=$(ubus call network.interface dump) > + for interface in $interfaces; do > + [ "$filter" = "*" ] && filter="@.interface='$interface'" || filter="$filter,@.interface='$interface'" > + done > + > + ntpservers=$(jsonfilter -s "$network_dump" -e "@.interface[$filter]['data']['ntpserver']") > + > + for ntpserver in $ntpservers; do > + local duplicate=0 > + local entry > + for entry in $server; do > + [ "$ntpserver" = "$entry" ] && duplicate=1 > + done > + [ "$duplicate" = 0 ] && server="$server $ntpserver" > + done > +} > + > validate_ntp_section() { > uci_validate_section system timeserver "${1}" \ > - 'server:list(host)' 'enabled:bool:1' 'enable_server:bool:0' > + 'server:list(host)' 'enabled:bool:1' 'enable_server:bool:0' 'use_dhcp:bool:0' 'dhcp_interface:list(string)' > } > > start_service() { > - local server enabled enable_server peer > + local server enabled enable_server use_dhcp dhcp_interface peer > > validate_ntp_section ntp || { > echo "validation failed" > @@ -22,6 +44,8 @@ start_service() { > > [ $enabled = 0 ] && return > > + [ $use_dhcp = 1 ] && get_dhcp_ntp_servers "$dhcp_interface" > + > [ -z "$server" ] && return > > procd_open_instance > @@ -35,8 +59,17 @@ start_service() { > procd_close_instance > } > > -service_triggers() > -{ > - procd_add_reload_trigger "system" > +service_triggers() { > + local script name > + > + script=$(readlink -f "$initscript") > + name=$(basename ${script:-$initscript}) > + > + procd_open_trigger > + procd_add_config_trigger "config.change" "system" /etc/init.d/$name reload > + > + procd_add_raw_trigger "interface.*" 2000 /etc/init.d/$name reload > + procd_close_trigger > + > procd_add_validation validate_ntp_section > } >
Hi Jo, On Thu, May 19, 2016 at 7:04 PM, Jo-Philipp Wich <jo@mein.io> wrote: > Hi Hans, > > staged into https://git.lede-project.org/?p=lede/jow/staging.git > > A few improvement ideas though; > > - maybe we can make the jsonfilter dependency conditionally depending > on the ntpd applet > - maybe pipe the ubus interface status dump directly to jsonfilter > - is there any reason to not make "use_dhcp" the default? I wanted to preserve the ntp server behavior and only change the behavior when configured in order to keep backwards compatibility. You favour enabling DHCP ntp server config without explicit config ? Regarding the improvements do you want me to send a patch containing the diff with the already staged commit or do you prefer a v2 patch ? Hans > > ~ Jo > > On 05/19/2016 06:57 PM, Hans Dedecker wrote: >> The busybox ntpd utility currently uses ntp servers specified in uci. >> This patch allows the ntpd utility to use NTP servers received via DHCP(v6) >> Following uci parameters have been added: >> use_dhcp : enables NTP server config via DHCP(v6) >> dhcp_interface : use NTP servers received only on the specified DHCP(v6) interfaces; if empty all interfaces are considered >> >> Signed-off-by: Hans Dedecker <dedeckeh@gmail.com> >> --- >> >> The patch is based on a previous discussion held on the OpenWRT-devel mailing list >> (https://lists.openwrt.org/pipermail/openwrt-devel/2016-January/039081.html) as per Felix's >> comments this solution is based on procd interface service triggers >> >> package/utils/busybox/Makefile | 2 +- >> package/utils/busybox/files/sysntpd | 43 ++++++++++++++++++++++++++++++++----- >> 2 files changed, 39 insertions(+), 6 deletions(-) >> >> diff --git a/package/utils/busybox/Makefile b/package/utils/busybox/Makefile >> index 24c064c..24e0e11 100644 >> --- a/package/utils/busybox/Makefile >> +++ b/package/utils/busybox/Makefile >> @@ -42,7 +42,7 @@ define Package/busybox >> MAINTAINER:=Felix Fietkau <nbd@openwrt.org> >> TITLE:=Core utilities for embedded Linux >> URL:=http://busybox.net/ >> - DEPENDS:=+BUSYBOX_USE_LIBRPC:librpc +BUSYBOX_CONFIG_PAM:libpam >> + DEPENDS:=+BUSYBOX_USE_LIBRPC:librpc +BUSYBOX_CONFIG_PAM:libpam +jsonfilter >> MENU:=1 >> endef >> >> diff --git a/package/utils/busybox/files/sysntpd b/package/utils/busybox/files/sysntpd >> index f73bb83..5c663d7 100755 >> --- a/package/utils/busybox/files/sysntpd >> +++ b/package/utils/busybox/files/sysntpd >> @@ -7,13 +7,35 @@ USE_PROCD=1 >> PROG=/usr/sbin/ntpd >> HOTPLUG_SCRIPT=/usr/sbin/ntpd-hotplug >> >> +get_dhcp_ntp_servers() { >> + local interfaces="$1" >> + local filter="*" >> + local network_dump interface ntpservers ntpserver >> + >> + network_dump=$(ubus call network.interface dump) >> + for interface in $interfaces; do >> + [ "$filter" = "*" ] && filter="@.interface='$interface'" || filter="$filter,@.interface='$interface'" >> + done >> + >> + ntpservers=$(jsonfilter -s "$network_dump" -e "@.interface[$filter]['data']['ntpserver']") >> + >> + for ntpserver in $ntpservers; do >> + local duplicate=0 >> + local entry >> + for entry in $server; do >> + [ "$ntpserver" = "$entry" ] && duplicate=1 >> + done >> + [ "$duplicate" = 0 ] && server="$server $ntpserver" >> + done >> +} >> + >> validate_ntp_section() { >> uci_validate_section system timeserver "${1}" \ >> - 'server:list(host)' 'enabled:bool:1' 'enable_server:bool:0' >> + 'server:list(host)' 'enabled:bool:1' 'enable_server:bool:0' 'use_dhcp:bool:0' 'dhcp_interface:list(string)' >> } >> >> start_service() { >> - local server enabled enable_server peer >> + local server enabled enable_server use_dhcp dhcp_interface peer >> >> validate_ntp_section ntp || { >> echo "validation failed" >> @@ -22,6 +44,8 @@ start_service() { >> >> [ $enabled = 0 ] && return >> >> + [ $use_dhcp = 1 ] && get_dhcp_ntp_servers "$dhcp_interface" >> + >> [ -z "$server" ] && return >> >> procd_open_instance >> @@ -35,8 +59,17 @@ start_service() { >> procd_close_instance >> } >> >> -service_triggers() >> -{ >> - procd_add_reload_trigger "system" >> +service_triggers() { >> + local script name >> + >> + script=$(readlink -f "$initscript") >> + name=$(basename ${script:-$initscript}) >> + >> + procd_open_trigger >> + procd_add_config_trigger "config.change" "system" /etc/init.d/$name reload >> + >> + procd_add_raw_trigger "interface.*" 2000 /etc/init.d/$name reload >> + procd_close_trigger >> + >> procd_add_validation validate_ntp_section >> } >> >
Hi Hans, > I wanted to preserve the ntp server behavior and only change the > behavior when configured in order to keep backwards compatibility. You > favour enabling DHCP ntp server config without explicit config ? Personally I do because thats likely what most users expect, but then trusting foreign NTP server advertisements might be a security sensitive topic - on the other hand one trusts the default gateway and DNS advertisements too, so I don't know. > Regarding the improvements do you want me to send a patch containing > the diff with the already staged commit or do you prefer a v2 patch ? Whatever you prefer - I think a v2 is the easiest and I can just replace the commit in my tree. ~ Jo
Hi, One feature that was requested is the ability to specify a list of interfaces to get servers from. You can save the list as an option to the config file and add a trigger to only those interfaces. Regards, Amine. On Fri, May 20, 2016 at 11:01 AM, Jo-Philipp Wich <jo@mein.io> wrote: > Hi Hans, > > > I wanted to preserve the ntp server behavior and only change the > > behavior when configured in order to keep backwards compatibility. You > > favour enabling DHCP ntp server config without explicit config ? > > Personally I do because thats likely what most users expect, but then > trusting foreign NTP server advertisements might be a security sensitive > topic - on the other hand one trusts the default gateway and DNS > advertisements too, so I don't know. > > > > Regarding the improvements do you want me to send a patch containing > > the diff with the already staged commit or do you prefer a v2 patch ? > > Whatever you prefer - I think a v2 is the easiest and I can just replace > the commit in my tree. > > ~ Jo > _______________________________________________ > openwrt-devel mailing list > openwrt-devel@lists.openwrt.org > https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel >
Hi, One feature that was requested is the ability to specify a list of interfaces to get servers from. You can save the list as an option to the config file and add a trigger to only those interfaces. Regards, Amine. On Thu, May 19, 2016 at 6:57 PM, Hans Dedecker <dedeckeh@gmail.com> wrote: > The busybox ntpd utility currently uses ntp servers specified in uci. > This patch allows the ntpd utility to use NTP servers received via DHCP(v6) > Following uci parameters have been added: > use_dhcp : enables NTP server config via DHCP(v6) > dhcp_interface : use NTP servers received only on the specified > DHCP(v6) interfaces; if empty all interfaces are considered > > Signed-off-by: Hans Dedecker <dedeckeh@gmail.com> > --- > > The patch is based on a previous discussion held on the OpenWRT-devel > mailing list > ( > https://lists.openwrt.org/pipermail/openwrt-devel/2016-January/039081.html) > as per Felix's > comments this solution is based on procd interface service triggers > > package/utils/busybox/Makefile | 2 +- > package/utils/busybox/files/sysntpd | 43 > ++++++++++++++++++++++++++++++++----- > 2 files changed, 39 insertions(+), 6 deletions(-) > > diff --git a/package/utils/busybox/Makefile > b/package/utils/busybox/Makefile > index 24c064c..24e0e11 100644 > --- a/package/utils/busybox/Makefile > +++ b/package/utils/busybox/Makefile > @@ -42,7 +42,7 @@ define Package/busybox > MAINTAINER:=Felix Fietkau <nbd@openwrt.org> > TITLE:=Core utilities for embedded Linux > URL:=http://busybox.net/ > - DEPENDS:=+BUSYBOX_USE_LIBRPC:librpc +BUSYBOX_CONFIG_PAM:libpam > + DEPENDS:=+BUSYBOX_USE_LIBRPC:librpc +BUSYBOX_CONFIG_PAM:libpam > +jsonfilter > MENU:=1 > endef > > diff --git a/package/utils/busybox/files/sysntpd > b/package/utils/busybox/files/sysntpd > index f73bb83..5c663d7 100755 > --- a/package/utils/busybox/files/sysntpd > +++ b/package/utils/busybox/files/sysntpd > @@ -7,13 +7,35 @@ USE_PROCD=1 > PROG=/usr/sbin/ntpd > HOTPLUG_SCRIPT=/usr/sbin/ntpd-hotplug > > +get_dhcp_ntp_servers() { > + local interfaces="$1" > + local filter="*" > + local network_dump interface ntpservers ntpserver > + > + network_dump=$(ubus call network.interface dump) > + for interface in $interfaces; do > + [ "$filter" = "*" ] && filter="@.interface='$interface'" > || filter="$filter,@.interface='$interface'" > + done > + > + ntpservers=$(jsonfilter -s "$network_dump" -e > "@.interface[$filter]['data']['ntpserver']") > + > + for ntpserver in $ntpservers; do > + local duplicate=0 > + local entry > + for entry in $server; do > + [ "$ntpserver" = "$entry" ] && duplicate=1 > + done > + [ "$duplicate" = 0 ] && server="$server $ntpserver" > + done > +} > + > validate_ntp_section() { > uci_validate_section system timeserver "${1}" \ > - 'server:list(host)' 'enabled:bool:1' 'enable_server:bool:0' > + 'server:list(host)' 'enabled:bool:1' > 'enable_server:bool:0' 'use_dhcp:bool:0' 'dhcp_interface:list(string)' > } > > start_service() { > - local server enabled enable_server peer > + local server enabled enable_server use_dhcp dhcp_interface peer > > validate_ntp_section ntp || { > echo "validation failed" > @@ -22,6 +44,8 @@ start_service() { > > [ $enabled = 0 ] && return > > + [ $use_dhcp = 1 ] && get_dhcp_ntp_servers "$dhcp_interface" > + > [ -z "$server" ] && return > > procd_open_instance > @@ -35,8 +59,17 @@ start_service() { > procd_close_instance > } > > -service_triggers() > -{ > - procd_add_reload_trigger "system" > +service_triggers() { > + local script name > + > + script=$(readlink -f "$initscript") > + name=$(basename ${script:-$initscript}) > + > + procd_open_trigger > + procd_add_config_trigger "config.change" "system" > /etc/init.d/$name reload > + > + procd_add_raw_trigger "interface.*" 2000 /etc/init.d/$name reload > + procd_close_trigger > + > procd_add_validation validate_ntp_section > } > -- > 1.9.1 > _______________________________________________ > openwrt-devel mailing list > openwrt-devel@lists.openwrt.org > https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel >
Hi, Please add raw triggers to only the interfaces specified in the list. Regards, Amine. On Thu, May 19, 2016 at 6:57 PM, Hans Dedecker <dedeckeh@gmail.com> wrote: > The busybox ntpd utility currently uses ntp servers specified in uci. > This patch allows the ntpd utility to use NTP servers received via DHCP(v6) > Following uci parameters have been added: > use_dhcp : enables NTP server config via DHCP(v6) > dhcp_interface : use NTP servers received only on the specified DHCP(v6) interfaces; if empty all interfaces are considered > > Signed-off-by: Hans Dedecker <dedeckeh@gmail.com> > --- > > The patch is based on a previous discussion held on the OpenWRT-devel mailing list > (https://lists.openwrt.org/pipermail/openwrt-devel/2016-January/039081.html) as per Felix's > comments this solution is based on procd interface service triggers > > package/utils/busybox/Makefile | 2 +- > package/utils/busybox/files/sysntpd | 43 ++++++++++++++++++++++++++++++++----- > 2 files changed, 39 insertions(+), 6 deletions(-) > > diff --git a/package/utils/busybox/Makefile b/package/utils/busybox/Makefile > index 24c064c..24e0e11 100644 > --- a/package/utils/busybox/Makefile > +++ b/package/utils/busybox/Makefile > @@ -42,7 +42,7 @@ define Package/busybox > MAINTAINER:=Felix Fietkau <nbd@openwrt.org> > TITLE:=Core utilities for embedded Linux > URL:=http://busybox.net/ > - DEPENDS:=+BUSYBOX_USE_LIBRPC:librpc +BUSYBOX_CONFIG_PAM:libpam > + DEPENDS:=+BUSYBOX_USE_LIBRPC:librpc +BUSYBOX_CONFIG_PAM:libpam +jsonfilter > MENU:=1 > endef > > diff --git a/package/utils/busybox/files/sysntpd b/package/utils/busybox/files/sysntpd > index f73bb83..5c663d7 100755 > --- a/package/utils/busybox/files/sysntpd > +++ b/package/utils/busybox/files/sysntpd > @@ -7,13 +7,35 @@ USE_PROCD=1 > PROG=/usr/sbin/ntpd > HOTPLUG_SCRIPT=/usr/sbin/ntpd-hotplug > > +get_dhcp_ntp_servers() { > + local interfaces="$1" > + local filter="*" > + local network_dump interface ntpservers ntpserver > + > + network_dump=$(ubus call network.interface dump) > + for interface in $interfaces; do > + [ "$filter" = "*" ] && filter="@.interface='$interface'" || filter="$filter,@.interface='$interface'" > + done > + > + ntpservers=$(jsonfilter -s "$network_dump" -e "@.interface[$filter]['data']['ntpserver']") > + > + for ntpserver in $ntpservers; do > + local duplicate=0 > + local entry > + for entry in $server; do > + [ "$ntpserver" = "$entry" ] && duplicate=1 > + done > + [ "$duplicate" = 0 ] && server="$server $ntpserver" > + done > +} > + > validate_ntp_section() { > uci_validate_section system timeserver "${1}" \ > - 'server:list(host)' 'enabled:bool:1' 'enable_server:bool:0' > + 'server:list(host)' 'enabled:bool:1' 'enable_server:bool:0' 'use_dhcp:bool:0' 'dhcp_interface:list(string)' > } > > start_service() { > - local server enabled enable_server peer > + local server enabled enable_server use_dhcp dhcp_interface peer > > validate_ntp_section ntp || { > echo "validation failed" > @@ -22,6 +44,8 @@ start_service() { > > [ $enabled = 0 ] && return > > + [ $use_dhcp = 1 ] && get_dhcp_ntp_servers "$dhcp_interface" > + > [ -z "$server" ] && return > > procd_open_instance > @@ -35,8 +59,17 @@ start_service() { > procd_close_instance > } > > -service_triggers() > -{ > - procd_add_reload_trigger "system" > +service_triggers() { > + local script name > + > + script=$(readlink -f "$initscript") > + name=$(basename ${script:-$initscript}) > + > + procd_open_trigger > + procd_add_config_trigger "config.change" "system" /etc/init.d/$name reload > + > + procd_add_raw_trigger "interface.*" 2000 /etc/init.d/$name reload > + procd_close_trigger > + > procd_add_validation validate_ntp_section > } > -- > 1.9.1 > _______________________________________________ > openwrt-devel mailing list > openwrt-devel@lists.openwrt.org > https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel
On Fri, 20 May 2016, Jo-Philipp Wich wrote: > Hi Hans, > >> I wanted to preserve the ntp server behavior and only change the >> behavior when configured in order to keep backwards compatibility. You >> favour enabling DHCP ntp server config without explicit config ? > > Personally I do because thats likely what most users expect, but then > trusting foreign NTP server advertisements might be a security sensitive > topic - on the other hand one trusts the default gateway and DNS > advertisements too, so I don't know. NTP isn't signed. If I can control your DNS, I can probably control your NTP by giving you the wrong IP for the NTP server If I can control your gateway, I can redirect all your NTP queries to someone else (NAT, redirects, etc) so why not trust the NTP server being provided? David Lang
On Fri, May 20, 2016 at 3:18 PM, David Lang <david@lang.hm> wrote: > On Fri, 20 May 2016, Jo-Philipp Wich wrote: > >> Hi Hans, >> >>> I wanted to preserve the ntp server behavior and only change the >>> behavior when configured in order to keep backwards compatibility. You >>> favour enabling DHCP ntp server config without explicit config ? >> >> >> Personally I do because thats likely what most users expect, but then >> trusting foreign NTP server advertisements might be a security sensitive >> topic - on the other hand one trusts the default gateway and DNS >> advertisements too, so I don't know. > > > NTP isn't signed. > > If I can control your DNS, I can probably control your NTP by giving you the > wrong IP for the NTP server > > If I can control your gateway, I can redirect all your NTP queries to > someone else (NAT, redirects, etc) > > so why not trust the NTP server being provided? OK let's make the concensus to enable use_dhcp by default Hans > > David Lang
On 20/05/16 14:43, Hans Dedecker wrote: > On Fri, May 20, 2016 at 3:18 PM, David Lang <david@lang.hm> wrote: >> On Fri, 20 May 2016, Jo-Philipp Wich wrote: >> >>> Hi Hans, >>> >>>> I wanted to preserve the ntp server behavior and only change the >>>> behavior when configured in order to keep backwards compatibility. You >>>> favour enabling DHCP ntp server config without explicit config ? >>> >>> Personally I do because thats likely what most users expect, but then >>> trusting foreign NTP server advertisements might be a security sensitive >>> topic - on the other hand one trusts the default gateway and DNS >>> advertisements too, so I don't know. >> >> NTP isn't signed. >> >> If I can control your DNS, I can probably control your NTP by giving you the >> wrong IP for the NTP server >> >> If I can control your gateway, I can redirect all your NTP queries to >> someone else (NAT, redirects, etc) >> >> so why not trust the NTP server being provided? > OK let's make the concensus to enable use_dhcp by default > > If there are none from dhcp, it'll fall back to the configured list? Servers from dhcp are extra? or replacing the configured?
On Fri, May 20, 2016 at 3:59 PM, Conor O'Gorman <i@conorogorman.net> wrote: > > > On 20/05/16 14:43, Hans Dedecker wrote: >> >> On Fri, May 20, 2016 at 3:18 PM, David Lang <david@lang.hm> wrote: >>> >>> On Fri, 20 May 2016, Jo-Philipp Wich wrote: >>> >>>> Hi Hans, >>>> >>>>> I wanted to preserve the ntp server behavior and only change the >>>>> behavior when configured in order to keep backwards compatibility. You >>>>> favour enabling DHCP ntp server config without explicit config ? >>>> >>>> >>>> Personally I do because thats likely what most users expect, but then >>>> trusting foreign NTP server advertisements might be a security sensitive >>>> topic - on the other hand one trusts the default gateway and DNS >>>> advertisements too, so I don't know. >>> >>> >>> NTP isn't signed. >>> >>> If I can control your DNS, I can probably control your NTP by giving you >>> the >>> wrong IP for the NTP server >>> >>> If I can control your gateway, I can redirect all your NTP queries to >>> someone else (NAT, redirects, etc) >>> >>> so why not trust the NTP server being provided? >> >> OK let's make the concensus to enable use_dhcp by default >> >> > If there are none from dhcp, it'll fall back to the configured list? > > Servers from dhcp are extra? or replacing the configured? Servers from DHCP are extra; thus on top of the configured ones
Hi, Please add raw triggers to only the interfaces specified in the list. Regards, Amine. On Fri, May 20, 2016 at 4:11 PM, Hans Dedecker <dedeckeh@gmail.com> wrote: > On Fri, May 20, 2016 at 3:59 PM, Conor O'Gorman <i@conorogorman.net> wrote: >> >> >> On 20/05/16 14:43, Hans Dedecker wrote: >>> >>> On Fri, May 20, 2016 at 3:18 PM, David Lang <david@lang.hm> wrote: >>>> >>>> On Fri, 20 May 2016, Jo-Philipp Wich wrote: >>>> >>>>> Hi Hans, >>>>> >>>>>> I wanted to preserve the ntp server behavior and only change the >>>>>> behavior when configured in order to keep backwards compatibility. You >>>>>> favour enabling DHCP ntp server config without explicit config ? >>>>> >>>>> >>>>> Personally I do because thats likely what most users expect, but then >>>>> trusting foreign NTP server advertisements might be a security sensitive >>>>> topic - on the other hand one trusts the default gateway and DNS >>>>> advertisements too, so I don't know. >>>> >>>> >>>> NTP isn't signed. >>>> >>>> If I can control your DNS, I can probably control your NTP by giving you >>>> the >>>> wrong IP for the NTP server >>>> >>>> If I can control your gateway, I can redirect all your NTP queries to >>>> someone else (NAT, redirects, etc) >>>> >>>> so why not trust the NTP server being provided? >>> >>> OK let's make the concensus to enable use_dhcp by default >>> >>> >> If there are none from dhcp, it'll fall back to the configured list? >> >> Servers from dhcp are extra? or replacing the configured? > Servers from DHCP are extra; thus on top of the configured ones > _______________________________________________ > openwrt-devel mailing list > openwrt-devel@lists.openwrt.org > https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel
diff --git a/package/utils/busybox/Makefile b/package/utils/busybox/Makefile index 24c064c..24e0e11 100644 --- a/package/utils/busybox/Makefile +++ b/package/utils/busybox/Makefile @@ -42,7 +42,7 @@ define Package/busybox MAINTAINER:=Felix Fietkau <nbd@openwrt.org> TITLE:=Core utilities for embedded Linux URL:=http://busybox.net/ - DEPENDS:=+BUSYBOX_USE_LIBRPC:librpc +BUSYBOX_CONFIG_PAM:libpam + DEPENDS:=+BUSYBOX_USE_LIBRPC:librpc +BUSYBOX_CONFIG_PAM:libpam +jsonfilter MENU:=1 endef diff --git a/package/utils/busybox/files/sysntpd b/package/utils/busybox/files/sysntpd index f73bb83..5c663d7 100755 --- a/package/utils/busybox/files/sysntpd +++ b/package/utils/busybox/files/sysntpd @@ -7,13 +7,35 @@ USE_PROCD=1 PROG=/usr/sbin/ntpd HOTPLUG_SCRIPT=/usr/sbin/ntpd-hotplug +get_dhcp_ntp_servers() { + local interfaces="$1" + local filter="*" + local network_dump interface ntpservers ntpserver + + network_dump=$(ubus call network.interface dump) + for interface in $interfaces; do + [ "$filter" = "*" ] && filter="@.interface='$interface'" || filter="$filter,@.interface='$interface'" + done + + ntpservers=$(jsonfilter -s "$network_dump" -e "@.interface[$filter]['data']['ntpserver']") + + for ntpserver in $ntpservers; do + local duplicate=0 + local entry + for entry in $server; do + [ "$ntpserver" = "$entry" ] && duplicate=1 + done + [ "$duplicate" = 0 ] && server="$server $ntpserver" + done +} + validate_ntp_section() { uci_validate_section system timeserver "${1}" \ - 'server:list(host)' 'enabled:bool:1' 'enable_server:bool:0' + 'server:list(host)' 'enabled:bool:1' 'enable_server:bool:0' 'use_dhcp:bool:0' 'dhcp_interface:list(string)' } start_service() { - local server enabled enable_server peer + local server enabled enable_server use_dhcp dhcp_interface peer validate_ntp_section ntp || { echo "validation failed" @@ -22,6 +44,8 @@ start_service() { [ $enabled = 0 ] && return + [ $use_dhcp = 1 ] && get_dhcp_ntp_servers "$dhcp_interface" + [ -z "$server" ] && return procd_open_instance @@ -35,8 +59,17 @@ start_service() { procd_close_instance } -service_triggers() -{ - procd_add_reload_trigger "system" +service_triggers() { + local script name + + script=$(readlink -f "$initscript") + name=$(basename ${script:-$initscript}) + + procd_open_trigger + procd_add_config_trigger "config.change" "system" /etc/init.d/$name reload + + procd_add_raw_trigger "interface.*" 2000 /etc/init.d/$name reload + procd_close_trigger + procd_add_validation validate_ntp_section }
The busybox ntpd utility currently uses ntp servers specified in uci. This patch allows the ntpd utility to use NTP servers received via DHCP(v6) Following uci parameters have been added: use_dhcp : enables NTP server config via DHCP(v6) dhcp_interface : use NTP servers received only on the specified DHCP(v6) interfaces; if empty all interfaces are considered Signed-off-by: Hans Dedecker <dedeckeh@gmail.com> --- The patch is based on a previous discussion held on the OpenWRT-devel mailing list (https://lists.openwrt.org/pipermail/openwrt-devel/2016-January/039081.html) as per Felix's comments this solution is based on procd interface service triggers package/utils/busybox/Makefile | 2 +- package/utils/busybox/files/sysntpd | 43 ++++++++++++++++++++++++++++++++----- 2 files changed, 39 insertions(+), 6 deletions(-)