Hi Aaron, I reported it as a bug with bit more details: https://bugs.launchpad.net/neutron/+bug/1321864. The report has examples showing the incompleteness in the overlap check due to cidr notation allowed in the allowed address pairs API.
Cheers, Praveen On Tue, May 20, 2014 at 7:54 PM, Aaron Rosen <[email protected]> wrote: > arosen@arosen-MacBookPro:~/devstack$ neutron port-show > f5117013-ac04-45af-a5d6-e9110213ad6f > > +-----------------------+----------------------------------------------------------------------------------+ > | Field | Value > | > > +-----------------------+----------------------------------------------------------------------------------+ > | admin_state_up | True > | > | allowed_address_pairs | > | > | binding:vnic_type | normal > | > | device_id | 99012a6c-a5ed-41c0-92c9-14d40af0c2df > | > | device_owner | compute:None > | > | extra_dhcp_opts | > | > | fixed_ips | {"subnet_id": > "505eb39c-32dc-4fe7-a497-f801a0677c54", "ip_address": "10.0.0.22"} | > | id | f5117013-ac04-45af-a5d6-e9110213ad6f > | > | mac_address | fa:16:3e:77:4d:2d > | > | name | > | > | network_id | 1b069199-bfa4-4efc-aebd-4a663d447964 > | > | security_groups | 0d5477cf-f63a-417e-be32-a12557fa4098 > | > | status | ACTIVE > | > | tenant_id | c71ebe8d1f6e47bab7d44046ec2f6b39 > | > > +-----------------------+----------------------------------------------------------------------------------+ > arosen@arosen-MacBookPro:~/devstack$ neutron port-update > f5117013-ac04-45af-a5d6-e9110213ad6f --allowed-address-pairs list=true > type=dict ip_address=10.0.0.0/24 > Updated port: f5117013-ac04-45af-a5d6-e9110213ad6f > arosen@arosen-MacBookPro:~/devstack$ neutron port-show > f5117013-ac04-45af-a5d6-e9110213ad6f > > +-----------------------+----------------------------------------------------------------------------------+ > | Field | Value > | > > +-----------------------+----------------------------------------------------------------------------------+ > | admin_state_up | True > | > | allowed_address_pairs | {"ip_address": "10.0.0.0/24", "mac_address": > "fa:16:3e:77:4d:2d"} | > | binding:vnic_type | normal > | > | device_id | 99012a6c-a5ed-41c0-92c9-14d40af0c2df > | > | device_owner | compute:None > | > | extra_dhcp_opts | > | > | fixed_ips | {"subnet_id": > "505eb39c-32dc-4fe7-a497-f801a0677c54", "ip_address": "10.0.0.22"} | > | id | f5117013-ac04-45af-a5d6-e9110213ad6f > | > | mac_address | fa:16:3e:77:4d:2d > | > | name | > | > | network_id | 1b069199-bfa4-4efc-aebd-4a663d447964 > | > | security_groups | 0d5477cf-f63a-417e-be32-a12557fa4098 > | > | status | ACTIVE > | > | tenant_id | c71ebe8d1f6e47bab7d44046ec2f6b39 > | > > +-----------------------+----------------------------------------------------------------------------------+ > > > > On Tue, May 20, 2014 at 7:52 PM, Aaron Rosen <[email protected]>wrote: > >> Hi Praveen, >> >> I think there is some confusion here. This function doesn't check if >> there is any overlap that occurs within the cidr block. It only checks that >> the fixed_ips+mac don't overlap with an allowed address pair. In your >> example if the host has an ip_address of 10.10.1.1 and you want to allow >> any ip in 10.10.1.0/24 to pass through the port you can just add a rule >> for 10.10.1.0/24 directly without having to break it up. >> >> Aaron >> >> >> On Tue, May 20, 2014 at 11:20 AM, Praveen Yalagandula < >> [email protected]> wrote: >> >>> Hi Aaron, >>> >>> The main motivation is simplicity. Consider the case where we want to >>> allow ip cidr 10.10.1.0/24 to be allowed on a port which has a fixed IP >>> of 10.10.1.1. Now if we do not want to allow overlapping, then one needs to >>> add 8 cidrs to get around this - (10.10.1.128/25, 10.10.1.64/26, >>> 10.10.1.32/27, ....10.10.1.0/32); which makes it cumbersome. >>> >>> In any case, allowed-address-pairs is ADDING on to what is allowed >>> because of the fixed IPs. So, there is no possibility of conflict. The >>> check will probably make sense if we are maintaining denied addresses >>> instead of allowed addresses. >>> >>> Cheers, >>> Praveen >>> >>> >>> On Tue, May 20, 2014 at 9:34 AM, Aaron Rosen <[email protected]>wrote: >>> >>>> Hi Praveen, >>>> >>>> I think we should fix the update_method instead to properly check for >>>> this. I don't see any advantage to allow the fixed_ips/mac to be in the >>>> allowed_address_pairs since they are explicitly allowed. What's your >>>> motivation for changing this? >>>> >>>> Aaron >>>> >>>> >>>> On Mon, May 19, 2014 at 4:05 PM, Praveen Yalagandula < >>>> [email protected]> wrote: >>>> >>>>> Hi Aaron, >>>>> >>>>> Thanks for the prompt response. >>>>> >>>>> If the overlap does not have any negative effect, can we please just >>>>> remove this check? It creates confusion as there are certain code paths >>>>> where we do not perform this check. For example, the current code does NOT >>>>> perform this check when we are updating the list of allowed-address-pairs >>>>> -- I can successfully assign an existing fixed IP address to the >>>>> allowed-address-pairs. The check is being performed on only one code path >>>>> - >>>>> when assigning fixed IPs. >>>>> >>>>> If it sounds right to you, I can submit my patch removing this check. >>>>> >>>>> Thanks, >>>>> Praveen >>>>> >>>>> >>>>> >>>>> On Mon, May 19, 2014 at 12:32 PM, Aaron Rosen >>>>> <[email protected]>wrote: >>>>> >>>>>> Hi, >>>>>> >>>>>> Sure, if you look at this method: >>>>>> >>>>>> def _check_fixed_ips_and_address_pairs_no_overlap(self, context, >>>>>> port): >>>>>> address_pairs = self.get_allowed_address_pairs(context, >>>>>> port['id']) >>>>>> for fixed_ip in port['fixed_ips']: >>>>>> >>>>>> for address_pair in address_pairs: >>>>>> >>>>>> if (fixed_ip['ip_address'] == >>>>>> address_pair['ip_address'] >>>>>> and port['mac_address'] == >>>>>> address_pair['mac_address']): >>>>>> raise >>>>>> addr_pair.AddressPairMatchesPortFixedIPAndMac() >>>>>> >>>>>> >>>>>> >>>>>> it checks that the allowed_address_pairs don't overlap with fixed_ips >>>>>> and mac_address on the port. The only reason we do this additional check >>>>>> is >>>>>> that having the same fixed_ip and mac_address pair as an >>>>>> allowed_address_pair would have no effect since the fixed_ip/mac on the >>>>>> port inherently allows that traffic through. >>>>>> >>>>>> Best, >>>>>> >>>>>> Aaron >>>>>> >>>>>> >>>>>> >>>>>> On Mon, May 19, 2014 at 12:22 PM, Praveen Yalagandula < >>>>>> [email protected]> wrote: >>>>>> >>>>>>> Hi Aaron, >>>>>>> >>>>>>> In OVS and ML2 plugins, on port-update, there is a check to make >>>>>>> sure that allowed-address-pairs and fixed-ips don't overlap. Can you >>>>>>> please >>>>>>> explain why that is needed? >>>>>>> >>>>>>> ------------- icehouse final: neutron/plugins/ml2/plugin.py >>>>>>> ------------ >>>>>>> >>>>>>> 677 elif changed_fixed_ips: >>>>>>> >>>>>>> 678 >>>>>>> self._check_fixed_ips_and_address_pairs_no_overlap( >>>>>>> >>>>>>> 679 context, updated_port) >>>>>>> ----------------------- >>>>>>> >>>>>>> Thanks, >>>>>>> Praveen >>>>>>> >>>>>>> >>>>>>> On Wed, Jul 17, 2013 at 3:45 PM, Aaron Rosen <[email protected]>wrote: >>>>>>> >>>>>>>> Hi Ian, >>>>>>>> >>>>>>>> For shared networks if the network is set to >>>>>>>> port_security_enabled=True then the tenant will not be able to remove >>>>>>>> port_security_enabled from their port if they are not the owner of the >>>>>>>> network. I believe this is the correct behavior we want. In addition, >>>>>>>> only >>>>>>>> admin's are able to create shared networks by default. >>>>>>>> >>>>>>>> I've created the following blueprint >>>>>>>> https://blueprints.launchpad.net/neutron/+spec/allowed-address-pairsand >>>>>>>> doc: >>>>>>>> https://docs.google.com/document/d/1hyB3dIkRF623JlUsvtQFo9fCKLsy0gN8Jf6SWnqbWWA/edit?usp=sharingwhich >>>>>>>> will provide us a way to do this. It would be awesome if you could >>>>>>>> check it out and let me know what you think. >>>>>>>> >>>>>>>> Thanks, >>>>>>>> >>>>>>>> Aaron >>>>>>>> >>>>>>>> >>>>>>>> On Tue, Jul 16, 2013 at 10:34 AM, Ian Wells <[email protected] >>>>>>>> > wrote: >>>>>>>> >>>>>>>>> On 10 July 2013 21:14, Vishvananda Ishaya <[email protected]> >>>>>>>>> wrote: >>>>>>>>> >> It used to be essential back when we had nova-network and all >>>>>>>>> tenants >>>>>>>>> >> ended up on one network. It became less useful when tenants >>>>>>>>> could >>>>>>>>> >> create their own networks and could use them as they saw fit. >>>>>>>>> >> >>>>>>>>> >> It's still got its uses - for instance, it's nice that the >>>>>>>>> metadata >>>>>>>>> >> server can be sure that a request is really coming from where it >>>>>>>>> >> claims - but I would very much like it to be possible to, as an >>>>>>>>> >> option, explicitly disable antispoof - perhaps on a per-network >>>>>>>>> basis >>>>>>>>> >> at network creation time - and I think we could do this without >>>>>>>>> >> breaking the security model beyond all hope of usefulness. >>>>>>>>> > >>>>>>>>> > Per network and per port makes sense. >>>>>>>>> > >>>>>>>>> > After all, this is conceptually the same as enabling or disabling >>>>>>>>> > port security on your switch. >>>>>>>>> >>>>>>>>> Bit late on the reply to this, but I think we should be specific on >>>>>>>>> the network, at least at creation time, on what disabling is >>>>>>>>> allowed >>>>>>>>> at port level (default off, may be off, must be on as now). Yes, >>>>>>>>> it's >>>>>>>>> exactly like disabling port security, and you're not always the >>>>>>>>> administrator of your own switch; if we extend the analogy you >>>>>>>>> probably wouldn't necessarily want people turning antispoof off on >>>>>>>>> an >>>>>>>>> explicitly shared-tenant network. >>>>>>>>> -- >>>>>>>>> Ian. >>>>>>>>> >>>>>>>>> _______________________________________________ >>>>>>>>> OpenStack-dev mailing list >>>>>>>>> [email protected] >>>>>>>>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev >>>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> _______________________________________________ >>>>>>>> OpenStack-dev mailing list >>>>>>>> [email protected] >>>>>>>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev >>>>>>>> >>>>>>>> >>>>>>> >>>>>>> _______________________________________________ >>>>>>> OpenStack-dev mailing list >>>>>>> [email protected] >>>>>>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev >>>>>>> >>>>>>> >>>>>> >>>>>> _______________________________________________ >>>>>> OpenStack-dev mailing list >>>>>> [email protected] >>>>>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev >>>>>> >>>>>> >>>>> >>>>> _______________________________________________ >>>>> OpenStack-dev mailing list >>>>> [email protected] >>>>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev >>>>> >>>>> >>>> >>>> _______________________________________________ >>>> OpenStack-dev mailing list >>>> [email protected] >>>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev >>>> >>>> >>> >>> _______________________________________________ >>> OpenStack-dev mailing list >>> [email protected] >>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev >>> >>> >> > > _______________________________________________ > OpenStack-dev mailing list > [email protected] > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev > >
_______________________________________________ OpenStack-dev mailing list [email protected] http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
