From 3337ae249f32a35ad674d702da9616a455f8a67c Mon Sep 17 00:00:00 2001 From: Luther Blissett Date: Wed, 7 Sep 2022 13:15:34 +0200 Subject: [PATCH] ircd: More robust handling of TOML in parse_configured_contacts. This is done to avoid thread panics which would not exit the program and leave it in some weird limbo. Take note, and add this elsewhere where applicable and be careful about indexing which you haven't confirmed exists. --- bin/ircd/ircd_config.toml | 2 +- bin/ircd/src/buffers.rs | 4 ++ bin/ircd/src/settings.rs | 80 +++++++++++++++++++++++----- src/net/p2p.rs | 2 +- src/net/protocol/protocol_address.rs | 2 +- 5 files changed, 73 insertions(+), 17 deletions(-) diff --git a/bin/ircd/ircd_config.toml b/bin/ircd/ircd_config.toml index c6039711b..5cb7d08b8 100644 --- a/bin/ircd/ircd_config.toml +++ b/bin/ircd/ircd_config.toml @@ -43,7 +43,7 @@ outbound_connections=5 seeds = ["tls://lilith0.dark.fi:25551", "tls://lilith1.dark.fi:25551"] # Prefered transports for outbound connections -#transports = ["tls", "tcp"] +#outbound_transports = ["tls", "tcp"] ## Only used for debugging. Compromises privacy when set. #node_id = "foo" diff --git a/bin/ircd/src/buffers.rs b/bin/ircd/src/buffers.rs index 821d6f0b0..eff93ae07 100644 --- a/bin/ircd/src/buffers.rs +++ b/bin/ircd/src/buffers.rs @@ -145,6 +145,10 @@ impl PrivmsgsBuffer { self.buffer.len() } + pub fn is_empty(&self) -> bool { + self.len() == 0 + } + pub fn last_term(&self) -> u64 { match self.buffer.len() { 0 => 0, diff --git a/bin/ircd/src/settings.rs b/bin/ircd/src/settings.rs index b552d18ce..5fbdc679d 100644 --- a/bin/ircd/src/settings.rs +++ b/bin/ircd/src/settings.rs @@ -1,6 +1,6 @@ use crypto_box::SalsaBox; use fxhash::FxHashMap; -use log::info; +use log::{info, warn}; use serde::Deserialize; use structopt::StructOpt; use structopt_toml::StructOptToml; @@ -135,6 +135,7 @@ fn parse_priv_key(data: &str) -> Result { pk = prv_key.0.into(); } + info!("Found secret key in config, noted it down."); Ok(pk) } @@ -148,9 +149,12 @@ fn parse_priv_key(data: &str) -> Result { pub fn parse_configured_contacts(data: &str) -> Result> { let mut ret = FxHashMap::default(); - let map = match toml::from_str(data)? { - Value::Table(m) => m, - _ => return Ok(ret), + let map = match toml::from_str(data) { + Ok(Value::Table(m)) => m, + _ => { + warn!("Invalid TOML string passed as argument to parse_configured_contacts()"); + return Ok(ret) + } }; if !map.contains_key("contact") { @@ -158,34 +162,82 @@ pub fn parse_configured_contacts(data: &str) -> Result v, + Err(_) => { + info!("Did not found private key in config, skipping contact configuration."); + return Ok(ret) + } + }; + + let bytes: [u8; 32] = match bs58::decode(found_priv).into_vec() { + Ok(v) => { + if v.len() != 32 { + warn!("Decoded base58 secret key string is not 32 bytes"); + warn!("Skipping private contact configuration"); + return Ok(ret) + } + v.try_into().unwrap() + } + Err(e) => { + warn!("Failed to decode base58 secret key from string: {}", e); + warn!("Skipping private contact configuration"); + return Ok(ret) + } + }; + + let secret = crypto_box::SecretKey::from(bytes); + for cnt in contacts { info!("Found configuration for contact {}", cnt.0); - let mut contact_info = ContactInfo::new()?; - if !cnt.1.is_table() && !cnt.1.as_table().unwrap().contains_key("contact_pubkey") { + if !cnt.1.is_table() { + warn!("Config for contact {} isn't a TOML table", cnt.0); + continue + } + + let table = cnt.1.as_table().unwrap(); + if table.is_empty() { + warn!("Configuration for contact {} is empty.", cnt.0); continue } // Build the NaCl box - //// public_key - let pubkey = cnt.1["contact_pubkey"].as_str().unwrap(); - let bytes: [u8; 32] = bs58::decode(pubkey).into_vec()?.try_into().unwrap(); + if !table.contains_key("contact_pubkey") || !table["contact_pubkey"].is_str() { + warn!("Contact {} doesn't have `contact_pubkey` set or is not a string.", cnt.0); + continue + } + + let pub_str = table["contact_pubkey"].as_str().unwrap(); + let bytes: [u8; 32] = match bs58::decode(pub_str).into_vec() { + Ok(v) => { + if v.len() != 32 { + warn!("Decoded base58 string is not 32 bytes"); + continue + } + + v.try_into().unwrap() + } + Err(e) => { + warn!("Failed to decode base58 pubkey from string: {}", e); + continue + } + }; + let public = crypto_box::PublicKey::from(bytes); - - //// private_key - let bytes: [u8; 32] = bs58::decode(parse_priv_key(data)?).into_vec()?.try_into().unwrap(); - let secret = crypto_box::SecretKey::from(bytes); - contact_info.salt_box = Some(SalsaBox::new(&public, &secret)); ret.insert(cnt.0.to_string(), contact_info); info!("Instantiated NaCl box for contact {}", cnt.0); } + Ok(ret) } diff --git a/src/net/p2p.rs b/src/net/p2p.rs index f2159eaf0..04604e755 100644 --- a/src/net/p2p.rs +++ b/src/net/p2p.rs @@ -253,7 +253,7 @@ impl P2p { // Wait for the process for each of the provided addresses, excluding our own inbound addresses async fn outbound_addr_loop( - self_inbound_addr: &Vec, + self_inbound_addr: &[Url], timeout: u64, stop_sub: Subscription<()>, addrs: &Vec, diff --git a/src/net/protocol/protocol_address.rs b/src/net/protocol/protocol_address.rs index 7f537dcf8..e8acfbd02 100644 --- a/src/net/protocol/protocol_address.rs +++ b/src/net/protocol/protocol_address.rs @@ -15,7 +15,7 @@ use super::{ }; const SEND_ADDR_SLEEP_SECONDS: u64 = 900; -const LOCALNET: [&'static str; 5] = ["localhost", "0.0.0.0", "[::]", "127.0.0.1", "[::1]"]; +const LOCALNET: [&str; 5] = ["localhost", "0.0.0.0", "[::]", "127.0.0.1", "[::1]"]; /// Defines address and get-address messages. pub struct ProtocolAddress {