Now with less bugs!

core/Certificate.cpp

@@ -24,10 +24,12 @@ Certificate::Certificate() noexcept
 	Utils::zero< sizeof(ZT_Certificate) >(sup);
 }
 
-Certificate::Certificate(const ZT_Certificate &apiCert)
+Certificate::Certificate(const ZT_Certificate &apiCert) :
+	Certificate()
 { *this = apiCert; }
 
-Certificate::Certificate(const Certificate &cert)
+Certificate::Certificate(const Certificate &cert) :
+	Certificate()
 { *this = cert; }
 
 Certificate::~Certificate()
@@ -305,19 +307,16 @@ bool Certificate::decode(const void *const data, const unsigned int len)
 	unsigned int cnt = (unsigned int)d.getUI("s.i$");
 	for (unsigned int i = 0; i < cnt; ++i) {
 		const Vector< uint8_t > &identityData = d[Dictionary::arraySubscript(tmp, sizeof(tmp), "s.i$.i", i)];
-		if (identityData.empty()) {
+		if (identityData.empty())
 			return false;
-		}
 		Identity id;
-		if (id.unmarshal(identityData.data(), (unsigned int)identityData.size()) <= 0) {
+		if (id.unmarshal(identityData.data(), (unsigned int)identityData.size()) <= 0)
 			return false;
-		}
 		const Vector< uint8_t > &locatorData = d[Dictionary::arraySubscript(tmp, sizeof(tmp), "s.i$.l", i)];
 		if (!locatorData.empty()) {
 			Locator loc;
-			if (loc.unmarshal(locatorData.data(), (unsigned int)locatorData.size()) <= 0) {
+			if (loc.unmarshal(locatorData.data(), (unsigned int)locatorData.size()) <= 0)
 				return false;
-			}
 			this->addSubjectIdentity(id, loc);
 		} else {
 			this->addSubjectIdentity(id);
@@ -328,22 +327,19 @@ bool Certificate::decode(const void *const data, const unsigned int len)
 	for (unsigned int i = 0; i < cnt; ++i) {
 		const uint64_t nwid = d.getUI(Dictionary::arraySubscript(tmp, sizeof(tmp), "s.nw$.i", i));
 		const Vector< uint8_t > &fingerprintData = d[Dictionary::arraySubscript(tmp, sizeof(tmp), "s.nw$.c", i)];
-		if ((nwid == 0) || (fingerprintData.empty())) {
+		if ((nwid == 0) || (fingerprintData.empty()))
 			return false;
-		}
 		Fingerprint fp;
-		if (fp.unmarshal(fingerprintData.data(), (unsigned int)fingerprintData.size()) <= 0) {
+		if (fp.unmarshal(fingerprintData.data(), (unsigned int)fingerprintData.size()) <= 0)
 			return false;
-		}
 		this->addSubjectNetwork(nwid, fp);
 	}
 
 	cnt = (unsigned int)d.getUI("s.c$");
 	for (unsigned int i = 0; i < cnt; ++i) {
 		const Vector< uint8_t > &serial = d[Dictionary::arraySubscript(tmp, sizeof(tmp), "s.c$", i)];
-		if (serial.size() != ZT_SHA384_DIGEST_SIZE) {
+		if (serial.size() != ZT_SHA384_DIGEST_SIZE)
 			return false;
-		}
 		this->addSubjectCertificate(serial.data());
 	}
 
@@ -425,15 +421,6 @@ bool Certificate::decode(const void *const data, const unsigned int len)
 	return true;
 }
 
-Vector< uint8_t > Certificate::encodeCSR()
-{
-	Vector< uint8_t > enc;
-	Dictionary d;
-	m_encodeSubject(this->subject, d, false);
-	d.encode(enc);
-	return enc;
-}
-
 bool Certificate::sign(const Identity &issuer)
 {
 	m_identities.push_front(issuer);
@@ -476,12 +463,13 @@ ZT_CertificateError Certificate::verify() const
 				(this->subject.uniqueIdSize != ZT_CERTIFICATE_UNIQUE_ID_TYPE_NIST_P_384_SIZE) ||
 				(this->subject.uniqueId[0] != ZT_CERTIFICATE_UNIQUE_ID_TYPE_NIST_P_384))
 				return ZT_CERTIFICATE_ERROR_INVALID_UNIQUE_ID_PROOF;
-			Dictionary tmp;
+			Dictionary d;
-			m_encodeSubject(this->subject, tmp, true);
+			m_encodeSubject(this->subject, d, true);
 			Vector< uint8_t > enc;
-			tmp.encode(enc);
+			d.encode(enc);
 			uint8_t h[ZT_SHA384_DIGEST_SIZE];
 			SHA384(h, enc.data(), (unsigned int)enc.size());
+			static_assert(ZT_CERTIFICATE_UNIQUE_ID_TYPE_NIST_P_384_SIZE == (ZT_ECC384_PUBLIC_KEY_SIZE + 1), "incorrect size");
 			if (!ECC384ECDSAVerify(this->subject.uniqueId + 1, h, this->subject.uniqueIdProofSignature))
 				return ZT_CERTIFICATE_ERROR_INVALID_UNIQUE_ID_PROOF;
 		} else if (this->subject.uniqueIdSize > 0) {
@@ -519,6 +507,45 @@ ZT_CertificateError Certificate::verify() const
 	return ZT_CERTIFICATE_ERROR_NONE;
 }
 
+Vector< uint8_t > Certificate::createCSR(const ZT_Certificate_Subject &s, const void *uniqueId, unsigned int uniqueIdSize, const void *uniqueIdPrivate, unsigned int uniqueIdPrivateSize)
+{
+	ZT_Certificate_Subject sc;
+	Utils::copy< sizeof(ZT_Certificate_Subject) >(&sc, &s);
+
+	if ((uniqueId) && (uniqueIdSize > 0) && (uniqueIdPrivate) && (uniqueIdPrivateSize > 0)) {
+		sc.uniqueId = reinterpret_cast<const uint8_t *>(uniqueId);
+		sc.uniqueIdSize = uniqueIdSize;
+	} else {
+		sc.uniqueId = nullptr;
+		sc.uniqueIdSize = 0;
+	}
+
+	Dictionary d;
+	m_encodeSubject(sc, d, true);
+	Vector< uint8_t > enc;
+	d.encode(enc);
+
+	if (sc.uniqueId) {
+		uint8_t h[ZT_SHA384_DIGEST_SIZE];
+		SHA384(h, enc.data(), (unsigned int)enc.size());
+		enc.clear();
+		if (
+			(reinterpret_cast<const uint8_t *>(uniqueId)[0] == ZT_CERTIFICATE_UNIQUE_ID_TYPE_NIST_P_384) &&
+			(uniqueIdSize == ZT_CERTIFICATE_UNIQUE_ID_TYPE_NIST_P_384_SIZE) &&
+			(uniqueIdPrivateSize == ZT_CERTIFICATE_UNIQUE_ID_TYPE_NIST_P_384_PRIVATE_SIZE)) {
+			uint8_t sig[ZT_ECC384_SIGNATURE_SIZE];
+			ECC384ECDSASign(reinterpret_cast<const uint8_t *>(uniqueIdPrivate), h, sig);
+			sc.uniqueIdProofSignature = sig;
+			sc.uniqueIdProofSignatureSize = ZT_ECC384_SIGNATURE_SIZE;
+			d.clear();
+			m_encodeSubject(sc, d, false);
+			d.encode(enc);
+		}
+	}
+
+	return enc;
+}
+
 void Certificate::m_clear()
 {
 	ZT_Certificate *const sup = this;
@@ -620,6 +647,7 @@ int ZT_Certificate_newSubjectUniqueId(
 	void *uniqueIdPrivate,
 	int *uniqueIdPrivateSize)
 {
+	try {
 		switch (type) {
 			case ZT_CERTIFICATE_UNIQUE_ID_TYPE_NIST_P_384:
 				if ((*uniqueIdSize < ZT_CERTIFICATE_UNIQUE_ID_TYPE_NIST_P_384_SIZE) || (*uniqueIdPrivateSize < ZT_CERTIFICATE_UNIQUE_ID_TYPE_NIST_P_384_PRIVATE_SIZE))
@@ -630,6 +658,9 @@ int ZT_Certificate_newSubjectUniqueId(
 				return ZT_RESULT_OK;
 		}
 		return ZT_RESULT_ERROR_BAD_PARAMETER;
+	} catch (...) {
+		return ZT_RESULT_FATAL_ERROR_INTERNAL;
+	}
 }
 
 int ZT_Certificate_newCSR(
@@ -641,20 +672,18 @@ int ZT_Certificate_newCSR(
 	void *csr,
 	int *csrSize)
 {
-	ZeroTier::Certificate c;
+	try {
-	ZeroTier::Utils::copy< sizeof(ZT_Certificate_Subject) >(&(c.subject), subject);
+		if (!subject)
-	if ((uniqueId) && (uniqueIdSize > 0) && (uniqueIdPrivate) && (uniqueIdPrivateSize > 0)) {
-		if ((reinterpret_cast<const uint8_t *>(uniqueId)[0] != ZT_CERTIFICATE_UNIQUE_ID_TYPE_NIST_P_384) || (uniqueIdSize != ZT_CERTIFICATE_UNIQUE_ID_TYPE_NIST_P_384_SIZE) || (uniqueIdPrivateSize != ZT_CERTIFICATE_UNIQUE_ID_TYPE_NIST_P_384_PRIVATE_SIZE))
 			return ZT_RESULT_ERROR_BAD_PARAMETER;
-		if (!c.setSubjectUniqueId(reinterpret_cast<const uint8_t *>(uniqueId), reinterpret_cast<const uint8_t *>(uniqueIdPrivate)))
+		const ZeroTier::Vector< uint8_t > csrV(ZeroTier::Certificate::createCSR(*subject, uniqueId, uniqueIdSize, uniqueIdPrivate, uniqueIdPrivateSize));
-			return ZT_RESULT_ERROR_INVALID_CREDENTIAL;
-	}
-	ZeroTier::Vector< uint8_t > csrV(c.encodeCSR());
 		if ((int)csrV.size() > *csrSize)
 			return ZT_RESULT_ERROR_BAD_PARAMETER;
 		ZeroTier::Utils::copy(csr, csrV.data(), (unsigned int)csrV.size());
 		*csrSize = (int)csrV.size();
 		return ZT_RESULT_OK;
+	} catch (...) {
+		return ZT_RESULT_FATAL_ERROR_INTERNAL;
+	}
 }
 
 int ZT_Certificate_sign(
@@ -663,15 +692,14 @@ int ZT_Certificate_sign(
 	void *signedCert,
 	int *signedCertSize)
 {
+	try {
 		if (!cert)
 			return ZT_RESULT_ERROR_BAD_PARAMETER;
-
+		ZeroTier::Certificate c(*cert);
-	try {
+		if (!c.sign(*reinterpret_cast<const ZeroTier::Identity *>(signer)))
-		const ZeroTier::ScopedPtr< ZeroTier::Certificate > c(new ZeroTier::Certificate(*cert));
-		if (!c->sign(*reinterpret_cast<const ZeroTier::Identity *>(signer)))
 			return ZT_RESULT_ERROR_INTERNAL;
 
-		const ZeroTier::Vector< uint8_t > enc(c->encode());
+		const ZeroTier::Vector< uint8_t > enc(c.encode());
 		if ((int)enc.size() > *signedCertSize)
 			return ZT_RESULT_ERROR_BAD_PARAMETER;
 		ZeroTier::Utils::copy(signedCert, enc.data(), (unsigned int)enc.size());
@@ -717,6 +745,7 @@ int ZT_Certificate_encode(
 	void *encoded,
 	int *encodedSize)
 {
+	try {
 		if ((!cert) || (!encoded) || (!encodedSize))
 			return ZT_RESULT_ERROR_BAD_PARAMETER;
 		ZeroTier::Certificate c(*cert);
@@ -726,6 +755,9 @@ int ZT_Certificate_encode(
 		ZeroTier::Utils::copy(encoded, enc.data(), (unsigned int)enc.size());
 		*encodedSize = (int)enc.size();
 		return ZT_RESULT_OK;
+	} catch (...) {
+		return ZT_RESULT_FATAL_ERROR_INTERNAL;
+	}
 }
 
 enum ZT_CertificateError ZT_Certificate_verify(const ZT_Certificate *cert)
@@ -752,8 +784,10 @@ const ZT_Certificate *ZT_Certificate_clone(const ZT_Certificate *cert)
 
 void ZT_Certificate_delete(const ZT_Certificate *cert)
 {
+	try {
 		if (cert)
 			delete (const ZeroTier::Certificate *)(cert);
+	} catch (...) {}
 }
 
 }

core/Certificate.hpp

@@ -145,13 +145,6 @@ public:
 	 */
 	bool decode(const void *data, unsigned int len);
 
-	/**
-	 * Encode only the subject portion of this certificate as a CSR
-	 *
-	 * @return Encoded CSR
-	 */
-	Vector< uint8_t > encodeCSR();
-
 	/**
 	 * Sign this certificate (and also fill in serialNo).
 	 *
@@ -170,6 +163,18 @@ public:
 	 */
 	ZT_CertificateError verify() const;
 
+	/**
+	 * Create a CSR that encodes the subject of this certificate
+	 *
+	 * @param s Subject to encode
+	 * @param uniqueId Unique ID to sign subject with or NULL if none
+	 * @param uniqueIdSize Size of unique ID or 0 if none
+	 * @param uniqueIdPrivate Unique ID private key for proof signature or NULL if none
+	 * @param uniqueIdPrivateSize Size of unique ID private key
+	 * @return Encoded subject (without any unique ID fields) or empty vector on error
+	 */
+	static Vector< uint8_t > createCSR(const ZT_Certificate_Subject &s, const void *uniqueId, unsigned int uniqueIdSize, const void *uniqueIdPrivate, unsigned int uniqueIdPrivateSize);
+
 	/**
 	 * Create a subject unique ID and corresponding private key required for use
 	 *

core/Tests.cpp

@@ -277,7 +277,8 @@ static bool ZTT_deepCompareCertificates(const Certificate &a, const Certificate
 		(a.subject.uniqueIdProofSignatureSize != b.subject.uniqueIdProofSignatureSize) ||
 		(a.maxPathLength != b.maxPathLength) ||
 		(a.signatureSize != b.signatureSize)
-		) return false;
+		)
+		return false;
 
 	if ((a.subject.uniqueId == nullptr) != (b.subject.uniqueId == nullptr))
 		return false;
@@ -379,6 +380,32 @@ extern "C" const char *ZTT_general()
 			ZT_T_PRINTF("OK" ZT_EOL_S);
 		}
 
+		{
+			ZT_T_PRINTF("[general] Sanity checking memory zero and copy functions... ");
+			for (unsigned long k = 0; k < 1000; ++k) {
+				uint8_t *tmp = new uint8_t[131072];
+				uint8_t *tmp2 = new uint8_t[131072];
+				for (unsigned long i = 0; i < 131072; ++i) {
+					tmp[i] = (uint8_t)i;
+					tmp2[i] = 0;
+				}
+				unsigned long l = ((unsigned long)Utils::random() % 131072) + 1;
+				Utils::copy(tmp2, tmp, l);
+				if (memcmp(tmp2, tmp, l) != 0) {
+					ZT_T_PRINTF("FAILED (copy)" ZT_EOL_S);
+					return "memory copy";
+				}
+				Utils::zero(tmp2, l);
+				for (unsigned long i = 0; i < l; ++i) {
+					if (tmp2[i] != 0) {
+						ZT_T_PRINTF("FAILED (zero)" ZT_EOL_S);
+						return "memory zero";
+					}
+				}
+			}
+			ZT_T_PRINTF("OK" ZT_EOL_S);
+		}
+
 #ifdef ZT_ARCH_X64
 		ZT_T_PRINTF("[general] X64 CPUID: aes=%d avx=%d avx2=%d avx512f=%d fsrm=%d rdrand=%d sha=%d vaes=%d vpclmulqdq=%d" ZT_EOL_S,
 		            Utils::CPUID.aes,

pkg/zerotier/certificate.go

@@ -491,24 +491,28 @@ func NewCertificateSubjectUniqueId(uniqueIdType int) (id []byte, priv []byte, er
 
 // NewCertificateCSR creates a new certificate signing request (CSR) from a certificate subject and optional unique ID.
 func NewCertificateCSR(subject *CertificateSubject, uniqueId []byte, uniqueIdPrivate []byte) ([]byte, error) {
-	var tmp Certificate
-	tmp.Subject = *subject
-	ctmp := tmp.CCertificate()
-	if ctmp == nil {
-		return nil, ErrInternal
-	}
-	ccert := (*C.ZT_Certificate)(ctmp.C)
 	var uid unsafe.Pointer
 	var uidp unsafe.Pointer
 	if len(uniqueId) > 0 && len(uniqueIdPrivate) > 0 {
 		uid = unsafe.Pointer(&uniqueId[0])
 		uidp = unsafe.Pointer(&uniqueIdPrivate[0])
 	}
+
+	var tmp Certificate
+	tmp.Subject = *subject
+	ctmp := tmp.CCertificate()
+	if ctmp == nil {
+		return nil, ErrInternal
+	}
+
 	var csr [16384]byte
 	csrSize := C.int(16384)
-	rv := int(C.ZT_Certificate_newCSR(&(ccert.subject), uid, C.int(len(uniqueId)), uidp, C.int(len(uniqueIdPrivate)), unsafe.Pointer(&csr[0]), &csrSize))
+	cc := (*C.ZT_Certificate)(ctmp.C)
+	rv := int(C.ZT_Certificate_newCSR(&(cc.subject), uid, C.int(len(uniqueId)), uidp, C.int(len(uniqueIdPrivate)), unsafe.Pointer(&csr[0]), &csrSize))
 	if rv != 0 {
 		return nil, fmt.Errorf("newCSR error %d", rv)
 	}
+	ctmp = nil
+
 	return append(make([]byte, 0, int(csrSize)), csr[0:int(csrSize)]...), nil
 }