Docs and a small build fix in debug mode.

node/IncomingPacket.cpp

@@ -212,15 +212,13 @@ bool IncomingPacket::_doHELLO(const RuntimeEnvironment *RR,const bool alreadyAut
 		const unsigned int vMinor = (*this)[ZT_PROTO_VERB_HELLO_IDX_MINOR_VERSION];
 		const unsigned int vRevision = at<uint16_t>(ZT_PROTO_VERB_HELLO_IDX_REVISION);
 		const uint64_t timestamp = at<uint64_t>(ZT_PROTO_VERB_HELLO_IDX_TIMESTAMP);
+		Identity id;
+		unsigned int ptr = ZT_PROTO_VERB_HELLO_IDX_IDENTITY + id.deserialize(*this,ZT_PROTO_VERB_HELLO_IDX_IDENTITY);
 
 		if (protoVersion < ZT_PROTO_VERSION_MIN) {
 			TRACE("dropped HELLO from %s(%s): protocol version too old",id.address().toString().c_str(),_path->address().toString().c_str());
 			return true;
 		}
-
-		Identity id;
-		unsigned int ptr = ZT_PROTO_VERB_HELLO_IDX_IDENTITY + id.deserialize(*this,ZT_PROTO_VERB_HELLO_IDX_IDENTITY);
-
 		if (fromAddress != id.address()) {
 			TRACE("dropped HELLO from %s(%s): identity does not match packet source address",fromAddress.toString().c_str(),_path->address().toString().c_str());
 			return true;
@@ -301,8 +299,11 @@ bool IncomingPacket::_doHELLO(const RuntimeEnvironment *RR,const bool alreadyAut
 
 		// Get external surface address if present (was not in old versions)
 		InetAddress externalSurfaceAddress;
-		if (ptr < size())
+		if (ptr < size()) {
 			ptr += externalSurfaceAddress.deserialize(*this,ptr);
+			if ((externalSurfaceAddress)&&(hops() == 0))
+				RR->sa->iam(id.address(),_path->localAddress(),_path->address(),externalSurfaceAddress,RR->topology->isUpstream(id),now);
+		}
 
 		// Get primary planet world ID and world timestamp if present
 		uint64_t planetWorldId = 0;
@@ -329,17 +330,16 @@ bool IncomingPacket::_doHELLO(const RuntimeEnvironment *RR,const bool alreadyAut
 
 			// Handle COR if present (older versions don't send this)
 			if ((ptr + 2) <= size()) {
-				//const unsigned int corSize = at<uint16_t>(ptr); ptr += 2;
-				ptr += 2;
-				CertificateOfRepresentation cor;
-				ptr += cor.deserialize(*this,ptr);
+				if (at<uint16_t>(ptr) > 0) {
+					CertificateOfRepresentation cor;
+					ptr += 2;
+					ptr += cor.deserialize(*this,ptr);
+				} else ptr += 2;
 			}
 		}
 
-		// Learn our external surface address from other peers to help us negotiate symmetric NATs
-		// and detect changes to our global IP that can trigger path renegotiation.
-		if ((externalSurfaceAddress)&&(hops() == 0))
-			RR->sa->iam(id.address(),_path->localAddress(),_path->address(),externalSurfaceAddress,RR->topology->isUpstream(id),now);
+		// Send OK(HELLO) with an echo of the packet's timestamp and some of the same
+		// information about us: version, sent-to address, etc.
 
 		Packet outp(id.address(),RR->identity.address(),Packet::VERB_OK);
 		outp.append((unsigned char)Packet::VERB_HELLO);
@@ -466,10 +466,11 @@ bool IncomingPacket::_doOK(const RuntimeEnvironment *RR,const SharedPtr<Peer> &p
 
 				// Handle COR if present (older versions don't send this)
 				if ((ptr + 2) <= size()) {
-					//const unsigned int corSize = at<uint16_t>(ptr); ptr += 2;
-					ptr += 2;
-					CertificateOfRepresentation cor;
-					ptr += cor.deserialize(*this,ptr);
+					if (at<uint16_t>(ptr) > 0) {
+						CertificateOfRepresentation cor;
+						ptr += 2;
+						ptr += cor.deserialize(*this,ptr);
+					} else ptr += 2;
 				}
 
 				TRACE("%s(%s): OK(HELLO), version %u.%u.%u, latency %u, reported external address %s",source().toString().c_str(),_path->address().toString().c_str(),vMajor,vMinor,vRevision,latency,((externalSurfaceAddress) ? externalSurfaceAddress.toString().c_str() : "(none)"));

node/Packet.hpp

@@ -538,8 +538,7 @@ public:
 		 *   <[2] software revision>
 		 *   <[8] timestamp for determining latency>
 		 *   <[...] binary serialized identity (see Identity)>
-		 *   <[1] destination address type>
-		 *   [<[...] destination address to which packet was sent>]
+		 *   <[...] physical destination address of packet>
 		 *   <[8] 64-bit world ID of current planet>
 		 *   <[8] 64-bit timestamp of current planet>
 		 *   [... remainder if packet is encrypted using cryptField() ...]
@@ -547,40 +546,39 @@ public:
 		 *   [<[1] 8-bit type ID of moon>]
 		 *   [<[8] 64-bit world ID of moon>]
 		 *   [<[8] 64-bit timestamp of moon>]
-		 *   [... additional moons ...]
+		 *   [... additional moon type/ID/timestamp tuples ...]
 		 *   <[2] 16-bit length of certificate of representation>
 		 *   [... certificate of representation ...]
 		 *
-		 * The initial fields of HELLO are sent in the clear. Fields after the
-		 * planet definition (which are common knowledge) are however encrypted
-		 * using the cryptField() function. The packet is MAC'd as usual using
-		 * the same MAC construct as other packets.
-		 *
-		 * The destination address is the wire address to which this packet is
-		 * being sent, and in OK is *also* the destination address of the OK
-		 * packet. This can be used by the receiver to detect NAT, learn its real
-		 * external address if behind NAT, and detect changes to its external
-		 * address that require re-establishing connectivity.
-		 *
-		 * Destination address types and formats (not all of these are used now):
-		 *   0x00 - None -- no destination address data present
-		 *   0x01 - Ethernet address -- format: <[6] Ethernet MAC>
-		 *   0x04 - 6-byte IPv4 UDP address/port -- format: <[4] IP>, <[2] port>
-		 *   0x06 - 18-byte IPv6 UDP address/port -- format: <[16] IP>, <[2] port>
-		 *
-		 * OK payload (note that OK is encrypted):
-		 *   <[8] timestamp (echoed from original HELLO)>
-		 *   <[1] protocol version (of responder)>
-		 *   <[1] software major version (of responder)>
-		 *   <[1] software minor version (of responder)>
-		 *   <[2] software revision (of responder)>
-		 *   <[1] destination address type (for this OK, not copied from HELLO)>
-		 *   [<[...] destination address>]
-		 *   <[2] 16-bit length of world update or 0 if none>
+		 * HELLO is sent in the clear as it is how peers share their identity
+		 * public keys. A few additional fields are sent in the clear too, but
+		 * these are things that are public info or are easy to determine. As
+		 * of 1.2.0 we have added a few more fields, but since these could have
+		 * the potential to be sensitive we introduced the encryption of the
+		 * remainder of the packet. See cryptField(). Packet MAC is still
+		 * performed of course, so authentication occurs as normal.
+		 *
+		 * Destination address is the actual wire address to which the packet
+		 * was sent. See InetAddress::serialize() for format.
+		 *
+		 * OK payload:
+		 *   <[8] HELLO timestamp field echo>
+		 *   <[1] protocol version>
+		 *   <[1] software major version>
+		 *   <[1] software minor version>
+		 *   <[2] software revision>
+		 *   <[...] physical destination address of packet>
+		 *   <[2] 16-bit length of world update(s) or 0 if none>
 		 *   [[...] updates to planets and/or moons]
-		 *   <[2] 16-bit length of certificate of representation (of responder)>
+		 *   <[2] 16-bit length of certificate of representation>
 		 *   [... certificate of representation ...]
 		 *
+		 * With the exception of the timestamp, the other fields pertain to the
+		 * respondent who is sending OK and are not echoes.
+		 *
+		 * Note that OK is fully encrypted so no selective cryptField() of
+		 * potentially sensitive fields is needed.
+		 *
 		 * ERROR has no payload.
 		 */
 		VERB_HELLO = 0x01,