Network constructor deuglification, remove unused old encrypt/decrypt methods from Identity.

node/Identity.cpp

@@ -225,81 +225,5 @@ Address Identity::deriveAddress(const void *keyBytes,unsigned int keyLen)
 	return Address(dig,ZT_ADDRESS_LENGTH); // first 5 bytes of dig[]
 }
 
-std::string Identity::encrypt(const Identity &to,const void *data,unsigned int len) const
-{
-	unsigned char key[64];
-	unsigned char mac[32];
-	unsigned char iv[8];
-
-	if (!agree(to,key,sizeof(key)))
-		return std::string();
-	Utils::getSecureRandom(iv,8);
-	for(int i=0;i<8;++i)
-		key[i + 32] ^= iv[i]; // perturb HMAC key with IV so IV is effectively included in HMAC
-	Salsa20 s20(key,256,iv);
-
-	std::string compressed;
-	compressed.reserve(len);
-	Utils::compress((const char *)data,(const char *)data + len,Utils::StringAppendOutput(compressed));
-	if (!compressed.length())
-		return std::string();
-
-	char *encrypted = new char[compressed.length() + 16];
-	try {
-		s20.encrypt(compressed.data(),encrypted + 16,(unsigned int)compressed.length());
-		HMAC::sha256(key + 32,32,encrypted + 16,(unsigned int)compressed.length(),mac);
-		for(int i=0;i<8;++i)
-			encrypted[i] = iv[i];
-		for(int i=0;i<8;++i)
-			encrypted[i + 8] = mac[i];
-
-		std::string s(encrypted,compressed.length() + 16);
-		delete [] encrypted;
-		return s;
-	} catch ( ... ) {
-		delete [] encrypted;
-		return std::string();
-	}
-}
-
-std::string Identity::decrypt(const Identity &from,const void *cdata,unsigned int len) const
-{
-	unsigned char key[64];
-	unsigned char mac[32];
-
-	if (len < 16)
-		return std::string();
-
-	if (!agree(from,key,sizeof(key)))
-		return std::string();
-
-	for(int i=0;i<8;++i)
-		key[i + 32] ^= ((const unsigned char *)cdata)[i]; // apply IV to HMAC key
-	HMAC::sha256(key + 32,32,((const char *)cdata) + 16,(unsigned int)(len - 16),mac);
-	for(int i=0;i<8;++i) {
-		if (((const unsigned char *)cdata)[i + 8] != mac[i])
-			return std::string();
-	}
-
-	char *decbuf = new char[len - 16];
-	try {
-		Salsa20 s20(key,256,cdata); // first 8 bytes are IV
-		len -= 16;
-		s20.decrypt((const char *)cdata + 16,decbuf,len);
-
-		std::string decompressed;
-		if (Utils::decompress((const char *)decbuf,(const char *)decbuf + len,Utils::StringAppendOutput(decompressed))) {
-			delete [] decbuf;
-			return decompressed;
-		} else {
-			delete [] decbuf;
-			return std::string();
-		}
-	} catch ( ... ) {
-		delete [] decbuf;
-		return std::string();
-	}
-}
-
 } // namespace ZeroTier
 

node/Identity.hpp

@@ -175,40 +175,6 @@ public:
 	 */
 	inline bool hasPrivate() const throw() { return (_keyPair); }
 
-	/**
-	 * Encrypt a block of data to send to another identity
-	 *
-	 * This identity must have a secret key.
-	 *
-	 * The encrypted data format is:
-	 *   <[8] Salsa20 initialization vector>
-	 *   <[8] first 8 bytes of HMAC-SHA-256 of ciphertext>
-	 *   <[...] encrypted compressed data>
-	 *
-	 * Keying is accomplished using agree() (KDF function is in the
-	 * EllipticCurveKeyPair.cpp source) to generate 64 bytes of key. The first
-	 * 32 bytes are used as the Salsa20 key, and the last 32 bytes are used
-	 * as the HMAC key.
-	 *
-	 * @param to Identity of recipient of encrypted message
-	 * @param data Data to encrypt
-	 * @param len Length of data
-	 * @return Encrypted data or empty string on failure
-	 */
-	std::string encrypt(const Identity &to,const void *data,unsigned int len) const;
-
-	/**
-	 * Decrypt a message encrypted with encrypt()
-	 *
-	 * This identity must have a secret key.
-	 *
-	 * @param from Identity of sender of encrypted message
-	 * @param cdata Encrypted message
-	 * @param len Length of encrypted message
-	 * @return Decrypted data or empty string on failure
-	 */
-	std::string decrypt(const Identity &from,const void *cdata,unsigned int len) const;
-
 	/**
 	 * Shortcut method to perform key agreement with another identity
 	 *

node/Network.cpp

@@ -104,20 +104,10 @@ bool Network::Certificate::qualifyMembership(const Network::Certificate &mc) con
 	return true;
 }
 
-Network::Network(const RuntimeEnvironment *renv,uint64_t id)
-	throw(std::runtime_error) :
-	_r(renv),
-	_tap(renv,renv->identity.address().toMAC(),ZT_IF_MTU,&_CBhandleTapData,this),
-	_id(id),
-	_lastConfigUpdate(0),
-	_destroyOnDelete(false)
-{
-	if (controller() == _r->identity.address())
-		throw std::runtime_error("cannot add a network for which I am the netconf master");
-}
-
 Network::~Network()
 {
+	delete _tap;
+
 	if (_destroyOnDelete) {
 		std::string confPath(_r->homePath + ZT_PATH_SEPARATOR_S + "networks.d" + ZT_PATH_SEPARATOR_S + toString() + ".conf");
 		std::string mcdbPath(_r->homePath + ZT_PATH_SEPARATOR_S + "networks.d" + ZT_PATH_SEPARATOR_S + toString() + ".mcerts");
@@ -129,26 +119,25 @@ Network::~Network()
 	}
 }
 
-void Network::restoreState()
+SharedPtr<Network> Network::newInstance(const RuntimeEnvironment *renv,uint64_t id)
+	throw(std::runtime_error)
 {
-	std::string confPath(_r->homePath + ZT_PATH_SEPARATOR_S + "networks.d" + ZT_PATH_SEPARATOR_S + toString() + ".conf");
-	std::string confs;
-	if (Utils::readFile(confPath.c_str(),confs)) {
-		try {
-			if (confs.length()) {
-				Config conf(confs);
-				if (conf.containsAllFields())
-					setConfiguration(conf);
-			}
-		} catch ( ... ) {} // ignore invalid config on disk, we will re-request
-	} else {
-		// If the conf file isn't present, "touch" it so we'll remember
-		// the existence of this network.
-		FILE *tmp = fopen(confPath.c_str(),"w");
-		if (tmp)
-			fclose(tmp);
-	}
-	// TODO: restore membership certs
+	// We construct Network via a static method to ensure that it is immediately
+	// wrapped in a SharedPtr<>. Otherwise if there is traffic on the Ethernet
+	// tap device, a SharedPtr<> wrap can occur in the Ethernet frame handler
+	// that then causes the Network instance to be deleted before it is finished
+	// being constructed. C++ edge cases, how I love thee.
+	SharedPtr<Network> nw(new Network());
+	nw->_r = renv;
+	nw->_tap = new EthernetTap(renv,renv->identity.address().toMAC(),ZT_IF_MTU,&_CBhandleTapData,nw.ptr());
+	nw->_id = id;
+	nw->_lastConfigUpdate = 0;
+	nw->_destroyOnDelete = false;
+	if (nw->controller() == renv->identity.address()) // sanity check, this isn't supported for now
+		throw std::runtime_error("cannot add a network for which I am the netconf master");
+	nw->_restoreState();
+	nw->requestConfiguration();
+	return nw;
 }
 
 void Network::setConfiguration(const Network::Config &conf)
@@ -160,7 +149,7 @@ void Network::setConfiguration(const Network::Config &conf)
 		_myCertificate = conf.certificateOfMembership();
 		_lastConfigUpdate = Utils::now();
 
-		_tap.setIps(conf.staticAddresses());
+		_tap->setIps(conf.staticAddresses());
 
 		std::string confPath(_r->homePath + ZT_PATH_SEPARATOR_S + "networks.d" + ZT_PATH_SEPARATOR_S + toString() + ".conf");
 		if (!Utils::writeFile(confPath.c_str(),conf.toString())) {
@@ -210,11 +199,13 @@ bool Network::isAllowed(const Address &peer) const
 
 void Network::clean()
 {
+	std::string mcdbPath(_r->homePath + ZT_PATH_SEPARATOR_S + "networks.d" + ZT_PATH_SEPARATOR_S + toString() + ".mcerts");
+
 	Mutex::Lock _l(_lock);
-	if (_configuration.isOpen())
+	if (_configuration.isOpen()) {
 		_membershipCertificates.clear();
-	else {
-		std::string mcdbPath(_r->homePath + ZT_PATH_SEPARATOR_S + "networks.d" + ZT_PATH_SEPARATOR_S + toString() + ".mcerts");
+		unlink(mcdbPath.c_str());
+	} else {
 		FILE *mcdb = fopen(mcdbPath.c_str(),"wb");
 		bool writeError = false;
 		if (!mcdb) {
@@ -263,4 +254,26 @@ void Network::_CBhandleTapData(void *arg,const MAC &from,const MAC &to,unsigned
 	}
 }
 
+void Network::_restoreState()
+{
+	std::string confPath(_r->homePath + ZT_PATH_SEPARATOR_S + "networks.d" + ZT_PATH_SEPARATOR_S + toString() + ".conf");
+	std::string confs;
+	if (Utils::readFile(confPath.c_str(),confs)) {
+		try {
+			if (confs.length()) {
+				Config conf(confs);
+				if (conf.containsAllFields())
+					setConfiguration(conf);
+			}
+		} catch ( ... ) {} // ignore invalid config on disk, we will re-request
+	} else {
+		// If the conf file isn't present, "touch" it so we'll remember
+		// the existence of this network.
+		FILE *tmp = fopen(confPath.c_str(),"w");
+		if (tmp)
+			fclose(tmp);
+	}
+	// TODO: restore membership certs
+}
+
 } // namespace ZeroTier

node/Network.hpp

@@ -267,26 +267,29 @@ public:
 
 private:
 	// Only NodeConfig can create, only SharedPtr can delete
-	Network(const RuntimeEnvironment *renv,uint64_t id)
-		throw(std::runtime_error);
+
+	// Actual construction happens in newInstance()
+	Network()
+		throw() :
+		_tap((EthernetTap *)0)
+	{
+	}
 
 	~Network();
 
 	/**
-	 * Called by NodeConfig after create
+	 * Create a new Network instance and restore any saved state
 	 *
-	 * This is called separately to avoid a rather evil race condition.
-	 * If config is restored in the constructor, then it's possible that
-	 * the tap will be assigned an IP and will start getting packets
-	 * before SharedPtr<Network> has gotten the pointer from the initial
-	 * object construct. That causes SharedPtr<Network> in the static
-	 * method that handles tap traffic to delete the object, resulting
-	 * in all sorts of utter madness. C++ is crazy like that.
+	 * If there is no saved state, a dummy .conf is created on disk to remember
+	 * this network across restarts.
 	 *
-	 * Actually the way we're using SharedPtr<Network> is hacky and
-	 * ugly, so it's our fault sorta.
+	 * @param renv Runtime environment
+	 * @param id Network ID
+	 * @return Reference counted pointer to new network
+	 * @throws std::runtime_error Unable to create tap device or other fatal error
 	 */
-	void restoreState();
+	static SharedPtr<Network> newInstance(const RuntimeEnvironment *renv,uint64_t id)
+		throw(std::runtime_error);
 
 	/**
 	 * Causes all persistent disk presence to be erased on delete
@@ -305,7 +308,7 @@ public:
 	/**
 	 * @return Ethernet tap
 	 */
-	inline EthernetTap &tap() throw() { return _tap; }
+	inline EthernetTap &tap() throw() { return *_tap; }
 
 	/**
 	 * @return Address of network's controlling node
@@ -344,7 +347,7 @@ public:
 	inline bool updateMulticastGroups()
 	{
 		Mutex::Lock _l(_lock);
-		return _tap.updateMulticastGroups(_multicastGroups);
+		return _tap->updateMulticastGroups(_multicastGroups);
 	}
 
 	/**
@@ -403,11 +406,12 @@ public:
 
 private:
 	static void _CBhandleTapData(void *arg,const MAC &from,const MAC &to,unsigned int etherType,const Buffer<4096> &data);
+	void _restoreState();
 
 	const RuntimeEnvironment *_r;
 
 	// Tap and tap multicast memberships
-	EthernetTap _tap;
+	EthernetTap *_tap;
 	std::set<MulticastGroup> _multicastGroups;
 
 	// Membership certificates supplied by peers

node/NodeConfig.cpp

@@ -76,10 +76,8 @@ NodeConfig::NodeConfig(const RuntimeEnvironment *renv,const char *authToken)
 
 	for(std::set<uint64_t>::iterator nwid(nwids.begin());nwid!=nwids.end();++nwid) {
 		try {
-			SharedPtr<Network> nw(new Network(_r,*nwid));
+			SharedPtr<Network> nw(Network::newInstance(_r,*nwid));
 			_networks[*nwid] = nw;
-			nw->restoreState();
-			nw->requestConfiguration();
 		} catch (std::exception &exc) {
 			LOG("unable to create network %.16llx: %s",(unsigned long long)*nwid,exc.what());
 		} catch ( ... ) {
@@ -181,10 +179,8 @@ std::vector<std::string> NodeConfig::execute(const char *command)
 					_P("400 already a member of %.16llx",(unsigned long long)nwid);
 				} else {
 					try {
-						SharedPtr<Network> nw(new Network(_r,nwid));
+						SharedPtr<Network> nw(Network::newInstance(_r,nwid));
 						_networks[nwid] = nw;
-						nw->restoreState();
-						nw->requestConfiguration();
 						_P("200 join %.16llx OK",(unsigned long long)nwid);
 					} catch (std::exception &exc) {
 						_P("500 join %.16llx ERROR: %s",(unsigned long long)nwid,exc.what());