From db00298111a53be1ad2be1abf50ddebf074afec3 Mon Sep 17 00:00:00 2001
From: reyk <>
Date: Wed, 27 Jan 2016 21:48:34 +0000
Subject: [PATCH] Don't attempt to kill() the constraint in the wrong process. 
 The process management of the contraint processes has been moved from ntp to
 the parent, for better privsep and pledge, but the ntp process still
 attempted to kill the constraints on timeout directly.  Fix this regression
 by introducing a new imsg from ntp to the parent and the related logic to
 kill a constraint at the right place.

Reported & tested by bcook@
Ok bcook@
---
 src/usr.sbin/ntpd/constraint.c | 36 ++++++++++++++++++++++++++++------
 src/usr.sbin/ntpd/ntpd.c       |  5 ++++-
 src/usr.sbin/ntpd/ntpd.h       |  5 ++++-
 3 files changed, 38 insertions(+), 8 deletions(-)

diff --git a/src/usr.sbin/ntpd/constraint.c b/src/usr.sbin/ntpd/constraint.c
index 91d6ceb5..169007e4 100644
--- a/src/usr.sbin/ntpd/constraint.c
+++ b/src/usr.sbin/ntpd/constraint.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: constraint.c,v 1.24 2015/12/19 17:55:29 reyk Exp $	*/
+/*	$OpenBSD: constraint.c,v 1.25 2016/01/27 21:48:34 reyk Exp $	*/
 
 /*
  * Copyright (c) 2015 Reyk Floeter <reyk@openbsd.org>
@@ -163,7 +163,10 @@ constraint_query(struct constraint *cstr)
 		}
 
 		/* Timeout, just kill the process to reset it. */
-		kill(cstr->pid, SIGTERM);
+		imsg_compose(ibuf_main, IMSG_CONSTRAINT_KILL,
+		    cstr->id, 0, -1, NULL, 0);
+
+		cstr->state = STATE_TIMEOUT;
 		return (-1);
 	case STATE_INVALID:
 		if (cstr->last + CONSTRAINT_SCAN_INTERVAL > now) {
@@ -380,6 +383,7 @@ priv_constraint_check_child(pid_t pid, int status)
 {
 	struct constraint	*cstr;
 	int			 fail, sig;
+	char			*signame;
 
 	fail = sig = 0;
 	if (WIFSIGNALED(status)) {
@@ -391,15 +395,35 @@ priv_constraint_check_child(pid_t pid, int status)
 		fatalx("unexpected cause of SIGCHLD");
 
 	if ((cstr = constraint_bypid(pid)) != NULL) {
-		if (sig)
-			fatalx("constraint %s, signal %d",
-			    log_sockaddr((struct sockaddr *)
-			    &cstr->addr->ss), sig);
+		if (sig) {
+			if (sig != SIGTERM) {
+				signame = strsignal(sig) ?
+				    strsignal(sig) : "unknown";
+				log_warnx("constraint %s; "
+				    "terminated with signal %d (%s)",
+				    log_sockaddr((struct sockaddr *)
+				    &cstr->addr->ss), sig, signame);
+			}
+			fail = 1;
+		}
 
 		priv_constraint_close(cstr->fd, fail);
 	}
 }
 
+void
+priv_constraint_kill(u_int32_t id)
+{
+	struct constraint	*cstr;
+
+	if ((cstr = constraint_byid(id)) == NULL) {
+		log_warnx("IMSG_CONSTRAINT_KILL for invalid id %d", id);
+		return;
+	}
+
+	kill(cstr->pid, SIGTERM);
+}
+
 struct constraint *
 constraint_byid(u_int32_t id)
 {
diff --git a/src/usr.sbin/ntpd/ntpd.c b/src/usr.sbin/ntpd/ntpd.c
index 9a54c524..fe79f6d9 100644
--- a/src/usr.sbin/ntpd/ntpd.c
+++ b/src/usr.sbin/ntpd/ntpd.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ntpd.c,v 1.104 2016/01/27 21:36:25 bcook Exp $ */
+/*	$OpenBSD: ntpd.c,v 1.105 2016/01/27 21:48:34 reyk Exp $ */
 
 /*
  * Copyright (c) 2003, 2004 Henning Brauer <henning@openbsd.org>
@@ -413,6 +413,9 @@ dispatch_imsg(struct ntpd_conf *lconf, const char *pw_dir,
 			    imsg.data, imsg.hdr.len - IMSG_HEADER_SIZE,
 			    pw_dir, pw_uid, pw_gid);
 			break;
+		case IMSG_CONSTRAINT_KILL:
+			priv_constraint_kill(imsg.hdr.peerid);
+			break;
 		default:
 			break;
 		}
diff --git a/src/usr.sbin/ntpd/ntpd.h b/src/usr.sbin/ntpd/ntpd.h
index 5e9099e0..51b67516 100644
--- a/src/usr.sbin/ntpd/ntpd.h
+++ b/src/usr.sbin/ntpd/ntpd.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ntpd.h,v 1.128 2016/01/27 21:36:25 bcook Exp $ */
+/*	$OpenBSD: ntpd.h,v 1.129 2016/01/27 21:48:34 reyk Exp $ */
 
 /*
  * Copyright (c) 2003, 2004 Henning Brauer <henning@openbsd.org>
@@ -89,6 +89,7 @@ enum client_state {
 	STATE_DNS_DONE,
 	STATE_QUERY_SENT,
 	STATE_REPLY_RECEIVED,
+	STATE_TIMEOUT,
 	STATE_INVALID
 };
 
@@ -286,6 +287,7 @@ enum imsg_type {
 	IMSG_CONSTRAINT_QUERY,
 	IMSG_CONSTRAINT_RESULT,
 	IMSG_CONSTRAINT_CLOSE,
+	IMSG_CONSTRAINT_KILL,
 	IMSG_CTL_SHOW_STATUS,
 	IMSG_CTL_SHOW_PEERS,
 	IMSG_CTL_SHOW_PEERS_END,
@@ -357,6 +359,7 @@ void	 constraint_msg_result(u_int32_t, u_int8_t *, size_t);
 void	 constraint_msg_close(u_int32_t, u_int8_t *, size_t);
 void	 priv_constraint_msg(u_int32_t, u_int8_t *, size_t,
 	    const char *, uid_t, gid_t);
+void	 priv_constraint_kill(u_int32_t);
 int	 priv_constraint_dispatch(struct pollfd *);
 void	 priv_constraint_check_child(pid_t, int);
 char	*get_string(u_int8_t *, size_t);