Bug 1924436 - Heat Common constraints accept an arbitrary string of 1 to 10 numbers as a valid IP
Summary: Heat Common constraints accept an arbitrary string of 1 to 10 numbers as a va...
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Red Hat OpenStack
Classification: Red Hat
Component: python-oslo-utils
Version: 13.0 (Queens)
Hardware: x86_64
OS: Linux
low
low
Target Milestone: ---
: ---
Assignee: Hervé Beraud
QA Contact: nlevinki
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2021-02-03 06:56 UTC by David Sedgmen
Modified: 2022-08-23 10:28 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2021-05-06 07:53:57 UTC
Target Upstream Version:
Embargoed:


Attachments (Terms of Use)


Links
System ID Private Priority Status Summary Last Updated
Launchpad 1914386 0 None None None 2021-02-03 10:04:57 UTC
OpenStack gerrit 773863 0 None MERGED Add a ``strict`` flag allowing users to restrict validation of IPv4 format 2021-05-06 07:47:10 UTC
Red Hat Issue Tracker OSP-3656 0 None None None 2022-08-23 10:28:42 UTC

Description David Sedgmen 2021-02-03 06:56:52 UTC
Description of problem:

Using the custom_constraint ip_addr in a template does not work as expect. 

Using something like 
- notanipadress 
- 1024.1024.10.24

Produce a parameter error like expected
~~~
(overcloud) [stack@undercloud-0 Parameter]$ openstack stack create -t Parameterconstraints.yaml teststack10 --parameter ip_address=notanipadress --parameter demo=demo --parameter demopassword=password
ERROR: Parameter 'ip_address' is invalid: Error validating value 'notanipadress': Invalid IP address
(overcloud) [stack@undercloud-0 Parameter]$ openstack stack create -t Parameterconstraints.yaml teststack10 --parameter ip_address=1024.1024.10.24 --parameter demo=demo --parameter demopassword=password
ERROR: Parameter 'ip_address' is invalid: Error validating value '1024.1024.10.24': Invalid IP address
~~~

But using arbitrary strings of numbers less then 11 numbers does not

~~~
(overcloud) [stack@undercloud-0 Parameter]$ openstack stack create -t Parameterconstraints.yaml teststack10 --parameter ip_address=1024 --parameter demo=demo --parameter demopassword=password
+---------------------+--------------------------------------------------------------------------------------------------------------------------------------------------+
| Field               | Value                                                                                                                                            |
+---------------------+--------------------------------------------------------------------------------------------------------------------------------------------------+
| id                  | 8cca736a-1f3c-4739-a85b-bd770ff7d561                                                                                                             |
| stack_name          | teststack10                                                                                                                                      |
| description         | Simple demonstration on what happens when use a function that is from a template version which higher then the version specified in the template |
| creation_time       | 2021-02-03T06:24:01Z                                                                                                                             |
| updated_time        | None                                                                                                                                             |
| stack_status        | CREATE_COMPLETE                                                                                                                                  |
| stack_status_reason | Stack CREATE completed successfully                                                                                                              |
+---------------------+--------------------------------------------------------------------------------------------------------------------------------------------------+
(overcloud) [stack@undercloud-0 Parameter]$ openstack stack create -t Parameterconstraints.yaml teststack11 --parameter ip_address=1 --parameter demo=demo --parameter demopassword=password
+---------------------+--------------------------------------------------------------------------------------------------------------------------------------------------+
| Field               | Value                                                                                                                                            |
+---------------------+--------------------------------------------------------------------------------------------------------------------------------------------------+
| id                  | f6739aad-285c-423e-a0ea-1668b4e340b5                                                                                                             |
| stack_name          | teststack11                                                                                                                                      |
| description         | Simple demonstration on what happens when use a function that is from a template version which higher then the version specified in the template |
| creation_time       | 2021-02-03T06:24:17Z                                                                                                                             |
| updated_time        | None                                                                                                                                             |
| stack_status        | CREATE_COMPLETE                                                                                                                                  |
| stack_status_reason | Stack CREATE completed successfully                                                                                                              |
+---------------------+--------------------------------------------------------------------------------------------------------------------------------------------------+

(overcloud) [stack@undercloud-0 Parameter]$ openstack stack create -t Parameterconstraints.yaml teststack12 --parameter ip_address=111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111 --parameter demo=demo --parameter demopassword=password
ERROR: Parameter 'ip_address' is invalid: Error validating value '111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111': Invalid IP address
(overcloud) [stack@undercloud-0 Parameter]$ openstack stack create -t Parameterconstraints.yaml teststack13 --parameter ip_address=0101010101010 --parameter demo=demo --parameter demopassword=password
ERROR: Parameter 'ip_address' is invalid: Error validating value '0101010101010': Invalid IP address
(overcloud) [stack@undercloud-0 Parameter]$ openstack stack create -t Parameterconstraints.yaml teststack13 --parameter ip_address=010101010101 --parameter demo=demo --parameter demopassword=password
+---------------------+--------------------------------------------------------------------------------------------------------------------------------------------------+
| Field               | Value                                                                                                                                            |
+---------------------+--------------------------------------------------------------------------------------------------------------------------------------------------+
| id                  | 3b60f530-84ab-4daa-bbb5-5647374e2079                                                                                                             |
| stack_name          | teststack13                                                                                                                                      |
| description         | Simple demonstration on what happens when use a function that is from a template version which higher then the version specified in the template |
| creation_time       | 2021-02-03T06:28:17Z                                                                                                                             |
| updated_time        | None                                                                                                                                             |
| stack_status        | CREATE_COMPLETE                                                                                                                                  |
| stack_status_reason | Stack CREATE completed successfully                                                                                                              |
~~~

Version-Release number of selected component (if applicable):
- python2-oslo-utils-3.35.1-2.el7ost.noarch
- openstack-heat-common-10.0.3-15.el7ost.noarch
- python2-netaddr-0.7.19-5.el7ost.noarch

How reproducible:

Every time

Steps to Reproduce:
1. use the a template like the following

openstack stack create -t Parameterconstraints.yaml teststack --parameter ip_address=010101010101 --parameter demo=demo --parameter demopassword=password
~~~
heat_template_version: 2013-05-23

description: Simple demonstration of heat parameter constraints 

parameters:
  demo:
    type: string
    label: demo
    description: just a useless var
  demopassword:
    type: string
    label: demo password
    description: just a useless var
    default: demopassword
    constraints:
      - length: { min: 8, max: 24 }
      - allowed_pattern: "[a-zA-Z0-9]*"
  ip_address:
    type: string
    label: IP ip_address
    constraints:
      - custom_constraint: ip_addr
resources:
  cloud_config:
    type: OS::Heat::CloudConfig
    properties:
      cloud_config:
        users:
          - name: {get_param: demo}
            passwd: { digest: ['sha512', {get_param: demopassword}]}
            ip_address: { get_param: ip_address }

~~~

Actual results:

~~~
(overcloud) [stack@undercloud-0 Parameter]$ openstack stack create -t Parameterconstraints.yaml teststack13 --parameter ip_address=010101010101 --parameter demo=demo --parameter demopassword=password
+---------------------+--------------------------------------------------------------------------------------------------------------------------------------------------+
| Field               | Value                                                                                                                                            |
+---------------------+--------------------------------------------------------------------------------------------------------------------------------------------------+
| id                  | 3b60f530-84ab-4daa-bbb5-5647374e2079                                                                                                             |
| stack_name          | teststack13                                                                                                                                      |
| description         | Simple demonstration on what happens when use a function that is from a template version which higher then the version specified in the template |
| creation_time       | 2021-02-03T06:28:17Z                                                                                                                             |
| updated_time        | None                                                                                                                                             |
| stack_status        | CREATE_COMPLETE                                                                                                                                  |
| stack_status_reason | Stack CREATE completed successfully      
~~~  
Expected results:

~~~
(overcloud) [stack@undercloud-0 Parameter]$ openstack stack create -t Parameterconstraints.yaml teststack13 --parameter ip_address=010101010101 --parameter demo=demo --parameter demopassword=password
ERROR: Parameter 'ip_address' is invalid: Error validating value '010101010101': Invalid IP address
~~~

Additional info:
 
/usr/lib/python2.7/site-packages/heat/engine/constraint/common_constraints.py
~~~
from oslo_utils import netutils
from oslo_utils import timeutils

from heat.common.i18n import _
from heat.common import netutils as heat_netutils
from heat.engine import constraints


class TestConstraintDelay(constraints.BaseCustomConstraint):

    def validate_with_client(self, client, value):
        eventlet.sleep(value)


class IPConstraint(constraints.BaseCustomConstraint):

    def validate(self, value, context, template=None):
        self._error_message = 'Invalid IP address'
        return netutils.is_valid_ip(value)
~~~
/usr/lib/python2.7/site-packages/oslo_utils/netutils.py
~~~
def is_valid_ipv4(address):
    """Verify that address represents a valid IPv4 address.

    :param address: Value to verify
    :type address: string
    :returns: bool

    .. versionadded:: 1.1
    """
    try:
        return netaddr.valid_ipv4(address)
    except netaddr.AddrFormatError:
        return False


def is_valid_ipv6(address):
    """Verify that address represents a valid IPv6 address.

    :param address: Value to verify
    :type address: string
    :returns: bool

    .. versionadded:: 1.1
    """
    if not address:
        return False

    parts = address.rsplit("%", 1)
    address = parts[0]
    scope = parts[1] if len(parts) > 1 else None
    if scope is not None and (len(scope) < 1 or len(scope) > 15):
        return False

    try:
        return netaddr.valid_ipv6(address, netaddr.core.INET_PTON)
    except netaddr.AddrFormatError:
        return False

...................

def is_valid_ip(address):
    """Verify that address represents a valid IP address.

    :param address: Value to verify
    :type address: string
    :returns: bool

    .. versionadded:: 1.1
    """
    return is_valid_ipv4(address) or is_valid_ipv6(address)
~~~~

Comment 1 David Sedgmen 2021-02-03 07:26:22 UTC
Loading into python it looks like it is validating it as a valid ipv4 address 

~~~
>>> from  heat.engine.constraint import common_constraints
>>> netutils.is_valid_ipv6("1")
False
>>> netutils.is_valid_ipv4("1")
True
>>> netutils.is_valid_ipv4("1024")
True
>>> netutils.is_valid_ipv6("1024")
False
>>> netutils.is_valid_ipv4("0101010101010")
False
>>> netutils.is_valid_ipv4("010101010101")
True
>>> netutils.is_valid_ipv6("010101010101")
False
>>> 
~~~

Comment 2 David Sedgmen 2021-02-03 08:02:39 UTC

Sorry my mistake I left out the import of netutils when cleaning up the output

~~~
>>> from oslo_utils import netutils
~~~
(In reply to David Sedgmen from comment #1)
> Loading into python it looks like it is validating it as a valid ipv4
> address 
> 
> ~~~
> >>> from  heat.engine.constraint import common_constraints
> >>> netutils.is_valid_ipv6("1")
> False
> >>> netutils.is_valid_ipv4("1")
> True
> >>> netutils.is_valid_ipv4("1024")
> True
> >>> netutils.is_valid_ipv6("1024")
> False
> >>> netutils.is_valid_ipv4("0101010101010")
> False
> >>> netutils.is_valid_ipv4("010101010101")
> True
> >>> netutils.is_valid_ipv6("010101010101")
> False
> >>> 
> ~~~

Comment 3 Hervé Beraud 2021-02-03 08:56:18 UTC
Hello,

First thanks for opening this bug.

At first glance the problem come from `netaddr` and not from `oslo.utils`:

```
$ tox -e venv -- python
>>> from oslo_utils import netutils
>>> netutils.is_valid_ipv4(1)
False
>>> netutils.is_valid_ipv4("1")
True
>>> import netaddr
>>> netaddr.valid_ipv4(1)
False
>>> netaddr.valid_ipv4("1")
True
```

It seems to work as expected with integers.
Oslo.utils just wrap the `netaddr`[1] module and return its outcome [2].

However I'll try to see why netaddr behave like this with string and try to see if this is something expected or not on their side.

[1] https://github.com/netaddr/netaddr/
[2] https://github.com/openstack/oslo.utils/blob/master/oslo_utils/netutils.py#L84,L96

Comment 4 Hervé Beraud 2021-02-03 09:07:31 UTC
Apparently this is behavior is something expected.

This is not an invalid input.

By default, valid_ipv4() uses the inet_aton() C function to verify input which follows the following input formatting rules:

       a.b.c.d   Each of the four numeric parts specifies a byte of the address; the bytes are assigned in left-to-right order to produce the binary address.

       a.b.c     Parts a and b specify the first two bytes of the binary address.  Part c is interpreted as a 16-bit value that defines the rightmost two bytes of the binary address. This notation is suitable for specifying (outmoded) Class B network addresses.

       a.b       Part a specifies the first byte of the binary address. Part b is interpreted as a 24-bit value that defines the rightmost three bytes of the binary address.  This notation is suitable for specifying (outmoded) Class A network addresses.

       a         The value is interpreted as a 32-bit value that is stored directly into the binary address without any byte rearrangement.

ref: http://man7.org/linux/man-pages/man3/inet_addr.3.html

Therefore, '10' falls under the 4th option (a) where the number is treated as a 32-bit value. Written in dotted decimal format it would be '0.0.0.10' or in hex \x00\x00\x00\x0A Python3 shows it as a byte array b'\x00 \x00\x00\n' (\n newline being the ASCII character 10)

To validate the dotted decimal notation pass the INET_PTON flag to have the function use inet_pton() instead of inet_aton() which evaluates ipv4 and ipv6 addresses using proper formatting requirements:

```
>>> import netaddr
>>> netaddr.valid_ipv4('10')
True
>>> netaddr.valid_ipv4('10', flags=netaddr.INET_PTON)
False
```

However oslo.utils doesn't accept `flags` so it could be an improvement if you decide to use `INET_PTON` rather than `INET_ATON` that's the default:

https://github.com/netaddr/netaddr/blob/master/netaddr/strategy/ipv4.py#L99,L105

For further details please take a look too:

https://github.com/netaddr/netaddr/issues/186

I keep this BZ opened even if this behavior is legit and if this isn't a bug, however I move this to the future version of OSP to manage the improvement (the flag adding).


Note You need to log in before you can comment on or make changes to this bug.