Re: [Freeipa-devel] [PATCHES] 267-294 Support multiple CA certificates in LDAP
On 06/12/2014 07:45 PM, Jan Cholasta wrote: ... Note that automatic distribution of CA certificates to IPA systems is not implemented yet (it's planned for IPA 4.2, see https://fedorahosted.org/freeipa/ticket/4322), so /etc/ipa/ca.crt, /etc/pki/nssdb, /etc/dirsrv/slapd-REALM and /etc/httpd/alias are updated *only* during client/server install. Honza For 4.0, we will need to come up with manual procedure how to renew the certificates *without* reinstalling the client or server. I think the best way would be to prepare a simple script to renew client/server, something like /usr/share/ipa/ipa-renew-client-certificate /usr/share/ipa/ipa-renew-server-certificate and refer to it in the ipa-cacert-manage man page. People could then pretty easily run those after a cert change, using whatever means their infrastructure uses - puppet, ssh, ... Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0224] cainstance: Read CS.cfg for preop.pin in a loop
On 06/12/2014 06:22 PM, Nathaniel McCallum wrote: On Thu, 2014-06-12 at 17:07 +0200, Tomas Babej wrote: On 06/12/2014 04:45 PM, Nathaniel McCallum wrote: On Thu, 2014-06-12 at 16:36 +0200, Tomas Babej wrote: On 06/12/2014 04:27 PM, Nathaniel McCallum wrote: On Thu, 2014-06-12 at 16:20 +0200, Martin Kosek wrote: On 06/12/2014 03:15 PM, Tomas Babej wrote: On 06/12/2014 02:37 PM, Nathaniel McCallum wrote: On Thu, 2014-06-12 at 13:29 +0200, Tomas Babej wrote: On 06/12/2014 10:45 AM, Martin Kosek wrote: On 06/11/2014 06:49 PM, Nathaniel McCallum wrote: On Wed, 2014-06-11 at 11:08 +0200, Tomas Babej wrote: Hi, As due to possible race conditions, the preop.pin might not be written in the CS.cfg at the time installer tries to read it. In case no value for preop.pin was found, retry until timeout was reached. https://fedorahosted.org/freeipa/ticket/3382 (applies on ipa-3-0 branch) There is inconsistent spacing around '='. For instance: +f=open(filename, 'r') +data = f.read() Also, I'm fairly certain this is incorrect: +total_timeout =+ 1 Nathaniel +1, this is certainly is not a deterministic, constant measuring of a timeout. Fixed. I would rather see something like op_timeout = time.time() + api.env.startup_timeout while preop_pin is None and time.time() op_timeout: Also, I do not think this will work, IOError is risen when a file is not found, so this code would not solve the problem if waiting on file to appear. Yes, but the problem was not in being unable to open the file, but that certain file content is not in the file yet. It seems that pkicreate creates the file in time, but the content written later and that causes the race condition. If you inspect the original code, and bugzilla, you can see that installation fails with: 2013-01-25T02:58:44Z INFO The ipa-server-install command failed, exception: RuntimeError: Unable to find preop.pin in /var/lib/pki-ca/conf/CS.cfg. Is your CA already configured? While the original code # read the config file and get the preop pin try: f=open(filename) except IOError, e: root_logger.error(Cannot open configuration file. + str(e)) raise e data = f.read() data = data.split('\n') pattern = re.compile(preop.pin=(.*) ) for line in data: match = re.search(pattern, line) if (match): preop_pin=match.group(1) break if preop_pin is None: raise RuntimeError(Unable to find preop.pin in %s. Is your CA already configured? % filename) does raise the IOError in case the file was not able to be opened. Since we get the Runtime error, file must exist, only the desired content is missing. That said, I have no objections to amending the patch so that it properly handles this race condition we did not encounter. Better safe than sorry, it's a simple change and already included in the current iteration of the patch. Updated patch attached. I think I would prefer something like inotify watching for IN_CLOSE_WRITE rather than this polling. Nathaniel Wouldn't that be a little bit of an overkill for that purpose? If would. Nathaniel, this is a fix for ipa-3-0 branch only, we do not have this problem in current master as this issue only affects Dogtag 9 installation. The target is to have small, contained and the non-intrusive patch. Thus the proposed solution to just implement a little wait loop. - we'll need to introduce an additional dependency for python-inotify - watching for IN_CLOSE_WRITE is not equivalent with the preop_pin written to CS.cfg (and we don't know that unless we inspect the code for pkicreate, which in turn would make our code dependant on code not located in our codebase), so we will still need to check if preop_pin is there, and loop over if not Sorry, I forgot this was for 3.0 branch. Nitpick: '=' spacing is still not fixed. I fixed
[Freeipa-devel] [PATCHES 0066-0067] Upgrade procedure for forwardzones
Patches attached, require patches mbasti 0052-0055. -- Martin^2 Basti From 4d7025f5bd5f3d069dda2da6d4795d3796778c5f Mon Sep 17 00:00:00 2001 From: Martin Basti mba...@redhat.com Date: Fri, 13 Jun 2014 10:15:23 +0200 Subject: [PATCH 1/2] Added upgrade step executed before schmema is upgraded Class PreSchemaUpdate is executed before ldap schema update This is required by ticket: https://fedorahosted.org/freeipa/ticket/3210 --- ipaserver/install/ipa_ldap_updater.py | 14 -- ipaserver/install/ldapupdate.py | 15 ++- ipaserver/install/plugins/__init__.py | 5 +++-- ipaserver/install/plugins/baseupdate.py | 13 - ipaserver/install/upgradeinstance.py| 17 + 5 files changed, 54 insertions(+), 10 deletions(-) diff --git a/ipaserver/install/ipa_ldap_updater.py b/ipaserver/install/ipa_ldap_updater.py index d894b302407a7f431102ce5cff2fed3cbed5ed47..cecc4ab308c4ab4046fcd8e728648c913baf364f 100644 --- a/ipaserver/install/ipa_ldap_updater.py +++ b/ipaserver/install/ipa_ldap_updater.py @@ -190,12 +190,6 @@ class LDAPUpdater_NonUpgrade(LDAPUpdater): modified = False -if options.update_schema: -modified = schemaupdate.update_schema( -options.schema_files, -dm_password=self.dirman_password, -live_run=not options.test) or modified - ld = LDAPUpdate( dm_password=self.dirman_password, sub_dict={}, @@ -203,6 +197,14 @@ class LDAPUpdater_NonUpgrade(LDAPUpdater): ldapi=options.ldapi, plugins=options.plugins or self.run_plugins) +modified = ld.pre_schema_update(ordered=True) + +if options.update_schema: +modified = schemaupdate.update_schema( +options.schema_files, +dm_password=self.dirman_password, +live_run=not options.test) or modified + if not self.files: self.files = ld.get_all_files(UPDATES_DIR) diff --git a/ipaserver/install/ldapupdate.py b/ipaserver/install/ldapupdate.py index a9167aeeed562732e66fd76e2410dfc62a4b145e..cdfcda9ee387423c36e263bc1d72d3c20f0282b7 100644 --- a/ipaserver/install/ldapupdate.py +++ b/ipaserver/install/ldapupdate.py @@ -43,7 +43,8 @@ from ipalib import errors from ipalib import api from ipapython.dn import DN from ipapython.ipa_log_manager import * -from ipaserver.install.plugins import PRE_UPDATE, POST_UPDATE +from ipaserver.install.plugins import (PRE_UPDATE, POST_UPDATE, + PRE_SCHEMA_UPDATE) from ipaserver.plugins import ldap2 @@ -793,6 +794,18 @@ class LDAPUpdate: for dn, update in sorted_updates: self._delete_record(update) +def pre_schema_update(self, ordered=False): +Execute the update before the LDPA schema is updated. + +if self.plugins: +self.info('PRE_SCHEMA_UPDATE') +all_updates = {} +updates = api.Backend.updateclient.update(PRE_SCHEMA_UPDATE, self.dm_password, self.ldapi, self.live_run) +self.merge_updates(all_updates, updates) +self._run_updates(all_updates) + +return self.modified + def update(self, files, ordered=False): Execute the update. files is a list of the update files to use. diff --git a/ipaserver/install/plugins/__init__.py b/ipaserver/install/plugins/__init__.py index 49bef4df80d9b8ea2f5861dcb69c7ae2fb882472..296831db754b45121b6c7260777c151827a95467 100644 --- a/ipaserver/install/plugins/__init__.py +++ b/ipaserver/install/plugins/__init__.py @@ -20,8 +20,9 @@ Provide a separate api for updates. -PRE_UPDATE = 1 -POST_UPDATE = 2 +PRE_SCHEMA_UPDATE = 1 +PRE_UPDATE = 2 +POST_UPDATE = 3 FIRST = 1 MIDDLE = 2 diff --git a/ipaserver/install/plugins/baseupdate.py b/ipaserver/install/plugins/baseupdate.py index a480a8ee289646374af33f98dcf1f6978a969aa5..b0f7b7b600231314b7e3c008a86e20cf8fed4288 100644 --- a/ipaserver/install/plugins/baseupdate.py +++ b/ipaserver/install/plugins/baseupdate.py @@ -20,7 +20,8 @@ from ipalib import api from ipalib import Updater, Object from ipaserver.install import service -from ipaserver.install.plugins import PRE_UPDATE, POST_UPDATE, MIDDLE +from ipaserver.install.plugins import (PRE_UPDATE, POST_UPDATE, + PRE_SCHEMA_UPDATE, MIDDLE) class DSRestart(service.Service): @@ -55,6 +56,16 @@ class update(Object): api.register(update) +class PreSchemaUpdate(Updater): + +Base class for updates that run after file processing. + +updatetype = PRE_SCHEMA_UPDATE +order = MIDDLE + +def __init__(self): +super(PreSchemaUpdate, self).__init__() + class PreUpdate(Updater): Base class for updates that run prior to file processing. diff --git a/ipaserver/install/upgradeinstance.py b/ipaserver/install/upgradeinstance.py index
Re: [Freeipa-devel] [PATCH] 0571 ipalib.frontend: Do API version check before converting arguments
On 06/09/2014 01:21 PM, Petr Viktorin wrote: On 06/09/2014 12:22 PM, Petr Viktorin wrote: Hello, This fixes https://fedorahosted.org/freeipa/ticket/3963 for master. I'll make a slightly less invasive patch for 3.x. Basically, the version argument is now expected for all commands (including e.g. ping env), and also when calling the commands in_server. If it's not given, a message is returned, so this should should only really affect tests that check the output strictly. For the 3.x maintenance branches, here's a much smaller fix. This only checks the version if it was given. Worked fine - tested on RHEL-6.5. Pushed to: ipa-3-0: 220539a3653b15e4f5679b53cab8e601abaf8990 ipa-3-1: 98f5abe37461844b42989766caee525c0d8864f8 ipa-3-2: b4d2637fc43798669b8ea1bc6fe0f851fd30401a ipa-3-3: 7486140e00c2f1e119250fb69040864fa902290d Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCHES] 0581-0582 ipalib.config: Only convert numeric values to float
First patch: minor fix in env loading Second patch: When api.env is loaded, strings that look like floats get auto-converted to floats. This is wrong, as the conversion can lose precision, which matters for the new api_version. Here's a patch that disables this conversion, and fixes places that the disabling might break. -- Petr³ From 47b6db3e91a16b6598c655edf17d1533b5e3dcc8 Mon Sep 17 00:00:00 2001 From: Petr Viktorin pvikt...@redhat.com Date: Fri, 13 Jun 2014 12:40:32 +0200 Subject: [PATCH] ipalib.config: Only convert basedn to DN The current code would convert values to DN if the key was a substring of 'basedn', e.g. 'base' or 'sed'. Only convert if we're actually dealing with 'basedn'. --- ipalib/config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ipalib/config.py b/ipalib/config.py index f86c0a5ea3885d2bf89712f91b0c0705dceedd32..709e067416046b2c5174554043be87fa27042bf9 100644 --- a/ipalib/config.py +++ b/ipalib/config.py @@ -257,7 +257,7 @@ def __setitem__(self, key, value): value = m[value] elif value.isdigit(): value = int(value) -elif key in ('basedn'): +elif key == 'basedn': value = DN(value) else: try: -- 1.9.0 From bb3d1e766140b0ed1cbf37562080070cf46cab1e Mon Sep 17 00:00:00 2001 From: Petr Viktorin pvikt...@redhat.com Date: Fri, 13 Jun 2014 12:47:48 +0200 Subject: [PATCH] ipalib.config: Don't autoconvert values to float When api.env is loaded, strings that look like floats got auto-converted to floats. This is wrong, as the conversion to float can lose precision. Case in point: the api_version (e.g. '2.88') should never be interpreted as float. Do not automatically convert to float. We have two numeric options: startup_timeout and wait_for_dns. wait_for_dns is already converted to int when used in the code. Convert startup_timeout to float explicitly when used, so configuration that specified it with a decimal point continues to work. --- ipalib/config.py | 5 - ipapython/ipautil.py | 2 ++ ipapython/platform/fedora16/service.py | 2 +- ipatests/test_ipalib/test_config.py| 5 ++--- 4 files changed, 5 insertions(+), 9 deletions(-) diff --git a/ipalib/config.py b/ipalib/config.py index 709e067416046b2c5174554043be87fa27042bf9..b12cfd32184edf964353fda9304dbb5149eb525f 100644 --- a/ipalib/config.py +++ b/ipalib/config.py @@ -259,11 +259,6 @@ def __setitem__(self, key, value): value = int(value) elif key == 'basedn': value = DN(value) -else: -try: -value = float(value) -except (TypeError, ValueError): -pass assert type(value) in (unicode, int, float, bool, NoneType, DN) object.__setattr__(self, key, value) self.__d[key] = value diff --git a/ipapython/ipautil.py b/ipapython/ipautil.py index 844dbb68792c483552149be7ae442a1e40eb9626..d95983b208f47ff42dfc254248e2f4f03d372bff 100644 --- a/ipapython/ipautil.py +++ b/ipapython/ipautil.py @@ -1135,6 +1135,7 @@ def wait_for_open_ports(host, ports, timeout=0): in seconds may be specified to limit the wait. If the timeout is exceeded, socket.timeout exception is raised. +timeout = float(timeout) if not isinstance(ports, (tuple, list)): ports = [ports] @@ -1156,6 +1157,7 @@ def wait_for_open_socket(socket_name, timeout=0): Wait until the specified socket on the local host is open. Timeout in seconds may be specified to limit the wait. +timeout = float(timeout) op_timeout = time.time() + timeout while True: diff --git a/ipapython/platform/fedora16/service.py b/ipapython/platform/fedora16/service.py index 41c241ae5c31df56544b5b2bebd71c5ef109dd6e..86403d82583ed1e70044ce788d7ead7a5f3544a1 100644 --- a/ipapython/platform/fedora16/service.py +++ b/ipapython/platform/fedora16/service.py @@ -152,7 +152,7 @@ def __wait_until_running(self): 'The httpd proxy is not installed, wait on local port') use_proxy = False root_logger.debug('Waiting until the CA is running') -timeout = api.env.startup_timeout +timeout = float(api.env.startup_timeout) op_timeout = time.time() + timeout while time.time() op_timeout: try: diff --git a/ipatests/test_ipalib/test_config.py b/ipatests/test_ipalib/test_config.py index f896b893601c3ce85213cac39338095d7ac946f7..e04dd953074342f09b0ca4ca6dbea37eb37aaf48 100644 --- a/ipatests/test_ipalib/test_config.py +++ b/ipatests/test_ipalib/test_config.py @@ -43,8 +43,7 @@ ('trailing_whitespace', u' value ', u'value'), ('an_int', 42, 42), ('int_repr', ' 42 ', 42), -('a_float', 3.14, 3.14), -('float_repr', ' 3.14 ', 3.14), +('not_a_float', '3.14', u'3.14'), ('true', True, True), ('true_repr', ' True ',
Re: [Freeipa-devel] [PATCH] 0571 ipalib.frontend: Do API version check before converting arguments
On 06/13/2014 10:41 AM, Martin Kosek wrote: On 06/09/2014 12:22 PM, Petr Viktorin wrote: Hello, This fixes https://fedorahosted.org/freeipa/ticket/3963 for master. I'll make a slightly less invasive patch for 3.x. Basically, the version argument is now expected for all commands (including e.g. ping env), and also when calling the commands in_server. If it's not given, a message is returned, so this should should only really affect tests that check the output strictly. Functionally works fine, tested with curl. I see one issue in the test that may be related: == FAIL: Test the `ipalib.config.Env._finalize_core` method. -- Traceback (most recent call last): File /usr/lib/python2.7/site-packages/nose/case.py, line 197, in runTest self.test(*self.arg) File /root/freeipa-master/ipatests/test_ipalib/test_config.py, line 572, in test_finalize_core assert o[key] == value, '%r is %r; should be %r' % (key, o[key], value) AssertionError: 'api_version' is 2.88; should be u'2.88' That's a different issue; the test's been broken since 2a8c509. I've been meaning to get to it for a while. Addressed in my patch 0582. -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 267-294 Support multiple CA certificates in LDAP
On Fri, 2014-06-13 at 09:05 +0200, Martin Kosek wrote: On 06/12/2014 07:45 PM, Jan Cholasta wrote: ... Note that automatic distribution of CA certificates to IPA systems is not implemented yet (it's planned for IPA 4.2, see https://fedorahosted.org/freeipa/ticket/4322), so /etc/ipa/ca.crt, /etc/pki/nssdb, /etc/dirsrv/slapd-REALM and /etc/httpd/alias are updated *only* during client/server install. Honza For 4.0, we will need to come up with manual procedure how to renew the certificates *without* reinstalling the client or server. I think the best way would be to prepare a simple script to renew client/server, something like /usr/share/ipa/ipa-renew-client-certificate /usr/share/ipa/ipa-renew-server-certificate I assume you mean /usr/bin or /usr/libexec/ipa ? and refer to it in the ipa-cacert-manage man page. People could then pretty easily run those after a cert change, using whatever means their infrastructure uses - puppet, ssh, ... -- 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] [PATCHES] 267-294 Support multiple CA certificates in LDAP
On 06/13/2014 02:55 PM, Simo Sorce wrote: On Fri, 2014-06-13 at 09:05 +0200, Martin Kosek wrote: On 06/12/2014 07:45 PM, Jan Cholasta wrote: ... Note that automatic distribution of CA certificates to IPA systems is not implemented yet (it's planned for IPA 4.2, see https://fedorahosted.org/freeipa/ticket/4322), so /etc/ipa/ca.crt, /etc/pki/nssdb, /etc/dirsrv/slapd-REALM and /etc/httpd/alias are updated *only* during client/server install. Honza For 4.0, we will need to come up with manual procedure how to renew the certificates *without* reinstalling the client or server. I think the best way would be to prepare a simple script to renew client/server, something like /usr/share/ipa/ipa-renew-client-certificate /usr/share/ipa/ipa-renew-server-certificate I assume you mean /usr/bin or /usr/libexec/ipa ? Right, that's better. I think we do not want to store it in /usr/bin as fully supported scripts as I would feel obliged to keep that scripts supported and around even when automatic renewal is available in FreeIPA 4.2. So maybe /usr/libexec/ipa would be better. and refer to it in the ipa-cacert-manage man page. People could then pretty easily run those after a cert change, using whatever means their infrastructure uses - puppet, ssh, ... ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 267-294 Support multiple CA certificates in LDAP
Martin Kosek wrote: On 06/13/2014 02:55 PM, Simo Sorce wrote: On Fri, 2014-06-13 at 09:05 +0200, Martin Kosek wrote: On 06/12/2014 07:45 PM, Jan Cholasta wrote: ... Note that automatic distribution of CA certificates to IPA systems is not implemented yet (it's planned for IPA 4.2, see https://fedorahosted.org/freeipa/ticket/4322), so /etc/ipa/ca.crt, /etc/pki/nssdb, /etc/dirsrv/slapd-REALM and /etc/httpd/alias are updated *only* during client/server install. Honza For 4.0, we will need to come up with manual procedure how to renew the certificates *without* reinstalling the client or server. I think the best way would be to prepare a simple script to renew client/server, something like /usr/share/ipa/ipa-renew-client-certificate /usr/share/ipa/ipa-renew-server-certificate I assume you mean /usr/bin or /usr/libexec/ipa ? Right, that's better. I think we do not want to store it in /usr/bin as fully supported scripts as I would feel obliged to keep that scripts supported and around even when automatic renewal is available in FreeIPA 4.2. So maybe /usr/libexec/ipa would be better. I guess it depends on what our expectations of user's running this are. If it is basically sample code, then yeah, /usr/share may be ok. If it's something supported we expect some people to run, /usr/[s]bin is probably the place. /usr/libexec is for binaries run by other programs IIRC. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCHES] 0583-0584 Convert DNS default permissions to managed
With the first patch, old SYSTEM permissions can be replaced. The Read DNS Entries did not have an associated ACI, but was rather rolled into a single ACI with the managedBy rule used for per-zone access. (and before that it was part of a deny rule.) We can't remove this permission in an update file, because we need to check that it is indeed an old SYSTEM perm and not a new one with the same name. The second patch converts DNS permissions to managed. The ACIs are put directly in $SUFFIX, because the cn=dns subtree does not exist in all installations. I hope to change this for https://fedorahosted.org/freeipa/ticket/4058, when I've thought more about relationships between plugins, packages, install options, and the updater. -- Petr³ From 8c7b9bcf64bc3762ce8060bd88fd0be6764c8cca Mon Sep 17 00:00:00 2001 From: Petr Viktorin pvikt...@redhat.com Date: Fri, 13 Jun 2014 15:58:24 +0200 Subject: [PATCH] managed permission updater: Add mechanism to replace SYSTEM permissions The Read DNS Entries permission, which was marked SYSTEM (no associated ACI), can now be converted to a regular managed permission. Add a mechanism for the updater to replace old SYSTEM permissions. This cannot be done in an update file because we do not want to replace V2 permissions with the same name. Part of the work for: https://fedorahosted.org/freeipa/ticket/4346 --- .../install/plugins/update_managed_permissions.py | 18 ++ 1 file changed, 18 insertions(+) diff --git a/ipaserver/install/plugins/update_managed_permissions.py b/ipaserver/install/plugins/update_managed_permissions.py index 7b1405a1974826fd90acd0d5082f51d8b25034cd..2ca054d50d11eec9527e0ef1e5d53d2f8e479ed0 100644 --- a/ipaserver/install/plugins/update_managed_permissions.py +++ b/ipaserver/install/plugins/update_managed_permissions.py @@ -67,6 +67,8 @@ * replaces - A list of ACIs corresponding to legacy default permissions replaced by this permission. +* replaces_system + - A list of names of old SYSTEM permissions this replaces. * fixup_function - A callable that may modify the template in-place before it is applied. - Called with the permission name, template dict, and keyword arguments: @@ -410,6 +412,21 @@ def update_permission(self, ldap, obj, name, template, anonymous_read_aci): self.log.info(Removing legacy permission '%s', legacy_name) self.api.Command[permission_del](unicode(legacy_name)) +for name in template.get('replaces_system', ()): +name = unicode(name) +try: +entry = ldap.get_entry(permission_plugin.get_dn(name), + ['ipapermissiontype']) +except errors.NotFound: +self.log.info(Legacy permission '%s' not found, name) +else: +flags = entry.get('ipapermissiontype', []) +if list(flags) == ['SYSTEM']: +self.log.info(Removing legacy permission '%s', name) +self.api.Command[permission_del](name, force=True) +else: +self.log.info(Ignoring V2 permission '%s', name) + def get_upgrade_attr_lists(self, current_acistring, default_acistrings): Compute included and excluded attributes for a new permission @@ -497,6 +514,7 @@ def update_entry(self, obj, entry, template, template = dict(template) template.pop('replaces', None) +template.pop('replaces_system', None) fixup_function = template.pop('fixup_function', None) if fixup_function: -- 1.9.0 From 46d49e16b2cfa3a7fb8d947e6ae33e6b3050f87a Mon Sep 17 00:00:00 2001 From: Petr Viktorin pvikt...@redhat.com Date: Mon, 9 Jun 2014 15:06:35 +0200 Subject: [PATCH] Convert DNS default permissions to managed Convewrt the existing default permissions. The Read permission is split between Read DNS Entries and Read DNS Configuration. Part of the work for: https://fedorahosted.org/freeipa/ticket/4346 --- ACI.txt | 12 + install/share/dns.ldif | 59 install/updates/40-delegation.update | 6 +-- install/updates/40-dns.update| 29 ++ ipalib/plugins/dns.py| 101 +++ 5 files changed, 119 insertions(+), 88 deletions(-) diff --git a/ACI.txt b/ACI.txt index 2ceaacc077467b6ef54e09d0aa7d3d5695c8fd40..6b75e79c3d771d33558750958f61ada82fd1e5eb 100644 --- a/ACI.txt +++ b/ACI.txt @@ -10,6 +10,18 @@ dn: cn=System: Read Global Configuration,cn=permissions,cn=pbac,dc=ipa,dc=exampl aci: (targetattr = cn || ipacertificatesubjectbase || ipaconfigstring || ipacustomfields || ipadefaultemaildomain || ipadefaultloginshell || ipadefaultprimarygroup || ipagroupobjectclasses || ipagroupsearchfields || ipahomesrootdir || ipakrbauthzdata || ipamaxusernamelength || ipamigrationenabled || ipapwdexpadvnotify || ipasearchrecordslimit ||
[Freeipa-devel] [PATCHES] 0585-0587 Convert Password Policy COSTemplate default permissions to managed
The first patch is preparation. As for the second two, this is how the bulk of the transition will look. -- Petr³ From 979ef3e1a8e37b8ad6ad60027993f3c8de6a3d98 Mon Sep 17 00:00:00 2001 From: Petr Viktorin pvikt...@redhat.com Date: Thu, 12 Jun 2014 09:46:36 +0200 Subject: [PATCH] Add $REALM to variables supported by the managed permission updater This will allow converting password policy permissions Part of the work for: https://fedorahosted.org/freeipa/ticket/4346 --- ipaserver/install/plugins/update_managed_permissions.py | 1 + 1 file changed, 1 insertion(+) diff --git a/ipaserver/install/plugins/update_managed_permissions.py b/ipaserver/install/plugins/update_managed_permissions.py index 7b1405a1974826fd90acd0d5082f51d8b25034cd..f68faf262da5bcfbd4167213dff33db4676f7b2e 100644 --- a/ipaserver/install/plugins/update_managed_permissions.py +++ b/ipaserver/install/plugins/update_managed_permissions.py @@ -343,6 +343,7 @@ def update_permission(self, ldap, obj, name, template, anonymous_read_aci): if 'replaces' in template: sub_dict = { 'SUFFIX': str(self.api.env.basedn), +'REALM': str(self.api.env.realm), } legacy_acistrs = [ipautil.template_str(r, sub_dict) for r in template['replaces']] -- 1.9.0 From 0b9cd3c194a8d80405cb754e5cbbf39c9d6f0579 Mon Sep 17 00:00:00 2001 From: Petr Viktorin pvikt...@redhat.com Date: Wed, 4 Jun 2014 17:39:10 +0200 Subject: [PATCH] Convert COSTemplate default permissions to managed Part of the work for: https://fedorahosted.org/freeipa/ticket/4346 --- ACI.txt | 6 ++ install/updates/40-delegation.update | 24 ipalib/plugins/pwpolicy.py | 22 ++ 3 files changed, 28 insertions(+), 24 deletions(-) diff --git a/ACI.txt b/ACI.txt index 2ceaacc077467b6ef54e09d0aa7d3d5695c8fd40..5573da2fa733955789377be8c3fcbfb2f821ed9c 100644 --- a/ACI.txt +++ b/ACI.txt @@ -8,6 +8,12 @@ dn: cn=System: Read Automount Configuration,cn=permissions,cn=pbac,dc=ipa,dc=exa aci: (targetattr = automountinformation || automountkey || automountmapname || cn || description || objectclass)(version 3.0;acl permission:System: Read Automount Configuration;allow (compare,read,search) userdn = ldap:///anyone;;) dn: cn=System: Read Global Configuration,cn=permissions,cn=pbac,dc=ipa,dc=example aci: (targetattr = cn || ipacertificatesubjectbase || ipaconfigstring || ipacustomfields || ipadefaultemaildomain || ipadefaultloginshell || ipadefaultprimarygroup || ipagroupobjectclasses || ipagroupsearchfields || ipahomesrootdir || ipakrbauthzdata || ipamaxusernamelength || ipamigrationenabled || ipapwdexpadvnotify || ipasearchrecordslimit || ipasearchtimelimit || ipaselinuxusermapdefault || ipaselinuxusermaporder || ipauserauthtype || ipauserobjectclasses || ipausersearchfields || objectclass)(targetfilter = (objectclass=ipaguiconfig))(version 3.0;acl permission:System: Read Global Configuration;allow (compare,read,search) userdn = ldap:///all;;) +dn: cn=System: Add Group Password Policy costemplate,cn=permissions,cn=pbac,dc=ipa,dc=example +aci: (targetfilter = (objectclass=costemplate))(version 3.0;acl permission:System: Add Group Password Policy costemplate;allow (add) groupdn = ldap:///cn=System: Add Group Password Policy costemplate,cn=permissions,cn=pbac,dc=ipa,dc=example;) +dn: cn=System: Delete Group Password Policy costemplate,cn=permissions,cn=pbac,dc=ipa,dc=example +aci: (targetfilter = (objectclass=costemplate))(version 3.0;acl permission:System: Delete Group Password Policy costemplate;allow (delete) groupdn = ldap:///cn=System: Delete Group Password Policy costemplate,cn=permissions,cn=pbac,dc=ipa,dc=example;) +dn: cn=System: Modify Group Password Policy costemplate,cn=permissions,cn=pbac,dc=ipa,dc=example +aci: (targetattr = cospriority)(targetfilter = (objectclass=costemplate))(version 3.0;acl permission:System: Modify Group Password Policy costemplate;allow (write) groupdn = ldap:///cn=System: Modify Group Password Policy costemplate,cn=permissions,cn=pbac,dc=ipa,dc=example;) dn: cn=System: Read Group Password Policy costemplate,cn=permissions,cn=pbac,dc=ipa,dc=example aci: (targetattr = cn || cospriority || krbpwdpolicyreference || objectclass)(targetfilter = (objectclass=costemplate))(version 3.0;acl permission:System: Read Group Password Policy costemplate;allow (compare,read,search) groupdn = ldap:///cn=System: Read Group Password Policy costemplate,cn=permissions,cn=pbac,dc=ipa,dc=example;) dn: cn=System: Read Group Membership,cn=permissions,cn=pbac,dc=ipa,dc=example diff --git a/install/updates/40-delegation.update b/install/updates/40-delegation.update index 7c3a284b8d2a0592240e56d8118c821a25fc7798..36a0ad020699f6391251d03bd664f55af90500f4 100644 --- a/install/updates/40-delegation.update +++ b/install/updates/40-delegation.update @@ -170,27 +170,6 @@ dn: cn=Password Policy
Re: [Freeipa-devel] DNSSEC key metadata handling
On 12.6.2014 17:49, Petr Spacek wrote: On 12.6.2014 17:19, Simo Sorce wrote: On Thu, 2014-06-12 at 17:08 +0200, Petr Spacek wrote: Hello list, I have realized that we need to store certain DNSSEC metadata for every (zone,key,replica) triplet. It is necessary to handle splits in replication topology. DNSSEC key can be in one of following states: - key created - published but not used for signing - published and used for signing - published and not used for signing but old signatures exist - unpublished Every state transition has to be postponed until relevant TTL expires, and of course, we need to consider TTL on all replicas. Example of a problem DNS TTL=10 units Key life time=100 units Replica=1 Key=1 Time=0 Published Replica=2 Key=1 Time=0 Published Replica=1 Key=1 Time=10 Published, signing Replica=2 Key=1 Time=10 Published, signing Replica=1 Key=2 Time=90 Generated, published, not signing yet Replica=2 Key=2 Time=90 replication is broken: key=2 is not on replica=2 Replica=1 Key=1 Time=100 ^^^ From time=100, all new signatures should be created with key=2 but that can break DNSSEC validation because key=2 is not available on replica=2. Can you explain how this break validation ? Aren't signatures regenerated on each replica ? They are. And so isn't each replica self-consistent ? Ah, sorry, I didn't mention one important detail. Keys published in the zone 'example.com.' have to match keys published in parent zone. There has to be a mechanism for synchronizing this. Validation will break if (keys published by parent) are not subset of (keys on replicas). Next logical step in the example above is to remove key1 from replica 1 so you will end replica1 having key2 and replica2 having only key1. How can we guarantee that synchronization mechanism will not wipe key1 from parent? Or the other way around? How can we guarantee that key2 was uploaded? Also, things will break is number of keys in parent exceeds reasonable number (because DNS replies will be to big etc.). Proposal 1 == - Store state and timestamps for (zone,key,replica) triplet - Do state transition only if all triplets (zone,key,?) indicate that all replicas reached desired state so the transition is safe. - This implicitly means that no transition will happen if one or more replicas is down. This is necessary otherwise DNSSEC validation can break mysteriously when keys got out of sync. dn: cn=some-replica-id,ipk11Label=zone1_keyid123_private, cn=keys, cn=sec, cn=dns, dc=example idnssecKeyCreated: timestamp idnssecKeyPublished: timestamp idnssecKeyActivated: timestamp idnssecKeyInactivated: timestamp idnssecKeyDeleted: timestamp Why do you care for all 5 states ? In short, to follow RFC 6781 and all it's requirements. Simo and I have discussed this off-line. The final decision is to rely on replication. The assumption is that if replication is broken, everything will break soon anyway, so the original proposal is overkill. We have to store one set of timestamps somewhere to be able to follow RFC 6781, so we decided to store it in the key-metadata object. I added other attributes to object class definition so it contains all necessary metadata. The new object class idnsSecKey is now complete. Please note that DN in the previous example was incorrect. It is necessary to store the metadata separately for pairs (zone, key) to cover the case where key is shared between zones. This also nicely splits metadata from actual key material. All attributes are single-valued. MUST attributes are: idnsSecKeyRef idnsSecKeyCreated idnsSecAlgorithm dn: cn=z/ksk+keytag, cn=keys, idnsname=example.com, cn=dns, dc=example objectClass: idnsSecKey idnsSecKeyRef: DN of the PKCS#11 key object under cn=keys, cn=sec, dn=dns idnsSecKeyCreated: timestamp idnsSecKeyPublish: timestamp idnsSecKeyActivate: timestamp idnsSecKeyInactive: timestamp idnsSecKeyDelete: timestamp idnsSecKeyZone: boolean equivalent to bit 7 (ZONE) in [1] idnsSecKeyRevoke: boolean equivalent to bit 8 (REVOKE) in [1] idnsSecKeySep: boolean equivalent to bit 15 (SEP) in [1] idnsSecAlgorithm: string used as mnemonic in [2] [1] http://www.iana.org/assignments/dnskey-flags/dnskey-flags.xhtml#dnskey-flags-1 [2] http://www.iana.org/assignments/dns-sec-alg-numbers/dns-sec-alg-numbers.xhtml#dns-sec-alg-numbers-1 It looks to me the only relevant states are Activated and (perhaps) Deactivated. But then again if replication is broken *many* other things are, so should we *really* care to handle broken replication by adding all this information ? We need to keep track of timestamps anyway (to follow RFC 6781) so we need some other way how to *make sure* that timestamps never go backwards, even if replication was broken and later restarted. What do you propose? Effectively, state machine will be controlled by max(attribute) over all replicas (for given key). Replication traffic estimation -- Number
Re: [Freeipa-devel] [PATCH] #3859: Better mechanism to retrieve keytabs
Simo Sorce wrote: On Wed, 2014-06-11 at 17:03 -0400, Rob Crittenden wrote: 0001 When is_allowed_to_access_attr() fails it should include the value of access in the error log for debugging. Ok added more detailed logging Nit: Coluld not fetch REALM backend Fixed There are still a ton of ber_scanf failed duplicated fatal errors. I'm fine keeping a common err_msg but the fatal error should be unique. Yeah thanks to this comment, I had a small change of heart. Instead of sending such detailed information to clients I reverted to send a little less information to the clients and instead LOG_FATAL in a more detailed way. HTH This breaks normal host delegation. If you add a host to another host's managedby, getting the keytab will fail. This is due to: [11/Jun/2014:16:56:45 -0400] NSACLPlugin - conn=4 op=3 (main): Deny write on entry(fqdn=client2.example.com,cn=computers,cn=accounts,dc=example,dc=com).attr(ipaProtectedOperation;write_keys) to fqdn=client1.example.com,cn=computers,cn=accounts,dc=example,dc=com: no aci matched the subject by aci(97): aciname= Groups allowed to create keytab keys, acidn=cn=accounts,dc=example,dc=com Ok this should be working now, I added a new ACI to allow also managedby#USERDN to operate on keytabs. New patches attached. Functionally these seem to work ok. I think there should be some documented way to enable the -r in ipa-getkeytab. Right now I'm not even entirely sure how one would add a permission to do so. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] #3859: Better mechanism to retrieve keytabs
On Fri, 2014-06-13 at 12:54 -0400, Rob Crittenden wrote: Simo Sorce wrote: On Wed, 2014-06-11 at 17:03 -0400, Rob Crittenden wrote: 0001 When is_allowed_to_access_attr() fails it should include the value of access in the error log for debugging. Ok added more detailed logging Nit: Coluld not fetch REALM backend Fixed There are still a ton of ber_scanf failed duplicated fatal errors. I'm fine keeping a common err_msg but the fatal error should be unique. Yeah thanks to this comment, I had a small change of heart. Instead of sending such detailed information to clients I reverted to send a little less information to the clients and instead LOG_FATAL in a more detailed way. HTH This breaks normal host delegation. If you add a host to another host's managedby, getting the keytab will fail. This is due to: [11/Jun/2014:16:56:45 -0400] NSACLPlugin - conn=4 op=3 (main): Deny write on entry(fqdn=client2.example.com,cn=computers,cn=accounts,dc=example,dc=com).attr(ipaProtectedOperation;write_keys) to fqdn=client1.example.com,cn=computers,cn=accounts,dc=example,dc=com: no aci matched the subject by aci(97): aciname= Groups allowed to create keytab keys, acidn=cn=accounts,dc=example,dc=com Ok this should be working now, I added a new ACI to allow also managedby#USERDN to operate on keytabs. New patches attached. Functionally these seem to work ok. I think there should be some documented way to enable the -r in ipa-getkeytab. Right now I'm not even entirely sure how one would add a permission to do so. rob ATM the only way is to add the ipaAllowedOperations objectclass to the object you want to allow retrieving a keyt from and the ipaAllowedToPerform;reasd_key attribute Example: dn: test/foo.example@example.com,cn=services,cn=accounts,dc=example,dc=com changetype: modify add: objectclass objectclass: ipaAllowedOperations - add: ipaAllowedToPerform;read_key ipaAllowedToPerform;reasd_key: uid=cluster-admin,cn=users,cn=accounts,dc=example,dc=com Once you do this the user called cluster-admin will be allowed to retrieve the keytab w/o changing it. Of course you can list there a group or another host/service DN. HTH, 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] #3859: Better mechanism to retrieve keytabs
On Fri, 2014-06-13 at 14:04 -0400, Simo Sorce wrote: On Fri, 2014-06-13 at 12:54 -0400, Rob Crittenden wrote: Simo Sorce wrote: On Wed, 2014-06-11 at 17:03 -0400, Rob Crittenden wrote: 0001 When is_allowed_to_access_attr() fails it should include the value of access in the error log for debugging. Ok added more detailed logging Nit: Coluld not fetch REALM backend Fixed There are still a ton of ber_scanf failed duplicated fatal errors. I'm fine keeping a common err_msg but the fatal error should be unique. Yeah thanks to this comment, I had a small change of heart. Instead of sending such detailed information to clients I reverted to send a little less information to the clients and instead LOG_FATAL in a more detailed way. HTH This breaks normal host delegation. If you add a host to another host's managedby, getting the keytab will fail. This is due to: [11/Jun/2014:16:56:45 -0400] NSACLPlugin - conn=4 op=3 (main): Deny write on entry(fqdn=client2.example.com,cn=computers,cn=accounts,dc=example,dc=com).attr(ipaProtectedOperation;write_keys) to fqdn=client1.example.com,cn=computers,cn=accounts,dc=example,dc=com: no aci matched the subject by aci(97): aciname= Groups allowed to create keytab keys, acidn=cn=accounts,dc=example,dc=com Ok this should be working now, I added a new ACI to allow also managedby#USERDN to operate on keytabs. New patches attached. Functionally these seem to work ok. I think there should be some documented way to enable the -r in ipa-getkeytab. Right now I'm not even entirely sure how one would add a permission to do so. rob ATM the only way is to add the ipaAllowedOperations objectclass to the object you want to allow retrieving a keyt from and the ipaAllowedToPerform;reasd_key attribute Example: dn: test/foo.example@example.com,cn=services,cn=accounts,dc=example,dc=com changetype: modify add: objectclass objectclass: ipaAllowedOperations - add: ipaAllowedToPerform;read_key ipaAllowedToPerform;reasd_key: uid=cluster-admin,cn=users,cn=accounts,dc=example,dc=com Once you do this the user called cluster-admin will be allowed to retrieve the keytab w/o changing it. Of course you can list there a group or another host/service DN. Doh, I realized we haven't created a feature page for this, I am going to create one now, so that the UI work we'll need in future can look it up and information like the above is registered. Will be available here: http://www.freeipa.org/page/V4/Keytab_Retrieval 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] #3859: Better mechanism to retrieve keytabs
Rob Crittenden wrote: Simo Sorce wrote: On Wed, 2014-06-11 at 17:03 -0400, Rob Crittenden wrote: 0001 When is_allowed_to_access_attr() fails it should include the value of access in the error log for debugging. Ok added more detailed logging Nit: Coluld not fetch REALM backend Fixed There are still a ton of ber_scanf failed duplicated fatal errors. I'm fine keeping a common err_msg but the fatal error should be unique. Yeah thanks to this comment, I had a small change of heart. Instead of sending such detailed information to clients I reverted to send a little less information to the clients and instead LOG_FATAL in a more detailed way. HTH This breaks normal host delegation. If you add a host to another host's managedby, getting the keytab will fail. This is due to: [11/Jun/2014:16:56:45 -0400] NSACLPlugin - conn=4 op=3 (main): Deny write on entry(fqdn=client2.example.com,cn=computers,cn=accounts,dc=example,dc=com).attr(ipaProtectedOperation;write_keys) to fqdn=client1.example.com,cn=computers,cn=accounts,dc=example,dc=com: no aci matched the subject by aci(97): aciname= Groups allowed to create keytab keys, acidn=cn=accounts,dc=example,dc=com Ok this should be working now, I added a new ACI to allow also managedby#USERDN to operate on keytabs. New patches attached. Functionally these seem to work ok. I think there should be some documented way to enable the -r in ipa-getkeytab. Right now I'm not even entirely sure how one would add a permission to do so. NACK Simo noticed that the ACIs are in cn=accounts. On the one hand this is a reasonable place to put these, on the other it is a bit broad. I think we'll need type-specific ACIs in a number of subtrees: users, computers and services. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0053] Implement OTP token importing
I am CC'ing Simo because he wants to review my PBKDF2 implementation. On Wed, 2014-06-11 at 17:41 -0400, Nathaniel McCallum wrote: On Wed, 2014-06-11 at 14:24 +0200, Jan Cholasta wrote: Hi, On 13.5.2014 18:40, Nathaniel McCallum wrote: On Tue, 2014-05-13 at 12:38 -0400, Nathaniel McCallum wrote: This patch adds support for importing tokens using RFC 6030 key container files. This includes decryption support. For sysadmin sanity, any tokens which fail to add will be written to the output file for examination. The main use case here is where a small subset of a large set of tokens fails to validate or add. Using the output file, the sysadmin can attempt to recover these specific tokens. This code is implemented as a server-side script. However, it doesn't actually need to run on the server. This was done because importing is an odd fit for the IPA command framework: 1. We need to write an output file. 2. The operation may be long-running (thousands of tokens). 3. Only admins need to perform this task and it only happens infrequently. I forgot to put the link to the ticket in the commit message. Fixed. 1) I think you should initialize NSS in ipa_otptoken_import.py, not in the ipa-otptoken-import script. Fixed. 2) The pep8 tool reports a lot of errors in ipa_otptoken_import.py. Fixed (mostly). The remaining output from pep8 is, I think, entirely justifiable. 3) Other error messages are in the form message: %s, I think this one should use that form as well: +if encoding != 'DECIMAL': +raise ValidationError('Unsupported encoding (%s)!' % encoding) Fixed. 4) This is not right: +except: +self.log.warn(Error adding token: + str(sys.exc_info()[1])) I think it should be something like this instead: except ValidationError, e: self.log.warn(Error adding token: %s, e) Fixed. 5) There is no man page for ipa-otptoken-import. TODO (tomorrow). Fixed. 6) Output file is created even when ipa-otptoken-import fails with Unable to connect to LDAP! Did you kinit? and other initialization errors, which makes subsequent ipa-otptoken-import fail with Output file already exists!. Fixed. 7) When a key is specified by reference in Key/KeyReference instead of directly in Key/Data/Secret like in http://git.savannah.gnu.org/cgit/oath-toolkit.git/tree/pskctool/tests/pskc-figure4.xml, import fails with Key not found in token!. I would expect a different error message. This error is now: Referenced keys are not supported! 8) Importing http://git.savannah.gnu.org/cgit/oath-toolkit.git/tree/pskctool/tests/pskc-figure5.xml produces this output: /usr/lib/python2.7/site-packages/ipaserver/install/ipa_otptoken_import.py:307: FutureWarning: The behavior of this method will change in future versions. Use specific 'len(elem)' or 'elem is not None' test instead. if data.get('pinpolicy', None): Error adding token: 'NoneType' object has no attribute 'strip' This now states: Error adding token: PINPolicy policy not supported! Error adding token: Unsupported token type! 9) Using an arbitrary file in -k produces this output: (SEC_ERROR_INVALID_KEY) The key does not support the requested operation. Traceback (most recent call last): File /usr/sbin/ipa-otptoken-import, line 29, in module nss.nss_shutdown() nss.error.NSPRError: (SEC_ERROR_BUSY) NSS could not shutdown. Objects are still in use. What do you mean by arbitrary file? A file that is not the key? Like /dev/null? I'm not able to reproduce this. 10) Importing http://git.savannah.gnu.org/cgit/oath-toolkit.git/tree/pskctool/tests/pskc-figure7.xml and http://git.savannah.gnu.org/cgit/oath-toolkit.git/tree/pskctool/tests/pskc-figure8.xml produces this output: Error adding token: object of type 'NoneType' has no len() Import fails with: Derived keys are not currently supported! or X.509 keys are not currently supported! It would be nice to support these in the future. I added support for derived keys. However, pskc-figure7.xml appears to have a problem. This problem is actually in RFC 6030 itself (where pskc-figure7.xml comes from). According to the xenc11 standard, all of these tags should be in the xenc11 namespace (not pkcs5 or undefined as given in RFC 6030): pkcs5:PBKDF2-params Salt SpecifiedEj7/PEpyEpw=/Specified /Salt IterationCount1000/IterationCount KeyLength16/KeyLength PRF/ /pkcs5:PBKDF2-params When fixing this error in pskc-figure7.xml, key derivation works in this latest patch. 11) Importing http://git.savannah.gnu.org/cgit/oath-toolkit.git/tree/pskctool/tests/pskc-all.xml or
Re: [Freeipa-devel] [PATCH 0049] Add support for protected tokens
On Wed, 2014-06-11 at 12:43 -0400, Nathaniel McCallum wrote: On Wed, 2014-06-11 at 12:12 +0200, Ludwig Krispenz wrote: On 05/13/2014 04:33 PM, Jan Cholasta wrote: On 12.5.2014 21:02, Nathaniel McCallum wrote: On Thu, 2014-05-08 at 13:51 -0400, Simo Sorce wrote: On Thu, 2014-05-08 at 12:26 -0400, Nathaniel McCallum wrote: On Wed, 2014-05-07 at 11:17 -0400, Simo Sorce wrote: On Wed, 2014-05-07 at 09:54 -0400, Dmitri Pal wrote: On 05/07/2014 09:05 AM, Nathaniel McCallum wrote: On Wed, 2014-05-07 at 11:42 +0200, Jan Cholasta wrote: Hi, On 6.5.2014 17:08, Nathaniel McCallum wrote: On Tue, 2014-05-06 at 09:49 -0400, Nathaniel McCallum wrote: On Mon, 2014-05-05 at 12:42 -0400, Nathaniel McCallum wrote: This also constitutes a rethinking of the token ACIs after the introduction of SELFDN support. Admins, as before, have full access to all token permissions. Normal users have read/search/compare access to all of the non-secret data for tokens assigned to them, whether protected or non-protected. Users can add or delete non-protected tokens and modify most of their metadata. However they cannot create, delete or modify protected tokens. Regardless of whether the token is protected or not, users cannot change a token's ownership or unique identity. In contrast, admins can create protected tokens. This protects the token from deletion or modification when assigned to users. Additionally, when a user account is deleted, the assigned non-protected tokens are deleted but the protected tokens are merely orphaned. This permits the token to be reassigned without having to recreate it. This last point is particularly useful in the case of hardware tokens. https://fedorahosted.org/freeipa/ticket/4228 NOTE: This patch depends on my patch 0048. This new version makes ipatokenDisabled visible for token owners. It is also writable if the token is non-protected. This additionally fixes: https://fedorahosted.org/freeipa/ticket/4259 This new version changes the way the default value of protected is setup in accordance with the changes made for the review of my patch 0048.2. Nathaniel Is using the ipatokenprotected attribute the final design? No. Alternate designs are welcome. The code is easy enough to modify. I did not dig too deep into this, but I think it might be better to instead use the managedby attribute on a token to limit who can delete (or administer in other way) the token. On otptoken-add, managedby would be set to the whoami user DN, unless run with --protected, in which case managedby would be left empty. Then, when deleting a user, the token would be deleted only if the user manages the token. It seems to me that the mechanics of this are roughly the same as protected, just with a different syntax. The cost of this is more complex ACIs. In particular, we'd have to use two userdn clauses (is this possible?) instead of a simple filter. If there is a clear benefit, we can justify the more obtuse syntax. We usually try not to create new attributes until it is fully justified. I would like Simo to chime in on this. I would also prefer to reuse existing attributes and mechanism if possible and if it will not preclude future work. In this case, it sounds like managed-by has the appropriate meaning: who manages the token ? - myself, admin, other, none ? Nathaniel can you send 2 lines showing the difference in ACIs between using managed-by vs a new attribute ? These are the ACIs using the protected mechanism: aci: (targetfilter = (objectClass=ipaToken))(targetattrs = objectclass || description || ipatokenUniqueID || ipatokenDisabled || ipatokenNotBefore || ipatokenNotAfter || ipatokenVendor || ipatokenModel || ipatokenSerial || ipatokenOwner || ipatokenProtected)(version 3.0; acl Users can read basic token info; allow (read, search, compare) userattr = ipatokenOwner#USERDN;) aci: (targetfilter = (objectClass=ipatokenTOTP))(targetattrs = ipatokenOTPalgorithm || ipatokenOTPdigits || ipatokenTOTPtimeStep)(version 3.0; acl Users can see TOTP details; allow (read, search, compare) userattr = ipatokenOwner#USERDN;) aci: (targetfilter = (objectClass=ipatokenHOTP))(targetattrs = ipatokenOTPalgorithm || ipatokenOTPdigits)(version 3.0; acl Users can see HOTP details; allow (read, search, compare) userattr = ipatokenOwner#USERDN;) aci: (targetfilter = ((objectClass=ipaToken)(!(ipatokenProtected=TRUE(targetattrs = description || ipatokenDisabled || ipatokenNotBefore || ipatokenNotAfter || ipatokenVendor || ipatokenModel || ipatokenSerial)(version 3.0; acl Users can write basic token info; allow (write) userattr = ipatokenOwner#USERDN;) aci: (target
Re: [Freeipa-devel] [PATCH] #3859: Better mechanism to retrieve keytabs
On Fri, 2014-06-13 at 14:29 -0400, Rob Crittenden wrote: Rob Crittenden wrote: Simo Sorce wrote: On Wed, 2014-06-11 at 17:03 -0400, Rob Crittenden wrote: 0001 When is_allowed_to_access_attr() fails it should include the value of access in the error log for debugging. Ok added more detailed logging Nit: Coluld not fetch REALM backend Fixed There are still a ton of ber_scanf failed duplicated fatal errors. I'm fine keeping a common err_msg but the fatal error should be unique. Yeah thanks to this comment, I had a small change of heart. Instead of sending such detailed information to clients I reverted to send a little less information to the clients and instead LOG_FATAL in a more detailed way. HTH This breaks normal host delegation. If you add a host to another host's managedby, getting the keytab will fail. This is due to: [11/Jun/2014:16:56:45 -0400] NSACLPlugin - conn=4 op=3 (main): Deny write on entry(fqdn=client2.example.com,cn=computers,cn=accounts,dc=example,dc=com).attr(ipaProtectedOperation;write_keys) to fqdn=client1.example.com,cn=computers,cn=accounts,dc=example,dc=com: no aci matched the subject by aci(97): aciname= Groups allowed to create keytab keys, acidn=cn=accounts,dc=example,dc=com Ok this should be working now, I added a new ACI to allow also managedby#USERDN to operate on keytabs. New patches attached. Functionally these seem to work ok. I think there should be some documented way to enable the -r in ipa-getkeytab. Right now I'm not even entirely sure how one would add a permission to do so. NACK Simo noticed that the ACIs are in cn=accounts. On the one hand this is a reasonable place to put these, on the other it is a bit broad. I think we'll need type-specific ACIs in a number of subtrees: users, computers and services. [Only patch 3 attached, as none of the others changed, and addiotional discussion is needed, see below.] Ok after looking carefully into this problem I see that there are really 2 issues. 1) managedby for users, we do not want someone adding a managedby attribute to inadvertently allow the manager to set a user's password. So I changed that ACI and restricted it only to ipaHost and ipaService objects (tested). I haven't touched any other ACI because in order to use them you need to have intentionally added an ipaAllowedToPerform attribute to the user entry. 2) and I think this is a MUCH bigger issue, the Admin users are unbounded and pass any Access Control Check and this means they can now retrieve any key for users or machines. It is already bad enough that admins can unconditionally set any key, but this at least leaves back a pretty big trail (the original client password/key fails to work), and is a necessary evil (password resets, hosts creation/recovery). But I am not very comfortable with the idea an admin can retrieve any key without actually ending up changing it. Petr do we have any short term plan to address the Admin's super ACI ? Otherwise we can add ipaProtectedOperation in the exclude list for the superACI (Admins can manage any entry) and add the following ACI in cn=accounts, to restore admin ability to set keys (but not retrieve them): aci: (targetattr=ipaProtectedOperation;write_keys)(version 3.0; acl Admins are allowed to rekey any entity; allow(write) groupdn=ldap :///cn=admins,cn=groups,cn=accounts,$SUFFIX;) I tested this combination and it effectively stops admin from retrieving all keys unless explicitly authorize by positive ACIs/ipaAllowedToPerform attributes. Thoughts ? -- Simo Sorce * Red Hat, Inc * New York From e3a19bf910f2e2bbaabb870471045597469e790e Mon Sep 17 00:00:00 2001 From: Simo Sorce s...@redhat.com Date: Tue, 17 Sep 2013 00:30:14 -0400 Subject: [PATCH 3/6] keytab: Add new extended operation to get a keytab. This new extended operation allow to create new keys or retrieve existing ones. The new set of keys is returned as a ASN.1 structure similar to the one that is passed in by the 'set keytab' extended operation. Access to the operation is regulated through a new special ACI that allows 'retrieval' only if the user has access to an attribute named ipaProtectedOperation postfixed by the subtypes 'read_keys' and 'write_keys' to distinguish between creation and retrieval operation. For example for allowing retrieval by a specific user the following ACI is set on cn=accounts: (targetattr=ipaProtectedOperation;read_keys) ... ... userattr=ipaAllowedToPerform;read_keys#USERDN) This ACI matches only if the service object hosts a new attribute named ipaAllowedToPerform that holds the DN of the user attempting the operation. Resolves: https://fedorahosted.org/freeipa/ticket/3859 --- .../ipa-pwd-extop/ipa_pwd_extop.c | 571 + install/share/60basev3.ldif| 3 + install/share/default-aci.ldif | 8 +- install/updates/20-aci.update
Re: [Freeipa-devel] [PATCH] #3859: Better mechanism to retrieve keytabs
On 06/13/2014 10:20 PM, Simo Sorce wrote: On Fri, 2014-06-13 at 14:29 -0400, Rob Crittenden wrote: Rob Crittenden wrote: Simo Sorce wrote: On Wed, 2014-06-11 at 17:03 -0400, Rob Crittenden wrote: 0001 When is_allowed_to_access_attr() fails it should include the value of access in the error log for debugging. Ok added more detailed logging Nit: Coluld not fetch REALM backend Fixed There are still a ton of ber_scanf failed duplicated fatal errors. I'm fine keeping a common err_msg but the fatal error should be unique. Yeah thanks to this comment, I had a small change of heart. Instead of sending such detailed information to clients I reverted to send a little less information to the clients and instead LOG_FATAL in a more detailed way. HTH This breaks normal host delegation. If you add a host to another host's managedby, getting the keytab will fail. This is due to: [11/Jun/2014:16:56:45 -0400] NSACLPlugin - conn=4 op=3 (main): Deny write on entry(fqdn=client2.example.com,cn=computers,cn=accounts,dc=example,dc=com).attr(ipaProtectedOperation;write_keys) to fqdn=client1.example.com,cn=computers,cn=accounts,dc=example,dc=com: no aci matched the subject by aci(97): aciname= Groups allowed to create keytab keys, acidn=cn=accounts,dc=example,dc=com Ok this should be working now, I added a new ACI to allow also managedby#USERDN to operate on keytabs. New patches attached. Functionally these seem to work ok. I think there should be some documented way to enable the -r in ipa-getkeytab. Right now I'm not even entirely sure how one would add a permission to do so. NACK Simo noticed that the ACIs are in cn=accounts. On the one hand this is a reasonable place to put these, on the other it is a bit broad. I think we'll need type-specific ACIs in a number of subtrees: users, computers and services. [Only patch 3 attached, as none of the others changed, and addiotional discussion is needed, see below.] Ok after looking carefully into this problem I see that there are really 2 issues. 1) managedby for users, we do not want someone adding a managedby attribute to inadvertently allow the manager to set a user's password. So I changed that ACI and restricted it only to ipaHost and ipaService objects (tested). I haven't touched any other ACI because in order to use them you need to have intentionally added an ipaAllowedToPerform attribute to the user entry. 2) and I think this is a MUCH bigger issue, the Admin users are unbounded and pass any Access Control Check and this means they can now retrieve any key for users or machines. It is already bad enough that admins can unconditionally set any key, but this at least leaves back a pretty big trail (the original client password/key fails to work), and is a necessary evil (password resets, hosts creation/recovery). But I am not very comfortable with the idea an admin can retrieve any key without actually ending up changing it. Petr do we have any short term plan to address the Admin's super ACI ? Otherwise we can add ipaProtectedOperation in the exclude list for the superACI (Admins can manage any entry) and add the following ACI in cn=accounts, to restore admin ability to set keys (but not retrieve them): aci: (targetattr=ipaProtectedOperation;write_keys)(version 3.0; acl Admins are allowed to rekey any entity; allow(write) groupdn=ldap :///cn=admins,cn=groups,cn=accounts,$SUFFIX;) I tested this combination and it effectively stops admin from retrieving all keys unless explicitly authorize by positive ACIs/ipaAllowedToPerform attributes. Thoughts ? Just a heads up, I managed to catch a typo with my sleepy eyes: --- a/install/share/default-aci.ldif +++ b/install/share/default-aci.ldif @@ -21,11 +21,17 @@ changetype: modify add: aci aci: (targetfilter = (|(objectClass=ipaConfigObject)(dnahostname=*)))(version 3.0;acl Admins can change GUI config; allow (delete) groupdn = ldap:///cn=admins,cn=groups,cn=accounts,$SUFFIX;;) -dn: cn=accounts,$SUFFIX +dn: cngaccounts,$SUFFIX ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel -- Tomas Babej Associate Software Engineer | Red Hat | Identity Management RHCE | Brno Site | IRC: tbabej | freeipa.org ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0053] Implement OTP token importing
On Fri, 2014-06-13 at 15:39 -0400, Nathaniel McCallum wrote: I am CC'ing Simo because he wants to review my PBKDF2 implementation. Thank you. I am a bit concerned you are using etree.parse(self.args[0]) directly with the default parser configuration. I think we should, at least, do something like this: parser = etree.XMLParser(resolve_entities=False) parser.parse(self.args[0]) We wouldn't want to inadvertently hit the network when reading this xml file, would we ? Reading the PBKDF2KeyDerivation code I see no particular issue, although I found it a bit hard to decod what it was doing as there are no comments, nor a direct reference to the algorithm you are implementing. It would be nice to have comments either before the function or within the function that gives an explanation of the algorithm being implemented so that whoever needs to read this in the future can readily understand what is going on. I've used this as reference to refresh my memory on the algorithm: http://en.wikipedia.org/wiki/PBKDF2 Btw for the final join, wouldn't it be more compact to use: dk += ''.join(map(chr,u)) ? Actually this creates a new string at each iteration... if you declare dk = [] before the loop and dk.append(''.join(map(chr, u))) in the loop. Then you can return ''.join(dk)[:self.klen] and build the final string only once. Or given you already use lambdas in there perhaps simplify even to: dk = [] for i ... ... dk.append(u) return ''.join(map(lambda x: ''.join(map(chr, x)), dk))[:self.klen] In general the flow is a bit hard to follow due to the clever use of map(), maybe a few comments sprinkled before functions about who/how is meant to use them would help the casual observer. Other than the first point though, anything else looks good to me. Simo. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel