[Git][NTPsec/ntpsec][master] 6 commits: Minor cleanup to sendpkt in ntp_io

Hal Murray gitlab at mg.gitlab.com
Mon Feb 18 23:31:17 UTC 2019


Hal Murray pushed to branch master at NTPsec / ntpsec


Commits:
ba81f773 by Hal Murray at 2019-02-18T23:30:50Z
Minor cleanup to sendpkt in ntp_io

- - - - -
cd445f20 by Hal Murray at 2019-02-18T23:30:50Z
change type of pkt.exten from uint32 to uint8

- - - - -
756e09ab by Hal Murray at 2019-02-18T23:30:50Z
First cut at adding NTS to request packets

- - - - -
c05c4f1d by Hal Murray at 2019-02-18T23:30:50Z
Cleanup several space-tabs

- - - - -
a3c8847b by Hal Murray at 2019-02-18T23:30:50Z
Update TODO-NTS

- - - - -
6bbb8ece by Hal Murray at 2019-02-18T23:30:50Z
Request side of NTS seems to be working.

- - - - -


12 changed files:

- devel/TODO-NTS
- include/ntp.h
- include/ntpd.h
- include/nts.h
- include/recvbuff.h
- + ntpd/ntp_extens.c
- ntpd/ntp_io.c
- ntpd/ntp_proto.c
- ntpd/nts.c
- ntpd/nts_client.c
- ntpd/nts_cookie.c
- ntpd/wscript


Changes:

=====================================
devel/TODO-NTS
=====================================
@@ -1,10 +1,11 @@
 flag for require NTS
 
-security level
-
+make_cookie needs to be thread safe
 multithread msyslog
 
-fixup seccomp
+security level
+
+fix seccomp
 
 show NTS flag via ntpq
   extra credit if we can find a place on the peers display


=====================================
include/ntp.h
=====================================
@@ -255,7 +255,7 @@ struct peer {
 	uint8_t	cast_flags;	/* additional flags */
 	uint8_t	last_event;	/* last peer error code */
 	uint8_t	num_events;	/* number of error events */
-	struct ntsstate_t nts_state;	/* per-peer Network Time Security state */
+	struct ntsclient_t nts_state;	/* per-peer NTS state */
 
 	/*
 	 * Variables used by reference clock support
@@ -413,14 +413,6 @@ struct parsed_pkt {
         uint64_t org;
         uint64_t rec;
         uint64_t xmt;
-        unsigned num_extensions;
-        struct exten *extensions;
-};
-
-struct exten {
-        uint16_t type;
-        uint16_t len;
-        uint8_t *body;
 };
 
 /* This is the old, insane way of representing packets. It'll gradually
@@ -450,7 +442,7 @@ struct pkt {
 #define MIN_MAC_LEN	(1 * sizeof(uint32_t))	/* crypto_NAK */
 #define	MAX_MAC_LEN	(6 * sizeof(uint32_t))	/* MAX of old style */
 
-	uint32_t	exten[(MAX_MAC_LEN + MAX_EXT_LEN) / sizeof(uint32_t)];
+	uint8_t	exten[MAX_MAC_LEN + MAX_EXT_LEN];
 } __attribute__ ((aligned));
 
 /* pythonize-header: stop ignoring */


=====================================
include/ntpd.h
=====================================
@@ -93,21 +93,21 @@ extern	endpt *	select_peerinterface	(struct peer *, sockaddr_u *,
 extern	endpt *	findinterface		(sockaddr_u *);
 extern	void	interface_update	(interface_receiver_t, void *);
 extern  void    io_handler              (void);
-extern	void	init_io 	(void);
+extern	void	init_io		(void);
 extern	void	io_open_sockets	(void);
 extern	void	io_clr_stats	(void);
-extern	void	sendpkt 	(sockaddr_u *, endpt *, void *, int);
+extern	void	sendpkt		(sockaddr_u *, endpt *, void *, unsigned int);
 extern const char * latoa(endpt *);
 
 /* ntp_loopfilter.c */
 extern	void	init_loopfilter(void);
-extern	int 	local_clock(struct peer *, double);
+extern	int	local_clock(struct peer *, double);
 extern	void	adj_host_clock(void);
 extern	void	loop_config(int, double);
 extern	void	select_loop(int);
 extern	void	huffpuff(void);
 extern	unsigned int	sys_tai;
-extern 	int	freq_cnt;
+extern	int	freq_cnt;
 
 /* ntp_monitor.c */
 #define MON_HASH_SIZE		(1U << mon_data.mon_hash_bits)
@@ -142,7 +142,7 @@ extern	void	peer_cleanup	(void);
 
 /* ntp_proto.c */
 extern	void	transmit	(struct peer *);
-extern	void	receive 	(struct recvbuf *);
+extern	void	receive		(struct recvbuf *);
 extern	void	peer_clear	(struct peer *, const char *, const bool);
 extern	void	set_sys_leap	(uint8_t);
 
@@ -191,6 +191,21 @@ extern	void	record_raw_stats (struct peer *,
 				  int ppoll, int precision, double root_delay,
 				  double root_dispersion, refid_t refid,
 				  unsigned int outcount);
+extern void record_ref_stats(
+    const struct peer *peer,
+    int     n,              /* Number of samples */
+    int     i,              /* Index of first sample used */
+    int     j,              /* Index of last sample used */
+    double  t1,             /* Value of first sample */
+    double  t2,             /* Value of first sample used */
+    double  t3,             /* answer/median */
+    double  t4,             /* Value of last sample used */
+    double  t5,             /* Value of last sample */
+    double  jitter,
+    double  std_dev,        /* std deviation of trimmed subset */
+    double  std_dev_all     /* std deviation of everything */
+    );
+
 extern	void	check_leap_file	(bool is_daily_check, time_t systime);
 
 /* packetstamp.c */
@@ -229,7 +244,7 @@ struct packet_counters {
    uint64_t packets_ignored;	/* received on wild card interface */
    uint64_t packets_received;	/* total number of packets received */
    uint64_t packets_sent;		/* total number of packets sent */
-   uint64_t packets_notsent; 	/* total number of packets which couldn't be sent */
+   uint64_t packets_notsent;	/* total number of packets which couldn't be sent */
   /* There used to be a signal handler for received packets. */
   /* It's not needed now that the kernel time stamps packets. */
   uint64_t handler_calls;	/* number of calls to interrupt handler */
@@ -306,7 +321,7 @@ struct monitor_data {
     uint64_t	mru_mindepth;		/* preempt above this */
     int	mru_maxage;		/* recycle if older than this */
     int	mru_minage;		/* recycle if older than this & full */
-    uint64_t	mru_maxdepth; 		/* MRU size hard limit */
+    uint64_t	mru_maxdepth;		/* MRU size hard limit */
     uint64_t	mru_exists;		/* slot already exists */
     uint64_t	mru_new;		/* allocated new slot */
     uint64_t	mru_recycleold;		/* recycle: age > maxage */
@@ -356,7 +371,7 @@ struct statistics_counters {
     uptime_t	sys_stattime;		/* time since sysstats reset */
     uint64_t	sys_received;		/* packets received */
     uint64_t	sys_processed;		/* packets for this host */
-    uint64_t	sys_restricted;	 	/* restricted packets */
+    uint64_t	sys_restricted;		/* restricted packets */
     uint64_t	sys_newversion;		/* current version  */
     uint64_t	sys_oldversion;		/* old version */
     uint64_t	sys_badlength;		/* bad length or format */
@@ -420,16 +435,20 @@ extern struct refclock * const refclock_conf[];
 extern const uint8_t	num_refclock_conf;
 #endif
 
+/* ntd_extens.c */
+int extens_client_send(struct peer *peer, struct pkt *xpkt);
+bool extens_server_recv(struct ntspacket_t *ntspacket, uint8_t *pkt, int lng);
+
 /* nts.c */
 void nts_init(void);
 bool nts_probe(struct peer *peer);
 int nts_client_ke_request(struct ntscfg_t *);
 int nts_server_ke_verify(struct ntscfg_t *);
-int nts_client_ke_verify(struct ntscfg_t *, struct ntsstate_t *);
+int nts_client_ke_verify(struct ntscfg_t *, struct ntsclient_t *);
 int nts_daily(struct ntscfg_t *);
-int nts_validate(const struct ntscfg_t *, struct ntsstate_t *,
+int nts_validate(const struct ntscfg_t *, struct ntsclient_t *,
 		 struct parsed_pkt *);
-int nts_decorate(const struct ntscfg_t *, struct ntsstate_t *,
-		 uint32_t *, size_t);
+int nts_decorate(const struct ntscfg_t *, struct ntsclient_t *,
+		 uint8_t *, size_t);
 
 #endif	/* GUARD_NTPD_H */


=====================================
include/nts.h
=====================================
@@ -9,6 +9,8 @@
 #define NTS_MAX_KEYLEN		64	/* used in cookies */
 #define NTS_MAX_COOKIELEN	192	/* see nts_cookie.c */
 #define NTS_MAX_COOKIES		8	/* RFC 4.1.6 */
+#define NTS_UID_LENGTH		32	/* RFC 5.3 */
+#define NTS_UID_MAX_LENGTH	64
 
 #define FLAG_NTS	0x01u	/* use NTS (network time security) */
 #define FLAG_NTS_ASK	0x02u	/* NTS, ask for specified server */
@@ -33,17 +35,34 @@ struct ntscfg_t {
 #define AEAD_AES_SIV_CMAC_512_KEYLEN 64
 
 /* Client-side state per connection to server */
-struct ntsstate_t {
-    int aead;
+struct ntsclient_t {
+    /* wire connection */
+    int aead;   /* AEAD algorithm used on wire */
     int keylen;
-    int next_cookie;
-    int cookie_count;
-    int cookie_length;
+    uint8_t c2s[NTS_MAX_KEYLEN], s2c[NTS_MAX_KEYLEN];
+    /* UID of last request sent - RFC 5.3 */
+    uint8_t UID[NTS_UID_LENGTH];
+    /* cookies */
+    int readIdx, writeIdx;
+    int count;
+    int cookielen;
     bool valid[NTS_MAX_COOKIES];
     uint8_t cookies[NTS_MAX_COOKIES][NTS_MAX_COOKIELEN];
+};
+
+/* Server-side state per packet */
+struct ntspacket_t {
+    bool valid;
+    int extra;
+    int uidlen;
+    uint8_t UID[NTS_UID_MAX_LENGTH];
+    int keylen;
     uint8_t c2s[NTS_MAX_KEYLEN], s2c[NTS_MAX_KEYLEN];
+    int aead;
+    uint8_t cookie[NTS_MAX_COOKIELEN];
 };
 
+
 /* Configuration data for an NTS server or client instance */
 struct ntsconfig_t {
     bool ntsenable; 		/* enable NTS KE server on this ntpd */


=====================================
include/recvbuff.h
=====================================
@@ -4,6 +4,7 @@
 #include "ntp.h"
 #include "ntp_net.h"
 #include "ntp_lists.h"
+#include "nts.h"
 
 /*
  * recvbuf memory management
@@ -46,6 +47,8 @@ struct recvbuf {
 	bool keyid_present;
 	keyid_t keyid;
 	int mac_len;
+	bool extens_present;
+	struct ntspacket_t ntspacket;
 #ifdef REFCLOCK
 	struct peer *	recv_peer;
 #endif /* REFCLOCK */


=====================================
ntpd/ntp_extens.c
=====================================
@@ -0,0 +1,210 @@
+/*
+ * ntp_extens.c - Network Time Protocol (NTP) extension processing
+ *
+ * NB: This module is working with the wire format packet.
+ *     It must do byte swapping.
+ *
+ * We carefully arrange things so that no padding is necessary.
+ *
+ */
+
+#include "config.h"
+
+#include <stdbool.h>
+#include <stdint.h>
+#include <string.h>
+
+#include <openssl/rand.h>
+#include <aes_siv.h>
+
+#include "ntp_stdlib.h"
+#include "ntp.h"
+#include "ntpd.h"
+#include "nts.h"
+
+// FIXME Duplicated in nts_cookie
+#define NONCE_LENGTH 16
+#define CMAC_LENGTH 16
+
+#define NTP_EX_HDR_LNG 4
+#define NTP_EX_U16_LNG 2
+
+
+
+enum NtpExtFieldType {
+   Unique_Identifier = 10,
+   NTS_Cookie = 11,
+   NTS_Cookie_Placeholder = 12,
+   NTS_AEEF = 13 /* Authenticated and Encrypted Extension Fields */
+};
+
+AES_SIV_CTX* wire_ctx = NULL;  /* need one per thread */
+
+
+int extens_client_send(struct peer *peer, struct pkt *xpkt) {
+  struct BufCtl_t buf;
+  int used, adlength, idx;
+  size_t left;
+  uint8_t *nonce, *packet;
+  bool ok;
+
+  // FIXME - need init routine
+  if (NULL == wire_ctx) {
+    wire_ctx = AES_SIV_CTX_new();
+    if (NULL == wire_ctx) {
+      msyslog(LOG_ERR, "NTS: Can't init wire_ctx");
+      exit(1);
+    }
+  }
+
+  packet = (uint8_t*)xpkt;
+  buf.next = xpkt->exten;
+  buf.left = MAX_EXT_LEN;
+
+  /* UID */
+  RAND_bytes(peer->nts_state.UID, NTS_UID_LENGTH);
+  nts_append_record_bytes(&buf, Unique_Identifier,
+      peer->nts_state.UID, NTS_UID_LENGTH);
+
+  /* cookie */
+  idx = peer->nts_state.readIdx++;
+  nts_append_record_bytes(&buf, NTS_Cookie,
+      peer->nts_state.cookies[idx], peer->nts_state.cookielen);
+  peer->nts_state.readIdx = peer->nts_state.readIdx % NTS_MAX_COOKIES;
+  peer->nts_state.count--;
+  // FIXME - what to do if out of cookies
+
+  // FIXME - need more cookies?
+
+  /* AEAD */
+  adlength = buf.next-packet;
+  nts_append_header(&buf, NTS_AEEF, NTP_EX_U16_LNG*2+NONCE_LENGTH+CMAC_LENGTH);
+  nts_append_uint16(&buf, NONCE_LENGTH);
+  nts_append_uint16(&buf, CMAC_LENGTH);
+  nonce = buf.next;
+  RAND_bytes(nonce, NONCE_LENGTH);
+  buf.next += NONCE_LENGTH;
+  buf.left -= NONCE_LENGTH;
+  left = buf.left;
+  ok = AES_SIV_Encrypt(wire_ctx,
+           buf.next, &left,   /* left: in: max out length, out: length used */
+           peer->nts_state.c2s, peer->nts_state.keylen,
+           nonce, NONCE_LENGTH,
+           NULL, 0,           /* no plain/cipher text */
+           packet, adlength);
+  if (!ok) {
+    msyslog(LOG_ERR, "NTS: extens_client_send - Error from AES_SIV_Encrypt");
+    /* I don't think this should happen,
+     * so crash rather than work incorrectly.
+     * Hal, 2019-Feb-17
+     * Similar code in nts_cookie
+     */
+    exit(1);
+  }
+  buf.next += left;
+  buf.left -= left;
+
+  used = buf.next-xpkt->exten;
+  return used;
+}
+
+bool extens_server_recv(struct ntspacket_t *ntspacket, uint8_t *pkt, int lng) {
+  struct BufCtl_t buf;
+  uint16_t aead;
+  int noncelen, cmaclen;
+
+  // FIXME - need init routine
+  if (NULL == wire_ctx) {
+    wire_ctx = AES_SIV_CTX_new();
+    if (NULL == wire_ctx) {
+      msyslog(LOG_ERR, "NTS: Can't init wire_ctx");
+      exit(1);
+    }
+  }
+
+  buf.next = pkt+LEN_PKT_NOMAC;
+  buf.left = lng-LEN_PKT_NOMAC;
+
+  while (buf.left > 0) {
+    uint16_t type;
+    bool critical = false;
+    int length, adlength;
+    size_t outlen;
+    uint8_t *nonce, *cmac;
+    bool ok;
+
+    type = nts_next_record(&buf, &length);
+    if (length > buf.left || length < 0)
+      return false;
+    if (NTS_CRITICAL & type) {
+      critical = true;
+      type &= ~NTS_CRITICAL;
+    }
+    switch (type) {
+      case Unique_Identifier:
+	if (length > NTS_UID_MAX_LENGTH)
+	  return false;
+        ntspacket->uidlen = length;
+        nts_next_bytes(&buf, ntspacket->UID, length);
+	break;
+      case NTS_Cookie:
+        ok = nts_unpack_cookie(buf.next, length,
+            &aead,
+	    ntspacket->c2s, ntspacket->s2c, &ntspacket->keylen);
+	if (!ok)
+	  return false;
+        buf.next += length;
+	buf.left -= length;
+	ntspacket->valid = true;
+	ntspacket->aead = aead;
+	break;
+      case NTS_Cookie_Placeholder:
+        ntspacket->extra++;
+        buf.next += length;
+	buf.left -= length;
+        break;
+      case NTS_AEEF:
+        if (!ntspacket->valid)
+	  return false;			/* no cookie yet, no c2s */
+        if (length != NTP_EX_HDR_LNG+NONCE_LENGTH+CMAC_LENGTH)
+	  return false;
+        /* Additional data is up to this exten. */
+	/* backup over header */
+        adlength = buf.next-NTP_EX_HDR_LNG-pkt;
+        noncelen = nts_next_uint16(&buf);
+        cmaclen = nts_next_uint16(&buf);
+	if (noncelen & 3)
+          return false;			/* would require padding */
+	if (CMAC_LENGTH != cmaclen)
+          return false;
+        nonce = buf.next;
+	cmac = nonce+NONCE_LENGTH;
+	outlen = 6;
+        ok = AES_SIV_Decrypt(wire_ctx,
+            NULL, &outlen,
+            ntspacket->c2s, ntspacket->keylen,
+            nonce, noncelen,
+            cmac, CMAC_LENGTH,
+            pkt, adlength);
+	if (!ok)
+	  return false;
+	if (0 != outlen)
+	  return false;
+        buf.next += length;
+	buf.left -= length;
+        break;
+      default:
+        if (critical)
+          return false;
+        buf.next += length;
+	buf.left -= length;
+    }
+  }
+
+  if (!ntspacket->valid)
+    return false;
+
+  return true;
+}
+
+/* end */


=====================================
ntpd/ntp_io.c
=====================================
@@ -1992,22 +1992,23 @@ open_socket(
 
 
 /*
- * sendpkt - send a packet to the specified destination. Maintain a
- * send error cache so that only the first consecutive error for a
- * destination is logged.
+ * sendpkt - send a packet to the specified destination.
  */
 void
 sendpkt(
 	sockaddr_u *		dest,
-	endpt *	ep,
+	endpt *			src,
 	void *			pkt,
-	int			len
+	unsigned int		len
 	)
 {
-	endpt *	src;
 	ssize_t	cc;
 
-	src = ep;
+	if (len > sizeof(struct pkt)) {
+		msyslog(LOG_ERR, "Err: sendpkt - buffer overflow %u", len);
+		exit(1);
+	}
+
 	if (NULL == src) {
 		/*
 		 * unbound peer - drop request and wait for better


=====================================
ntpd/ntp_proto.c
=====================================
@@ -216,25 +216,6 @@ is_control_packet(
 	    PKT_MODE(rbufp->recv_buffer[0]) == MODE_CONTROL;
 }
 
-/* There used to be a calloc/free for each received packet.
-   Now, the parse_pkt version lives in a recvbuf.
-   The alloc/free only happens for extensions and we don't support
-   any of them.
-*/
-static void
-free_extens(
-	struct parsed_pkt *pkt
-	)
-{
-	if(pkt->extensions != NULL) {
-		for(size_t i = 0; i < pkt->num_extensions; i++) {
-			free(pkt->extensions[i].body);
-			pkt->extensions[i].body = NULL;
-		}
-		free(pkt->extensions);
-		pkt->extensions = NULL;
-	}
-}
 
 static bool
 parse_packet(
@@ -251,12 +232,6 @@ parse_packet(
 		return false;
 	}
 
-	if(recv_length > sizeof(struct pkt)) {
-		/* Data is too long to be punned into the wire-packet struct. */
-		/* If this happens it will be due to too much extension data. */
-		return false;
-	}
-
 	struct parsed_pkt * pkt = &rbufp->pkt;
 	uint8_t const* bufptr = recv_buf + LEN_PKT_NOMAC;
 
@@ -273,53 +248,24 @@ parse_packet(
 	pkt->rec = ntp_be64dec(recv_buf + 32);
 	pkt->xmt = ntp_be64dec(recv_buf + 40);
 
-	/* Make sure these are clean before we might bail. */
-        pkt->num_extensions = 0;
-	pkt->extensions = NULL;
-
 	rbufp->keyid_present = false;
 	rbufp->keyid = 0;
 	rbufp->mac_len = 0;
 
+	rbufp->extens_present = false;
+	rbufp->ntspacket.valid = false;
+	rbufp->ntspacket.extra = 0;
+	rbufp->ntspacket.uidlen = 0;
+
 	if(PKT_VERSION(pkt->li_vn_mode) > 4) {
 		/* Unsupported version */
-		goto fail;
+		return false;
 	} else if(PKT_VERSION(pkt->li_vn_mode) == 4) {
 		/* Only version 4 packets support extensions. */
-
-		/* Count and validate extensions */
-		size_t ext_count = 0;
-		size_t extlen = 0;
-		while(bufptr <= recv_buf + recv_length - 28) {
-			extlen = ntp_be16dec(bufptr + 2);
-			if(extlen % 4 != 0 || extlen < 16) {
-				/* Illegal extension length */
-				goto fail;
-			}
-			if((size_t)(recv_buf + recv_length - bufptr) < extlen) {
-				/* Extension length field points past
-				 * end of packet */
-				goto fail;
-			}
-			bufptr += extlen;
-			ext_count++;
-		}
-
-		pkt->num_extensions = (unsigned int)ext_count;
-		pkt->extensions = calloc(ext_count, sizeof (struct exten));
-		if(pkt->extensions == NULL) { goto fail; }
-
-		/* Copy extensions */
-		bufptr = recv_buf + LEN_PKT_NOMAC;
-		for(size_t i = 0; i < ext_count; i++) {
-			pkt->extensions[i].type = ntp_be16dec(bufptr);
-			pkt->extensions[i].len = ntp_be16dec(bufptr + 2) - 4;
-			pkt->extensions[i].body =
-			    calloc(1, pkt->extensions[i].len);
-			if(pkt->extensions[i].body == NULL) { goto fail; }
-			memcpy(pkt->extensions[i].body, bufptr + 4,
-			       pkt->extensions[i].len);
-			bufptr += pkt->extensions[i].len + 4;
+		/* But they also support shared key authentication. */
+		if (recv_length > (LEN_PKT_NOMAC+MAX_MAC_LEN)) {
+			rbufp->extens_present = true;
+			return true;
 		}
 	}
 
@@ -335,7 +281,7 @@ parse_packet(
 		/* crypto-NAK */
 		if(PKT_VERSION(pkt->li_vn_mode) < 3) {
 			/* Only allowed as of NTPv3 */
-			goto fail;
+			return false;
 		}
 		rbufp->keyid_present = true;
 		rbufp->keyid = ntp_be32dec(bufptr);
@@ -344,7 +290,7 @@ parse_packet(
 	    case 6:
 		/* NTPv2 authenticator, which we allow but strip because
 		   we don't support it any more */
-		if(PKT_VERSION(pkt->li_vn_mode) != 2) { goto fail; }
+		if(PKT_VERSION(pkt->li_vn_mode) != 2) { return false; }
 		rbufp->keyid_present = false;
 		rbufp->keyid = 0;
 		rbufp->mac_len = 0;
@@ -353,7 +299,7 @@ parse_packet(
 		/* AES-128 CMAC, MD5 digest */
 		if(PKT_VERSION(pkt->li_vn_mode) < 3) {
 			/* Only allowed as of NTPv3 */
-			goto fail;
+			return false;
 		}
 		rbufp->keyid_present = true;
 		rbufp->keyid = ntp_be32dec(bufptr);
@@ -363,7 +309,7 @@ parse_packet(
 		/* SHA-1 digest */
 		if(PKT_VERSION(pkt->li_vn_mode) < 3) {
 			/* Only allowed as of NTPv3 */
-			goto fail;
+			return false;
 		}
 		rbufp->keyid_present = true;
 		rbufp->keyid = ntp_be32dec(bufptr);
@@ -373,7 +319,7 @@ parse_packet(
 		/* MS-SNTP */
 		if(PKT_VERSION(pkt->li_vn_mode) != 3) {
 			/* Only allowed for NTPv3 */
-			goto fail;
+			return false;
 		}
 
 		/* We don't deal with the MS-SNTP fields, so just strip
@@ -386,13 +332,10 @@ parse_packet(
 		break;
 	    default:
 		/* Any other length is illegal */
-		goto fail;
+		return false;
 	}
 
 	return true;
-  fail:
-	free_extens(&rbufp->pkt);
-	return false;
 }
 
 /* Returns true if we should not accept any unauthenticated packets from
@@ -656,7 +599,7 @@ receive(
 
 	if(!is_vn_mode_acceptable(rbufp)) {
 		stat_count.sys_badlength++;
-		goto done;
+		return;
 	}
 
 	/* FIXME: This is lots more cleanup to do in this area. */
@@ -665,19 +608,19 @@ receive(
 
 	if(check_early_restrictions(rbufp, restrict_mask)) {
 		stat_count.sys_restricted++;
-		goto done;
+		return;
 	}
 
 	restrict_mask = ntp_monitor(rbufp, restrict_mask);
 	if (restrict_mask & RES_LIMITED) {
 		stat_count.sys_limitrejected++;
-		if(!(restrict_mask & RES_KOD)) { goto done; }
+		if(!(restrict_mask & RES_KOD)) { return; }
 	}
 
 	if(is_control_packet(rbufp)) {
 		process_control(rbufp, restrict_mask);
 		stat_count.sys_processed++;
-		goto done;
+		return;
 	}
 
 	/*
@@ -693,13 +636,13 @@ receive(
 		stat_count.sys_oldversion++;		/* previous version */
 	} else {
 		stat_count.sys_badlength++;
-		goto done;			/* old version */
+		return;			/* old version */
 	}
 	}
 
 	if (!parse_packet(rbufp)) {
 		stat_count.sys_badlength++;
-		goto done;
+		return;
 	}
 
 	if (MODE_SERVER == PKT_MODE(rbufp->pkt.li_vn_mode)) {
@@ -710,7 +653,7 @@ receive(
 	    peer = findpeer(rbufp);
 	    if (NULL == peer) {
 		stat_count.sys_declined++;
-		goto done;
+		return;
 	    }
 	}
 
@@ -748,7 +691,7 @@ receive(
 				peer->cfg.flags &= ~FLAG_AUTHENTIC;
 				peer->flash |= BOGON5;
 			}
-			goto done;
+			return;
 		}
 	}
 
@@ -761,9 +704,12 @@ receive(
 	switch (PKT_MODE(rbufp->pkt.li_vn_mode)) {
 	    case MODE_ACTIVE:  /* remote site using "peer" in config file */
 	    case MODE_CLIENT:  /* Request for us as a server. */
-		if (nts_validate(NULL, NULL, &rbufp->pkt) != 0) {
+		if (rbufp->extens_present) {
+		    if (!extens_server_recv(&rbufp->ntspacket,
+			  rbufp->recv_buffer, rbufp->recv_length)) {
 			stat_count.sys_declined++;
 			break;
+		    }
 		}
 		handle_fastxmit(rbufp, restrict_mask, auth);
 		stat_count.sys_processed++;
@@ -787,8 +733,6 @@ receive(
 		break;
 	}
 
-  done:
-	free_extens(&rbufp->pkt);
 }
 
 /*
@@ -2108,17 +2052,17 @@ peer_xmit(
 		peer->org_rand = peer->org_ts;
 	}
 
+	/* Maybe we should bump org_ts to compensate for crypto */
 	xpkt.xmt = htonl_fp(peer->org_rand);	/* out in xmt, back in org */
 
-
-	sendlen += nts_decorate(&peer->cfg.nts_cfg, &peer->nts_state,
-				xpkt.exten, sizeof(xpkt.exten));
-
-	/*
-	 * If the peer (aka server) was configured with a key, authenticate
-	 * the packet.  Else, the packet is not authenticated.
+	/* 3 way branch to add authentication:
+         *  1) NTS
+         *  2) Shared KEY
+         *  3) none
 	 */
-	if (0 != peer->cfg.peerkey) {
+	if (0 < peer->nts_state.count) {
+		sendlen += extens_client_send(peer, &xpkt);
+        } else if (0 != peer->cfg.peerkey) {
 		auth_info *auth = authlookup(peer->cfg.peerkey, true);
 		if (NULL == auth) {
 			report_event(PEVNT_AUTH, peer, "no key");
@@ -2126,12 +2070,7 @@ peer_xmit(
 			peer->badauth++;
 			return;
 		}
-		/* Maybe bump peer->org_ts to account for crypto time */
 		sendlen += authencrypt(auth, (uint32_t *)&xpkt, sendlen);
-		if (sendlen > sizeof(xpkt)) {
-			msyslog(LOG_ERR, "PROTO: buffer overflow %u", sendlen);
-			exit(1);
-		}
 	}
 
 	sendpkt(&peer->srcadr, peer->dstadr, &xpkt, sendlen);


=====================================
ntpd/nts.c
=====================================
@@ -75,7 +75,7 @@ int nts_server_ke_verify(struct ntscfg_t *cfg)
  * - Verify server response message
  * - Extract cookie(s).
  */
-int nts_client_ke_verify(struct ntscfg_t *cfg, struct ntsstate_t *state)
+int nts_client_ke_verify(struct ntscfg_t *cfg, struct ntsclient_t *state)
 {
 	UNUSED_ARG(cfg);
 	UNUSED_ARG(state);
@@ -100,7 +100,7 @@ int nts_daily(struct ntscfg_t *cfg)
  * there is no per-client server state.  A nonzero return causes the
  * packet to be discarded.
  */
-int nts_validate(const struct ntscfg_t *cfg, struct ntsstate_t *state,
+int nts_validate(const struct ntscfg_t *cfg, struct ntsclient_t *state,
 		 struct parsed_pkt *pkt)
 {
 	UNUSED_ARG(cfg);
@@ -115,8 +115,8 @@ int nts_validate(const struct ntscfg_t *cfg, struct ntsstate_t *state,
  * the ntscfg and state pointers are expected to be NULL as there
  * is no per-client server state.  Return the count of words appended.
  */
-int nts_decorate(const struct ntscfg_t *cfg, struct ntsstate_t *state,
-		 uint32_t *extdata, size_t extlen)
+int nts_decorate(const struct ntscfg_t *cfg, struct ntsclient_t *state,
+		 uint8_t *extdata, size_t extlen)
 {
 	UNUSED_ARG(cfg);
 	UNUSED_ARG(extdata);
@@ -142,7 +142,7 @@ void nts_log_ssl_error(void) {
 
 // 2 byte type, 2 byte length
 #define NTS_KE_HDR_LNG 4
-#define NTS_KE_DATA2_LNG 2
+#define NTS_KE_U16_LNG 2
 
 /* Troubles with signed/unsigned compares when using sizeof() */
 
@@ -151,9 +151,9 @@ void nts_append_record_null(BufCtl* buf, uint16_t type) {
 }
 
 void nts_append_record_uint16(BufCtl* buf, uint16_t type, uint16_t data) {
-  if (NTS_KE_HDR_LNG+NTS_KE_DATA2_LNG > buf->left)
+  if (NTS_KE_HDR_LNG+NTS_KE_U16_LNG > buf->left)
     return;
-  nts_append_header(buf, type, NTS_KE_DATA2_LNG);
+  nts_append_header(buf, type, NTS_KE_U16_LNG);
   nts_append_uint16(buf, data);
 }
 
@@ -177,11 +177,11 @@ void nts_append_header(BufCtl* buf, uint16_t type, uint16_t length) {
 
 void nts_append_uint16(BufCtl* buf, uint16_t data) {
   uint16_t * ptr = (uint16_t *)buf->next;
-  if (NTS_KE_DATA2_LNG > buf->left)
+  if (NTS_KE_U16_LNG > buf->left)
     return;
   *ptr++ = htons(data);
-  buf->next += NTS_KE_DATA2_LNG;
-  buf->left -= NTS_KE_DATA2_LNG;
+  buf->next += NTS_KE_U16_LNG;
+  buf->left -= NTS_KE_U16_LNG;
 }
 
 void nts_append_bytes(BufCtl* buf, uint8_t *data, int length) {
@@ -205,8 +205,8 @@ uint16_t nts_next_record(BufCtl* buf, int *length) {
 uint16_t nts_next_uint16(BufCtl* buf) {
   uint16_t *ptr = (uint16_t *)buf->next;
   uint16_t data = ntohs(*ptr++);
-  buf->next += NTS_KE_DATA2_LNG;
-  buf->left -= NTS_KE_DATA2_LNG;
+  buf->next += NTS_KE_U16_LNG;
+  buf->left -= NTS_KE_U16_LNG;
   return data;
 }
 


=====================================
ntpd/nts_client.c
=====================================
@@ -308,8 +308,9 @@ bool nts_client_process_response(struct peer* peer, SSL *ssl) {
 
   peer->nts_state.aead = -1;
   peer->nts_state.keylen = 0;
-  peer->nts_state.next_cookie = 0;
-  peer->nts_state.cookie_count = 0;
+  peer->nts_state.writeIdx = 0;
+  peer->nts_state.readIdx = 0;
+  peer->nts_state.count = 0;
   for (int i=0; i<NTS_MAX_COOKIES; i++) peer->nts_state.valid[i] = false;
 
   buf.next = buff;
@@ -355,22 +356,22 @@ bool nts_client_process_response(struct peer* peer, SSL *ssl) {
           msyslog(LOG_ERR, "NTSc: NC cookie too big: %d", length);
           return false;
         }
-        if (0 == peer->nts_state.cookie_length)
-           peer->nts_state.cookie_length = length;
-        if (length != peer->nts_state.cookie_length) {
+        if (0 == peer->nts_state.cookielen)
+           peer->nts_state.cookielen = length;
+        if (length != peer->nts_state.cookielen) {
           msyslog(LOG_ERR, "NTSc: Cookie length mismatch %d, %d.",
-            length, peer->nts_state.cookie_length);
+            length, peer->nts_state.cookielen);
           return false;
         }
-        idx = peer->nts_state.next_cookie;
-        nts_next_bytes(&buf, (uint8_t*)&peer->nts_state.cookies[idx], length);
-        if (NTS_MAX_COOKIES <= peer->nts_state.cookie_count) {
-          msyslog(LOG_ERR, "NTSc: Extra cookie.");
+        idx = peer->nts_state.writeIdx;
+        if (NTS_MAX_COOKIES <= peer->nts_state.count) {
+          msyslog(LOG_ERR, "NTSc: Extra cookie ignored.");
           break;
         }
+        nts_next_bytes(&buf, (uint8_t*)&peer->nts_state.cookies[idx], length);
         peer->nts_state.valid[idx] = true;
-        peer->nts_state.next_cookie++;
-        peer->nts_state.cookie_count++;
+        peer->nts_state.writeIdx++;
+        peer->nts_state.count++;
         break;
       case nts_end_of_message:
         if ((0 != length) || !critical) {
@@ -399,13 +400,13 @@ bool nts_client_process_response(struct peer* peer, SSL *ssl) {
     msyslog(LOG_ERR, "NTSc: No AEAD algorithim.");
     return false;
   }
-  if (0 == peer->nts_state.cookie_count) {
+  if (0 == peer->nts_state.count) {
     msyslog(LOG_ERR, "NTSc: No cookies.");
     return false;
   }
 
   msyslog(LOG_ERR, "NTSc: Got %d cookies, length %d.",
-    peer->nts_state.cookie_count, peer->nts_state.cookie_length);
+    peer->nts_state.count, peer->nts_state.cookielen);
   return true;
 }
 


=====================================
ntpd/nts_cookie.c
=====================================
@@ -56,16 +56,22 @@
  * 168
  */
 
-/* cookies use same algorithms as wire */
+// FIXME - save K/I on disk
+// FIXME - rotate K/I occasionally
+
+/* cookies use same AEAD algorithms as wire */
 uint8_t K[NTS_MAX_KEYLEN];
 uint32_t I;
 
 AES_SIV_CTX* cookie_ctx;  /* need one per thread */
 
-/* This determines which algorithm we use. */
-/* making this a variable rather than #define
+/* This determines which algorithm we use.
+ * Valid choices are 32, 48, and 64
+ * making this a variable rather than #define
  * opens up the opportunity to pick one at run time. */
 int K_length = AEAD_AES_SIV_CMAC_256_KEYLEN;
+
+// FIXME duplicated in ntp_extens
 #define NONCE_LENGTH 16
 
 /* Associated data: aead (rounded up to 4) plus NONCE */
@@ -82,8 +88,10 @@ bool nts_cookie_init(void) {
   OK &= RAND_bytes((uint8_t *)&I, sizeof(I));
 #endif
   cookie_ctx = AES_SIV_CTX_new();
-  if (NULL == cookie_ctx)
-    OK = false;
+  if (NULL == cookie_ctx) {
+    msyslog(LOG_ERR, "NTS: Can't init cookie_ctx");
+    exit(1);
+  }
   return OK;
 }
 
@@ -138,7 +146,12 @@ int nts_make_cookie(uint8_t *cookie,
            plaintext, plainlength,
            cookie, AD_LENGTH);
   if (!ok) {
-    msyslog(LOG_ERR, "NTS: Error from AES_SIV_Encrypt");
+    msyslog(LOG_ERR, "NTS: nts_make_cookie - Error from AES_SIV_Encrypt");
+    /* I don't think this should happen,
+     * so crash rather than work incorrectly.
+     * Hal, 2019-Feb-17
+     * Similar code in ntp_extens
+     */
     exit(1);
   }
 
@@ -153,6 +166,7 @@ int nts_make_cookie(uint8_t *cookie,
 bool nts_unpack_cookie(uint8_t *cookie, int cookielen,
   uint16_t *aead,
   uint8_t *c2s, uint8_t *s2c, int *keylen) {
+
   uint8_t *finger;
   uint8_t plaintext[NTS_MAX_COOKIELEN];
   uint8_t *nonce;
@@ -161,6 +175,10 @@ bool nts_unpack_cookie(uint8_t *cookie, int cookielen,
   int cipherlength;
   bool ok;
 
+// FIXME - these are our cookies.  We should know the exact length
+  if (cookielen > NTS_MAX_COOKIELEN)
+    return false;
+
   finger = cookie;
   // FIXME should call routine to return key
   if (0 != memcmp(finger, &I, sizeof(I)))
@@ -179,7 +197,7 @@ bool nts_unpack_cookie(uint8_t *cookie, int cookielen,
            K, K_length,
            nonce, NONCE_LENGTH,
            finger, cipherlength,
-           cookie, AEAD_LENGTH);
+           cookie, AD_LENGTH);
   if (!ok)
     return false;
 


=====================================
ntpd/wscript
=====================================
@@ -50,6 +50,7 @@ def build(ctx):
 
     libntpd_source = [
         "ntp_control.c",
+        "ntp_extens.c",
         "ntp_filegen.c",
         "ntp_leapsec.c",
         "ntp_monitor.c",    # Needed by the restrict code



View it on GitLab: https://gitlab.com/NTPsec/ntpsec/compare/3e543bde0670b893432599994c77513ce80b79e0...6bbb8eceee1193a132ebbd8e55e3fc35cd3c13b5

-- 
View it on GitLab: https://gitlab.com/NTPsec/ntpsec/compare/3e543bde0670b893432599994c77513ce80b79e0...6bbb8eceee1193a132ebbd8e55e3fc35cd3c13b5
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/20190218/a6248588/attachment-0001.html>


More information about the vc mailing list