new(userspace/falco,userspace/engine): rule json schema validation #3313

FedeDP · 2024-09-06T12:59:38Z

What type of PR is this?

/kind feature

Any specific area of the project related to this PR?

/area engine

What this PR does / why we need it:

Follow-up to #3302 , validate rule syntax against a json schema.
This PR also adds a --rule-schema CLI option to print the rule schema (just like #3312).

Which issue(s) this PR fixes:

Fixes #3149

Special notes for your reviewer:

moved yaml_helper to engine folder
the validation is done by the engine itself
validation errors are treated as warnings, just like we did for new(userspace,unit_tests): validate configs against schema #3302

See #3313 (comment) for output examples.

Does this PR introduce a user-facing change?:

new(userspace/falco,userspace/engine): rule json schema validation

github-actions · 2024-09-06T13:00:14Z

This PR may bring feature or behavior changes in the Falco engine and may require the engine version to be bumped.

Please double check userspace/engine/falco_engine_version.h file. See versioning for FALCO_ENGINE_VERSION.

/hold

FedeDP · 2024-09-06T13:04:33Z

/milestone 0.39.0

FedeDP · 2024-09-06T13:10:59Z

Here is how the -V option looks like:

Fri Sep  6 15:09:01 2024: Validating rules file(s):
Fri Sep  6 15:09:01 2024:    ../rules/falco_rules.yaml | schema validation: ok
../rules/falco_rules.yaml: Ok

So, we keep the exact same behavior of the normal rule loading schema validation (and the config schema validation), with $filename | schema validation: $res template.
Another option would be to break the consistency and print:

Fri Sep  6 15:09:01 2024: Validating rules file(s):
Fri Sep  6 15:09:01 2024:    ../rules/falco_rules.yaml
../rules/falco_rules.yaml: Ok | schema validation: ok

Don't really know which is better, i tend to really prefer the consistency of the former.

cc @leogr

FedeDP · 2024-09-06T13:11:35Z

Also cc @jasondellaluce am not sure whether engine version should really be bumped 🤔

FedeDP · 2024-09-06T13:13:06Z

userspace/engine/rule_loader_reader.cpp

 	rule_loader::context ctx(cfg.name);
 	try
 	{
-		docs = YAML::LoadAll(cfg.content);
+		docs = reader.loadall_from_string(cfg.content, schema, &schema_validation);


New yaml_helper method to YAML::LoadAll and then validate each loaded document against provided schema.

FedeDP · 2024-09-06T13:13:58Z

userspace/engine/rule_loader_reader.cpp

@@ -807,7 +810,7 @@ bool rule_loader::reader::read(rule_loader::configuration& cfg, collector& colle
 		cfg.res->add_error(falco::load_result::LOAD_ERR_YAML_PARSE, "unknown YAML parsing error", ctx);
 		return false;
 	}
-
+	cfg.res->set_schema_validation_status(schema_validation);


We will have a schema validation status only if the YAML could be actually loaded, otherwise it will have its default value "none". Not a big deal since an error is thrown if we cannot parse the YAML of course.

FedeDP · 2024-09-06T13:14:07Z

userspace/engine/rule_loader_reader.h

@@ -43,7 +43,7 @@ class reader
 		\brief Reads the contents of a ruleset and uses a collector to store
        thew new definitions
 	*/
-	virtual bool read(configuration& cfg, collector& loader);
+	virtual bool read(configuration& cfg, collector& loader, const nlohmann::json& schema={});


Non-breaking change.

FedeDP · 2024-09-06T13:14:37Z

userspace/engine/yaml_helper.h

@@ -89,7 +84,37 @@ class yaml_helper
 	inline static const std::string configs_key = "config_files";
 	inline static const std::string validation_ok = "ok";
 	inline static const std::string validation_failed = "failed";
-	inline static const std::string validation_none = "schema not provided";
+	inline static const std::string validation_none = "none";


none better reflect the fact that no validation took place, without exposing more info than needed.

FedeDP · 2024-09-06T13:16:16Z

userspace/falco/app/actions/load_rules_files.cpp

@@ -66,12 +66,14 @@ falco::app::run_result falco::app::actions::load_rules_files(falco::app::state&
 	}

 	std::string err = "";
+	falco_logger::log(falco_logger::level::INFO, "Loading rules from:\n");


Follow same behavior as Falco config files loading, ie: print a first line with Loading rules from and then proceed to list all loaded rules with their schema validation status each on a new line.

FedeDP · 2024-09-06T13:52:50Z

userspace/engine/falco_engine.cpp

@@ -47,6 +47,8 @@ limitations under the License.

 const std::string falco_engine::s_default_ruleset = "falco-default-ruleset";

+static const std::string rule_schema_string = R"({"$schema":"http://json-schema.org/draft-06/schema#","type":"array","items":{"anyOf":[{"$ref":"#definitions/RequiredEngineVersionRef"},{"$ref":"#definitions/ListRef"},{"$ref":"#definitions/MacroRef"},{"$ref":"#definitions/RuleRef"},{"$ref":"#definitions/AppendOnlyRuleRef"},{"$ref":"#definitions/EnableOnlyRuleRef"}]},"definitions":{"RequiredEngineVersionRef":{"type":"object","additionalProperties":false,"properties":{"required_engine_version":{"type":"integer"}},"required":["required_engine_version"],"title":"RequiredEngineVersion"},"ListRef":{"type":"object","additionalProperties":false,"properties":{"list":{"type":"string"},"items":{"type":"array","items":{"anyOf":[{"type":"integer"},{"type":"string"}]}},"append":{"type":"boolean"}},"required":["list","items"],"title":"List"},"MacroRef":{"type":"object","additionalProperties":false,"properties":{"macro":{"type":"string"},"condition":{"type":"string"},"append":{"type":"boolean"}},"required":["macro","condition"],"title":"Macro"},"RuleRef":{"type":"object","additionalProperties":false,"properties":{"rule":{"type":"string"},"condition":{"type":"string"},"desc":{"type":"string"},"enabled":{"type":"boolean"},"append":{"type":"boolean"},"output":{"type":"string"},"priority":{"type":"string","enum":["ALERT","EMERGENCY","NOTICE","WARNING","ERROR","DEBUG","INFO","INFORMATIONAL","CRITICAL"]},"tags":{"type":"array","items":{"anyOf":[{"type":"integer"},{"type":"string"}]}},"warn_evttypes":{"type":"boolean"},"skip-if-unknown-filter":{"type":"boolean"}},"required":["rule","desc","condition","output","priority"],"title":"Rule"},"AppendOnlyRuleRef":{"type":"object","additionalProperties":false,"properties":{"rule":{"type":"string"},"condition":{"type":"string"},"append":{"type":"boolean"}},"required":["rule","append","condition"],"title":"AppendOnlyRule"},"EnableOnlyRuleRef":{"type":"object","additionalProperties":false,"properties":{"rule":{"type":"string"},"enabled":{"type":"boolean"}},"required":["rule","enabled"],"title":"EnableOnlyRule"}}})";


It passes all falco engine rule loader tests ;)

leogr · 2024-09-09T08:01:21Z

Here is how the -V option looks like:
Fri Sep  6 15:09:01 2024: Validating rules file(s):
Fri Sep  6 15:09:01 2024:    ../rules/falco_rules.yaml | schema validation: ok
../rules/falco_rules.yaml: Ok
So, we keep the exact same behavior of the normal rule loading schema validation (and the config schema validation), with $filename | schema validation: $res template. Another option would be to break the consistency and print:
Fri Sep  6 15:09:01 2024: Validating rules file(s):
Fri Sep  6 15:09:01 2024:    ../rules/falco_rules.yaml
../rules/falco_rules.yaml: Ok | schema validation: ok 
Don't really know which is better, i tend to really prefer the consistency of the former.

cc @leogr

If we assume that a proper message will be printed when the schema validation fails, another option would be not to expose the schema validation: ok when validation passes. In the end, schema validation is part of validation, and if everything is ok, everything is just ok :)

leogr · 2024-09-09T08:02:56Z

Also cc @jasondellaluce am not sure whether engine version should really be bumped 🤔

It is a false positive because you touched some engine files, but no actual feature or behavior was changed.
/unhold

userspace/engine/falco_engine.cpp

FedeDP · 2024-09-09T13:10:38Z

/hold
Working on the -V changes ;)

Also, a new `--rule-schema` cli option was added to print the schema and leave. Signed-off-by: Federico Di Pierro <nierro92@gmail.com>

Also, moved yaml_helper under engine/ folder. Ported rule json schema validation in the engine. Also, updated rule_loader tests to check for validation. Signed-off-by: Federico Di Pierro <nierro92@gmail.com>

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

Signed-off-by: Federico Di Pierro <nierro92@gmail.com> Co-authored-by: Leonardo Grasso <me@leonardograsso.com>

FedeDP · 2024-09-10T07:36:56Z

Rebased on top of master + added schema validation info (-V output) to rule_loader::result as_json and as_string methods.
New output:

Text:

Mon Sep  9 10:53:35 2024: Validating rules file(s):
Mon Sep  9 10:53:35 2024:    ../rules/falco_rules.yaml
../rules/falco_rules.yaml: Ok | schema validation: ok

Json:

Mon Sep  9 10:48:42 2024: Validating rules file(s):
Mon Sep  9 10:48:42 2024:    ../rules/falco_rules.yaml
{"falco_load_results":[{"errors":[],"name":"../rules/falco_rules.yaml","schema":"ok","successful":true,"warnings":[]}]}

…t` `as_json` and `as_string` outputs. Signed-off-by: Federico Di Pierro <nierro92@gmail.com>

…rnings from yaml_helper::validate_node(). `-V` option will print all warnings, while normal run will only print foremost warning. Signed-off-by: Federico Di Pierro <nierro92@gmail.com>

FedeDP · 2024-09-10T09:52:53Z

Last commit enables yaml_helper to report all schema validation warnings (ie a vector of strings), instead of only the foremost one.

Example outputs:

-V json multiple schema validation issues:

Tue Sep 10 11:19:16 2024: Validating rules file(s):
Tue Sep 10 11:19:16 2024:    ../rules/falco_rules.yaml
{"falco_load_results":[{"errors":[],"name":"../rules/falco_rules.yaml","schema_valid":false,"schema_warnings":["failed for <root>[1]: Object contains a property that could not be validated using 'properties' or 'additionalProperties' constraints: 'conddd'.","failed for <root>: Failed to validate item #1 in array.","failed for <root>[2]: Object contains a property that could not be validated using 'properties' or 'additionalProperties' constraints: 'foo'.","failed for <root>: Failed to validate item #2 in array."],"successful":true,"warnings":[]}]}

-V json single schema validation issue:

Tue Sep 10 11:17:16 2024: Validating rules file(s):
Tue Sep 10 11:17:16 2024:    ../rules/falco_rules.yaml
{"falco_load_results":[{"errors":[],"name":"../rules/falco_rules.yaml","schema_valid":false,"schema_warnings":["failed for <root>[1]: Object contains a property that could not be validated using 'properties' or 'additionalProperties' constraints: 'conddd'.","failed for <root>: Failed to validate item #1 in array."],"successful":true,"warnings":[]}]}

-V json ok schema validation:

{"falco_load_results":[{"errors":[],"name":"../rules/falco_rules.yaml","schema_valid":true,"schema_warnings":[],"successful":true,"warnings":[]}]}

-V text multiple schema validation issues:

Tue Sep 10 11:34:54 2024: Validating rules file(s):
Tue Sep 10 11:34:54 2024:    ../rules/falco_rules.yaml
../rules/falco_rules.yaml: Ok
4 schema warnings: [failed for <root>[1]: Object contains a property that could not be validated using 'properties' or 'additionalProperties' constraints: 'vefgg'. failed for <root>: Failed to validate item #1 in array. failed for <root>[2]: Object contains a property that could not be validated using 'properties' or 'additionalProperties' constraints: 'foo'. failed for <root>: Failed to validate item #2 in array.]

-V text single schema validation issue:

Tue Sep 10 11:33:29 2024: Validating rules file(s):
Tue Sep 10 11:33:29 2024:    ../rules/falco_rules.yaml
../rules/falco_rules.yaml: Ok
2 schema warnings: [failed for <root>[1]: Object contains a property that could not be validated using 'properties' or 'additionalProperties' constraints: 'vefgg'. failed for <root>: Failed to validate item #1 in array.]

-V text ok schema validation:

Tue Sep 10 11:35:28 2024: Validating rules file(s):
Tue Sep 10 11:35:28 2024:    ../rules/falco_rules.yaml
../rules/falco_rules.yaml: Ok

NOTE: in normal run, while validating config and rule files, only foremost warning will be reported, example:

Tue Sep 10 11:52:36 2024: Falco version: 0.39.0-96+272986c (x86_64)
Tue Sep 10 11:52:36 2024: Falco initialized with configuration files:
Tue Sep 10 11:52:36 2024:    ../falco.yaml | schema validation: ok
Tue Sep 10 11:52:36 2024: System info: Linux version 6.8.0-41-generic (buildd@lcy02-amd64-100) (x86_64-linux-gnu-gcc-13 (Ubuntu 13.2.0-23ubuntu4) 13.2.0, GNU ld (GNU Binutils for Ubuntu) 2.42) #41-Ubuntu SMP PREEMPT_DYNAMIC Fri Aug  2 20:41:06 UTC 2024
Tue Sep 10 11:52:36 2024: Loading rules from:
Tue Sep 10 11:52:36 2024:    ../rules/falco_rules.yaml | schema validation: ok

FedeDP · 2024-09-10T10:58:21Z

/unhold

FedeDP · 2024-09-10T10:58:28Z

/cc @jasondellaluce

FedeDP · 2024-09-10T11:46:54Z

/hold

…ollowing errors and warnings output layout. Signed-off-by: Federico Di Pierro <nierro92@gmail.com>

FedeDP · 2024-09-10T11:57:55Z

/unhold

Andreagit97

/approve

poiana · 2024-09-11T11:19:49Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Andreagit97, FedeDP

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [Andreagit97,FedeDP]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

poiana added kind/feature release-note dco-signoff: yes area/engine approved labels Sep 6, 2024

poiana requested review from Kaizhe and mstemm September 6, 2024 12:59

poiana added the size/L label Sep 6, 2024

poiana added the do-not-merge/hold label Sep 6, 2024

poiana added this to the 0.39.0 milestone Sep 6, 2024

FedeDP commented Sep 6, 2024

View reviewed changes

FedeDP mentioned this pull request Sep 6, 2024

falcoSchema.json should be generated by Falco itself #3149

Closed

FedeDP commented Sep 6, 2024

View reviewed changes

FedeDP force-pushed the new/rule_json_schema branch from a681024 to 08aef99 Compare September 6, 2024 14:20

poiana removed the do-not-merge/hold label Sep 9, 2024

leogr reviewed Sep 9, 2024

View reviewed changes

userspace/engine/falco_engine.cpp Outdated Show resolved Hide resolved

poiana added the do-not-merge/hold label Sep 9, 2024

FedeDP added 2 commits September 10, 2024 09:35

new(userspace): added json schema validation for rules.

9f1d123

Also, a new `--rule-schema` cli option was added to print the schema and leave. Signed-off-by: Federico Di Pierro <nierro92@gmail.com>

cleanup(userspace,unit_tests): moved rule schema under engine.

0054a6f

Also, moved yaml_helper under engine/ folder. Ported rule json schema validation in the engine. Also, updated rule_loader tests to check for validation. Signed-off-by: Federico Di Pierro <nierro92@gmail.com>

FedeDP and others added 3 commits September 10, 2024 09:35

cleanup(userspace): drop unused includes from yaml_helper.

0321085

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

chore(userspace/falco): reverted file to master version.

9ab6e35

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

update(userspace/engine): fixed priorities in rules schema.

e413976

Signed-off-by: Federico Di Pierro <nierro92@gmail.com> Co-authored-by: Leonardo Grasso <me@leonardograsso.com>

FedeDP force-pushed the new/rule_json_schema branch from d9377cb to 272986c Compare September 10, 2024 07:35

FedeDP added 2 commits September 10, 2024 10:53

chore(userspace): added schema validation info to `rule_loader::resul…

482ea97

…t` `as_json` and `as_string` outputs. Signed-off-by: Federico Di Pierro <nierro92@gmail.com>

chore(userspace,unit_tests): properly report all schema validation wa…

6630176

…rnings from yaml_helper::validate_node(). `-V` option will print all warnings, while normal run will only print foremost warning. Signed-off-by: Federico Di Pierro <nierro92@gmail.com>

FedeDP force-pushed the new/rule_json_schema branch from 272986c to 6630176 Compare September 10, 2024 09:50

poiana added size/XL and removed size/L labels Sep 10, 2024

leogr previously approved these changes Sep 10, 2024

View reviewed changes

poiana assigned leogr Sep 10, 2024

poiana added the lgtm label Sep 10, 2024

poiana removed the do-not-merge/hold label Sep 10, 2024

poiana requested a review from jasondellaluce September 10, 2024 10:58

poiana added the do-not-merge/hold label Sep 10, 2024

chore(userspace): adjusted rule_loader::result::as_verbose_string f…

054c091

…ollowing errors and warnings output layout. Signed-off-by: Federico Di Pierro <nierro92@gmail.com>

FedeDP dismissed leogr’s stale review via 054c091 September 10, 2024 11:57

poiana removed the do-not-merge/hold label Sep 10, 2024

leogr approved these changes Sep 10, 2024

View reviewed changes

Andreagit97 approved these changes Sep 11, 2024

View reviewed changes

poiana assigned Andreagit97 Sep 11, 2024

poiana merged commit 0f26e3c into master Sep 11, 2024
35 checks passed

poiana deleted the new/rule_json_schema branch September 11, 2024 11:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

new(userspace/falco,userspace/engine): rule json schema validation #3313

new(userspace/falco,userspace/engine): rule json schema validation #3313

FedeDP commented Sep 6, 2024 •

edited

Loading

github-actions bot commented Sep 6, 2024

FedeDP commented Sep 6, 2024

FedeDP commented Sep 6, 2024 •

edited

Loading

FedeDP commented Sep 6, 2024

FedeDP Sep 6, 2024

FedeDP Sep 6, 2024

FedeDP Sep 6, 2024

FedeDP Sep 6, 2024

FedeDP Sep 6, 2024

FedeDP Sep 6, 2024

leogr commented Sep 9, 2024

leogr commented Sep 9, 2024

FedeDP commented Sep 9, 2024

FedeDP commented Sep 10, 2024

FedeDP commented Sep 10, 2024

FedeDP commented Sep 10, 2024

FedeDP commented Sep 10, 2024

FedeDP commented Sep 10, 2024

FedeDP commented Sep 10, 2024

Andreagit97 left a comment

poiana commented Sep 11, 2024

		@@ -47,6 +47,8 @@ limitations under the License.

		const std::string falco_engine::s_default_ruleset = "falco-default-ruleset";

		static const std::string rule_schema_string = R"({"$schema":"http://json-schema.org/draft-06/schema#","type":"array","items":{"anyOf":[{"$ref":"#definitions/RequiredEngineVersionRef"},{"$ref":"#definitions/ListRef"},{"$ref":"#definitions/MacroRef"},{"$ref":"#definitions/RuleRef"},{"$ref":"#definitions/AppendOnlyRuleRef"},{"$ref":"#definitions/EnableOnlyRuleRef"}]},"definitions":{"RequiredEngineVersionRef":{"type":"object","additionalProperties":false,"properties":{"required_engine_version":{"type":"integer"}},"required":["required_engine_version"],"title":"RequiredEngineVersion"},"ListRef":{"type":"object","additionalProperties":false,"properties":{"list":{"type":"string"},"items":{"type":"array","items":{"anyOf":[{"type":"integer"},{"type":"string"}]}},"append":{"type":"boolean"}},"required":["list","items"],"title":"List"},"MacroRef":{"type":"object","additionalProperties":false,"properties":{"macro":{"type":"string"},"condition":{"type":"string"},"append":{"type":"boolean"}},"required":["macro","condition"],"title":"Macro"},"RuleRef":{"type":"object","additionalProperties":false,"properties":{"rule":{"type":"string"},"condition":{"type":"string"},"desc":{"type":"string"},"enabled":{"type":"boolean"},"append":{"type":"boolean"},"output":{"type":"string"},"priority":{"type":"string","enum":["ALERT","EMERGENCY","NOTICE","WARNING","ERROR","DEBUG","INFO","INFORMATIONAL","CRITICAL"]},"tags":{"type":"array","items":{"anyOf":[{"type":"integer"},{"type":"string"}]}},"warn_evttypes":{"type":"boolean"},"skip-if-unknown-filter":{"type":"boolean"}},"required":["rule","desc","condition","output","priority"],"title":"Rule"},"AppendOnlyRuleRef":{"type":"object","additionalProperties":false,"properties":{"rule":{"type":"string"},"condition":{"type":"string"},"append":{"type":"boolean"}},"required":["rule","append","condition"],"title":"AppendOnlyRule"},"EnableOnlyRuleRef":{"type":"object","additionalProperties":false,"properties":{"rule":{"type":"string"},"enabled":{"type":"boolean"}},"required":["rule","enabled"],"title":"EnableOnlyRule"}}})";

new(userspace/falco,userspace/engine): rule json schema validation #3313

new(userspace/falco,userspace/engine): rule json schema validation #3313

Conversation

FedeDP commented Sep 6, 2024 • edited Loading

github-actions bot commented Sep 6, 2024

FedeDP commented Sep 6, 2024

FedeDP commented Sep 6, 2024 • edited Loading

FedeDP commented Sep 6, 2024

FedeDP Sep 6, 2024

Choose a reason for hiding this comment

FedeDP Sep 6, 2024

Choose a reason for hiding this comment

FedeDP Sep 6, 2024

Choose a reason for hiding this comment

FedeDP Sep 6, 2024

Choose a reason for hiding this comment

FedeDP Sep 6, 2024

Choose a reason for hiding this comment

FedeDP Sep 6, 2024

Choose a reason for hiding this comment

leogr commented Sep 9, 2024

leogr commented Sep 9, 2024

FedeDP commented Sep 9, 2024

FedeDP commented Sep 10, 2024

FedeDP commented Sep 10, 2024

FedeDP commented Sep 10, 2024

FedeDP commented Sep 10, 2024

FedeDP commented Sep 10, 2024

FedeDP commented Sep 10, 2024

Andreagit97 left a comment

Choose a reason for hiding this comment

poiana commented Sep 11, 2024

FedeDP commented Sep 6, 2024 •

edited

Loading

FedeDP commented Sep 6, 2024 •

edited

Loading