From e7b7d7444e4e9e5f0e3e7b40819d8075926f9b14 Mon Sep 17 00:00:00 2001 From: Adam Ierymenko Date: Tue, 20 Apr 2021 17:32:56 -0400 Subject: [PATCH] Stub out certificate revocations and harden certificate structure verification to make sure there are no dangling pointers. --- core/Certificate.cpp | 90 ++++++--- core/Certificate.hpp | 15 +- core/Path.hpp | 34 ++-- core/Topology.hpp | 13 +- core/TrustStore.cpp | 274 +++++++++++++++----------- core/TrustStore.hpp | 48 +++-- core/zerotier.h | 81 ++++++-- rust-zerotier-core/src/certificate.rs | 73 ++++--- 8 files changed, 396 insertions(+), 232 deletions(-) diff --git a/core/Certificate.cpp b/core/Certificate.cpp index 2563103ce..d1e7706c5 100644 --- a/core/Certificate.cpp +++ b/core/Certificate.cpp @@ -360,8 +360,64 @@ ZT_CertificateError Certificate::verify(const int64_t clock, const bool checkSig if (this->validity[0] > this->validity[1]) { return ZT_CERTIFICATE_ERROR_INVALID_FORMAT; } - if ((clock >= 0) && ((this->validity[0] > clock) || (this->validity[1] < clock))) { - return ZT_CERTIFICATE_ERROR_OUT_OF_VALID_TIME_WINDOW; + + if (this->subject.identityCount > 0) { + if (this->subject.identities) { + for (unsigned int i = 0; i < this->subject.identityCount; ++i) { + if (!this->subject.identities[i].identity) { + return ZT_CERTIFICATE_ERROR_INVALID_FORMAT; + } + if (checkSignatures) { + if (!reinterpret_cast(this->subject.identities[i].identity)->locallyValidate()) { + return ZT_CERTIFICATE_ERROR_INVALID_IDENTITY; + } + if ((this->subject.identities[i].locator) && (!reinterpret_cast(this->subject.identities[i].locator)->verify(*reinterpret_cast(this->subject.identities[i].identity)))) { + return ZT_CERTIFICATE_ERROR_INVALID_COMPONENT_SIGNATURE; + } + } + } + } else { + return ZT_CERTIFICATE_ERROR_INVALID_FORMAT; + } + } + + if (this->subject.networkCount > 0) { + if (this->subject.networks) { + for (unsigned int i = 0; i < this->subject.networkCount; ++i) { + if (!this->subject.networks[i].id) { + return ZT_CERTIFICATE_ERROR_MISSING_REQUIRED_FIELDS; + } + } + } else { + return ZT_CERTIFICATE_ERROR_INVALID_FORMAT; + } + } + + if (this->subject.updateURLCount > 0) { + if (this->subject.updateURLs) { + for (unsigned int i = 0; i < this->subject.updateURLCount; ++i) { + if (!this->subject.updateURLs[i]) + return ZT_CERTIFICATE_ERROR_MISSING_REQUIRED_FIELDS; + } + } else { + return ZT_CERTIFICATE_ERROR_MISSING_REQUIRED_FIELDS; + } + } + + if ((this->subject.uniqueIdSize > sizeof(this->subject.uniqueId)) || (this->subject.uniqueIdSignatureSize > sizeof(this->subject.uniqueIdSignature))) { + return ZT_CERTIFICATE_ERROR_INVALID_FORMAT; + } + + if ((this->issuerPublicKeySize > sizeof(this->issuerPublicKey)) || (this->publicKeySize > sizeof(this->publicKey))) { + return ZT_CERTIFICATE_ERROR_INVALID_FORMAT; + } + + if ((this->extendedAttributesSize > 0) && (!this->extendedAttributes)) { + return ZT_CERTIFICATE_ERROR_INVALID_FORMAT; + } + + if (this->signatureSize > sizeof(this->signature)) { + return ZT_CERTIFICATE_ERROR_INVALID_FORMAT; } if (checkSignatures) { @@ -423,33 +479,9 @@ ZT_CertificateError Certificate::verify(const int64_t clock, const bool checkSig } } - for (unsigned int i = 0; i < this->subject.identityCount; ++i) { - if (!this->subject.identities[i].identity) - return ZT_CERTIFICATE_ERROR_MISSING_REQUIRED_FIELDS; - if (checkSignatures) { - if (!reinterpret_cast(this->subject.identities[i].identity)->locallyValidate()) { - return ZT_CERTIFICATE_ERROR_INVALID_IDENTITY; - } - if ((this->subject.identities[i].locator) && (!reinterpret_cast(this->subject.identities[i].locator)->verify(*reinterpret_cast(this->subject.identities[i].identity)))) { - return ZT_CERTIFICATE_ERROR_INVALID_COMPONENT_SIGNATURE; - } - } - } - - for (unsigned int i = 0; i < this->subject.networkCount; ++i) { - if (!this->subject.networks[i].id) { - return ZT_CERTIFICATE_ERROR_MISSING_REQUIRED_FIELDS; - } - } - - if (this->subject.updateURLCount > 0) { - if (!this->subject.updateURLs) { - return ZT_CERTIFICATE_ERROR_MISSING_REQUIRED_FIELDS; - } - for (unsigned int i = 0; i < this->subject.updateURLCount; ++i) { - if (!this->subject.updateURLs[i]) - return ZT_CERTIFICATE_ERROR_MISSING_REQUIRED_FIELDS; - } + if (clock >= 0) { + if (!this->verifyTimeWindow(clock)) + return ZT_CERTIFICATE_ERROR_OUT_OF_VALID_TIME_WINDOW; } } catch (...) { return ZT_CERTIFICATE_ERROR_INVALID_FORMAT; diff --git a/core/Certificate.hpp b/core/Certificate.hpp index 336822a42..2e9840a61 100644 --- a/core/Certificate.hpp +++ b/core/Certificate.hpp @@ -143,15 +143,24 @@ public: /** * Verify self-contained signatures and validity of certificate structure * - * This doesn't check the entire certificate chain, just the validity of - * the certificate's internal signature and fields. + * This cannot check the chain of trust back to a CA, only the internal validity + * of this certificate. * - * @param clock If non-negative, check that certificate is in valid time window + * @param clock If non-negative, also do verifyTimeWindow() * @param checkSignatures If true, perform full signature check (which is more expensive than other checks) * @return OK (0) or error code indicating why certificate failed verification. */ ZT_CertificateError verify(int64_t clock, bool checkSignatures) const; + /** + * Check this certificate's expiration status + * + * @param clock Current real world time in milliseconds since epoch + * @return True if certificate is not expired or outside window + */ + ZT_INLINE bool verifyTimeWindow(int64_t clock) const noexcept + { return ((clock >= this->validity[0]) && (clock <= this->validity[1]) && (this->validity[0] <= this->validity[1])); } + /** * Create a new certificate public/private key pair * diff --git a/core/Path.hpp b/core/Path.hpp index 5934b0c8b..557d5c09c 100644 --- a/core/Path.hpp +++ b/core/Path.hpp @@ -49,9 +49,6 @@ public: class Key { public: - /** - * Construct key with undefined value - */ ZT_INLINE Key() noexcept {} @@ -61,7 +58,7 @@ public: if (family == AF_INET) { const uint16_t p = (uint16_t)ip.as.sa_in.sin_port; m_hashCode = Utils::hash64((((uint64_t)ip.as.sa_in.sin_addr.s_addr) << 16U) ^ ((uint64_t)p) ^ Utils::s_mapNonce); - m_v664 = 0; // IPv6 /64 is 0 for IPv4 + m_ipv6Net64 = 0; // 0 for IPv4, never 0 for IPv6 m_port = p; } else { if (likely(family == AF_INET6)) { @@ -69,13 +66,12 @@ public: const uint64_t b = Utils::loadMachineEndian< uint64_t >(reinterpret_cast(ip.as.sa_in6.sin6_addr.s6_addr) + 8); const uint16_t p = ip.as.sa_in6.sin6_port; m_hashCode = Utils::hash64(a ^ b ^ ((uint64_t)p) ^ Utils::s_mapNonce); - m_v664 = a; // IPv6 /64 + m_ipv6Net64 = a; // IPv6 /64 m_port = p; } else { - // This isn't reachable since only IPv4 and IPv6 are used with InetAddress, but implement - // something here for technical completeness. + // This is not reachable since InetAddress can only be AF_INET or AF_INET6, but implement something. m_hashCode = Utils::fnv1a32(&ip, sizeof(InetAddress)); - m_v664 = Utils::fnv1a32(ip.as.sa.sa_data, sizeof(ip.as.sa.sa_data)); + m_ipv6Net64 = 0; m_port = (uint16_t)family; } } @@ -85,7 +81,7 @@ public: { return (unsigned long)m_hashCode; } ZT_INLINE bool operator==(const Key &k) const noexcept - { return (m_hashCode == k.m_hashCode) && (m_v664 == k.m_v664) && (m_port == k.m_port); } + { return (m_hashCode == k.m_hashCode) && (m_ipv6Net64 == k.m_ipv6Net64) && (m_port == k.m_port); } ZT_INLINE bool operator!=(const Key &k) const noexcept { return (!(*this == k)); } @@ -95,9 +91,9 @@ public: if (m_hashCode < k.m_hashCode) { return true; } else if (m_hashCode == k.m_hashCode) { - if (m_v664 < k.m_v664) { + if (m_ipv6Net64 < k.m_ipv6Net64) { return true; - } else if (m_v664 == k.m_v664) { + } else if (m_ipv6Net64 == k.m_ipv6Net64) { return (m_port < k.m_port); } } @@ -115,7 +111,7 @@ public: private: uint64_t m_hashCode; - uint64_t m_v664; + uint64_t m_ipv6Net64; uint16_t m_port; }; @@ -167,11 +163,11 @@ public: */ ZT_INLINE void updateLatency(const unsigned int newMeasurement) noexcept { - int lat = m_latency; + const int lat = m_latency.load(std::memory_order_relaxed); if (likely(lat > 0)) { - m_latency = (lat + newMeasurement) / 2; + m_latency.store((lat + (int)newMeasurement) >> 1U, std::memory_order_relaxed); } else { - m_latency = newMeasurement; + m_latency.store((int)newMeasurement, std::memory_order_relaxed); } } @@ -179,7 +175,7 @@ public: * @return Latency in milliseconds or -1 if unknown */ ZT_INLINE int latency() const noexcept - { return m_latency; } + { return m_latency.load(std::memory_order_relaxed); } /** * Check path aliveness @@ -187,7 +183,7 @@ public: * @param now Current time */ ZT_INLINE bool alive(const CallContext &cc) const noexcept - { return ((cc.ticks - m_lastIn.load()) < ZT_PATH_ALIVE_TIMEOUT); } + { return ((cc.ticks - m_lastIn.load(std::memory_order_relaxed)) < ZT_PATH_ALIVE_TIMEOUT); } /** * @return Physical address @@ -205,13 +201,13 @@ public: * @return Last time we received anything */ ZT_INLINE int64_t lastIn() const noexcept - { return m_lastIn.load(); } + { return m_lastIn.load(std::memory_order_relaxed); } /** * @return Last time we sent something */ ZT_INLINE int64_t lastOut() const noexcept - { return m_lastOut.load(); } + { return m_lastOut.load(std::memory_order_relaxed); } private: const int64_t m_localSocket; diff --git a/core/Topology.hpp b/core/Topology.hpp index 0c330f1e9..48d362be4 100644 --- a/core/Topology.hpp +++ b/core/Topology.hpp @@ -45,11 +45,11 @@ public: /** * Add peer to database * - * This will not replace existing peers. In that case the existing peer - * record is returned. + * If there's already a peer with this address, the existing peer is + * returned. Otherwise the new peer is added and returned. * * @param peer Peer to add - * @return New or existing peer (should replace 'peer') + * @return New or existing peer */ SharedPtr< Peer > add(const CallContext &cc, const SharedPtr< Peer > &peer); @@ -118,7 +118,8 @@ public: } /** - * @param allPeers vector to fill with all current peers + * @param allPeers Vector to fill with all current peers + * @param rootPeers Vector to fill with peers that are roots */ void allPeers(Vector< SharedPtr< Peer > > &allPeers, Vector< SharedPtr< Peer > > &rootPeers) const; @@ -129,8 +130,6 @@ public: /** * Rank root servers in descending order of quality - * - * @param now Current time */ ZT_INLINE void rankRoots(const CallContext &cc) { @@ -162,7 +161,7 @@ private: RWMutex m_peers_l; // m_peers RWMutex m_paths_l; // m_paths - Mutex m_roots_l; // m_roots and m_lastRankedRoots + Mutex m_roots_l; // m_roots SharedPtr< Peer > m_bestRoot; Spinlock l_bestRoot; diff --git a/core/TrustStore.cpp b/core/TrustStore.cpp index 767dfdef8..529fd6cea 100644 --- a/core/TrustStore.cpp +++ b/core/TrustStore.cpp @@ -33,19 +33,27 @@ Map< Identity, SharedPtr< const Locator > > TrustStore::roots() { RWMutex::RLock l(m_lock); Map< Identity, SharedPtr< const Locator > > r; + + // Iterate using m_bySubjectIdentity to only scan certificates with subject identities. + // This map also does not contian error or deprecated certificates. for (Map< Fingerprint, Vector< SharedPtr< Entry > > >::const_iterator cv(m_bySubjectIdentity.begin()); cv != m_bySubjectIdentity.end(); ++cv) { 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)) { + + if (((*c)->m_localTrust & ZT_CERTIFICATE_LOCAL_TRUST_FLAG_ZEROTIER_ROOT_SET) != 0) { for (unsigned int j = 0; j < (*c)->certificate().subject.identityCount; ++j) { const Identity *const id = reinterpret_cast((*c)->certificate().subject.identities[j].identity); - if (likely((id != nullptr) && (*id))) { // sanity check + if ((id) && (*id)) { // sanity check SharedPtr< const Locator > &existingLoc = r[*id]; const Locator *const loc = reinterpret_cast((*c)->certificate().subject.identities[j].locator); - if ((loc != nullptr) && ((!existingLoc) || (existingLoc->revision() < loc->revision()))) - existingLoc.set(new Locator(*loc)); + if (loc) { + // If more than one certificate names a root, this ensures that the newest locator is used. + if ((!existingLoc) || (existingLoc->revision() < loc->revision())) + existingLoc.set(new Locator(*loc)); + } } } } + } } return r; @@ -66,7 +74,7 @@ Vector< SharedPtr< TrustStore::Entry > > TrustStore::all(const bool includeRejec void TrustStore::add(const Certificate &cert, const unsigned int localTrust) { RWMutex::Lock l(m_lock); - m_addQueue.push_front(SharedPtr< Entry >(new Entry(cert, localTrust))); + m_addQueue.push_front(SharedPtr< Entry >(new Entry(this->m_lock, cert, localTrust))); } void TrustStore::erase(const H384 &serial) @@ -79,23 +87,32 @@ bool TrustStore::update(const int64_t clock, Vector< SharedPtr< Entry > > *const { RWMutex::Lock l(m_lock); - // (Re)compute error codes for existing certs, but we don't have to do a full - // signature check here since that's done when they're taken out of the add queue. + // Check for certificate time validity status changes. If any of these occur then + // full re-validation is required. bool errorStateModified = false; for (Map< H384, SharedPtr< Entry > >::const_iterator c(m_bySerial.begin()); c != m_bySerial.end(); ++c) { - const ZT_CertificateError err = c->second->error(); - if ((err != ZT_CERTIFICATE_ERROR_HAVE_NEWER_CERT) && (err != ZT_CERTIFICATE_ERROR_INVALID_CHAIN)) { - const ZT_CertificateError newErr = c->second->m_certificate.verify(clock, false); - if (newErr != err) { - c->second->m_error.store((int)newErr, std::memory_order_relaxed); - errorStateModified = true; - } + const bool timeValid = c->second->m_certificate.verifyTimeWindow(clock); + switch (c->second->m_error) { + case ZT_CERTIFICATE_ERROR_NONE: + case ZT_CERTIFICATE_ERROR_INVALID_CHAIN: + if (!timeValid) { + c->second->m_error = ZT_CERTIFICATE_ERROR_OUT_OF_VALID_TIME_WINDOW; + errorStateModified = true; + } + break; + case ZT_CERTIFICATE_ERROR_OUT_OF_VALID_TIME_WINDOW: + if (timeValid) { + c->second->m_error = c->second->m_certificate.verify(-1, false); + errorStateModified = true; + } + break; + default: + break; } } - // 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 there were not any such changes and if the add and delete queues are empty, + // there is nothing more to be done. if ((!errorStateModified) && (m_addQueue.empty()) && (m_deleteQueue.empty())) return false; @@ -103,8 +120,9 @@ bool TrustStore::update(const int64_t clock, Vector< SharedPtr< Entry > > *const // have yet to have their full certificate chains validated. Full signature checking is // performed here. while (!m_addQueue.empty()) { - m_addQueue.front()->m_error.store((int)m_addQueue.front()->m_certificate.verify(clock, true), std::memory_order_relaxed); - m_bySerial[H384(m_addQueue.front()->m_certificate.serialNo)].move(m_addQueue.front()); + SharedPtr< Entry > &qi = m_addQueue.front(); + qi->m_error = qi->m_certificate.verify(clock, true); + m_bySerial[H384(qi->m_certificate.serialNo)].move(qi); m_addQueue.pop_front(); } @@ -114,81 +132,102 @@ bool TrustStore::update(const int64_t clock, Vector< SharedPtr< Entry > > *const m_deleteQueue.pop_front(); } - Map< H384, Vector< SharedPtr< Entry > > > bySignedCert; - for (;;) { - // Validate certificate paths and reject any certificates that do not trace back to a CA. - for (Map< H384, SharedPtr< Entry > >::iterator c(m_bySerial.begin()); c != m_bySerial.end(); ++c) { - if (c->second->error() == ZT_CERTIFICATE_ERROR_NONE) { - unsigned int pathLength = 0; - Map< H384, SharedPtr< Entry > >::const_iterator current(c); - Set< H384 > visited; // prevent infinite loops if there's a cycle - for (;;) { - if (pathLength <= current->second->m_certificate.maxPathLength) { - if ((current->second->localTrust() & ZT_CERTIFICATE_LOCAL_TRUST_FLAG_ROOT_CA) == 0) { - visited.insert(current->second->m_certificate.getSerialNo()); - const H384 next(current->second->m_certificate.issuer); - if (visited.find(next) == visited.end()) { - current = m_bySerial.find(next); - if ((current != m_bySerial.end()) && (current->second->error() == ZT_CERTIFICATE_ERROR_NONE)) { - ++pathLength; - continue; - } - } - } else { - break; // traced to root CA, abort without setting error - } - } - 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< H384, SharedPtr< Entry > >::const_iterator c(m_bySerial.begin()); c != m_bySerial.end();) { - if (c->second->error() == ZT_CERTIFICATE_ERROR_NONE) { - const unsigned int uniqueIdSize = c->second->m_certificate.subject.uniqueIdSize; - if ((uniqueIdSize > 0) && (uniqueIdSize <= ZT_CERTIFICATE_MAX_PUBLIC_KEY_SIZE)) { - SharedPtr< Entry > ¤t = m_bySubjectUniqueId[Blob< ZT_CERTIFICATE_MAX_PUBLIC_KEY_SIZE >(c->second->m_certificate.subject.uniqueId, uniqueIdSize)]; - if (current) { - exitLoop = false; - if (c->second->m_certificate.subject.timestamp > current->m_certificate.subject.timestamp) { - 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; - } - } - } - } - - // 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(); + // Reset flags for deprecation and a cert being on a trust path, which are + // recomputed when chain and subjects are checked below. + for (Map< H384, SharedPtr< Entry > >::const_iterator c(m_bySerial.begin()); c != m_bySerial.end(); ++c) { + if (c->second->m_error == ZT_CERTIFICATE_ERROR_NONE) { + c->second->m_subjectDeprecated = false; + c->second->m_onTrustPath = false; } } - // Populate mapping of identities to certificates whose subjects reference them. + // Validate certificate trust paths. + { + Vector< Entry * > visited; + visited.reserve(8); + for (Map< H384, SharedPtr< Entry > >::iterator c(m_bySerial.begin()); c != m_bySerial.end(); ++c) { + if ((c->second->m_error == ZT_CERTIFICATE_ERROR_NONE) && (!c->second->m_onTrustPath) && ((c->second->m_localTrust & ZT_CERTIFICATE_LOCAL_TRUST_FLAG_ROOT_CA) == 0)) { + // Trace the path of each certificate all the way back to a trusted CA. + unsigned int pathLength = 0; + Map< H384, SharedPtr< Entry > >::const_iterator current(c); + visited.clear(); + for (;;) { + if (pathLength <= current->second->m_certificate.maxPathLength) { + + // Check if this cert isn't a CA or already part of a valid trust path. If so then step upward toward CA. + if (((current->second->m_localTrust & ZT_CERTIFICATE_LOCAL_TRUST_FLAG_ROOT_CA) == 0) && (!current->second->m_onTrustPath)) { + // If the issuer (parent) certificiate is (1) valid, (2) not already visited (to prevent loops), + // and (3) has a public key that matches this cert's issuer public key (sanity check), proceed + // up the certificate graph toward a potential CA. + visited.push_back(current->second.ptr()); + const Map< H384, SharedPtr< Entry > >::const_iterator prevChild(current); + current = m_bySerial.find(H384(current->second->m_certificate.issuer)); + if ((current != m_bySerial.end()) && + (std::find(visited.begin(), visited.end(), current->second.ptr()) == visited.end()) && + (current->second->m_error == ZT_CERTIFICATE_ERROR_NONE) && + (current->second->m_certificate.publicKeySize == prevChild->second->m_certificate.issuerPublicKeySize) && + (memcmp(current->second->m_certificate.publicKey, prevChild->second->m_certificate.issuerPublicKey, current->second->m_certificate.publicKeySize) == 0)) { + ++pathLength; + continue; + } + } else { + // If we've traced this to a root CA, flag its parents as also being on a trust path. Then + // break the loop without setting an error. We don't flag the current cert as being on a + // trust path since no other certificates depend on it. + for (Vector< Entry * >::const_iterator v(visited.begin()); v != visited.end(); ++v) { + if (*v != c->second.ptr()) + (*v)->m_onTrustPath = true; + } + break; + } + + } + + // If we made it here without breaking or continuing, no path to a + // CA was found and the certificate's chain is invalid. + c->second->m_error = ZT_CERTIFICATE_ERROR_INVALID_CHAIN; + break; + } + } + } + } + + // Repopulate mapping of subject unique IDs to their certificates, marking older + // certificates for the same subject as deprecated. A deprecated certificate is not invalid + // but will be purged if it is also not part of a trust path. Error certificates are ignored. + m_bySubjectUniqueId.clear(); + for (Map< H384, SharedPtr< Entry > >::const_iterator c(m_bySerial.begin()); c != m_bySerial.end();) { + if (c->second->m_error == ZT_CERTIFICATE_ERROR_NONE) { + const unsigned int uniqueIdSize = c->second->m_certificate.subject.uniqueIdSize; + if ((uniqueIdSize > 0) && (uniqueIdSize <= ZT_CERTIFICATE_MAX_PUBLIC_KEY_SIZE)) { + SharedPtr< Entry > &entry = m_bySubjectUniqueId[Blob< ZT_CERTIFICATE_MAX_PUBLIC_KEY_SIZE >(c->second->m_certificate.subject.uniqueId, uniqueIdSize)]; + if (entry) { + if (c->second->m_certificate.subject.timestamp > entry->m_certificate.subject.timestamp) { + entry->m_subjectDeprecated = true; + entry = c->second; + } else if (c->second->m_certificate.subject.timestamp < entry->m_certificate.subject.timestamp) { + c->second->m_subjectDeprecated = true; + } else { + // Equal timestamps should never happen, but handle it anyway by comparing serials. + if (memcmp(c->second->m_certificate.serialNo, entry->m_certificate.serialNo, ZT_CERTIFICATE_HASH_SIZE) > 0) { + entry->m_subjectDeprecated = true; + entry = c->second; + } else { + c->second->m_subjectDeprecated = true; + } + } + } else { + entry = c->second; + } + } + } + } + + // Populate mapping of identities to certificates whose subjects reference them, ignoring + // error or deprecated certificates. m_bySubjectIdentity.clear(); for (Map< H384, SharedPtr< Entry > >::const_iterator c(m_bySerial.begin()); c != m_bySerial.end(); ++c) { - if (c->second->error() == ZT_CERTIFICATE_ERROR_NONE) { + if ((c->second->m_error == ZT_CERTIFICATE_ERROR_NONE) && (!c->second->m_subjectDeprecated)) { for (unsigned int i = 0; i < c->second->m_certificate.subject.identityCount; ++i) { const Identity *const id = reinterpret_cast(c->second->m_certificate.subject.identities[i].identity); if ((id) && (*id)) // sanity check @@ -197,10 +236,12 @@ bool TrustStore::update(const int64_t clock, Vector< SharedPtr< Entry > > *const } } - // Purge and return purged certificates if this option is selected. + // Purge error certificates and return them if 'purge' is non-NULL. This purges error certificates + // and deprecated certificates not on a trust path. Deprecated certificates on a trust path remain + // as they are still technically valid and other possibly wanted certificates depend on them. if (purge) { for (Map< H384, SharedPtr< Entry > >::const_iterator c(m_bySerial.begin()); c != m_bySerial.end();) { - if (c->second->error() != ZT_CERTIFICATE_ERROR_NONE) { + if ( (c->second->error() != ZT_CERTIFICATE_ERROR_NONE) || ((c->second->m_subjectDeprecated) && (!c->second->m_onTrustPath)) ) { purge->push_back(c->second); m_bySerial.erase(c++); } else { @@ -214,7 +255,7 @@ bool TrustStore::update(const int64_t clock, Vector< SharedPtr< Entry > > *const Vector< uint8_t > TrustStore::save() const { - Vector< uint8_t> comp; + Vector< uint8_t > comp; int compSize; { @@ -223,24 +264,26 @@ Vector< uint8_t > TrustStore::save() const RWMutex::RLock l(m_lock); - b.push_back(0); // version byte + // A version byte. + b.push_back(0); + // tuples terminated by a 0 size. for (Map< H384, SharedPtr< Entry > >::const_iterator c(m_bySerial.begin()); c != m_bySerial.end(); ++c) { const Vector< uint8_t > cdata(c->second->certificate().encode()); - if (!cdata.empty()) { + const unsigned long size = (uint32_t)cdata.size(); + if ((size > 0) && (size <= 0xffff)) { + b.push_back((uint8_t)(size >> 8U)); + b.push_back((uint8_t)size); + b.insert(b.end(), cdata.begin(), cdata.end()); const uint32_t localTrust = (uint32_t)c->second->localTrust(); b.push_back((uint8_t)(localTrust >> 24U)); b.push_back((uint8_t)(localTrust >> 16U)); b.push_back((uint8_t)(localTrust >> 8U)); b.push_back((uint8_t)localTrust); - const uint32_t size = (uint32_t)cdata.size(); - b.push_back((uint8_t)(size >> 24U)); - b.push_back((uint8_t)(size >> 16U)); - b.push_back((uint8_t)(size >> 8U)); - b.push_back((uint8_t)size); - b.insert(b.end(), cdata.begin(), cdata.end()); } } + b.push_back(0); + b.push_back(0); comp.resize((unsigned long)LZ4_COMPRESSBOUND(b.size()) + 8); compSize = LZ4_compress_fast(reinterpret_cast(b.data()), reinterpret_cast(comp.data() + 8), (int)b.size(), (int)(comp.size() - 8)); @@ -271,10 +314,8 @@ int TrustStore::load(const Vector< uint8_t > &data) if (data.size() < 8) return -1; - const unsigned long uncompSize = Utils::loadBigEndian(data.data()); - if (uncompSize > (data.size() * 256)) // sanity check - return -1; - if (uncompSize < 1) // no room for at least version and count + const unsigned long uncompSize = Utils::loadBigEndian< uint32_t >(data.data()); + if ((uncompSize == 0) || (uncompSize > (data.size() * 128))) return -1; Vector< uint8_t > uncomp; @@ -283,7 +324,7 @@ int TrustStore::load(const Vector< uint8_t > &data) if (LZ4_decompress_safe(reinterpret_cast(data.data() + 8), reinterpret_cast(uncomp.data()), (int)(data.size() - 8), (int)uncompSize) != (int)uncompSize) return -1; const uint8_t *b = uncomp.data(); - if (Utils::fnv1a32(b, (unsigned int)uncompSize) != Utils::loadBigEndian(data.data() + 4)) + if (Utils::fnv1a32(b, (unsigned int)uncompSize) != Utils::loadBigEndian< uint32_t >(data.data() + 4)) return -1; const uint8_t *const eof = b + uncompSize; @@ -291,23 +332,28 @@ int TrustStore::load(const Vector< uint8_t > &data) return -1; int readCount = 0; - for(;;) { - if ((b + 8) > eof) - break; - const uint32_t localTrust = Utils::loadBigEndian(b); - b += 4; - const uint32_t cdataSize = Utils::loadBigEndian(b); - b += 4; - if ((b + cdataSize) > eof) + for (;;) { + if ((b + 2) > eof) + break; + const uint32_t cdataSize = Utils::loadBigEndian< uint16_t >(b); + b += 2; + + if (cdataSize == 0) + break; + + if ((b + cdataSize + 4) > eof) break; Certificate c; if (c.decode(b, (unsigned int)cdataSize)) { + b += cdataSize; + const uint32_t localTrust = Utils::loadBigEndian< uint32_t >(b); + b += 4; this->add(c, (unsigned int)localTrust); ++readCount; } - b += cdataSize; } + return readCount; } diff --git a/core/TrustStore.hpp b/core/TrustStore.hpp index 90b85c1b3..a6401cb1d 100644 --- a/core/TrustStore.hpp +++ b/core/TrustStore.hpp @@ -28,7 +28,20 @@ namespace ZeroTier { /** - * Certificate store and chain validator + * Certificate store and chain validator. + * + * WARNING: SharedPtr entries returned from a trust store are valid + * only as long as the trust store exists. The trust store is a core object + * that lives as long as a Node, so this isn't an issue in the core, but it + * should be remembered when testing. + * + * This is because each Entry includes a reference to its parent's mutex and + * is synchronized by this mutex so its fields are safe to access while the + * parent trust store is being modified or synchronized. + * + * This also means entries can't be moved between TrustStore instances, + * hence there are no methods for doing that. There's only one instance in a + * node anyway. */ class TrustStore { @@ -57,7 +70,10 @@ public: * @return Local trust bit mask */ ZT_INLINE unsigned int localTrust() const noexcept - { return m_localTrust.load(std::memory_order_relaxed); } + { + RWMutex::RLock l(m_lock); + return m_localTrust; + } /** * Change the local trust of this entry @@ -65,7 +81,10 @@ public: * @param lt New local trust bit mask */ ZT_INLINE void setLocalTrust(const unsigned int lt) noexcept - { m_localTrust.store(lt, std::memory_order_relaxed); } + { + RWMutex::Lock l(m_lock); + m_localTrust = lt; + } /** * Get the error code for this certificate @@ -73,23 +92,30 @@ public: * @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); } + { + RWMutex::RLock l(m_lock); + return m_error; + } private: - Entry() {} - Entry(const Entry &) {} Entry &operator=(const Entry &) { return *this; } - ZT_INLINE Entry(const Certificate &cert, const unsigned int lt) noexcept: + ZT_INLINE Entry(RWMutex &l, const Certificate &cert, const unsigned int lt) noexcept: + m_lock(l), m_certificate(cert), m_localTrust(lt), - m_error((int)ZT_CERTIFICATE_ERROR_NONE) + m_error(ZT_CERTIFICATE_ERROR_NONE), + m_subjectDeprecated(false), + m_onTrustPath(false) {} - Certificate m_certificate; - std::atomic< unsigned int > m_localTrust; - std::atomic< int > m_error; + RWMutex &m_lock; + const Certificate m_certificate; + unsigned int m_localTrust; + ZT_CertificateError m_error; std::atomic< int > __refCount; + bool m_subjectDeprecated; + bool m_onTrustPath; }; TrustStore(); diff --git a/core/zerotier.h b/core/zerotier.h index b9b366968..05dc7fa3a 100644 --- a/core/zerotier.h +++ b/core/zerotier.h @@ -322,11 +322,6 @@ typedef struct */ #define ZT_CERTIFICATE_MAX_STRING_LENGTH 127 -/** - * Maximum certificate path length to CA (a sanity limit value) - */ -#define ZT_CERTIFICATE_MAX_PATH_LENGTH 256 - /** * Certificate is a root CA (local trust flag) */ @@ -347,50 +342,50 @@ enum ZT_CertificateError */ ZT_CERTIFICATE_ERROR_NONE = 0, - /** - * A newer certificate with the same issuer and subject serial plus CN exists. - */ - ZT_CERTIFICATE_ERROR_HAVE_NEWER_CERT = 1, - /** * Certificate format is invalid or required fields are missing */ - ZT_CERTIFICATE_ERROR_INVALID_FORMAT = -1, + ZT_CERTIFICATE_ERROR_INVALID_FORMAT = 1, /** * One or more identities in the certificate are invalid or fail consistency check */ - ZT_CERTIFICATE_ERROR_INVALID_IDENTITY = -2, + ZT_CERTIFICATE_ERROR_INVALID_IDENTITY = 2, /** * Certificate primary signature is invalid */ - ZT_CERTIFICATE_ERROR_INVALID_PRIMARY_SIGNATURE = -3, + ZT_CERTIFICATE_ERROR_INVALID_PRIMARY_SIGNATURE = 3, /** * Full chain validation of certificate failed */ - ZT_CERTIFICATE_ERROR_INVALID_CHAIN = -4, + ZT_CERTIFICATE_ERROR_INVALID_CHAIN = 4, /** * One or more signed components (e.g. a Locator) has an invalid signature. */ - ZT_CERTIFICATE_ERROR_INVALID_COMPONENT_SIGNATURE = -5, + ZT_CERTIFICATE_ERROR_INVALID_COMPONENT_SIGNATURE = 5, /** * Unique ID proof signature in subject was not valid. */ - ZT_CERTIFICATE_ERROR_INVALID_UNIQUE_ID_PROOF = -6, + ZT_CERTIFICATE_ERROR_INVALID_UNIQUE_ID_PROOF = 6, /** * Certificate is missing a required field */ - ZT_CERTIFICATE_ERROR_MISSING_REQUIRED_FIELDS = -7, + ZT_CERTIFICATE_ERROR_MISSING_REQUIRED_FIELDS = 7, /** * Certificate is expired or not yet in effect */ - ZT_CERTIFICATE_ERROR_OUT_OF_VALID_TIME_WINDOW = -8 + ZT_CERTIFICATE_ERROR_OUT_OF_VALID_TIME_WINDOW = 8, + + /** + * Certificate explicitly revoked + */ + ZT_CERTIFICATE_ERROR_REVOKED = 9 }; /** @@ -429,6 +424,13 @@ enum ZT_CertificatePublicKeyAlgorithm */ #define ZT_CERTIFICATE_HASH_SIZE 48 +/* + * Maximum number of certificates that can be revoked at once. + * + * This shouldn't be changed and is set to be small enough to fit in a packet. + */ +#define ZT_CERTIFICATE_REVOCATION_MAX_CERTIFICATES 24 + /** * Information about a real world entity. * @@ -506,7 +508,7 @@ typedef struct /** * URLs that can be consulted for updates to this certificate. */ - const char *const *updateURLs; + const char **updateURLs; /** * Number of identities @@ -644,6 +646,47 @@ typedef struct unsigned int maxPathLength; } ZT_Certificate; +/** + * A revocation for one or more certificates. + */ +typedef struct +{ + /** + * Certificate issuing this revocation. + */ + uint8_t issuer[ZT_CERTIFICATE_HASH_SIZE]; + + /** + * Timestamp in milliseconds since epoch. + */ + int64_t timestamp; + + /** + * Revoked certificate serials. + */ + uint8_t serials[ZT_CERTIFICATE_REVOCATION_MAX_CERTIFICATES][ZT_CERTIFICATE_HASH_SIZE]; + + /** + * Short optional human-readable reason or URL. + */ + char reason[ZT_CERTIFICATE_MAX_STRING_LENGTH + 1]; + + /** + * Signature of revocation by revoking issuer. + */ + uint8_t signature[ZT_CERTIFICATE_MAX_SIGNATURE_SIZE]; + + /** + * Number of revoked certificates. + */ + unsigned int count; + + /** + * Size of signature in bytes. + */ + unsigned int signatureSize; +} ZT_CertificateRevocation; + /** * A list of certificates */ diff --git a/rust-zerotier-core/src/certificate.rs b/rust-zerotier-core/src/certificate.rs index 3a58733a0..661ae31f9 100644 --- a/rust-zerotier-core/src/certificate.rs +++ b/rust-zerotier-core/src/certificate.rs @@ -105,7 +105,6 @@ impl<'de> serde::Deserialize<'de> for CertificateSerialNo { #[derive(FromPrimitive, ToPrimitive, PartialEq, Eq, Clone, Copy)] pub enum CertificateError { None = ztcore::ZT_CertificateError_ZT_CERTIFICATE_ERROR_NONE as isize, - HaveNewerCert = ztcore::ZT_CertificateError_ZT_CERTIFICATE_ERROR_HAVE_NEWER_CERT as isize, InvalidFormat = ztcore::ZT_CertificateError_ZT_CERTIFICATE_ERROR_INVALID_FORMAT as isize, InvalidIdentity = ztcore::ZT_CertificateError_ZT_CERTIFICATE_ERROR_INVALID_IDENTITY as isize, InvalidPrimarySignature = ztcore::ZT_CertificateError_ZT_CERTIFICATE_ERROR_INVALID_PRIMARY_SIGNATURE as isize, @@ -114,6 +113,7 @@ pub enum CertificateError { InvalidUniqueIdProof = ztcore::ZT_CertificateError_ZT_CERTIFICATE_ERROR_INVALID_UNIQUE_ID_PROOF as isize, MissingRequiredFields = ztcore::ZT_CertificateError_ZT_CERTIFICATE_ERROR_MISSING_REQUIRED_FIELDS as isize, OutOfValidTimeWindow = ztcore::ZT_CertificateError_ZT_CERTIFICATE_ERROR_OUT_OF_VALID_TIME_WINDOW as isize, + Revoked = ztcore::ZT_CertificateError_ZT_CERTIFICATE_ERROR_REVOKED as isize, } impl ToString for CertificateError { @@ -121,7 +121,6 @@ impl ToString for CertificateError { String::from( match self { CertificateError::None => "None", - CertificateError::HaveNewerCert => "HaveNewerCert", CertificateError::InvalidFormat => "InvalidFormat", CertificateError::InvalidIdentity => "InvalidIdentity", CertificateError::InvalidPrimarySignature => "InvalidPrimarySignature", @@ -129,7 +128,8 @@ impl ToString for CertificateError { CertificateError::InvalidComponentSignature => "InvalidComponentSignature", CertificateError::InvalidUniqueIdProof => "InvalidUniqueIdProof", CertificateError::MissingRequiredFields => "MissingRequiredFields", - CertificateError::OutOfValidTimeWindow => "OutOfValidTimeWindow" + CertificateError::OutOfValidTimeWindow => "OutOfValidTimeWindow", + CertificateError::Revoked => "Revoked", } ) } @@ -138,7 +138,6 @@ impl ToString for CertificateError { impl> From for CertificateError { fn from(s: S) -> CertificateError { match s.as_ref().to_ascii_lowercase().as_str() { - "havenewercert" => CertificateError::HaveNewerCert, "invalidformat" => CertificateError::InvalidFormat, "invalididentity" => CertificateError::InvalidIdentity, "invalidprimarysignature" => CertificateError::InvalidPrimarySignature, @@ -147,6 +146,7 @@ impl> From for CertificateError { "invaliduniqueidproof" => CertificateError::InvalidUniqueIdProof, "missingrequiredfields" => CertificateError::MissingRequiredFields, "outofvalidtimewindow" => CertificateError::OutOfValidTimeWindow, + "revoked" => CertificateError::Revoked, _ => CertificateError::None } } @@ -348,10 +348,10 @@ pub struct CertificateSubject { #[allow(unused)] pub(crate) struct CertificateSubjectCAPIContainer { pub(crate) subject: ztcore::ZT_Certificate_Subject, - subject_identities: Pin>, - subject_networks: Pin>, - subject_urls: Pin>, - subject_urls_strs: Pin>, + subject_identities: Option>>, + subject_networks: Option>>, + subject_urls: Option>>, + subject_urls_strs: Option>>, } impl CertificateSubject { @@ -407,51 +407,64 @@ impl CertificateSubject { } pub(crate) unsafe fn to_capi(&self) -> CertificateSubjectCAPIContainer { - let mut capi_identities: Vec = Vec::new(); - let mut capi_networks: Vec = Vec::new(); - let mut capi_urls: Vec<*const c_char> = Vec::new(); - let mut capi_urls_strs: Vec = Vec::new(); + let mut identity_count: c_uint = 0; + let mut network_count: c_uint = 0; + let mut update_url_count: c_uint = 0; - if !self.identities.is_empty() { + let mut capi_identities = if self.identities.is_empty() { + None + } else { + let mut capi_identities: Vec = Vec::new(); capi_identities.reserve(self.identities.len()); for i in self.identities.iter() { capi_identities.push((*i).to_capi()); } - } - if !self.networks.is_empty() { + identity_count = capi_identities.len() as c_uint; + Some(Pin::from(capi_identities.into_boxed_slice())) + }; + + let mut capi_networks = if self.networks.is_empty() { + None + } else { + let mut capi_networks: Vec = Vec::new(); capi_networks.reserve(self.networks.len()); for i in self.networks.iter() { capi_networks.push((*i).to_capi()); } - } - if !self.update_urls.is_empty() { - capi_urls.reserve(self.update_urls.len()); + network_count = capi_networks.len() as c_uint; + Some(Pin::from(capi_networks.into_boxed_slice())) + }; + + let (mut capi_urls, capi_urls_strs) = if self.update_urls.is_empty() { + (None, None) + } else { + let mut capi_urls_strs: Vec = Vec::new(); + let mut capi_urls: Vec<*const c_char> = Vec::new(); capi_urls_strs.reserve(self.update_urls.len()); + capi_urls.reserve(self.update_urls.len()); for i in self.update_urls.iter() { let cs = CString::new((*i).as_str()); if cs.is_ok() { capi_urls_strs.push(cs.unwrap()); } } + let capi_urls_strs = Pin::from(capi_urls_strs.into_boxed_slice()); for i in capi_urls_strs.iter() { capi_urls.push((*i).as_ptr()); } - } - - let mut capi_identities = Pin::from(capi_identities.into_boxed_slice()); - let mut capi_networks = Pin::from(capi_networks.into_boxed_slice()); - let mut capi_urls = Pin::from(capi_urls.into_boxed_slice()); - let mut capi_urls_strs = Pin::from(capi_urls_strs.into_boxed_slice()); + update_url_count = capi_urls.len() as c_uint; + (Some(Pin::from(capi_urls.into_boxed_slice())), Some(capi_urls_strs)) + }; CertificateSubjectCAPIContainer { subject: ztcore::ZT_Certificate_Subject { timestamp: self.timestamp, - identities: capi_identities.as_mut_ptr(), - networks: capi_networks.as_mut_ptr(), - updateURLs: capi_urls.as_ptr(), - identityCount: capi_identities.len() as c_uint, - networkCount: capi_networks.len() as c_uint, - updateURLCount: capi_urls.len() as c_uint, + identities: capi_identities.as_mut().map_or(null_mut(), |v| v.as_mut_ptr()), + networks: capi_networks.as_mut().map_or(null_mut(), |v| v.as_mut_ptr()), + updateURLs: capi_urls.as_mut().map_or(null_mut(), |v| v.as_mut_ptr()), + identityCount: identity_count, + networkCount: network_count, + updateURLCount: update_url_count, name: self.name.to_capi(), uniqueId: vec_to_array(&self.unique_id), uniqueIdSignature: vec_to_array(&self.unique_id_signature),