More fixes to legacy support, and to a potential issue on quit.

node/IncomingPacket.cpp

@@ -554,6 +554,8 @@ bool IncomingPacket::_doP5_MULTICAST_FRAME(const RuntimeEnvironment *RR,const Sh
 		const unsigned int signatureLen = at<uint16_t>(ZT_PROTO_VERB_P5_MULTICAST_FRAME_IDX_FRAME + frameLen);
 
 		{
+			if (origin == RR->identity.address())
+				return true;
 			Mutex::Lock _l(p5MulticastDedupBuffer_m);
 			if (!p5MulticastDedupBufferPtr) {
 				memset(p5MulticastDedupBuffer,0,sizeof(p5MulticastDedupBuffer));
@@ -566,6 +568,32 @@ bool IncomingPacket::_doP5_MULTICAST_FRAME(const RuntimeEnvironment *RR,const Sh
 			p5MulticastDedupBuffer[p5MulticastDedupBufferPtr++ % 1024] = guid;
 		}
 
+		SharedPtr<Network> network(RR->nc->network(nwid));
+		if (network) {
+			if ((flags & ZT_PROTO_VERB_P5_MULTICAST_FRAME_FLAGS_HAS_MEMBERSHIP_CERTIFICATE) != 0) {
+				CertificateOfMembership com;
+				com.deserialize(*this,ZT_PROTO_VERB_P5_MULTICAST_FRAME_IDX_FRAME + frameLen + 2 + signatureLen);
+				if (com.hasRequiredFields())
+					network->addMembershipCertificate(com,false);
+			}
+			if (!network->isAllowed(origin)) {
+				SharedPtr<Peer> originPeer(RR->topology->getPeer(origin));
+				if (originPeer)
+					_sendErrorNeedCertificate(RR,originPeer,nwid);
+			} else if ((frameLen > 0)&&(frameLen <= 2800)) {
+				if (!dest.mac().isMulticast())
+					return true;
+				if ((!sourceMac)||(sourceMac.isMulticast())||(sourceMac == network->mac()))
+					return true;
+				if (sourceMac != MAC(origin,nwid)) {
+					if (network->permitsBridging(origin)) {
+						network->learnBridgeRoute(sourceMac,origin);
+					} else return true;
+				}
+				network->tapPut(sourceMac,dest.mac(),etherType,frame,frameLen);
+			}
+		}
+
 		peer->receive(RR,_fromSock,_remoteAddress,hops(),packetId(),Packet::VERB_P5_MULTICAST_FRAME,0,Packet::VERB_NOP,Utils::now());
 
 		if (RR->topology->amSupernode()) {
@@ -577,12 +605,14 @@ bool IncomingPacket::_doP5_MULTICAST_FRAME(const RuntimeEnvironment *RR,const Sh
 			const unsigned int limit = 128; // use a fairly generous limit since we want legacy peers to always work until they go away
 
 			std::vector<Address> members(RR->mc->getMembers(nwid,dest,limit));
+			SharedPtr<Peer> lpp;
 
 			setAt(ZT_PROTO_VERB_P5_MULTICAST_FRAME_IDX_PROPAGATION_DEPTH,(uint16_t)0xffff);
 			setSource(RR->identity.address());
 			compress();
 			for(std::vector<Address>::iterator lp(members.begin());lp!=members.end();++lp) {
-				SharedPtr<Peer> lpp(RR->topology->getPeer(*lp));
+				if (!senderIsLegacy)
+					lpp = RR->topology->getPeer(*lp);
 				if ( (*lp != origin) && (*lp != peer->address()) && ((senderIsLegacy) || (!lpp) || (lpp->remoteVersionMajor() < 1)) ) {
 					newInitializationVector();
 					setDestination(*lp);
@@ -605,35 +635,6 @@ bool IncomingPacket::_doP5_MULTICAST_FRAME(const RuntimeEnvironment *RR,const Sh
 				sn->send(RR,data(),size(),Utils::now());
 			}
 		}
-
-		SharedPtr<Network> network(RR->nc->network(nwid));
-		if (network) {
-			if ((flags & ZT_PROTO_VERB_P5_MULTICAST_FRAME_FLAGS_HAS_MEMBERSHIP_CERTIFICATE)) {
-				CertificateOfMembership com;
-				com.deserialize(*this,ZT_PROTO_VERB_MULTICAST_FRAME_IDX_FRAME + frameLen + 2 + signatureLen);
-				if (com.hasRequiredFields())
-					network->addMembershipCertificate(com,false);
-			}
-
-			if (!network->isAllowed(origin)) {
-				_sendErrorNeedCertificate(RR,peer,network->id());
-				return true;
-			}
-
-			if ((frameLen > 0)&&(frameLen <= 2800)) {
-				if (!dest.mac().isMulticast())
-					return true;
-				if ((!sourceMac)||(sourceMac.isMulticast())||(sourceMac == network->mac()))
-					return true;
-				if (sourceMac != MAC(origin,network->id())) {
-					if (network->permitsBridging(origin)) {
-						network->learnBridgeRoute(sourceMac,origin);
-					} else return true;
-				}
-
-				network->tapPut(sourceMac,dest.mac(),etherType,frame,frameLen);
-			}
-		}
 	} catch (std::exception &ex) {
 		TRACE("dropped P5_MULTICAST_FRAME from %s(%s): unexpected exception: %s",source().toString().c_str(),_remoteAddress.toString().c_str(),ex.what());
 	} catch ( ... ) {

node/Multicaster.cpp

@@ -270,8 +270,7 @@ void Multicaster::send(
 			outp.append((uint16_t)0xffff); // do not forward
 			outp.append((unsigned char)0,320 + 1024); // empty queue and bloom filter
 
-			unsigned int signedPortionStart = outp.size();
-			outp.append((unsigned char)0);
+			outp.append((unsigned char)((com) ? ZT_PROTO_VERB_P5_MULTICAST_FRAME_FLAGS_HAS_MEMBERSHIP_CERTIFICATE : 0));
 			outp.append((uint64_t)nwid);
 			outp.append((uint16_t)0);
 			outp.append((unsigned char)0);
@@ -286,9 +285,9 @@ void Multicaster::send(
 			outp.append((uint16_t)etherType);
 			outp.append((uint16_t)len);
 			outp.append(data,len);
-			unsigned int signedPortionLen = outp.size() - signedPortionStart;
+			unsigned int signedPortionLen = outp.size() - ZT_PROTO_VERB_P5_MULTICAST_FRAME_IDX__START_OF_SIGNED_PORTION;
 
-			C25519::Signature sig(RR->identity.sign(outp.field(signedPortionStart,signedPortionLen),signedPortionLen));
+			C25519::Signature sig(RR->identity.sign(outp.field(ZT_PROTO_VERB_P5_MULTICAST_FRAME_IDX__START_OF_SIGNED_PORTION,signedPortionLen),signedPortionLen));
 
 			outp.append((uint16_t)sig.size());
 			outp.append(sig.data,sig.size());

node/Network.cpp

@@ -310,25 +310,31 @@ void Network::addMembershipCertificate(const CertificateOfMembership &cert,bool
 		return;
 
 	if (!forceAccept) {
-		if (cert.signedBy() != controller())
+		if (cert.signedBy() != controller()) {
+			LOG("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;
+		}
+
 		SharedPtr<Peer> signer(RR->topology->getPeer(cert.signedBy()));
-		if (!signer)
-			return; // we should already have done a WHOIS on this peer, since this is our netconf master
-		if (!cert.verify(signer->identity()))
+
+		if (!signer) {
+			// This would be rather odd, since this is our netconf master... could happen
+			// if we get packets before we've gotten config.
+			RR->sw->requestWhois(cert.signedBy());
 			return;
+		}
+
+		if (!cert.verify(signer->identity())) {
+			LOG("rejected network membership certificate for %.16llx signed by %s: signature check failed",(unsigned long long)_id,cert.signedBy().toString().c_str());
+			return;
+		}
 	}
 
 	Mutex::Lock _l(_lock);
 
-	// We go ahead and accept certs provisionally even if _isOpen is true, since
-	// that might be changed in short order if the user is fiddling in the UI.
-
 	CertificateOfMembership &old = _membershipCertificates[cert.issuedTo()];
-	if (cert.timestamp() >= old.timestamp()) {
-		//TRACE("got new certificate for %s on network %.16llx",cert.issuedTo().toString().c_str(),cert.networkId());
+	if (cert.timestamp() >= old.timestamp())
 		old = cert;
-	}
 }
 
 bool Network::peerNeedsOurMembershipCertificate(const Address &to,uint64_t now)
@@ -632,6 +638,7 @@ void Network::_dumpMembershipCerts()
 	FILE *mcdb = fopen(mcdbPath.c_str(),"wb");
 	if (!mcdb)
 		return;
+
 	if (fwrite("ZTMCD0",6,1,mcdb) != 1) {
 		fclose(mcdb);
 		Utils::rm(mcdbPath);
@@ -639,29 +646,14 @@ void Network::_dumpMembershipCerts()
 	}
 
 	for(std::map<Address,CertificateOfMembership>::iterator c=(_membershipCertificates.begin());c!=_membershipCertificates.end();++c) {
-		try {
-			c->second.serialize(buf);
-			if (buf.size() >= (ZT_NETWORK_CERT_WRITE_BUF_SIZE / 2)) {
-				if (fwrite(buf.data(),buf.size(),1,mcdb) != 1) {
-					fclose(mcdb);
-					Utils::rm(mcdbPath);
-					return;
-				}
-				buf.clear();
+		buf.clear();
+		c->second.serialize(buf);
+		if (buf.size() > 0) {
+			if (fwrite(buf.data(),buf.size(),1,mcdb) != 1) {
+				fclose(mcdb);
+				Utils::rm(mcdbPath);
+				return;
 			}
-		} catch ( ... ) {
-			// Sanity check... no cert will ever be big enough to overflow buf
-			fclose(mcdb);
-			Utils::rm(mcdbPath);
-			return;
-		}
-	}
-
-	if (buf.size()) {
-		if (fwrite(buf.data(),buf.size(),1,mcdb) != 1) {
-			fclose(mcdb);
-			Utils::rm(mcdbPath);
-			return;
 		}
 	}