Enable metadata for Network::RBAC#5106
Conversation
Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com>
|
/retest |
|
🔨 rebuilding |
Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com>
…data Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com>
|
/retest |
|
🔨 rebuilding |
|
cc @yangminzhu / @quanjielin |
Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com>
|
/retest |
|
🔨 rebuilding |
liminw
left a comment
There was a problem hiding this comment.
Thanks for the change. It looks good to me overall. One comment regarding shadow rules evaluation.
| // calls to onData() could just use the cached result and no need to increase the stats anymore. | ||
| // When the enforcement type is continuous always do the RBAC checks. If its connect time check | ||
| // only, run the check once and skip for subsequent onData calls. | ||
| if (config_->enforcementType() == envoy::config::filter::network::rbac::v2::RBAC::CONTINUOUS) { |
There was a problem hiding this comment.
In the continuous case, shadow rules should stop evaluation if the result is DENY:
if (config_->enforcementType() == envoy::config::filter::network::rbac::v2::RBAC::CONTINUOUS) {
if (shadow_engine_result_ != DENY) {
shadow_engine_result_ = checkEngine(Filters::Common::RBAC::EnforcementMode::Shadow);
}
engine_result_ = checkEngine(Filters::Common::RBAC::EnforcementMode::Enforced);
}
...
There was a problem hiding this comment.
Would you mind to clarify more about why should we stop the shadow rule evaluation if the result is DENY?
The metadata generated by other filters could be different in following onData() calls which means the shadow rule evaluation could also have different results for each onData() call.
There was a problem hiding this comment.
Thats a good point.. Unlike the real rule that terminates the connection, SHADOW rules simply spit out stats about whether each query was allowed or denied..
| // establishment to upstream. When used in conjunction with filters that emit | ||
| // dynamic metadata after decoding a payload (e.g., Mongo, MySQL, Kafka) set the | ||
| // enforcement type to CONTINUOUS. | ||
| EnforcementType enforcement_type = 4; |
There was a problem hiding this comment.
A general question: how this work with allow-list type of rules? i.e. during the connection establishment the connection doesn't have enough metadata, but after decoding it will gain the access, won't the connection be dropped during connection establishment? Definitely worth more comment.
| // calls to onData() could just use the cached result and no need to increase the stats anymore. | ||
| // When the enforcement type is continuous always do the RBAC checks. If its connect time check | ||
| // only, run the check once and skip for subsequent onData calls. | ||
| if (config_->enforcementType() == envoy::config::filter::network::rbac::v2::RBAC::CONTINUOUS) { |
There was a problem hiding this comment.
Would you mind to clarify more about why should we stop the shadow rule evaluation if the result is DENY?
The metadata generated by other filters could be different in following onData() calls which means the shadow rule evaluation could also have different results for each onData() call.
|
/assign @mattklein123 |
Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com>
…data Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com>
|
/retest |
|
🔨 rebuilding |
| // TODO(quanlin): Pass the shadow engine results to other filters. | ||
| // Only check the engine and increase stats for the first time call to onData(), any following | ||
| // calls to onData() could just use the cached result and no need to increase the stats anymore. | ||
| // When the enforcement type is continuous always do the RBAC checks. If its connect time check |
|
/retest |
|
🔨 rebuilding |
Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com> Signed-off-by: Fred Douglas <fredlas@google.com>
Description: PR #4910 missed removing validation constraints that PR #4910 forgot to address. Also fixed API docs to indicate that metadata matching is supported for network level RBAC. Since metadata can be generated during every onData call by a codec (e.g., mongo/mysql/kafka), the RBAC network filter implementation is also modified to use a new (user configured) enforcement type (continuous). When the enforcement type is set to continuous, policy checks will be performed at every onData call.
Risk Level: Low
Testing: Unit tests
Docs Changes: API
Release Notes:
Signed-off-by: Shriram Rajagopalan shriramr@vmware.com