reflection RFC / policies iteration

RFCs/2021-04-16-76-collection-policies.md

@@ -8,6 +8,8 @@ given directly to pktvisor via command line or through the Admin API if availabl
 
 Policies require a `kind` to indicate the type of policy being applied.
 
+NOTE: this is not yet a complete, formal specification, and not all of it is implemented. See src/tests/test_policies.cpp for current, tested implementation.
+
 `collection-policy-anycast.yaml`
 
 ```yaml
@@ -25,7 +27,7 @@ visor:
         tap: anycast
         # this must match the input_type of the matching tap name, or application of the policy will fail
         input_type: pcap
-        config:
+        filter:
           bpf: "port 53"
       # stream handlers to attach to this input stream
       # these decide exactly which data to summarize and expose for collection
@@ -40,7 +42,7 @@ visor:
             type: net
           udp_traffic:
             type: net
-            config:
+            filter:
               protocols: [ udp ]
             metrics:
               enable:
@@ -58,7 +60,7 @@ visor:
             type: dns
             # specify that the stream handler module requires >= specific version to be successfully applied 
             require_version: "1.0"
-            config:
+            filter:
               # must match the available configuration options for this version of this stream handler
               qname_suffix: .mydomain.com
             metrics:

RFCs/2021-04-16-77-module-reflection.md

@@ -31,30 +31,51 @@ All interfaces and schemas are versioned.
       "eth1": {}
     }
   },
+  "filter": {
+    "bpf": {
+      "type": "string",
+      "input": "text",
+      "label": "Filter Expression",
+      "description": "tcpdump compatible filter expression for limiting the traffic examined (with BPF). See https://www.tcpdump.org/manpages/tcpdump.1.html",
+      "props": {
+        "example": "udp port 53 and host 127.0.0.1"
+      }
+    }
+  },
   "config": {
     "iface": {
-      "required": true,
       "type": "string",
-      "title": "Interface",
-      "description": "The ethernet interface to capture on"
-    },
-    "bpf": {
-      "required": false,
-      "type": "string",
-      "title": "Filter Expression",
-      "description": "tcpdump compatible filter expression for limiting the traffic examined (with BPF). Example: \"port 53\""
+      "input": "text",
+      "label": "Network Interface",
+      "description": "The network interface to capture traffic from",
+      "props": {
+        "required": true,
+        "example": "eth0"
+      }
     },
     "host_spec": {
-      "required": false,
       "type": "string",
-      "title": "Host Specification",
-      "description": "Subnets (comma separated) to consider this HOST, in CIDR form. Example: \"10.0.1.0/24,10.0.2.1/32,2001:db8::/64\""
+      "input": "text",
+      "label": "Host Specification",
+      "description": "Subnets (comma separated) which should be considered belonging to this host, in CIDR form. Used for ingress/egress determination, defaults to host attached to the network interface.",
+      "props": {
+        "advanced": true,
+        "example": "10.0.1.0/24,10.0.2.1/32,2001:db8::/64"
+      }
     },
     "pcap_source": {
-      "required": false,
       "type": "string",
-      "title": "pcap Engine",
-      "description": "pcap backend engine to use. Defaults to best for platform."
+      "input": "select",
+      "label": "Packet Capture Engine",
+      "description": "Packet capture engine to use. Defaults to best for platform.",
+      "props": {
+        "advanced": true,
+        "example": "libpcap",
+        "options": {
+          "libpcap": "libpcap",
+          "af_packet (linux only)": "af_packet"
+        }
+      }
     }
   }
 }
@@ -81,23 +102,39 @@ All interfaces and schemas are versioned.
 ```json
 {
   "version": "1.0",
-  "config": {
-    "filter_exclude_noerror": {
-      "title": "Filter: Exclude NOERROR",
+  "filter": {
+    "exclude_noerror": {
+      "label": "Exclude NOERROR",
       "type": "bool",
+      "input": "checkbox",
       "description": "Filter out all NOERROR responses"
     },
-    "filter_only_rcode": {
-      "title": "Filter: Include Only RCode",
-      "type": "integer",
-      "description": "Filter out any queries which are not the given RCODE"
+    "only_rcode": {
+      "label": "Include Only RCODE",
+      "type": "number",
+      "input": "select",
+      "description": "Filter out any queries which are not the given RCODE",
+      "props": {
+        "allow_custom_options": true,
+        "options": {
+          "NOERROR": 0,
+          "SERVFAIL": 2,
+          "NXDOMAIN": 3,
+          "REFUSED": 5
+        }
+      }
     },
-    "filter_only_qname_suffix": {
-      "title": "Filter: Include Only QName With Suffix",
-      "type": "array[string]",
-      "description": "Filter out any queries whose QName does not end in a suffix on the list"
+    "only_qname_suffix": {
+      "label": "Include Only QName With Suffix",
+      "type": "string[]",
+      "input": "text",
+      "description": "Filter out any queries whose QName does not end in a suffix on the list",
+      "props": {
+        "example": ".foo.com,.example.com"
+      }
     }
   },
+  "config": {},
   "metrics": {
     "cardinality.qname": {
       "type": "cardinality",
@@ -167,8 +204,8 @@ All interfaces and schemas are versioned.
 ```json
 {
   "version": "1.0",
-  "config": {
-  },
+  "filter": { },
+  "config": { },
   "metrics": {
     "cardinality.dst_ips_out": {
       "type": "cardinality",

src/Policies.cpp

@@ -86,19 +86,31 @@ std::vector<Policy *> PolicyManager::load(const YAML::Node &policy_yaml)
             throw PolicyException(fmt::format("unable to retrieve tap '{}': {}", tap_name, e.what()));
         }
 
-        // Tap Input Filter
+        // Tap Input Config and Filter
         Config tap_filter;
+        if (input_node["filter"]) {
+            if (!input_node["filter"].IsMap()) {
+                throw PolicyException("input filter configuration is not a map");
+            }
+            try {
+                tap_filter.config_set_yaml(input_node["filter"]);
+            } catch (ConfigException &e) {
+                throw PolicyException(fmt::format("invalid input filter for tap '{}': {}", tap_name, e.what()));
+            }
+        }
+        Config tap_config;
         if (input_node["config"]) {
             if (!input_node["config"].IsMap()) {
-                throw PolicyException("input filter configuration is not a map");
+                throw PolicyException("input configuration is not a map");
             }
             try {
-                tap_filter.config_set_yaml(input_node["config"]);
+                tap_config.config_set_yaml(input_node["config"]);
             } catch (ConfigException &e) {
                 throw PolicyException(fmt::format("invalid input config for tap '{}': {}", tap_name, e.what()));
             }
         }
 
+
         // Create Policy
         auto policy = std::make_unique<Policy>(policy_name, tap);
         // if and only if policy succeeds, we will return this in result set
@@ -109,6 +121,8 @@ std::vector<Policy *> PolicyManager::load(const YAML::Node &policy_yaml)
         std::string input_stream_module_name;
         try {
             spdlog::get("visor")->info("policy [{}]: instantiating Tap: {}", policy_name, tap_name);
+            // TODO separate config and filter. for now, they merge
+            tap_filter.config_merge(tap_config);
             input_stream = tap->instantiate(policy.get(), &tap_filter);
             // ensure tap input type matches policy input tap
             if (input_node["input_type"].as<std::string>() != tap->input_plugin()->plugin()) {
@@ -155,6 +169,17 @@ std::vector<Policy *> PolicyManager::load(const YAML::Node &policy_yaml)
             if (handler_plugin == _registry->handler_plugins().end()) {
                 throw PolicyException(fmt::format("Policy '{}' requires stream handler type '{}' which is not available", policy_name, handler_module_type));
             }
+            Config handler_filter;
+            if (h_it->second["filter"]) {
+                if (!h_it->second["filter"].IsMap()) {
+                    throw PolicyException("stream handler filter configuration is not a map");
+                }
+                try {
+                    handler_filter.config_set_yaml(h_it->second["filter"]);
+                } catch (ConfigException &e) {
+                    throw PolicyException(fmt::format("invalid stream handler filter config for handler '{}': {}", handler_module_name, e.what()));
+                }
+            }
             Config handler_config;
             if (h_it->second["config"]) {
                 if (!h_it->second["config"].IsMap()) {
@@ -168,6 +193,8 @@ std::vector<Policy *> PolicyManager::load(const YAML::Node &policy_yaml)
             }
             spdlog::get("visor")->info("policy [{}]: instantiating Handler {} of type {}", policy_name, handler_module_name, handler_module_type);
             // note, currently merging the handler config with the window config. do they need to be separate?
+            // TODO separate filter config
+            handler_config.config_merge(handler_filter);
             handler_config.config_merge(window_config);
             auto handler_module = handler_plugin->second->instantiate(policy_name + "-" + handler_module_name, input_stream.get(), &handler_config);
             policy->add_module(handler_module.get());

src/handlers/dns/DnsStreamHandler.cpp

@@ -38,11 +38,11 @@ void DnsStreamHandler::start()
     }
 
     // Setup Filters
-    if (config_exists("filter_exclude_noerror") && config_get<bool>("filter_exclude_noerror")) {
+    if (config_exists("exclude_noerror") && config_get<bool>("exclude_noerror")) {
         _f_enabled.set(Filters::ExcludingRCode);
         _f_rcode = NoError;
-    } else if (config_exists("filter_only_rcode")) {
-        auto want_code = config_get<uint64_t>("filter_only_rcode");
+    } else if (config_exists("only_rcode")) {
+        auto want_code = config_get<uint64_t>("only_rcode");
         switch (want_code) {
         case NoError:
         case NXDomain:
@@ -52,12 +52,12 @@ void DnsStreamHandler::start()
             _f_rcode = want_code;
             break;
         default:
-            throw ConfigException("filter_only_rcode contained an invalid/unsupported rcode");
+            throw ConfigException("only_rcode contained an invalid/unsupported rcode");
         }
     }
-    if (config_exists("filter_only_qname_suffix")) {
+    if (config_exists("only_qname_suffix")) {
         _f_enabled.set(Filters::OnlyQNameSuffix);
-        for (const auto &qname : config_get<StringList>("filter_only_qname_suffix")) {
+        for (const auto &qname : config_get<StringList>("only_qname_suffix")) {
             // note, this currently copies the strings, meaning there could be a big list that is duplicated
             // we can work on trying to make this a string_view instead
             // we copy it out so that we don't have to hit the config mutex

src/handlers/dns/tests/test_dns_layer.cpp

@@ -253,7 +253,7 @@ TEST_CASE("Parse DNS random UDP/TCP tests", "[pcap][net]")
     CHECK(j["top_qtype"][6]["estimate"] == 620);
 }
 
-TEST_CASE("DNS Filters: filter_exclude_noerror", "[pcap][net]")
+TEST_CASE("DNS Filters: exclude_noerror", "[pcap][net]")
 {
 
     PcapInputStream stream{"pcap-test"};
@@ -266,7 +266,7 @@ TEST_CASE("DNS Filters: filter_exclude_noerror", "[pcap][net]")
     c.config_set<uint64_t>("num_periods", 1);
     DnsStreamHandler dns_handler{"dns-test", &stream, &c};
 
-    dns_handler.config_set<bool>("filter_exclude_noerror", true);
+    dns_handler.config_set<bool>("exclude_noerror", true);
 
     dns_handler.start();
     stream.start();
@@ -284,7 +284,7 @@ TEST_CASE("DNS Filters: filter_exclude_noerror", "[pcap][net]")
     REQUIRE(j["wire_packets"]["filtered"] == 22);
 }
 
-TEST_CASE("DNS Filters: filter_only_rcode nx", "[pcap][net]")
+TEST_CASE("DNS Filters: only_rcode nx", "[pcap][net]")
 {
 
     PcapInputStream stream{"pcap-test"};
@@ -297,7 +297,7 @@ TEST_CASE("DNS Filters: filter_only_rcode nx", "[pcap][net]")
     c.config_set<uint64_t>("num_periods", 1);
     DnsStreamHandler dns_handler{"dns-test", &stream, &c};
 
-    dns_handler.config_set<uint64_t>("filter_only_rcode", NXDomain);
+    dns_handler.config_set<uint64_t>("only_rcode", NXDomain);
 
     dns_handler.start();
     stream.start();
@@ -315,7 +315,7 @@ TEST_CASE("DNS Filters: filter_only_rcode nx", "[pcap][net]")
     REQUIRE(j["wire_packets"]["filtered"] == 23);
 }
 
-TEST_CASE("DNS Filters: filter_only_rcode refused", "[pcap][net]")
+TEST_CASE("DNS Filters: only_rcode refused", "[pcap][net]")
 {
 
     PcapInputStream stream{"pcap-test"};
@@ -328,7 +328,7 @@ TEST_CASE("DNS Filters: filter_only_rcode refused", "[pcap][net]")
     c.config_set<uint64_t>("num_periods", 1);
     DnsStreamHandler dns_handler{"dns-test", &stream, &c};
 
-    dns_handler.config_set<uint64_t>("filter_only_rcode", Refused);
+    dns_handler.config_set<uint64_t>("only_rcode", Refused);
 
     dns_handler.start();
     stream.start();
@@ -346,7 +346,7 @@ TEST_CASE("DNS Filters: filter_only_rcode refused", "[pcap][net]")
     REQUIRE(j["wire_packets"]["filtered"] == 23);
 }
 
-TEST_CASE("DNS Filters: filter_only_qname_suffix", "[pcap][net]")
+TEST_CASE("DNS Filters: only_qname_suffix", "[pcap][net]")
 {
 
     PcapInputStream stream{"pcap-test"};
@@ -360,7 +360,7 @@ TEST_CASE("DNS Filters: filter_only_qname_suffix", "[pcap][net]")
     DnsStreamHandler dns_handler{"dns-test", &stream, &c};
 
     // notice, case insensitive
-    dns_handler.config_set<visor::Configurable::StringList>("filter_only_qname_suffix", {"GooGle.com"});
+    dns_handler.config_set<visor::Configurable::StringList>("only_qname_suffix", {"GooGle.com"});
     dns_handler.start();
     stream.start();
     stream.stop();

src/tests/test_policies.cpp

@@ -33,6 +33,8 @@ version: "1.0"
         tap: anycast
         input_type: mock
         config:
+          sample: value
+        filter:
           bpf: "tcp or udp"
       # stream handlers to attach to this input stream
       # these decide exactly which data to summarize and expose for collection
@@ -51,8 +53,8 @@ version: "1.0"
 #              max_deep_sample: 75
           special_domain:
             type: dns
-            config:
-              filter_only_qname_suffix:
+            filter:
+              only_qname_suffix:
                 - ".google.com"
                 - ".ns1.com"
                 - "slack.com"
@@ -96,7 +98,7 @@ version: "1.0"
       input:
         tap: anycast
         input_type: mock
-        config:
+        filter:
           bpf:
             badmap: "bad value"
 )";
@@ -174,11 +176,12 @@ TEST_CASE("Policies", "[policies]")
         auto [policy, lock] = registry.policy_manager()->module_get_locked("default_view");
         CHECK(policy->name() == "default_view");
         CHECK(policy->input_stream()->name() == "anycast-default_view");
-        CHECK(policy->input_stream()->config_get<std::string>("bpf") == "tcp or udp");
+        CHECK(policy->input_stream()->config_get<std::string>("bpf") == "tcp or udp"); // TODO this will move to filter member variable
+        CHECK(policy->input_stream()->config_get<std::string>("sample") == "value");
         CHECK(policy->modules()[0]->name() == "default_view-default_net");
         CHECK(policy->modules()[1]->name() == "default_view-default_dns");
         CHECK(policy->modules()[2]->name() == "default_view-special_domain");
-        CHECK(policy->modules()[2]->config_get<Configurable::StringList>("filter_only_qname_suffix")[0] == ".google.com");
+        CHECK(policy->modules()[2]->config_get<Configurable::StringList>("only_qname_suffix")[0] == ".google.com");
         // TODO check window config settings made it through
         CHECK(policy->input_stream()->running());
         CHECK(policy->modules()[0]->running());
@@ -224,7 +227,7 @@ TEST_CASE("Policies", "[policies]")
         YAML::Node config_file = YAML::Load(policies_config_bad3);
 
         REQUIRE_NOTHROW(registry.tap_manager()->load(config_file["visor"]["taps"], true));
-        REQUIRE_THROWS_WITH(registry.policy_manager()->load(config_file["visor"]["policies"]), "invalid input config for tap 'anycast': invalid value for key: bpf");
+        REQUIRE_THROWS_WITH(registry.policy_manager()->load(config_file["visor"]["policies"]), "invalid input filter for tap 'anycast': invalid value for key: bpf");
     }
 
     SECTION("Bad Config: exception on input start")