From daef6a24d133de6871d0688b52c63b6cde198827 Mon Sep 17 00:00:00 2001 From: Adam Ierymenko Date: Sat, 27 Jun 2020 13:52:12 -0700 Subject: [PATCH] Now with less bugs, and less limits that could pose an issue later. --- core/Certificate.cpp | 81 +++++++++++++++++++++++++------------------- core/Certificate.hpp | 13 ++----- core/Dictionary.cpp | 22 ++++-------- core/Dictionary.hpp | 24 ++++++------- core/Tests.cpp | 42 ++++++++++++++++------- core/Topology.cpp | 8 ++--- core/Topology.hpp | 2 +- core/zerotier.h | 16 ++------- 8 files changed, 104 insertions(+), 104 deletions(-) diff --git a/core/Certificate.cpp b/core/Certificate.cpp index d2d7a91d2..6ef585b71 100644 --- a/core/Certificate.cpp +++ b/core/Certificate.cpp @@ -31,6 +31,9 @@ void Certificate::clear() m_updateUrls.clear(); m_subjectCertificates.clear(); m_extendedAttributes.clear(); + m_subjectUniqueId.clear(); + m_subjectUniqueIdProofSignature.clear(); + m_signature.clear(); } Certificate &Certificate::operator=(const ZT_Certificate &apiCert) @@ -176,8 +179,8 @@ Vector< uint8_t > Certificate::encode(const bool omitSignature) const if (this->flags != 0) d.add("f", this->flags); d.add("t", (uint64_t)this->timestamp); - d.add("v0", (uint64_t)this->validity[0]); - d.add("v1", (uint64_t)this->validity[1]); + d.add("v#0", (uint64_t)this->validity[0]); + d.add("v#1", (uint64_t)this->validity[1]); if ((this->extendedAttributes) && (this->extendedAttributesSize > 0)) d["x"].assign(this->extendedAttributes, this->extendedAttributes + this->extendedAttributesSize); d.add("mP", (uint64_t)this->maxPathLength); @@ -231,14 +234,16 @@ bool Certificate::decode(const Vector< uint8_t > &data) this->flags = d.getUI("f"); this->timestamp = (int64_t)d.getUI("t"); - this->validity[0] = (int64_t)d.getUI("v0"); - this->validity[1] = (int64_t)d.getUI("v1"); + this->validity[0] = (int64_t)d.getUI("v#0"); + this->validity[1] = (int64_t)d.getUI("v#1"); this->maxPathLength = (unsigned int)d.getUI("mP"); + m_extendedAttributes = d["x"]; if (!m_extendedAttributes.empty()) { this->extendedAttributes = m_extendedAttributes.data(); this->extendedAttributesSize = (unsigned int)m_extendedAttributes.size(); } + this->subject.timestamp = (int64_t)d.getUI("s.t"); unsigned int cnt = (unsigned int)d.getUI("s.i$"); @@ -280,6 +285,10 @@ bool Certificate::decode(const Vector< uint8_t > &data) this->addSubjectCertificate(serial.data()); } + cnt = (unsigned int)d.getUI("s.u$"); + for (unsigned int i = 0; i < cnt; ++i) + addSubjectUpdateUrl(d.getS(Dictionary::arraySubscript(tmp, "s.u$", i), tmp, sizeof(tmp))); + d.getS("s.n.sN", this->subject.name.serialNo, sizeof(this->subject.name.serialNo)); d.getS("s.n.cN", this->subject.name.commonName, sizeof(this->subject.name.commonName)); d.getS("s.n.c", this->subject.name.country, sizeof(this->subject.name.country)); @@ -293,6 +302,17 @@ bool Certificate::decode(const Vector< uint8_t > &data) d.getS("s.n.ur", this->subject.name.url, sizeof(this->subject.name.url)); d.getS("s.n.h", this->subject.name.host, sizeof(this->subject.name.host)); + m_subjectUniqueId = d["s.uI"]; + if (!m_subjectUniqueId.empty()) { + this->subject.uniqueId = m_subjectUniqueId.data(); + this->subject.uniqueIdSize = (unsigned int)m_subjectUniqueId.size(); + } + m_subjectUniqueIdProofSignature = d["s.uS"]; + if (!m_subjectUniqueIdProofSignature.empty()) { + this->subject.uniqueIdProofSignature = m_subjectUniqueIdProofSignature.data(); + this->subject.uniqueIdProofSignatureSize = (unsigned int)m_subjectUniqueIdProofSignature.size(); + } + const Vector< uint8_t > &issuerData = d["i"]; if (!issuerData.empty()) { Identity id; @@ -323,13 +343,13 @@ bool Certificate::decode(const Vector< uint8_t > &data) else return false; } - const Vector< uint8_t > &sig = d["si"]; - if (sig.size() > sizeof(this->signature)) - return false; - Utils::copy(this->signature, sig.data(), (unsigned int)sig.size()); - this->signatureSize = (unsigned int)sig.size(); + m_signature = d["si"]; + if (!m_signature.empty()) { + this->signature = m_signature.data(); + this->signatureSize = (unsigned int)m_signature.size(); + } - Vector< uint8_t > enc(encode(true)); + const Vector< uint8_t > enc(encode(true)); SHA384(this->serialNo, enc.data(), (unsigned int)enc.size()); return true; @@ -339,7 +359,18 @@ bool Certificate::sign(const Identity &issuer) { Vector< uint8_t > enc(encode(true)); SHA384(this->serialNo, enc.data(), (unsigned int)enc.size()); - return (this->signatureSize = issuer.sign(enc.data(), (unsigned int)enc.size(), this->signature, sizeof(this->signature))) > 0; + uint8_t sig[ZT_SIGNATURE_BUFFER_SIZE]; + const unsigned int sigSize = issuer.sign(enc.data(), (unsigned int)enc.size(), sig, sizeof(sig)); + if (sigSize > 0) { + m_signature.assign(sig, sig + sigSize); + this->signature = m_signature.data(); + this->signatureSize = sigSize; + return true; + } + m_signature.clear(); + this->signature = nullptr; + this->signatureSize = 0; + return false; } ZT_CertificateError Certificate::verify() const @@ -354,8 +385,6 @@ ZT_CertificateError Certificate::verify() const } if (this->subject.uniqueIdProofSignatureSize > 0) { - static_assert(ZT_ECC384_SIGNATURE_SIZE <= ZT_CERTIFICATE_MAX_SIGNATURE_SIZE, "overflow"); - static_assert((ZT_ECC384_PUBLIC_KEY_SIZE + 1) <= ZT_CERTIFICATE_MAX_UNIQUE_ID_SIZE, "overflow"); if ( (this->subject.uniqueIdProofSignatureSize != ZT_ECC384_SIGNATURE_SIZE) || (this->subject.uniqueIdSize != (ZT_ECC384_PUBLIC_KEY_SIZE + 1)) || @@ -369,8 +398,8 @@ ZT_CertificateError Certificate::verify() const SHA384(h, enc.data(), (unsigned int)enc.size()); if (!ECC384ECDSAVerify(this->subject.uniqueId + 1, h, this->subject.uniqueIdProofSignature)) return ZT_CERTIFICATE_ERROR_INVALID_UNIQUE_ID_PROOF; - } else if (this->subject.uniqueIdSize > ZT_CERTIFICATE_MAX_UNIQUE_ID_SIZE) { - return ZT_CERTIFICATE_ERROR_INVALID_FORMAT; + } else if (this->subject.uniqueIdSize > 0) { + return ZT_CERTIFICATE_ERROR_INVALID_UNIQUE_ID_PROOF; } for (unsigned int i = 0; i < this->subject.identityCount; ++i) { @@ -404,24 +433,6 @@ ZT_CertificateError Certificate::verify() const return ZT_CERTIFICATE_ERROR_NONE; } -bool Certificate::setSubjectUniqueId(ZT_Certificate_Subject &s, const uint8_t uniqueId[ZT_CERTIFICATE_UNIQUE_ID_SIZE_TYPE_NIST_P_384], const uint8_t uniqueIdPrivate[ZT_CERTIFICATE_UNIQUE_ID_PRIVATE_KEY_SIZE_TYPE_NIST_P_384]) -{ - Utils::copy(s.uniqueId, uniqueId); - s.uniqueIdSize = ZT_CERTIFICATE_UNIQUE_ID_SIZE_TYPE_NIST_P_384; - - Dictionary d; - m_encodeSubject(s, d, true); - Vector< uint8_t > enc; - d.encode(enc); - uint8_t h[ZT_ECC384_SIGNATURE_HASH_SIZE]; - SHA384(h, enc.data(), (unsigned int)enc.size()); - - ECC384ECDSASign(uniqueIdPrivate, h, s.uniqueIdProofSignature); - s.uniqueIdProofSignatureSize = ZT_ECC384_SIGNATURE_SIZE; - - return true; -} - void Certificate::m_encodeSubject(const ZT_Certificate_Subject &s, Dictionary &d, bool omitUniqueIdProofSignature) { char tmp[256]; @@ -480,9 +491,9 @@ void Certificate::m_encodeSubject(const ZT_Certificate_Subject &s, Dictionary &d if (s.name.host[0]) d.add("s.n.h", s.name.host); - if ((s.uniqueIdSize > 0) && (s.uniqueIdSize <= ZT_CERTIFICATE_MAX_UNIQUE_ID_SIZE)) + if ((s.uniqueIdSize > 0) && (s.uniqueId != nullptr)) d["s.uI"].assign(s.uniqueId, s.uniqueId + s.uniqueIdSize); - if ((!omitUniqueIdProofSignature) && (s.uniqueIdProofSignatureSize > 0) && (s.uniqueIdProofSignatureSize <= ZT_CERTIFICATE_MAX_SIGNATURE_SIZE)) + if ((!omitUniqueIdProofSignature) && (s.uniqueIdProofSignatureSize > 0) && (s.uniqueIdProofSignature != nullptr)) d["s.uS"].assign(s.uniqueIdProofSignature, s.uniqueIdProofSignature + s.uniqueIdProofSignatureSize); } diff --git a/core/Certificate.hpp b/core/Certificate.hpp index 161a19a9f..44c8ff695 100644 --- a/core/Certificate.hpp +++ b/core/Certificate.hpp @@ -170,16 +170,6 @@ public: ECC384GenerateKey(uniqueId + 1, uniqueIdPrivate); } - /** - * Set the unique ID and unique ID proof signature fields in a subject. - * - * @param s Subject to set - * @param uniqueId Unique ID (public) - * @param uniqueIdPrivate Unique ID private key - * @return True on success - */ - static bool setSubjectUniqueId(ZT_Certificate_Subject &s, const uint8_t uniqueId[ZT_CERTIFICATE_UNIQUE_ID_SIZE_TYPE_NIST_P_384], const uint8_t uniqueIdPrivate[ZT_CERTIFICATE_UNIQUE_ID_PRIVATE_KEY_SIZE_TYPE_NIST_P_384]); - ZT_INLINE unsigned long hashCode() const noexcept { return (unsigned long)Utils::loadAsIsEndian< uint32_t >(this->serialNo); } @@ -213,6 +203,9 @@ private: Vector< const uint8_t * > m_subjectCertificates; Vector< const char * > m_updateUrls; Vector< uint8_t > m_extendedAttributes; + Vector< uint8_t > m_subjectUniqueId; + Vector< uint8_t > m_subjectUniqueIdProofSignature; + Vector< uint8_t > m_signature; std::atomic __refCount; }; diff --git a/core/Dictionary.cpp b/core/Dictionary.cpp index 765372c13..b2c93207d 100644 --- a/core/Dictionary.cpp +++ b/core/Dictionary.cpp @@ -95,19 +95,11 @@ bool Dictionary::getB(const char *k, bool dfl) const uint64_t Dictionary::getUI(const char *k, uint64_t dfl) const { - uint8_t tmp[18]; - uint64_t v = dfl; - const Vector< uint8_t > &e = (*this)[k]; - if (!e.empty()) { - if (e.back() != 0) { - const unsigned long sl = e.size(); - Utils::copy(tmp, e.data(), (sl > 17) ? 17 : sl); - tmp[17] = 0; - return Utils::unhex((const char *)tmp); - } - return Utils::unhex((const char *)e.data()); - } - return v; + char tmp[32]; + getS(k, tmp, sizeof(tmp)); + if (tmp[0]) + return Utils::unhex(tmp); + return dfl; } char *Dictionary::getS(const char *k, char *v, const unsigned int cap) const @@ -196,10 +188,8 @@ bool Dictionary::decode(const void *data, unsigned int len) } else if (c == (uint8_t)'=') { k.push_back(0); v = &m_entries[k]; - } else if (k.size() < 7) { - k.push_back(c); } else { - return false; + k.push_back(c); } } } diff --git a/core/Dictionary.hpp b/core/Dictionary.hpp index 653be053f..3e74f0049 100644 --- a/core/Dictionary.hpp +++ b/core/Dictionary.hpp @@ -404,14 +404,14 @@ private: } template< typename V > - ZT_INLINE static void s_appendKey(V &out, const char *const k) + ZT_INLINE static void s_appendKey(V &out, const char *k) { - for (unsigned int i = 0; i < 7; ++i) { - const char kc = k[i]; - if (kc == 0) + for (;;) { + char c = *(k++); + if (c == 0) break; - if ((kc >= 33) && (kc <= 126) && (kc != 61) && (kc != 92)) // printable ASCII with no spaces, equals, or backslash - out.push_back((uint8_t)kc); + if ((c >= 33) && (c <= 126) && (c != 61) && (c != 92)) // printable ASCII with no spaces, equals, or backslash + out.push_back((uint8_t)c); } out.push_back((uint8_t)'='); } @@ -419,15 +419,13 @@ private: ZT_INLINE static String s_key(const char *k) noexcept { String buf; - buf.clear(); - for (unsigned int i = 0; i < 7; ++i) { - const char kc = k[i]; - if (kc == 0) + for(;;) { + char c = *(k++); + if (c == 0) break; - if ((kc >= 33) && (kc <= 126) && (kc != 61) && (kc != 92)) // printable ASCII with no spaces, equals, or backslash - buf.push_back(kc); + if ((c >= 33) && (c <= 126) && (c != 61) && (c != 92)) // printable ASCII with no spaces, equals, or backslash + buf.push_back(c); } - buf.push_back(0); return buf; } diff --git a/core/Tests.cpp b/core/Tests.cpp index 21bba9945..35bbaf7b6 100644 --- a/core/Tests.cpp +++ b/core/Tests.cpp @@ -242,7 +242,7 @@ static bool ZTT_deepCompareCertificateIdentities(const ZT_Certificate_Identity * static bool ZTT_deepCompareCertificateName(const ZT_Certificate_Name &a, const ZT_Certificate_Name &b) { - return ( + return !( (strcmp(a.serialNo, b.serialNo) != 0) || (strcmp(a.streetAddress, b.streetAddress) != 0) || (strcmp(a.organization, b.organization) != 0) || @@ -260,14 +260,11 @@ static bool ZTT_deepCompareCertificateName(const ZT_Certificate_Name &a, const Z // This performs a detailed deep comparison of two certificates to catch any // potential encode/decode errors that might not be caught by just testing // for serial number (hash) equivalency... as the hash is computed from the -// decode output! +// decode output! Note that serial number is not compared here as this is +// checked by normal == operators. static bool ZTT_deepCompareCertificates(const Certificate &a, const Certificate &b) { - if (a != b) - return false; - if ( - (memcmp(a.serialNo, b.serialNo, sizeof(a.serialNo)) != 0) || (a.flags != b.flags) || (a.timestamp != b.timestamp) || (a.validity[0] != b.validity[0]) || @@ -282,11 +279,18 @@ static bool ZTT_deepCompareCertificates(const Certificate &a, const Certificate (a.signatureSize != b.signatureSize) ) return false; - if ( - (memcmp(a.subject.uniqueId, b.subject.uniqueId, a.subject.uniqueIdSize) != 0) || - (memcmp(a.subject.uniqueIdProofSignature, b.subject.uniqueIdProofSignature, a.subject.uniqueIdProofSignatureSize) != 0) || - (memcmp(a.signature, b.signature, a.signatureSize) != 0) - ) return false; + if ((a.subject.uniqueId == nullptr) != (b.subject.uniqueId == nullptr)) + return false; + if ((a.subject.uniqueIdProofSignature == nullptr) != (b.subject.uniqueIdProofSignature == nullptr)) + return false; + if ((a.subject.uniqueId != nullptr) && (a.subject.uniqueIdProofSignature != nullptr)) { + if ( + (memcmp(a.subject.uniqueId, b.subject.uniqueId, a.subject.uniqueIdSize) != 0) || + (memcmp(a.subject.uniqueIdProofSignature, b.subject.uniqueIdProofSignature, a.subject.uniqueIdProofSignatureSize) != 0) || + (memcmp(a.signature, b.signature, a.signatureSize) != 0) + ) + return false; + } if ((!ZTT_deepCompareCertificateName(a.subject.name, b.subject.name)) || (!ZTT_deepCompareCertificateName(a.issuerName, b.issuerName))) return false; @@ -300,6 +304,7 @@ static bool ZTT_deepCompareCertificates(const Certificate &a, const Certificate if (!ZTT_deepCompareCertificateIdentities(a.subject.identities + i, b.subject.identities + i)) return false; } + for(unsigned int i=0;i(&cert.issuerName, &cert.subject.name); - Certificate::setSubjectUniqueId(cert.subject, uniqueId, uniqueIdPrivate); + //Certificate::setSubjectUniqueId(cert.subject, uniqueId, uniqueIdPrivate); cert.sign(testIssuerId); Vector< uint8_t > enc(cert.encode()); ZT_T_PRINTF("OK (%d bytes)" ZT_EOL_S, (int)enc.size()); + + ZT_T_PRINTF(" Test certificate decode and verify... "); + Certificate cert2; + if (!cert2.decode(enc)) { + ZT_T_PRINTF("FAILED (decode)" ZT_EOL_S); + return "Certificate decode"; + } + if (!ZTT_deepCompareCertificates(cert, cert2)) { + ZT_T_PRINTF("FAILED (compare decoded with original)" ZT_EOL_S); + return "Certificate decode and compare"; + } } } catch (std::exception &e) { ZT_T_PRINTF(ZT_EOL_S "[crypto] Unexpected exception: %s" ZT_EOL_S, e.what()); diff --git a/core/Topology.cpp b/core/Topology.cpp index 028ef3d4b..4badc50d9 100644 --- a/core/Topology.cpp +++ b/core/Topology.cpp @@ -234,8 +234,8 @@ ZT_CertificateError Topology::addCertificate(void *tPtr, const Certificate &cert // the one we have if newer. Otherwise replace it. Note that the verification // function will have checked the unique ID proof signature already if a unique // ID was present. - FCV< uint8_t, ZT_CERTIFICATE_MAX_UNIQUE_ID_SIZE > uniqueId(cert.subject.uniqueId, cert.subject.uniqueIdSize); - if (!uniqueId.empty()) { + if ((cert.subject.uniqueId) && (cert.subject.uniqueIdSize > 0)) { + const Vector< uint8_t > uniqueId(cert.subject.uniqueId, cert.subject.uniqueId + cert.subject.uniqueIdSize); std::pair< SharedPtr< const Certificate >, unsigned int > &bySubjectUniqueId = m_certsBySubjectUniqueId[uniqueId]; if (bySubjectUniqueId.first) { if (bySubjectUniqueId.first->subject.timestamp >= cert.subject.timestamp) @@ -289,7 +289,7 @@ void Topology::m_eraseCertificate_l_certs(const SharedPtr< const Certificate > & m_certs.erase(SHA384Hash(cert->serialNo)); if (cert->subject.uniqueIdSize > 0) - m_certsBySubjectUniqueId.erase(FCV< uint8_t, ZT_CERTIFICATE_MAX_UNIQUE_ID_SIZE >(cert->subject.uniqueId, cert->subject.uniqueIdSize)); + m_certsBySubjectUniqueId.erase(Vector< uint8_t >(cert->subject.uniqueId, cert->subject.uniqueId + cert->subject.uniqueIdSize)); for (unsigned int i = 0; i < cert->subject.identityCount; ++i) { const Identity *const ii = reinterpret_cast(cert->subject.identities[i].identity); @@ -431,7 +431,7 @@ void Topology::m_updateRootPeers_l_roots_certs(void *tPtr) // Populate m_roots from certificate subject identities from certificates flagged // as local root set certificates. - for (SortedMap< FCV< uint8_t, ZT_CERTIFICATE_MAX_UNIQUE_ID_SIZE >, std::pair< SharedPtr< const Certificate >, unsigned int > >::const_iterator c(m_certsBySubjectUniqueId.begin()); c != m_certsBySubjectUniqueId.end(); ++c) { + for (SortedMap< Vector< uint8_t >, std::pair< SharedPtr< const Certificate >, unsigned int > >::const_iterator c(m_certsBySubjectUniqueId.begin()); c != m_certsBySubjectUniqueId.end(); ++c) { if ((c->second.second & ZT_CERTIFICATE_LOCAL_TRUST_FLAG_ZEROTIER_ROOT_SET) != 0) { for (unsigned int i = 0; i < c->second.first->subject.identityCount; ++i) m_roots[*reinterpret_cast(c->second.first->subject.identities[i].identity)].insert(c->second.first); diff --git a/core/Topology.hpp b/core/Topology.hpp index eb40c616f..462475926 100644 --- a/core/Topology.hpp +++ b/core/Topology.hpp @@ -273,7 +273,7 @@ private: Map< SHA384Hash, std::pair< SharedPtr< const Certificate >, unsigned int > > m_certs; Map< Fingerprint, Map< SharedPtr< const Certificate >, unsigned int > > m_certsBySubjectIdentity; - SortedMap< FCV< uint8_t, ZT_CERTIFICATE_MAX_UNIQUE_ID_SIZE >, std::pair< SharedPtr< const Certificate >, unsigned int > > m_certsBySubjectUniqueId; + SortedMap< Vector< uint8_t >, std::pair< SharedPtr< const Certificate >, unsigned int > > m_certsBySubjectUniqueId; }; } // namespace ZeroTier diff --git a/core/zerotier.h b/core/zerotier.h index 58d38353d..0358be8a0 100644 --- a/core/zerotier.h +++ b/core/zerotier.h @@ -298,16 +298,6 @@ typedef struct */ #define ZT_CERTIFICATE_MAX_STRING_LENGTH 127 -/** - * Maximum length of a signature - */ -#define ZT_CERTIFICATE_MAX_SIGNATURE_SIZE 96 - -/** - * Maximum size of a unique ID field in a certificate subject - */ -#define ZT_CERTIFICATE_MAX_UNIQUE_ID_SIZE 64 - /** * Certificate is a root CA */ @@ -517,12 +507,12 @@ typedef struct * A subject is valid if it has no unique ID or has one with a valid * proof signature. */ - uint8_t uniqueId[ZT_CERTIFICATE_MAX_UNIQUE_ID_SIZE]; + const uint8_t *uniqueId; /** * Signature proving ownership of unique ID. */ - uint8_t uniqueIdProofSignature[ZT_CERTIFICATE_MAX_SIGNATURE_SIZE]; + const uint8_t *uniqueIdProofSignature; /** * Size of unique ID in bytes or 0 if none. @@ -604,7 +594,7 @@ typedef struct /** * Signature by issuer (algorithm determined by identity type). */ - uint8_t signature[ZT_CERTIFICATE_MAX_SIGNATURE_SIZE]; + const uint8_t *signature; /** * Size of signature in bytes.