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. -- Petr^2 Spacek
From 07580f5adcb7c91b8423e03a6573cdefebc41c81 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 3fb01cac58c60e9e6f07db7e639d5fa5c3fa2548..28c529b7ea01947794ca707e94e6eac515f14e42 100755 --- a/daemons/dnssec/ipa-ods-exporter +++ b/daemons/dnssec/ipa-ods-exporter @@ -32,6 +32,7 @@ import systemd.daemon import systemd.journal import sqlite3 import time +import traceback import ipalib from ipapython.dn import DN @@ -575,7 +576,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 37d5da70df2f5875541fdc2b531289d1de3e782d 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 28c529b7ea01947794ca707e94e6eac515f14e42..bdafa79b11a80bb0af8df89cc378bb052d87c5ad 100755 --- a/daemons/dnssec/ipa-ods-exporter +++ b/daemons/dnssec/ipa-ods-exporter @@ -57,6 +57,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', @@ -122,6 +130,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') @@ -172,25 +251,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 549122d7125dc9812a23c0a188e1d00d89b93287 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 a0a9f2eb28ddb9dce8d1d71ab10afe6a0e0146ec..2112db7ffe9fac6ef8b5f26224d05864c9290146 100644 --- a/ipapython/dnssec/bindmgr.py +++ b/ipapython/dnssec/bindmgr.py @@ -58,17 +58,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 ef41945b566c55b350d3f0f9d29c4c97601836b5 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 8069993758b534f86311e800115e797fb28e7f70..c78424617ef8cedf8c591e5f0da0d27138c19960 100644 --- a/ipapython/dnssec/ldapkeydb.py +++ b/ipapython/dnssec/ldapkeydb.py @@ -182,7 +182,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 89d975f401cac260c9eca02ae0d008f85e481fb0 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 c78424617ef8cedf8c591e5f0da0d27138c19960..f8851670bc43e625cab7387895fe7d60a68f2d7b 100644 --- a/ipapython/dnssec/ldapkeydb.py +++ b/ipapython/dnssec/ldapkeydb.py @@ -4,13 +4,16 @@ from binascii import hexlify import collections +import logging +from pprint import pprint import sys import time import ipalib from ipapython.dn import DN from ipapython import ipaldap from ipapython import ipautil +from ipapython import ipa_log_manager from ipaplatform.paths import paths from ipapython.dnssec.abshsm import (attrs_name2id, attrs_id2name, AbstractHSM, @@ -136,8 +139,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""" @@ -348,3 +355,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 4c1de4f7b935e9ec6d60b118d72ea7a60f85aaa2 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 bdafa79b11a80bb0af8df89cc378bb052d87c5ad..f32597457e25a86ba43a9783145c167f04800688 100755 --- a/daemons/dnssec/ipa-ods-exporter +++ b/daemons/dnssec/ipa-ods-exporter @@ -491,6 +491,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()) @@ -523,30 +528,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 8e445f66b0f36775a58b84f620b2b57e2a43d900 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 f32597457e25a86ba43a9783145c167f04800688..d4ba11534a7a0a2758bf826505d1f901b5e7871a 100755 --- a/daemons/dnssec/ipa-ods-exporter +++ b/daemons/dnssec/ipa-ods-exporter @@ -387,19 +387,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)) @@ -420,6 +423,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() @@ -599,7 +625,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)) @@ -611,7 +637,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: @@ -665,6 +691,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 f8851670bc43e625cab7387895fe7d60a68f2d7b..51819783f7d698b2eabba8cfaaf38ce5097afff2 100644 --- a/ipapython/dnssec/ldapkeydb.py +++ b/ipapython/dnssec/ldapkeydb.py @@ -106,40 +106,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: @@ -154,6 +170,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): @@ -240,21 +299,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 e72ec2e41f135c8c88801fb6d020836daca1fcd9 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 2112db7ffe9fac6ef8b5f26224d05864c9290146..e9e5f89ba5ec8d8dea92c99faceabe04c8aee97d 100644 --- a/ipapython/dnssec/bindmgr.py +++ b/ipapython/dnssec/bindmgr.py @@ -192,10 +192,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 426dd940a8a613319f33cbd11710779428f7e51c..e0c6df3a6215543f923a7723e47d7c324e9c4b64 100644 --- a/ipapython/dnssec/keysyncer.py +++ b/ipapython/dnssec/keysyncer.py @@ -6,6 +6,8 @@ import logging import ldap.dn import os +import dns.name + from ipaplatform.paths import paths from ipapython import ipautil @@ -33,6 +35,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): @@ -112,40 +115,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 4587a250444ac6fbe58751b915858fbfd3b906e5 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 d4ba11534a7a0a2758bf826505d1f901b5e7871a..e588a9955b4c738cfbda26b29788ce356c91374c 100755 --- a/daemons/dnssec/ipa-ods-exporter +++ b/daemons/dnssec/ipa-ods-exporter @@ -227,7 +227,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 @@ -481,25 +483,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. @@ -510,7 +524,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] @@ -584,6 +598,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 @@ -655,7 +688,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: @@ -685,7 +718,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 e6279cd588317d3e30be43a3906500ed0c3e4ca9 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 ebcd3aa242531689fee697b6d12d0269e5dba27e..7bbc92c626c9a97bb761687dd974acdbfef40778 100644 --- a/ipapython/dnssec/odsmgr.py +++ b/ipapython/dnssec/odsmgr.py @@ -153,12 +153,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 85dc37f3a3f1e76bf1fdc83d8b9624776fbb1229 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 b90157c4e271098ae42fb3e02a01fa910ec373fc..3fb01cac58c60e9e6f07db7e639d5fa5c3fa2548 100755 --- a/daemons/dnssec/ipa-ods-exporter +++ b/daemons/dnssec/ipa-ods-exporter @@ -222,7 +222,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
