On 25.5.2011 09:46, Martin Kosek wrote:
On Tue, 2011-05-24 at 15:42 +0200, Jan Cholasta wrote:
On 24.5.2011 14:44, Jan Cholasta wrote:
On 24.5.2011 14:43, Martin Kosek wrote:
On Fri, 2011-05-20 at 20:34 +0200, Jan Cholasta wrote:
On 18.5.2011 10:51, Martin Kosek wrote:
On Mon, 2011-05-16 at 19:15 +0200, Jan Cholasta wrote:
On 16.5.2011 17:26, Martin Kosek wrote:
On Tue, 2011-05-10 at 20:11 +0200, Jan Cholasta wrote:
Split from patch 3, requires patch 18.

https://fedorahosted.org/freeipa/ticket/1213

Honza


I tested all patches (3.6, 18, 19), but I think some work still
needs to
be done:

1) What about adding /sbin/ip package to Requires in spec? I thought
there was an agreement to do it.

Will do.

Ok.



2) When I run `ipa-server-install --ip-address=$ADDR`, and $ADDR is
invalid address (e.g. $ADDR==foo), loopback address (e.g.
$ADDR==127.0.0.1) or just another that the local address (e.g.
$ADDR==123.123.123.123) the installer always fails with "the hostname
resolves to an IP address that is different from the one provided
on the
command line".

I think we may want a different error message in those 3 cases - it
should be easy to do it now, with the improved IP handling.

It looks like the print statements from verify_ip_address doesn't
actually print anything to the user. Will look onto that.

Ok.



3) When I pass netmask to ipa-server-install --ip-address=$ADDR, the
installation always fails with the above message. Even though I
took the
addr+netmask from "/sbin/ip address" output.

Works for me. Please make sure you've added your hostname to
/etc/hosts.

I think I had. But I will recheck when you send a fix.



4) I miss IP address checks in --ip-address and --forwarder
parameters
of ipa-dns-install script. I can pass invalid or local addresses to
these parameters. This breaks Bind configuration.

--ip-address is checked, but --forwarder is not. Will fix that.

Ok, I will recheck both of them when you do.



5) I think we may want to check also for local address in
#ipa host-add $HOST --ip-address=127.0.0.1

6) I couldn't add IP address with netmask in host module:
# ipa host-add $HOST --ip-address=10.16.78.102/22
ipa: ERROR: invalid 'ip_address': invalid IP address

The patches are for the installer, as are the tickets they fix, so
these
issues are out of scope. A new ticket should be opened for them.


You touched this parameter in your patches, that's why I tested it. I
created a new ticket for it:

https://fedorahosted.org/freeipa/ticket/1234

Ticket 1234, yey :-)


7) Why is the _ParsedIPAddress named with a leading underscore?
It's not
really an internal use since it is returned by new IP handling
functions
and used in other modules.

_ParsedIPAddress is not for public use. The fact that object of this
class is returned by parse_ip_address doesn't really matter - this is
Python, not C++ or Java.

Hm, snappy... And I was wondering why my /usr/bin/java doesn't want to
run FreeIPA, now I know - it's because its Python.

Martin


Patch updated. Requires patch 18.1

Honza


All reported issues were fixed, good idea with a new type for our
IPAOptionParser.

Still, NACK from me:

ipa-replica-install doesn't use IPAOptionParser, but the good old
OptionParser which doesn't know the new type. This makes
ipa-replica-prepare crash all the time. I know, I am nitpicker :-)

Martin


Thanks, I missed that.

Honza


Fixed and added a unit test.


NACK. Please test your patches before you send them for a review. It
saves reviewer's time.

Sorry, I'll do better next time.


1) Unwanted warning about unmatching network interface when replica is
installed:

# ipa-replica-prepare vm-059.idm.lab.bos.redhat.com
--ip-address=10.16.78.59
Warning: No network interface matches IP address 10.16.78.59
Directory Manager (existing master) password:
...

Fixed.


2) ipa-replica-install crashes
# ipa-replica-install 
/home/mkosek/replica-info-vm-059.idm.lab.bos.redhat.com.gpg
Directory Manager (existing master) password:

Configuring ntpd
   [1/4]: stopping ntpd
   [2/4]: writing configuration
   [3/4]: configuring ntpd to start on boot
   [4/4]: starting ntpd
done configuring ntpd.
creation of replica failed: unsupported operand type(s) for /: 'NoneType' and 
'int'

Your system may be partly configured.
Run /usr/sbin/ipa-server-install --uninstall to clean up.


ipa-replica-install log:
2011-05-25 03:36:18,503 DEBUG unsupported operand type(s) for /: 'NoneType' and 
'int'
   File "/usr/sbin/ipa-replica-install", line 550, in<module>
     main()

   File "/usr/sbin/ipa-replica-install", line 496, in main
     install_dns_records(config, options)

   File "/usr/sbin/ipa-replica-install", line 329, in install_dns_records
     options.conf_ntp)

   File "/usr/lib/python2.7/site-packages/ipaserver/install/bindinstance.py", 
line 469, in add_master_dns_records
     self.__add_self()

   File "/usr/lib/python2.7/site-packages/ipaserver/install/bindinstance.py", 
line 399, in __add_self
     if dns_zone_exists(get_reverse_zone(self.ip_address, 
self.ip_prefix_len)[0]):

   File "/usr/lib/python2.7/site-packages/ipaserver/install/bindinstance.py", 
line 106, in get_reverse_zone
     pos = 4 - ip_prefix_len / 8

Also fixed.



Martin


Honza

--
Jan Cholasta
>From a0d128feb3d95f962134f8747e1d8cf91f6cbd0f Mon Sep 17 00:00:00 2001
From: Jan Cholasta <jchol...@redhat.com>
Date: Fri, 27 May 2011 13:51:21 +0200
Subject: [PATCH] Do stricter checking of IP addressed passed to server
 install.

ticket 1213
---
 ipapython/ipautil.py                 |   11 +++++++++++
 tests/test_ipapython/test_ipautil.py |    9 +++++++++
 2 files changed, 20 insertions(+), 0 deletions(-)

diff --git a/ipapython/ipautil.py b/ipapython/ipautil.py
index 444487a..acfd70c 100644
--- a/ipapython/ipautil.py
+++ b/ipapython/ipautil.py
@@ -95,6 +95,12 @@ class CheckedIPAddress(netaddr.IPAddress):
             raise ValueError("unsupported IP version")
         if addr.is_loopback():
             raise ValueError("cannot use loopback IP address")
+        if addr.is_reserved() or addr in netaddr.ip.IPV4_6TO4:
+            raise ValueError("cannot use IANA reserved IP address")
+        if addr.is_link_local():
+            raise ValueError("cannot use link-local IP address")
+        if addr.is_multicast():
+            raise ValueError("cannot use multicast IP address")
 
         if match_local:
             if addr.version == 4:
@@ -122,6 +128,11 @@ class CheckedIPAddress(netaddr.IPAddress):
             elif addr.version == 6:
                 net = netaddr.IPNetwork(str(addr) + '/64')
 
+        if addr == net.network:
+            raise ValueError("cannot use IP network address")
+        if addr.version == 4 and addr == net.broadcast:
+            raise ValueError("cannot use broadcast IP address")
+
         super(CheckedIPAddress, self).__init__(addr)
         self.prefixlen = net.prefixlen
         self.defaultnet = defnet
diff --git a/tests/test_ipapython/test_ipautil.py b/tests/test_ipapython/test_ipautil.py
index 03f5f7b..68391c2 100644
--- a/tests/test_ipapython/test_ipautil.py
+++ b/tests/test_ipapython/test_ipautil.py
@@ -42,12 +42,21 @@ def test_ip_address():
         ('10.11.12.1337',),
         ('10.11.12.13/33',),
         ('127.0.0.1',),
+        ('241.1.2.3',),
+        ('169.254.1.2',),
+        ('10.11.12.0/24',),
+        ('224.5.6.7',),
+        ('10.11.12.255/24',),
 
         ('2001::1',         (0x2001, 0, 0, 0, 0, 0, 0, 1), 64),
         ('2001::1/72',      (0x2001, 0, 0, 0, 0, 0, 0, 1), 72),
         ('2001::1beef',),
         ('2001::1/129',),
         ('::1',),
+        ('6789::1',),
+        ('fe89::1',),
+        ('2001::/64',),
+        ('ff01::1',),
 
         ('junk',)
     ]
-- 
1.7.4.4

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

Reply via email to