date:20120914

On 09/13/2012 09:19 PM, Rob Crittenden wrote:
 Martin Kosek wrote:
 When ADD command is being executed and a single-value object attribute
 is being set with both option and addattr IPA ends up in an internal
 error.

 Make better value sanitizing job in this case and let IPA throw
 a user-friendly error. Unit test exercising this situation is added.

 https://fedorahosted.org/freeipa/ticket/2429
 
 +if not isinstance(val, (list, tuple)):
 +val = [val]
 +val.extend(adddict[attr])
 
 I val is a tuple the extend is still going to fail. Can val ever be a tuple? 
 If
 so we'd need to convert it to a list.
 
 rob

I don't think it could be a tuple, I am about 99% certain. So for this 1% I
added a special clause for tuple. Patch attached.

We will be able to be even more certain when Honza finishes his strict encoding
patch he was working on in the summer. With his patch, the attributes should
always be a list.

Martin
From 915f8ae9540b71f009c43b308bb21ebe6a2e2849 Mon Sep 17 00:00:00 2001
From: Martin Kosek mko...@redhat.com
Date: Thu, 13 Sep 2012 15:51:51 +0200
Subject: [PATCH] Fix addattr internal error

When ADD command is being executed and a single-value object attribute
is being set with both option and addattr IPA ends up in an internal
error.

Make better value sanitizing job in this case and let IPA throw
a user-friendly error. Unit test exercising this situation is added.

https://fedorahosted.org/freeipa/ticket/2429
---
 ipalib/plugins/baseldap.py | 12 +++-
 tests/test_xmlrpc/test_attr.py | 10 ++
 2 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/ipalib/plugins/baseldap.py b/ipalib/plugins/baseldap.py
index 6a054ffd801b159769bc2ce2871cb03afeba5c3d..14a46f2d0344c4276ec98091314b15e6e552ed77 100644
--- a/ipalib/plugins/baseldap.py
+++ b/ipalib/plugins/baseldap.py
@@ -882,7 +882,17 @@ last, after all sets and adds.),
 entry_attrs[attr] = val
 
 for attr in direct_add:
-entry_attrs.setdefault(attr, []).extend(adddict[attr])
+try:
+val = entry_attrs[attr]
+except KeyError:
+val = []
+else:
+if not isinstance(val, (list, tuple)):
+val = [val]
+elif isinstance(val, tuple):
+val = list(val)
+val.extend(adddict[attr])
+entry_attrs[attr] = val
 
 for attr in direct_del:
 for delval in deldict[attr]:
diff --git a/tests/test_xmlrpc/test_attr.py b/tests/test_xmlrpc/test_attr.py
index f5353e1b217fec96e18353923a11b509224a9083..39320875bd5edd4fd6022ed66ce1a8b87ccc8e92 100644
--- a/tests/test_xmlrpc/test_attr.py
+++ b/tests/test_xmlrpc/test_attr.py
@@ -37,6 +37,16 @@ class test_attr(Declarative):
 tests = [
 
 dict(
+desc='Try to add user %r with single-value attribute set via '
+ 'option and --addattr' % user1,
+command=(
+'user_add', [user1], dict(givenname=u'Test', sn=u'User1',
+addattr=u'sn=User2')
+),
+expected=errors.OnlyOneValueAllowed(attr='sn'),
+),
+
+dict(
 desc='Create %r' % user1,
 command=(
 'user_add', [user1], dict(givenname=u'Test', sn=u'User1',
-- 
1.7.11.4

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

Re: [Freeipa-devel] [PATCH] 0077 Check direct/reverse hostname/address resolution in ipa-replica-install

On 09/13/2012 10:35 PM, Rob Crittenden wrote:
 Petr Viktorin wrote:
 On 09/11/2012 11:05 PM, Rob Crittenden wrote:
 Petr Viktorin wrote:
 On 09/04/2012 07:44 PM, Rob Crittenden wrote:
 Petr Viktorin wrote:

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

 Shouldn't this also call verify_fqdn() on the local hostname and not
 just the master? I think this would eventually fail in the conncheck
 but
 what if that was skipped?

 rob

 A few lines above there is a call to get_host_name, which will call
 verify_fqdn.


 I double-checked this, it fails in conncheck. Here are my steps:

 # ipa-server-install --setup-dns
 # ipa-replica-prepare replica.example.com --ip-address=192.168.100.2
 # ipa host-del replica.example.com

 On replica, set DNS to IPA master, with hostname in /etc/hosts.

 # ipa-replica-install ...

 The verify_fqdn() passes because the resolver uses /etc/hosts.

 The conncheck fails:

 Execute check on remote master
 Check connection from master to remote replica 'replica.example.com':

 Remote master check failed with following error message(s):
 Could not chdir to home directory /home/admin: No such file or directory
 Port check failed! Unable to resolve host name 'replica.example.com'

 Connection check failed!
 Please fix your network settings according to error messages above.
 If the check results are not valid it can be skipped with
 --skip-conncheck parameter.

 The DNS test happens much further after this, and I get why, I just
 don't see how useful it is unless the --skip-conncheck is used.

 For the record, it's because we need to check if the host has DNS
 installed. We need a LDAP connection to check this.

 ipa-replica-install ~rcrit/replica-info-replica.example.com.gpg
 --skip-conncheck
 Directory Manager (existing master) password:

 ipa : ERRORCould not resolve hostname replica.example.com
 using DNS. Clients may not function properly. Please check your DNS
 setup. (Note that this check queries IPA DNS directly and ignores
 /etc/hosts.)
 Continue? [no]:

 So I guess, what are the intentions here? It is certainly better than
 before.

 rob

 If the replica is in the master's /etc/hosts, but not in DNS, the
 conncheck will succeed. This check explicitly queries IPA records only
 and ignores /etc/hosts so it'll notice this case and warn.

 
 Ok, like I said, this is better than we have. Just one nit then you get an 
 ack:
 
 +# If remote host has DNS, check forward/reverse resolution
 +try:
 +entry = conn.find_entries(u'cn=dns', base_dn=DN(api.env.basedn))
 +except errors.NotFound:
 
 u'cn=dns' should be str(constants.container_dns).
 
 rob

This is a search filter, Petr could use the one I already have in
dns.py::get_dns_masters() function:
'((objectClass=ipaConfigObject)(cn=DNS))'

For performance sake, I would also not search in the entire tree, but limit the
search only to:

DN(('cn', 'masters'), ('cn', 'ipa'), ('cn', 'etc'), api.env.basedn)

Martin

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

Re: [Freeipa-devel] [PATCH] 305-308 Expand Referential Integrity checks

On 09/13/2012 06:40 PM, Rob Crittenden wrote:
 Martin Kosek wrote:
 To test, add sudo commands, hosts or users to a sudo rule or hbac rule and 
 then
 rename or delete the linked object. After the update, the links should be
 amended.

 -

 Many attributes in IPA (e.g. manager, memberuser, managedby, ...)
 are used to store DNs of linked objects in IPA (users, hosts, sudo
 commands, etc.). However, when the linked objects is deleted or
 renamed, the attribute pointing to it stays with the objects and
 thus may create a dangling link causing issues in client software
 reading the data.

 Directory Server has a plugin to enforce referential integrity (RI)
 by checking DEL and MODRDN operations and updating affected links.
 It was already used for manager and secretary attributes and
 should be expanded for the missing attributes to avoid dangling
 links.

 As a prerequisite, all attributes checked for RI must have pres
 and eq indexes to avoid performance issues. The following indexes
 have been added:
* manager (pres index only)
* secretary (pres index only)
* memberHost
* memberUser
* sourcehost
* memberservice
* managedby
* memberallowcmd
* memberdenycmd
* ipasudorunas
* ipasudorunasgroup

 Referential Integrity plugin was updated to check all these
 attributes.

 Note: this update will only fix RI on one master as RI plugin does
 not check replicated operations.

 https://fedorahosted.org/freeipa/ticket/2866
 
 These patches look good but I'd like to see some tests associated with the
 referential integrity changes in patch 308. I'm not sure we need a test for
 every single combination where RI comes into play but at least testing that 
 the
 original sequence (sudorule/sudocmd) works as expected.
 
 rob

Right, I should have seen that coming. I want this feature to be checked
properly so I added a tests for all RI-checked attributes.

Patches attached.

Martin
From f54743bc5f9d5b83b1376c99483eb38b181d2556 Mon Sep 17 00:00:00 2001
From: Martin Kosek mko...@redhat.com
Date: Wed, 12 Sep 2012 09:28:36 +0200
Subject: [PATCH 1/4] Add attributeTypes to safe schema updater

AttributeType updates are sensitive to case, whitespace or X-ORIGIN mismatch
just like ObjectClass attribute which is already being normalized before
an update value is compared with update instructions.

Expand safe schema updater routine to cover both ObjectClasses and
AttributeTypes updates.

https://fedorahosted.org/freeipa/ticket/2440
---
 ipaserver/install/ldapupdate.py | 68 +++--
 1 file changed, 39 insertions(+), 29 deletions(-)

diff --git a/ipaserver/install/ldapupdate.py b/ipaserver/install/ldapupdate.py
index 111769ffee1d04f2036d3abe49190c715e13f99a..528e349d7975022005d2f91d70a5abed0ab42307 100644
--- a/ipaserver/install/ldapupdate.py
+++ b/ipaserver/install/ldapupdate.py
@@ -35,7 +35,7 @@ from ipalib import errors
 from ipalib import api
 from ipapython.dn import DN
 import ldap
-from ldap.schema.models import ObjectClass
+from ldap.schema.models import ObjectClass, AttributeType
 from ipapython.ipa_log_manager import *
 import krbV
 import platform
@@ -551,23 +551,32 @@ class LDAPUpdate:
 # Replacing objectClassess needs a special handling and
 # normalization of OC definitions to avoid update failures for
 # example when X-ORIGIN is the only difference
-objectclass_replacement = False
-if action == replace and entry.dn == DN(('cn', 'schema')) and \
-attr.lower() == objectclasses:
-objectclass_replacement = True
-oid_index = {}
-# build the OID index for replacing
-for objectclass in entry_values:
-try:
-objectclass_object = ObjectClass(str(objectclass))
-except Exception, e:
-self.error('replace: cannot parse ObjectClass %s: %s',
-objectclass, e)
-continue
-# In a corner case, there may be more representations of
-# the same objectclass due to the previous updates
-# We want to replace them all
-oid_index.setdefault(objectclass_object.oid, []).append(objectclass)
+schema_update = False
+schema_elem_class = None
+schema_elem_name = None
+if action == replace and entry.dn == DN(('cn', 'schema')):
+if attr.lower() == objectclasses:
+schema_elem_class = ObjectClass
+schema_elem_name = ObjectClass
+elif attr.lower() == attributetypes:
+schema_elem_class = AttributeType
+schema_elem_name = AttributeType
+
+if schema_elem_class is not None:
+schema_update = True
+

[Freeipa-devel] [PATCH 0062] Prevent memory read outside allocated space in str_alloc()


Hello,

Prevent memory read outside allocated space in str_alloc().

Found by Valgrind during nsupdate stress test.

--
Petr^2 Spacek
From c53ec9cf2cc22e29630767b6b2259d145192ff62 Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Fri, 14 Sep 2012 10:48:04 +0200
Subject: [PATCH] Prevent memory read outside allocated space in str_alloc().

Signed-off-by: Petr Spacek pspa...@redhat.com
---
 src/str.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/str.c b/src/str.c
index 0096536de071f7d0dd2ff327e4d6ac72e81dab5f..83645365ee6eff7bda5fbeda6837f30d4dec41ae 100644
--- a/src/str.c
+++ b/src/str.c
@@ -102,7 +102,7 @@ str_alloc(ld_string_t *str, size_t len)
 		return ISC_R_NOMEMORY;
 
 	if (str-data != NULL) {
-		memcpy(new_buffer, str-data, len);
+		memcpy(new_buffer, str-data, str-allocated);
 		new_buffer[len] = '\0';
 		isc_mem_put(str-mctx, str-data, str-allocated);
 	} else {
-- 
1.7.11.4

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

Re: [Freeipa-devel] [PATCH] 0079 Update the pot file (translation source)


On 09/13/2012 09:21 PM, Rob Crittenden wrote:

Petr Viktorin wrote:

Transifex is watching our repository, so pushing this patch will update
the translations on the site.


Okay, I lied. Some time ago (before I joined), Transifex changed its 
watching mechanism from VCS pulls to URL polling. I guess IPA got 
unwatched then.


I pushed the pot manually.
Since we have infrequent explicit string freezes I don't think it's 
necessary to configure automatic pot updates again.




ACK, pushed to master and ipa-3-0

rob



--
Petr³

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

Re: [Freeipa-devel] [PATCH] 1050 prevent replica orphans

On 09/10/2012 08:34 PM, Rob Crittenden wrote:
 Martin Kosek wrote:
 On Thu, 2012-09-06 at 17:22 -0400, Rob Crittenden wrote:
 Martin Kosek wrote:
 On 08/31/2012 07:40 PM, Rob Crittenden wrote:
 Rob Crittenden wrote:
 It was possible use ipa-replica-manage connect/disconnect/del to end up
 orphaning or or more IPA masters. This is an attempt to catch and
 prevent that case.

 I tested with this topology, trying to delete B.

 A - B - C

 I got here by creating B and C from A, connecting B to C then deleting
 the link from A to B, so it went from A - B and A - C to the above.

 What I do is look up the servers that the delete candidate host has
 connections to and see if we're the last link.

 I added an escape clause if there are only two masters.

 rob

 Oh, this relies on my cleanruv patch 1031.

 rob


 1) When I run ipa-replica-manage del --force to an already uninstalled 
 host,
 the new code will prevent me the deletation because it cannot connect to
 it. It
 also crashes with UnboundLocalError:

 # ipa-replica-manage del vm-055.idm.lab.bos.redhat.com --force

 Unable to connect to replica vm-055.idm.lab.bos.redhat.com, forcing removal
 Traceback (most recent call last):
 File /sbin/ipa-replica-manage, line 708, in module
   main()
 File /sbin/ipa-replica-manage, line 677, in main
   del_master(realm, args[1], options)
 File /sbin/ipa-replica-manage, line 476, in del_master
   sys.exit(Failed read master data from '%s': %s % (delrepl.hostname,
 str(e)))
 UnboundLocalError: local variable 'delrepl' referenced before assignment

 Fixed.



 I also hit this error when removing a winsync replica.

 Fixed.



 2) As I wrote before, I think having --force option override the user
 inquiries
 would benefit test automation:

 +if not ipautil.user_input(Continue to delete?, False):
 +sys.exit(Aborted)

 Fixed.



 3) I don't think this code won't cover this topology:

 A - B - C - D - E

 It would allow you deleting a replica C even though it would separate A-B 
 and
 D-E. Though we may not want to cover this situation now, what you got is
 definitely helping.

 I think you may be right. I only tested with 4 servers. With this B and
 D would both still have 2 agreements so wouldn't be covered by the last
 link test.

 Everything looks good now, so ACK. We just need to push it along with
 CLEANALLRUV patch.

 Martin

 
 Not to look a gift ACK In the mouth but here is a revised patch. I've added a
 cleanup routine to remove an orphaned master properly. If you had tried the
 mechanism I outlined in the man page it would have worked but was
 less-than-complete. This way is better, just don't try it on a live master.
 
 I also added a cleanruv abort command, in case you want to kill an existing 
 task.
 
 rob
 

1) CLEANRUV stuff should be in your patch 1031 and not here (but I will comment
on the code in this mail since it is in this patch)


2) In new command defitinion:

+abort-clean-ruv:(1, 1, Replica ID of to abort cleaning, ),

I miss error message in case REPLICA_ID is not passed, this way error message
when I omit the parameter is confusing:

# ipa-replica-manage abort-clean-ruv
Usage: ipa-replica-manage [options]

ipa-replica-manage: error: must provide a command [force-sync | clean-ruv |
disconnect | connect | list-ruv | del | re-initialize | list | abort-clean-ruv]

On another note, I am thinking about the new command(s). Since we now have
abort-clean-ruv command, we may want to also implement list-clean-ruv commands
in the future to see all CLEANALLRUV commands in process... Or we may enhance
list-ruv to write a flag like [CLEANALLRUV in process] for RUV's for which
CLEANALLRUV task is in process.


3) Will clean-ruv - abort-clean-ruv - clean-ruv sequence work? If the aborted
CLEANALLRUV task stays in DIT, we may not be able to enter new CLEANALLRUV task
because we always use the same DN...

Btw. did abort CLEANALLRUV command worked for you? Mine seemed to be stuck on
replicas that are down just like CLEANALLRUV command. I had then paralelly
running CLEANALLRUV and ABORT CLEANALLRUV running for the same RUV ID. Then, it
is unclear to me what this command is actually good for.


4) Man page now contains non-existent command:

--- a/install/tools/man/ipa-replica-manage.1
+++ b/install/tools/man/ipa-replica-manage.1
@@ -42,12 +42,18 @@ Manages the replication agreements of an IPA server.
 \fBforce\-sync\fR
 \- Immediately flush any data to be replicated from a server specified with
the \-\-from option
 .TP
+\fBcleanup\fR
+\- Remove leftover references to a deleted master.
+.TP


The cleanup procedure was implemented rather as an option to the del command
than a separate command.


5) My favorite - new cleanup procedure user prompt miss the --force option
useful for test automation

+if not ipautil.user_input(Continue to clean master?, False):
+sys.exit(Cleanup aborted)


Otherwise the patch looks good.

Martin

Re: [Freeipa-devel] [PATCH] 1031 run cleanallruv task

On 09/06/2012 11:17 PM, Rob Crittenden wrote:
Martin Kosek wrote:
On 09/06/2012 05:55 PM, Rob Crittenden wrote:
Rob Crittenden wrote:
Rob Crittenden wrote:
Martin Kosek wrote:
On 09/05/2012 08:06 PM, Rob Crittenden wrote:
Rob Crittenden wrote:
Martin Kosek wrote:
On 07/05/2012 08:39 PM, Rob Crittenden wrote:
Martin Kosek wrote:
On 07/03/2012 04:41 PM, Rob Crittenden wrote:
Deleting a replica can leave a replication vector (RUV) on the
other servers.
This can confuse things if the replica is re-added, and it also
causes the
server to calculate changes against a server that may no longer
exist.

389-ds-base provides a new task that self-propogates itself to all
available
replicas to clean this RUV data.

This patch will create this task at deletion time to hopefully
clean things up.

It isn't perfect. If any replica is down or unavailable at the
time
the
cleanruv task fires, and then comes back up, the old RUV data
may be
re-propogated around.

To make things easier in this case I've added two new commands to
ipa-replica-manage. The first lists the replication ids of all the
servers we
have a RUV for. Using this you can call clean_ruv with the
replication id of a
server that no longer exists to try the cleanallruv step again.

This is quite dangerous though. If you run cleanruv against a
replica id that
does exist it can cause a loss of data. I believe I've put in
enough scary
warnings about this.

rob

Good work there, this should make cleaning RUVs much easier than
with the
previous version.

This is what I found during review:

1) list_ruv and clean_ruv command help in man is quite lost. I
think
it would
help if we for example have all info for commands indented. This
way
user could
simply over-look the new commands in the man page.

2) I would rename new commands to clean-ruv and list-ruv to make
them
consistent with the rest of the commands (re-initialize,
force-sync).

3) It would be nice to be able to run clean_ruv command in an
unattended way
(for better testing), i.e. respect --force option as we already
do for
ipa-replica-manage del. This fix would aid test automation in the
future.

4) (minor) The new question (and the del too) does not react too
well for
CTRL+D:

# ipa-replica-manage clean_ruv 3 --force
Clean the Replication Update Vector for
vm-055.idm.lab.bos.redhat.com:389

Cleaning the wrong replica ID will cause that server to no
longer replicate so it may miss updates while the process
is running. It would need to be re-initialized to maintain
consistency. Be very careful.
Continue to clean? [no]: unexpected error:

5) Help for clean_ruv command without a required parameter is quite
confusing
as it reports that command is wrong and not the parameter:

# ipa-replica-manage clean_ruv
Usage: ipa-replica-manage [options]

It seems you just forgot to specify the error message in the
command
definition

6) When the remote replica is down, the clean_ruv command fails
with an
unexpected error:

[root@vm-086 ~]# ipa-replica-manage clean_ruv 5
Clean the Replication Update Vector for
vm-055.idm.lab.bos.redhat.com:389

/var/log/dirsrv/slapd-IDM-LAB-BOS-REDHAT-COM/errors:
[04/Jul/2012:06:28:16 -0400] NSMMReplicationPlugin -
cleanAllRUV_task: failed
to connect to replagreement connection
(cn=meTovm-055.idm.lab.bos.redhat.com,cn=replica,

cn=dc\3Didm\2Cdc\3Dlab\2Cdc\3Dbos\2Cdc\3Dredhat\2Cdc\3Dcom,cn=mapping

tree,cn=config), error 105
[04/Jul/2012:06:28:16 -0400] NSMMReplicationPlugin -
cleanAllRUV_task: replica
(cn=meTovm-055.idm.lab.
bos.redhat.com,cn=replica,cn=dc\3Didm\2Cdc\3Dlab\2Cdc\3Dbos\2Cdc\3Dredhat\2Cdc\3Dcom,cn=mapping

tree, cn=config) has not been cleaned. You will need to rerun
the
CLEANALLRUV task on this replica.
[04/Jul/2012:06:28:16 -0400] NSMMReplicationPlugin -
cleanAllRUV_task: Task
failed (1)

In this case I think we should inform user that the command failed,
possibly
because of disconnected replicas and that they could enable the
replicas and
try again.

7) (minor) pass is now redundant in replication.py:
+except ldap.INSUFFICIENT_ACCESS:
+# We can't make the server we're removing read-only
but
+# this isn't a show-stopper
+root_logger.debug(No permission to switch replica to
read-only,
continuing anyway)
+pass

I think this addresses everything.

rob

Thanks, almost there! I just found one more issue which needs to be
fixed
before we push:

[Freeipa-devel] [PATCH] 0081 Only stop the main DS instance when upgrading it


This fixes a 2.2→3.0 upgrade bug found while testing the Dogtag 10 work.
See commit or ticket for details.

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



I also suspect that waiting for ports is not a good way to check if the 
CMS is fully initialized, but I don't know of a better way. If you know 
one, please speak up.


--
Petr³
From 4d5f1869572a552bde393d78c5d16d08a351acaf Mon Sep 17 00:00:00 2001
From: Petr Viktorin pvikt...@redhat.com
Date: Thu, 13 Sep 2012 11:43:09 -0400
Subject: [PATCH] Only stop the main DS instance when upgrading it

We've been stopping both DS instances (main and PKI) when upgrading.
This can happen while the CA is running. In some cases stopping the PKI
DS also killed the CA.

Only stop the specific instance for upgrades.

Also, wait for open ports after the upgrade is complete. The wait was
skipped previously. This can prevent bugs if scripts that need a DS are
run after the upgrade.

https://fedorahosted.org/freeipa/ticket/3083
---
 ipaserver/install/upgradeinstance.py | 15 ++-
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/ipaserver/install/upgradeinstance.py b/ipaserver/install/upgradeinstance.py
index f1f702b1c51eed0277fd4f02f5c1d4048292d894..3c6bbec5b386768ed951dffc65fcdb34f3014dad 100644
--- a/ipaserver/install/upgradeinstance.py
+++ b/ipaserver/install/upgradeinstance.py
@@ -59,19 +59,24 @@ def __init__(self, realm_name, files=[], live_run=True):
 self.modified = False
 self.badsyntax = False
 self.upgradefailed = False
+self.serverid = serverid
 
-def start(self, instance_name=, capture_output=True, wait=True):
+def __start_nowait(self):
 # Don't wait here because we've turned off port 389. The connection
 # we make will wait for the socket.
-super(IPAUpgrade, self).start(instance_name, capture_output, wait=False)
+super(IPAUpgrade, self).start(wait=False)
+
+def __stop_instance(self):
+Stop only the main DS instance
+super(IPAUpgrade, self).stop(self.serverid)
 
 def create_instance(self):
-self.step(stopping directory server, self.stop)
+self.step(stopping directory server, self.__stop_instance)
 self.step(saving configuration, self.__save_config)
 self.step(disabling listeners, self.__disable_listeners)
-self.step(starting directory server, self.start)
+self.step(starting directory server, self.__start_nowait)
 self.step(upgrading server, self.__upgrade)
-self.step(stopping directory server, self.stop)
+self.step(stopping directory server, self.__stop_instance)
 self.step(restoring configuration, self.__restore_config)
 self.step(starting directory server, self.start)
 
-- 
1.7.11.4

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

Re: [Freeipa-devel] [PATCH] Patch to allow IPA to work with dogtag 10 on f18


On 09/11/2012 09:20 PM, Rob Crittenden wrote:

Rob Crittenden wrote:

Petr Viktorin wrote:

On 09/11/2012 04:38 PM, Rob Crittenden wrote:

Ade Lee wrote:

On Tue, 2012-09-11 at 08:59 -0400, Rob Crittenden wrote:

Petr Viktorin wrote:

On 09/11/2012 04:04 AM, Ade Lee wrote:

On Mon, 2012-09-10 at 16:58 -0400, Rob Crittenden wrote:

Petr Viktorin wrote:

Attaching rebased and squashed patches. I've done some testing
with
them
but please test some more.



Most of these aren't IPA issues, but dogtag issues. I'll try to
split
them out.

IPA:

For the configuration files in install/conf to be updated at rpm
update
time the VERSION needs to be incremented.


These files should stay the same since on upgrade we're still
using a
Dogtag 9 style instance. The Dogtag 10 ports are only used in new
installs.


The ipa package lacks any updated dogtag dependencies, so I abused
it.


What should the updated dependencies be? Since it should work with
both
dogtag 9 and 10, I don't see how they should change.


I don't know either, but we need to prevent people from installing
incompatible package combinations.


Would'nt the Conflicts: ipa  3.0 in pki-ca mentioned below satisfy
this
requirement?  The main concern is that you must have ipa 3.0 if you
have
dogtag 10.

Given that dogtag is consumed by IPA though, it makes more sense to
put
the relevant conflicts in IPA rather than in dogtag.  So in this case,
that would mean putting Conflicts: pki-ca = 10.0 in IPA 2.x.
Recall that dogtag 10 will only be officially available in f18+.


That isn't enough. If a F-17 user with IPA 2.2 installed upgrades to
F-18 they would be able to install dogtag 10 and blow up their IPA
server.




I installed IPA with dogtag 9 and created a replica.

I updated the IPA bits, that worked fine.

I updated to dogtag 10 and now the CA doesn't work on the master,
including starting the dogtag instance. Note that the rpm update
process
worked, no notice that the CA service didn't restart.


Did you try to restart the CA with selinux in permissive mode?
This is
still required right now until I get the selinux policy all
straightened
out.

There is also a separate dogtag ticket (which is currently being
worked
on) to restart existing dogtag instances when dogtag is upgraded
from
9-10.


In permissive mode, this upgrade works for me.


I was in enforcing mode but saw no AVCs. What is the ETA on fixing
this?



Within the next week or two, I need to finish the IPA merge database
patch first, and then co-ordinate changes with the selinux guys.




Sometimes I do get this error intermittently:

ipa: ERROR: Certificate operation cannot be completed: Unable to
communicate with CMS (Service Temporarily Unavailable)

Usually, waiting a couple of minutes clears this up. Perhaps we are
not
doing startup detection correctly. Ade mentioned that waiting for
ports
may not be ideal. How do we know if Dogtag is initialized?


Years ago we had discussed with Andrew and Matt creating a URI that
can
be queried to determine dogtag status. I don't think that ever went
anywhere.


Petr, this happens only on reboot, right?  And not on regular service
ipa restart?


I've now seen it happen right after a 9 → 10 upgrade.


Yeah, I remember this conversation - and even created a bug for it at
some point.  This went away because the mechanism you were using
seemed
to be working.  The timing may be very different now with tomcat 7 and
systemd.  I'll open a dogtag trac ticket for this.


Ok.






Uninstalling failed because it tried to run pkidestroy and not
pkiremove.


I was under the impression that pkidestroy was the correct
command to
remove an upgraded instance. I'll check with Ade.


I'll test this too.


The contents of the file passed to pkispawn should be logged so we
can
see exactly what was passed in.


Its a pretty big file.  You might want to only log the
modifications.
Or save the file somewhere.


Our logs are pretty verbose, so that shouldn't be a problem. I'll
put it
in the next version of the patch.


The question to ask is: would you need the contents of this file if
all
you got were logs and needed to evaluate why installation failed? In
most cases this is yes.


Up to you guys.  There is a patch I am working on in which I will be
logging the object that is being passed to the server from pkispawn.
That - and the diffs to the standard config file as I mentioned
above -
will likely be sufficient to debug almost all cases.

Make sure not to log any passwords.



Thanks for the catch. Attaching updated patch that sanitizes the
passwords.


DOGTAG:

When upgrading using the dogtag-devel repo I had to specify
pki-tools.x86_64 otherwise it tried to install both 32 and 64-bit
versions (and failed).

I ended up running: yum update pki-ca tomcatjss pki-tools.x86_64
--enablerepo=dogtag-devel --enablerepo=updates-testing


We'll look into this.  I think I know why this happens.


What happens if someone manually upgrades pki-ca without first
updating
ipa? I think that

Re: [Freeipa-devel] [PATCH 0055] Fix race condition in addrdataset() during SOA serial update

On Fri, Sep 07, 2012 at 01:05:37PM +0200, Petr Spacek wrote:
 Hello,
 
 Fix race condition in addrdataset() during SOA serial update.
 
 https://fedorahosted.org/bind-dyndb-ldap/ticket/89

Good catch, thanks. Ack

A

 From 5e8bc8f943345d8d92900474905288939958dcd8 Mon Sep 17 00:00:00 2001
 From: Petr Spacek pspa...@redhat.com
 Date: Fri, 7 Sep 2012 13:01:57 +0200
 Subject: [PATCH] Fix race condition in addrdataset() during SOA serial
  update.
 
 https://fedorahosted.org/bind-dyndb-ldap/ticket/89
 
 Signed-off-by: Petr Spacek pspa...@redhat.com
 ---
  src/ldap_driver.c | 44 ++--
  src/ldap_helper.c |  4 ++--
  2 files changed, 36 insertions(+), 12 deletions(-)
 
 diff --git a/src/ldap_driver.c b/src/ldap_driver.c
 index 
 2cdde30cdad9544d530475f5cf4a0b8275a56f03..3a802238028145d35390f6a8d00f156bfdf8e7a1
  100644
 --- a/src/ldap_driver.c
 +++ b/src/ldap_driver.c
 @@ -936,6 +936,7 @@ addrdataset(dns_db_t *db, dns_dbnode_t *node, 
 dns_dbversion_t *version,
   dns_rdatalist_t diff;
   isc_result_t result;
   isc_boolean_t rdatalist_exists = ISC_FALSE;
 + isc_boolean_t soa_simulated_write = ISC_FALSE;
  
   UNUSED(now);
   UNUSED(db);
 @@ -975,42 +976,65 @@ addrdataset(dns_db_t *db, dns_dbnode_t *node, 
 dns_dbversion_t *version,
   rdatalist_removedups(found_rdlist, new_rdlist,
ISC_FALSE, diff);
  
 - if ((options  DNS_DBADD_MERGE) != 0)
 + if ((options  DNS_DBADD_MERGE) == 0 
 + (rdatalist_length(diff) != 0)) {
 + CLEANUP_WITH(DNS_R_NOTEXACT);
 + } else {
   free_rdatalist(ldapdb-common.mctx, diff);
 - else if (rdatalist_length(diff) != 0) {
 - free_rdatalist(ldapdb-common.mctx, diff);
 - result = DNS_R_NOTEXACT;
 - goto cleanup;
   }
   } else {
   /* Replace existing rdataset */
   free_rdatalist(ldapdb-common.mctx, found_rdlist);
   }
   }
  
 - CHECK(write_to_ldap(ldapdbnode-owner, ldapdb-ldap_inst, new_rdlist));
 + /* HACK: SOA addition will never fail with DNS_R_UNCHANGED.
 +  * This prevents warning from BIND's diff_apply(), it has too strict
 +  * checks for us.
 +  *
 +  * Reason: There is a race condition between SOA serial update
 +  * from BIND's update_action() and our persistent search watcher, 
 because
 +  * they don't know about each other.
 +  * BIND's update_action() changes data with first addrdataset() call and
 +  * then changes serial with second addrdataset() call.
 +  * It can lead to empty diff if persistent search watcher
 +  * incremented serial in meanwhile.
 +  */
 + if (HEAD(new_rdlist-rdata) == NULL) {
 + if (rdlist-type == dns_rdatatype_soa)
 + soa_simulated_write = ISC_TRUE;
 + else
 + CLEANUP_WITH(DNS_R_UNCHANGED);
 + } else {
 + CHECK(write_to_ldap(ldapdbnode-owner, ldapdb-ldap_inst, 
 new_rdlist));
 + }
 +
  
   if (addedrdataset != NULL) {
 - result = dns_rdatalist_tordataset(new_rdlist, addedrdataset);
 - /* Use strong condition here, returns only SUCCESS */
 - INSIST(result == ISC_R_SUCCESS);
 + if (soa_simulated_write) {
 + dns_rdataset_clone(rdataset, addedrdataset);
 + } else {
 + result = dns_rdatalist_tordataset(new_rdlist, 
 addedrdataset);
 + /* Use strong condition here, returns only SUCCESS */
 + INSIST(result == ISC_R_SUCCESS);
 + }
   }
  
   if (rdatalist_exists) {
   ISC_LIST_APPENDLIST(found_rdlist-rdata, new_rdlist-rdata,
   link);
   SAFE_MEM_PUT_PTR(ldapdb-common.mctx, new_rdlist);
   } else
   APPEND(ldapdbnode-rdatalist, new_rdlist, link);
  
 -
   return ISC_R_SUCCESS;
  
  cleanup:
   if (new_rdlist != NULL) {
   free_rdatalist(ldapdb-common.mctx, new_rdlist);
   SAFE_MEM_PUT_PTR(ldapdb-common.mctx, new_rdlist);
   }
 + free_rdatalist(ldapdb-common.mctx, diff);
  
   return result;
  }
 diff --git a/src/ldap_helper.c b/src/ldap_helper.c
 index 
 3241ffe486205fa03a6fd1a0a14edf1245c5c4aa..e636a84b35d0bcdc8573c6e7146f38ee21a42076
  100644
 --- a/src/ldap_helper.c
 +++ b/src/ldap_helper.c
 @@ -2973,10 +2973,10 @@ soa_serial_increment(isc_mem_t *mctx, ldap_instance_t 
 *inst,
  
   /* put the new SOA to inst-cache and compare old and new serials */
   CHECK(ldap_get_zone_serial(inst, zone_name, new_serial));
 - INSIST(isc_serial_gt(new_serial, old_serial) == ISC_TRUE);

Re: [Freeipa-devel] [PATCH 0056] Fix crash caused by zone deletion vs. SOA serial increment race condition

On Wed, Sep 12, 2012 at 12:33:47PM +0200, Petr Spacek wrote:
 Hello,
 
 The patch fixes crash caused by stupid bug in logging code.

Ack.

 From 01aa00f9ba4feac9f97b34b81c3697b2b7f8122f Mon Sep 17 00:00:00 2001
 From: Petr Spacek pspa...@redhat.com
 Date: Fri, 7 Sep 2012 16:21:27 +0200
 Subject: [PATCH] Fix crash caused by zone deletion vs. SOA serial increment
  race condition.
 
 Signed-off-by: Petr Spacek pspa...@redhat.com
 ---
  src/ldap_helper.c | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)
 
 diff --git a/src/ldap_helper.c b/src/ldap_helper.c
 index 
 e636a84b35d0bcdc8573c6e7146f38ee21a42076..058048f41485999be0d8ffeadea02f2e25879370
  100644
 --- a/src/ldap_helper.c
 +++ b/src/ldap_helper.c
 @@ -2931,6 +2931,7 @@ soa_serial_increment(isc_mem_t *mctx, ldap_instance_t 
 *inst,
   dns_name_t *zone_name) {
   isc_result_t result = ISC_R_FAILURE;
   ld_string_t *zone_dn = NULL;
 + const char *zone_dn_char = INACTIVE/UNKNOWN;
   ldapdb_rdatalist_t rdatalist;
   dns_rdatalist_t *rdlist = NULL;
   dns_rdata_t *soa_rdata = NULL;
 @@ -2944,6 +2945,7 @@ soa_serial_increment(isc_mem_t *mctx, ldap_instance_t 
 *inst,
   INIT_LIST(rdatalist);
   CHECK(str_new(mctx, zone_dn));
   CHECK(dnsname_to_dn(inst-zone_register, zone_name, zone_dn));
 + zone_dn_char = str_buf(zone_dn);
   log_debug(5, incrementing SOA serial number in zone '%s',
   str_buf(zone_dn));
  
 @@ -2978,7 +2980,7 @@ cleanup:
   if (result != ISC_R_SUCCESS ||
   isc_serial_gt(new_serial, old_serial) != ISC_TRUE)
   log_error(SOA serial number incrementation failed in zone 
 '%s',
 - str_buf(zone_dn));
 + zone_dn_char);
  
   str_destroy(zone_dn);
   ldapdb_rdatalist_destroy(mctx, rdatalist);
 -- 
 1.7.11.4
 


-- 
Adam Tkac, Red Hat, Inc.

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

Re: [Freeipa-devel] [PATCH] 0081 Only stop the main DS instance when upgrading it

2012-09-14 Thread Simo Sorce

On Fri, 2012-09-14 at 14:53 +0200, Petr Viktorin wrote:
 This fixes a 2.2→3.0 upgrade bug found while testing the Dogtag 10 work.
 See commit or ticket for details.
 
 https://fedorahosted.org/freeipa/ticket/3083
 
 
 
 I also suspect that waiting for ports is not a good way to check if the 
 CMS is fully initialized, but I don't know of a better way. If you know 
 one, please speak up.
 

Won;t we get back to square zero when the work to merge DS and CS
instances into one will be completed ?

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] 309 Fix addattr internal error

2012-09-14 Thread Jan Cholasta


Dne 14.9.2012 08:40, Martin Kosek napsal(a):

On 09/13/2012 09:19 PM, Rob Crittenden wrote:

Martin Kosek wrote:

When ADD command is being executed and a single-value object attribute
is being set with both option and addattr IPA ends up in an internal
error.

Make better value sanitizing job in this case and let IPA throw
a user-friendly error. Unit test exercising this situation is added.

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


+if not isinstance(val, (list, tuple)):
+val = [val]
+val.extend(adddict[attr])

I val is a tuple the extend is still going to fail. Can val ever be a tuple? If
so we'd need to convert it to a list.

rob


I don't think it could be a tuple, I am about 99% certain. So for this 1% I
added a special clause for tuple. Patch attached.


It is possible that it can be a tuple ATM.



We will be able to be even more certain when Honza finishes his strict encoding
patch he was working on in the summer. With his patch, the attributes should
always be a list.



Yes.

Also, this was already fixed in my patch for 
https://fedorahosted.org/freeipa/ticket/2954, but it was reverted.





Martin



Honza

--
Jan Cholasta

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

Re: [Freeipa-devel] [PATCH 0056] Fix crash caused by zone deletion vs. SOA serial increment race condition


On 09/14/2012 03:10 PM, Adam Tkac wrote:

On Wed, Sep 12, 2012 at 12:33:47PM +0200, Petr Spacek wrote:

Hello,

The patch fixes crash caused by stupid bug in logging code.


Ack.



Pushed to master:

https://fedorahosted.org/bind-dyndb-ldap/changeset/da558d15329f6e2308ad69118545363b9adbd0d1

Petr^2 Spacek

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

Re: [Freeipa-devel] [PATCH 0055] Fix race condition in addrdataset() during SOA serial update


On 09/14/2012 03:07 PM, Adam Tkac wrote:

On Fri, Sep 07, 2012 at 01:05:37PM +0200, Petr Spacek wrote:

Hello,

 Fix race condition in addrdataset() during SOA serial update.

 https://fedorahosted.org/bind-dyndb-ldap/ticket/89


Good catch, thanks. Ack

A


Pushed to master:
https://fedorahosted.org/bind-dyndb-ldap/changeset/dca6558ae5224674878972dc1e71e846a13bf139

Petr^2 Spacek

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

Re: [Freeipa-devel] [PATCH 0057] Fix LDAP operation selection logic in ldap_modify_do()

On Wed, Sep 12, 2012 at 12:35:25PM +0200, Petr Spacek wrote:
 Hello,
 
 There is a fix for LDAP operation selection logic in ldap_modify_do().
 
 Each operation code in LDAPMod structure can be ORed
 with LDAP_MOD_BVALUES.

Ack

 From ab11e62ec2496f2c7245c4d8d80c2fd189b68aa9 Mon Sep 17 00:00:00 2001
 From: Petr Spacek pspa...@redhat.com
 Date: Tue, 11 Sep 2012 16:23:18 +0200
 Subject: [PATCH] Fix LDAP operation selection logic in ldap_modify_do().
 
 Each operation code in LDAPMod structure can be ORed
 with LDAP_MOD_BVALUES.
 
 Signed-off-by: Petr Spacek pspa...@redhat.com
 ---
  src/ldap_helper.c | 29 +
  1 file changed, 17 insertions(+), 12 deletions(-)
 
 diff --git a/src/ldap_helper.c b/src/ldap_helper.c
 index 
 058048f41485999be0d8ffeadea02f2e25879370..d9c7ce5d84c3944a86ff1865ff6be073ddc294c8
  100644
 --- a/src/ldap_helper.c
 +++ b/src/ldap_helper.c
 @@ -2149,33 +2149,38 @@ ldap_modify_do(ldap_instance_t *ldap_inst, 
 ldap_connection_t *ldap_conn,
   CHECK(ldap_connect(ldap_inst, ldap_conn, ISC_FALSE));
   }
  
 + /* Any mod_op can be ORed with LDAP_MOD_BVALUES. */
 + if ((mods[0]-mod_op  ~LDAP_MOD_BVALUES) == LDAP_MOD_ADD)
 + operation_str = modifying(add);
 + else if ((mods[0]-mod_op  ~LDAP_MOD_BVALUES) == LDAP_MOD_DELETE)
 + operation_str = modifying(del);
 + else if ((mods[0]-mod_op  ~LDAP_MOD_BVALUES) == LDAP_MOD_REPLACE)
 + operation_str = modifying(replace);
 + else {
 + operation_str = modifying(unknown operation);
 + log_bug(%s: 0x%x, operation_str, mods[0]-mod_op);
 + CHECK(ISC_R_NOTIMPLEMENTED);
 + }
 +
   if (delete_node) {
   log_debug(2, deleting whole node: '%s', dn);
   ret = ldap_delete_ext_s(ldap_conn-handle, dn, NULL, NULL);
   } else {
 - log_debug(2, writing to '%s', dn);
 + log_debug(2, writing to '%s': %s, dn, operation_str);
   ret = ldap_modify_ext_s(ldap_conn-handle, dn, mods, NULL, 
 NULL);
   }
  
   result = (ret == LDAP_SUCCESS) ? ISC_R_SUCCESS : ISC_R_FAILURE;
   if (ret == LDAP_SUCCESS)
   goto cleanup;
  
 - if (mods[0]-mod_op == LDAP_MOD_ADD)
 - operation_str = modifying(add);
 - else if (mods[0]-mod_op == LDAP_MOD_DELETE)
 - operation_str = modifying(del);
 - else {
 - operation_str = modifying(unknown operation);
 - CHECK(ISC_R_NOTIMPLEMENTED);
 - }
 -
   LDAP_OPT_CHECK(ldap_get_option(ldap_conn-handle, LDAP_OPT_RESULT_CODE,
   err_code), ldap_modify_do(%s) failed to obtain ldap 
 error code,
   operation_str);
  
   /* If there is no object yet, create it with an ldap add operation. */
 - if (mods[0]-mod_op == LDAP_MOD_ADD  err_code == LDAP_NO_SUCH_OBJECT) 
 {
 + if ((mods[0]-mod_op  ~LDAP_MOD_BVALUES) == LDAP_MOD_ADD 
 +  err_code == LDAP_NO_SUCH_OBJECT) {
   int i;
   LDAPMod **new_mods;
   char *obj_str[] = { idnsRecord, NULL };
 @@ -2211,7 +2216,7 @@ ldap_modify_do(ldap_instance_t *ldap_inst, 
 ldap_connection_t *ldap_conn,
  
   /* do not error out if we are trying to delete an
* unexisting attribute */
 - if (mods[0]-mod_op != LDAP_MOD_DELETE ||
 + if ((mods[0]-mod_op  ~LDAP_MOD_BVALUES) != LDAP_MOD_DELETE ||
   err_code != LDAP_NO_SUCH_ATTRIBUTE) {
   result = ISC_R_FAILURE;
   }
 -- 
 1.7.11.4
 


-- 
Adam Tkac, Red Hat, Inc.

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

Re: [Freeipa-devel] [PATCH 0058] Improve persistent search logging

On Wed, Sep 12, 2012 at 12:36:38PM +0200, Petr Spacek wrote:
 Hello,
 
 this patch adds result codes to error messages in persistent search code.

Ack.

 From f6cb53278d8f39ac6da4fb8e26820f6ee02ae6e3 Mon Sep 17 00:00:00 2001
 From: Petr Spacek pspa...@redhat.com
 Date: Wed, 12 Sep 2012 12:27:51 +0200
 Subject: [PATCH] Improve persistent search logging.
 
 Signed-off-by: Petr Spacek pspa...@redhat.com
 ---
  src/ldap_helper.c | 14 +++---
  1 file changed, 7 insertions(+), 7 deletions(-)
 
 diff --git a/src/ldap_helper.c b/src/ldap_helper.c
 index 
 d9c7ce5d84c3944a86ff1865ff6be073ddc294c8..92edbe7159272772e1c993d46da7c93382cbc5d4
  100644
 --- a/src/ldap_helper.c
 +++ b/src/ldap_helper.c
 @@ -3069,9 +3069,9 @@ update_zone(isc_task_t *task, isc_event_t *event)
  
  cleanup:
   if (result != ISC_R_SUCCESS)
 - log_error(update_action (psearch) failed for '%s': %s. 
 + log_error_r(update_zone (psearch) failed for '%s'. 
 Zones can be outdated, run `rndc reload`,
 -   pevent-dn, isc_result_totext(result));
 +   pevent-dn);
  
   ldap_query_free(ISC_FALSE, ldap_qresult_zone);
   ldap_query_free(ISC_FALSE, ldap_qresult_record);
 @@ -3125,7 +3125,7 @@ update_config(isc_task_t *task, isc_event_t *event)
  
  cleanup:
   if (result != ISC_R_SUCCESS)
 - log_error(update_config (psearch) failed for %s. 
 + log_error_r(update_config (psearch) failed for '%s'. 
 Configuration can be outdated, run `rndc reload`,
 pevent-dn);
  
 @@ -3221,9 +3221,9 @@ update_record(isc_task_t *task, isc_event_t *event)
   }
  cleanup:
   if (result != ISC_R_SUCCESS)
 - log_error(update_record (psearch) failed, dn '%s'. 
 + log_error_r(update_record (psearch) failed, dn '%s' change 
 type 0x%x. 
 Records can be outdated, run `rndc reload`,
 -   pevent-dn);
 +   pevent-dn, pevent-chgtype);
  
   if (dns_name_dynamic(name))
   dns_name_free(name, inst-mctx);
 @@ -3400,7 +3400,7 @@ cleanup:
   if (prevdn_ldap != NULL)
   ldap_memfree(prevdn);
  
 - log_error(psearch_update failed for %s zone. 
 + log_error_r(psearch_update failed for '%s' zone. 
 Zone can be outdated, run `rndc reload`,
 entry-dn);
   }
 @@ -3586,7 +3586,7 @@ restart:
* Error means inconsistency of our zones
* data.
*/
 - log_error(ldap_psearch_watcher failed, zones 
 + log_error_r(ldap_psearch_watcher failed, zones 
 
 might be outdated. Run `rndc 
 reload`);
   goto soft_err;
   }
 -- 
 1.7.11.4
 


-- 
Adam Tkac, Red Hat, Inc.

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

Re: [Freeipa-devel] [PATCH 0059] Fix potential crash after free(uninitialized variable)

On Wed, Sep 12, 2012 at 01:07:56PM +0200, Petr Spacek wrote:
 Hello,
 
 This patch fixes potential crash after free(uninitialized variable)
 in persistent search code.
 
 Coverity CID 13088.

Ack

 From 3197b4ace3e852495bf4f9fdc32192459160027c Mon Sep 17 00:00:00 2001
 From: Petr Spacek pspa...@redhat.com
 Date: Wed, 12 Sep 2012 13:04:39 +0200
 Subject: [PATCH] Fix potential crash after free(uninitialized variable) in
  persistent search code.
 
 Coverity CID 13088.
 
 Signed-off-by: Petr Spacek pspa...@redhat.com
 ---
  src/ldap_helper.c | 15 +++
  1 file changed, 7 insertions(+), 8 deletions(-)
 
 diff --git a/src/ldap_helper.c b/src/ldap_helper.c
 index 
 92edbe7159272772e1c993d46da7c93382cbc5d4..67a64b79159983c83cb1bfc73c4b02a9bce986a7
  100644
 --- a/src/ldap_helper.c
 +++ b/src/ldap_helper.c
 @@ -2878,8 +2878,8 @@ cleanup:
  static isc_result_t
  ldap_pscontrol_create(LDAPControl **ctrlp)
  {
 - BerElement *ber;
 - struct berval *berval;
 + BerElement *ber = NULL;
 + struct berval *berval = NULL;
   isc_result_t result = ISC_R_FAILURE;
  
   REQUIRE(ctrlp != NULL  *ctrlp == NULL);
 @@ -2905,14 +2905,13 @@ ldap_pscontrol_create(LDAPControl **ctrlp)
   != LDAP_SUCCESS)
   goto cleanup;
  
 - ber_free(ber, 1);
 - ber_bvfree(berval);
 -
 - return ISC_R_SUCCESS;
 + result = ISC_R_SUCCESS;
  
  cleanup:
 - ber_free(ber, 1);
 - ber_bvfree(berval);
 + if (ber != NULL)
 + ber_free(ber, 1);
 + if (berval != NULL)
 + ber_bvfree(berval);
  
   return result;
  }
 -- 
 1.7.11.4
 


-- 
Adam Tkac, Red Hat, Inc.

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

Re: [Freeipa-devel] [PATCH 0057] Fix LDAP operation selection logic in ldap_modify_do()


On 09/14/2012 03:23 PM, Adam Tkac wrote:

On Wed, Sep 12, 2012 at 12:35:25PM +0200, Petr Spacek wrote:

Hello,

 There is a fix for LDAP operation selection logic in ldap_modify_do().

 Each operation code in LDAPMod structure can be ORed
 with LDAP_MOD_BVALUES.


Ack


Pushed to master:
https://fedorahosted.org/bind-dyndb-ldap/changeset/ff9f644ec02b8900cb3bdaed91023f6a10983e28

Petr^2 Spacek

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

Re: [Freeipa-devel] [PATCH 0058] Improve persistent search logging


On 09/14/2012 03:24 PM, Adam Tkac wrote:

On Wed, Sep 12, 2012 at 12:36:38PM +0200, Petr Spacek wrote:

Hello,

this patch adds result codes to error messages in persistent search code.


Ack.


Pushed to master:
https://fedorahosted.org/bind-dyndb-ldap/changeset/30e3cad6267d4b57330b091001623a897478d73b

Petr^2 Spacek

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

Re: [Freeipa-devel] [PATCH 0059] Fix potential crash after free(uninitialized variable)


On 09/14/2012 03:34 PM, Adam Tkac wrote:

On Wed, Sep 12, 2012 at 01:07:56PM +0200, Petr Spacek wrote:

Hello,

This patch fixes potential crash after free(uninitialized variable)
in persistent search code.

Coverity CID 13088.


Ack


Pushed to master:

https://fedorahosted.org/bind-dyndb-ldap/changeset/58fdb803e8cfd4ead174c8107ff7854a2be15b40

Petr^2 Spacek

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

Re: [Freeipa-devel] [PATCH] 0081 Only stop the main DS instance when upgrading it


On 09/14/2012 03:12 PM, Simo Sorce wrote:

On Fri, 2012-09-14 at 14:53 +0200, Petr Viktorin wrote:

This fixes a 2.2→3.0 upgrade bug found while testing the Dogtag 10 work.
See commit or ticket for details.

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



I also suspect that waiting for ports is not a good way to check if the
CMS is fully initialized, but I don't know of a better way. If you know
one, please speak up.



Won;t we get back to square zero when the work to merge DS and CS
instances into one will be completed ?

Simo.



When they're merged we'll probably need to bring down the CA while 
upgrading the server. Or just stop all the IPA services to be on the 
safe side, and of course bring them back up afterwards.

Meanwhile, this patch shouldn't hurt things.

--
Petr³

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

[Freeipa-devel] [PATCH] ipasam: Fixes build with samba4 rc1

2012-09-14 Thread Sumit Bose

Hi,

in samba4 rc1 there is an API change which we have to adopt in ipasam.
This patch updates ipasam and unbreaks the build with samba4 rc1.

bye,
Sumit
From 4e39eb306da08b29f694b9ff44ccb53865e33d92 Mon Sep 17 00:00:00 2001
From: Sumit Bose sb...@redhat.com
Date: Fri, 14 Sep 2012 14:14:23 +0200
Subject: [PATCH] ipasam: Fixes build with samba4 rc1

---
 daemons/ipa-sam/ipa_sam.c | 20 ++--
 freeipa.spec.in   |  5 -
 2 Dateien geändert, 14 Zeilen hinzugefügt(+), 11 Zeilen entfernt(-)

diff --git a/daemons/ipa-sam/ipa_sam.c b/daemons/ipa-sam/ipa_sam.c
index 
b3c336443d28d6850a283a373351043b2460eeaa..3d4f741c157fb624e272d800bd9e0cdf9edb9444
 100644
--- a/daemons/ipa-sam/ipa_sam.c
+++ b/daemons/ipa-sam/ipa_sam.c
@@ -476,7 +476,7 @@ static NTSTATUS ldapsam_lookup_rids(struct pdb_methods 
*methods,
ldap_state-ipasam_privates-base_dn,
LDAP_SCOPE_SUBTREE, filter, ldap_attrs, 0,
msg);
-   talloc_autofree_ldapmsg(mem_ctx, msg);
+   smbldap_talloc_autofree_ldapmsg(mem_ctx, msg);
}
 
if (rc != LDAP_SUCCESS)
@@ -546,7 +546,7 @@ static NTSTATUS ldapsam_lookup_rids(struct pdb_methods 
*methods,
ldap_state-ipasam_privates-base_dn,
LDAP_SCOPE_SUBTREE, filter, ldap_attrs, 0,
msg);
-   talloc_autofree_ldapmsg(mem_ctx, msg);
+   smbldap_talloc_autofree_ldapmsg(mem_ctx, msg);
}
 
if (rc != LDAP_SUCCESS)
@@ -665,7 +665,7 @@ static bool ldapsam_sid_to_id(struct pdb_methods *methods,
if (rc != LDAP_SUCCESS) {
goto done;
}
-   talloc_autofree_ldapmsg(mem_ctx, result);
+   smbldap_talloc_autofree_ldapmsg(mem_ctx, result);
 
if (ldap_count_entries(priv2ld(priv), result) != 1) {
DEBUG(10, (Got %d entries, expected one\n,
@@ -755,7 +755,7 @@ static bool ldapsam_uid_to_sid(struct pdb_methods *methods, 
uid_t uid,
if (rc != LDAP_SUCCESS) {
goto done;
}
-   talloc_autofree_ldapmsg(tmp_ctx, result);
+   smbldap_talloc_autofree_ldapmsg(tmp_ctx, result);
 
if (ldap_count_entries(priv2ld(priv), result) != 1) {
DEBUG(3, (ERROR: Got %d entries for uid %u, expected one\n,
@@ -822,7 +822,7 @@ static bool ldapsam_gid_to_sid(struct pdb_methods *methods, 
gid_t gid,
if (rc != LDAP_SUCCESS) {
goto done;
}
-   talloc_autofree_ldapmsg(tmp_ctx, result);
+   smbldap_talloc_autofree_ldapmsg(tmp_ctx, result);
 
if (ldap_count_entries(priv2ld(priv), result) != 1) {
DEBUG(3, (ERROR: Got %d entries for gid %u, expected one\n,
@@ -1482,7 +1482,7 @@ static bool search_krb_princ(struct ldapsam_privates 
*ldap_state,
LDAP_SCOPE_SUBTREE, filter, NULL, 0, result);
 
if (result != NULL) {
-   talloc_autofree_ldapmsg(mem_ctx, result);
+   smbldap_talloc_autofree_ldapmsg(mem_ctx, result);
}
 
if (rc == LDAP_NO_SUCH_OBJECT) {
@@ -1782,7 +1782,7 @@ static bool get_trusted_domain_int(struct 
ldapsam_privates *ldap_state,
LDAP_SCOPE_SUBTREE, filter, NULL, 0, result);
 
if (result != NULL) {
-   talloc_autofree_ldapmsg(mem_ctx, result);
+   smbldap_talloc_autofree_ldapmsg(mem_ctx, result);
}
 
if (rc == LDAP_NO_SUCH_OBJECT) {
@@ -2275,7 +2275,7 @@ static NTSTATUS ipasam_set_trusted_domain(struct 
pdb_methods *methods,
  td-trust_forest_trust_info);
}
 
-   talloc_autofree_ldapmod(tmp_ctx, mods);
+   smbldap_talloc_autofree_ldapmod(tmp_ctx, mods);
 
trusted_dn = trusted_domain_dn(tmp_ctx, ldap_state, domain);
if (trusted_dn == NULL) {
@@ -2405,7 +2405,7 @@ static NTSTATUS ipasam_enum_trusted_domains(struct 
pdb_methods *methods,
TALLOC_FREE(filter);
 
if (result != NULL) {
-   talloc_autofree_ldapmsg(mem_ctx, result);
+   smbldap_talloc_autofree_ldapmsg(mem_ctx, result);
}
 
if (rc == LDAP_NO_SUCH_OBJECT) {
@@ -2668,7 +2668,7 @@ static bool ipasam_nthash_regen(struct ldapsam_privates 
*ldap_state,
int ret;
 
smbldap_set_mod(mods, LDAP_MOD_ADD, LDAP_ATTRIBUTE_NTHASH, 
MagicRegen);
-   talloc_autofree_ldapmod(mem_ctx, mods);
+   smbldap_talloc_autofree_ldapmod(mem_ctx, mods);
 
ret = smbldap_modify(ldap_state-smbldap_state, entry_dn, mods);
if (ret != LDAP_SUCCESS) {
diff --git a/freeipa.spec.in b/freeipa.spec.in
index 
dee262bbe0c5298b4a15b452214105345712e9d9..ed487f0d7124d517a2e53d4467e2d31c62cf169c
 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -31,7 +31,7 @@ BuildRequires:  policycoreutils = %{POLICYCOREUTILSVER}
 %if 0%{?fedora} = 16

Re: [Freeipa-devel] [PATCH] ipasam: Fixes build with samba4 rc1

2012-09-14 Thread Simo Sorce

On Fri, 2012-09-14 at 16:43 +0200, Sumit Bose wrote:
 Hi,
 
 in samba4 rc1 there is an API change which we have to adopt in ipasam.
 This patch updates ipasam and unbreaks the build with samba4 rc1.

Ack.

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] ipasam: Fixes build with samba4 rc1

On 09/14/2012 04:49 PM, Simo Sorce wrote:
 On Fri, 2012-09-14 at 16:43 +0200, Sumit Bose wrote:
 Hi,

 in samba4 rc1 there is an API change which we have to adopt in ipasam.
 This patch updates ipasam and unbreaks the build with samba4 rc1.
 
 Ack.
 
 Simo.
 

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] 0081 Only stop the main DS instance when upgrading it


On 09/14/2012 04:12 PM, Petr Viktorin wrote:

On 09/14/2012 03:12 PM, Simo Sorce wrote:

On Fri, 2012-09-14 at 14:53 +0200, Petr Viktorin wrote:

This fixes a 2.2→3.0 upgrade bug found while testing the Dogtag 10 work.
See commit or ticket for details.

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



I also suspect that waiting for ports is not a good way to check if the
CMS is fully initialized, but I don't know of a better way. If you know
one, please speak up.



Such a better way is coming with 
https://fedorahosted.org/pki/ticket/314. I opened 
https://fedorahosted.org/freeipa/ticket/3084 so we won't forget to take 
advantage of it.



Won;t we get back to square zero when the work to merge DS and CS
instances into one will be completed ?

Simo.



When they're merged we'll probably need to bring down the CA while
upgrading the server. Or just stop all the IPA services to be on the
safe side, and of course bring them back up afterwards.
Meanwhile, this patch shouldn't hurt things.




--
Petr³

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

Re: [Freeipa-devel] [PATCH] 1056 sudorule cn uniqueness

2012-09-14 Thread Rob Crittenden


Rob Crittenden wrote:

A sudorule dn uses ipaUniqueId as the cn so we have to do a search to
ensure uniqueness. This leaves us vulnerable to a race. Configure the
uniqueness plugin to ensure no dups.

rob


Add missing attribute to the fresh install ldif. I had already fixed 
this in the update, missed it on fresh install.


rob

From 2783dc65b80d1de1e69873f4b6efef45b470f3fd Mon Sep 17 00:00:00 2001
From: Rob Crittenden rcrit...@redhat.com
Date: Thu, 13 Sep 2012 15:11:57 -0400
Subject: [PATCH] Add uniqueness plugin configuration for sudorule cn

We do a search looking for duplicate values but this leaves open the
possibility that two adds are happening at the same time so both
searches return NotFound therefore we get two entries with the same
cn value.

https://fedorahosted.org/freeipa/ticket/3017
---
 install/share/unique-attributes.ldif | 18 ++
 install/updates/10-uniqueness.update | 17 +
 install/updates/Makefile.am  |  1 +
 3 files changed, 36 insertions(+)
 create mode 100644 install/updates/10-uniqueness.update

diff --git a/install/share/unique-attributes.ldif b/install/share/unique-attributes.ldif
index 4537e7468ad69891565ccd51f7b67e9db8889857..0e680a0e45b455469f9be9555aed1e63f1d97faf 100644
--- a/install/share/unique-attributes.ldif
+++ b/install/share/unique-attributes.ldif
@@ -70,6 +70,24 @@ nsslapd-pluginVersion: 1.1.0
 nsslapd-pluginVendor: Fedora Project
 nsslapd-pluginDescription: Enforce unique attribute values
 
+dn: cn=sudorule name uniqueness,cn=plugins,cn=config
+changetype: add
+objectClass: top
+objectClass: nsSlapdPlugin
+objectClass: extensibleObject
+cn: sudorule name uniqueness
+nsslapd-pluginDescription: Enforce unique attribute values
+nsslapd-pluginPath: libattr-unique-plugin
+nsslapd-pluginInitfunc: NSUniqueAttr_Init
+nsslapd-pluginType: preoperation
+nsslapd-pluginEnabled: on
+nsslapd-pluginarg0: cn
+nsslapd-pluginarg1: cn=sudorules,cn=sudo,$SUFFIX
+nsslapd-plugin-depends-on-type: database
+nsslapd-pluginId: NSUniqueAttr
+nsslapd-pluginVersion: 1.1.0
+nsslapd-pluginVendor: Fedora Project
+
 #dn: cn=uid uniqueness,cn=plugins,cn=config
 #objectClass: top
 #objectClass: nsSlapdPlugin
diff --git a/install/updates/10-uniqueness.update b/install/updates/10-uniqueness.update
new file mode 100644
index ..33bd2fc09e12f52200de83b245b89e26ebf8af8e
--- /dev/null
+++ b/install/updates/10-uniqueness.update
@@ -0,0 +1,17 @@
+dn: cn=sudorule name uniqueness,cn=plugins,cn=config
+default:objectClass: top
+default:objectClass: nsSlapdPlugin
+default:objectClass: extensibleObject
+default:cn: sudorule name uniqueness
+default:nsslapd-pluginDescription: Enforce unique attribute values
+default:nsslapd-pluginPath: libattr-unique-plugin
+default:nsslapd-pluginInitfunc: NSUniqueAttr_Init
+default:nsslapd-pluginType: preoperation
+default:nsslapd-pluginEnabled: on
+default:nsslapd-pluginarg0: cn
+default:nsslapd-pluginarg1: cn=sudorules,cn=sudo,$SUFFIX
+default:nsslapd-plugin-depends-on-type: database
+default:nsslapd-pluginId: NSUniqueAttr
+default:nsslapd-pluginVersion: 1.1.0
+default:nsslapd-pluginVendor: Fedora Project
+
diff --git a/install/updates/Makefile.am b/install/updates/Makefile.am
index 9e068966530d897fe18802c9dfa13406aeb3b010..54e57ef3e441e5f2f4ce0a6af97d6856506df8f8 100644
--- a/install/updates/Makefile.am
+++ b/install/updates/Makefile.am
@@ -11,6 +11,7 @@ app_DATA =\
 	10-sudo.update			\
 	10-ssh.update			\
 	10-bind-schema.update		\
+	10-uniqueness.update		\
 	19-managed-entries.update	\
 	20-aci.update			\
 	20-dna.update			\
-- 
1.7.11.4

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

Re: [Freeipa-devel] [PATCH] 0081 Only stop the main DS instance when upgrading it