Re: [Freeipa-devel] [PATCH] 19 Do stricter checking of IP addressed passed to server install

2011-05-30 Thread Martin Kosek
On Fri, 2011-05-27 at 16:50 +0200, Jan Cholasta wrote:
> 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

Re: [Freeipa-devel] [PATCH] 19 Do stricter checking of IP addressed passed to server install

2011-05-27 Thread Jan Cholasta

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
 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_z

Re: [Freeipa-devel] [PATCH] 19 Do stricter checking of IP addressed passed to server install

2011-05-25 Thread Martin Kosek
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.

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: 
...

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 op

Re: [Freeipa-devel] [PATCH] 19 Do stricter checking of IP addressed passed to server install

2011-05-24 Thread Jan Cholasta

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.

--
Jan Cholasta
>From 7cd30f0be8ae4556f67d39348cbb3205d6867f21 Mon Sep 17 00:00:00 2001
From: Jan Cholasta 
Date: Tue, 24 May 2011 15:41:41 +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 2ad9240..c77a93c 100644
--- a/ipapython/ipautil.py
+++ b/ipapython/ipautil.py
@@ -93,6 +93,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:
@@ -119,6 +125,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.interfac

Re: [Freeipa-devel] [PATCH] 19 Do stricter checking of IP addressed passed to server install

2011-05-24 Thread Jan Cholasta

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

--
Jan Cholasta

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


Re: [Freeipa-devel] [PATCH] 19 Do stricter checking of IP addressed passed to server install

2011-05-24 Thread Martin Kosek
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

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


Re: [Freeipa-devel] [PATCH] 19 Do stricter checking of IP addressed passed to server install

2011-05-20 Thread Jan Cholasta

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

--
Jan Cholasta
>From 3f9fbc8dbf8d6981fb7c20a554a78f94ba12d36a Mon Sep 17 00:00:00 2001
From: Jan Cholasta 
Date: Fri, 20 May 2011 20:21:11 +0200
Subject: [PATCH] Do stricter checking of IP addressed passed to server
 install.

ticket 1213
---
 ipapython/ipautil.py |   11 +++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/ipapython/ipautil.py b/ipapython/ipautil.py
index 2ad9240..c77a93c 100644
--- a/ipapython/ipautil.py
+++ b/ipapython/ipautil.py
@@ -93,6 +93,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:
@@ -119,6 +125,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.interface = iface
-- 
1.7.4.4

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

Re: [Freeipa-devel] [PATCH] 19 Do stricter checking of IP addressed passed to server install

2011-05-18 Thread Martin Kosek
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

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


Re: [Freeipa-devel] [PATCH] 19 Do stricter checking of IP addressed passed to server install

2011-05-16 Thread Jan Cholasta

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.



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.




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.



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.



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.




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.





Martin



Honza


--
Jan Cholasta

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


Re: [Freeipa-devel] [PATCH] 19 Do stricter checking of IP addressed passed to server install

2011-05-16 Thread Jakub Hrozek
On 05/16/2011 05:26 PM, Martin Kosek wrote:
> 5) I think we may want to check also for local address in 
> #ipa host-add $HOST --ip-address=127.0.0.1

Just a note - IPAddress.check is_link_local() from python-netaddr can do
the check for you



signature.asc
Description: OpenPGP digital signature
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH] 19 Do stricter checking of IP addressed passed to server install

2011-05-16 Thread Martin Kosek
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.

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.

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.

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.

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

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.


Martin

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