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

@@ -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");
                 }
             }
         },

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<u8>,
+    hostname_raw: *const libc::c_char,
     entry_group: *mut AvahiEntryGroup,
     services: BTreeMap<(PackageId, InterfaceId), TorSecretKeyV3>,
+    client_error: std::pin::Pin<Box<i32>>,
 }
 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::<libc::c_char>(),
+            );
+            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::<AvahiClient>() {
+                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);
         }
     }

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<PKey<Private>, Error> {
 }
 fn make_root_cert(root_key: &PKey<Private>) -> Result<X509, Error> {
     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<Private>,
 ) -> Result<X509, Error> {
     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<Private>, &str),
 ) -> Result<X509, Error> {
     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)?;