On 22.12.2015 14:32, Petr Spacek wrote: > On 21.12.2015 18:56, Martin Basti wrote: >> >> >> On 21.12.2015 15:45, Martin Basti wrote: >>> >>> >>> On 21.12.2015 15:33, Petr Spacek wrote: >>>> Hello, >>>> >>>> this patch set fixes key rotation in DNSSEC. >>>> >>>> You can use attached template files for OpenDNSSEC config to shorten time >>>> intervals between key rotations. >>>> >>>> Please let me know if you have any questions, I'm all ears! >>>> >>> Please fix whitespace error: >>> >>> Applying: DNSSEC: logging improvements in ldapkeydb.py >>> /home/mbasti/work/freeipa-devel/.git/rebase-apply/patch:14: trailing >>> whitespace. >>> >>> warning: 1 line adds whitespace errors. >>> >> *) DNSSEC: ipa-dnskeysyncd: call ods-signer ldap-cleanup on zone removal >> >> Is is safe to do not use try - except with ipatuil.run()? What if ods-signer >> command failed? > > That is intentional. The call should never fail, so if it fails there is no > way how to recover cleanly except restarting the daemon. > > The unhandled exception will kill the daemon and systemd will restart it > later on. > > >> *) DNSSEC: Improve error reporting from ipa-ods-exporter >> IMO log.exception(ex) is enough, do we need to add traceback to msg? > > msg is sent over socket to another process (see send_systemd_reply(conn, msg) > call in finally: block). Without this the remote party would not receive the > error information. > > >> *) DNSSEC: Make sure that current state in OpenDNSSEC matches key state in >> LDAP >> I think this is okay because we want to use KSK instantly, but just to be >> sure, is Publish->Activate okay? >> + bind_times['idnsSecKeyActivate'] = >> ods_times['idnsSecKeyPublish'] >> >> Just to be sure how this will be handle during KSK key rotation? > > We have to copy semantics from OpenDNSSEC. Please see design page > https://fedorahosted.org/bind-dyndb-ldap/wiki/BIND9/Design/DNSSEC/OpenDNSSEC2BINDKeyStates > , it describes in detail why I done it this way. > > >> *) DNSSEC: Make sure that current key state in LDAP matches key state in BIND >> LGTM >> >> *) DNSSEC: remove obsolete TODO note >> ACK >> >> *) DNSSEC: add debug mode to ldapkeydb.py >> A) >> You can remove __str__ method, python will use __repr__ as default > > Done. > > >> B) >> for attr in ['ipaPrivateKey', 'ipaPublicKey', 'ipk11publickeyinfo']: >> Do we need to sanitize *public*Key and publicKeyinfo? > > Yes, we need it. The output with any of ['ipaPrivateKey', 'ipaPublicKey', > 'ipk11publickeyinfo'] is huge blob and printing it does not help readability. > Purpose of the patch is to make it easy to read and debug so printing useless > blobs would go directly against the purpose :-) > > >> C) >> in odsmgr.py is used ipa_log_manager, can we use the same for consistency? > > Fixed, thanks. > > >> D) >> Do we need logging there, everything is printed via print except debug info >> about connecting, can you just redirect it to stderr, and usable data leave >> in >> stdout? > > Yes, we need it because it eases debugging. print() prints useful information > to stdout. 'Garbage' about connecting to LDAP, IPA framework initialization > and so on does via logger to stderr, so it can be easily separated from useful > information using redirection in BASH. > > I've added a comment right below if __name__ == '__main__': to make it clear > why we do not use logger in there. > > >> *) DNSSEC: logging improvements in ldapkeydb.py >> IMO commit message should be: ".... in ipa-ods-exporter" >> >> Otherwise LGTM > > Fixed, thanks. > > >> *) DNSSEC: remove keys purged by OpenDNSSEC from master HSM from LDAP >> >> A) coding style: please use (), instead of "\" >> assert set(pubkeys_local) == set(privkeys_local), ( >> "IDs of private and public keys for DNS zones in local HSM does " >> "not match to key pairs: %s vs. %s" % >> (hex_set(pubkeys_local), hex_set(privkeys_local)) >> ) > > Fixed. > > >> B) coding style >> assert not matched_already, ( >> "key %s is in more than one keyset" % hexlify(keyid) >> ) > > Not relevant anymore, see below. > > >> C) schedule_key_deletion() >> how about case when keyid is not in any keyset, then keyid will not be >> replaced by object and it blow up somewhere else > > Not relevant anymore, see below. > > >> D) +class KeyDeleter(object): >> I would like to have a check there which blows up nicely if _update_key() is >> called twice on the same object. With current implementation you will get >> NoneType has no delete_entry method. > > Not relevant anymore, see below. > > >> E) >> I somehow does not like the placeholder object. Could we just extend Key >> object with attribute "to_be_deleted" or something similar, and if this >> attribute is set to True, Key._update_key() can remove, instead of creation a >> new object. >> Key.prepare_deletion() can set the value "to_be_deleted" to True. > > Main purpose of the KeyDeleter object was to be incompatible the Key object. I > want to be 100 % that is not possible to call schedule_delete() and > subsequenty modify the Key object. > > I've reworked the Key object so it has schedule_deletion() method and that all > other methods call __assert_not_deleted() to make sure that the object was not > deleted. > > Is it better? > > >> *) DNSSEC: ipa-dnskeysyncd: Skip zones with old DNSSEC metadata in LDAP >> How often is keysyncer initialized? Might happen the case where one of >> dnssec_zones has been disabled and keysyncer is not aware of this change? > > KeySyncer is SyncReplConsumer, so it gets constant stream of updates from > LDAP. IMHO the answer is no, it cannot miss the update. > > >> You may want to use DNSName from ipapython.dnsutil instead of pure dns.name > > Other places in daemons are using dns.name too, so I will keep it that way. > For this particular case there would be no advantage anyway. > > >> *) DNSSEC: ipa-ods-exporter: add ldap-cleanup command >> LGTM > > Old and new version of patches can be found on Github: > * old: https://github.com/pspacek/freeipa/tree/dnssec_fixes > * new: https://github.com/pspacek/freeipa/tree/dnssec_fixes2 > > Fixed patched (+1 new) are attached.
Rebased patches are attached (and were also force-pushed to https://github.com/pspacek/freeipa/tree/dnssec_fixes2) -- Petr^2 Spacek
From 477cf2498a0321f28dc37151e2757db325525515 Mon Sep 17 00:00:00 2001 From: Petr Spacek <pspa...@redhat.com> Date: Thu, 26 Nov 2015 14:56:00 +0100 Subject: [PATCH] DNSSEC: Improve error reporting from ipa-ods-exporter https://fedorahosted.org/freeipa/ticket/5348 --- daemons/dnssec/ipa-ods-exporter | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/daemons/dnssec/ipa-ods-exporter b/daemons/dnssec/ipa-ods-exporter index bb7292c2e06607a74f17f26cdf37b67784dd8fc8..0f6022576b092dbb79fc8619b03f79b9d6e892e4 100755 --- a/daemons/dnssec/ipa-ods-exporter +++ b/daemons/dnssec/ipa-ods-exporter @@ -30,6 +30,7 @@ import sys import systemd.daemon import systemd.journal import sqlite3 +import traceback import ipalib from ipapython.dn import DN @@ -571,7 +572,8 @@ try: sync_zone(log, ldap, dns_dn, zone_row['name']) except Exception as ex: - msg = "ipa-ods-exporter exception: %s" % ex + msg = "ipa-ods-exporter exception: %s" % traceback.format_exc(ex) + log.exception(ex) raise ex finally: -- 2.5.0
From 5174a9aba3ffe12fa3f7718c3332bd7b80da0799 Mon Sep 17 00:00:00 2001 From: Petr Spacek <pspa...@redhat.com> Date: Tue, 24 Nov 2015 12:49:40 +0100 Subject: [PATCH] DNSSEC: Make sure that current state in OpenDNSSEC matches key state in LDAP Previously we published timestamps of planned state changes in LDAP. This led to situations where state transition in OpenDNSSEC was blocked by an additional condition (or unavailability of OpenDNSSEC) but BIND actually did the transition as planned. Additionally key state mapping was incorrect for KSK so sometimes KSK was not used for signing when it should. Example (for code without this fix): - Add a zone and let OpenDNSSEC to generate keys. - Wait until keys are in state "published" and next state is "inactive". - Shutdown OpenDNSSEC or break replication from DNSSEC key master. - See that keys on DNS replicas will transition to state "inactive" even though it should not happen because OpenDNSSEC is not available (i.e. new keys may not be available). - End result is that affected zone will not be signed anymore, even though it should stay signed with the old keys. https://fedorahosted.org/freeipa/ticket/5348 --- daemons/dnssec/ipa-ods-exporter | 105 ++++++++++++++++++++++++++++++++++++---- 1 file changed, 95 insertions(+), 10 deletions(-) diff --git a/daemons/dnssec/ipa-ods-exporter b/daemons/dnssec/ipa-ods-exporter index 0f6022576b092dbb79fc8619b03f79b9d6e892e4..9d4e237955a117a9a7149937c9c9add552dea1cb 100755 --- a/daemons/dnssec/ipa-ods-exporter +++ b/daemons/dnssec/ipa-ods-exporter @@ -53,6 +53,14 @@ ODS_DB_LOCK_PATH = "%s%s" % (paths.OPENDNSSEC_KASP_DB, '.our_lock') SECRETKEY_WRAPPING_MECH = 'rsaPkcsOaep' PRIVKEY_WRAPPING_MECH = 'aesKeyWrapPad' +# Constants from OpenDNSSEC's enforcer/ksm/include/ksm/ksm.h +KSM_STATE_PUBLISH = 2 +KSM_STATE_READY = 3 +KSM_STATE_ACTIVE = 4 +KSM_STATE_RETIRE = 5 +KSM_STATE_DEAD = 6 +KSM_STATE_KEYPUBLISH = 10 + # DNSKEY flag constants dnskey_flag_by_value = { 0x0001: 'SEP', @@ -118,6 +126,77 @@ def sql2ldap_keyid(sql_keyid): #uri += '%'.join(sql_keyid[i:i+2] for i in range(0, len(sql_keyid), 2)) return {"idnsSecKeyRef": uri} +def ods2bind_timestamps(key_state, key_type, ods_times): + """Transform (timestamps and key states) from ODS to set of BIND timestamps + with equivalent meaning. At the same time, remove timestamps + for future/planned state transitions to prevent ODS & BIND + from desynchronizing. + + OpenDNSSEC database may contain timestamps for state transitions planned + in the future, but timestamp itself is not sufficient information because + there could be some additional condition which is guaded by OpenDNSSEC + itself. + + BIND works directly with timestamps without any additional conditions. + This difference causes problem when state transition planned in OpenDNSSEC + does not happen as originally planned for some reason. + + At the same time, this difference causes problem when OpenDNSSEC on DNSSEC + key master and BIND instances on replicas are not synchronized. This + happens when DNSSEC key master is down, or a replication is down. Even + a temporary desynchronization could cause DNSSEC validation failures + which could have huge impact. + + To prevent this problem, this function removes all timestamps corresponding + to future state transitions. As a result, BIND will not do state transition + until it happens in OpenDNSSEC first and until the change is replicated. + + Also, timestamp mapping depends on key type and is not 1:1. + For detailed description of the mapping please see + https://fedorahosted.org/bind-dyndb-ldap/wiki/BIND9/Design/DNSSEC/OpenDNSSEC2BINDKeyStates + """ + bind_times = {} + # idnsSecKeyCreated is equivalent to SQL column 'created' + bind_times['idnsSecKeyCreated'] = ods_times['idnsSecKeyCreated'] + + # set of key states where publishing in DNS zone is desired is taken from + # opendnssec/enforcer/ksm/ksm_request.c:KsmRequestIssueKeys() + # TODO: support for RFC 5011, requires OpenDNSSEC v1.4.8+ + if ('idnsSecKeyPublish' in ods_times and + key_state in {KSM_STATE_PUBLISH, KSM_STATE_READY, KSM_STATE_ACTIVE, + KSM_STATE_RETIRE, KSM_STATE_KEYPUBLISH}): + bind_times['idnsSecKeyPublish'] = ods_times['idnsSecKeyPublish'] + + # ZSK and KSK handling differs in enforcerd, see + # opendnssec/enforcer/enforcerd/enforcer.c:commKeyConfig() + if key_type == 'ZSK': + # idnsSecKeyActivate cannot be set before the key reaches ACTIVE state + if ('idnsSecKeyActivate' in ods_times and + key_state in {KSM_STATE_ACTIVE, KSM_STATE_RETIRE, KSM_STATE_DEAD}): + bind_times['idnsSecKeyActivate'] = ods_times['idnsSecKeyActivate'] + + # idnsSecKeyInactive cannot be set before the key reaches RETIRE state + if ('idnsSecKeyInactive' in ods_times and + key_state in {KSM_STATE_RETIRE, KSM_STATE_DEAD}): + bind_times['idnsSecKeyInactive'] = ods_times['idnsSecKeyInactive'] + + elif key_type == 'KSK': + # KSK is special: it is used for signing as long as it is in zone + if ('idnsSecKeyPublish' in ods_times and + key_state in {KSM_STATE_PUBLISH, KSM_STATE_READY, KSM_STATE_ACTIVE, + KSM_STATE_RETIRE, KSM_STATE_KEYPUBLISH}): + bind_times['idnsSecKeyActivate'] = ods_times['idnsSecKeyPublish'] + # idnsSecKeyInactive is ignored for KSK on purpose + + else: + assert False, "unsupported key type %s" % key_type + + # idnsSecKeyDelete is relevant only in DEAD state + if 'idnsSecKeyDelete' in ods_times and key_state == KSM_STATE_DEAD: + bind_times['idnsSecKeyDelete'] = ods_times['idnsSecKeyDelete'] + + return bind_times + class ods_db_lock(object): def __enter__(self): self.f = open(ODS_DB_LOCK_PATH, 'w') @@ -168,25 +247,31 @@ def get_ods_keys(zone_name): assert len(rows) == 1, "exactly one DNS zone should exist in ODS DB" zone_id = rows[0][0] - # 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.keypair_id " - "WHERE dnsk.zone_id = ?", (zone_id,)) + # get relevant keys for given zone ID: + # ignore keys which were generated but not used yet + # key state check is using constants from + # OpenDNSSEC's enforcer/ksm/include/ksm/ksm.h + # WARNING! OpenDNSSEC version 1 and 2 are using different constants! + cur = db.execute("SELECT kp.HSMkey_id, kp.generate, kp.algorithm, " + "dnsk.publish, dnsk.active, dnsk.retire, dnsk.dead, " + "dnsk.keytype, dnsk.state " + "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: - key_data = sql2datetimes(row) - if 'idnsSecKeyDelete' in key_data \ - and key_data['idnsSecKeyDelete'] > datetime.now(): - continue # ignore deleted keys - - key_data.update(sql2ldap_flags(row['keytype'])) + key_data = sql2ldap_flags(row['keytype']) assert key_data.get('idnsSecKeyZONE', None) == 'TRUE', \ 'unexpected key type 0x%x' % row['keytype'] if key_data.get('idnsSecKeySEP', 'FALSE') == 'TRUE': key_type = 'KSK' else: key_type = 'ZSK' + # transform key state to timestamps for BIND with equivalent semantics + ods_times = sql2datetimes(row) + key_data.update(ods2bind_timestamps(row['state'], key_type, ods_times)) + key_data.update(sql2ldap_algorithm(row['algorithm'])) key_id = "%s-%s-%s" % (key_type, datetime2ldap(key_data['idnsSecKeyCreated']), -- 2.5.0
From 59b296b15a57a1921aaff099af2b91b93fc96cff Mon Sep 17 00:00:00 2001 From: Petr Spacek <pspa...@redhat.com> Date: Thu, 26 Nov 2015 15:19:03 +0100 Subject: [PATCH] DNSSEC: Make sure that current key state in LDAP matches key state in BIND We have to explicitly specify "none" value to prevent dnssec-keyfromlabel utility from using current time for keys without "publish" and "activate" timestamps. Previously this lead to situation where key was in (intermediate) state "generated" in OpenDNSSEC but BIND started to use this key for signing. https://fedorahosted.org/freeipa/ticket/5348 --- ipapython/dnssec/bindmgr.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/ipapython/dnssec/bindmgr.py b/ipapython/dnssec/bindmgr.py index 6bf509d4fb0ef83350845dfa63d7dff8f8e27c27..e92afff97c98c55c35155c487abbd02ea2719276 100644 --- a/ipapython/dnssec/bindmgr.py +++ b/ipapython/dnssec/bindmgr.py @@ -55,17 +55,21 @@ class BINDMgr(object): return dt.strftime(time_bindfmt) def dates2params(self, ldap_attrs): + """Convert LDAP timestamps to list of parameters suitable + for dnssec-keyfromlabel utility""" attr2param = {'idnsseckeypublish': '-P', 'idnsseckeyactivate': '-A', 'idnsseckeyinactive': '-I', 'idnsseckeydelete': '-D'} params = [] for attr, param in attr2param.items(): + params.append(param) if attr in ldap_attrs: - params.append(param) assert len(ldap_attrs[attr]) == 1, 'Timestamp %s is expected to be single-valued' % attr params.append(self.time_ldap2bindfmt(ldap_attrs[attr][0])) + else: + params.append('none') return params -- 2.5.0
From c4f15aa5d656426eebf4e921aa43c7ed652b1d6f Mon Sep 17 00:00:00 2001 From: Petr Spacek <pspa...@redhat.com> Date: Wed, 2 Dec 2015 12:58:23 +0100 Subject: [PATCH] DNSSEC: remove obsolete TODO note https://fedorahosted.org/freeipa/ticket/5348 --- ipapython/dnssec/ldapkeydb.py | 1 - 1 file changed, 1 deletion(-) diff --git a/ipapython/dnssec/ldapkeydb.py b/ipapython/dnssec/ldapkeydb.py index 384bffee1adaeb0ab3b330f85e68d8b19d653367..7895832d5089bc578363b63fda74cd4498e608c8 100644 --- a/ipapython/dnssec/ldapkeydb.py +++ b/ipapython/dnssec/ldapkeydb.py @@ -180,7 +180,6 @@ class MasterKey(Key): # TODO: replace this with 'autogenerate' to prevent collisions uuid_rdn = DN('ipk11UniqueId=%s' % uuid.uuid1()) entry_dn = DN(uuid_rdn, self.ldapkeydb.base_dn) - # TODO: add ipaWrappingMech attribute entry = self.ldap.make_entry(entry_dn, objectClass=['ipaSecretKeyObject', 'ipk11Object'], ipaSecretKey=data, -- 2.5.0
From fe0dcbbe0b08412ae2643b5a91f013c3169b5990 Mon Sep 17 00:00:00 2001 From: Petr Spacek <pspa...@redhat.com> Date: Tue, 15 Dec 2015 14:13:23 +0100 Subject: [PATCH] DNSSEC: add debug mode to ldapkeydb.py ldapkeydb.py can be executed directly now. In that case it will print out key metadata as obtained using IPA LDAP API. Kerberos credential cache has to be filled with principal posessing appropriate access rights before the script is execured. https://fedorahosted.org/freeipa/ticket/5348 --- ipapython/dnssec/ldapkeydb.py | 54 +++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 52 insertions(+), 2 deletions(-) diff --git a/ipapython/dnssec/ldapkeydb.py b/ipapython/dnssec/ldapkeydb.py index 7895832d5089bc578363b63fda74cd4498e608c8..3f9fbcfa743fc30bc599e614dfb56e9d33b82889 100644 --- a/ipapython/dnssec/ldapkeydb.py +++ b/ipapython/dnssec/ldapkeydb.py @@ -4,9 +4,12 @@ from binascii import hexlify import collections +from pprint import pprint import ipalib from ipapython.dn import DN +from ipapython import ipaldap +from ipapython import ipa_log_manager from ipapython.dnssec.abshsm import ( attrs_name2id, @@ -134,8 +137,12 @@ class Key(collections.MutableMapping): def __len__(self): return len(self.entry) - def __str__(self): - return str(self.entry) + def __repr__(self): + sanitized = dict(self.entry) + for attr in ['ipaPrivateKey', 'ipaPublicKey', 'ipk11publickeyinfo']: + if attr in sanitized: + del sanitized[attr] + return repr(sanitized) def _cleanup_key(self): """remove default values from LDAP entry""" @@ -346,3 +353,46 @@ class LdapKeyDB(AbstractHSM): '(&(objectClass=ipk11PrivateKey)(objectClass=ipaPrivateKeyObject)(objectClass=ipk11PublicKey)(objectClass=ipaPublicKeyObject))')) return self.cache_zone_keypairs + +if __name__ == '__main__': + # this is debugging mode + # print information we think are useful to stdout + # other garbage goes via logger to stderr + ipa_log_manager.standard_logging_setup(debug=True) + log = ipa_log_manager.root_logger + + # IPA framework initialization + ipalib.api.bootstrap(in_server=True, log=None) # no logging to file + ipalib.api.finalize() + + # LDAP initialization + dns_dn = DN(ipalib.api.env.container_dns, ipalib.api.env.basedn) + ldap = ipaldap.LDAPClient(ipalib.api.env.ldap_uri) + log.debug('Connecting to LDAP') + # GSSAPI will be used, used has to be kinited already + ldap.gssapi_bind() + log.debug('Connected') + + ldapkeydb = LdapKeyDB(log, ldap, DN(('cn', 'keys'), ('cn', 'sec'), + ipalib.api.env.container_dns, + ipalib.api.env.basedn)) + + print('replica public keys: CKA_WRAP = TRUE') + print('====================================') + for pubkey_id, pubkey in ldapkeydb.replica_pubkeys_wrap.items(): + print(hexlify(pubkey_id)) + pprint(pubkey) + + print('') + print('master keys') + print('===========') + for mkey_id, mkey in ldapkeydb.master_keys.items(): + print(hexlify(mkey_id)) + pprint(mkey) + + print('') + print('zone key pairs') + print('==============') + for key_id, key in ldapkeydb.zone_keypairs.items(): + print(hexlify(key_id)) + pprint(key) -- 2.5.0
From 03339e8eb600ca9bb6418fb3658aed769ad57260 Mon Sep 17 00:00:00 2001 From: Petr Spacek <pspa...@redhat.com> Date: Tue, 15 Dec 2015 14:16:52 +0100 Subject: [PATCH] DNSSEC: logging improvements in ipa-ods-exporter https://fedorahosted.org/freeipa/ticket/5348 --- daemons/dnssec/ipa-ods-exporter | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/daemons/dnssec/ipa-ods-exporter b/daemons/dnssec/ipa-ods-exporter index 9d4e237955a117a9a7149937c9c9add552dea1cb..8cae73bb4d047633ab210196c642edf76a475f18 100755 --- a/daemons/dnssec/ipa-ods-exporter +++ b/daemons/dnssec/ipa-ods-exporter @@ -487,6 +487,11 @@ def cmd2ods_zone_name(cmd): return zone_name def sync_zone(log, ldap, dns_dn, zone_name): + """synchronize metadata about zone keys for single DNS zone + + Key material has to be synchronized elsewhere. + Keep in mind that keys could be shared among multiple zones!""" + log.getChild("%s.%s" % (__name__, zone_name)) log.debug('synchronizing zone "%s"', zone_name) ods_keys = get_ods_keys(zone_name) ods_keys_id = set(ods_keys.keys()) @@ -519,30 +524,30 @@ def sync_zone(log, ldap, dns_dn, zone_name): ldap_keys_id = set(ldap_keys.keys()) new_keys_id = ods_keys_id - ldap_keys_id - log.info('new keys from ODS: %s', new_keys_id) + log.info('new key metadata from ODS: %s', new_keys_id) for key_id in new_keys_id: cn = "cn=%s" % key_id key_dn = DN(cn, keys_dn) - log.debug('adding key "%s" to LDAP', key_dn) + log.debug('adding key metadata "%s" to LDAP', key_dn) ldap_key = ldap.make_entry(key_dn, objectClass=['idnsSecKey'], **ods_keys[key_id]) ldap.add_entry(ldap_key) deleted_keys_id = ldap_keys_id - ods_keys_id - log.info('deleted keys in LDAP: %s', deleted_keys_id) + log.info('deleted key metadata in LDAP: %s', deleted_keys_id) for key_id in deleted_keys_id: cn = "cn=%s" % key_id key_dn = DN(cn, keys_dn) - log.debug('deleting key "%s" from LDAP', key_dn) + log.debug('deleting key metadata "%s" from LDAP', key_dn) ldap.delete_entry(key_dn) update_keys_id = ldap_keys_id.intersection(ods_keys_id) - log.info('keys in LDAP & ODS: %s', update_keys_id) + log.info('key metadata in LDAP & ODS: %s', update_keys_id) for key_id in update_keys_id: ldap_key = ldap_keys[key_id] ods_key = ods_keys[key_id] - log.debug('updating key "%s" in LDAP', ldap_key.dn) + log.debug('updating key metadata "%s" in LDAP', ldap_key.dn) ldap_key.update(ods_key) try: ldap.update_entry(ldap_key) -- 2.5.0
From db1c887599e24ed5e0f3a9021d0c580bc1d61d34 Mon Sep 17 00:00:00 2001 From: Petr Spacek <pspa...@redhat.com> Date: Tue, 15 Dec 2015 15:22:45 +0100 Subject: [PATCH] DNSSEC: remove keys purged by OpenDNSSEC from master HSM from LDAP Key purging has to be only only after key metadata purging so ipa-dnskeysyncd on replices does not fail while dereferencing non-existing keys. https://fedorahosted.org/freeipa/ticket/5334 --- daemons/dnssec/ipa-ods-exporter | 45 ++++++++++++++++++++++---- ipapython/dnssec/ldapkeydb.py | 72 ++++++++++++++++++++++++++++++++++------- 2 files changed, 99 insertions(+), 18 deletions(-) diff --git a/daemons/dnssec/ipa-ods-exporter b/daemons/dnssec/ipa-ods-exporter index 8cae73bb4d047633ab210196c642edf76a475f18..def9ca39ec8462bf7ab3e4b9453fce9decbee665 100755 --- a/daemons/dnssec/ipa-ods-exporter +++ b/daemons/dnssec/ipa-ods-exporter @@ -383,19 +383,22 @@ def master2ldap_master_keys_sync(log, ldapkeydb, localhsm): ldapkeydb.flush() def master2ldap_zone_keys_sync(log, ldapkeydb, localhsm): - # synchroniza zone keys + """add and update zone key material from local HSM to LDAP + + No key material will be removed, only new keys will be added or updated. + Key removal is hanled by master2ldap_zone_keys_purge().""" log = log.getChild('master2ldap_zone_keys') keypairs_ldap = ldapkeydb.zone_keypairs log.debug("zone keys in LDAP: %s", hex_set(keypairs_ldap)) pubkeys_local = localhsm.zone_pubkeys privkeys_local = localhsm.zone_privkeys log.debug("zone keys in local HSM: %s", hex_set(privkeys_local)) - assert set(pubkeys_local) == set(privkeys_local), \ - "IDs of private and public keys for DNS zones in local HSM does " \ - "not match to key pairs: %s vs. %s" % \ - (hex_set(pubkeys_local), hex_set(privkeys_local)) + assert set(pubkeys_local) == set(privkeys_local), ( + "IDs of private and public keys for DNS zones in local HSM does " + "not match to key pairs: %s vs. %s" % + (hex_set(pubkeys_local), hex_set(privkeys_local))) new_keys = set(pubkeys_local) - set(keypairs_ldap) log.debug("new zone keys in local HSM: %s", hex_set(new_keys)) @@ -416,6 +419,29 @@ def master2ldap_zone_keys_sync(log, ldapkeydb, localhsm): sync_set_metadata_2ldap(log, privkeys_local, keypairs_ldap) ldapkeydb.flush() +def master2ldap_zone_keys_purge(log, ldapkeydb, localhsm): + """purge removed key material from LDAP (but not metadata) + + Keys which are present in LDAP but not in local HSM will be removed. + Key metadata must be removed first so references to removed key material + are removed before actually removing the keys.""" + keypairs_ldap = ldapkeydb.zone_keypairs + log.debug("zone keys in LDAP: %s", hex_set(keypairs_ldap)) + + pubkeys_local = localhsm.zone_pubkeys + privkeys_local = localhsm.zone_privkeys + log.debug("zone keys in local HSM: %s", hex_set(privkeys_local)) + assert set(pubkeys_local) == set(privkeys_local), \ + "IDs of private and public keys for DNS zones in local HSM does " \ + "not match to key pairs: %s vs. %s" % \ + (hex_set(pubkeys_local), hex_set(privkeys_local)) + + deleted_key_ids = set(keypairs_ldap) - set(pubkeys_local) + log.debug("zone keys deleted from local HSM but present in LDAP: %s", + hex_set(deleted_key_ids)) + for zkey_id in deleted_key_ids: + keypairs_ldap[zkey_id].schedule_deletion() + ldapkeydb.flush() def hex_set(s): out = set() @@ -595,7 +621,7 @@ ldap.gssapi_bind() log.debug('Connected') -### DNSSEC master: key synchronization +### DNSSEC master: key material upload & synchronization (but not deletion) ldapkeydb = LdapKeyDB(log, ldap, DN(('cn', 'keys'), ('cn', 'sec'), ipalib.api.env.container_dns, ipalib.api.env.basedn)) @@ -607,7 +633,7 @@ master2ldap_master_keys_sync(log, ldapkeydb, localhsm) master2ldap_zone_keys_sync(log, ldapkeydb, localhsm) -### DNSSEC master: DNSSEC key metadata upload +### DNSSEC master: DNSSEC key metadata upload & synchronization & deletion # command receive is delayed so the command will stay in socket queue until # the problem with LDAP server or HSM is fixed try: @@ -661,6 +687,11 @@ try: for zone_row in db.execute("SELECT name FROM zones"): sync_zone(log, ldap, dns_dn, zone_row['name']) + ### DNSSEC master: DNSSEC key material purging + # references to old key material were removed above in sync_zone() + # so now we can purge old key material from LDAP + master2ldap_zone_keys_purge(log, ldapkeydb, localhsm) + except Exception as ex: msg = "ipa-ods-exporter exception: %s" % traceback.format_exc(ex) log.exception(ex) diff --git a/ipapython/dnssec/ldapkeydb.py b/ipapython/dnssec/ldapkeydb.py index 3f9fbcfa743fc30bc599e614dfb56e9d33b82889..b0b259f0ecb995a0eb49eedc732152fdb73cbf30 100644 --- a/ipapython/dnssec/ldapkeydb.py +++ b/ipapython/dnssec/ldapkeydb.py @@ -104,40 +104,56 @@ def get_default_attrs(object_classes): result.update(defaults[cls]) return result + class Key(collections.MutableMapping): """abstraction to hide LDAP entry weirdnesses: - non-normalized attribute names - boolean attributes returned as strings + - planned entry deletion prevents subsequent use of the instance """ def __init__(self, entry, ldap, ldapkeydb): self.entry = entry + self._delentry = None # indicates that object was deleted self.ldap = ldap self.ldapkeydb = ldapkeydb self.log = ldap.log.getChild(__name__) + def __assert_not_deleted(self): + assert self.entry and not self._delentry, ( + "attempt to use to-be-deleted entry %s detected" + % self._delentry.dn) + def __getitem__(self, key): + self.__assert_not_deleted() val = self.entry.single_value[key] if key.lower() in bool_attr_names: val = ldap_bool(val) return val def __setitem__(self, key, value): + self.__assert_not_deleted() self.entry[key] = value def __delitem__(self, key): + self.__assert_not_deleted() del self.entry[key] def __iter__(self): """generates list of ipa names of all PKCS#11 attributes present in the object""" + self.__assert_not_deleted() for ipa_name in list(self.entry.keys()): lowercase = ipa_name.lower() if lowercase in attrs_name2id: yield lowercase def __len__(self): + self.__assert_not_deleted() return len(self.entry) def __repr__(self): + if self._delentry: + return 'deleted entry: %s' % repr(self._delentry) + sanitized = dict(self.entry) for attr in ['ipaPrivateKey', 'ipaPublicKey', 'ipk11publickeyinfo']: if attr in sanitized: @@ -152,6 +168,49 @@ class Key(collections.MutableMapping): if self.get(attr, empty) == default_attrs[attr]: del self[attr] + def _update_key(self): + """remove default values from LDAP entry and write back changes""" + if self._delentry: + self._delete_key() + return + + self._cleanup_key() + + try: + self.ldap.update_entry(self.entry) + except ipalib.errors.EmptyModlist: + pass + + def _delete_key(self): + """remove key metadata entry from LDAP + + After calling this, the python object is no longer valid and all + subsequent method calls on it will fail. + """ + assert not self.entry, ( + "Key._delete_key() called before Key.schedule_deletion()") + assert self._delentry, "Key._delete_key() called more than once" + self.log.debug('deleting key id 0x%s DN %s from LDAP', + hexlify(self._delentry.single_value['ipk11id']), + self._delentry.dn) + self.ldap.delete_entry(self._delentry) + self._delentry = None + self.ldap = None + self.ldapkeydb = None + + def schedule_deletion(self): + """schedule key deletion from LDAP + + Calling schedule_deletion() will make this object incompatible with + normal Key. After that the object must not be read or modified. + Key metadata will be actually deleted when LdapKeyDB.flush() is called. + """ + assert not self._delentry, ( + "Key.schedule_deletion() called more than once") + self._delentry = self.entry + self.entry = None + + class ReplicaKey(Key): # TODO: object class assert def __init__(self, entry, ldap, ldapkeydb): @@ -238,21 +297,12 @@ class LdapKeyDB(AbstractHSM): self._update_keys() return keys - def _update_key(self, key): - """remove default values from LDAP entry and write back changes""" - key._cleanup_key() - - try: - self.ldap.update_entry(key.entry) - except ipalib.errors.EmptyModlist: - pass - def _update_keys(self): for cache in [self.cache_masterkeys, self.cache_replica_pubkeys_wrap, - self.cache_zone_keypairs]: + self.cache_zone_keypairs]: if cache: for key in cache.values(): - self._update_key(key) + key._update_key() def flush(self): """write back content of caches to LDAP""" -- 2.5.0
From 6421534e282c62911bbd27d922bb972344d398ea Mon Sep 17 00:00:00 2001 From: Petr Spacek <pspa...@redhat.com> Date: Sun, 20 Dec 2015 18:36:48 +0100 Subject: [PATCH] DNSSEC: ipa-dnskeysyncd: Skip zones with old DNSSEC metadata in LDAP This filtering is useful in cases where LDAP contains DNS zones which have old metadata objects and DNSSEC disabled. Such zones must be ignored to prevent errors while calling dnssec-keyfromlabel or rndc. https://fedorahosted.org/freeipa/ticket/5348 --- ipapython/dnssec/bindmgr.py | 16 +++++++++++++--- ipapython/dnssec/keysyncer.py | 24 ++++++++++++++++++------ 2 files changed, 31 insertions(+), 9 deletions(-) diff --git a/ipapython/dnssec/bindmgr.py b/ipapython/dnssec/bindmgr.py index e92afff97c98c55c35155c487abbd02ea2719276..33d071f45519fd23f57a8e784941eb36692d7b9f 100644 --- a/ipapython/dnssec/bindmgr.py +++ b/ipapython/dnssec/bindmgr.py @@ -189,10 +189,20 @@ class BINDMgr(object): self.notify_zone(zone) - def sync(self): - """Synchronize list of zones in LDAP with BIND.""" + def sync(self, dnssec_zones): + """Synchronize list of zones in LDAP with BIND. + + dnssec_zones lists zones which should be processed. All other zones + will be ignored even though they were modified using ldap_event(). + + This filter is useful in cases where LDAP contains DNS zones which + have old metadata objects and DNSSEC disabled. Such zones must be + ignored to prevent errors while calling dnssec-keyfromlabel or rndc. + """ self.log.debug('Key metadata in LDAP: %s' % self.ldap_keys) - for zone in self.modified_zones: + self.log.debug('Zones modified but skipped during bindmgr.sync: %s', + self.modified_zones - dnssec_zones) + for zone in self.modified_zones.intersection(dnssec_zones): self.sync_zone(zone) self.modified_zones = set() diff --git a/ipapython/dnssec/keysyncer.py b/ipapython/dnssec/keysyncer.py index aa96dba2010bd778589991f801de368fd2dd01cf..20039a068ccc1924f8aff43adddf5fd9d04972c1 100644 --- a/ipapython/dnssec/keysyncer.py +++ b/ipapython/dnssec/keysyncer.py @@ -5,6 +5,8 @@ import ldap.dn import os +import dns.name + from ipaplatform.paths import paths from ipapython import ipautil @@ -32,6 +34,7 @@ class KeySyncer(SyncReplConsumer): self.bindmgr = BINDMgr(self.api) self.init_done = False + self.dnssec_zones = set() SyncReplConsumer.__init__(self, *args, **kwargs) def _get_objclass(self, attrs): @@ -111,40 +114,49 @@ class KeySyncer(SyncReplConsumer): self.ods_sync() self.hsm_replica_sync() self.hsm_master_sync() - self.bindmgr.sync() + self.bindmgr.sync(self.dnssec_zones) # idnsSecKey wrapper # Assumption: metadata points to the same key blob all the time, # i.e. it is not necessary to re-download blobs because of change in DNSSEC # metadata - DNSSEC flags or timestamps. def key_meta_add(self, uuid, dn, newattrs): self.hsm_replica_sync() self.bindmgr.ldap_event('add', uuid, newattrs) - self.bindmgr_sync() + self.bindmgr_sync(self.dnssec_zones) def key_meta_del(self, uuid, dn, oldattrs): self.bindmgr.ldap_event('del', uuid, oldattrs) - self.bindmgr_sync() + self.bindmgr_sync(self.dnssec_zones) self.hsm_replica_sync() def key_metadata_sync(self, uuid, dn, oldattrs, newattrs): self.bindmgr.ldap_event('mod', uuid, newattrs) - self.bindmgr_sync() + self.bindmgr_sync(self.dnssec_zones) - def bindmgr_sync(self): + def bindmgr_sync(self, dnssec_zones): if self.init_done: - self.bindmgr.sync() + self.bindmgr.sync(dnssec_zones) # idnsZone wrapper def zone_add(self, uuid, dn, newattrs): + zone = dns.name.from_text(newattrs['idnsname'][0]) + if self.__is_dnssec_enabled(newattrs): + self.dnssec_zones.add(zone) + else: + self.dnssec_zones.discard(zone) + if not self.ismaster: return if self.__is_dnssec_enabled(newattrs): self.odsmgr.ldap_event('add', uuid, newattrs) self.ods_sync() def zone_del(self, uuid, dn, oldattrs): + zone = dns.name.from_text(oldattrs['idnsname'][0]) + self.dnssec_zones.discard(zone) + if not self.ismaster: return -- 2.5.0
From c4f6fcfa5135ee770ee9393637e0b07f6a18888f Mon Sep 17 00:00:00 2001 From: Petr Spacek <pspa...@redhat.com> Date: Sun, 20 Dec 2015 19:19:28 +0100 Subject: [PATCH] DNSSEC: ipa-ods-exporter: add ldap-cleanup command Command "ldap-cleanup <zone name>" will remove all key metadata from LDAP. This can be used manually in sequence like: ldap-cleanup <zone name> update <zone name> to delete all key metadata from LDAP and re-export them from OpenDNSSEC. ldap-cleanup command should be called when disabling DNSSEC on a DNS zone to remove stale key metadata from LDAP. https://fedorahosted.org/freeipa/ticket/5348 --- daemons/dnssec/ipa-ods-exporter | 60 ++++++++++++++++++++++++++++++++--------- 1 file changed, 48 insertions(+), 12 deletions(-) diff --git a/daemons/dnssec/ipa-ods-exporter b/daemons/dnssec/ipa-ods-exporter index def9ca39ec8462bf7ab3e4b9453fce9decbee665..2aa936040c373e366e7e15539ed6e3413aac7d55 100755 --- a/daemons/dnssec/ipa-ods-exporter +++ b/daemons/dnssec/ipa-ods-exporter @@ -223,7 +223,9 @@ def get_ldap_zone(ldap, dns_base, name): except ipalib.errors.NotFound: continue - assert ldap_zone is not None, 'DNS zone "%s" should exist in LDAP' % name + if ldap_zone is None: + raise ipalib.errors.NotFound( + reason='DNS zone "%s" not found in LDAP' % name) return ldap_zone @@ -477,25 +479,37 @@ def parse_command(cmd): if cmd == 'ipa-hsm-update': return (0, 'HSM synchronization finished, skipping zone synchronization.', - None) + None, + cmd) elif cmd == 'ipa-full-update': return (None, 'Synchronization of all zones was finished.', - None) + None, + cmd) - elif not cmd.startswith('update '): + elif cmd.startswith('ldap-cleanup '): + zone_name = cmd2ods_zone_name(cmd) + return (None, + 'Zone "%s" metadata will be removed from LDAP.\n' % zone_name, + zone_name, + 'ldap-cleanup') + + elif cmd.startswith('update '): + zone_name = cmd2ods_zone_name(cmd) + return (None, + 'Zone "%s" metadata will be updated in LDAP.\n' % zone_name, + zone_name, + 'update') + + else: return (0, 'Command "%s" is not supported by IPA; ' 'HSM synchronization was finished and the command ' 'will be ignored.' % cmd, + None, None) - else: - zone_name = cmd2ods_zone_name(cmd) - return (None, - 'Zone was "%s" updated.\n' % zone_name, - zone_name) def send_systemd_reply(conn, reply): # Reply & close connection early. @@ -506,7 +520,7 @@ def send_systemd_reply(conn, reply): def cmd2ods_zone_name(cmd): # ODS stores zone name without trailing period - zone_name = cmd[7:].strip() + zone_name = cmd.split(' ', 1)[1].strip() if len(zone_name) > 1 and zone_name[-1] == '.': zone_name = zone_name[:-1] @@ -580,6 +594,25 @@ def sync_zone(log, ldap, dns_dn, zone_name): except ipalib.errors.EmptyModlist: continue +def cleanup_ldap_zone(log, ldap, dns_dn, zone_name): + """delete all key metadata about zone keys for single DNS zone + + Key material has to be synchronized elsewhere. + Keep in mind that keys could be shared among multiple zones!""" + log = log.getChild("%s.%s" % (__name__, zone_name)) + log.debug('cleaning up key metadata from zone "%s"', zone_name) + + try: + ldap_zone = get_ldap_zone(ldap, dns_dn, zone_name) + ldap_keys = get_ldap_keys(ldap, ldap_zone.dn) + except ipalib.errors.NotFound as ex: + # zone or cn=keys container does not exist, we are done + log.debug(str(ex)) + return + + for ldap_key in ldap_keys: + log.debug('deleting key metadata "%s"', ldap_key.dn) + ldap.delete_entry(ldap_key) log = logging.getLogger('root') # this service is usually socket-activated @@ -651,7 +684,7 @@ except KeyError as e: conn = None cmd = sys.argv[1] -exitcode, msg, zone_name = parse_command(cmd) +exitcode, msg, zone_name, cmd = parse_command(cmd) if exitcode is not None: if conn: @@ -681,7 +714,10 @@ try: if zone_name is not None: # only one zone should be processed - sync_zone(log, ldap, dns_dn, zone_name) + if cmd == 'update': + sync_zone(log, ldap, dns_dn, zone_name) + elif cmd == 'ldap-cleanup': + cleanup_ldap_zone(log, ldap, dns_dn, zone_name) else: # process all zones for zone_row in db.execute("SELECT name FROM zones"): -- 2.5.0
From f983dd7f9ff1f600026de9aa983704a366692943 Mon Sep 17 00:00:00 2001 From: Petr Spacek <pspa...@redhat.com> Date: Sun, 20 Dec 2015 19:35:55 +0100 Subject: [PATCH] DNSSEC: ipa-dnskeysyncd: call ods-signer ldap-cleanup on zone removal Command "ldap-cleanup <zone name>" is called to remove all key metadata from LDAP. This command is now called when disabling DNSSEC on a DNS zone. The stale metadata were causing problems when re-enabling DNSSEC on the same zone. https://fedorahosted.org/freeipa/ticket/5348 --- ipapython/dnssec/odsmgr.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/ipapython/dnssec/odsmgr.py b/ipapython/dnssec/odsmgr.py index 041ad1fc9567465241b97e181d3a0826d8c56395..fb6d696af5be52c6179638338a231c57e3a1891a 100644 --- a/ipapython/dnssec/odsmgr.py +++ b/ipapython/dnssec/odsmgr.py @@ -151,12 +151,18 @@ class ODSMgr(object): output = self.ksmutil(cmd) self.log.info(output) self.notify_enforcer() + self.cleanup_signer(name) def notify_enforcer(self): cmd = ['notify'] output = self.ksmutil(cmd) self.log.info(output) + def cleanup_signer(self, zone_name): + cmd = ['ods-signer', 'ldap-cleanup', str(zone_name)] + output = ipautil.run(cmd, capture_output=True) + self.log.info(output) + def ldap_event(self, op, uuid, attrs): """Record single LDAP event - zone addition or deletion. -- 2.5.0
From 2ca9db5f7901e26d598937b22ab48a25e77e4820 Mon Sep 17 00:00:00 2001 From: Petr Spacek <pspa...@redhat.com> Date: Tue, 24 Nov 2015 12:49:16 +0100 Subject: [PATCH] DNSSEC: Log debug messages at log level DEBUG https://fedorahosted.org/freeipa/ticket/5348 --- 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 403ba05550efaa6e4954bb32a31d7e022e40da40..bb7292c2e06607a74f17f26cdf37b67784dd8fc8 100755 --- a/daemons/dnssec/ipa-ods-exporter +++ b/daemons/dnssec/ipa-ods-exporter @@ -218,7 +218,7 @@ def ldap2master_replica_keys_sync(log, ldapkeydb, localhsm): log.info("new replica keys in LDAP: %s", hex_set(new_replica_keys)) for key_id in new_replica_keys: new_key_ldap = ldapkeydb.replica_pubkeys_wrap[key_id] - log.error('label=%s, id=%s, data=%s', + log.debug('label=%s, id=%s, data=%s', new_key_ldap['ipk11label'], hexlify(new_key_ldap['ipk11id']), hexlify(new_key_ldap['ipapublickey'])) -- 2.5.0
-- 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