yahoo-eng-team team mailing list archive

[Praveen Yalagandula <ypraveen@xxxxxxxxx>]

Public bug reported:

Current code intends to disallow assigning a fixed ip to a port when that ip
overlaps with one of the addresses in the allowed-address-pairs list. However, it is an unnecessary check and also the current code does not enforce it in all cases.

Cases where it enforces:

1) A port-update with allowed-address-pairs list containing an IP address which is *exactly* same as one of the fixed-ips on the port. For example, for a port with fixed-ip of 10.10.8.6, the following fails:
$> neutron port-update  58062310-ee5a-4b14-b554-5df699064bc9 --allowed-address-pairs type=dict list=true ip_address=10.10.8.6
400-{u'NeutronError': {u'message': u"Port's Fixed IP and Mac Address match an address pair entry.", u'type': u'AddressPairMatchesPortFixedIPAndMac', u'detail': u''}}

2) A port-update with a fixed-ip which is exactly same as one of the allowed
IP addresses in the allowed-address-pairs list. For example, for a port
with allowed-address-pairs with "", the following fails:
$> neutron port-show 58062310-ee5a-4b14-b554-5df699064bc9 | grep allowed
| allowed_address_pairs | {"ip_address": "10.10.8.6", "mac_address": "fa:16:3e:7f:3c:06"}                    |
$> neutron port-update 58062310-ee5a-4b14-b554-5df699064bc9 -- --fixed-ips type=dict list=true ip_address=10.10.8.6
400-{u'NeutronError': {u'message': u"Port's Fixed IP and Mac Address match an address pair entry.", u'type': u'AddressPairMatchesPortFixedIPAndMac', u'detail': u''}}

However, allowed-address-pairs can work with IP CIDRs and the overlap check
is *not* properly enforced when IP addresses are specified in the CIDR notation.

Case where the current code is incomplete:
Same as case (1) above but IP address specified in cidr notation. In this case, the code does not check for overlaps.
$> neutron port-update  58062310-ee5a-4b14-b554-5df699064bc9 --allowed-address-pairs type=dict list=true ip_address=10.10.8.6/32
Updated port: 58062310-ee5a-4b14-b554-5df699064bc9
$> neutron port-show 58062310-ee5a-4b14-b554-5df699064bc9 | grep allowed
| allowed_address_pairs | {"ip_address": "10.10.8.6/32", "mac_address": "fa:16:3e:7f:3c:06"}                 |

Functionally, it is incorrect to allow overlaps in one type of inputs
and not allow in other types of input.

On the other hand, if we fix this bug and check overlaps in all cases, then the API will become hard to use. For example, if a fixed IP 10.10.1.1 exists on a port and we want to allow addresses in 10.10.1.0/24 cidr on that port, then one has to configure a list of 8 cidrs ([10.10.1.0/32, 10.10.1.2/31,
10.10.1.4/30, ..., 10.10.1.128/25]) on the allowed-address-pairs.

In any case, this is an unnecessary check as the overlap does not have
any negative effect. 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).

My suggestion is to remove this check entirely.
https://review.openstack.org/#/c/94508/

** Affects: neutron
     Importance: Undecided
     Assignee: Praveen Yalagandula (ypraveen-5)
         Status: In Progress

** Changed in: neutron
       Status: New => In Progress

** Description changed:

  Current code intends to disallow assigning a fixed ip to a port when that ip
- overlaps with one of the addresses in the allowed-address-pairs list. However,
- it is an unnecessary check and also the current code does not enforce it
- in all cases.
+ overlaps with one of the addresses in the allowed-address-pairs list. However, it is an unnecessary check and also the current code does not enforce it in all cases.
  
  Cases where it enforces:
  
- 1) A port-update with allowed-address-pairs list containing an IP address which
- is *exactly* same as one of the fixed-ips on the port. For example, for a port
- with fixed-ip of 10.10.8.6, the following fails:
+ 1) A port-update with allowed-address-pairs list containing an IP address which is *exactly* same as one of the fixed-ips on the port. For example, for a port with fixed-ip of 10.10.8.6, the following fails:
  $> neutron port-update  58062310-ee5a-4b14-b554-5df699064bc9 --allowed-address-pairs type=dict list=true ip_address=10.10.8.6
  400-{u'NeutronError': {u'message': u"Port's Fixed IP and Mac Address match an address pair entry.", u'type': u'AddressPairMatchesPortFixedIPAndMac', u'detail': u''}}
  
  2) A port-update with a fixed-ip which is exactly same as one of the allowed
  IP addresses in the allowed-address-pairs list. For example, for a port
  with allowed-address-pairs with "", the following fails:
  $> neutron port-show 58062310-ee5a-4b14-b554-5df699064bc9 | grep allowed
  | allowed_address_pairs | {"ip_address": "10.10.8.6", "mac_address": "fa:16:3e:7f:3c:06"}                    |
  $> neutron port-update 58062310-ee5a-4b14-b554-5df699064bc9 -- --fixed-ips type=dict list=true ip_address=10.10.8.6
  400-{u'NeutronError': {u'message': u"Port's Fixed IP and Mac Address match an address pair entry.", u'type': u'AddressPairMatchesPortFixedIPAndMac', u'detail': u''}}
  
- 
  However, allowed-address-pairs can work with IP CIDRs and the overlap check
  is *not* properly enforced when IP addresses are specified in the CIDR notation.
  
  Case where the current code is incomplete:
- Same as case (1) above but IP address specified in cidr notation. In this case,
- the code does not check for overlaps.
+ Same as case (1) above but IP address specified in cidr notation. In this case, the code does not check for overlaps.
  $> neutron port-update  58062310-ee5a-4b14-b554-5df699064bc9 --allowed-address-pairs type=dict list=true ip_address=10.10.8.6/32
  Updated port: 58062310-ee5a-4b14-b554-5df699064bc9
  $> neutron port-show 58062310-ee5a-4b14-b554-5df699064bc9 | grep allowed
  | allowed_address_pairs | {"ip_address": "10.10.8.6/32", "mac_address": "fa:16:3e:7f:3c:06"}                 |
  
+ Functionally, it is incorrect to allow overlaps in one type of inputs
+ and not allow in other types of input.
  
- Functionally, it is incorrect to allow overlaps in one type of inputs and not allow
- in other types of input.
- 
- On the other hand, if we fix this bug and check overlaps in all cases, then the API
- will become hard to use. For example, if a fixed IP 10.10.1.1 exists on a port and we
- want to allow addresses in 10.10.1.0/24 cidr on that port, then one
- has to configure a list of 8 cidrs ([10.10.1.0/32, 10.10.1.2/31,
+ On the other hand, if we fix this bug and check overlaps in all cases, then the API will become hard to use. For example, if a fixed IP 10.10.1.1 exists on a port and we want to allow addresses in 10.10.1.0/24 cidr on that port, then one has to configure a list of 8 cidrs ([10.10.1.0/32, 10.10.1.2/31,
  10.10.1.4/30, ..., 10.10.1.128/25]) on the allowed-address-pairs.
  
- In any case, this is an unnecessary check as the overlap does not have any negative
- effect. 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).
+ In any case, this is an unnecessary check as the overlap does not have
+ any negative effect. 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).
  
  My suggestion is to remove this check entirely.
  https://review.openstack.org/#/c/94508/

-- 
You received this bug notification because you are a member of Yahoo!
Engineering Team, which is subscribed to neutron.
https://bugs.launchpad.net/bugs/1321864

Title:
  allowed address pairs - unnecessary and incomplete overlap check

Status in OpenStack Neutron (virtual network service):
  In Progress

Bug description:
  Current code intends to disallow assigning a fixed ip to a port when that ip
  overlaps with one of the addresses in the allowed-address-pairs list. However, it is an unnecessary check and also the current code does not enforce it in all cases.

  Cases where it enforces:

  1) A port-update with allowed-address-pairs list containing an IP address which is *exactly* same as one of the fixed-ips on the port. For example, for a port with fixed-ip of 10.10.8.6, the following fails:
  $> neutron port-update  58062310-ee5a-4b14-b554-5df699064bc9 --allowed-address-pairs type=dict list=true ip_address=10.10.8.6
  400-{u'NeutronError': {u'message': u"Port's Fixed IP and Mac Address match an address pair entry.", u'type': u'AddressPairMatchesPortFixedIPAndMac', u'detail': u''}}

  2) A port-update with a fixed-ip which is exactly same as one of the allowed
  IP addresses in the allowed-address-pairs list. For example, for a port
  with allowed-address-pairs with "", the following fails:
  $> neutron port-show 58062310-ee5a-4b14-b554-5df699064bc9 | grep allowed
  | allowed_address_pairs | {"ip_address": "10.10.8.6", "mac_address": "fa:16:3e:7f:3c:06"}                    |
  $> neutron port-update 58062310-ee5a-4b14-b554-5df699064bc9 -- --fixed-ips type=dict list=true ip_address=10.10.8.6
  400-{u'NeutronError': {u'message': u"Port's Fixed IP and Mac Address match an address pair entry.", u'type': u'AddressPairMatchesPortFixedIPAndMac', u'detail': u''}}

  However, allowed-address-pairs can work with IP CIDRs and the overlap check
  is *not* properly enforced when IP addresses are specified in the CIDR notation.

  Case where the current code is incomplete:
  Same as case (1) above but IP address specified in cidr notation. In this case, the code does not check for overlaps.
  $> neutron port-update  58062310-ee5a-4b14-b554-5df699064bc9 --allowed-address-pairs type=dict list=true ip_address=10.10.8.6/32
  Updated port: 58062310-ee5a-4b14-b554-5df699064bc9
  $> neutron port-show 58062310-ee5a-4b14-b554-5df699064bc9 | grep allowed
  | allowed_address_pairs | {"ip_address": "10.10.8.6/32", "mac_address": "fa:16:3e:7f:3c:06"}                 |

  Functionally, it is incorrect to allow overlaps in one type of inputs
  and not allow in other types of input.

  On the other hand, if we fix this bug and check overlaps in all cases, then the API will become hard to use. For example, if a fixed IP 10.10.1.1 exists on a port and we want to allow addresses in 10.10.1.0/24 cidr on that port, then one has to configure a list of 8 cidrs ([10.10.1.0/32, 10.10.1.2/31,
  10.10.1.4/30, ..., 10.10.1.128/25]) on the allowed-address-pairs.

  In any case, this is an unnecessary check as the overlap does not have
  any negative effect. 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).

  My suggestion is to remove this check entirely.
  https://review.openstack.org/#/c/94508/

To manage notifications about this bug go to:
https://bugs.launchpad.net/neutron/+bug/1321864/+subscriptions