[Git][NTPsec/ntpsec][master] Cleanup of NTS-KE record packing

Hal Murray gitlab at mg.gitlab.com
Mon Feb 11 02:02:56 UTC 2019


Hal Murray pushed to branch master at NTPsec / ntpsec


Commits:
8b4ffcd1 by Hal Murray at 2019-02-11T02:02:04Z
Cleanup of NTS-KE record packing

- - - - -


4 changed files:

- include/nts.h
- ntpd/nts.c
- ntpd/nts_client.c
- ntpd/nts_server.c


Changes:

=====================================
include/nts.h
=====================================
@@ -80,16 +80,22 @@ int make_cookie(uint8_t *cookie, uint16_t aead,
 /* working finger into a buffer - updated by append/unpack routines */
 struct BufCtl_t {
   uint8_t *next;  /* pointer to next data/space */
-  int      left;  /* data/space left */
+  int left;       /* data left or  space available */
 };
 typedef struct BufCtl_t BufCtl;
 
-void nts_append_record(BufCtl* buf, uint16_t type, uint16_t length);
-void nts_append_uint16(BufCtl* buf, uint16_t data);
-
+/* maybe should return bool to indicate overflow */
+/* nts_append_record_foo makes whole record with one foo */
+/* ntp_append_foo appends foo to existing partial record */
+void nts_append_record_null(BufCtl* buf, uint16_t type);
 void nts_append_record_uint16(BufCtl* buf, uint16_t type, uint16_t data);
+void nts_append_record_bytes(BufCtl* buf, uint16_t type, uint8_t *data, int length);
+
+void nts_append_header(BufCtl* buf, uint16_t type, uint16_t length);
+void nts_append_uint16(BufCtl* buf, uint16_t data);
+void nts_append_bytes(BufCtl* buf, uint8_t *data, int length);
 
-uint16_t nts_next_record(BufCtl* buf, uint16_t *length);
+uint16_t nts_next_record(BufCtl* buf, int *length);
 uint16_t nts_next_uint16(BufCtl* buf);
 
 


=====================================
ntpd/nts.c
=====================================
@@ -125,31 +125,60 @@ int nts_decorate(struct ntscfg_t *cfg, struct ntsstate_t *state,
 
 /*****************************************************/
 
-void nts_append_record(BufCtl* buf, uint16_t type, uint16_t length) {
+// 2 byte type, 2 byte length
+#define NTS_KE_HDR_LNG 4
+#define NTS_KE_DATA2_LNG 2
+
+/* Troubles with signed/unsigned compares when using sizeof() */
+
+void nts_append_record_null(BufCtl* buf, uint16_t type) {
+  nts_append_header(buf, type, NTS_KE_DATA2_LNG);
+}
+
+void nts_append_record_uint16(BufCtl* buf, uint16_t type, uint16_t data) {
+  if (NTS_KE_HDR_LNG+NTS_KE_DATA2_LNG > buf->left)
+    return;
+  nts_append_header(buf, type, NTS_KE_DATA2_LNG);
+  nts_append_uint16(buf, data);
+}
+
+void nts_append_record_bytes(BufCtl* buf, uint16_t type, uint8_t *data, int length) {
+  if (NTS_KE_HDR_LNG+length > buf->left)
+    return;
+  nts_append_header(buf, type, length);
+  nts_append_bytes(buf, data, length);
+}
+
+void nts_append_header(BufCtl* buf, uint16_t type, uint16_t length) {
   uint16_t * ptr = (uint16_t *)buf->next;
+  if (NTS_KE_HDR_LNG > buf->left)
+    return;
   *ptr++ = htons(type);
   *ptr++ = htons(length);
-  buf->next += sizeof(type)+sizeof(length);
-  buf->left -= sizeof(type)+sizeof(length);
+  buf->next += NTS_KE_HDR_LNG;
+  buf->left -= NTS_KE_HDR_LNG;
   /* leaves buf pointing to where data will go */
-  return;
 }
 
 void nts_append_uint16(BufCtl* buf, uint16_t data) {
   uint16_t * ptr = (uint16_t *)buf->next;
+  if (NTS_KE_DATA2_LNG > buf->left)
+    return;
   *ptr++ = htons(data);
-  buf->next += sizeof(data);
-  buf->left -= sizeof(data);
-  return;
+  buf->next += NTS_KE_DATA2_LNG;
+  buf->left -= NTS_KE_DATA2_LNG;
 }
 
-void nts_append_record_uint16(BufCtl* buf, uint16_t type, uint16_t data) {
-  nts_append_record(buf, type, sizeof(uint16_t));
-  nts_append_uint16(buf, data);
+void nts_append_bytes(BufCtl* buf, uint8_t *data, int length) {
+  if (length > buf->left)
+    return;
+  memcpy(buf->next, data, length);
+  buf->next += length;
+  buf->left -= length;
 }
 
 
-uint16_t nts_next_record(BufCtl* buf, uint16_t *length) {
+uint16_t nts_next_record(BufCtl* buf, int *length) {
   uint16_t *ptr = (uint16_t *)buf->next;
   uint16_t type = ntohs(*ptr++);
   *length = ntohs(*ptr++);


=====================================
ntpd/nts_client.c
=====================================
@@ -48,6 +48,7 @@ bool nts_probe(struct peer * peer) {
 // CentOS 7:   0x100020bfL  1.0.2k
 // CentOS 6:   0x1000105fL  1.0.1e
 // NetBSD 8:   0x100020bfL  1.0.2k
+// NetBSD 7:   0x1000115fL  1.0.1u
 // FreeBSD 12: 0x1010101fL  1.1.1a-freebsd
 #if (OPENSSL_VERSION_NUMBER > 0x1010000fL)
   ctx = SSL_CTX_new(TLS_client_method());
@@ -157,7 +158,7 @@ bool nts_probe(struct peer * peer) {
     nts_append_record_uint16(&buf, algorithm_negotiation, AEAD_AES_SIV_CMAC_256);
 
     /* 4.1.1: End, Critical */
-    nts_append_record(&buf, CRITICAL+end_of_message, 0);
+    nts_append_record_null(&buf, CRITICAL+end_of_message);
 
     used = sizeof(buff)-buf.left;
     transfered = SSL_write(ssl, buff, used);
@@ -283,10 +284,11 @@ bool process_recv_data(struct peer* peer, SSL *ssl) {
   buf.next = buff;
   buf.left = transfered;
   while (buf.left > 0) {
-    uint16_t length, data;
-    uint16_t type = nts_next_record(&buf, &length);
+    uint16_t type, data;
     bool critical = false;
+    int length;
 
+    type = nts_next_record(&buf, &length);
     if (CRITICAL & type) {
       critical = true;
       type &= ~CRITICAL;


=====================================
ntpd/nts_server.c
=====================================
@@ -18,6 +18,7 @@
 #include "ntp.h"
 #include "ntpd.h"
 #include "ntp_stdlib.h"
+#include "nts_lib.h"
 
 static int create_listener(int port);
 static void* nts_ke_listener(void*);
@@ -128,7 +129,8 @@ void nts_ke_request(SSL *ssl) {
     uint8_t c2s[NTS_MAX_KEYLEN], s2c[NTS_MAX_KEYLEN];
     uint8_t cookie[NTS_COOKIELEN];
     int aead, keylen, cookielen;
-
+    struct BufCtl_t buf;
+    int used;
 
     bytes_read = SSL_read(ssl, buff, sizeof(buff));
     if (0 >= bytes_read) {
@@ -136,30 +138,40 @@ void nts_ke_request(SSL *ssl) {
         return;
     }
 
+    // FIXME Ignore request for now
     aead = IANA_AEAD_AES_SIV_CMAC_256;
 
+    buf.next = buff;
+    buf.left = sizeof(buff);
     keylen = get_key_length(aead);
     nts_make_keys(ssl, c2s, s2c, keylen);
 
+    /* 4.1.2 Next Protocol, 0 for NTP */
+    nts_append_record_uint16(&buf, next_protocol_negotiation, 0);
+    /* 4.1.5 AEAD Algorithm List */
+    nts_append_record_uint16(&buf, algorithm_negotiation, AEAD_AES_SIV_CMAC_256);
+
     for (int i=0; i<NTS_MAX_COOKIES; i++) {
       cookielen = make_cookie(cookie, aead, c2s, s2c, keylen);
+      nts_append_record_bytes(&buf, new_cookie, cookie, cookielen);
     }
-    cookielen = cookielen;
     
-    // Hack, echo it back
-    bytes_written = SSL_write(ssl, buff, bytes_read);
+    /* 4.1.1: End, Critical */
+    nts_append_record_null(&buf, CRITICAL+end_of_message);
+    used = sizeof(buff)-buf.left;
+
+    bytes_written = SSL_write(ssl, buff, used);
     if (bytes_written != bytes_read) {
         msyslog(LOG_INFO, "NTSs: SSL_write error");
         return;
     }
 
-    msyslog(LOG_INFO, "NTSs: Echoed %d bytes", bytes_written);
+    msyslog(LOG_INFO, "NTSs: Returned %d bytes", bytes_written);
     
     return;
 }
 
-int create_listener(int port)
-{
+int create_listener(int port) {
     int sock;
     struct sockaddr_in addr;
 



View it on GitLab: https://gitlab.com/NTPsec/ntpsec/commit/8b4ffcd1dd87791e5a58217e9cdc8ff95e6c120c

-- 
View it on GitLab: https://gitlab.com/NTPsec/ntpsec/commit/8b4ffcd1dd87791e5a58217e9cdc8ff95e6c120c
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/20190211/1f7400d3/attachment-0001.html>


More information about the vc mailing list