wip

Signed-off-by: Federico Di Pierro <nierro92@gmail.com>

.github/PULL_REQUEST_TEMPLATE.md

@@ -22,6 +22,8 @@
 
 > /kind feature
 
+> /kind release
+
 > If contributing rules or changes to rules, please make sure to also uncomment one of the following line:
 
 > /kind rule-update
@@ -46,6 +48,8 @@ Please remove the leading whitespace before the `/kind <>` you uncommented.
 
 > /area proposals
 
+> /area CI
+
 <!--
 Please remove the leading whitespace before the `/area <>` you uncommented.
 -->

CHANGELOG.md

@@ -1,5 +1,17 @@
 # Change Log
 
+## v0.32.2
+
+Released on 2022-08-09
+
+### Major Changes
+
+
+### Bug Fixes
+
+* fix: Added ARCH to bpf download URL [[#2142](https://github.com/falcosecurity/falco/pull/2142)] - [@eric-engberg](https://github.com/eric-engberg)
+
+
 ## v0.32.1
 
 Released on 2022-07-11

cmake/modules/driver.cmake

@@ -26,8 +26,8 @@ else()
   # In case you want to test against another driver version (or branch, or commit) just pass the variable -
   # ie., `cmake -DDRIVER_VERSION=dev ..`
   if(NOT DRIVER_VERSION)
-    set(DRIVER_VERSION "b4c198773bf05486e122f6d3f7f63be125242413")
-    set(DRIVER_CHECKSUM "SHA256=e85fa42a0b58ba21ca7efb38c20ce25207f4816245bdf154e6b9a037a1cce930")
+    set(DRIVER_VERSION "d673b2722e8a730013bfe525eaa417945ef8b723")
+    set(DRIVER_CHECKSUM "SHA256=de7a2fb42da781d5b867f25a009cfdf8b03a1c12dc9a5d2abba08c7afb5a14d4")
   endif()
 
   # cd /path/to/build && cmake /path/to/source
@@ -45,4 +45,4 @@ set(DRIVER_NAME "falco")
 set(DRIVER_PACKAGE_NAME "falco")
 set(DRIVER_COMPONENT_NAME "falco-driver")
 
-add_subdirectory(${DRIVER_SOURCE_DIR} ${PROJECT_BINARY_DIR}/driver)
+add_subdirectory(${DRIVER_SOURCE_DIR} ${PROJECT_BINARY_DIR}/driver)

cmake/modules/falcosecurity-libs.cmake

@@ -27,8 +27,8 @@ else()
   # In case you want to test against another falcosecurity/libs version (or branch, or commit) just pass the variable -
   # ie., `cmake -DFALCOSECURITY_LIBS_VERSION=dev ..`
   if(NOT FALCOSECURITY_LIBS_VERSION)
-    set(FALCOSECURITY_LIBS_VERSION "b4c198773bf05486e122f6d3f7f63be125242413")
-    set(FALCOSECURITY_LIBS_CHECKSUM "SHA256=e85fa42a0b58ba21ca7efb38c20ce25207f4816245bdf154e6b9a037a1cce930")
+    set(FALCOSECURITY_LIBS_VERSION "d673b2722e8a730013bfe525eaa417945ef8b723")
+    set(FALCOSECURITY_LIBS_CHECKSUM "SHA256=de7a2fb42da781d5b867f25a009cfdf8b03a1c12dc9a5d2abba08c7afb5a14d4")
   endif()
 
   # cd /path/to/build && cmake /path/to/source
@@ -84,4 +84,4 @@ endif()
 
 include(driver)
 include(libscap)
-include(libsinsp)
+include(libsinsp)

rules/falco_rules.yaml

@@ -2152,20 +2152,13 @@
 # In some environments, any attempt by a interpreted program (perl,
 # python, ruby, etc) to listen for incoming connections or perform
 # outgoing connections might be suspicious. These rules are not
-# enabled by default, but you can modify the following macros to
-# enable them.
-
-- macro: consider_interpreted_inbound
-  condition: (never_true)
-
-- macro: consider_interpreted_outbound
-  condition: (never_true)
+# enabled by default.
 
 - rule: Interpreted procs inbound network activity
   desc: Any inbound network activity performed by any interpreted program (perl, python, ruby, etc.)
   condition: >
-    (inbound and consider_interpreted_inbound
-     and interpreted_procs)
+    (inbound and interpreted_procs)
+  enabled: false
   output: >
     Interpreted program received/listened for network traffic
     (user=%user.name user_loginuid=%user.loginuid command=%proc.cmdline connection=%fd.name container_id=%container.id image=%container.image.repository)
@@ -2175,8 +2168,8 @@
 - rule: Interpreted procs outbound network activity
   desc: Any outbound network activity performed by any interpreted program (perl, python, ruby, etc.)
   condition: >
-    (outbound and consider_interpreted_outbound
-     and interpreted_procs)
+    (outbound and interpreted_procs)
+  enabled: false
   output: >
     Interpreted program performed outgoing network connection
     (user=%user.name user_loginuid=%user.loginuid command=%proc.cmdline connection=%fd.name container_id=%container.id image=%container.image.repository)
@@ -2362,10 +2355,7 @@
 
 
 # This rule is not enabled by default, since this rule is for cloud environment(GCP, AWS and Azure) only.
-# If you want to enable this rule, overwrite the first macro,
-# And you can filter the container that you want to allow access to metadata by overwriting the second macro.
-- macro: consider_metadata_access
-  condition: (never_true)
+# You can filter the container that you want to allow access to metadata by overwriting user_known_metadata_access macro.
 
 - macro: user_known_metadata_access
   condition: (k8s.ns.name = "kube-system")
@@ -2374,7 +2364,8 @@
 # metadata about the instance. The metadata could be used to get credentials by attackers.
 - rule: Contact cloud metadata service from container
   desc: Detect attempts to contact the Cloud Instance Metadata Service from a container
-  condition: outbound and fd.sip="169.254.169.254" and container and consider_metadata_access and not user_known_metadata_access
+  condition: outbound and fd.sip="169.254.169.254" and container and not user_known_metadata_access
+  enabled: false
   output: Outbound connection to cloud instance metadata service (command=%proc.cmdline connection=%fd.name %container.info image=%container.image.repository:%container.image.tag)
   priority: NOTICE
   tags: [network, container, mitre_discovery]

userspace/engine/falco_engine.cpp

@@ -18,6 +18,7 @@ limitations under the License.
 #include <unistd.h>
 #include <string>
 #include <fstream>
+#include <functional>
 #include <utility>
 
 #include <sinsp.h>
@@ -171,20 +172,7 @@ void falco_engine::load_rules(const string &rules_content, bool verbose, bool al
 
 	std::unique_ptr<load_result> res = load_rules(rules_content, no_name);
 
-	if(verbose)
-	{
-		// Here, verbose controls whether to additionally
-		// "log" e.g. print to stderr. What's logged is always
-		// non-verbose so it fits on a single line.
-		// todo(jasondellaluce): introduce a logging callback in Falco
-		fprintf(stderr, "%s\n", res->as_string(false).c_str());
-	}
-
-	if(!res->successful())
-	{
-		// The output here is always the full e.g. "verbose" output.
-		throw falco_exception(res->as_string(true).c_str());
-	}
+	interpret_load_result(res, no_name, rules_content, verbose);
 }
 
 std::unique_ptr<load_result> falco_engine::load_rules(const std::string &rules_content, const std::string &name)
@@ -211,44 +199,33 @@ std::unique_ptr<load_result> falco_engine::load_rules(const std::string &rules_c
 
 void falco_engine::load_rules_file(const std::string &rules_filename, bool verbose, bool all_events)
 {
-	std::unique_ptr<load_result> res = load_rules_file(rules_filename);
+	std::string rules_content;
 
-	if(verbose)
-	{
-		// Here, verbose controls whether to additionally
-		// "log" e.g. print to stderr. What's logged is always
-		// non-verbose so it fits on a single line.
-		// todo(jasondellaluce): introduce a logging callback in Falco
-		fprintf(stderr, "%s\n", res->as_string(false).c_str());
-	}
+	read_file(rules_filename, rules_content);
 
-	if(!res->successful())
-	{
-		// The output here is always the full e.g. "verbose" output.
-		throw falco_exception(res->as_string(true).c_str());
-	}
+	std::unique_ptr<load_result> res = load_rules(rules_content, rules_filename);
+
+	interpret_load_result(res, rules_filename, rules_content, verbose);
 }
 
 std::unique_ptr<load_result> falco_engine::load_rules_file(const string &rules_filename)
 {
-	ifstream is;
+	std::string rules_content;
 
-	is.open(rules_filename);
-	if (!is.is_open())
+	try {
+		read_file(rules_filename, rules_content);
+	}
+	catch (falco_exception &e)
 	{
 		rule_loader::context ctx(rules_filename);
-		std::string empty;
 
 		std::unique_ptr<rule_loader::result> res(new rule_loader::result(rules_filename));
 
-		res->add_error(load_result::LOAD_ERR_FILE_READ, "Could not open for reading.", ctx, empty);
+		res->add_error(load_result::LOAD_ERR_FILE_READ, e.what(), ctx);
 
 		return std::move(res);
 	}
 
-	string rules_content((istreambuf_iterator<char>(is)),
-			     istreambuf_iterator<char>());
-
 	return load_rules(rules_content, rules_filename);
 }
 
@@ -426,6 +403,44 @@ bool falco_engine::is_source_valid(const std::string &source)
 	return m_sources.at(source) != nullptr;
 }
 
+void falco_engine::read_file(const std::string& filename, std::string& contents)
+{
+	ifstream is;
+
+	is.open(filename);
+	if (!is.is_open())
+	{
+		throw falco_exception("Could not open " + filename + " for reading.");
+	}
+
+	contents.assign(istreambuf_iterator<char>(is),
+			istreambuf_iterator<char>());
+}
+
+void falco_engine::interpret_load_result(std::unique_ptr<load_result>& res,
+					 const std::string& rules_filename,
+					 const std::string& rules_content,
+					 bool verbose)
+{
+	falco::load_result::rules_contents_t rc = {{rules_filename, rules_content}};
+
+	if(!res->successful())
+	{
+		// The output here is always the full e.g. "verbose" output.
+		throw falco_exception(res->as_string(true, rc).c_str());
+	}
+
+	if(verbose && res->has_warnings())
+	{
+		// Here, verbose controls whether to additionally
+		// "log" e.g. print to stderr. What's logged is always
+		// non-verbose so it fits on a single line.
+		// todo(jasondellaluce): introduce a logging callback in Falco
+		fprintf(stderr, "%s\n", res->as_string(false, rc).c_str());
+	}
+}
+
+
 bool falco_engine::check_plugin_requirements(
 		const std::vector<plugin_version_requirement>& plugins,
 		std::string& err)

userspace/engine/falco_engine.h

@@ -233,6 +233,17 @@ public:
 		std::string& err);
 
 private:
+
+	// Throws falco_exception if the file can not be read
+	void read_file(const std::string& filename, std::string& contents);
+
+	// For load_rules methods that throw exceptions on error,
+	// interpret a load_result and throw an exception if needed.
+	void interpret_load_result(std::unique_ptr<falco::load_result>& res,
+				   const std::string& rules_filename,
+				   const std::string& rules_content,
+				   bool verbose);
+
 	indexed_vector<falco_source> m_sources;
 
 	falco_source* find_source(std::size_t index);

userspace/engine/falco_load_result.h

@@ -16,6 +16,7 @@ limitations under the License.
 
 #pragma once
 
+#include <functional>
 #include <string>
 #include <nlohmann/json.hpp>
 
@@ -75,20 +76,35 @@ public:
 	// has_warnings() can both be true if there were only warnings.
 	virtual bool has_warnings() = 0;
 
+	// This represents a set of rules contents as a mapping from
+	// rules content name (usually filename) to rules content. The
+	// rules content is actually a reference to the actual string
+	// to avoid copies. Using reference_wrapper allows the
+	// reference to be held in the stl map (bare references can't
+	// be copied/assigned, but reference_wrappers can).
+	//
+	// It's used in the as_string/as_json() methods below.
+	typedef std::map<std::string, std::reference_wrapper<const std::string>> rules_contents_t;
+
 	// This contains a human-readable version of the result,
 	// suitable for display to end users.
 	//
+	// The provided rules_contents_t should map from content name
+	// to rules content (reference) for each rules_content that has
+	// been passed to rule_loader::compile() or
+	// rule_reader::load().
+	//
 	// When verbose is true, the returned value has full details
 	// on the result including document locations/context.
 	//
 	// When verbose is false, the returned value is a short string
 	// with the success value and a list of
 	// errors/warnings. Suitable for simple one-line display.
-	virtual const std::string& as_string(bool verbose) = 0;
+	virtual const std::string& as_string(bool verbose, const rules_contents_t& contents) = 0;
 
 	// This contains the full result structure as json, suitable
 	// for automated parsing/interpretation downstream.
-	virtual const nlohmann::json& as_json() = 0;
+	virtual const nlohmann::json& as_json(const rules_contents_t& contents) = 0;
 };
 
 } // namespace falco

userspace/engine/rule_loader.cpp

@@ -135,46 +135,76 @@ nlohmann::json rule_loader::context::as_json()
 	return ret;
 }
 
-std::string rule_loader::context::snippet(const std::string& content) const
+std::string rule_loader::context::snippet(const falco::load_result::rules_contents_t& rules_contents,
+					  size_t snippet_width) const
 {
 	std::string ret;
 
-	if(m_locs.empty() || content.size() == 0)
+	if(m_locs.empty())
 	{
 		return "<No context available>\n";
 	}
 
 	rule_loader::context::location loc = m_locs.back();
 
+	auto it = rules_contents.find(name);
+
+	if(it == rules_contents.end())
+	{
+		return "<No context available>\n";
+	}
+
+	const std::string& snip_content = it->second;
+
 	size_t from = loc.mark.pos;
 
 	// In some cases like this, where the content ends with a
 	// dangling property value:
 	//   tags:
 	// The YAML::Mark position can be past the end of the file.
-	for(; from > 0 && from >= content.size(); from--);
+	for(; from > 0 && from >= snip_content.size(); from--);
 
-	// Add the line that includes the mark and a marker
-	// at the column number.
-	size_t to = from;
+	// The snippet is generally the line that contains the
+	// position. So walk backwards from pos to the preceding
+	// newline, and walk forwards from pos to the following
+	// newline.
+	//
+	// However, some lines can be very very long, so the walk
+	// forwards/walk backwards is capped at a maximum of
+	// snippet_width/2 characters in either direction.
+	for(; from > 0 && snip_content.at(from) != '\n' && (loc.mark.pos - from) < (snippet_width/2); from--);
 
-	for(; from > 0 && content.at(from) != '\n'; from--);
-	for(; to < content.size()-1 && content.at(to) != '\n'; to++);
+	size_t to = loc.mark.pos;
+	for(; to < snip_content.size()-1 && snip_content.at(to) != '\n' && (to - loc.mark.pos) < (snippet_width/2); to++);
 
 	// Don't include the newlines
-	if(content.at(from) == '\n')
+	if(snip_content.at(from) == '\n')
 	{
 		from++;
 	}
-	if(content.at(to) == '\n')
+	if(snip_content.at(to) == '\n')
 	{
 		to--;
 	}
 
-	ret = content.substr(from, to-from+1) + "\n";
+	ret = snip_content.substr(from, to-from+1);
 
-	// Add a blank line with a marker at the column number
-	ret += std::string(loc.mark.column, ' ') + '^' + "\n";
+	// Replace the initial/end characters with '...' if the walk
+	// forwards/backwards was incomplete
+	if(loc.mark.pos - from >= (snippet_width/2))
+	{
+		ret.replace(0, 2, "...");
+	}
+
+	if(to - loc.mark.pos >= (snippet_width/2))
+	{
+		ret.replace(ret.size()-2, ret.size(), "...");
+	}
+
+	ret += "\n";
+
+	// Add a blank line with a marker at the position within the snippet
+	ret += std::string(loc.mark.pos-from, ' ') + '^' + "\n";
 
 	return ret;
 }
@@ -195,26 +225,26 @@ bool rule_loader::result::has_warnings()
 	return (warnings.size() > 0);
 }
 
-void rule_loader::result::add_error(load_result::error_code ec, const std::string& msg, const context& ctx, const std::string& rules_content)
+void rule_loader::result::add_error(load_result::error_code ec, const std::string& msg, const context& ctx)
 {
-	error err = {ec, msg, ctx, ctx.snippet(rules_content)};
+	error err = {ec, msg, ctx};
 	success = false;
 
 	errors.push_back(err);
 }
 
-void rule_loader::result::add_warning(load_result::warning_code wc, const std::string& msg, const context& ctx, const std::string& rules_content)
+void rule_loader::result::add_warning(load_result::warning_code wc, const std::string& msg, const context& ctx)
 {
-	warning warn = {wc, msg, ctx, ctx.snippet(rules_content)};
+	warning warn = {wc, msg, ctx};
 
 	warnings.push_back(warn);
 }
 
-const std::string& rule_loader::result::as_string(bool verbose)
+const std::string& rule_loader::result::as_string(bool verbose, const rules_contents_t& contents)
 {
 	if(verbose)
 	{
-		return as_verbose_string();
+		return as_verbose_string(contents);
 	}
 	else
 	{
@@ -236,7 +266,19 @@ const std::string& rule_loader::result::as_summary_string()
 		os << name << ": ";
 	}
 
-	os << (success ? "Ok" : "Invalid");
+	if(success)
+	{
+		os << "Ok";
+
+		if (!warnings.empty())
+		{
+			os << ", with warnings";
+		}
+	}
+	else
+	{
+		os << "Invalid";
+	}
 
 	if(!errors.empty())
 	{
@@ -282,7 +324,7 @@ const std::string& rule_loader::result::as_summary_string()
 	return res_summary_string;
 }
 
-const std::string& rule_loader::result::as_verbose_string()
+const std::string& rule_loader::result::as_verbose_string(const rules_contents_t& contents)
 {
 	std::ostringstream os;
 
@@ -296,7 +338,19 @@ const std::string& rule_loader::result::as_verbose_string()
 		os << name << ": ";
 	}
 
-	os << (success ? "Ok" : "Invalid");
+	if(success)
+	{
+		os << "Ok";
+
+		if (!warnings.empty())
+		{
+			os << ", with warnings";
+		}
+	}
+	else
+	{
+		os << "Invalid";
+	}
 
 	if (!errors.empty())
 	{
@@ -310,7 +364,7 @@ const std::string& rule_loader::result::as_verbose_string()
 			os << err.ctx.as_string();
 
 			os << "------" << std::endl;
-			os << err.snippet;
+			os << err.ctx.snippet(contents);
 			os << "------" << std::endl;
 
 			os << load_result::error_code_str(err.ec)
@@ -331,7 +385,7 @@ const std::string& rule_loader::result::as_verbose_string()
 			os << warn.ctx.as_string();
 
 			os << "------" << std::endl;
-			os << warn.snippet;
+			os << warn.ctx.snippet(contents);
 			os << "------" << std::endl;
 
 			os << load_result::warning_code_str(warn.wc)
@@ -345,7 +399,7 @@ const std::string& rule_loader::result::as_verbose_string()
 	return res_verbose_string;
 }
 
-const nlohmann::json& rule_loader::result::as_json()
+const nlohmann::json& rule_loader::result::as_json(const rules_contents_t& contents)
 {
 	nlohmann::json j;
 
@@ -364,7 +418,7 @@ const nlohmann::json& rule_loader::result::as_json()
 		nlohmann::json jerr;
 
 		jerr["context"] = err.ctx.as_json();
-		jerr["context"]["snippet"] = err.snippet;
+		jerr["context"]["snippet"] = err.ctx.snippet(contents);
 
 		jerr["code"] = load_result::error_code_str(err.ec);
 		jerr["codedesc"] = load_result::error_desc(err.ec);
@@ -380,7 +434,7 @@ const nlohmann::json& rule_loader::result::as_json()
 		nlohmann::json jwarn;
 
 		jwarn["context"] = warn.ctx.as_json();
-		jwarn["context"]["snippet"] = warn.snippet;
+		jwarn["context"]["snippet"] = warn.ctx.snippet(contents);
 
 		jwarn["code"] = load_result::warning_code_str(warn.wc);
 		jwarn["codedesc"] = load_result::warning_desc(warn.wc);
@@ -842,8 +896,7 @@ void rule_loader::define(configuration& cfg, rule_info& info)
 	{
 		cfg.res->add_warning(load_result::LOAD_UNKNOWN_SOURCE,
 				     "Unknown source " + info.source + ", skipping",
-				     info.ctx,
-				     cfg.content);
+				     info.ctx);
 		return;
 	}
 
@@ -1044,7 +1097,7 @@ void rule_loader::compile_rule_infos(
 		{
 			for (auto &w : warn_codes)
 			{
-				cfg.res->add_warning(w, "", r.ctx, cfg.content);
+				cfg.res->add_warning(w, "", r.ctx);
 			}
 		}
 
@@ -1091,8 +1144,7 @@ void rule_loader::compile_rule_infos(
 				cfg.res->add_warning(
 					load_result::LOAD_UNKNOWN_FIELD,
 					e.what(),
-					r.cond_ctx,
-					cfg.content);
+					r.cond_ctx);
 			}
 			else
 			{
@@ -1125,8 +1177,7 @@ void rule_loader::compile_rule_infos(
 				cfg.res->add_warning(
 					load_result::LOAD_NO_EVTTYPE,
 					"Rule matches too many evt.type values. This has a significant performance penalty.",
-					r.ctx,
-					cfg.content);
+					r.ctx);
 			}
 		}
 	}
@@ -1146,7 +1197,7 @@ void rule_loader::compile(configuration& cfg, indexed_vector<falco_rule>& out) c
 	}
 	catch(rule_load_exception &e)
 	{
-		cfg.res->add_error(e.ec, e.msg, e.ctx, cfg.content);
+		cfg.res->add_error(e.ec, e.msg, e.ctx);
 	}
 
 	// print info on any dangling lists or macros that were not used anywhere
@@ -1157,8 +1208,7 @@ void rule_loader::compile(configuration& cfg, indexed_vector<falco_rule>& out) c
 			cfg.res->add_warning(
 				load_result::LOAD_UNUSED_MACRO,
 				"Macro not referred to by any other rule/macro",
-				m.ctx,
-				cfg.content);
+				m.ctx);
 		}
 	}
 	for (auto &l : lists)
@@ -1168,8 +1218,7 @@ void rule_loader::compile(configuration& cfg, indexed_vector<falco_rule>& out) c
 			cfg.res->add_warning(
 				load_result::LOAD_UNUSED_LIST,
 				"List not referred to by any other rule/macro",
-				l.ctx,
-				cfg.content);
+				l.ctx);
 		}
 	}
 }

userspace/engine/rule_loader.h

@@ -36,6 +36,8 @@ public:
 	class context
 	{
 	public:
+		static const size_t default_snippet_width = 160;
+
 		struct location
 		{
 			// The original location in the document
@@ -51,7 +53,7 @@ public:
 		};
 
 		context(const std::string& name);
-		context(const YAML::Node& mark,
+		context(const YAML::Node& item,
 			const std::string item_type,
 			const std::string item_name,
 			const context& parent);
@@ -59,7 +61,9 @@ public:
 
 		// Return a snippet of the provided rules content
 		// corresponding to this context.
-		std::string snippet(const std::string& content) const;
+		// Uses the provided rules_contents to look up the original
+		// rules content for a given location name.
+		std::string snippet(const falco::load_result::rules_contents_t& rules_contents, size_t snippet_width = default_snippet_width) const;
 
 		std::string as_string();
 		nlohmann::json as_json();
@@ -77,7 +81,6 @@ public:
 		falco::load_result::warning_code wc;
 		std::string msg;
 		context ctx;
-		std::string snippet;
 	};
 
 	struct error
@@ -85,7 +88,6 @@ public:
 		falco::load_result::error_code ec;
 		std::string msg;
 		context ctx;
-		std::string snippet;
 	};
 
 	class rule_load_exception : public std::exception
@@ -113,22 +115,21 @@ public:
 
 		virtual bool successful() override;
 		virtual bool has_warnings() override;
-		virtual const std::string& as_string(bool verbose) override;
-		virtual const nlohmann::json& as_json() override;
+
+		virtual const std::string& as_string(bool verbose, const falco::load_result::rules_contents_t& contents) override;
+		virtual const nlohmann::json& as_json(const falco::load_result::rules_contents_t& contents) override;
 
 		void add_error(falco::load_result::error_code ec,
 			       const std::string& msg,
-			       const context& ctx,
-			       const std::string& rules_content);
+			       const context& ctx);
 
 		void add_warning(falco::load_result::warning_code ec,
 				 const std::string& msg,
-				 const context& ctx,
-				 const std::string& rules_content);
+				 const context& ctx);
 	protected:
 
 		const std::string& as_summary_string();
-		const std::string& as_verbose_string();
+		const std::string& as_verbose_string(const falco::load_result::rules_contents_t& contents);
 		std::string name;
 		bool success;
 

userspace/engine/rule_reader.cpp

@@ -411,7 +411,7 @@ static void read_item(
 	else
 	{
 		rule_loader::context ctx(item, "unknown", "", parent);
-		cfg.res->add_warning(load_result::LOAD_UNKNOWN_ITEM, "Unknown top level item", ctx, cfg.content);
+		cfg.res->add_warning(load_result::LOAD_UNKNOWN_ITEM, "Unknown top level item", ctx);
 	}
 }
 
@@ -425,7 +425,7 @@ bool rule_reader::load(rule_loader::configuration& cfg, rule_loader& loader)
 	catch(const exception& e)
 	{
 		rule_loader::context ctx(cfg.name);
-		cfg.res->add_error(load_result::LOAD_ERR_YAML_PARSE, e.what(), ctx, cfg.content);
+		cfg.res->add_error(load_result::LOAD_ERR_YAML_PARSE, e.what(), ctx);
 		return false;
 	}
 
@@ -454,7 +454,7 @@ bool rule_reader::load(rule_loader::configuration& cfg, rule_loader& loader)
 			}
 			catch (rule_loader::rule_load_exception &e)
 			{
-				cfg.res->add_error(e.ec, e.msg, e.ctx, cfg.content);
+				cfg.res->add_error(e.ec, e.msg, e.ctx);
 
 				// Although we *could* continue on to the next doc,
 				// as it's effectively a new rules file, for

userspace/falco/app_actions/load_rules_files.cpp

@@ -93,27 +93,37 @@ application::run_result application::load_rules_files()
 		falco_configuration::read_rules_file_directory(path, m_state->config->m_loaded_rules_filenames, m_state->config->m_loaded_rules_folders);
 	}
 
-	for (const auto& filename : m_state->config->m_loaded_rules_filenames)
+	std::vector<std::string> rules_contents;
+	falco::load_result::rules_contents_t rc;
+
+	try {
+		read_files(m_state->config->m_loaded_rules_filenames.begin(),
+			   m_state->config->m_loaded_rules_filenames.end(),
+			   rules_contents,
+			   rc);
+	}
+	catch(falco_exception& e)
+	{
+		return run_result::fatal(e.what());
+	}
+
+	for(auto &filename : m_state->config->m_loaded_rules_filenames)
 	{
 		falco_logger::log(LOG_INFO, "Loading rules from file " + filename + "\n");
 		std::unique_ptr<falco::load_result> res;
 
-		res = m_state->engine->load_rules_file(filename);
-
-		// Print the full output if verbose is true
-		if(m_options.verbose &&
-		   (!res->successful() || res->has_warnings()))
-		{
-			printf("%s\n",
-			       (m_state->config->m_json_output ?
-				res->as_json().dump().c_str() :
-				res->as_string(true).c_str()));
-		}
+		res = m_state->engine->load_rules(rc.at(filename), filename);
 
 		if(!res->successful())
 		{
 			// Return the summary version as the error
-			return run_result::fatal(res->as_string(false));
+			return run_result::fatal(res->as_string(true, rc));
+		}
+
+		// If verbose is true, also print any warnings
+		if(m_options.verbose && res->has_warnings())
+		{
+			fprintf(stderr, "%s\n", res->as_string(true, rc).c_str());
 		}
 	}
 

userspace/falco/app_actions/validate_rules_files.cpp

@@ -23,7 +23,43 @@ application::run_result application::validate_rules_files()
 {
 	if(m_options.validate_rules_filenames.size() > 0)
 	{
+
+		std::vector<std::string> rules_contents;
+		falco::load_result::rules_contents_t rc;
+
+		try {
+			read_files(m_options.validate_rules_filenames.begin(),
+				   m_options.validate_rules_filenames.end(),
+				   rules_contents,
+				   rc);
+		}
+		catch(falco_exception& e)
+		{
+			return run_result::fatal(e.what());
+		}
+
 		bool successful = true;
+
+		// The validation result is *always* printed to
+		// stdout. When json_output is true, the output is in
+		// json format and contains all errors/warnings for
+		// all files.
+		//
+
+		// When json_output is false, it contains a summary of
+		// each file and whether it was valid or not, along
+		// with any errors.  To match older falco behavior,
+		// this *only* contains errors.
+		//
+		// So for each file stdout will contain:
+		//
+		// <filename>: Ok
+		// or
+		// <filename>: Invalid
+		// [All Validation Errors]
+		//
+		// Warnings are only printed to stderr, and only
+		// printed when verbose is true.
 		std::string summary;
 
 		falco_logger::log(LOG_INFO, "Validating rules file(s):\n");
@@ -36,31 +72,45 @@ application::run_result application::validate_rules_files()
 		// validation result is a single json object.
 		nlohmann::json results = nlohmann::json::array();
 
-		for(auto file : m_options.validate_rules_filenames)
+		for(auto &filename : m_options.validate_rules_filenames)
 		{
 			std::unique_ptr<falco::load_result> res;
 
-			res = m_state->engine->load_rules_file(file);
+			res = m_state->engine->load_rules(rc.at(filename), filename);
 
 			successful &= res->successful();
 
-			if(summary != "")
-			{
-				summary += "\n";
-			}
-			summary += file + ": " + (res->successful() ? "Ok" : "Invalid");
-
 			if(m_state->config->m_json_output)
 			{
-				results.push_back(res->as_json());
+				results.push_back(res->as_json(rc));
 			}
 			else
 			{
-				// Print the full output when verbose is true
-				if(m_options.verbose &&
-				   (!res->successful() || res->has_warnings()))
+				if(summary != "")
 				{
-					printf("%s\n", res->as_string(true).c_str());
+					summary += "\n";
+				}
+
+				// Add to the summary if not successful, or successful
+				// with no warnings.
+				if(!res->successful() ||
+				   (res->successful() && !res->has_warnings()))
+				{
+					summary += res->as_string(true, rc);
+				}
+				else
+				{
+					// If here, there must be only warnings.
+					// Add a line to the summary noting that the
+					// file was ok with warnings, without actually
+					// printing the warnings.
+					summary += filename + ": Ok, with warnings";
+
+					// If verbose is true, print the warnings now.
+					if(m_options.verbose)
+					{
+						fprintf(stderr, "%s\n", res->as_string(true, rc).c_str());
+					}
 				}
 			}
 		}
@@ -71,10 +121,13 @@ application::run_result application::validate_rules_files()
 			res["falco_load_results"] = results;
 			printf("%s\n", res.dump().c_str());
 		}
+		else
+		{
+			printf("%s\n", summary.c_str());
+		}
 
 		if(successful)
 		{
-			printf("%s\n", summary.c_str());
 			return run_result::exit();
 		}
 		else

userspace/falco/application.h

@@ -134,6 +134,49 @@ private:
 		bool proceed;
 	};
 
+	// Convenience method. Read a sequence of filenames and fill
+	// in a vector of rules contents.
+        // Also fill in the provided rules_contents_t with a mapping from
+        // filename (reference) to content (reference).
+	// falco_exception if any file could not be read.
+	template<class InputIterator>
+	void read_files(InputIterator begin, InputIterator end,
+			std::vector<std::string>& rules_contents,
+			falco::load_result::rules_contents_t& rc)
+	{
+		// Read the contents in a first pass
+		for(auto it = begin; it != end; it++)
+		{
+			std::string &filename = *it;
+			std::ifstream is;
+			is.open(filename);
+			if (!is.is_open())
+			{
+				throw falco_exception("Could not open file " + filename + " for reading");
+			}
+
+			std::string rules_content((istreambuf_iterator<char>(is)),
+						  istreambuf_iterator<char>());
+			rules_contents.emplace_back(std::move(rules_content));
+		}
+
+		// Populate the map in a second pass to avoid
+		// references becoming invalid.
+		auto it = begin;
+		auto rit = rules_contents.begin();
+		for(; it != end && rit != rules_contents.end(); it++, rit++)
+		{
+			rc.emplace(*it, *rit);
+		}
+
+		// Both it and rit must be at the end, otherwise
+		// there's a bug in the above
+		if(it != end || rit != rules_contents.end())
+		{
+			throw falco_exception("Unexpected mismatch in rules content name/rules content sets?");
+		}
+	}
+
 	// These methods comprise the code the application "runs". The
 	// order in which the methods run is in application.cpp.
 	run_result create_signal_handlers();