[ovs firewall] Port masking adds wrong masks in several cases.

[Inessa Vasilevskaya]

Seen on master devstack, ubuntu xenial.

Steps to reproduce:

1. Enable ovs firewall in /etc/neutron/plugins/ml2/ml2.conf

[securitygroup]
firewall_driver = openvswitch

2. Create a security group with icmp, tcp to 22.

3. Boot a VM, assign a floating ip.

4. Check that port 23 can be accessed via tcp (telnet, nc, etc).

[OpenStack Infra (hudson-openstack)]

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

[Jakub Libosvar (libosvar)]

The bug is in port masking, 22 is masked by tp_src=0x16/0xfffe which matches number 23 as well. Good catch!

[Jeremy Stanley (fungi)]

What change introduced this bug? Is it present in stable branches too, or just master?

[Inessa Vasilevskaya (ivasilevskaya)]

commit 9af8f56d1d7d96517b6252ee1bd7c03ca1cbab72
Author: Jakub Libosvar <email address hidden>
Date: Tue Feb 23 15:52:57 2016 +0000

    ovs-fw: Enhance port ranges with masks

    The algorithm for masking port range was taken from networking-ovs-dpdk.
    Future step will be to move the algorithm to neutron-lib and reuse in
    networking-ovs-dpdk.

    Change-Id: I4573eac9a2e04c1f126d26591d2e3207b6150337

As far as I know ovs firewall was introduced in mitaka, so stable mitaka is affected as well.

Jakub can correct me if I got something wrong.

[Inessa Vasilevskaya (ivasilevskaya)]

Hm, a quick unit test in networking-ovs-dpdk shows that this bug affects it as well http://paste.openstack.org/show/555911/

I'll file a bug there as well and backport the fix once it is ready.

[Kevin Benton (kevinbenton)]

This was released in Mitaka as an experimental security groups driver. It is not the default and it required a newer OVS version not shipped with many distros. So it was possible to use but I'm not sure how large the user base was for it.

[Tristan Cacqueray (tristan-cacqueray)]

Closing the OSSA task, reason: B3 type of bug according to VMT taxonomy ( https://security.openstack.org/vmt-process.html#incident-report-taxonomy ).

[IWAMOTO Toshihiro (iwamoto)]

Shouldn't this be targeted for newton-rc1 in order to get proper attention?

[Armando Migliaccio (armando-migliaccio)]

Kuba felt that [1] was more readable, we need to take the time to assess which one we feel comfortable of merging.

[1] https://review.openstack.org/#/c/353782/16

[Armando Migliaccio (armando-migliaccio)]

Let's take more time to think about the best strategy forward.

[Armando Migliaccio (armando-migliaccio)]

Retargeted:

http://eavesdrop.openstack.org/meetings/neutron_drivers/2016/neutron_drivers.2016-09-22-22.03.log.html

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/353782
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=0494f212aa625a03587af3d75e823008f1198012
Submitter: Jenkins
Branch: master

commit 0494f212aa625a03587af3d75e823008f1198012
Author: Inessa Vasilevskaya <email address hidden>
Date: Thu Aug 11 02:21:29 2016 +0300

    ovsfw: fix troublesome port_rule_masking

    In several cases port masking algorithm borrowed
    from networking_ovs_dpdk didn't behave correctly.
    This caused non-restricted ports to be open due to
    wrong tp_src field value in resulting ovs rules.

    This was fixed by alternative port masking
    implementation.

    Functional and unit tests to cover the bug added as well.

    Co-Authored-By: Jakub Libosvar <email address hidden>
    Co-Authored-By: IWAMOTO Toshihiro <email address hidden>

    Closes-Bug: #1611991
    Change-Id: Idfc0e9c52e0dd08852c91c17e12edb034606a361

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/newton
Review: https://review.openstack.org/375892

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/375892
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=e55b3e329ed1424c11e0fc1d2a18fd70307da9a2
Submitter: Jenkins
Branch: stable/newton

commit e55b3e329ed1424c11e0fc1d2a18fd70307da9a2
Author: Inessa Vasilevskaya <email address hidden>
Date: Thu Aug 11 02:21:29 2016 +0300

    ovsfw: fix troublesome port_rule_masking

    In several cases port masking algorithm borrowed
    from networking_ovs_dpdk didn't behave correctly.
    This caused non-restricted ports to be open due to
    wrong tp_src field value in resulting ovs rules.

    This was fixed by alternative port masking
    implementation.

    Functional and unit tests to cover the bug added as well.

    Co-Authored-By: Jakub Libosvar <email address hidden>
    Co-Authored-By: IWAMOTO Toshihiro <email address hidden>

    Closes-Bug: #1611991
    Change-Id: Idfc0e9c52e0dd08852c91c17e12edb034606a361
    (cherry picked from commit 0494f212aa625a03587af3d75e823008f1198012)

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/neutron 9.0.0.0rc2 release candidate.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/mitaka
Review: https://review.openstack.org/388015

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/neutron 10.0.0.0b1 development milestone.

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/388015
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=dd75f7e96afc713b57ad4ab21f01175be7b571fe
Submitter: Jenkins
Branch: stable/mitaka

commit dd75f7e96afc713b57ad4ab21f01175be7b571fe
Author: Inessa Vasilevskaya <email address hidden>
Date: Thu Aug 11 02:21:29 2016 +0300

    ovsfw: fix troublesome port_rule_masking

    In several cases port masking algorithm borrowed
    from networking_ovs_dpdk didn't behave correctly.
    This caused non-restricted ports to be open due to
    wrong tp_src field value in resulting ovs rules.

    This was fixed by alternative port masking
    implementation.

    Functional and unit tests to cover the bug added as well.

    Co-Authored-By: Jakub Libosvar <email address hidden>
    Co-Authored-By: IWAMOTO Toshihiro <email address hidden>

    Closes-Bug: #1611991

    Conflicts:
     neutron/common/utils.py
     neutron/tests/unit/common/test_utils.py

    (cherry-picked from 0494f212aa625a03587af3d75e823008f1198012)

    Change-Id: Idfc0e9c52e0dd08852c91c17e12edb034606a361

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/376401
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=aa5985dff17957e0250a73787aff2d5e35307836
Submitter: Jenkins
Branch: master

commit aa5985dff17957e0250a73787aff2d5e35307836
Author: Jakub Libosvar <email address hidden>
Date: Mon Sep 26 07:03:36 2016 -0400

    Compare port_rule_masking() results with different approach

    This patch creates test that generates port ranges and
    uses software diversity for comparison the results.

    Change-Id: I77a76aa8288b505a0f083357f26a3bce23625897
    Related-bug: 1611991

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: stable/newton
Review: https://review.openstack.org/421958

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/421958
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=dfb55ba869d71374ee10e53ce9ab5dcc8831e49c
Submitter: Jenkins
Branch: stable/newton

commit dfb55ba869d71374ee10e53ce9ab5dcc8831e49c
Author: Jakub Libosvar <email address hidden>
Date: Mon Sep 26 07:03:36 2016 -0400

    Compare port_rule_masking() results with different approach

    This patch creates test that generates port ranges and
    uses software diversity for comparison the results.

    Conflicts:
     neutron/tests/unit/common/test_utils.py

    Change-Id: I77a76aa8288b505a0f083357f26a3bce23625897
    Related-bug: 1611991
    (cherry picked from commit aa5985dff17957e0250a73787aff2d5e35307836)

[OpenStack Infra (hudson-openstack)]

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