Re: [Freeipa-devel] [PATCH] 957 ipa-client-install: fix typo in nslcd service name

2016-04-21 Thread Rob Crittenden

Lukas Slebodnik wrote:

On (21/04/16 16:42), Rob Crittenden wrote:

Lukas Slebodnik wrote:

On (21/04/16 19:25), Petr Vobornik wrote:

related but does not implement https://fedorahosted.org/freeipa/ticket/5806
--
Petr Vobornik



 From b9b8716ec9ba5a5cdbed1f6cdedf7cff8878f08f Mon Sep 17 00:00:00 2001
From: Petr Vobornik 
Date: Thu, 21 Apr 2016 19:23:31 +0200
Subject: [PATCH] ipa-client-install: fix typo in nslcd service name

related but does not implement https://fedorahosted.org/freeipa/ticket/5806
---
client/ipa-client-install | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/client/ipa-client-install b/client/ipa-client-install
index 
c38843f85639a9118cd1a471709992690643d79a..0e6e65c4ad4ce01fe1257eee4bb2633a70c3de4e
 100755
--- a/client/ipa-client-install
+++ b/client/ipa-client-install
@@ -2938,7 +2938,7 @@ def install(options, env, fstore, statestore):
  nscd.service_name)

  nslcd = services.knownservices.nslcd
-if nscd.is_installed():
+if nslcd.is_installed():
  save_state(nslcd)


I thought that milestone "Future Releases" has lower priority
then "FreeIPA 4.4 Backlog"

Therefore I would prefer to close ticket #5806 and implement following one
https://fedorahosted.org/freeipa/ticket/5557#comment:2


I don't understand what you are suggesting. Tickets aren't swapped like this
and certainly non-related bugs aren't closed for another.

This patch just fixes an obvious one-liner as agreed upon in triage. The rest
will be potentially addressed later.

ACK on the patch.


I cannot see a reason to fix oneliner.
This code is not tested and should be removed (#5557)


I fail to see the controversy here. These are two completely and 
absolutely unrelated bugs.



Or someone should write integration test.

I'm sorry but conditional-NACK (missing integration test)


I've been out of the project a while, so perhaps my knowledge is stale, 
but AFAIU upstream QE writes the integration tests, not the developers. 
This was at least true when I was a daily contributor.


At least at one time QE did testing using --no-sssd. Whether that is 
still the case I don't know but I do remember fixing bugs around it. And 
this particular bug might not be immediately apparent unless a specific 
configuration was present. I noticed the bug while reading code, not in 
production.


rob

--
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] 957 ipa-client-install: fix typo in nslcd service name

2016-04-21 Thread Lukas Slebodnik
On (21/04/16 16:42), Rob Crittenden wrote:
>Lukas Slebodnik wrote:
>> On (21/04/16 19:25), Petr Vobornik wrote:
>> > related but does not implement https://fedorahosted.org/freeipa/ticket/5806
>> > --
>> > Petr Vobornik
>> 
>> > From b9b8716ec9ba5a5cdbed1f6cdedf7cff8878f08f Mon Sep 17 00:00:00 2001
>> > From: Petr Vobornik 
>> > Date: Thu, 21 Apr 2016 19:23:31 +0200
>> > Subject: [PATCH] ipa-client-install: fix typo in nslcd service name
>> > 
>> > related but does not implement https://fedorahosted.org/freeipa/ticket/5806
>> > ---
>> > client/ipa-client-install | 2 +-
>> > 1 file changed, 1 insertion(+), 1 deletion(-)
>> > 
>> > diff --git a/client/ipa-client-install b/client/ipa-client-install
>> > index 
>> > c38843f85639a9118cd1a471709992690643d79a..0e6e65c4ad4ce01fe1257eee4bb2633a70c3de4e
>> >  100755
>> > --- a/client/ipa-client-install
>> > +++ b/client/ipa-client-install
>> > @@ -2938,7 +2938,7 @@ def install(options, env, fstore, statestore):
>> >  nscd.service_name)
>> > 
>> >  nslcd = services.knownservices.nslcd
>> > -if nscd.is_installed():
>> > +if nslcd.is_installed():
>> >  save_state(nslcd)
>> 
>> I thought that milestone "Future Releases" has lower priority
>> then "FreeIPA 4.4 Backlog"
>> 
>> Therefore I would prefer to close ticket #5806 and implement following one
>> https://fedorahosted.org/freeipa/ticket/5557#comment:2
>
>I don't understand what you are suggesting. Tickets aren't swapped like this
>and certainly non-related bugs aren't closed for another.
>
>This patch just fixes an obvious one-liner as agreed upon in triage. The rest
>will be potentially addressed later.
>
>ACK on the patch.
>
I cannot see a reason to fix oneliner.
This code is not tested and should be removed (#5557)

Or someone should write integration test.

I'm sorry but conditional-NACK (missing integration test)

LS

-- 
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 0441] Configure httpd service from installer

2016-04-21 Thread Timo Aaltonen
21.04.2016, 20:50, Martin Basti kirjoitti:
> 
> 
> On 21.04.2016 19:28, Stanislav Laznicka wrote:
>> On 04/21/2016 11:19 AM, Martin Basti wrote:
>>>
>>>
>>> On 20.04.2016 17:27, Martin Basti wrote:


 On 24.03.2016 14:27, Martin Basti wrote:
>
>
> On 24.03.2016 13:55, Jan Cholasta wrote:
>> On 18.3.2016 23:27, Timo Aaltonen wrote:
>>> On 17.03.2016 18:36, Martin Basti wrote:
 https://fedorahosted.org/freeipa/ticket/5681
>>>
>>> would be nicer if ipa-httpd.conf was a template with the current
>>> hardcoded values replaced with platform paths..
>>
>> +1, I would also prefer if the file was renamed to
>> init/systemd/httpd.conf rather than install/share/ipa-httpd.conf.
> ipa-httpd.conf.template should be in /user/share/ipa, directory
> init/systemd copied only to rpm and then copied to
> /etc/systemd/system AFAIK
>
>>
>>>
>>>
>>> not relevant to this patch, but there are others candidates for
>>> templates like:
>>>
>>> daemons/dnssec/ipa-dnskeysyncd.service
>>> daemons/dnssec/ipa-ods-exporter.service
>>> install/conf/ipa.conf
>>
>

 Updated patch attached, sorry for delay.


>>> Updated patch attached (fixed unused import).
>>>
>>>
>>
>> Seems to work as expected. However, wouldn't it be better to use
>> installutils.remove_file instead of remove_httpd_service_ipa_conf (or
>> at least log the possible error during os.unlink) to get the same
>> behavior as with the other config files? 
> 
> It could be, but because I created platform specific method for adding
> httpd service config, it seems natural to me to create inverse operation
> platform specific too.
> I have no strong opinion about this, Timo what might be better, you use
> platform specific code more than we? :)

Well, with this patch in I'd just reuse the methods from
RedHatTaskNamespace() just like some others are being used right now.
Systemd is all I support anyway.



-- 
t

-- 
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] 957 ipa-client-install: fix typo in nslcd service name

2016-04-21 Thread Rob Crittenden

Lukas Slebodnik wrote:

On (21/04/16 19:25), Petr Vobornik wrote:

related but does not implement https://fedorahosted.org/freeipa/ticket/5806
--
Petr Vobornik



From b9b8716ec9ba5a5cdbed1f6cdedf7cff8878f08f Mon Sep 17 00:00:00 2001
From: Petr Vobornik 
Date: Thu, 21 Apr 2016 19:23:31 +0200
Subject: [PATCH] ipa-client-install: fix typo in nslcd service name

related but does not implement https://fedorahosted.org/freeipa/ticket/5806
---
client/ipa-client-install | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/client/ipa-client-install b/client/ipa-client-install
index 
c38843f85639a9118cd1a471709992690643d79a..0e6e65c4ad4ce01fe1257eee4bb2633a70c3de4e
 100755
--- a/client/ipa-client-install
+++ b/client/ipa-client-install
@@ -2938,7 +2938,7 @@ def install(options, env, fstore, statestore):
 nscd.service_name)

 nslcd = services.knownservices.nslcd
-if nscd.is_installed():
+if nslcd.is_installed():
 save_state(nslcd)


I thought that milestone "Future Releases" has lower priority
then "FreeIPA 4.4 Backlog"

Therefore I would prefer to close ticket #5806 and implement following one
https://fedorahosted.org/freeipa/ticket/5557#comment:2


I don't understand what you are suggesting. Tickets aren't swapped like 
this and certainly non-related bugs aren't closed for another.


This patch just fixes an obvious one-liner as agreed upon in triage. The 
rest will be potentially addressed later.


ACK on the patch.

rob

--
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] 957 ipa-client-install: fix typo in nslcd service name

2016-04-21 Thread Lukas Slebodnik
On (21/04/16 19:25), Petr Vobornik wrote:
>related but does not implement https://fedorahosted.org/freeipa/ticket/5806
>-- 
>Petr Vobornik

>From b9b8716ec9ba5a5cdbed1f6cdedf7cff8878f08f Mon Sep 17 00:00:00 2001
>From: Petr Vobornik 
>Date: Thu, 21 Apr 2016 19:23:31 +0200
>Subject: [PATCH] ipa-client-install: fix typo in nslcd service name
>
>related but does not implement https://fedorahosted.org/freeipa/ticket/5806
>---
> client/ipa-client-install | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/client/ipa-client-install b/client/ipa-client-install
>index 
>c38843f85639a9118cd1a471709992690643d79a..0e6e65c4ad4ce01fe1257eee4bb2633a70c3de4e
> 100755
>--- a/client/ipa-client-install
>+++ b/client/ipa-client-install
>@@ -2938,7 +2938,7 @@ def install(options, env, fstore, statestore):
> nscd.service_name)
> 
> nslcd = services.knownservices.nslcd
>-if nscd.is_installed():
>+if nslcd.is_installed():
> save_state(nslcd)

I thought that milestone "Future Releases" has lower priority
then "FreeIPA 4.4 Backlog"

Therefore I would prefer to close ticket #5806 and implement following one
https://fedorahosted.org/freeipa/ticket/5557#comment:2

LS

-- 
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 0441] Configure httpd service from installer

2016-04-21 Thread Martin Basti



On 21.04.2016 19:28, Stanislav Laznicka wrote:

On 04/21/2016 11:19 AM, Martin Basti wrote:



On 20.04.2016 17:27, Martin Basti wrote:



On 24.03.2016 14:27, Martin Basti wrote:



On 24.03.2016 13:55, Jan Cholasta wrote:

On 18.3.2016 23:27, Timo Aaltonen wrote:

On 17.03.2016 18:36, Martin Basti wrote:

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


would be nicer if ipa-httpd.conf was a template with the current
hardcoded values replaced with platform paths..


+1, I would also prefer if the file was renamed to 
init/systemd/httpd.conf rather than install/share/ipa-httpd.conf.
ipa-httpd.conf.template should be in /user/share/ipa, directory 
init/systemd copied only to rpm and then copied to 
/etc/systemd/system AFAIK







not relevant to this patch, but there are others candidates for
templates like:

daemons/dnssec/ipa-dnskeysyncd.service
daemons/dnssec/ipa-ods-exporter.service
install/conf/ipa.conf






Updated patch attached, sorry for delay.



Updated patch attached (fixed unused import).




Seems to work as expected. However, wouldn't it be better to use 
installutils.remove_file instead of remove_httpd_service_ipa_conf (or 
at least log the possible error during os.unlink) to get the same 
behavior as with the other config files? 


It could be, but because I created platform specific method for adding 
httpd service config, it seems natural to me to create inverse operation 
platform specific too.
I have no strong opinion about this, Timo what might be better, you use 
platform specific code more than we? :)


Martin
-- 
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] Locations design v2: LDAP schema & user interface

2016-04-21 Thread Martin Basti



On 21.04.2016 18:58, Simo Sorce wrote:

On Thu, 2016-04-21 at 17:39 +0200, Petr Spacek wrote:

On 19.4.2016 19:17, Simo Sorce wrote:

On Tue, 2016-04-19 at 11:11 +0200, Petr Spacek wrote:

On 18.4.2016 21:33, Simo Sorce wrote:

On Mon, 2016-04-18 at 17:44 +0200, Petr Spacek wrote:

* Find, filter and copy hand-made records from main tree into the
_locations sub-trees. This means that every hand-made record
needs to be copied and synchronized N-times where N = number of IPA
locations.

This ^^ seem the one that provides the best semantics for admins and the
least unexpected results.


My favorite option for the first version is 'document that enabling
DNS location will hide hand-made records in IPA domain.'

I do not think this is acceptable, sorry.


The feature is disabled by default and needs additional configuration
anyway so simply upgrading should not break anything.

It is also useless this way.


I'm eager to hear opinions and answers to questions above.

HTH,

Well it does not help because you did not answer the questions listed in the
design page.

Anyway, here is third version of the design. It avoids copying user-made
records (basically 2 DNAMEs were replaced with bunch of CNAMEs):

http://www.freeipa.org/page/V4/DNS_Location_Mechanism#Design_.28Version_3:_CNAME_per_service_name.29

It seems like a good middle ground:
http://www.freeipa.org/page/V4/DNS_Location_Mechanism#Comparison_of_proposals

It does seem like a decent middle ground.
And I guess an admin would be able to add custom templates if he wants
to have specific services forwarded to the location specific subtree ?

Yes, the bind-dyndb-ldap's RecordGenerator and PerServerConfigInLDAP are
generic enough. At the moment we do not plan to expose these mechanisms in
user interface, we might do that later on.



This required changes in RecordGenerator design, too:
https://fedorahosted.org/bind-dyndb-ldap/wiki/Design/RecordGenerator

I do not see where you specify the specific record names you forward to
the location trees here?

I do not understand the question. Let's have a look at the example:

# DN specifies DNS node name which will hold the generated record:
dn: idnsName=_udp,idnsname=example.com.,cn=dns,dc=example,dc=com
# this is equivalent to _udp.example.com.

objectClass: idnsTemplateObject
objectClass: top
objectClass: idnsRecord
idnsName: _udp

# sub-type determines type of the generated record = DNAME
idnsTemplateAttribute;dnamerecord:
_udp.\{substitutionvariable_ipalocation\}._locations
# generated value will be _udp.your-location._locations
# it is a relative name so zone name (example.com) will be automatically 
appended

The template is just string, so you can specify an absolute name if you want:
idnsTemplateAttribute;dnamerecord:
_udp.\{substitutionvariable_ipalocation\}._locations.another.zone.example.

Of course 'ipalocation' is just a variable name so user can define his own in
PerServerConfigInLDAP.

Is it clearer now?

Sorry I thought you said in option 3 that you would only create records
for specific services using CNAMEs
I was looking for how you configure which services you are going to pick
in that case and couldn't see it.
This example is a DNAME one and looks to me it is about option 2 ?

I put there image for version 3 there, and put/fix some implementation 
details there. I will add more implementation details tomorrow.


Basically, IPA knows what services are on which server (except NTP, will 
be fixed), so based on this we are able to generate proper SRV records 
in all locations, and mark the original one by attribute 
'idnsTemplateAttribute;cnamerecord'  Please see example here, I will 
refer on it later 
http://www.freeipa.org/page/V4/DNS_Location_Mechanism#CNAME_data_generation



In case that server is not configured to provide Location specific data, 
or the server is old, the original SRV record (marked with 
'idnsTemplateAttribute') will be used. In case that server is configured 
to provide location specific data, bind-dyndb-ldap will replace the 
original SRV record by CNAME according to location.


Other SRV records (those not marked by 'idnsTemplateAttribute') are 
untouched.


Martin

--
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 0441] Configure httpd service from installer

2016-04-21 Thread Stanislav Laznicka

On 04/21/2016 11:19 AM, Martin Basti wrote:



On 20.04.2016 17:27, Martin Basti wrote:



On 24.03.2016 14:27, Martin Basti wrote:



On 24.03.2016 13:55, Jan Cholasta wrote:

On 18.3.2016 23:27, Timo Aaltonen wrote:

On 17.03.2016 18:36, Martin Basti wrote:

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


would be nicer if ipa-httpd.conf was a template with the current
hardcoded values replaced with platform paths..


+1, I would also prefer if the file was renamed to 
init/systemd/httpd.conf rather than install/share/ipa-httpd.conf.
ipa-httpd.conf.template should be in /user/share/ipa, directory 
init/systemd copied only to rpm and then copied to 
/etc/systemd/system AFAIK







not relevant to this patch, but there are others candidates for
templates like:

daemons/dnssec/ipa-dnskeysyncd.service
daemons/dnssec/ipa-ods-exporter.service
install/conf/ipa.conf






Updated patch attached, sorry for delay.



Updated patch attached (fixed unused import).




Seems to work as expected. However, wouldn't it be better to use 
installutils.remove_file instead of remove_httpd_service_ipa_conf (or at 
least log the possible error during os.unlink) to get the same behavior 
as with the other config files?
-- 
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] 957 ipa-client-install: fix typo in nslcd service name

2016-04-21 Thread Petr Vobornik
related but does not implement https://fedorahosted.org/freeipa/ticket/5806
-- 
Petr Vobornik
From b9b8716ec9ba5a5cdbed1f6cdedf7cff8878f08f Mon Sep 17 00:00:00 2001
From: Petr Vobornik 
Date: Thu, 21 Apr 2016 19:23:31 +0200
Subject: [PATCH] ipa-client-install: fix typo in nslcd service name

related but does not implement https://fedorahosted.org/freeipa/ticket/5806
---
 client/ipa-client-install | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/client/ipa-client-install b/client/ipa-client-install
index c38843f85639a9118cd1a471709992690643d79a..0e6e65c4ad4ce01fe1257eee4bb2633a70c3de4e 100755
--- a/client/ipa-client-install
+++ b/client/ipa-client-install
@@ -2938,7 +2938,7 @@ def install(options, env, fstore, statestore):
 nscd.service_name)
 
 nslcd = services.knownservices.nslcd
-if nscd.is_installed():
+if nslcd.is_installed():
 save_state(nslcd)
 
 retcode, conf, filename = (0, None, None)
-- 
2.5.5

-- 
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] Locations design v2: LDAP schema & user interface

2016-04-21 Thread Simo Sorce
On Thu, 2016-04-21 at 17:39 +0200, Petr Spacek wrote:
> On 19.4.2016 19:17, Simo Sorce wrote:
> > On Tue, 2016-04-19 at 11:11 +0200, Petr Spacek wrote:
> >> On 18.4.2016 21:33, Simo Sorce wrote:
> >>> On Mon, 2016-04-18 at 17:44 +0200, Petr Spacek wrote:
>  * Find, filter and copy hand-made records from main tree into the
>  _locations sub-trees. This means that every hand-made record
>  needs to be copied and synchronized N-times where N = number of IPA
>  locations.
> >>>
> >>> This ^^ seem the one that provides the best semantics for admins and the
> >>> least unexpected results.
> >>>
>  My favorite option for the first version is 'document that enabling
>  DNS location will hide hand-made records in IPA domain.'
> >>>
> >>> I do not think this is acceptable, sorry.
> >>>
>  The feature is disabled by default and needs additional configuration
>  anyway so simply upgrading should not break anything.
> >>>
> >>> It is also useless this way.
> >>>
>  I'm eager to hear opinions and answers to questions above.
> >>>
> >>> HTH,
> >>
> >> Well it does not help because you did not answer the questions listed in 
> >> the
> >> design page.
> >>
> >> Anyway, here is third version of the design. It avoids copying user-made
> >> records (basically 2 DNAMEs were replaced with bunch of CNAMEs):
> >>
> >> http://www.freeipa.org/page/V4/DNS_Location_Mechanism#Design_.28Version_3:_CNAME_per_service_name.29
> >>
> >> It seems like a good middle ground:
> >> http://www.freeipa.org/page/V4/DNS_Location_Mechanism#Comparison_of_proposals
> > 
> > It does seem like a decent middle ground.
> > And I guess an admin would be able to add custom templates if he wants
> > to have specific services forwarded to the location specific subtree ?
> 
> Yes, the bind-dyndb-ldap's RecordGenerator and PerServerConfigInLDAP are
> generic enough. At the moment we do not plan to expose these mechanisms in
> user interface, we might do that later on.
> 
> 
> >> This required changes in RecordGenerator design, too:
> >> https://fedorahosted.org/bind-dyndb-ldap/wiki/Design/RecordGenerator
> > 
> > I do not see where you specify the specific record names you forward to
> > the location trees here?
> 
> I do not understand the question. Let's have a look at the example:
> 
> # DN specifies DNS node name which will hold the generated record:
> dn: idnsName=_udp,idnsname=example.com.,cn=dns,dc=example,dc=com
> # this is equivalent to _udp.example.com.
> 
> objectClass: idnsTemplateObject
> objectClass: top
> objectClass: idnsRecord
> idnsName: _udp
> 
> # sub-type determines type of the generated record = DNAME
> idnsTemplateAttribute;dnamerecord:
> _udp.\{substitutionvariable_ipalocation\}._locations
> # generated value will be _udp.your-location._locations
> # it is a relative name so zone name (example.com) will be automatically 
> appended
> 
> The template is just string, so you can specify an absolute name if you want:
> idnsTemplateAttribute;dnamerecord:
> _udp.\{substitutionvariable_ipalocation\}._locations.another.zone.example.
> 
> Of course 'ipalocation' is just a variable name so user can define his own in
> PerServerConfigInLDAP.
> 
> Is it clearer now?

Sorry I thought you said in option 3 that you would only create records
for specific services using CNAMEs
I was looking for how you configure which services you are going to pick
in that case and couldn't see it.
This example is a DNAME one and looks to me it is about option 2 ?

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


Re: [Freeipa-devel] Locations design v2: LDAP schema & user interface

2016-04-21 Thread Petr Spacek
On 19.4.2016 19:17, Simo Sorce wrote:
> On Tue, 2016-04-19 at 11:11 +0200, Petr Spacek wrote:
>> On 18.4.2016 21:33, Simo Sorce wrote:
>>> On Mon, 2016-04-18 at 17:44 +0200, Petr Spacek wrote:
 * Find, filter and copy hand-made records from main tree into the
 _locations sub-trees. This means that every hand-made record
 needs to be copied and synchronized N-times where N = number of IPA
 locations.
>>>
>>> This ^^ seem the one that provides the best semantics for admins and the
>>> least unexpected results.
>>>
 My favorite option for the first version is 'document that enabling
 DNS location will hide hand-made records in IPA domain.'
>>>
>>> I do not think this is acceptable, sorry.
>>>
 The feature is disabled by default and needs additional configuration
 anyway so simply upgrading should not break anything.
>>>
>>> It is also useless this way.
>>>
 I'm eager to hear opinions and answers to questions above.
>>>
>>> HTH,
>>
>> Well it does not help because you did not answer the questions listed in the
>> design page.
>>
>> Anyway, here is third version of the design. It avoids copying user-made
>> records (basically 2 DNAMEs were replaced with bunch of CNAMEs):
>>
>> http://www.freeipa.org/page/V4/DNS_Location_Mechanism#Design_.28Version_3:_CNAME_per_service_name.29
>>
>> It seems like a good middle ground:
>> http://www.freeipa.org/page/V4/DNS_Location_Mechanism#Comparison_of_proposals
> 
> It does seem like a decent middle ground.
> And I guess an admin would be able to add custom templates if he wants
> to have specific services forwarded to the location specific subtree ?

Yes, the bind-dyndb-ldap's RecordGenerator and PerServerConfigInLDAP are
generic enough. At the moment we do not plan to expose these mechanisms in
user interface, we might do that later on.


>> This required changes in RecordGenerator design, too:
>> https://fedorahosted.org/bind-dyndb-ldap/wiki/Design/RecordGenerator
> 
> I do not see where you specify the specific record names you forward to
> the location trees here?

I do not understand the question. Let's have a look at the example:

# DN specifies DNS node name which will hold the generated record:
dn: idnsName=_udp,idnsname=example.com.,cn=dns,dc=example,dc=com
# this is equivalent to _udp.example.com.

objectClass: idnsTemplateObject
objectClass: top
objectClass: idnsRecord
idnsName: _udp

# sub-type determines type of the generated record = DNAME
idnsTemplateAttribute;dnamerecord:
_udp.\{substitutionvariable_ipalocation\}._locations
# generated value will be _udp.your-location._locations
# it is a relative name so zone name (example.com) will be automatically 
appended

The template is just string, so you can specify an absolute name if you want:
idnsTemplateAttribute;dnamerecord:
_udp.\{substitutionvariable_ipalocation\}._locations.another.zone.example.

Of course 'ipalocation' is just a variable name so user can define his own in
PerServerConfigInLDAP.

Is it clearer now?

Petr^2 Spacek


>> Also, CLI was updated to follow Honza's recommendations from previous 
>> e-mails:
>> http://www.freeipa.org/page/V4/DNS_Location_Mechanism#CLI
> 
> Thanks for updating all designs in concert.
> 
> Simo.

-- 
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 0035] ipatests: Add test case for requesting a certificate with full principal.

2016-04-21 Thread Martin Basti



On 21.04.2016 16:30, Martin Babinsky wrote:

On 04/21/2016 03:55 PM, Milan Kubík wrote:

On 04/21/2016 03:29 PM, Martin Babinsky wrote:

On 04/21/2016 03:25 PM, Martin Babinsky wrote:

On 04/21/2016 11:24 AM, Milan Kubík wrote:

On 04/05/2016 12:07 PM, Martin Babinsky wrote:

On 04/05/2016 10:24 AM, Milan Kubík wrote:

On 04/05/2016 10:17 AM, Milan Kubík wrote:

On 04/05/2016 09:31 AM, Martin Babinsky wrote:

On 04/01/2016 12:02 PM, Milan Kubík wrote:


Patches attached.



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








Hi Milan,



I would be more happy if you could send a separate patch for the
context
manager fix, since the issue is orthogonal to the added test case
(even
if the test suite explodes without it).



Otherwise LGTM.







Done. Patch 0035 now applies to all branches, context manager fix
needs separate patch for ipa-4-2.


Updated commit message in patches 0036 to include the ticket.


Thanks, ACK.


Add freeipa-devel back to the loop & push request :)

--
Milan Kubik


Ah sorry I forgot how to mailing list.

ACK again for our push-bot (aka mbasti)



I see that the fix for the failing test was already pushed so you can
remove the xfail mark from the test and it should be all green now.

Sorry for the confusion.



I haven't noticed, sorry. Updated patch attached.


--
Milan Kubik



All is green, ACK.

I would recommend pushing patch 0036 first, then patch 0035 to avoid 
false negative errors when bisecting.



Tests pushed to:

master:
* b0b9972213760dcf351cdd85dbfe2c38fc21b2e6 ipatests: fix for 
change_principal context manager
* 0472300dffc1b77533a6bb7397d6a5fa11439303 ipatests: Add test case for 
requesting a certificate with full principal.

ipa-4-3:
* c4fa656b0e0850ddd6400caaa676eae4ec46da06 ipatests: fix for 
change_principal context manager
* e183030067bae2df318324e9fcaafa8ea272f4b4 ipatests: Add test case for 
requesting a certificate with full principal.

ipa-4-2:
* eadd47eec6ceb38b001bc9bff14e2a5aa83eb2ab ipatests: fix for 
change_principal context manager
* ffd670379b16940499c1ef86d676c05886cb1116 ipatests: Add test case for 
requesting a certificate with full principal.


--
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 0035] ipatests: Add test case for requesting a certificate with full principal.

2016-04-21 Thread Martin Babinsky

On 04/21/2016 03:55 PM, Milan Kubík wrote:

On 04/21/2016 03:29 PM, Martin Babinsky wrote:

On 04/21/2016 03:25 PM, Martin Babinsky wrote:

On 04/21/2016 11:24 AM, Milan Kubík wrote:

On 04/05/2016 12:07 PM, Martin Babinsky wrote:

On 04/05/2016 10:24 AM, Milan Kubík wrote:

On 04/05/2016 10:17 AM, Milan Kubík wrote:

On 04/05/2016 09:31 AM, Martin Babinsky wrote:

On 04/01/2016 12:02 PM, Milan Kubík wrote:


Patches attached.



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








Hi Milan,



I would be more happy if you could send a separate patch for the
context
manager fix, since the issue is orthogonal to the added test case
(even
if the test suite explodes without it).



Otherwise LGTM.







Done. Patch 0035 now applies to all branches, context manager fix
needs separate patch for ipa-4-2.


Updated commit message in patches 0036 to include the ticket.


Thanks, ACK.


Add freeipa-devel back to the loop & push request :)

--
Milan Kubik


Ah sorry I forgot how to mailing list.

ACK again for our push-bot (aka mbasti)



I see that the fix for the failing test was already pushed so you can
remove the xfail mark from the test and it should be all green now.

Sorry for the confusion.



I haven't noticed, sorry. Updated patch attached.


--
Milan Kubik



All is green, ACK.

I would recommend pushing patch 0036 first, then patch 0035 to avoid 
false negative errors when bisecting.


--
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 0035] ipatests: Add test case for requesting a certificate with full principal.

2016-04-21 Thread Milan Kubík

On 04/21/2016 03:29 PM, Martin Babinsky wrote:

On 04/21/2016 03:25 PM, Martin Babinsky wrote:

On 04/21/2016 11:24 AM, Milan Kubík wrote:

On 04/05/2016 12:07 PM, Martin Babinsky wrote:

On 04/05/2016 10:24 AM, Milan Kubík wrote:

On 04/05/2016 10:17 AM, Milan Kubík wrote:

On 04/05/2016 09:31 AM, Martin Babinsky wrote:

On 04/01/2016 12:02 PM, Milan Kubík wrote:


Patches attached.



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








Hi Milan,



I would be more happy if you could send a separate patch for the
context
manager fix, since the issue is orthogonal to the added test case
(even
if the test suite explodes without it).



Otherwise LGTM.







Done. Patch 0035 now applies to all branches, context manager fix
needs separate patch for ipa-4-2.


Updated commit message in patches 0036 to include the ticket.


Thanks, ACK.


Add freeipa-devel back to the loop & push request :)

--
Milan Kubik


Ah sorry I forgot how to mailing list.

ACK again for our push-bot (aka mbasti)



I see that the fix for the failing test was already pushed so you can 
remove the xfail mark from the test and it should be all green now.


Sorry for the confusion.



I haven't noticed, sorry. Updated patch attached.


--
Milan Kubik

From cd49ec790918c7b006afe9ee542ff055739b0b74 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Milan=20Kub=C3=ADk?= 
Date: Tue, 5 Apr 2016 10:04:03 +0200
Subject: [PATCH] ipatests: Add test case for requesting a certificate with
 full principal.

https://fedorahosted.org/freeipa/ticket/5733
---
 ipatests/test_xmlrpc/test_caacl_profile_enforcement.py | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/ipatests/test_xmlrpc/test_caacl_profile_enforcement.py b/ipatests/test_xmlrpc/test_caacl_profile_enforcement.py
index dca4151d614a4c2e2f5a09455426d117da4c1c80..11c040966003e2ea86149359c8aacb953e9fdd37 100644
--- a/ipatests/test_xmlrpc/test_caacl_profile_enforcement.py
+++ b/ipatests/test_xmlrpc/test_caacl_profile_enforcement.py
@@ -130,6 +130,13 @@ class TestCertSignMIME(XMLRPC_test):
 api.Command.cert_request(csr, principal=smime_user,
  profile_id=smime_profile.name)
 
+def test_sign_smime_csr_full_principal(self, smime_profile, smime_user):
+csr = generate_user_csr(smime_user)
+smime_user_principal = '@'.join((smime_user, api.env.realm))
+with change_principal(smime_user, SMIME_USER_PW):
+api.Command.cert_request(csr, principal=smime_user_principal,
+ profile_id=smime_profile.name)
+
 
 @pytest.mark.tier1
 class TestSignWithDisabledACL(XMLRPC_test):
-- 
2.8.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 0035] ipatests: Add test case for requesting a certificate with full principal.

2016-04-21 Thread Martin Basti



On 21.04.2016 15:30, Martin Basti wrote:



On 21.04.2016 15:25, Martin Babinsky wrote:

On 04/21/2016 11:24 AM, Milan Kubík wrote:

On 04/05/2016 12:07 PM, Martin Babinsky wrote:

On 04/05/2016 10:24 AM, Milan Kubík wrote:

On 04/05/2016 10:17 AM, Milan Kubík wrote:

On 04/05/2016 09:31 AM, Martin Babinsky wrote:

On 04/01/2016 12:02 PM, Milan Kubík wrote:


Patches attached.



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








Hi Milan,



I would be more happy if you could send a separate patch for the
context
manager fix, since the issue is orthogonal to the added test case
(even
if the test suite explodes without it).



Otherwise LGTM.







Done. Patch 0035 now applies to all branches, context manager fix
needs separate patch for ipa-4-2.


Updated commit message in patches 0036 to include the ticket.


Thanks, ACK.


Add freeipa-devel back to the loop & push request :)

--
Milan Kubik


Ah sorry I forgot how to mailing list.

ACK again for our push-bot (aka mbasti)

PushError: Push rejected by bot (Reason: test should not be marked as 
xfail)



... because issue was already fixed

--
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 0035] ipatests: Add test case for requesting a certificate with full principal.

2016-04-21 Thread Martin Basti



On 21.04.2016 15:25, Martin Babinsky wrote:

On 04/21/2016 11:24 AM, Milan Kubík wrote:

On 04/05/2016 12:07 PM, Martin Babinsky wrote:

On 04/05/2016 10:24 AM, Milan Kubík wrote:

On 04/05/2016 10:17 AM, Milan Kubík wrote:

On 04/05/2016 09:31 AM, Martin Babinsky wrote:

On 04/01/2016 12:02 PM, Milan Kubík wrote:


Patches attached.



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








Hi Milan,



I would be more happy if you could send a separate patch for the
context
manager fix, since the issue is orthogonal to the added test case
(even
if the test suite explodes without it).



Otherwise LGTM.







Done. Patch 0035 now applies to all branches, context manager fix
needs separate patch for ipa-4-2.


Updated commit message in patches 0036 to include the ticket.


Thanks, ACK.


Add freeipa-devel back to the loop & push request :)

--
Milan Kubik


Ah sorry I forgot how to mailing list.

ACK again for our push-bot (aka mbasti)


PushError: Push rejected by bot (Reason: test should not be marked as xfail)

--
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 0035] ipatests: Add test case for requesting a certificate with full principal.

2016-04-21 Thread Martin Babinsky

On 04/21/2016 03:25 PM, Martin Babinsky wrote:

On 04/21/2016 11:24 AM, Milan Kubík wrote:

On 04/05/2016 12:07 PM, Martin Babinsky wrote:

On 04/05/2016 10:24 AM, Milan Kubík wrote:

On 04/05/2016 10:17 AM, Milan Kubík wrote:

On 04/05/2016 09:31 AM, Martin Babinsky wrote:

On 04/01/2016 12:02 PM, Milan Kubík wrote:


Patches attached.



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








Hi Milan,



I would be more happy if you could send a separate patch for the
context
manager fix, since the issue is orthogonal to the added test case
(even
if the test suite explodes without it).



Otherwise LGTM.







Done. Patch 0035 now applies to all branches, context manager fix
needs separate patch for ipa-4-2.


Updated commit message in patches 0036 to include the ticket.


Thanks, ACK.


Add freeipa-devel back to the loop & push request :)

--
Milan Kubik


Ah sorry I forgot how to mailing list.

ACK again for our push-bot (aka mbasti)



I see that the fix for the failing test was already pushed so you can 
remove the xfail mark from the test and it should be all green now.


Sorry for the confusion.

--
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 0035] ipatests: Add test case for requesting a certificate with full principal.

2016-04-21 Thread Martin Babinsky

On 04/21/2016 11:24 AM, Milan Kubík wrote:

On 04/05/2016 12:07 PM, Martin Babinsky wrote:

On 04/05/2016 10:24 AM, Milan Kubík wrote:

On 04/05/2016 10:17 AM, Milan Kubík wrote:

On 04/05/2016 09:31 AM, Martin Babinsky wrote:

On 04/01/2016 12:02 PM, Milan Kubík wrote:


Patches attached.



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








Hi Milan,



I would be more happy if you could send a separate patch for the
context
manager fix, since the issue is orthogonal to the added test case
(even
if the test suite explodes without it).



Otherwise LGTM.







Done. Patch 0035 now applies to all branches, context manager fix
needs separate patch for ipa-4-2.


Updated commit message in patches 0036 to include the ticket.


Thanks, ACK.


Add freeipa-devel back to the loop & push request :)

--
Milan Kubik


Ah sorry I forgot how to mailing list.

ACK again for our push-bot (aka mbasti)

--
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] [python-pytest-multihost]-Ticket-6 run_command produces exit code 1 on windows

2016-04-21 Thread Niranjan
Petr Viktorin wrote:
> On 04/21/2016 07:25 AM, Niranjan wrote:
> > Petr Viktorin wrote:
> >> On 04/20/2016 06:11 AM, Niranjan wrote:
> >>> Petr Viktorin wrote:
>  On 04/18/2016 12:39 PM, Niranjan wrote:
> > Niranjan wrote:
> >> Niranjan wrote:
> >>> Petr Viktorin wrote:
>  On 04/06/2016 10:55 AM, Niranjan wrote:
> > Greetings,
> >
> > For https://fedorahosted.org/python-pytest-multihost/ticket/6 , i 
> > have proposed
> > a patch, I think this patch is more of a workaround , than a 
> > solution. I would
> > like to get more inputs on how to use pytest-multihost to execute 
> > commands 
> > on Windows. The method i am using requires cygwin with openssh/bash 
> > to be
> > installed. 
> 
>  Wow. I'm surprised the only problem on Windows is the "set -e".
> 
>  Regarding the patch:
> 
>  - Why is get_winhost_class needed? I don't see it called anywhere.
>  - Similarly, why is host_type needed? Your patch only sets it.
> 
>  If run_command of WinHost and Unix hosts are this similar, I would 
>  put
>  the 'set -e\n' for Unix (and an empty string for Windows) in a class
>  attribute named e.g. "command_prelude", use it in run_command, and 
>  move
>  run_command from Host to BaseHost.
>  Would that work, or am I missing something?
> >>> How do we detect the host is windows/unix then , we still need 
> >>> host_type 
> >>> to know what the type of host is (unix/windows)?
> 
> 
>  -- 
>  Petr Viktorin
> >> Please review the attached patch.
> 
>  Sorry for the delay.
> 
>  The information about whether the host is Unix or Windows is available:
>  the class is either Host or WinHost, so I don't think an extra host_type
>  is necessary.
>  I'd be open to adding host_type as *configuration* (and corresponding
>  attribute) if it also selected the actual Host class. Currently to use
>  WinHost you need to write custom code.
> >>> I would like to have host_type available. We would like to add more
> >>> functions in classes that override Host/WinHost class , which 
> >>> can be then called depending upon the host_type. 
> >>
> >> Ah, I see; you're not using the WinHost subclass right now.
> >>
> >> I added a host_type; it is used to select the Host class.
> >> You can define a "host_classes" dict in your Domain class to list Host
> >> classes, and the particular Host class will be selected according to the
> >> host_type.
> >>
> >> The usage would be like this:
> >>
> >> -
> >>
> >> class QeHost(QeBaseHost):
> >>"""Linux host class"""
> >> @classmethod
> >> def gethostname(self):
> >> """ Return system hostname """
> >> cmd = self.run_command(['hostname'], raiseonerr=False)
> >> return cmd.stdout_text.strip()
> >>...
> >>
> >> class QeWinHost(QeBaseHost, pytest_multihost.host.WinHost):
> >>"""Windows host class"""
> >>
> >>@classmethod
> >>def gethostname(self):
> >>""" Return system hostname """
> >>cmd = self.run_command(['hostname', '-A'], set_env=False,
> >> raiseonerr=False)
> >>return cmd.stdout_text.strip()
> >>...
> >>
> >>
> >> class QeDomain:
> >>...
> >>host_classes = {'default': QeHost, 'windows': QeWinHost}
> >>
> >># remove your existing "get_host_class" method when host_classes is
> >> defined
> >>...
> >>
> >> -
> >>
> >> And in the config, define host_type to 'windows'.
> >>
> >>
> >> Would it work for you like this?
> > Most of the things worked [...]
> > 
> > Specifically i see that you have "transport_class = transport.SSHTransport" 
> > added to Host Class
> > but not in WinHost class. So initially it failed transport class not 
> > available,  but after i added below lines
> > it worked:
> > 
> > class QeBaseHost(pytest_multihost.host.BaseHost):
> > """ QeBaseHost subclass of multhost plugin BaseHost class """
> > transport_class = transport.SSHTransport
> > 
> > Apart from the above minor thing, Everything else works. Again the above 3 
> > lines solves the minor issue, 
> > just wanted to know if the above lines are to be added and is by design. 
> 
> Right, since SSHTransport works for Windows hosts as well, I've added it
> to BaseHost. With this version of patch it shouldn't be necessary to add
> it in your code.
> 
> If this works for you, I'll commit it and release.

With this latest patch it worked after removing the line "transport_class =
transport.SSHTransport" from QeBaseHost class. 

Please go ahead and commit it.
> 
> 
> -- 
> Petr Viktorin

> From 7eb346c0bf29b1fddfd962675258a3895fbc8300 Mon Sep 17 00:00:00 2001
> From: Niranjan MR 
> Date: Tue, 12 Apr 2016 17:18:10 +0530
> Subject: [PATCH] Add 'host_type', make it possible 

Re: [Freeipa-devel] [python-pytest-multihost]-Ticket-6 run_command produces exit code 1 on windows

2016-04-21 Thread Niranjan
Petr Viktorin wrote:
> On 04/21/2016 07:25 AM, Niranjan wrote:
> > Petr Viktorin wrote:
> >> On 04/20/2016 06:11 AM, Niranjan wrote:
> >>> Petr Viktorin wrote:
>  On 04/18/2016 12:39 PM, Niranjan wrote:
> > Niranjan wrote:
> >> Niranjan wrote:
> >>> Petr Viktorin wrote:
>  On 04/06/2016 10:55 AM, Niranjan wrote:
> > Greetings,
> >
> > For https://fedorahosted.org/python-pytest-multihost/ticket/6 , i 
> > have proposed
> > a patch, I think this patch is more of a workaround , than a 
> > solution. I would
> > like to get more inputs on how to use pytest-multihost to execute 
> > commands 
> > on Windows. The method i am using requires cygwin with openssh/bash 
> > to be
> > installed. 
> 
>  Wow. I'm surprised the only problem on Windows is the "set -e".
> 
>  Regarding the patch:
> 
>  - Why is get_winhost_class needed? I don't see it called anywhere.
>  - Similarly, why is host_type needed? Your patch only sets it.
> 
>  If run_command of WinHost and Unix hosts are this similar, I would 
>  put
>  the 'set -e\n' for Unix (and an empty string for Windows) in a class
>  attribute named e.g. "command_prelude", use it in run_command, and 
>  move
>  run_command from Host to BaseHost.
>  Would that work, or am I missing something?
> >>> How do we detect the host is windows/unix then , we still need 
> >>> host_type 
> >>> to know what the type of host is (unix/windows)?
> 
> 
>  -- 
>  Petr Viktorin
> >> Please review the attached patch.
> 
>  Sorry for the delay.
> 
>  The information about whether the host is Unix or Windows is available:
>  the class is either Host or WinHost, so I don't think an extra host_type
>  is necessary.
>  I'd be open to adding host_type as *configuration* (and corresponding
>  attribute) if it also selected the actual Host class. Currently to use
>  WinHost you need to write custom code.
> >>> I would like to have host_type available. We would like to add more
> >>> functions in classes that override Host/WinHost class , which 
> >>> can be then called depending upon the host_type. 
> >>
> >> Ah, I see; you're not using the WinHost subclass right now.
> >>
> >> I added a host_type; it is used to select the Host class.
> >> You can define a "host_classes" dict in your Domain class to list Host
> >> classes, and the particular Host class will be selected according to the
> >> host_type.
> >>
> >> The usage would be like this:
> >>
> >> -
> >>
> >> class QeHost(QeBaseHost):
> >>"""Linux host class"""
> >> @classmethod
> >> def gethostname(self):
> >> """ Return system hostname """
> >> cmd = self.run_command(['hostname'], raiseonerr=False)
> >> return cmd.stdout_text.strip()
> >>...
> >>
> >> class QeWinHost(QeBaseHost, pytest_multihost.host.WinHost):
> >>"""Windows host class"""
> >>
> >>@classmethod
> >>def gethostname(self):
> >>""" Return system hostname """
> >>cmd = self.run_command(['hostname', '-A'], set_env=False,
> >> raiseonerr=False)
> >>return cmd.stdout_text.strip()
> >>...
> >>
> >>
> >> class QeDomain:
> >>...
> >>host_classes = {'default': QeHost, 'windows': QeWinHost}
> >>
> >># remove your existing "get_host_class" method when host_classes is
> >> defined
> >>...
> >>
> >> -
> >>
> >> And in the config, define host_type to 'windows'.
> >>
> >>
> >> Would it work for you like this?
> > Most of the things worked [...]
> > 
> > Specifically i see that you have "transport_class = transport.SSHTransport" 
> > added to Host Class
> > but not in WinHost class. So initially it failed transport class not 
> > available,  but after i added below lines
> > it worked:
> > 
> > class QeBaseHost(pytest_multihost.host.BaseHost):
> > """ QeBaseHost subclass of multhost plugin BaseHost class """
> > transport_class = transport.SSHTransport
> > 
> > Apart from the above minor thing, Everything else works. Again the above 3 
> > lines solves the minor issue, 
> > just wanted to know if the above lines are to be added and is by design. 
> 
> Right, since SSHTransport works for Windows hosts as well, I've added it
> to BaseHost. With this version of patch it shouldn't be necessary to add
> it in your code.
> 
> If this works for you, I'll commit it and release.
I will test and let you know asap
> 
> 
> -- 
> Petr Viktorin

> From 7eb346c0bf29b1fddfd962675258a3895fbc8300 Mon Sep 17 00:00:00 2001
> From: Niranjan MR 
> Date: Tue, 12 Apr 2016 17:18:10 +0530
> Subject: [PATCH] Add 'host_type', make it possible to use Windows hosts
> 
> Add global parameter windows_test_dir to specify test directory
> for Windows
> Windows hosts 

Re: [Freeipa-devel] [WIP PATCH] server-del: perform full master removal in managed topology

2016-04-21 Thread Martin Babinsky

On 04/21/2016 12:12 PM, Petr Vobornik wrote:

On 04/21/2016 10:41 AM, Ludwig Krispenz wrote:


On 04/21/2016 10:11 AM, Martin Babinsky wrote:

On 04/21/2016 09:21 AM, Jan Cholasta wrote:

On 19.4.2016 12:42, Martin Babinsky wrote:

On 04/14/2016 11:46 AM, Ludwig Krispenz wrote:


On 04/14/2016 10:59 AM, Martin Babinsky wrote:

On 04/14/2016 08:24 AM, Jan Cholasta wrote:

On 13.4.2016 17:10, Rob Crittenden wrote:

Martin Babinsky wrote:

This is a WIP patch which moves the `ipa-replica-manage del`
subcommand
to the 'server-del' API method and exposes it as CLI command[1].
A CI
test suite is also included.

There are some issues with the patch I would like to discuss in
more
detail on the list:

1.) In the original subcommand there was a lot of output (mostly
print
statements) during all stages of master removal. I have tried to
port
these as messages to the command which results in quite voluminous
response sent back to the frontend. Should we try to reduce the
output?


I don't think it applies anymore. These messages were there so the
user
would know something was happening. If it is an API command there
isn't
much we can do other than add something to the cli to print "This
could
take a bit" before making the call.


+1


This is already implemented in PoC. So I guess we may reduce the
output only to the following:


In CLI print:
"Removing {server} from replication topology,"
"please wait...

The adding info messages:

"checking topology connectivity" | "skipping topology connectivity
check"
"checking remaining services" | "skipping check for remaining
services"
"performing cleanup"
"Deleted server {server}"





2.) In the original discussion[2] we assumed that the cleanup part
would
me a separate API method called during server_del postcallback.
However
since the two objects ended up sharing a lot of state (e.g.
topology
state from pre-callback, messages) i have merged it to server-del.
That
makes the code rather unwieldy but I found it difficult to keep
the
two
entities separate without some hacking around framework
capabilities


I haven't looked at the code but as a general principal having
separate
operations has saved our bacon on more than one occasion.


The patch adds a force option, which allows you to re-run server-del
even if the master entry does not exist anymore, so I think we are
safe.




3.) since actions in post-callback require a knowledge about
topology
state gathered in pre-callback, I had to store some information in
the
command's context. Sorry about that, if you know about some way to
circumvent me, let me know.


Looks like it is the only way since you are extending server_del.
Another option would be to drop pre/post and add all this topology
stuff
directly to server_del execute.


4.) The master can not remove itself. I have implemented an ad-hoc
forwarding of the request to other master that can do the job. Is
this
okay?


Why can't the master remove itself?


Because it removes its own replication agreements hence any
changes in
DIT (like removed principals, s4u2 proxy targets etc.) won't
replicate
to other masters.

It shouldn't remove replication agreements, in fact this should be
prevented by the topology plugin.
The removal of the agreements will be triggered by removing the master
entry


That is true, but there is a plenty of cleanup code that is executed
*after* the master entry is removed and these changes would not
replicate if the agreements were removed by topology plugin in the
meanwhile.


What kind of cleanup is it? Can it be done before instead?



Well most of the code can be run in pre-callback if all the checks are
passed/forced.

However there is a check for deleted segments and this one should be
run after the removal of master entry to see if topology plugin
removed all dangling segments pointing to master. I am not quite sure
if it make sense to run this check in the master which is being removed.

no, it is not guranteed that the information on the removed master will
be correct. If the del is applied to the to be removed master the
topology plugin will only on a master which is remaining start the
removal of segments, these will alos be replicated back to the removed
master, but the repl agreements to this master will also be removed, so
no gurantee which mods will be available on the removed master, and you
should also be able to remove a master if it is down - so applying the
full removal on a remaining server makes sense.

What is the behaviour if the removal of a server would disconnect
topology ?




What would be the use-case for master to remove itself?

The only one I see, which was proposed in design page is that in
`ipa-server-install --uninstall` the installer would call `ipa
server-del $to_be_removed` on different replica so that uninstallation
would be done in single step. But effectively this is not removing
itself from API point of view.

Calling `ipa server-del $me` without subsequent uninstallation seems
pointless to me. In 

Re: [Freeipa-devel] [PATCH] 0015, 16 webui: Add 'Skip overlap check' checkbox to the dns adder dialogs

2016-04-21 Thread Martin Basti



On 20.04.2016 17:59, Pavel Vomacka wrote:



On 04/14/2016 02:03 PM, Martin Basti wrote:



On 14.04.2016 13:03, Pavel Vomacka wrote:

Hello,

The first patch (0015) adds the checkbox to the dns zone adder dialog.

The second patch (0016) adds the 'skip overlap check' checkbox to 
the dns forward zone adder dialog. This patch requires the previous 
one. The patch 0016 might not be used in case that the dns forward 
zone dialog shouldn't contain that checkbox.


--
Pavel^3 Vomacka


Can we add hint to webUI what 'skip overlap check' means? Maybe 
description from ipa dns[forward]zone-add --help.


Martin^2



Yes, it is included in these new pathes.

--
Pavel^3 Vomacka




Works for me
-- 
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] [WIP PATCH] server-del: perform full master removal in managed topology

2016-04-21 Thread Martin Babinsky

On 04/21/2016 12:22 PM, Ludwig Krispenz wrote:


On 04/21/2016 12:12 PM, Petr Vobornik wrote:

On 04/21/2016 10:41 AM, Ludwig Krispenz wrote:

On 04/21/2016 10:11 AM, Martin Babinsky wrote:

On 04/21/2016 09:21 AM, Jan Cholasta wrote:

On 19.4.2016 12:42, Martin Babinsky wrote:

On 04/14/2016 11:46 AM, Ludwig Krispenz wrote:

On 04/14/2016 10:59 AM, Martin Babinsky wrote:

On 04/14/2016 08:24 AM, Jan Cholasta wrote:

On 13.4.2016 17:10, Rob Crittenden wrote:

Martin Babinsky wrote:

This is a WIP patch which moves the `ipa-replica-manage del`
subcommand
to the 'server-del' API method and exposes it as CLI command[1].
A CI
test suite is also included.

There are some issues with the patch I would like to discuss in
more
detail on the list:

1.) In the original subcommand there was a lot of output (mostly
print
statements) during all stages of master removal. I have tried to
port
these as messages to the command which results in quite
voluminous
response sent back to the frontend. Should we try to reduce the
output?

I don't think it applies anymore. These messages were there so
the
user
would know something was happening. If it is an API command there
isn't
much we can do other than add something to the cli to print "This
could
take a bit" before making the call.

+1


This is already implemented in PoC. So I guess we may reduce the
output only to the following:


In CLI print:
"Removing {server} from replication topology,"
"please wait...

The adding info messages:

"checking topology connectivity" | "skipping topology connectivity
check"
"checking remaining services" | "skipping check for remaining
services"
"performing cleanup"
"Deleted server {server}"



2.) In the original discussion[2] we assumed that the cleanup
part
would
me a separate API method called during server_del postcallback.
However
since the two objects ended up sharing a lot of state (e.g.
topology
state from pre-callback, messages) i have merged it to
server-del.
That
makes the code rather unwieldy but I found it difficult to keep
the
two
entities separate without some hacking around framework
capabilities

I haven't looked at the code but as a general principal having
separate
operations has saved our bacon on more than one occasion.

The patch adds a force option, which allows you to re-run
server-del
even if the master entry does not exist anymore, so I think we are
safe.


3.) since actions in post-callback require a knowledge about
topology
state gathered in pre-callback, I had to store some
information in
the
command's context. Sorry about that, if you know about some
way to
circumvent me, let me know.

Looks like it is the only way since you are extending server_del.
Another option would be to drop pre/post and add all this
topology
stuff
directly to server_del execute.


4.) The master can not remove itself. I have implemented an
ad-hoc
forwarding of the request to other master that can do the
job. Is
this
okay?

Why can't the master remove itself?


Because it removes its own replication agreements hence any
changes in
DIT (like removed principals, s4u2 proxy targets etc.) won't
replicate
to other masters.

It shouldn't remove replication agreements, in fact this should be
prevented by the topology plugin.
The removal of the agreements will be triggered by removing the
master
entry

That is true, but there is a plenty of cleanup code that is executed
*after* the master entry is removed and these changes would not
replicate if the agreements were removed by topology plugin in the
meanwhile.

What kind of cleanup is it? Can it be done before instead?


Well most of the code can be run in pre-callback if all the checks are
passed/forced.

However there is a check for deleted segments and this one should be
run after the removal of master entry to see if topology plugin
removed all dangling segments pointing to master. I am not quite sure
if it make sense to run this check in the master which is being
removed.

no, it is not guranteed that the information on the removed master will
be correct. If the del is applied to the to be removed master the
topology plugin will only on a master which is remaining start the
removal of segments, these will alos be replicated back to the removed
master, but the repl agreements to this master will also be removed, so
no gurantee which mods will be available on the removed master, and you
should also be able to remove a master if it is down - so applying the
full removal on a remaining server makes sense.

What is the behaviour if the removal of a server would disconnect
topology ?



What would be the use-case for master to remove itself?

I don't think that there is a use case except uninstall, but this
doesen't prevent users to run the command. And then there are a few
options how to handle this.
- accept it and try to do the best
- reject it and say it should be run on any other master
- automatically execute this on another master - I think this is
Martin's current plan


Yes, also if you 

Re: [Freeipa-devel] [WIP PATCH] server-del: perform full master removal in managed topology

2016-04-21 Thread Ludwig Krispenz


On 04/21/2016 12:12 PM, Petr Vobornik wrote:

On 04/21/2016 10:41 AM, Ludwig Krispenz wrote:

On 04/21/2016 10:11 AM, Martin Babinsky wrote:

On 04/21/2016 09:21 AM, Jan Cholasta wrote:

On 19.4.2016 12:42, Martin Babinsky wrote:

On 04/14/2016 11:46 AM, Ludwig Krispenz wrote:

On 04/14/2016 10:59 AM, Martin Babinsky wrote:

On 04/14/2016 08:24 AM, Jan Cholasta wrote:

On 13.4.2016 17:10, Rob Crittenden wrote:

Martin Babinsky wrote:

This is a WIP patch which moves the `ipa-replica-manage del`
subcommand
to the 'server-del' API method and exposes it as CLI command[1].
A CI
test suite is also included.

There are some issues with the patch I would like to discuss in
more
detail on the list:

1.) In the original subcommand there was a lot of output (mostly
print
statements) during all stages of master removal. I have tried to
port
these as messages to the command which results in quite voluminous
response sent back to the frontend. Should we try to reduce the
output?

I don't think it applies anymore. These messages were there so the
user
would know something was happening. If it is an API command there
isn't
much we can do other than add something to the cli to print "This
could
take a bit" before making the call.

+1


This is already implemented in PoC. So I guess we may reduce the
output only to the following:


In CLI print:
"Removing {server} from replication topology,"
"please wait...

The adding info messages:

"checking topology connectivity" | "skipping topology connectivity
check"
"checking remaining services" | "skipping check for remaining
services"
"performing cleanup"
"Deleted server {server}"



2.) In the original discussion[2] we assumed that the cleanup part
would
me a separate API method called during server_del postcallback.
However
since the two objects ended up sharing a lot of state (e.g.
topology
state from pre-callback, messages) i have merged it to server-del.
That
makes the code rather unwieldy but I found it difficult to keep
the
two
entities separate without some hacking around framework
capabilities

I haven't looked at the code but as a general principal having
separate
operations has saved our bacon on more than one occasion.

The patch adds a force option, which allows you to re-run server-del
even if the master entry does not exist anymore, so I think we are
safe.


3.) since actions in post-callback require a knowledge about
topology
state gathered in pre-callback, I had to store some information in
the
command's context. Sorry about that, if you know about some way to
circumvent me, let me know.

Looks like it is the only way since you are extending server_del.
Another option would be to drop pre/post and add all this topology
stuff
directly to server_del execute.


4.) The master can not remove itself. I have implemented an ad-hoc
forwarding of the request to other master that can do the job. Is
this
okay?

Why can't the master remove itself?


Because it removes its own replication agreements hence any
changes in
DIT (like removed principals, s4u2 proxy targets etc.) won't
replicate
to other masters.

It shouldn't remove replication agreements, in fact this should be
prevented by the topology plugin.
The removal of the agreements will be triggered by removing the master
entry

That is true, but there is a plenty of cleanup code that is executed
*after* the master entry is removed and these changes would not
replicate if the agreements were removed by topology plugin in the
meanwhile.

What kind of cleanup is it? Can it be done before instead?


Well most of the code can be run in pre-callback if all the checks are
passed/forced.

However there is a check for deleted segments and this one should be
run after the removal of master entry to see if topology plugin
removed all dangling segments pointing to master. I am not quite sure
if it make sense to run this check in the master which is being removed.

no, it is not guranteed that the information on the removed master will
be correct. If the del is applied to the to be removed master the
topology plugin will only on a master which is remaining start the
removal of segments, these will alos be replicated back to the removed
master, but the repl agreements to this master will also be removed, so
no gurantee which mods will be available on the removed master, and you
should also be able to remove a master if it is down - so applying the
full removal on a remaining server makes sense.

What is the behaviour if the removal of a server would disconnect
topology ?



What would be the use-case for master to remove itself?
I don't think that there is a use case except uninstall, but this 
doesen't prevent users to run the command. And then there are a few 
options how to handle this.

- accept it and try to do the best
- reject it and say it should be run on any other master
- automatically execute this on another master - I think this is 
Martin's current plan


The only one I see, which was proposed in design page is that 

Re: [Freeipa-devel] [python-pytest-multihost]-Ticket-6 run_command produces exit code 1 on windows

2016-04-21 Thread Petr Viktorin
On 04/21/2016 07:25 AM, Niranjan wrote:
> Petr Viktorin wrote:
>> On 04/20/2016 06:11 AM, Niranjan wrote:
>>> Petr Viktorin wrote:
 On 04/18/2016 12:39 PM, Niranjan wrote:
> Niranjan wrote:
>> Niranjan wrote:
>>> Petr Viktorin wrote:
 On 04/06/2016 10:55 AM, Niranjan wrote:
> Greetings,
>
> For https://fedorahosted.org/python-pytest-multihost/ticket/6 , i 
> have proposed
> a patch, I think this patch is more of a workaround , than a 
> solution. I would
> like to get more inputs on how to use pytest-multihost to execute 
> commands 
> on Windows. The method i am using requires cygwin with openssh/bash 
> to be
> installed. 

 Wow. I'm surprised the only problem on Windows is the "set -e".

 Regarding the patch:

 - Why is get_winhost_class needed? I don't see it called anywhere.
 - Similarly, why is host_type needed? Your patch only sets it.

 If run_command of WinHost and Unix hosts are this similar, I would put
 the 'set -e\n' for Unix (and an empty string for Windows) in a class
 attribute named e.g. "command_prelude", use it in run_command, and move
 run_command from Host to BaseHost.
 Would that work, or am I missing something?
>>> How do we detect the host is windows/unix then , we still need 
>>> host_type 
>>> to know what the type of host is (unix/windows)?


 -- 
 Petr Viktorin
>> Please review the attached patch.

 Sorry for the delay.

 The information about whether the host is Unix or Windows is available:
 the class is either Host or WinHost, so I don't think an extra host_type
 is necessary.
 I'd be open to adding host_type as *configuration* (and corresponding
 attribute) if it also selected the actual Host class. Currently to use
 WinHost you need to write custom code.
>>> I would like to have host_type available. We would like to add more
>>> functions in classes that override Host/WinHost class , which 
>>> can be then called depending upon the host_type. 
>>
>> Ah, I see; you're not using the WinHost subclass right now.
>>
>> I added a host_type; it is used to select the Host class.
>> You can define a "host_classes" dict in your Domain class to list Host
>> classes, and the particular Host class will be selected according to the
>> host_type.
>>
>> The usage would be like this:
>>
>> -
>>
>> class QeHost(QeBaseHost):
>>"""Linux host class"""
>> @classmethod
>> def gethostname(self):
>> """ Return system hostname """
>> cmd = self.run_command(['hostname'], raiseonerr=False)
>> return cmd.stdout_text.strip()
>>...
>>
>> class QeWinHost(QeBaseHost, pytest_multihost.host.WinHost):
>>"""Windows host class"""
>>
>>@classmethod
>>def gethostname(self):
>>""" Return system hostname """
>>cmd = self.run_command(['hostname', '-A'], set_env=False,
>> raiseonerr=False)
>>return cmd.stdout_text.strip()
>>...
>>
>>
>> class QeDomain:
>>...
>>host_classes = {'default': QeHost, 'windows': QeWinHost}
>>
>># remove your existing "get_host_class" method when host_classes is
>> defined
>>...
>>
>> -
>>
>> And in the config, define host_type to 'windows'.
>>
>>
>> Would it work for you like this?
> Most of the things worked [...]
> 
> Specifically i see that you have "transport_class = transport.SSHTransport" 
> added to Host Class
> but not in WinHost class. So initially it failed transport class not 
> available,  but after i added below lines
> it worked:
> 
> class QeBaseHost(pytest_multihost.host.BaseHost):
> """ QeBaseHost subclass of multhost plugin BaseHost class """
> transport_class = transport.SSHTransport
> 
> Apart from the above minor thing, Everything else works. Again the above 3 
> lines solves the minor issue, 
> just wanted to know if the above lines are to be added and is by design. 

Right, since SSHTransport works for Windows hosts as well, I've added it
to BaseHost. With this version of patch it shouldn't be necessary to add
it in your code.

If this works for you, I'll commit it and release.


-- 
Petr Viktorin
From 7eb346c0bf29b1fddfd962675258a3895fbc8300 Mon Sep 17 00:00:00 2001
From: Niranjan MR 
Date: Tue, 12 Apr 2016 17:18:10 +0530
Subject: [PATCH] Add 'host_type', make it possible to use Windows hosts

Add global parameter windows_test_dir to specify test directory
for Windows
Windows hosts (WinHost) use this directory by default.
test_dir can now be set individually for each host.
Move run_command from Host class to BaseHost class
Add a "host_type" configuration option and Host attribute. This
is used to select the Host subclass. By default it can
be 'default' or 'windows' (but Config subclasses can define more).

Signed-off-by: 

Re: [Freeipa-devel] [patch 0035] ipatests: Add test case for requesting a certificate with full principal.

2016-04-21 Thread Milan Kubík

On 04/05/2016 12:07 PM, Martin Babinsky wrote:

On 04/05/2016 10:24 AM, Milan Kubík wrote:

On 04/05/2016 10:17 AM, Milan Kubík wrote:

On 04/05/2016 09:31 AM, Martin Babinsky wrote:

On 04/01/2016 12:02 PM, Milan Kubík wrote:


Patches attached.



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








Hi Milan,



I would be more happy if you could send a separate patch for the 
context
manager fix, since the issue is orthogonal to the added test case 
(even

if the test suite explodes without it).



Otherwise LGTM.







Done. Patch 0035 now applies to all branches, context manager fix
needs separate patch for ipa-4-2.


Updated commit message in patches 0036 to include the ticket.


Thanks, ACK.


Add freeipa-devel back to the loop & push request :)

--
Milan Kubik

-- 
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 0441] Configure httpd service from installer

2016-04-21 Thread Martin Basti



On 20.04.2016 17:27, Martin Basti wrote:



On 24.03.2016 14:27, Martin Basti wrote:



On 24.03.2016 13:55, Jan Cholasta wrote:

On 18.3.2016 23:27, Timo Aaltonen wrote:

On 17.03.2016 18:36, Martin Basti wrote:

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


would be nicer if ipa-httpd.conf was a template with the current
hardcoded values replaced with platform paths..


+1, I would also prefer if the file was renamed to 
init/systemd/httpd.conf rather than install/share/ipa-httpd.conf.
ipa-httpd.conf.template should be in /user/share/ipa, directory 
init/systemd copied only to rpm and then copied to 
/etc/systemd/system AFAIK







not relevant to this patch, but there are others candidates for
templates like:

daemons/dnssec/ipa-dnskeysyncd.service
daemons/dnssec/ipa-ods-exporter.service
install/conf/ipa.conf






Updated patch attached, sorry for delay.



Updated patch attached (fixed unused import).
From c828c7d4bd45b783862c8bba63adacfb035b25db Mon Sep 17 00:00:00 2001
From: Martin Basti 
Date: Wed, 16 Mar 2016 09:04:42 +0100
Subject: [PATCH] Configure httpd service from installer instead of directly
 from RPM

File httpd.service was created by RPM, what causes that httpd service may
fail due IPA specific configuration even if IPA wasn't installed or was
uninstalled (without erasing RPMs).

With this patch httpd service is configured by httpd.d/ipa.conf during
IPA installation and this config is removed by uninstaller, so no
residual http configuration related to IPA should stay there.

https://fedorahosted.org/freeipa/ticket/5681
---
 freeipa.spec.in   |  3 +--
 init/systemd/httpd.service|  7 ---
 install/share/Makefile.am |  1 +
 install/share/ipa-httpd.conf.template |  7 +++
 ipaplatform/base/paths.py |  3 +++
 ipaplatform/base/tasks.py |  8 
 ipaplatform/redhat/tasks.py   | 26 ++
 ipaserver/install/httpinstance.py |  6 ++
 ipaserver/install/server/upgrade.py   |  5 +
 9 files changed, 57 insertions(+), 9 deletions(-)
 delete mode 100644 init/systemd/httpd.service
 create mode 100644 install/share/ipa-httpd.conf.template

diff --git a/freeipa.spec.in b/freeipa.spec.in
index 1ded3048873fb9d4cb97b7aca52132345c209a96..aaa40cc9a2246ed1d244e160edf935da216c75c5 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -832,7 +832,6 @@ mkdir -p %{buildroot}%{_unitdir}
 mkdir -p %{buildroot}%{etc_systemd_dir}
 install -m 644 init/systemd/ipa.service %{buildroot}%{_unitdir}/ipa.service
 install -m 644 init/systemd/ipa_memcached.service %{buildroot}%{_unitdir}/ipa_memcached.service
-install -m 644 init/systemd/httpd.service %{buildroot}%{etc_systemd_dir}/httpd.service
 install -m 644 init/systemd/ipa-custodia.service %{buildroot}%{_unitdir}/ipa-custodia.service
 # END
 mkdir -p %{buildroot}/%{_localstatedir}/lib/ipa/backup
@@ -1143,7 +1142,7 @@ fi
 %{_tmpfilesdir}/%{name}.conf
 %attr(644,root,root) %{_unitdir}/ipa_memcached.service
 %attr(644,root,root) %{_unitdir}/ipa-custodia.service
-%attr(644,root,root) %{etc_systemd_dir}/httpd.service
+%ghost %attr(644,root,root) %{etc_systemd_dir}/httpd.d/ipa.conf
 # END
 %dir %{_usr}/share/ipa
 %{_usr}/share/ipa/wsgi.py*
diff --git a/init/systemd/httpd.service b/init/systemd/httpd.service
deleted file mode 100644
index 7ce8f04d8b9bb3663e59d4fdc610af0eb4478178..
--- a/init/systemd/httpd.service
+++ /dev/null
@@ -1,7 +0,0 @@
-.include /usr/lib/systemd/system/httpd.service
-
-[Service]
-Environment=KRB5CCNAME=/var/run/httpd/ipa/krbcache/krb5ccache
-Environment=KDCPROXY_CONFIG=/etc/ipa/kdcproxy/kdcproxy.conf
-ExecStartPre=/usr/libexec/ipa/ipa-httpd-kdcproxy
-ExecStopPost=-/usr/bin/kdestroy -A
diff --git a/install/share/Makefile.am b/install/share/Makefile.am
index b4cb8312471a68d8cd855f542478afe10d200c39..3a3bd2699efaf45ab79dd0257c2d26e7952891eb 100644
--- a/install/share/Makefile.am
+++ b/install/share/Makefile.am
@@ -88,6 +88,7 @@ app_DATA =\
 	kdcproxy.conf			\
 	kdcproxy-enable.uldif		\
 	kdcproxy-disable.uldif		\
+	ipa-httpd.conf.template		\
 	$(NULL)
 
 EXTRA_DIST =\
diff --git a/install/share/ipa-httpd.conf.template b/install/share/ipa-httpd.conf.template
new file mode 100644
index ..a907d73cccac13cbb9d99423a1b739a48ad4f769
--- /dev/null
+++ b/install/share/ipa-httpd.conf.template
@@ -0,0 +1,7 @@
+# Do not edit. Created by IPA installer.
+
+[Service]
+Environment=KRB5CCNAME=$KRB5CC_HTTPD
+Environment=KDCPROXY_CONFIG=$KDCPROXY_CONFIG
+ExecStartPre=$IPA_HTTPD_KDCPROXY
+ExecStopPost=$POST
diff --git a/ipaplatform/base/paths.py b/ipaplatform/base/paths.py
index 4aa55d870bc9fbea1f67d28fef9bbb3c0a2d836f..585a5d26ed32a5f60cdb5d28de05b6468d03baa6 100644
--- a/ipaplatform/base/paths.py
+++ b/ipaplatform/base/paths.py
@@ -127,6 +127,8 @@ class BasePathNamespace(object):
 SYSCONFIG_PKI_TOMCAT = "/etc/sysconfig/pki-tomcat"
 

Re: [Freeipa-devel] [WIP PATCH] server-del: perform full master removal in managed topology

2016-04-21 Thread Ludwig Krispenz


On 04/21/2016 10:11 AM, Martin Babinsky wrote:

On 04/21/2016 09:21 AM, Jan Cholasta wrote:

On 19.4.2016 12:42, Martin Babinsky wrote:

On 04/14/2016 11:46 AM, Ludwig Krispenz wrote:


On 04/14/2016 10:59 AM, Martin Babinsky wrote:

On 04/14/2016 08:24 AM, Jan Cholasta wrote:

On 13.4.2016 17:10, Rob Crittenden wrote:

Martin Babinsky wrote:

This is a WIP patch which moves the `ipa-replica-manage del`
subcommand
to the 'server-del' API method and exposes it as CLI command[1].
A CI
test suite is also included.

There are some issues with the patch I would like to discuss in 
more

detail on the list:

1.) In the original subcommand there was a lot of output (mostly
print
statements) during all stages of master removal. I have tried to
port
these as messages to the command which results in quite voluminous
response sent back to the frontend. Should we try to reduce the
output?


I don't think it applies anymore. These messages were there so the
user
would know something was happening. If it is an API command there
isn't
much we can do other than add something to the cli to print "This
could
take a bit" before making the call.


+1


This is already implemented in PoC. So I guess we may reduce the
output only to the following:


In CLI print:
"Removing {server} from replication topology, "
"please wait...

The adding info messages:

"checking topology connectivity" | "skipping topology connectivity
check"
"checking remaining services" | "skipping check for remaining 
services"

"performing cleanup"
"Deleted server {server}"





2.) In the original discussion[2] we assumed that the cleanup part
would
me a separate API method called during server_del postcallback.
However
since the two objects ended up sharing a lot of state (e.g. 
topology

state from pre-callback, messages) i have merged it to server-del.
That
makes the code rather unwieldy but I found it difficult to keep 
the

two
entities separate without some hacking around framework 
capabilities


I haven't looked at the code but as a general principal having
separate
operations has saved our bacon on more than one occasion.


The patch adds a force option, which allows you to re-run server-del
even if the master entry does not exist anymore, so I think we are
safe.




3.) since actions in post-callback require a knowledge about
topology
state gathered in pre-callback, I had to store some information in
the
command's context. Sorry about that, if you know about some way to
circumvent me, let me know.


Looks like it is the only way since you are extending server_del.
Another option would be to drop pre/post and add all this topology
stuff
directly to server_del execute.


4.) The master can not remove itself. I have implemented an ad-hoc
forwarding of the request to other master that can do the job. Is
this
okay?


Why can't the master remove itself?

Because it removes its own replication agreements hence any 
changes in
DIT (like removed principals, s4u2 proxy targets etc.) won't 
replicate

to other masters.

It shouldn't remove replication agreements, in fact this should be
prevented by the topology plugin.
The removal of the agreements will be triggered by removing the master
entry


That is true, but there is a plenty of cleanup code that is executed
*after* the master entry is removed and these changes would not
replicate if the agreements were removed by topology plugin in the
meanwhile.


What kind of cleanup is it? Can it be done before instead?



Well most of the code can be run in pre-callback if all the checks are 
passed/forced.


However there is a check for deleted segments and this one should be 
run after the removal of master entry to see if topology plugin 
removed all dangling segments pointing to master. I am not quite sure 
if it make sense to run this check in the master which is being removed.
no, it is not guranteed that the information on the removed master will 
be correct. If the del is applied to the to be removed master the 
topology plugin will only on a master which is remaining start the 
removal of segments, these will alos be replicated back to the removed 
master, but the repl agreements to this master will also be removed, so 
no gurantee which mods will be available on the removed master, and you 
should also be able to remove a master if it is down - so applying the 
full removal on a remaining server makes sense.


What is the behaviour if the removal of a server would disconnect topology ?


--
Red Hat GmbH, http://www.de.redhat.com/, Registered seat: Grasbrunn,
Commercial register: Amtsgericht Muenchen, HRB 153243,
Managing Directors: Paul Argiry, Charles Cachera, Michael Cunningham, Michael 
O'Neill

--
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] [WIP PATCH] server-del: perform full master removal in managed topology

2016-04-21 Thread Martin Babinsky

On 04/21/2016 09:21 AM, Jan Cholasta wrote:

On 19.4.2016 12:42, Martin Babinsky wrote:

On 04/14/2016 11:46 AM, Ludwig Krispenz wrote:


On 04/14/2016 10:59 AM, Martin Babinsky wrote:

On 04/14/2016 08:24 AM, Jan Cholasta wrote:

On 13.4.2016 17:10, Rob Crittenden wrote:

Martin Babinsky wrote:

This is a WIP patch which moves the `ipa-replica-manage del`
subcommand
to the 'server-del' API method and exposes it as CLI command[1].
A CI
test suite is also included.

There are some issues with the patch I would like to discuss in more
detail on the list:

1.) In the original subcommand there was a lot of output (mostly
print
statements) during all stages of master removal. I have tried to
port
these as messages to the command which results in quite voluminous
response sent back to the frontend. Should we try to reduce the
output?


I don't think it applies anymore. These messages were there so the
user
would know something was happening. If it is an API command there
isn't
much we can do other than add something to the cli to print "This
could
take a bit" before making the call.


+1


This is already implemented in PoC. So I guess we may reduce the
output only to the following:


In CLI print:
"Removing {server} from replication topology, "
"please wait...

The adding info messages:

"checking topology connectivity" | "skipping topology connectivity
check"
"checking remaining services" | "skipping check for remaining services"
"performing cleanup"
"Deleted server {server}"





2.) In the original discussion[2] we assumed that the cleanup part
would
me a separate API method called during server_del postcallback.
However
since the two objects ended up sharing a lot of state (e.g. topology
state from pre-callback, messages) i have merged it to server-del.
That
makes the code rather unwieldy but I found it difficult to keep the
two
entities separate without some hacking around framework capabilities


I haven't looked at the code but as a general principal having
separate
operations has saved our bacon on more than one occasion.


The patch adds a force option, which allows you to re-run server-del
even if the master entry does not exist anymore, so I think we are
safe.




3.) since actions in post-callback require a knowledge about
topology
state gathered in pre-callback, I had to store some information in
the
command's context. Sorry about that, if you know about some way to
circumvent me, let me know.


Looks like it is the only way since you are extending server_del.
Another option would be to drop pre/post and add all this topology
stuff
directly to server_del execute.


4.) The master can not remove itself. I have implemented an ad-hoc
forwarding of the request to other master that can do the job. Is
this
okay?


Why can't the master remove itself?


Because it removes its own replication agreements hence any changes in
DIT (like removed principals, s4u2 proxy targets etc.) won't replicate
to other masters.

It shouldn't remove replication agreements, in fact this should be
prevented by the topology plugin.
The removal of the agreements will be triggered by removing the master
entry


That is true, but there is a plenty of cleanup code that is executed
*after* the master entry is removed and these changes would not
replicate if the agreements were removed by topology plugin in the
meanwhile.


What kind of cleanup is it? Can it be done before instead?



Well most of the code can be run in pre-callback if all the checks are 
passed/forced.


However there is a check for deleted segments and this one should be run 
after the removal of master entry to see if topology plugin removed all 
dangling segments pointing to master. I am not quite sure if it make 
sense to run this check in the master which is being removed.


--
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] V4/RFC 2818 review

2016-04-21 Thread Fraser Tweedale
On Thu, Apr 21, 2016 at 10:22:33AM +0300, Alexander Bokovoy wrote:
> On Thu, 21 Apr 2016, Fraser Tweedale wrote:
> >On Tue, Apr 19, 2016 at 11:06:15AM -0400, Rob Crittenden wrote:
> >>Christian Heimes wrote:
> >>>Hi Fraser,
> >>>
> >>>and now to the review of your design doc for RFC 2818-compliant subject
> >>>alternative names in certs,
> >>>http://www.freeipa.org/page/V4/RFC_2818_certificate_compliance
> >>>
> >>>
> >>>1) RFC 2818 vs. RFC 6125
> >>>
> >>>First I like to address a more general topic. Your design mentions RFC
> >>>6125 shortly. IMHO RFC 6125 supersedes 2818 for CN/SAN hostname
> >>>verification and we should follow the rules in RFC 6125, whenever 2818
> >>>lacks specification or there is a conflict between both RFCs. I can tell
> >>>you some horror stories from Python's ssl module related to both RFCs.
> >>>
> >>>https://tools.ietf.org/html/rfc2818, HTTP Over TLS
> >>>
> >>>https://tools.ietf.org/html/rfc6125, Representation and Verification of
> >>>Domain-Based Application Service Identity within Internet Public Key
> >>>Infrastructure Using X.509 (PKIX) Certificates in the Context of
> >>>Transport Layer Security (TLS)
> >>>
> >>>As far as I'm familiar with RFC 6125, your proposal doesn't conflict
> >>>with the more modern RFC. It also makes sense to name the design after
> >>>the RFC, which has deprecated CN. I still like to check your design
> >>>against RFC 6125.
> >>>
> >>>Fraser, do you agree?
> >>>
> >>>
> >>>2) SAN validation in ipa cert-request
> >>>
> >>>In the paragraph "ipa cert-request changes" you write that the plugin
> >>>"[...] ensure that one element of the DNS names list matches the
> >>>principal name". Shouldn't the plugin validate *all* DNS names and
> >>>verify that the principal is allowed to request a cert for all fields in
> >>>SAN?
> >>
> >>Are there plans for any other SAN types? IP address or other oddball types
> >>like MS UPN?
> >>
> >We support almost all of them in Dogtag, and only support a few
> >types in FreeIPA (dnsName, rfc822Name, KRB5PrincipalName, UPN).
> >
> >We should work out what we can do with IP address; I recall it is
> >needed (or wanted) for some cloud/IaaS use cases.
> Yes, some of Openstack code expects IP address in the certificates.
> 
> >DirectoryName would be simple to support (just check that it matches
> >DN of target principal).  I wonder if there is a use case for it, or
> >for any other SAN types...
> dNSName and id-pkinit-san are used by Kerberos PKINIT support in MIT
> Kerberos for host-based principals. id-pkinit-san is also in use for
> mapping of the principal in general.
> 
> In fact, Kerberos PKINIT retrieves all SANs and UPNs from the certificate and
> allows to match them by the matching rules -- id-pkinit-san goes to list
> of principals, id-ms-san-upn goes to the list of UPNs which are then
> merged together and can be addressed with  keyword in the matching
> rule.
> 
> This allows very flexible matching:
>pkinit_cert_match = ||.*DoE.*.*@EXAMPLE.COM
>pkinit_cert_match = &&msScLogin,clientAuth.*DoE.*
>pkinit_cert_match = 
> msScLogin,clientAuthdigitalSignature
> 
Thank you for the information!

> 
> 
> >>>
> >>>3) Should FreeIPA deprecate cert request without SAN or at least warn
> >>>the user?
> >>>
> >>>IMHO it makes sense to deprecate CN only cert requests.
> >>
> >>I'd mark it as deprecated over at least a major release in order to handle
> >>older versions that may still make requests without a SAN.
> >>
> >We have to be careful here.  CN is depreated for DNS name checking
> >in HTTP (or other network protocols using TLS).  Fine - but there
> >could be other certificate use cases that require CN, e.g. user
> >certs where Subject DN corresponds to a directory name.
> Yes. pkinit_cert_match's  rule is using Subject DN for checks.
> 
> 
> >We can deprecate it and eventually reject requests that include CN -
> >but only for service cert profile(s).  (This would require a new
> >profile component designed for this purpose).
> Please do not do it. There are known uses for it at least with Kerberos.
> Of course, most of them are rather for user certificates but in reality
> Kerberos does not require you to have service principals always as DNS
> names.
> 
There is the option of implementing the component that prohibits CN.
I was by no means suggesting it should be used for all profiles.
Customers can use it on a custom profile if they want.  We could use
it on the default service cert profile if we want.

Cheers,
Fraser

-- 
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] V4/RFC 2818 review

2016-04-21 Thread Alexander Bokovoy

On Thu, 21 Apr 2016, Fraser Tweedale wrote:

On Tue, Apr 19, 2016 at 11:06:15AM -0400, Rob Crittenden wrote:

Christian Heimes wrote:
>Hi Fraser,
>
>and now to the review of your design doc for RFC 2818-compliant subject
>alternative names in certs,
>http://www.freeipa.org/page/V4/RFC_2818_certificate_compliance
>
>
>1) RFC 2818 vs. RFC 6125
>
>First I like to address a more general topic. Your design mentions RFC
>6125 shortly. IMHO RFC 6125 supersedes 2818 for CN/SAN hostname
>verification and we should follow the rules in RFC 6125, whenever 2818
>lacks specification or there is a conflict between both RFCs. I can tell
>you some horror stories from Python's ssl module related to both RFCs.
>
>https://tools.ietf.org/html/rfc2818, HTTP Over TLS
>
>https://tools.ietf.org/html/rfc6125, Representation and Verification of
>Domain-Based Application Service Identity within Internet Public Key
>Infrastructure Using X.509 (PKIX) Certificates in the Context of
>Transport Layer Security (TLS)
>
>As far as I'm familiar with RFC 6125, your proposal doesn't conflict
>with the more modern RFC. It also makes sense to name the design after
>the RFC, which has deprecated CN. I still like to check your design
>against RFC 6125.
>
>Fraser, do you agree?
>
>
>2) SAN validation in ipa cert-request
>
>In the paragraph "ipa cert-request changes" you write that the plugin
>"[...] ensure that one element of the DNS names list matches the
>principal name". Shouldn't the plugin validate *all* DNS names and
>verify that the principal is allowed to request a cert for all fields in
>SAN?

Are there plans for any other SAN types? IP address or other oddball types
like MS UPN?


We support almost all of them in Dogtag, and only support a few
types in FreeIPA (dnsName, rfc822Name, KRB5PrincipalName, UPN).

We should work out what we can do with IP address; I recall it is
needed (or wanted) for some cloud/IaaS use cases.

Yes, some of Openstack code expects IP address in the certificates.


DirectoryName would be simple to support (just check that it matches
DN of target principal).  I wonder if there is a use case for it, or
for any other SAN types...

dNSName and id-pkinit-san are used by Kerberos PKINIT support in MIT
Kerberos for host-based principals. id-pkinit-san is also in use for
mapping of the principal in general. 


In fact, Kerberos PKINIT retrieves all SANs and UPNs from the certificate and
allows to match them by the matching rules -- id-pkinit-san goes to list
of principals, id-ms-san-upn goes to the list of UPNs which are then
merged together and can be addressed with  keyword in the matching
rule.

This allows very flexible matching:
   pkinit_cert_match = ||.*DoE.*.*@EXAMPLE.COM
   pkinit_cert_match = &&msScLogin,clientAuth.*DoE.*
   pkinit_cert_match = msScLogin,clientAuthdigitalSignature




>
>3) Should FreeIPA deprecate cert request without SAN or at least warn
>the user?
>
>IMHO it makes sense to deprecate CN only cert requests.

I'd mark it as deprecated over at least a major release in order to handle
older versions that may still make requests without a SAN.


We have to be careful here.  CN is depreated for DNS name checking
in HTTP (or other network protocols using TLS).  Fine - but there
could be other certificate use cases that require CN, e.g. user
certs where Subject DN corresponds to a directory name.

Yes. pkinit_cert_match's  rule is using Subject DN for checks.



We can deprecate it and eventually reject requests that include CN -
but only for service cert profile(s).  (This would require a new
profile component designed for this purpose).

Please do not do it. There are known uses for it at least with Kerberos.
Of course, most of them are rather for user certificates but in reality
Kerberos does not require you to have service principals always as DNS
names.

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


Re: [Freeipa-devel] [WIP PATCH] server-del: perform full master removal in managed topology

2016-04-21 Thread Jan Cholasta

On 19.4.2016 12:42, Martin Babinsky wrote:

On 04/14/2016 11:46 AM, Ludwig Krispenz wrote:


On 04/14/2016 10:59 AM, Martin Babinsky wrote:

On 04/14/2016 08:24 AM, Jan Cholasta wrote:

On 13.4.2016 17:10, Rob Crittenden wrote:

Martin Babinsky wrote:

This is a WIP patch which moves the `ipa-replica-manage del`
subcommand
to the 'server-del' API method and exposes it as CLI command[1]. A CI
test suite is also included.

There are some issues with the patch I would like to discuss in more
detail on the list:

1.) In the original subcommand there was a lot of output (mostly
print
statements) during all stages of master removal. I have tried to port
these as messages to the command which results in quite voluminous
response sent back to the frontend. Should we try to reduce the
output?


I don't think it applies anymore. These messages were there so the
user
would know something was happening. If it is an API command there
isn't
much we can do other than add something to the cli to print "This
could
take a bit" before making the call.


+1


This is already implemented in PoC. So I guess we may reduce the
output only to the following:


In CLI print:
"Removing {server} from replication topology, "
"please wait...

The adding info messages:

"checking topology connectivity" | "skipping topology connectivity
check"
"checking remaining services" | "skipping check for remaining services"
"performing cleanup"
"Deleted server {server}"





2.) In the original discussion[2] we assumed that the cleanup part
would
me a separate API method called during server_del postcallback.
However
since the two objects ended up sharing a lot of state (e.g. topology
state from pre-callback, messages) i have merged it to server-del.
That
makes the code rather unwieldy but I found it difficult to keep the
two
entities separate without some hacking around framework capabilities


I haven't looked at the code but as a general principal having
separate
operations has saved our bacon on more than one occasion.


The patch adds a force option, which allows you to re-run server-del
even if the master entry does not exist anymore, so I think we are
safe.




3.) since actions in post-callback require a knowledge about topology
state gathered in pre-callback, I had to store some information in
the
command's context. Sorry about that, if you know about some way to
circumvent me, let me know.


Looks like it is the only way since you are extending server_del.
Another option would be to drop pre/post and add all this topology
stuff
directly to server_del execute.


4.) The master can not remove itself. I have implemented an ad-hoc
forwarding of the request to other master that can do the job. Is
this
okay?


Why can't the master remove itself?


Because it removes its own replication agreements hence any changes in
DIT (like removed principals, s4u2 proxy targets etc.) won't replicate
to other masters.

It shouldn't remove replication agreements, in fact this should be
prevented by the topology plugin.
The removal of the agreements will be triggered by removing the master
entry


That is true, but there is a plenty of cleanup code that is executed
*after* the master entry is removed and these changes would not
replicate if the agreements were removed by topology plugin in the
meanwhile.


What kind of cleanup is it? Can it be done before instead?

--
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 0463] Performance: do not download password attributes in host/find-user command

2016-04-21 Thread Martin Basti



On 20.04.2016 16:57, Martin Basti wrote:

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

Patch attached.



selfNACK
-- 
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] [WIP PATCH] server-del: perform full master removal in managed topology

2016-04-21 Thread Jan Cholasta

On 19.4.2016 13:49, Martin Babinsky wrote:

On 04/14/2016 10:48 AM, Martin Babinsky wrote:

On 04/14/2016 08:42 AM, Jan Cholasta wrote:

Hi,

On 13.4.2016 16:49, Martin Babinsky wrote:

This is a WIP patch which moves the `ipa-replica-manage del` subcommand
to the 'server-del' API method and exposes it as CLI command[1]. A CI
test suite is also included.



`server-del` now accepts the following options:
* `--cleanup`: perform a cleanup after an already deleted master


I would prefer if this was actually called --force, for reasons
explained in the design thread:
.



* `--force-removal`: force master removal, i.e. ignore topology errors


So, this is actually the all-powerful --force option we always try to
avoid, but with a different name (and not very good one - if you are
removing something, what other than removal would you need to force?).

Could you split this into separate options?


There are actually two checks that we need to pass/bypass before we can
remove the master entry and run all the cleanup shenanigans:

1.) the topology is not disconnected already or is not being
disconnected by the action

2.) the action does leave at least one CA/DNS server, does not remove
DNSSec keymaster and we can promote other master to CA renewal master

So IIUC we would need three options actually:

* one that bypasses topology checks ('--ignore-topology-disconnect')
* one that bypasses the check for remaining services
('--ignore-last-services?')
* one that will cleanup leftovers only, ignoring NotFound error
('--cleanup'), this one is already there


Actually '--force' should replace '--cleanup' as it does basically the
same job.


Right.


What about the remaining two proposed options?


--ignore-topology-disconnect is good. The other one should use "role" 
rather than "service", e.g. --ignore-last-of-role.


--
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] [PATCHES 551-552] ipalib: add basecert plugins

2016-04-21 Thread Jan Cholasta

On 6.4.2016 15:46, Pavel Vomacka wrote:



On 03/16/2016 01:50 PM, Jan Cholasta wrote:

Hi,

the attached patches implement the server-side part of
.

Honza


Hi,

thank you for the patches. I tested them and they work well. But I would
like to ask you whether would be possible to extend the response of
'basecert_find' method and probably also 'basecert_show' response. I
think of these information:

1) information whether the certificate is issued by our CA or not.


You can check for that by comparing the issuer name of the certificate 
to "CN=Certificate Authority,$SUBJECT_BASE". You can get subject base 
from config-show.




2) this probably wouldn't be possible (as we discussed), but I rather
write it too - the information about revocation reason. The same as the
'cert_show' provides.


Added --check-revocation flag to request this information. Currently it 
works only on certificates issued by our CA.




3) MD5 and SHA1 fingerprints as the 'cert_show' method returns


Added, also included SHA-256.



Thank you again.


Updated patches attached.

--
Jan Cholasta
From 1fa86fdec1d4adf0113038d78503e110af84e64b Mon Sep 17 00:00:00 2001
From: Jan Cholasta 
Date: Wed, 16 Mar 2016 13:09:11 +0100
Subject: [PATCH 1/2] ldap: fix handling of binary data in search filters

This fixes a UnicodeDecodeError when passing non-UTF-8 binary data to
LDAPClient.make_filter() and friends.

https://fedorahosted.org/freeipa/ticket/5381
---
 ipapython/ipaldap.py | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/ipapython/ipaldap.py b/ipapython/ipaldap.py
index 405f1ee..eb56ad2 100644
--- a/ipapython/ipaldap.py
+++ b/ipapython/ipaldap.py
@@ -1208,7 +1208,12 @@ class LDAPClient(object):
 ]
 return self.combine_filters(flts, rules)
 elif value is not None:
-value = ldap.filter.escape_filter_chars(value_to_utf8(value))
+if isinstance(value, bytes):
+if six.PY3:
+value = value.decode('raw_unicode_escape')
+else:
+value = value_to_utf8(value)
+value = ldap.filter.escape_filter_chars(value)
 if not exact:
 template = '%s'
 if leading_wildcard:
-- 
2.5.5

From aae7d41b8e547ddb360d8b88cf2c572b08c82ddd Mon Sep 17 00:00:00 2001
From: Jan Cholasta 
Date: Wed, 16 Mar 2016 13:11:05 +0100
Subject: [PATCH 2/2] api: add basecert plugins

Introduces basecert-show and basecert-find commands for getting
information about certificates.

The commands allow querying for a specific certificate or all certificates
owned by a specific user, host or service and return selected fields
extracted from the certificate as well as owner information.

https://fedorahosted.org/freeipa/ticket/5381
---
 API.txt|  36 
 VERSION|   4 +-
 ipalib/plugins/basecert.py | 497 +
 3 files changed, 535 insertions(+), 2 deletions(-)
 create mode 100644 ipalib/plugins/basecert.py

diff --git a/API.txt b/API.txt
index 3598b08..23841ae 100644
--- a/API.txt
+++ b/API.txt
@@ -444,6 +444,42 @@ option: Str('version?', exclude='webui')
 output: Entry('result', , Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None))
 output: Output('summary', (, ), None)
 output: PrimaryKey('value', None, None)
+command: basecert_find
+args: 1,16,4
+arg: Str('criteria?', noextrawhitespace=False)
+option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
+option: Bytes('certificate', attribute=True, autofill=False, cli_name='certificate', multivalue=False, primary_key=True, query=True, required=False)
+option: Flag('check_revocation', autofill=True, default=False)
+option: File('file?', include='cli')
+option: Str('host*', cli_name='hosts', csv=True)
+option: Str('no_host*', cli_name='no_hosts', csv=True)
+option: Flag('no_members', autofill=True, default=False, exclude='webui')
+option: Str('no_service*', cli_name='no_services', csv=True)
+option: Str('no_user*', cli_name='no_users', csv=True)
+option: Flag('pkey_only?', autofill=True, default=False)
+option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui')
+option: Str('service*', cli_name='services', csv=True)
+option: Int('sizelimit?', autofill=False, minvalue=0)
+option: Int('timelimit?', autofill=False, minvalue=0)
+option: Str('user*', cli_name='users', csv=True)
+option: Str('version?', exclude='webui')
+output: Output('count', , None)
+output: ListOfEntries('result', (, ), Gettext('A list of LDAP entries', domain='ipa', localedir=None))
+output: Output('summary', (, ), None)
+output: Output('truncated', , None)
+command: basecert_show
+args: 1,7,3
+arg: File('file?', include='cli')
+option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
+option: Bytes('certificate', 

Re: [Freeipa-devel] V4/RFC 2818 review

2016-04-21 Thread Fraser Tweedale
On Tue, Apr 19, 2016 at 04:14:01PM +0200, Christian Heimes wrote:
> Hi Fraser,
> 
> and now to the review of your design doc for RFC 2818-compliant subject
> alternative names in certs,
> http://www.freeipa.org/page/V4/RFC_2818_certificate_compliance
> 
> 
> 1) RFC 2818 vs. RFC 6125
> 
> First I like to address a more general topic. Your design mentions RFC
> 6125 shortly. IMHO RFC 6125 supersedes 2818 for CN/SAN hostname
> verification and we should follow the rules in RFC 6125, whenever 2818
> lacks specification or there is a conflict between both RFCs. I can tell
> you some horror stories from Python's ssl module related to both RFCs.
> 
> https://tools.ietf.org/html/rfc2818, HTTP Over TLS
> 
> https://tools.ietf.org/html/rfc6125, Representation and Verification of
> Domain-Based Application Service Identity within Internet Public Key
> Infrastructure Using X.509 (PKIX) Certificates in the Context of
> Transport Layer Security (TLS)
> 
> As far as I'm familiar with RFC 6125, your proposal doesn't conflict
> with the more modern RFC. It also makes sense to name the design after
> the RFC, which has deprecated CN. I still like to check your design
> against RFC 6125.
> 
> Fraser, do you agree?
> 
The scope of RFC 6125 seems much larger than RFC 2818, e.g. moving
toward support for SRVName and uniformResourceIdentifier, and
deprecating wildcard certs.

It absolutely makes sense to check my design against RFC 6125 but I
don't think it makes sense to indicate that the design aspires to
"RFC 6125 compliance" (it does not).

I think I will leave the design name and scope as-is but I will
update the design to mention RFC 6125 and the need to comply with
relevant aspects thereof.

> 
> 2) SAN validation in ipa cert-request
> 
> In the paragraph "ipa cert-request changes" you write that the plugin
> "[...] ensure that one element of the DNS names list matches the
> principal name". Shouldn't the plugin validate *all* DNS names and
> verify that the principal is allowed to request a cert for all fields in
> SAN?
> 
The preceding point states:

 For each dNSName in the subjectAltName extension, in addition
 to the existing checks, append the value to the list of DNS
 names.

These existing checks ensure that each dnsName matches a principal
that is "managed by" the nominated subject principal (nb: each
host/service manages itself).

The final point ensure that one of the dnsNames does actually
correspond to the subject principal.  i.e. you can't issue
containing dnsNames managed by the subject principal, yet omit a
dnsName for the subject principal itself.  I will expand the wording
of the design to (try to) clarify this.

> 
> 3) Should FreeIPA deprecate cert request without SAN or at least warn
> the user?
> 
> IMHO it makes sense to deprecate CN only cert requests.
> 
See my response to Rob's reply.

> 
> 4) update "Issue New Certificate for Host" dialog and documentation
> 
> The web UI has an update "Issue New Certificate for Host" dialog which
> explains how to create a CSR with certutil. This dialog should be
> updated to explain how to add a SAN DNS field. The option for SAN DNS is
> '-8 fqdn' or '--extSAN dns:fqdn', e.g.
> 
> Create a CSR with subject CN=,O=, for example:
> # certutil -R -d  -a -g  -s
> 'CN=client1.ipa.example,O=IPA.EXAMPLE' -8 'client1.ipa.example'
> 
Yes, good idea.

Thanks for the review!
Fraser

-- 
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] V4/RFC 2818 review

2016-04-21 Thread Fraser Tweedale
On Tue, Apr 19, 2016 at 11:06:15AM -0400, Rob Crittenden wrote:
> Christian Heimes wrote:
> >Hi Fraser,
> >
> >and now to the review of your design doc for RFC 2818-compliant subject
> >alternative names in certs,
> >http://www.freeipa.org/page/V4/RFC_2818_certificate_compliance
> >
> >
> >1) RFC 2818 vs. RFC 6125
> >
> >First I like to address a more general topic. Your design mentions RFC
> >6125 shortly. IMHO RFC 6125 supersedes 2818 for CN/SAN hostname
> >verification and we should follow the rules in RFC 6125, whenever 2818
> >lacks specification or there is a conflict between both RFCs. I can tell
> >you some horror stories from Python's ssl module related to both RFCs.
> >
> >https://tools.ietf.org/html/rfc2818, HTTP Over TLS
> >
> >https://tools.ietf.org/html/rfc6125, Representation and Verification of
> >Domain-Based Application Service Identity within Internet Public Key
> >Infrastructure Using X.509 (PKIX) Certificates in the Context of
> >Transport Layer Security (TLS)
> >
> >As far as I'm familiar with RFC 6125, your proposal doesn't conflict
> >with the more modern RFC. It also makes sense to name the design after
> >the RFC, which has deprecated CN. I still like to check your design
> >against RFC 6125.
> >
> >Fraser, do you agree?
> >
> >
> >2) SAN validation in ipa cert-request
> >
> >In the paragraph "ipa cert-request changes" you write that the plugin
> >"[...] ensure that one element of the DNS names list matches the
> >principal name". Shouldn't the plugin validate *all* DNS names and
> >verify that the principal is allowed to request a cert for all fields in
> >SAN?
> 
> Are there plans for any other SAN types? IP address or other oddball types
> like MS UPN?
> 
We support almost all of them in Dogtag, and only support a few
types in FreeIPA (dnsName, rfc822Name, KRB5PrincipalName, UPN).

We should work out what we can do with IP address; I recall it is
needed (or wanted) for some cloud/IaaS use cases.

DirectoryName would be simple to support (just check that it matches
DN of target principal).  I wonder if there is a use case for it, or
for any other SAN types...

> >
> >3) Should FreeIPA deprecate cert request without SAN or at least warn
> >the user?
> >
> >IMHO it makes sense to deprecate CN only cert requests.
> 
> I'd mark it as deprecated over at least a major release in order to handle
> older versions that may still make requests without a SAN.
> 
We have to be careful here.  CN is depreated for DNS name checking
in HTTP (or other network protocols using TLS).  Fine - but there
could be other certificate use cases that require CN, e.g. user
certs where Subject DN corresponds to a directory name.

We can deprecate it and eventually reject requests that include CN -
but only for service cert profile(s).  (This would require a new
profile component designed for this purpose).

> >
> >4) update "Issue New Certificate for Host" dialog and documentation
> >
> >The web UI has an update "Issue New Certificate for Host" dialog which
> >explains how to create a CSR with certutil. This dialog should be
> >updated to explain how to add a SAN DNS field. The option for SAN DNS is
> >'-8 fqdn' or '--extSAN dns:fqdn', e.g.
> >
> >Create a CSR with subject CN=,O=, for example:
> ># certutil -R -d  -a -g  -s
> >'CN=client1.ipa.example,O=IPA.EXAMPLE' -8 'client1.ipa.example'
> 
> rob
> 

-- 
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] V4/Sub-CAs review

2016-04-21 Thread Fraser Tweedale
Christian, thank you for the review.

Responses inline.  I will update the design page soon with
clarifications and information about backup.

On Tue, Apr 19, 2016 at 01:24:54PM +0200, Christian Heimes wrote:
> Hi Fraser,
> 
> I'm the reviewer for your Sub-CAs and RFC 2818 designs. Let's start with
> Sub-CAs first. http://www.freeipa.org/page/V4/Sub-CAs
> 
> In general the design is well written -- accurate as usual. I didn't
> want to ACK the design with a simple LGTM, so I put myself in the
> position of a customer and potential user of Sub-CAs. From the end-users
> perspective couple of points in the design doc are either unclear or are
> not addressed details.
> 
> 
> 1) How can I restrict a Sub-CA to a specific key usage or DNS suffix?
> 
> The design doc mentions a comment from the puppet community or the
> possibility to use a SubCA for short-lived certs for VPN authentication.
> As a customer I would like to restrict the KU, EKU and maybe name
> constraints, e.g. a SubCA for hosts should be limited to EKU "TLS
> webserver auth". Would it be possible to use a custom profile to
> generate a SubCA and let users select the profile in ipa ca-add?
> 
For things that go in EE cert, specify the target sub-CA in addition
an appropriate profile.  (More on that below).

For things that go in CA cert (e.g. NameConstraints) - not an
initial requirement but definitely a nice-to-have - a
special-purpose profile is needed.  Such profiles can be
added/managed via the `certprofile' plugin.  Dogtag will have to be
updated to provide the means of specifying an alternative profile
when creating the CA (see ticket[1]), and the means of conveying any
data required.  These options will be reflected in the ca-add
options.

[1] https://fedorahosted.org/pki/ticket/1338


> 2) What is the relationship between Sub-CAs and profiles?
> 
> From the design doc it is unclear how cert profiles and Sub-CAs
> interact. The certificate profile doc has
> http://www.freeipa.org/page/V4/Certificate_Profiles#Schema_2, but that's
> too technical. I'm not even sure I fully understand the meaning of the
> schema and how memberCa affects profiles.
> 
tl;dr profiles and authorities don't affect each other, but caacls
determine which profiles and authorities can be used together, for
which subject principals.

CA ACLs currently determine which profiles can be used with which
subject principals.  With Lightweight CAs, CA ACLs are expanded in
scope to also include which CA(s) can be used with which profiles
for a particular subject principal.

This is what the memberCa attribute of the caacl object class
accomplishes.  A single caacl rule allows specified (or all)
profiles to be used by specified (or all - if caCategory=all) CAs to
issue certificates to specified principals or principal types.

> 
> 3) How can I make FreeIPA use a specific Sub-CA in a cert request?
> 
> IMO a 1:n relationship between CAs and profiles would make sense. That
> way ipa cert-request --profile-id=caVPNCert could automatically select
> the VPN Sub-CA.
> 
Simply supply the ``--ca `` option.
http://www.freeipa.org/page/V4/Sub-CAs#ipa_cert-request

Your suggestion about a default sub-CA for a profile is interesting.
We still need the option to specify both CA and profile but it is
good UX becaues it will cover many use cases.  I filed a ticket[2].

[2] https://fedorahosted.org/freeipa/ticket/5836

> 
> 4) Where is the private key of a Sub-CAs stored locally and how is it
> secured?

> Customers will like to know where Dogtag keeps its crown jewels and how
> they are secured.
> 
Stored in the NSSDB.  For key replication, Custodia transports the
key wrapped by the signing key of the "host authority" (i.e. the CA
that Dogtag was installed with).  There's a ticket for HSM
support[3] but FreeIPA doesn't support HSM yet, anyway.

[3] https://fedorahosted.org/pki/ticket/2292

> 
> 5) What is the backup and export strategy for a Sub-CA private key?
> 
> Similar to 4), customers want to backup the private keys.
> 
I'll look at what the main IPA backup process does.  If it already
backs up the whole of /etc/pki/pki-tomcat/alias, we're good.  If
not, I'll extend it to include whatever is needed.

> 
> Christian
> 

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