Re: [Freeipa-devel] [PATCH] 0751 spec: Split out python-ipap11helper and, python-default_encoding_utf8

2016-01-22 Thread Petr Viktorin
On 01/21/2016 01:14 PM, Jan Cholasta wrote:
> We got rid of both default_encoding_utf8 and _ipap11helper, so
> python-ipalib can be packaged as noarch. See the attached patch.

The patch looks good to me, so ACK (though an ACK for me probably
doesn't count).

-- 
Petr Viktorin

-- 
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] 948 stop installer when setup-ds.pl fail

2016-01-22 Thread Martin Babinsky

On 01/21/2016 07:28 PM, Petr Vobornik wrote:


Petr Vobornik

ACK.

--
Martin^3 Babinsky

--
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 540] cert renewal: import all external CA certs on IPA CA cert renewal

2016-01-22 Thread Martin Babinsky

On 01/21/2016 10:27 AM, Jan Cholasta wrote:

Hi,

the attached patch fixes .

Honza




ACK

--
Martin^3 Babinsky

--
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 540] cert renewal: import all external CA certs on IPA CA cert renewal

2016-01-22 Thread Jan Cholasta

On 22.1.2016 10:34, Martin Babinsky wrote:

On 01/21/2016 10:27 AM, Jan Cholasta wrote:

Hi,

the attached patch fixes .

Honza




ACK


Self-NACK. Doesn't work with external CA install.

--
Jan Cholasta

--
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 0030] Modernize mod_nss's cipher suites

2016-01-22 Thread Martin Kosek

On 01/21/2016 04:21 PM, Christian Heimes wrote:

The list of supported TLS cipher suites in /etc/httpd/conf.d/nss.conf
has been modernized. Insecure or less secure algorithms such as RC4,
DES and 3DES are removed. Perfect forward secrecy suites with ephemeral
ECDH key exchange have been added. IE 8 on Windows XP is no longer
supported.

The list of enabled cipher suites has been generated with the script
contrib/nssciphersuite/nssciphersuite.py.

The supported suites are currently:

TLS_RSA_WITH_AES_128_CBC_SHA256
TLS_RSA_WITH_AES_256_CBC_SHA256
TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256
TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA
TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384
TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA
TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256
TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA
TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384
TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA
TLS_RSA_WITH_AES_128_GCM_SHA256
TLS_RSA_WITH_AES_128_CBC_SHA
TLS_RSA_WITH_AES_256_GCM_SHA384
TLS_RSA_WITH_AES_256_CBC_SHA

https://fedorahosted.org/freeipa/ticket/5589


Thanks for the patch! I updated the ticket to make sure this change is release 
notes.


--
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 0011-0012][RFE] ipa-replica-manage: automatically clean dangling RUVs

2016-01-22 Thread Martin Basti



On 15.01.2016 13:47, Stanislav Laznicka wrote:


On 01/14/2016 04:59 PM, Petr Vobornik wrote:

On 01/14/2016 04:16 PM, Ludwig Krispenz wrote:


On 01/14/2016 03:59 PM, Stanislav Laznicka wrote:

On 01/14/2016 03:21 PM, Rob Crittenden wrote:

Stanislav Laznicka wrote:

Please see the rebased patches attached.

On 01/13/2016 02:01 PM, Martin Basti wrote:


On 18.12.2015 12:46, Stanislav Laznicka wrote:

Hi,

Attached are the patches for auto-find and clean of dangling
(cs)ruvs. Currently, the cleaning of an RUV waits for all 
replicas to

be online, even on --force. If that were an issue, I can make the
command fail before trying to clean any of RUVs. However, the 
user is
shown a replica is offline and is prompted to confirm the 
cleaning so

the possible wait should not be a problem I believe.

Standa L.



Hello,

patches needs rebase, I cannot apply them.

Will this confuse people? Currently, for good or bad, there are two
commands for managing the two different topologies. This mixes 
some CA

work into ipa-replica-manage.

rob


Well, in the patch, I was just following the discussion at
https://fedorahosted.org/freeipa/ticket/5411. Ludwig mentions that
ipa-csreplica-manage should go deprecated and does not want to enhance
it. Also, the only thing the code does is removing trash from the ds
so it makes sense to me to do it in just one command, as well as the
users might expect that, too.

I guess it would be possible to add an option that would select which
of the subtrees should be cleaned of RUVs. It should stay as one
command nonetheless. Adding such an option for this command would then
probably mean all the commands should have it as it would make more
sense, though.

Let me add Petr and Ludwig to CC: as they both had inputs on keeping
the command in just ipa-replica-manage.
yes, that was the idea to keep ipa-csreplica-manage (which does not 
have

clean-ruv,..) for domain-level 0, but not add new features. Also
"ipa-replica-manage del" now triggers the ruv cleaning of ipaca



Yes, ipa-csreplica-manage should be deprecated.

I think that one of the reasons why dangling CA RUVs are not uncommon 
is that users forget about `ipa-csreplica-manage del` command when 
removing a replica.


New `ipa-replica-manage del` also removes replication agreements and 
therefore cleans RUVs of CA suffix (on domain level 1). In this 
context it is not inconsistent.


Btw, one of the good example why this commands will be helpful is 
following bz, especially a sentence in: 
https://bugzilla.redhat.com/show_bug.cgi?id=1295971#c5

"""
I had some mistakes to clean some valid RUV, for example, 52 for eupre1
"""

We should think about list-clean-ruv and abort-clean-ruv commands. 
There is no counterpart for CA suffix now. Could be in different patch.


With clean-dangling-ruvs command it would be good to deprecate 
clean-ruv command of ipa-replica-manage - should be different patch.


I'm not sure if it should abort if some replica is down. Maybe yes 
until https://fedorahosted.org/freeipa/ticket/5396 is fixed.


The path set misses update of man page.
Attached are the patches with the description for the man page. Abort 
of the clean-dangling-ruv operation on any replica offline status was 
also added.




Hello,

I have a few comments

PATCH Automatically detect and remove dangling RUVs

1)
+# get the Directory Manager password
+if options.dirman_passwd:
+dirman_passwd = options.dirman_passwd
+else:
+dirman_passwd = installutils.read_password('Directory Manager',
+confirm=False, validate=False, retry=False)
+if dirman_passwd is None:
+sys.exit('Directory Manager password is required')
+
+options.dirman_passwd = dirman_passwd

IMO you need only else branch here

if not options.dirman_password:
dirman_passwd = installutils.read_password('Directory Manager',
confirm=False, validate=False, retry=False)
if dirman_passwd is None:
sys.exit('Directory Manager password is required')
   options.dirman_passwd = dirman_passwd


2)
We should use new formatting in new code (more times in code)

+sys.exit(
+"Failed to get data from '%s' while trying to list 
replicas: %s" %

+(host, e)
+)

sys.exit(
"Failed to get data from '{host}' while trying to list 
replicas: {e}".format(

  host=host, e=e
)
)

3)
+# get all masters
+masters_dn = DN(('cn', 'masters'), ('cn', 'ipa'), ('cn', 'etc'),
+ipautil.realm_to_suffix(realm))

IMO you should use constants:
 masters_dn = DN(api.env.container_masters, api.env.basedn)

4)
+# Get realm string for config tree
+s = realm.split('.')
+s = ['dc={dc},'.format(dc=x.lower()) for x in s]
+realm_config = DN(('cn', ''.join(s)[0:-1]))

Can be api.env.basedn used instead of this block of code?

5)
+masters = [x.single_value['cn'] for x in masters]

+for 

Re: [Freeipa-devel] [PATCH 0031] ipatests: fix the install of external ca

2016-01-22 Thread Martin Babinsky

On 01/19/2016 05:56 PM, Milan Kubík wrote:

On 01/19/2016 05:31 PM, Milan Kubík wrote:

Patch attached.




This actually has a ticket opened. Patch with fixed commit message. ;)

--
Milan Kubik





Hi Milan,

for the step 1 installation I would rather reuse the 
tasks:install_master function which already does (nearly) all CLI 
option-related magic. You can extend its signature by adding a parameter 
to pass on additional options like this:


--- a/ipatests/test_integration/tasks.py
+++ b/ipatests/test_integration/tasks.py
@@ -258,7 +258,7 @@ def enable_replication_debugging(host):
  stdin_text=logging_ldif)


-def install_master(host, setup_dns=True, setup_kra=False):
+def install_master(host, setup_dns=True, setup_kra=False, extra_args=()):
 host.collect_log(paths.IPASERVER_INSTALL_LOG)
 host.collect_log(paths.IPACLIENT_INSTALL_LOG)
 inst = host.domain.realm.replace('.', '-')
@@ -284,6 +284,8 @@ def install_master(host, setup_dns=True, 
setup_kra=False):

 '--auto-reverse'
 ])

+args.extend(extra_args)
+
 host.run_command(args)
 enable_replication_debugging(host)
 setup_sssd_debugging(host)

--
Martin^3 Babinsky

--
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 0408] CI DNSSEC: add missing glue record

2016-01-22 Thread Martin Basti

Patch attached.
From 15e6c98420c9ffc7b840373910a06f251cca653a Mon Sep 17 00:00:00 2001
From: Martin Basti 
Date: Fri, 22 Jan 2016 14:23:53 +0100
Subject: [PATCH] Fix DNSSEC test: add glue record

Missing glue record causes test failure in cases when DNS zone was not
managed by IPA DNS
---
 ipatests/test_integration/test_dnssec.py | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/ipatests/test_integration/test_dnssec.py b/ipatests/test_integration/test_dnssec.py
index 65b1bdaf0e5dd4991b5f4dc1e8173f0fc8ad0537..111d39855b399e52052716cd83da8dfb2213fbee 100644
--- a/ipatests/test_integration/test_dnssec.py
+++ b/ipatests/test_integration/test_dnssec.py
@@ -275,7 +275,14 @@ class TestInstallDNSSECFirst(IntegrationTest):
 ]
 self.master.run_command(args)
 
-# make BIND happy, and delegate zone which contains A record of master
+# make BIND happy: add the glue record and delegate zone
+args = [
+"ipa", "dnsrecord-add", root_zone, self.master.domain.name,
+"--a-rec=" + self.master.ip
+]
+self.master.run_command(args)
+time.sleep(10)  # sleep a bit until data are provided by bind-dyndb-ldap
+
 args = [
 "ipa", "dnsrecord-add", root_zone, self.master.domain.name,
 "--ns-rec=" + self.master.hostname
-- 
2.5.0

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

Re: [Freeipa-devel] [PATCH] 0760 - Split ipa-client/ into ipaclient/ and client/

2016-01-22 Thread Petr Viktorin
On 01/14/2016 05:49 PM, Petr Viktorin wrote:
> On 01/14/2016 11:09 AM, Jan Cholasta wrote:
>> On 14.1.2016 10:48, Petr Viktorin wrote:
>>> On 01/14/2016 07:55 AM, Jan Cholasta wrote:
 Hi,

 On 13.1.2016 13:03, Martin Babinsky wrote:
> On 01/13/2016 11:34 AM, Petr Viktorin wrote:
>> Hello,
>> I'm planning to port the ipa-client to Python 3, and I'm likely to end
>> up shaking out some dusty corners of the codebase, rather than
>> doing the
>> minimal amount of work :)
>> So I'd like to get your opinions before I commit significant time to
>> this.
> 
> Here's a patch for review.
> (I'm sending the full diff for applying; the result is nicer to look at
> with `git show -C`)

Ping, was there any progress on the patch review?


-- 
Petr Viktorin

-- 
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