[Git][NTPsec/ntpsec][master] Update NTS cookie code to save 10 days of cookie keys.

Hal Murray (@hal.murray) gitlab at mg.gitlab.com
Sun Dec 18 06:48:34 UTC 2022



Hal Murray pushed to branch master at NTPsec / ntpsec


Commits:
87358bc9 by Hal Murray at 2022-12-17T22:42:47-08:00
Update NTS cookie code to save 10 days of cookie keys.
This will let NTS clients that only probe once a day work
without needing to hammer on the NTS-KE server.
Previous code only saved 1 old key so clients probing
once a day would have to get new keys every other day.

- - - - -


6 changed files:

- NEWS.adoc
- include/nts.h
- ntpclients/ntpq.py
- ntpd/ntp_control.c
- ntpd/nts_cookie.c
- tests/ntpd/nts_cookie.c


Changes:

=====================================
NEWS.adoc
=====================================
@@ -12,6 +12,10 @@ on user-visible changes.
 
 == Reposatory Head ==
 
+The NTS server now saves 10 days worth of cookie keys.
+  This will allow clients that only poll once a day to
+  use NTS without using NTS-KE to keep cookies up to date.
+
 rawstats now logs dropped packets and their BOGON code
   Only one per request to avoid DoSing the log file
   This lets you see packets that take too long.


=====================================
include/nts.h
=====================================
@@ -93,6 +93,17 @@ uint16_t next_bytes(BufCtl* buf, uint8_t *data, int length);
 #define NTS_UID_LENGTH		32	/* RFC 5.3 */
 #define NTS_UID_MAX_LENGTH	64
 
+/* Here for tester */
+struct NTS_Key {
+  uint8_t K[NTS_MAX_KEYLEN];
+  uint32_t I;
+  };
+#ifndef NTS_KEYS
+  #define NTS_nKEYS 10
+#endif
+extern struct NTS_Key nts_keys[NTS_nKEYS];
+extern int nts_nKeys;
+
 
 /* Client side configuration data for an NTS association
  * All are optional.
@@ -230,6 +241,8 @@ extern uint64_t nts_server_recv_bad;
 extern uint64_t nts_cookie_make;
 extern uint64_t nts_cookie_decode;
 extern uint64_t nts_cookie_decode_old;
+extern uint64_t nts_cookie_decode_old2;
+extern uint64_t nts_cookie_decode_older;
 extern uint64_t nts_cookie_decode_too_old;
 extern uint64_t nts_cookie_decode_error;
 extern uint64_t nts_ke_serves_good;


=====================================
ntpclients/ntpq.py
=====================================
@@ -1653,6 +1653,8 @@ usage: authinfo
    ("nts_cookie_make",           "NTS make cookies:          ", NTP_UINT),
    ("nts_cookie_decode",         "NTS decode cookies:        ", NTP_UINT),
    ("nts_cookie_decode_old",     "NTS decode cookies old:    ", NTP_UINT),
+   ("nts_cookie_decode_old2",    "NTS decode cookies old2:   ", NTP_UINT),
+   ("nts_cookie_decode_older",   "NTS decode cookies older:  ", NTP_UINT),
    ("nts_cookie_decode_too_old", "NTS decode cookies too old:", NTP_UINT),
    ("nts_cookie_decode_error",   "NTS decode cookies error:  ", NTP_UINT),
    ("nts_ke_probes_good",        "NTS KE client probes good: ", NTP_UINT),


=====================================
ntpd/ntp_control.c
=====================================
@@ -392,18 +392,22 @@ static const struct ctl_var sys_var[] = {
 	{ CS_nts_cookie_decode,		RO, "nts_cookie_decode" },
 #define CS_nts_cookie_decode_old	126
 	{ CS_nts_cookie_decode_old,	RO, "nts_cookie_decode_old" },
-#define CS_nts_cookie_decode_too_old	127
+#define CS_nts_cookie_decode_old2	127
+	{ CS_nts_cookie_decode_old2,	RO, "nts_cookie_decode_old2" },
+#define CS_nts_cookie_decode_older	128
+	{ CS_nts_cookie_decode_older,	RO, "nts_cookie_decode_older" },
+#define CS_nts_cookie_decode_too_old	129
 	{ CS_nts_cookie_decode_too_old,	RO, "nts_cookie_decode_too_old" },
-#define CS_nts_cookie_decode_error	128
+#define CS_nts_cookie_decode_error	130
 	{ CS_nts_cookie_decode_error,	RO, "nts_cookie_decode_error" },
 
-#define CS_nts_ke_serves_good	129
+#define CS_nts_ke_serves_good	131
 	{ CS_nts_ke_serves_good,	RO, "nts_ke_serves_good" },
-#define CS_nts_ke_serves_bad	130
+#define CS_nts_ke_serves_bad	132
 	{ CS_nts_ke_serves_bad,		RO, "nts_ke_serves_bad" },
-#define CS_nts_ke_probes_good	131
+#define CS_nts_ke_probes_good	133
 	{ CS_nts_ke_probes_good,	RO, "nts_ke_probes_good" },
-#define CS_nts_ke_probes_bad	132
+#define CS_nts_ke_probes_bad	134
 	{ CS_nts_ke_probes_bad,		RO, "nts_ke_probes_bad" },
 #endif
 #define	CS_MAXCODE		((sizeof(sys_var)/sizeof(sys_var[0])) - 1)
@@ -1857,7 +1861,8 @@ ctl_putsys(
 	CASE_UINT(CS_nts_cookie_decode, nts_cookie_decode);
 
 	CASE_UINT(CS_nts_cookie_decode_old, nts_cookie_decode_old);
-
+	CASE_UINT(CS_nts_cookie_decode_old2, nts_cookie_decode_old2);
+	CASE_UINT(CS_nts_cookie_decode_older, nts_cookie_decode_older);
 	CASE_UINT(CS_nts_cookie_decode_too_old, nts_cookie_decode_too_old);
 
 	CASE_UINT(CS_nts_cookie_decode_error, nts_cookie_decode_error);


=====================================
ntpd/nts_cookie.c
=====================================
@@ -90,9 +90,9 @@
  * You can change that by editing the keys file.
  */
 int K_length = AEAD_AES_SIV_CMAC_256_KEYLEN;
-uint8_t K[NTS_MAX_KEYLEN], K2[NTS_MAX_KEYLEN];
-uint32_t I, I2;
 time_t K_time = 0;	/* time K was created, 0 for none */
+struct NTS_Key nts_keys[NTS_nKEYS];
+int nts_nKeys = 0;
 
 /* The mutex protects cookie_ctx
  * The NTS-KE servers can make cookies
@@ -104,7 +104,9 @@ AES_SIV_CTX* cookie_ctx;
 /* Statistics for ntpq */
 uint64_t nts_cookie_make = 0;
 uint64_t nts_cookie_decode = 0;
-uint64_t nts_cookie_decode_old = 0;
+uint64_t nts_cookie_decode_old = 0;	/* one day old */
+uint64_t nts_cookie_decode_old2 = 0;	/* two days old */
+uint64_t nts_cookie_decode_older = 0;	/* more than 2 days old */
 uint64_t nts_cookie_decode_too_old = 0;
 uint64_t nts_cookie_decode_error = 0;
 
@@ -130,15 +132,15 @@ bool nts_cookie_init(void) {
 bool nts_cookie_init2(void) {
 	bool OK = true;
 	if (!nts_read_cookie_keys()) {
-		nts_make_cookie_key();  /* make new cookie key */
-		nts_make_cookie_key();  /* push new to old, make new */
+		/* Can't read cookie file.  Make one */
+		nts_make_cookie_key();
 		K_time = time(NULL);
 		nts_write_cookie_keys();
 	}
 	return OK;
 }
 
-/* Rotate key -- 24 hours after last rotate
+/* Rotate key -- 24 hours after last rotation
  * That allows a cluster NTS-KE server to keep in sync
  * if we use ratchet rather than random.
  */
@@ -146,6 +148,7 @@ bool nts_cookie_init2(void) {
 // Set this shorter for debugging
 //  keys will timeout, packets will get dropped
 //  after 8 lost packets, it should go through the NTS-KE dance again
+// Just uncommenting the next line will generate a warning reminder.
 // #define SecondsPerDay 3600
 void nts_cookie_timer(void) {
 	time_t now;
@@ -162,9 +165,9 @@ void nts_cookie_timer(void) {
 		K_time += SecondsPerDay;
 	}
 	if (nts_write_cookie_keys() )
-		msyslog(LOG_INFO, "NTS: Wrote new cookie key.");
+		msyslog(LOG_INFO, "NTS: Wrote new cookie file, %d keys.", nts_nKeys);
 	else
-		msyslog(LOG_INFO, "NTS: Trouble writing new cookie key.");
+		msyslog(LOG_INFO, "NTS: Trouble writing new cookie file.");
 	return;
 }
 
@@ -195,39 +198,30 @@ bool nts_read_cookie_keys(void) {
 	if ( !((32 == K_length) || (48 == K_length) || (64 == K_length))) {
 		goto bail;
 	}
-	if (1 != fscanf(in, "I: %u\n", &I)) {
+	nts_nKeys = 0;
+	for (int i=0; i<NTS_nKEYS; i++) {
+	  struct NTS_Key *key = &nts_keys[i];
+	  if (1 != fscanf(in, "I: %u\n", &key->I)) {
+		if (0 < nts_nKeys) break;
 		goto bail;
-	}
-	if (0 != fscanf(in, "K: ")) {
-		goto bail;
-	}
-	for (int i=0; i< K_length; i++) {
-		unsigned int temp;
-		if (1 != fscanf(in, "%02x", &temp)) {
-			goto bail;
-		}
-		K[i] = temp;
-	}
-	if (0 != fscanf(in, "\n")) {
-		goto bail;
-	}
-	if (1 != fscanf(in, "I: %u\n", &I2)) {
+	  }
+	  if (0 != fscanf(in, "K: ")) {
 		goto bail;
-	}
-	if (0 != fscanf(in, "K: ")) {
-		goto bail;
-	}
-	for (int i=0; i< K_length; i++) {
+	  }
+	  for (int j=0; j< K_length; j++) {
 		unsigned int temp;
 		if (1 != fscanf(in, "%02x", &temp)) {
 			goto bail;
 		}
-		K2[i] = temp;
-	}
-	if (0 != fscanf(in, "\n")) {
+		key->K[j] = temp;
+	  }
+	  if (0 != fscanf(in, "\n")) {
 		goto bail;
+	  }
+	  nts_nKeys = i+1;
 	}
 	fclose(in);
+	msyslog(LOG_INFO, "NTS: Read cookie file, %d keys.", nts_nKeys);
 	return true;
 
   bail:
@@ -236,50 +230,65 @@ bool nts_read_cookie_keys(void) {
 	return false;
 }
 
-/* The draft describes a ratchet mode to make new keys
+/* RFC 8915 describes a ratchet mode to make new keys
  * That's one way to implement a KE server for a cluster of NTP servers.
  * The KE server and the NTP servers stay in sync without communication
  * after a one-time copy of the cookie file from NTP server to KE server.
+ * An alternative is to elect one system to generate new keys and
+ * they copy the key file to other systems and have them load it.
  */
 void nts_make_cookie_key(void) {
-	memcpy(&K2, &K, sizeof(K2));	/* Push current cookie to old */
-	I2 = I;
-	ntp_RAND_priv_bytes(K, sizeof(K));
-	ntp_RAND_bytes((uint8_t *)&I, sizeof(I));
+	if (nts_nKeys < NTS_nKEYS) nts_nKeys++;
+	for (int i=nts_nKeys-1; i>0; i--) {
+	  nts_keys[i] = nts_keys[i-1];
+	}
+	ntp_RAND_priv_bytes(nts_keys[0].K, K_length);
+	ntp_RAND_bytes((uint8_t *)&nts_keys[0].I, sizeof(nts_keys[0].I));
 	return;
 }
 
 bool nts_write_cookie_keys(void) {
-	const char *cookie_filename = NTS_COOKIE_KEY_FILE;
+	const char *cookiefile = NTS_COOKIE_KEY_FILE;
+	char tempfile[PATH_MAX];
 	int fd;
 	FILE *out;
 	char errbuf[100];
 	if (NULL != ntsconfig.KI)
-		cookie_filename = ntsconfig.KI;
-	fd = open(cookie_filename, O_CREAT|O_WRONLY, S_IRUSR|S_IWUSR);
+		cookiefile = ntsconfig.KI;
+	strlcpy(tempfile, cookiefile, sizeof(tempfile));
+	strlcat(tempfile, "-tmp", sizeof(tempfile));
+	fd = open(tempfile, O_CREAT|O_WRONLY, S_IRUSR|S_IWUSR);
 	if (-1 == fd) {
 		ntp_strerror_r(errno, errbuf, sizeof(errbuf));
-		msyslog(LOG_ERR, "ERR: can't open %s: %s", cookie_filename, errbuf);
+		msyslog(LOG_ERR, "ERR: can't open %s: %s", tempfile, errbuf);
 		return false;
 	}
 	out = fdopen(fd, "w");
 	if (NULL == out) {
 		ntp_strerror_r(errno, errbuf, sizeof(errbuf));
-		msyslog(LOG_ERR, "ERR: can't fdopen %s: %s", cookie_filename, errbuf);
+		msyslog(LOG_ERR, "ERR: can't fdopen %s: %s", tempfile, errbuf);
 		close(fd);
 		return false;
 	}
+
 	fprintf(out, "T: %lu\n", (unsigned long)K_time);
 	fprintf(out, "L: %d\n", K_length);
-	fprintf(out, "I: %u\n", I);
-	fprintf(out, "K: ");
-	for (int i=0; i< K_length; i++) fprintf(out, "%02x", K[i]);
-	fprintf(out, "\n");
-	fprintf(out, "I: %u\n", I2);
-	fprintf(out, "K: ");
-	for (int i=0; i< K_length; i++) fprintf(out, "%02x", K2[i]);
-	fprintf(out, "\n");
+	for (int i=0; i<nts_nKeys; i++) {
+	  struct NTS_Key *key = &nts_keys[i];
+	  fprintf(out, "I: %u\n", key->I);
+	  fprintf(out, "K: ");
+	    for (int j=0; j< K_length; j++) fprintf(out, "%02x", key->K[j]);
+	  fprintf(out, "\n");
+	  key++;
+	}
 	fclose(out);
+        if (rename(tempfile, cookiefile)) {
+	    ntp_strerror_r(errno, errbuf, sizeof(errbuf));
+            msyslog(LOG_WARNING,
+                    "LOG: Unable to rename temp cookie file %s to %s, %s",
+                    tempfile, cookiefile, errbuf);
+	    return false;
+	}
 	return true;
 }
 
@@ -319,8 +328,8 @@ int nts_make_cookie(uint8_t *cookie,
 	/* collect associated data */
 	finger = cookie;
 
-	memcpy(finger, &I, sizeof(I));
-	finger += sizeof(I);
+	memcpy(finger, &nts_keys[0].I, sizeof(nts_keys[0].I));
+	finger += sizeof(nts_keys[0].I);
 
 	nonce = finger;
 	ntp_RAND_bytes(finger, NONCE_LENGTH);
@@ -333,7 +342,7 @@ int nts_make_cookie(uint8_t *cookie,
 
 	ok = AES_SIV_Encrypt(cookie_ctx,
 			     finger, &left,   /* left: in: max out length, out: length used */
-			     K, K_length,
+			     nts_keys[0].K, K_length,
 			     nonce, NONCE_LENGTH,
 			     plaintext, plainlength,
 			     cookie, AD_LENGTH);
@@ -362,12 +371,13 @@ bool nts_unpack_cookie(uint8_t *cookie, int cookielen,
   uint8_t *c2s, uint8_t *s2c, int *keylen) {
 	uint8_t *finger;
 	uint8_t plaintext[NTS_MAX_COOKIELEN];
-	uint8_t *key;
 	uint8_t *nonce;
 	uint32_t temp;
 	size_t plainlength;
 	int cipherlength;
 	bool ok;
+	struct NTS_Key *key;
+	int i;
 
 	if (NULL == cookie_ctx)
 		return false;	/* We aren't initialized yet. */
@@ -377,17 +387,34 @@ bool nts_unpack_cookie(uint8_t *cookie, int cookielen,
 		return false;
 
 	finger = cookie;
-	if (0 == memcmp(finger, &I, sizeof(I))) {
-		key = K;
+	key = NULL;		/* squash uninitialized warning */
+	for (i=0; i<nts_nKeys; i++) {
+	  key = &nts_keys[i];
+	  if (0 == memcmp(finger, &key->I, sizeof(key->I))) {
+		break;
+	  }
+	}
+	if (0 == i) {
 		nts_cookie_decode++;
-	} else if (0 == memcmp(finger, &I2, sizeof(I2))) {
-		key = K2;
-		nts_cookie_decode_old++;
-	} else {
+	} else if (nts_nKeys == i) {
 		nts_cookie_decode_too_old++;
 		return false;
+	} else if (1 == i) {
+		nts_cookie_decode_old++;
+	} else if (2 == i) {
+		nts_cookie_decode_old2++;
+	} else {
+		nts_cookie_decode_older++;
+	}
+#if 0
+	if (1<i) {
+	  /* Hack for debugging */
+	  /* Beware: DoS possibility on a public server */
+	  msyslog(LOG_INFO, "NTS: Old cookie: %d days.", i);
 	}
-	finger += sizeof(I);
+#endif
+
+	finger += sizeof(key->I);
 	nonce = finger;
 	finger += NONCE_LENGTH;
 
@@ -400,7 +427,7 @@ bool nts_unpack_cookie(uint8_t *cookie, int cookielen,
 
 	ok = AES_SIV_Decrypt(cookie_ctx,
 			     plaintext, &plainlength,
-			     key, K_length,
+			     key->K, K_length,
 			     nonce, NONCE_LENGTH,
 			     finger, cipherlength,
 			     cookie, AD_LENGTH);


=====================================
tests/ntpd/nts_cookie.c
=====================================
@@ -20,31 +20,31 @@ TEST_SETUP(nts_cookie) {}
 
 TEST_TEAR_DOWN(nts_cookie) {}
 
-
 TEST(nts_cookie, nts_make_cookie_key) {
 	/* init */
-	uint8_t kStart1[NTS_MAX_KEYLEN] = {1, 2, 3, 4, 5};
-	uint8_t kStart2[NTS_MAX_KEYLEN] = {10, 20, 30, 40, 50};
-	uint32_t iStart = I;
+	struct NTS_Key k0 = {.K={1, 2, 3, 4, 5}, .I=123};
+	struct NTS_Key k1 = {.K={10, 20, 30, 40, 50}, .I=456};
 	/* copy to key variables */
-	memcpy(K, kStart1, sizeof(K));
-	memcpy(K2, kStart2, sizeof(K2));
+	nts_keys[0] = k0;
+	nts_keys[1] = k1;
 	/* run test */
-	nts_make_cookie_key();
-	/* check that K2 now equals former-K */
-	TEST_ASSERT_EQUAL_UINT8_ARRAY(kStart1, K2, sizeof(K2));
-	/* check that K does not equal former-K */
+	nts_nKeys = 2;
+	nts_make_cookie_key();  /* push k0 to k1 */
+	/* check that K[1] now equals former-K[0] */
+	TEST_ASSERT_EQUAL_UINT8_ARRAY(nts_keys[1].K, k0.K, NTS_MAX_KEYLEN);
+	TEST_ASSERT_EQUAL(nts_keys[1].I, k0.I);
+	/* check that K[0] does not equal former-K[0] */
 	/* There is no "TEST UNEQUAL", do it manually */
 	bool equal = true;
-	for (unsigned int i = 0; i < sizeof(K); i++) {
-		if (K[i] != kStart1[i]) {
+	for (unsigned int i = 0; i < NTS_MAX_KEYLEN; i++) {
+		if (nts_keys[0].K[i] != k0.K[i]) {
 			equal = false;
 			break;
 		}
 	}
 	TEST_ASSERT_EQUAL(false, equal);
-	/* Check that I does not equal former-I */
-	TEST_ASSERT_NOT_EQUAL(iStart, I);
+	/* Check that I[0] does not equal former-I[0] */
+	TEST_ASSERT_NOT_EQUAL(nts_keys[0].I, k0.I);
 }
 
 TEST(nts_cookie, nts_make_unpack_cookie) {
@@ -74,7 +74,36 @@ TEST(nts_cookie, nts_make_unpack_cookie) {
 	TEST_ASSERT_EQUAL_UINT8_ARRAY(s2c, s2c_2, 16);
 }
 
+TEST(nts_cookie, nts_read_write_cookies) {
+	struct NTS_Key k0, k1, k2;
+	bool ok;
+	ntsconfig.KI = "test-cookie-keys";
+	nts_nKeys = 0;
+	nts_make_cookie_key();
+	nts_make_cookie_key();
+	nts_make_cookie_key();
+	k0 = nts_keys[0];
+	k1 = nts_keys[1];
+	k2 = nts_keys[2];
+	TEST_ASSERT_EQUAL(nts_nKeys, 3);
+	ok = nts_write_cookie_keys();
+	TEST_ASSERT_EQUAL(true, ok);
+	nts_make_cookie_key();    /* scramble things */
+	ZERO(nts_keys);
+	nts_nKeys = 377;
+	ok = nts_read_cookie_keys();
+	TEST_ASSERT_EQUAL(true, ok);
+	TEST_ASSERT_EQUAL(nts_nKeys, 3);
+	TEST_ASSERT_EQUAL_UINT8_ARRAY(nts_keys[0].K, k0.K, NTS_MAX_KEYLEN);
+	TEST_ASSERT_EQUAL(nts_keys[0].I, k0.I);
+	TEST_ASSERT_EQUAL_UINT8_ARRAY(nts_keys[1].K, k1.K, NTS_MAX_KEYLEN);
+	TEST_ASSERT_EQUAL(nts_keys[1].I, k1.I);
+	TEST_ASSERT_EQUAL_UINT8_ARRAY(nts_keys[2].K, k2.K, NTS_MAX_KEYLEN);
+	TEST_ASSERT_EQUAL(nts_keys[2].I, k2.I);
+}
+
 TEST_GROUP_RUNNER(nts_cookie) {
 	RUN_TEST_CASE(nts_cookie, nts_make_unpack_cookie);
 	RUN_TEST_CASE(nts_cookie, nts_make_cookie_key);
+	RUN_TEST_CASE(nts_cookie, nts_read_write_cookies);
 }



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

-- 
View it on GitLab: https://gitlab.com/NTPsec/ntpsec/-/commit/87358bc90bc4c73c079c6caa47eceab4caa4800c
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/20221218/981bdcd7/attachment-0001.htm>


More information about the vc mailing list