From 49128a55ccb09cc8488a78359256a0104998eca7 Mon Sep 17 00:00:00 2001 From: Adam Ierymenko Date: Fri, 2 Dec 2022 16:08:15 -0500 Subject: [PATCH] Use ordered sets and maps in network configs to make them deterministic, and some other cleanup. --- controller/src/controller.rs | 93 ++++++++++++++------- controller/src/model/mod.rs | 14 ++-- controller/src/model/network.rs | 22 ++--- network-hypervisor/src/vl1/path.rs | 1 + network-hypervisor/src/vl2/networkconfig.rs | 2 +- 5 files changed, 84 insertions(+), 48 deletions(-) diff --git a/controller/src/controller.rs b/controller/src/controller.rs index bd529c4cf..c06b8cb93 100644 --- a/controller/src/controller.rs +++ b/controller/src/controller.rs @@ -23,7 +23,7 @@ use zerotier_utils::{ms_monotonic, ms_since_epoch}; use zerotier_vl1_service::VL1Service; use crate::database::*; -use crate::model::{AuthorizationResult, Member, RequestLogItem, CREDENTIAL_WINDOW_SIZE_DEFAULT}; +use crate::model::{AuthenticationResult, Member, RequestLogItem, CREDENTIAL_WINDOW_SIZE_DEFAULT}; // A netconf per-query task timeout, just a sanity limit. const REQUEST_TIMEOUT: Duration = Duration::from_secs(10); @@ -270,10 +270,10 @@ impl Controller { source_identity: &Verified, network_id: NetworkId, time_clock: i64, - ) -> Result<(AuthorizationResult, Option, Option>), Box> { + ) -> Result<(AuthenticationResult, Option, Option>), Box> { let network = self.database.get_network(network_id).await?; if network.is_none() { - return Ok((AuthorizationResult::Rejected, None, None)); + return Ok((AuthenticationResult::Rejected, None, None)); } let network = network.unwrap(); @@ -284,37 +284,45 @@ impl Controller { // Read and modify with extreme care. // If we have a member object and a pinned identity, check to make sure it matches. Also accept - // upgraded identities to replace old versions if they are properly formed and inherit. + // upgraded identities to replace old versions if they are properly formed and their signatures + // all check out (see Identity::is_upgraded_from()). Note that we do not pin the identity here + // if it is unspecified. That's not done until we fully authorize this member, since we don't + // want to have a way to somehow pin the wrong person's identity (if someone manages to somehow + // create a colliding identity and get it to us). if let Some(member) = member.as_mut() { if let Some(pinned_identity) = member.identity.as_ref() { if !pinned_identity.eq(&source_identity) { if source_identity.is_upgraded_from(pinned_identity) { // Upgrade identity types if we have a V2 identity upgraded from a V1 identity. let _ = member.identity.replace(source_identity.clone_without_secret()); + let _ = member.identity_fingerprint.replace(Blob::from(source_identity.fingerprint)); member_changed = true; } else { - return Ok((AuthorizationResult::RejectedIdentityMismatch, None, None)); + return Ok((AuthenticationResult::RejectedIdentityMismatch, None, None)); } } - } else if let Some(pinned_fingerprint) = member.identity_fingerprint.as_ref() { + } + + if let Some(pinned_fingerprint) = member.identity_fingerprint.as_ref() { if pinned_fingerprint.as_bytes().eq(&source_identity.fingerprint) { - // Learn the FULL identity if the fingerprint is pinned and they match. This - // lets us add membrers by address/fingerprint with full SHA384 identity - // verification instead of just the address. - let _ = member.identity.replace(source_identity.clone_without_secret()); - member_changed = true; + if member.identity.is_none() { + // Learn the FULL identity if the fingerprint is pinned and they match. This + // lets us add members by address/fingerprint with full SHA384 identity + // verification instead of just by short address. + let _ = member.identity.replace(source_identity.clone_without_secret()); + member_changed = true; + } } else { - return Ok((AuthorizationResult::RejectedIdentityMismatch, None, None)); + return Ok((AuthenticationResult::RejectedIdentityMismatch, None, None)); } } } - let mut authorization_result = AuthorizationResult::Rejected; + let mut authentication_result = AuthenticationResult::Rejected; - // This is the main "authorized" flag on the member record. If it is true then - // the member is allowed, but with the caveat that SSO must be checked if it's - // enabled on the network. If this is false then the member is rejected unless - // authorized by a token or unless it's a public network. + // This is the main "authorized" state of the member record. If it is true then the member is allowed, + // but with the caveat that SSO must be checked if it's enabled on the network. If this is false then + // the member is rejected unless auto-authorized via a mechanism like public networks below. let mut member_authorized = member.as_ref().map_or(false, |m| m.authorized()); // If the member isn't authorized, check to see if it should be auto-authorized. @@ -324,14 +332,14 @@ impl Controller { let _ = member.insert(Member::new_with_identity(source_identity.as_ref().clone(), network_id)); member_changed = true; } else { - return Ok((AuthorizationResult::Rejected, None, None)); + return Ok((AuthenticationResult::Rejected, None, None)); } } if network.private { // TODO: check token authorization } else { - authorization_result = AuthorizationResult::ApprovedOnPublicNetwork; + authentication_result = AuthenticationResult::ApprovedIsPublicNetwork; member.as_mut().unwrap().last_authorized_time = Some(time_clock); member_authorized = true; member_changed = true; @@ -344,21 +352,48 @@ impl Controller { // is enabled on the network and disagrees. Skip if the verdict is already one of the approved // values, which would indicate auth-authorization above. if member_authorized { - if !authorization_result.approved() { + if !authentication_result.approved() { // TODO: check SSO if enabled on network! - authorization_result = AuthorizationResult::Approved; + authentication_result = AuthenticationResult::Approved; } } else { // This should not be able to be in approved state if member_authorized is still false. - assert!(!authorization_result.approved()); + assert!(!authentication_result.approved()); } + // drop 'mut' from these + let member_authorized = member_authorized; + let authentication_result = authentication_result; + let mut network_config = None; let mut revocations = None; - if authorization_result.approved() { + if authentication_result.approved() { // We should not be able to make it here if this is still false. assert!(member_authorized); + // Pin member identity if not pinned already. This is analogous to SSH "trust on first use" except + // that the ZeroTier address is akin to the host name. Once we've seen the full identity once then + // it becomes truly "impossible" to collide the address. (Unless you can break ECC and SHA384.) + if member.identity.is_none() { + let _ = member.identity.replace(source_identity.clone_without_secret()); + debug_assert!(member.identity_fingerprint.is_none()); + let _ = member.identity_fingerprint.replace(Blob::from(source_identity.fingerprint)); + member_changed = true; + } + + // Make sure these agree. It should be impossible to end up with a member that's authorized and + // whose identity and identity fingerprint don't match. + if !member + .identity + .as_ref() + .unwrap() + .fingerprint + .eq(member.identity_fingerprint.as_ref().unwrap().as_bytes()) + { + debug_assert!(false); + return Ok((AuthenticationResult::RejectedDueToError, None, None)); + } + // Figure out TTL for credentials (time window in V1). let credential_ttl = network.credential_ttl.unwrap_or(CREDENTIAL_WINDOW_SIZE_DEFAULT); @@ -373,10 +408,10 @@ impl Controller { nc.multicast_limit = network.multicast_limit.unwrap_or(DEFAULT_MULTICAST_LIMIT as u32); nc.multicast_like_expire = Some(protocol::VL2_DEFAULT_MULTICAST_LIKE_EXPIRE as u32); nc.mtu = network.mtu.unwrap_or(ZEROTIER_VIRTUAL_NETWORK_DEFAULT_MTU as u16); - nc.routes = network.ip_routes; + nc.routes = network.ip_routes.iter().cloned().collect(); nc.static_ips = member.ip_assignments.iter().cloned().collect(); nc.rules = network.rules; - nc.dns = network.dns; + nc.dns = network.dns.iter().map(|(k, v)| (k.clone(), v.iter().cloned().collect())).collect(); if network.min_supported_version.unwrap_or(0) < (protocol::PROTOCOL_VERSION_V2 as u32) { // If this network supports V1 nodes we have to include V1 credentials. Otherwise we can skip @@ -399,7 +434,7 @@ impl Controller { coo.add_ip(ip); } if !coo.sign(&self.local_identity, &source_identity) { - return Ok((AuthorizationResult::RejectedDueToError, None, None)); + return Ok((AuthenticationResult::RejectedDueToError, None, None)); } v1cred.certificates_of_ownership.push(coo); } @@ -407,7 +442,7 @@ impl Controller { for (id, value) in member.tags.iter() { let tag = vl2::v1::Tag::new(*id, *value, &self.local_identity, network_id, &source_identity, time_clock); if tag.is_none() { - return Ok((AuthorizationResult::RejectedDueToError, None, None)); + return Ok((AuthenticationResult::RejectedDueToError, None, None)); } let _ = v1cred.tags.insert(*id, tag.unwrap()); } @@ -433,7 +468,7 @@ impl Controller { nc.v1_credentials = Some(v1cred); } else { - return Ok((AuthorizationResult::RejectedDueToError, None, None)); + return Ok((AuthenticationResult::RejectedDueToError, None, None)); } } @@ -458,7 +493,7 @@ impl Controller { self.database.save_member(member, false).await?; } - Ok((authorization_result, network_config, revocations)) + Ok((authentication_result, network_config, revocations)) } } diff --git a/controller/src/model/mod.rs b/controller/src/model/mod.rs index 2d1f7598b..ff519c285 100644 --- a/controller/src/model/mod.rs +++ b/controller/src/model/mod.rs @@ -31,7 +31,7 @@ pub struct NetworkExport { #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] #[repr(u8)] -pub enum AuthorizationResult { +pub enum AuthenticationResult { #[serde(rename = "r")] Rejected = 0, #[serde(rename = "rs")] @@ -51,10 +51,10 @@ pub enum AuthorizationResult { #[serde(rename = "at")] ApprovedViaToken = 130, #[serde(rename = "ap")] - ApprovedOnPublicNetwork = 131, + ApprovedIsPublicNetwork = 131, } -impl AuthorizationResult { +impl AuthenticationResult { pub fn as_str(&self) -> &'static str { // These short codes should match the serde enum names above. match self { @@ -67,20 +67,20 @@ impl AuthorizationResult { Self::Approved => "a", Self::ApprovedViaSSO => "as", Self::ApprovedViaToken => "at", - Self::ApprovedOnPublicNetwork => "ap", + Self::ApprovedIsPublicNetwork => "ap", } } /// Returns true if this result is one of the 'approved' result types. pub fn approved(&self) -> bool { match self { - Self::Approved | Self::ApprovedViaSSO | Self::ApprovedViaToken | Self::ApprovedOnPublicNetwork => true, + Self::Approved | Self::ApprovedViaSSO | Self::ApprovedViaToken | Self::ApprovedIsPublicNetwork => true, _ => false, } } } -impl ToString for AuthorizationResult { +impl ToString for AuthenticationResult { fn to_string(&self) -> String { self.as_str().to_string() } @@ -120,7 +120,7 @@ pub struct RequestLogItem { pub source_hops: u8, #[serde(rename = "r")] - pub result: AuthorizationResult, + pub result: AuthenticationResult, #[serde(rename = "nc")] pub config: Option, diff --git a/controller/src/model/network.rs b/controller/src/model/network.rs index a7cf69383..5f01b4327 100644 --- a/controller/src/model/network.rs +++ b/controller/src/model/network.rs @@ -1,6 +1,6 @@ // (c) 2020-2022 ZeroTier, Inc. -- currently proprietary pending actual release and licensing. See LICENSE.md. -use std::collections::{HashMap, HashSet}; +use std::collections::{BTreeMap, BTreeSet}; use std::hash::Hash; use serde::{Deserialize, Serialize}; @@ -28,7 +28,7 @@ pub struct Ipv6AssignMode { pub _6plane: bool, } -#[derive(Clone, PartialEq, Eq, Serialize, Deserialize, Hash, Debug)] +#[derive(Clone, PartialEq, Eq, Serialize, Deserialize, PartialOrd, Ord, Hash, Debug)] pub struct IpAssignmentPool { #[serde(rename = "ipRangeStart")] ip_range_start: InetAddress, @@ -74,21 +74,21 @@ pub struct Network { pub v6_assign_mode: Option, /// IPv4 or IPv6 auto-assignment pools available, must be present to use 'zt' mode. - #[serde(skip_serializing_if = "HashSet::is_empty")] + #[serde(skip_serializing_if = "BTreeSet::is_empty")] #[serde(rename = "ipAssignmentPools")] #[serde(default)] - pub ip_assignment_pools: HashSet, + pub ip_assignment_pools: BTreeSet, /// IPv4 or IPv6 routes to advertise. #[serde(rename = "ipRoutes")] - #[serde(skip_serializing_if = "HashSet::is_empty")] + #[serde(skip_serializing_if = "BTreeSet::is_empty")] #[serde(default)] - pub ip_routes: HashSet, + pub ip_routes: BTreeSet, /// DNS records to push to members. #[serde(default)] - #[serde(skip_serializing_if = "HashMap::is_empty")] - pub dns: HashMap>, + #[serde(skip_serializing_if = "BTreeMap::is_empty")] + pub dns: BTreeMap>, /// Network rule set. #[serde(skip_serializing_if = "Vec::is_empty")] @@ -148,9 +148,9 @@ impl Network { enable_broadcast: None, v4_assign_mode: None, v6_assign_mode: None, - ip_assignment_pools: HashSet::new(), - ip_routes: HashSet::new(), - dns: HashMap::new(), + ip_assignment_pools: BTreeSet::new(), + ip_routes: BTreeSet::new(), + dns: BTreeMap::new(), rules: Vec::new(), credential_ttl: None, min_supported_version: None, diff --git a/network-hypervisor/src/vl1/path.rs b/network-hypervisor/src/vl1/path.rs index df3dbd584..e6b1aee98 100644 --- a/network-hypervisor/src/vl1/path.rs +++ b/network-hypervisor/src/vl1/path.rs @@ -22,6 +22,7 @@ pub(crate) enum PathServiceResult { } /// A remote endpoint paired with a local socket and a local interface. +/// /// These are maintained in Node and canonicalized so that all unique paths have /// one and only one unique path object. That enables statistics to be tracked /// for them and uniform application of things like keepalives. diff --git a/network-hypervisor/src/vl2/networkconfig.rs b/network-hypervisor/src/vl2/networkconfig.rs index c1cc7e665..bf4f1d953 100644 --- a/network-hypervisor/src/vl2/networkconfig.rs +++ b/network-hypervisor/src/vl2/networkconfig.rs @@ -447,7 +447,7 @@ pub struct V1Credentials { } /// Statically pushed L3 IP routes included with a network configuration. -#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, Hash)] +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, PartialOrd, Ord, Hash)] pub struct IpRoute { pub target: InetAddress, #[serde(skip_serializing_if = "Option::is_none")]