ovsfw sometimes rejects legitimate traffic when multiple remote SG rules are in use

[IWAMOTO Toshihiro]

ovsfw uses conjunction to represent SG rules with remote_group_id.
When there are multiple rules which differ only in remote_group_id, the ovsfw code generates flows with the same match fields and different conjuction actions. Such flows don't work well as the openflow spec says so.

An sequence to reproduce the bug:

$ openstack security group create sg1
$ openstack security group create sg2
$ openstack security group rule create --remote-group sg2 --dst-port 22:80 --protocol tcp --ingress sg1
$ openstack security group rule create --remote-group sg1 --dst-port 80 --protocol tcp --ingress sg1

Boot 3 instances: hoge1 (sg1), hoge2 (sg2), hoge12 (sg1 and sg2)

Start "nc -l -p 80" on hoge12.
Try to connect to hoge12:80 from hoge1 and hoge2. Either one should fail.

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: master
Review: https://review.openstack.org/489918

[Jakub Libosvar (libosvar)]

I was debugging today a failure where I saw flows with actions=conjunction() were not matched. When I replaced the conjunction action with output, it worked fine. Is it the same case?

[IWAMOTO Toshihiro (iwamoto)]

Hi Jakub,
that sounds like a different issue.

This bug report is about multiple flows with the same match fields but with different conjunction actions.

We need something like ConjIpManager that merges those actions=(...,2/2) flows.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/492404

[OpenStack Infra (hudson-openstack)]

Change abandoned by IWAMOTO Toshihiro (<email address hidden>) on branch: master
Review: https://review.openstack.org/489918
Reason: squashed into 492404.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/494428

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: master
Review: https://review.openstack.org/512480

[OpenStack Infra (hudson-openstack)]

Change abandoned by IWAMOTO Toshihiro (<email address hidden>) on branch: master
Review: https://review.openstack.org/512480

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/492404
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=237ec30ca94322716a1af5e59c0960f0eef24194
Submitter: Zuul
Branch: master

commit 237ec30ca94322716a1af5e59c0960f0eef24194
Author: IWAMOTO Toshihiro <email address hidden>
Date: Wed Aug 2 17:12:56 2017 +0900

    ovsfw: Merge multiple conjunction flows

    The ovsfw code generated multiple flows with the same or overlapping
    match fields and different actions=conjunction(nnn,2/2) flows.
    Merge such flows and generate only one flow with
    actions=conjunction(mmm,2/2),conjunction(nnn,2/2) so that filtering
    are correctly performed.

    Change-Id: I0cd325b02f35e103606595b8b124010fff8dc397
    Partial-bug: #1708092

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/pike
Review: https://review.openstack.org/521786

[Jakub Libosvar (libosvar)]

Iwamoto, is there anything else we need to work on?

[IWAMOTO Toshihiro (iwamoto)]

Yup, please review remaining patches.

https://review.openstack.org/#/q/status:open+branch:master+topic:bug/1708092

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/521786
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=04b155443a324d6d38b67e614085902963f315ec
Submitter: Zuul
Branch: stable/pike

commit 04b155443a324d6d38b67e614085902963f315ec
Author: IWAMOTO Toshihiro <email address hidden>
Date: Wed Aug 2 17:12:56 2017 +0900

    ovsfw: Merge multiple conjunction flows

    The ovsfw code generated multiple flows with the same or overlapping
    match fields and different actions=conjunction(nnn,2/2) flows.
    Merge such flows and generate only one flow with
    actions=conjunction(mmm,2/2),conjunction(nnn,2/2) so that filtering
    are correctly performed.

    Change-Id: I0cd325b02f35e103606595b8b124010fff8dc397
    Partial-bug: #1708092
    (cherry picked from commit 237ec30ca94322716a1af5e59c0960f0eef24194)

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/494428
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=4ac4c22a646799aaecca61334a8bb44147ab881a
Submitter: Zuul
Branch: master

commit 4ac4c22a646799aaecca61334a8bb44147ab881a
Author: IWAMOTO Toshihiro <email address hidden>
Date: Thu Aug 17 15:13:53 2017 +0900

    ovsfw: Use multiple priorities in RULES_*_TABLE

    The OpenFlow spec says packets shouldn't match against multiple flows
    at the same priority or the result is undefined. In ovsfw, 8 priority
    levels are needed to comply with this rule.

    Note: unlike overlapping TCP port ranges cases, the current version
    of OVS seems to handle this case magically.

    Change-Id: I6deaee8dbe81453285b1fc685282952bc9456949
    Closes-bug: #1708092

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/pike
Review: https://review.openstack.org/524533

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/524533
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=5c16f2bd79eabc7f7a069d9af9ff832dbdb4be54
Submitter: Zuul
Branch: stable/pike

commit 5c16f2bd79eabc7f7a069d9af9ff832dbdb4be54
Author: IWAMOTO Toshihiro <email address hidden>
Date: Thu Aug 17 15:13:53 2017 +0900

    ovsfw: Use multiple priorities in RULES_*_TABLE

    The OpenFlow spec says packets shouldn't match against multiple flows
    at the same priority or the result is undefined. In ovsfw, 8 priority
    levels are needed to comply with this rule.

    Note: unlike overlapping TCP port ranges cases, the current version
    of OVS seems to handle this case magically.

    Change-Id: I6deaee8dbe81453285b1fc685282952bc9456949
    Closes-bug: #1708092
    (cherry picked from commit 4ac4c22a646799aaecca61334a8bb44147ab881a)

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/neutron 12.0.0.0b2 development milestone.

[woody89 (a1007881221)]

good

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/neutron 11.0.3 release.

[OpenStack Infra (hudson-openstack)]

Change abandoned by Slawek Kaplonski (<email address hidden>) on branch: master
Review: https://review.openstack.org/520251
Reason: This review is > 4 weeks without comment, and failed Jenkins the last time it was checked. We are abandoning this for now. Feel free to reactivate the review by pressing the restore button and leaving a 'recheck' comment to get fresh test results.

[OpenStack Infra (hudson-openstack)]

Change abandoned by Slawek Kaplonski (<email address hidden>) on branch: master
Review: https://review.opendev.org/520251
Reason: This review is > 4 weeks without comment, and failed Zuul jobs the last time it was checked. We are abandoning this for now. Feel free to reactivate the review by pressing the restore button and leaving a 'recheck' comment to get fresh test results.

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/c/openstack/neutron-tempest-plugin/+/520251
Committed: https://opendev.org/openstack/neutron-tempest-plugin/commit/25df8e296ec1dbda91abb5b6e2b32a85469971eb
Submitter: "Zuul (22348)"
Branch: master

commit 25df8e296ec1dbda91abb5b6e2b32a85469971eb
Author: IWAMOTO Toshihiro <email address hidden>
Date: Thu Nov 16 10:22:00 2017 +0900

    Add a test for overlapping SG rules

    Need to make sure that security group rules with overlapping
    port ranges are correctly enforced

    Co-Authored-By: Maciej Jozefczyk <email address hidden>

    Change-Id: I0c3cafe7f346169741da17f024adf19c71bc1217
    Related-bug: #1708092