From 7f107efbf35c420b9cbdd1e7873ed233a7ee8366 Mon Sep 17 00:00:00 2001 From: Keagan McClelland Date: Sat, 2 Oct 2021 14:05:20 -0600 Subject: [PATCH] Bugfix/mdns sync semantics (#550) * fixes issue where mdns.sync would reset the entry group without readding the original A record, causing CNAME's to point to nothing * make analyzer mad to make cargo happy * please both analyzer and cargo * debug logs * demote successful healthcheck logs to trace since they happen once every second * I have no words for this commit message * 2 is 3 --- appmgr/src/bin/embassyd.rs | 4 +- appmgr/src/net/mdns.rs | 108 +++++++++++++++++++++++++++---------- appmgr/src/net/ssl.rs | 8 +-- 3 files changed, 88 insertions(+), 32 deletions(-) diff --git a/appmgr/src/bin/embassyd.rs b/appmgr/src/bin/embassyd.rs index cc80fd2eb..ba120cc57 100644 --- a/appmgr/src/bin/embassyd.rs +++ b/appmgr/src/bin/embassyd.rs @@ -168,7 +168,7 @@ async fn inner_main( log::error!("Error in Status Sync daemon: {}", e); log::debug!("{:?}", e); } else { - log::debug!("Status Sync completed successfully"); + log::trace!("Status Sync completed successfully"); } } }, @@ -184,7 +184,7 @@ async fn inner_main( log::error!("Error in Health Check daemon: {}", e); log::debug!("{:?}", e); } else { - log::debug!("Health Check completed successfully"); + log::trace!("Health Check completed successfully"); } } }, diff --git a/appmgr/src/net/mdns.rs b/appmgr/src/net/mdns.rs index cbed5819f..ab2b83728 100644 --- a/appmgr/src/net/mdns.rs +++ b/appmgr/src/net/mdns.rs @@ -1,8 +1,9 @@ use std::collections::BTreeMap; use avahi_sys::{ - self, avahi_entry_group_add_service, avahi_entry_group_commit, avahi_entry_group_free, - avahi_entry_group_reset, avahi_free, AvahiEntryGroup, + self, avahi_client_errno, avahi_entry_group_add_service, avahi_entry_group_commit, + avahi_entry_group_free, avahi_entry_group_reset, avahi_free, avahi_strerror, AvahiClient, + AvahiEntryGroup, }; use libc::c_void; use tokio::sync::Mutex; @@ -34,8 +35,10 @@ impl MdnsController { pub struct MdnsControllerInner { hostname: Vec, + hostname_raw: *const libc::c_char, entry_group: *mut AvahiEntryGroup, services: BTreeMap<(PackageId, InterfaceId), TorSecretKeyV3>, + client_error: std::pin::Pin>, } unsafe impl Send for MdnsControllerInner {} unsafe impl Sync for MdnsControllerInner {} @@ -43,15 +46,48 @@ unsafe impl Sync for MdnsControllerInner {} impl MdnsControllerInner { fn load_services(&mut self) { unsafe { + log::debug!("Loading services for mDNS"); + let mut res; + let http_tcp_cstr = std::ffi::CString::new("_http._tcp") + .expect("Could not cast _http._tcp to c string"); + res = avahi_entry_group_add_service( + self.entry_group, + avahi_sys::AVAHI_IF_UNSPEC, + avahi_sys::AVAHI_PROTO_UNSPEC, + avahi_sys::AvahiPublishFlags_AVAHI_PUBLISH_USE_MULTICAST, + self.hostname_raw, + http_tcp_cstr.as_ptr(), + std::ptr::null(), + std::ptr::null(), + 443, + // below is a secret final argument that the type signature of this function does not tell you that it + // needs. This is because the C lib function takes a variable number of final arguments indicating the + // desired TXT records to add to this service entry. The way it decides when to stop taking arguments + // from the stack and dereferencing them is when it finds a null pointer...because fuck you, that's why. + // The consequence of this is that forgetting this last argument will cause segfaults or other undefined + // behavior. Welcome back to the stone age motherfucker. + std::ptr::null::(), + ); + if res < avahi_sys::AVAHI_OK { + let e_str = avahi_strerror(res); + log::error!( + "Could not add service to Avahi entry group: {:?}", + std::ffi::CStr::from_ptr(e_str) + ); + avahi_free(e_str as *mut c_void); + panic!("Failed to load Avahi services"); + } + log::info!("Published {:?}", self.hostname_raw); for key in self.services.values() { let lan_address = key .public() .get_onion_address() .get_address_without_dot_onion() + ".local"; + log::debug!("Adding mdns CNAME entry for {}", &lan_address); let lan_address_ptr = std::ffi::CString::new(lan_address) .expect("Could not cast lan address to c string"); - let _ = avahi_sys::avahi_entry_group_add_record( + res = avahi_sys::avahi_entry_group_add_record( self.entry_group, avahi_sys::AVAHI_IF_UNSPEC, avahi_sys::AVAHI_PROTO_UNSPEC, @@ -64,16 +100,27 @@ impl MdnsControllerInner { self.hostname.as_ptr().cast(), self.hostname.len(), ); + if res < avahi_sys::AVAHI_OK { + let e_str = avahi_strerror(res); + log::error!( + "Could not add record for {:?} to Avahi entry group: {:?}", + lan_address_ptr, + std::ffi::CStr::from_ptr(e_str) + ); + avahi_free(e_str as *mut c_void); + panic!("Failed to load Avahi services"); + } log::info!("Published {:?}", lan_address_ptr); } } } fn init() -> Self { unsafe { + log::debug!("Initializing mDNS controller"); let simple_poll = avahi_sys::avahi_simple_poll_new(); let poll = avahi_sys::avahi_simple_poll_get(simple_poll); - let mut stack_err = 0; - let err_c: *mut i32 = &mut stack_err; + let mut box_err = Box::pin(0 as i32); + let err_c: *mut i32 = box_err.as_mut().get_mut(); let avahi_client = avahi_sys::avahi_client_new( poll, avahi_sys::AvahiClientFlags::AVAHI_CLIENT_NO_FAIL, @@ -81,28 +128,31 @@ impl MdnsControllerInner { std::ptr::null_mut(), err_c, ); + if avahi_client == std::ptr::null_mut::() { + let e_str = avahi_strerror(*box_err); + log::error!( + "Could not create avahi client: {:?}", + std::ffi::CStr::from_ptr(e_str) + ); + avahi_free(e_str as *mut c_void); + panic!("Failed to create Avahi Client"); + } let group = avahi_sys::avahi_entry_group_new(avahi_client, Some(noop), std::ptr::null_mut()); - let mut hostname_buf = vec![0]; - { - let hostname_raw = avahi_sys::avahi_client_get_host_name_fqdn(avahi_client); - hostname_buf - .extend_from_slice(std::ffi::CStr::from_ptr(hostname_raw).to_bytes_with_nul()); - let http_tcp_cstr = std::ffi::CString::new("_http._tcp") - .expect("Could not cast _http._tcp to c string"); - avahi_entry_group_add_service( - group, - avahi_sys::AVAHI_IF_UNSPEC, - avahi_sys::AVAHI_PROTO_UNSPEC, - avahi_sys::AvahiPublishFlags_AVAHI_PUBLISH_USE_MULTICAST, - hostname_raw, - http_tcp_cstr.as_ptr(), - std::ptr::null(), - std::ptr::null(), - 443, + if group == std::ptr::null_mut() { + let e_str = avahi_strerror(avahi_client_errno(avahi_client)); + log::error!( + "Could not create avahi entry group: {:?}", + std::ffi::CStr::from_ptr(e_str) ); - avahi_free(hostname_raw as *mut c_void); + avahi_free(e_str as *mut c_void); + panic!("Failed to create Avahi Entry Group"); } + let mut hostname_buf = vec![0]; + let hostname_raw = avahi_sys::avahi_client_get_host_name_fqdn(avahi_client); + log::debug!("hostname_raw: {:?}", std::ffi::CStr::from_ptr(hostname_raw)); + hostname_buf + .extend_from_slice(std::ffi::CStr::from_ptr(hostname_raw).to_bytes_with_nul()); let buflen = hostname_buf.len(); debug_assert!(hostname_buf.ends_with(b".local\0")); debug_assert!(!hostname_buf[..(buflen - 7)].contains(&b'.')); @@ -110,13 +160,16 @@ impl MdnsControllerInner { hostname_buf[0] = (buflen - 8) as u8; // set the prefix length to len - 8 (leading byte, .local, nul) for the main address hostname_buf[buflen - 7] = 5; // set the prefix length to 5 for "local" - avahi_entry_group_commit(group); - - MdnsControllerInner { + let mut res = MdnsControllerInner { hostname: hostname_buf, + hostname_raw, entry_group: group, services: BTreeMap::new(), - } + client_error: box_err, + }; + res.load_services(); + avahi_entry_group_commit(res.entry_group); + res } } fn sync(&mut self) { @@ -148,6 +201,7 @@ impl MdnsControllerInner { impl Drop for MdnsControllerInner { fn drop(&mut self) { unsafe { + avahi_free(self.hostname_raw as *mut c_void); avahi_entry_group_free(self.entry_group); } } diff --git a/appmgr/src/net/ssl.rs b/appmgr/src/net/ssl.rs index e3827b004..bf4c36808 100644 --- a/appmgr/src/net/ssl.rs +++ b/appmgr/src/net/ssl.rs @@ -14,6 +14,8 @@ use tokio::sync::Mutex; use crate::{Error, ErrorKind}; +static CERTIFICATE_VERSION: i32 = 2; // X509 version 3 is actually encoded as '2' in the cert because fuck you. + pub struct SslManager { store: SslStore, root_cert: X509, @@ -204,7 +206,7 @@ fn generate_key() -> Result, Error> { } fn make_root_cert(root_key: &PKey) -> Result { let mut builder = X509Builder::new()?; - builder.set_version(3)?; + builder.set_version(CERTIFICATE_VERSION)?; let embargo = Asn1Time::days_from_now(0)?; builder.set_not_before(&embargo)?; @@ -257,7 +259,7 @@ fn make_int_cert( applicant: &PKey, ) -> Result { let mut builder = X509Builder::new()?; - builder.set_version(3)?; + builder.set_version(CERTIFICATE_VERSION)?; let embargo = Asn1Time::days_from_now(0)?; builder.set_not_before(&embargo)?; @@ -318,7 +320,7 @@ fn make_leaf_cert( applicant: (&PKey, &str), ) -> Result { let mut builder = X509Builder::new()?; - builder.set_version(3)?; + builder.set_version(CERTIFICATE_VERSION)?; let embargo = Asn1Time::days_from_now(0)?; builder.set_not_before(&embargo)?;