Outgoing traffic filter part 2: named address (metalbear-co#1648)

* outgoing filter metalbear-co#702 * Update .lock * Refactoring connect, move connect to local address into UserSocket impl. * Revert to free function to avoid dup issue. * Extracting values from config (unsatisfied). * Changes im not sure about. * Add outgoing filter parsing. * Add some invalid cases for testing. * Outgoing filter initialization. * No need to re-create filters with TCP UDP distinction for ANY, as they will be checked in connect_outgoing ConnectType anyway. * Filter is plugged in. * Appease clippy * Add default to config. * Add default for remote/local config. Fix comparing resolved address, now compares user address on port 0 (resolved addresses always have port 0). * inline comparison. * Docs in layer. * unused. * remove comments. * Improve bypass for filtered out. Add to analytics. * Update schema. * Log more stuff in connect_outgoing. * Only bypass on selector if its not unix. * Add more logs. * Log on socket close. * Revert localhost connect refactor. * Add reference, fix compilation * Dont bypass on unix stream, or on empty option for selector. * Better if else. * Better connect_to_local_address (question mark). * Small cleanup and docs. * Add docs for config. * More docs for parsing. * Appease clippy * Modify outgoing test to take filter. Fix filter not properly checking ip port when unspecified. * Docs detailing precedence. * changelog * Removed unused type. * debug->trace * Remove some more logs. * Missed log. * Adding DNS resolution to outgoing filter (issue metalbear-co#702). * Notes on what to do in getaddrinfo * Fix broken doc link. * s/input/rest as the return binding in the parser functions. * Change getaddrinfo, now its possible to resolve dns with it through the remote directly. Resolve DNS for named addresses in the outgoing filter when REMOTE_DNS is enabled. * Address (hehe) CR. Improve docs. Improve names for bindings. Remove many1 concats where digit1 is used. * appease clipy * Docs for outgoing local test. * Only allow either remote or local to be specified. Remove intersection check (cant specify both anymore). * deal with strs instead of bytes. * Improve docs. Co-authored-by: t4lz <t4lz.git@gmail.com> * Fix check on connect_remote. Update docs. * Convert outgoing filter into enum, to typefy that the user can only specify 1 variant. * Improve config, now it works and no long allows remote + local together. * Fix docs (update them to the new config). * Mark with unstable. * Update schema. * Add error on empty values when using from_env outgoing filter. * Change log level for connect. * Improve config, take out inner filter struct. * Remove outdated file. * Fix config for tests. * Run test on mac and linux * Improve config path handling in test. Co-authored-by: t4lz <t4lz.git@gmail.com> * Working config. * Build test for macos. * Sanity check that missing remote address doesnt trigger daemon messages and hangs. * Fix docs. Co-authored-by: t4lz <t4lz.git@gmail.com> * update schema * Fix compilation. Added some notes on how to improve DNS resolution. * Appease clippy * Fix filtering unresolved hosts (bool flag was wrong). Add a few logs. Outgoing named filter should be working now. * Add test for DNS resolving filter. * panic on unexpected message. * use magic service * trying to get the flow right * the test keeps growing (and not working) * Use e2e outgoing_traffic_udp_with_connect to test outgoing named filter. * Remove integration changes. * revert files * Fix config * Use dynamic internal service name in config. * cleanup, fix docs * changelog * use service name as the random string for test file * remove commented code * Resolve DNS locally when local is used. * Address review. Better length calculation. Improve name of closure. Dont reuse test. Better order for filtering. * Warn on potential misuse of remote + dns turned off. * new warning Co-authored-by: t4lz <t4lz.git@gmail.com> * Move warning to cli execution thingy. * use warning to print warning * Improving DNS resolving for connect filter. Now local = port 7777 resolves with the correct local address, and swap it on the users connect call. * remote filter on port now resolves and connects to address from cluster * fix local resolve dns for filters not having port * simplify retrieval of connection address from dns_cache. * docs * appease clippy * docs for DNS_CACHE * cargo.lock * improve docs. refactor some names. simplify local resolve check in dns_cache. * debug -> trace * cache -> reverse mapping * docs for local selector * improve docs Co-authored-by: t4lz <t4lz.git@gmail.com> * appease fmt lint --------- Co-authored-by: t4lz <t4lz.git@gmail.com>
meowjesty · Aug 21, 2023 · 4eef744 · 4eef744
1 parent b60f9b7
commit 4eef744
Show file tree

Hide file tree

Showing 13 changed files with 477 additions and 109 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/changelog.d/702.added.md b/changelog.d/702.added.md
@@ -0,0 +1 @@
+Support DNS resolution for the outgoing filter config.
diff --git a/mirrord/agent/src/dns.rs b/mirrord/agent/src/dns.rs
@@ -32,7 +32,7 @@ pub struct DnsRequest {
 /// Uses `AsyncResolver:::lookup_ip` to resolve `host`.
 ///
 /// `root_path` is used to read `/proc/{pid}/root` configuration files when creating a resolver.
-#[tracing::instrument(level = "trace")]
+#[tracing::instrument(level = "trace", ret)]
 async fn dns_lookup(root_path: &Path, host: String) -> RemoteResult<DnsLookup> {
     let resolv_conf_path = root_path.join("etc").join("resolv.conf");
     let hosts_path = root_path.join("etc").join("hosts");
@@ -113,13 +113,15 @@ impl DnsApi {
         }
     }
 
+    #[tracing::instrument(level = "trace", ret, skip(self))]
     async fn try_make_request(&self, request: GetAddrInfoRequest) -> Result<GetAddrInfoResponse> {
         let (tx, rx) = oneshot::channel();
         let request = DnsRequest { request, tx };
         self.sender.send(request).await?;
         rx.await.map_err(Into::into)
     }
 
+    #[tracing::instrument(level = "trace", ret, skip(self))]
     pub async fn make_request(
         &mut self,
         request: GetAddrInfoRequest,

diff --git a/mirrord/cli/src/connection.rs b/mirrord/cli/src/connection.rs
@@ -1,6 +1,6 @@
 use std::time::Duration;
 
-use mirrord_config::LayerConfig;
+use mirrord_config::{feature::network::outgoing::OutgoingFilterConfig, LayerConfig};
 use mirrord_kube::api::{kubernetes::KubernetesAPI, AgentManagment};
 use mirrord_operator::client::{OperatorApi, OperatorApiError, OperatorSessionInformation};
 use mirrord_progress::Progress;
@@ -75,6 +75,18 @@ pub(crate) async fn create_and_connect<P>(
 where
     P: Progress + Send + Sync,
 {
+    if let Some(outgoing_filter) = &config.feature.network.outgoing.filter {
+        if matches!(outgoing_filter, OutgoingFilterConfig::Remote(_)) && !config.feature.network.dns
+        {
+            progress.warning(
+                    "The mirrord outgoing traffic filter includes host names to be connected remotely,\
+                     but the remote DNS feature is disabled, so the addresses of these hosts will be\
+                     resolved locally!\n\
+                     > Consider enabling the remote DNS resolution feature.",
+            );
+        }
+    }
+
     if config.operator && let Some((sender, receiver, operator_information)) = create_operator_session(config, progress).await? {
         Ok((
             AgentConnectInfo::Operator(operator_information),

diff --git a/mirrord/config/src/feature/network/outgoing.rs b/mirrord/config/src/feature/network/outgoing.rs
@@ -154,7 +154,7 @@ impl MirrordToggleableConfig for OutgoingFileConfig {
 }
 
 /// <!--${internal}-->
-/// Errors related to parsing an [`OutgoingFilter`].  
+/// Errors related to parsing an [`OutgoingFilter`].
 #[derive(Debug, Error)]
 pub enum OutgoingFilterError {
     #[error("Nom: failed parsing with {0}!")]

diff --git a/mirrord/layer/Cargo.toml b/mirrord/layer/Cargo.toml
@@ -57,6 +57,7 @@ hyper-util.workspace = true
 http-body-util  = { workspace = true }
 bimap.workspace = true
 dashmap = "5.4"
+hashbrown = "0.14"
 exec.workspace = true
 
 [target.'cfg(target_os = "macos")'.dependencies]

diff --git a/mirrord/layer/src/detour.rs b/mirrord/layer/src/detour.rs
@@ -200,10 +200,6 @@ pub(crate) enum Bypass {
     /// Hooked a `connect` to a target that is disabled in the configuration.
     DisabledOutgoing,
 
-    /// Outgoing connection either did not match any `remote` selector, or it matched a `local`
-    /// one.
-    FilteredConnection,
-
     /// Incoming traffic is disabled, bypass.
     DisabledIncoming,
 }

diff --git a/mirrord/layer/src/lib.rs b/mirrord/layer/src/lib.rs
@@ -232,12 +232,8 @@ pub(crate) static INCOMING_CONFIG: OnceLock<IncomingConfig> = OnceLock::new();
 /// mirroring/stealing from a targetless agent.
 pub(crate) static TARGETLESS: OnceLock<bool> = OnceLock::new();
 
-// TODO(alex): To support DNS on the selector, change it to `LazyLock<Arc<Mutex>>`, so we can modify
-// the global on `OutgoingSelector::connect_remote`, converting `AddressFilter:Name` to however
-// many addresses we resolve into `AddressFilter::Socket`.
-//
-// Also, we need a global for `REMOTE_DNS`, so we can check it in
-// `OutgoingSelector::connect_remote`, and only resolve DNS through remote if it's `true`.
+/// Whether to resolve DNS locally or through the remote pod.
+pub(crate) static REMOTE_DNS: OnceLock<bool> = OnceLock::new();
 
 /// Selector for how outgoing connection will behave, either sending traffic via the remote or from
 /// local app, according to how the user set up the `remote`, and `local` filter, in
@@ -366,6 +362,9 @@ fn set_globals(config: &LayerConfig) {
     FILE_MODE
         .set(config.feature.fs.clone())
         .expect("Setting FILE_MODE failed.");
+    INCOMING_CONFIG
+        .set(config.feature.network.incoming.clone())
+        .expect("SETTING INCOMING_CONFIG singleton");
 
     // These must come before `OutgoingSelector::new`.
     ENABLED_TCP_OUTGOING
@@ -375,12 +374,12 @@ fn set_globals(config: &LayerConfig) {
         .set(config.feature.network.outgoing.udp)
         .expect("Setting ENABLED_UDP_OUTGOING singleton");
 
-    INCOMING_CONFIG
-        .set(config.feature.network.incoming.clone())
-        .expect("SETTING INCOMING_CONFIG singleton");
+    REMOTE_DNS
+        .set(config.feature.network.dns)
+        .expect("Setting REMOTE_DNS singleton");
 
     {
-        let outgoing_selector = config
+        let outgoing_selector: OutgoingSelector = config
             .feature
             .network
             .outgoing
@@ -1062,13 +1061,11 @@ pub(crate) unsafe extern "C" fn fork_detour() -> pid_t {
     res
 }
 
-// TODO(alex) [mid] 2023-01-24: What is this?
-///
 /// No need to guard because we call another detour which will do the guard for us.
 ///
 /// ## Hook
 ///
-/// Replaces `?`.
+/// One of the many [`libc::close`]-ish functions.
 #[hook_fn]
 pub(crate) unsafe extern "C" fn close_nocancel_detour(fd: c_int) -> c_int {
     close_detour(fd)
@@ -1084,8 +1081,6 @@ pub(crate) unsafe extern "C" fn __close_detour(fd: c_int) -> c_int {
     close_detour(fd)
 }
 
-// TODO(alex) [mid] 2023-01-24: What is this?
-///
 /// ## Hook
 ///
 /// Needed for libuv that calls the syscall directly.