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


--
Jan Cholasta

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to