date:20121002

Re: [Freeipa-devel] [PATCH] 0081 Support both unified samba and samba/samba4-packages

2012-10-02 Thread Martin Kosek

On 10/01/2012 06:08 PM, Alexander Bokovoy wrote:
 On Mon, 01 Oct 2012, Martin Kosek wrote:
 +%else
 Requires: samba4-python
 Requires: samba4
 -Requires: libsss_idmap
 Requires: samba4-winbind
 +%endif
 +Requires: libsss_idmap   
 :) Thanks.
 I was not looking properly.

 ACK

 Pushed to master, ipa-3-0.

 I just added ticket #3118 to patch description (I discovered there is 
 already a
 filed ticket for this change).

 I think we do not need to update our Fedora packages until RC2 release since
 dependencies are not broken - samba packages have samba4 provides... I 
 verified
 by installing freeipa-server-3.0.0-0.6.fc18 on F18 box with new unified samba
 packages.
 Aside from binary compatibility, there are regulard rebuilds of Rawhide
 and they failed for us on Friday, as Stephen has discovered. So, maybe
 we'd better update Rawhide with the patch?
 

Ok. I updated Fedora GIT and produced builds for both F18 and rawhide.

Martin

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

[Freeipa-devel] [PATCHES] 3 enhancements for the ipa-adtrust-install page

2012-10-02 Thread Sumit Bose

Hi,

the following three patches should fix
https://fedorahosted.org/freeipa/ticket/2967
https://fedorahosted.org/freeipa/ticket/2972
https://fedorahosted.org/freeipa/ticket/3038 respectively.

bye,
Sumit
From bab787a651773ec9bead34cfaaec05991ebc74c4 Mon Sep 17 00:00:00 2001
From: Sumit Bose sb...@redhat.com
Date: Mon, 1 Oct 2012 13:36:00 +0200
Subject: [PATCH] Add man page paragraph about running ipa-adtrust-install
 multiple times

Fixes https://fedorahosted.org/freeipa/ticket/2967
---
 install/tools/man/ipa-adtrust-install.1 | 8 
 1 Datei geändert, 8 Zeilen hinzugefügt(+)

diff --git a/install/tools/man/ipa-adtrust-install.1 
b/install/tools/man/ipa-adtrust-install.1
index 
5303ec27be2af3d36c0e83d839625c3bdd6816a4..dc48ac8cdf5342ff3750b2bf1965ee25224e26fb
 100644
--- a/install/tools/man/ipa-adtrust-install.1
+++ b/install/tools/man/ipa-adtrust-install.1
@@ -25,6 +25,14 @@ ipa\-adtrust\-install [\fIOPTION\fR]...
 Adds all necessary objects and configuration to allow an IPA server to create a
 trust to an Active Directory domain. This requires that the IPA server is
 already installed and configured.
+
+ipa\-adtrust\-install can be run multiple times to reinstall deleted objects or
+broken configuration files. E.g. a fresh samba configuration (smb.conf file and
+registry based configuration can be created. Other items like e.g. the
+configuration of the local range cannot be changed by running
+ipa\-adtrust\-install a second time because with changes here other objects
+might be affected as well.
+
 .SH OPTIONS
 .TP
 \fB\-d\fR, \fB\-\-debug\fR
-- 
1.7.11.4

From ff2700ab7b793ae167823dc3d93c131e0d8ea998 Mon Sep 17 00:00:00 2001
From: Sumit Bose sb...@redhat.com
Date: Mon, 1 Oct 2012 21:43:45 +0200
Subject: [PATCH] Enhance description of --no-msdcs in man page

Fixes https://fedorahosted.org/freeipa/ticket/2972
---
 install/tools/man/ipa-adtrust-install.1 | 26 +-
 1 Datei geändert, 25 Zeilen hinzugefügt(+), 1 Zeile entfernt(-)

diff --git a/install/tools/man/ipa-adtrust-install.1 
b/install/tools/man/ipa-adtrust-install.1
index 
dc48ac8cdf5342ff3750b2bf1965ee25224e26fb..13f111004eda4477db948355d26f524409805b7b
 100644
--- a/install/tools/man/ipa-adtrust-install.1
+++ b/install/tools/man/ipa-adtrust-install.1
@@ -45,7 +45,31 @@ The IP address of the IPA server. If not provided then this 
is determined based
 The NetBIOS name for the IPA domain. If not provided then this is determined 
based on the leading component of the DNS domain name.
 .TP
 \fB\-\-no\-msdcs\fR
-Do not create DNS service records for Windows in managed DNS server
+Do not create DNS service records for Windows in managed DNS server. Since 
those
+DNS service records are the only way to discover domain controllers of other
+domains they must be added manually to a different DNS server to allow trust
+realationships work properly. All needed service records are listed when
+ipa\-adtrust\-install finishes and either \-\-no\-msdcs was given or no IPA DNS
+service is configured. Typically service records for the following service 
names
+are needed for the IPA domain which should point to all IPA servers:
+.IP
+\(bu _ldap._tcp
+.IP
+\(bu _kerberos._tcp
+.IP
+\(bu _kerberos._udp
+.IP
+\(bu _ldap._tcp.dc._msdcs
+.IP
+\(bu _kerberos._tcp.dc._msdcs
+.IP
+\(bu _kerberos._udp.dc._msdcs
+.IP
+\(bu _ldap._tcp.Default-First-Site-Name._sites.dc._msdcs
+.IP
+\(bu _kerberos._tcp.Default-First-Site-Name._sites.dc._msdcs
+.IP
+\(bu _kerberos._udp.Default-First-Site-Name._sites.dc._msdcs
 .TP
 \fB\-U\fR, \fB\-\-unattended\fR
 An unattended installation that will never prompt for user input
-- 
1.7.11.4

From 335ea2644ba8b171a288223dedbe6a237316e8f7 Mon Sep 17 00:00:00 2001
From: Sumit Bose sb...@redhat.com
Date: Mon, 1 Oct 2012 21:54:00 +0200
Subject: [PATCH] Add --rid-base and --secondary-rid-base to
 ipa-adtrust-install man page

Fixes https://fedorahosted.org/freeipa/ticket/3038
---
 install/tools/man/ipa-adtrust-install.1 | 10 ++
 1 Datei geändert, 10 Zeilen hinzugefügt(+)

diff --git a/install/tools/man/ipa-adtrust-install.1 
b/install/tools/man/ipa-adtrust-install.1
index 
13f111004eda4477db948355d26f524409805b7b..fa63bca3c4859325acb5891de6ad1e21b97dc754
 100644
--- a/install/tools/man/ipa-adtrust-install.1
+++ b/install/tools/man/ipa-adtrust-install.1
@@ -74,6 +74,16 @@ are needed for the IPA domain which should point to all IPA 
servers:
 \fB\-U\fR, \fB\-\-unattended\fR
 An unattended installation that will never prompt for user input
 .TP
+\fB\-U\fR, \fB\-\-rid-base\fR=\fIRID_BASE\fR
+First RID value of the local domain. The first Posix ID of the local domain 
will
+be assigned to this RID, the second to RID+1 etc. See the online help of the
+idrange CLI for details.
+.TP
+\fB\-U\fR, \fB\-\-secondary-rid-base\fR=\fISECONDARY_RID_BASE\fR
+Start value of the secondary RID range, which is only used in the case a user
+and a group share numerically the same Posix ID. See the online help of the
+idrange CLI for

Re: [Freeipa-devel] [PATCH] 1037 optimize restoring SELinux booleans

2012-10-02 Thread Petr Viktorin


On 10/01/2012 09:29 PM, Rob Crittenden wrote:

Petr Viktorin wrote:

On 10/01/2012 04:41 PM, Rob Crittenden wrote:

The web uninstall step can be very long because we restore two SELinux
booleans individually. This patch combines them into a single step, and
skips setting them if the values won't actually change.

rob




Is there a reason to not reuse the code that sets the values on install?
As far as I can tell it does the same thing slightly differently.



The differences are enough that trying to consolidate them would likely
end up taking considerable more time, require considerable more testing,
etc. It would be worthwhile to revisit this at the beginning of a new
version, but at the end it seems safer to take the simplest route.

rob


Well, okay then, ACK. But please keep the bug open.

--
Petr³

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

Re: [Freeipa-devel] [PATCH] 316 Improve DN usage in ipa-client-install

2012-10-02 Thread Petr Viktorin


On 09/27/2012 01:35 PM, Martin Kosek wrote:

A hotfix pushed in a scope of ticket 3088 forced conversion of DN
object (baseDN) in IPA client discovery so that ipa-client-install
does not crash when creating an IPA default.conf. Since this is not
a preferred way to handle DN objects, improve its usage:

- make sure, that baseDN retrieved by client discovery is always
   a DN object
- update ipachangeconf.py code to handle strings better and instead
   of concatenating objects, make sure they are converted to string
   first

As a side-effect of ipachangeconf changes, default.conf config file
generated by ipa-client-install has no longer empty new line at the
end of a file.

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


ACK, but since you're reformatting the code, here are some minor issues 
found by the PEP8 checker.



diff --git a/ipa-client/ipaclient/ipachangeconf.py 
b/ipa-client/ipaclient/ipachangeconf.py
index 
f6288062be5c5d1b29341ed90814e1fa1431019c..2bb0cc487670ce74833fc09b06449d0a176ffaf5
 100644
--- a/ipa-client/ipaclient/ipachangeconf.py
+++ b/ipa-client/ipaclient/ipachangeconf.py
@@ -68,7 +68,7 @@ class IPAChangeConf:
  elif type(indent) is str:
  self.indent = (indent, )
  else:
-   raise ValueError, 'Indent must be a list of strings'
+   raise ValueError('Indent must be a list of strings')


Please re-indent to 4 spaces.



@@ -143,35 +143,50 @@ class IPAChangeConf:
  def getSectionLine(self, section):
  if len(self.sectnamdel) != 2:
  return section
-return self.sectnamdel[0]+section+self.sectnamdel[1]+self.deol
+return self._dump_line(self.sectnamdel[0],
+   section,
+   self.sectnamdel[1],
+   self.deol)
+
+def _dump_line(self, *args):
+return u.join(unicode(x) for x in args)

  def dump(self, options, level=0):
-output = 
+output = []
  if level = len(self.indent):
  level = len(self.indent)-1

  for o in options:
  if o['type'] == section:
-output += 
self.sectnamdel[0]+o['name']+self.sectnamdel[1]+self.deol
-output += self.dump(o['value'], level+1)
+output.append(self._dump_line(self.sectnamdel[0],
+  o['name'],
+  self.sectnamdel[1]))
+output.append(self.dump(o['value'], level+1))


Please surround the + operator by spaces (here  below).


@@ -274,15 +297,19 @@ class IPAChangeConf:
  opts.append(o)
  continue
  if no['action'] == 'comment':
-   opts.append({'name':'comment', 'type':'comment',
-
'value':self.dcomment+o['name']+self.dassign+o['value']})
+value = self._dump_line(self.dcomment,
+o['name'],
+self.dassign,
+o['value'])
+opts.append({'name':'comment', 'type':'comment',
+ 'value':value})


Please add a space after each colon.

--
Petr³

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

[Freeipa-devel] [PATCH] Fix various issues found by Coverity

2012-10-02 Thread Sumit Bose

Hi,

this patch fixes a couple of resource leaks and unchecked return and an
uninitialised value found by Coverity.

bye,
Sumit
From b39269b5adf5d2ae6076d5aa4394e68924027ce6 Mon Sep 17 00:00:00 2001
From: Sumit Bose sb...@redhat.com
Date: Tue, 2 Oct 2012 11:25:04 +0200
Subject: [PATCH] Fix various issues found by Coverity

---
 daemons/ipa-sam/ipa_sam.c|  2 +-
 daemons/ipa-slapi-plugins/ipa-cldap/ipa_cldap.c  |  6 --
 .../ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_common.c   | 12 +++-
 .../ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_extop.c|  6 --
 daemons/ipa-slapi-plugins/ipa-range-check/ipa_range_check.c  |  3 ++-
 daemons/ipa-slapi-plugins/ipa-sidgen/ipa_sidgen_task.c   |  5 -
 6 Dateien geändert, 22 Zeilen hinzugefügt(+), 12 Zeilen entfernt(-)

diff --git a/daemons/ipa-sam/ipa_sam.c b/daemons/ipa-sam/ipa_sam.c
index 
3d4f741c157fb624e272d800bd9e0cdf9edb9444..db300022af0a29c32a2df5e5ef2bc12f39ed2886
 100644
--- a/daemons/ipa-sam/ipa_sam.c
+++ b/daemons/ipa-sam/ipa_sam.c
@@ -1522,7 +1522,7 @@ static int set_cross_realm_pw(struct ldapsam_privates 
*ldap_state,
krb5_error_code krberr;
krb5_context krbctx;
krb5_principal service_princ;
-   struct keys_container keys;
+   struct keys_container keys = {0, NULL};
char *err_msg;
struct berval *reqdata = NULL;
struct berval *retdata = NULL;
diff --git a/daemons/ipa-slapi-plugins/ipa-cldap/ipa_cldap.c 
b/daemons/ipa-slapi-plugins/ipa-cldap/ipa_cldap.c
index 
d7a59d51229ca9fbf754a2600e42339dcc235698..54d44ebf64b1efa0dda06773736d3413a6b70977
 100644
--- a/daemons/ipa-slapi-plugins/ipa-cldap/ipa_cldap.c
+++ b/daemons/ipa-slapi-plugins/ipa-cldap/ipa_cldap.c
@@ -124,13 +124,15 @@ static int ipa_cldap_init_service(Slapi_PBlock *pb,
 slapi_pblock_get(pb, SLAPI_PLUGIN_CONFIG_ENTRY, e);
 if (!e) {
 LOG_FATAL(Plugin configuration not found!\n);
-return -1;
+ret = -1;
+goto done;
 }
 
 ctx-base_dn = slapi_entry_attr_get_charptr(e, nsslapd-basedn);
 if (!ctx-base_dn) {
 LOG_FATAL(Plugin configuration not found!\n);
-return -1;
+ret = -1;
+goto done;
 }
 
 /* create a stop pipe so the main DS thread can interrupt the poll()
diff --git a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_common.c 
b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_common.c
index 
f48bead04cbb18b040557b7b78a6cb27a3368422..47d4d68d1d7f5e4f02ad68849b840eaa63f7c33d
 100644
--- a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_common.c
+++ b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_common.c
@@ -137,10 +137,12 @@ int parse_request_data(struct berval *req_val, struct 
extdom_req **_req)
 break;
 default:
 ber_free(ber, 1);
+free(req);
 return LDAP_PROTOCOL_ERROR;
 }
 ber_free(ber, 1);
 if (tag == LBER_ERROR) {
+free(req);
 return LDAP_PROTOCOL_ERROR;
 }
 
@@ -284,11 +286,6 @@ static int get_domain_info(struct ipa_extdom_ctx *ctx, 
const char *domain_name,
 domain_info-flat_name = slapi_entry_attr_get_charptr(e[0],
   ipaNTFlatName);
 
-/* TODO: read range from LDAP server */
-/*
-range.min = 20;
-range.max = 40;
-*/
 ret = set_domain_range(ctx, domain_info-sid, range);
 if (ret != 0) {
 goto done;
@@ -313,6 +310,11 @@ done:
 slapi_free_search_results_internal(pb);
 slapi_pblock_destroy(pb);
 free(filter);
+
+if (ret != 0) {
+free_domain_info(domain_info);
+}
+
 return ret;
 
 }
diff --git a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_extop.c 
b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_extop.c
index 
d5a2f604c4a61bda04dd026ace4b53ea5c2c3645..f36878c37a7fca2b06db70a4f7694ee53d7c9422
 100644
--- a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_extop.c
+++ b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_extop.c
@@ -170,13 +170,15 @@ static int ipa_extdom_init_ctx(Slapi_PBlock *pb, struct 
ipa_extdom_ctx **_ctx)
 slapi_pblock_get(pb, SLAPI_PLUGIN_CONFIG_ENTRY, e);
 if (!e) {
 LOG_FATAL(Plugin configuration not found!\n);
-return -1;
+ret = -1;
+goto done;
 }
 
 ctx-base_dn = slapi_entry_attr_get_charptr(e, nsslapd-basedn);
 if (!ctx-base_dn) {
 LOG_FATAL(Base DN not found in plugin configuration not found!\n);
-return -1;
+ret = -1;
+goto done;
 }
 
 
diff --git a/daemons/ipa-slapi-plugins/ipa-range-check/ipa_range_check.c 
b/daemons/ipa-slapi-plugins/ipa-range-check/ipa_range_check.c
index 
499e54a9c4a4c9134a231c0cd09e700390565a14..82479bec8184adb28cbf641d8ff019de64099818
 100644
--- a/daemons/ipa-slapi-plugins/ipa-range-check/ipa_range_check.c
+++ b/daemons/ipa-slapi-plugins/ipa-range-check/ipa_range_check.c
@@

Re: [Freeipa-devel] [PATCH] 319 Make ipakrbprincipal objectclass optional

2012-10-02 Thread Petr Viktorin


On 10/01/2012 05:28 PM, Martin Kosek wrote:

From IPA 3.0, services have by default ipakrbprincipal objectclass which

allows ipakrbprincipalalias attribute used for case-insensitive principal
searches. However, as services created in previous version do not have
this objectclass (and attribute), they are not listed in service list
produced by service-find.

Treat the ipakrbprincipal as optional to avoid missing services in
service-find command. Add flag to service-mod command which can fill
ipakrbprincipalalias attribute when case-insensitive principal searches
for a 2.x service are required.

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


This works, I'm getting all services now  the tests pass.



-

I am still pondering about a right way to fill ipakrbprincipalalias used in for
IPA 3.0 case-insensitive searches, so far I implemented this command:

ipa service-mod PRINCIPAL --update-principal-alias

But I am thinking it may be a better approach to generalize it and do something
like that:

ipa service-mod PRINCIPAL --upgrade/--update



This command would do a general update of service entry to an up-to-date 3.0
style, in this case it could do 2 things:
* fill ipakrbprincipalalias
* fill ipakrbauthzdata (based on default value in IPA config).


I don't think you're generalizing enough; `service-mod --upgrade` isn't 
that different from `service-mod --update-principal-alias 
--update-authzdata`. Scripting this to happen for all services could be 
a nuisance, though. There should be a way to upgrade all services at 
once, and since we already have ipa-ldap-updater for it, it should run 
as part of that.


I think we should keep ipakrbprincipal optional, in case the upgrade 
goes wrong.



--
Petr³

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

Re: [Freeipa-devel] [PATCH] 316 Improve DN usage in ipa-client-install

2012-10-02 Thread Martin Kosek

On 10/02/2012 10:49 AM, Petr Viktorin wrote:
 On 09/27/2012 01:35 PM, Martin Kosek wrote:
 A hotfix pushed in a scope of ticket 3088 forced conversion of DN
 object (baseDN) in IPA client discovery so that ipa-client-install
 does not crash when creating an IPA default.conf. Since this is not
 a preferred way to handle DN objects, improve its usage:

 - make sure, that baseDN retrieved by client discovery is always
a DN object
 - update ipachangeconf.py code to handle strings better and instead
of concatenating objects, make sure they are converted to string
first

 As a side-effect of ipachangeconf changes, default.conf config file
 generated by ipa-client-install has no longer empty new line at the
 end of a file.

 https://fedorahosted.org/freeipa/ticket/3088
 
 ACK, but since you're reformatting the code, here are some minor issues found
 by the PEP8 checker.
 
 diff --git a/ipa-client/ipaclient/ipachangeconf.py
 b/ipa-client/ipaclient/ipachangeconf.py
 index
 f6288062be5c5d1b29341ed90814e1fa1431019c..2bb0cc487670ce74833fc09b06449d0a176ffaf5
 100644
 --- a/ipa-client/ipaclient/ipachangeconf.py
 +++ b/ipa-client/ipaclient/ipachangeconf.py
 @@ -68,7 +68,7 @@ class IPAChangeConf:
   elif type(indent) is str:
   self.indent = (indent, )
   else:
 -   raise ValueError, 'Indent must be a list of strings'
 +   raise ValueError('Indent must be a list of strings')
 
 Please re-indent to 4 spaces.
 

 @@ -143,35 +143,50 @@ class IPAChangeConf:
   def getSectionLine(self, section):
   if len(self.sectnamdel) != 2:
   return section
 -return self.sectnamdel[0]+section+self.sectnamdel[1]+self.deol
 +return self._dump_line(self.sectnamdel[0],
 +   section,
 +   self.sectnamdel[1],
 +   self.deol)
 +
 +def _dump_line(self, *args):
 +return u.join(unicode(x) for x in args)

   def dump(self, options, level=0):
 -output = 
 +output = []
   if level = len(self.indent):
   level = len(self.indent)-1

   for o in options:
   if o['type'] == section:
 -output +=
 self.sectnamdel[0]+o['name']+self.sectnamdel[1]+self.deol
 -output += self.dump(o['value'], level+1)
 +output.append(self._dump_line(self.sectnamdel[0],
 +  o['name'],
 +  self.sectnamdel[1]))
 +output.append(self.dump(o['value'], level+1))
 
 Please surround the + operator by spaces (here  below).
 
 @@ -274,15 +297,19 @@ class IPAChangeConf:
   opts.append(o)
   continue
   if no['action'] == 'comment':
 -   opts.append({'name':'comment', 'type':'comment',
 -   
 'value':self.dcomment+o['name']+self.dassign+o['value']})
 +value = self._dump_line(self.dcomment,
 +o['name'],
 +self.dassign,
 +o['value'])
 +opts.append({'name':'comment', 'type':'comment',
 + 'value':value})
 
 Please add a space after each colon.
 

Since the whole ipachangeconf.py is not much PEP8 compliant, I did not want to
create just small islands of compliance with different coding style that the
rest of the file.

In attached patch I fixed the PEP8 warnings in the whole file.

Martin
From 35ed5e11024981dae2b49ef785b2067af20ace93 Mon Sep 17 00:00:00 2001
From: Martin Kosek mko...@redhat.com
Date: Thu, 27 Sep 2012 12:40:55 +0200
Subject: [PATCH] Improve DN usage in ipa-client-install

A hotfix pushed in a scope of ticket 3088 forced conversion of DN
object (baseDN) in IPA client discovery so that ipa-client-install
does not crash when creating an IPA default.conf. Since this is not
a preferred way to handle DN objects, improve its usage:

- make sure, that baseDN retrieved by client discovery is always
  a DN object
- update ipachangeconf.py code to handle strings better and instead
  of concatenating objects, make sure they are converted to string
  first

As a side-effect of ipachangeconf changes, default.conf config file
generated by ipa-client-install has no longer empty new line at the
end of a file.

Whole ipachangeconf.py has been modified to be compliant with PEP8.

https://fedorahosted.org/freeipa/ticket/3088
---
 ipa-client/ipaclient/ipachangeconf.py | 179 ++
 ipa-client/ipaclient/ipadiscovery.py  |   2 +-
 ipapython/ipautil.py  |   2 +-
 3 files changed, 119 insertions(+), 64 deletions(-)

diff --git a/ipa-client/ipaclient/ipachangeconf.py b/ipa-client/ipaclient/ipachangeconf.py
index

Re: [Freeipa-devel] [PATCH] 316 Improve DN usage in ipa-client-install

2012-10-02 Thread Petr Viktorin


On 10/02/2012 12:48 PM, Martin Kosek wrote:

On 10/02/2012 10:49 AM, Petr Viktorin wrote:

On 09/27/2012 01:35 PM, Martin Kosek wrote:

A hotfix pushed in a scope of ticket 3088 forced conversion of DN
object (baseDN) in IPA client discovery so that ipa-client-install
does not crash when creating an IPA default.conf. Since this is not
a preferred way to handle DN objects, improve its usage:

- make sure, that baseDN retrieved by client discovery is always
a DN object
- update ipachangeconf.py code to handle strings better and instead
of concatenating objects, make sure they are converted to string
first

As a side-effect of ipachangeconf changes, default.conf config file
generated by ipa-client-install has no longer empty new line at the
end of a file.

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


ACK, but since you're reformatting the code, here are some minor issues found
by the PEP8 checker.


diff --git a/ipa-client/ipaclient/ipachangeconf.py
b/ipa-client/ipaclient/ipachangeconf.py
index
f6288062be5c5d1b29341ed90814e1fa1431019c..2bb0cc487670ce74833fc09b06449d0a176ffaf5
100644
--- a/ipa-client/ipaclient/ipachangeconf.py
+++ b/ipa-client/ipaclient/ipachangeconf.py
@@ -68,7 +68,7 @@ class IPAChangeConf:
   elif type(indent) is str:
   self.indent = (indent, )
   else:
-   raise ValueError, 'Indent must be a list of strings'
+   raise ValueError('Indent must be a list of strings')


Please re-indent to 4 spaces.



@@ -143,35 +143,50 @@ class IPAChangeConf:
   def getSectionLine(self, section):
   if len(self.sectnamdel) != 2:
   return section
-return self.sectnamdel[0]+section+self.sectnamdel[1]+self.deol
+return self._dump_line(self.sectnamdel[0],
+   section,
+   self.sectnamdel[1],
+   self.deol)
+
+def _dump_line(self, *args):
+return u.join(unicode(x) for x in args)

   def dump(self, options, level=0):
-output = 
+output = []
   if level = len(self.indent):
   level = len(self.indent)-1

   for o in options:
   if o['type'] == section:
-output +=
self.sectnamdel[0]+o['name']+self.sectnamdel[1]+self.deol
-output += self.dump(o['value'], level+1)
+output.append(self._dump_line(self.sectnamdel[0],
+  o['name'],
+  self.sectnamdel[1]))
+output.append(self.dump(o['value'], level+1))


Please surround the + operator by spaces (here  below).


@@ -274,15 +297,19 @@ class IPAChangeConf:
   opts.append(o)
   continue
   if no['action'] == 'comment':
-   opts.append({'name':'comment', 'type':'comment',
-
'value':self.dcomment+o['name']+self.dassign+o['value']})
+value = self._dump_line(self.dcomment,
+o['name'],
+self.dassign,
+o['value'])
+opts.append({'name':'comment', 'type':'comment',
+ 'value':value})


Please add a space after each colon.



Since the whole ipachangeconf.py is not much PEP8 compliant, I did not want to
create just small islands of compliance with different coding style that the
rest of the file.

In attached patch I fixed the PEP8 warnings in the whole file.

Martin



Looking better. ACK

--
Petr³

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

Re: [Freeipa-devel] [PATCH] 316 Improve DN usage in ipa-client-install

2012-10-02 Thread Martin Kosek

On 10/02/2012 01:33 PM, Petr Viktorin wrote:
 On 10/02/2012 12:48 PM, Martin Kosek wrote:
 On 10/02/2012 10:49 AM, Petr Viktorin wrote:
 On 09/27/2012 01:35 PM, Martin Kosek wrote:
 A hotfix pushed in a scope of ticket 3088 forced conversion of DN
 object (baseDN) in IPA client discovery so that ipa-client-install
 does not crash when creating an IPA default.conf. Since this is not
 a preferred way to handle DN objects, improve its usage:

 - make sure, that baseDN retrieved by client discovery is always
 a DN object
 - update ipachangeconf.py code to handle strings better and instead
 of concatenating objects, make sure they are converted to string
 first

 As a side-effect of ipachangeconf changes, default.conf config file
 generated by ipa-client-install has no longer empty new line at the
 end of a file.

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

 ACK, but since you're reformatting the code, here are some minor issues 
 found
 by the PEP8 checker.

 diff --git a/ipa-client/ipaclient/ipachangeconf.py
 b/ipa-client/ipaclient/ipachangeconf.py
 index
 f6288062be5c5d1b29341ed90814e1fa1431019c..2bb0cc487670ce74833fc09b06449d0a176ffaf5

 100644
 --- a/ipa-client/ipaclient/ipachangeconf.py
 +++ b/ipa-client/ipaclient/ipachangeconf.py
 @@ -68,7 +68,7 @@ class IPAChangeConf:
elif type(indent) is str:
self.indent = (indent, )
else:
 -   raise ValueError, 'Indent must be a list of strings'
 +   raise ValueError('Indent must be a list of strings')

 Please re-indent to 4 spaces.


 @@ -143,35 +143,50 @@ class IPAChangeConf:
def getSectionLine(self, section):
if len(self.sectnamdel) != 2:
return section
 -return self.sectnamdel[0]+section+self.sectnamdel[1]+self.deol
 +return self._dump_line(self.sectnamdel[0],
 +   section,
 +   self.sectnamdel[1],
 +   self.deol)
 +
 +def _dump_line(self, *args):
 +return u.join(unicode(x) for x in args)

def dump(self, options, level=0):
 -output = 
 +output = []
if level = len(self.indent):
level = len(self.indent)-1

for o in options:
if o['type'] == section:
 -output +=
 self.sectnamdel[0]+o['name']+self.sectnamdel[1]+self.deol
 -output += self.dump(o['value'], level+1)
 +output.append(self._dump_line(self.sectnamdel[0],
 +  o['name'],
 +  self.sectnamdel[1]))
 +output.append(self.dump(o['value'], level+1))

 Please surround the + operator by spaces (here  below).

 @@ -274,15 +297,19 @@ class IPAChangeConf:
opts.append(o)
continue
if no['action'] == 'comment':
 -   opts.append({'name':'comment', 'type':'comment',
 -
 'value':self.dcomment+o['name']+self.dassign+o['value']})
 +value = self._dump_line(self.dcomment,
 +o['name'],
 +self.dassign,
 +o['value'])
 +opts.append({'name':'comment', 'type':'comment',
 + 'value':value})

 Please add a space after each colon.


 Since the whole ipachangeconf.py is not much PEP8 compliant, I did not want 
 to
 create just small islands of compliance with different coding style that 
 the
 rest of the file.

 In attached patch I fixed the PEP8 warnings in the whole file.

 Martin

 
 Looking better. ACK
 

Pushed to master, ipa-3-0.

Martin

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

Re: [Freeipa-devel] [PATCH 0015] Restrict admins group modifications

2012-10-02 Thread Tomas Babej


On 09/26/2012 05:44 PM, Martin Kosek wrote:

On 09/25/2012 02:59 PM, Tomas Babej wrote:

On 09/25/2012 02:31 PM, Martin Kosek wrote:

On 09/25/2012 02:22 PM, Tomas Babej wrote:

Hi,

Group-mod command no longer allows --rename and/or --external
changes made to the admins group. In such cases, ProtectedEntryError
is being raised.

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

Tomas

Just based on a quick glance, I see few issues: 1) I would like to 
see unit tests for this scenario 2) Some overlooked debug output: + 
self.log.info(str(keys)) Martin 

I removed the unfortunate debug output and added two unit tests to test the
scenarios.

Tomas

I finally got to a proper review and here it is:

1) I think we should use some common global variable containing protected
groups and not define it in every command separately (duplication - errors).
One is already used:

...
  protected_group_name = u'admins'
...

I would like to see that to be used in both group-del and group-mod. I also
think we should protect trust admins too as this group is also encoded in
trust ACI, i.e. using a global variable like this one:

PROTECTED_GROUPS = (u'admins', u'trust admins')


2) Minor issue, I think instead of:

+is_admins_group = u'admins' in keys

a more common pattern in IPA is the following:

+is_admins_group = keys[-1] == u'admins'


3) We usually do not end our error messages in exceptions with .:

...
+raise errors.ProtectedEntryError(label=u'group',
key=u'admins', reason=u'Cannot be renamed.')
...
+raise errors.ProtectedEntryError(label=u'group',
key=u'admins', reason=u'Cannot support external non-IPA members.')
...

Otherwise the patch looks OK.

Martin

I fixed the relevant issues and added few more test cases as well.

Please check the new patch version.

Tomas
From 123b154d8e4bbb09ac4c150a46930366298f3d99 Mon Sep 17 00:00:00 2001
From: Tomas Babej tba...@redhat.com
Date: Tue, 25 Sep 2012 08:14:57 -0400
Subject: [PATCH] Restrict admins group modifications

Group-mod command no longer allows --rename and/or --external
changes made to the admins group. In such cases, ProtectedEntryError
is being raised.

https://fedorahosted.org/freeipa/ticket/3098
---
 ipalib/errors.py   |  6 +++---
 ipalib/plugins/group.py| 20 ---
 tests/test_xmlrpc/test_group_plugin.py | 36 ++
 3 files changed, 56 insertions(+), 6 deletions(-)

diff --git a/ipalib/errors.py b/ipalib/errors.py
index 6a4e2c5d68f6a6f9b94d8e6b3d7a81d5c1922093..3dc38a4fba1ce826dba03f75937e2609baf3b5bf 100644
--- a/ipalib/errors.py
+++ b/ipalib/errors.py
@@ -1643,18 +1643,18 @@ class LastMemberError(ExecutionError):
 
 class ProtectedEntryError(ExecutionError):
 
-**4309** Raised when an entry being deleted is protected
+**4309** Raised when an entry being deleted or modified in a forbidden way is protected
 
 For example:
  raise ProtectedEntryError(label=u'group', key=u'admins', reason=_(u'privileged group'))
 Traceback (most recent call last):
   ...
-ProtectedEntryError: group admins cannot be deleted: privileged group
+ProtectedEntryError: group admins cannot be deleted/modified: privileged group
 
 
 
 errno = 4309
-format = _('%(label)s %(key)s cannot be deleted: %(reason)s')
+format = _('%(label)s %(key)s cannot be deleted/modified: %(reason)s')
 
 
 ##
diff --git a/ipalib/plugins/group.py b/ipalib/plugins/group.py
index f1e34bd56fc2427e2e9f60da89cab731021e1db0..7ee30f8b5ee884a04948422de70b6325959776ab 100644
--- a/ipalib/plugins/group.py
+++ b/ipalib/plugins/group.py
@@ -107,7 +107,7 @@ Example:
ipa group-add-member ad_admins --groups ad_admins_external
 )
 
-protected_group_name = u'admins'
+PROTECTED_GROUPS = (u'admins', u'trust admins')
 
 class group(LDAPObject):
 
@@ -222,7 +222,7 @@ class group_del(LDAPDelete):
 group_attrs = self.obj.methods.show(
 self.obj.get_primary_key_from_dn(dn), all=True
 )['result']
-if keys[0] == protected_group_name:
+if keys[0] in PROTECTED_GROUPS:
 raise errors.ProtectedEntryError(label=_(u'group'), key=keys[0],
 reason=_(u'privileged group'))
 if 'mepmanagedby' in group_attrs:
@@ -260,6 +260,15 @@ class group_mod(LDAPUpdate):
 
 def pre_callback(self, ldap, dn, entry_attrs, *keys, **options):
 assert isinstance(dn, DN)
+
+is_protected_group = False
+if keys[-1] in PROTECTED_GROUPS:
+is_protected_group = True
+
+if 'rename' in options:
+if is_protected_group:
+raise errors.ProtectedEntryError(label=u'group', key=keys[-1], reason=u'Cannot be renamed')
+
 if ('posix' in options and options['posix']) or 'gidnumber' in options:
 (dn, old_entry_attrs) = ldap.get_entry(dn, ['objectclass'])

Re: [Freeipa-devel] [PATCH] Changes to use a single database for dogtag and IPA

2012-10-02 Thread Petr Viktorin


On 10/01/2012 05:02 PM, Ade Lee wrote:

On Mon, 2012-10-01 at 16:09 +0200, Martin Kosek wrote:

On 10/01/2012 03:35 PM, Petr Viktorin wrote:

On 09/27/2012 10:26 AM, Petr Viktorin wrote:

On 09/20/2012 05:58 AM, Ade Lee wrote:

Changes to use a single database for dogtag and IPA

  New servers that are installed with dogtag 10 instances will use
  a single database instance for dogtag and IPA, albeit with different
  suffixes.  Dogtag will communicate with the instance through a
  database user with permissions to modify the dogtag  suffix only.
  This user will authenticate using client auth using the subsystem
cert
  for the instance.

  This patch includes changes to allow the creation of masters and
clones
  with single ds instances.

I have tested being able to create a master and a clone using f17 and
dogtag 10.  Note that you will need to use the latest builds on the
dogtag repo to get some changes that were checked in today.  We'll kick
off another official f18 dogtag build in a day or so.

This is a pretty big change - so I expect many issues to come up as
things get tested.  But as this will take awhile to get resolved, its
better to get this out for review as fast as possible.

Happy reviewing.

Ade




Attaching a rebased patch with a couple of style issues fixed.
- PEP8 compliance (remove trailing whitespace, use parentheses rather
than \ for line continuation, wrap touched lines at 80 characters)
- for files, use the with statement instead of the open/close sandwich
- don't mix tabs and spaces in install/share/certmap.conf.template

I've also adjusted the spec file, as we need dogtag 10.0 and pki-server
now obsoletes pki-setup.


I still need selinux in permissive mode to install on f17, and I still
need to exclude *.i686 packages when updating.



Are the following limitations expected?

IPA and Dogtag have to be updated simultaneously; it's not possible to have
current IPA master with Dogtag 10, or IPA with this patch with D9.

It is not possible to create a replica from a machine with a single DS to an
older version without the patch -- the older version will try the wrong ports.


In this case, I think we are covered - we do not support installation of a
replica with a lower version than the master where the replica info file was
created. Rob's patch 26dfbe61dd399e9c34f6f5bdeb25a197f1f461cb should ensure
this for next version release. For 3.0 I think we will have to settle with a
note in Documentation.



There is currently a dogtag bug where when the master is dogtag 9 (or
dogtag 9 converted to 10), and the clone is dogtag 10, the clone will
fail to get the installation token from the security domain.  This is
because the dogtag 10 code tries the new restful interface call -- which
is not present on a dogtag 9 subsystem.
https://fedorahosted.org/pki/ticket/334


This has been fixed in the latest dogtag 10 nightly builds.  And will be
in the next dogtag 10 official build, which we plan to create and
release today.

Incidentally, to see whats coming up in the new dogtag build, look for
the 10.0.0-0.X.a2 milestone (plus some of what is closed in 9.0.24)



Okay, testing with the dogtag-devel repo, on f17.

The following scenarios don't work:

- Start with a master on D9
- install a replica on D10, without a CA
- run ipa-ca-install on the replica
  ipa-replica-conncheck: error: no such option: --dogtag-master-ds-port


- Start with a master on D9
- install a replica without a CA (either Dogtag version)
- Update all machines
- run ipa-ca-install on the replica
  com.netscape.certsrv.base.PKIException: 
com.netscape.certsrv.base.PKIException: Failed to obtain installation 
token from security domain


I get the following errors in catalina.out on the replica:
08:40:11,149 DEBUG 
(org.jboss.resteasy.plugins.providers.DocumentProvider:60) - Unable to 
retrieve ServletContext: expandEntityReferences defaults to true
08:40:11,158 DEBUG 
(org.jboss.resteasy.plugins.providers.DocumentProvider:60) - Unable to 
retrieve ServletContext: expandEntityReferences defaults to true
CMS Warning: FAILURE: Cannot build CA chain. Error 
java.security.cert.CertificateException: Certificate is not a PKCS #11 
certificate|FAILURE: authz instance DirAclAuthz initialization failed 
and skipped, error=Property internaldb.ldapconn.port missing value|



--
Petr³

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

Re: [Freeipa-devel] [PATCH] 319 Make ipakrbprincipal objectclass optional

2012-10-02 Thread Martin Kosek

On 10/02/2012 12:19 PM, Petr Viktorin wrote:
 On 10/01/2012 05:28 PM, Martin Kosek wrote:
 From IPA 3.0, services have by default ipakrbprincipal objectclass which
 allows ipakrbprincipalalias attribute used for case-insensitive principal
 searches. However, as services created in previous version do not have
 this objectclass (and attribute), they are not listed in service list
 produced by service-find.

 Treat the ipakrbprincipal as optional to avoid missing services in
 service-find command. Add flag to service-mod command which can fill
 ipakrbprincipalalias attribute when case-insensitive principal searches
 for a 2.x service are required.

 https://fedorahosted.org/freeipa/ticket/3106
 
 This works, I'm getting all services now  the tests pass.
 

 -

 I am still pondering about a right way to fill ipakrbprincipalalias used in 
 for
 IPA 3.0 case-insensitive searches, so far I implemented this command:

 ipa service-mod PRINCIPAL --update-principal-alias

 But I am thinking it may be a better approach to generalize it and do 
 something
 like that:

 ipa service-mod PRINCIPAL --upgrade/--update

 This command would do a general update of service entry to an up-to-date 3.0
 style, in this case it could do 2 things:
 * fill ipakrbprincipalalias
 * fill ipakrbauthzdata (based on default value in IPA config).
 
 I don't think you're generalizing enough; `service-mod --upgrade` isn't that
 different from `service-mod --update-principal-alias --update-authzdata`.
 Scripting this to happen for all services could be a nuisance, though. There
 should be a way to upgrade all services at once, and since we already have
 ipa-ldap-updater for it, it should run as part of that.
 
 I think we should keep ipakrbprincipal optional, in case the upgrade goes 
 wrong.
 

I agree. I created an upgrade plugin which should update all services and fill
ipakrbprincipalalias during upgrade (attached). I tested 2.2 - 3.0 upgrade and
it worked fine.

Martin
From fc7715885ac643c37c8813b4d05473d1db91ea52 Mon Sep 17 00:00:00 2001
From: Martin Kosek mko...@redhat.com
Date: Mon, 1 Oct 2012 16:49:34 +0200
Subject: [PATCH] Fill ipakrbprincipalalias on upgrades

From IPA 3.0, services have by default ipakrbprincipal objectclass which
allows ipakrbprincipalalias attribute used for case-insensitive principal
searches. However, services created in previous version do not have
this objectclass (and attribute) and thus case-insensitive searches
may return inconsistent results.

Fill ipakrbprincipalalias on upgrades for all 2.x services. Also treat
Treat the ipakrbprincipal as optional to avoid missing services in
service-find command if the upgrade fails for any reason.

https://fedorahosted.org/freeipa/ticket/3106
---
 ipalib/plugins/service.py|  7 ++-
 ipaserver/install/plugins/Makefile.am|  1 +
 ipaserver/install/plugins/update_services.py | 94 
 3 files changed, 101 insertions(+), 1 deletion(-)
 create mode 100644 ipaserver/install/plugins/update_services.py

diff --git a/ipalib/plugins/service.py b/ipalib/plugins/service.py
index a7201f525941023fb5caa8610836156a6df79bab..551990d7cabc4cfa331a019edb721bdfc99a6b2d 100644
--- a/ipalib/plugins/service.py
+++ b/ipalib/plugins/service.py
@@ -218,8 +218,9 @@ class service(LDAPObject):
 object_name_plural = _('services')
 object_class = [
 'krbprincipal', 'krbprincipalaux', 'krbticketpolicyaux', 'ipaobject',
-'ipaservice', 'pkiuser', 'ipakrbprincipal'
+'ipaservice', 'pkiuser'
 ]
+possible_objectclasses = ['ipakrbprincipal']
 search_attributes = ['krbprincipalname', 'managedby', 'ipakrbauthzdata']
 default_attributes = ['krbprincipalname', 'usercertificate', 'managedby',
 'ipakrbauthzdata',]
@@ -311,6 +312,10 @@ class service_add(LDAPCreate):
 # schema
 entry_attrs['ipakrbprincipalalias'] = keys[-1]
 
+# Objectclass ipakrbprincipal providing ipakrbprincipalalias is not in
+# in a list of default objectclasses, add it manually
+entry_attrs['objectclass'].append('ipakrbprincipal')
+
 return dn
 
 api.register(service_add)
diff --git a/ipaserver/install/plugins/Makefile.am b/ipaserver/install/plugins/Makefile.am
index 9670273c8cd14c2ec98da9d228664b06289483a1..d29103a90afdffa74d768b3438acb9733b825d53 100644
--- a/ipaserver/install/plugins/Makefile.am
+++ b/ipaserver/install/plugins/Makefile.am
@@ -8,6 +8,7 @@ app_PYTHON = 			\
 	rename_managed.py	\
 	dns.py			\
 	updateclient.py		\
+	update_services.py	\
 	$(NULL)
 
 EXTRA_DIST =			\
diff --git a/ipaserver/install/plugins/update_services.py b/ipaserver/install/plugins/update_services.py
new file mode 100644
index ..6f9e8cb0ed9670db8796bece13d73539acb64190
--- /dev/null
+++ b/ipaserver/install/plugins/update_services.py
@@ -0,0 +1,94 @@
+# Authors:
+#   Martin Kosek mko...@redhat.com
+#
+# Copyright (C) 2012  Red Hat
+# see file 'COPYING' for use

[Freeipa-devel] [PATCH 0071] Fix potential crash caused by failing zone_register allocation.

2012-10-02 Thread Petr Spacek


Hello,

Fix potential crash caused by failing zone_register allocation.

Problematic call flow:
new_ldap_instance - zr_create (returns failure) - destroy_ldap_instance - 
zr_get_rbt (*crash*)


--
Petr^2 Spacek
From 9d96a9c4a4ac5b592ed5874132e0618b1b259de0 Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Tue, 2 Oct 2012 15:16:27 +0200
Subject: [PATCH] Fix potential crash caused by failing zone_register
 allocation.

Signed-off-by: Petr Spacek pspa...@redhat.com
---
 src/ldap_helper.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index 38d86afa521dcf0e6b58ebb38635ff0fffbedd2a..629c76732c86af2a614e589a5afff18136068a66 100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -516,7 +516,7 @@ destroy_ldap_instance(ldap_instance_t **ldap_instp)
 {
 	ldap_instance_t *ldap_inst;
 	dns_rbtnodechain_t chain;
-	dns_rbt_t *rbt;
+	dns_rbt_t *rbt = NULL;
 	isc_result_t result = ISC_R_SUCCESS;
 	const char *db_name;
 
@@ -530,7 +530,10 @@ destroy_ldap_instance(ldap_instance_t **ldap_instp)
 	 *
 	 * NOTE: This should be probably done in zone_register.c
 	 */
-	rbt = zr_get_rbt(ldap_inst-zone_register);
+	if (ldap_inst-zone_register != NULL)
+		rbt = zr_get_rbt(ldap_inst-zone_register);
+	if (rbt == NULL)
+		result = ISC_R_NOTFOUND;
 
 	/* Potentially ISC_R_NOSPACE can occur. Destroy codepath has no way to
 	 * return errors, so kill BIND.
-- 
1.7.11.4

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

[Freeipa-devel] [PATCH 0017] Improve error message in ipa-replica-manage

2012-10-02 Thread Tomas Babej


Hi,

When executing ipa-replica-manage connect to an unknown or irrelevant
master, we now print a sensible error message informing the user
about this possiblity as well.

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

Tomas
From dac062488a4f7989a87358433a83ee1195e21237 Mon Sep 17 00:00:00 2001
From: Tomas Babej tba...@redhat.com
Date: Tue, 2 Oct 2012 09:15:33 -0400
Subject: [PATCH] Improve error message in ipa-replica-manage

When executing ipa-replica-manage connect to an unknown or irrelevant
master, we now print a sensible error message informing the user
about this possiblity as well.

https://fedorahosted.org/freeipa/ticket/3105
---
 install/tools/ipa-replica-manage | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/install/tools/ipa-replica-manage b/install/tools/ipa-replica-manage
index 897d117681d3e1559d5710366101b50540b705c8..fd03b0d767e6806c036216aad3e4f62198613ca5 100755
--- a/install/tools/ipa-replica-manage
+++ b/install/tools/ipa-replica-manage
@@ -709,7 +709,7 @@ def add_link(realm, replica1, replica2, dirman_passwd, options):
 repl2.conn.getEntry(master2_dn, ldap.SCOPE_BASE)
 
 except errors.NotFound:
-sys.exit(You cannot connect to a previously deleted master)
+sys.exit(Connection unsuccessful. You might be trying to connect to unknown, foreign or previously deleted master.)
 repl1.setup_gssapi_replication(replica2, DN(('cn', 'Directory Manager')), dirman_passwd)
 print Connected '%s' to '%s' % (replica1, replica2)
 
-- 
1.7.11.4

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

Re: [Freeipa-devel] [PATCH 0017] Improve error message in ipa-replica-manage

2012-10-02 Thread Rob Crittenden


Tomas Babej wrote:

Hi,

When executing ipa-replica-manage connect to an unknown or irrelevant
master, we now print a sensible error message informing the user
about this possiblity as well.

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

Tomas


I put a whole bunch of code into a try/except and this may be catching 
errors in unexpected ways.


I'm not entirely sure right now what we should do, but looking at the 
code in the try:


repl1.conn.getEntry(master1_dn, ldap.SCOPE_BASE)
repl1.conn.getEntry(master2_dn, ldap.SCOPE_BASE)

We take in replica1 and replica1 as arguments (the default for replica1 
is the current host).


If either of these raise a NotFound it means there there is no master of 
that name. Does that mean that the master was deleted? Well, clearly not.


A lot has changed since I did this, I may have been relying on a 
side-effect, or just hadn't tested well-enough.


I wonder if we need that message at all. Is foo is not an IPA server 
good enough? It still might be confusing if someone didn't know that 
foo was deleted and it was still running. We could probably verify 
that it is at least an IPA server by doing similar checking in the 
client, it all depends on how far we want to take it.


rob

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

[Freeipa-devel] [PATCH] 320 Only use service PAC type as an override

2012-10-02 Thread Martin Kosek

PAC type (ipakrbauthzdata attribute) was being filled for all new
service automatically. However, the PAC type attribute was designed
to serve only as an override to default PAC type configured in
IPA config. With PAC type set in all services, users would have
to update all services to get new PAC types configured in IPA config.

Do not set PAC type for new services. Add new NONE value meaning that
we do not want any PAC for the service (empty/missing attribute means
that the default PAC type list from IPA config is read).

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

---

Note: the new NONE value of service PAC type was planned in a scope of ticket
#2960.
From 957e814b2c43637d3f493e8b902b8e494df5b04b Mon Sep 17 00:00:00 2001
From: Martin Kosek mko...@redhat.com
Date: Tue, 2 Oct 2012 17:06:10 +0200
Subject: [PATCH] Only use service PAC type as an override

PAC type (ipakrbauthzdata attribute) was being filled for all new
service automatically. However, the PAC type attribute was designed
to serve only as an override to default PAC type configured in
IPA config. With PAC type set in all services, users would have
to update all services to get new PAC types configured in IPA config.

Do not set PAC type for new services. Add new NONE value meaning that
we do not want any PAC for the service (empty/missing attribute means
that the default PAC type list from IPA config is read).

https://fedorahosted.org/freeipa/ticket/2184
---
 API.txt  |  6 ++--
 VERSION  |  2 +-
 ipalib/plugins/service.py| 27 ++
 tests/test_xmlrpc/test_host_plugin.py|  1 -
 tests/test_xmlrpc/test_service_plugin.py | 48 
 5 files changed, 61 insertions(+), 23 deletions(-)

diff --git a/API.txt b/API.txt
index 1906e22fe92f76f1a628d37fcdb23d73a1b1297f..7bd046c8d504bb7e39059a4f2b6743c7c0b6d8ef 100644
--- a/API.txt
+++ b/API.txt
@@ -2738,7 +2738,7 @@ command: service_add
 args: 1,8,3
 arg: Str('krbprincipalname', attribute=True, cli_name='principal', multivalue=False, primary_key=True, required=True)
 option: Bytes('usercertificate', attribute=True, cli_name='certificate', multivalue=False, required=False)
-option: StrEnum('ipakrbauthzdata', attribute=True, cli_name='pac_type', csv=True, multivalue=True, required=False, values=(u'MS-PAC', u'PAD'))
+option: StrEnum('ipakrbauthzdata', attribute=True, cli_name='pac_type', csv=True, multivalue=True, required=False, values=(u'MS-PAC', u'PAD', u'NONE'))
 option: Str('setattr*', cli_name='setattr', exclude='webui')
 option: Str('addattr*', cli_name='addattr', exclude='webui')
 option: Flag('force', autofill=True, default=False)
@@ -2775,7 +2775,7 @@ command: service_find
 args: 1,10,4
 arg: Str('criteria?', noextrawhitespace=False)
 option: Str('krbprincipalname', attribute=True, autofill=False, cli_name='principal', multivalue=False, primary_key=True, query=True, required=False)
-option: StrEnum('ipakrbauthzdata', attribute=True, autofill=False, cli_name='pac_type', csv=True, multivalue=True, query=True, required=False, values=(u'MS-PAC', u'PAD'))
+option: StrEnum('ipakrbauthzdata', attribute=True, autofill=False, cli_name='pac_type', csv=True, multivalue=True, query=True, required=False, values=(u'MS-PAC', u'PAD', u'NONE'))
 option: Int('timelimit?', autofill=False, minvalue=0)
 option: Int('sizelimit?', autofill=False, minvalue=0)
 option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
@@ -2792,7 +2792,7 @@ command: service_mod
 args: 1,9,3
 arg: Str('krbprincipalname', attribute=True, cli_name='principal', multivalue=False, primary_key=True, query=True, required=True)
 option: Bytes('usercertificate', attribute=True, autofill=False, cli_name='certificate', multivalue=False, required=False)
-option: StrEnum('ipakrbauthzdata', attribute=True, autofill=False, cli_name='pac_type', csv=True, multivalue=True, required=False, values=(u'MS-PAC', u'PAD'))
+option: StrEnum('ipakrbauthzdata', attribute=True, autofill=False, cli_name='pac_type', csv=True, multivalue=True, required=False, values=(u'MS-PAC', u'PAD', u'NONE'))
 option: Str('setattr*', cli_name='setattr', exclude='webui')
 option: Str('addattr*', cli_name='addattr', exclude='webui')
 option: Str('delattr*', cli_name='delattr', exclude='webui')
diff --git a/VERSION b/VERSION
index 962d476e7e152c0c189361ea38de0a5642798971..c1f1bceffe53b3fcfa6526448f6aebca475073b2 100644
--- a/VERSION
+++ b/VERSION
@@ -79,4 +79,4 @@ IPA_DATA_VERSION=2010061412
 #  #
 
 IPA_API_VERSION_MAJOR=2
-IPA_API_VERSION_MINOR=43
+IPA_API_VERSION_MINOR=44
diff --git a/ipalib/plugins/service.py b/ipalib/plugins/service.py
index a7201f525941023fb5caa8610836156a6df79bab..cc044bb7e2cb716e880d4773b6116fd281fd394c 100644
--- a/ipalib/plugins/service.py
+++ b/ipalib/plugins/service.py
@@ -254,11 +254,26 @@ class

Re: [Freeipa-devel] [PATCH] Changes to use a single database for dogtag and IPA

2012-10-02 Thread Petr Viktorin


On 10/02/2012 03:02 PM, Petr Viktorin wrote:

On 10/01/2012 05:02 PM, Ade Lee wrote:

On Mon, 2012-10-01 at 16:09 +0200, Martin Kosek wrote:

On 10/01/2012 03:35 PM, Petr Viktorin wrote:

On 09/27/2012 10:26 AM, Petr Viktorin wrote:

On 09/20/2012 05:58 AM, Ade Lee wrote:

Changes to use a single database for dogtag and IPA

  New servers that are installed with dogtag 10 instances will
use
  a single database instance for dogtag and IPA, albeit with
different
  suffixes.  Dogtag will communicate with the instance through a
  database user with permissions to modify the dogtag  suffix
only.
  This user will authenticate using client auth using the
subsystem
cert
  for the instance.

  This patch includes changes to allow the creation of masters
and
clones
  with single ds instances.

I have tested being able to create a master and a clone using f17 and
dogtag 10.  Note that you will need to use the latest builds on the
dogtag repo to get some changes that were checked in today.  We'll
kick
off another official f18 dogtag build in a day or so.

This is a pretty big change - so I expect many issues to come up as
things get tested.  But as this will take awhile to get resolved, its
better to get this out for review as fast as possible.

Happy reviewing.

Ade




Attaching a rebased patch with a couple of style issues fixed.
- PEP8 compliance (remove trailing whitespace, use parentheses rather
than \ for line continuation, wrap touched lines at 80 characters)
- for files, use the with statement instead of the open/close
sandwich
- don't mix tabs and spaces in install/share/certmap.conf.template

I've also adjusted the spec file, as we need dogtag 10.0 and
pki-server
now obsoletes pki-setup.


I still need selinux in permissive mode to install on f17, and I still
need to exclude *.i686 packages when updating.



Are the following limitations expected?

IPA and Dogtag have to be updated simultaneously; it's not possible
to have
current IPA master with Dogtag 10, or IPA with this patch with D9.

It is not possible to create a replica from a machine with a single
DS to an
older version without the patch -- the older version will try the
wrong ports.


In this case, I think we are covered - we do not support installation
of a
replica with a lower version than the master where the replica info
file was
created. Rob's patch 26dfbe61dd399e9c34f6f5bdeb25a197f1f461cb should
ensure
this for next version release. For 3.0 I think we will have to settle
with a
note in Documentation.



There is currently a dogtag bug where when the master is dogtag 9 (or
dogtag 9 converted to 10), and the clone is dogtag 10, the clone will
fail to get the installation token from the security domain.  This is
because the dogtag 10 code tries the new restful interface call -- which
is not present on a dogtag 9 subsystem.
https://fedorahosted.org/pki/ticket/334


This has been fixed in the latest dogtag 10 nightly builds.  And will be
in the next dogtag 10 official build, which we plan to create and
release today.

Incidentally, to see whats coming up in the new dogtag build, look for
the 10.0.0-0.X.a2 milestone (plus some of what is closed in 9.0.24)



Okay, testing with the dogtag-devel repo, on f17.

The following scenarios don't work:

- Start with a master on D9
- install a replica on D10, without a CA
- run ipa-ca-install on the replica
   ipa-replica-conncheck: error: no such option: --dogtag-master-ds-port


- Start with a master on D9
- install a replica without a CA (either Dogtag version)
- Update all machines
- run ipa-ca-install on the replica
   com.netscape.certsrv.base.PKIException:
com.netscape.certsrv.base.PKIException: Failed to obtain installation
token from security domain

I get the following errors in catalina.out on the replica:
08:40:11,149 DEBUG
(org.jboss.resteasy.plugins.providers.DocumentProvider:60) - Unable to
retrieve ServletContext: expandEntityReferences defaults to true
08:40:11,158 DEBUG
(org.jboss.resteasy.plugins.providers.DocumentProvider:60) - Unable to
retrieve ServletContext: expandEntityReferences defaults to true
CMS Warning: FAILURE: Cannot build CA chain. Error
java.security.cert.CertificateException: Certificate is not a PKCS #11
certificate|FAILURE: authz instance DirAclAuthz initialization failed
and skipped, error=Property internaldb.ldapconn.port missing value|




I did the second scenario again and got a slightly different error 
message: White spaces are required between publicId and systemId. See 
attached logs.
I started with IPA 2.2 (from f17 repos) on both machines, then updated 
to patched IPA w/ D10, then ran ipa-ca-install.


--
Petr³


replica-logs.tar.gz
Description: GNU Zip compressed data
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH] 319 Make ipakrbprincipal objectclass optional

2012-10-02 Thread Martin Kosek

On 10/02/2012 03:04 PM, Martin Kosek wrote:
 On 10/02/2012 12:19 PM, Petr Viktorin wrote:
 On 10/01/2012 05:28 PM, Martin Kosek wrote:
 From IPA 3.0, services have by default ipakrbprincipal objectclass which
 allows ipakrbprincipalalias attribute used for case-insensitive principal
 searches. However, as services created in previous version do not have
 this objectclass (and attribute), they are not listed in service list
 produced by service-find.

 Treat the ipakrbprincipal as optional to avoid missing services in
 service-find command. Add flag to service-mod command which can fill
 ipakrbprincipalalias attribute when case-insensitive principal searches
 for a 2.x service are required.

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

 This works, I'm getting all services now  the tests pass.


 -

 I am still pondering about a right way to fill ipakrbprincipalalias used in 
 for
 IPA 3.0 case-insensitive searches, so far I implemented this command:

 ipa service-mod PRINCIPAL --update-principal-alias

 But I am thinking it may be a better approach to generalize it and do 
 something
 like that:

 ipa service-mod PRINCIPAL --upgrade/--update

 This command would do a general update of service entry to an up-to-date 3.0
 style, in this case it could do 2 things:
 * fill ipakrbprincipalalias
 * fill ipakrbauthzdata (based on default value in IPA config).

 I don't think you're generalizing enough; `service-mod --upgrade` isn't that
 different from `service-mod --update-principal-alias --update-authzdata`.
 Scripting this to happen for all services could be a nuisance, though. There
 should be a way to upgrade all services at once, and since we already have
 ipa-ldap-updater for it, it should run as part of that.

 I think we should keep ipakrbprincipal optional, in case the upgrade goes 
 wrong.

 
 I agree. I created an upgrade plugin which should update all services and fill
 ipakrbprincipalalias during upgrade (attached). I tested 2.2 - 3.0 upgrade 
 and
 it worked fine.
 
 Martin
 

There was a glitch in the loop repeating the update when LDAP limits are hit -
thanks Petr Viktorin for noticing the issue. It is working now, I tried with 10
affected services and search limit set to 1 entry - and the loop executed 10
times as it was supposed to.

I also disabled size/time limits for the search in the upgrade plugin. But it
would also work if default IPA search limits (100 entries) are used, it should
just make things faster.

Martin
From 1a4bd467bda9eb668aead2514b431c6b949c4a5d Mon Sep 17 00:00:00 2001
From: Martin Kosek mko...@redhat.com
Date: Mon, 1 Oct 2012 16:49:34 +0200
Subject: [PATCH] Fill ipakrbprincipalalias on upgrades

From IPA 3.0, services have by default ipakrbprincipal objectclass which
allows ipakrbprincipalalias attribute used for case-insensitive principal
searches. However, services created in previous version do not have
this objectclass (and attribute) and thus case-insensitive searches
may return inconsistent results.

Fill ipakrbprincipalalias on upgrades for all 2.x services. Also treat
Treat the ipakrbprincipal as optional to avoid missing services in
service-find command if the upgrade fails for any reason.

https://fedorahosted.org/freeipa/ticket/3106
---
 ipalib/plugins/service.py|  7 +-
 ipaserver/install/plugins/Makefile.am|  1 +
 ipaserver/install/plugins/update_services.py | 95 
 3 files changed, 102 insertions(+), 1 deletion(-)
 create mode 100644 ipaserver/install/plugins/update_services.py

diff --git a/ipalib/plugins/service.py b/ipalib/plugins/service.py
index a7201f525941023fb5caa8610836156a6df79bab..551990d7cabc4cfa331a019edb721bdfc99a6b2d 100644
--- a/ipalib/plugins/service.py
+++ b/ipalib/plugins/service.py
@@ -218,8 +218,9 @@ class service(LDAPObject):
 object_name_plural = _('services')
 object_class = [
 'krbprincipal', 'krbprincipalaux', 'krbticketpolicyaux', 'ipaobject',
-'ipaservice', 'pkiuser', 'ipakrbprincipal'
+'ipaservice', 'pkiuser'
 ]
+possible_objectclasses = ['ipakrbprincipal']
 search_attributes = ['krbprincipalname', 'managedby', 'ipakrbauthzdata']
 default_attributes = ['krbprincipalname', 'usercertificate', 'managedby',
 'ipakrbauthzdata',]
@@ -311,6 +312,10 @@ class service_add(LDAPCreate):
 # schema
 entry_attrs['ipakrbprincipalalias'] = keys[-1]
 
+# Objectclass ipakrbprincipal providing ipakrbprincipalalias is not in
+# in a list of default objectclasses, add it manually
+entry_attrs['objectclass'].append('ipakrbprincipal')
+
 return dn
 
 api.register(service_add)
diff --git a/ipaserver/install/plugins/Makefile.am b/ipaserver/install/plugins/Makefile.am
index 9670273c8cd14c2ec98da9d228664b06289483a1..d29103a90afdffa74d768b3438acb9733b825d53 100644
--- a/ipaserver/install/plugins/Makefile.am
+++ b/ipaserver/install/plugins/Makefile.am
@@ -8,6 +8,7 @@ app_PYTHON = 			\

[Freeipa-devel] [PATCH] 1058 clear session key

2012-10-02 Thread Rob Crittenden


Clear the host session key when enrolling a client.

Make sure dbdir is preserved when a new connection is created.

rob
From b9d21ae9082e84853d316a49729aac21d848501f Mon Sep 17 00:00:00 2001
From: Rob Crittenden rcrit...@redhat.com
Date: Mon, 1 Oct 2012 13:05:11 -0400
Subject: [PATCH] Clear kernel keyring in client installer, save dbdir on new
 connections

This patch addresses two issues:

1. If a client is previously enrolled in an IPA server and the server
   gets re-installed then the client machine may still have a keyring
   entry for the old server. This can cause a redirect from the
   session URI to the negotiate one. As a rule, always clear the keyring
   when enrolling a new client.

2. We save the NSS dbdir in the connection so that when creating a new
   session we can determine if we need to re-initialize NSS or not. Most
   of the time we do not. The dbdir was not always being preserved between
   connections which could cause an NSS_Shutdown() to happen which would
   fail because of existing usage. This preserves the dbdir information when
   a new connection is created as part of the session mechanism.

https://fedorahosted.org/freeipa/ticket/3108
---
 ipa-client/ipa-install/ipa-client-install | 11 ++-
 ipalib/rpc.py | 15 +++
 2 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/ipa-client/ipa-install/ipa-client-install b/ipa-client/ipa-install/ipa-client-install
index a9408eed7cca44700e6b444987a0d93d51b2251e..146450963aebcab491ea2367256d8fa2d7213850 100755
--- a/ipa-client/ipa-install/ipa-client-install
+++ b/ipa-client/ipa-install/ipa-client-install
@@ -42,6 +42,8 @@ try:
 from ipalib import api, errors
 from ipapython.dn import DN
 from ipapython.ssh import SSHPublicKey
+from ipapython import kernel_keyring
+from ipalib.rpc import COOKIE_NAME
 import SSSDConfig
 from ConfigParser import RawConfigParser
 from optparse import SUPPRESS_HELP, OptionGroup
@@ -1583,13 +1585,14 @@ def install(options, env, fstore, statestore):
 root_logger.info(Failed to add CA to the default NSS database.)
 return CLIENT_INSTALL_ERROR
 
+host_principal = 'host/%s@%s' % (hostname, cli_realm)
 if options.on_master:
 # If on master assume kerberos is already configured properly.
 # Get the host TGT.
 os.environ['KRB5CCNAME'] = CCACHE_FILE
 try:
 run(['/usr/bin/kinit', '-k', '-t', '/etc/krb5.keytab',
-'host/%s@%s' % (hostname, cli_realm)])
+host_principal])
 except CalledProcessError, e:
 root_logger.error(Failed to obtain host TGT.)
 return CLIENT_INSTALL_ERROR
@@ -1610,6 +1613,12 @@ def install(options, env, fstore, statestore):
 root_logger.info(
 Configured /etc/krb5.conf for IPA realm %s, cli_realm)
 
+# Clear out any current session keyring information
+try:
+kernel_keyring.del_key(COOKIE_NAME % host_principal)
+except ValueError:
+pass
+
 # Now, let's try to connect to the server's XML-RPC interface
 try:
 api.Backend.xmlclient.connect()
diff --git a/ipalib/rpc.py b/ipalib/rpc.py
index fc135f4f6a1bc4e98e3c4c3bbf6857b98fdc94db..3c37b376d43158647a9cb33d55c343af7fb43f10 100644
--- a/ipalib/rpc.py
+++ b/ipalib/rpc.py
@@ -546,8 +546,23 @@ class xmlclient(Connectible):
 # This shouldn't happen if we have a session but
 # it isn't fatal.
 pass
+
+# Create a new serverproxy with the non-session URI. If there
+# is an existing connection we need to save the NSS dbdir so
+# we can skip an unnecessary NSS_Initialize() and avoid
+# NSS_Shutdown issues.
 serverproxy = self.create_connection(os.environ.get('KRB5CCNAME'), self.env.verbose, self.env.fallback, self.env.delegate)
+
+dbdir = None
+current_conn = getattr(context, self.id, None)
+if (current_conn is not None and
+  hasattr(current_conn.conn._ServerProxy__transport, 'dbdir')):
+dbdir = current_conn.conn._ServerProxy__transport.dbdir
+self.debug('Using dbdir %s' % dbdir)
 setattr(context, self.id, Connection(serverproxy, self.disconnect))
+if dbdir is not None:
+current_conn = getattr(context, self.id, None)
+current_conn.conn._ServerProxy__transport.dbdir = dbdir
 return self.forward(name, *args, **kw)
 raise NetworkError(uri=server, error=e.errmsg)
 except socket.error, e:
-- 
1.7.11.4

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

Re: [Freeipa-devel] [PATCH] 1037 optimize restoring SELinux booleans

2012-10-02 Thread Rob Crittenden


Petr Viktorin wrote:

On 10/01/2012 09:29 PM, Rob Crittenden wrote:

Petr Viktorin wrote:

On 10/01/2012 04:41 PM, Rob Crittenden wrote:

The web uninstall step can be very long because we restore two SELinux
booleans individually. This patch combines them into a single step, and
skips setting them if the values won't actually change.

rob




Is there a reason to not reuse the code that sets the values on install?
As far as I can tell it does the same thing slightly differently.



The differences are enough that trying to consolidate them would likely
end up taking considerable more time, require considerable more testing,
etc. It would be worthwhile to revisit this at the beginning of a new
version, but at the end it seems safer to take the simplest route.

rob


Well, okay then, ACK. But please keep the bug open.



I'm going to withdraw this patch for now. I think it can wait for a more 
complete fix in a future release.


rob

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

Re: [Freeipa-devel] [PATCH 0016] Adds port to connection error message in ipa-client-install

2012-10-02 Thread Rob Crittenden


Tomas Babej wrote:

On 09/26/2012 09:32 PM, Rob Crittenden wrote:

Tomas Babej wrote:

Hi,

Connection error message in ipa-client-install now warns the user
about the need of opening 389 port for directory server.

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

I think this can be pushed as a one-liner.


I think we should list all ports that are required for client enrollment.

From my calculations we need at a minimum tcp ports 80 and 389, either
or both udp/tcp for port 88 and if NTP is enabled 123 udp for
enrollment alone. The NTP failure won't cause enrollment to fail
though, so we may be able to skip that.

Similarly 464 should be enabled but we don't use it during enrollment.

rob

I improved the error message. Please check if there are any issues.

Thanks

Tomas


This only works if port 389 is blocked, not 88 or 80.

rob

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

[Freeipa-devel] [PATCH] 75-78 Add fallback group

2012-10-02 Thread Sumit Bose

Hi,

this patch should fix https://fedorahosted.org/freeipa/ticket/2955 by
adding a fallback group as described in comment 2 of the ticket in
ipa-adtrust-install.

If you prefer to use a different kind of group I can change the patch
accordingly.

bye,
Sumit
From 9cb3514cd7c73810ce4b5dceb82d36b739124854 Mon Sep 17 00:00:00 2001
From: Sumit Bose sb...@redhat.com
Date: Tue, 18 Sep 2012 11:32:10 +0200
Subject: [PATCH 75/78] ipa-adtrust-install: Add fallback group

---
 ipaserver/install/adtrustinstance.py | 79 ++--
 1 Datei geändert, 67 Zeilen hinzugefügt(+), 12 Zeilen entfernt(-)

diff --git a/ipaserver/install/adtrustinstance.py 
b/ipaserver/install/adtrustinstance.py
index 
c44037754dfd74cf6a9ceda4c9f4d2a6a224b1ea..1aebdad3ce8169952489d2ddce8f19917635b5e8
 100644
--- a/ipaserver/install/adtrustinstance.py
+++ b/ipaserver/install/adtrustinstance.py
@@ -96,9 +96,11 @@ class ADTRUSTInstance(service.Service):
 ATTR_SID = ipaNTSecurityIdentifier
 ATTR_FLAT_NAME = ipaNTFlatName
 ATTR_GUID = ipaNTDomainGUID
+ATTR_FALLBACK_GROUP = ipaNTFallbackPrimaryGroup
 OBJC_USER = ipaNTUserAttrs
 OBJC_GROUP = ipaNTGroupAttrs
 OBJC_DOMAIN = ipaNTDomainAttrs
+FALLBACK_GROUP_NAME = u'fallback_primary_group'
 
 def __init__(self, fstore=None):
 self.fqdn = None
@@ -134,6 +136,18 @@ class ADTRUSTInstance(service.Service):
 return S-1-5-21-%d-%d-%d % (sub_ids[0], sub_ids[1], sub_ids[2])
 
 def __add_admin_sids(self):
+
+The IPA admin and the IPA admins group with get the well knows SIDs
+used by AD for the administrator and the administrator group.
+Additonally the well know domain users SID will be assinged to a
+special fallback group.
+
+By default new users belong only to a user private group (UPG) and no
+other Posix group since ipausers is not a Posix group anymore. To be
+able to add a RID to the primary RID attribute in a PAC a fallback
+group is added.
+
+
 admin_dn = DN(('uid', 'admin'), api.env.container_user,
   self.suffix)
 admin_group_dn = DN(('cn', 'admins'), api.env.container_group,
@@ -163,24 +177,65 @@ class ADTRUSTInstance(service.Service):
 print IPA admin group object not found
 return
 
-if admin_entry.getValue(self.ATTR_SID) or \
-   admin_group_entry.getValue(self.ATTR_SID):
-print Admin SID already set, nothing to do
+if admin_entry.getValue(self.ATTR_SID):
+self.print_msg(Admin SID already set, nothing to do)
+else:
+try:
+self.admin_conn.modify_s(admin_dn, \
+[(ldap.MOD_ADD, objectclass, self.OBJC_USER), \
+ (ldap.MOD_ADD, self.ATTR_SID, dom_sid + -500)])
+except:
+self.print_msg(Failed to modify IPA admin object)
+
+if admin_group_entry.getValue(self.ATTR_SID):
+self.print_msg(Admin group SID already set, nothing to do)
+else:
+try:
+self.admin_conn.modify_s(admin_group_dn, \
+[(ldap.MOD_ADD, objectclass, self.OBJC_GROUP), \
+ (ldap.MOD_ADD, self.ATTR_SID, dom_sid + -512)])
+except:
+self.print_msg(Failed to modify IPA admin group object)
+
+if dom_entry.getValue(self.ATTR_FALLBACK_GROUP):
+self.print_msg(Fallback group already set, nothing to do)
 return
 
+fb_group_dn = DN(('cn', self.FALLBACK_GROUP_NAME),
+ api.env.container_group, self.suffix)
+has_sid = False
 try:
-self.admin_conn.modify_s(admin_dn, \
-[(ldap.MOD_ADD, objectclass, self.OBJC_USER), \
- (ldap.MOD_ADD, self.ATTR_SID, dom_sid + -500)])
-except:
-print Failed to modify IPA admin object
+fb_group = self.admin_conn.getEntry(fb_group_dn, ldap.SCOPE_BASE)
+
+if fb_group.getValue(self.ATTR_SID):
+has_sid = True
+except errors.NotFound:
+try:
+fallback = api.Command['group_add'](self.FALLBACK_GROUP_NAME,
+   description= u'Fallback group for ' 
\
+ 'primary group RID, ' 
\
+ 'do not add user to ' 
\
+ 'this group',
+   nonposix=True)
+fb_group_dn = fallback['result']['dn']
+except Exception, e:
+self.print_msg(Failed to add fallback group.)
+raise e
+
+if not has_sid:
+try:
+mod = [(ldap.MOD_ADD, objectclass, self.OBJC_GROUP), \
+

Re: [Freeipa-devel] [PATCH] 314-315 Limit unindexed searches

2012-10-02 Thread Rob Crittenden


Martin Kosek wrote:

On 09/26/2012 08:58 PM, Rob Crittenden wrote:

Martin Kosek wrote:

These 2 patches significantly limit the number of unindexed LDAP searches we do
in IPA. I used our unit test suite as a good source of different LDAP searches
run by our command suite.

Most of the remaining unindexed searches are produced either by our general
term search (ipa service-find TERM) which hit every object parameter and DNS
commands (idnsname is not indexed yet). I am thinking about indexing about
idnsName as well...



Patch 314 looks ok. It addresses ticket 3025 as well (ipakrbprincipalalias).



I added this ticket number to the patch too. Btw. managedBy attribute mentioned
in the ticket was already added to indexed in ticket #2866.


For 315 I'd prefer we add a new exception type to mirror
ldap.NOT_ALLOWED_ON_NONLEAF and catch that in delete instead of DatabaseError.



This is a very good catch, it will make this procedure safer when there is an
actual DatabaseError raised.

Updated patches attached.

Martin



ACK, pushed to master and ipa-3-0

rob

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

Re: [Freeipa-devel] [PATCH] 75-78 Add fallback group

2012-10-02 Thread Simo Sorce

On Tue, 2012-10-02 at 21:29 +0200, Sumit Bose wrote:
 Hi,
 
 this patch should fix https://fedorahosted.org/freeipa/ticket/2955 by
 adding a fallback group as described in comment 2 of the ticket in
 ipa-adtrust-install.
 
 If you prefer to use a different kind of group I can change the patch
 accordingly.

Yes I think we should use a more natural group name. In my recent
testing I have been using the name 'Trust Users' that pairs well with
another group we create called 'Trust Admins'. But I am open to
suggestions on a better name, 'Domain Users' may be better if we really
want to associate the wellknown SID to this group.

On the SID side I wonder if using the wellknown 'Domain Users' SID is
the right thing to do. I do not see any special reasons why it shouldn't
but I also do not have any special reason why we should.
Anyone can think of any pros/cons of doing that ?

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York

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

Re: [Freeipa-devel] [PATCH] 320 Only use service PAC type as an override

2012-10-02 Thread Rob Crittenden


Martin Kosek wrote:

PAC type (ipakrbauthzdata attribute) was being filled for all new
service automatically. However, the PAC type attribute was designed
to serve only as an override to default PAC type configured in
IPA config. With PAC type set in all services, users would have
to update all services to get new PAC types configured in IPA config.

Do not set PAC type for new services. Add new NONE value meaning that
we do not want any PAC for the service (empty/missing attribute means
that the default PAC type list from IPA config is read).

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

---

Note: the new NONE value of service PAC type was planned in a scope of ticket
#2960.


ACK, but before you push can you add the jist of this commit message to 
the help for PAC type in the service command help so users will 
understand the difference between NONE and blank?


rob

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

[Freeipa-devel] [Fwd: [Pki-announce] Announcing Dogtag 10.0.0 alpha 2 release]

2012-10-02 Thread Ade Lee


---BeginMessage---
The Dogtag team is proud to announce version Dogtag v10.0.0 alpha 2.

A build is available for Fedora 18 in the updates-testing repo.  Please
try it out and provide karma to move it to the F18 stable repo.

Daily developer builds for Fedora 17 and 18 are available at
http://nkinder.fedorapeople.org/dogtag-devel/fedora/

== Build Versions ==
pki-core-10.0.0-0.37.a2.fc18
pki-ra-10.0.0-0.8.a2.fc18
pki-tps-10.0.0-0.8.a2.fc18
dogtag-pki-10.0.0-0.9.a2.fc18
dogtag-pki-theme-10.0.0-0.2.a2.fc18
pki-console-10.0.0-0.8.a2.fc18

== Highlights since Dogtag v. 10.0.0 alpha 1 (Sept 14 2012) ==

* Dogtag 10 can now clone from Dogtag 9 masters.  We will fall back to
the old installation servlet if needed. (**)
* Startup state of a server can be determined from the getStatus()
servlet (**)
* Consistent database user provided during installation for client
authentication (used in IPA merged database install)
* Fixed package dependency issue with pki-symkey (**)
* pki-setup merged into pki-server (**)
* Audit Cert Renewal time extended to two years (**)
* Versioning added to jar manifest files and VERSION file and reported
in getStatus
* ECC enhancements in DRM and TMS environment
* Addition of time based searches in preparation for randomized serial
numbers
* Enhanced escaping of LDAP attributes
* Improved transition control for token operations in the TPS.

** denotes IPA related/reported issue

== Feedback ==

Please provide comments, bugs and other feedback via the pki-devel 
mailing list: http://www.redhat.com/mailman/listinfo/pki-devel

== Detailed Changelog ==
Ade Lee (4):
761a047 Updated release to a2
854ecce fall back to old interface for installtoken if needed
11e05d3 Use getStatus servlet to provide startup status
e1666df Changes to use standard dbuser

Andrew Wnuk (1):
f944641 time based searches

Christina Fu (5):
7b3df7e https://fedorahosted.org/pki/ticket/252 - TMS - ECC Key Recovery
528fda5 TMS key recovery part of - Bug 737122 - DRM: during archiving
and recovering, wrapping unwrapping keys should be done in the token
f4ecf48 (fixed warning for) task #304 TMS ECC infrastructure (enrollment
with client-side and server-side key generation, and key archival)
e689561 https://fedorahosted.org/pki/ticket/304 TMS ECC infrastructure
(enrollment with client-side and server-side key generation, and key
archival)
6257d32 https://fedorahosted.org/pki/ticket/304 TMS ECC infrastructure
(enrollment with client-side and server-side key generation, and key
archival)

Endi Dewata (11):
f81718c Using RPM version number in CMake.
aa1c7e7 Added package checking for pkispawn.
87d290d Added version number into server status.
9368ef4 Added VERSION file.
1726794 Renamed escapeDN() into escapeRDNValue().
4ba74f7 Merged pki-setup into pki-server.
156ba56 Added DN and filter escaping in ConfigurationUtils.
947ab8a Removed duplicate DN escaping methods.
715d89d Added DN and filter escaping in UGSubsystem.
7b737b2 Fixed conflicting log4j.properties.
8ed86a7 Fixed problems with optional pki-symkey.
 
Jack Magne (1):
9173b43 Provide default for operations transition list, # 858816.

Matthew Harmsen (2):
d450525 Correctly resolve symlinks in subdirectories
f5b8ea5 Audit Cert Renewal 

___
Pki-announce mailing list
pki-annou...@redhat.com
https://www.redhat.com/mailman/listinfo/pki-announce
---End Message---
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH] 0081 Support both unified samba and samba/samba4-packages

[Freeipa-devel] [PATCHES] 3 enhancements for the ipa-adtrust-install page

Re: [Freeipa-devel] [PATCH] 1037 optimize restoring SELinux booleans

Re: [Freeipa-devel] [PATCH] 316 Improve DN usage in ipa-client-install

[Freeipa-devel] [PATCH] Fix various issues found by Coverity

Re: [Freeipa-devel] [PATCH] 319 Make ipakrbprincipal objectclass optional

Re: [Freeipa-devel] [PATCH] 316 Improve DN usage in ipa-client-install

Re: [Freeipa-devel] [PATCH] 316 Improve DN usage in ipa-client-install

Re: [Freeipa-devel] [PATCH] 316 Improve DN usage in ipa-client-install

Re: [Freeipa-devel] [PATCH 0015] Restrict admins group modifications

Re: [Freeipa-devel] [PATCH] Changes to use a single database for dogtag and IPA

Re: [Freeipa-devel] [PATCH] 319 Make ipakrbprincipal objectclass optional

[Freeipa-devel] [PATCH 0071] Fix potential crash caused by failing zone_register allocation.

[Freeipa-devel] [PATCH 0017] Improve error message in ipa-replica-manage

Re: [Freeipa-devel] [PATCH 0017] Improve error message in ipa-replica-manage

[Freeipa-devel] [PATCH] 320 Only use service PAC type as an override

Re: [Freeipa-devel] [PATCH] Changes to use a single database for dogtag and IPA

Re: [Freeipa-devel] [PATCH] 319 Make ipakrbprincipal objectclass optional

[Freeipa-devel] [PATCH] 1058 clear session key

Re: [Freeipa-devel] [PATCH] 1037 optimize restoring SELinux booleans

Re: [Freeipa-devel] [PATCH 0016] Adds port to connection error message in ipa-client-install

[Freeipa-devel] [PATCH] 75-78 Add fallback group

Re: [Freeipa-devel] [PATCH] 314-315 Limit unindexed searches

Re: [Freeipa-devel] [PATCH] 75-78 Add fallback group

Re: [Freeipa-devel] [PATCH] 320 Only use service PAC type as an override

[Freeipa-devel] [Fwd: [Pki-announce] Announcing Dogtag 10.0.0 alpha 2 release]

26 matches

Site Navigation

Mail list logo

Footer information