Break out certificate trust store into a separate object and simplify and clean up certificate verification code.

core/CAPI.cpp

@@ -669,7 +669,7 @@ enum ZT_CertificateError ZT_Certificate_decode(
 			return ZT_CERTIFICATE_ERROR_INVALID_FORMAT;
 		}
 		if (verify) {
-			const ZT_CertificateError err = c->verify();
+			const ZT_CertificateError err = c->verify(-1, true);
 			if (err != ZT_CERTIFICATE_ERROR_NONE) {
 				delete c;
 				return err;
@@ -702,12 +702,14 @@ int ZT_Certificate_encode(
 	}
 }
 
-enum ZT_CertificateError ZT_Certificate_verify(const ZT_Certificate *cert)
+enum ZT_CertificateError ZT_Certificate_verify(
+	const ZT_Certificate *cert,
+	int64_t clock)
 {
 	try {
 		if (!cert)
 			return ZT_CERTIFICATE_ERROR_INVALID_FORMAT;
-		return ZeroTier::Certificate(*cert).verify();
+		return ZeroTier::Certificate(*cert).verify(clock, true);
 	} catch (...) {
 		return ZT_CERTIFICATE_ERROR_INVALID_FORMAT;
 	}

core/Certificate.hpp

@@ -63,6 +63,12 @@ public:
 		return *this;
 	}
 
+	/**
+	 * @return SHA384Hash containing serial number
+	 */
+	ZT_INLINE SHA384Hash getSerialNo() const noexcept
+	{ return SHA384Hash(this->serialNo); }
+
 	/**
 	 * Add a subject node/identity without a locator
 	 *

core/Containers.hpp

@@ -59,6 +59,9 @@ struct intl_MapHasher
 	std::size_t operator()(const O &obj) const noexcept
 	{ return (std::size_t)obj.hashCode(); }
 
+	std::size_t operator()(const Vector< uint8_t > &bytes) const noexcept
+	{ return (std::size_t)Utils::fnv1a32(bytes.data(), (unsigned int)bytes.size()); }
+
 	std::size_t operator()(const uint64_t i) const noexcept
 	{ return (std::size_t)Utils::hash64(i ^ Utils::s_mapNonce); }
 

core/Node.cpp

@@ -26,6 +26,7 @@
 #include "VL1.hpp"
 #include "VL2.hpp"
 #include "Buf.hpp"
+#include "TrustStore.hpp"
 
 namespace ZeroTier {
 
@@ -41,23 +42,26 @@ struct _NodeObjects
 		expect(),
 		vl2(RR),
 		vl1(RR),
+		topology(RR, tPtr, now),
 		sa(RR),
-		topology(RR, tPtr, now)
+		ts()
 	{
 		RR->t = &t;
 		RR->expect = &expect;
 		RR->vl2 = &vl2;
 		RR->vl1 = &vl1;
-		RR->sa = &sa;
 		RR->topology = &topology;
+		RR->sa = &sa;
+		RR->ts = &ts;
 	}
 
 	Trace t;
 	Expect expect;
 	VL2 vl2;
 	VL1 vl1;
-	SelfAwareness sa;
 	Topology topology;
+	SelfAwareness sa;
+	TrustStore ts;
 };
 
 } // anonymous namespace
@@ -153,7 +157,7 @@ Node::~Node()
 	m_networks_l.unlock();
 	m_networks.clear();
 
-	delete (_NodeObjects *)m_objects;
+	delete reinterpret_cast<_NodeObjects *>(m_objects);
 
 	// Let go of cached Buf objects. If other nodes happen to be running in this
 	// same process space new Bufs will be allocated as needed, but this is almost
@@ -584,7 +588,10 @@ ZT_CertificateError Node::addCertificate(
 		if (!c.decode(certData, certSize))
 			return ZT_CERTIFICATE_ERROR_INVALID_FORMAT;
 	}
-	return RR->topology->addCertificate(tptr, c, now, localTrust, true, true, true);
+	RR->ts->add(c, localTrust);
+	RR->ts->update(now, nullptr);
+	SharedPtr< TrustStore::Entry > ent(RR->ts->get(c.getSerialNo()));
+	return (ent) ? ent->error() : ZT_CERTIFICATE_ERROR_INVALID_FORMAT; // should never be null, but if so it means invalid
 }
 
 ZT_ResultCode Node::deleteCertificate(
@@ -593,13 +600,15 @@ ZT_ResultCode Node::deleteCertificate(
 {
 	if (!serialNo)
 		return ZT_RESULT_ERROR_BAD_PARAMETER;
-	RR->topology->deleteCertificate(tptr, reinterpret_cast<const uint8_t *>(serialNo));
+	RR->ts->erase(SHA384Hash(serialNo));
+	RR->ts->update(-1, nullptr);
 	return ZT_RESULT_OK;
 }
 
 struct p_certificateListInternal
 {
-	Vector< SharedPtr< const Certificate > > c;
+	Vector< SharedPtr< TrustStore::Entry > > entries;
+	Vector< const ZT_Certificate * > c;
 	Vector< unsigned int > t;
 };
 
@@ -619,11 +628,17 @@ ZT_CertificateList *Node::listCertificates()
 
 	p_certificateListInternal *const clint = reinterpret_cast<p_certificateListInternal *>(reinterpret_cast<uint8_t *>(cl) + sizeof(ZT_CertificateList));
 	new (clint) p_certificateListInternal;
-	RR->topology->allCerts(clint->c, clint->t);
+
+	clint->entries = RR->ts->all(false);
+	clint->c.reserve(clint->entries.size());
+	clint->t.reserve(clint->entries.size());
+	for(Vector< SharedPtr< TrustStore::Entry > >::const_iterator i(clint->entries.begin()); i!=clint->entries.end(); ++i) {
+		clint->c.push_back(&((*i)->certificate()));
+		clint->t.push_back((*i)->localTrust());
+	}
 
 	cl->freeFunction = p_freeCertificateList;
-	static_assert(sizeof(SharedPtr< const Certificate >) == sizeof(void *), "SharedPtr<> is not just a wrapped pointer");
+	cl->certs = clint->c.data();
-	cl->certs = reinterpret_cast<const ZT_Certificate **>(clint->c.data());
 	cl->localTrust = clint->t.data();
 	cl->certCount = (unsigned long)clint->c.size();
 

core/RuntimeEnvironment.hpp

@@ -29,6 +29,7 @@ class NetworkController;
 class SelfAwareness;
 class Trace;
 class Expect;
+class TrustStore;
 
 /**
  * ZeroTier::Node execution context
@@ -44,13 +45,13 @@ public:
 		instanceId(Utils::getSecureRandomU64()),
 		node(n),
 		localNetworkController(nullptr),
-		rtmem(nullptr),
 		t(nullptr),
 		expect(nullptr),
 		vl2(nullptr),
 		vl1(nullptr),
 		topology(nullptr),
-		sa(nullptr)
+		sa(nullptr),
+		ts(nullptr)
 	{
 		publicIdentityStr[0] = 0;
 		secretIdentityStr[0] = 0;
@@ -70,15 +71,13 @@ public:
 	// This is set externally to an instance of this base class
 	NetworkController *localNetworkController;
 
-	// Memory actually occupied by Trace, Switch, etc.
-	void *rtmem;
-
 	Trace *t;
 	Expect *expect;
 	VL2 *vl2;
 	VL1 *vl1;
 	Topology *topology;
 	SelfAwareness *sa;
+	TrustStore *ts;
 
 	// This node's identity and string representations thereof
 	Identity identity;

core/Tests.cpp

@@ -44,6 +44,7 @@
 #include "Locator.hpp"
 #include "Certificate.hpp"
 #include "MIMC52.hpp"
+#include "ScopedPtr.hpp"
 
 #ifdef __UNIX_LIKE__
 
@@ -1210,7 +1211,7 @@ extern "C" const char *ZTT_crypto()
 			ZT_T_PRINTF("OK %s" ZT_EOL_S, tmp);
 
 			ZT_T_PRINTF("  Create and sign certificate... ");
-			SharedPtr< Certificate > cert(new Certificate());
+			ScopedPtr< Certificate > cert(new Certificate());
 			cert->subject.timestamp = now();
 			cert->addSubjectIdentity(testSubjectId);
 			cert->addSubjectNetwork(12345, testSubjectId.fingerprint());
@@ -1237,7 +1238,7 @@ extern "C" const char *ZTT_crypto()
 			ZT_T_PRINTF("OK (%d bytes)" ZT_EOL_S, (int)enc.size());
 
 			ZT_T_PRINTF("  Testing certificate verify... ");
-			ZT_CertificateError vr = cert->verify();
+			ZT_CertificateError vr = cert->verify(-1, true);
 			if (vr != ZT_CERTIFICATE_ERROR_NONE) {
 				ZT_T_PRINTF("FAILED (verify original) (%d)" ZT_EOL_S, (int)vr);
 				return "Verify original certificate";
@@ -1245,12 +1246,12 @@ extern "C" const char *ZTT_crypto()
 			ZT_T_PRINTF("OK" ZT_EOL_S);
 
 			ZT_T_PRINTF("  Test certificate decode from marshaled format... ");
-			SharedPtr< Certificate > cert2(new Certificate());
+			ScopedPtr< Certificate > cert2(new Certificate());
 			if (!cert2->decode(enc.data(), (unsigned int)enc.size())) {
 				ZT_T_PRINTF("FAILED (decode)" ZT_EOL_S);
 				return "Certificate decode";
 			}
-			if (cert2->verify() != ZT_CERTIFICATE_ERROR_NONE) {
+			if (cert2->verify(-1, true) != ZT_CERTIFICATE_ERROR_NONE) {
 				ZT_T_PRINTF("FAILED (verify decoded certificate)" ZT_EOL_S);
 				return "Verify decoded certificate";
 			}
@@ -1261,7 +1262,7 @@ extern "C" const char *ZTT_crypto()
 			ZT_T_PRINTF("OK" ZT_EOL_S);
 
 			ZT_T_PRINTF("  Test certificate copy/construct... ");
-			SharedPtr< Certificate > cert3(new Certificate(*cert2));
+			ScopedPtr< Certificate > cert3(new Certificate(*cert2));
 			if (!ZTT_deepCompareCertificates(*cert2, *cert3)) {
 				ZT_T_PRINTF("FAILED (compare copy with original)" ZT_EOL_S);
 				return "Certificate copy";

core/TrustStore.cpp

@@ -12,7 +12,6 @@
 /****/
 
 #include "TrustStore.hpp"
-#include "Topology.hpp"
 
 namespace ZeroTier {
 
@@ -22,63 +21,73 @@ TrustStore::TrustStore()
 TrustStore::~TrustStore()
 {}
 
-SharedPtr< const TrustStore::Entry > TrustStore::get(const SHA384Hash &serial) const
+SharedPtr< TrustStore::Entry > TrustStore::get(const SHA384Hash &serial) const
 {
 	RWMutex::RLock l(m_lock);
 	Map< SHA384Hash, SharedPtr< Entry > >::const_iterator i(m_bySerial.find(serial));
-	return (i == m_bySerial.end()) ? SharedPtr< const TrustStore::Entry >() : i->second.constify();
+	return (i != m_bySerial.end()) ? i->second : SharedPtr< TrustStore::Entry >();
 }
 
-Vector< SharedPtr< Peer > > TrustStore::roots(void *const tPtr, const RuntimeEnvironment *RR)
+Map< Identity, SharedPtr< const Locator > > TrustStore::roots()
 {
 	RWMutex::RLock l(m_lock);
-
+	Map< Identity, SharedPtr< const Locator > > r;
-	Vector< SharedPtr< Peer > > r;
+	for (Map< Fingerprint, Vector< SharedPtr< Entry > > >::const_iterator cv(m_bySubjectIdentity.begin()); cv != m_bySubjectIdentity.end(); ++cv) {
-	r.reserve(m_bySerial.size());
+		for (Vector< SharedPtr< Entry > >::const_iterator c(cv->second.begin()); c != cv->second.end(); ++c) {
-
+			if (((*c)->error() == ZT_CERTIFICATE_ERROR_NONE) && (((*c)->localTrust() & ZT_CERTIFICATE_LOCAL_TRUST_FLAG_ZEROTIER_ROOT_SET) != 0)) {
-	for (Map< SHA384Hash, SharedPtr< Entry > >::const_iterator c(m_bySerial.begin()); c != m_bySerial.end(); ++c) {
+				for (unsigned int j = 0; j < (*c)->certificate().subject.identityCount; ++j) {
-		if ((c->second->localTrust() & ZT_CERTIFICATE_LOCAL_TRUST_FLAG_ZEROTIER_ROOT_SET) != 0) {
+					const Identity *const id = reinterpret_cast<const Identity *>((*c)->certificate().subject.identities[j].identity);
-			for (unsigned int j = 0; j < c->second->certificate().subject.identityCount; ++j) {
+					if (likely((id != nullptr) && (*id))) { // sanity check
-				const Identity *const id = reinterpret_cast<const Identity *>(c->second->certificate().subject.identities[j].identity);
+						SharedPtr< const Locator > &existingLoc = r[*id];
-				if ((id != nullptr) && (*id)) { // sanity check
+						const Locator *const loc = reinterpret_cast<const Locator *>((*c)->certificate().subject.identities[j].locator);
-					SharedPtr< Peer > peer(RR->topology->peer(tPtr, id->address(), true));
+						if ((loc != nullptr) && ((!existingLoc) || (existingLoc->timestamp() < loc->timestamp())))
-					if (!peer) {
+							existingLoc.set(new Locator(*loc));
-						peer.set(new Peer(RR));
-						peer->init(*id);
-						peer = RR->topology->add(tPtr, peer);
 					}
-
-					const Locator *const loc = reinterpret_cast<const Locator *>(c->second->certificate().subject.identities[j].locator);
-					if (loc)
-						peer->setLocator(SharedPtr< const Locator >(new Locator(*loc)), true);
-
-					r.push_back(peer);
 				}
 			}
 		}
 	}
-
 	return r;
 }
 
-Vector< SharedPtr< const TrustStore::Entry > > TrustStore::all() const
+Vector< SharedPtr< TrustStore::Entry > > TrustStore::all(const bool includeRejectedCertificates) const
 {
-	Vector< SharedPtr< const TrustStore::Entry > > r;
 	RWMutex::RLock l(m_lock);
+	Vector< SharedPtr< Entry > > r;
 	r.reserve(m_bySerial.size());
-	for (Map< SHA384Hash, SharedPtr< Entry > >::const_iterator i(m_bySerial.begin()); i != m_bySerial.end(); ++i)
+	for (Map< SHA384Hash, SharedPtr< Entry > >::const_iterator i(m_bySerial.begin()); i != m_bySerial.end(); ++i) {
-		r.push_back(i->second.constify());
+		if ((includeRejectedCertificates) || (i->second->error() == ZT_CERTIFICATE_ERROR_NONE))
+			r.push_back(i->second);
+	}
+	return r;
+}
+
+Vector< SharedPtr< TrustStore::Entry > > TrustStore::rejects() const
+{
+	RWMutex::RLock l(m_lock);
+	Vector< SharedPtr< Entry > > r;
+	for (Map< SHA384Hash, SharedPtr< Entry > >::const_iterator c(m_bySerial.begin()); c != m_bySerial.end(); ++c) {
+		if (c->second->error() != ZT_CERTIFICATE_ERROR_NONE)
+			r.push_back(c->second);
+	}
 	return r;
 }
 
 void TrustStore::add(const Certificate &cert, const unsigned int localTrust)
 {
 	RWMutex::Lock l(m_lock);
-	m_addQueue.push_front(SharedPtr(new Entry(cert, localTrust)));
+	m_addQueue.push_front(SharedPtr< Entry >(new Entry(cert, localTrust)));
+}
+
+void TrustStore::erase(const SHA384Hash &serial)
+{
+	RWMutex::Lock l(m_lock);
+	m_deleteQueue.push_front(serial);
 }
 
 // Recursive function to trace a certificate up the chain to a CA, returning true
-// if the CA is reached and the path length is less than the maximum.
+// if the CA is reached and the path length is less than the maximum. Note that only
+// non-rejected (no errors) certificates will be in bySignedCert.
 static bool p_validatePath(const Map< SHA384Hash, Vector< SharedPtr< TrustStore::Entry > > > &bySignedCert, const SharedPtr< TrustStore::Entry > &entry, unsigned int pathLength)
 {
 	if (((entry->localTrust() & ZT_CERTIFICATE_LOCAL_TRUST_FLAG_ROOT_CA) != 0) && (pathLength <= entry->certificate().maxPathLength))
@@ -95,124 +104,123 @@ static bool p_validatePath(const Map< SHA384Hash, Vector< SharedPtr< TrustStore:
 	return false;
 }
 
-void TrustStore::update(const int64_t clock, Vector< std::pair< SharedPtr< Entry >, ZT_CertificateError > > *const purge)
+void TrustStore::update(const int64_t clock, Vector< SharedPtr< Entry > > *const purge)
 {
 	RWMutex::Lock l(m_lock);
 
-	// Re-verify existing and rejected certificates, excluding signatures which
+	// (Re)compute error codes for existing certs, but we don't have to do a full
-	// will have already been checked (and checking these is CPU-intensive). This
+	// signature check here since that's done when they're taken out of the add queue.
-	// catches certificate expiry and un-expiry if the system's clock has been
+	bool errorStateModified = false;
-	// changed. When a formerly rejected cert is revived it ends up getting
+	for (Map< SHA384Hash, SharedPtr< Entry > >::const_iterator c(m_bySerial.begin()); c != m_bySerial.end(); ++c) {
-	// checked twice, but optimizing this out would be about as costly as just
-	// doing this as verify() without signature check is cheap.
-	for (Map< SharedPtr< Entry >, ZT_CertificateError >::iterator c(m_rejected.begin()); c != m_rejected.end();) {
-		const ZT_CertificateError err = c->first->m_certificate.verify(clock, false);
-		if (err == ZT_CERTIFICATE_ERROR_NONE) {
-			m_bySerial[SHA384Hash(c->first->m_certificate.serialNo)] = c->first;
-			m_rejected.erase(c++);
-		} else {
-			++c;
-		}
-	}
-	for (Map< SHA384Hash, SharedPtr< Entry > >::const_iterator c(m_bySerial.begin()); c != m_bySerial.end();) {
 		const ZT_CertificateError err = c->second->m_certificate.verify(clock, false);
-		if (err == ZT_CERTIFICATE_ERROR_NONE) {
+		errorStateModified |= (c->second->m_error.exchange((int)err, std::memory_order_relaxed) != (int)err);
-			++c;
-		} else {
-			m_rejected[c->second] = err;
-			m_bySerial.erase(c++);
-		}
 	}
 
+	// If no certificate error statuses changed and there are no new certificates to
+	// add, there is nothing to do and we don't need to do more expensive path validation
+	// and structure rebuilding.
+	if ((!errorStateModified) && (m_addQueue.empty()) && (m_deleteQueue.empty()))
+		return;
+
 	// Add new certificates to m_bySerial, which is the master certificate set. They still
 	// have yet to have their full certificate chains validated. Full signature checking is
 	// performed here.
 	while (!m_addQueue.empty()) {
-		const ZT_CertificateError err = m_addQueue.front()->m_certificate.verify(clock, true);
+		m_addQueue.front()->m_error.store((int)m_addQueue.front()->m_certificate.verify(clock, true), std::memory_order_relaxed);
-		if (err == ZT_CERTIFICATE_ERROR_NONE) {
+		m_bySerial[SHA384Hash(m_addQueue.front()->m_certificate.serialNo)].move(m_addQueue.front());
-			m_bySerial[SHA384Hash(m_addQueue.front()->m_certificate.serialNo)].move(m_addQueue.front());
-		} else {
-			m_rejected[m_addQueue.front()] = err;
-		}
 		m_addQueue.pop_front();
 	}
 
-	// Verify certificate paths and replace old certificates with newer certificates
+	// Delete any certificates enqueued to be deleted.
-	// when subject unique ID mapping dictates, repeating the process until a stable
+	while (!m_deleteQueue.empty()) {
-	// state is achieved. A loop is needed because deleting old certs when new
+		m_bySerial.erase(m_deleteQueue.front());
-	// certs (with the same subject unique ID) replace them could in theory alter
+		m_deleteQueue.pop_front();
-	// certificate validation path checking outcomes, though in practice it should
+	}
-	// not since mixing certificate roles this way would be strange.
-	for (;;) {
-		// Create a reverse lookup mapping from signed certs to signer certs for
-		// certificate path validation.
-		Map< SHA384Hash, Vector< SharedPtr< Entry > > > bySignedCert;
-		for (Map< SHA384Hash, SharedPtr< Entry > >::const_iterator c(m_bySerial.begin()); c != m_bySerial.end(); ++c) {
-			for (unsigned int j = 0; j < c->second->m_certificate.subject.certificateCount; ++j)
-				bySignedCert[SHA384Hash(c->second->m_certificate.subject.certificates[j])].push_back(c->second);
-		}
 
-		// Validate certificate paths and reject any certificates that do not trace
+	Map< SHA384Hash, Vector< SharedPtr< Entry > > > bySignedCert;
-		// back to a CA.
+	for (;;) {
-		for (Map< SHA384Hash, SharedPtr< Entry > >::const_iterator c(m_bySerial.begin()); c != m_bySerial.end();) {
+		// Create a reverse lookup mapping from signed certs to signer certs for certificate
-			if (p_validatePath(bySignedCert, c->second, 0)) {
+		// path validation. Only include good certificates.
-				++c;
+		for (Map< SHA384Hash, SharedPtr< Entry > >::const_iterator c(m_bySerial.begin()); c != m_bySerial.end(); ++c) {
-			} else {
+			if (c->second->error() == ZT_CERTIFICATE_ERROR_NONE) {
-				m_rejected[c->second] = ZT_CERTIFICATE_ERROR_INVALID_CHAIN;
+				for (unsigned int j = 0; j < c->second->m_certificate.subject.certificateCount; ++j)
-				m_bySerial.erase(c++);
+					bySignedCert[SHA384Hash(c->second->m_certificate.subject.certificates[j])].push_back(c->second);
 			}
 		}
 
-		// Populate mapping of subject unique IDs to certificates and reject any
+		// Validate certificate paths and reject any certificates that do not trace back to a CA.
-		// certificates that have been superseded by newly issued certificates with
+		for (Map< SHA384Hash, SharedPtr< Entry > >::const_iterator c(m_bySerial.begin()); c != m_bySerial.end(); ++c) {
-		// the same subject.
+			if (c->second->error() == ZT_CERTIFICATE_ERROR_NONE) {
+				if (!p_validatePath(bySignedCert, c->second, 0))
+					c->second->m_error.store((int)ZT_CERTIFICATE_ERROR_INVALID_CHAIN, std::memory_order_relaxed);
+			}
+		}
+
+		// Populate mapping of subject unique IDs to certificates and reject any certificates
+		// that have been superseded by newly issued certificates with the same subject.
 		bool exitLoop = true;
 		m_bySubjectUniqueId.clear();
 		for (Map< SHA384Hash, SharedPtr< Entry > >::const_iterator c(m_bySerial.begin()); c != m_bySerial.end();) {
-			const unsigned int uniqueIdSize = c->second->m_certificate.subject.uniqueIdSize;
+			if (c->second->error() == ZT_CERTIFICATE_ERROR_NONE) {
-			if ((uniqueIdSize > 0) && (uniqueIdSize <= 1024)) { // 1024 is a sanity check value, actual unique IDs are <100 bytes
+				const unsigned int uniqueIdSize = c->second->m_certificate.subject.uniqueIdSize;
-				SharedPtr< Entry > &current = m_bySubjectUniqueId[Vector< uint8_t >(c->second->m_certificate.subject.uniqueId, c->second->m_certificate.subject.uniqueId + uniqueIdSize)];
+				if ((uniqueIdSize > 0) && (uniqueIdSize <= 1024)) { // 1024 is a sanity check value, actual unique IDs are <100 bytes
-				if (current) {
+					SharedPtr< Entry > &current = m_bySubjectUniqueId[Vector< uint8_t >(c->second->m_certificate.subject.uniqueId, c->second->m_certificate.subject.uniqueId + uniqueIdSize)];
-					if (c->second->m_certificate.subject.timestamp > current->m_certificate.subject.timestamp) {
+					if (current) {
 						exitLoop = false;
-						m_rejected[current] = ZT_CERTIFICATE_ERROR_HAVE_NEWER_CERT;
+						if (c->second->m_certificate.subject.timestamp > current->m_certificate.subject.timestamp) {
-						m_bySerial.erase(SHA384Hash(current->m_certificate.serialNo));
+							current->m_error.store((int)ZT_CERTIFICATE_ERROR_HAVE_NEWER_CERT, std::memory_order_relaxed);
+							current = c->second;
+						} else if (c->second->m_certificate.subject.timestamp < current->m_certificate.subject.timestamp) {
+							c->second->m_error.store((int)ZT_CERTIFICATE_ERROR_HAVE_NEWER_CERT, std::memory_order_relaxed);
+						} else {
+							// Equal timestamps should never happen, but handle it by comparing serials for deterministic completeness.
+							if (memcmp(c->second->m_certificate.serialNo, current->m_certificate.serialNo, ZT_SHA384_DIGEST_SIZE) > 0) {
+								current->m_error.store((int)ZT_CERTIFICATE_ERROR_HAVE_NEWER_CERT, std::memory_order_relaxed);
+								current = c->second;
+							} else {
+								c->second->m_error.store((int)ZT_CERTIFICATE_ERROR_HAVE_NEWER_CERT, std::memory_order_relaxed);
+							}
+						}
+					} else {
 						current = c->second;
 					}
-				} else {
-					current = c->second;
 				}
 			}
 		}
 
-		if (exitLoop)
+		// If no certificates were tagged out during the unique ID pass, we can exit. Otherwise
+		// the last few steps have to be repeated because removing any certificate could in
+		// theory affect the result of certificate path validation.
+		if (exitLoop) {
 			break;
+		} else {
+			bySignedCert.clear();
+		}
 	}
 
-	// Populate a mapping of identities to certificates whose subjects reference them.
+	// Populate mapping of identities to certificates whose subjects reference them.
 	m_bySubjectIdentity.clear();
-	for (Map< SHA384Hash, SharedPtr< Entry > >::const_iterator c(m_bySerial.begin()); c != m_bySerial.end();) {
+	for (Map< SHA384Hash, SharedPtr< Entry > >::const_iterator c(m_bySerial.begin()); c != m_bySerial.end(); ++c) {
-		for (unsigned int i = 0; i < c->second->m_certificate.subject.identityCount; ++i)
+		if (c->second->error() == ZT_CERTIFICATE_ERROR_NONE) {
-			m_bySubjectIdentity[reinterpret_cast<const Identity *>(c->second->m_certificate.subject.identities[i].identity)->fingerprint()].push_back(c->second);
+			for (unsigned int i = 0; i < c->second->m_certificate.subject.identityCount; ++i) {
+				const Identity *const id = reinterpret_cast<const Identity *>(c->second->m_certificate.subject.identities[i].identity);
+				if ((id) && (*id)) // sanity check
+					m_bySubjectIdentity[id->fingerprint()].push_back(c->second);
+			}
+		}
 	}
 
 	// Purge and return purged certificates if this option is selected.
 	if (purge) {
-		purge->reserve(m_rejected.size());
+		for (Map< SHA384Hash, SharedPtr< Entry > >::const_iterator c(m_bySerial.begin()); c != m_bySerial.end();) {
-		for (Map< SharedPtr< Entry >, ZT_CertificateError >::const_iterator c(m_rejected.begin()); c != m_rejected.end(); ++c)
+			if (c->second->error() != ZT_CERTIFICATE_ERROR_NONE) {
-			purge->push_back(std::pair< SharedPtr< Entry >, ZT_CertificateError >(c->first, c->second));
+				purge->push_back(c->second);
-		m_rejected.clear();
+				m_bySerial.erase(c++);
+			} else {
+				++c;
+			}
+		}
 	}
 }
 
-Vector< std::pair< SharedPtr<TrustStore::Entry>, ZT_CertificateError > > TrustStore::rejects() const
-{
-	Vector< std::pair< SharedPtr<Entry>, ZT_CertificateError > > r;
-	RWMutex::RLock l(m_lock);
-	r.reserve(m_rejected.size());
-	for (Map< SharedPtr< Entry >, ZT_CertificateError >::const_iterator c(m_rejected.begin()); c != m_rejected.end(); ++c)
-		r.push_back(std::pair< SharedPtr< Entry >, ZT_CertificateError >(c->first, c->second));
-	return r;
-}
-
 } // namespace ZeroTier

core/TrustStore.hpp

@@ -43,61 +43,100 @@ public:
 		friend class SharedPtr< const TrustStore::Entry >;
 		friend class TrustStore;
 
-	private:
-		ZT_INLINE Entry(const Certificate &cert, const unsigned int lt) noexcept:
-			m_certificate(cert),
-			m_localTrust(lt)
-		{}
-
 	public:
+		/**
+		 * @return Reference to held certificate
+		 */
 		ZT_INLINE const Certificate &certificate() const noexcept
 		{ return m_certificate; }
 
+		/**
+		 * Get the local trust for this certificate
+		 *
+		 * This value may be changed dynamically by calls to update().
+		 *
+		 * @return Local trust bit mask
+		 */
 		ZT_INLINE unsigned int localTrust() const noexcept
 		{ return m_localTrust.load(std::memory_order_relaxed); }
 
+		/**
+		 * Change the local trust of this entry
+		 *
+		 * @param lt New local trust bit mask
+		 */
+		ZT_INLINE void setLocalTrust(const unsigned int lt) noexcept
+		{ m_localTrust.store(lt, std::memory_order_relaxed); }
+
+		/**
+		 * Get the error code for this certificate
+		 *
+		 * @return Error or ZT_CERTIFICATE_ERROR_NONE if none
+		 */
+		ZT_INLINE ZT_CertificateError error() const noexcept
+		{ return (ZT_CertificateError)m_error.load(std::memory_order_relaxed); }
+
 	private:
+		Entry() {}
+		Entry(const Entry &) {}
+		Entry &operator=(const Entry &) { return *this; }
+
+		ZT_INLINE Entry(const Certificate &cert, const unsigned int lt) noexcept:
+			m_certificate(cert),
+			m_localTrust(lt),
+			m_error((int)ZT_CERTIFICATE_ERROR_NONE)
+		{}
+
 		Certificate m_certificate;
 		std::atomic< unsigned int > m_localTrust;
+		std::atomic< int > m_error;
 		std::atomic< int > __refCount;
 	};
 
 	TrustStore();
-
 	~TrustStore();
 
 	/**
 	 * Get certificate by certificate serial number
 	 *
+	 * Note that the error code should be checked. The certificate may be
+	 * rejected and may still be in the store unless the store has been
+	 * purged.
+	 *
 	 * @param serial SHA384 hash of certificate
 	 * @return Entry or empty/nil if not found
 	 */
-	SharedPtr< const Entry > get(const SHA384Hash &serial) const;
+	SharedPtr< Entry > get(const SHA384Hash &serial) const;
 
 	/**
-	 * Get current root peers based on root-enumerating certs in trust store
+	 * Get roots specified by root set certificates in the local store.
 	 *
-	 * Root peers are created or obtained via this node's Topology. This should
+	 * If more than one certificate locally trusted as a root set specifies
-	 * never be called while relevant data structures in Topology are locked.
+	 * the root, it will be returned once (as per Map behavior) but the latest
+	 * locator will be returned from among those available.
 	 *
-	 * Locators in root peers are also updated if the locator present in the
+	 * @return Roots and the latest locator specified for each (if any)
-	 * certificate is valid and newer.
-	 *
-	 * @param tPtr Caller pointer
-	 * @param RR Runtime environment
-	 * @return All roots (sort order undefined)
 	 */
-	Vector< SharedPtr< Peer > > roots(void *tPtr, const RuntimeEnvironment *RR);
+	Map< Identity, SharedPtr< const Locator > > roots();
 
 	/**
+	 * @param includeRejectedCertificates If true, also include certificates with error codes
 	 * @return All certificates in asecending sort order by serial
 	 */
-	Vector< SharedPtr< const Entry > > all() const;
+	Vector< SharedPtr< Entry > > all(bool includeRejectedCertificates) const;
+
+	/**
+	 * Get a copy of the current rejected certificate set.
+	 *
+	 * @return Rejected certificates
+	 */
+	Vector< SharedPtr< Entry > > rejects() const;
 
 	/**
 	 * Add a certificate
 	 *
-	 * A copy is made so it's fine if the original is freed after this call.
+	 * A copy is made so it's fine if the original is freed after this call. If
+	 * the certificate already exists its local trust flags are updated.
 	 *
 	 * IMPORTANT: The caller MUST also call update() after calling add() one or
 	 * more times to actually add and revalidate certificates and their signature
@@ -107,29 +146,31 @@ public:
 	 */
 	void add(const Certificate &cert, unsigned int localTrust);
 
+	/**
+	 * Queue a certificate to be deleted
+	 *
+	 * Actual delete does not happen until the next update().
+	 *
+	 * @param serial Serial of certificate to delete
+	 */
+	void erase(const SHA384Hash &serial);
+
 	/**
 	 * Validate all certificates and their certificate chains
 	 *
 	 * This also processes any certificates added with add() since the last call to update().
 	 *
-	 * @param clock Current time in milliseconds since epoch
+	 * @param clock Current time in milliseconds since epoch, or -1 to not check times on this pass
 	 * @param purge If non-NULL, purge rejected certificates and return them in this vector (vector should be empty)
 	 */
-	void update(int64_t clock, Vector< std::pair< SharedPtr<Entry>, ZT_CertificateError > > *purge);
+	void update(int64_t clock, Vector< SharedPtr< Entry > > *purge);
-
-	/**
-	 * Get a copy of the current rejected certificate set.
-	 *
-	 * @return Rejected certificates
-	 */
-	Vector< std::pair< SharedPtr<Entry>, ZT_CertificateError > > rejects() const;
 
 private:
-	Map< SHA384Hash, SharedPtr< Entry > > m_bySerial;
+	Map< SHA384Hash, SharedPtr< Entry > > m_bySerial; // all certificates
-	Map< Vector< uint8_t >, SharedPtr< Entry > > m_bySubjectUniqueId;
+	Map< Vector< uint8_t >, SharedPtr< Entry > > m_bySubjectUniqueId; // non-rejected certificates only
-	Map< Fingerprint, Vector< SharedPtr< Entry > > > m_bySubjectIdentity;
+	Map< Fingerprint, Vector< SharedPtr< Entry > > > m_bySubjectIdentity; // non-rejected certificates only
 	ForwardList< SharedPtr< Entry > > m_addQueue;
-	Map< SharedPtr< Entry >, ZT_CertificateError > m_rejected;
+	ForwardList< SHA384Hash > m_deleteQueue;
 	RWMutex m_lock;
 };
 

core/zerotier.h

@@ -2900,9 +2900,12 @@ ZT_SDK_API int ZT_Certificate_encode(
  * Verify certificate signatures and internal structure.
  *
  * @param cert Certificate to verify
+ * @param clock Clock to check timestamp or -1 to skip this check
  * @return Certificate error or ZT_CERTIFICATE_ERROR_NONE if no errors found.
  */
-ZT_SDK_API enum ZT_CertificateError ZT_Certificate_verify(const ZT_Certificate *cert);
+ZT_SDK_API enum ZT_CertificateError ZT_Certificate_verify(
+	const ZT_Certificate *cert,
+	int64_t clock);
 
 /**
  * Deep clone a certificate, returning one allocated C-side.

rust-zerotier-core/src/certificate.rs

@@ -694,10 +694,10 @@ impl Certificate {
         return Ok(signed_cert);
     }
 
-    pub fn verify(&self) -> CertificateError {
+    pub fn verify(&self, clock: i64) -> CertificateError {
         unsafe {
             let capi = self.to_capi();
-            return CertificateError::from_i32(ztcore::ZT_Certificate_verify(&capi.certificate as *const ztcore::ZT_Certificate) as i32).unwrap_or(CertificateError::InvalidFormat);
+            return CertificateError::from_i32(ztcore::ZT_Certificate_verify(&capi.certificate as *const ztcore::ZT_Certificate, clock) as i32).unwrap_or(CertificateError::InvalidFormat);
         }
     }
 }