Hello,

Attached patch set should fix the deadlock you discovered + few more issues I
spotted when testing the original patch.

Known problems (more patches needed):
- /etc/opendnssec/zonelist.xml should be restored during server uninstall
- ccache for ipa-ods-exporter should be removed during server uninstall
- timestamps in .private key files do not reflect (?) current key state in
OpenDNSSEC database (I will look into this tomorrow.)

-- 
Petr^2 Spacek
From e8f1b8f2f4342087a654feb6e9fde58cc8607aba Mon Sep 17 00:00:00 2001
From: Petr Spacek <pspa...@redhat.com>
Date: Mon, 31 Aug 2015 17:58:07 +0200
Subject: [PATCH] DNSSEC: prevent ipa-ods-exporter from looping after service
 auto-restart

It might happen that systemd will restart the service even if there is
no incomming connection to service socket. In that case we want to exit
because HSM synchronization is done before socket.accept() and we want
to synchronize HSM and DNS zones at the same time.
---
 daemons/dnssec/ipa-ods-exporter | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/daemons/dnssec/ipa-ods-exporter b/daemons/dnssec/ipa-ods-exporter
index e7d30014e7d0af00bc645b1f78ad0544752042c9..3036e8e4a9896781037888c86cd9fa4282eaa390 100755
--- a/daemons/dnssec/ipa-ods-exporter
+++ b/daemons/dnssec/ipa-ods-exporter
@@ -25,6 +25,7 @@ import logging
 import os
 import subprocess
 import socket
+import select
 import sys
 import systemd.daemon
 import systemd.journal
@@ -346,7 +347,12 @@ def receive_systemd_command(log):
         raise KeyError('Exactly one socket is expected.')
 
     sck = socket.fromfd(fds[0], socket.AF_UNIX, socket.SOCK_STREAM)
+    rlist, wlist, xlist = select.select([sck], [], [], 0)
+    if not rlist:
+        log.critical('socket activation did not return socket with a command')
+        sys.exit(0)
 
+    log.debug('accepting new connection')
     conn, addr = sck.accept()
     log.debug('accepted new connection %s', repr(conn))
 
-- 
2.4.3

From f2713aff15c5bdc04c646a3d3b036f51b1a3c106 Mon Sep 17 00:00:00 2001
From: Petr Spacek <pspa...@redhat.com>
Date: Mon, 31 Aug 2015 18:01:12 +0200
Subject: [PATCH] DNSSEC: Fix deadlock in ipa-ods-exporter <-> ods-enforcerd
 interaction

https://fedorahosted.org/freeipa/ticket/5273
---
 daemons/dnssec/ipa-ods-exporter | 39 +++++++++++++++++++++++++++++++--------
 1 file changed, 31 insertions(+), 8 deletions(-)

diff --git a/daemons/dnssec/ipa-ods-exporter b/daemons/dnssec/ipa-ods-exporter
index 3036e8e4a9896781037888c86cd9fa4282eaa390..04188652b50a06cf45c77c62f0c0220f3c1eea2c 100755
--- a/daemons/dnssec/ipa-ods-exporter
+++ b/daemons/dnssec/ipa-ods-exporter
@@ -368,12 +368,12 @@ def parse_command(cmd):
     """
     if cmd == 'ipa-hsm-update':
         return (0,
-                'HSM synchronization finished, exiting.',
+                'HSM synchronization finished, skipping zone synchronization.',
                 None)
 
     elif cmd == 'ipa-full-update':
         return (None,
-                'Synchronization of all zones requested.',
+                'Synchronization of all zones was finished.',
                 None)
 
     elif not cmd.startswith('update '):
@@ -386,7 +386,7 @@ def parse_command(cmd):
     else:
         zone_name = cmd2ods_zone_name(cmd)
         return (None,
-                'Update request for zone "%s" queued.\n' % zone_name,
+                'Zone was "%s" updated.\n' % zone_name,
                 zone_name)
 
 def send_systemd_reply(conn, reply):
@@ -541,18 +541,29 @@ except KeyError as e:
 
 exitcode, msg, zone_name = parse_command(cmd)
 
-if conn:
-    send_systemd_reply(conn, msg)
 if exitcode is not None:
+    if conn:
+        send_systemd_reply(conn, msg)
     log.info(msg)
     sys.exit(exitcode)
 else:
     log.debug(msg)
 
 # Open DB directly and read key timestamps etc.
-with ods_db_lock():
-    db = sqlite3.connect(paths.OPENDNSSEC_KASP_DB,
-            isolation_level="EXCLUSIVE")
+db = None
+try:
+    # LOCK WARNING:
+    # ods-enforcerd is holding kasp.db.our_lock when processing all zones and
+    # the lock is unlocked only after all calls to ods-signer are finished,
+    # i.e. when ods-enforcerd receives reply from each ods-signer call.
+    #
+    # Consequently, ipa-ods-exporter (ods-signerd implementation) must not
+    # request kasp.db.our_lock to prevent deadlocks.
+    # SQLite transaction isolation should suffice.
+    # Beware: Reply can be sent back only after DB is unlocked and closed
+    #         otherwise ods-enforcerd will fail.
+
+    db = sqlite3.connect(paths.OPENDNSSEC_KASP_DB)
     db.row_factory = sqlite3.Row
     db.execute('BEGIN')
 
@@ -564,4 +575,16 @@ with ods_db_lock():
         for zone_row in db.execute("SELECT name FROM zones"):
             sync_zone(log, ldap, dns_dn, zone_row['name'])
 
+except Exception as ex:
+    msg = "ipa-ods-exporter exception: %s" % ex
+    raise ex
+
+finally:
+    try:
+        if db:
+            db.close()
+    finally:
+        if conn:
+            send_systemd_reply(conn, msg)
+
 log.debug('Done')
-- 
2.4.3

From 97838d3316adcd6c2c658308bc79ac9a503db4c5 Mon Sep 17 00:00:00 2001
From: Petr Spacek <pspa...@redhat.com>
Date: Mon, 31 Aug 2015 18:03:33 +0200
Subject: [PATCH] DNSSEC: Fix HSM synchronization in ipa-dnskeysyncd when
 running on DNSSEC key master

---
 ipapython/dnssec/keysyncer.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ipapython/dnssec/keysyncer.py b/ipapython/dnssec/keysyncer.py
index d1f400e2ea7819fd9791c51a191ab75ca2a19bec..426dd940a8a613319f33cbd11710779428f7e51c 100644
--- a/ipapython/dnssec/keysyncer.py
+++ b/ipapython/dnssec/keysyncer.py
@@ -177,4 +177,4 @@ class KeySyncer(SyncReplConsumer):
             return
         if not self.init_done:
             return
-        ipautil.run([paths.ODS_SIGNER])
+        ipautil.run([paths.ODS_SIGNER, 'ipa-hsm-update'])
-- 
2.4.3

From 8e612096975d67034d178760a230e6daed8165b5 Mon Sep 17 00:00:00 2001
From: Petr Spacek <pspa...@redhat.com>
Date: Mon, 31 Aug 2015 18:40:50 +0200
Subject: [PATCH] DNSSEC: Fix key metadata export

Incorrect SQL join condition could lead to situation where metadata from
ZSK and KSK were interchanged.
---
 daemons/dnssec/ipa-ods-exporter | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/daemons/dnssec/ipa-ods-exporter b/daemons/dnssec/ipa-ods-exporter
index 04188652b50a06cf45c77c62f0c0220f3c1eea2c..23c9caa6c5501de8c2be1122ba3c634ccf575a4c 100755
--- a/daemons/dnssec/ipa-ods-exporter
+++ b/daemons/dnssec/ipa-ods-exporter
@@ -174,7 +174,7 @@ def get_ods_keys(zone_name):
 
     # get all keys for given zone ID
     cur = db.execute("SELECT kp.HSMkey_id, kp.generate, kp.algorithm, dnsk.publish, dnsk.active, dnsk.retire, dnsk.dead, dnsk.keytype "
-             "FROM keypairs AS kp JOIN dnsseckeys AS dnsk ON kp.id = dnsk.id "
+             "FROM keypairs AS kp JOIN dnsseckeys AS dnsk ON kp.id = dnsk.keypair_id "
              "WHERE dnsk.zone_id = ?", (zone_id,))
     keys = {}
     for row in cur:
-- 
2.4.3

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Reply via email to