[Git][NTPsec/ntpsec][master] MS-SNTP: Replace glue logic and add much logging up to about four messages/hour.

Hal Murray (@hal.murray) gitlab at mg.gitlab.com
Sat Oct 21 02:42:28 UTC 2023



Hal Murray pushed to branch master at NTPsec / ntpsec


Commits:
bd34d905 by James Browning at 2023-10-21T02:39:14+00:00
MS-SNTP: Replace glue logic and add much logging up to about four messages/hour.

- - - - -


5 changed files:

- include/ntp_stdlib.h
- include/ntpd.h
- libntp/msyslog.c
- ntpd/ntp_proto.c
- ntpd/ntp_signd.c


Changes:

=====================================
include/ntp_stdlib.h
=====================================
@@ -26,8 +26,16 @@
 #define NTP_PRINTF(fmt, args)
 #endif
 
+struct do_we_log {
+	double c_decay;	/* hours, exponential decay time */
+	double c_limit;	/* packets per hour */
+	double score;	/* score, packets/hour */
+	time_t last;	/* time of last attempted print */
+};
+
 extern const char *ntpd_version(void);
 
+extern	void	maybe_log(struct do_we_log*, int, const char *, ...) NTP_PRINTF(3, 4);
 extern	void	msyslog(int, const char *, ...) NTP_PRINTF(2, 3);
 extern	void	ntp_strerror_r(int errnum, char *buf, size_t buflen);
 extern	void	init_logging	(const char *, uint32_t, int);


=====================================
include/ntpd.h
=====================================
@@ -444,7 +444,7 @@ extern struct restriction_data rstrct;
 
 #ifdef ENABLE_MSSNTP
 /* ntp_signd.c */
-extern void send_via_ntp_signd(struct recvbuf *, keyid_t, int, void *);
+extern void send_via_ntp_signd(struct recvbuf *, void *);
 #endif
 
 /* ntp_timer.c */


=====================================
libntp/msyslog.c
=====================================
@@ -7,7 +7,9 @@
 
 #include "config.h"
 
+#include <limits.h>
 #include <sys/types.h>
+#include <time.h>
 #include <unistd.h>
 #include <stdio.h>
 #include <string.h>
@@ -28,7 +30,9 @@ static FILE *	syslog_file;
 static char *	syslog_fname;
 static char *	syslog_abs_fname;
 
+#ifdef DEBUG
 int		debug;
+#endif // DEBUG
 
 /* Counters */
 struct log_counters {   /* LOG_EMERG, LOG_ALERT, LOG_CRIT not used */
@@ -216,6 +220,49 @@ msyslog(
 }
 
 
+/*
+  Do we log this sundry error?
+*/
+void
+maybe_log(
+	struct do_we_log *maybe,
+	int		level,
+	const char *	fmt,
+	...
+	)
+{
+	struct timespec now;	/* time of current attempted print */
+	char	buf[PIPE_BUF];
+	va_list	ap;
+
+	if (NULL != maybe) {
+		clock_gettime(CLOCK_MONOTONIC, &now);
+		if (0 == maybe->last) {
+			/* first time */
+			maybe->last = now.tv_sec;
+		} else {
+			time_t interval_fp = now.tv_sec - maybe->last;
+			double since_last = ((double)interval_fp)/3600.0;
+			maybe->last = now.tv_sec;
+			maybe->score *= exp(-since_last/maybe->c_decay);
+			if (maybe->c_limit < maybe->score) {
+				return;
+			}
+		}
+		maybe->score += 1.0/maybe->c_decay;  /* only count the ones we print */
+#if (1)  // Do we reject logging attempts when argument 0 is NULL?
+	} else {
+		return;
+#endif
+	}
+
+	va_start(ap, fmt);
+	vsnprintf(buf, sizeof(buf), fmt, ap);
+	va_end(ap);
+	addto_syslog(level, buf);
+}
+
+
 /*
  * Initialize the logging
  *


=====================================
ntpd/ntp_proto.c
=====================================
@@ -4,6 +4,7 @@
  */
 #include "config.h"
 
+#include "ntp.h"
 #include "ntpd.h"
 #include "ntp_endian.h"
 #include "ntp_stdlib.h"
@@ -19,6 +20,7 @@
 #endif
 #include <unistd.h>
 
+#define MSSNTP_QUERY_MAC_LEN 16
 
 /*
  * Byte order conversion
@@ -661,6 +663,11 @@ receive(
 	auth_info* auth = NULL;  /* !NULL if authenticated */
 	int mode;
 
+#ifdef ENABLE_MSSNTP
+	uint8_t zero_key[MSSNTP_QUERY_MAC_LEN];
+	memset(&zero_key, 0, MSSNTP_QUERY_MAC_LEN);
+#endif /* ENABLE_MSSNTP */
+
 	stat_proto_total.sys_received++;
 
 #ifdef NTPv1
@@ -755,6 +762,21 @@ receive(
 	    }
 	}
 
+#ifdef ENABLE_MSSNTP
+	// If in an MS-SNTP restrict range, with a 16-byte all zero authenticator,
+	if((RES_MSSNTP == (restrict_mask & RES_MSSNTP))
+	    &&(MSSNTP_QUERY_MAC_LEN == rbufp->mac_len)
+	    &&(0 == memcmp(&(rbufp->recv_buffer[LEN_PKT_NOMAC + 4]),
+		                 zero_key, MSSNTP_QUERY_MAC_LEN))
+	) {
+	    // switch off the check that was breaking MS-SNTP;
+	    rbufp->keyid_present = false;
+	} else {
+	    // otherwise, switch off MS-SNTP to not break all other time service.
+	    restrict_mask &= ~RES_MSSNTP;
+	}
+#endif /* ENABLE_MSSNTP */
+
 	if(i_require_authentication(peer, restrict_mask) ||
 	    /* He wants authentication */
 	    rbufp->keyid_present) {
@@ -2355,10 +2377,7 @@ fast_xmit(
 
 #ifdef ENABLE_MSSNTP
 	if (flags & RES_MSSNTP) {
-		keyid_t keyid = 0;
-		if (NULL != auth) keyid = auth->keyid;
-		// FIXME need counter
-		send_via_ntp_signd(rbufp, keyid, flags, &xpkt);
+		send_via_ntp_signd(rbufp, &xpkt); // Simplified the API
 		return;
 	}
 #endif /* ENABLE_MSSNTP */


=====================================
ntpd/ntp_signd.c
=====================================
@@ -4,6 +4,8 @@
    Licensed under the same terms as NTP itself.
  */
 #include "config.h"
+#include "ntp.h"
+#include <sys/socket.h>
 
 #ifdef ENABLE_MSSNTP
 
@@ -12,8 +14,10 @@
 #include "ntp_stdlib.h"
 
 #include <string.h>
-#include <stdio.h>
+#include <stdbool.h>
 #include <stddef.h>
+#include <stdint.h>
+#include <stdio.h>
 
 #include <sys/un.h>
 
@@ -25,6 +29,17 @@
  * but unextended and MACless packet headers, so it can't be used with NTS.
  */
 
+#define ERR_BUF_LEN 96 // arbitary length for error buffer (6x16) -- JamesB192
+// some tinkering suggests malloc allocates 16n byte chunks with 8byte gaps
+
+static struct do_we_log do_we_log_signd = {
+	.c_decay=2,	// What is the period length in hours
+	.c_limit=4,	// How many messages/period
+	.score=0,	// Zero the score to begin
+	.last=0,	// Zero the last to begin
+};
+
+static uint32_t signd_count = 1;
 
 /*
   connect to a unix domain socket
@@ -32,6 +47,7 @@
 static int
 ux_socket_connect(const char *name)
 {
+	struct timeval tv = { .tv_sec=2, .tv_usec=400000};
 	int fd;
 	struct sockaddr_un addr;
 	if (!name) {
@@ -44,10 +60,23 @@ ux_socket_connect(const char *name)
 
 	fd = socket(AF_UNIX, SOCK_STREAM, 0);
 	if (fd == -1) {
+		char errbuf[ERR_BUF_LEN];
+		ntp_strerror_r(errno, errbuf, sizeof(errbuf));
+		maybe_log(&do_we_log_signd, LOG_ERR,
+			"SIGND: can not create socket: %s",
+			errbuf);
 		return -1;
 	}
 
+	// FIXME: Chutzpah code ... always works so well.
+	setsockopt(fd, SOL_SOCKET, SO_RCVTIMEO, (const void *)&tv, sizeof(tv));
+
 	if (connect(fd, (struct sockaddr *)&addr, sizeof(addr)) == -1) {
+		char errbuf[ERR_BUF_LEN];
+		ntp_strerror_r(errno, errbuf, sizeof(errbuf));
+		maybe_log(&do_we_log_signd, LOG_ERR,
+			"SIGND: can not connect socket '%s': %s",
+			name, errbuf);
 		close(fd);
 		return -1;
 	}
@@ -59,12 +88,12 @@ ux_socket_connect(const char *name)
 /*
   keep writing until its all sent
 */
-static int
+static size_t
 write_all(int fd, const void *buf, size_t len)
 {
 	size_t total = 0;
 	while (len) {
-		int n = write(fd, buf, len);
+		ssize_t n = write(fd, buf, len);
 		if (n <= 0) return total;
 		buf = n + (const char *)buf;
 		len -= (unsigned int)n;
@@ -76,12 +105,12 @@ write_all(int fd, const void *buf, size_t len)
 /*
   keep reading until its all read
 */
-static int
+static size_t
 read_all(int fd, void *buf, size_t len)
 {
 	size_t total = 0;
 	while (len) {
-		int n = read(fd, buf, len);
+		ssize_t n = read(fd, buf, len);
 		if (n <= 0) return total;
 		buf = n + (char *)buf;
 		len -= (unsigned int)n;
@@ -97,8 +126,21 @@ static int
 send_packet(int fd, const char *buf, uint32_t len)
 {
 	uint32_t net_len = htonl(len);
-	if (write_all(fd, &net_len, sizeof(net_len)) != sizeof(net_len)) return -1;
-	if (write_all(fd, buf, len) != (int)len) return -1;
+	if (write_all(fd, &net_len, sizeof(net_len)) != sizeof(net_len)) {
+		char errbuf[ERR_BUF_LEN];
+		ntp_strerror_r(errno, errbuf, sizeof(errbuf));
+		maybe_log(&do_we_log_signd, LOG_DEBUG,
+			"SIGND: can not send length: %s",
+			errbuf);
+		return -1;
+	}
+	if (write_all(fd, buf, len) != (size_t)len) {
+		char errbuf[ERR_BUF_LEN];
+		ntp_strerror_r(errno, errbuf, sizeof(errbuf));
+		maybe_log(&do_we_log_signd, LOG_DEBUG,
+			"SIGND: can not send packet to samba: %s", errbuf);
+		return -1;
+	}
 	return 0;
 }
 
@@ -108,11 +150,42 @@ send_packet(int fd, const char *buf, uint32_t len)
 static int
 recv_packet(int fd, char **buf, uint32_t *len)
 {
-	if (read_all(fd, len, sizeof(*len)) != sizeof(*len)) return -1;
+	if (read_all(fd, len, sizeof(*len)) != sizeof(*len)) {
+		char errbuf[ERR_BUF_LEN];
+		ntp_strerror_r(errno, errbuf, sizeof(errbuf));
+		maybe_log(&do_we_log_signd, LOG_DEBUG,
+			"SIGND: can not receive length: %s",
+			errbuf);
+		return -1;
+	}
+	// There are constraints on length.
+	// anything not in 64 <= len <= 1512 is probably garbage
+	// 12 is metadata len before the signed packet
+	// 48 is packet len, and 4 the keyID len
+	// anything > 1512 will unacceptably fragment on ethernet 
+	// FIXME: Word is the one true length is 80 (12+48+4+16)
+	//        and all others are as an abominataton unto MS-SNTP
 	*len = ntohl(*len);
+	if (1512 < *len) {
+		maybe_log(&do_we_log_signd, LOG_DEBUG,
+			"SIGND: rejecting too long packet: got %u, max is 1512",
+			*len);
+		return -1;
+	}
+	if (12 > *len) {
+		maybe_log(&do_we_log_signd, LOG_DEBUG,
+			"SIGND: rejecting too short packet: got %u, min is 12",
+			*len);
+		return -1;
+	}
 	(*buf) = emalloc(*len);
-	if (read_all(fd, *buf, *len) != (int)*len) {
+	if (read_all(fd, *buf, *len) != (size_t)*len) {
+		char errbuf[ERR_BUF_LEN];
+		ntp_strerror_r(errno, errbuf, sizeof(errbuf));
 		free(*buf);
+		maybe_log(&do_we_log_signd, LOG_DEBUG,
+			"SIGND: can not receive signed packet from samba: %s",
+			errbuf);
 		return -1;
 	}
 	return 0;
@@ -121,13 +194,9 @@ recv_packet(int fd, char **buf, uint32_t *len)
 void
 send_via_ntp_signd(
 	struct recvbuf *rbufp,	/* receive packet pointer */
-	keyid_t	xkeyid,
-	int flags,
 	void *xpkt
 	)
 {
-	UNUSED_ARG(flags);
-
 	/* We are here because it was detected that the client
 	 * sent an all-zero signature, and we therefore know
 	 * it's windows trying to talk to an AD server
@@ -138,11 +207,16 @@ send_via_ntp_signd(
 	 * packet over to Samba to sign, and return to us.
 	 *
 	 * The signing method Samba will use is described by
-	 * Microsoft in MS-SNTP, found here:
+	 * Microsoft in MS-SNTP, previously found here:
 	 * http://msdn.microsoft.com/en-us/library/cc212930.aspx
+	 *
+	 * which now seems to be located at
+	 * https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-sntp/8106cb73-ab3a-4542-8bc8-784dd32031cc
 	 */
 
-	int fd, sendlen;
+	bool quit = false;
+	int fd;
+	size_t sendlen;
 	struct samba_key_in {
 		uint32_t version;
 		uint32_t op;
@@ -155,9 +229,9 @@ send_via_ntp_signd(
 		uint32_t version;
 		uint32_t op;
 		uint32_t packet_id;
-		char pkt[LEN_PKT_NOMAC];
 	} samba_reply;
 
+	uint32_t header_length = sizeof(samba_reply);
 	char full_socket[256];
 
 	char *reply = NULL;
@@ -169,64 +243,81 @@ send_via_ntp_signd(
 	 * implementation might want multiple packets
 	 * awaiting signing */
 
-	samba_pkt.packet_id = 1;
+	samba_pkt.packet_id = signd_count++;
 
 	/* Swap the byte order back - it's actually little
 	 * endian on the wire, but it was read above as
 	 * network byte order */
-	samba_pkt.key_id_le = htonl(xkeyid);
+	samba_pkt.key_id_le = htonl(rbufp->keyid);
 	memcpy(&samba_pkt.pkt, xpkt, sizeof(samba_pkt.pkt));
 
 	snprintf(full_socket, sizeof(full_socket), "%s/socket", ntp_signd_socket);
 
 	fd = ux_socket_connect(full_socket);
 	/* Only continue with this if we can talk to Samba */
-	if (fd != -1) {
-		/* Send old packet to Samba, expect response */
-		/* Packet to Samba is quite simple:
-		   All values BIG endian except key ID as noted
-		   [packet size as BE] - 4 bytes
-		   [protocol version (0)] - 4 bytes
-		   [packet ID] - 4 bytes
-		   [operation (sign message=0)] - 4 bytes
-		   [key id] - LITTLE endian (as on wire) - 4 bytes
-		   [message to sign] - as marshalled, without signature
-		*/
-
-		if (send_packet(fd, (char *)&samba_pkt, offsetof(struct samba_key_in, pkt) + LEN_PKT_NOMAC) != 0) {
-			/* Huh?  could not talk to Samba... */
-			close(fd);
-			return;
-		}
+	if (fd < 4) {
+		goto signd_cleanup;
+	}
+	
+	/* Send old packet to Samba, expect response */
+	/* Packet to Samba is quite simple:
+	   All values BIG endian except key ID as noted
+	   [packet size as BE] - 4 bytes
+	   [protocol version (0)] - 4 bytes
+	   [packet ID] - 4 bytes
+	   [operation (sign message=0)] - 4 bytes
+	   [key id] - LITTLE endian (as on wire) - 4 bytes
+	   [message to sign] - as marshalled, without signature
+	*/
+
+	if (send_packet(fd, (char *)&samba_pkt, offsetof(struct samba_key_in, pkt) + LEN_PKT_NOMAC) != 0) {
+		/* Huh?  could not talk to Samba... */
+		goto signd_cleanup;
+	}
 
-		if (recv_packet(fd, &reply, &reply_len) != 0) {
-			close(fd);
-			return;
+	if (recv_packet(fd, &reply, &reply_len) != 0) {
+		goto signd_cleanup;
+	}
+	/* Return packet is also simple:
+	   [packet size] - network byte order - 4 bytes
+	   [protocol version (0)] network byte order - - 4 bytes
+	   [operation (signed success=3, failure=4)] network byte order - - 4 byte
+	   (optional) [signed message] - as provided before, with signature appended
+	*/
+
+	{
+		uint32_t op_reply = 0;
+		memcpy(&samba_reply, reply, header_length > reply_len ? reply_len: header_length);
+		op_reply = ntohl(samba_reply.op);
+		if (reply_len < header_length) {
+			maybe_log(&do_we_log_signd, LOG_INFO,
+				  "SIGND: bad samba reply length %u <= %u reply header length.\n",
+				  reply_len, header_length);
+			quit = true;
 		}
-		/* Return packet is also simple:
-		   [packet size] - network byte order - 4 bytes
-		   [protocol version (0)] network byte order - - 4 bytes
-		   [operation (signed success=3, failure=4)] network byte order - - 4 byte
-		   (optional) [signed message] - as provided before, with signature appended
-		*/
-
-		if (reply_len <= sizeof(samba_reply)) {
-			memcpy(&samba_reply, reply, reply_len);
-			if (ntohl(samba_reply.op) == 3 && reply_len >  offsetof(struct samba_key_out, pkt)) {
-				sendlen = reply_len - offsetof(struct samba_key_out, pkt);
-				xpkt = &samba_reply.pkt;
-				sendpkt(&rbufp->recv_srcadr, rbufp->dstadr, xpkt, sendlen);
-				DPRINT(1, ("transmit ntp_signd packet: at %u %s->%s keyid %08x len %d\n",
-					   current_time, socktoa(&rbufp->dstadr->sin),
-					   socktoa(&rbufp->recv_srcadr), xkeyid, sendlen));
-			}
+		if (3 != op_reply) {
+			maybe_log(&do_we_log_signd, LOG_INFO,
+				   "SIGND: bad Samba repy op want 3, got %u.\n",
+				   op_reply);
+			quit = true;
 		}
-
-		if (reply) {
-			free(reply);
+		if (quit) {
+			goto signd_cleanup;
 		}
-		close(fd);
+	}
 
+	sendlen = reply_len - header_length;
+	xpkt = reply + header_length;
+	sendpkt(&rbufp->recv_srcadr, rbufp->dstadr, xpkt, (uint32_t)sendlen);
+	DPRINT(1, ("transmit ntp_signd packet: at %u %s->%s keyid %08x len %zu\n",
+		   current_time, socktoa(&rbufp->dstadr->sin),
+		   socktoa(&rbufp->recv_srcadr), rbufp->keyid, sendlen));
+
+signd_cleanup:
+	if (reply) {
+		free(reply);
+		reply = NULL;
 	}
+	close(fd);
 }
 #endif



View it on GitLab: https://gitlab.com/NTPsec/ntpsec/-/commit/bd34d905858b626822f52fde9a6c5cdf252e27c3

-- 
View it on GitLab: https://gitlab.com/NTPsec/ntpsec/-/commit/bd34d905858b626822f52fde9a6c5cdf252e27c3
You're receiving this email because of your account on gitlab.com.


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.ntpsec.org/pipermail/vc/attachments/20231021/f328032a/attachment-0001.htm>


More information about the vc mailing list