From c863ff3f02e9d68eb9bea32160d252eaddb7f1f5 Mon Sep 17 00:00:00 2001 From: Adam Ierymenko Date: Tue, 7 Jul 2015 08:54:48 -0700 Subject: [PATCH] A bunch of comments and cleanup, including some to yesterday's direct path pushing changes. Move path viability check to one place, and stop trying to use link-local addresses since they are not reliable. --- include/ZeroTierOne.h | 9 +++++- node/IncomingPacket.cpp | 66 +++++++++++++++++++++++++++++------------ node/Node.cpp | 22 +++++++++----- node/Node.hpp | 2 +- node/Packet.hpp | 7 +++-- node/Path.hpp | 33 +++++++++++++++++++++ service/OneService.cpp | 32 +++----------------- 7 files changed, 111 insertions(+), 60 deletions(-) diff --git a/include/ZeroTierOne.h b/include/ZeroTierOne.h index 4790795e2..446bbc774 100644 --- a/include/ZeroTierOne.h +++ b/include/ZeroTierOne.h @@ -978,12 +978,19 @@ void ZT1_Node_freeQueryResult(ZT1_Node *node,void *qr); * Take care that these are never ZeroTier interface addresses, otherwise * strange things might happen or they simply won't work. * + * This returns a boolean indicating whether or not the address was + * accepted. ZeroTier will only communicate over certain address types + * and (for IP) address classes. Thus it's safe to just dump your OS's + * entire remote IP list (excluding ZeroTier interface IPs) into here + * and let ZeroTier determine which addresses it will use. + * * @param addr Local interface address * @param metric Local interface metric * @param trust How much do you trust the local network under this interface? * @param reliable If nonzero, this interface doesn't link to anything behind a NAT or stateful firewall + * @return Boolean: non-zero if address was accepted and added */ -void ZT1_Node_addLocalInterfaceAddress(ZT1_Node *node,const struct sockaddr_storage *addr,int metric,ZT1_LocalInterfaceAddressTrust trust,int reliable); +int ZT1_Node_addLocalInterfaceAddress(ZT1_Node *node,const struct sockaddr_storage *addr,int metric,ZT1_LocalInterfaceAddressTrust trust,int reliable); /** * Clear local interface addresses diff --git a/node/IncomingPacket.cpp b/node/IncomingPacket.cpp index 801591948..7e8832212 100644 --- a/node/IncomingPacket.cpp +++ b/node/IncomingPacket.cpp @@ -134,6 +134,9 @@ bool IncomingPacket::_doERROR(const RuntimeEnvironment *RR,const SharedPtr break; case Packet::ERROR_NEED_MEMBERSHIP_CERTIFICATE: { + /* Note: certificates are public so it's safe to push them to anyone + * who asks. We won't communicate unless we also get a certificate + * from the remote that agrees. */ SharedPtr network(RR->node->network(at(ZT_PROTO_VERB_ERROR_IDX_PAYLOAD))); if (network) { SharedPtr nconf(network->config2()); @@ -170,8 +173,20 @@ bool IncomingPacket::_doERROR(const RuntimeEnvironment *RR,const SharedPtr bool IncomingPacket::_doHELLO(const RuntimeEnvironment *RR) { + /* Note: this is the only packet ever sent in the clear, and it's also + * the only packet that we authenticate via a different path. Authentication + * occurs here and is based on the validity of the identity and the + * integrity of the packet's MAC, but it must be done after we check + * the identity since HELLO is a mechanism for learning new identities + * in the first place. */ + try { const unsigned int protoVersion = (*this)[ZT_PROTO_VERB_HELLO_IDX_PROTOCOL_VERSION]; + if (protoVersion < ZT_PROTO_VERSION_MIN) { + TRACE("dropped HELLO from %s(%s): protocol version too old",id.address().toString().c_str(),_remoteAddress.toString().c_str()); + return true; + } + const unsigned int vMajor = (*this)[ZT_PROTO_VERB_HELLO_IDX_MAJOR_VERSION]; const unsigned int vMinor = (*this)[ZT_PROTO_VERB_HELLO_IDX_MINOR_VERSION]; const unsigned int vRevision = at(ZT_PROTO_VERB_HELLO_IDX_REVISION); @@ -179,6 +194,10 @@ bool IncomingPacket::_doHELLO(const RuntimeEnvironment *RR) Identity id; unsigned int destAddrPtr = id.deserialize(*this,ZT_PROTO_VERB_HELLO_IDX_IDENTITY) + ZT_PROTO_VERB_HELLO_IDX_IDENTITY; + if (source() != id.address()) { + TRACE("dropped HELLO from %s(%s): identity not for sending address",source().toString().c_str(),_remoteAddress.toString().c_str()); + return true; + } InetAddress destAddr; if (destAddrPtr < size()) { // ZeroTier One < 1.0.3 did not include this field @@ -193,16 +212,6 @@ bool IncomingPacket::_doHELLO(const RuntimeEnvironment *RR) } } - if (source() != id.address()) { - TRACE("dropped HELLO from %s(%s): identity not for sending address",source().toString().c_str(),_remoteAddress.toString().c_str()); - return true; - } - - if (protoVersion < ZT_PROTO_VERSION_MIN) { - TRACE("dropped HELLO from %s(%s): protocol version too old",id.address().toString().c_str(),_remoteAddress.toString().c_str()); - return true; - } - SharedPtr peer(RR->topology->getPeer(id.address())); if (peer) { // We already have an identity with this address -- check for collisions @@ -245,12 +254,14 @@ bool IncomingPacket::_doHELLO(const RuntimeEnvironment *RR) } else { // We don't already have an identity with this address -- validate and learn it + // Check identity proof of work if (!id.locallyValidate()) { RR->node->postEvent(ZT1_EVENT_AUTHENTICATION_FAILURE,(const void *)&_remoteAddress); TRACE("dropped HELLO from %s(%s): identity invalid",id.address().toString().c_str(),_remoteAddress.toString().c_str()); return true; } + // Check packet integrity and authentication SharedPtr newPeer(new Peer(RR->identity,id)); if (!dearmor(newPeer->key())) { RR->node->postEvent(ZT1_EVENT_AUTHENTICATION_FAILURE,(const void *)&_remoteAddress); @@ -554,14 +565,17 @@ bool IncomingPacket::_doEXT_FRAME(const RuntimeEnvironment *RR,const SharedPtr

validateAndAddMembershipCertificate(com); + if (com.hasRequiredFields()) { + if (!network->validateAndAddMembershipCertificate(com)) + comFailed = true; // technically this check is redundant to isAllowed(), but do it anyway for thoroughness + } } - if (!network->isAllowed(peer->address())) { + if ((comFailed)||(!network->isAllowed(peer->address()))) { TRACE("dropped EXT_FRAME from %s(%s): not a member of private network %.16llx",peer->address().toString().c_str(),_remoteAddress.toString().c_str(),network->id()); _sendErrorNeedCertificate(RR,peer,network->id()); return true; @@ -647,8 +661,21 @@ bool IncomingPacket::_doNETWORK_MEMBERSHIP_CERTIFICATE(const RuntimeEnvironment ptr += com.deserialize(*this,ptr); if (com.hasRequiredFields()) { SharedPtr network(RR->node->network(com.networkId())); - if (network) - network->validateAndAddMembershipCertificate(com); + if (network) { + if (network->validateAndAddMembershipCertificate(com)) { + if ((network->isAllowed(peer->address()))&&(network->peerNeedsOurMembershipCertificate(peer->address(),RR->node->now()))) { + // If peer passed our check and we haven't sent it our cert yet, respond + // and push our cert as well for instant authorization setup. + SharedPtr nconf(network->config2()); + if ((nconf)&&(nconf->com())) { + Packet outp(peer->address(),RR->identity.address(),Packet::VERB_NETWORK_MEMBERSHIP_CERTIFICATE); + nconf->com().serialize(outp); + outp.armor(peer->key(),true); + RR->node->putPacket(_remoteAddress,outp.data(),outp.size()); + } + } + } + } } } @@ -890,24 +917,25 @@ bool IncomingPacket::_doPUSH_DIRECT_PATHS(const RuntimeEnvironment *RR,const Sha unsigned int ptr = ZT_PACKET_IDX_PAYLOAD + 2; while (count) { // if ptr overflows Buffer will throw + // TODO: properly handle blacklisting, support other features... see Packet.hpp. + unsigned int flags = (*this)[ptr++]; /*int metric = (*this)[ptr++];*/ ++ptr; unsigned int extLen = at(ptr); ptr += 2; ptr += extLen; // unused right now unsigned int addrType = (*this)[ptr++]; + unsigned int addrLen = (*this)[ptr++]; switch(addrType) { case 4: { InetAddress a(field(ptr,4),4,at(ptr + 4)); - if ((flags & (0x01 | 0x02)) == 0) { + if ( ((flags & (0x01 | 0x02)) == 0) && (Path::isAddressValidForPath(a)) ) peer->attemptToContactAt(RR,a,RR->node->now()); - } } break; case 6: { InetAddress a(field(ptr,16),16,at(ptr + 16)); - if ((flags & (0x01 | 0x02)) == 0) { + if ( ((flags & (0x01 | 0x02)) == 0) && (Path::isAddressValidForPath(a)) ) peer->attemptToContactAt(RR,a,RR->node->now()); - } } break; } ptr += addrLen; diff --git a/node/Node.cpp b/node/Node.cpp index 057dc285c..d8bd8910f 100644 --- a/node/Node.cpp +++ b/node/Node.cpp @@ -426,12 +426,16 @@ void Node::freeQueryResult(void *qr) ::free(qr); } -void Node::addLocalInterfaceAddress(const struct sockaddr_storage *addr,int metric,ZT1_LocalInterfaceAddressTrust trust,int reliable) +int Node::addLocalInterfaceAddress(const struct sockaddr_storage *addr,int metric,ZT1_LocalInterfaceAddressTrust trust,int reliable) { - Mutex::Lock _l(_directPaths_m); - _directPaths.push_back(Path(*(reinterpret_cast(addr)),metric,(Path::Trust)trust,reliable != 0)); - std::sort(_directPaths.begin(),_directPaths.end()); - _directPaths.erase(std::unique(_directPaths.begin(),_directPaths.end()),_directPaths.end()); + if (Path::isAddressValidForPath(*(reinterpret_cast(addr)))) { + Mutex::Lock _l(_directPaths_m); + _directPaths.push_back(Path(*(reinterpret_cast(addr)),metric,(Path::Trust)trust,reliable != 0)); + std::sort(_directPaths.begin(),_directPaths.end()); + _directPaths.erase(std::unique(_directPaths.begin(),_directPaths.end()),_directPaths.end()); + return 1; + } + return 0; } void Node::clearLocalInterfaceAddresses() @@ -693,11 +697,13 @@ void ZT1_Node_setNetconfMaster(ZT1_Node *node,void *networkControllerInstance) } catch ( ... ) {} } -void ZT1_Node_addLocalInterfaceAddress(ZT1_Node *node,const struct sockaddr_storage *addr,int metric,ZT1_LocalInterfaceAddressTrust trust,int reliable) +int ZT1_Node_addLocalInterfaceAddress(ZT1_Node *node,const struct sockaddr_storage *addr,int metric,ZT1_LocalInterfaceAddressTrust trust,int reliable) { try { - reinterpret_cast(node)->addLocalInterfaceAddress(addr,metric,trust,reliable); - } catch ( ... ) {} + return reinterpret_cast(node)->addLocalInterfaceAddress(addr,metric,trust,reliable); + } catch ( ... ) { + return 0; + } } void ZT1_Node_clearLocalInterfaceAddresses(ZT1_Node *node) diff --git a/node/Node.hpp b/node/Node.hpp index 1c260545c..fe31576c9 100644 --- a/node/Node.hpp +++ b/node/Node.hpp @@ -104,7 +104,7 @@ public: ZT1_VirtualNetworkConfig *networkConfig(uint64_t nwid) const; ZT1_VirtualNetworkList *networks() const; void freeQueryResult(void *qr); - void addLocalInterfaceAddress(const struct sockaddr_storage *addr,int metric,ZT1_LocalInterfaceAddressTrust trust,int reliable); + int addLocalInterfaceAddress(const struct sockaddr_storage *addr,int metric,ZT1_LocalInterfaceAddressTrust trust,int reliable); void clearLocalInterfaceAddresses(); void setNetconfMaster(void *networkControllerInstance); diff --git a/node/Packet.hpp b/node/Packet.hpp index 51a241ba9..9787edb74 100644 --- a/node/Packet.hpp +++ b/node/Packet.hpp @@ -678,9 +678,10 @@ public: * <[...] serialized certificate of membership> * [ ... additional certificates may follow ...] * - * Certificate contains network ID, peer it was issued for, etc. - * - * OK/ERROR are not generated. + * OK/ERROR are not generated, but the recipient should push its network + * membership certificate if the certificate the peer pushed is valid + * and agrees and if it hasn't done so in too long. This ensures instant + * network authentication setup between valid and authorized peers. */ VERB_NETWORK_MEMBERSHIP_CERTIFICATE = 10, diff --git a/node/Path.hpp b/node/Path.hpp index 80b9a3c01..cd21444b8 100644 --- a/node/Path.hpp +++ b/node/Path.hpp @@ -94,6 +94,39 @@ public: inline bool operator<=(const Path &p) const throw() { return (_addr <= p._addr); } inline bool operator>=(const Path &p) const throw() { return (_addr >= p._addr); } + /** + * Check whether this address is valid for a ZeroTier path + * + * This checks the address type and scope against address types and scopes + * that we currently support for ZeroTier communication. + * + * @param a Address to check + * @return True if address is good for ZeroTier path use + */ + static inline bool isAddressValidForPath(const InetAddress &a) + throw() + { + if ((a.ss_family == AF_INET)||(a.ss_family == AF_INET6)) { + switch(a.ipScope()) { + /* Note: we don't do link-local at the moment. Unfortunately these + * cause several issues. The first is that they usually require a + * device qualifier, which we don't handle yet and can't portably + * push in PUSH_DIRECT_PATHS. The second is that some OSes assign + * these very ephemerally or otherwise strangely. So we'll use + * private, pseudo-private, shared (e.g. carrier grade NAT), or + * global IP addresses. */ + case InetAddress::IP_SCOPE_PRIVATE: + case InetAddress::IP_SCOPE_PSEUDOPRIVATE: + case InetAddress::IP_SCOPE_SHARED: + case InetAddress::IP_SCOPE_GLOBAL: + return true; + default: + return false; + } + } + return false; + } + protected: InetAddress _addr; int _metric; // negative == blacklisted diff --git a/service/OneService.cpp b/service/OneService.cpp index 527ac1b03..bde59d56c 100644 --- a/service/OneService.cpp +++ b/service/OneService.cpp @@ -591,20 +591,8 @@ public: } if (!isZT) { InetAddress ip(ifa->ifa_addr); - if ((ip.ss_family == AF_INET)||(ip.ss_family == AF_INET6)) { - switch(ip.ipScope()) { - case InetAddress::IP_SCOPE_LINK_LOCAL: - case InetAddress::IP_SCOPE_PRIVATE: - case InetAddress::IP_SCOPE_PSEUDOPRIVATE: - case InetAddress::IP_SCOPE_SHARED: - case InetAddress::IP_SCOPE_GLOBAL: - ip.setPort(_port); - _node->addLocalInterfaceAddress(reinterpret_cast(&ip),0,ZT1_LOCAL_INTERFACE_ADDRESS_TRUST_NORMAL,0); - break; - default: - break; - } - } + ip.setPort(_port); + _node->addLocalInterfaceAddress(reinterpret_cast(&ip),0,ZT1_LOCAL_INTERFACE_ADDRESS_TRUST_NORMAL,0); } } ifa = ifa->ifa_next; @@ -637,20 +625,8 @@ public: PIP_ADAPTER_UNICAST_ADDRESS ua = a->FirstUnicastAddress; while (ua) { InetAddress ip(ua->Address.lpSockaddr); - if ((ip.ss_family == AF_INET)||(ip.ss_family == AF_INET6)) { - switch(ip.ipScope()) { - case InetAddress::IP_SCOPE_LINK_LOCAL: - case InetAddress::IP_SCOPE_PRIVATE: - case InetAddress::IP_SCOPE_PSEUDOPRIVATE: - case InetAddress::IP_SCOPE_SHARED: - case InetAddress::IP_SCOPE_GLOBAL: - ip.setPort(_port); - _node->addLocalInterfaceAddress(reinterpret_cast(&ip),0,ZT1_LOCAL_INTERFACE_ADDRESS_TRUST_NORMAL,0); - break; - default: - break; - } - } + ip.setPort(_port); + _node->addLocalInterfaceAddress(reinterpret_cast(&ip),0,ZT1_LOCAL_INTERFACE_ADDRESS_TRUST_NORMAL,0); ua = ua->Next; } }