diff --git a/test/falco_tests.yaml b/test/falco_tests.yaml index 2cb6075a..aecc1a9a 100644 --- a/test/falco_tests.yaml +++ b/test/falco_tests.yaml @@ -242,8 +242,9 @@ trace_files: !mux exit_status: 1 stdout_is: |+ Rules content is not yaml + --- This is not yaml - ^ + --- validate_rules_file: - rules/invalid_not_yaml.yaml trace_file: trace_files/cat_write.scap @@ -252,8 +253,9 @@ trace_files: !mux exit_status: 1 stdout_is: |+ Rules content is not yaml array of objects + --- foo: bar - ^ + --- validate_rules_file: - rules/invalid_not_array.yaml trace_file: trace_files/cat_write.scap @@ -262,8 +264,9 @@ trace_files: !mux exit_status: 1 stdout_is: |+ Unexpected element of type string. Each element should be a yaml associative array. + --- - foo - ^ + --- validate_rules_file: - rules/invalid_array_item_not_object.yaml trace_file: trace_files/cat_write.scap @@ -272,8 +275,9 @@ trace_files: !mux exit_status: 1 stdout_is: |+ Unknown rule object: {foo="bar"} + --- - foo: bar - ^ + --- validate_rules_file: - rules/invalid_unexpected_object.yaml trace_file: trace_files/cat_write.scap @@ -282,8 +286,9 @@ trace_files: !mux exit_status: 1 stdout_is: |+ Value of required_engine_version must be a number + --- - required_engine_version: not-a-number - ^ + --- validate_rules_file: - rules/invalid_engine_version_not_number.yaml trace_file: trace_files/cat_write.scap @@ -292,8 +297,9 @@ trace_files: !mux exit_status: 1 stdout_is: |+ mapping values are not allowed in this context + --- this : is : not : yaml - ^ + --- validate_rules_file: - rules/invalid_yaml_parse_error.yaml trace_file: trace_files/cat_write.scap @@ -302,8 +308,10 @@ trace_files: !mux exit_status: 1 stdout_is: |+ List must have property items + --- - list: bad_list - ^ + no_items: foo + --- validate_rules_file: - rules/invalid_list_without_items.yaml trace_file: trace_files/cat_write.scap @@ -312,8 +320,10 @@ trace_files: !mux exit_status: 1 stdout_is: |+ Macro must have property condition + --- - macro: bad_macro - ^ + nope: 1 + --- validate_rules_file: - rules/invalid_macro_without_condition.yaml trace_file: trace_files/cat_write.scap @@ -322,8 +332,12 @@ trace_files: !mux exit_status: 1 stdout_is: |+ Rule must have property output + --- - rule: no output rule - ^ + desc: some desc + condition: evt.type=fork + priority: INFO + --- validate_rules_file: - rules/invalid_rule_without_output.yaml trace_file: trace_files/cat_write.scap @@ -332,8 +346,10 @@ trace_files: !mux exit_status: 1 stdout_is: |+ Rule must have property condition + --- - rule: no condition rule - ^ + append: true + --- validate_rules_file: - rules/invalid_append_rule_without_condition.yaml trace_file: trace_files/cat_write.scap @@ -342,8 +358,11 @@ trace_files: !mux exit_status: 1 stdout_is: |+ Macro dangling append has 'append' key but no macro by that name already exists + --- - macro: dangling append - ^ + condition: and evt.type=execve + append: true + --- validate_rules_file: - rules/invalid_append_macro_dangling.yaml trace_file: trace_files/cat_write.scap @@ -352,8 +371,11 @@ trace_files: !mux exit_status: 1 stdout_is: |+ List my_list has 'append' key but no list by that name already exists + --- - list: my_list - ^ + items: [not-cat] + append: true + --- validate_rules_file: - rules/list_append_failure.yaml trace_file: trace_files/cat_write.scap @@ -362,8 +384,11 @@ trace_files: !mux exit_status: 1 stdout_is: |+ Rule my_rule has 'append' key but no rule by that name already exists + --- - rule: my_rule - ^ + condition: evt.type=open + append: true + --- validate_rules_file: - rules/rule_append_failure.yaml trace_file: trace_files/cat_write.scap @@ -372,8 +397,12 @@ trace_files: !mux exit_status: 1 stdout_is: |+ Rule name is empty + --- - rule: - ^ + desc: some desc + condition: evt.type=execve + output: some output + --- validate_rules_file: - rules/invalid_missing_rule_name.yaml trace_file: trace_files/cat_write.scap @@ -382,8 +411,10 @@ trace_files: !mux exit_status: 1 stdout_is: |+ List name is empty + --- - list: - ^ + items: [foo] + --- validate_rules_file: - rules/invalid_missing_list_name.yaml trace_file: trace_files/cat_write.scap @@ -392,8 +423,10 @@ trace_files: !mux exit_status: 1 stdout_is: |+ Macro name is empty + --- - macro: - ^ + condition: evt.type=execve + --- validate_rules_file: - rules/invalid_missing_macro_name.yaml trace_file: trace_files/cat_write.scap diff --git a/userspace/engine/lua/rule_loader.lua b/userspace/engine/lua/rule_loader.lua index 6c91842e..6e1698f4 100644 --- a/userspace/engine/lua/rule_loader.lua +++ b/userspace/engine/lua/rule_loader.lua @@ -179,29 +179,70 @@ function table.tostring( tbl ) return "{" .. table.concat( result, "," ) .. "}" end -function find_indices(rules_content) - -- To provide context when iterating over the objects in the list, - -- record the line numbers of all top-level objects in the - -- file. This doesn't actually use a yaml parser but simply looks - -- for lines starting with '-'. +-- Split rules_content by lines and also remember the line numbers for +-- each top -level object. Returns a table of lines and a table of +-- line numbers for objects. + +function split_lines(rules_content) + lines = {} indices = {} - line = 1 - if string.sub(rules_content, 1, 1) == '-' then - indices[#indices+1] = line - end - + idx = 1 + last_pos = 1 pos = string.find(rules_content, "\n", 1, true) while pos ~= nil do - line = line + 1 - if string.sub(rules_content, pos+1, pos+1) == '-' then - indices[#indices+1] = line + line = string.sub(rules_content, last_pos, pos-1) + if line ~= "" then + lines[#lines+1] = line + if string.sub(line, 1, 1) == '-' then + indices[#indices+1] = idx + end + + idx = idx + 1 end + + last_pos = pos+1 pos = string.find(rules_content, "\n", pos+1, true) end - return indices + if last_pos < string.len(rules_content) then + line = string.sub(rules_content, last_pos) + lines[#lines+1] = line + if string.sub(line, 1, 1) == '-' then + indices[#indices+1] = idx + end + + idx = idx + 1 + end + + -- Add a final index for last line in document + indices[#indices+1] = idx + + return lines, indices +end + +function get_context(rules_lines, row, num_lines) + + local ret = "---\n" + + idx = row + while (idx < (row + num_lines) and idx <= #rules_lines) do + ret = ret..rules_lines[idx].."\n" + idx = idx + 1 + end + + ret = ret.."---" + + return ret + +end + +function build_error(rules_lines, row, num_lines, err) + + local ret = err.."\n"..get_context(rules_lines, row, num_lines) + + return ret end function load_rules(sinsp_lua_parser, @@ -216,6 +257,8 @@ function load_rules(sinsp_lua_parser, local required_engine_version = 0 + local lines, indices = split_lines(rules_content) + local status, rules = pcall(yaml.load, rules_content) if status == false then @@ -230,7 +273,10 @@ function load_rules(sinsp_lua_parser, rules = string.gsub(rules, pat, "") end - return false, row, col, rules + row = tonumber(row) + col = tonumber(col) + + return false, build_error(lines, row, 3, rules) end if rules == nil then @@ -239,19 +285,17 @@ function load_rules(sinsp_lua_parser, end if type(rules) ~= "table" then - return false, 1, 1, "Rules content is not yaml" + return false, build_error(lines, 1, 1, "Rules content is not yaml") end -- Look for non-numeric indices--implies that document is not array -- of objects. for key, val in pairs(rules) do if type(key) ~= "number" then - return false, 1, 1, "Rules content is not yaml array of objects" + return false, build_error(lines, 1, 1, "Rules content is not yaml array of objects") end end - indices = find_indices(rules_content) - -- Iterate over yaml list. In this pass, all we're doing is -- populating the set of rules, macros, and lists. We're not -- expanding/compiling anything yet. All that will happen in a @@ -259,23 +303,23 @@ function load_rules(sinsp_lua_parser, for i,v in ipairs(rules) do if (not (type(v) == "table")) then - return false, indices[i], 1, "Unexpected element of type " ..type(v)..". Each element should be a yaml associative array." + return false, build_error(lines, indices[i], (indices[i+1]-indices[i]), "Unexpected element of type " ..type(v)..". Each element should be a yaml associative array.") end if (v['required_engine_version']) then required_engine_version = v['required_engine_version'] if type(required_engine_version) ~= "number" then - return false, indices[i], 1, "Value of required_engine_version must be a number" + return false, build_error(lines, indices[i], (indices[i+1]-indices[i]), "Value of required_engine_version must be a number") end if falco_rules.engine_version(rules_mgr) < v['required_engine_version'] then - return false, indices[i], 1, "Rules require engine version "..v['required_engine_version']..", but engine version is "..falco_rules.engine_version(rules_mgr) + return false, build_error(lines, indices[i], (indices[i+1]-indices[i]), "Rules require engine version "..v['required_engine_version']..", but engine version is "..falco_rules.engine_version(rules_mgr)) end elseif (v['macro']) then if (v['macro'] == nil or type(v['macro']) == "table") then - return false, indices[i], 3, "Macro name is empty" + return false, build_error(lines, indices[i], (indices[i+1]-indices[i]), "Macro name is empty") end if v['source'] == nil then @@ -288,7 +332,7 @@ function load_rules(sinsp_lua_parser, for j, field in ipairs({'condition'}) do if (v[field] == nil) then - return false, indices[i], 3, "Macro must have property "..field + return false, build_error(lines, indices[i], (indices[i+1]-indices[i]), "Macro must have property "..field) end end @@ -301,7 +345,7 @@ function load_rules(sinsp_lua_parser, if append then if state.macros_by_name[v['macro']] == nil then - return false, indices[i], 1, "Macro " ..v['macro'].. " has 'append' key but no macro by that name already exists" + return false, build_error(lines, indices[i], (indices[i+1]-indices[i]), "Macro " ..v['macro'].. " has 'append' key but no macro by that name already exists") end state.macros_by_name[v['macro']]['condition'] = state.macros_by_name[v['macro']]['condition'] .. " " .. v['condition'] @@ -313,7 +357,7 @@ function load_rules(sinsp_lua_parser, elseif (v['list']) then if (v['list'] == nil or type(v['list']) == "table") then - return false, indices[i], 3, "List name is empty" + return false, build_error(lines, indices[i], (indices[i+1]-indices[i]), "List name is empty") end if state.lists_by_name[v['list']] == nil then @@ -322,7 +366,7 @@ function load_rules(sinsp_lua_parser, for j, field in ipairs({'items'}) do if (v[field] == nil) then - return false, indices[i], 3, "List must have property "..field + return false, build_error(lines, indices[i], (indices[i+1]-indices[i]), "List must have property "..field) end end @@ -335,7 +379,7 @@ function load_rules(sinsp_lua_parser, if append then if state.lists_by_name[v['list']] == nil then - return false, indices[i], 1, "List " ..v['list'].. " has 'append' key but no list by that name already exists" + return false, build_error(lines, indices[i], (indices[i+1]-indices[i]), "List " ..v['list'].. " has 'append' key but no list by that name already exists") end for j, elem in ipairs(v['items']) do @@ -348,7 +392,7 @@ function load_rules(sinsp_lua_parser, elseif (v['rule']) then if (v['rule'] == nil or type(v['rule']) == "table") then - return false, indices[i], 3, "Rule name is empty" + return false, build_error(lines, indices[i], (indices[i+1]-indices[i]), "Rule name is empty") end -- By default, if a rule's condition refers to an unknown @@ -373,13 +417,13 @@ function load_rules(sinsp_lua_parser, -- For append rules, all you need is the condition for j, field in ipairs({'condition'}) do if (v[field] == nil) then - return false, indices[i], 3, "Rule must have property "..field + return false, build_error(lines, indices[i], (indices[i+1]-indices[i]), "Rule must have property "..field) end end if state.rules_by_name[v['rule']] == nil then if state.skipped_rules_by_name[v['rule']] == nil then - return false, indices[i], 1, "Rule " ..v['rule'].. " has 'append' key but no rule by that name already exists" + return false, build_error(lines, indices[i], (indices[i+1]-indices[i]), "Rule " ..v['rule'].. " has 'append' key but no rule by that name already exists") end else state.rules_by_name[v['rule']]['condition'] = state.rules_by_name[v['rule']]['condition'] .. " " .. v['condition'] @@ -389,7 +433,7 @@ function load_rules(sinsp_lua_parser, for j, field in ipairs({'condition', 'output', 'desc', 'priority'}) do if (v[field] == nil) then - return false, indices[i], 3, "Rule must have property "..field + return false, build_error(lines, indices[i], (indices[i+1]-indices[i]), "Rule must have property "..field) end end @@ -418,7 +462,7 @@ function load_rules(sinsp_lua_parser, end end else - return false, indices[i], 1, "Unknown rule object: "..table.tostring(v) + return false, build_error(lines, indices[i], (indices[i+1]-indices[i]), "Unknown rule object: "..table.tostring(v)) end end @@ -458,7 +502,7 @@ function load_rules(sinsp_lua_parser, local status, ast = compiler.compile_macro(v['condition'], state.macros, state.lists) if status == false then - return false, indices[i], 1, ast + return false, build_error(lines, indices[i], (indices[i+1]-indices[i]), ast) end if v['source'] == "syscall" then @@ -483,7 +527,7 @@ function load_rules(sinsp_lua_parser, state.macros, state.lists) if status == false then - return false, indices[i], 1, filter_ast + return false, build_error(lines, indices[i], (indices[i+1]-indices[i]), filter_ast) end local evtttypes = {} @@ -621,7 +665,7 @@ function load_rules(sinsp_lua_parser, formatter = formats.formatter(v['source'], v['output']) formats.free_formatter(v['source'], formatter) else - return false, indices[i], 1, "Unexpected type in load_rule: "..filter_ast.type + return false, build_error(lines, indices[i], (indices[i+1]-indices[i]), "Unexpected type in load_rule: "..filter_ast.type) end ::next_rule:: diff --git a/userspace/engine/rules.cpp b/userspace/engine/rules.cpp index da2a5f2d..610c6230 100644 --- a/userspace/engine/rules.cpp +++ b/userspace/engine/rules.cpp @@ -207,31 +207,6 @@ void falco_rules::enable_rule(string &rule, bool enabled) m_engine->enable_rule(rule, enabled); } -std::string falco_rules::get_context(const std::string &content, uint64_t lineno, uint64_t colno) -{ - istringstream ctx(content); - std::string line; - std::string ret; - - for(uint32_t i=1; ctx && i<=lineno; i++) - { - getline(ctx, line); - if(i == lineno) - { - ret += line + "\n"; - break; - } - } - - for(uint32_t j=1; j= 0, use them to - // extract meaninful context from the result. - if(line >= 1 && col >= 1) - { - err += "\n"; - err += get_context(rules_content, line, col); - } - throw falco_exception(err); }