Make Service communicate via empty-line-delimited Dictionary objects instead of the old size prefix way.

netconf-service/index.js

@@ -45,22 +45,21 @@ var ZEROTIER_IDTOOL = '/usr/local/bin/zerotier-idtool';
 
 // From Constants.hpp in node/
 var ZT_NETWORK_AUTOCONF_DELAY = 60000;
+var ZT_NETWORK_CERTIFICATE_TTL_WINDOW = (ZT_NETWORK_AUTOCONF_DELAY * 4);
 
 // Connect to redis, assuming database 0 and no auth (for now)
 var redis = require('redis');
 var DB = redis.createClient();
-DB.on("error",function(err) {
+DB.on("error",function(err) { console.error('redis query error: '+err); });
-	console.error('redis query error: '+err);
-});
 
 // Global variables -- these are initialized on startup or netconf-init message
 var netconfSigningIdentity = null; // identity of netconf master, with private key portion
 
+// spawn() function to launch sub-processes
 var spawn = require('child_process').spawn;
 
+// Returns true for fields that are "true" according to ZT redis schema
 function ztDbTrue(v) { return ((v === '1')||(v === 'true')||(v > 0)); }
-function csvToArray(csv) { return (((typeof csv === 'string')&&(csv.length > 0)) ? csv.split(',') : []); }
-function arrayToCsv(a) { return ((Array.isArray(a)) ? ((a.length > 0) ? a.join(',') : '') : (((a !== null)&&(typeof a !== 'undefined')) ? a.toString() : '')); }
 
 //
 // ZeroTier One Dictionary -- encoding-compatible with Dictionary in C++ code base
@@ -214,11 +213,16 @@ function Identity(idstr)
 
 function generateCertificateOfMembership(nwid,peerAddress,callback)
 {
-	var comTimestamp = '0,' + Date.now().toString(16) + ',' + (ZT_NETWORK_AUTOCONF_DELAY * 4).toString(16);
+	// The first fields of these COM tuples come from
+	// CertificateOfMembership.hpp's enum of required
+	// certificate default fields.
+	var comTimestamp = '0,' + Date.now().toString(16) + ',' + ZT_NETWORK_CERTIFICATE_TTL_WINDOW.toString(16);
 	var comNwid = '1,' + nwid + ',0';
 	var comIssuedTo = '2,' + peerAddress + ',ffffffffffffffff';
+
 	var cert = '';
 	var certErr = '';
+
 	var idtool = spawn(ZEROTIER_IDTOOL,[ 'mkcom',netconfSigningIdentity,comTimestamp,comNwid,comIssuedTo ]);
 	idtool.stdout.on('data',function(data) {
 		cert += data;
@@ -332,14 +336,18 @@ function doNetconfRequest(message)
 		v4NeedAssign = (network['v4AssignMode'] === 'zt');
 		v6NeedAssign = (network['v6AssignMode'] === 'zt');
 
-		var ipa = csvToArray(member['ipAssignments']);
+		var ipacsv = member['ipAssignments'];
-		for(var i=0;i<ipa.length;++i) {
+		if (ipacsv) {
-			if (ipa[i])
+			var ipa = ipacsv.split(',');
-				ipAssignments.push(ipa[i]);
+			for(var i=0;i<ipa.length;++i) {
-			if ((ipa[i].indexOf('.') > 0)&&(v4NeedAssign))
+				if (ipa[i]) {
-				v4Assignments.push(ipa[i]);
+ 					ipAssignments.push(ipa[i]);
-			else if ((ipa[i].indexOf(':') > 0)&&(v6NeedAssign))
+					if ((ipa[i].indexOf('.') > 0)&&(v4NeedAssign))
-				v6Assignments.push(ipa[i]);
+						v4Assignments.push(ipa[i]);
+					else if ((ipa[i].indexOf(':') > 0)&&(v6NeedAssign))
+						v6Assignments.push(ipa[i]);
+				}
+			}
 		}
 
 		return next(null);

node/Constants.hpp

@@ -366,6 +366,14 @@ error_no_byte_order_defined;
  */
 #define ZT_ANTIRECURSION_HISTORY_SIZE 16
 
+/**
+ * TTL for certificates of membership on private networks
+ *
+ * This is the max delta for the timestamp field of a COM, so it's a window
+ * plus or minus the certificate's timestamp. In milliseconds.
+ */
+#define ZT_NETWORK_CERTIFICATE_TTL_WINDOW (ZT_NETWORK_AUTOCONF_DELAY * 4)
+
 /**
  * How often to broadcast beacons over physical local LANs
  */

node/Service.cpp

@@ -43,6 +43,7 @@
 #include <sys/select.h>
 #include <sys/wait.h>
 
+#include "Constants.hpp"
 #include "Service.hpp"
 #include "RuntimeEnvironment.hpp"
 #include "Utils.hpp"
@@ -91,32 +92,19 @@ bool Service::send(const Dictionary &msg)
 {
 	if (_childStdin <= 0)
 		return false;
-
+	std::string mser(msg.toString());
-	std::string mser = msg.toString();
+	mser.append(ZT_EOL_S);
-	if (mser.length() > ZT_SERVICE_MAX_MESSAGE_SIZE)
+	return ((long)::write(_childStdin,mser.data(),mser.length()) == (long)mser.length());
-		return false;
-
-	// This can technically block. We'll fix this if it ends up being a
-	// problem.
-	uint32_t len = Utils::hton((uint32_t)mser.length());
-	if (write(_childStdin,&len,4) != 4)
-		return false;
-	if ((int)write(_childStdin,mser.data(),mser.length()) != (int)mser.length())
-		return false;
-
-	return true;
 }
 
 void Service::threadMain()
 	throw()
 {
-	char buf[131072];
+	char buf[16384];
 	fd_set readfds,writefds,exceptfds;
 	struct timeval tv;
-
+	int eolsInARow = 0;
-	std::string stderrBuf;
+	std::string stderrBuf,stdoutBuf;
-	std::string stdoutBuf;
-	unsigned int stdoutExpecting = 0;
 
 	while (_run) {
 		if (_pid <= 0) {
@@ -184,18 +172,18 @@ void Service::threadMain()
 
 		tv.tv_sec = 1;
 		tv.tv_usec = 0;
-		select(std::max(_childStdout,_childStderr)+1,&readfds,&writefds,&exceptfds,&tv);
+		::select(std::max(_childStdout,_childStderr)+1,&readfds,&writefds,&exceptfds,&tv);
 
 		if (!_run) {
-			if (_childStdin > 0) close(_childStdin);
+			if (_childStdin > 0) ::close(_childStdin);
 			_childStdin = 0;
-			if (_childStdout > 0) close(_childStdout);
+			if (_childStdout > 0) ::close(_childStdout);
-			if (_childStderr > 0) close(_childStderr);
+			if (_childStderr > 0) ::close(_childStderr);
 			return;
 		}
 
 		if ((_childStderr > 0)&&(FD_ISSET(_childStderr,&readfds))) {
-			int n = (int)read(_childStderr,buf,sizeof(buf));
+			int n = (int)::read(_childStderr,buf,sizeof(buf));
 			for(int i=0;i<n;++i) {
 				if ((buf[i] == '\r')||(buf[i] == '\n')) {
 					stderrBuf = Utils::trim(stderrBuf);
@@ -207,29 +195,20 @@ void Service::threadMain()
 		}
 
 		if ((_childStdout > 0)&&(FD_ISSET(_childStdout,&readfds))) {
-			int n = (int)read(_childStdout,buf,sizeof(buf));
+			int n = (int)::read(_childStdout,buf,sizeof(buf));
 			for(int i=0;i<n;++i) {
-				stdoutBuf.push_back(buf[i]);
+				if ((buf[i] == '\n')||(buf[i] == '\r')) {
-				if (stdoutExpecting) {
+					if (buf[i] == '\n')
-					if (stdoutBuf.length() == stdoutExpecting) {
+						++eolsInARow;
-						try {
+				} else eolsInARow = 0;
-							_handler(_arg,*this,Dictionary(stdoutBuf));
+
-						} catch ( ... ) {
+				if (eolsInARow >= 2) {
-							LOG("unexpected exception handling message from service %s",_name.c_str());
+					// Two CRs in a row ends a message
-						}
+					try {
+						_handler(_arg,*this,Dictionary(stdoutBuf));
 						stdoutBuf = "";
-						stdoutExpecting = 0;
+					} catch ( ... ) {} // handlers should not throw
-					}
+				} else stdoutBuf.push_back(buf[i]);
-				} else if (stdoutBuf.length() == 4) {
-					stdoutExpecting = Utils::ntoh(*((const uint32_t *)stdoutBuf.data()));
-					stdoutBuf = "";
-					if (stdoutExpecting > ZT_SERVICE_MAX_MESSAGE_SIZE) {
-						LOG("message size overrun from service %s: %u bytes -- restarting service",_name.c_str(),stdoutExpecting);
-						stdoutExpecting = 0;
-						kill(_pid,SIGKILL);
-						break;
-					}
-				}
 			}
 		}
 	}

node/Service.hpp

@@ -34,12 +34,6 @@
 #include "Constants.hpp"
 #include "Dictionary.hpp"
 #include "Thread.hpp"
-#include "Mutex.hpp"
-
-/**
- * Maximum size of a service message in bytes (sanity limit)
- */
-#define ZT_SERVICE_MAX_MESSAGE_SIZE 131072
 
 namespace ZeroTier {
 
@@ -91,20 +85,12 @@ public:
 	/**
 	 * @return Name of service
 	 */
-	inline const char *name() const
+	inline const char *name() const throw() { return _name.c_str(); }
-		throw()
-	{
-		return _name.c_str();
-	}
 
 	/**
 	 * @return True if subprocess is running
 	 */
-	inline bool running() const
+	inline bool running() const throw() { return (_pid > 0); }
-		throw()
-	{
-		return (_pid > 0);
-	}
 
 	/**
 	 * Thread main method; do not call elsewhere
@@ -114,15 +100,19 @@ public:
 
 private:
 	const RuntimeEnvironment *_r;
+
 	Thread _thread;
+
 	std::string _path;
 	std::string _name;
 	void *_arg;
 	void (*_handler)(void *,Service &,const Dictionary &);
-	long _pid;
+	volatile long _pid;
-	int _childStdin;
+
-	int _childStdout;
+	volatile int _childStdin;
-	int _childStderr;
+	volatile int _childStdout;
+	volatile int _childStderr;
+
 	volatile bool _run;
 };
 #endif // __WINDOWS__