If we get messages but they are not ntp replies (e.g. caused by

incoming icmp) do not register them as replies.  Also, fix a bug
introduced in the previous commit: first recompute scale, then
recompute interval, so that when the offset increases and thus the
scale is lowered both the poll interval and the check interval use
the same scale.
First issue spotted by naddy@ second one by and ok semarie@

2 files changed, 53 insertions, 26 deletions

diff --git a/usr.sbin/ntpd/client.c b/usr.sbin/ntpd/client.c
index d2e6119e232..f53d2420657 100644
--- a/usr.sbin/ntpd/client.c
+++ b/usr.sbin/ntpd/client.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: client.c,v 1.113 2020/01/30 15:55:41 otto Exp $ */
+/*	$OpenBSD: client.c,v 1.114 2020/09/11 07:09:41 otto Exp $ */
 
 /*
  * Copyright (c) 2003, 2004 Henning Brauer <henning@openbsd.org>
@@ -264,6 +264,12 @@ handle_auto(uint8_t trusted, double offset)
 	priv_settime(offset, "");
 }
 
+
+/*
+ * -1: Not processed, not an NTP message (e.g. icmp induced  ECONNREFUSED)
+ *  0: Not prrocessed due to validation issues
+ *  1: NTP message validated and processed
+ */
 int
 client_dispatch(struct ntp_peer *p, u_int8_t settime, u_int8_t automatic)
 {
@@ -278,7 +284,7 @@ client_dispatch(struct ntp_peer *p, u_int8_t settime, u_int8_t automatic)
 	} cmsgbuf;
 	struct cmsghdr		*cmsg;
 	ssize_t			 size;
-	double			 T1, T2, T3, T4;
+	double			 T1, T2, T3, T4, offset, delay;
 	time_t			 interval;
 
 	memset(&somsg, 0, sizeof(somsg));
@@ -297,7 +303,7 @@ client_dispatch(struct ntp_peer *p, u_int8_t settime, u_int8_t automatic)
 		    errno == ENOPROTOOPT || errno == ENOENT) {
 			client_log_error(p, "recvmsg", errno);
 			set_next(p, error_interval());
-			return (0);
+			return (-1);
 		} else
 			fatal("recvfrom");
 	}
@@ -418,16 +424,6 @@ client_dispatch(struct ntp_peer *p, u_int8_t settime, u_int8_t automatic)
 	} else
 		p->reply[p->shift].status.send_refid = msg.xmttime.fractionl;
 
-	if (p->trustlevel < TRUSTLEVEL_PATHETIC)
-		interval = scale_interval(INTERVAL_QUERY_PATHETIC);
-	else if (p->trustlevel < TRUSTLEVEL_AGGRESSIVE)
-		interval = (conf->settime && conf->automatic) ?
-		    INTERVAL_QUERY_ULTRA_VIOLENCE :
-		    scale_interval(INTERVAL_QUERY_AGGRESSIVE);
-	else
-		interval = scale_interval(INTERVAL_QUERY_NORMAL);
-
-	set_next(p, interval);
 	p->state = STATE_REPLY_RECEIVED;
 
 	/* every received reply which we do not discard increases trust */
@@ -439,11 +435,8 @@ client_dispatch(struct ntp_peer *p, u_int8_t settime, u_int8_t automatic)
 		p->trustlevel++;
 	}
 
-	log_debug("reply from %s: offset %f delay %f, "
-	    "next query %llds",
-	    log_sockaddr((struct sockaddr *)&p->addr->ss),
-	    p->reply[p->shift].offset, p->reply[p->shift].delay,
-	    (long long)interval);
+	offset = p->reply[p->shift].offset;
+	delay = p->reply[p->shift].delay;
 
 	client_update(p);
 	if (settime) {
@@ -453,10 +446,27 @@ client_dispatch(struct ntp_peer *p, u_int8_t settime, u_int8_t automatic)
 			priv_settime(p->reply[p->shift].offset, "");
 	}
 
+	if (p->trustlevel < TRUSTLEVEL_PATHETIC)
+		interval = scale_interval(INTERVAL_QUERY_PATHETIC);
+	else if (p->trustlevel < TRUSTLEVEL_AGGRESSIVE)
+		interval = (conf->settime && conf->automatic) ?
+		    INTERVAL_QUERY_ULTRA_VIOLENCE :
+		    scale_interval(INTERVAL_QUERY_AGGRESSIVE);
+	else
+		interval = scale_interval(INTERVAL_QUERY_NORMAL);
+
+	log_debug("reply from %s: offset %f delay %f, "
+	    "next query %llds",
+	    log_sockaddr((struct sockaddr *)&p->addr->ss),
+	    offset, delay,
+	    (long long)interval);
+
+	set_next(p, interval);
+
 	if (++p->shift >= OFFSET_ARRAY_SIZE)
 		p->shift = 0;
 
-	return (0);
+	return (1);
 }
 
 int
diff --git a/usr.sbin/ntpd/ntp.c b/usr.sbin/ntpd/ntp.c
index 4d09f7adcb1..42812df695f 100644
--- a/usr.sbin/ntpd/ntp.c
+++ b/usr.sbin/ntpd/ntp.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ntp.c,v 1.166 2020/08/30 16:21:29 otto Exp $ */
+/*	$OpenBSD: ntp.c,v 1.167 2020/09/11 07:09:41 otto Exp $ */
 
 /*
  * Copyright (c) 2003, 2004 Henning Brauer <henning@openbsd.org>
@@ -402,12 +402,29 @@ ntp_main(struct ntpd_conf *nconf, struct passwd *pw, int argc, char **argv)
 
 		for (; nfds > 0 && j < idx_clients; j++) {
 			if (pfd[j].revents & (POLLIN|POLLERR)) {
+				struct ntp_peer *pp = idx2peer[j - idx_peers];
+
 				nfds--;
-				last_action = now;
-				if (client_dispatch(idx2peer[j - idx_peers],
-				    conf->settime, conf->automatic) == -1) {
-					log_warn("pipe write error (settime)");
-					ntp_quit = 1;
+				switch (client_dispatch(pp, conf->settime,
+				    conf->automatic)) {
+				case -1:
+					log_debug("no reply from %s "
+					    "received", log_sockaddr(
+					    (struct sockaddr *) &pp->addr->ss));
+					if (pp->trustlevel >=
+					    TRUSTLEVEL_BADPEER &&
+					    (pp->trustlevel /= 2) <
+					    TRUSTLEVEL_BADPEER)
+						log_info("peer %s now invalid",
+						    log_sockaddr(
+						    (struct sockaddr *)
+						    &pp->addr->ss));
+					break;
+				case 0: /* invalid replies are ignored */
+					break;
+				case 1:
+					last_action = now;
+					break;
 				}
 			}
 		}
@@ -433,7 +450,7 @@ ntp_main(struct ntpd_conf *nconf, struct passwd *pw, int argc, char **argv)
 		interval = INTERVAL_QUERY_NORMAL * conf->scale;
 		interval += SCALE_INTERVAL(interval) - 1;
 		if (conf->status.synced && last_action + 3 * interval < now) {
-			log_info("clock is now unsynced");
+			log_info("clock is now unsynced due to lack of replies");
 			conf->status.synced = 0;
 			conf->scale = 1;
 			priv_dns(IMSG_UNSYNCED, NULL, 0);