diff --git a/api/envoy/config/rbac/v3/rbac.proto b/api/envoy/config/rbac/v3/rbac.proto index 10520b1ba38f..278e6857603f 100644 --- a/api/envoy/config/rbac/v3/rbac.proto +++ b/api/envoy/config/rbac/v3/rbac.proto @@ -24,8 +24,14 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE; // [#protodoc-title: Role Based Access Control (RBAC)] // Role Based Access Control (RBAC) provides service-level and method-level access control for a -// service. RBAC policies are additive. The policies are examined in order. A request is allowed -// once a matching policy is found (suppose the `action` is ALLOW). +// service. RBAC policies are additive. The policies are examined in order. Requests are allowed +// or denied based on the `action` and whether a matching policy is found. For instance, if the +// action is ALLOW and a matching policy is found the request should be allowed. +// +// RBAC can also be used to make access logging decisions by communicating with access loggers +// through dynamic metadata. When the action is LOG and at least one policy matches, the +// `access_log_hint` value in the shared key namespace 'envoy.common' is set to `true` indicating +// the request should be logged. // // Here is an example of RBAC configuration. It has two policies: // @@ -68,39 +74,55 @@ message RBAC { // Should we do safe-list or block-list style access control? enum Action { - // The policies grant access to principals. The rest is denied. This is safe-list style + // The policies grant access to principals. The rest are denied. This is safe-list style // access control. This is the default type. ALLOW = 0; - // The policies deny access to principals. The rest is allowed. This is block-list style + // The policies deny access to principals. The rest are allowed. This is block-list style // access control. DENY = 1; + + // The policies set the `access_log_hint` dynamic metadata key based on if requests match. + // All requests are allowed. + LOG = 2; } - // The action to take if a policy matches. The request is allowed if and only if: + // The action to take if a policy matches. Every action either allows or denies a request, + // and can also carry out action-specific operations. + // + // Actions: + // + // * ALLOW: Allows the request if and only if there is a policy that matches + // the request. + // * DENY: Allows the request if and only if there are no policies that + // match the request. + // * LOG: Allows all requests. If at least one policy matches, the dynamic + // metadata key `access_log_hint` is set to the value `true` under the shared + // key namespace 'envoy.common'. If no policies match, it is set to `false`. + // Other actions do not modify this key. // - // * `action` is "ALLOWED" and at least one policy matches - // * `action` is "DENY" and none of the policies match Action action = 1; // Maps from policy name to policy. A match occurs when at least one policy matches the request. map policies = 2; } -// Policy specifies a role and the principals that are assigned/denied the role. A policy matches if -// and only if at least one of its permissions match the action taking place AND at least one of its -// principals match the downstream AND the condition is true if specified. +// Policy specifies a role and the principals that are assigned/denied the role. +// A policy matches if and only if at least one of its permissions match the +// action taking place AND at least one of its principals match the downstream +// AND the condition is true if specified. message Policy { option (udpa.annotations.versioning).previous_message_type = "envoy.config.rbac.v2.Policy"; - // Required. The set of permissions that define a role. Each permission is matched with OR - // semantics. To match all actions for this policy, a single Permission with the `any` field set - // to true should be used. + // Required. The set of permissions that define a role. Each permission is + // matched with OR semantics. To match all actions for this policy, a single + // Permission with the `any` field set to true should be used. repeated Permission permissions = 1 [(validate.rules).repeated = {min_items: 1}]; - // Required. The set of principals that are assigned/denied the role based on “action”. Each - // principal is matched with OR semantics. To match all downstreams for this policy, a single - // Principal with the `any` field set to true should be used. + // Required. The set of principals that are assigned/denied the role based on + // “action”. Each principal is matched with OR semantics. To match all + // downstreams for this policy, a single Principal with the `any` field set to + // true should be used. repeated Principal principals = 2 [(validate.rules).repeated = {min_items: 1}]; // An optional symbolic expression specifying an access control @@ -161,9 +183,9 @@ message Permission { // Metadata that describes additional information about the action. type.matcher.v3.MetadataMatcher metadata = 7; - // Negates matching the provided permission. For instance, if the value of `not_rule` would - // match, this permission would not match. Conversely, if the value of `not_rule` would not - // match, this permission would match. + // Negates matching the provided permission. For instance, if the value of + // `not_rule` would match, this permission would not match. Conversely, if + // the value of `not_rule` would not match, this permission would match. Permission not_rule = 8; // The request server from the client's connection request. This is @@ -176,7 +198,8 @@ message Permission { // // * If the :ref:`TLS Inspector ` // filter is not added, and if a `FilterChainMatch` is not defined for - // the :ref:`server name `, + // the :ref:`server name + // `, // a TLS connection's requested SNI server name will be treated as if it // wasn't present. // @@ -189,13 +212,14 @@ message Permission { } } -// Principal defines an identity or a group of identities for a downstream subject. +// Principal defines an identity or a group of identities for a downstream +// subject. // [#next-free-field: 12] message Principal { option (udpa.annotations.versioning).previous_message_type = "envoy.config.rbac.v2.Principal"; - // Used in the `and_ids` and `or_ids` fields in the `identifier` oneof. Depending on the context, - // each are applied with the associated behavior. + // Used in the `and_ids` and `or_ids` fields in the `identifier` oneof. + // Depending on the context, each are applied with the associated behavior. message Set { option (udpa.annotations.versioning).previous_message_type = "envoy.config.rbac.v2.Principal.Set"; @@ -210,19 +234,21 @@ message Principal { reserved 1; - // The name of the principal. If set, The URI SAN or DNS SAN in that order is used from the - // certificate, otherwise the subject field is used. If unset, it applies to any user that is - // authenticated. + // The name of the principal. If set, The URI SAN or DNS SAN in that order + // is used from the certificate, otherwise the subject field is used. If + // unset, it applies to any user that is authenticated. type.matcher.v3.StringMatcher principal_name = 2; } oneof identifier { option (validate.required) = true; - // A set of identifiers that all must match in order to define the downstream. + // A set of identifiers that all must match in order to define the + // downstream. Set and_ids = 1; - // A set of identifiers at least one must match in order to define the downstream. + // A set of identifiers at least one must match in order to define the + // downstream. Set or_ids = 2; // When any is set, it matches any downstream. @@ -237,21 +263,23 @@ message Principal { // A CIDR block that describes the downstream remote/origin address. // Note: This is always the physical peer even if the - // :ref:`remote_ip ` is inferred - // from for example the x-forwarder-for header, proxy protocol, etc. + // :ref:`remote_ip ` is + // inferred from for example the x-forwarder-for header, proxy protocol, + // etc. core.v3.CidrRange direct_remote_ip = 10; // A CIDR block that describes the downstream remote/origin address. // Note: This may not be the physical peer and could be different from the - // :ref:`direct_remote_ip `. - // E.g, if the remote ip is inferred from for example the x-forwarder-for header, - // proxy protocol, etc. + // :ref:`direct_remote_ip + // `. E.g, if the + // remote ip is inferred from for example the x-forwarder-for header, proxy + // protocol, etc. core.v3.CidrRange remote_ip = 11; - // A header (or pseudo-header such as :path or :method) on the incoming HTTP request. Only - // available for HTTP request. - // Note: the pseudo-header :path includes the query and fragment string. Use the `url_path` - // field if you want to match the URL path without the query and fragment string. + // A header (or pseudo-header such as :path or :method) on the incoming HTTP + // request. Only available for HTTP request. Note: the pseudo-header :path + // includes the query and fragment string. Use the `url_path` field if you + // want to match the URL path without the query and fragment string. route.v3.HeaderMatcher header = 6; // A URL path on the incoming HTTP request. Only available for HTTP. @@ -260,9 +288,9 @@ message Principal { // Metadata that describes additional information about the principal. type.matcher.v3.MetadataMatcher metadata = 7; - // Negates matching the provided principal. For instance, if the value of `not_id` would match, - // this principal would not match. Conversely, if the value of `not_id` would not match, this - // principal would match. + // Negates matching the provided principal. For instance, if the value of + // `not_id` would match, this principal would not match. Conversely, if the + // value of `not_id` would not match, this principal would match. Principal not_id = 8; } } diff --git a/api/envoy/config/rbac/v4alpha/rbac.proto b/api/envoy/config/rbac/v4alpha/rbac.proto index 11b69b16e679..cc9d8933abab 100644 --- a/api/envoy/config/rbac/v4alpha/rbac.proto +++ b/api/envoy/config/rbac/v4alpha/rbac.proto @@ -23,8 +23,14 @@ option (udpa.annotations.file_status).package_version_status = NEXT_MAJOR_VERSIO // [#protodoc-title: Role Based Access Control (RBAC)] // Role Based Access Control (RBAC) provides service-level and method-level access control for a -// service. RBAC policies are additive. The policies are examined in order. A request is allowed -// once a matching policy is found (suppose the `action` is ALLOW). +// service. RBAC policies are additive. The policies are examined in order. Requests are allowed +// or denied based on the `action` and whether a matching policy is found. For instance, if the +// action is ALLOW and a matching policy is found the request should be allowed. +// +// RBAC can also be used to make access logging decisions by communicating with access loggers +// through dynamic metadata. When the action is LOG and at least one policy matches, the +// `access_log_hint` value in the shared key namespace 'envoy.common' is set to `true` indicating +// the request should be logged. // // Here is an example of RBAC configuration. It has two policies: // @@ -67,39 +73,55 @@ message RBAC { // Should we do safe-list or block-list style access control? enum Action { - // The policies grant access to principals. The rest is denied. This is safe-list style + // The policies grant access to principals. The rest are denied. This is safe-list style // access control. This is the default type. ALLOW = 0; - // The policies deny access to principals. The rest is allowed. This is block-list style + // The policies deny access to principals. The rest are allowed. This is block-list style // access control. DENY = 1; + + // The policies set the `access_log_hint` dynamic metadata key based on if requests match. + // All requests are allowed. + LOG = 2; } - // The action to take if a policy matches. The request is allowed if and only if: + // The action to take if a policy matches. Every action either allows or denies a request, + // and can also carry out action-specific operations. + // + // Actions: + // + // * ALLOW: Allows the request if and only if there is a policy that matches + // the request. + // * DENY: Allows the request if and only if there are no policies that + // match the request. + // * LOG: Allows all requests. If at least one policy matches, the dynamic + // metadata key `access_log_hint` is set to the value `true` under the shared + // key namespace 'envoy.common'. If no policies match, it is set to `false`. + // Other actions do not modify this key. // - // * `action` is "ALLOWED" and at least one policy matches - // * `action` is "DENY" and none of the policies match Action action = 1; // Maps from policy name to policy. A match occurs when at least one policy matches the request. map policies = 2; } -// Policy specifies a role and the principals that are assigned/denied the role. A policy matches if -// and only if at least one of its permissions match the action taking place AND at least one of its -// principals match the downstream AND the condition is true if specified. +// Policy specifies a role and the principals that are assigned/denied the role. +// A policy matches if and only if at least one of its permissions match the +// action taking place AND at least one of its principals match the downstream +// AND the condition is true if specified. message Policy { option (udpa.annotations.versioning).previous_message_type = "envoy.config.rbac.v3.Policy"; - // Required. The set of permissions that define a role. Each permission is matched with OR - // semantics. To match all actions for this policy, a single Permission with the `any` field set - // to true should be used. + // Required. The set of permissions that define a role. Each permission is + // matched with OR semantics. To match all actions for this policy, a single + // Permission with the `any` field set to true should be used. repeated Permission permissions = 1 [(validate.rules).repeated = {min_items: 1}]; - // Required. The set of principals that are assigned/denied the role based on “action”. Each - // principal is matched with OR semantics. To match all downstreams for this policy, a single - // Principal with the `any` field set to true should be used. + // Required. The set of principals that are assigned/denied the role based on + // “action”. Each principal is matched with OR semantics. To match all + // downstreams for this policy, a single Principal with the `any` field set to + // true should be used. repeated Principal principals = 2 [(validate.rules).repeated = {min_items: 1}]; oneof expression_specifier { @@ -160,9 +182,9 @@ message Permission { // Metadata that describes additional information about the action. type.matcher.v4alpha.MetadataMatcher metadata = 7; - // Negates matching the provided permission. For instance, if the value of `not_rule` would - // match, this permission would not match. Conversely, if the value of `not_rule` would not - // match, this permission would match. + // Negates matching the provided permission. For instance, if the value of + // `not_rule` would match, this permission would not match. Conversely, if + // the value of `not_rule` would not match, this permission would match. Permission not_rule = 8; // The request server from the client's connection request. This is @@ -175,7 +197,8 @@ message Permission { // // * If the :ref:`TLS Inspector ` // filter is not added, and if a `FilterChainMatch` is not defined for - // the :ref:`server name `, + // the :ref:`server name + // `, // a TLS connection's requested SNI server name will be treated as if it // wasn't present. // @@ -188,13 +211,14 @@ message Permission { } } -// Principal defines an identity or a group of identities for a downstream subject. +// Principal defines an identity or a group of identities for a downstream +// subject. // [#next-free-field: 12] message Principal { option (udpa.annotations.versioning).previous_message_type = "envoy.config.rbac.v3.Principal"; - // Used in the `and_ids` and `or_ids` fields in the `identifier` oneof. Depending on the context, - // each are applied with the associated behavior. + // Used in the `and_ids` and `or_ids` fields in the `identifier` oneof. + // Depending on the context, each are applied with the associated behavior. message Set { option (udpa.annotations.versioning).previous_message_type = "envoy.config.rbac.v3.Principal.Set"; @@ -209,9 +233,9 @@ message Principal { reserved 1; - // The name of the principal. If set, The URI SAN or DNS SAN in that order is used from the - // certificate, otherwise the subject field is used. If unset, it applies to any user that is - // authenticated. + // The name of the principal. If set, The URI SAN or DNS SAN in that order + // is used from the certificate, otherwise the subject field is used. If + // unset, it applies to any user that is authenticated. type.matcher.v4alpha.StringMatcher principal_name = 2; } @@ -222,10 +246,12 @@ message Principal { oneof identifier { option (validate.required) = true; - // A set of identifiers that all must match in order to define the downstream. + // A set of identifiers that all must match in order to define the + // downstream. Set and_ids = 1; - // A set of identifiers at least one must match in order to define the downstream. + // A set of identifiers at least one must match in order to define the + // downstream. Set or_ids = 2; // When any is set, it matches any downstream. @@ -236,21 +262,23 @@ message Principal { // A CIDR block that describes the downstream remote/origin address. // Note: This is always the physical peer even if the - // :ref:`remote_ip ` is inferred - // from for example the x-forwarder-for header, proxy protocol, etc. + // :ref:`remote_ip ` is + // inferred from for example the x-forwarder-for header, proxy protocol, + // etc. core.v4alpha.CidrRange direct_remote_ip = 10; // A CIDR block that describes the downstream remote/origin address. // Note: This may not be the physical peer and could be different from the - // :ref:`direct_remote_ip `. - // E.g, if the remote ip is inferred from for example the x-forwarder-for header, - // proxy protocol, etc. + // :ref:`direct_remote_ip + // `. E.g, if the + // remote ip is inferred from for example the x-forwarder-for header, proxy + // protocol, etc. core.v4alpha.CidrRange remote_ip = 11; - // A header (or pseudo-header such as :path or :method) on the incoming HTTP request. Only - // available for HTTP request. - // Note: the pseudo-header :path includes the query and fragment string. Use the `url_path` - // field if you want to match the URL path without the query and fragment string. + // A header (or pseudo-header such as :path or :method) on the incoming HTTP + // request. Only available for HTTP request. Note: the pseudo-header :path + // includes the query and fragment string. Use the `url_path` field if you + // want to match the URL path without the query and fragment string. route.v4alpha.HeaderMatcher header = 6; // A URL path on the incoming HTTP request. Only available for HTTP. @@ -259,9 +287,9 @@ message Principal { // Metadata that describes additional information about the principal. type.matcher.v4alpha.MetadataMatcher metadata = 7; - // Negates matching the provided principal. For instance, if the value of `not_id` would match, - // this principal would not match. Conversely, if the value of `not_id` would not match, this - // principal would match. + // Negates matching the provided principal. For instance, if the value of + // `not_id` would match, this principal would not match. Conversely, if the + // value of `not_id` would not match, this principal would match. Principal not_id = 8; } } diff --git a/docs/root/configuration/advanced/well_known_dynamic_metadata.rst b/docs/root/configuration/advanced/well_known_dynamic_metadata.rst index 0088a85d9b94..4d7f8ed3872c 100644 --- a/docs/root/configuration/advanced/well_known_dynamic_metadata.rst +++ b/docs/root/configuration/advanced/well_known_dynamic_metadata.rst @@ -27,3 +27,24 @@ The following Envoy filters can be configured to consume dynamic metadata emitte * :ref:`External Authorization Filter via the metadata context namespaces ` * :ref:`RateLimit Filter limit override ` + +.. _shared_dynamic_metadata: + +Shared Dynamic Metadata +----------------------- +Dynamic metadata that is set by multiple filters is placed in the common key namespace `envoy.common`. Refer to the corresponding rules when setting this metadata. + +.. csv-table:: + :header: Name, Type, Description, Rules + :widths: 1, 1, 3, 3 + + access_log_hint, boolean, Whether access loggers should log the request., "When this metadata is already set: A `true` value should not be overwritten by a `false` value, while a `false` value can be overwritten by a `true` value." + +The following Envoy filters emit shared dynamic metadata. + +* :ref:`Role Based Access Control (RBAC) Filter ` +* :ref:`Role Based Access Control (RBAC) Network Filter ` + +The following filters consume shared dynamic metadata. + +* :ref:`Metadata Access Log Filter` diff --git a/docs/root/configuration/http/http_filters/rbac_filter.rst b/docs/root/configuration/http/http_filters/rbac_filter.rst index d6068bbdcc6c..5db112d924ef 100644 --- a/docs/root/configuration/http/http_filters/rbac_filter.rst +++ b/docs/root/configuration/http/http_filters/rbac_filter.rst @@ -36,6 +36,8 @@ owning HTTP connection manager. denied, Counter, Total requests that were denied access shadow_allowed, Counter, Total requests that would be allowed access by the filter's shadow rules shadow_denied, Counter, Total requests that would be denied access by the filter's shadow rules + logged, Counter, Total requests that should be logged + not_logged, Counter, Total requests that should not be logged .. _config_http_filters_rbac_dynamic_metadata: @@ -50,3 +52,4 @@ The RBAC filter emits the following dynamic metadata. shadow_effective_policy_id, string, The effective shadow policy ID matching the action (if any). shadow_engine_result, string, The engine result for the shadow rules (i.e. either `allowed` or `denied`). + access_log_hint, boolean, Whether the request should be logged. This metadata is shared and set under the key namespace 'envoy.common' (See :ref:`Shared Dynamic Metadata`). diff --git a/docs/root/configuration/listeners/network_filters/rbac_filter.rst b/docs/root/configuration/listeners/network_filters/rbac_filter.rst index d07417492045..68ae9f2172d4 100644 --- a/docs/root/configuration/listeners/network_filters/rbac_filter.rst +++ b/docs/root/configuration/listeners/network_filters/rbac_filter.rst @@ -26,6 +26,8 @@ The RBAC network filter outputs statistics in the *.rbac.* namespac denied, Counter, Total requests that were denied access shadow_allowed, Counter, Total requests that would be allowed access by the filter's shadow rules shadow_denied, Counter, Total requests that would be denied access by the filter's shadow rules + logged, Counter, Total requests that should be logged + not_logged, Counter, Total requests that should not be logged .. _config_network_filters_rbac_dynamic_metadata: @@ -40,3 +42,4 @@ The RBAC filter emits the following dynamic metadata. shadow_effective_policy_id, string, The effective shadow policy ID matching the action (if any). shadow_engine_result, string, The engine result for the shadow rules (i.e. either `allowed` or `denied`). + access_log_hint, boolean, Whether the request should be logged. This metadata is shared and set under the key namespace 'envoy.common' (See :ref:`Shared Dynamic Metadata`). diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst index f5b6217a1757..4a6ac04a2576 100644 --- a/docs/root/version_history/current.rst +++ b/docs/root/version_history/current.rst @@ -60,6 +60,7 @@ New Features * lua: added Lua APIs to access :ref:`SSL connection info ` object. * postgres network filter: :ref:`metadata ` is produced based on SQL query. * ratelimit: added :ref:`enable_x_ratelimit_headers ` option to enable `X-RateLimit-*` headers as defined in `draft RFC `_. +* rbac filter: added a log action to the :ref:`RBAC filter ` which sets dynamic metadata to inform access loggers whether to log. * router: added new :ref:`envoy-ratelimited` retry policy, which allows retrying envoy's own rate limited responses. diff --git a/generated_api_shadow/envoy/config/rbac/v3/rbac.proto b/generated_api_shadow/envoy/config/rbac/v3/rbac.proto index 10520b1ba38f..278e6857603f 100644 --- a/generated_api_shadow/envoy/config/rbac/v3/rbac.proto +++ b/generated_api_shadow/envoy/config/rbac/v3/rbac.proto @@ -24,8 +24,14 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE; // [#protodoc-title: Role Based Access Control (RBAC)] // Role Based Access Control (RBAC) provides service-level and method-level access control for a -// service. RBAC policies are additive. The policies are examined in order. A request is allowed -// once a matching policy is found (suppose the `action` is ALLOW). +// service. RBAC policies are additive. The policies are examined in order. Requests are allowed +// or denied based on the `action` and whether a matching policy is found. For instance, if the +// action is ALLOW and a matching policy is found the request should be allowed. +// +// RBAC can also be used to make access logging decisions by communicating with access loggers +// through dynamic metadata. When the action is LOG and at least one policy matches, the +// `access_log_hint` value in the shared key namespace 'envoy.common' is set to `true` indicating +// the request should be logged. // // Here is an example of RBAC configuration. It has two policies: // @@ -68,39 +74,55 @@ message RBAC { // Should we do safe-list or block-list style access control? enum Action { - // The policies grant access to principals. The rest is denied. This is safe-list style + // The policies grant access to principals. The rest are denied. This is safe-list style // access control. This is the default type. ALLOW = 0; - // The policies deny access to principals. The rest is allowed. This is block-list style + // The policies deny access to principals. The rest are allowed. This is block-list style // access control. DENY = 1; + + // The policies set the `access_log_hint` dynamic metadata key based on if requests match. + // All requests are allowed. + LOG = 2; } - // The action to take if a policy matches. The request is allowed if and only if: + // The action to take if a policy matches. Every action either allows or denies a request, + // and can also carry out action-specific operations. + // + // Actions: + // + // * ALLOW: Allows the request if and only if there is a policy that matches + // the request. + // * DENY: Allows the request if and only if there are no policies that + // match the request. + // * LOG: Allows all requests. If at least one policy matches, the dynamic + // metadata key `access_log_hint` is set to the value `true` under the shared + // key namespace 'envoy.common'. If no policies match, it is set to `false`. + // Other actions do not modify this key. // - // * `action` is "ALLOWED" and at least one policy matches - // * `action` is "DENY" and none of the policies match Action action = 1; // Maps from policy name to policy. A match occurs when at least one policy matches the request. map policies = 2; } -// Policy specifies a role and the principals that are assigned/denied the role. A policy matches if -// and only if at least one of its permissions match the action taking place AND at least one of its -// principals match the downstream AND the condition is true if specified. +// Policy specifies a role and the principals that are assigned/denied the role. +// A policy matches if and only if at least one of its permissions match the +// action taking place AND at least one of its principals match the downstream +// AND the condition is true if specified. message Policy { option (udpa.annotations.versioning).previous_message_type = "envoy.config.rbac.v2.Policy"; - // Required. The set of permissions that define a role. Each permission is matched with OR - // semantics. To match all actions for this policy, a single Permission with the `any` field set - // to true should be used. + // Required. The set of permissions that define a role. Each permission is + // matched with OR semantics. To match all actions for this policy, a single + // Permission with the `any` field set to true should be used. repeated Permission permissions = 1 [(validate.rules).repeated = {min_items: 1}]; - // Required. The set of principals that are assigned/denied the role based on “action”. Each - // principal is matched with OR semantics. To match all downstreams for this policy, a single - // Principal with the `any` field set to true should be used. + // Required. The set of principals that are assigned/denied the role based on + // “action”. Each principal is matched with OR semantics. To match all + // downstreams for this policy, a single Principal with the `any` field set to + // true should be used. repeated Principal principals = 2 [(validate.rules).repeated = {min_items: 1}]; // An optional symbolic expression specifying an access control @@ -161,9 +183,9 @@ message Permission { // Metadata that describes additional information about the action. type.matcher.v3.MetadataMatcher metadata = 7; - // Negates matching the provided permission. For instance, if the value of `not_rule` would - // match, this permission would not match. Conversely, if the value of `not_rule` would not - // match, this permission would match. + // Negates matching the provided permission. For instance, if the value of + // `not_rule` would match, this permission would not match. Conversely, if + // the value of `not_rule` would not match, this permission would match. Permission not_rule = 8; // The request server from the client's connection request. This is @@ -176,7 +198,8 @@ message Permission { // // * If the :ref:`TLS Inspector ` // filter is not added, and if a `FilterChainMatch` is not defined for - // the :ref:`server name `, + // the :ref:`server name + // `, // a TLS connection's requested SNI server name will be treated as if it // wasn't present. // @@ -189,13 +212,14 @@ message Permission { } } -// Principal defines an identity or a group of identities for a downstream subject. +// Principal defines an identity or a group of identities for a downstream +// subject. // [#next-free-field: 12] message Principal { option (udpa.annotations.versioning).previous_message_type = "envoy.config.rbac.v2.Principal"; - // Used in the `and_ids` and `or_ids` fields in the `identifier` oneof. Depending on the context, - // each are applied with the associated behavior. + // Used in the `and_ids` and `or_ids` fields in the `identifier` oneof. + // Depending on the context, each are applied with the associated behavior. message Set { option (udpa.annotations.versioning).previous_message_type = "envoy.config.rbac.v2.Principal.Set"; @@ -210,19 +234,21 @@ message Principal { reserved 1; - // The name of the principal. If set, The URI SAN or DNS SAN in that order is used from the - // certificate, otherwise the subject field is used. If unset, it applies to any user that is - // authenticated. + // The name of the principal. If set, The URI SAN or DNS SAN in that order + // is used from the certificate, otherwise the subject field is used. If + // unset, it applies to any user that is authenticated. type.matcher.v3.StringMatcher principal_name = 2; } oneof identifier { option (validate.required) = true; - // A set of identifiers that all must match in order to define the downstream. + // A set of identifiers that all must match in order to define the + // downstream. Set and_ids = 1; - // A set of identifiers at least one must match in order to define the downstream. + // A set of identifiers at least one must match in order to define the + // downstream. Set or_ids = 2; // When any is set, it matches any downstream. @@ -237,21 +263,23 @@ message Principal { // A CIDR block that describes the downstream remote/origin address. // Note: This is always the physical peer even if the - // :ref:`remote_ip ` is inferred - // from for example the x-forwarder-for header, proxy protocol, etc. + // :ref:`remote_ip ` is + // inferred from for example the x-forwarder-for header, proxy protocol, + // etc. core.v3.CidrRange direct_remote_ip = 10; // A CIDR block that describes the downstream remote/origin address. // Note: This may not be the physical peer and could be different from the - // :ref:`direct_remote_ip `. - // E.g, if the remote ip is inferred from for example the x-forwarder-for header, - // proxy protocol, etc. + // :ref:`direct_remote_ip + // `. E.g, if the + // remote ip is inferred from for example the x-forwarder-for header, proxy + // protocol, etc. core.v3.CidrRange remote_ip = 11; - // A header (or pseudo-header such as :path or :method) on the incoming HTTP request. Only - // available for HTTP request. - // Note: the pseudo-header :path includes the query and fragment string. Use the `url_path` - // field if you want to match the URL path without the query and fragment string. + // A header (or pseudo-header such as :path or :method) on the incoming HTTP + // request. Only available for HTTP request. Note: the pseudo-header :path + // includes the query and fragment string. Use the `url_path` field if you + // want to match the URL path without the query and fragment string. route.v3.HeaderMatcher header = 6; // A URL path on the incoming HTTP request. Only available for HTTP. @@ -260,9 +288,9 @@ message Principal { // Metadata that describes additional information about the principal. type.matcher.v3.MetadataMatcher metadata = 7; - // Negates matching the provided principal. For instance, if the value of `not_id` would match, - // this principal would not match. Conversely, if the value of `not_id` would not match, this - // principal would match. + // Negates matching the provided principal. For instance, if the value of + // `not_id` would match, this principal would not match. Conversely, if the + // value of `not_id` would not match, this principal would match. Principal not_id = 8; } } diff --git a/generated_api_shadow/envoy/config/rbac/v4alpha/rbac.proto b/generated_api_shadow/envoy/config/rbac/v4alpha/rbac.proto index 3d8dae2402ea..7139dfaa1485 100644 --- a/generated_api_shadow/envoy/config/rbac/v4alpha/rbac.proto +++ b/generated_api_shadow/envoy/config/rbac/v4alpha/rbac.proto @@ -23,8 +23,14 @@ option (udpa.annotations.file_status).package_version_status = NEXT_MAJOR_VERSIO // [#protodoc-title: Role Based Access Control (RBAC)] // Role Based Access Control (RBAC) provides service-level and method-level access control for a -// service. RBAC policies are additive. The policies are examined in order. A request is allowed -// once a matching policy is found (suppose the `action` is ALLOW). +// service. RBAC policies are additive. The policies are examined in order. Requests are allowed +// or denied based on the `action` and whether a matching policy is found. For instance, if the +// action is ALLOW and a matching policy is found the request should be allowed. +// +// RBAC can also be used to make access logging decisions by communicating with access loggers +// through dynamic metadata. When the action is LOG and at least one policy matches, the +// `access_log_hint` value in the shared key namespace 'envoy.common' is set to `true` indicating +// the request should be logged. // // Here is an example of RBAC configuration. It has two policies: // @@ -67,39 +73,55 @@ message RBAC { // Should we do safe-list or block-list style access control? enum Action { - // The policies grant access to principals. The rest is denied. This is safe-list style + // The policies grant access to principals. The rest are denied. This is safe-list style // access control. This is the default type. ALLOW = 0; - // The policies deny access to principals. The rest is allowed. This is block-list style + // The policies deny access to principals. The rest are allowed. This is block-list style // access control. DENY = 1; + + // The policies set the `access_log_hint` dynamic metadata key based on if requests match. + // All requests are allowed. + LOG = 2; } - // The action to take if a policy matches. The request is allowed if and only if: + // The action to take if a policy matches. Every action either allows or denies a request, + // and can also carry out action-specific operations. + // + // Actions: + // + // * ALLOW: Allows the request if and only if there is a policy that matches + // the request. + // * DENY: Allows the request if and only if there are no policies that + // match the request. + // * LOG: Allows all requests. If at least one policy matches, the dynamic + // metadata key `access_log_hint` is set to the value `true` under the shared + // key namespace 'envoy.common'. If no policies match, it is set to `false`. + // Other actions do not modify this key. // - // * `action` is "ALLOWED" and at least one policy matches - // * `action` is "DENY" and none of the policies match Action action = 1; // Maps from policy name to policy. A match occurs when at least one policy matches the request. map policies = 2; } -// Policy specifies a role and the principals that are assigned/denied the role. A policy matches if -// and only if at least one of its permissions match the action taking place AND at least one of its -// principals match the downstream AND the condition is true if specified. +// Policy specifies a role and the principals that are assigned/denied the role. +// A policy matches if and only if at least one of its permissions match the +// action taking place AND at least one of its principals match the downstream +// AND the condition is true if specified. message Policy { option (udpa.annotations.versioning).previous_message_type = "envoy.config.rbac.v3.Policy"; - // Required. The set of permissions that define a role. Each permission is matched with OR - // semantics. To match all actions for this policy, a single Permission with the `any` field set - // to true should be used. + // Required. The set of permissions that define a role. Each permission is + // matched with OR semantics. To match all actions for this policy, a single + // Permission with the `any` field set to true should be used. repeated Permission permissions = 1 [(validate.rules).repeated = {min_items: 1}]; - // Required. The set of principals that are assigned/denied the role based on “action”. Each - // principal is matched with OR semantics. To match all downstreams for this policy, a single - // Principal with the `any` field set to true should be used. + // Required. The set of principals that are assigned/denied the role based on + // “action”. Each principal is matched with OR semantics. To match all + // downstreams for this policy, a single Principal with the `any` field set to + // true should be used. repeated Principal principals = 2 [(validate.rules).repeated = {min_items: 1}]; oneof expression_specifier { @@ -160,9 +182,9 @@ message Permission { // Metadata that describes additional information about the action. type.matcher.v4alpha.MetadataMatcher metadata = 7; - // Negates matching the provided permission. For instance, if the value of `not_rule` would - // match, this permission would not match. Conversely, if the value of `not_rule` would not - // match, this permission would match. + // Negates matching the provided permission. For instance, if the value of + // `not_rule` would match, this permission would not match. Conversely, if + // the value of `not_rule` would not match, this permission would match. Permission not_rule = 8; // The request server from the client's connection request. This is @@ -175,7 +197,8 @@ message Permission { // // * If the :ref:`TLS Inspector ` // filter is not added, and if a `FilterChainMatch` is not defined for - // the :ref:`server name `, + // the :ref:`server name + // `, // a TLS connection's requested SNI server name will be treated as if it // wasn't present. // @@ -188,13 +211,14 @@ message Permission { } } -// Principal defines an identity or a group of identities for a downstream subject. +// Principal defines an identity or a group of identities for a downstream +// subject. // [#next-free-field: 12] message Principal { option (udpa.annotations.versioning).previous_message_type = "envoy.config.rbac.v3.Principal"; - // Used in the `and_ids` and `or_ids` fields in the `identifier` oneof. Depending on the context, - // each are applied with the associated behavior. + // Used in the `and_ids` and `or_ids` fields in the `identifier` oneof. + // Depending on the context, each are applied with the associated behavior. message Set { option (udpa.annotations.versioning).previous_message_type = "envoy.config.rbac.v3.Principal.Set"; @@ -209,19 +233,21 @@ message Principal { reserved 1; - // The name of the principal. If set, The URI SAN or DNS SAN in that order is used from the - // certificate, otherwise the subject field is used. If unset, it applies to any user that is - // authenticated. + // The name of the principal. If set, The URI SAN or DNS SAN in that order + // is used from the certificate, otherwise the subject field is used. If + // unset, it applies to any user that is authenticated. type.matcher.v4alpha.StringMatcher principal_name = 2; } oneof identifier { option (validate.required) = true; - // A set of identifiers that all must match in order to define the downstream. + // A set of identifiers that all must match in order to define the + // downstream. Set and_ids = 1; - // A set of identifiers at least one must match in order to define the downstream. + // A set of identifiers at least one must match in order to define the + // downstream. Set or_ids = 2; // When any is set, it matches any downstream. @@ -236,21 +262,23 @@ message Principal { // A CIDR block that describes the downstream remote/origin address. // Note: This is always the physical peer even if the - // :ref:`remote_ip ` is inferred - // from for example the x-forwarder-for header, proxy protocol, etc. + // :ref:`remote_ip ` is + // inferred from for example the x-forwarder-for header, proxy protocol, + // etc. core.v4alpha.CidrRange direct_remote_ip = 10; // A CIDR block that describes the downstream remote/origin address. // Note: This may not be the physical peer and could be different from the - // :ref:`direct_remote_ip `. - // E.g, if the remote ip is inferred from for example the x-forwarder-for header, - // proxy protocol, etc. + // :ref:`direct_remote_ip + // `. E.g, if the + // remote ip is inferred from for example the x-forwarder-for header, proxy + // protocol, etc. core.v4alpha.CidrRange remote_ip = 11; - // A header (or pseudo-header such as :path or :method) on the incoming HTTP request. Only - // available for HTTP request. - // Note: the pseudo-header :path includes the query and fragment string. Use the `url_path` - // field if you want to match the URL path without the query and fragment string. + // A header (or pseudo-header such as :path or :method) on the incoming HTTP + // request. Only available for HTTP request. Note: the pseudo-header :path + // includes the query and fragment string. Use the `url_path` field if you + // want to match the URL path without the query and fragment string. route.v4alpha.HeaderMatcher header = 6; // A URL path on the incoming HTTP request. Only available for HTTP. @@ -259,9 +287,9 @@ message Principal { // Metadata that describes additional information about the principal. type.matcher.v4alpha.MetadataMatcher metadata = 7; - // Negates matching the provided principal. For instance, if the value of `not_id` would match, - // this principal would not match. Conversely, if the value of `not_id` would not match, this - // principal would match. + // Negates matching the provided principal. For instance, if the value of + // `not_id` would match, this principal would not match. Conversely, if the + // value of `not_id` would not match, this principal would match. Principal not_id = 8; } } diff --git a/source/extensions/filters/common/rbac/engine.h b/source/extensions/filters/common/rbac/engine.h index a833867dd02a..7174d4edb860 100644 --- a/source/extensions/filters/common/rbac/engine.h +++ b/source/extensions/filters/common/rbac/engine.h @@ -19,32 +19,32 @@ class RoleBasedAccessControlEngine { virtual ~RoleBasedAccessControlEngine() = default; /** - * Returns whether or not the current action is permitted. + * Handles action-specific operations and returns whether or not the request is permitted. * * @param connection the downstream connection used to identify the action/principal. * @param headers the headers of the incoming request used to identify the action/principal. An * empty map should be used if there are no headers available. * @param info the per-request or per-connection stream info with additional information - * about the action/principal. + * about the action/principal. Can be modified by the LOG Action. * @param effective_policy_id it will be filled by the matching policy's ID, * which is used to identity the source of the allow/deny. */ - virtual bool allowed(const Network::Connection& connection, - const Envoy::Http::RequestHeaderMap& headers, - const StreamInfo::StreamInfo& info, - std::string* effective_policy_id) const PURE; + virtual bool handleAction(const Network::Connection& connection, + const Envoy::Http::RequestHeaderMap& headers, + StreamInfo::StreamInfo& info, + std::string* effective_policy_id) const PURE; /** - * Returns whether or not the current action is permitted. + * Handles action-specific operations and returns whether or not the request is permitted. * * @param connection the downstream connection used to identify the action/principal. * @param info the per-request or per-connection stream info with additional information - * about the action/principal. + * about the action/principal. Can be modified by the LOG Action. * @param effective_policy_id it will be filled by the matching policy's ID, * which is used to identity the source of the allow/deny. */ - virtual bool allowed(const Network::Connection& connection, const StreamInfo::StreamInfo& info, - std::string* effective_policy_id) const PURE; + virtual bool handleAction(const Network::Connection& connection, StreamInfo::StreamInfo& info, + std::string* effective_policy_id) const PURE; }; } // namespace RBAC diff --git a/source/extensions/filters/common/rbac/engine_impl.cc b/source/extensions/filters/common/rbac/engine_impl.cc index d9717ef509c0..dc2a6ba79222 100644 --- a/source/extensions/filters/common/rbac/engine_impl.cc +++ b/source/extensions/filters/common/rbac/engine_impl.cc @@ -11,8 +11,8 @@ namespace Common { namespace RBAC { RoleBasedAccessControlEngineImpl::RoleBasedAccessControlEngineImpl( - const envoy::config::rbac::v3::RBAC& rules) - : allowed_if_matched_(rules.action() == envoy::config::rbac::v3::RBAC::ALLOW) { + const envoy::config::rbac::v3::RBAC& rules, const EnforcementMode mode) + : action_(rules.action()), mode_(mode) { // guard expression builder by presence of a condition in policies for (const auto& policy : rules.policies()) { if (policy.second.has_condition()) { @@ -26,10 +26,43 @@ RoleBasedAccessControlEngineImpl::RoleBasedAccessControlEngineImpl( } } -bool RoleBasedAccessControlEngineImpl::allowed(const Network::Connection& connection, - const Envoy::Http::RequestHeaderMap& headers, - const StreamInfo::StreamInfo& info, - std::string* effective_policy_id) const { +bool RoleBasedAccessControlEngineImpl::handleAction(const Network::Connection& connection, + StreamInfo::StreamInfo& info, + std::string* effective_policy_id) const { + return handleAction(connection, *Http::StaticEmptyHeaders::get().request_headers, info, + effective_policy_id); +} + +bool RoleBasedAccessControlEngineImpl::handleAction(const Network::Connection& connection, + const Envoy::Http::RequestHeaderMap& headers, + StreamInfo::StreamInfo& info, + std::string* effective_policy_id) const { + bool matched = checkPolicyMatch(connection, info, headers, effective_policy_id); + + switch (action_) { + case envoy::config::rbac::v3::RBAC::ALLOW: + return matched; + case envoy::config::rbac::v3::RBAC::DENY: + return !matched; + case envoy::config::rbac::v3::RBAC::LOG: { + // If not shadow enforcement, set shared log metadata + if (mode_ != EnforcementMode::Shadow) { + ProtobufWkt::Struct log_metadata; + auto& log_fields = *log_metadata.mutable_fields(); + log_fields[DynamicMetadataKeysSingleton::get().AccessLogKey].set_bool_value(matched); + info.setDynamicMetadata(DynamicMetadataKeysSingleton::get().CommonNamespace, log_metadata); + } + + return true; + } + default: + NOT_REACHED_GCOVR_EXCL_LINE; + } +} + +bool RoleBasedAccessControlEngineImpl::checkPolicyMatch( + const Network::Connection& connection, const StreamInfo::StreamInfo& info, + const Envoy::Http::RequestHeaderMap& headers, std::string* effective_policy_id) const { bool matched = false; for (const auto& policy : policies_) { @@ -42,17 +75,7 @@ bool RoleBasedAccessControlEngineImpl::allowed(const Network::Connection& connec } } - // only allowed if: - // - matched and ALLOW action - // - not matched and DENY action - return matched == allowed_if_matched_; -} - -bool RoleBasedAccessControlEngineImpl::allowed(const Network::Connection& connection, - const StreamInfo::StreamInfo& info, - std::string* effective_policy_id) const { - return allowed(connection, *Http::StaticEmptyHeaders::get().request_headers, info, - effective_policy_id); + return matched; } } // namespace RBAC diff --git a/source/extensions/filters/common/rbac/engine_impl.h b/source/extensions/filters/common/rbac/engine_impl.h index 261b45b0aa13..0aacfb41f8e1 100644 --- a/source/extensions/filters/common/rbac/engine_impl.h +++ b/source/extensions/filters/common/rbac/engine_impl.h @@ -11,18 +11,40 @@ namespace Filters { namespace Common { namespace RBAC { +class DynamicMetadataKeys { +public: + const std::string ShadowEffectivePolicyIdField{"shadow_effective_policy_id"}; + const std::string ShadowEngineResultField{"shadow_engine_result"}; + const std::string EngineResultAllowed{"allowed"}; + const std::string EngineResultDenied{"denied"}; + const std::string AccessLogKey{"access_log_hint"}; + const std::string CommonNamespace{"envoy.common"}; +}; + +using DynamicMetadataKeysSingleton = ConstSingleton; + +enum class EnforcementMode { Enforced, Shadow }; + class RoleBasedAccessControlEngineImpl : public RoleBasedAccessControlEngine, NonCopyable { public: - RoleBasedAccessControlEngineImpl(const envoy::config::rbac::v3::RBAC& rules); + RoleBasedAccessControlEngineImpl(const envoy::config::rbac::v3::RBAC& rules, + const EnforcementMode mode = EnforcementMode::Enforced); - bool allowed(const Network::Connection& connection, const Envoy::Http::RequestHeaderMap& headers, - const StreamInfo::StreamInfo& info, std::string* effective_policy_id) const override; + bool handleAction(const Network::Connection& connection, + const Envoy::Http::RequestHeaderMap& headers, StreamInfo::StreamInfo& info, + std::string* effective_policy_id) const override; - bool allowed(const Network::Connection& connection, const StreamInfo::StreamInfo& info, - std::string* effective_policy_id) const override; + bool handleAction(const Network::Connection& connection, StreamInfo::StreamInfo& info, + std::string* effective_policy_id) const override; private: - const bool allowed_if_matched_; + // Checks whether the request matches any policies + bool checkPolicyMatch(const Network::Connection& connection, const StreamInfo::StreamInfo& info, + const Envoy::Http::RequestHeaderMap& headers, + std::string* effective_policy_id) const; + + const envoy::config::rbac::v3::RBAC::Action action_; + const EnforcementMode mode_; std::map> policies_; diff --git a/source/extensions/filters/common/rbac/utility.h b/source/extensions/filters/common/rbac/utility.h index a48efb813234..04635eb37411 100644 --- a/source/extensions/filters/common/rbac/utility.h +++ b/source/extensions/filters/common/rbac/utility.h @@ -12,16 +12,6 @@ namespace Filters { namespace Common { namespace RBAC { -class DynamicMetadataKeys { -public: - const std::string ShadowEffectivePolicyIdField{"shadow_effective_policy_id"}; - const std::string ShadowEngineResultField{"shadow_engine_result"}; - const std::string EngineResultAllowed{"allowed"}; - const std::string EngineResultDenied{"denied"}; -}; - -using DynamicMetadataKeysSingleton = ConstSingleton; - /** * All stats for the RBAC filter. @see stats_macros.h */ @@ -40,19 +30,18 @@ struct RoleBasedAccessControlFilterStats { RoleBasedAccessControlFilterStats generateStats(const std::string& prefix, Stats::Scope& scope); -enum class EnforcementMode { Enforced, Shadow }; - template std::unique_ptr createEngine(const ConfigType& config) { - return config.has_rules() ? std::make_unique(config.rules()) + return config.has_rules() ? std::make_unique( + config.rules(), EnforcementMode::Enforced) : nullptr; } template std::unique_ptr createShadowEngine(const ConfigType& config) { - return config.has_shadow_rules() - ? std::make_unique(config.shadow_rules()) - : nullptr; + return config.has_shadow_rules() ? std::make_unique( + config.shadow_rules(), EnforcementMode::Shadow) + : nullptr; } } // namespace RBAC diff --git a/source/extensions/filters/http/rbac/rbac_filter.cc b/source/extensions/filters/http/rbac/rbac_filter.cc index 6e1ff3ea3318..d396db7f52bc 100644 --- a/source/extensions/filters/http/rbac/rbac_filter.cc +++ b/source/extensions/filters/http/rbac/rbac_filter.cc @@ -80,8 +80,8 @@ RoleBasedAccessControlFilter::decodeHeaders(Http::RequestHeaderMap& headers, boo if (shadow_engine != nullptr) { std::string shadow_resp_code = Filters::Common::RBAC::DynamicMetadataKeysSingleton::get().EngineResultAllowed; - if (shadow_engine->allowed(*callbacks_->connection(), headers, callbacks_->streamInfo(), - &effective_policy_id)) { + if (shadow_engine->handleAction(*callbacks_->connection(), headers, callbacks_->streamInfo(), + &effective_policy_id)) { ENVOY_LOG(debug, "shadow allowed"); config_->stats().shadow_allowed_.inc(); } else { @@ -109,7 +109,8 @@ RoleBasedAccessControlFilter::decodeHeaders(Http::RequestHeaderMap& headers, boo const auto engine = config_->engine(callbacks_->route(), Filters::Common::RBAC::EnforcementMode::Enforced); if (engine != nullptr) { - if (engine->allowed(*callbacks_->connection(), headers, callbacks_->streamInfo(), nullptr)) { + if (engine->handleAction(*callbacks_->connection(), headers, callbacks_->streamInfo(), + nullptr)) { ENVOY_LOG(debug, "enforced allowed"); config_->stats().allowed_.inc(); return Http::FilterHeadersStatus::Continue; diff --git a/source/extensions/filters/network/rbac/rbac_filter.cc b/source/extensions/filters/network/rbac/rbac_filter.cc index 1bc12017b3b6..3b328ed2815f 100644 --- a/source/extensions/filters/network/rbac/rbac_filter.cc +++ b/source/extensions/filters/network/rbac/rbac_filter.cc @@ -85,8 +85,10 @@ RoleBasedAccessControlFilter::checkEngine(Filters::Common::RBAC::EnforcementMode const auto engine = config_->engine(mode); if (engine != nullptr) { std::string effective_policy_id; - if (engine->allowed(callbacks_->connection(), callbacks_->connection().streamInfo(), - &effective_policy_id)) { + + // Check authorization decision and do Action operations + if (engine->handleAction(callbacks_->connection(), callbacks_->connection().streamInfo(), + &effective_policy_id)) { if (mode == Filters::Common::RBAC::EnforcementMode::Shadow) { ENVOY_LOG(debug, "shadow allowed"); config_->stats().shadow_allowed_.inc(); diff --git a/test/extensions/filters/common/rbac/engine_impl_test.cc b/test/extensions/filters/common/rbac/engine_impl_test.cc index 8f4f3d7e6ad1..b9d8608a9208 100644 --- a/test/extensions/filters/common/rbac/engine_impl_test.cc +++ b/test/extensions/filters/common/rbac/engine_impl_test.cc @@ -24,21 +24,56 @@ namespace Common { namespace RBAC { namespace { +enum class LogResult { Yes, No, Undecided }; + void checkEngine( - const RBAC::RoleBasedAccessControlEngineImpl& engine, bool expected, + RBAC::RoleBasedAccessControlEngineImpl& engine, bool expected, LogResult expected_log, + StreamInfo::StreamInfo& info, const Envoy::Network::Connection& connection = Envoy::Network::MockConnection(), - const Envoy::Http::RequestHeaderMap& headers = Envoy::Http::TestRequestHeaderMapImpl(), - const StreamInfo::StreamInfo& info = NiceMock()) { - EXPECT_EQ(expected, engine.allowed(connection, headers, info, nullptr)); + const Envoy::Http::RequestHeaderMap& headers = Envoy::Http::TestRequestHeaderMapImpl()) { + + bool engineRes = engine.handleAction(connection, headers, info, nullptr); + EXPECT_EQ(expected, engineRes); + + if (expected_log != LogResult::Undecided) { + auto filter_meta = info.dynamicMetadata().filter_metadata().at( + RBAC::DynamicMetadataKeysSingleton::get().CommonNamespace); + EXPECT_EQ(expected_log == LogResult::Yes, + filter_meta.fields() + .at(RBAC::DynamicMetadataKeysSingleton::get().AccessLogKey) + .bool_value()); + } else { + EXPECT_EQ(info.dynamicMetadata().filter_metadata().end(), + info.dynamicMetadata().filter_metadata().find( + Filters::Common::RBAC::DynamicMetadataKeysSingleton::get().CommonNamespace)); + } +} + +void checkEngine( + RBAC::RoleBasedAccessControlEngineImpl& engine, bool expected, LogResult expected_log, + const Envoy::Network::Connection& connection = Envoy::Network::MockConnection(), + const Envoy::Http::RequestHeaderMap& headers = Envoy::Http::TestRequestHeaderMapImpl()) { + + NiceMock empty_info; + checkEngine(engine, expected, expected_log, empty_info, connection, headers); +} + +void onMetadata(NiceMock& info) { + ON_CALL(info, setDynamicMetadata("envoy.common", _)) + .WillByDefault(Invoke([&info](const std::string&, const ProtobufWkt::Struct& obj) { + (*info.metadata_.mutable_filter_metadata())["envoy.common"] = obj; + })); } TEST(RoleBasedAccessControlEngineImpl, Disabled) { envoy::config::rbac::v3::RBAC rbac; rbac.set_action(envoy::config::rbac::v3::RBAC::ALLOW); - checkEngine(RBAC::RoleBasedAccessControlEngineImpl(rbac), false); + RBAC::RoleBasedAccessControlEngineImpl engine_allow(rbac); + checkEngine(engine_allow, false, LogResult::Undecided); rbac.set_action(envoy::config::rbac::v3::RBAC::DENY); - checkEngine(RBAC::RoleBasedAccessControlEngineImpl(rbac), true); + RBAC::RoleBasedAccessControlEngineImpl engine_deny(rbac); + checkEngine(engine_deny, true, LogResult::Undecided); } // Test various invalid policies to validate the fix for @@ -143,11 +178,11 @@ TEST(RoleBasedAccessControlEngineImpl, AllowedAllowlist) { Envoy::Network::Address::InstanceConstSharedPtr addr = Envoy::Network::Utility::parseInternetAddress("1.2.3.4", 123, false); EXPECT_CALL(Const(info), downstreamLocalAddress()).WillOnce(ReturnRef(addr)); - checkEngine(engine, true, conn, headers, info); + checkEngine(engine, true, LogResult::Undecided, info, conn, headers); addr = Envoy::Network::Utility::parseInternetAddress("1.2.3.4", 456, false); EXPECT_CALL(Const(info), downstreamLocalAddress()).WillOnce(ReturnRef(addr)); - checkEngine(engine, false, conn, headers, info); + checkEngine(engine, false, LogResult::Undecided, info, conn, headers); } TEST(RoleBasedAccessControlEngineImpl, DeniedDenylist) { @@ -166,11 +201,11 @@ TEST(RoleBasedAccessControlEngineImpl, DeniedDenylist) { Envoy::Network::Address::InstanceConstSharedPtr addr = Envoy::Network::Utility::parseInternetAddress("1.2.3.4", 123, false); EXPECT_CALL(Const(info), downstreamLocalAddress()).WillOnce(ReturnRef(addr)); - checkEngine(engine, false, conn, headers, info); + checkEngine(engine, false, LogResult::Undecided, info, conn, headers); addr = Envoy::Network::Utility::parseInternetAddress("1.2.3.4", 456, false); EXPECT_CALL(Const(info), downstreamLocalAddress()).WillOnce(ReturnRef(addr)); - checkEngine(engine, true, conn, headers, info); + checkEngine(engine, true, LogResult::Undecided, info, conn, headers); } TEST(RoleBasedAccessControlEngineImpl, BasicCondition) { @@ -187,7 +222,7 @@ TEST(RoleBasedAccessControlEngineImpl, BasicCondition) { rbac.set_action(envoy::config::rbac::v3::RBAC::ALLOW); (*rbac.mutable_policies())["foo"] = policy; RBAC::RoleBasedAccessControlEngineImpl engine(rbac); - checkEngine(engine, false); + checkEngine(engine, false, LogResult::Undecided); } TEST(RoleBasedAccessControlEngineImpl, MalformedCondition) { @@ -209,6 +244,10 @@ TEST(RoleBasedAccessControlEngineImpl, MalformedCondition) { EXPECT_THROW_WITH_REGEX(RBAC::RoleBasedAccessControlEngineImpl engine(rbac), EnvoyException, "failed to create an expression: .*"); + + rbac.set_action(envoy::config::rbac::v3::RBAC::LOG); + EXPECT_THROW_WITH_REGEX(RBAC::RoleBasedAccessControlEngineImpl engine_log(rbac), EnvoyException, + "failed to create an expression: .*"); } TEST(RoleBasedAccessControlEngineImpl, MistypedCondition) { @@ -225,7 +264,7 @@ TEST(RoleBasedAccessControlEngineImpl, MistypedCondition) { rbac.set_action(envoy::config::rbac::v3::RBAC::ALLOW); (*rbac.mutable_policies())["foo"] = policy; RBAC::RoleBasedAccessControlEngineImpl engine(rbac); - checkEngine(engine, false); + checkEngine(engine, false, LogResult::Undecided); } TEST(RoleBasedAccessControlEngineImpl, ErrorCondition) { @@ -250,7 +289,7 @@ TEST(RoleBasedAccessControlEngineImpl, ErrorCondition) { rbac.set_action(envoy::config::rbac::v3::RBAC::ALLOW); (*rbac.mutable_policies())["foo"] = policy; RBAC::RoleBasedAccessControlEngineImpl engine(rbac); - checkEngine(engine, false, Envoy::Network::MockConnection()); + checkEngine(engine, false, LogResult::Undecided, Envoy::Network::MockConnection()); } TEST(RoleBasedAccessControlEngineImpl, HeaderCondition) { @@ -286,7 +325,7 @@ TEST(RoleBasedAccessControlEngineImpl, HeaderCondition) { std::string value = "bar"; headers.setReference(key, value); - checkEngine(engine, true, Envoy::Network::MockConnection(), headers); + checkEngine(engine, true, LogResult::Undecided, Envoy::Network::MockConnection(), headers); } TEST(RoleBasedAccessControlEngineImpl, MetadataCondition) { @@ -331,7 +370,7 @@ TEST(RoleBasedAccessControlEngineImpl, MetadataCondition) { Protobuf::MapPair("other", label)); EXPECT_CALL(Const(info), dynamicMetadata()).WillRepeatedly(ReturnRef(metadata)); - checkEngine(engine, true, Envoy::Network::MockConnection(), headers, info); + checkEngine(engine, true, LogResult::Undecided, info, Envoy::Network::MockConnection(), headers); } TEST(RoleBasedAccessControlEngineImpl, ConjunctiveCondition) { @@ -354,8 +393,44 @@ TEST(RoleBasedAccessControlEngineImpl, ConjunctiveCondition) { NiceMock info; Envoy::Network::Address::InstanceConstSharedPtr addr = Envoy::Network::Utility::parseInternetAddress("1.2.3.4", 123, false); + EXPECT_CALL(Const(info), downstreamLocalAddress()).Times(1).WillRepeatedly(ReturnRef(addr)); + checkEngine(engine, false, LogResult::Undecided, info, conn, headers); +} + +// Log tests +TEST(RoleBasedAccessControlEngineImpl, DisabledLog) { + NiceMock info; + onMetadata(info); + + envoy::config::rbac::v3::RBAC rbac; + rbac.set_action(envoy::config::rbac::v3::RBAC::LOG); + RBAC::RoleBasedAccessControlEngineImpl engine(rbac); + checkEngine(engine, true, RBAC::LogResult::No, info); +} + +TEST(RoleBasedAccessControlEngineImpl, LogIfMatched) { + envoy::config::rbac::v3::Policy policy; + policy.add_permissions()->set_destination_port(123); + policy.add_principals()->set_any(true); + + envoy::config::rbac::v3::RBAC rbac; + rbac.set_action(envoy::config::rbac::v3::RBAC::LOG); + (*rbac.mutable_policies())["foo"] = policy; + RBAC::RoleBasedAccessControlEngineImpl engine(rbac); + + Envoy::Network::MockConnection conn; + Envoy::Http::TestRequestHeaderMapImpl headers; + NiceMock info; + onMetadata(info); + + Envoy::Network::Address::InstanceConstSharedPtr addr = + Envoy::Network::Utility::parseInternetAddress("1.2.3.4", 123, false); + EXPECT_CALL(Const(info), downstreamLocalAddress()).WillOnce(ReturnRef(addr)); + checkEngine(engine, true, RBAC::LogResult::Yes, info, conn, headers); + + addr = Envoy::Network::Utility::parseInternetAddress("1.2.3.4", 456, false); EXPECT_CALL(Const(info), downstreamLocalAddress()).WillOnce(ReturnRef(addr)); - checkEngine(engine, false, conn, headers, info); + checkEngine(engine, true, RBAC::LogResult::No, info, conn, headers); } } // namespace diff --git a/test/extensions/filters/common/rbac/mocks.h b/test/extensions/filters/common/rbac/mocks.h index fda95244c893..a99e97aa9ea6 100644 --- a/test/extensions/filters/common/rbac/mocks.h +++ b/test/extensions/filters/common/rbac/mocks.h @@ -14,16 +14,17 @@ namespace RBAC { class MockEngine : public RoleBasedAccessControlEngineImpl { public: - MockEngine(const envoy::config::rbac::v3::RBAC& rules) - : RoleBasedAccessControlEngineImpl(rules){}; + MockEngine(const envoy::config::rbac::v3::RBAC& rules, + const EnforcementMode mode = EnforcementMode::Enforced) + : RoleBasedAccessControlEngineImpl(rules, mode){}; - MOCK_METHOD(bool, allowed, + MOCK_METHOD(bool, handleAction, (const Envoy::Network::Connection&, const Envoy::Http::RequestHeaderMap&, - const StreamInfo::StreamInfo&, std::string* effective_policy_id), + StreamInfo::StreamInfo&, std::string* effective_policy_id), (const)); - MOCK_METHOD(bool, allowed, - (const Envoy::Network::Connection&, const StreamInfo::StreamInfo&, + MOCK_METHOD(bool, handleAction, + (const Envoy::Network::Connection&, StreamInfo::StreamInfo&, std::string* effective_policy_id), (const)); }; diff --git a/test/extensions/filters/http/rbac/rbac_filter_integration_test.cc b/test/extensions/filters/http/rbac/rbac_filter_integration_test.cc index 28e4420db014..b7fcd3ebcbb7 100644 --- a/test/extensions/filters/http/rbac/rbac_filter_integration_test.cc +++ b/test/extensions/filters/http/rbac/rbac_filter_integration_test.cc @@ -64,6 +64,20 @@ name: rbac - any: true )EOF"; +const std::string RBAC_CONFIG_WITH_LOG_ACTION = R"EOF( +name: rbac +typed_config: + "@type": type.googleapis.com/envoy.extensions.filters.http.rbac.v3.RBAC + rules: + action: LOG + policies: + foo: + permissions: + - header: { name: ":method", exact_match: "GET" } + principals: + - any: true +)EOF"; + using RBACIntegrationTest = HttpProtocolIntegrationTest; INSTANTIATE_TEST_SUITE_P(Protocols, RBACIntegrationTest, @@ -277,5 +291,28 @@ TEST_P(RBACIntegrationTest, PathIgnoreCase) { } } +TEST_P(RBACIntegrationTest, LogConnectionAllow) { + config_helper_.addFilter(RBAC_CONFIG_WITH_LOG_ACTION); + initialize(); + + codec_client_ = makeHttpConnection(lookupPort("http")); + + auto response = codec_client_->makeRequestWithBody( + Http::TestRequestHeaderMapImpl{ + {":method", "POST"}, + {":path", "/"}, + {":scheme", "http"}, + {":authority", "host"}, + {"x-forwarded-for", "10.0.0.1"}, + }, + 1024); + waitForNextUpstreamRequest(); + upstream_request_->encodeHeaders(Http::TestResponseHeaderMapImpl{{":status", "200"}}, true); + + response->waitForEndStream(); + ASSERT_TRUE(response->complete()); + EXPECT_EQ("200", response->headers().getStatusValue()); +} + } // namespace } // namespace Envoy diff --git a/test/extensions/filters/http/rbac/rbac_filter_test.cc b/test/extensions/filters/http/rbac/rbac_filter_test.cc index d445860f8394..519a49126bbb 100644 --- a/test/extensions/filters/http/rbac/rbac_filter_test.cc +++ b/test/extensions/filters/http/rbac/rbac_filter_test.cc @@ -24,9 +24,12 @@ namespace HttpFilters { namespace RBACFilter { namespace { +enum class LogResult { Yes, No, Undecided }; + class RoleBasedAccessControlFilterTest : public testing::Test { public: - RoleBasedAccessControlFilterConfigSharedPtr setupConfig() { + RoleBasedAccessControlFilterConfigSharedPtr + setupConfig(envoy::config::rbac::v3::RBAC::Action action) { envoy::extensions::filters::http::rbac::v3::RBAC config; envoy::config::rbac::v3::Policy policy; @@ -36,7 +39,7 @@ class RoleBasedAccessControlFilterTest : public testing::Test { policy_rules->add_rules()->set_destination_port(123); policy_rules->add_rules()->mutable_url_path()->mutable_path()->set_suffix("suffix"); policy.add_principals()->set_any(true); - config.mutable_rules()->set_action(envoy::config::rbac::v3::RBAC::ALLOW); + config.mutable_rules()->set_action(action); (*config.mutable_rules()->mutable_policies())["foo"] = policy; envoy::config::rbac::v3::Policy shadow_policy; @@ -44,13 +47,14 @@ class RoleBasedAccessControlFilterTest : public testing::Test { shadow_policy_rules->add_rules()->mutable_requested_server_name()->set_exact("xyz.cncf.io"); shadow_policy_rules->add_rules()->set_destination_port(456); shadow_policy.add_principals()->set_any(true); - config.mutable_shadow_rules()->set_action(envoy::config::rbac::v3::RBAC::ALLOW); + config.mutable_shadow_rules()->set_action(action); (*config.mutable_shadow_rules()->mutable_policies())["bar"] = shadow_policy; return std::make_shared(config, "test", store_); } - RoleBasedAccessControlFilterTest() : config_(setupConfig()), filter_(config_) {} + RoleBasedAccessControlFilterTest() + : config_(setupConfig(envoy::config::rbac::v3::RBAC::ALLOW)), filter_(config_) {} void SetUp() override { EXPECT_CALL(callbacks_, connection()).WillRepeatedly(Return(&connection_)); @@ -68,6 +72,21 @@ class RoleBasedAccessControlFilterTest : public testing::Test { ON_CALL(connection_, requestedServerName()).WillByDefault(Return(requested_server_name_)); } + void checkAccessLogMetadata(LogResult expected) { + if (expected != LogResult::Undecided) { + auto filter_meta = req_info_.dynamicMetadata().filter_metadata().at( + Filters::Common::RBAC::DynamicMetadataKeysSingleton::get().CommonNamespace); + EXPECT_EQ(expected == LogResult::Yes, + filter_meta.fields() + .at(Filters::Common::RBAC::DynamicMetadataKeysSingleton::get().AccessLogKey) + .bool_value()); + } else { + EXPECT_EQ(req_info_.dynamicMetadata().filter_metadata().end(), + req_info_.dynamicMetadata().filter_metadata().find( + Filters::Common::RBAC::DynamicMetadataKeysSingleton::get().CommonNamespace)); + } + } + void setMetadata() { ON_CALL(req_info_, setDynamicMetadata(HttpFilterNames::get().Rbac, _)) .WillByDefault(Invoke([this](const std::string&, const ProtobufWkt::Struct& obj) { @@ -75,6 +94,15 @@ class RoleBasedAccessControlFilterTest : public testing::Test { Protobuf::MapPair(HttpFilterNames::get().Rbac, obj)); })); + + ON_CALL(req_info_, + setDynamicMetadata( + Filters::Common::RBAC::DynamicMetadataKeysSingleton::get().CommonNamespace, _)) + .WillByDefault(Invoke([this](const std::string&, const ProtobufWkt::Struct& obj) { + req_info_.metadata_.mutable_filter_metadata()->insert( + Protobuf::MapPair( + Filters::Common::RBAC::DynamicMetadataKeysSingleton::get().CommonNamespace, obj)); + })); } NiceMock callbacks_; @@ -82,8 +110,8 @@ class RoleBasedAccessControlFilterTest : public testing::Test { NiceMock req_info_; Stats::IsolatedStoreImpl store_; RoleBasedAccessControlFilterConfigSharedPtr config_; - RoleBasedAccessControlFilter filter_; + Network::Address::InstanceConstSharedPtr address_; std::string requested_server_name_; Http::TestRequestHeaderMapImpl headers_; @@ -92,6 +120,7 @@ class RoleBasedAccessControlFilterTest : public testing::Test { TEST_F(RoleBasedAccessControlFilterTest, Allowed) { setDestinationPort(123); + setMetadata(); EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_.decodeHeaders(headers_, false)); Http::MetadataMap metadata_map{{"metadata", "metadata"}}; @@ -102,11 +131,14 @@ TEST_F(RoleBasedAccessControlFilterTest, Allowed) { Buffer::OwnedImpl data(""); EXPECT_EQ(Http::FilterDataStatus::Continue, filter_.decodeData(data, false)); EXPECT_EQ(Http::FilterTrailersStatus::Continue, filter_.decodeTrailers(trailers_)); + + checkAccessLogMetadata(LogResult::Undecided); } TEST_F(RoleBasedAccessControlFilterTest, RequestedServerName) { setDestinationPort(999); setRequestedServerName("www.cncf.io"); + setMetadata(); EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_.decodeHeaders(headers_, false)); EXPECT_EQ(1U, config_->stats().allowed_.value()); @@ -117,10 +149,13 @@ TEST_F(RoleBasedAccessControlFilterTest, RequestedServerName) { Buffer::OwnedImpl data(""); EXPECT_EQ(Http::FilterDataStatus::Continue, filter_.decodeData(data, false)); EXPECT_EQ(Http::FilterTrailersStatus::Continue, filter_.decodeTrailers(trailers_)); + + checkAccessLogMetadata(LogResult::Undecided); } TEST_F(RoleBasedAccessControlFilterTest, Path) { setDestinationPort(999); + setMetadata(); auto headers = Http::TestRequestHeaderMapImpl{ {":method", "GET"}, @@ -129,6 +164,7 @@ TEST_F(RoleBasedAccessControlFilterTest, Path) { {":authority", "host"}, }; EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_.decodeHeaders(headers, false)); + checkAccessLogMetadata(LogResult::Undecided); } TEST_F(RoleBasedAccessControlFilterTest, Denied) { @@ -151,23 +187,65 @@ TEST_F(RoleBasedAccessControlFilterTest, Denied) { EXPECT_EQ("allowed", filter_meta.fields().at("shadow_engine_result").string_value()); EXPECT_EQ("bar", filter_meta.fields().at("shadow_effective_policy_id").string_value()); EXPECT_EQ("rbac_access_denied", callbacks_.details_); + checkAccessLogMetadata(LogResult::Undecided); } TEST_F(RoleBasedAccessControlFilterTest, RouteLocalOverride) { setDestinationPort(456); + setMetadata(); envoy::extensions::filters::http::rbac::v3::RBACPerRoute route_config; route_config.mutable_rbac()->mutable_rules()->set_action(envoy::config::rbac::v3::RBAC::DENY); NiceMock engine{route_config.rbac().rules()}; NiceMock per_route_config_{route_config}; - EXPECT_CALL(engine, allowed(_, _, _, _)).WillRepeatedly(Return(true)); + EXPECT_CALL(engine, handleAction(_, _, _, _)).WillRepeatedly(Return(true)); EXPECT_CALL(per_route_config_, engine()).WillRepeatedly(ReturnRef(engine)); EXPECT_CALL(callbacks_.route_->route_entry_, perFilterConfig(HttpFilterNames::get().Rbac)) .WillRepeatedly(Return(&per_route_config_)); EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_.decodeHeaders(headers_, true)); + checkAccessLogMetadata(LogResult::Undecided); +} + +// Log Tests +TEST_F(RoleBasedAccessControlFilterTest, ShouldLog) { + config_ = setupConfig(envoy::config::rbac::v3::RBAC::LOG); + filter_ = RoleBasedAccessControlFilter(config_); + filter_.setDecoderFilterCallbacks(callbacks_); + + setDestinationPort(123); + setMetadata(); + + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_.decodeHeaders(headers_, false)); + EXPECT_EQ(1U, config_->stats().allowed_.value()); + EXPECT_EQ(0U, config_->stats().shadow_denied_.value()); + + Buffer::OwnedImpl data(""); + EXPECT_EQ(Http::FilterDataStatus::Continue, filter_.decodeData(data, false)); + EXPECT_EQ(Http::FilterTrailersStatus::Continue, filter_.decodeTrailers(trailers_)); + + checkAccessLogMetadata(LogResult::Yes); +} + +TEST_F(RoleBasedAccessControlFilterTest, ShouldNotLog) { + config_ = setupConfig(envoy::config::rbac::v3::RBAC::LOG); + filter_ = RoleBasedAccessControlFilter(config_); + filter_.setDecoderFilterCallbacks(callbacks_); + + setDestinationPort(456); + setMetadata(); + + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_.decodeHeaders(headers_, false)); + EXPECT_EQ(1U, config_->stats().allowed_.value()); + EXPECT_EQ(0U, config_->stats().shadow_denied_.value()); + + Buffer::OwnedImpl data(""); + EXPECT_EQ(Http::FilterDataStatus::Continue, filter_.decodeData(data, false)); + EXPECT_EQ(Http::FilterTrailersStatus::Continue, filter_.decodeTrailers(trailers_)); + + checkAccessLogMetadata(LogResult::No); } } // namespace diff --git a/test/extensions/filters/network/rbac/filter_test.cc b/test/extensions/filters/network/rbac/filter_test.cc index 2e8fd2642da3..fe042854baa7 100644 --- a/test/extensions/filters/network/rbac/filter_test.cc +++ b/test/extensions/filters/network/rbac/filter_test.cc @@ -22,8 +22,10 @@ namespace RBACFilter { class RoleBasedAccessControlNetworkFilterTest : public testing::Test { public: - RoleBasedAccessControlFilterConfigSharedPtr setupConfig(bool with_policy = true, - bool continuous = false) { + RoleBasedAccessControlFilterConfigSharedPtr + setupConfig(bool with_policy = true, bool continuous = false, + envoy::config::rbac::v3::RBAC::Action action = envoy::config::rbac::v3::RBAC::ALLOW) { + envoy::extensions::filters::network::rbac::v3::RBAC config; config.set_stat_prefix("tcp."); @@ -34,7 +36,7 @@ class RoleBasedAccessControlNetworkFilterTest : public testing::Test { ".*cncf.io"); policy_rules->add_rules()->set_destination_port(123); policy.add_principals()->set_any(true); - config.mutable_rules()->set_action(envoy::config::rbac::v3::RBAC::ALLOW); + config.mutable_rules()->set_action(action); (*config.mutable_rules()->mutable_policies())["foo"] = policy; envoy::config::rbac::v3::Policy shadow_policy; @@ -42,7 +44,7 @@ class RoleBasedAccessControlNetworkFilterTest : public testing::Test { shadow_policy_rules->add_rules()->mutable_requested_server_name()->set_exact("xyz.cncf.io"); shadow_policy_rules->add_rules()->set_destination_port(456); shadow_policy.add_principals()->set_any(true); - config.mutable_shadow_rules()->set_action(envoy::config::rbac::v3::RBAC::ALLOW); + config.mutable_shadow_rules()->set_action(action); (*config.mutable_shadow_rules()->mutable_policies())["bar"] = shadow_policy; } @@ -72,6 +74,15 @@ class RoleBasedAccessControlNetworkFilterTest : public testing::Test { .WillByDefault(Return(requested_server_name_)); } + void checkAccessLogMetadata(bool expected) { + auto filter_meta = stream_info_.dynamicMetadata().filter_metadata().at( + Filters::Common::RBAC::DynamicMetadataKeysSingleton::get().CommonNamespace); + EXPECT_EQ(expected, + filter_meta.fields() + .at(Filters::Common::RBAC::DynamicMetadataKeysSingleton::get().AccessLogKey) + .bool_value()); + } + void setMetadata() { ON_CALL(stream_info_, setDynamicMetadata(NetworkFilterNames::get().Rbac, _)) .WillByDefault(Invoke([this](const std::string&, const ProtobufWkt::Struct& obj) { @@ -79,6 +90,15 @@ class RoleBasedAccessControlNetworkFilterTest : public testing::Test { Protobuf::MapPair(NetworkFilterNames::get().Rbac, obj)); })); + + ON_CALL(stream_info_, + setDynamicMetadata( + Filters::Common::RBAC::DynamicMetadataKeysSingleton::get().CommonNamespace, _)) + .WillByDefault(Invoke([this](const std::string&, const ProtobufWkt::Struct& obj) { + stream_info_.metadata_.mutable_filter_metadata()->insert( + Protobuf::MapPair( + Filters::Common::RBAC::DynamicMetadataKeysSingleton::get().CommonNamespace, obj)); + })); } NiceMock callbacks_; @@ -173,6 +193,49 @@ TEST_F(RoleBasedAccessControlNetworkFilterTest, Denied) { EXPECT_EQ("allowed", filter_meta.fields().at("shadow_engine_result").string_value()); } +// Log Tests +TEST_F(RoleBasedAccessControlNetworkFilterTest, ShouldLog) { + config_ = setupConfig(true, false, envoy::config::rbac::v3::RBAC::LOG); + filter_ = std::make_unique(config_); + filter_->initializeReadFilterCallbacks(callbacks_); + + setDestinationPort(123); + setMetadata(); + + EXPECT_EQ(Network::FilterStatus::Continue, filter_->onData(data_, false)); + EXPECT_EQ(1U, config_->stats().allowed_.value()); + EXPECT_EQ(0U, config_->stats().shadow_denied_.value()); + + checkAccessLogMetadata(true); +} + +TEST_F(RoleBasedAccessControlNetworkFilterTest, ShouldNotLog) { + config_ = setupConfig(true, false, envoy::config::rbac::v3::RBAC::LOG); + filter_ = std::make_unique(config_); + filter_->initializeReadFilterCallbacks(callbacks_); + + setDestinationPort(456); + setMetadata(); + + EXPECT_EQ(Network::FilterStatus::Continue, filter_->onData(data_, false)); + EXPECT_EQ(1U, config_->stats().allowed_.value()); + EXPECT_EQ(0U, config_->stats().shadow_denied_.value()); + + checkAccessLogMetadata(false); +} + +TEST_F(RoleBasedAccessControlNetworkFilterTest, AllowNoChangeLog) { + setDestinationPort(123); + setMetadata(); + + EXPECT_EQ(Network::FilterStatus::Continue, filter_->onData(data_, false)); + + // Check that Allow action does not set access log metadata + EXPECT_EQ(stream_info_.dynamicMetadata().filter_metadata().end(), + stream_info_.dynamicMetadata().filter_metadata().find( + Filters::Common::RBAC::DynamicMetadataKeysSingleton::get().CommonNamespace)); +} + } // namespace RBACFilter } // namespace NetworkFilters } // namespace Extensions