summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--node/IncomingPacket.cpp8
-rw-r--r--node/Network.cpp54
-rw-r--r--node/Network.hpp4
-rw-r--r--node/Packet.hpp6
4 files changed, 34 insertions, 38 deletions
diff --git a/node/IncomingPacket.cpp b/node/IncomingPacket.cpp
index ab771b7e..80159194 100644
--- a/node/IncomingPacket.cpp
+++ b/node/IncomingPacket.cpp
@@ -429,7 +429,7 @@ bool IncomingPacket::_doOK(const RuntimeEnvironment *RR,const SharedPtr<Peer> &p
offset += com.deserialize(*this,ZT_PROTO_VERB_MULTICAST_FRAME__OK__IDX_COM_AND_GATHER_RESULTS);
SharedPtr<Network> network(RR->node->network(nwid));
if ((network)&&(com.hasRequiredFields()))
- network->addMembershipCertificate(com,false);
+ network->validateAndAddMembershipCertificate(com);
}
if ((flags & 0x02) != 0) {
@@ -558,7 +558,7 @@ bool IncomingPacket::_doEXT_FRAME(const RuntimeEnvironment *RR,const SharedPtr<P
CertificateOfMembership com;
comLen = com.deserialize(*this,ZT_PROTO_VERB_EXT_FRAME_IDX_COM);
if (com.hasRequiredFields())
- network->addMembershipCertificate(com,false);
+ network->validateAndAddMembershipCertificate(com);
}
if (!network->isAllowed(peer->address())) {
@@ -648,7 +648,7 @@ bool IncomingPacket::_doNETWORK_MEMBERSHIP_CERTIFICATE(const RuntimeEnvironment
if (com.hasRequiredFields()) {
SharedPtr<Network> network(RR->node->network(com.networkId()));
if (network)
- network->addMembershipCertificate(com,false);
+ network->validateAndAddMembershipCertificate(com);
}
}
@@ -806,7 +806,7 @@ bool IncomingPacket::_doMULTICAST_FRAME(const RuntimeEnvironment *RR,const Share
CertificateOfMembership com;
offset += com.deserialize(*this,ZT_PROTO_VERB_MULTICAST_FRAME_IDX_COM);
if (com.hasRequiredFields())
- network->addMembershipCertificate(com,false);
+ network->validateAndAddMembershipCertificate(com);
}
// Check membership after we've read any included COM, since
diff --git a/node/Network.cpp b/node/Network.cpp
index 4414e4d1..84b8d320 100644
--- a/node/Network.cpp
+++ b/node/Network.cpp
@@ -267,53 +267,55 @@ void Network::requestConfiguration()
RR->sw->send(outp,true,_id);
}
-void Network::addMembershipCertificate(const CertificateOfMembership &cert,bool forceAccept)
+bool Network::validateAndAddMembershipCertificate(const CertificateOfMembership &cert)
{
if (!cert) // sanity check
- return;
+ return false;
Mutex::Lock _l(_lock);
CertificateOfMembership &old = _membershipCertificates[cert.issuedTo()];
// Nothing to do if the cert hasn't changed -- we get duplicates due to zealous cert pushing
if (old == cert)
- return;
+ return true; // but if it's a duplicate of one we already accepted, return is 'true'
// Check signature, log and return if cert is invalid
- if (!forceAccept) {
- if (cert.signedBy() != controller()) {
- TRACE("rejected network membership certificate for %.16llx signed by %s: signer not a controller of this network",(unsigned long long)_id,cert.signedBy().toString().c_str());
- return;
+ if (cert.signedBy() != controller()) {
+ TRACE("rejected network membership certificate for %.16llx signed by %s: signer not a controller of this network",(unsigned long long)_id,cert.signedBy().toString().c_str());
+ return false; // invalid signer
+ }
+
+ 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 false; // invalid signature
}
- 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 {
+ } else {
- SharedPtr<Peer> signer(RR->topology->getPeer(cert.signedBy()));
+ SharedPtr<Peer> 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 (!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 false; // signer unknown
+ }
- 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;
- }
+ 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 false; // invalid signature
}
}
// If we made it past authentication, update cert
if (cert.revision() != old.revision())
old = cert;
+
+ return true;
}
bool Network::peerNeedsOurMembershipCertificate(const Address &to,uint64_t now)
diff --git a/node/Network.hpp b/node/Network.hpp
index daa4554e..53a22dcd 100644
--- a/node/Network.hpp
+++ b/node/Network.hpp
@@ -179,9 +179,9 @@ public:
* Add or update a membership certificate
*
* @param cert Certificate of membership
- * @param forceAccept If true, accept without validating signature
+ * @return True if certificate was accepted as valid
*/
- void addMembershipCertificate(const CertificateOfMembership &cert,bool forceAccept);
+ bool validateAndAddMembershipCertificate(const CertificateOfMembership &cert);
/**
* Check if we should push membership certificate to a peer, and update last pushed
diff --git a/node/Packet.hpp b/node/Packet.hpp
index 50471b06..51a241ba 100644
--- a/node/Packet.hpp
+++ b/node/Packet.hpp
@@ -680,12 +680,6 @@ public:
*
* Certificate contains network ID, peer it was issued for, etc.
*
- * Note that in the current code base, separate COM pushes are not done.
- * Instead the "bundled COM" options are utilized in EXT_FRAME and
- * MULTICAST_FRAME to push the COM along with frames. This is slightly
- * more efficient. But we'll keep this simple message around in case we
- * want to use it in the future.
- *
* OK/ERROR are not generated.
*/
VERB_NETWORK_MEMBERSHIP_CERTIFICATE = 10,