Fix an oversight in signed dictionaries: the timestamp and signing identity should themselves be part of the signature. Also include the raw dictionary in addition to the bin2c version in root-topology/

2 files changed, 24 insertions, 13 deletions

diff --git a/node/Dictionary.cpp b/node/Dictionary.cpp
index 0572783b..835cc0c9 100644
--- a/node/Dictionary.cpp
+++ b/node/Dictionary.cpp
@@ -32,13 +32,14 @@
 
 namespace ZeroTier {
 
-void Dictionary::fromString(const char *s)
+void Dictionary::fromString(const char *s,unsigned int maxlen)
 {
 	clear();
 	bool escapeState = false;
 	std::string keyBuf;
 	std::string *element = &keyBuf;
-	while (*s) {
+	const char *end = s + maxlen;
+	while ((*s)&&(s < end)) {
 		if (escapeState) {
 			escapeState = false;
 			switch(*s) {
@@ -77,16 +78,25 @@ void Dictionary::fromString(const char *s)
 bool Dictionary::sign(const Identity &id)
 {
 	try {
-		std::string buf;
-		_mkSigBuf(buf);
+		// Sign identity and timestamp fields too. If there's an existing
+		// signature, _mkSigBuf() ignores it.
 		char nows[32];
 		Utils::snprintf(nows,sizeof(nows),"%llx",(unsigned long long)Utils::now());
-		C25519::Signature sig(id.sign(buf.data(),buf.length()));
-		(*this)[ZT_DICTIONARY_SIGNATURE] = Utils::hex(sig.data,sig.size());
 		(*this)[ZT_DICTIONARY_SIGNATURE_IDENTITY] = id.toString(false);
 		(*this)[ZT_DICTIONARY_SIGNATURE_TIMESTAMP] = nows;
+
+		// Create a blob to hash and sign from fields in sorted order
+		std::string buf;
+		_mkSigBuf(buf);
+
+		// Add signature field
+		C25519::Signature sig(id.sign(buf.data(),(unsigned int)buf.length()));
+		(*this)[ZT_DICTIONARY_SIGNATURE] = Utils::hex(sig.data,(unsigned int)sig.size());
+
 		return true;
 	} catch ( ... ) {
+		// Probably means identity has no secret key field
+		removeSignature();
 		return false;
 	}
 }
@@ -110,7 +120,7 @@ void Dictionary::_mkSigBuf(std::string &buf) const
 {
 	unsigned long pairs = 0;
 	for(const_iterator i(begin());i!=end();++i) {
-		if ((i->first.length() < 2)||( (i->first[0] != '~')&&(i->first[1] != '!') )) {
+		if (i->first != ZT_DICTIONARY_SIGNATURE) {
 			buf.append(i->first);
 			buf.push_back('=');
 			buf.append(i->second);
diff --git a/node/Dictionary.hpp b/node/Dictionary.hpp
index 31930eff..22eb8a7e 100644
--- a/node/Dictionary.hpp
+++ b/node/Dictionary.hpp
@@ -51,8 +51,7 @@ class Identity;
  * human-readable if the keys and values in the dictionary are also
  * human-readable. Otherwise it might contain unprintable characters.
  *
- * Keys beginning with "~!" are reserved for signatures and are ignored
- * during the signature process.
+ * Keys beginning with "~!" are reserved for signature data fields.
  *
  * Note: the signature code depends on std::map<> being sorted, but no
  * other code does. So if the underlying data structure is ever swapped
@@ -66,13 +65,14 @@ public:
 
 	/**
 	 * @param s String-serialized dictionary
+	 * @param maxlen Maximum length of buffer
 	 */
-	Dictionary(const char *s) { fromString(s); }
+	Dictionary(const char *s,unsigned int maxlen) { fromString(s,maxlen); }
 
 	/**
 	 * @param s String-serialized dictionary
 	 */
-	Dictionary(const std::string &s) { fromString(s.c_str()); }
+	Dictionary(const std::string &s) { fromString(s.c_str(),(unsigned int)s.length()); }
 
 	/**
 	 * Get a key, throwing an exception if it is not present
@@ -130,9 +130,10 @@ public:
 	 * Clear and initialize from a string
 	 *
 	 * @param s String-serialized dictionary
+	 * @param maxlen Maximum length of string buffer
 	 */
-	void fromString(const char *s);
-	inline void fromString(const std::string &s) { fromString(s.c_str()); }
+	void fromString(const char *s,unsigned int maxlen);
+	inline void fromString(const std::string &s) { fromString(s.c_str(),(unsigned int)s.length()); }
 
 	/**
 	 * @return True if this dictionary is cryptographically signed