On Wed, Feb 2, 2022 at 8:27 PM Mark Michelson <mmich...@redhat.com> wrote:
>
> It can be desirable for replies to stateful ACLs to be logged. And in
> some cases, it can actually be a bit confusing why they aren't logged.
> Consider a situation where a port group called "port_group" exists and
> logical switch ports swp1 and swp2 belong to it. We create the following
> ACL, where logging is enabled:
>
> from-lport 100 'inport == @port_group' allow-stateless
>
> swp1 sends traffic to swp2 and swp2 responds within the same connection.
> In this case, the logs will show both the packets from swp1 to swp2, as
> well as the response packets from swp2 to swp1.
>
> Now change the ACL:
>
> from-lport 100 'inport == @port_group' allow-related
>
> Now with the same traffic pattern, the packets from swp1 to swp2 are
> logged, but the packets from swp2 to swp1 are not. Why is that?
>
> The reason is that as a shortcut, when stateful ACLs are used, a single
> priority 65532 flow is programmed to allow reply traffic to pass. When
> no stateful ACL is present, the reply traffic is at the mercy of the
> stateless ACL on the reply. Therefore, with the stateful ACL, the reply
> traffic is not actually hitting an ACL but is let through by default,
> but with the stateless ACL, the reply traffic does hit the ACL
> evaluation, so the reply traffic is logged.
>
> This change adds a feature that allows for reply traffic to be
> optionally logged for stateful ACLs, therefore allowing for the behavior
> to be similar for both ACL types. Since logging reply traffic requires
> adding more flows, it is not enabled by default. In order to have reply
> traffic logged, the ACL must have logging enabled, be stateful, have a
> label, and have the new log_related column set to true,
>
> Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2031150
>
> Signed-off-by: Mark Michelson <mmich...@redhat.com>
Hi Mark,
The patch LGTM.
Can you please update the documentation in ovn-northd.8.xml for the
new flows added.
Instead of adding a new column in the ACL table for the log direction,
how about adding options column ?
I'd suggest a generic options column.
Thanks
Numan
> ---
> v2 -> v3:
> * Seeing Dumitru's ACL direction patch, I realized check_acl_log
> shouldn't be hard-coded to think "severity" is the final ACL field.
> Tweaked script to be more resilient in the face of change.
> * Added a few more (possibly) common acl_log packet fields to check.
> * Fixed errors in system-ovn.at where I was checking the icmp_type
> instead of the icmp_code
> * Updated commit message to mention that a label is required on the ACL.
> ---
> northd/northd.c | 41 +++++
> ovn-nb.ovsschema | 5 +-
> ovn-nb.xml | 10 ++
> tests/automake.mk | 3 +-
> tests/check_acl_log.py | 107 ++++++++++++
> tests/ovn-northd.at | 253 +++++++++++++++++++++++++++++
> tests/system-ovn.at | 359 +++++++++++++++++++++++++++++++++++++++++
> 7 files changed, 775 insertions(+), 3 deletions(-)
> create mode 100644 tests/check_acl_log.py
>
> diff --git a/northd/northd.c b/northd/northd.c
> index 22e783ff6..c1a8f71f5 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -6242,6 +6242,47 @@ consider_acl(struct hmap *lflows, struct ovn_datapath
> *od,
> acl->priority + OVN_ACL_PRI_OFFSET,
> ds_cstr(match), ds_cstr(actions),
> &acl->header_);
> +
> + /* Related and reply traffic are universally allowed by priority
> + * 65532 flows created in build_acls(). If logging is enabled on
> + * the ACL, then we need to ensure that the related and reply
> + * traffic is logged, so we install a slightly higher-priority
> + * flow that matches the ACL, allows the traffic, and logs it.
> + */
> + if (acl->log && acl->label && acl->log_related) {
> + /* Related/reply flows need to be set on the opposite
> pipeline
> + * from where the ACL itself is set.
> + */
> + enum ovn_stage log_related_stage = ingress ?
> + S_SWITCH_OUT_ACL :
> + S_SWITCH_IN_ACL;
> + ds_clear(match);
> + ds_clear(actions);
> +
> + ds_put_format(match, "ct.est && !ct.rel && !ct.new%s && "
> + "ct.rpl && ct_label.blocked == 0 && "
> + "ct_label.label == %" PRId64,
> + use_ct_inv_match ? " && !ct.inv" : "",
> + acl->label);
> + build_acl_log(actions, acl, meter_groups);
> + ds_put_cstr(actions, "next;");
> + ovn_lflow_add_with_hint(lflows, od, log_related_stage,
> + UINT16_MAX - 2,
> + ds_cstr(match), ds_cstr(actions),
> + &acl->header_);
> +
> + ds_clear(match);
> + ds_put_format(match, "!ct.est && ct.rel && !ct.new%s && "
> + "ct_label.blocked == 0 && "
> + "ct_label.label == %" PRId64,
> + use_ct_inv_match ? " && !ct.inv" : "",
> + acl->label);
> + ovn_lflow_add_with_hint(lflows, od, log_related_stage,
> + UINT16_MAX - 2,
> + ds_cstr(match), ds_cstr(actions),
> + &acl->header_);
> + }
> +
> }
> } else if (!strcmp(acl->action, "drop")
> || !strcmp(acl->action, "reject")) {
> diff --git a/ovn-nb.ovsschema b/ovn-nb.ovsschema
> index 55977339a..b72a5bf63 100644
> --- a/ovn-nb.ovsschema
> +++ b/ovn-nb.ovsschema
> @@ -1,7 +1,7 @@
> {
> "name": "OVN_Northbound",
> - "version": "5.34.1",
> - "cksum": "2177334725 30782",
> + "version": "5.35.0",
> + "cksum": "2223177829 30834",
> "tables": {
> "NB_Global": {
> "columns": {
> @@ -258,6 +258,7 @@
> "notice", "info",
> "debug"]]},
> "min": 0, "max": 1}},
> + "log_related": {"type": "boolean"},
> "meter": {"type": {"key": "string", "min": 0, "max": 1}},
> "label": {"type": {"key": {"type": "integer",
> "minInteger": 0,
> diff --git a/ovn-nb.xml b/ovn-nb.xml
> index 6a6972856..154bd79e8 100644
> --- a/ovn-nb.xml
> +++ b/ovn-nb.xml
> @@ -2061,6 +2061,16 @@
> separately, set the <ref column="fair" table="meter"/> column.
> </p>
> </column>
> +
> + <column name="log_related">
> + <p>
> + If set to <code>true</code>, then log when reply or related
> + traffic is admitted from a stateful ACL. In order for this
> + option to function, the <ref column="log"/> option must be
> + set to <code>true</code> and a <ref column="label"/> must
> + be set.
> + </p>
> + </column>
> </group>
>
> <group title="Common Columns">
> diff --git a/tests/automake.mk b/tests/automake.mk
> index a08dfa632..a5934d2b9 100644
> --- a/tests/automake.mk
> +++ b/tests/automake.mk
> @@ -270,7 +270,8 @@ tests_ovstest_LDADD = $(OVS_LIBDIR)/daemon.lo \
> CHECK_PYFILES = \
> tests/test-l7.py \
> tests/uuidfilt.py \
> - tests/test-tcp-rst.py
> + tests/test-tcp-rst.py \
> + tests/check_acl_log.py
>
> EXTRA_DIST += $(CHECK_PYFILES)
> PYCOV_CLEAN_FILES += $(CHECK_PYFILES:.py=.py,cover) .coverage
> diff --git a/tests/check_acl_log.py b/tests/check_acl_log.py
> new file mode 100644
> index 000000000..1dd9630c0
> --- /dev/null
> +++ b/tests/check_acl_log.py
> @@ -0,0 +1,107 @@
> +#!/usr/bin/env python3
> +import argparse
> +import string
> +
> +
> +def strip(val):
> + """Strip whitespace and quotation marks from val"""
> + return val.strip(f"{string.whitespace}\"'")
> +
> +
> +def parse_acl_log(line):
> + """Convert an ACL log string into a dict"""
> + # First cut off the logging preamble.
> + # We're assuming the default log format.
> + acl_log = {}
> + _, _, details = line.rpartition("|")
> +
> + # acl_details are things like the acl name, direction,
> + # verdict, and severity. packet_details are things like
> + # the protocol, addresses, and ports of the packet being
> + # logged.
> + acl_details, _, packet_details = details.partition(":")
> + for datum in acl_details.split(","):
> + name, _, value = datum.rpartition("=")
> + acl_log[strip(name)] = strip(value)
> +
> + for datum in packet_details.split(","):
> + name, _, value = datum.rpartition("=")
> + if not name:
> + # The protocol is not preceded by "protocol="
> + # so we need to add it manually.
> + name = "protocol"
> + acl_log[strip(name)] = strip(value)
> +
> + return acl_log
> +
> +
> +def get_acl_log(entry_num=1):
> + with open("ovn-controller.log", "r") as controller_log:
> + acl_logs = [line for line in controller_log if "acl_log" in line]
> + try:
> + return acl_logs[entry_num - 1]
> + except IndexError:
> + print(
> + f"There were not {entry_num} acl_log entries, \
> + only {len(acl_logs)}"
> + )
> + exit(1)
> +
> +
> +def add_parser_args(parser):
> + parser.add_argument("--entry-num", type=int, default=1)
> +
> + # There are other possible things that can be in an ACL log,
> + # and if we need those in the future, we can add them later.
> + parser.add_argument("--name")
> + parser.add_argument("--verdict")
> + parser.add_argument("--severity")
> + parser.add_argument("--protocol")
> + parser.add_argument("--vlan_tci")
> + parser.add_argument("--dl_src")
> + parser.add_argument("--dl_dst")
> + parser.add_argument("--nw_src")
> + parser.add_argument("--nw_dst")
> + parser.add_argument("--nw_tos")
> + parser.add_argument("--nw_ecn")
> + parser.add_argument("--nw_ttl")
> + parser.add_argument("--icmp_type")
> + parser.add_argument("--icmp_code")
> + parser.add_argument("--tp_src")
> + parser.add_argument("--tp_dst")
> + parser.add_argument("--tcp_flags")
> + parser.add_argument("--ipv6_src")
> + parser.add_argument("--ipv6_dst")
> +
> +
> +def main():
> + parser = argparse.ArgumentParser()
> + add_parser_args(parser)
> + args = parser.parse_args()
> +
> + acl_log = get_acl_log(args.entry_num)
> + parsed_log = parse_acl_log(acl_log)
> +
> + # Express command line arguments as a dict, omitting any arguments that
> + # were not provided by the user.
> + expected = {k: v for k, v in vars(args).items() if v is not None}
> + del expected["entry_num"]
> +
> + for key, val in expected.items():
> + try:
> + if parsed_log[key] != val:
> + print(
> + f"Expected log {key}={val} but got
> {key}={parsed_log[key]} \
> + in:\n\t'{acl_log}"
> + )
> + exit(1)
> + except KeyError:
> + print(
> + f"Expected log {key}={val} but {key} does not exist \
> + in:\n\t'{acl_log}'"
> + )
> + exit(1)
> +
> +
> +if __name__ == "__main__":
> + main()
> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> index 652903761..060f2af0a 100644
> --- a/tests/ovn-northd.at
> +++ b/tests/ovn-northd.at
> @@ -5888,5 +5888,258 @@ AT_CHECK([grep -e "(lr_in_ip_routing ).*outport"
> lr0flows | sed 's/table=../ta
> table=??(lr_in_ip_routing ), priority=97 , match=(reg7 == 2 && ip4.dst
> == 1.1.1.1/32), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 = 192.168.0.20;
> reg1 = 192.168.0.1; eth.src = 00:00:00:00:00:01; outport = "lrp0";
> flags.loopback = 1; next;)
> ])
>
> +AT_CLEANUP
> +])
> +
> +OVN_FOR_EACH_NORTHD([
> +AT_SETUP([ACL log replies -- flows])
> +
> +set_acl_options() {
> + local acl_name=$1
> + local label=$2
> + local log_related=$3
> +
> + local acl_uuid=$(fetch_column nb:ACL _uuid name=$acl_name)
> + check ovn-nbctl set ACL $acl_uuid label=$label log_related=$log_related
> +}
> +
> +record_log_flows() {
> + ovn-sbctl lflow-list sw0 | grep -E 'ls_(out|in)_acl.*, priority=65533' |
> sed 's/table=../table=??/' | sort > log_flows
> +}
> +
> +check_log_flows_count() {
> + local expected=$1
> + local table=$2
> + local count=
> +
> + echo $table
> + if test -f log_flows; then
> + count=$(grep -c -E ls_${table}_acl log_flows)
> + else
> + count=$(ovn-sbctl lflow-list sw0 | grep -c -E "ls_$table_acl.*,
> priority=65533")
> + fi
> +
> + check test "$count" -eq "$expected"
> +}
> +
> +ovn_start
> +
> +check ovn-nbctl ls-add sw0
> +check ovn-nbctl lsp-add sw0 sw0-p1 -- lsp-set-addresses sw0-p1
> "00:00:00:00:00:01 10.0.0.1"
> +check ovn-nbctl lsp-add sw0 sw0-p2 -- lsp-set-addresses sw0-p2
> "00:00:00:00:00:02 10.0.0.2"
> +
> +check ovn-nbctl pg-add pg1 sw0-p1 sw0-p2
> +check ovn-nbctl pg-add pg2 sw0-p1 sw0-p2
> +check ovn-nbctl pg-add pg3 sw0-p1 sw0-p2
> +
> +check ovn-nbctl --log --name=allow_acl acl-add pg1 from-lport 100
> 'inport=@pg1 && ip4' allow
> +set_acl_options allow_acl 1 true
> +
> +check ovn-nbctl --wait=sb sync
> +
> +# An allow ACL should *not* result in a priority 65533 log flow being
> installed
> +# since there are no stateful ACLs on the system.
> +check_log_flows_count 0 in
> +check_log_flows_count 0 out
> +
> +# Now add an allow-related ACL. This should result in both the allow-related
> +# ACL and the allow ACL having priority 65533 log flows added.
> +check ovn-nbctl --log --name=allow_related_acl acl-add pg2 from-lport 100
> 'inport=@pg2 && ip4' allow-related
> +set_acl_options allow_related_acl 2 true
> +check ovn-nbctl --wait=sb sync
> +
> +record_log_flows
> +
> +# The count will be 4 since we have
> +# 2 flows for reply traffic for each ACL
> +# 2 flows for related traffic for each ACL
> +check_log_flows_count 4 out
> +# Since the ACLs are ingress, the ingress table
> +# should have no log flows
> +check_log_flows_count 0 in
> +
> +# Now ensure the flows are what we expect them to be for the ACLs we created
> +AT_CHECK([cat log_flows], [0], [dnl
> + table=??(ls_out_acl ), priority=65533, match=(!ct.est && ct.rel &&
> !ct.new && !ct.inv && ct_label.blocked == 0 && ct_label.label == 1),
> action=(log(name="allow_acl", severity=info, verdict=allow); next;)
> + table=??(ls_out_acl ), priority=65533, match=(!ct.est && ct.rel &&
> !ct.new && !ct.inv && ct_label.blocked == 0 && ct_label.label == 2),
> action=(log(name="allow_related_acl", severity=info, verdict=allow); next;)
> + table=??(ls_out_acl ), priority=65533, match=(ct.est && !ct.rel &&
> !ct.new && !ct.inv && ct.rpl && ct_label.blocked == 0 && ct_label.label ==
> 1), action=(log(name="allow_acl", severity=info, verdict=allow); next;)
> + table=??(ls_out_acl ), priority=65533, match=(ct.est && !ct.rel &&
> !ct.new && !ct.inv && ct.rpl && ct_label.blocked == 0 && ct_label.label ==
> 2), action=(log(name="allow_related_acl", severity=info, verdict=allow);
> next;)
> +])
> +
> +rm log_flows
> +
> +# Now add a stateless-allow ACL.
> +check ovn-nbctl --log --name=allow_stateless_acl acl-add pg3 from-lport 100
> 'inport=@pg3 && ip4' allow-stateless
> +set_acl_options allow_stateless_acl 3 true
> +check ovn-nbctl --wait=sb sync
> +
> +record_log_flows
> +
> +# The count will still be 4 since the stateless ACL should not have special
> log flows created
> +check_log_flows_count 4 out
> +check_log_flows_count 0 in
> +
> +# And the log flows will remain the same since the stateless ACL will not be
> represented.
> +AT_CHECK([cat log_flows], [0], [dnl
> + table=??(ls_out_acl ), priority=65533, match=(!ct.est && ct.rel &&
> !ct.new && !ct.inv && ct_label.blocked == 0 && ct_label.label == 1),
> action=(log(name="allow_acl", severity=info, verdict=allow); next;)
> + table=??(ls_out_acl ), priority=65533, match=(!ct.est && ct.rel &&
> !ct.new && !ct.inv && ct_label.blocked == 0 && ct_label.label == 2),
> action=(log(name="allow_related_acl", severity=info, verdict=allow); next;)
> + table=??(ls_out_acl ), priority=65533, match=(ct.est && !ct.rel &&
> !ct.new && !ct.inv && ct.rpl && ct_label.blocked == 0 && ct_label.label ==
> 1), action=(log(name="allow_acl", severity=info, verdict=allow); next;)
> + table=??(ls_out_acl ), priority=65533, match=(ct.est && !ct.rel &&
> !ct.new && !ct.inv && ct.rpl && ct_label.blocked == 0 && ct_label.label ==
> 2), action=(log(name="allow_related_acl", severity=info, verdict=allow);
> next;)
> +])
> +
> +rm log_flows
> +
> +# Now remove the label from the allow-related ACL.
> +set_acl_options allow_related_acl 0 true
> +ovn-nbctl --wait=sb sync
> +
> +record_log_flows
> +
> +# The count should now be 2 since the allow_related ACL will not have special
> +# log flows created. But since there there is an allow-related ACL present,
> the
> +# allow ACL will be stateful and have special log flows created.
> +check_log_flows_count 2 out
> +check_log_flows_count 0 in
> +
> +# And make sure only the allow ACL has the log flows installed
> +AT_CHECK([cat log_flows], [0], [dnl
> + table=??(ls_out_acl ), priority=65533, match=(!ct.est && ct.rel &&
> !ct.new && !ct.inv && ct_label.blocked == 0 && ct_label.label == 1),
> action=(log(name="allow_acl", severity=info, verdict=allow); next;)
> + table=??(ls_out_acl ), priority=65533, match=(ct.est && !ct.rel &&
> !ct.new && !ct.inv && ct.rpl && ct_label.blocked == 0 && ct_label.label ==
> 1), action=(log(name="allow_acl", severity=info, verdict=allow); next;)
> +])
> +
> +rm log_flows
> +
> +# And now add the label back, but disable log_related on the allow-related
> ACL.
> +set_acl_options allow_related_acl 2 false
> +
> +record_log_flows
> +
> +# The count will again be 2 because only the allow ACL will have log flows
> installed.
> +check_log_flows_count 2 out
> +check_log_flows_count 0 in
> +
> +# And make sure only the allow ACL has the log flows installed
> +AT_CHECK([cat log_flows], [0], [dnl
> + table=??(ls_out_acl ), priority=65533, match=(!ct.est && ct.rel &&
> !ct.new && !ct.inv && ct_label.blocked == 0 && ct_label.label == 1),
> action=(log(name="allow_acl", severity=info, verdict=allow); next;)
> + table=??(ls_out_acl ), priority=65533, match=(ct.est && !ct.rel &&
> !ct.new && !ct.inv && ct.rpl && ct_label.blocked == 0 && ct_label.label ==
> 1), action=(log(name="allow_acl", severity=info, verdict=allow); next;)
> +])
> +
> +rm log_flows
> +
> +# And just for sanity's sake, let's remove the allow-related ACL and make
> sure
> +# all the special log messages are gone.
> +check ovn-nbctl acl-del pg2
> +check ovn-nbctl --wait=sb sync
> +
> +check_log_flows_count 0 out
> +check_log_flows_count 0 in
> +
> +# Now let's clear out all the ACLs, and re-do everything but with egress
> ACLs.
> +check ovn-nbctl acl-del pg1
> +check ovn-nbctl acl-del pg3
> +check_row_count nb:ACL 0
> +
> +# Start again with an allow_acl only
> +check ovn-nbctl --log --name=allow_acl acl-add pg1 to-lport 100 'inport=@pg1
> && ip4' allow
> +set_acl_options allow_acl 1 true
> +
> +check ovn-nbctl --wait=sb sync
> +
> +# Again, the allow ACL is stateless, so no related log flows.
> +check_log_flows_count 0 in
> +check_log_flows_count 0 out
> +
> +# Adding a new allow-related ACL...
> +check ovn-nbctl --log --name=allow_related_acl acl-add pg2 to-lport 100
> 'inport=@pg2 && ip4' allow-related
> +set_acl_options allow_related_acl 2 true
> +check ovn-nbctl --wait=sb sync
> +
> +record_log_flows
> +
> +# The count will be 4 since we have
> +# 2 flows for reply traffic for each ACL
> +# 2 flows for related traffic for each ACL
> +check_log_flows_count 4 in
> +# And this time, we should have no egress flows
> +check_log_flows_count 0 out
> +
> +# Now ensure the flows are what we expect them to be for the ACLs we created
> +AT_CHECK([cat log_flows], [0], [dnl
> + table=??(ls_in_acl ), priority=65533, match=(!ct.est && ct.rel &&
> !ct.new && !ct.inv && ct_label.blocked == 0 && ct_label.label == 1),
> action=(log(name="allow_acl", severity=info, verdict=allow); next;)
> + table=??(ls_in_acl ), priority=65533, match=(!ct.est && ct.rel &&
> !ct.new && !ct.inv && ct_label.blocked == 0 && ct_label.label == 2),
> action=(log(name="allow_related_acl", severity=info, verdict=allow); next;)
> + table=??(ls_in_acl ), priority=65533, match=(ct.est && !ct.rel &&
> !ct.new && !ct.inv && ct.rpl && ct_label.blocked == 0 && ct_label.label ==
> 1), action=(log(name="allow_acl", severity=info, verdict=allow); next;)
> + table=??(ls_in_acl ), priority=65533, match=(ct.est && !ct.rel &&
> !ct.new && !ct.inv && ct.rpl && ct_label.blocked == 0 && ct_label.label ==
> 2), action=(log(name="allow_related_acl", severity=info, verdict=allow);
> next;)
> +])
> +
> +rm log_flows
> +
> +# Now add a stateless-allow ACL.
> +check ovn-nbctl --log --name=allow_stateless_acl acl-add pg3 from-lport 100
> 'inport=@pg3 && ip4' allow-stateless
> +set_acl_options allow_stateless_acl 3 true
> +check ovn-nbctl --wait=sb sync
> +
> +record_log_flows
> +
> +# The count will still be 4 since the stateless ACL should not have special
> log flows created
> +check_log_flows_count 4 in
> +check_log_flows_count 0 out
> +
> +# And the log flows will remain the same since the stateless ACL will not be
> represented.
> +AT_CHECK([cat log_flows], [0], [dnl
> + table=??(ls_in_acl ), priority=65533, match=(!ct.est && ct.rel &&
> !ct.new && !ct.inv && ct_label.blocked == 0 && ct_label.label == 1),
> action=(log(name="allow_acl", severity=info, verdict=allow); next;)
> + table=??(ls_in_acl ), priority=65533, match=(!ct.est && ct.rel &&
> !ct.new && !ct.inv && ct_label.blocked == 0 && ct_label.label == 2),
> action=(log(name="allow_related_acl", severity=info, verdict=allow); next;)
> + table=??(ls_in_acl ), priority=65533, match=(ct.est && !ct.rel &&
> !ct.new && !ct.inv && ct.rpl && ct_label.blocked == 0 && ct_label.label ==
> 1), action=(log(name="allow_acl", severity=info, verdict=allow); next;)
> + table=??(ls_in_acl ), priority=65533, match=(ct.est && !ct.rel &&
> !ct.new && !ct.inv && ct.rpl && ct_label.blocked == 0 && ct_label.label ==
> 2), action=(log(name="allow_related_acl", severity=info, verdict=allow);
> next;)
> +])
> +
> +rm log_flows
> +
> +# Now remove the label from the allow-related ACL.
> +set_acl_options allow_related_acl 0 true
> +ovn-nbctl --wait=sb sync
> +
> +record_log_flows
> +
> +# The count should now be 2 since the allow_related ACL will not have special
> +# log flows created. But since there there is an allow-related ACL present,
> the
> +# allow ACL will be stateful and have special log flows created.
> +check_log_flows_count 2 in
> +check_log_flows_count 0 out
> +
> +# And make sure only the allow ACL has the log flows installed
> +AT_CHECK([cat log_flows], [0], [dnl
> + table=??(ls_in_acl ), priority=65533, match=(!ct.est && ct.rel &&
> !ct.new && !ct.inv && ct_label.blocked == 0 && ct_label.label == 1),
> action=(log(name="allow_acl", severity=info, verdict=allow); next;)
> + table=??(ls_in_acl ), priority=65533, match=(ct.est && !ct.rel &&
> !ct.new && !ct.inv && ct.rpl && ct_label.blocked == 0 && ct_label.label ==
> 1), action=(log(name="allow_acl", severity=info, verdict=allow); next;)
> +])
> +
> +rm log_flows
> +
> +# And now add the label back, but disable log_related on the allow-related
> ACL.
> +set_acl_options allow_related_acl 2 false
> +
> +record_log_flows
> +
> +# The count will again be 2 because only the allow ACL will have log flows
> installed.
> +check_log_flows_count 2 in
> +check_log_flows_count 0 out
> +
> +# And make sure only the allow ACL has the log flows installed
> +AT_CHECK([cat log_flows], [0], [dnl
> + table=??(ls_in_acl ), priority=65533, match=(!ct.est && ct.rel &&
> !ct.new && !ct.inv && ct_label.blocked == 0 && ct_label.label == 1),
> action=(log(name="allow_acl", severity=info, verdict=allow); next;)
> + table=??(ls_in_acl ), priority=65533, match=(ct.est && !ct.rel &&
> !ct.new && !ct.inv && ct.rpl && ct_label.blocked == 0 && ct_label.label ==
> 1), action=(log(name="allow_acl", severity=info, verdict=allow); next;)
> +])
> +
> +rm log_flows
> +
> +# And just for sanity's sake, let's remove the allow-related ACL and make
> sure
> +# all the special log messages are gone.
> +check ovn-nbctl acl-del pg2
> +check ovn-nbctl --wait=sb sync
> +
> +check_log_flows_count 0 out
> +check_log_flows_count 0 in
> +
> +
> +
> AT_CLEANUP
> ])
> diff --git a/tests/system-ovn.at b/tests/system-ovn.at
> index 3ae812296..9acd97ea5 100644
> --- a/tests/system-ovn.at
> +++ b/tests/system-ovn.at
> @@ -7002,3 +7002,362 @@ OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port
> patch-.*/d
>
> AT_CLEANUP
> ])
> +
> +OVN_FOR_EACH_NORTHD([
> +AT_SETUP([ACL log_related])
> +
> +CHECK_CONNTRACK()
> +ovn_start
> +
> +OVS_TRAFFIC_VSWITCHD_START()
> +ADD_BR([br-int])
> +
> +set_acl_options() {
> + local acl_name=$1; shift
> +
> + local acl_uuid=$(fetch_column nb:ACL _uuid name=$acl_name)
> + check ovn-nbctl set ACL $acl_uuid "$@"
> +}
> +
> +clear_log() {
> + ovn-appctl -t ovn-controller vlog/close
> + rm ovn-controller.log
> + ovn-appctl -t ovn-controller vlog/reopen
> +}
> +
> +test_ping() {
> + NS_CHECK_EXEC([sw0-p1], [ping -q -c 1 -i 0.3 -w 2 10.0.0.2 |
> FORMAT_PING], \
> +[0], [dnl
> +1 packets transmitted, 1 received, 0% packet loss, time 0ms
> +])
> +}
> +
> +check_acl_log_count() {
> + local expected_count=$1
> +
> + AT_CHECK_UNQUOTED([grep -c acl_log ovn-controller.log], [0], [dnl
> +$expected_count
> +])
> +}
> +
> +# Set external-ids in br-int needed for ovn-controller
> +ovs-vsctl \
> + -- set Open_vSwitch . external-ids:system-id=hv1 \
> + -- set Open_vSwitch .
> external-ids:ovn-remote=unix:$ovs_base/ovn-sb/ovn-sb.sock \
> + -- set Open_vSwitch . external-ids:ovn-encap-type=geneve \
> + -- set Open_vSwitch . external-ids:ovn-encap-ip=169.0.0.1 \
> + -- set bridge br-int fail-mode=secure
> other-config:disable-in-band=true
> +
> +# Start ovn-controller
> +start_daemon ovn-controller
> +
> +check ovn-nbctl ls-add sw0
> +check ovn-nbctl lsp-add sw0 sw0-p1 -- lsp-set-addresses sw0-p1
> "00:00:00:00:00:01 10.0.0.1"
> +check ovn-nbctl lsp-add sw0 sw0-p2 -- lsp-set-addresses sw0-p2
> "00:00:00:00:00:02 10.0.0.2"
> +
> +check ovn-nbctl pg-add pg1 sw0-p1 sw0-p2
> +
> +ADD_NAMESPACES(sw0-p1)
> +ADD_VETH(sw0-p1, sw0-p1, br-int, "10.0.0.1/24", "00:00:00:00:00:01")
> +ADD_NAMESPACES(sw0-p2)
> +ADD_VETH(sw0-p2, sw0-p2, br-int, "10.0.0.2/24", "00:00:00:00:00:02")
> +
> +wait_for_ports_up
> +
> +check ovn-nbctl --log --name=allow_acl acl-add pg1 from-lport 100 'inport ==
> @pg1 && ip4' allow
> +
> +check ovn-nbctl --wait=hv sync
> +
> +test_ping
> +
> +# The allow ACL should match on the request and reply traffic, resulting in
> 2 logs.
> +check_acl_log_count 2
> +
> +check $PYTHON $srcdir/check_acl_log.py \
> + --entry-num=1 \
> + --name=allow_acl \
> + --verdict=allow \
> + --protocol=icmp \
> + --dl_src=00:00:00:00:00:01 \
> + --dl_dst=00:00:00:00:00:02 \
> + --nw_src=10.0.0.1 \
> + --nw_dst=10.0.0.2 \
> + --icmp_type=8 \
> + --icmp_code=0
> +
> +check $PYTHON $srcdir/check_acl_log.py \
> + --entry-num=2 \
> + --name=allow_acl \
> + --verdict=allow \
> + --protocol=icmp \
> + --dl_src=00:00:00:00:00:02 \
> + --dl_dst=00:00:00:00:00:01 \
> + --nw_src=10.0.0.2 \
> + --nw_dst=10.0.0.1 \
> + --icmp_type=0 \
> + --icmp_code=0
> +
> +# Now add a higher-priority stateful ACL that matches on the same
> +# parameters. Don't enable reply logging.
> +check ovn-nbctl --log --name=allow_related_acl acl-add pg1 from-lport 200
> 'inport == @pg1 && ip4' allow-related
> +check ovn-nbctl --wait=hv sync
> +
> +clear_log
> +test_ping
> +
> +# Since reply logging is not enabled, the allow-related ACL should match on
> the
> +# request, but the reply will not be logged.
> +check_acl_log_count 1
> +
> +check $PYTHON $srcdir/check_acl_log.py \
> + --entry-num=1 \
> + --name=allow_related_acl \
> + --verdict=allow \
> + --protocol=icmp \
> + --dl_src=00:00:00:00:00:01 \
> + --dl_dst=00:00:00:00:00:02 \
> + --nw_src=10.0.0.1 \
> + --nw_dst=10.0.0.2 \
> + --icmp_type=8 \
> + --icmp_code=0
> +
> +# As a control, set a label on the allow-related ACL, but still don't enable
> +# reply traffic logging.
> +set_acl_options allow_related_acl label=1 log_related=false
> +check ovn-nbctl --wait=hv sync
> +
> +clear_log
> +test_ping
> +
> +# This should have the same result as the previous ping
> +check_acl_log_count 1
> +
> +check $PYTHON $srcdir/check_acl_log.py \
> + --entry-num=1 \
> + --name=allow_related_acl \
> + --verdict=allow \
> + --protocol=icmp \
> + --dl_src=00:00:00:00:00:01 \
> + --dl_dst=00:00:00:00:00:02 \
> + --nw_src=10.0.0.1 \
> + --nw_dst=10.0.0.2 \
> + --icmp_type=8 \
> + --icmp_code=0
> +
> +# As another control, remove the label but enable reply logging.
> +set_acl_options allow_related_acl label=0 log_related=true
> +check ovn-nbctl --wait=hv sync
> +
> +clear_log
> +test_ping
> +
> +# This should have the same result as the previous ping
> +check_acl_log_count 1
> +
> +check $PYTHON $srcdir/check_acl_log.py \
> + --entry-num=1 \
> + --name=allow_related_acl \
> + --verdict=allow \
> + --protocol=icmp \
> + --dl_src=00:00:00:00:00:01 \
> + --dl_dst=00:00:00:00:00:02 \
> + --nw_src=10.0.0.1 \
> + --nw_dst=10.0.0.2 \
> + --icmp_type=8 \
> + --icmp_code=0
> +
> +# This time, add a label and enable reply logging on the allow_related ACL.
> +set_acl_options allow_related_acl label=1 log_related=true
> +check ovn-nbctl --wait=hv sync
> +
> +clear_log
> +test_ping
> +
> +# Now we should have the request and reply logged.
> +check_acl_log_count 2
> +
> +check $PYTHON $srcdir/check_acl_log.py \
> + --entry-num=1 \
> + --name=allow_related_acl \
> + --verdict=allow \
> + --protocol=icmp \
> + --dl_src=00:00:00:00:00:01 \
> + --dl_dst=00:00:00:00:00:02 \
> + --nw_src=10.0.0.1 \
> + --nw_dst=10.0.0.2 \
> + --icmp_type=8 \
> + --icmp_code=0
> +
> +check $PYTHON $srcdir/check_acl_log.py \
> + --entry-num=2 \
> + --name=allow_related_acl \
> + --verdict=allow \
> + --protocol=icmp \
> + --dl_src=00:00:00:00:00:02 \
> + --dl_dst=00:00:00:00:00:01 \
> + --nw_src=10.0.0.2 \
> + --nw_dst=10.0.0.1 \
> + --icmp_type=0 \
> + --icmp_code=0
> +
> +
> +# And now, let's start from scratch but make sure everything works when
> +# using egress ACLs.
> +check ovn-nbctl acl-del pg1
> +check_row_count nb:ACL 0
> +
> +check ovn-nbctl --log --name=allow_acl acl-add pg1 to-lport 100 'outport ==
> @pg1 && ip4' allow
> +
> +check ovn-nbctl --wait=hv sync
> +
> +clear_log
> +test_ping
> +
> +# The allow ACL should match on the request and reply traffic, resulting in
> 2 logs.
> +check_acl_log_count 2
> +
> +check $PYTHON $srcdir/check_acl_log.py \
> + --entry-num=1 \
> + --name=allow_acl \
> + --verdict=allow \
> + --protocol=icmp \
> + --dl_src=00:00:00:00:00:01 \
> + --dl_dst=00:00:00:00:00:02 \
> + --nw_src=10.0.0.1 \
> + --nw_dst=10.0.0.2 \
> + --icmp_type=8 \
> + --icmp_code=0
> +
> +check $PYTHON $srcdir/check_acl_log.py \
> + --entry-num=2 \
> + --name=allow_acl \
> + --verdict=allow \
> + --protocol=icmp \
> + --dl_src=00:00:00:00:00:02 \
> + --dl_dst=00:00:00:00:00:01 \
> + --nw_src=10.0.0.2 \
> + --nw_dst=10.0.0.1 \
> + --icmp_type=0 \
> + --icmp_code=0
> +
> +# Now add a higher-priority stateful ACL that matches on the same
> +# parameters. Don't enable reply logging.
> +check ovn-nbctl --log --name=allow_related_acl acl-add pg1 to-lport 200
> 'outport == @pg1 && ip4' allow-related
> +check ovn-nbctl --wait=hv sync
> +
> +clear_log
> +test_ping
> +
> +# Since reply logging is not enabled, the allow-related ACL should match on
> the
> +# request, but the reply will not be logged.
> +check_acl_log_count 1
> +
> +check $PYTHON $srcdir/check_acl_log.py \
> + --entry-num=1 \
> + --name=allow_related_acl \
> + --verdict=allow \
> + --protocol=icmp \
> + --dl_src=00:00:00:00:00:01 \
> + --dl_dst=00:00:00:00:00:02 \
> + --nw_src=10.0.0.1 \
> + --nw_dst=10.0.0.2 \
> + --icmp_type=8 \
> + --icmp_code=0
> +
> +# As a control, set a label on the allow-related ACL, but still don't enable
> +# reply traffic logging.
> +set_acl_options allow_related_acl label=1 log_related=false
> +check ovn-nbctl --wait=hv sync
> +
> +clear_log
> +test_ping
> +
> +# This should have the same result as the previous ping
> +check_acl_log_count 1
> +
> +check $PYTHON $srcdir/check_acl_log.py \
> + --entry-num=1 \
> + --name=allow_related_acl \
> + --verdict=allow \
> + --protocol=icmp \
> + --dl_src=00:00:00:00:00:01 \
> + --dl_dst=00:00:00:00:00:02 \
> + --nw_src=10.0.0.1 \
> + --nw_dst=10.0.0.2 \
> + --icmp_type=8 \
> + --icmp_code=0
> +
> +# As another control, remove the label but enable reply logging.
> +set_acl_options allow_related_acl label=0 log_related=true
> +check ovn-nbctl --wait=hv sync
> +
> +clear_log
> +test_ping
> +
> +# This should have the same result as the previous ping
> +check_acl_log_count 1
> +
> +check $PYTHON $srcdir/check_acl_log.py \
> + --entry-num=1 \
> + --name=allow_related_acl \
> + --verdict=allow \
> + --protocol=icmp \
> + --dl_src=00:00:00:00:00:01 \
> + --dl_dst=00:00:00:00:00:02 \
> + --nw_src=10.0.0.1 \
> + --nw_dst=10.0.0.2 \
> + --icmp_type=8 \
> + --icmp_code=0
> +
> +# This time, add a label and enable reply logging on the allow_related ACL.
> +set_acl_options allow_related_acl label=1 log_related=true
> +check ovn-nbctl --wait=hv sync
> +
> +clear_log
> +test_ping
> +
> +# Now we should have the request and reply logged.
> +check_acl_log_count 2
> +
> +check $PYTHON $srcdir/check_acl_log.py \
> + --entry-num=1 \
> + --name=allow_related_acl \
> + --verdict=allow \
> + --protocol=icmp \
> + --dl_src=00:00:00:00:00:01 \
> + --dl_dst=00:00:00:00:00:02 \
> + --nw_src=10.0.0.1 \
> + --nw_dst=10.0.0.2 \
> + --icmp_type=8 \
> + --icmp_code=0
> +
> +check $PYTHON $srcdir/check_acl_log.py \
> + --entry-num=2 \
> + --name=allow_related_acl \
> + --verdict=allow \
> + --protocol=icmp \
> + --dl_src=00:00:00:00:00:02 \
> + --dl_dst=00:00:00:00:00:01 \
> + --nw_src=10.0.0.2 \
> + --nw_dst=10.0.0.1 \
> + --icmp_type=0 \
> + --icmp_code=0
> +
> +
> +OVS_APP_EXIT_AND_WAIT([ovn-controller])
> +
> +as ovn-sb
> +OVS_APP_EXIT_AND_WAIT([ovsdb-server])
> +
> +as ovn-nb
> +OVS_APP_EXIT_AND_WAIT([ovsdb-server])
> +
> +as northd
> +OVS_APP_EXIT_AND_WAIT([NORTHD_TYPE])
> +
> +as
> +OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port patch-.*/d
> +/connection dropped.*/d"])
> +
> +AT_CLEANUP
> +])
> --
> 2.31.1
>
> _______________________________________________
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
