[filters] Add RBAC condition fuzzer (CEL expressions)#9110
[filters] Add RBAC condition fuzzer (CEL expressions)#9110htuch merged 10 commits intoenvoyproxy:masterfrom
Conversation
Signed-off-by: Asra Ali <asraa@google.com>
Signed-off-by: Asra Ali <asraa@google.com>
Signed-off-by: Asra Ali <asraa@google.com>
Signed-off-by: Asra Ali <asraa@google.com>
Signed-off-by: Asra Ali <asraa@google.com>
Signed-off-by: Asra Ali <asraa@google.com>
htuch
left a comment
There was a problem hiding this comment.
This is really cool. A few point comments, looks like the right way to model this problem.
/wait
| Protobuf::Arena arena; | ||
| Expr::evaluate(*expr, nullptr, stream_info, &request_headers, &response_headers, | ||
| &response_trailers); | ||
| } catch (const EnvoyException& e) { |
There was a problem hiding this comment.
Does this only happen during proto validation? If so, maybe move Expr::evaluate outside the try/catch, to allow the fuzzer to discover any surprise evaluation exceptions.
There was a problem hiding this comment.
Ahh... it's an awkward case. The CEL expressions that are fuzzed have some required proto constraints that aren't enforced in proto validations, so they'll get caught in the code by an exception (for example, an expr_kind must be set).
There was a problem hiding this comment.
Can we add a dedicated EnvoyException subclass for this and do a narrower catch around that?
There was a problem hiding this comment.
Done! We throw CelException's for errors thrown in the CEL library
Signed-off-by: Asra Ali <asraa@google.com>
htuch
left a comment
There was a problem hiding this comment.
LGTM modulo the one question around making the exception catch more precise.
|
Thanks! @TristonianJones FYI. |
Signed-off-by: Asra Ali <asraa@google.com>
Signed-off-by: Asra Ali <asraa@google.com>
This adds a fuzzer for CEL expression matching. These conditions are used in the RBAC filter for complementing the existing principal/permission model.
About a quarter of the execution time is coming from google::api::expr::runtime::RegisterBuiltinFunctions. The test runs locally at about 250 exec/sec.
See: #7716
Risk Level: Low
Testing: Converted unit tests into corpus entries
Re-open of #8752 -- all history the same, merged to up to date master, and pushed all changes.
/review @htuch