From b2d048aa0e01a350eaf524cc752ca5fa9a5a1140 Mon Sep 17 00:00:00 2001 From: Adam Ierymenko Date: Tue, 21 Jun 2016 07:32:58 -0700 Subject: [PATCH] Make Dictionary templatable so it can be used where we want a higher capacity. --- controller/SqliteNetworkController.cpp | 4 +- controller/SqliteNetworkController.hpp | 4 +- node/Dictionary.hpp | 52 +++++++++++++++----------- node/IncomingPacket.cpp | 6 +-- node/Network.cpp | 6 +-- node/NetworkConfig.cpp | 10 ++--- node/NetworkConfig.hpp | 7 +++- node/NetworkController.hpp | 6 +-- osdep/LinuxEthernetTap.cpp | 2 +- osdep/OSXEthernetTap.cpp | 2 +- selftest.cpp | 8 ++-- service/OneService.cpp | 9 ++++- 12 files changed, 67 insertions(+), 49 deletions(-) diff --git a/controller/SqliteNetworkController.cpp b/controller/SqliteNetworkController.cpp index 62808de0e..e68ad143b 100644 --- a/controller/SqliteNetworkController.cpp +++ b/controller/SqliteNetworkController.cpp @@ -402,7 +402,7 @@ SqliteNetworkController::~SqliteNetworkController() } } -NetworkController::ResultCode SqliteNetworkController::doNetworkConfigRequest(const InetAddress &fromAddr,const Identity &signingId,const Identity &identity,uint64_t nwid,const Dictionary &metaData,NetworkConfig &nc) +NetworkController::ResultCode SqliteNetworkController::doNetworkConfigRequest(const InetAddress &fromAddr,const Identity &signingId,const Identity &identity,uint64_t nwid,const Dictionary &metaData,NetworkConfig &nc) { Mutex::Lock _l(_lock); return _doNetworkConfigRequest(fromAddr,signingId,identity,nwid,metaData,nc); @@ -1576,7 +1576,7 @@ unsigned int SqliteNetworkController::_doCPGet( return 404; } -NetworkController::ResultCode SqliteNetworkController::_doNetworkConfigRequest(const InetAddress &fromAddr,const Identity &signingId,const Identity &identity,uint64_t nwid,const Dictionary &metaData,NetworkConfig &nc) +NetworkController::ResultCode SqliteNetworkController::_doNetworkConfigRequest(const InetAddress &fromAddr,const Identity &signingId,const Identity &identity,uint64_t nwid,const Dictionary &metaData,NetworkConfig &nc) { // Assumes _lock is locked diff --git a/controller/SqliteNetworkController.hpp b/controller/SqliteNetworkController.hpp index 3bb53bd96..02aeec106 100644 --- a/controller/SqliteNetworkController.hpp +++ b/controller/SqliteNetworkController.hpp @@ -62,7 +62,7 @@ public: const Identity &signingId, const Identity &identity, uint64_t nwid, - const Dictionary &metaData, + const Dictionary &metaData, NetworkConfig &nc); unsigned int handleControlPlaneHttpGET( @@ -113,7 +113,7 @@ private: const Identity &signingId, const Identity &identity, uint64_t nwid, - const Dictionary &metaData, + const Dictionary &metaData, NetworkConfig &nc); static void _circuitTestCallback(ZT_Node *node,ZT_CircuitTest *test,const ZT_CircuitTestReport *report); diff --git a/node/Dictionary.hpp b/node/Dictionary.hpp index 658878f12..dd6c24f57 100644 --- a/node/Dictionary.hpp +++ b/node/Dictionary.hpp @@ -26,9 +26,6 @@ #include -// Can be increased if it's ever needed, but not too much. -#define ZT_DICTIONARY_MAX_SIZE 8194 - namespace ZeroTier { /** @@ -48,7 +45,10 @@ namespace ZeroTier { * * There is code to test and fuzz this in selftest.cpp. Fuzzing a blob of * pointer tricks like this is important after any modifications. + * + * @tparam C Dictionary max capacity in bytes */ +template class Dictionary { public: @@ -64,8 +64,8 @@ public: Dictionary(const char *s,unsigned int len) { - memcpy(_d,s,(len > ZT_DICTIONARY_MAX_SIZE) ? (unsigned int)ZT_DICTIONARY_MAX_SIZE : len); - _d[ZT_DICTIONARY_MAX_SIZE-1] = (char)0; + memcpy(_d,s,(len > C) ? (unsigned int)C : len); + _d[C-1] = (char)0; } Dictionary(const Dictionary &d) @@ -83,7 +83,7 @@ public: * Load a dictionary from a C-string * * @param s Dictionary in string form - * @return False if 's' was longer than ZT_DICTIONARY_MAX_SIZE + * @return False if 's' was longer than our capacity */ inline bool load(const char *s) { @@ -103,11 +103,11 @@ public: */ inline unsigned int sizeBytes() const { - for(unsigned int i=0;i - inline bool get(const char *key,Buffer &dest) const + template + inline bool get(const char *key,Buffer &dest) const { const int r = this->get(key,const_cast(reinterpret_cast(dest.data())),C); if (r >= 0) { @@ -254,13 +255,13 @@ public: */ inline bool add(const char *key,const char *value,int vlen = -1) { - for(unsigned int i=0;i 0) { _d[j++] = '\n'; - if (j == ZT_DICTIONARY_MAX_SIZE) { + if (j == C) { _d[i] = (char)0; return false; } @@ -269,14 +270,14 @@ public: const char *p = key; while (*p) { _d[j++] = *(p++); - if (j == ZT_DICTIONARY_MAX_SIZE) { + if (j == C) { _d[i] = (char)0; return false; } } _d[j++] = '='; - if (j == ZT_DICTIONARY_MAX_SIZE) { + if (j == C) { _d[i] = (char)0; return false; } @@ -291,7 +292,7 @@ public: case '\\': case '=': _d[j++] = '\\'; - if (j == ZT_DICTIONARY_MAX_SIZE) { + if (j == C) { _d[i] = (char)0; return false; } @@ -302,14 +303,14 @@ public: case '\\': _d[j++] = '\\'; break; case '=': _d[j++] = 'e'; break; } - if (j == ZT_DICTIONARY_MAX_SIZE) { + if (j == C) { _d[i] = (char)0; return false; } break; default: _d[j++] = *p; - if (j == ZT_DICTIONARY_MAX_SIZE) { + if (j == C) { _d[i] = (char)0; return false; } @@ -356,10 +357,12 @@ public: } /** - * Add a binary buffer + * Add a binary buffer's contents as a value + * + * @tparam BC Buffer capacity (usually inferred) */ - template - inline bool add(const char *key,const Buffer &value) + template + inline bool add(const char *key,const Buffer &value) { return this->add(key,(const char *)value.data(),(int)value.size()); } @@ -385,7 +388,7 @@ public: */ inline bool erase(const char *key) { - char d2[ZT_DICTIONARY_MAX_SIZE]; + char d2[C]; char *saveptr = (char *)0; unsigned int d2ptr = 0; bool found = false; @@ -419,8 +422,13 @@ public: */ inline const char *data() const { return _d; } + /** + * @return Value of C template parameter + */ + inline unsigned int capacity() const { return C; } + private: - char _d[ZT_DICTIONARY_MAX_SIZE]; + char _d[C]; }; } // namespace ZeroTier diff --git a/node/IncomingPacket.cpp b/node/IncomingPacket.cpp index f57fa2a8e..532abafa6 100644 --- a/node/IncomingPacket.cpp +++ b/node/IncomingPacket.cpp @@ -403,7 +403,7 @@ bool IncomingPacket::_doOK(const RuntimeEnvironment *RR,const SharedPtr &p if ((nw)&&(nw->controller() == peer->address())) { const unsigned int nclen = at(ZT_PROTO_VERB_NETWORK_CONFIG_REQUEST__OK__IDX_DICT_LEN); if (nclen) { - Dictionary dconf((const char *)field(ZT_PROTO_VERB_NETWORK_CONFIG_REQUEST__OK__IDX_DICT,nclen),nclen); + Dictionary dconf((const char *)field(ZT_PROTO_VERB_NETWORK_CONFIG_REQUEST__OK__IDX_DICT,nclen),nclen); NetworkConfig nconf; if (nconf.fromDictionary(dconf)) { nw->setConfiguration(nconf,true); @@ -684,7 +684,7 @@ bool IncomingPacket::_doNETWORK_CONFIG_REQUEST(const RuntimeEnvironment *RR,cons const unsigned int metaDataLength = at(ZT_PROTO_VERB_NETWORK_CONFIG_REQUEST_IDX_DICT_LEN); const char *metaDataBytes = (const char *)field(ZT_PROTO_VERB_NETWORK_CONFIG_REQUEST_IDX_DICT,metaDataLength); - const Dictionary metaData(metaDataBytes,metaDataLength); + const Dictionary metaData(metaDataBytes,metaDataLength); //const uint64_t haveRevision = ((ZT_PROTO_VERB_NETWORK_CONFIG_REQUEST_IDX_DICT + metaDataLength + 8) <= size()) ? at(ZT_PROTO_VERB_NETWORK_CONFIG_REQUEST_IDX_DICT + metaDataLength) : 0ULL; @@ -697,7 +697,7 @@ bool IncomingPacket::_doNETWORK_CONFIG_REQUEST(const RuntimeEnvironment *RR,cons switch(RR->localNetworkController->doNetworkConfigRequest((h > 0) ? InetAddress() : _remoteAddress,RR->identity,peer->identity(),nwid,metaData,netconf)) { case NetworkController::NETCONF_QUERY_OK: { - Dictionary dconf; + Dictionary dconf; if (netconf.toDictionary(dconf,metaData.getUI(ZT_NETWORKCONFIG_REQUEST_METADATA_KEY_VERSION,0) < 6)) { Packet outp(peer->address(),RR->identity.address(),Packet::VERB_OK); outp.append((unsigned char)Packet::VERB_NETWORK_CONFIG_REQUEST); diff --git a/node/Network.cpp b/node/Network.cpp index 82011c3d5..251166479 100644 --- a/node/Network.cpp +++ b/node/Network.cpp @@ -64,7 +64,7 @@ Network::Network(const RuntimeEnvironment *renv,uint64_t nwid,void *uptr) : try { std::string conf(RR->node->dataStoreGet(confn)); if (conf.length()) { - Dictionary dconf(conf.c_str()); + Dictionary dconf(conf.c_str()); NetworkConfig nconf; if (nconf.fromDictionary(dconf)) { this->setConfiguration(nconf,false); @@ -193,7 +193,7 @@ int Network::setConfiguration(const NetworkConfig &nconf,bool saveToDisk) if (saveToDisk) { char n[64]; Utils::snprintf(n,sizeof(n),"networks.d/%.16llx.conf",_id); - Dictionary d; + Dictionary d; if (nconf.toDictionary(d,false)) RR->node->dataStorePut(n,(const void *)d.data(),d.sizeBytes(),true); } @@ -210,7 +210,7 @@ void Network::requestConfiguration() if (_id == ZT_TEST_NETWORK_ID) // pseudo-network-ID, uses locally generated static config return; - Dictionary rmd; + Dictionary rmd; rmd.add(ZT_NETWORKCONFIG_REQUEST_METADATA_KEY_VERSION,(uint64_t)ZT_NETWORKCONFIG_VERSION); rmd.add(ZT_NETWORKCONFIG_REQUEST_METADATA_KEY_PROTOCOL_VERSION,(uint64_t)ZT_PROTO_VERSION); rmd.add(ZT_NETWORKCONFIG_REQUEST_METADATA_KEY_NODE_MAJOR_VERSION,(uint64_t)ZEROTIER_ONE_VERSION_MAJOR); diff --git a/node/NetworkConfig.cpp b/node/NetworkConfig.cpp index fe4a5a0d8..d906005e7 100644 --- a/node/NetworkConfig.cpp +++ b/node/NetworkConfig.cpp @@ -23,9 +23,9 @@ namespace ZeroTier { -bool NetworkConfig::toDictionary(Dictionary &d,bool includeLegacy) const +bool NetworkConfig::toDictionary(Dictionary &d,bool includeLegacy) const { - Buffer tmp; + Buffer tmp; d.clear(); @@ -259,11 +259,11 @@ bool NetworkConfig::toDictionary(Dictionary &d,bool includeLegacy) const return true; } -bool NetworkConfig::fromDictionary(const Dictionary &d) +bool NetworkConfig::fromDictionary(const Dictionary &d) { try { - Buffer tmp; - char tmp2[ZT_DICTIONARY_MAX_SIZE]; + Buffer tmp; + char tmp2[ZT_NETWORKCONFIG_DICT_CAPACITY]; memset(this,0,sizeof(NetworkConfig)); diff --git a/node/NetworkConfig.hpp b/node/NetworkConfig.hpp index 30d8c9fcc..c137a2a5d 100644 --- a/node/NetworkConfig.hpp +++ b/node/NetworkConfig.hpp @@ -64,6 +64,9 @@ namespace ZeroTier { +// Maximum size of a network config dictionary (can be increased) +#define ZT_NETWORKCONFIG_DICT_CAPACITY 8194 + // Network config version #define ZT_NETWORKCONFIG_VERSION 6 @@ -234,7 +237,7 @@ public: * @param includeLegacy If true, include legacy fields for old node versions * @return True if dictionary was successfully created, false if e.g. overflow */ - bool toDictionary(Dictionary &d,bool includeLegacy) const; + bool toDictionary(Dictionary &d,bool includeLegacy) const; /** * Read this network config from a dictionary @@ -242,7 +245,7 @@ public: * @param d Dictionary * @return True if dictionary was valid and network config successfully initialized */ - bool fromDictionary(const Dictionary &d); + bool fromDictionary(const Dictionary &d); /** * @return True if passive bridging is allowed (experimental) diff --git a/node/NetworkController.hpp b/node/NetworkController.hpp index b91ada1bd..fa90fb759 100644 --- a/node/NetworkController.hpp +++ b/node/NetworkController.hpp @@ -22,12 +22,12 @@ #include #include "Constants.hpp" +#include "Dictionary.hpp" +#include "NetworkConfig.hpp" namespace ZeroTier { class RuntimeEnvironment; -class NetworkConfig; -class Dictionary; class Identity; class Address; struct InetAddress; @@ -75,7 +75,7 @@ public: const Identity &signingId, const Identity &identity, uint64_t nwid, - const Dictionary &metaData, + const Dictionary &metaData, NetworkConfig &nc) = 0; }; diff --git a/osdep/LinuxEthernetTap.cpp b/osdep/LinuxEthernetTap.cpp index c43c7bb7d..b10c4cd17 100644 --- a/osdep/LinuxEthernetTap.cpp +++ b/osdep/LinuxEthernetTap.cpp @@ -95,7 +95,7 @@ LinuxEthernetTap::LinuxEthernetTap( // Try to recall our last device name, or pick an unused one if that fails. bool recalledDevice = false; std::string devmapbuf; - Dictionary devmap; + Dictionary<8194> devmap; if (OSUtils::readFile((_homePath + ZT_PATH_SEPARATOR_S + "devicemap").c_str(),devmapbuf)) { devmap.load(devmapbuf.c_str()); char desiredDevice[128]; diff --git a/osdep/OSXEthernetTap.cpp b/osdep/OSXEthernetTap.cpp index 9921049f6..e8c5c1eac 100644 --- a/osdep/OSXEthernetTap.cpp +++ b/osdep/OSXEthernetTap.cpp @@ -354,7 +354,7 @@ OSXEthernetTap::OSXEthernetTap( // Try to reopen the last device we had, if we had one and it's still unused. bool recalledDevice = false; std::string devmapbuf; - Dictionary devmap; + Dictionary<8194> devmap; if (OSUtils::readFile((_homePath + ZT_PATH_SEPARATOR_S + "devicemap").c_str(),devmapbuf)) { devmap.load(devmapbuf.c_str()); char desiredDevice[128]; diff --git a/selftest.cpp b/selftest.cpp index eeffb9bd4..0f777dfd0 100644 --- a/selftest.cpp +++ b/selftest.cpp @@ -766,7 +766,7 @@ static int testOther() std::cout << "[other] Testing/fuzzing Dictionary... "; std::cout.flush(); for(int k=0;k<1000;++k) { - Dictionary test; + Dictionary<8194> test; char key[32][16]; char value[32][128]; for(unsigned int q=0;q<32;++q) { @@ -807,12 +807,12 @@ static int testOther() int foo = 0; volatile int *volatile bar = &foo; // force compiler not to optimize out test.get() below for(int k=0;k<100;++k) { - int r = rand() % ZT_DICTIONARY_MAX_SIZE; - unsigned char tmp[ZT_DICTIONARY_MAX_SIZE]; + int r = rand() % 8194; + unsigned char tmp[8194]; for(int q=0;q test((const char *)tmp); for(unsigned int q=0;q<100;++q) { char tmp[16]; Utils::snprintf(tmp,16,"%.8lx",(unsigned long)rand()); diff --git a/service/OneService.cpp b/service/OneService.cpp index 865a849e0..804e3d36a 100644 --- a/service/OneService.cpp +++ b/service/OneService.cpp @@ -1571,6 +1571,7 @@ public: inline int nodePathCheckFunction(const struct sockaddr_storage *localAddr,const struct sockaddr_storage *remoteAddr) { Mutex::Lock _l(_nets_m); + for(std::map::const_iterator n(_nets.begin());n!=_nets.end();++n) { if (n->second.tap) { std::vector ips(n->second.tap->ips()); @@ -1581,7 +1582,13 @@ public: } } } - // TODO: also check routing table for L3 routes via ZeroTier managed devices + + /* Note: I do not think we need to scan for overlap with managed routes + * because of the "route forking" and interface binding that we do. This + * ensures (we hope) that ZeroTier traffic will still take the physical + * path even if its managed routes override this for other traffic. Will + * revisit if we see problems with this. */ + return 1; }