Log the Windows service's connection failures at warn

The service now answers Pause and Continue only after rewriting the hosts
file, returning the write's error so the tray's ControlService call fails
and shows it. The tray's popup includes the error's source, which
windows-service's own message hides.

core/src/config/mod.rs

@@ -1,10 +1,10 @@
 use crate::policies::registries::{Registry, BUILT_IN};
 use directories::ProjectDirs;
 use serde::Deserialize;
-use serde_json::{json, Map, Value};
+use serde_json::{Map, Value};
 use std::collections::HashMap;
 use std::io::{Error, ErrorKind};
-use std::path::{Path, PathBuf};
+use std::path::PathBuf;
 
 pub const DEFAULT_PORT: u16 = 8080;
 
@@ -129,31 +129,3 @@ fn merge(base: &mut Map<String, Value>, top: Map<String, Value>) {
         }
     }
 }
-
-/// Adds an `allow` entry to the file, leaving the rest of it as written.
-pub fn allow(path: &Path, eco: &str, name: &str, version: Option<&str>) -> std::io::Result<()> {
-    let invalid = |e: String| Error::new(ErrorKind::InvalidData, format!("{}: {e}", path.display()));
-    let mut config = match std::fs::read_to_string(path) {
-        Ok(text) => serde_json::from_str(&text).map_err(|e| invalid(e.to_string()))?,
-        Err(e) if e.kind() == ErrorKind::NotFound => json!({}),
-        Err(e) => return Err(e),
-    };
-    if !config.is_object() {
-        return Err(invalid("not an object".into()));
-    }
-    let entry = json!({ "name": name, "version": version });
-    let entries = &mut config["allow"][eco];
-    if entries.is_null() {
-        *entries = Value::Array(Vec::new());
-    }
-    let Some(entries) = entries.as_array_mut() else {
-        return Err(invalid(format!("allow.{eco} is not a list")));
-    };
-    if !entries.contains(&entry) {
-        entries.push(entry);
-    }
-    if let Some(dir) = path.parent() {
-        std::fs::create_dir_all(dir)?;
-    }
-    std::fs::write(path, serde_json::to_string_pretty(&config)?)
-}

core/src/os/macos/mod.rs

@@ -4,7 +4,7 @@ mod uninstall;
 
 pub use uninstall::uninstall;
 
-use crate::config::{self, FirewallConfig};
+use crate::config::FirewallConfig;
 use crate::{blocks, mitm, telemetry};
 use crate::Firewall;
 use rama::bytes::Bytes;
@@ -22,7 +22,6 @@ use rama::net::apple::networkextension::{transparent_proxy_ffi, NwTcpStream, Tcp
 use rama::rt::Executor;
 use rama::telemetry::tracing::apple::oslog::{OsLogLayer, Privacy};
 use rama::{Layer, Service};
-use serde::Deserialize;
 use std::convert::Infallible;
 use std::path::PathBuf;
 use std::sync::Arc;
@@ -60,11 +59,7 @@ impl TransparentProxyHandler for Handler {
         Some(Bytes::from(match message.as_str() {
             "ca" => self.ca.pem.clone(),
             "blocks" => serde_json::to_string(&blocks::take()).expect("blocks serialize"),
-            other => match other.strip_prefix("allow ").map(allow) {
-                Some(Ok(())) => "ok".to_owned(),
-                Some(Err(error)) => format!("error: {error}"),
-                None => format!("error: unknown message {other}"),
-            },
+            other => format!("error: unknown message {other}"),
         }))
     }
 
@@ -123,20 +118,6 @@ fn handler(exec: Executor) -> Result<Handler, BoxError> {
     Ok(Handler { ca, hosts, service })
 }
 
-#[derive(Deserialize)]
-struct AllowRequest {
-    eco: String,
-    name: String,
-    version: Option<String>,
-}
-
-/// Takes effect when the provider restarts, which rebuilds it from the config.
-fn allow(request: &str) -> Result<(), BoxError> {
-    let request: AllowRequest = serde_json::from_str(request)?;
-    config::allow(CONFIG_FILE.as_ref(), &request.eco, &request.name, request.version.as_deref())?;
-    Ok(())
-}
-
 /// stderr is invisible in a system extension, so events go to the unified log:
 /// `log stream --predicate 'subsystem == "com.usestrix.firewall"'`.
 fn init(_config: Option<&TransparentProxyInitConfig>) -> bool {

core/src/os/windows/service.rs

@@ -26,6 +26,7 @@ use std::net::{IpAddr, Ipv4Addr};
 use std::sync::Arc;
 use std::time::Duration;
 use tracing_appender::rolling::{RollingFileAppender, Rotation};
+use windows::Win32::Foundation::ERROR_GEN_FAILURE;
 use windows_service::service::*;
 use windows_service::service_control_handler::{
     self, ServiceControlHandlerResult, ServiceStatusHandle,
@@ -73,8 +74,12 @@ fn run_until_stopped() -> Result<(), BoxError> {
         | ServiceControl::Shutdown
         | ServiceControl::Pause
         | ServiceControl::Continue => {
-            let _ = control_sender.send(control);
-            ServiceControlHandlerResult::NoError
+            // Answered once handled, so a failed pause fails the caller's ControlService.
+            let (reply, handled) = tokio::sync::oneshot::channel();
+            let _ = control_sender.send((control, reply));
+            handled
+                .blocking_recv()
+                .unwrap_or(ServiceControlHandlerResult::NoError)
         }
         ServiceControl::Interrogate => ServiceControlHandlerResult::NoError,
         _ => ServiceControlHandlerResult::NotImplemented,
@@ -89,16 +94,26 @@ fn run_until_stopped() -> Result<(), BoxError> {
             }
         };
         // Only pausing removes our hosts entries; a stop or crash keeps them, failing closed.
-        while let Some(control) = control_receiver.recv().await {
+        while let Some((control, reply)) = control_receiver.recv().await {
             let (hosts, state) = match control {
                 ServiceControl::Pause => (&[][..], ServiceState::Paused),
                 ServiceControl::Continue => (&hosts[..], ServiceState::Running),
                 _ => break,
             };
-            match write_hosts_entries(hosts) {
-                Ok(()) => set_status(&status, state, false)?,
-                Err(error) => tracing::error!(%error, "could not rewrite the hosts file"),
-            }
+            let result = match write_hosts_entries(hosts) {
+                Ok(()) => {
+                    set_status(&status, state, false)?;
+                    ServiceControlHandlerResult::NoError
+                }
+                Err(error) => {
+                    tracing::error!(%error, "could not rewrite the hosts file");
+                    let code = error
+                        .raw_os_error()
+                        .map_or(ERROR_GEN_FAILURE.0, |code| code as u32);
+                    ServiceControlHandlerResult::Other(code)
+                }
+            };
+            let _ = reply.send(result);
         }
         Ok::<_, BoxError>(())
     });
@@ -147,7 +162,7 @@ async fn start_screening(status: &ServiceStatusHandle) -> Result<Vec<&'static st
                 resolver: resolver.clone(),
                 service: service.clone(),
             };
-            let upstream = ConsumeErrLayer::trace_as_debug().into_layer(upstream);
+            let upstream = ConsumeErrLayer::trace_as_warning().into_layer(upstream);
             tokio::spawn(listen_on(address, port, &exec)?.serve(upstream));
         }
     }

core/src/os/windows/tray.rs

@@ -144,7 +144,11 @@ impl Tray {
 }
 
 fn show_error(error: BoxError) {
-    let text = HSTRING::from(error.to_string());
+    let text = match error.source() {
+        Some(source) => format!("{error}: {source}"),
+        None => error.to_string(),
+    };
+    let text = HSTRING::from(text);
     unsafe { MessageBoxW(None, &text, w!("Strix Firewall"), MB_ICONERROR) };
 }
 

core/tests/config.rs

@@ -57,18 +57,3 @@ async fn failing_closed_blocks_what_could_not_be_checked() {
     let blocked = screen(url, &firewall).await.unwrap_or_default();
     assert!(blocked.contains("could not be checked"), "{blocked:?}");
 }
-
-#[test]
-fn allowing_keeps_the_rest_of_the_file() {
-    let path = std::env::temp_dir().join(format!("firewall-allow-{}.json", std::process::id()));
-    std::fs::write(&path, r#"{"fail_open": false}"#).expect("write");
-    for _ in 0..2 {
-        firewall::config::allow(&path, "npm", "left-pad", Some("1.3.0")).expect("allow");
-    }
-    let text = std::fs::read_to_string(&path).expect("read");
-    std::fs::remove_file(&path).ok();
-    let config: FirewallConfig = serde_json::from_str(&text).expect("parses");
-    assert!(!config.fail_open);
-    assert_eq!(config.allow["npm"].len(), 1);
-    assert_eq!(config.allow["npm"][0].version.as_deref(), Some("1.3.0"));
-}

macos/App/BlockAlerts.swift

@@ -1,5 +1,4 @@
 import AppKit
-import Security
 
 struct BlockedPackage: Codable, Hashable, Identifiable {
     let eco: String
@@ -12,46 +11,22 @@ struct BlockedPackage: Codable, Hashable, Identifiable {
     var label: String { version.map { "\(name) \($0)" } ?? name }
     /// The denylist and OSV only flag packages known to be malicious; the age check only holds back new ones.
     var malicious: Bool { filter != "age" }
-    /// Malware is never allowed, and a new package only by version.
-    var trustable: Bool { !malicious && version != nil }
 }
 
 @MainActor
 enum BlockAlert {
-    /// Returns whether the user chose to trust the package.
-    static func show(_ package: BlockedPackage) -> Bool {
+    static func show(_ package: BlockedPackage) {
         let alert = NSAlert()
         alert.addButton(withTitle: "OK")
         if package.malicious {
             alert.alertStyle = .critical
-            alert.messageText = "Blocked malware: \(package.label) (\(package.eco))"
-            alert.informativeText = "\(package.label) \(package.reason).\n\n"
-                + "It was not installed. Don't install it from another source. "
-                + "If you didn't expect this package, a dependency may be compromised; tell your security team."
+            alert.messageText = "Blocked a malicious package"
+            alert.informativeText = "\(package.label) (\(package.eco)) \(package.reason). Do not install it."
         } else {
             alert.messageText = "Held back a new package"
             alert.informativeText = "\(package.label) (\(package.eco)) \(package.reason)."
-            if package.trustable { alert.addButton(withTitle: "Trust this version") }
         }
         NSApp.activate(ignoringOtherApps: true)
-        return alert.runModal() == .alertSecondButtonReturn
-    }
-}
-
-enum AdminAuthorization {
-    /// The allowlist applies to every user of the Mac, so changing it takes an admin, as `sudo` would.
-    static func confirm() -> Bool {
-        var authorization: AuthorizationRef?
-        guard AuthorizationCreate(nil, nil, [], &authorization) == errAuthorizationSuccess,
-              let authorization else { return false }
-        defer { AuthorizationFree(authorization, []) }
-        return "system.preferences".withCString { name in
-            var item = AuthorizationItem(name: name, valueLength: 0, value: nil, flags: 0)
-            return withUnsafeMutablePointer(to: &item) { item in
-                var rights = AuthorizationRights(count: 1, items: item)
-                let flags: AuthorizationFlags = [.interactionAllowed, .extendRights]
-                return AuthorizationCopyRights(authorization, &rights, nil, flags, nil) == errAuthorizationSuccess
-            }
-        }
+        alert.runModal()
     }
 }

macos/App/FirewallModel.swift

@@ -29,13 +29,11 @@ enum FirewallError: Error, LocalizedError {
     case extensionUnavailable
     case screeningDidNotStart(NEVPNStatus)
     case extensionStopped(String)
-    case extensionRefused(String)
 
     var errorDescription: String? {
         switch self {
         case .extensionUnavailable: return "The extension is not running"
         case let .screeningDidNotStart(status): return "Screening did not start (\(status.label))"
-        case let .extensionRefused(reply): return "The extension replied \(reply)"
         case let .extensionStopped(reason):
             return "The extension stopped: \(reason). Its log: log show --last 5m --predicate 'subsystem BEGINSWITH \"\(FirewallIDs.appBundleID)\"'"
         }
@@ -203,28 +201,6 @@ final class FirewallModel: NSObject, ObservableObject {
         if let url = URL(string: pane) { NSWorkspace.shared.open(url) }
     }
 
-    /// Allowing a version writes it into the extension's config, which a restart then loads.
-    func trust(_ package: BlockedPackage) async {
-        guard package.trustable else { return }
-        guard AdminAuthorization.confirm() else {
-            log("Trust cancelled for \(package.label)")
-            return
-        }
-        do {
-            var request = ["eco": package.eco, "name": package.name]
-            request["version"] = package.version
-            let json = String(decoding: try JSONSerialization.data(withJSONObject: request), as: UTF8.self)
-            let reply = try await providerMessage("allow \(json)")
-            guard reply == "ok" else { throw FirewallError.extensionRefused(reply) }
-            blocked.removeAll { $0 == package }
-            log("Trusted \(package.label) (\(package.eco))")
-        } catch {
-            log("Could not trust \(package.label): \(error.localizedDescription)")
-            return
-        }
-        await restart()
-    }
-
     /// The extension cannot message the app, so the app asks it for blocks.
     private func watchBlocks() async {
         while true {
@@ -232,7 +208,7 @@ final class FirewallModel: NSObject, ObservableObject {
                let packages = try? JSONDecoder().decode([BlockedPackage].self, from: Data(reply.utf8)) {
                 for package in packages where !blocked.contains(package) {
                     blocked.append(package)
-                    if BlockAlert.show(package) { await trust(package) }
+                    BlockAlert.show(package)
                 }
                 blocked = Array(blocked.suffix(10))
             }

macos/App/MenuBarView.swift

@@ -29,10 +29,6 @@ struct MenuBarView: View {
                 if !package.malicious {
                     Menu("Held back \(package.label) (\(package.eco))") {
                         Text(package.reason)
-                        Button("Trust this version") {
-                            Task { await model.trust(package) }
-                        }
-                        .disabled(model.busy || !package.trustable)
                     }
                 } else {
                     Menu("Blocked \(package.label) (\(package.eco))") {