[Git][NTPsec/ntpsec][master] 6 commits: NTS: Add hack to use TLS1.2 on OpenSSL 1.1.1a

Hal Murray gitlab at mg.gitlab.com
Mon Apr 1 10:59:44 UTC 2019



Hal Murray pushed to branch master at NTPsec / ntpsec


Commits:
524935ed by Hal Murray at 2019-04-01T05:35:04Z
NTS: Add hack to use TLS1.2 on OpenSSL 1.1.1a
  This bypasses a bug in making keys.

- - - - -
78b56cb5 by Hal Murray at 2019-04-01T07:48:54Z
    NTS: check for troubles making keys
      Catches bug in OpenSSL 1.1.1a

- - - - -
d54a716d by Hal Murray at 2019-04-01T08:30:55Z
NTS: Move cookie procedures around - maybe enable testing

- - - - -
7ccf21d7 by Hal Murray at 2019-04-01T10:29:19Z
Add FIXME

- - - - -
6f9c0a91 by Hal Murray at 2019-04-01T10:30:08Z
NTS: Update devel/TODO-NTS

- - - - -
bd89f0dd by Hal Murray at 2019-04-01T10:30:35Z
NTS: Rearrage nts_server to enable testing

- - - - -


7 changed files:

- devel/TODO-NTS
- include/nts.h
- include/nts2.h
- ntpd/nts.c
- ntpd/nts_client.c
- ntpd/nts_cookie.c
- ntpd/nts_server.c


Changes:

=====================================
devel/TODO-NTS
=====================================
@@ -2,9 +2,14 @@ BUGS:
   timeout on client connect too long (system default)
   Is 3 seconds timeout OK? (both client and server)
 
+nts_log_ssl_error()  No SSL param ??
+  ERR_error_string_n
+
+Fix SecondsPerDay in nts_cookie.c
+  Set to 3600 for testing
 
 multithread msyslog
-  libntp/lib_strbuf.c too
+  libntp/lib_strbuf.c is used by socktoa and sockporttoa
   strerror
 
 documentation:


=====================================
include/nts.h
=====================================
@@ -23,9 +23,63 @@ bool nts_server_init2(void);    /* after sandbox */
 bool nts_cookie_init2(void);
 
 
+bool nts_read_cookie_keys(void);
+bool nts_make_cookie_key(void);
+bool nts_write_cookie_keys(void);
+
+int nts_make_cookie(uint8_t *cookie,
+  uint16_t aead,
+  uint8_t *c2s, uint8_t *s2c, int keylen);
+bool nts_unpack_cookie(uint8_t *cookie, int cookielen,
+  uint16_t *aead,
+  uint8_t *c2s, uint8_t *s2c, int *keylen);
+
+/* working finger into a buffer - updated by append/unpack routines */
+struct BufCtl_t {
+  uint8_t *next;  /* pointer to next data/space */
+  int left;       /* data left or space available */
+};
+typedef struct BufCtl_t BufCtl;
+
+bool nts_ke_process_receive(struct BufCtl_t *buf, int *aead);
+bool nts_ke_setup_send(struct BufCtl_t *buf, int aead,
+       uint8_t *c2s, uint8_t *s2c, int keylen);
+
+/***********************************************************/
+
+/* buffer packing/unpacking routines. 
+ * NB: The length field in NTP extensions includes the header
+ * while the length field in NTS-KE data streams does not.
+ *
+ * These routines do not handle padding.  NTS-KE has no padding.
+ * NTP extensions are padded to word (4 byte) boundaries.
+ *
+ * Note that data on the wire is big endian.
+ * buffer is wire format, not host format.
+ */
+
+
+/* xxx_append_record_foo makes whole record with one foo */
+/* append_foo appends foo to existing partial record */
+void ke_append_record_null(BufCtl* buf, uint16_t type);
+void ke_append_record_uint16(BufCtl* buf, uint16_t type, uint16_t data);
+void ke_append_record_bytes(BufCtl* buf, uint16_t type, uint8_t *data, int length);
+
+void ex_append_record_null(BufCtl* buf, uint16_t type);
+void ex_append_record_uint16(BufCtl* buf, uint16_t type, uint16_t data);
+void ex_append_record_bytes(BufCtl* buf, uint16_t type, uint8_t *data, int length);
 
+void ex_append_header(BufCtl* buf, uint16_t type, uint16_t length);
+void append_header(BufCtl* buf, uint16_t type, uint16_t length);
+void append_uint16(BufCtl* buf, uint16_t data);
+void append_bytes(BufCtl* buf, uint8_t *data, int length);
 
+uint16_t ke_next_record(BufCtl* buf, int *length);
+uint16_t ex_next_record(BufCtl* buf, int *length);  /* body length */
+uint16_t next_uint16(BufCtl* buf);
+uint16_t next_bytes(BufCtl* buf, uint8_t *data, int length);
 
+/***********************************************************/
 
 #define NTS_MAX_KEYLEN		64	/* used in cookies */
 #define NTS_MAX_COOKIELEN	192	/* see nts_cookie.c */


=====================================
include/nts2.h
=====================================
@@ -1,6 +1,7 @@
 /*
  * nts2.h - NTS (Network Time Security) declarations
  * other half of nts.h which doesn't include openssl/ssh.h
+ * This split helps test routines.
  */
 #ifndef GUARD_NTS2_H
 #define GUARD_NTS2_H
@@ -9,11 +10,16 @@
 #include <stdint.h>
 #include <openssl/ssl.h>
 
+#include "nts.h"
+
 
 bool nts_load_certificate(SSL_CTX *ctx);
 bool nts_load_ciphers(SSL_CTX *ctx);
 bool nts_load_versions(SSL_CTX *ctx);
 
+int nts_ssl_read(SSL *ssl, uint8_t *buff, int buff_length);
+int nts_ssl_write(SSL *ssl, uint8_t *buff, int buff_length);
+
 void nts_log_ssl_error(void);
 
 int nts_get_key_length(uint16_t aead);
@@ -23,53 +29,5 @@ uint16_t nts_string_to_aead(const char* text);
 bool nts_make_keys(SSL *ssl, uint16_t aead,
   uint8_t *c2s, uint8_t *s2c, int keylen);
 
-int nts_make_cookie(uint8_t *cookie,
-  uint16_t aead,
-  uint8_t *c2s, uint8_t *s2c, int keylen);
-bool nts_unpack_cookie(uint8_t *cookie, int cookielen,
-  uint16_t *aead,
-  uint8_t *c2s, uint8_t *s2c, int *keylen);
-
-
-
-/* buffer packing/unpacking routines.
- * NB: The length field in NTP extensions includes the header
- * while the length field in NTS-KE data streams does not.
- *
- * These routines do not handle padding.  NTS-KE has no padding.
- * NTP extensions are padded to word (4 byte) boundaries.
- *
- * Note that data on the wire is big endian.
- * buffer is wire format, not host format.
- */
-
-
-/* working finger into a buffer - updated by append/unpack routines */
-struct BufCtl_t {
-  uint8_t *next;  /* pointer to next data/space */
-  int left;       /* data left or space available */
-};
-typedef struct BufCtl_t BufCtl;
-
-/* xxx_append_record_foo makes whole record with one foo */
-/* append_foo appends foo to existing partial record */
-void ke_append_record_null(BufCtl* buf, uint16_t type);
-void ke_append_record_uint16(BufCtl* buf, uint16_t type, uint16_t data);
-void ke_append_record_bytes(BufCtl* buf, uint16_t type, uint8_t *data, int length);
-
-void ex_append_record_null(BufCtl* buf, uint16_t type);
-void ex_append_record_uint16(BufCtl* buf, uint16_t type, uint16_t data);
-void ex_append_record_bytes(BufCtl* buf, uint16_t type, uint8_t *data, int length);
-
-void ex_append_header(BufCtl* buf, uint16_t type, uint16_t length);
-void append_header(BufCtl* buf, uint16_t type, uint16_t length);
-void append_uint16(BufCtl* buf, uint16_t data);
-void append_bytes(BufCtl* buf, uint8_t *data, int length);
-
-uint16_t ke_next_record(BufCtl* buf, int *length);
-uint16_t ex_next_record(BufCtl* buf, int *length);  /* body length */
-uint16_t next_uint16(BufCtl* buf);
-uint16_t next_bytes(BufCtl* buf, uint8_t *data, int length);
-
 
 #endif /* GUARD_NTS2_H */


=====================================
ntpd/nts.c
=====================================
@@ -121,6 +121,12 @@ bool nts_load_versions(SSL_CTX *ctx) {
   maxver = nts_translate_version(ntsconfig.maxtls);
   if ((-1 == minver) || (-1 == maxver))
     return false;
+#if (OPENSSL_VERSION_NUMBER == 0x1010101fL)
+  if (0 == maxver) {
+      msyslog(LOG_INFO, "NTS: Using TLS1.2 to avoid bug in OpenSSL 1.1.1a.");
+      maxver = TLS1_2_VERSION;
+  }
+#endif
 #if (OPENSSL_VERSION_NUMBER > 0x1010000fL)
   if(0 == minver) minver = TLS1_2_VERSION;   // 3.
   SSL_CTX_set_min_proto_version(ctx, minver);
@@ -199,6 +205,28 @@ bool nts_load_certificate(SSL_CTX *ctx) {
 }
 
 
+int nts_ssl_read(SSL *ssl, uint8_t *buff, int buff_length) {
+    int bytes_read;
+    bytes_read = SSL_read(ssl, buff, buff_length);
+    if (0 >= bytes_read) {
+        msyslog(LOG_INFO, "NTS: SSL_read error: %s", strerror(errno));
+        nts_log_ssl_error();
+        return -1;
+    }
+    return bytes_read;
+}
+
+int nts_ssl_write(SSL *ssl, uint8_t *buff, int buff_length) {
+    int bytes_written;
+    bytes_written = SSL_write(ssl, buff, buff_length);
+    if (0 >= bytes_written) {
+        msyslog(LOG_INFO, "NTS: SSL_write error: %s", strerror(errno));
+        nts_log_ssl_error();
+        return -1;
+    }
+    return bytes_written;
+}
+
 void nts_log_ssl_error(void) {
   char buff[256];
   int err = ERR_get_error();


=====================================
ntpd/nts_client.c
=====================================
@@ -175,17 +175,20 @@ bool nts_probe(struct peer * peer) {
     msyslog(LOG_ERR, "NTSc: Unknown AEAD code: %d", peer->nts_state.aead);
     goto bail;
   }
-  nts_make_keys(ssl,
-    peer->nts_state.aead,
-    peer->nts_state.c2s,
-    peer->nts_state.s2c,
-    peer->nts_state.keylen);
+  if (!nts_make_keys(ssl,
+        peer->nts_state.aead,
+        peer->nts_state.c2s,
+        peer->nts_state.s2c,
+        peer->nts_state.keylen))
+    goto bail;
 
   addrOK = true;
 
 bail:
-  if (!addrOK)
+  if (!addrOK) {
     nts_ke_probes_bad++;
+    peer->nts_state.count = -1;
+  }
   SSL_shutdown(ssl);
   SSL_free(ssl);
   close(server);
@@ -406,12 +409,11 @@ bool nts_client_send_request(SSL *ssl, struct peer* peer) {
         used, (long)sizeof(buff), strerror(errno));
     return false;
   }
-  transferred = SSL_write(ssl, buff, used);
-  if (used != transferred) {
-    msyslog(LOG_ERR, "NTSc: write failed: %d, %d, %s", used, transferred, strerror(errno));
-    nts_log_ssl_error();
+
+  transferred = nts_ssl_write(ssl, buff, used);
+  if (used != transferred)
     return false;
-  }
+
   return true;
 }
 
@@ -420,12 +422,9 @@ bool nts_client_process_response(SSL *ssl, struct peer* peer) {
   int transferred, idx;
   struct BufCtl_t buf;
 
-  transferred = SSL_read(ssl, buff, sizeof(buff));
-  if (0 > transferred) {
-    msyslog(LOG_ERR, "NTSc: read failed: %d, %s", transferred, strerror(errno));
-    nts_log_ssl_error();
+  transferred = nts_ssl_read(ssl, buff, sizeof(buff));
+  if (0 > transferred)
     return false;
-  }
   msyslog(LOG_ERR, "NTSc: read %d bytes", transferred);
 
   peer->nts_state.aead = NO_AEAD;


=====================================
ntpd/nts_cookie.c
=====================================
@@ -64,11 +64,6 @@
  * 168
  */
 
-bool nts_read_cookie_keys(void);
-bool nts_make_cookie_key(void);
-bool nts_write_cookie_keys(void);
-
-
 /* cookies use same AEAD algorithms as wire */
 /* This determines which algorithm we use.
  * Valid choices are 32, 48, and 64
@@ -354,6 +349,7 @@ bool nts_unpack_cookie(uint8_t *cookie, int cookielen,
   cipherlength = cookielen - AD_LENGTH;
   plainlength = NTS_MAX_COOKIELEN;
 
+  // FIXME - needs lock
   ok = AES_SIV_Decrypt(cookie_ctx,
            plaintext, &plainlength,
            key, K_length,


=====================================
ntpd/nts_server.c
=====================================
@@ -25,7 +25,6 @@
 static int create_listener(int port, int family);
 static void* nts_ke_listener(void*);
 static bool nts_ke_request(SSL *ssl);
-static bool nts_ke_do_receive(SSL *ssl, int *aead);
 
 
 static SSL_CTX *server_ctx = NULL;
@@ -172,19 +171,25 @@ return NULL;
 }
 
 bool nts_ke_request(SSL *ssl) {
-    /* Our cookies can be 104, 136, or 168 for AES_SIV_CMAC_xxx
+    /* RFC 4: servers must accept 1024
+     * Our cookies can be 104, 136, or 168 for AES_SIV_CMAC_xxx
      * 8*168 fits comfortably into 2K.
      */
     uint8_t buff[2048];
-    int bytes_written;
     uint8_t c2s[NTS_MAX_KEYLEN], s2c[NTS_MAX_KEYLEN];
-    uint8_t cookie[NTS_MAX_COOKIELEN];
-    int aead, keylen, cookielen;
+    int aead, keylen;
     struct BufCtl_t buf;
+    int bytes_read, bytes_written;
     int used;
 
+    bytes_read = nts_ssl_read(ssl, buff, sizeof(buff));
+    if (0 > bytes_read)
+        return false;
+
+    buf.next = buff;
+    buf.left = bytes_read;
     aead = NO_AEAD;
-    if (!nts_ke_do_receive(ssl, &aead))
+    if (!nts_ke_process_receive(&buf, &aead))
         return false;
 
     if ((NO_AEAD == aead) && (NULL != ntsconfig.aead))
@@ -192,33 +197,22 @@ bool nts_ke_request(SSL *ssl) {
     if (NO_AEAD == aead)
       aead = AEAD_AES_SIV_CMAC_256;    /* default */
 
-    buf.next = buff;
-    buf.left = sizeof(buff);
     keylen = nts_get_key_length(aead);
-    nts_make_keys(ssl, aead, c2s, s2c, keylen);
-
-    /* 4.1.2 Next Protocol, 0 for NTP */
-    ke_append_record_uint16(&buf, NTS_CRITICAL+nts_next_protocol_negotiation, 0);
-    /* 4.1.5 AEAD Algorithm List */
-    ke_append_record_uint16(&buf, nts_algorithm_negotiation, aead);
+    if (!nts_make_keys(ssl, aead, c2s, s2c, keylen))
+        return false;
 
-    for (int i=0; i<NTS_MAX_COOKIES; i++) {
-      cookielen = nts_make_cookie(cookie, aead, c2s, s2c, keylen);
-      ke_append_record_bytes(&buf, nts_new_cookie, cookie, cookielen);
-    }
+    buf.next = buff;
+    buf.left = sizeof(buff);
+    if (!nts_ke_setup_send(&buf, aead, c2s, s2c, keylen))
+        return false;
 
-    /* 4.1.1: End, Critical */
-    ke_append_record_null(&buf, NTS_CRITICAL+nts_end_of_message);
     used = sizeof(buff)-buf.left;
-
-    bytes_written = SSL_write(ssl, buff, used);
-    if (bytes_written != used) {
-        msyslog(LOG_INFO, "NTSs: SSL_write error: %s", strerror(errno));
-        nts_log_ssl_error();
+    bytes_written = nts_ssl_write(ssl, buff, used);
+    if (bytes_written != used)
         return false;
-    }
 
-    msyslog(LOG_INFO, "NTSs: Returned %d bytes", bytes_written);
+    msyslog(LOG_INFO, "NTSs: Read %d, wrote %d bytes.  AEAD=%d",
+        bytes_read, bytes_written, aead);
 
     return true;
 }
@@ -289,7 +283,7 @@ int create_listener(int port, int family) {
         }
         if (listen(sock, 6) < 0) {
           msyslog(LOG_ERR, "NTSs: can't listen6: %s", strerror(errno));
-	  close(sock);
+          close(sock);
           return -1;
         }
         msyslog(LOG_INFO, "NTSs: listen6 worked");
@@ -301,27 +295,13 @@ int create_listener(int port, int family) {
     return sock;
 }
 
-bool nts_ke_do_receive(SSL *ssl, int *aead) {
-    /* RFC 4: servers must accept 1024 */
-    uint8_t buff[1024];
-    int bytes_read;
-    struct BufCtl_t buf;
-
-    bytes_read = SSL_read(ssl, buff, sizeof(buff));
-    if (0 >= bytes_read) {
-        msyslog(LOG_INFO, "NTSs: SSL_read error: %s", strerror(errno));
-        nts_log_ssl_error();
-        return false;
-    }
-
-    buf.next = buff;
-    buf.left = bytes_read;
-    while (buf.left > 0) {
+bool nts_ke_process_receive(struct BufCtl_t *buf, int *aead) {
+    while (buf->left > 0) {
       uint16_t type, data;
       int length;
       bool critical = false;
 
-      type = ke_next_record(&buf, &length);
+      type = ke_next_record(buf, &length);
       if (NTS_CRITICAL & type) {
         critical = true;
         type &= ~NTS_CRITICAL;
@@ -330,13 +310,13 @@ bool nts_ke_do_receive(SSL *ssl, int *aead) {
         msyslog(LOG_ERR, "NTSs: Record: T=%d, L=%d, C=%d", type, length, critical);
       switch (type) {
         case nts_error:
-          data = next_uint16(&buf);
+          data = next_uint16(buf);
           if (sizeof(data) != length)
             msyslog(LOG_ERR, "NTSs: wrong length on error: %d", length);
           msyslog(LOG_ERR, "NTSs: error: %d", data);
           return false;
         case nts_next_protocol_negotiation:
-          data = next_uint16(&buf);
+          data = next_uint16(buf);
           if ((sizeof(data) != length) || (data != 0)) {
             msyslog(LOG_ERR, "NTSs: NPN-Wrong length or bad data: %d, %d",
                 length, data);
@@ -345,7 +325,7 @@ bool nts_ke_do_receive(SSL *ssl, int *aead) {
           break;
         case nts_algorithm_negotiation:
           for (int i=0; i<length; i+=sizeof(uint16_t)) {
-            data = next_uint16(&buf);
+            data = next_uint16(buf);
             if (0 == nts_get_key_length(data)) {
               if (0)  /* for debugging */
                 msyslog(LOG_ERR, "NTSs: AN-Unsupported AEAN type: %d", data);
@@ -362,8 +342,8 @@ bool nts_ke_do_receive(SSL *ssl, int *aead) {
                 length, critical);
             return false;
           }
-          if (0 != buf.left) {
-            msyslog(LOG_ERR, "NTSs: EOM not at end: %d", buf.left);
+          if (0 != buf->left) {
+            msyslog(LOG_ERR, "NTSs: EOM not at end: %d", buf->left);
             return false;
           }
           break;
@@ -371,8 +351,8 @@ bool nts_ke_do_receive(SSL *ssl, int *aead) {
           msyslog(LOG_ERR, "NTSs: received strange type: T=%d, C=%d, L=%d",
             type, critical, length);
           if (critical) return false;
-          buf.next += length;
-          buf.left -= length;
+          buf->next += length;
+          buf->left -= length;
           break;
       } /* case */
     }   /* while */
@@ -381,4 +361,26 @@ bool nts_ke_do_receive(SSL *ssl, int *aead) {
 
 }
 
+bool nts_ke_setup_send(struct BufCtl_t *buf, int aead,
+       uint8_t *c2s, uint8_t *s2c, int keylen) {
+    uint8_t cookie[NTS_MAX_COOKIELEN];
+    int cookielen;
+
+    /* 4.1.2 Next Protocol, 0 for NTP */
+    ke_append_record_uint16(buf, NTS_CRITICAL+nts_next_protocol_negotiation, 0);
+    /* 4.1.5 AEAD Algorithm List */
+    ke_append_record_uint16(buf, nts_algorithm_negotiation, aead);
+
+    for (int i=0; i<NTS_MAX_COOKIES; i++) {
+      cookielen = nts_make_cookie(cookie, aead, c2s, s2c, keylen);
+      ke_append_record_bytes(buf, nts_new_cookie, cookie, cookielen);
+    }
+
+    /* 4.1.1: End, Critical */
+    ke_append_record_null(buf, NTS_CRITICAL+nts_end_of_message);
+
+    return true;
+
+}
+
 /* end */



View it on GitLab: https://gitlab.com/NTPsec/ntpsec/compare/0a5a780fbd407a7f0e119b3c0cd3769356920a9c...bd89f0dd230643e0fd689f1075e1e8f367d81061

-- 
View it on GitLab: https://gitlab.com/NTPsec/ntpsec/compare/0a5a780fbd407a7f0e119b3c0cd3769356920a9c...bd89f0dd230643e0fd689f1075e1e8f367d81061
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/20190401/4fb14d6d/attachment-0001.html>


More information about the vc mailing list