Re: [Freeipa-devel] [freeipa] #3668: CA-less install fails when intermediate CA is used

2013-06-12 Thread Jan Pazdziora
On Fri, Jun 07, 2013 at 09:23:48AM -0400, Dmitri Pal wrote:
 
  The problem is that if you pass IPA certificates issued by CA2 and
  point it to CA1 at the same time, it does not work (despite having the
  complete trust chain).
 
 But why would you do so? What would be the reason and business case? Why
 not to point to CA2?

Could the business case be an IPA server in DMZ which does not have
access to CA2 but it can get to (public) CA1?

-- 
Jan Pazdziora | adelton at #ipa*, #brno
Principal Software Engineer, Identity Management Engineering, Red Hat

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


Re: [Freeipa-devel] [PATCH] 0033 Fail when adding a trust with a different range

2013-06-12 Thread Alexander Bokovoy

On Mon, 10 Jun 2013, Ana Krivokapic wrote:

And once here(added by your patch):

+if not self.validate_range(*keys, **options):
+raise errors.ValidationError(
+name=_('id range'),
+error=_(
+'An id range already exists for this trust. '
+'You should either delete the old range, or '
+'exclude --base-id/--range-size options from the
command.'

I'd suggest we have one common place, say validate_range() function as
we have now,
that contains all the checks (more coming in the future for sure).

That would mean that the whole trust-add command will fail in the case
that ID range
with the same name but different domain SID already exists, since we
now call validate_range()
within execute() method. I checked with Alexander and we agreed that
this is more desired behaviour.

This would also result to reducement of the number of API calls, which
is a nice benefit.

Tomas


This updated patch completely separates validation logic and object
creation logic of the trust_add command. I added two new methods:

* validate_range(), which ensures that the options and environment
related to idrange is valid, and
* validate_options, which takes care of other general validation

One change introduced in this patch is making the
__populate_remote_domain() method of TrustDomainJoins class public, and
calling it from trust_add. This was necessary in order to enable
discovering details of the trusted domain in validation phase.

Looks good overall but I'd suggest to wrap populate_remote_domain()
calls in join_ad_* methods instead of removing them. If remote domain is
not initialized (i.e. not(isinstance(self.remote_domain,TrustedDomainInstance)),
then call populate_remote_domain() as it was before.

--
/ Alexander Bokovoy

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


Re: [Freeipa-devel] [PATCH] 0033 Fail when adding a trust with a different range

2013-06-12 Thread Tomas Babej

On 06/12/2013 10:23 AM, Alexander Bokovoy wrote:

On Mon, 10 Jun 2013, Ana Krivokapic wrote:

And once here(added by your patch):

+if not self.validate_range(*keys, **options):
+raise errors.ValidationError(
+name=_('id range'),
+error=_(
+'An id range already exists for this trust. '
+'You should either delete the old range, or '
+'exclude --base-id/--range-size options from the
command.'

I'd suggest we have one common place, say validate_range() function as
we have now,
that contains all the checks (more coming in the future for sure).

That would mean that the whole trust-add command will fail in the case
that ID range
with the same name but different domain SID already exists, since we
now call validate_range()
within execute() method. I checked with Alexander and we agreed that
this is more desired behaviour.

This would also result to reducement of the number of API calls, which
is a nice benefit.

Tomas


This updated patch completely separates validation logic and object
creation logic of the trust_add command. I added two new methods:

* validate_range(), which ensures that the options and environment
related to idrange is valid, and
* validate_options, which takes care of other general validation

One change introduced in this patch is making the
__populate_remote_domain() method of TrustDomainJoins class public, and
calling it from trust_add. This was necessary in order to enable
discovering details of the trusted domain in validation phase.

Looks good overall but I'd suggest to wrap populate_remote_domain()
calls in join_ad_* methods instead of removing them. If remote domain is
not initialized (i.e. 
not(isinstance(self.remote_domain,TrustedDomainInstance)),

then call populate_remote_domain() as it was before.


I tested the patch and it works fine.

There's a lot of refactoring done, but the changes preserve equal state. 
Aside from

Alexander's comment up here, I have but one nitpick.

There are few constructs of the form:

try:
   value = dictionary['keyword']
except KeyError:
   value = None

or

if 'keyword' in dictionary:
value = dictionary['keyword']
else:
value = None

Can you please substitute these with value = dictionary.get(keyword, 
None)?
Not everywhere, but there are places where it can improve readability of 
the code.


All from me for now,

Tomas

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


Re: [Freeipa-devel] [PATCH] 0033 Fail when adding a trust with a different range

2013-06-12 Thread Martin Kosek
On 06/12/2013 11:40 AM, Tomas Babej wrote:
 On 06/12/2013 10:23 AM, Alexander Bokovoy wrote:
 On Mon, 10 Jun 2013, Ana Krivokapic wrote:
 And once here(added by your patch):

 +if not self.validate_range(*keys, **options):
 +raise errors.ValidationError(
 +name=_('id range'),
 +error=_(
 +'An id range already exists for this trust. '
 +'You should either delete the old range, or '
 +'exclude --base-id/--range-size options from the
 command.'

 I'd suggest we have one common place, say validate_range() function as
 we have now,
 that contains all the checks (more coming in the future for sure).

 That would mean that the whole trust-add command will fail in the case
 that ID range
 with the same name but different domain SID already exists, since we
 now call validate_range()
 within execute() method. I checked with Alexander and we agreed that
 this is more desired behaviour.

 This would also result to reducement of the number of API calls, which
 is a nice benefit.

 Tomas

 This updated patch completely separates validation logic and object
 creation logic of the trust_add command. I added two new methods:

 * validate_range(), which ensures that the options and environment
 related to idrange is valid, and
 * validate_options, which takes care of other general validation

 One change introduced in this patch is making the
 __populate_remote_domain() method of TrustDomainJoins class public, and
 calling it from trust_add. This was necessary in order to enable
 discovering details of the trusted domain in validation phase.
 Looks good overall but I'd suggest to wrap populate_remote_domain()
 calls in join_ad_* methods instead of removing them. If remote domain is
 not initialized (i.e. 
 not(isinstance(self.remote_domain,TrustedDomainInstance)),
 then call populate_remote_domain() as it was before.

 I tested the patch and it works fine.
 
 There's a lot of refactoring done, but the changes preserve equal state. Aside
 from
 Alexander's comment up here, I have but one nitpick.
 
 There are few constructs of the form:
 
 try:
value = dictionary['keyword']
 except KeyError:
value = None
 
 or
 
 if 'keyword' in dictionary:
 value = dictionary['keyword']
 else:
 value = None
 
 Can you please substitute these with value = dictionary.get(keyword, None)?
 Not everywhere, but there are places where it can improve readability of the 
 code.

You can even use just value = dictionary.get(keyword) as None is the default
return value:

 print {}.get(foo)
None

Martin

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


Re: [Freeipa-devel] [PATCHES] 134-139 CA-less fixes

2013-06-12 Thread Petr Viktorin

On 06/07/2013 03:39 PM, Jan Cholasta wrote:

Hi,

the attached patches fix some of the issues I have found while working
on test plan for CA-less install (see
http://www.freeipa.org/index.php/V3/CA-less_install for more
information on that).

https://fedorahosted.org/freeipa/ticket/3665
https://fedorahosted.org/freeipa/ticket/3667
https://fedorahosted.org/freeipa/ticket/3673
https://fedorahosted.org/freeipa/ticket/3674
https://fedorahosted.org/freeipa/ticket/3675

Honza



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



ACK, pushed to master  ipa-3-2.

--
PetrĀ³

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


[Freeipa-devel] [PATCH 0073] Remove support for IPA deployments with no persistent search

2013-06-12 Thread Tomas Babej

Hi,

Drops the code from ipa-server-install, ipa-dns-install and the
BindInstance itself. Also changed ipa-upgradeconfig script so
that it does not set zone_refresh to 0 on upgrades, as the option
is deprecated, but rather removes it altogether.

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

Tomas
From 7afe2861a85ec18ec19a7dbf47e8ba1da9ff18de Mon Sep 17 00:00:00 2001
From: Tomas Babej tba...@redhat.com
Date: Mon, 3 Jun 2013 14:37:20 +0200
Subject: [PATCH] Remove support for IPA deployments with no persistent search

Drops the code from ipa-server-install, ipa-dns-install and the
BindInstance itself. Also changed ipa-upgradeconfig script so
that it does not set zone_refresh to 0 on upgrades, as the option
is deprecated.

https://fedorahosted.org/freeipa/ticket/3632
---
 install/share/bind.named.conf.template |  1 -
 install/tools/ipa-dns-install  | 14 --
 install/tools/ipa-server-install   | 18 --
 install/tools/ipa-upgradeconfig|  3 ++-
 install/tools/man/ipa-dns-install.1|  6 --
 install/tools/man/ipa-server-install.1 |  6 --
 ipaserver/install/bindinstance.py  |  7 ++-
 7 files changed, 4 insertions(+), 51 deletions(-)

diff --git a/install/share/bind.named.conf.template b/install/share/bind.named.conf.template
index e4ce6058399e8d9a1f112f55907e060075dff00b..f78e18b5fd1d44e4d75d8b412994f2810ede8d97 100644
--- a/install/share/bind.named.conf.template
+++ b/install/share/bind.named.conf.template
@@ -44,7 +44,6 @@ dynamic-db ipa {
 	arg auth_method sasl;
 	arg sasl_mech GSSAPI;
 	arg sasl_user DNS/$FQDN;
-	arg zone_refresh $ZONE_REFRESH;
 	arg psearch $PERSISTENT_SEARCH;
 	arg serial_autoincrement $SERIAL_AUTOINCREMENT;
 };
diff --git a/install/tools/ipa-dns-install b/install/tools/ipa-dns-install
index e12a0465ca2d09a6a8d25157a737f620f3ff4b1a..bc430c1e5e87f66d3d0db94b850c578f00dd798a 100755
--- a/install/tools/ipa-dns-install
+++ b/install/tools/ipa-dns-install
@@ -55,13 +55,6 @@ def parse_options():
 # this option name has been deprecated, persistent search has been enabled by default
 parser.add_option(--zone-notif, dest=zone_notif,
   action=store_true, default=False, help=SUPPRESS_HELP)
-parser.add_option(--no-persistent-search, dest=persistent_search,
-  default=True, action=store_false,
-  help=Do not enable persistent search feature in the name server)
-parser.add_option(--zone-refresh, dest=zone_refresh,
-  default=0, type=int,
-  help=When set to non-zero the name server will use DNS zone 
-   detection based on polling instead of a persistent search)
 parser.add_option(--no-serial-autoincrement, dest=serial_autoincrement,
   default=True, action=store_false,
   help=Do not enable SOA serial autoincrement)
@@ -80,11 +73,6 @@ def parse_options():
 if not options.forwarders and not options.no_forwarders:
 parser.error(You must specify at least one --forwarder option or --no-forwarders option)
 
-if options.zone_refresh  0:
-parser.error(negative numbers not allowed for --zone-refresh)
-elif options.zone_refresh  0:
-options.persistent_search = False   # mutually exclusive features
-
 if options.zone_notif:
 print sys.stderr, WARNING: --zone-notif option is deprecated and has no effect
 
@@ -232,8 +220,6 @@ def main():
 
 bind.setup(api.env.host, ip_address, api.env.realm, api.env.domain,
dns_forwarders, conf_ntp, reverse_zone, zonemgr=options.zonemgr,
-   zone_refresh=options.zone_refresh,
-   persistent_search=options.persistent_search,
serial_autoincrement=options.serial_autoincrement)
 bind.create_instance()
 
diff --git a/install/tools/ipa-server-install b/install/tools/ipa-server-install
index 62adbd5bc5183793f3371e46e276b9ad20077b84..f0a0fd754558d3503532be3bf6d92773d61d1cff 100755
--- a/install/tools/ipa-server-install
+++ b/install/tools/ipa-server-install
@@ -209,13 +209,6 @@ def parse_options():
 # this option name has been deprecated, persistent search has been enabled by default
 dns_group.add_option(--zone-notif, dest=zone_notif,
   action=store_true, default=False, help=SUPPRESS_HELP)
-dns_group.add_option(--no-persistent-search, dest=persistent_search,
-  default=True, action=store_false,
-  help=Do not enable persistent search feature in the name server)
-dns_group.add_option(--zone-refresh, dest=zone_refresh,
-  default=0, type=int,
-  help=When set to non-zero the name server will use DNS zone 
-   detection based on polling instead of a persistent search)
 dns_group.add_option(--no-host-dns, dest=no_host_dns, action=store_true,
   default=False,
 

Re: [Freeipa-devel] [PATCH 0030] Require rid-base and secondary-rid-base options in idrange-add when trust exists

2013-06-12 Thread Ana Krivokapic
On 06/11/2013 06:44 PM, Alexander Bokovoy wrote:
 On Tue, 11 Jun 2013, Martin Kosek wrote:
 2) Is the used ldapsearch really the best way to find out if Trust is
 configured on a given master? Isn't a search in cn=masters,cn=ipa,... 
 better?
 Alexander?
 What would the search in cn=masters,cn=ipa,.. give?

 We can have multiple CIFS services per realm. However, only those in
 'adtrust agents' group are the ones which are real DCs. And since
 membership in the group is not handled via framework or UI, it is clear
 indication that ipa-adtrust-install was run.

 It would say if there as an appropriate service configured by
 ipa-adtrust-install. In this case,
 cn=ADTRUST,cn=FQDN,cn=masters,cn=ipa,cn=etc,SUFFIX. I am asking because this
 is a standard way in FreeIPA to ask for configured services.

 If that does not work for Trust, then your alternative way should be OK too.
 This would work for making sure that ipa-adtrust-install was run on a
 specific server. It will not work for making sure trusts are enabled
 but in this case we only need to know that we have configured the host
 to be a DC so your approach is fine.

 I'm fine to use this approach, somehow it slipped out of my view when we
 discussed it with Ana..



I amended the name of the new command to 'adtrust_is_enabled'. I also simplified
the LDAP search used in the command, as suggested by Martin and Alexander.

Updated patch is attached.

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

From 7091646092771d1bcd5be26c97db72e63122cde2 Mon Sep 17 00:00:00 2001
From: Ana Krivokapic akriv...@redhat.com
Date: Mon, 10 Jun 2013 18:57:08 -0400
Subject: [PATCH] Require rid-base and secondary-rid-base in idrange-add after
 ipa-adtrust-install

Add a new API command 'adtrust_is_enabled', which can be used to determine
whether ipa-adtrust-install has been run on the system. This new command is not
visible in IPA CLI.

Use this command in idrange_add to conditionally require rid-base and
secondary-rid-base options.

Add tests to cover the new functionality

https://fedorahosted.org/freeipa/ticket/3634
---
 API.txt|  4 ++
 VERSION|  2 +-
 ipalib/plugins/idrange.py  | 35 +-
 ipalib/plugins/trust.py| 32 +++--
 tests/test_cmdline/test_cli.py | 74 +
 tests/test_xmlrpc/test_range_plugin.py | 85 ++
 tests/util.py  | 37 ---
 7 files changed, 196 insertions(+), 73 deletions(-)

diff --git a/API.txt b/API.txt
index 1313460de66d8e12fc7a068cda0cf30658bcdd1b..bbffbd4b37d939ec79520ff8d3e0184e28b7d3e2 100644
--- a/API.txt
+++ b/API.txt
@@ -101,6 +101,10 @@ command: aci_show
 output: Entry('result', type 'dict', Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None))
 output: Output('summary', (type 'unicode', type 'NoneType'), None)
 output: Output('value', type 'unicode', None)
+command: adtrust_is_enabled
+args: 0,1,1
+option: Str('version?', exclude='webui')
+output: Output('result', None, None)
 command: automember_add
 args: 1,7,3
 arg: Str('cn', cli_name='automember_rule')
diff --git a/VERSION b/VERSION
index a95ccb91457c4caf9767843951b8290b15b377d6..c713b18b7ce42db7fb290912ec6028342015f142 100644
--- a/VERSION
+++ b/VERSION
@@ -89,4 +89,4 @@ IPA_DATA_VERSION=2010061412
 #  #
 
 IPA_API_VERSION_MAJOR=2
-IPA_API_VERSION_MINOR=59
+IPA_API_VERSION_MINOR=60
diff --git a/ipalib/plugins/idrange.py b/ipalib/plugins/idrange.py
index 54b835e244fb60ee212a9c00223d4294ff8f4363..f258cbb15565f62a56017d0909dfaff17202282a 100644
--- a/ipalib/plugins/idrange.py
+++ b/ipalib/plugins/idrange.py
@@ -356,7 +356,7 @@ class idrange_add(LDAPCreate):
 
 may be given for a new ID range for the local domain while
 
---rid-bas
+--rid-base
 --dom-sid
 
 must be given to add a new range for a trusted AD domain.
@@ -381,6 +381,9 @@ def interactive_prompt_callback(self, kw):
 
 Also ensure that secondary-rid-base is prompted for when rid-base is
 specified and vice versa, in case that dom-sid was not specified.
+
+Also ensure that rid-base and secondary-rid-base is prompted for
+if ipa-adtrust-install has been run on the system.
 
 
 # dom-sid can be specified using dom-sid or dom-name options
@@ -410,6 +413,22 @@ def interactive_prompt_callback(self, kw):
 value = self.prompt_param(self.params['ipabaserid'])
 kw.update(dict(ipabaserid=value))
 
+# Prompt for rid-base and secondary-rid-base if ipa-adtrust-install
+# has been run on the system
+adtrust_is_enabled = api.Command['adtrust_is_enabled']()['result']
+
+if adtrust_is_enabled:
+rid_base = 

Re: [Freeipa-devel] [freeipa] #3668: CA-less install fails when intermediate CA is used

2013-06-12 Thread Rob Crittenden

Jan Pazdziora wrote:

On Fri, Jun 07, 2013 at 09:23:48AM -0400, Dmitri Pal wrote:


The problem is that if you pass IPA certificates issued by CA2 and
point it to CA1 at the same time, it does not work (despite having the
complete trust chain).


But why would you do so? What would be the reason and business case? Why
not to point to CA2?


Could the business case be an IPA server in DMZ which does not have
access to CA2 but it can get to (public) CA1?



A client does need to be able to contact a CA in order to trust it.

rob

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