This fix has two major points in it:
- when `std::stoll` is used in parse_as_int64 handle all the exceptions it
can throw (https://en.cppreference.com/w/cpp/string/basic_string/stol)
- when `process_k8s_audit_event` an eventual exception in it does not
stop the webserver process. This is done by doing a catch all handle
outside it and by logging an error message to the caller as well as in
stderr
Signed-off-by: Lorenzo Fontana <lo@linux.com>
We want users to continue using rules without having to use exceptions.
Exceptions are an additional feature for more advanced use-cases, having
a warning in there will mean that everyone now adds an empty exception
to avoid the warning.
Co-Authored-By: Leonardo Grasso <me@leonardograsso.com>
Signed-off-by: Lorenzo Fontana <lo@linux.com>
While testing, I found a case when creating a pod where:
1) the first container had no securityContext value
2) the second container had a security context with privileged=true
and this did not match the default rule Create Privileged Pod, when it
should match.
The rule Create Privileged Pod uses the field
ka.req.pod.containers.privileged, which in turn uses
json_event_filter_check::def_extract(). def_extract() iterates
over a set of json_pointers, potentially expanding arrays as they are
returned. Many k8s audit fields use this extract function.
For ka.req.pod.containers.privileged, the first json_pointer is
/requestObject/spec/containers to find the list of containers, and the
second is /securityContext/privileged to extract the privileged property
out of the securityContext object. What's returned is an array of
true/false noting if each container is privileged.
The problem is that def_extract() aborts when iterating over arrays if
extracting a pointer from an array can't be done.
In this case, the first pointer extracts the array of containers, and
then when iterating over the array of containers, the security context
pointer doesn't extract, causing the whole filter field to abort and
return ::no_value.
The fix is to not abort when iterating over arrays, but use ::no_value
for that array item's value instead. This allows def_extract() to
extract the privileged value out of the second container.
Signed-off-by: Mark Stemm <mark.stemm@gmail.com>
Falco won't properly parse a rule like this:
---
- rule: Some Rule
desc: Some Desc
condition: evt.type=execve and container.image.repository = 271931939120.dkr
output: Some output
priority: INFO
---
This is the error when validating the rules:
Tue Mar 30 12:00:40 2021: Validating rules file(s):
Tue Mar 30 12:00:40 2021: /home/mstemm/test.yaml
1 errors:
Compilation error when compiling "evt.type=execve and container.image.repository = 271931939120.dkr": 63: syntax error, unexpected 'dkr', expecting 'or', 'and'
The parsing of the string on the right hand side stops at the period
before the dkr. The dkr then doesn't match the grammar, resulting in the
error.
Looking at the parser implementation more closely, the problem is in the
definition of "Number":
---
- Number = C(V "Hex" + V "Float" + V "Int") / function(n)
return tonumber(n)
end,
---
Note that it stops after the number, but does not have any requirement
about what follows.
This changes the definition of number to require that what follows the
number is not an identifier character. With this change, values that are
only numbers are parsed as numbers, and values that start with numbers
don't match the Number definition and are parsed as BareStrings instead.
Signed-off-by: Mark Stemm <mark.stemm@gmail.com>
When returning a rule_result struct, also include a set of field names
used by all exceptions for this rule. This may make building exception
values a bit easier.
Signed-off-by: Mark Stemm <mark.stemm@gmail.com>
If a list:
- list: foo
items: [a, b, c]
Was referenced in another list:
- list: bar
items: [foo, d, e, f]
The first list would not be marked as used, when it should.
This avoids mistaken messages like "list xxx not refered to by any rule/macro/list"
Signed-off-by: Mark Stemm <mark.stemm@gmail.com>
Support exceptions properties on rules as described in
https://github.com/falcosecurity/falco/pull/1376.
- When parsing rules, add an empty exceptions table if not specified.
- If exceptions are specified, they must contain names and lists of
fields, and optionally can contain lists of comps and lists of lists of
values.
- If comps are not specified, = is used.
- If a rule has exceptions and append:true, add values to the original rule's
exception values with the matching name.
- It's a warning but not an error to have exception values with a name
not matching any fields.
After loading all rules, build the exception condition string based on
any exceptions:
- If an exception has a single value for the "fields" property, values are
combined into a single set to build a condition string like "field
cmp (val1, val2, ...)".
- Otherwise, iterate through each rule's exception
values, finding the matching field names (field1, field2, ...) and
comp operators (cmp1, cmp2, ...), then
iterating over the list of field values (val1a, val1b, ...), (val2a,
val2b, ...), building up a string of the form:
and not ((field1 cmp1 val1a and field2 cmp2 val1b and ...) or
(field1 cmp1 val2a and field2 cmp2 val2b and ...)...
)"
- If a value is not already quoted and contains a space, quote it in the
string.
Signed-off-by: Mark Stemm <mark.stemm@gmail.com>
When parsing a rules file, if a top level object is not one of the known
types rule, macro, list, required_engine_version, instead of failing
parsing, add a warning instead.
This adds some forwards-compatibility to rules files.
Signed-off-by: Mark Stemm <mark.stemm@gmail.com>
Add the notion of warnings when loading rules, which are printed if
verbose is true:
- load_rules now returns a tuple (success, required engine version,
error array, warnings array) instead of (true, required engine
version) or (false, error string)
- build_error/build_error_with_context now returns an array instead of
string value.
- warnings are combined across calls to load_rules_doc
- Current warnings include:
- a rule that contains an unknown filter
- a macro not referred to by any rule
- a list not referred to by any rule/macro/list
Any errors/warnings are concatenated into the exception if success was
false. Any errors/warnings will be printed if verbose is true.
Signed-off-by: Mark Stemm <mark.stemm@gmail.com>
This is needed because Luajit does not support many architectures
such as aarch64 and ppcle64.
Note: some operating systems, such as Alpine, already use moonjit as a dropin
replacement for luajit.
Signed-off-by: Lorenzo Fontana <fontanalorenz@gmail.com>
Previously, formatters were freed by LUA code when re-opening outputs.
Since now, outputs are not controlling anymore the falco_formats class (see #1412), we just free formatters only if were already initialized.
That is needed when the engine restarts (see #1446).
By doing so, we also ensure that correct inspector instance is set to the formatter cache.
Signed-off-by: Leonardo Grasso <me@leonardograsso.com>
Currently, when calling enable_rule, the provided rule name pattern is a
substring match, that is if the rules file has a rule "My fantastic
rule", and you call engine->enable_rule("fantastic", true), the rule
will be enabled.
This can cause problems if one rule name is a complete subset of another
rule name e.g. rules "My rule" and "My rule is great", and calling
engine->enable_rule("My rule", true).
To allow for this case, add an alternate method enable_rule_exact() in
both default ruleset and ruleset variants. In this case, the rule name
must be an exact match.
In the underlying ruleset code, add a "match_exact" option to
falco_ruleset::enable() that denotes whether the substring is an exact
or substring match.
This doesn't change the default behavior of falco in any way, as the
existing calls still use enable_rule().
Signed-off-by: Mark Stemm <mark.stemm@gmail.com>
BAN_ALTERNATIVE is same as BAN but the message also provides an alternative
function that the user could use instead of the banned function.
Fixes#1035
Signed-off-by: Vaibhav <vrongmeal@gmail.com>
These include:
* vsprintf()
* sprintf()
* strcat()
* strncat()
* strncpy()
* swprintf()
* vswprintf()
This also changes `userspace/falco/logger.cpp` to remove a `sprintf`
statement. The statement did not affect the codebase in any form so
it was simply removed rather than being substituted.
Fixes#1035
Signed-off-by: Vaibhav <vrongmeal@gmail.com>
This defines certain functions as invalid tokens, i.e., when
compiled, the compiler throws an error.
Currently only `strcpy` is included as a banned function.
Fixes#788
Signed-off-by: Vaibhav <vrongmeal@gmail.com>
Callers aren't expected to catch execeptions and instead rely on the
bool return value to indicate whether or not the parsing was successful.
Signed-off-by: Mark Stemm <mark.stemm@gmail.com>
Currently, the json object POSTed to the /k8s_audit endpoint is assumed
to be an obect, with a "type" of either "Event" or "EventList". When the
K8s API Server POSTs events, it aggregates them into an EventList,
ensuring that there is always a single object.
However, we're going to add some intermediate tools that tail log files
and send them to the endpoint, and the easiest way to send a batch of
events is to pass them as a json array instead of a single object.
To properly handle this, modify parse_k8s_audit_event_json to also
handle a json array. For arrays, it iterates over the objects, calling
parse_k8s_audit_json recursively. This only iterates an initial top
level array to avoid excessive recursion/attacks involving degenerate
json objects with excessively nested arrays.
Signed-off-by: Mark Stemm <mark.stemm@gmail.com>
In all extraction functions, always catch json type errors alongside
json out of range errors. Both cases result in not extracting any value
from the event.
Signed-off-by: Mark Stemm <mark.stemm@gmail.com>
First of a handful of PRs to start clarifying the independence of Falco
I don't see any breaking changes here, just cosmetic changes.
Signed-off-by: Kris Nova <kris@nivenly.com>
The call to rule_loader.load_rules only returns 2 values, so only pop
two values from the stack. This fixes#906.
Signed-off-by: Mark Stemm <mark.stemm@gmail.com>
As a part of the changes in
https://github.com/falcosecurity/falco/pull/826/, we added several
breaking changes to rules files like renaming/removing some filter
fields. This isn't ideal for customers who are using their own rules
files.
We shouldn't break older rules files in this way, so add some minimal
backwards compatibility which adds back the fields that were
removed *and* actually used in k8s_audit_rules.yaml. They have the same
functionality as before. One exception is
ka.req.binding.subject.has_name, which was only used in a single output
field for debugging and shouldn't have been in the rules file in the
first place. This always returns the string "N/A".
Signed-off-by: Mark Stemm <mark.stemm@gmail.com>
