From 4c029388523e5c96f884c3ec351329845c077398 Mon Sep 17 00:00:00 2001 From: Kees Bos Date: Wed, 24 Jun 2015 01:15:14 +0200 Subject: [PATCH 1/4] Fix signing verification The only signing that's accepted is what we (the controller) signed. if (cert.signedBy() != controller()) then fail So, there's no need to get a Peer entry for ourself (and that fails too). Just use the controller identity. --- node/Network.cpp | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/node/Network.cpp b/node/Network.cpp index a217595af..951fd2902 100644 --- a/node/Network.cpp +++ b/node/Network.cpp @@ -286,16 +286,9 @@ void Network::addMembershipCertificate(const CertificateOfMembership &cert,bool return; } - SharedPtr signer(RR->topology->getPeer(cert.signedBy())); - - if (!signer) { - // This would be rather odd, since this is our controller... could happen - // if we get packets before we've gotten config. - RR->sw->requestWhois(cert.signedBy()); - return; - } - - if (!cert.verify(signer->identity())) { + // We are the controller: RR->identity.address() == controller() == cert.signedBy() + // So, verify that we signed th cert ourself + if (!cert.verify(RR->identity)) { TRACE("rejected network membership certificate for %.16llx signed by %s: signature check failed",(unsigned long long)_id,cert.signedBy().toString().c_str()); return; } From 9e1a384edf1ffa0742052943080e58f5ac25c479 Mon Sep 17 00:00:00 2001 From: Kees Bos Date: Wed, 24 Jun 2015 08:00:50 +0200 Subject: [PATCH 2/4] Partially revert previous commit. That solved self signed, but broke certs signed by other controllers. --- node/Network.cpp | 27 ++++++++++++++++++++++----- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/node/Network.cpp b/node/Network.cpp index 951fd2902..0f1be03e3 100644 --- a/node/Network.cpp +++ b/node/Network.cpp @@ -286,11 +286,28 @@ void Network::addMembershipCertificate(const CertificateOfMembership &cert,bool return; } - // We are the controller: RR->identity.address() == controller() == cert.signedBy() - // So, verify that we signed th cert ourself - if (!cert.verify(RR->identity)) { - TRACE("rejected network membership certificate for %.16llx signed by %s: signature check failed",(unsigned long long)_id,cert.signedBy().toString().c_str()); - return; + if (cert.signedBy() == RR->identity.address()) { + // We are the controller: RR->identity.address() == controller() == cert.signedBy() + // So, verify that we signed th cert ourself + if (!cert.verify(RR->identity)) { + TRACE("rejected network membership certificate for %.16llx self signed by %s: signature check failed",(unsigned long long)_id,cert.signedBy().toString().c_str()); + return; + } + } else { + + SharedPtr signer(RR->topology->getPeer(cert.signedBy())); + + if (!signer) { + // This would be rather odd, since this is our controller... could happen + // if we get packets before we've gotten config. + RR->sw->requestWhois(cert.signedBy()); + return; + } + + if (!cert.verify(signer->identity())) { + TRACE("rejected network membership certificate for %.16llx signed by %s: signature check failed",(unsigned long long)_id,cert.signedBy().toString().c_str()); + return; + } } } From 305bec8a2836b0bda03932bf00efe27435f63c45 Mon Sep 17 00:00:00 2001 From: Kees Bos Date: Wed, 24 Jun 2015 08:03:14 +0200 Subject: [PATCH 3/4] Fix lookup of networks This might be done more efficient, but it fails for some networks and sometimes even results in duplicate entries in 'zerotier-cli listnetworks' --- node/Node.hpp | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/node/Node.hpp b/node/Node.hpp index b0f4ab222..a85809fd7 100644 --- a/node/Node.hpp +++ b/node/Node.hpp @@ -214,12 +214,11 @@ private: inline SharedPtr _network(uint64_t nwid) const { - std::vector< SharedPtr >::const_iterator iter = std::lower_bound(_networks.begin(), _networks.end(), nwid, NetworkComparator()); - if(iter != _networks.end() && (*iter)->id() == nwid) { - return *iter; - } else { - return SharedPtr(); + for(std::vector< SharedPtr >::const_iterator iter(_networks.begin());iter!=_networks.end();++iter) { + if((*iter)->id() == nwid) + return *iter; } + return SharedPtr(); } RuntimeEnvironment _RR; From a002c95d54a2f5827d3324d88ea8024c9f6092ca Mon Sep 17 00:00:00 2001 From: Kees Bos Date: Thu, 25 Jun 2015 18:35:13 +0200 Subject: [PATCH 4/4] Fix reporting of ipAssignments for ipv4 --- controller/SqliteNetworkController.cpp | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/controller/SqliteNetworkController.cpp b/controller/SqliteNetworkController.cpp index a1827a72e..b3e025a18 100644 --- a/controller/SqliteNetworkController.cpp +++ b/controller/SqliteNetworkController.cpp @@ -1176,7 +1176,14 @@ unsigned int SqliteNetworkController::_doCPGet( sqlite3_bind_text(_sGetIpAssignmentsForNode2,2,addrs,10,SQLITE_STATIC); bool firstIp = true; while (sqlite3_step(_sGetIpAssignmentsForNode2) == SQLITE_ROW) { - InetAddress ip((const void *)sqlite3_column_blob(_sGetIpAssignmentsForNode2,0),(sqlite3_column_int(_sGetIpAssignmentsForNode2,2) == 6) ? 16 : 4,(unsigned int)sqlite3_column_int(_sGetIpAssignmentPools2,1)); + int ipversion = sqlite3_column_int(_sGetIpAssignmentsForNode2,2); + char ipBlob[16]; + memcpy(ipBlob,(const void *)sqlite3_column_blob(_sGetIpAssignmentsForNode2,0),16); + InetAddress ip( + (const void *)(ipversion == 6 ? ipBlob : &ipBlob[12]), + (ipversion == 6 ? 16 : 4), + (unsigned int)sqlite3_column_int(_sGetIpAssignmentsForNode2,1) + ); responseBody.append(firstIp ? "\"" : ",\""); firstIp = false; responseBody.append(_jsonEscape(ip.toString()));