RootSet tests and fix, other cleanup.

network-hypervisor/src/util/mod.rs

@@ -64,7 +64,7 @@ pub(crate) fn store_u64_be(i: u64, d: &mut [u8]) {
 #[cfg(any(target_arch = "x86_64", target_arch = "x86", target_arch = "aarch64"))]
 #[inline(always)]
 pub(crate) fn load_u16_be(d: &[u8]) -> u16 {
-    unsafe { *d.as_ptr().cast::<u16>() }
+    u16::from_be(unsafe { *d.as_ptr().cast::<u16>() })
 }
 
 #[cfg(not(any(target_arch = "x86_64", target_arch = "x86", target_arch = "aarch64")))]
@@ -76,7 +76,7 @@ pub(crate) fn load_u16_be(d: &[u8]) -> u16 {
 #[cfg(any(target_arch = "x86_64", target_arch = "x86", target_arch = "aarch64"))]
 #[inline(always)]
 pub(crate) fn load_u32_be(d: &[u8]) -> u32 {
-    unsafe { *d.as_ptr().cast::<u32>() }
+    u32::from_be(unsafe { *d.as_ptr().cast::<u32>() })
 }
 
 #[cfg(not(any(target_arch = "x86_64", target_arch = "x86", target_arch = "aarch64")))]
@@ -88,7 +88,7 @@ pub(crate) fn load_u32_be(d: &[u8]) -> u32 {
 #[cfg(any(target_arch = "x86_64", target_arch = "x86", target_arch = "aarch64"))]
 #[inline(always)]
 pub(crate) fn load_u64_be(d: &[u8]) -> u64 {
-    unsafe { *d.as_ptr().cast::<u64>() }
+    u64::from_be(unsafe { *d.as_ptr().cast::<u64>() })
 }
 
 #[cfg(not(any(target_arch = "x86_64", target_arch = "x86", target_arch = "aarch64")))]

network-hypervisor/src/vl1/buffer.rs

@@ -72,11 +72,21 @@ impl<const L: usize> Buffer<L> {
 
     /// Set the size of this buffer's data.
     ///
-    /// This is marked unsafe because no bounds checking is done here and because it
+    /// If the new size is larger than L (the capacity) it will be limited to L. Any new
-    /// technically violates the assurance that all data in the buffer is valid. Use
+    /// space will be filled with zeroes.
-    /// with care.
     #[inline(always)]
-    pub unsafe fn set_size(&mut self, s: usize) { self.0 = s; }
+    pub fn set_size(&mut self, s: usize) {
+        let prev_len = self.0;
+        if s < prev_len {
+            self.0 = s;
+            self.1[s..prev_len].fill(0);
+        } else {
+            self.0 = s.min(L);
+        }
+    }
+
+    #[inline(always)]
+    pub unsafe fn set_size_unchecked(&mut self, s: usize) { self.0 = s; }
 
     /// Append a packed structure and call a function to initialize it in place.
     /// Anything not initialized will be zero.

network-hypervisor/src/vl1/endpoint.rs

@@ -130,11 +130,14 @@ impl Endpoint {
     pub fn unmarshal<const BL: usize>(buf: &Buffer<BL>, cursor: &mut usize) -> std::io::Result<Endpoint> {
         let type_byte = buf.read_u8(cursor)?;
         if type_byte < 16 {
-            let ip = InetAddress::unmarshal(buf, cursor)?;
+            if type_byte == 4 {
-            if ip.is_nil() {
+                let b: &[u8; 6] = buf.read_bytes_fixed(cursor)?;
-                Ok(Endpoint::Nil)
+                Ok(Endpoint::IpUdp(InetAddress::from_ip_port(&b[0..4], crate::util::load_u16_be(&b[4..6]))))
+            } else if type_byte == 6 {
+                let b: &[u8; 18] = buf.read_bytes_fixed(cursor)?;
+                Ok(Endpoint::IpUdp(InetAddress::from_ip_port(&b[0..16], crate::util::load_u16_be(&b[16..18]))))
             } else {
-                Ok(Endpoint::IpUdp(ip))
+                Ok(Endpoint::Nil)
             }
         } else {
             let read_mac = |buf: &Buffer<BL>, cursor: &mut usize| {

network-hypervisor/src/vl1/identity.rs

@@ -17,7 +17,7 @@ use crate::crypto::secret::Secret;
 use crate::error::InvalidFormatError;
 use crate::vl1::Address;
 use crate::vl1::buffer::Buffer;
-use crate::vl1::protocol::PACKET_SIZE_MAX;
+use crate::vl1::protocol::{PACKET_SIZE_MAX, ADDRESS_SIZE};
 
 pub const IDENTITY_TYPE_0_SIGNATURE_SIZE: usize = 96;
 pub const IDENTITY_TYPE_1_SIGNATURE_SIZE: usize = P521_ECDSA_SIGNATURE_SIZE + ED25519_SIGNATURE_SIZE;
@@ -359,9 +359,15 @@ impl Identity {
     /// Deserialize an Identity from a buffer.
     /// The supplied cursor is advanced.
     pub fn unmarshal<const BL: usize>(buf: &Buffer<BL>, cursor: &mut usize) -> std::io::Result<Identity> {
-        let addr = Address::from_bytes(buf.read_bytes_fixed::<5>(cursor)?).unwrap();
+        let addr = Address::from_bytes(buf.read_bytes_fixed::<{ ADDRESS_SIZE }>(cursor)?);
+        if addr.is_none() {
+            return std::io::Result::Err(std::io::Error::new(std::io::ErrorKind::InvalidData, "invalid address"));
+        }
+        let addr = addr.unwrap();
+
         let id_type = buf.read_u8(cursor)?;
         if id_type == Type::C25519 as u8 {
+
             let c25519_public_bytes = buf.read_bytes_fixed::<{ C25519_PUBLIC_KEY_SIZE }>(cursor)?;
             let ed25519_public_bytes = buf.read_bytes_fixed::<{ ED25519_PUBLIC_KEY_SIZE }>(cursor)?;
             let secrets_len = buf.read_u8(cursor)?;
@@ -388,9 +394,11 @@ impl Identity {
                     secrets: None,
                 })
             } else {
-                std::io::Result::Err(std::io::Error::new(std::io::ErrorKind::InvalidData, "unrecognized scret key length (type 0)"))
+                std::io::Result::Err(std::io::Error::new(std::io::ErrorKind::InvalidData, "unrecognized secret key length (type 0)"))
             }
+
         } else if id_type == Type::P521 as u8 {
+
             let c25519_public_bytes = buf.read_bytes_fixed::<{ C25519_PUBLIC_KEY_SIZE }>(cursor)?;
             let ed25519_public_bytes = buf.read_bytes_fixed::<{ ED25519_PUBLIC_KEY_SIZE }>(cursor)?;
             let p521_ecdh_public_bytes = buf.read_bytes_fixed::<{ P521_PUBLIC_KEY_SIZE }>(cursor)?;
@@ -425,6 +433,7 @@ impl Identity {
             } else {
                 std::io::Result::Err(std::io::Error::new(std::io::ErrorKind::InvalidData, "invalid secret key length (type 1)"))
             }
+
         } else {
             std::io::Result::Err(std::io::Error::new(std::io::ErrorKind::InvalidData, "unrecognized identity type"))
         }

network-hypervisor/src/vl1/inetaddress.rs

@@ -379,16 +379,15 @@ impl InetAddress {
     }
 
     pub fn unmarshal<const BL: usize>(buf: &Buffer<BL>, cursor: &mut usize) -> std::io::Result<InetAddress> {
-        match buf.read_u8(cursor)? {
+        let t = buf.read_u8(cursor)?;
-            4 => {
+        if t == 4 {
-                let b: &[u8; 6] = buf.read_bytes_fixed(cursor)?;
+            let b: &[u8; 6] = buf.read_bytes_fixed(cursor)?;
-                Ok(InetAddress::from_ip_port(&b[0..4], crate::util::load_u16_be(&b[4..6])))
+            Ok(InetAddress::from_ip_port(&b[0..4], crate::util::load_u16_be(&b[4..6])))
-            }
+        } else if t == 6 {
-            6 => {
+            let b: &[u8; 18] = buf.read_bytes_fixed(cursor)?;
-                let b: &[u8; 18] = buf.read_bytes_fixed(cursor)?;
+            Ok(InetAddress::from_ip_port(&b[0..16], crate::util::load_u16_be(&b[16..18])))
-                Ok(InetAddress::from_ip_port(&b[0..16], crate::util::load_u16_be(&b[16..18])))
+        } else {
-            }
+            Ok(InetAddress::new())
-            _ => Ok(InetAddress::new())
         }
     }
 }

network-hypervisor/src/vl1/node.rs

@@ -95,13 +95,13 @@ pub trait VL1PacketHandler {
     /// True should be returned even if the packet is not valid, since the return value is used
     /// to determine if this is a VL2 or VL1 packet. ERROR and OK should not be handled here but
     /// in handle_error() and handle_ok() instead.
-    fn handle_packet(&self, peer: &Peer, source_path: &Arc<Path>, forward_secrecy: bool, verb: u8, payload: &Buffer<{ PACKET_SIZE_MAX }>) -> bool;
+    fn handle_packet(&self, peer: &Peer, source_path: &Arc<Path>, forward_secrecy: bool, extended_authentication: bool, verb: u8, payload: &Buffer<{ PACKET_SIZE_MAX }>) -> bool;
 
     /// Handle errors, returning true if the error was recognized.
-    fn handle_error(&self, peer: &Peer, source_path: &Arc<Path>, forward_secrecy: bool, in_re_verb: u8, in_re_packet_id: PacketID, error_code: u8, payload: &Buffer<{ PACKET_SIZE_MAX }>, cursor: &mut usize) -> bool;
+    fn handle_error(&self, peer: &Peer, source_path: &Arc<Path>, forward_secrecy: bool, extended_authentication: bool, in_re_verb: u8, in_re_packet_id: PacketID, error_code: u8, payload: &Buffer<{ PACKET_SIZE_MAX }>, cursor: &mut usize) -> bool;
 
     /// Handle an OK, returing true if the OK was recognized.
-    fn handle_ok(&self, peer: &Peer, source_path: &Arc<Path>, forward_secrecy: bool, in_re_verb: u8, in_re_packet_id: PacketID, payload: &Buffer<{ PACKET_SIZE_MAX }>, cursor: &mut usize) -> bool;
+    fn handle_ok(&self, peer: &Peer, source_path: &Arc<Path>, forward_secrecy: bool, extended_authentication: bool, in_re_verb: u8, in_re_packet_id: PacketID, payload: &Buffer<{ PACKET_SIZE_MAX }>, cursor: &mut usize) -> bool;
 }
 
 #[derive(Default)]

network-hypervisor/src/vl1/peer.rs

@@ -194,7 +194,7 @@ impl Peer {
     /// those fragments after the main packet header and first chunk.
     pub(crate) fn receive<CI: VL1CallerInterface, PH: VL1PacketHandler>(&self, node: &Node, ci: &CI, ph: &PH, time_ticks: i64, source_path: &Arc<Path>, header: &PacketHeader, packet: &Buffer<{ PACKET_SIZE_MAX }>, fragments: &[Option<PacketBuffer>]) {
         let _ = packet.as_bytes_starting_at(PACKET_VERB_INDEX).map(|packet_frag0_payload_bytes| {
-            let mut payload: Buffer<{ PACKET_SIZE_MAX }> = Buffer::new();
+            let mut payload = node.get_packet_buffer();
 
             let mut forward_secrecy = true; // set to false below if ephemeral fails
             let mut packet_id = header.id as u64;
@@ -279,40 +279,50 @@ impl Peer {
             self.last_receive_time_ticks.store(time_ticks, Ordering::Relaxed);
             self.total_bytes_received.fetch_add((payload.len() + PACKET_HEADER_SIZE) as u64, Ordering::Relaxed);
 
-            let _ = payload.u8_at(0).map(|verb| {
+            let _ = payload.u8_at(0).map(|mut verb| {
-                let mut extended_authentication = false;
+                let extended_authentication = (verb & VERB_FLAG_EXTENDED_AUTHENTICATION) != 0;
-                if (verb & VERB_FLAG_EXTENDED_AUTHENTICATION) != 0 {
+                if extended_authentication {
                     let auth_bytes = payload.as_bytes();
                     if auth_bytes.len() >= (1 + SHA384_HASH_SIZE) {
                         let packet_hmac_start = auth_bytes.len() - SHA384_HASH_SIZE;
                         if !SHA384::hmac(self.static_secret_packet_hmac.as_ref(), &auth_bytes[1..packet_hmac_start]).eq(&auth_bytes[packet_hmac_start..]) {
                             return;
                         }
-                        extended_authentication = true;
+                        let new_len = payload.len() - SHA384_HASH_SIZE;
-                        unsafe { payload.set_size(payload.len() - SHA384_HASH_SIZE) };
+                        payload.set_size(new_len);
                     } else {
                         return;
                     }
                 }
 
                 if (verb & VERB_FLAG_COMPRESSED) != 0 {
+                    let mut decompressed_payload = node.get_packet_buffer();
+                    let _ = decompressed_payload.append_u8(verb);
+                    let dlen = lz4_flex::block::decompress_into(&payload.as_bytes()[1..], &mut decompressed_payload.as_bytes_mut(), 1);
+                    if dlen.is_ok() {
+                        decompressed_payload.set_size(dlen.unwrap());
+                        payload = decompressed_payload;
+                    } else {
+                        return;
+                    }
                 }
 
-                let verb = verb & VERB_MASK;
-
                 // For performance reasons we let VL2 handle packets first. It returns false
-                // if it didn't handle the packet, in which case it's handled at VL1.
+                // if it didn't handle the packet, in which case it's handled at VL1. This is
-                if !ph.handle_packet(self, source_path, forward_secrecy, verb, &payload) {
+                // because the most performance critical path is the handling of the ???_FRAME
+                // verbs, which are in VL2.
+                verb &= VERB_MASK;
+                if !ph.handle_packet(self, source_path, forward_secrecy, extended_authentication, verb, payload.as_ref()) {
                     match verb {
                         //VERB_VL1_NOP => {}
-                        VERB_VL1_HELLO => self.receive_hello(ci, node, time_ticks, source_path, &payload),
+                        VERB_VL1_HELLO => self.receive_hello(ci, node, time_ticks, source_path, payload.as_ref()),
-                        VERB_VL1_ERROR => self.receive_error(ci, ph, node, time_ticks, source_path, forward_secrecy, &payload),
+                        VERB_VL1_ERROR => self.receive_error(ci, ph, node, time_ticks, source_path, forward_secrecy, extended_authentication, payload.as_ref()),
-                        VERB_VL1_OK => self.receive_ok(ci, ph, node, time_ticks, source_path, forward_secrecy, &payload),
+                        VERB_VL1_OK => self.receive_ok(ci, ph, node, time_ticks, source_path, forward_secrecy, extended_authentication, payload.as_ref()),
-                        VERB_VL1_WHOIS => self.receive_whois(ci, node, time_ticks, source_path, &payload),
+                        VERB_VL1_WHOIS => self.receive_whois(ci, node, time_ticks, source_path, payload.as_ref()),
-                        VERB_VL1_RENDEZVOUS => self.receive_rendezvous(ci, node, time_ticks, source_path, &payload),
+                        VERB_VL1_RENDEZVOUS => self.receive_rendezvous(ci, node, time_ticks, source_path, payload.as_ref()),
-                        VERB_VL1_ECHO => self.receive_echo(ci, node, time_ticks, source_path, &payload),
+                        VERB_VL1_ECHO => self.receive_echo(ci, node, time_ticks, source_path, payload.as_ref()),
-                        VERB_VL1_PUSH_DIRECT_PATHS => self.receive_push_direct_paths(ci, node, time_ticks, source_path, &payload),
+                        VERB_VL1_PUSH_DIRECT_PATHS => self.receive_push_direct_paths(ci, node, time_ticks, source_path, payload.as_ref()),
-                        VERB_VL1_USER_MESSAGE => self.receive_user_message(ci, node, time_ticks, source_path, &payload),
+                        VERB_VL1_USER_MESSAGE => self.receive_user_message(ci, node, time_ticks, source_path, payload.as_ref()),
                         _ => {}
                     }
                 }
@@ -508,7 +518,7 @@ impl Peer {
     fn receive_hello<CI: VL1CallerInterface>(&self, ci: &CI, node: &Node, time_ticks: i64, source_path: &Arc<Path>, payload: &Buffer<{ PACKET_SIZE_MAX }>) {}
 
     #[inline(always)]
-    fn receive_error<CI: VL1CallerInterface, PH: VL1PacketHandler>(&self, ci: &CI, ph: &PH, node: &Node, time_ticks: i64, source_path: &Arc<Path>, forward_secrecy: bool, payload: &Buffer<{ PACKET_SIZE_MAX }>) {
+    fn receive_error<CI: VL1CallerInterface, PH: VL1PacketHandler>(&self, ci: &CI, ph: &PH, node: &Node, time_ticks: i64, source_path: &Arc<Path>, forward_secrecy: bool, extended_authentication: bool, payload: &Buffer<{ PACKET_SIZE_MAX }>) {
         let mut cursor: usize = 0;
         let _ = payload.read_struct::<message_component_structs::ErrorHeader>(&mut cursor).map(|error_header| {
             let in_re_packet_id = error_header.in_re_packet_id;
@@ -520,7 +530,7 @@ impl Peer {
             }) {
                 match error_header.in_re_verb {
                     _ => {
-                        ph.handle_error(self, source_path, forward_secrecy, error_header.in_re_verb, in_re_packet_id, error_header.error_code, payload, &mut cursor);
+                        ph.handle_error(self, source_path, forward_secrecy, extended_authentication, error_header.in_re_verb, in_re_packet_id, error_header.error_code, payload, &mut cursor);
                     }
                 }
             }
@@ -528,7 +538,7 @@ impl Peer {
     }
 
     #[inline(always)]
-    fn receive_ok<CI: VL1CallerInterface, PH: VL1PacketHandler>(&self, ci: &CI, ph: &PH, node: &Node, time_ticks: i64, source_path: &Arc<Path>, forward_secrecy: bool, payload: &Buffer<{ PACKET_SIZE_MAX }>) {
+    fn receive_ok<CI: VL1CallerInterface, PH: VL1PacketHandler>(&self, ci: &CI, ph: &PH, node: &Node, time_ticks: i64, source_path: &Arc<Path>, forward_secrecy: bool, extended_authentication: bool, payload: &Buffer<{ PACKET_SIZE_MAX }>) {
         let mut cursor: usize = 0;
         let _ = payload.read_struct::<message_component_structs::OkHeader>(&mut cursor).map(|ok_header| {
             let in_re_packet_id = ok_header.in_re_packet_id;
@@ -544,7 +554,7 @@ impl Peer {
                     VERB_VL1_WHOIS => {
                     }
                     _ => {
-                        ph.handle_ok(self, source_path, forward_secrecy, ok_header.in_re_verb, in_re_packet_id, payload, &mut cursor);
+                        ph.handle_ok(self, source_path, forward_secrecy, extended_authentication, ok_header.in_re_verb, in_re_packet_id, payload, &mut cursor);
                     }
                 }
             }

network-hypervisor/src/vl1/protocol.rs

@@ -1,7 +1,8 @@
 use std::mem::MaybeUninit;
 
 use crate::vl1::Address;
-use crate::vl1::buffer::RawObject;
+use crate::vl1::buffer::{RawObject, Buffer};
+use crate::crypto::hash::SHA384;
 
 pub const VERB_VL1_NOP: u8 = 0x00;
 pub const VERB_VL1_HELLO: u8 = 0x01;
@@ -158,6 +159,38 @@ pub const WHOIS_RETRY_MAX: u16 = 3;
 /// Maximum number of packets to queue up behind a WHOIS.
 pub const WHOIS_MAX_WAITING_PACKETS: usize = 64;
 
+/// Compress a packet and return true if compressed.
+/// The 'dest' buffer must be empty (will panic otherwise). A return value of false indicates an error or
+/// that the data was not compressible. The state of the destination buffer is undefined on a return
+/// value of false.
+pub fn compress_packet(src: &[u8], dest: &mut Buffer<{ PACKET_SIZE_MAX }>) -> bool {
+    if src.len() > PACKET_VERB_INDEX {
+        debug_assert!(dest.is_empty());
+        let cs = {
+            let d = dest.as_bytes_mut();
+            d[0..PACKET_VERB_INDEX].copy_from_slice(&src[0..PACKET_VERB_INDEX]);
+            d[PACKET_VERB_INDEX] = src[PACKET_VERB_INDEX] | VERB_FLAG_COMPRESSED;
+            lz4_flex::block::compress_into(&src[PACKET_VERB_INDEX + 1..], d, PACKET_VERB_INDEX + 1)
+        };
+        if cs.is_ok() {
+            let cs = cs.unwrap();
+            if cs > 0 && cs < (src.len() - PACKET_VERB_INDEX) {
+                unsafe { dest.set_size_unchecked(PACKET_VERB_INDEX + 1 + cs) };
+                return true;
+            }
+        }
+    }
+    return false;
+}
+
+/// Add HMAC-SHA384 to the end of a packet and set verb flag.
+#[inline(always)]
+pub fn add_extended_auth(pkt: &mut Buffer<{ PACKET_SIZE_MAX }>, hmac_secret_key: &[u8]) -> std::io::Result<()> {
+    pkt.append_bytes_fixed(&SHA384::hmac(hmac_secret_key, pkt.as_bytes_starting_at(PACKET_VERB_INDEX + 1)?))?;
+    pkt.as_bytes_mut()[PACKET_VERB_INDEX] |= VERB_FLAG_EXTENDED_AUTHENTICATION;
+    Ok(())
+}
+
 /// A unique packet identifier, also the cryptographic nonce.
 ///
 /// Packet IDs are stored as u64s for efficiency but they should be treated as

network-hypervisor/src/vl1/rootset.rs

@@ -1,4 +1,3 @@
-use std::collections::BTreeSet;
 use std::hash::{Hash, Hasher};
 use std::io::Write;
 
@@ -22,17 +21,16 @@ const ROOT_SET_TYPE_ED25519_P521: u8 = 128;
 /// Two of these are legacy from ZeroTier V1. The third is a root set signed by both
 /// an Ed25519 key and a NIST P-521 key with these keys being bundled together.
 #[derive(Clone, PartialEq, Eq)]
-pub enum Type {
+pub enum TypeAndID {
     LegacyPlanet(u64),
     LegacyMoon(u64),
     Ed25519P521RootSet([u8; 48]),
 }
 
-impl Hash for Type {
+impl Hash for TypeAndID {
     fn hash<H: Hasher>(&self, state: &mut H) {
         match self {
-            Self::LegacyPlanet(id) => state.write_u64(*id),
+            Self::LegacyPlanet(id) | Self::LegacyMoon(id) => state.write_u64(*id),
-            Self::LegacyMoon(id) => state.write_u64(*id),
             Self::Ed25519P521RootSet(id) => state.write(id),
         }
     }
@@ -92,7 +90,7 @@ pub struct Root {
     pub identity: Identity,
 
     /// Static endpoints at which this root node may be reached.
-    pub endpoints: BTreeSet<Endpoint>,
+    pub endpoints: Vec<Endpoint>,
 }
 
 /// A signed bundle of root nodes.
@@ -103,26 +101,30 @@ pub struct Root {
 /// as at least one of the old roots is up to distribute the new ones.
 #[derive(PartialEq, Eq)]
 pub struct RootSet {
-    timestamp: i64,
+    pub id: TypeAndID,
-    name: String,
+    pub timestamp: i64,
-    contact: String,
+    pub url: String,
-    roots: BTreeSet<Root>,
+    pub roots: Vec<Root>,
-    signer: Vec<u8>,
+    pub signer: Vec<u8>,
-    signature: Vec<u8>,
+    pub signature: Vec<u8>,
-    root_set_type: Type,
 }
 
 impl RootSet {
     pub const MAX_ROOTS: usize = u8::MAX as usize;
     pub const MAX_ENDPOINTS_PER_ROOT: usize = u8::MAX as usize;
 
+    /// Shortcut to copy a byte array to a Buffer and unmarshal().
+    pub fn from_bytes(bytes: &[u8]) -> std::io::Result<RootSet> {
+        let mut tmp: Buffer<{ PACKET_SIZE_MAX }> = Buffer::new();
+        tmp.append_bytes(bytes)?;
+        let mut c: usize = 0;
+        RootSet::unmarshal(&tmp, &mut c)
+    }
+
     /// Sign this root set and return true on success.
-    /// The fields timestamp, name, contact, and roots must have been set. The signer, signature, and type will be set.
-    /// This can only sign new format root sets. Legacy "planet" and "moon" root sets can be used by V2 but
-    /// cannot be created by this code.
     pub fn sign(&mut self, keys: &RootSetSecretKeys) -> bool {
         self.signer = keys.to_public_bytes().to_vec();
-        self.root_set_type = Type::Ed25519P521RootSet(SHA384::hash(self.signer.as_slice()));
+        self.id = TypeAndID::Ed25519P521RootSet(SHA384::hash(self.signer.as_slice()));
 
         let mut buf: Buffer<{ PACKET_SIZE_MAX }> = Buffer::new();
         if self.marshal_internal(&mut buf, true).is_err() {
@@ -147,22 +149,13 @@ impl RootSet {
             return Err(std::io::Error::new(std::io::ErrorKind::InvalidData, "maximum roots per root set: 255"));
         }
 
-        let name = self.name.as_bytes();
-        let contact = self.contact.as_bytes();
-        if name.len() > u8::MAX as usize {
-            return Err(std::io::Error::new(std::io::ErrorKind::InvalidData, "maximum roots per root set: 255"));
-        }
-        if contact.len() > u8::MAX as usize {
-            return Err(std::io::Error::new(std::io::ErrorKind::InvalidData, "maximum roots per root set: 255"));
-        }
-
         if for_signing {
             buf.append_u64(0x7f7f7f7f7f7f7f7f)?;
         }
 
-        match &self.root_set_type {
+        match &self.id {
-            Type::LegacyPlanet(id) | Type::LegacyMoon(id) => {
+            TypeAndID::LegacyPlanet(id) | TypeAndID::LegacyMoon(id) => {
-                buf.append_u8(if matches!(self.root_set_type, Type::LegacyPlanet(_)) {
+                buf.append_u8(if matches!(self.id, TypeAndID::LegacyPlanet(_)) {
                     ROOT_SET_TYPE_LEGACY_PLANET
                 } else {
                     ROOT_SET_TYPE_LEGACY_MOON
@@ -181,23 +174,20 @@ impl RootSet {
                 }
             }
 
-            Type::Ed25519P521RootSet(_) => {
+            TypeAndID::Ed25519P521RootSet(_) => {
                 buf.append_u8(ROOT_SET_TYPE_ED25519_P521)?;
-                buf.append_u64(self.timestamp as u64)?;
+                buf.append_varint(self.timestamp as u64)?;
-                buf.append_u8(name.len() as u8)?;
+                let url = self.url.as_bytes();
-                buf.append_bytes(name)?;
+                buf.append_varint(url.len() as u64)?;
-                buf.append_u8(contact.len() as u8)?;
+                buf.append_bytes(url)?;
-                buf.append_bytes(contact)?;
                 if self.signer.len() != (ED25519_PUBLIC_KEY_SIZE + P521_PUBLIC_KEY_SIZE) {
                     return Err(std::io::Error::new(std::io::ErrorKind::InvalidData, "signer can only be 164 bytes"));
                 }
-                buf.append_u8(self.signer.len() as u8)?;
                 buf.append_bytes(self.signer.as_slice())?;
                 if !for_signing {
                     if self.signature.len() != (ED25519_SIGNATURE_SIZE + P521_ECDSA_SIGNATURE_SIZE) {
                         return Err(std::io::Error::new(std::io::ErrorKind::InvalidData, "signature can only be 192 bytes"));
                     }
-                    buf.append_u8(self.signature.len() as u8)?;
                     buf.append_bytes(self.signature.as_slice())?;
                 }
             }
@@ -215,7 +205,7 @@ impl RootSet {
             }
         }
 
-        if matches!(self.root_set_type, Type::LegacyMoon(_)) {
+        if matches!(self.id, TypeAndID::LegacyMoon(_)) {
             buf.append_u8(0)?;
         }
 
@@ -232,20 +222,17 @@ impl RootSet {
     }
 
     pub fn unmarshal<const BL: usize>(buf: &Buffer<BL>, cursor: &mut usize) -> std::io::Result<Self> {
-        let read_roots = |buf: &Buffer<BL>, cursor: &mut usize| -> std::io::Result<BTreeSet<Root>> {
+        let read_roots = |buf: &Buffer<BL>, cursor: &mut usize| -> std::io::Result<Vec<Root>> {
-            let mut roots = BTreeSet::<Root>::new();
             let root_count = buf.read_u8(cursor)? as usize;
+            let mut roots = Vec::<Root>::with_capacity(root_count);
             for _ in 0..root_count {
                 let identity = Identity::unmarshal(buf, cursor)?;
-                let mut endpoints = BTreeSet::<Endpoint>::new();
                 let endpoint_count = buf.read_u8(cursor)? as usize;
+                let mut endpoints = Vec::<Endpoint>::with_capacity(endpoint_count);
                 for _ in 0..endpoint_count {
-                    endpoints.insert(Endpoint::unmarshal(buf, cursor)?);
+                    endpoints.push(Endpoint::unmarshal(buf, cursor)?);
                 }
-                roots.insert(Root {
+                roots.push(Root { identity, endpoints });
-                    identity,
-                    endpoints
-                });
             }
             Ok(roots)
         };
@@ -253,8 +240,8 @@ impl RootSet {
         let type_id = buf.read_u8(cursor)?;
         match type_id {
             ROOT_SET_TYPE_LEGACY_PLANET | ROOT_SET_TYPE_LEGACY_MOON => {
-                let root_set_type = buf.read_u64(cursor)?;
+                let id = buf.read_u64(cursor)?;
-                let root_set_type = if type_id == ROOT_SET_TYPE_LEGACY_PLANET { Type::LegacyPlanet(root_set_type) } else { Type::LegacyMoon(root_set_type) };
+                let id = if type_id == ROOT_SET_TYPE_LEGACY_PLANET { TypeAndID::LegacyPlanet(id) } else { TypeAndID::LegacyMoon(id) };
                 let timestamp = buf.read_u64(cursor)?;
                 let signer = buf.read_bytes(64, cursor)?.to_vec();
                 let signature = buf.read_bytes(96, cursor)?.to_vec();
@@ -263,49 +250,32 @@ impl RootSet {
                     *cursor += buf.read_u8(cursor)? as usize;
                 }
                 Ok(Self {
+                    id,
                     timestamp: timestamp as i64,
-                    name: String::new(),
+                    url: String::new(),
-                    contact: String::new(),
                     roots,
                     signer,
                     signature,
-                    root_set_type,
                 })
             }
 
             ROOT_SET_TYPE_ED25519_P521 => {
-                let timestamp = buf.read_u64(cursor)?;
+                let timestamp = buf.read_varint(cursor)? as i64;
-                let name = String::from_utf8_lossy(buf.read_bytes(buf.read_u8(cursor)? as usize, cursor)?).to_string();
+                let url = String::from_utf8_lossy(buf.read_bytes(buf.read_varint(cursor)? as usize, cursor)?).to_string();
-                let contact = String::from_utf8_lossy(buf.read_bytes(buf.read_u8(cursor)? as usize, cursor)?).to_string();
+                let signer = buf.read_bytes(ED25519_PUBLIC_KEY_SIZE + P521_PUBLIC_KEY_SIZE, cursor)?.to_vec();
-                let signer = buf.read_bytes(buf.read_u8(cursor)? as usize, cursor)?.to_vec();
+                let signature = buf.read_bytes(ED25519_SIGNATURE_SIZE + P521_ECDSA_SIGNATURE_SIZE, cursor)?.to_vec();
-                let signature = buf.read_bytes(buf.read_u8(cursor)? as usize, cursor)?.to_vec();
+                let id = TypeAndID::Ed25519P521RootSet(SHA384::hash(signer.as_slice()));
-                let root_set_type = Type::Ed25519P521RootSet(SHA384::hash(signer.as_slice()));
                 Ok(Self {
-                    timestamp: timestamp as i64,
+                    id,
-                    name,
+                    timestamp,
-                    contact,
+                    url,
                     roots: read_roots(buf, cursor)?,
                     signer,
                     signature,
-                    root_set_type,
                 })
             }
 
-            _ => {
+            _ => Err(std::io::Error::new(std::io::ErrorKind::InvalidData, "unrecognized type"))
-                Err(std::io::Error::new(std::io::ErrorKind::InvalidData, "unrecognized type"))
-            }
-        }
-    }
-
-    /// Get this root set's globally unique ID.
-    ///
-    /// For new root set format this is a hash of its public keys. For old style planet/moon
-    /// this is a user assigned 64-bit ID. The latter is deprecated but still supported.
-    pub fn id(&self) -> Vec<u8> {
-        match self.root_set_type {
-            Type::LegacyPlanet(id) => id.to_be_bytes().to_vec(),
-            Type::LegacyMoon(id) => id.to_be_bytes().to_vec(),
-            Type::Ed25519P521RootSet(id) => id.to_vec(),
         }
     }
 }
@@ -313,6 +283,24 @@ impl RootSet {
 impl Hash for RootSet {
     fn hash<H: Hasher>(&self, state: &mut H) {
         state.write_u64(self.timestamp as u64);
-        self.root_set_type.hash(state);
+        self.id.hash(state);
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use crate::vl1::rootset::RootSet;
+
+    #[test]
+    fn default_root_set() {
+        let rs = RootSet::from_bytes(&crate::defaults::ROOT_SET).unwrap();
+        /*
+        rs.roots.iter().for_each(|r| {
+            println!("{}", r.identity.to_string());
+            r.endpoints.iter().for_each(|ep| {
+                println!("  {}", ep.to_string());
+            });
+        });
+        */
+    }
+}