[Freeipa-devel] [PATCH] 3 Add ability to specify netmask with IP addresses during installation

[Jan Cholasta]

On 21.4.2011 09:36, Jan Cholasta wrote:
> On 20.4.2011 22:08, Rob Crittenden wrote:
>> Jan Cholasta wrote:
>>> On 11.4.2011 12:48, Jan Cholasta wrote:
>>>> On 8.4.2011 16:26, Rob Crittenden wrote:
>>>>> Jan Cholasta wrote:
>>>>>> On 29.3.2011 22:15, Rob Crittenden wrote:
>>>>>>> Jan Cholasta wrote:
>>>>>>>> Sorry, forgot to attach the patch.
>>>>>>>>
>>>>>>>
>>>>>>> Is this why you have some blind excepts?
>>>>>>>
>>>>>>> installutils._IPAddressWithPrefix('192.168.0.1/33')
>>>>>>> Traceback (most recent call last):
>>>>>>> File "<stdin>", line 1, in <module>
>>>>>>> File "ipaserver/install/installutils.py", line 167, in __init__
>>>>>>> net = netaddr.IPNetwork(addr)
>>>>>>> File "/usr/lib/python2.7/site-packages/netaddr/ip/__init__.py", line
>>>>>>> 919, in __init__
>>>>>>> implicit_prefix, flags)
>>>>>>> File "/usr/lib/python2.7/site-packages/netaddr/ip/__init__.py", line
>>>>>>> 782, in parse_ip_network
>>>>>>> value = ip._value
>>>>>>> UnboundLocalError: local variable 'ip' referenced before assignment
>>>>>>>
>>>>>>> We should get an upstream bug filed on python-netaddr about this.
>>>>>>
>>>>>> https://github.com/drkjam/netaddr/issues/closed#issue/5
>>>>>> https://github.com/drkjam/netaddr/issues/closed#issue/6
>>>>>> https://github.com/drkjam/netaddr/issues/closed#issue/8
>>>>>>
>>>>>> Apparently it's already been fixed for the next release.
>>>>>>
>>>>>> IMHO it's not much of an issue for us, because the exception gets
>>>>>> caught
>>>>>> in parse_ip_address and that's currently the only place where
>>>>>> _IPAddressWithPrefix is used.
>>>>>>
>>>>>>>
>>>>>>> Shoudl parse_ip_address() raise an exception on bad data rather than
>>>>>>> returning 0.0.0.0?
>>>>>>
>>>>>> I've been down that road and it would need a rewrite of the
>>>>>> fragile IP
>>>>>> address handling logic of ipa-server-install, which is something I'd
>>>>>> rather avoid.
>>>>>>
>>>>>>>
>>>>>>> >>> installutils.parse_ip_address('355.555.3.3')
>>>>>>> _IPAddressWithPrefix('0.0.0.0')
>>>>>>>
>>>>>>> or
>>>>>>>
>>>>>>> >>> installutils.parse_ip_address('192.168.0.1/55')
>>>>>>> _IPAddressWithPrefix('0.0.0.0')
>>>>>>>
>>>>>>> Should it disallow net addresses like 192.168.0.0?
>>>>>>
>>>>>> If you mean network and broadcast addresses, it probably should. It
>>>>>> might be a good idea to disallow localhost, multicast and/or
>>>>>> link-local
>>>>>> addresses too.
>>>>>
>>>>> Are you going to resubmit the patch with these added or should we
>>>>> open a
>>>>> separate ticket?
>>>>
>>>> I'm going to resubmit it. Right now it disallows loopback, IANA
>>>> reserved, link-local, network, multicast and broadcast IP addresses.
>>>> Does it make sense to also allow only IP addresses attached to one of
>>>> the local network interfaces? Perhaps it would be sufficient just to
>>>> print a warning. Or should we not care about that at all?
>>>
>>> Sending the updated patch.
>>
>> This looks ok, just one question. Should we add a dependency on the
>> iproute package because of the /sbin/ip package?
>
> Yes, we should.
>
>>
>> rob
>
>

Split the patch to 3 smaller pieces:

Patch 18 adds the ability to parse netmasks in IP addresses passed to 
server install.
https://fedorahosted.org/freeipa/ticket/1212

This patch requires patch 18 and fixes DNS reverse zone setup to honor 
the netmask.
https://fedorahosted.org/freeipa/ticket/910

Patch 19 requires patch 18 and adds stricter checking of IP addresses.
https://fedorahosted.org/freeipa/ticket/1213

Honza

-- 
Jan Cholasta
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-3.5-reverse-zone.patch
Type: text/x-patch
Size: 8844 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20110510/f93ce48d/attachment.bin>