[Freeipa-devel] [freeipa PR#73][synchronized] Tests for certificates with SAN
URL: https://github.com/freeipa/freeipa/pull/73 Author: apophys Title: #73: Tests for certificates with SAN Action: synchronized To pull the PR as Git branch: git remote add ghfreeipa https://github.com/freeipa/freeipa git fetch ghfreeipa pull/73/head:pr73 git checkout pr73 From 909ab9fa6405acb346162508729cda8b56e08f9e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Milan=20Kub=C3=ADk?=Date: Mon, 12 Sep 2016 14:52:05 +0200 Subject: [PATCH 1/5] ipatests: provide context manager for keytab usage in RPC tests https://fedorahosted.org/freeipa/ticket/6291 --- ipatests/util.py | 52 +++- 1 file changed, 47 insertions(+), 5 deletions(-) diff --git a/ipatests/util.py b/ipatests/util.py index 8878993..4c1a77a 100644 --- a/ipatests/util.py +++ b/ipatests/util.py @@ -40,7 +40,9 @@ from ipalib.plugable import Plugin from ipalib.request import context from ipapython.dn import DN -from ipapython.ipautil import private_ccache, kinit_password, run +from ipapython.ipautil import ( +private_ccache, kinit_password, kinit_keytab, run +) from ipaplatform.paths import paths if six.PY3: @@ -693,8 +695,8 @@ def unlock_principal_password(user, oldpw, newpw): @contextmanager -def change_principal(user, password, client=None, path=None, - canonicalize=False, enterprise=False): +def change_principal(principal, password=None, client=None, path=None, + canonicalize=False, enterprise=False, keytab=None): if path: ccache_name = path @@ -709,8 +711,12 @@ def change_principal(user, password, client=None, path=None, try: with private_ccache(ccache_name): -kinit_password(user, password, ccache_name, - canonicalize=canonicalize, enterprise=enterprise) +if keytab: +kinit_keytab(principal, keytab, ccache_name) +else: +kinit_password(principal, password, ccache_name, + canonicalize=canonicalize, + enterprise=enterprise) client.Backend.rpcclient.connect() try: @@ -720,6 +726,42 @@ def change_principal(user, password, client=None, path=None, finally: client.Backend.rpcclient.connect() + +@contextmanager +def get_entity_keytab(principal, options=None): +"""Requests a keytab for an entity + +The keytab will generate new keys if not specified +otherwise in the options. +To retrieve existing keytab, use the -r option +""" +keytab_filename = os.path.join('/tmp', str(uuid.uuid4())) + +try: +cmd = [paths.IPA_GETKEYTAB, '-p', principal, '-k', keytab_filename] + +if options: +cmd.extend(options) +run(cmd) + +yield keytab_filename +finally: +os.remove(keytab_filename) + + +@contextmanager +def host_keytab(hostname, options=None): +"""Retrieves keytab for a particular host + +After leaving the context manager, the keytab file is +deleted. +""" +principal = u'host/{}'.format(hostname) + +with get_entity_keytab(principal, options) as keytab: +yield keytab + + def get_group_dn(cn): return DN(('cn', cn), api.env.container_group, api.env.basedn) From 7b962b358198559966f0f750edd9210a140b57c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Milan=20Kub=C3=ADk?= Date: Mon, 12 Sep 2016 14:53:48 +0200 Subject: [PATCH 2/5] ipatests: Fix name property on a service tracker https://fedorahosted.org/freeipa/ticket/6291 --- ipatests/test_xmlrpc/tracker/service_plugin.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ipatests/test_xmlrpc/tracker/service_plugin.py b/ipatests/test_xmlrpc/tracker/service_plugin.py index a0bb884..0a90115 100644 --- a/ipatests/test_xmlrpc/tracker/service_plugin.py +++ b/ipatests/test_xmlrpc/tracker/service_plugin.py @@ -52,7 +52,7 @@ class ServiceTracker(KerberosAliasMixin, Tracker): def __init__(self, name, host_fqdn, options=None): super(ServiceTracker, self).__init__(default_version=None) -self._name = "{0}/{1}@{2}".format(name, host_fqdn, api.env.realm) +self._name = u"{0}/{1}@{2}".format(name, host_fqdn, api.env.realm) self.dn = DN( ('krbprincipalname', self.name), api.env.container_service, api.env.basedn) From 941b2f8e7b0661837d6fb10bfd9a4d26c45158e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Milan=20Kub=C3=ADk?= Date: Mon, 12 Sep 2016 14:54:40 +0200 Subject: [PATCH 3/5] ipatests: Implement tests with CSRs requesting SAN The patch implements several test cases testing the enforcement of CA ACLs on certificate requests with subject alternative names. https://fedorahosted.org/freeipa/ticket/6291 --- freeipa.spec.in| 2 + .../test_xmlrpc/test_caacl_profile_enforcement.py | 240 - 2 files
Re: [Freeipa-devel] CA-less installs: passive certmonger - watch-and-warn mode
On 18.7.2016 08:22, Jan Cholasta wrote: > On 8.7.2016 15:59, Rob Crittenden wrote: >> Petr Spacek wrote: >>> On 8.7.2016 15:31, Rob Crittenden wrote: Petr Spacek wrote: > Hi, > > our docs > > https://access.redhat.com/documentation/en-US/Red_Hat_Enterprise_Linux/7/html/Linux_Domain_Identity_Authentication_and_Policy_Guide/install-server.html#install-determine-ca > > > > > claim this: > "The certmonger service is not used to track certificates. > Therefore, it does > not warn you of impending certificate expiration." > > Is this correct? > > Can we at least configure certmonger to passively track the > certificates and > throw warning about impending expiration into logs? > > +1, I have already suggested we do this several times. > > Throw a warning where? Register an e-mail address as part of the tracking perhaps? It would probably be fairly easy to write a "CA" that sends an e-mail. The trick, and this has always tripped us up, is having an MTA configured. >>> >>> I would start with logs, as I wrote in the original message. This will >>> naturally evolve into something else when we finally get >>> user-configurable hooks. >>> >>> In any case, having certmonger configured to track the certs is >>> prerequisite >>> for all cases... >> >> "Logs" is not very specific, do you mean syslog/journal? >> >> Feel free to open an RFE against certmonger with your proposal. I >> suspect that anything logged will just get lost in most cases. Finally, here is the ticket: https://fedorahosted.org/certmonger/ticket/59 > For IPA CA certificate, we log warnings to syslog with ALERT level. I think > doing that for other certs would be good enough for starters. -- Petr^2 Spacek -- 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
[Freeipa-devel] [freeipa PR#114][comment] Raise errors from service.py:_ldap_mod() by default
URL: https://github.com/freeipa/freeipa/pull/114 Title: #114: Raise errors from service.py:_ldap_mod() by default mbasti-rh commented: """ This issues are caused mostly by newer replica install, so I don't think that earlier devel cycle will help us , we need good upgrade testing. However I agree that is better to stop with first error and not continou later and break things even more """ See the full comment at https://github.com/freeipa/freeipa/pull/114#issuecomment-249827802 -- 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
Re: [Freeipa-devel] What would break if loopback addresses were allowed for IPA server?
On Wed, Sep 21, 2016 at 12:01:44PM +0200, Jan Pazdziora wrote: > > I've recently hit again the situation of IPA installer not happy > about the provided IP address not being local to it, this time in > containerized environment: > > https://bugzilla.redhat.com/show_bug.cgi?id=1377973 > > During the discussion, we came to an interesting question: > > What would break if loopback addresses were allowed for IPA > server? > > Of course, the idea is that it would only be used for installation and > then IPA would change its IP address in DNS to whatever is the real IP > address under which it is accessible. > > Where does the allow_loopback=False requirement in the installer come > from and what would break if it was removed altogether? I also see messages like Adding [10.11.12.13 ipa.example.com] to your /etc/hosts file in some cases. Actually, it's 10.11.12.13 ipa.example.com ipa which gets added so the message is not accurate. Modification of /etc/hosts itself seems unfortunate. Should the IP address change in the future, there will be one more place where the IP address stays hardcoded. I wonder why hosts: files dns myhostname isn't enough, and whether hosts: files myhostname dns might actually be better order. When the value is not in /etc/hosts, I see weird startup issues, presumably because individual components time out resolving $HOSTNAME, so systemctl start ipa fails. Perhaps it has something to do with named being up at that point, rather than unreachable, just not resolving anything yet. Chicken and egg. I wonder why we cannot add ipa.example.com to 127.0.0.1. I've tried that and have seen named-pkcs11[453]: LDAP error: Local error: SASL(-1): generic failure: GSSAPI Error: Unspecified GSS failure. Minor code may provide more information (Server ldap/localh...@example.test not found in Kerberos database): bind to LDAP server failed which suggests something derives the hostname and thus the principal from the IP address used. Why is not $HOSTNAME used everywhere? What part of the system cares about the IP address (and the reverse resolution)? If overloading 127.0.0.1 with the $HOSTNAME does not work, could 127.0.0.2 do the trick? It seems to work for subsequent starts (did not try it during ipa-server-install) in containers. -- Jan Pazdziora Senior Principal Software Engineer, Identity Management Engineering, Red Hat -- 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
[Freeipa-devel] [freeipa PR#122][opened] Acceptance tests
URL: https://github.com/freeipa/freeipa/pull/122 Author: dkupka Title: #122: Acceptance tests Action: opened PR body: """ Starting with minimal suite that will grow as necessary. """ To pull the PR as Git branch: git remote add ghfreeipa https://github.com/freeipa/freeipa git fetch ghfreeipa pull/122/head:pr122 git checkout pr122 From c4f25ae2c78584299c80894b62253372eea4f309 Mon Sep 17 00:00:00 2001 From: David KupkaDate: Wed, 21 Sep 2016 13:09:19 +0200 Subject: [PATCH 1/2] tests: Mark Dogtag acceptance tests --- ipatests/pytest.ini| 1 + ipatests/test_integration/test_installation.py | 21 + 2 files changed, 22 insertions(+) diff --git a/ipatests/pytest.ini b/ipatests/pytest.ini index 233cf43..277e26d 100644 --- a/ipatests/pytest.ini +++ b/ipatests/pytest.ini @@ -23,3 +23,4 @@ addopts = --doctest-modules markers = tier0: basic unit tests and critical functionality tier1: functional API tests +cs_acceptance: Acceptance test suite for Dogtag Certificate Server diff --git a/ipatests/test_integration/test_installation.py b/ipatests/test_integration/test_installation.py index c3d194f..868e8e7 100644 --- a/ipatests/test_integration/test_installation.py +++ b/ipatests/test_integration/test_installation.py @@ -7,6 +7,7 @@ installed. """ +import pytest from ipatests.test_integration.base import IntegrationTest from ipatests.test_integration import tasks @@ -196,3 +197,23 @@ def test_install_master(self): def test_install_kra(self): tasks.install_kra(self.master, first_instance=True) + + +@pytest.mark.cs_acceptance +class TestCSAcceptance(IntegrationTest): +num_replicas = 2 + +@classmethod +def install(cls, mh): +tasks.install_master(cls.master, setup_dns=True) + +def test_install_master_kra(self): +tasks.install_kra(self.master, first_instance=True) + +def test_install_replica0_with_ca(self): +tasks.install_replica(self.master, self.replicas[0], setup_ca=True, + setup_kra=False, setup_dns=False) + +def test_install_replica1_with_ca_kra(self): +tasks.install_replica(self.master, self.replicas[1], setup_ca=True, + setup_kra=True, setup_dns=False) From b63cb30e0f48b0b94a52b909372f924d17d2ab3c Mon Sep 17 00:00:00 2001 From: David Kupka Date: Wed, 21 Sep 2016 13:09:43 +0200 Subject: [PATCH 2/2] tests: Mark 389-ds acceptance tests --- ipatests/pytest.ini| 1 + ipatests/test_integration/test_installation.py | 13 + 2 files changed, 14 insertions(+) diff --git a/ipatests/pytest.ini b/ipatests/pytest.ini index 277e26d..11198dd 100644 --- a/ipatests/pytest.ini +++ b/ipatests/pytest.ini @@ -24,3 +24,4 @@ markers = tier0: basic unit tests and critical functionality tier1: functional API tests cs_acceptance: Acceptance test suite for Dogtag Certificate Server +ds_acceptance: Acceptance test suite for 389 Directory Server diff --git a/ipatests/test_integration/test_installation.py b/ipatests/test_integration/test_installation.py index 868e8e7..d4aba00 100644 --- a/ipatests/test_integration/test_installation.py +++ b/ipatests/test_integration/test_installation.py @@ -199,6 +199,19 @@ def test_install_kra(self): tasks.install_kra(self.master, first_instance=True) +@pytest.mark.ds_acceptance +class TestDSAcceptance(IntegrationTest): +num_replicas = 1 + +@classmethod +def install(cls, mh): +tasks.install_master(cls.master, setup_dns=True) + +def test_install_replica0(self): +tasks.install_replica(self.master, self.replicas[0], setup_ca=False, + setup_kra=False, setup_dns=False) + + @pytest.mark.cs_acceptance class TestCSAcceptance(IntegrationTest): num_replicas = 2 -- 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
[Freeipa-devel] [freeipa PR#115][synchronized] Don't show traceback when ipa config file is not an absolute path
URL: https://github.com/freeipa/freeipa/pull/115 Author: tomaskrizek Title: #115: Don't show traceback when ipa config file is not an absolute path Action: synchronized To pull the PR as Git branch: git remote add ghfreeipa https://github.com/freeipa/freeipa git fetch ghfreeipa pull/115/head:pr115 git checkout pr115 From 278f0133cfe40d322455a7ecc5ce24d780c16aae Mon Sep 17 00:00:00 2001 From: Tomas KrizekDate: Tue, 27 Sep 2016 13:39:22 +0200 Subject: [PATCH] ipa: file path validation for config file option Convert the config file attribute of ipa command to an absolute path. Additionaly, check the file exists. https://fedorahosted.org/freeipa/ticket/6114 --- ipalib/plugable.py | 12 ++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/ipalib/plugable.py b/ipalib/plugable.py index af35f5b..27db8cb 100644 --- a/ipalib/plugable.py +++ b/ipalib/plugable.py @@ -494,6 +494,13 @@ def build_global_parser(self, parser=None, context=None): """ Add global options to an optparse.OptionParser instance. """ +def config_file_callback(option, opt, value, parser): +value = os.path.abspath(value) +if not os.path.isfile(value): +raise optparse.OptionValueError( +"%s option '%s' is not a file" % (opt, value)) +parser.values.conf = value + if parser is None: parser = optparse.OptionParser( add_help_option=False, @@ -517,8 +524,9 @@ def build_global_parser(self, parser=None, context=None): parser.add_option('-e', dest='env', metavar='KEY=VAL', action='append', help='Set environment variable KEY to VAL', ) -parser.add_option('-c', dest='conf', metavar='FILE', -help='Load configuration from FILE', +parser.add_option('-c', dest='conf', metavar='FILE', action='callback', +callback=config_file_callback, type='string', +help='Load configuration from FILE. This must be an absolute path', ) parser.add_option('-d', '--debug', action='store_true', help='Produce full debuging output', -- 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
[Freeipa-devel] [freeipa PR#121][comment] Pylint: enable unused-variable check
URL: https://github.com/freeipa/freeipa/pull/121 Title: #121: Pylint: enable unused-variable check mbasti-rh commented: """ I disagree, I really think that there should not be assert """ See the full comment at https://github.com/freeipa/freeipa/pull/121#issuecomment-249816977 -- 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
[Freeipa-devel] [freeipa PR#115][comment] Don't show traceback when ipa config file is not an absolute path
URL: https://github.com/freeipa/freeipa/pull/115 Title: #115: Don't show traceback when ipa config file is not an absolute path mbasti-rh commented: """ NACK, please see inline comments """ See the full comment at https://github.com/freeipa/freeipa/pull/115#issuecomment-249846654 -- 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
[Freeipa-devel] [freeipa PR#122][comment] Acceptance tests
URL: https://github.com/freeipa/freeipa/pull/122 Title: #122: Acceptance tests mbasti-rh commented: """ I quite disagree with marks, please read my inline comments """ See the full comment at https://github.com/freeipa/freeipa/pull/122#issuecomment-249858212 -- 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
[Freeipa-devel] [freeipa PR#123][opened] Tests: Remove silent deleting and creating entries by tracker
URL: https://github.com/freeipa/freeipa/pull/123 Author: mirielka Title: #123: Tests: Remove silent deleting and creating entries by tracker Action: opened PR body: """ https://fedorahosted.org/freeipa/ticket/6123 """ To pull the PR as Git branch: git remote add ghfreeipa https://github.com/freeipa/freeipa git fetch ghfreeipa pull/123/head:pr123 git checkout pr123 From 6fd6fcfc282675014e2a4704d4211a2f8e18d8c9 Mon Sep 17 00:00:00 2001 From: Lenka DoudovaDate: Tue, 27 Sep 2016 14:46:32 +0200 Subject: [PATCH] Tests: Remove silent deleting and creating entries by tracker https://fedorahosted.org/freeipa/ticket/6123 --- ipatests/test_xmlrpc/test_group_plugin.py | 4 ++-- ipatests/test_xmlrpc/test_stageuser_plugin.py | 1 + ipatests/test_xmlrpc/test_user_plugin.py | 2 ++ ipatests/test_xmlrpc/tracker/base.py | 5 - 4 files changed, 5 insertions(+), 7 deletions(-) diff --git a/ipatests/test_xmlrpc/test_group_plugin.py b/ipatests/test_xmlrpc/test_group_plugin.py index 27a8a33..2f824de 100644 --- a/ipatests/test_xmlrpc/test_group_plugin.py +++ b/ipatests/test_xmlrpc/test_group_plugin.py @@ -231,7 +231,7 @@ def test_search_for_all_groups_with_members(self, group, group2): def test_search_for_all_groups(self, group, group2): """ Search for all groups """ group.ensure_exists() -group2.create() +group2.ensure_exists() command = group.make_command('group_find') result = command() assert_deepequal(dict( @@ -631,7 +631,7 @@ class TestManagedGroupObjectclasses(XMLRPC_test): def test_check_objectclasses_after_detach(self, user, managed_group): """ Check objectclasses after user was detached from managed group """ # https://fedorahosted.org/freeipa/ticket/4909#comment:1 -user.create() +user.ensure_exists() user.run_command('group_detach', *[user.uid]) managed_group.retrieve(all=True) managed_group.add_member(dict(user=user.uid)) diff --git a/ipatests/test_xmlrpc/test_stageuser_plugin.py b/ipatests/test_xmlrpc/test_stageuser_plugin.py index 34cfaf8..4a859e8 100644 --- a/ipatests/test_xmlrpc/test_stageuser_plugin.py +++ b/ipatests/test_xmlrpc/test_stageuser_plugin.py @@ -255,6 +255,7 @@ def test_delete_stageduser(self, stageduser): stageduser.delete() def test_find_stageduser(self, stageduser): +stageduser.ensure_exists() stageduser.find() def test_findall_stageduser(self, stageduser): diff --git a/ipatests/test_xmlrpc/test_user_plugin.py b/ipatests/test_xmlrpc/test_user_plugin.py index 7c27abc..7508578 100644 --- a/ipatests/test_xmlrpc/test_user_plugin.py +++ b/ipatests/test_xmlrpc/test_user_plugin.py @@ -177,6 +177,7 @@ def test_rename_nonexistent(self, user, renameduser): class TestUser(XMLRPC_test): def test_retrieve(self, user): """ Create user and try to retrieve it """ +user.ensure_exists() user.retrieve() def test_delete(self, user): @@ -216,6 +217,7 @@ def test_remove_userclass(self, user): class TestFind(XMLRPC_test): def test_find(self, user): """ Basic check of user-find """ +user.ensure_exists() user.find() def test_find_with_all(self, user): diff --git a/ipatests/test_xmlrpc/tracker/base.py b/ipatests/test_xmlrpc/tracker/base.py index ecaadc6..a2b7406 100644 --- a/ipatests/test_xmlrpc/tracker/base.py +++ b/ipatests/test_xmlrpc/tracker/base.py @@ -199,7 +199,6 @@ def make_update_command(self, updates): def create(self): """Helper function to create an entry and check the result""" -self.ensure_missing() self.track_create() command = self.make_create_command() result = command() @@ -227,7 +226,6 @@ def check_create(self, result): def delete(self): """Helper function to delete a host and check the result""" -self.ensure_exists() self.track_delete() command = self.make_delete_command() result = command() @@ -244,7 +242,6 @@ def check_delete(self, result): def retrieve(self, all=False, raw=False): """Helper function to retrieve an entry and check the result""" -self.ensure_exists() command = self.make_retrieve_command(all=all, raw=raw) result = command() self.check_retrieve(result, all=all, raw=raw) @@ -255,7 +252,6 @@ def check_retrieve(self, result, all=False, raw=False): def find(self, all=False, raw=False): """Helper function to search for this hosts and check the result""" -self.ensure_exists() command = self.make_find_command(self.name, all=all, raw=raw) result = command() self.check_find(result, all=all, raw=raw) @@ -274,7 +270,6 @@ def update(self, updates, expected_updates=None): if expected_updates is None: expected_updates = {} -self.ensure_exists() command =
[Freeipa-devel] [freeipa PR#121][comment] Pylint: enable unused-variable check
URL: https://github.com/freeipa/freeipa/pull/121 Title: #121: Pylint: enable unused-variable check flo-renaud commented: """ Agree with you, ACK. """ See the full comment at https://github.com/freeipa/freeipa/pull/121#issuecomment-249822167 -- 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
[Freeipa-devel] [freeipa PR#114][comment] Raise errors from service.py:_ldap_mod() by default
URL: https://github.com/freeipa/freeipa/pull/114 Title: #114: Raise errors from service.py:_ldap_mod() by default mbasti-rh commented: """ This issues are caused mostly by newer replica install, so I don't think that earlier devel cycle will help us , we need good upgrade testing. However I agree that is better to stop with first error and not continue later and break things even more """ See the full comment at https://github.com/freeipa/freeipa/pull/114#issuecomment-249827802 -- 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
[Freeipa-devel] [freeipa PR#114][comment] Raise errors from service.py:_ldap_mod() by default
URL: https://github.com/freeipa/freeipa/pull/114 Title: #114: Raise errors from service.py:_ldap_mod() by default mbasti-rh commented: """ This issues are caused mostly by newer replica install, so I don't think that earlier devel cycle will help us, we need good upgrade testing. However I agree that is better to stop with first error and not continue and break things even more later """ See the full comment at https://github.com/freeipa/freeipa/pull/114#issuecomment-249827802 -- 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
[Freeipa-devel] [freeipa PR#121][+ack] Pylint: enable unused-variable check
URL: https://github.com/freeipa/freeipa/pull/121 Title: #121: Pylint: enable unused-variable check Label: +ack -- 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
[Freeipa-devel] [freeipa PR#121][comment] Pylint: enable unused-variable check
URL: https://github.com/freeipa/freeipa/pull/121 Title: #121: Pylint: enable unused-variable check stlaz commented: """ The latest changes fixed the nitpicks mentioned, ACK. Thanks :+1: """ See the full comment at https://github.com/freeipa/freeipa/pull/121#issuecomment-249830361 -- 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
[Freeipa-devel] [freeipa PR#121][closed] Pylint: enable unused-variable check
URL: https://github.com/freeipa/freeipa/pull/121 Author: mbasti-rh Title: #121: Pylint: enable unused-variable check Action: closed To pull the PR as Git branch: git remote add ghfreeipa https://github.com/freeipa/freeipa git fetch ghfreeipa pull/121/head:pr121 git checkout pr121 -- 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
[Freeipa-devel] [freeipa PR#121][+pushed] Pylint: enable unused-variable check
URL: https://github.com/freeipa/freeipa/pull/121 Title: #121: Pylint: enable unused-variable check Label: +pushed -- 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
[Freeipa-devel] [freeipa PR#121][comment] Pylint: enable unused-variable check
URL: https://github.com/freeipa/freeipa/pull/121 Title: #121: Pylint: enable unused-variable check mbasti-rh commented: """ Fixed upstream master: https://fedorahosted.org/freeipa/changeset/0f88f8fe889ae4801fc8d5ece1ad51c5246718ac https://fedorahosted.org/freeipa/changeset/9d83be3647547cfca4e129cfeb63771213232cf7 https://fedorahosted.org/freeipa/changeset/45e3aee35219c89c07d590003a334f8db658a3b2 """ See the full comment at https://github.com/freeipa/freeipa/pull/121#issuecomment-249840099 -- 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
[Freeipa-devel] [freeipa PR#115][comment] Don't show traceback when ipa config file is not an absolute path
URL: https://github.com/freeipa/freeipa/pull/115 Title: #115: Don't show traceback when ipa config file is not an absolute path pspacek commented: """ Why the file must be absolute? I would rather remove this requirement and be done with it. `open()` the file and if it succeeds - use it. If it fails, print error returned from `open`. """ See the full comment at https://github.com/freeipa/freeipa/pull/115#issuecomment-249854141 -- 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
[Freeipa-devel] [freeipa PR#117][synchronized] Make ipa-replica-install run in interactive mode
URL: https://github.com/freeipa/freeipa/pull/117 Author: stlaz Title: #117: Make ipa-replica-install run in interactive mode Action: synchronized To pull the PR as Git branch: git remote add ghfreeipa https://github.com/freeipa/freeipa git fetch ghfreeipa pull/117/head:pr117 git checkout pr117 From 30d1e65e23ca099f91f2c43f2d57127cc66c142c Mon Sep 17 00:00:00 2001 From: Stanislav LaznickaDate: Mon, 26 Sep 2016 12:43:24 +0200 Subject: [PATCH 1/2] replicainstall: don't assume default principal If --admin-password is set during ipa-replica-install but --principal is not, 'admin' is assumed. This is wrong and it's not advertised anywhere so fail instead. https://fedorahosted.org/freeipa/ticket/6068 --- ipaserver/install/server/replicainstall.py | 77 +++--- 1 file changed, 39 insertions(+), 38 deletions(-) diff --git a/ipaserver/install/server/replicainstall.py b/ipaserver/install/server/replicainstall.py index aefe158..65ea6bb 100644 --- a/ipaserver/install/server/replicainstall.py +++ b/ipaserver/install/server/replicainstall.py @@ -918,47 +918,48 @@ def install(installer): def ensure_enrolled(installer): -config = installer._config +# Prepare options for the installer script +args = [paths.IPA_CLIENT_INSTALL, "--no-ntp"] +stdin = None + +if installer.domain_name: +args.extend(["--domain", installer.domain_name]) +if installer.server: +args.extend(["--server", installer.server]) +if installer.realm_name: +args.extend(["--realm", installer.realm_name]) +if installer.host_name: +args.extend(["--hostname", installer.host_name]) +if installer.password: +args.extend(["--password", installer.password]) +else: +if installer.principal: +args.extend(["--principal", installer.principal]) +if installer.admin_password: +if installer.principal is None: +raise ScriptError("The --admin-password option must be used " + "with the --principal option.") +stdin = installer.admin_password +if installer.keytab: +args.extend(["--keytab", installer.keytab]) + +if installer.no_dns_sshfp: +args.append("--no-dns-sshfp") +if installer.ssh_trust_dns: +args.append("--ssh-trust-dns") +if installer.no_ssh: +args.append("--no-ssh") +if installer.no_sshd: +args.append("--no-sshd") +if installer.mkhomedir: +args.append("--mkhomedir") -# Call client install script -service.print_msg("Configuring client side components") try: +service.print_msg("Configuring client side components") +# Set _enrollment_performed to True so that any mess left behind in +# case of an enrollment failure gets cleaned installer._enrollment_performed = True - -args = [paths.IPA_CLIENT_INSTALL, "--unattended", "--no-ntp"] -stdin = None - -if installer.domain_name: -args.extend(["--domain", installer.domain_name]) -if installer.server: -args.extend(["--server", installer.server]) -if installer.realm_name: -args.extend(["--realm", installer.realm_name]) -if installer.host_name: -args.extend(["--hostname", installer.host_name]) - -if installer.password: -args.extend(["--password", installer.password]) -else: -if installer.admin_password: -# Always set principal if password was set explicitly, -# the password itself gets passed directly via stdin -args.extend(["--principal", installer.principal or "admin"]) -stdin = installer.admin_password -if installer.keytab: -args.extend(["--keytab", installer.keytab]) - -if installer.no_dns_sshfp: -args.append("--no-dns-sshfp") -if installer.ssh_trust_dns: -args.append("--ssh-trust-dns") -if installer.no_ssh: -args.append("--no-ssh") -if installer.no_sshd: -args.append("--no-sshd") -if installer.mkhomedir: -args.append("--mkhomedir") - +# Call client install script ipautil.run(args, stdin=stdin, redirect_output=True) print() except Exception: From 13c6d00733be4235b171348e00b06cb3387b025c Mon Sep 17 00:00:00 2001 From: Stanislav Laznicka Date: Mon, 26 Sep 2016 12:45:49 +0200 Subject: [PATCH 2/2] replicainstall: run client-install in attended mode by default Running ipa-client-install in unattended mode during enrollment process in ipa-replica-install only made everyone confused, run it in attended mode by default instead. https://fedorahosted.org/freeipa/ticket/6068 --- ipaserver/install/server/replicainstall.py | 9 ++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git
Re: [Freeipa-devel] [PATCH 0097] Properly handle LDAP socket closures in ipa-otpd
On ti, 27 syys 2016, Nathaniel McCallum wrote: In at least one case, when an LDAP socket closes, a read event is fired rather than an error event. Without this patch, ipa-otpd silently ignores this event and enters a state where all bind auths fail. To remedy this problem, we pass error events along the same path as read events. Should the actual read fail, we exit. Please add the bugzilla link. -- / Alexander Bokovoy -- 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
[Freeipa-devel] [freeipa PR#117][comment] Make ipa-replica-install run in interactive mode
URL: https://github.com/freeipa/freeipa/pull/117 Title: #117: Make ipa-replica-install run in interactive mode tomaskrizek commented: """ NACK, please see inline comments. """ See the full comment at https://github.com/freeipa/freeipa/pull/117#issuecomment-249814914 -- 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
[Freeipa-devel] [freeipa PR#124][opened] Fix: find OSCP certificate test
URL: https://github.com/freeipa/freeipa/pull/124 Author: mbasti-rh Title: #124: Fix: find OSCP certificate test Action: opened PR body: """ Test should check if any OSCP certificate has been returned https://fedorahosted.org/freeipa/ticket/6359 """ To pull the PR as Git branch: git remote add ghfreeipa https://github.com/freeipa/freeipa git fetch ghfreeipa pull/124/head:pr124 git checkout pr124 From d462bab999d037dff50207e3b8d05559202f727e Mon Sep 17 00:00:00 2001 From: Martin BastiDate: Tue, 27 Sep 2016 15:10:19 +0200 Subject: [PATCH] Fix: find OSCP certificate test Test should check if any OSCP certificate has been returned https://fedorahosted.org/freeipa/ticket/6359 --- ipatests/test_xmlrpc/test_cert_plugin.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/ipatests/test_xmlrpc/test_cert_plugin.py b/ipatests/test_xmlrpc/test_cert_plugin.py index f07bb17..4537002 100644 --- a/ipatests/test_xmlrpc/test_cert_plugin.py +++ b/ipatests/test_xmlrpc/test_cert_plugin.py @@ -267,7 +267,9 @@ def test_0003_find_OCSP(self): """ Search for the OCSP certificate. """ -api.Command['cert_find'](subject=u'OCSP Subsystem') +res = api.Command['cert_find'](subject=u'OCSP Subsystem') +assert 'count' in res +assert res['count'], "No OSCP certificate found" def test_0004_find_this_host(self): """ -- 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
[Freeipa-devel] [freeipa PR#73][synchronized] Tests for certificates with SAN
URL: https://github.com/freeipa/freeipa/pull/73 Author: apophys Title: #73: Tests for certificates with SAN Action: synchronized To pull the PR as Git branch: git remote add ghfreeipa https://github.com/freeipa/freeipa git fetch ghfreeipa pull/73/head:pr73 git checkout pr73 From 2159887dc5d01d3cf578d45825163e1add52e8a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Milan=20Kub=C3=ADk?=Date: Mon, 12 Sep 2016 14:52:05 +0200 Subject: [PATCH 1/5] ipatests: provide context manager for keytab usage in RPC tests https://fedorahosted.org/freeipa/ticket/6291 --- ipatests/util.py | 52 +++- 1 file changed, 47 insertions(+), 5 deletions(-) diff --git a/ipatests/util.py b/ipatests/util.py index 0b50f85..48a0faf 100644 --- a/ipatests/util.py +++ b/ipatests/util.py @@ -40,7 +40,9 @@ from ipalib.plugable import Plugin from ipalib.request import context from ipapython.dn import DN -from ipapython.ipautil import private_ccache, kinit_password, run +from ipapython.ipautil import ( +private_ccache, kinit_password, kinit_keytab, run +) from ipaplatform.paths import paths if six.PY3: @@ -693,8 +695,8 @@ def unlock_principal_password(user, oldpw, newpw): @contextmanager -def change_principal(user, password, client=None, path=None, - canonicalize=False, enterprise=False): +def change_principal(principal, password=None, client=None, path=None, + canonicalize=False, enterprise=False, keytab=None): if path: ccache_name = path @@ -709,8 +711,12 @@ def change_principal(user, password, client=None, path=None, try: with private_ccache(ccache_name): -kinit_password(user, password, ccache_name, - canonicalize=canonicalize, enterprise=enterprise) +if keytab: +kinit_keytab(principal, keytab, ccache_name) +else: +kinit_password(principal, password, ccache_name, + canonicalize=canonicalize, + enterprise=enterprise) client.Backend.rpcclient.connect() try: @@ -720,6 +726,42 @@ def change_principal(user, password, client=None, path=None, finally: client.Backend.rpcclient.connect() + +@contextmanager +def get_entity_keytab(principal, options=None): +"""Requests a keytab for an entity + +The keytab will generate new keys if not specified +otherwise in the options. +To retrieve existing keytab, use the -r option +""" +keytab_filename = os.path.join('/tmp', str(uuid.uuid4())) + +try: +cmd = [paths.IPA_GETKEYTAB, '-p', principal, '-k', keytab_filename] + +if options: +cmd.extend(options) +run(cmd) + +yield keytab_filename +finally: +os.remove(keytab_filename) + + +@contextmanager +def host_keytab(hostname, options=None): +"""Retrieves keytab for a particular host + +After leaving the context manager, the keytab file is +deleted. +""" +principal = u'host/{}'.format(hostname) + +with get_entity_keytab(principal, options) as keytab: +yield keytab + + def get_group_dn(cn): return DN(('cn', cn), api.env.container_group, api.env.basedn) From 9ed0d71a133fdd0d888f0b588a8e56e67b12e774 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Milan=20Kub=C3=ADk?= Date: Mon, 12 Sep 2016 14:53:48 +0200 Subject: [PATCH 2/5] ipatests: Fix name property on a service tracker https://fedorahosted.org/freeipa/ticket/6291 --- ipatests/test_xmlrpc/tracker/service_plugin.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ipatests/test_xmlrpc/tracker/service_plugin.py b/ipatests/test_xmlrpc/tracker/service_plugin.py index a0bb884..0a90115 100644 --- a/ipatests/test_xmlrpc/tracker/service_plugin.py +++ b/ipatests/test_xmlrpc/tracker/service_plugin.py @@ -52,7 +52,7 @@ class ServiceTracker(KerberosAliasMixin, Tracker): def __init__(self, name, host_fqdn, options=None): super(ServiceTracker, self).__init__(default_version=None) -self._name = "{0}/{1}@{2}".format(name, host_fqdn, api.env.realm) +self._name = u"{0}/{1}@{2}".format(name, host_fqdn, api.env.realm) self.dn = DN( ('krbprincipalname', self.name), api.env.container_service, api.env.basedn) From 2d75883302db07061f8062751aae392ece23bcf9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Milan=20Kub=C3=ADk?= Date: Mon, 12 Sep 2016 14:54:40 +0200 Subject: [PATCH 3/5] ipatests: Implement tests with CSRs requesting SAN The patch implements several test cases testing the enforcement of CA ACLs on certificate requests with subject alternative names. https://fedorahosted.org/freeipa/ticket/6291 --- freeipa.spec.in| 2 + .../test_xmlrpc/test_caacl_profile_enforcement.py | 240 - 2 files
[Freeipa-devel] [freeipa PR#115][synchronized] Don't show traceback when ipa config file is not an absolute path
URL: https://github.com/freeipa/freeipa/pull/115 Author: tomaskrizek Title: #115: Don't show traceback when ipa config file is not an absolute path Action: synchronized To pull the PR as Git branch: git remote add ghfreeipa https://github.com/freeipa/freeipa git fetch ghfreeipa pull/115/head:pr115 git checkout pr115 From d625b8071a828e283cad863958acc832b9a33da9 Mon Sep 17 00:00:00 2001 From: Tomas KrizekDate: Tue, 27 Sep 2016 17:23:17 +0200 Subject: [PATCH 1/2] ipa: allow relative paths for config file Remove unnecessary check for absolute file paths for config file. https://fedorahosted.org/freeipa/ticket/6114 --- ipalib/config.py | 15 +-- 1 file changed, 1 insertion(+), 14 deletions(-) diff --git a/ipalib/config.py b/ipalib/config.py index eb6c3ae..a273e3d 100644 --- a/ipalib/config.py +++ b/ipalib/config.py @@ -352,23 +352,10 @@ def _merge_from_file(self, config_file): containing first the number of variables that were actually set, and second the total number of variables found in ``config_file``. -This method will raise a ``ValueError`` if ``config_file`` is not an -absolute path. For example: - ->>> env = Env() ->>> env._merge_from_file('my/config.conf') -Traceback (most recent call last): - ... -ValueError: config_file must be an absolute path; got 'my/config.conf' - Also see `Env._merge()`. -:param config_file: Absolute path of the configuration file to load. +:param config_file: Path of the configuration file to load. """ -if path.abspath(config_file) != config_file: -raise ValueError( -'config_file must be an absolute path; got %r' % config_file -) if not path.isfile(config_file): return parser = RawConfigParser() From 5d4b4609d1b788d26703f07f5d9c7ad195162274 Mon Sep 17 00:00:00 2001 From: Tomas Krizek Date: Tue, 27 Sep 2016 17:23:38 +0200 Subject: [PATCH 2/2] ipa: check if provided config file exists Add a parser check to verify config file supplied to the ipa command exists. Previously, invalid file paths would not results in any error and would just silently proceed with default config. https://fedorahosted.org/freeipa/ticket/6114 --- ipalib/plugable.py | 13 +++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/ipalib/plugable.py b/ipalib/plugable.py index af35f5b..28c4042 100644 --- a/ipalib/plugable.py +++ b/ipalib/plugable.py @@ -44,6 +44,7 @@ from ipalib.util import classproperty from ipalib.base import ReadOnly, lock, islocked from ipalib.constants import DEFAULT_CONFIG +from ipapython import ipautil from ipapython.ipa_log_manager import ( log_mgr, LOGGING_FORMAT_FILE, @@ -494,6 +495,13 @@ def build_global_parser(self, parser=None, context=None): """ Add global options to an optparse.OptionParser instance. """ +def config_file_callback(option, opt, value, parser): +if not ipautil.file_exists(value): +raise optparse.OptionValueError( +_("%s: file not found") % value) + +parser.values.conf = value + if parser is None: parser = optparse.OptionParser( add_help_option=False, @@ -517,8 +525,9 @@ def build_global_parser(self, parser=None, context=None): parser.add_option('-e', dest='env', metavar='KEY=VAL', action='append', help='Set environment variable KEY to VAL', ) -parser.add_option('-c', dest='conf', metavar='FILE', -help='Load configuration from FILE', +parser.add_option('-c', dest='conf', metavar='FILE', action='callback', +callback=config_file_callback, type='string', +help='Load configuration from FILE.', ) parser.add_option('-d', '--debug', action='store_true', help='Produce full debuging output', -- 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
[Freeipa-devel] [freeipa PR#115][synchronized] Don't show traceback when ipa config file is not an absolute path
URL: https://github.com/freeipa/freeipa/pull/115 Author: tomaskrizek Title: #115: Don't show traceback when ipa config file is not an absolute path Action: synchronized To pull the PR as Git branch: git remote add ghfreeipa https://github.com/freeipa/freeipa git fetch ghfreeipa pull/115/head:pr115 git checkout pr115 From f445e300e4098da8ab10fd7952f57e421bc79f59 Mon Sep 17 00:00:00 2001 From: Tomas KrizekDate: Tue, 27 Sep 2016 17:05:48 +0200 Subject: [PATCH 1/2] ipa: check if provided config file exists Add a parser check to verify config file supplied to the ipa command exists. Previously, invalid file paths would not results in any error and would just silently proceed with default config. https://fedorahosted.org/freeipa/ticket/6114 --- ipalib/plugable.py | 11 +-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/ipalib/plugable.py b/ipalib/plugable.py index af35f5b..d45e9ee 100644 --- a/ipalib/plugable.py +++ b/ipalib/plugable.py @@ -494,6 +494,12 @@ def build_global_parser(self, parser=None, context=None): """ Add global options to an optparse.OptionParser instance. """ +def config_file_callback(option, opt, value, parser): +if not os.path.isfile(value): +raise optparse.OptionValueError( +"%s option '%s' is not a file" % (opt, value)) +parser.values.conf = value + if parser is None: parser = optparse.OptionParser( add_help_option=False, @@ -517,8 +523,9 @@ def build_global_parser(self, parser=None, context=None): parser.add_option('-e', dest='env', metavar='KEY=VAL', action='append', help='Set environment variable KEY to VAL', ) -parser.add_option('-c', dest='conf', metavar='FILE', -help='Load configuration from FILE', +parser.add_option('-c', dest='conf', metavar='FILE', action='callback', +callback=config_file_callback, type='string', +help='Load configuration from FILE.', ) parser.add_option('-d', '--debug', action='store_true', help='Produce full debuging output', From add8f1898c321d5b3bc1de86b14c75a8903ecf5e Mon Sep 17 00:00:00 2001 From: Tomas Krizek Date: Tue, 27 Sep 2016 17:09:55 +0200 Subject: [PATCH 2/2] ipa: allow relative paths for config file Remove unnecessary check for absolute file paths for config file. https://fedorahosted.org/freeipa/ticket/6114 --- ipalib/config.py | 15 +-- 1 file changed, 1 insertion(+), 14 deletions(-) diff --git a/ipalib/config.py b/ipalib/config.py index eb6c3ae..a273e3d 100644 --- a/ipalib/config.py +++ b/ipalib/config.py @@ -352,23 +352,10 @@ def _merge_from_file(self, config_file): containing first the number of variables that were actually set, and second the total number of variables found in ``config_file``. -This method will raise a ``ValueError`` if ``config_file`` is not an -absolute path. For example: - ->>> env = Env() ->>> env._merge_from_file('my/config.conf') -Traceback (most recent call last): - ... -ValueError: config_file must be an absolute path; got 'my/config.conf' - Also see `Env._merge()`. -:param config_file: Absolute path of the configuration file to load. +:param config_file: Path of the configuration file to load. """ -if path.abspath(config_file) != config_file: -raise ValueError( -'config_file must be an absolute path; got %r' % config_file -) if not path.isfile(config_file): return parser = RawConfigParser() -- 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
[Freeipa-devel] [freeipa PR#115][comment] Don't show traceback when ipa config file is not an absolute path
URL: https://github.com/freeipa/freeipa/pull/115 Title: #115: Don't show traceback when ipa config file is not an absolute path tomaskrizek commented: """ I found no reason why the path should be absolute, so I removed that constraint. The parser check to verify if file exists should remain, since non-existent files are otherwise ignored without any notice. """ See the full comment at https://github.com/freeipa/freeipa/pull/115#issuecomment-249901658 -- 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
[Freeipa-devel] [freeipa PR#115][comment] Don't show traceback when ipa config file is not an absolute path
URL: https://github.com/freeipa/freeipa/pull/115 Title: #115: Don't show traceback when ipa config file is not an absolute path mbasti-rh commented: """ NACK, please see my inline comments """ See the full comment at https://github.com/freeipa/freeipa/pull/115#issuecomment-249783814 -- 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
[Freeipa-devel] [PATCH 0097] Properly handle LDAP socket closures in ipa-otpd
In at least one case, when an LDAP socket closes, a read event is fired rather than an error event. Without this patch, ipa-otpd silently ignores this event and enters a state where all bind auths fail. To remedy this problem, we pass error events along the same path as read events. Should the actual read fail, we exit.From 43a8cd4f991115bcebcbe829b4b1be13849e288f Mon Sep 17 00:00:00 2001 From: Nathaniel McCallumDate: Tue, 27 Sep 2016 14:34:05 -0400 Subject: [PATCH] Properly handle LDAP socket closures in ipa-otpd In at least one case, when an LDAP socket closes, a read event is fired rather than an error event. Without this patch, ipa-otpd silently ignores this event and enters a state where all bind auths fail. To remedy this problem, we pass error events along the same path as read events. Should the actual read fail, we exit. --- daemons/ipa-otpd/bind.c | 10 -- daemons/ipa-otpd/query.c | 13 ++--- 2 files changed, 10 insertions(+), 13 deletions(-) diff --git a/daemons/ipa-otpd/bind.c b/daemons/ipa-otpd/bind.c index 022525b786705b4f58f861bc3b0a745ab8693755..a98312f906a785bfa9c98603a3577561552bfc0a 100644 --- a/daemons/ipa-otpd/bind.c +++ b/daemons/ipa-otpd/bind.c @@ -85,6 +85,9 @@ static void on_bind_readable(verto_ctx *vctx, verto_ev *ev) if (rslt <= 0) results = NULL; ldap_msgfree(results); +otpd_log_err(EIO, "IO error received on bind socket"); +verto_break(ctx.vctx); +ctx.exitstatus = 1; return; } @@ -137,11 +140,6 @@ void otpd_on_bind_io(verto_ctx *vctx, verto_ev *ev) flags = verto_get_fd_state(ev); if (flags & VERTO_EV_FLAG_IO_WRITE) on_bind_writable(vctx, ev); -if (flags & VERTO_EV_FLAG_IO_READ) +if (flags & (VERTO_EV_FLAG_IO_READ | VERTO_EV_FLAG_IO_ERROR)) on_bind_readable(vctx, ev); -if (flags & VERTO_EV_FLAG_IO_ERROR) { -otpd_log_err(EIO, "IO error received on bind socket"); -verto_break(ctx.vctx); -ctx.exitstatus = 1; -} } diff --git a/daemons/ipa-otpd/query.c b/daemons/ipa-otpd/query.c index 67e2d751d8d1511d077a93d7673439be11812e6f..50e15603322c550a0eb14e1e3c502e1a229d1ebe 100644 --- a/daemons/ipa-otpd/query.c +++ b/daemons/ipa-otpd/query.c @@ -133,7 +133,11 @@ static void on_query_readable(verto_ctx *vctx, verto_ev *ev) if (i != LDAP_RES_SEARCH_ENTRY && i != LDAP_RES_SEARCH_RESULT) { if (i <= 0) results = NULL; -goto egress; +ldap_msgfree(results); +otpd_log_err(EIO, "IO error received on query socket"); +verto_break(ctx.vctx); +ctx.exitstatus = 1; +return; } item = otpd_queue_pop_msgid(, ldap_msgid(results)); @@ -243,11 +247,6 @@ void otpd_on_query_io(verto_ctx *vctx, verto_ev *ev) flags = verto_get_fd_state(ev); if (flags & VERTO_EV_FLAG_IO_WRITE) on_query_writable(vctx, ev); -if (flags & VERTO_EV_FLAG_IO_READ) +if (flags & (VERTO_EV_FLAG_IO_READ | VERTO_EV_FLAG_IO_ERROR)) on_query_readable(vctx, ev); -if (flags & VERTO_EV_FLAG_IO_ERROR) { -otpd_log_err(EIO, "IO error received on query socket"); -verto_break(ctx.vctx); -ctx.exitstatus = 1; -} } -- 2.10.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
Re: [Freeipa-devel] [PATCH 0097] Properly handle LDAP socket closures in ipa-otpd
On Tue, 2016-09-27 at 14:54 -0400, Nathaniel McCallum wrote: > In at least one case, when an LDAP socket closes, a read event is > fired > rather than an error event. Without this patch, ipa-otpd silently > ignores this event and enters a state where all bind auths fail. > > To remedy this problem, we pass error events along the same path as > read events. Should the actual read fail, we exit. LGTM Simo. -- Simo Sorce * Red Hat, Inc * New York -- 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
[Freeipa-devel] [freeipa PR#121][comment] Pylint: enable unused-variable check
URL: https://github.com/freeipa/freeipa/pull/121 Title: #121: Pylint: enable unused-variable check stlaz commented: """ The changes seem fine except for the two small nitpicks. """ See the full comment at https://github.com/freeipa/freeipa/pull/121#issuecomment-249823191 -- 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
[Freeipa-devel] [freeipa PR#121][synchronized] Pylint: enable unused-variable check
URL: https://github.com/freeipa/freeipa/pull/121 Author: mbasti-rh Title: #121: Pylint: enable unused-variable check Action: synchronized To pull the PR as Git branch: git remote add ghfreeipa https://github.com/freeipa/freeipa git fetch ghfreeipa pull/121/head:pr121 git checkout pr121 From cd58bc977d4594795cd934d54c22af8115bdce56 Mon Sep 17 00:00:00 2001 From: Martin BastiDate: Mon, 26 Sep 2016 14:08:17 +0200 Subject: [PATCH 1/3] Remove unused variables in the code This commit removes unused variables or rename variables as "expected to be unused" by using "_" prefix. This covers only cases where fix was easy or only one unused variable was in a module --- daemons/dnssec/ipa-dnskeysync-replica | 2 +- doc/examples/examples.py| 2 +- install/oddjob/com.redhat.idm.trust-fetch-domains | 2 +- install/share/copy-schema-to-ca.py | 2 +- install/tools/ipa-csreplica-manage | 4 install/tools/ipa-dns-install | 2 +- install/tools/ipa-replica-conncheck | 2 +- ipaclient/ipa_certupdate.py | 2 +- ipaclient/plugins/automount.py | 6 +++--- ipaclient/plugins/location.py | 2 +- ipaclient/plugins/vault.py | 6 -- ipalib/aci.py | 2 -- ipalib/cli.py | 2 +- ipapython/dn.py | 9 - ipapython/dogtag.py | 2 +- ipapython/graph.py | 4 ++-- ipapython/install/cli.py| 2 +- ipapython/install/common.py | 4 ++-- ipapython/nsslib.py | 2 +- ipaserver/install/adtrustinstance.py| 4 ++-- ipaserver/install/installutils.py | 2 +- ipaserver/install/ipa_otptoken_import.py| 2 +- ipaserver/install/krbinstance.py| 2 +- ipaserver/install/ldapupdate.py | 2 +- ipaserver/install/ntpinstance.py| 2 +- ipaserver/install/odsexporterinstance.py| 2 -- ipaserver/install/plugins/adtrust.py| 4 ++-- ipaserver/install/plugins/dns.py| 1 - ipaserver/install/plugins/update_idranges.py| 2 +- ipaserver/install/plugins/update_managed_permissions.py | 2 +- ipaserver/install/plugins/update_passsync.py| 2 +- ipaserver/install/plugins/update_uniqueness.py | 2 +- ipaserver/install/plugins/upload_cacrt.py | 2 +- ipaserver/install/schemaupdate.py | 2 +- ipaserver/install/service.py| 4 ++-- ipaserver/plugins/config.py | 4 ++-- ipaserver/plugins/domainlevel.py| 2 +- ipaserver/plugins/group.py | 2 +- ipaserver/plugins/hbactest.py | 3 ++- ipaserver/plugins/host.py | 2 +- ipaserver/plugins/privilege.py | 2 +- ipaserver/plugins/selinuxusermap.py | 2 +- ipaserver/plugins/server.py | 2 +- ipaserver/plugins/service.py| 2 +- ipaserver/plugins/sudocmd.py| 2 +- ipaserver/session.py| 2 +- makeapi | 2 +- 47 files changed, 54 insertions(+), 69 deletions(-) diff --git a/daemons/dnssec/ipa-dnskeysync-replica b/daemons/dnssec/ipa-dnskeysync-replica index 69a3a68..fbfee93 100755 --- a/daemons/dnssec/ipa-dnskeysync-replica +++ b/daemons/dnssec/ipa-dnskeysync-replica @@ -49,7 +49,7 @@ def update_metadata_set(log, source_set, target_set): def find_unwrapping_key(log, localhsm, wrapping_key_uri): wrap_keys = localhsm.find_keys(uri=wrapping_key_uri) # find usable unwrapping key with matching ID -for key_id, key in wrap_keys.items(): +for key_id in wrap_keys.keys(): unwrap_keys = localhsm.find_keys(id=key_id, cka_unwrap=True) if len(unwrap_keys) > 0: return unwrap_keys.popitem()[1] diff --git a/doc/examples/examples.py b/doc/examples/examples.py index 0ecbf1e..3389230 100644 --- a/doc/examples/examples.py +++ b/doc/examples/examples.py @@ -415,7 +415,7 @@ def execute(self, *args, **options): # patter expects them in one dict. We need to arrange that. for e in entries: e[1]['dn'] = e[0] -entries = [e for (dn, e) in entries] +entries = [e for (_dn, e) in entries] return dict(result=entries, count=len(entries), truncated=truncated) diff --git