From c3b5c45feae7d3dbd89a5cb8c6a3a3b5bfaf4b7f Mon Sep 17 00:00:00 2001 From: Adam Ierymenko Date: Thu, 20 Feb 2020 13:55:09 -0800 Subject: [PATCH] Fix InetAddress sizing by delving into crazy C++ weeds, fix Peer compile issues. --- node/InetAddress.cpp | 44 ++++++++++++++++++------------------ node/InetAddress.hpp | 46 +++++++++++++++++++++++--------------- node/Peer.cpp | 25 +++++++++------------ node/Peer.hpp | 3 ++- node/Tests.cpp | 1 + node/TriviallyCopyable.hpp | 5 ++--- 6 files changed, 66 insertions(+), 58 deletions(-) diff --git a/node/InetAddress.cpp b/node/InetAddress.cpp index 16680e933..553897a2e 100644 --- a/node/InetAddress.cpp +++ b/node/InetAddress.cpp @@ -26,7 +26,7 @@ const InetAddress InetAddress::NIL; InetAddress::IpScope InetAddress::ipScope() const noexcept { - switch(ss_family) { + switch(_data.ss_family) { case AF_INET: { const uint32_t ip = Utils::ntoh((uint32_t)reinterpret_cast(this)->sin_addr.s_addr); @@ -100,11 +100,11 @@ void InetAddress::set(const void *ipBytes,unsigned int ipLen,unsigned int port) if (ipLen == 4) { uint32_t ipb[1]; memcpy(ipb,ipBytes,4); - ss_family = AF_INET; + _data.ss_family = AF_INET; reinterpret_cast(this)->sin_addr.s_addr = ipb[0]; reinterpret_cast(this)->sin_port = Utils::hton((uint16_t)port); } else if (ipLen == 16) { - ss_family = AF_INET6; + _data.ss_family = AF_INET6; memcpy(reinterpret_cast(this)->sin6_addr.s6_addr,ipBytes,16); reinterpret_cast(this)->sin6_port = Utils::hton((uint16_t)port); } @@ -112,7 +112,7 @@ void InetAddress::set(const void *ipBytes,unsigned int ipLen,unsigned int port) bool InetAddress::isDefaultRoute() const noexcept { - switch(ss_family) { + switch(_data.ss_family) { case AF_INET: return ( (reinterpret_cast(this)->sin_addr.s_addr == 0) && (reinterpret_cast(this)->sin_port == 0) ); case AF_INET6: @@ -140,7 +140,7 @@ char *InetAddress::toString(char buf[ZT_INETADDRESS_STRING_SIZE_MAX]) const noex char *InetAddress::toIpString(char buf[ZT_INETADDRESS_STRING_SIZE_MAX]) const noexcept { buf[0] = (char)0; - switch(ss_family) { + switch(_data.ss_family) { case AF_INET: { #ifdef _WIN32 inet_ntop(AF_INET, (void*)&reinterpret_cast(this)->sin_addr.s_addr, buf, INET_ADDRSTRLEN); @@ -200,7 +200,7 @@ bool InetAddress::fromString(const char *ipSlashPort) noexcept InetAddress InetAddress::netmask() const noexcept { InetAddress r(*this); - switch(r.ss_family) { + switch(r._data.ss_family) { case AF_INET: reinterpret_cast(&r)->sin_addr.s_addr = Utils::hton((uint32_t)(0xffffffffU << (32 - netmaskBits()))); break; @@ -222,7 +222,7 @@ InetAddress InetAddress::netmask() const noexcept InetAddress InetAddress::broadcast() const noexcept { - if (ss_family == AF_INET) { + if (_data.ss_family == AF_INET) { InetAddress r(*this); reinterpret_cast(&r)->sin_addr.s_addr |= Utils::hton((uint32_t)(0xffffffffU >> netmaskBits())); return r; @@ -233,7 +233,7 @@ InetAddress InetAddress::broadcast() const noexcept InetAddress InetAddress::network() const noexcept { InetAddress r(*this); - switch(r.ss_family) { + switch(r._data.ss_family) { case AF_INET: reinterpret_cast(&r)->sin_addr.s_addr &= Utils::hton((uint32_t)(0xffffffffU << (32 - netmaskBits()))); break; @@ -251,8 +251,8 @@ InetAddress InetAddress::network() const noexcept bool InetAddress::isEqualPrefix(const InetAddress &addr) const noexcept { - if (addr.ss_family == ss_family) { - switch(ss_family) { + if (addr._data.ss_family == _data.ss_family) { + switch(_data.ss_family) { case AF_INET6: { const InetAddress mask(netmask()); InetAddress addr_mask(addr.netmask()); @@ -273,8 +273,8 @@ bool InetAddress::isEqualPrefix(const InetAddress &addr) const noexcept bool InetAddress::containsAddress(const InetAddress &addr) const noexcept { - if (addr.ss_family == ss_family) { - switch(ss_family) { + if (addr._data.ss_family == _data.ss_family) { + switch(_data.ss_family) { case AF_INET: { const unsigned int bits = netmaskBits(); if (bits == 0) @@ -299,9 +299,9 @@ bool InetAddress::containsAddress(const InetAddress &addr) const noexcept unsigned long InetAddress::hashCode() const noexcept { - if (ss_family == AF_INET) { + if (_data.ss_family == AF_INET) { return ((unsigned long)reinterpret_cast(this)->sin_addr.s_addr + (unsigned long)reinterpret_cast(this)->sin_port); - } else if (ss_family == AF_INET6) { + } else if (_data.ss_family == AF_INET6) { unsigned long tmp = reinterpret_cast(this)->sin6_port; const uint8_t *a = reinterpret_cast(reinterpret_cast(this)->sin6_addr.s6_addr); for(long i=0;i<16;++i) @@ -319,7 +319,7 @@ unsigned long InetAddress::hashCode() const noexcept void InetAddress::forTrace(ZT_TraceEventPathAddress &ta) const noexcept { uint32_t tmp; - switch(ss_family) { + switch(_data.ss_family) { default: memset(&ta,0,sizeof(ZT_TraceEventPathAddress)); break; @@ -344,7 +344,7 @@ void InetAddress::forTrace(ZT_TraceEventPathAddress &ta) const noexcept bool InetAddress::isNetwork() const noexcept { - switch(ss_family) { + switch(_data.ss_family) { case AF_INET: { unsigned int bits = netmaskBits(); if (bits <= 0) @@ -377,7 +377,7 @@ bool InetAddress::isNetwork() const noexcept int InetAddress::marshal(uint8_t data[ZT_INETADDRESS_MARSHAL_SIZE_MAX]) const noexcept { unsigned int port; - switch(ss_family) { + switch(_data.ss_family) { case AF_INET: port = Utils::ntoh((uint16_t)reinterpret_cast(this)->sin_port); data[0] = 4; @@ -438,8 +438,8 @@ int InetAddress::unmarshal(const uint8_t *restrict data,const int len) noexcept bool InetAddress::operator==(const InetAddress &a) const noexcept { - if (ss_family == a.ss_family) { - switch(ss_family) { + if (_data.ss_family == a._data.ss_family) { + switch(_data.ss_family) { case AF_INET: return ( (reinterpret_cast(this)->sin_port == reinterpret_cast(&a)->sin_port)&& @@ -461,10 +461,10 @@ bool InetAddress::operator==(const InetAddress &a) const noexcept bool InetAddress::operator<(const InetAddress &a) const noexcept { - if (ss_family < a.ss_family) + if (_data.ss_family < a._data.ss_family) return true; - else if (ss_family == a.ss_family) { - switch(ss_family) { + else if (_data.ss_family == a._data.ss_family) { + switch(_data.ss_family) { case AF_INET: if (reinterpret_cast(this)->sin_port < reinterpret_cast(&a)->sin_port) return true; diff --git a/node/InetAddress.hpp b/node/InetAddress.hpp index 3cbbfa54c..c76eafe72 100644 --- a/node/InetAddress.hpp +++ b/node/InetAddress.hpp @@ -29,14 +29,14 @@ namespace ZeroTier { #define ZT_INETADDRESS_STRING_SIZE_MAX 64 /** - * Extends sockaddr_storage with friendly C++ methods + * C++ class that overlaps in size with sockaddr_storage and adds convenience methods * * This is basically a "mixin" for sockaddr_storage. It adds methods and * operators, but does not modify the structure. This can be cast to/from * sockaddr_storage and used interchangeably. DO NOT change this by e.g. * adding non-static fields, since much code depends on this identity. */ -struct InetAddress : public sockaddr_storage,public TriviallyCopyable +struct InetAddress : public TriviallyCopyable { private: // Internal function to copy any sockaddr_X structure to this one even if it's smaller and unpadded. @@ -161,6 +161,11 @@ public: return *this; } + /** + * @return Address family (ss_family in sockaddr_storage) + */ + ZT_ALWAYS_INLINE uint8_t family() const noexcept { return _data.ss_family; } + /** * @return IP scope classification (e.g. loopback, link-local, private, global) */ @@ -182,7 +187,7 @@ public: */ ZT_ALWAYS_INLINE void setPort(unsigned int port) noexcept { - switch(ss_family) { + switch(_data.ss_family) { case AF_INET: reinterpret_cast(this)->sin_port = Utils::hton((uint16_t)port); break; @@ -218,7 +223,7 @@ public: */ ZT_ALWAYS_INLINE unsigned int port() const noexcept { - switch(ss_family) { + switch(_data.ss_family) { case AF_INET: return Utils::ntoh((uint16_t)(reinterpret_cast(this)->sin_port)); case AF_INET6: return Utils::ntoh((uint16_t)(reinterpret_cast(this)->sin6_port)); default: return 0; @@ -242,7 +247,7 @@ public: ZT_ALWAYS_INLINE bool netmaskBitsValid() const noexcept { const unsigned int n = port(); - switch(ss_family) { + switch(_data.ss_family) { case AF_INET: return (n <= 32); case AF_INET6: return (n <= 128); } @@ -302,19 +307,19 @@ public: /** * @return True if this is an IPv4 address */ - ZT_ALWAYS_INLINE bool isV4() const noexcept { return (ss_family == AF_INET); } + ZT_ALWAYS_INLINE bool isV4() const noexcept { return (family() == AF_INET); } /** * @return True if this is an IPv6 address */ - ZT_ALWAYS_INLINE bool isV6() const noexcept { return (ss_family == AF_INET6); } + ZT_ALWAYS_INLINE bool isV6() const noexcept { return (family() == AF_INET6); } /** * @return pointer to raw address bytes or NULL if not available */ ZT_ALWAYS_INLINE const void *rawIpData() const noexcept { - switch(ss_family) { + switch(_data.ss_family) { case AF_INET: return (const void *)&(reinterpret_cast(this)->sin_addr.s_addr); case AF_INET6: return (const void *)(reinterpret_cast(this)->sin6_addr.s6_addr); default: return nullptr; @@ -327,13 +332,13 @@ public: ZT_ALWAYS_INLINE InetAddress ipOnly() const noexcept { InetAddress r; - switch(ss_family) { + switch(_data.ss_family) { case AF_INET: - r.ss_family = AF_INET; + reinterpret_cast(&r)->sin_family = AF_INET; reinterpret_cast(&r)->sin_addr.s_addr = reinterpret_cast(this)->sin_addr.s_addr; break; case AF_INET6: - r.ss_family = AF_INET6; + reinterpret_cast(&r)->sin6_family = AF_INET; memcpy(reinterpret_cast(&r)->sin6_addr.s6_addr,reinterpret_cast(this)->sin6_addr.s6_addr,16); break; } @@ -348,10 +353,11 @@ public: */ ZT_ALWAYS_INLINE bool ipsEqual(const InetAddress &a) const noexcept { - if (ss_family == a.ss_family) { - if (ss_family == AF_INET) + const uint8_t f = _data.ss_family; + if (f == a._data.ss_family) { + if (f == AF_INET) return (reinterpret_cast(this)->sin_addr.s_addr == reinterpret_cast(&a)->sin_addr.s_addr); - if (ss_family == AF_INET6) + if (f == AF_INET6) return (memcmp(reinterpret_cast(this)->sin6_addr.s6_addr,reinterpret_cast(&a)->sin6_addr.s6_addr,16) == 0); return (memcmp(this,&a,sizeof(InetAddress)) == 0); } @@ -368,10 +374,11 @@ public: */ ZT_ALWAYS_INLINE bool ipsEqual2(const InetAddress &a) const noexcept { - if (ss_family == a.ss_family) { - if (ss_family == AF_INET) + const uint8_t f = _data.ss_family; + if (f == a._data.ss_family) { + if (f == AF_INET) return (reinterpret_cast(this)->sin_addr.s_addr == reinterpret_cast(&a)->sin_addr.s_addr); - if (ss_family == AF_INET6) + if (f == AF_INET6) return (memcmp(reinterpret_cast(this)->sin6_addr.s6_addr,reinterpret_cast(&a)->sin6_addr.s6_addr,8) == 0); return (memcmp(this,&a,sizeof(InetAddress)) == 0); } @@ -400,7 +407,7 @@ public: /** * @return True if address family is non-zero */ - explicit ZT_ALWAYS_INLINE operator bool() const noexcept { return (ss_family != 0); } + explicit ZT_ALWAYS_INLINE operator bool() const noexcept { return (family() != 0); } static constexpr int marshalSizeMax() noexcept { return ZT_INETADDRESS_MARSHAL_SIZE_MAX; } int marshal(uint8_t data[ZT_INETADDRESS_MARSHAL_SIZE_MAX]) const noexcept; @@ -468,6 +475,9 @@ public: * Compute a private IPv6 "6plane" unicast address from network ID and ZeroTier address */ static InetAddress makeIpv66plane(uint64_t nwid,uint64_t zeroTierAddress) noexcept; + +private: + sockaddr_storage _data; }; static ZT_ALWAYS_INLINE InetAddress *asInetAddress(sockaddr_in *p) noexcept { return reinterpret_cast(p); } diff --git a/node/Peer.cpp b/node/Peer.cpp index 9caa9a13a..53e4463d0 100644 --- a/node/Peer.cpp +++ b/node/Peer.cpp @@ -20,6 +20,7 @@ #include "SelfAwareness.hpp" #include "InetAddress.hpp" #include "Protocol.hpp" +#include "Endpoint.hpp" #include @@ -93,7 +94,7 @@ void Peer::received( int64_t lastReceiveTimeMax = 0; int lastReceiveTimeMaxAt = 0; for(int i=0;iaddress().ss_family == path->address().ss_family) && + if ((_paths[i]->address().family() == path->address().family()) && (_paths[i]->localSocket() == path->localSocket()) && // TODO: should be localInterface when multipath is integrated (_paths[i]->address().ipsEqual2(path->address()))) { // Replace older path if everything is the same except the port number. @@ -117,14 +118,13 @@ void Peer::received( if (_paths[lastReceiveTimeMaxAt]) old = _paths[lastReceiveTimeMaxAt]->address(); _paths[lastReceiveTimeMaxAt] = path; - _bootstrap = path->address(); + _bootstrap = Endpoint(path->address()); _prioritizePaths(now); RR->t->learnedNewPath(tPtr,0x582fabdd,packetId,_id,path->address(),old); } else { if (RR->node->shouldUsePathForZeroTierTraffic(tPtr,_id,path->localSocket(),path->address())) { RR->t->tryingNewPath(tPtr,0xb7747ddd,_id,path->address(),path->address(),packetId,(uint8_t)verb,_id.address(),_id.hash().data(),ZT_TRACE_TRYING_NEW_PATH_REASON_PACKET_RECEIVED_FROM_UNKNOWN_PATH); - sendHELLO(tPtr,path->localSocket(),path->address(),now); - path->sent(now); + path->sent(now,sendHELLO(tPtr,path->localSocket(),path->address(),now)); } } } @@ -134,7 +134,7 @@ path_check_done: _lastAttemptedP2PInit = now; InetAddress addr; - if ((_bootstrap.type() == Endpoint::INETADDR_V4)||(_bootstrap.type() == Endpoint::INETADDR_V6)) { + if ((_bootstrap.type() == Endpoint::TYPE_INETADDR_V4)||(_bootstrap.type() == Endpoint::TYPE_INETADDR_V6)) { RR->t->tryingNewPath(tPtr,0x0a009444,_id,_bootstrap.inetAddr(),InetAddress::NIL,0,0,0,nullptr,ZT_TRACE_TRYING_NEW_PATH_REASON_BOOTSTRAP_ADDRESS); sendHELLO(tPtr,-1,_bootstrap.inetAddr(),now); } if (RR->node->externalPathLookup(tPtr,_id,-1,addr)) { @@ -207,7 +207,7 @@ path_check_done: } } -void Peer::sendHELLO(void *tPtr,const int64_t localSocket,const InetAddress &atAddress,int64_t now) +unsigned int Peer::sendHELLO(void *tPtr,const int64_t localSocket,const InetAddress &atAddress,int64_t now) { #if 0 Packet outp(_id.address(),RR->identity.address(),Packet::VERB_HELLO); @@ -253,23 +253,21 @@ void Peer::ping(void *tPtr,int64_t now,const bool pingAllAddressTypes) if (_alivePathCount > 0) { for (unsigned int i = 0; i < _alivePathCount; ++i) { - sendHELLO(tPtr,_paths[i]->localSocket(),_paths[i]->address(),now); - _paths[i]->sent(now); + _paths[i]->sent(now,sendHELLO(tPtr,_paths[i]->localSocket(),_paths[i]->address(),now)); if (!pingAllAddressTypes) return; } return; } - if ((_bootstrap.type() == Endpoint::INETADDR_V4)||(_bootstrap.type() == Endpoint::INETADDR_V6)) + if ((_bootstrap.type() == Endpoint::TYPE_INETADDR_V4)||(_bootstrap.type() == Endpoint::TYPE_INETADDR_V6)) sendHELLO(tPtr,-1,_bootstrap.inetAddr(),now); SharedPtr r(RR->topology->root()); if ((r)&&(r.ptr() != this)) { SharedPtr rp(r->path(now)); if (rp) { - sendHELLO(tPtr,rp->localSocket(),rp->address(),now); - rp->sent(now); + rp->sent(now,sendHELLO(tPtr,rp->localSocket(),rp->address(),now)); return; } } @@ -279,9 +277,8 @@ void Peer::resetWithinScope(void *tPtr,InetAddress::IpScope scope,int inetAddres { RWMutex::RLock l(_lock); for(unsigned int i=0; i < _alivePathCount; ++i) { - if ((_paths[i])&&((_paths[i]->address().ss_family == inetAddressFamily)&&(_paths[i]->address().ipScope() == scope))) { - sendHELLO(tPtr,_paths[i]->localSocket(),_paths[i]->address(),now); - _paths[i]->sent(now); + if ((_paths[i])&&((_paths[i]->address().family() == inetAddressFamily)&&(_paths[i]->address().ipScope() == scope))) { + _paths[i]->sent(now,sendHELLO(tPtr,_paths[i]->localSocket(),_paths[i]->address(),now)); } } } diff --git a/node/Peer.hpp b/node/Peer.hpp index 481c91a84..ab7106447 100644 --- a/node/Peer.hpp +++ b/node/Peer.hpp @@ -121,8 +121,9 @@ public: * @param localSocket Local source socket * @param atAddress Destination address * @param now Current time + * @return Number of bytes sent */ - void sendHELLO(void *tPtr,int64_t localSocket,const InetAddress &atAddress,int64_t now); + unsigned int sendHELLO(void *tPtr,int64_t localSocket,const InetAddress &atAddress,int64_t now); /** * Send a NOP message to e.g. probe a new link diff --git a/node/Tests.cpp b/node/Tests.cpp index 2402cd804..35608d302 100644 --- a/node/Tests.cpp +++ b/node/Tests.cpp @@ -205,6 +205,7 @@ extern "C" const char *ZTT_general() ZT_T_ASSERT(ZT_PROTO_PACKET_ENCRYPTED_SECTION_START < ZT_PROTO_MIN_PACKET_LENGTH); ZT_T_ASSERT(sizeof(Protocol::Header) == ZT_PROTO_MIN_PACKET_LENGTH); ZT_T_ASSERT(sizeof(Protocol::FragmentHeader) == ZT_PROTO_MIN_FRAGMENT_LENGTH); + ZT_T_ASSERT(sizeof(sockaddr_storage) == sizeof(InetAddress)); ZT_T_PRINTF("OK" ZT_EOL_S); } diff --git a/node/TriviallyCopyable.hpp b/node/TriviallyCopyable.hpp index 07571de7b..9525be997 100644 --- a/node/TriviallyCopyable.hpp +++ b/node/TriviallyCopyable.hpp @@ -27,9 +27,8 @@ namespace ZeroTier { * * It also includes some static methods to do this conveniently. */ -class TriviallyCopyable +ZT_PACKED_STRUCT(struct TriviallyCopyable { -public: /** * Be absolutely sure a TriviallyCopyable object is zeroed using Utils::burn() * @@ -165,7 +164,7 @@ public: TriviallyCopyable *const tmp = &dest; memcpy(tmp,&src,sizeof(T)); } -}; +}); } // namespace ZeroTier