| Message ID | 20180115124521.43778-1-ldir@darbyshire-bryant.me.uk |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [LEDE-DEV,v1] dnsmasq: use SIGINT for dnssec time valid | expand |
On Mon, Jan 15, 2018 at 1:45 PM, Kevin Darbyshire-Bryant <ldir@darbyshire-bryant.me.uk> wrote: > Dnsmasq used SIGHUP to do too many things: 1) set dnssec time validation > enabled, 2) bump SOA zone serial, 3) clear dns cache, 4) reload hosts > files, 5) reload resolvers/servers files. > > Many subsystems within LEDE can send SIGHUP to dnsmasq: 1) ntpd hotplug > (to indicate time is valid for dnssec) 2) odhcpd (to indicate a > new/removed host - typically DHCPv6 leases) 3) procd on interface state > changes 4) procd on system config state changes, 5) service reload. > > If dnssec time validation is enabled before the system clock has been > set to a sensible time, name resolution will fail. Because name > resolution fails, ntpd is unable to resolve time server names to > addresses, so is unable to set time. Classic chicken/egg. > > Since commits 23bba9cb330cd298739a16e350b0029ed9429eef (service reload) & > 4f02285d8b4a66359a8fa46f22a3efde391b5419 (system config) make it more > likely a SIGHUP will be sent for events other than 'ntpd has set time' > it is more likely that an errant 'name resolution is failing for > everything' situation will be encountered. > > Fortunately the upstream dnsmasq people agree and have moved 'check > dnssec timestamp enable' from SIGHUP handler to SIGINT. > > Backport the upstream patch to use SIGINT. > ntpd hotplug script updated to use SIGINT. > > Signed-off-by: Kevin Darbyshire-Bryant <ldir@darbyshire-bryant.me.uk> > --- > package/network/services/dnsmasq/Makefile | 2 +- > .../services/dnsmasq/files/dnsmasqsec.hotplug | 2 +- > .../dnsmasq/patches/260-dnssec-SIGINT.patch | 120 +++++++++++++++++++++ > 3 files changed, 122 insertions(+), 2 deletions(-) > create mode 100644 package/network/services/dnsmasq/patches/260-dnssec-SIGINT.patch > > diff --git a/package/network/services/dnsmasq/Makefile b/package/network/services/dnsmasq/Makefile > index c6d2739f03..1224ad86f8 100644 > --- a/package/network/services/dnsmasq/Makefile > +++ b/package/network/services/dnsmasq/Makefile > @@ -9,7 +9,7 @@ include $(TOPDIR)/rules.mk > > PKG_NAME:=dnsmasq > PKG_VERSION:=2.78 > -PKG_RELEASE:=7 > +PKG_RELEASE:=8 > > PKG_SOURCE:=$(PKG_NAME)-$(PKG_VERSION).tar.xz > PKG_SOURCE_URL:=http://thekelleys.org.uk/dnsmasq/ > diff --git a/package/network/services/dnsmasq/files/dnsmasqsec.hotplug b/package/network/services/dnsmasq/files/dnsmasqsec.hotplug > index a155eb0f6e..781d533734 100644 > --- a/package/network/services/dnsmasq/files/dnsmasqsec.hotplug > +++ b/package/network/services/dnsmasq/files/dnsmasqsec.hotplug > @@ -9,6 +9,6 @@ TIMEVALIDFILE="/var/state/dnsmasqsec" > [ -f "$TIMEVALIDFILE" ] || { > echo "ntpd says time is valid" >$TIMEVALIDFILE > /etc/init.d/dnsmasq enabled && { > - procd_send_signal dnsmasq > + procd_send_signal dnsmasq '*' INT > } > } > diff --git a/package/network/services/dnsmasq/patches/260-dnssec-SIGINT.patch b/package/network/services/dnsmasq/patches/260-dnssec-SIGINT.patch > new file mode 100644 > index 0000000000..e280142f75 > --- /dev/null > +++ b/package/network/services/dnsmasq/patches/260-dnssec-SIGINT.patch > @@ -0,0 +1,120 @@ > +From 3c973ad92d317df736d5a8fde67baba6b102d91e Mon Sep 17 00:00:00 2001 > +From: Simon Kelley <simon@thekelleys.org.uk> > +Date: Sun, 14 Jan 2018 21:05:37 +0000 > +Subject: [PATCH] Use SIGINT (instead of overloading SIGHUP) to turn on DNSSEC > + time validation. > + > +--- > + src/dnsmasq.c | 36 +++++++++++++++++++++++++----------- > + src/dnsmasq.h | 1 + > + src/helper.c | 3 ++- > + 5 files changed, 38 insertions(+), 14 deletions(-) > + > +--- a/src/dnsmasq.c > ++++ b/src/dnsmasq.c > +@@ -137,7 +137,8 @@ int main (int argc, char **argv) > + sigaction(SIGTERM, &sigact, NULL); > + sigaction(SIGALRM, &sigact, NULL); > + sigaction(SIGCHLD, &sigact, NULL); > +- > ++ sigaction(SIGINT, &sigact, NULL); > ++ > + /* ignore SIGPIPE */ > + sigact.sa_handler = SIG_IGN; > + sigaction(SIGPIPE, &sigact, NULL); > +@@ -815,7 +816,7 @@ int main (int argc, char **argv) > + > + daemon->dnssec_no_time_check = option_bool(OPT_DNSSEC_TIME); > + if (option_bool(OPT_DNSSEC_TIME) && !daemon->back_to_the_future) > +- my_syslog(LOG_INFO, _("DNSSEC signature timestamps not checked until first cache reload")); > ++ my_syslog(LOG_INFO, _("DNSSEC signature timestamps not checked until receipt of SIGINT")); > + > + if (rc == 1) > + my_syslog(LOG_INFO, _("DNSSEC signature timestamps not checked until system time valid")); > +@@ -1142,7 +1143,7 @@ static void sig_handler(int sig) > + { > + /* ignore anything other than TERM during startup > + and in helper proc. (helper ignore TERM too) */ > +- if (sig == SIGTERM) > ++ if (sig == SIGTERM || sig == SIGINT) > + exit(EC_MISC); > + } > + else if (pid != getpid()) > +@@ -1168,6 +1169,15 @@ static void sig_handler(int sig) > + event = EVENT_DUMP; > + else if (sig == SIGUSR2) > + event = EVENT_REOPEN; > ++ else if (sig == SIGINT) > ++ { > ++ /* Handle SIGINT normally in debug mode, so > ++ ctrl-c continues to operate. */ > ++ if (option_bool(OPT_DEBUG)) > ++ exit(EC_MISC); > ++ else > ++ event = EVENT_TIME; > ++ } > + else > + return; > + > +@@ -1295,14 +1305,7 @@ static void async_event(int pipe, time_t > + { > + case EVENT_RELOAD: > + daemon->soa_sn++; /* Bump zone serial, as it may have changed. */ > +- > +-#ifdef HAVE_DNSSEC > +- if (daemon->dnssec_no_time_check && option_bool(OPT_DNSSEC_VALID) && option_bool(OPT_DNSSEC_TIME)) > +- { > +- my_syslog(LOG_INFO, _("now checking DNSSEC signature timestamps")); > +- daemon->dnssec_no_time_check = 0; > +- } > +-#endif > ++ > + /* fall through */ > + > + case EVENT_INIT: > +@@ -1411,6 +1414,17 @@ static void async_event(int pipe, time_t > + poll_resolv(0, 1, now); > + break; > + > ++ case EVENT_TIME: > ++#ifdef HAVE_DNSSEC > ++ if (daemon->dnssec_no_time_check && option_bool(OPT_DNSSEC_VALID) && option_bool(OPT_DNSSEC_TIME)) > ++ { > ++ my_syslog(LOG_INFO, _("now checking DNSSEC signature timestamps")); > ++ daemon->dnssec_no_time_check = 0; > ++ clear_cache_and_reload(now); > ++ } > ++#endif > ++ break; > ++ > + case EVENT_TERM: > + /* Knock all our children on the head. */ > + for (i = 0; i < MAX_PROCS; i++) > +--- a/src/dnsmasq.h > ++++ b/src/dnsmasq.h > +@@ -175,6 +175,7 @@ struct event_desc { > + #define EVENT_NEWROUTE 23 > + #define EVENT_TIME_ERR 24 > + #define EVENT_SCRIPT_LOG 25 > ++#define EVENT_TIME 26 > + > + /* Exit codes. */ > + #define EC_GOOD 0 > +--- a/src/helper.c > ++++ b/src/helper.c > +@@ -97,13 +97,14 @@ int create_helper(int event_fd, int err_ > + return pipefd[1]; > + } > + > +- /* ignore SIGTERM, so that we can clean up when the main process gets hit > ++ /* ignore SIGTERM and SIGINT, so that we can clean up when the main process gets hit > + and SIGALRM so that we can use sleep() */ > + sigact.sa_handler = SIG_IGN; > + sigact.sa_flags = 0; > + sigemptyset(&sigact.sa_mask); > + sigaction(SIGTERM, &sigact, NULL); > + sigaction(SIGALRM, &sigact, NULL); > ++ sigaction(SIGINT, &sigact, NULL); > + > + if (!option_bool(OPT_DEBUG) && uid != 0) > + { > -- > 2.14.3 (Apple Git-98) > > > _______________________________________________ > Lede-dev mailing list > Lede-dev@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/lede-dev Thanks; patch applied in commit https://git.openwrt.org/?p=openwrt/openwrt.git;a=commit;h=aba3b1c6a3639bf821d2cf305de22ec276fbff33 Hans
diff --git a/package/network/services/dnsmasq/Makefile b/package/network/services/dnsmasq/Makefile index c6d2739f03..1224ad86f8 100644 --- a/package/network/services/dnsmasq/Makefile +++ b/package/network/services/dnsmasq/Makefile @@ -9,7 +9,7 @@ include $(TOPDIR)/rules.mk PKG_NAME:=dnsmasq PKG_VERSION:=2.78 -PKG_RELEASE:=7 +PKG_RELEASE:=8 PKG_SOURCE:=$(PKG_NAME)-$(PKG_VERSION).tar.xz PKG_SOURCE_URL:=http://thekelleys.org.uk/dnsmasq/ diff --git a/package/network/services/dnsmasq/files/dnsmasqsec.hotplug b/package/network/services/dnsmasq/files/dnsmasqsec.hotplug index a155eb0f6e..781d533734 100644 --- a/package/network/services/dnsmasq/files/dnsmasqsec.hotplug +++ b/package/network/services/dnsmasq/files/dnsmasqsec.hotplug @@ -9,6 +9,6 @@ TIMEVALIDFILE="/var/state/dnsmasqsec" [ -f "$TIMEVALIDFILE" ] || { echo "ntpd says time is valid" >$TIMEVALIDFILE /etc/init.d/dnsmasq enabled && { - procd_send_signal dnsmasq + procd_send_signal dnsmasq '*' INT } } diff --git a/package/network/services/dnsmasq/patches/260-dnssec-SIGINT.patch b/package/network/services/dnsmasq/patches/260-dnssec-SIGINT.patch new file mode 100644 index 0000000000..e280142f75 --- /dev/null +++ b/package/network/services/dnsmasq/patches/260-dnssec-SIGINT.patch @@ -0,0 +1,120 @@ +From 3c973ad92d317df736d5a8fde67baba6b102d91e Mon Sep 17 00:00:00 2001 +From: Simon Kelley <simon@thekelleys.org.uk> +Date: Sun, 14 Jan 2018 21:05:37 +0000 +Subject: [PATCH] Use SIGINT (instead of overloading SIGHUP) to turn on DNSSEC + time validation. + +--- + src/dnsmasq.c | 36 +++++++++++++++++++++++++----------- + src/dnsmasq.h | 1 + + src/helper.c | 3 ++- + 5 files changed, 38 insertions(+), 14 deletions(-) + +--- a/src/dnsmasq.c ++++ b/src/dnsmasq.c +@@ -137,7 +137,8 @@ int main (int argc, char **argv) + sigaction(SIGTERM, &sigact, NULL); + sigaction(SIGALRM, &sigact, NULL); + sigaction(SIGCHLD, &sigact, NULL); +- ++ sigaction(SIGINT, &sigact, NULL); ++ + /* ignore SIGPIPE */ + sigact.sa_handler = SIG_IGN; + sigaction(SIGPIPE, &sigact, NULL); +@@ -815,7 +816,7 @@ int main (int argc, char **argv) + + daemon->dnssec_no_time_check = option_bool(OPT_DNSSEC_TIME); + if (option_bool(OPT_DNSSEC_TIME) && !daemon->back_to_the_future) +- my_syslog(LOG_INFO, _("DNSSEC signature timestamps not checked until first cache reload")); ++ my_syslog(LOG_INFO, _("DNSSEC signature timestamps not checked until receipt of SIGINT")); + + if (rc == 1) + my_syslog(LOG_INFO, _("DNSSEC signature timestamps not checked until system time valid")); +@@ -1142,7 +1143,7 @@ static void sig_handler(int sig) + { + /* ignore anything other than TERM during startup + and in helper proc. (helper ignore TERM too) */ +- if (sig == SIGTERM) ++ if (sig == SIGTERM || sig == SIGINT) + exit(EC_MISC); + } + else if (pid != getpid()) +@@ -1168,6 +1169,15 @@ static void sig_handler(int sig) + event = EVENT_DUMP; + else if (sig == SIGUSR2) + event = EVENT_REOPEN; ++ else if (sig == SIGINT) ++ { ++ /* Handle SIGINT normally in debug mode, so ++ ctrl-c continues to operate. */ ++ if (option_bool(OPT_DEBUG)) ++ exit(EC_MISC); ++ else ++ event = EVENT_TIME; ++ } + else + return; + +@@ -1295,14 +1305,7 @@ static void async_event(int pipe, time_t + { + case EVENT_RELOAD: + daemon->soa_sn++; /* Bump zone serial, as it may have changed. */ +- +-#ifdef HAVE_DNSSEC +- if (daemon->dnssec_no_time_check && option_bool(OPT_DNSSEC_VALID) && option_bool(OPT_DNSSEC_TIME)) +- { +- my_syslog(LOG_INFO, _("now checking DNSSEC signature timestamps")); +- daemon->dnssec_no_time_check = 0; +- } +-#endif ++ + /* fall through */ + + case EVENT_INIT: +@@ -1411,6 +1414,17 @@ static void async_event(int pipe, time_t + poll_resolv(0, 1, now); + break; + ++ case EVENT_TIME: ++#ifdef HAVE_DNSSEC ++ if (daemon->dnssec_no_time_check && option_bool(OPT_DNSSEC_VALID) && option_bool(OPT_DNSSEC_TIME)) ++ { ++ my_syslog(LOG_INFO, _("now checking DNSSEC signature timestamps")); ++ daemon->dnssec_no_time_check = 0; ++ clear_cache_and_reload(now); ++ } ++#endif ++ break; ++ + case EVENT_TERM: + /* Knock all our children on the head. */ + for (i = 0; i < MAX_PROCS; i++) +--- a/src/dnsmasq.h ++++ b/src/dnsmasq.h +@@ -175,6 +175,7 @@ struct event_desc { + #define EVENT_NEWROUTE 23 + #define EVENT_TIME_ERR 24 + #define EVENT_SCRIPT_LOG 25 ++#define EVENT_TIME 26 + + /* Exit codes. */ + #define EC_GOOD 0 +--- a/src/helper.c ++++ b/src/helper.c +@@ -97,13 +97,14 @@ int create_helper(int event_fd, int err_ + return pipefd[1]; + } + +- /* ignore SIGTERM, so that we can clean up when the main process gets hit ++ /* ignore SIGTERM and SIGINT, so that we can clean up when the main process gets hit + and SIGALRM so that we can use sleep() */ + sigact.sa_handler = SIG_IGN; + sigact.sa_flags = 0; + sigemptyset(&sigact.sa_mask); + sigaction(SIGTERM, &sigact, NULL); + sigaction(SIGALRM, &sigact, NULL); ++ sigaction(SIGINT, &sigact, NULL); + + if (!option_bool(OPT_DEBUG) && uid != 0) + {
Dnsmasq used SIGHUP to do too many things: 1) set dnssec time validation enabled, 2) bump SOA zone serial, 3) clear dns cache, 4) reload hosts files, 5) reload resolvers/servers files. Many subsystems within LEDE can send SIGHUP to dnsmasq: 1) ntpd hotplug (to indicate time is valid for dnssec) 2) odhcpd (to indicate a new/removed host - typically DHCPv6 leases) 3) procd on interface state changes 4) procd on system config state changes, 5) service reload. If dnssec time validation is enabled before the system clock has been set to a sensible time, name resolution will fail. Because name resolution fails, ntpd is unable to resolve time server names to addresses, so is unable to set time. Classic chicken/egg. Since commits 23bba9cb330cd298739a16e350b0029ed9429eef (service reload) & 4f02285d8b4a66359a8fa46f22a3efde391b5419 (system config) make it more likely a SIGHUP will be sent for events other than 'ntpd has set time' it is more likely that an errant 'name resolution is failing for everything' situation will be encountered. Fortunately the upstream dnsmasq people agree and have moved 'check dnssec timestamp enable' from SIGHUP handler to SIGINT. Backport the upstream patch to use SIGINT. ntpd hotplug script updated to use SIGINT. Signed-off-by: Kevin Darbyshire-Bryant <ldir@darbyshire-bryant.me.uk> --- package/network/services/dnsmasq/Makefile | 2 +- .../services/dnsmasq/files/dnsmasqsec.hotplug | 2 +- .../dnsmasq/patches/260-dnssec-SIGINT.patch | 120 +++++++++++++++++++++ 3 files changed, 122 insertions(+), 2 deletions(-) create mode 100644 package/network/services/dnsmasq/patches/260-dnssec-SIGINT.patch