Re: [Freeipa-devel] [PATCH 0065] Fix ugly quit during external CA installation

2016-10-05 Thread David Kupka

On 23/08/16 13:58, Standa Laznicka wrote:

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



Thanks for the patch. This fixes the ugly error message and also the 
return code, ACK.


Pushed to:
master: 889f0863b80a0c13a14aa69cd8563b5adde984b2
ipa-4-4: 03a0f5a105f5625e6a4d373abb1f4d8b8044a026

--
David Kupka

--
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] 0065, 66: webui: authentication indicators on host page

2016-06-30 Thread Petr Vobornik
On 06/29/2016 06:38 PM, Petr Vobornik wrote:
> On 06/28/2016 04:32 PM, Pavel Vomacka wrote:
>> Hello,
>>
>> please review attached patches. I moved strings used by authentication
>> indicators widget to another dict so the second patch changes strings in
>> custom_checkbox widget on service page.
>>
>> https://fedorahosted.org/freeipa/ticket/5872
>>
> 
> 
> ACK
> 
> push should wait on server side.
> 

master:
* 55049fceb978f2e20b13800b5377428de386 Add authentication
identificator to host page
* ec6925e775598602e909d7a1f226f0c1e28cb074 Change paths of strings in
auth indicators widget on service page
-- 
Petr Vobornik

-- 
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] 0065, 66: webui: authentication indicators on host page

2016-06-29 Thread Petr Vobornik
On 06/28/2016 04:32 PM, Pavel Vomacka wrote:
> Hello,
> 
> please review attached patches. I moved strings used by authentication
> indicators widget to another dict so the second patch changes strings in
> custom_checkbox widget on service page.
> 
> https://fedorahosted.org/freeipa/ticket/5872
> 


ACK

push should wait on server side.

-- 
Petr Vobornik

-- 
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] 0065 Remove service and host cert issuer validation

2016-06-06 Thread Jan Cholasta

On 3.6.2016 07:15, Fraser Tweedale wrote:

The attached patch enables cert issuance to hosts and services using
sub-CAs.


Thanks, ACK.

Rebased and pushed to master: fa149cff86a67ebfe2739df6467a6e10e47742cd

--
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 0065] ipa-replica-install prints incorrect error message when replica is already installed

2015-12-11 Thread Martin Basti

ACK

Pushed to master: 12e7f71600e62eab9d48a13fba37d2f182c8bdee

On 09.12.2015 14:44, Gabe Alford wrote:

Fixed. Updated patch attached.

On Wed, Dec 9, 2015 at 2:37 AM, Martin Basti > wrote:


NACK

Patch contains syntax error, missing brace

ipaserver/install/server/replicainstall.py:850:
[E0001(syntax-error), ] invalid syntax)

Martin


On 09.12.2015 07:08, Jan Cholasta wrote:

LGTM

On 8.12.2015 17:04, Gabe Alford wrote:

Updated patch attached.

On Tue, Dec 8, 2015 at 8:27 AM, Martin Basti

>> wrote:



On 08.12.2015 16:26, Gabe Alford wrote:

Just to confirm:

if server is installed:
 Let's stop here and not do anything else

if domain level 0:
 check if client installed and stop here

Right?

yes




On Tue, Dec 8, 2015 at 8:20 AM, Jan Cholasta

>> wrote:

On 8.12.2015 16:17, Martin Basti wrote:



On 08.12.2015 16:14, Jan Cholasta wrote:

On 8.12.2015 16:09, Martin Basti wrote:



On 01.12.2015 14:57, Gabe Alford
wrote:

Sorry guys, I forgot to add a
meaningful
subject to this message.
Ignore the previous thread start.

-- Forwarded message
--
From: *Gabe Alford*

>


>
   



Re: [Freeipa-devel] [PATCH 0065] ipa-replica-install prints incorrect error message when replica is already installed

2015-12-09 Thread Martin Basti

NACK

Patch contains syntax error, missing brace

ipaserver/install/server/replicainstall.py:850: [E0001(syntax-error), ] 
invalid syntax)


Martin

On 09.12.2015 07:08, Jan Cholasta wrote:

LGTM

On 8.12.2015 17:04, Gabe Alford wrote:

Updated patch attached.

On Tue, Dec 8, 2015 at 8:27 AM, Martin Basti > wrote:



On 08.12.2015 16:26, Gabe Alford wrote:

Just to confirm:

if server is installed:
 Let's stop here and not do anything else

if domain level 0:
 check if client installed and stop here

Right?

yes





On Tue, Dec 8, 2015 at 8:20 AM, Jan Cholasta > wrote:

On 8.12.2015 16:17, Martin Basti wrote:



On 08.12.2015 16:14, Jan Cholasta wrote:

On 8.12.2015 16:09, Martin Basti wrote:



On 01.12.2015 14:57, Gabe Alford wrote:

Sorry guys, I forgot to add a meaningful
subject to this message.
Ignore the previous thread start.

-- Forwarded message --
From: *Gabe Alford* 
>>
Date: Mon, Nov 30, 2015 at 7:31 PM
Subject: [PATCH 0065]
To: freeipa-devel 
>>


Hello,

Patch fix for the following tickets:

https://fedorahosted.org/freeipa/ticket/5022
https://fedorahosted.org/freeipa/ticket/5320

Thanks,

Gabe



ACK


NACK, you can't install a server over an already
installed client,
thus the original check is correct.

Ahh domain level 0, right, but this check can be added
before the client
check.


Yes.

With domain level 1, this check should stay there IMO.


Yes. It should say "IPA server is already configured" rather
than "IPA replica is already configured", though.

--
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 0065] ipa-replica-install prints incorrect error message when replica is already installed

2015-12-09 Thread Gabe Alford
Fixed. Updated patch attached.

On Wed, Dec 9, 2015 at 2:37 AM, Martin Basti  wrote:

> NACK
>
> Patch contains syntax error, missing brace
>
> ipaserver/install/server/replicainstall.py:850: [E0001(syntax-error), ]
> invalid syntax)
>
> Martin
>
>
> On 09.12.2015 07:08, Jan Cholasta wrote:
>
>> LGTM
>>
>> On 8.12.2015 17:04, Gabe Alford wrote:
>>
>>> Updated patch attached.
>>>
>>> On Tue, Dec 8, 2015 at 8:27 AM, Martin Basti >> > wrote:
>>>
>>>
>>>
>>> On 08.12.2015 16:26, Gabe Alford wrote:
>>>
 Just to confirm:

 if server is installed:
  Let's stop here and not do anything else

 if domain level 0:
  check if client installed and stop here

 Right?

>>> yes
>>>
>>>
>>>

 On Tue, Dec 8, 2015 at 8:20 AM, Jan Cholasta > wrote:

 On 8.12.2015 16:17, Martin Basti wrote:



 On 08.12.2015 16:14, Jan Cholasta wrote:

 On 8.12.2015 16:09, Martin Basti wrote:



 On 01.12.2015 14:57, Gabe Alford wrote:

 Sorry guys, I forgot to add a meaningful
 subject to this message.
 Ignore the previous thread start.

 -- Forwarded message --
 From: *Gabe Alford* 
 >>
 Date: Mon, Nov 30, 2015 at 7:31 PM
 Subject: [PATCH 0065]
 To: freeipa-devel 
 >>


 Hello,

 Patch fix for the following tickets:

 https://fedorahosted.org/freeipa/ticket/5022
 https://fedorahosted.org/freeipa/ticket/5320

 Thanks,

 Gabe



 ACK


 NACK, you can't install a server over an already
 installed client,
 thus the original check is correct.

 Ahh domain level 0, right, but this check can be added
 before the client
 check.


 Yes.

 With domain level 1, this check should stay there IMO.


 Yes. It should say "IPA server is already configured" rather
 than "IPA replica is already configured", though.

 --
 Jan Cholasta



>>>
>>>
>>
>>
>
From 41af20d4ef76186f4099858e12e6e954d282f70f Mon Sep 17 00:00:00 2001
From: Gabe 
Date: Wed, 9 Dec 2015 06:41:30 -0700
Subject: [PATCH] ipa-replica-install prints incorrect error message when
 replica is already installed

https://fedorahosted.org/freeipa/ticket/5022
https://fedorahosted.org/freeipa/ticket/5320
---
 ipaserver/install/server/replicainstall.py | 15 ---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/ipaserver/install/server/replicainstall.py b/ipaserver/install/server/replicainstall.py
index 4554166752ce4e5db2a98a8f495aa061aec963e9..1f4b133e1a11c915b229514456c8624148a741f1 100644
--- a/ipaserver/install/server/replicainstall.py
+++ b/ipaserver/install/server/replicainstall.py
@@ -31,9 +31,8 @@ from ipaserver.install import (
 bindinstance, ca, cainstance, certs, dns, dsinstance, httpinstance,
 installutils, kra, krainstance, krbinstance, memcacheinstance,
 ntpinstance, otpdinstance, custodiainstance, service)
-from ipaserver.install.installutils import create_replica_config
-from ipaserver.install.installutils import ReplicaConfig
-from ipaserver.install.installutils import load_pkcs12
+from ipaserver.install.installutils import (
+create_replica_config, ReplicaConfig, load_pkcs12, is_ipa_configured)
 from ipaserver.install.replication import (
 ReplicationManager, replica_conn_check)
 import SSSDConfig
@@ -423,6 +422,11 @@ def install_check(installer):
 
 tasks.check_selinux_status()
 
+if is_ipa_configured():
+sys.exit("IPA server is already configured on this system.\n"
+ "If you want to reinstall the IPA server, please uninstall "
+ "it first using 'ipa-server-install --uninstall'.")
+
 client_fstore = sysrestore.FileStore(paths.IPA_CLIENT_SYSRESTORE)
 if client_fstore.has_files():
 sys.exit("IPA client is already configured on this system.\n"
@@ -828,6 +832,11 @@ def 

Re: [Freeipa-devel] [PATCH 0065] ipa-replica-install prints incorrect error message when replica is already installed

2015-12-08 Thread Martin Basti



On 08.12.2015 16:14, Jan Cholasta wrote:

On 8.12.2015 16:09, Martin Basti wrote:



On 01.12.2015 14:57, Gabe Alford wrote:

Sorry guys, I forgot to add a meaningful subject to this message.
Ignore the previous thread start.

-- Forwarded message --
From: *Gabe Alford* >

Date: Mon, Nov 30, 2015 at 7:31 PM
Subject: [PATCH 0065]
To: freeipa-devel >


Hello,

Patch fix for the following tickets:

https://fedorahosted.org/freeipa/ticket/5022
https://fedorahosted.org/freeipa/ticket/5320

Thanks,

Gabe




ACK


NACK, you can't install a server over an already installed client, 
thus the original check is correct.


Ahh domain level 0, right, but this check can be added before the client 
check.

With domain level 1, this check should stay there IMO.

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 0065] ipa-replica-install prints incorrect error message when replica is already installed

2015-12-08 Thread Jan Cholasta

On 8.12.2015 16:09, Martin Basti wrote:



On 01.12.2015 14:57, Gabe Alford wrote:

Sorry guys, I forgot to add a meaningful subject to this message.
Ignore the previous thread start.

-- Forwarded message --
From: *Gabe Alford* >
Date: Mon, Nov 30, 2015 at 7:31 PM
Subject: [PATCH 0065]
To: freeipa-devel >


Hello,

Patch fix for the following tickets:

https://fedorahosted.org/freeipa/ticket/5022
https://fedorahosted.org/freeipa/ticket/5320

Thanks,

Gabe




ACK


NACK, you can't install a server over an already installed client, thus 
the original check is correct.


--
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 0065] ipa-replica-install prints incorrect error message when replica is already installed

2015-12-08 Thread Gabe Alford
Just to confirm:

if server is installed:
 Let's stop here and not do anything else

if domain level 0:
 check if client installed and stop here

Right?


On Tue, Dec 8, 2015 at 8:20 AM, Jan Cholasta  wrote:

> On 8.12.2015 16:17, Martin Basti wrote:
>
>>
>>
>> On 08.12.2015 16:14, Jan Cholasta wrote:
>>
>>> On 8.12.2015 16:09, Martin Basti wrote:
>>>


 On 01.12.2015 14:57, Gabe Alford wrote:

> Sorry guys, I forgot to add a meaningful subject to this message.
> Ignore the previous thread start.
>
> -- Forwarded message --
> From: *Gabe Alford*  >
> Date: Mon, Nov 30, 2015 at 7:31 PM
> Subject: [PATCH 0065]
> To: freeipa-devel  >
>
>
> Hello,
>
> Patch fix for the following tickets:
>
> https://fedorahosted.org/freeipa/ticket/5022
> https://fedorahosted.org/freeipa/ticket/5320
>
> Thanks,
>
> Gabe
>
>
>
> ACK

>>>
>>> NACK, you can't install a server over an already installed client,
>>> thus the original check is correct.
>>>
>>> Ahh domain level 0, right, but this check can be added before the client
>> check.
>>
>
> Yes.
>
> With domain level 1, this check should stay there IMO.
>>
>
> Yes. It should say "IPA server is already configured" rather than "IPA
> replica is already configured", though.
>
> --
> 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 0065] ipa-replica-install prints incorrect error message when replica is already installed

2015-12-08 Thread Jan Cholasta

On 8.12.2015 16:17, Martin Basti wrote:



On 08.12.2015 16:14, Jan Cholasta wrote:

On 8.12.2015 16:09, Martin Basti wrote:



On 01.12.2015 14:57, Gabe Alford wrote:

Sorry guys, I forgot to add a meaningful subject to this message.
Ignore the previous thread start.

-- Forwarded message --
From: *Gabe Alford* >
Date: Mon, Nov 30, 2015 at 7:31 PM
Subject: [PATCH 0065]
To: freeipa-devel >


Hello,

Patch fix for the following tickets:

https://fedorahosted.org/freeipa/ticket/5022
https://fedorahosted.org/freeipa/ticket/5320

Thanks,

Gabe




ACK


NACK, you can't install a server over an already installed client,
thus the original check is correct.


Ahh domain level 0, right, but this check can be added before the client
check.


Yes.


With domain level 1, this check should stay there IMO.


Yes. It should say "IPA server is already configured" rather than "IPA 
replica is already configured", though.


--
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 0065] ipa-replica-install prints incorrect error message when replica is already installed

2015-12-08 Thread Martin Basti



On 08.12.2015 16:26, Gabe Alford wrote:

Just to confirm:

if server is installed:
 Let's stop here and not do anything else

if domain level 0:
 check if client installed and stop here

Right?

yes




On Tue, Dec 8, 2015 at 8:20 AM, Jan Cholasta > wrote:


On 8.12.2015 16:17, Martin Basti wrote:



On 08.12.2015 16:14, Jan Cholasta wrote:

On 8.12.2015 16:09, Martin Basti wrote:



On 01.12.2015 14:57, Gabe Alford wrote:

Sorry guys, I forgot to add a meaningful subject
to this message.
Ignore the previous thread start.

-- Forwarded message --
From: *Gabe Alford* 
>>
Date: Mon, Nov 30, 2015 at 7:31 PM
Subject: [PATCH 0065]
To: freeipa-devel 
>>


Hello,

Patch fix for the following tickets:

https://fedorahosted.org/freeipa/ticket/5022
https://fedorahosted.org/freeipa/ticket/5320

Thanks,

Gabe



ACK


NACK, you can't install a server over an already installed
client,
thus the original check is correct.

Ahh domain level 0, right, but this check can be added before
the client
check.


Yes.

With domain level 1, this check should stay there IMO.


Yes. It should say "IPA server is already configured" rather than
"IPA replica is already configured", though.

-- 
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 0065] ipa-replica-install prints incorrect error message when replica is already installed

2015-12-08 Thread Martin Basti



On 01.12.2015 14:57, Gabe Alford wrote:
Sorry guys, I forgot to add a meaningful subject to this message. 
Ignore the previous thread start.


-- Forwarded message --
From: *Gabe Alford* >
Date: Mon, Nov 30, 2015 at 7:31 PM
Subject: [PATCH 0065]
To: freeipa-devel >



Hello,

Patch fix for the following tickets:

https://fedorahosted.org/freeipa/ticket/5022
https://fedorahosted.org/freeipa/ticket/5320

Thanks,

Gabe




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

Re: [Freeipa-devel] [PATCH 0065] ipa-replica-install prints incorrect error message when replica is already installed

2015-12-08 Thread Gabe Alford
Updated patch attached.

On Tue, Dec 8, 2015 at 8:27 AM, Martin Basti  wrote:

>
>
> On 08.12.2015 16:26, Gabe Alford wrote:
>
> Just to confirm:
>
> if server is installed:
>  Let's stop here and not do anything else
>
> if domain level 0:
>  check if client installed and stop here
>
> Right?
>
> yes
>
>
>
>
> On Tue, Dec 8, 2015 at 8:20 AM, Jan Cholasta  wrote:
>
>> On 8.12.2015 16:17, Martin Basti wrote:
>>
>>>
>>>
>>> On 08.12.2015 16:14, Jan Cholasta wrote:
>>>
 On 8.12.2015 16:09, Martin Basti wrote:

>
>
> On 01.12.2015 14:57, Gabe Alford wrote:
>
>> Sorry guys, I forgot to add a meaningful subject to this message.
>> Ignore the previous thread start.
>>
>> -- Forwarded message --
>> From: *Gabe Alford* > >
>> Date: Mon, Nov 30, 2015 at 7:31 PM
>> Subject: [PATCH 0065]
>> To: freeipa-devel > >
>>
>>
>> Hello,
>>
>> Patch fix for the following tickets:
>>
>> https://fedorahosted.org/freeipa/ticket/5022
>> https://fedorahosted.org/freeipa/ticket/5320
>>
>> Thanks,
>>
>> Gabe
>>
>>
>>
>> ACK
>

 NACK, you can't install a server over an already installed client,
 thus the original check is correct.

 Ahh domain level 0, right, but this check can be added before the client
>>> check.
>>>
>>
>> Yes.
>>
>> With domain level 1, this check should stay there IMO.
>>>
>>
>> Yes. It should say "IPA server is already configured" rather than "IPA
>> replica is already configured", though.
>>
>> --
>> Jan Cholasta
>>
>
>
>
From 340a1316d8a71a4a3d7246fa87d2307f34484776 Mon Sep 17 00:00:00 2001
From: Gabe 
Date: Tue, 8 Dec 2015 08:58:56 -0700
Subject: [PATCH] ipa-replica-install prints incorrect error message when
 replica is already installed

https://fedorahosted.org/freeipa/ticket/5022
https://fedorahosted.org/freeipa/ticket/5320
---
 ipaserver/install/server/replicainstall.py | 15 ---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/ipaserver/install/server/replicainstall.py b/ipaserver/install/server/replicainstall.py
index 4554166752ce4e5db2a98a8f495aa061aec963e9..e3f061a171e48f060464ef8e32630c8ca394c0b8 100644
--- a/ipaserver/install/server/replicainstall.py
+++ b/ipaserver/install/server/replicainstall.py
@@ -31,9 +31,8 @@ from ipaserver.install import (
 bindinstance, ca, cainstance, certs, dns, dsinstance, httpinstance,
 installutils, kra, krainstance, krbinstance, memcacheinstance,
 ntpinstance, otpdinstance, custodiainstance, service)
-from ipaserver.install.installutils import create_replica_config
-from ipaserver.install.installutils import ReplicaConfig
-from ipaserver.install.installutils import load_pkcs12
+from ipaserver.install.installutils import (
+create_replica_config, ReplicaConfig, load_pkcs12, is_ipa_configured)
 from ipaserver.install.replication import (
 ReplicationManager, replica_conn_check)
 import SSSDConfig
@@ -423,6 +422,11 @@ def install_check(installer):
 
 tasks.check_selinux_status()
 
+if is_ipa_configured():
+sys.exit("IPA server is already configured on this system.\n"
+ "If you want to reinstall the IPA server, please uninstall "
+ "it first using 'ipa-server-install --uninstall'.")
+
 client_fstore = sysrestore.FileStore(paths.IPA_CLIENT_SYSRESTORE)
 if client_fstore.has_files():
 sys.exit("IPA client is already configured on this system.\n"
@@ -828,6 +832,11 @@ def promote_check(installer):
 
 tasks.check_selinux_status()
 
+if is_ipa_configured():
+sys.exit("IPA server is already configured on this system.\n"
+ "If you want to reinstall the IPA server, please uninstall "
+ "it first using 'ipa-server-install --uninstall'."
+
 client_fstore = sysrestore.FileStore(paths.IPA_CLIENT_SYSRESTORE)
 if not client_fstore.has_files():
 ensure_enrolled(installer)
-- 
1.8.3.1

-- 
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 0065] ipa-replica-install prints incorrect error message when replica is already installed

2015-12-08 Thread Jan Cholasta

LGTM

On 8.12.2015 17:04, Gabe Alford wrote:

Updated patch attached.

On Tue, Dec 8, 2015 at 8:27 AM, Martin Basti > wrote:



On 08.12.2015 16:26, Gabe Alford wrote:

Just to confirm:

if server is installed:
 Let's stop here and not do anything else

if domain level 0:
 check if client installed and stop here

Right?

yes





On Tue, Dec 8, 2015 at 8:20 AM, Jan Cholasta > wrote:

On 8.12.2015 16:17, Martin Basti wrote:



On 08.12.2015 16:14, Jan Cholasta wrote:

On 8.12.2015 16:09, Martin Basti wrote:



On 01.12.2015 14:57, Gabe Alford wrote:

Sorry guys, I forgot to add a meaningful
subject to this message.
Ignore the previous thread start.

-- Forwarded message --
From: *Gabe Alford* 
>>
Date: Mon, Nov 30, 2015 at 7:31 PM
Subject: [PATCH 0065]
To: freeipa-devel 
>>


Hello,

Patch fix for the following tickets:

https://fedorahosted.org/freeipa/ticket/5022
https://fedorahosted.org/freeipa/ticket/5320

Thanks,

Gabe



ACK


NACK, you can't install a server over an already
installed client,
thus the original check is correct.

Ahh domain level 0, right, but this check can be added
before the client
check.


Yes.

With domain level 1, this check should stay there IMO.


Yes. It should say "IPA server is already configured" rather
than "IPA replica is already configured", though.

--
Jan Cholasta








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

2015-12-07 Thread Gabe Alford
Bump for review.

On Mon, Nov 30, 2015 at 7:31 PM, Gabe Alford  wrote:

> Hello,
>
> Patch fix for the following tickets:
>
> https://fedorahosted.org/freeipa/ticket/5022
> https://fedorahosted.org/freeipa/ticket/5320
>
> Thanks,
>
> Gabe
>
-- 
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 0065]

2015-12-07 Thread Gabe Alford
Yup you are right. I meant to bump the other one.

> It is on my TODO list.
Awesome.

On Mon, Dec 7, 2015 at 7:20 AM, Martin Basti  wrote:

>
>
> On 07.12.2015 14:55, Gabe Alford wrote:
>
> Bump for review.
>
> On Mon, Nov 30, 2015 at 7:31 PM, Gabe Alford 
> wrote:
>
>> Hello,
>>
>> Patch fix for the following tickets:
>>
>> https://fedorahosted.org/freeipa/ticket/5022
>> https://fedorahosted.org/freeipa/ticket/5320
>>
>> Thanks,
>>
>> Gabe
>>
>
>
>
> Hello, IIRC you said that we should ignore this in thread
> [PATCH 0065] ipa-replica-install prints incorrect error message when
> replica is already installed
>
> It is on my TODO list.
>
> Martin^2
>
-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH 0065] CI test for IPA install/backup/uninstall/install/restore scenario

2015-09-23 Thread Martin Babinsky

On 09/23/2015 12:53 PM, Martin Babinsky wrote:

CI test for full IPA restore into a running IPA server

self-NACK

--
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 0065] vault: Limit size of data stored in vault

2015-08-26 Thread Petr Vobornik

On 08/26/2015 03:56 PM, David Kupka wrote:

On 26/08/15 15:45, Petr Vobornik wrote:

On 08/26/2015 02:13 PM, David Kupka wrote:

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




Attaching updated patch. With changes discussed offline.


Changes works for me, ACK.


(with the changes it is also ACK from me)

Pushed to:
master: 02ab34c60b5e624ef0653a473316633a5618b07c
ipa-4-2: 9fc82bc66992eaa5daeed80e366e10986a8583d8





Not related to the patch:
This patch limits the size to 1MB instead of proposed 10MB. Testing
showed that even 10MB raises a MemoryError in archive_encrypted_data
which is AFAIK a KraClient method - Endi, this sounds as something which
should be also handled in PKI.

Especially when it happens the subsequent vault-archive command ends
with HTTPError: 503 Server Error: Service Unavailable. After restart of
pki, subsequent vault-archive with 1M file took about 4mins (in
vault_retrieve_internal). Next archive command with 1M file took only
18s.

10k file took 9s.

Why is it so slow?






--
Petr Vobornik

--
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 0065] vault: Limit size of data stored in vault

2015-08-26 Thread Petr Vobornik

On 08/26/2015 02:13 PM, David Kupka wrote:

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




Attaching updated patch. With changes discussed offline.

Not related to the patch:
This patch limits the size to 1MB instead of proposed 10MB. Testing 
showed that even 10MB raises a MemoryError in archive_encrypted_data 
which is AFAIK a KraClient method - Endi, this sounds as something which 
should be also handled in PKI.


Especially when it happens the subsequent vault-archive command ends 
with HTTPError: 503 Server Error: Service Unavailable. After restart of 
pki, subsequent vault-archive with 1M file took about 4mins (in 
vault_retrieve_internal). Next archive command with 1M file took only 18s.


10k file took 9s.

Why is it so slow?
--
Petr Vobornik
From c08848ad37010fa72e774305837db49a078ef5ea Mon Sep 17 00:00:00 2001
From: David Kupka dku...@redhat.com
Date: Wed, 26 Aug 2015 14:11:21 +0200
Subject: [PATCH] vault: Limit size of data stored in vault

https://fedorahosted.org/freeipa/ticket/5231
---
 ipalib/plugins/vault.py | 22 +-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/ipalib/plugins/vault.py b/ipalib/plugins/vault.py
index da1a58cfb77932e8a907725eb88f9f5c6df023c9..3f23c57be830fe85369bfc19e0b93581ded4115a 100644
--- a/ipalib/plugins/vault.py
+++ b/ipalib/plugins/vault.py
@@ -236,6 +236,8 @@ def validated_read(argname, filename, mode='r', encoding=None):
 
 register = Registry()
 
+MAX_VAULT_DATA_SIZE = 2**20  # = 1 MB
+
 vaultcontainer_options = (
 Str(
 'service?',
@@ -1242,10 +1244,28 @@ class vault_archive(PKQuery, Local):
 raise errors.MutuallyExclusiveError(
 reason=_('Input data specified multiple times'))
 
+elif data:
+if len(data)  MAX_VAULT_DATA_SIZE:
+raise errors.ValidationError(name=data, error=_(
+Size of data exceeds the limit. Current vault data size 
+limit is %(limit)d B)
+% {'limit': MAX_VAULT_DATA_SIZE})
+
 elif input_file:
+try:
+stat = os.stat(input_file)
+except OSError as exc:
+raise errors.ValidationError(name=in, error=_(
+Cannot read file '%(filename)s': %(exc)s)
+% {'filename': input_file, 'exc': exc[1]})
+if stat.st_size  MAX_VAULT_DATA_SIZE:
+raise errors.ValidationError(name=in, error=_(
+Size of data exceeds the limit. Current vault data size 
+limit is %(limit)d B)
+% {'limit': MAX_VAULT_DATA_SIZE})
 data = validated_read('in', input_file, mode='rb')
 
-elif not data:
+else:
 data = ''
 
 if self.api.env.in_server:
-- 
2.4.3

-- 
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 0065] vault: Limit size of data stored in vault

2015-08-26 Thread David Kupka

On 26/08/15 15:45, Petr Vobornik wrote:

On 08/26/2015 02:13 PM, David Kupka wrote:

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




Attaching updated patch. With changes discussed offline.


Changes works for me, ACK.



Not related to the patch:
This patch limits the size to 1MB instead of proposed 10MB. Testing
showed that even 10MB raises a MemoryError in archive_encrypted_data
which is AFAIK a KraClient method - Endi, this sounds as something which
should be also handled in PKI.

Especially when it happens the subsequent vault-archive command ends
with HTTPError: 503 Server Error: Service Unavailable. After restart of
pki, subsequent vault-archive with 1M file took about 4mins (in
vault_retrieve_internal). Next archive command with 1M file took only
18s.

10k file took 9s.

Why is it so slow?



--
David Kupka

--
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 0065] Don't allow users to create tokens with a specified ID

2014-09-22 Thread Martin Kosek
On 09/20/2014 10:22 PM, Nathaniel McCallum wrote:
 On Wed, 2014-09-17 at 12:31 +0200, Martin Kosek wrote:
 On 09/17/2014 08:51 AM, Jan Cholasta wrote:
 Hi,

 Dne 16.9.2014 v 19:32 Nathaniel McCallum napsal(a):
 We perform this enforcement at the API level since:
 * DS level enforcement would be difficult
 * ipatokenUniqueID generation already happens at the API level

 It may be nice in the future to perform enforcement in the DS itself.
 However, the question of the location of enforcement is largely an
 aesthetic issue.

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

 That's a rather beefy check. I would prefer something like this (untested):

 group_dn = self.api.Object.group.get_dn(u'admins')
 filter = ldap.make_filter(
 {'krbprincipalname': context.principal, 'memberof': group_dn},
 ldap.MATCH_ALL)
 try:
 ldap.find_entries(
 base_dn=self.api.env.basedn, filter=filter, attrs_list=[''])
 except errors.NotFound:
 raise ValidationError(name='ipatokenuniqueid',
   error='can only be specified by admins')

 Honza


 Also, do we want to hard code it to admins group only?
 
 Preferably, no. But I don't have another workable solution.
 
 Wouldn't it be more
 flexible to create a new Virtual Operation and let realm admin configure who
 can change the UID. See Jan's patch d6fb110b77e2c585f0bfc5eb11b0187a43263fa1
 for an example how that's done.
 
 Modifications are already not permitted. The problem is that we need to
 restrict the format of an attribute for only some users on add only.
 
 Nathaniel
 

Hmm, however note that we have a mechanism to limit even the values of the
added object. See this example with groups.

This is the ACI:
# ipa permission-show System: Add Groups --all --raw | grep aci
  aci: (targetfilter =
(|(objectclass=ipausergroup)(objectclass=posixgroup)))(version 3.0;acl
permission:System: Add Groups;allow (add) groupdn = ldap:///cn=System: Add
Groups,cn=permissions,cn=pbac,dc=mkosek-fedora20,dc=test;)

Now I add custom user with this ACI:
# ipa role-add test --desc test
# ipa role-add-privilege test --privileges 'Group Administrators'
# ipa role-add-member test --users fbar

Now I try to add LDAP object that does not fit the ACI:

# kinit fbar
# ldapadd -h `hostname` -Y GSSAPI
SASL/GSSAPI authentication started
SASL username: f...@mkosek-fedora20.test
SASL SSF: 56
SASL data security layer installed.
dn: cn=test,cn=groups,cn=accounts,dc=mkosek-fedora20,dc=test
objectclass: nscontainer
objectclass: top
cn: test

adding new entry cn=test,cn=groups,cn=accounts,dc=mkosek-fedora20,dc=test
ldap_add: Insufficient access (50)
additional info: Insufficient 'add' privilege to add the entry
'cn=test,cn=groups,cn=accounts,dc=mkosek-fedora20,dc=test'.


Now the right one:
# ldapadd -h `hostname` -Y GSSAPI
SASL/GSSAPI authentication started
SASL username: f...@mkosek-fedora20.test
SASL SSF: 56
SASL data security layer installed.
dn: cn=test,cn=groups,cn=accounts,dc=mkosek-fedora20,dc=test
objectclass: nscontainer
objectclass: top
objectclass: ipausergroup
cn: test

adding new entry cn=test,cn=groups,cn=accounts,dc=mkosek-fedora20,dc=test


Could we use that mechanism and only allow setting ipatokenUniqueID to -1 so
that it is always generated for normal users? The attribute is single valued so
user should not be able to circumvent it with multiple values.

Martin

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH 0065] Don't allow users to create tokens with a specified ID

2014-09-22 Thread Martin Kosek
On 09/20/2014 10:22 PM, Nathaniel McCallum wrote:
 On Wed, 2014-09-17 at 12:31 +0200, Martin Kosek wrote:
 On 09/17/2014 08:51 AM, Jan Cholasta wrote:
 Hi,

 Dne 16.9.2014 v 19:32 Nathaniel McCallum napsal(a):
 We perform this enforcement at the API level since:
 * DS level enforcement would be difficult
 * ipatokenUniqueID generation already happens at the API level

 It may be nice in the future to perform enforcement in the DS itself.
 However, the question of the location of enforcement is largely an
 aesthetic issue.

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

 That's a rather beefy check. I would prefer something like this (untested):

 group_dn = self.api.Object.group.get_dn(u'admins')
 filter = ldap.make_filter(
 {'krbprincipalname': context.principal, 'memberof': group_dn},
 ldap.MATCH_ALL)
 try:
 ldap.find_entries(
 base_dn=self.api.env.basedn, filter=filter, attrs_list=[''])
 except errors.NotFound:
 raise ValidationError(name='ipatokenuniqueid',
   error='can only be specified by admins')

 Honza


 Also, do we want to hard code it to admins group only?
 
 Preferably, no. But I don't have another workable solution.
 
 Wouldn't it be more
 flexible to create a new Virtual Operation and let realm admin configure who
 can change the UID. See Jan's patch d6fb110b77e2c585f0bfc5eb11b0187a43263fa1
 for an example how that's done.
 
 Modifications are already not permitted. The problem is that we need to
 restrict the format of an attribute for only some users on add only.
 
 Nathaniel

Ah, that's a valid point. We need some way to get the modified DN after UUID
generation.

I discussed this issue briefly with Nathaniel and I had an idea. Doesn't DS
have a control for modification to return an updated object after ldapmodify?
Then we could trigger it, get the DN and work with that DN in the token plugin.
We do not need to make it super general in the framework right now, but we
could do at least some minimal version for the token use case.

Martin

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH 0065] Don't allow users to create tokens with a specified ID

2014-09-22 Thread thierry bordaz

On 09/22/2014 05:37 PM, Martin Kosek wrote:

On 09/20/2014 10:22 PM, Nathaniel McCallum wrote:

On Wed, 2014-09-17 at 12:31 +0200, Martin Kosek wrote:

On 09/17/2014 08:51 AM, Jan Cholasta wrote:

Hi,

Dne 16.9.2014 v 19:32 Nathaniel McCallum napsal(a):

We perform this enforcement at the API level since:
* DS level enforcement would be difficult
* ipatokenUniqueID generation already happens at the API level

It may be nice in the future to perform enforcement in the DS itself.
However, the question of the location of enforcement is largely an
aesthetic issue.

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

That's a rather beefy check. I would prefer something like this (untested):

 group_dn = self.api.Object.group.get_dn(u'admins')
 filter = ldap.make_filter(
 {'krbprincipalname': context.principal, 'memberof': group_dn},
 ldap.MATCH_ALL)
 try:
 ldap.find_entries(
 base_dn=self.api.env.basedn, filter=filter, attrs_list=[''])
 except errors.NotFound:
 raise ValidationError(name='ipatokenuniqueid',
   error='can only be specified by admins')

Honza


Also, do we want to hard code it to admins group only?

Preferably, no. But I don't have another workable solution.


Wouldn't it be more
flexible to create a new Virtual Operation and let realm admin configure who
can change the UID. See Jan's patch d6fb110b77e2c585f0bfc5eb11b0187a43263fa1
for an example how that's done.

Modifications are already not permitted. The problem is that we need to
restrict the format of an attribute for only some users on add only.

Nathaniel

Ah, that's a valid point. We need some way to get the modified DN after UUID
generation.

I discussed this issue briefly with Nathaniel and I had an idea. Doesn't DS
have a control for modification to return an updated object after ldapmodify?

Do you mean RFC 4527 (read entry control). Ability to read our writes.

thierry

Then we could trigger it, get the DN and work with that DN in the token plugin.
We do not need to make it super general in the framework right now, but we
could do at least some minimal version for the token use case.

Martin


___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH 0065] Don't allow users to create tokens with a specified ID

2014-09-22 Thread Simo Sorce
On Mon, 22 Sep 2014 17:42:39 +0200
thierry bordaz tbor...@redhat.com wrote:

 RFC 4527

Thanks a lot Thierry, this is exactly the control I had in mind last
week. If we could implement it then we could solve any issue where the
RDN needs to be modified by the ADD operation.

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH 0065] Don't allow users to create tokens with a specified ID

2014-09-22 Thread Simo Sorce
On Mon, 22 Sep 2014 12:58:58 -0400
Simo Sorce s...@redhat.com wrote:

 On Mon, 22 Sep 2014 17:42:39 +0200
 thierry bordaz tbor...@redhat.com wrote:
 
  RFC 4527
 
 Thanks a lot Thierry, this is exactly the control I had in mind last
 week. If we could implement it then we could solve any issue where the
 RDN needs to be modified by the ADD operation.

Ha, Rich confirmed on IRC that we do have this control available since
DS 1.3.2

Given this I think this is the best solution as it allows us to use a
simple ACI and no code to solve the problem.

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH 0065] Don't allow users to create tokens with a specified ID

2014-09-22 Thread Martin Kosek

On 09/22/2014 06:58 PM, Simo Sorce wrote:

On Mon, 22 Sep 2014 17:42:39 +0200
thierry bordaz tbor...@redhat.com wrote:


RFC 4527


Thanks a lot Thierry, this is exactly the control I had in mind last
week. If we could implement it then we could solve any issue where the
RDN needs to be modified by the ADD operation.

Simo.



Ah, so do I understand it correctly that we do not have that control in the DS 
implemented yet? If this is the case, we should file a DS ticket and do some 
simple temporary solution in FreeIPA for now.


I would personally did not want to go with the custom DS plugin or other 
complicated route is it takes development time and may be difficult to get rid 
of it later.


Martin

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH 0065] Don't allow users to create tokens with a specified ID

2014-09-22 Thread Rich Megginson

On 09/22/2014 01:28 PM, Martin Kosek wrote:

On 09/22/2014 06:58 PM, Simo Sorce wrote:

On Mon, 22 Sep 2014 17:42:39 +0200
thierry bordaz tbor...@redhat.com wrote:


RFC 4527


Thanks a lot Thierry, this is exactly the control I had in mind last
week. If we could implement it then we could solve any issue where the
RDN needs to be modified by the ADD operation.

Simo.



Ah, so do I understand it correctly that we do not have that control 
in the DS implemented yet?


It was implemented in 1.3.2, which means Fedora 20 and later.

If this is the case, we should file a DS ticket and do some simple 
temporary solution in FreeIPA for now.


I would personally did not want to go with the custom DS plugin or 
other complicated route is it takes development time and may be 
difficult to get rid of it later.


Martin

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH 0065] Don't allow users to create tokens with a specified ID

2014-09-21 Thread Nathaniel McCallum
On Sat, 2014-09-20 at 16:21 -0400, Nathaniel McCallum wrote:
 On Wed, 2014-09-17 at 08:51 +0200, Jan Cholasta wrote:
  Hi,
  
  Dne 16.9.2014 v 19:32 Nathaniel McCallum napsal(a):
   We perform this enforcement at the API level since:
   * DS level enforcement would be difficult
   * ipatokenUniqueID generation already happens at the API level
  
   It may be nice in the future to perform enforcement in the DS itself.
   However, the question of the location of enforcement is largely an
   aesthetic issue.
  
   https://fedorahosted.org/freeipa/ticket/4456
  
  That's a rather beefy check. I would prefer something like this (untested):
  
   group_dn = self.api.Object.group.get_dn(u'admins')
   filter = ldap.make_filter(
   {'krbprincipalname': context.principal, 'memberof': group_dn},
   ldap.MATCH_ALL)
   try:
   ldap.find_entries(
   base_dn=self.api.env.basedn, filter=filter, attrs_list=[''])
   except errors.NotFound:
   raise ValidationError(name='ipatokenuniqueid',
 error='can only be specified by admins')
 
 Fixed.

Please note that another approach has been posted here:

https://www.redhat.com/archives/freeipa-devel/2014-September/msg00433.html

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH 0065] Don't allow users to create tokens with a specified ID

2014-09-20 Thread Nathaniel McCallum
On Wed, 2014-09-17 at 08:51 +0200, Jan Cholasta wrote:
 Hi,
 
 Dne 16.9.2014 v 19:32 Nathaniel McCallum napsal(a):
  We perform this enforcement at the API level since:
  * DS level enforcement would be difficult
  * ipatokenUniqueID generation already happens at the API level
 
  It may be nice in the future to perform enforcement in the DS itself.
  However, the question of the location of enforcement is largely an
  aesthetic issue.
 
  https://fedorahosted.org/freeipa/ticket/4456
 
 That's a rather beefy check. I would prefer something like this (untested):
 
  group_dn = self.api.Object.group.get_dn(u'admins')
  filter = ldap.make_filter(
  {'krbprincipalname': context.principal, 'memberof': group_dn},
  ldap.MATCH_ALL)
  try:
  ldap.find_entries(
  base_dn=self.api.env.basedn, filter=filter, attrs_list=[''])
  except errors.NotFound:
  raise ValidationError(name='ipatokenuniqueid',
error='can only be specified by admins')

Fixed.
From ffbddd628a123e3cdbef18de2c4b7d37fc7cd804 Mon Sep 17 00:00:00 2001
From: Nathaniel McCallum npmccal...@redhat.com
Date: Tue, 16 Sep 2014 13:21:05 -0400
Subject: [PATCH] Don't allow users to create tokens with a specified ID

We perform this enforcement at the API level since:
* DS level enforcement would be difficult
* ipatokenUniqueID generation already happens at the API level

It may be nice in the future to perform enforcement in the DS itself.
However, the question of the location of enforcement is largely an
aesthetic issue.

https://fedorahosted.org/freeipa/ticket/4456
---
 ipalib/plugins/otptoken.py | 14 +-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/ipalib/plugins/otptoken.py b/ipalib/plugins/otptoken.py
index 1bd85d4b952dc51ea800ed37c49b3c50aeb31492..f7780648f2396e0b5f042e4edd1f4eb133b1726b 100644
--- a/ipalib/plugins/otptoken.py
+++ b/ipalib/plugins/otptoken.py
@@ -21,7 +21,7 @@ from ipalib.plugins.baseldap import DN, LDAPObject, LDAPAddMember, LDAPRemoveMem
 from ipalib.plugins.baseldap import LDAPCreate, LDAPDelete, LDAPUpdate, LDAPSearch, LDAPRetrieve
 from ipalib import api, Int, Str, Bool, DateTime, Flag, Bytes, IntEnum, StrEnum, Password, _, ngettext
 from ipalib.plugable import Registry
-from ipalib.errors import PasswordMismatch, ConversionError, LastMemberError, NotFound, ValidationError
+from ipalib.errors import PasswordMismatch, ConversionError, NotFound, ValidationError
 from ipalib.request import context
 from ipalib.frontend import Local
 
@@ -259,6 +259,18 @@ class otptoken_add(LDAPCreate):
 entry_attrs['ipatokenuniqueid'] = str(uuid.uuid4())
 dn = DN(ipatokenuniqueid=%s % entry_attrs['ipatokenuniqueid'], dn)
 
+# Ensure that no token id is specified if the user is not an admin.
+else:
+try:
+filt = {}
+filt['krbPrincipalName'] = str(context.principal)
+filt['memberOf'] = self.api.Object.group.get_dn(u'admins')
+ldap.find_entries(ldap.make_filter(filt, rules=ldap.MATCH_ALL),
+  [''], self.api.env.container_user)
+except NotFound:
+raise ValidationError(name='ipatokenuniqueid',
+  error='can only be specified by admins')
+
 if not _check_interval(options.get('ipatokennotbefore', None),
options.get('ipatokennotafter', None)):
 raise ValidationError(name='not_after',
-- 
2.1.0

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH 0065] Don't allow users to create tokens with a specified ID

2014-09-20 Thread Nathaniel McCallum
On Wed, 2014-09-17 at 12:31 +0200, Martin Kosek wrote:
 On 09/17/2014 08:51 AM, Jan Cholasta wrote:
  Hi,
  
  Dne 16.9.2014 v 19:32 Nathaniel McCallum napsal(a):
  We perform this enforcement at the API level since:
  * DS level enforcement would be difficult
  * ipatokenUniqueID generation already happens at the API level
 
  It may be nice in the future to perform enforcement in the DS itself.
  However, the question of the location of enforcement is largely an
  aesthetic issue.
 
  https://fedorahosted.org/freeipa/ticket/4456
  
  That's a rather beefy check. I would prefer something like this (untested):
  
  group_dn = self.api.Object.group.get_dn(u'admins')
  filter = ldap.make_filter(
  {'krbprincipalname': context.principal, 'memberof': group_dn},
  ldap.MATCH_ALL)
  try:
  ldap.find_entries(
  base_dn=self.api.env.basedn, filter=filter, attrs_list=[''])
  except errors.NotFound:
  raise ValidationError(name='ipatokenuniqueid',
error='can only be specified by admins')
  
  Honza
  
 
 Also, do we want to hard code it to admins group only?

Preferably, no. But I don't have another workable solution.

 Wouldn't it be more
 flexible to create a new Virtual Operation and let realm admin configure who
 can change the UID. See Jan's patch d6fb110b77e2c585f0bfc5eb11b0187a43263fa1
 for an example how that's done.

Modifications are already not permitted. The problem is that we need to
restrict the format of an attribute for only some users on add only.

Nathaniel

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH 0065] Don't allow users to create tokens with a specified ID

2014-09-17 Thread Jan Cholasta

Hi,

Dne 16.9.2014 v 19:32 Nathaniel McCallum napsal(a):

We perform this enforcement at the API level since:
* DS level enforcement would be difficult
* ipatokenUniqueID generation already happens at the API level

It may be nice in the future to perform enforcement in the DS itself.
However, the question of the location of enforcement is largely an
aesthetic issue.

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


That's a rather beefy check. I would prefer something like this (untested):

group_dn = self.api.Object.group.get_dn(u'admins')
filter = ldap.make_filter(
{'krbprincipalname': context.principal, 'memberof': group_dn},
ldap.MATCH_ALL)
try:
ldap.find_entries(
base_dn=self.api.env.basedn, filter=filter, attrs_list=[''])
except errors.NotFound:
raise ValidationError(name='ipatokenuniqueid',
  error='can only be specified by admins')

Honza

--
Jan Cholasta

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH 0065] Don't allow users to create tokens with a specified ID

2014-09-17 Thread Martin Kosek
On 09/17/2014 08:51 AM, Jan Cholasta wrote:
 Hi,
 
 Dne 16.9.2014 v 19:32 Nathaniel McCallum napsal(a):
 We perform this enforcement at the API level since:
 * DS level enforcement would be difficult
 * ipatokenUniqueID generation already happens at the API level

 It may be nice in the future to perform enforcement in the DS itself.
 However, the question of the location of enforcement is largely an
 aesthetic issue.

 https://fedorahosted.org/freeipa/ticket/4456
 
 That's a rather beefy check. I would prefer something like this (untested):
 
 group_dn = self.api.Object.group.get_dn(u'admins')
 filter = ldap.make_filter(
 {'krbprincipalname': context.principal, 'memberof': group_dn},
 ldap.MATCH_ALL)
 try:
 ldap.find_entries(
 base_dn=self.api.env.basedn, filter=filter, attrs_list=[''])
 except errors.NotFound:
 raise ValidationError(name='ipatokenuniqueid',
   error='can only be specified by admins')
 
 Honza
 

Also, do we want to hard code it to admins group only? Wouldn't it be more
flexible to create a new Virtual Operation and let realm admin configure who
can change the UID. See Jan's patch d6fb110b77e2c585f0bfc5eb11b0187a43263fa1
for an example how that's done.

Martin

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH] 0065 Follow tmpfiles.d packaging guidelines

2013-09-16 Thread Petr Viktorin

On 09/04/2013 06:27 PM, Ana Krivokapic wrote:

Hello,

This patch addresses ticket https://fedorahosted.org/freeipa/ticket/3881.



Thank you! ACK, pushed to:
master: 7c22b852c73b94148043dd35636e2dd21a80d531
ipa-3-3: 771511fd2597c907fc5293ce1289070551240a91



--
PetrĀ³

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH 0065] Use private ccache in ipa-server-install

2013-06-05 Thread Tomas Babej

On 06/04/2013 06:09 PM, Petr Viktorin wrote:

On 06/04/2013 01:29 PM, Tomas Babej wrote:

On 06/03/2013 02:58 PM, Martin Kosek wrote:

On 06/03/2013 02:43 PM, Tomas Babej wrote:

Hi,

this patch fixes the installation problems on master on F19 with krb5
packages

= 1.11.2-6

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

Tomas

1) Leaving cache_desc open:

+(cache_desc, cache_path) = tempfile.mkstemp(prefix='krbcc')
+os.environ['KRB5CCNAME'] = cache_path

Why do we keep the descriptor open and close it at the and of the
installation?
Can we close it right after tempfile.mkstemp? I think we do it this
way in
other places in installation.

2) What about other installers where we handle Kerberos auth, like
ipa-{replica,dns,ca}-install?

A common function, other shared means, of handling KRB5CCNAME may be
appropriate to avoid duplicating code too much.

Martin

I moved the code responsible to PrivateCCache class, both for
readability and conciseness.

Private ccache now used in replica,dns and ca the installers. I managed
to reproduce the error only with
dns-install though(fails on adding the service principal), but having a
private ccache for the installer should not hurt.

Ipa-adtrust-install requires the admin ticket, so there shouldn't be an
issue.

Tomas


Shouldn't the context manager restore os.environ['KRB5CCNAME'] on exit?


There's no need to, since the value of the environment variable is 
inherited only into child processes (pkispawn, etc.). Hence the KRB5CCNAME

environment variable is not available outside the install script.

[root@vm-002 labtool]# ipa-server-install
[snip]
Be sure to back up the CA certificate stored in /root/cacert.p12
This file is required to create replicas. The password for this
file is the Directory Manager password

[root@vm-002 labtool]# echo $KRB5CCNAME

[root@vm-002 labtool]#



Two nitpicks:

ipaserver/install/installutils.py: new blank line at EOF

For most context managers I prefer @contextlib.contextmanager rather 
than writing out the class, it makes them shorter and easier to read 


Addressed in the updated patch.

Tomas
From 1e8fac58c0af6626129ba8934d5d4ed6e29698f2 Mon Sep 17 00:00:00 2001
From: Tomas Babej tba...@redhat.com
Date: Mon, 3 Jun 2013 12:06:06 +0200
Subject: [PATCH] Use private ccache in ipa install tools

All installers that handle Kerberos auth, have been altered to use
private ccache, that is ipa-server-install, ipa-dns-install,
ipa-replica-install, ipa-ca-install.

https://fedorahosted.org/freeipa/ticket/3666
---
 install/tools/ipa-ca-install  | 13 +++--
 install/tools/ipa-dns-install |  5 +++--
 install/tools/ipa-replica-install | 13 +++--
 install/tools/ipa-server-install  |  7 +--
 ipaserver/install/installutils.py | 14 ++
 5 files changed, 36 insertions(+), 16 deletions(-)

diff --git a/install/tools/ipa-ca-install b/install/tools/ipa-ca-install
index 81c11834547c37b01c4749079284affd13bb10d7..fcc8240583402eabb80a6bc701ae05d46adf0f60 100755
--- a/install/tools/ipa-ca-install
+++ b/install/tools/ipa-ca-install
@@ -28,9 +28,9 @@ from ipapython import services as ipaservices
 
 from ipaserver.install import installutils, service
 from ipaserver.install import certs
-from ipaserver.install.installutils import HostnameLocalhost
-from ipaserver.install.installutils import ReplicaConfig, expand_replica_info, read_replica_info
-from ipaserver.install.installutils import get_host_name, BadHostError
+from ipaserver.install.installutils import (HostnameLocalhost, ReplicaConfig,
+expand_replica_info, read_replica_info, get_host_name, BadHostError,
+privateCCache)
 from ipaserver.install import dsinstance, cainstance, bindinstance
 from ipaserver.install.replication import replica_conn_check
 from ipapython import version
@@ -212,9 +212,10 @@ Run /usr/sbin/ipa-server-install --uninstall to clean up.
 
 if __name__ == '__main__':
 try:
-installutils.run_script(main, log_file_name=log_file_name,
-operation_name='ipa-ca-install',
-fail_message=fail_message)
+with privateCCache():
+installutils.run_script(main, log_file_name=log_file_name,
+operation_name='ipa-ca-install',
+fail_message=fail_message)
 finally:
 # always try to remove decrypted replica file
 try:
diff --git a/install/tools/ipa-dns-install b/install/tools/ipa-dns-install
index e12a0465ca2d09a6a8d25157a737f620f3ff4b1a..8321ca1161229bdb1462b4dff380bf7f0d4af3bf 100755
--- a/install/tools/ipa-dns-install
+++ b/install/tools/ipa-dns-install
@@ -258,5 +258,6 @@ def main():
 return 0
 
 if __name__ == '__main__':
-installutils.run_script(main, log_file_name=log_file_name,
-operation_name='ipa-dns-install')
+with privateCCache():
+installutils.run_script(main, log_file_name=log_file_name,
+operation_name='ipa-dns-install')
diff --git a/install/tools/ipa-replica-install 

Re: [Freeipa-devel] [PATCH 0065] Use private ccache in ipa-server-install

2013-06-05 Thread Petr Viktorin

On 06/05/2013 09:20 AM, Tomas Babej wrote:

On 06/04/2013 06:09 PM, Petr Viktorin wrote:

On 06/04/2013 01:29 PM, Tomas Babej wrote:

On 06/03/2013 02:58 PM, Martin Kosek wrote:

On 06/03/2013 02:43 PM, Tomas Babej wrote:

Hi,

this patch fixes the installation problems on master on F19 with krb5
packages

= 1.11.2-6

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

Tomas

1) Leaving cache_desc open:

+(cache_desc, cache_path) = tempfile.mkstemp(prefix='krbcc')
+os.environ['KRB5CCNAME'] = cache_path

Why do we keep the descriptor open and close it at the and of the
installation?
Can we close it right after tempfile.mkstemp? I think we do it this
way in
other places in installation.

2) What about other installers where we handle Kerberos auth, like
ipa-{replica,dns,ca}-install?

A common function, other shared means, of handling KRB5CCNAME may be
appropriate to avoid duplicating code too much.

Martin

I moved the code responsible to PrivateCCache class, both for
readability and conciseness.

Private ccache now used in replica,dns and ca the installers. I managed
to reproduce the error only with
dns-install though(fails on adding the service principal), but having a
private ccache for the installer should not hurt.

Ipa-adtrust-install requires the admin ticket, so there shouldn't be an
issue.

Tomas


Shouldn't the context manager restore os.environ['KRB5CCNAME'] on exit?


There's no need to, since the value of the environment variable is
inherited only into child processes (pkispawn, etc.). Hence the KRB5CCNAME
environment variable is not available outside the install script.


Yes, but what if you call a child process after the context manager exits?
I know that currently there is no code after the context manager exits, 
but that's no reason to surprise people who will want to reuse it later.


Context managers are expected to clean up after themselves. If there's 
no way to do this then you should at least document that this one is 
special. But in this case it shouldn't be that hard to clean up.



[root@vm-002 labtool]# ipa-server-install
[snip]
Be sure to back up the CA certificate stored in /root/cacert.p12
This file is required to create replicas. The password for this
file is the Directory Manager password

[root@vm-002 labtool]# echo $KRB5CCNAME

[root@vm-002 labtool]#



Two nitpicks:

ipaserver/install/installutils.py: new blank line at EOF

For most context managers I prefer @contextlib.contextmanager rather
than writing out the class, it makes them shorter and easier to read


Addressed in the updated patch.

Tomas



--
PetrĀ³

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH 0065] Use private ccache in ipa-server-install

2013-06-05 Thread Tomas Babej

On 06/05/2013 10:07 AM, Petr Viktorin wrote:

On 06/05/2013 09:20 AM, Tomas Babej wrote:

On 06/04/2013 06:09 PM, Petr Viktorin wrote:

On 06/04/2013 01:29 PM, Tomas Babej wrote:

On 06/03/2013 02:58 PM, Martin Kosek wrote:

On 06/03/2013 02:43 PM, Tomas Babej wrote:

Hi,

this patch fixes the installation problems on master on F19 with 
krb5

packages

= 1.11.2-6

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

Tomas

1) Leaving cache_desc open:

+(cache_desc, cache_path) = tempfile.mkstemp(prefix='krbcc')
+os.environ['KRB5CCNAME'] = cache_path

Why do we keep the descriptor open and close it at the and of the
installation?
Can we close it right after tempfile.mkstemp? I think we do it this
way in
other places in installation.

2) What about other installers where we handle Kerberos auth, like
ipa-{replica,dns,ca}-install?

A common function, other shared means, of handling KRB5CCNAME may be
appropriate to avoid duplicating code too much.

Martin

I moved the code responsible to PrivateCCache class, both for
readability and conciseness.

Private ccache now used in replica,dns and ca the installers. I 
managed

to reproduce the error only with
dns-install though(fails on adding the service principal), but 
having a

private ccache for the installer should not hurt.

Ipa-adtrust-install requires the admin ticket, so there shouldn't 
be an

issue.

Tomas


Shouldn't the context manager restore os.environ['KRB5CCNAME'] on exit?


There's no need to, since the value of the environment variable is
inherited only into child processes (pkispawn, etc.). Hence the 
KRB5CCNAME

environment variable is not available outside the install script.


Yes, but what if you call a child process after the context manager 
exits?
I know that currently there is no code after the context manager 
exits, but that's no reason to surprise people who will want to reuse 
it later.


Context managers are expected to clean up after themselves. If there's 
no way to do this then you should at least document that this one is 
special. But in this case it shouldn't be that hard to clean up.



Not at all, I actually had the code there at some point, but removed it.

Updated patch attached.

Tomas

[root@vm-002 labtool]# ipa-server-install
[snip]
Be sure to back up the CA certificate stored in /root/cacert.p12
This file is required to create replicas. The password for this
file is the Directory Manager password

[root@vm-002 labtool]# echo $KRB5CCNAME

[root@vm-002 labtool]#



Two nitpicks:

ipaserver/install/installutils.py: new blank line at EOF

For most context managers I prefer @contextlib.contextmanager rather
than writing out the class, it makes them shorter and easier to read


Addressed in the updated patch.

Tomas





From 24ea3e0f7b717eff0928bf7bbe783328a12d4107 Mon Sep 17 00:00:00 2001
From: Tomas Babej tba...@redhat.com
Date: Mon, 3 Jun 2013 12:06:06 +0200
Subject: [PATCH] Use private ccache in ipa install tools

All installers that handle Kerberos auth, have been altered to use
private ccache, that is ipa-server-install, ipa-dns-install,
ipa-replica-install, ipa-ca-install.

https://fedorahosted.org/freeipa/ticket/3666
---
 install/tools/ipa-ca-install  | 13 +++--
 install/tools/ipa-dns-install |  5 +++--
 install/tools/ipa-replica-install | 13 +++--
 install/tools/ipa-server-install  |  7 +--
 ipaserver/install/installutils.py | 22 ++
 5 files changed, 44 insertions(+), 16 deletions(-)

diff --git a/install/tools/ipa-ca-install b/install/tools/ipa-ca-install
index 81c11834547c37b01c4749079284affd13bb10d7..3b7e9d206e35e68aef7af64172d34a2ee9f25342 100755
--- a/install/tools/ipa-ca-install
+++ b/install/tools/ipa-ca-install
@@ -28,9 +28,9 @@ from ipapython import services as ipaservices
 
 from ipaserver.install import installutils, service
 from ipaserver.install import certs
-from ipaserver.install.installutils import HostnameLocalhost
-from ipaserver.install.installutils import ReplicaConfig, expand_replica_info, read_replica_info
-from ipaserver.install.installutils import get_host_name, BadHostError
+from ipaserver.install.installutils import (HostnameLocalhost, ReplicaConfig,
+expand_replica_info, read_replica_info, get_host_name, BadHostError,
+private_ccache)
 from ipaserver.install import dsinstance, cainstance, bindinstance
 from ipaserver.install.replication import replica_conn_check
 from ipapython import version
@@ -212,9 +212,10 @@ Run /usr/sbin/ipa-server-install --uninstall to clean up.
 
 if __name__ == '__main__':
 try:
-installutils.run_script(main, log_file_name=log_file_name,
-operation_name='ipa-ca-install',
-fail_message=fail_message)
+with private_ccache():
+installutils.run_script(main, log_file_name=log_file_name,
+operation_name='ipa-ca-install',
+fail_message=fail_message)
 finally:
 # always try to remove 

Re: [Freeipa-devel] [PATCH 0065] Use private ccache in ipa-server-install

2013-06-05 Thread Petr Viktorin

On 06/05/2013 10:47 AM, Tomas Babej wrote:

On 06/05/2013 10:07 AM, Petr Viktorin wrote:

On 06/05/2013 09:20 AM, Tomas Babej wrote:

On 06/04/2013 06:09 PM, Petr Viktorin wrote:

On 06/04/2013 01:29 PM, Tomas Babej wrote:

On 06/03/2013 02:58 PM, Martin Kosek wrote:

On 06/03/2013 02:43 PM, Tomas Babej wrote:

Hi,

this patch fixes the installation problems on master on F19 with
krb5
packages

= 1.11.2-6

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

Tomas

1) Leaving cache_desc open:

+(cache_desc, cache_path) = tempfile.mkstemp(prefix='krbcc')
+os.environ['KRB5CCNAME'] = cache_path

Why do we keep the descriptor open and close it at the and of the
installation?
Can we close it right after tempfile.mkstemp? I think we do it this
way in
other places in installation.

2) What about other installers where we handle Kerberos auth, like
ipa-{replica,dns,ca}-install?

A common function, other shared means, of handling KRB5CCNAME may be
appropriate to avoid duplicating code too much.

Martin

I moved the code responsible to PrivateCCache class, both for
readability and conciseness.

Private ccache now used in replica,dns and ca the installers. I
managed
to reproduce the error only with
dns-install though(fails on adding the service principal), but
having a
private ccache for the installer should not hurt.

Ipa-adtrust-install requires the admin ticket, so there shouldn't
be an
issue.

Tomas


Shouldn't the context manager restore os.environ['KRB5CCNAME'] on exit?


There's no need to, since the value of the environment variable is
inherited only into child processes (pkispawn, etc.). Hence the
KRB5CCNAME
environment variable is not available outside the install script.


Yes, but what if you call a child process after the context manager
exits?
I know that currently there is no code after the context manager
exits, but that's no reason to surprise people who will want to reuse
it later.

Context managers are expected to clean up after themselves. If there's
no way to do this then you should at least document that this one is
special. But in this case it shouldn't be that hard to clean up.


Not at all, I actually had the code there at some point, but removed it.

Updated patch attached.

Tomas



Thanks. ACK, pushed to master, ipa-3-2.

master: 6f51f92138ff12eff732bf028751dcfa8ef9b442
ipa-3-2: 4ec1de1a65f1fabe7f5b26b4c4487deec5cea0cf

--
PetrĀ³

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH 0065] Use private ccache in ipa-server-install

2013-06-05 Thread Tomas Babej

On 06/04/2013 01:29 PM, Tomas Babej wrote:

On 06/03/2013 02:58 PM, Martin Kosek wrote:

On 06/03/2013 02:43 PM, Tomas Babej wrote:

Hi,

this patch fixes the installation problems on master on F19 with 
krb5 packages

= 1.11.2-6

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

Tomas

1) Leaving cache_desc open:

+(cache_desc, cache_path) = tempfile.mkstemp(prefix='krbcc')
+os.environ['KRB5CCNAME'] = cache_path

Why do we keep the descriptor open and close it at the and of the 
installation?
Can we close it right after tempfile.mkstemp? I think we do it this 
way in

other places in installation.

2) What about other installers where we handle Kerberos auth, like
ipa-{replica,dns,ca}-install?

A common function, other shared means, of handling KRB5CCNAME may be
appropriate to avoid duplicating code too much.

Martin
I moved the code responsible to PrivateCCache class, both for 
readability and conciseness.


Private ccache now used in replica,dns and ca the installers. I 
managed to reproduce the error only with
dns-install though(fails on adding the service principal), but having 
a private ccache for the installer should not hurt.


Ipa-adtrust-install requires the admin ticket, so there shouldn't be 
an issue.


My reasoning was flawed here, ipa-adtrust-install attempts to re-kinit 
admin ticket, so it needs the private ccache as well.


Sending one-liner fix.

Tomas



Tomas


From 0177d6a7f14b87f42647376001e6ac580ca38e57 Mon Sep 17 00:00:00 2001
From: Tomas Babej tba...@redhat.com
Date: Wed, 5 Jun 2013 13:17:19 +0200
Subject: [PATCH] Use private ccache in ipa-adtrust-install

The ipa-adtrust-install script attempts to automatically re-kinit
admin user ticket, hence it needs private ccache or the usage
of the ipa-adtrust-install with sudo/su will fail.

https://fedorahosted.org/freeipa/ticket/3666
---
 install/tools/ipa-adtrust-install | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/install/tools/ipa-adtrust-install b/install/tools/ipa-adtrust-install
index 5744c6f67aee5f55877d7ef1691e98dfdb8d8718..09831617de7daf03e876897eef1d99d9a1a4a8c6 100755
--- a/install/tools/ipa-adtrust-install
+++ b/install/tools/ipa-adtrust-install
@@ -405,5 +405,6 @@ information
 return 0
 
 if __name__ == '__main__':
-run_script(main, log_file_name=log_file_name,
-operation_name='ipa-adtrust-install')
+with private_ccache():
+run_script(main, log_file_name=log_file_name,
+   operation_name='ipa-adtrust-install')
-- 
1.8.1.4

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH 0065] Use private ccache in ipa-server-install

2013-06-05 Thread Martin Kosek
On 06/05/2013 01:23 PM, Tomas Babej wrote:
 On 06/04/2013 01:29 PM, Tomas Babej wrote:
 On 06/03/2013 02:58 PM, Martin Kosek wrote:
 On 06/03/2013 02:43 PM, Tomas Babej wrote:
 Hi,

 this patch fixes the installation problems on master on F19 with krb5 
 packages
 = 1.11.2-6
 https://fedorahosted.org/freeipa/ticket/3666

 Tomas
 1) Leaving cache_desc open:

 +(cache_desc, cache_path) = tempfile.mkstemp(prefix='krbcc')
 +os.environ['KRB5CCNAME'] = cache_path

 Why do we keep the descriptor open and close it at the and of the 
 installation?
 Can we close it right after tempfile.mkstemp? I think we do it this way in
 other places in installation.

 2) What about other installers where we handle Kerberos auth, like
 ipa-{replica,dns,ca}-install?

 A common function, other shared means, of handling KRB5CCNAME may be
 appropriate to avoid duplicating code too much.

 Martin
 I moved the code responsible to PrivateCCache class, both for readability and
 conciseness.

 Private ccache now used in replica,dns and ca the installers. I managed to
 reproduce the error only with
 dns-install though(fails on adding the service principal), but having a
 private ccache for the installer should not hurt.

 Ipa-adtrust-install requires the admin ticket, so there shouldn't be an 
 issue.
 
 My reasoning was flawed here, ipa-adtrust-install attempts to re-kinit admin
 ticket, so it needs the private ccache as well.
 
 Sending one-liner fix.
 
 Tomas


As also discussed with Alexander on IRC, we do not want to have private ccache
for ipa-adtrust-install as we deliberately re-kinit admin user to add new
MS-PAC information to the ticket so that subsequent trust commands work. In
other install scripts, we want to have private ccache so that we don't mess
with user's default ccache.

This entire problem should go away when krb5 is fixed, see
https://bugzilla.redhat.com/show_bug.cgi?id=961235

Thus, your current fix for private ccaches is correct.

Thanks,
Martin

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH 0065] Use private ccache in ipa-server-install

2013-06-04 Thread Tomas Babej

On 06/03/2013 02:58 PM, Martin Kosek wrote:

On 06/03/2013 02:43 PM, Tomas Babej wrote:

Hi,

this patch fixes the installation problems on master on F19 with krb5 packages

= 1.11.2-6

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

Tomas

1) Leaving cache_desc open:

+(cache_desc, cache_path) = tempfile.mkstemp(prefix='krbcc')
+os.environ['KRB5CCNAME'] = cache_path

Why do we keep the descriptor open and close it at the and of the installation?
Can we close it right after tempfile.mkstemp? I think we do it this way in
other places in installation.

2) What about other installers where we handle Kerberos auth, like
ipa-{replica,dns,ca}-install?

A common function, other shared means, of handling KRB5CCNAME may be
appropriate to avoid duplicating code too much.

Martin
I moved the code responsible to PrivateCCache class, both for 
readability and conciseness.


Private ccache now used in replica,dns and ca the installers. I managed 
to reproduce the error only with
dns-install though(fails on adding the service principal), but having a 
private ccache for the installer should not hurt.


Ipa-adtrust-install requires the admin ticket, so there shouldn't be an 
issue.


Tomas
From 199ade8c7f3eaae15dca3693a92600c635e61d57 Mon Sep 17 00:00:00 2001
From: Tomas Babej tba...@redhat.com
Date: Mon, 3 Jun 2013 12:06:06 +0200
Subject: [PATCH] Use private ccache in ipa install tools

All installers that handle Kerberos auth, have been altered to use
private ccache, that is ipa-server-install, ipa-dns-install,
ipa-replica-install, ipa-ca-install.

https://fedorahosted.org/freeipa/ticket/3666
---
 install/tools/ipa-ca-install  | 13 +++--
 install/tools/ipa-dns-install |  5 +++--
 install/tools/ipa-replica-install | 13 +++--
 install/tools/ipa-server-install  |  7 +--
 ipaserver/install/installutils.py | 16 
 5 files changed, 38 insertions(+), 16 deletions(-)

diff --git a/install/tools/ipa-ca-install b/install/tools/ipa-ca-install
index 81c11834547c37b01c4749079284affd13bb10d7..0f889afac0165f56646778b74b6368fd28b313d8 100755
--- a/install/tools/ipa-ca-install
+++ b/install/tools/ipa-ca-install
@@ -28,9 +28,9 @@ from ipapython import services as ipaservices
 
 from ipaserver.install import installutils, service
 from ipaserver.install import certs
-from ipaserver.install.installutils import HostnameLocalhost
-from ipaserver.install.installutils import ReplicaConfig, expand_replica_info, read_replica_info
-from ipaserver.install.installutils import get_host_name, BadHostError
+from ipaserver.install.installutils import (HostnameLocalhost, ReplicaConfig,
+expand_replica_info, read_replica_info, get_host_name, BadHostError,
+PrivateCCache)
 from ipaserver.install import dsinstance, cainstance, bindinstance
 from ipaserver.install.replication import replica_conn_check
 from ipapython import version
@@ -212,9 +212,10 @@ Run /usr/sbin/ipa-server-install --uninstall to clean up.
 
 if __name__ == '__main__':
 try:
-installutils.run_script(main, log_file_name=log_file_name,
-operation_name='ipa-ca-install',
-fail_message=fail_message)
+with PrivateCCache():
+installutils.run_script(main, log_file_name=log_file_name,
+operation_name='ipa-ca-install',
+fail_message=fail_message)
 finally:
 # always try to remove decrypted replica file
 try:
diff --git a/install/tools/ipa-dns-install b/install/tools/ipa-dns-install
index e12a0465ca2d09a6a8d25157a737f620f3ff4b1a..c8b0aa3b8f2728510b7419975c2d937bf9188ac3 100755
--- a/install/tools/ipa-dns-install
+++ b/install/tools/ipa-dns-install
@@ -258,5 +258,6 @@ def main():
 return 0
 
 if __name__ == '__main__':
-installutils.run_script(main, log_file_name=log_file_name,
-operation_name='ipa-dns-install')
+with PrivateCCache():
+installutils.run_script(main, log_file_name=log_file_name,
+operation_name='ipa-dns-install')
diff --git a/install/tools/ipa-replica-install b/install/tools/ipa-replica-install
index b194b85a201c2d842938d3251fa9179c57d0bd68..2ab67933257b6ec82b39372b20c1fe854d4a92f2 100755
--- a/install/tools/ipa-replica-install
+++ b/install/tools/ipa-replica-install
@@ -36,9 +36,9 @@ from ipaserver.install import dsinstance, installutils, krbinstance, service
 from ipaserver.install import bindinstance, httpinstance, ntpinstance, certs
 from ipaserver.install import memcacheinstance
 from ipaserver.install.replication import replica_conn_check, ReplicationManager
-from ipaserver.install.installutils import HostnameLocalhost, resolve_host
-from ipaserver.install.installutils import ReplicaConfig, expand_replica_info, read_replica_info
-from ipaserver.install.installutils import get_host_name, BadHostError
+from ipaserver.install.installutils import (HostnameLocalhost, resolve_host,
+ReplicaConfig, expand_replica_info, read_replica_info 

Re: [Freeipa-devel] [PATCH 0065] Use private ccache in ipa-server-install

2013-06-04 Thread Petr Viktorin

On 06/04/2013 01:29 PM, Tomas Babej wrote:

On 06/03/2013 02:58 PM, Martin Kosek wrote:

On 06/03/2013 02:43 PM, Tomas Babej wrote:

Hi,

this patch fixes the installation problems on master on F19 with krb5
packages

= 1.11.2-6

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

Tomas

1) Leaving cache_desc open:

+(cache_desc, cache_path) = tempfile.mkstemp(prefix='krbcc')
+os.environ['KRB5CCNAME'] = cache_path

Why do we keep the descriptor open and close it at the and of the
installation?
Can we close it right after tempfile.mkstemp? I think we do it this
way in
other places in installation.

2) What about other installers where we handle Kerberos auth, like
ipa-{replica,dns,ca}-install?

A common function, other shared means, of handling KRB5CCNAME may be
appropriate to avoid duplicating code too much.

Martin

I moved the code responsible to PrivateCCache class, both for
readability and conciseness.

Private ccache now used in replica,dns and ca the installers. I managed
to reproduce the error only with
dns-install though(fails on adding the service principal), but having a
private ccache for the installer should not hurt.

Ipa-adtrust-install requires the admin ticket, so there shouldn't be an
issue.

Tomas


Shouldn't the context manager restore os.environ['KRB5CCNAME'] on exit?


Two nitpicks:

ipaserver/install/installutils.py: new blank line at EOF

For most context managers I prefer @contextlib.contextmanager rather 
than writing out the class, it makes them shorter and easier to read.



--
PetrĀ³

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH 0065] Use private ccache in ipa-server-install

2013-06-03 Thread Martin Kosek
On 06/03/2013 02:43 PM, Tomas Babej wrote:
 Hi,
 
 this patch fixes the installation problems on master on F19 with krb5 packages
= 1.11.2-6
 
 https://fedorahosted.org/freeipa/ticket/3666
 
 Tomas

1) Leaving cache_desc open:

+(cache_desc, cache_path) = tempfile.mkstemp(prefix='krbcc')
+os.environ['KRB5CCNAME'] = cache_path

Why do we keep the descriptor open and close it at the and of the installation?
Can we close it right after tempfile.mkstemp? I think we do it this way in
other places in installation.

2) What about other installers where we handle Kerberos auth, like
ipa-{replica,dns,ca}-install?

A common function, other shared means, of handling KRB5CCNAME may be
appropriate to avoid duplicating code too much.

Martin

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH 0065] Bump version in .spec file to 2.0

2012-09-20 Thread Adam Tkac
On Thu, Sep 20, 2012 at 04:16:41PM +0200, Petr Spacek wrote:
 Hello,
 
 this patch bumps version in .spec file to 2.0.

Ack

 From b4fc1e119e5d602c196af47bde07d3cfe3091a3d Mon Sep 17 00:00:00 2001
 From: Petr Spacek pspa...@redhat.com
 Date: Thu, 20 Sep 2012 16:14:05 +0200
 Subject: [PATCH] Bump version in .spec file to 2.0.
 
 Signed-off-by: Petr Spacek pspa...@redhat.com
 ---
  contrib/bind-dyndb-ldap.spec | 10 +++---
  1 file changed, 3 insertions(+), 7 deletions(-)
 
 diff --git a/contrib/bind-dyndb-ldap.spec b/contrib/bind-dyndb-ldap.spec
 index 
 664e6a9af9629f07193c82382debac028c425fe1..5f1e0262e4a4707a6bb5a5820f807a98390581ce
  100644
 --- a/contrib/bind-dyndb-ldap.spec
 +++ b/contrib/bind-dyndb-ldap.spec
 @@ -1,12 +1,8 @@
 -#%define PATCHVER P4
 -%define PREVER rc1
 -#%define VERSION %{version}
 -#%define VERSION %{version}-%{PATCHVER}
 -%define VERSION %{version}%{PREVER}
 +%define VERSION %{version}
  
  Name:   bind-dyndb-ldap
 -Version:1.1.0
 -Release:0.1.%{PREVER}%{?dist}
 +Version:2.0
 +Release:0%{?dist}
  Summary:LDAP back-end plug-in for BIND
  
  Group:  System Environment/Libraries
 -- 
 1.7.11.4
 


-- 
Adam Tkac, Red Hat, Inc.

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH 0065] Bump version in .spec file to 2.0

2012-09-20 Thread Petr Spacek

On 09/20/2012 04:18 PM, Adam Tkac wrote:

On Thu, Sep 20, 2012 at 04:16:41PM +0200, Petr Spacek wrote:

Hello,

this patch bumps version in .spec file to 2.0.


Ack


Pushed to master:
https://fedorahosted.org/bind-dyndb-ldap/changeset/bd1e312c74921f2572cad0a6ba7db7d25196d758

--
Petr^2 Spacek

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH] 0065 Ensure ipa-adtrust-install is run with administrator privileges and Kerberos ticket

2012-07-31 Thread Alexander Bokovoy

On Mon, 30 Jul 2012, Martin Kosek wrote:

On 07/30/2012 01:34 PM, Alexander Bokovoy wrote:

On Fri, 27 Jul 2012, Rob Crittenden wrote:

Alexander Bokovoy wrote:

On Thu, 26 Jul 2012, Alexander Bokovoy wrote:

Hi,

When setting up AD trusts support, ipa-adtrust-install utility
needs to be run as:
 - root, for performing Samba configuration and using LDAPI/autobind
 - kinit-ed IPA admin user, to ensure proper ACIs are granted to
   fetch keytab

As result, we can get rid of Directory Manager credentials in
ipa-adtrust-install

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

This ticket also simplifies a bit the way we handle admin connection in
Service class and particulary in Service._ldap_mod() by defaulting to
LDAPI/autobind in case of running as root and to GSSAPI otherwise.
Except few cases in remote replica management (not applicable in
_ldap_mod() case) we always run installation tools as root and can
benefit from using autobind feature. Unfortunately, it is not yet
possible to get away from using DM credentials for all cases as the same
class is used to perform initial directory server instance
configuration.

One side effect is explicit disconnect and reconnect in
Service.add_cert_to_service() due to way how SimpleLDAPObject class
handles stale connections (no handling at all). I've put some comments
in place so that others would not try to err out optimizing it in
future.

Finally, with next patch series which will introduce syncing ipaNTHash
attribute with RC4 key in existing kerberos credentials, we can remove
requirements to change passwords or re-kinit for majority of trust
cases. This should then conclude our trusts content for beta2 release.


Patch updated, fixed small typo (auth_parms was initialized as
auth_params which led to non-existing auth_parms in ipa-adtrust-install
case).


Nack, a couple of minor issues:

The exception handling is rather unusual in
ensure_kerberos_admin_rights(api). I'm not sure if this is any more efficient
than a series of excepts...

I've rewrote this code and put it directly in the main.


You don't need to pass in api, it's a global.

Fixed.



It may be safe to see if the user is in the group the way you are doing it, I
wonder if it would be clearer to cast those into DN objects.

Not sure if checking DNs would be sustaining in long run. Ideally we
should check ACI here, not just hardcoded group name. I'd like to keep
it explicit with memberof for now because it shows what exactly we want
to check.


In the Service class what is the point of ldapi if it is going to be ignored
in the case we know the realm? What if I really, really just want to use a
password?

LDAPI bind in IPAAdmin.__local_init() requires that there is realm known.
No realm -- no LDAPI use because we otherwise cannot construct the
socket name. For 'just want to use a password' case you can simply set
self.dm_password.

However, I've changed the code in Service.ldap_connect() to do
following:

1. if DM password is provided, we'll try to use it
2. Otherwise, if LDAPI is asked for and realm is set, we'll use LDAPI and realm
3. Otherwise (ldapi was False or realm not provided), we'll try to
   connect to fqdn:389 with GSSAPI

I think this covers all cases.


And later where it forces ldapi, it seems better to either commit all the way
and drop the ldapi argument or convert it to a better name (like autobind).

ldapi requires realm but can be used with either GSSAPI or autobind.
Calling it autobind isn't really correct as autobind only available on
ldapi under root.



Works fine, I also have just few minor-ish issues:

1) Uncatched exception

We may want to also catch for DatabaseException in this section:

+api.Backend.ldap2.connect(ccache.name)
+except errors.ACIError, e:
+sys.exit(Outdated Kerberos credentials. Use kdestroy and kinit to
update your ticket)

Otherwise ipa-adtrust-install throws unexpected exception when IPA is down:

# ipactl stop
# ipa-adtrust-install
...
NetBIOS domain name [IDM]:

Unexpected error - see /var/log/ipaserver-install.log for details:
DatabaseError: Can't contact LDAP server:


2) Wrong indentation:
...
+except errors.RequirementError, e:
+   sys.exit(Must have administrative privileges to setup AD trusts on
server)
+except Exception, e:
+   sys.exit(Unrecognized error during check of admin rights: %s %
(str(e)))

Updated patch, includes fixes for issues mentioned above and also
implements autobind suggestions by Simo.

We have SimpleServiceInstance() that doesn't have realm known by default
and this means certain protection is needed for missing realm. Also DS
and CA DS instances cannot use LDAPI and autobind when being setup, only
DM password. The patch handles these cases too.

--
/ Alexander Bokovoy
From 29bfb9491614afba470787df05a150f424a4e26f Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy aboko...@redhat.com
Date: Fri, 13 Jul 2012 18:12:48 +0300
Subject: [PATCH 2/6] Ensure ipa-adtrust-install is run with 

Re: [Freeipa-devel] [PATCH] 0065 Ensure ipa-adtrust-install is run with administrator privileges and Kerberos ticket

2012-07-31 Thread Martin Kosek
On 07/31/2012 02:00 PM, Alexander Bokovoy wrote:
 On Mon, 30 Jul 2012, Martin Kosek wrote:
 On 07/30/2012 01:34 PM, Alexander Bokovoy wrote:
 On Fri, 27 Jul 2012, Rob Crittenden wrote:
 Alexander Bokovoy wrote:
 On Thu, 26 Jul 2012, Alexander Bokovoy wrote:
 Hi,

 When setting up AD trusts support, ipa-adtrust-install utility
 needs to be run as:
  - root, for performing Samba configuration and using LDAPI/autobind
  - kinit-ed IPA admin user, to ensure proper ACIs are granted to
fetch keytab

 As result, we can get rid of Directory Manager credentials in
 ipa-adtrust-install

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

 This ticket also simplifies a bit the way we handle admin connection in
 Service class and particulary in Service._ldap_mod() by defaulting to
 LDAPI/autobind in case of running as root and to GSSAPI otherwise.
 Except few cases in remote replica management (not applicable in
 _ldap_mod() case) we always run installation tools as root and can
 benefit from using autobind feature. Unfortunately, it is not yet
 possible to get away from using DM credentials for all cases as the same
 class is used to perform initial directory server instance
 configuration.

 One side effect is explicit disconnect and reconnect in
 Service.add_cert_to_service() due to way how SimpleLDAPObject class
 handles stale connections (no handling at all). I've put some comments
 in place so that others would not try to err out optimizing it in
 future.

 Finally, with next patch series which will introduce syncing ipaNTHash
 attribute with RC4 key in existing kerberos credentials, we can remove
 requirements to change passwords or re-kinit for majority of trust
 cases. This should then conclude our trusts content for beta2 release.

 Patch updated, fixed small typo (auth_parms was initialized as
 auth_params which led to non-existing auth_parms in ipa-adtrust-install
 case).

 Nack, a couple of minor issues:

 The exception handling is rather unusual in
 ensure_kerberos_admin_rights(api). I'm not sure if this is any more 
 efficient
 than a series of excepts...
 I've rewrote this code and put it directly in the main.

 You don't need to pass in api, it's a global.
 Fixed.


 It may be safe to see if the user is in the group the way you are doing 
 it, I
 wonder if it would be clearer to cast those into DN objects.
 Not sure if checking DNs would be sustaining in long run. Ideally we
 should check ACI here, not just hardcoded group name. I'd like to keep
 it explicit with memberof for now because it shows what exactly we want
 to check.

 In the Service class what is the point of ldapi if it is going to be 
 ignored
 in the case we know the realm? What if I really, really just want to use a
 password?
 LDAPI bind in IPAAdmin.__local_init() requires that there is realm known.
 No realm -- no LDAPI use because we otherwise cannot construct the
 socket name. For 'just want to use a password' case you can simply set
 self.dm_password.

 However, I've changed the code in Service.ldap_connect() to do
 following:

 1. if DM password is provided, we'll try to use it
 2. Otherwise, if LDAPI is asked for and realm is set, we'll use LDAPI and 
 realm
 3. Otherwise (ldapi was False or realm not provided), we'll try to
connect to fqdn:389 with GSSAPI

 I think this covers all cases.

 And later where it forces ldapi, it seems better to either commit all the 
 way
 and drop the ldapi argument or convert it to a better name (like autobind).
 ldapi requires realm but can be used with either GSSAPI or autobind.
 Calling it autobind isn't really correct as autobind only available on
 ldapi under root.


 Works fine, I also have just few minor-ish issues:

 1) Uncatched exception

 We may want to also catch for DatabaseException in this section:

 +api.Backend.ldap2.connect(ccache.name)
 +except errors.ACIError, e:
 +sys.exit(Outdated Kerberos credentials. Use kdestroy and kinit to
 update your ticket)

 Otherwise ipa-adtrust-install throws unexpected exception when IPA is down:

 # ipactl stop
 # ipa-adtrust-install
 ...
 NetBIOS domain name [IDM]:

 Unexpected error - see /var/log/ipaserver-install.log for details:
 DatabaseError: Can't contact LDAP server:


 2) Wrong indentation:
 ...
 +except errors.RequirementError, e:
 +   sys.exit(Must have administrative privileges to setup AD trusts 
 on
 server)
 +except Exception, e:
 +   sys.exit(Unrecognized error during check of admin rights: %s %
 (str(e)))
 Updated patch, includes fixes for issues mentioned above and also
 implements autobind suggestions by Simo.
 
 We have SimpleServiceInstance() that doesn't have realm known by default
 and this means certain protection is needed for missing realm. Also DS
 and CA DS instances cannot use LDAPI and autobind when being setup, only
 DM password. The patch handles these cases too.
 

Hm, ipa-dns-install is now broken:

# ipa-dns-install

The log file for this installation can be 

Re: [Freeipa-devel] [PATCH] 0065 Ensure ipa-adtrust-install is run with administrator privileges and Kerberos ticket

2012-07-31 Thread Martin Kosek
On 07/31/2012 04:20 PM, Alexander Bokovoy wrote:
 On Tue, 31 Jul 2012, Martin Kosek wrote:
 On 07/31/2012 02:00 PM, Alexander Bokovoy wrote:
 On Mon, 30 Jul 2012, Martin Kosek wrote:
 On 07/30/2012 01:34 PM, Alexander Bokovoy wrote:
 On Fri, 27 Jul 2012, Rob Crittenden wrote:
 Alexander Bokovoy wrote:
 On Thu, 26 Jul 2012, Alexander Bokovoy wrote:
 Hi,

 When setting up AD trusts support, ipa-adtrust-install utility
 needs to be run as:
  - root, for performing Samba configuration and using LDAPI/autobind
  - kinit-ed IPA admin user, to ensure proper ACIs are granted to
fetch keytab

 As result, we can get rid of Directory Manager credentials in
 ipa-adtrust-install

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

 This ticket also simplifies a bit the way we handle admin connection in
 Service class and particulary in Service._ldap_mod() by defaulting to
 LDAPI/autobind in case of running as root and to GSSAPI otherwise.
 Except few cases in remote replica management (not applicable in
 _ldap_mod() case) we always run installation tools as root and can
 benefit from using autobind feature. Unfortunately, it is not yet
 possible to get away from using DM credentials for all cases as the 
 same
 class is used to perform initial directory server instance
 configuration.

 One side effect is explicit disconnect and reconnect in
 Service.add_cert_to_service() due to way how SimpleLDAPObject class
 handles stale connections (no handling at all). I've put some comments
 in place so that others would not try to err out optimizing it in
 future.

 Finally, with next patch series which will introduce syncing ipaNTHash
 attribute with RC4 key in existing kerberos credentials, we can remove
 requirements to change passwords or re-kinit for majority of trust
 cases. This should then conclude our trusts content for beta2 release.

 Patch updated, fixed small typo (auth_parms was initialized as
 auth_params which led to non-existing auth_parms in ipa-adtrust-install
 case).

 Nack, a couple of minor issues:

 The exception handling is rather unusual in
 ensure_kerberos_admin_rights(api). I'm not sure if this is any more
 efficient
 than a series of excepts...
 I've rewrote this code and put it directly in the main.

 You don't need to pass in api, it's a global.
 Fixed.


 It may be safe to see if the user is in the group the way you are doing
 it, I
 wonder if it would be clearer to cast those into DN objects.
 Not sure if checking DNs would be sustaining in long run. Ideally we
 should check ACI here, not just hardcoded group name. I'd like to keep
 it explicit with memberof for now because it shows what exactly we want
 to check.

 In the Service class what is the point of ldapi if it is going to be 
 ignored
 in the case we know the realm? What if I really, really just want to use 
 a
 password?
 LDAPI bind in IPAAdmin.__local_init() requires that there is realm known.
 No realm -- no LDAPI use because we otherwise cannot construct the
 socket name. For 'just want to use a password' case you can simply set
 self.dm_password.

 However, I've changed the code in Service.ldap_connect() to do
 following:

 1. if DM password is provided, we'll try to use it
 2. Otherwise, if LDAPI is asked for and realm is set, we'll use LDAPI and
 realm
 3. Otherwise (ldapi was False or realm not provided), we'll try to
connect to fqdn:389 with GSSAPI

 I think this covers all cases.

 And later where it forces ldapi, it seems better to either commit all the
 way
 and drop the ldapi argument or convert it to a better name (like 
 autobind).
 ldapi requires realm but can be used with either GSSAPI or autobind.
 Calling it autobind isn't really correct as autobind only available on
 ldapi under root.


 Works fine, I also have just few minor-ish issues:

 1) Uncatched exception

 We may want to also catch for DatabaseException in this section:

 +api.Backend.ldap2.connect(ccache.name)
 +except errors.ACIError, e:
 +sys.exit(Outdated Kerberos credentials. Use kdestroy and kinit to
 update your ticket)

 Otherwise ipa-adtrust-install throws unexpected exception when IPA is down:

 # ipactl stop
 # ipa-adtrust-install
 ...
 NetBIOS domain name [IDM]:

 Unexpected error - see /var/log/ipaserver-install.log for details:
 DatabaseError: Can't contact LDAP server:


 2) Wrong indentation:
 ...
 +except errors.RequirementError, e:
 +   sys.exit(Must have administrative privileges to setup AD
 trusts on
 server)
 +except Exception, e:
 +   sys.exit(Unrecognized error during check of admin rights: %s 
 %
 (str(e)))
 Updated patch, includes fixes for issues mentioned above and also
 implements autobind suggestions by Simo.

 We have SimpleServiceInstance() that doesn't have realm known by default
 and this means certain protection is needed for missing realm. Also DS
 and CA DS instances cannot use LDAPI and autobind when being setup, only
 DM password. The patch handles these cases too.


 Hm, 

Re: [Freeipa-devel] [PATCH] 0065 Ensure ipa-adtrust-install is run with administrator privileges and Kerberos ticket

2012-07-30 Thread Alexander Bokovoy

On Fri, 27 Jul 2012, Rob Crittenden wrote:

Alexander Bokovoy wrote:

On Thu, 26 Jul 2012, Alexander Bokovoy wrote:

Hi,

When setting up AD trusts support, ipa-adtrust-install utility
needs to be run as:
 - root, for performing Samba configuration and using LDAPI/autobind
 - kinit-ed IPA admin user, to ensure proper ACIs are granted to
   fetch keytab

As result, we can get rid of Directory Manager credentials in
ipa-adtrust-install

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

This ticket also simplifies a bit the way we handle admin connection in
Service class and particulary in Service._ldap_mod() by defaulting to
LDAPI/autobind in case of running as root and to GSSAPI otherwise.
Except few cases in remote replica management (not applicable in
_ldap_mod() case) we always run installation tools as root and can
benefit from using autobind feature. Unfortunately, it is not yet
possible to get away from using DM credentials for all cases as the same
class is used to perform initial directory server instance
configuration.

One side effect is explicit disconnect and reconnect in
Service.add_cert_to_service() due to way how SimpleLDAPObject class
handles stale connections (no handling at all). I've put some comments
in place so that others would not try to err out optimizing it in
future.

Finally, with next patch series which will introduce syncing ipaNTHash
attribute with RC4 key in existing kerberos credentials, we can remove
requirements to change passwords or re-kinit for majority of trust
cases. This should then conclude our trusts content for beta2 release.


Patch updated, fixed small typo (auth_parms was initialized as
auth_params which led to non-existing auth_parms in ipa-adtrust-install
case).


Nack, a couple of minor issues:

The exception handling is rather unusual in 
ensure_kerberos_admin_rights(api). I'm not sure if this is any more 
efficient than a series of excepts...

I've rewrote this code and put it directly in the main.


You don't need to pass in api, it's a global.

Fixed.


It may be safe to see if the user is in the group the way you are 
doing it, I wonder if it would be clearer to cast those into DN 
objects.

Not sure if checking DNs would be sustaining in long run. Ideally we
should check ACI here, not just hardcoded group name. I'd like to keep
it explicit with memberof for now because it shows what exactly we want
to check.

In the Service class what is the point of ldapi if it is going to be 
ignored in the case we know the realm? What if I really, really just 
want to use a password?

LDAPI bind in IPAAdmin.__local_init() requires that there is realm known.
No realm -- no LDAPI use because we otherwise cannot construct the
socket name. For 'just want to use a password' case you can simply set
self.dm_password.

However, I've changed the code in Service.ldap_connect() to do
following:

1. if DM password is provided, we'll try to use it
2. Otherwise, if LDAPI is asked for and realm is set, we'll use LDAPI and realm
3. Otherwise (ldapi was False or realm not provided), we'll try to
   connect to fqdn:389 with GSSAPI

I think this covers all cases.

And later where it forces ldapi, it seems better to either commit all 
the way and drop the ldapi argument or convert it to a better name 
(like autobind).

ldapi requires realm but can be used with either GSSAPI or autobind.
Calling it autobind isn't really correct as autobind only available on
ldapi under root.

--
/ Alexander Bokovoy
From d7fea3f6f50218821560ea739a26030f3a76640f Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy aboko...@redhat.com
Date: Fri, 13 Jul 2012 18:12:48 +0300
Subject: [PATCH 2/6] Ensure ipa-adtrust-install is run with Kerberos ticket
 for admin user

When setting up AD trusts support, ipa-adtrust-install utility
needs to be run as:
   - root, for performing Samba configuration and using LDAPI/autobind
   - kinit-ed IPA admin user, to ensure proper ACIs are granted to
 fetch keytab

As result, we can get rid of Directory Manager credentials in 
ipa-adtrust-install

https://fedorahosted.org/freeipa/ticket/2815
---
 install/tools/ipa-adtrust-install   | 37 +--
 install/tools/man/ipa-adtrust-install.1 |  3 --
 ipaserver/install/adtrustinstance.py| 21 ++---
 ipaserver/install/service.py| 53 -
 4 files changed, 71 insertions(+), 43 deletions(-)

diff --git a/install/tools/ipa-adtrust-install 
b/install/tools/ipa-adtrust-install
index 
6678018e6346d75d5042894cfb833d38079d3f21..f1cff30bd439ee07226034b3e49ae17cc59ce6b0
 100755
--- a/install/tools/ipa-adtrust-install
+++ b/install/tools/ipa-adtrust-install
@@ -37,8 +37,6 @@ log_file_name = /var/log/ipaserver-install.log
 
 def parse_options():
 parser = IPAOptionParser(version=version.VERSION)
-parser.add_option(-p, --ds-password, dest=dm_password,
-  sensitive=True, help=directory manager password)
 parser.add_option(-d, --debug, dest=debug, 

Re: [Freeipa-devel] [PATCH] 0065 Ensure ipa-adtrust-install is run with administrator privileges and Kerberos ticket

2012-07-30 Thread Simo Sorce
On Mon, 2012-07-30 at 14:34 +0300, Alexander Bokovoy wrote:
 On Fri, 27 Jul 2012, Rob Crittenden wrote:
 Alexander Bokovoy wrote:
 On Thu, 26 Jul 2012, Alexander Bokovoy wrote:
 Hi,
 
 When setting up AD trusts support, ipa-adtrust-install utility
 needs to be run as:
   - root, for performing Samba configuration and using LDAPI/autobind
   - kinit-ed IPA admin user, to ensure proper ACIs are granted to
 fetch keytab
 
 As result, we can get rid of Directory Manager credentials in
 ipa-adtrust-install
 
 https://fedorahosted.org/freeipa/ticket/2815
 
 This ticket also simplifies a bit the way we handle admin connection in
 Service class and particulary in Service._ldap_mod() by defaulting to
 LDAPI/autobind in case of running as root and to GSSAPI otherwise.
 Except few cases in remote replica management (not applicable in
 _ldap_mod() case) we always run installation tools as root and can
 benefit from using autobind feature. Unfortunately, it is not yet
 possible to get away from using DM credentials for all cases as the same
 class is used to perform initial directory server instance
 configuration.
 
 One side effect is explicit disconnect and reconnect in
 Service.add_cert_to_service() due to way how SimpleLDAPObject class
 handles stale connections (no handling at all). I've put some comments
 in place so that others would not try to err out optimizing it in
 future.
 
 Finally, with next patch series which will introduce syncing ipaNTHash
 attribute with RC4 key in existing kerberos credentials, we can remove
 requirements to change passwords or re-kinit for majority of trust
 cases. This should then conclude our trusts content for beta2 release.
 
 Patch updated, fixed small typo (auth_parms was initialized as
 auth_params which led to non-existing auth_parms in ipa-adtrust-install
 case).
 
 Nack, a couple of minor issues:
 
 The exception handling is rather unusual in 
 ensure_kerberos_admin_rights(api). I'm not sure if this is any more 
 efficient than a series of excepts...
 I've rewrote this code and put it directly in the main.
 
 You don't need to pass in api, it's a global.
 Fixed.
 
 
 It may be safe to see if the user is in the group the way you are 
 doing it, I wonder if it would be clearer to cast those into DN 
 objects.
 Not sure if checking DNs would be sustaining in long run. Ideally we
 should check ACI here, not just hardcoded group name. I'd like to keep
 it explicit with memberof for now because it shows what exactly we want
 to check.
 
 In the Service class what is the point of ldapi if it is going to be 
 ignored in the case we know the realm? What if I really, really just 
 want to use a password?
 LDAPI bind in IPAAdmin.__local_init() requires that there is realm known.
 No realm -- no LDAPI use because we otherwise cannot construct the
 socket name. For 'just want to use a password' case you can simply set
 self.dm_password.
 
 However, I've changed the code in Service.ldap_connect() to do
 following:
 
 1. if DM password is provided, we'll try to use it
 2. Otherwise, if LDAPI is asked for and realm is set, we'll use LDAPI and 
 realm
 3. Otherwise (ldapi was False or realm not provided), we'll try to
 connect to fqdn:389 with GSSAPI
 
 I think this covers all cases.
 
 And later where it forces ldapi, it seems better to either commit all 
 the way and drop the ldapi argument or convert it to a better name 
 (like autobind).
 ldapi requires realm but can be used with either GSSAPI or autobind.
 Calling it autobind isn't really correct as autobind only available on
 ldapi under root.

Alexander, the realm is always available ion api.* so it seem that the
case where the realm is missing can basically never happen, and it seem
like it make no sense to explicitly check that case.

This means you'll never fall back to GSSAPI unless you also check what
user are you running as. I think we should not try ldapi unless
explicitly requested if we are not root (uid 0).

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH] 0065 Ensure ipa-adtrust-install is run with administrator privileges and Kerberos ticket

2012-07-30 Thread Martin Kosek
On 07/30/2012 01:34 PM, Alexander Bokovoy wrote:
 On Fri, 27 Jul 2012, Rob Crittenden wrote:
 Alexander Bokovoy wrote:
 On Thu, 26 Jul 2012, Alexander Bokovoy wrote:
 Hi,

 When setting up AD trusts support, ipa-adtrust-install utility
 needs to be run as:
  - root, for performing Samba configuration and using LDAPI/autobind
  - kinit-ed IPA admin user, to ensure proper ACIs are granted to
fetch keytab

 As result, we can get rid of Directory Manager credentials in
 ipa-adtrust-install

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

 This ticket also simplifies a bit the way we handle admin connection in
 Service class and particulary in Service._ldap_mod() by defaulting to
 LDAPI/autobind in case of running as root and to GSSAPI otherwise.
 Except few cases in remote replica management (not applicable in
 _ldap_mod() case) we always run installation tools as root and can
 benefit from using autobind feature. Unfortunately, it is not yet
 possible to get away from using DM credentials for all cases as the same
 class is used to perform initial directory server instance
 configuration.

 One side effect is explicit disconnect and reconnect in
 Service.add_cert_to_service() due to way how SimpleLDAPObject class
 handles stale connections (no handling at all). I've put some comments
 in place so that others would not try to err out optimizing it in
 future.

 Finally, with next patch series which will introduce syncing ipaNTHash
 attribute with RC4 key in existing kerberos credentials, we can remove
 requirements to change passwords or re-kinit for majority of trust
 cases. This should then conclude our trusts content for beta2 release.

 Patch updated, fixed small typo (auth_parms was initialized as
 auth_params which led to non-existing auth_parms in ipa-adtrust-install
 case).

 Nack, a couple of minor issues:

 The exception handling is rather unusual in
 ensure_kerberos_admin_rights(api). I'm not sure if this is any more efficient
 than a series of excepts...
 I've rewrote this code and put it directly in the main.
 
 You don't need to pass in api, it's a global.
 Fixed.
 
 
 It may be safe to see if the user is in the group the way you are doing it, I
 wonder if it would be clearer to cast those into DN objects.
 Not sure if checking DNs would be sustaining in long run. Ideally we
 should check ACI here, not just hardcoded group name. I'd like to keep
 it explicit with memberof for now because it shows what exactly we want
 to check.
 
 In the Service class what is the point of ldapi if it is going to be ignored
 in the case we know the realm? What if I really, really just want to use a
 password?
 LDAPI bind in IPAAdmin.__local_init() requires that there is realm known.
 No realm -- no LDAPI use because we otherwise cannot construct the
 socket name. For 'just want to use a password' case you can simply set
 self.dm_password.
 
 However, I've changed the code in Service.ldap_connect() to do
 following:
 
 1. if DM password is provided, we'll try to use it
 2. Otherwise, if LDAPI is asked for and realm is set, we'll use LDAPI and 
 realm
 3. Otherwise (ldapi was False or realm not provided), we'll try to
connect to fqdn:389 with GSSAPI
 
 I think this covers all cases.
 
 And later where it forces ldapi, it seems better to either commit all the way
 and drop the ldapi argument or convert it to a better name (like autobind).
 ldapi requires realm but can be used with either GSSAPI or autobind.
 Calling it autobind isn't really correct as autobind only available on
 ldapi under root.
 

Works fine, I also have just few minor-ish issues:

1) Uncatched exception

We may want to also catch for DatabaseException in this section:

+api.Backend.ldap2.connect(ccache.name)
+except errors.ACIError, e:
+sys.exit(Outdated Kerberos credentials. Use kdestroy and kinit to
update your ticket)

Otherwise ipa-adtrust-install throws unexpected exception when IPA is down:

# ipactl stop
# ipa-adtrust-install
...
NetBIOS domain name [IDM]:

Unexpected error - see /var/log/ipaserver-install.log for details:
DatabaseError: Can't contact LDAP server:


2) Wrong indentation:
...
+except errors.RequirementError, e:
+   sys.exit(Must have administrative privileges to setup AD trusts on
server)
+except Exception, e:
+   sys.exit(Unrecognized error during check of admin rights: %s %
(str(e)))

Martin

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH] 0065 Ensure ipa-adtrust-install is run with administrator privileges and Kerberos ticket

2012-07-30 Thread Alexander Bokovoy

On Mon, 30 Jul 2012, Simo Sorce wrote:

On Mon, 2012-07-30 at 14:34 +0300, Alexander Bokovoy wrote:

On Fri, 27 Jul 2012, Rob Crittenden wrote:
Alexander Bokovoy wrote:
On Thu, 26 Jul 2012, Alexander Bokovoy wrote:
Hi,

When setting up AD trusts support, ipa-adtrust-install utility
needs to be run as:
  - root, for performing Samba configuration and using LDAPI/autobind
  - kinit-ed IPA admin user, to ensure proper ACIs are granted to
fetch keytab

As result, we can get rid of Directory Manager credentials in
ipa-adtrust-install

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

This ticket also simplifies a bit the way we handle admin connection in
Service class and particulary in Service._ldap_mod() by defaulting to
LDAPI/autobind in case of running as root and to GSSAPI otherwise.
Except few cases in remote replica management (not applicable in
_ldap_mod() case) we always run installation tools as root and can
benefit from using autobind feature. Unfortunately, it is not yet
possible to get away from using DM credentials for all cases as the same
class is used to perform initial directory server instance
configuration.

One side effect is explicit disconnect and reconnect in
Service.add_cert_to_service() due to way how SimpleLDAPObject class
handles stale connections (no handling at all). I've put some comments
in place so that others would not try to err out optimizing it in
future.

Finally, with next patch series which will introduce syncing ipaNTHash
attribute with RC4 key in existing kerberos credentials, we can remove
requirements to change passwords or re-kinit for majority of trust
cases. This should then conclude our trusts content for beta2 release.

Patch updated, fixed small typo (auth_parms was initialized as
auth_params which led to non-existing auth_parms in ipa-adtrust-install
case).

Nack, a couple of minor issues:

The exception handling is rather unusual in
ensure_kerberos_admin_rights(api). I'm not sure if this is any more
efficient than a series of excepts...
I've rewrote this code and put it directly in the main.

You don't need to pass in api, it's a global.
Fixed.


It may be safe to see if the user is in the group the way you are
doing it, I wonder if it would be clearer to cast those into DN
objects.
Not sure if checking DNs would be sustaining in long run. Ideally we
should check ACI here, not just hardcoded group name. I'd like to keep
it explicit with memberof for now because it shows what exactly we want
to check.

In the Service class what is the point of ldapi if it is going to be
ignored in the case we know the realm? What if I really, really just
want to use a password?
LDAPI bind in IPAAdmin.__local_init() requires that there is realm known.
No realm -- no LDAPI use because we otherwise cannot construct the
socket name. For 'just want to use a password' case you can simply set
self.dm_password.

However, I've changed the code in Service.ldap_connect() to do
following:

1. if DM password is provided, we'll try to use it
2. Otherwise, if LDAPI is asked for and realm is set, we'll use LDAPI and realm
3. Otherwise (ldapi was False or realm not provided), we'll try to
connect to fqdn:389 with GSSAPI

I think this covers all cases.

And later where it forces ldapi, it seems better to either commit all
the way and drop the ldapi argument or convert it to a better name
(like autobind).
ldapi requires realm but can be used with either GSSAPI or autobind.
Calling it autobind isn't really correct as autobind only available on
ldapi under root.


Alexander, the realm is always available ion api.* so it seem that the
case where the realm is missing can basically never happen, and it seem
like it make no sense to explicitly check that case.

This is the code that has no access to api.* because it is run during
install when config files are not yet written.



This means you'll never fall back to GSSAPI unless you also check what
user are you running as. I think we should not try ldapi unless
explicitly requested if we are not root (uid 0).

There are checks already, look at Service.__get_conn() for real logic.

In Service class there are two stages:
1. Service.ldap_connect() which takes existing values from Service's
self and decides how to call __get_conn().
2. Service.__get_conn() which does connection and binds depending on its
arguments.

This split seems to be historical as nobody else is calling into
__get_conn() at all so all possible users of __get_conn() are in
Service.ldap_connect().



--
/ Alexander Bokovoy

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH] 0065 Ensure ipa-adtrust-install is run with administrator privileges and Kerberos ticket

2012-07-27 Thread Rob Crittenden

Alexander Bokovoy wrote:

On Thu, 26 Jul 2012, Alexander Bokovoy wrote:

Hi,

When setting up AD trusts support, ipa-adtrust-install utility
needs to be run as:
  - root, for performing Samba configuration and using LDAPI/autobind
  - kinit-ed IPA admin user, to ensure proper ACIs are granted to
fetch keytab

As result, we can get rid of Directory Manager credentials in
ipa-adtrust-install

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

This ticket also simplifies a bit the way we handle admin connection in
Service class and particulary in Service._ldap_mod() by defaulting to
LDAPI/autobind in case of running as root and to GSSAPI otherwise.
Except few cases in remote replica management (not applicable in
_ldap_mod() case) we always run installation tools as root and can
benefit from using autobind feature. Unfortunately, it is not yet
possible to get away from using DM credentials for all cases as the same
class is used to perform initial directory server instance
configuration.

One side effect is explicit disconnect and reconnect in
Service.add_cert_to_service() due to way how SimpleLDAPObject class
handles stale connections (no handling at all). I've put some comments
in place so that others would not try to err out optimizing it in
future.

Finally, with next patch series which will introduce syncing ipaNTHash
attribute with RC4 key in existing kerberos credentials, we can remove
requirements to change passwords or re-kinit for majority of trust
cases. This should then conclude our trusts content for beta2 release.


Patch updated, fixed small typo (auth_parms was initialized as
auth_params which led to non-existing auth_parms in ipa-adtrust-install
case).


Nack, a couple of minor issues:

The exception handling is rather unusual in 
ensure_kerberos_admin_rights(api). I'm not sure if this is any more 
efficient than a series of excepts...


You don't need to pass in api, it's a global.

It may be safe to see if the user is in the group the way you are doing 
it, I wonder if it would be clearer to cast those into DN objects.


In the Service class what is the point of ldapi if it is going to be 
ignored in the case we know the realm? What if I really, really just 
want to use a password?


And later where it forces ldapi, it seems better to either commit all 
the way and drop the ldapi argument or convert it to a better name (like 
autobind).


Typo in big comment block, 'largerly'

rob

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH] 0065 Ensure ipa-adtrust-install is run with administrator privileges and Kerberos ticket

2012-07-26 Thread Alexander Bokovoy

On Thu, 26 Jul 2012, Alexander Bokovoy wrote:

Hi,

When setting up AD trusts support, ipa-adtrust-install utility
needs to be run as:
  - root, for performing Samba configuration and using LDAPI/autobind
  - kinit-ed IPA admin user, to ensure proper ACIs are granted to
fetch keytab

As result, we can get rid of Directory Manager credentials in
ipa-adtrust-install

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

This ticket also simplifies a bit the way we handle admin connection in
Service class and particulary in Service._ldap_mod() by defaulting to
LDAPI/autobind in case of running as root and to GSSAPI otherwise.
Except few cases in remote replica management (not applicable in
_ldap_mod() case) we always run installation tools as root and can
benefit from using autobind feature. Unfortunately, it is not yet
possible to get away from using DM credentials for all cases as the same
class is used to perform initial directory server instance
configuration.

One side effect is explicit disconnect and reconnect in
Service.add_cert_to_service() due to way how SimpleLDAPObject class
handles stale connections (no handling at all). I've put some comments
in place so that others would not try to err out optimizing it in
future.

Finally, with next patch series which will introduce syncing ipaNTHash
attribute with RC4 key in existing kerberos credentials, we can remove
requirements to change passwords or re-kinit for majority of trust
cases. This should then conclude our trusts content for beta2 release.


Patch updated, fixed small typo (auth_parms was initialized as
auth_params which led to non-existing auth_parms in ipa-adtrust-install
case).

--
/ Alexander Bokovoy
From 46391aa5953426d763c0c1b627c8abbf80d6fec2 Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy aboko...@redhat.com
Date: Fri, 13 Jul 2012 18:12:48 +0300
Subject: [PATCH 2/6] Ensure ipa-adtrust-install is run with Kerberos ticket
 for admin user

When setting up AD trusts support, ipa-adtrust-install utility
needs to be run as:
   - root, for performing Samba configuration and using LDAPI/autobind
   - kinit-ed IPA admin user, to ensure proper ACIs are granted to
 fetch keytab

As result, we can get rid of Directory Manager credentials in 
ipa-adtrust-install

https://fedorahosted.org/freeipa/ticket/2815
---
 install/tools/ipa-adtrust-install   |   42 +
 install/tools/man/ipa-adtrust-install.1 |3 ---
 ipaserver/install/adtrustinstance.py|   21 ---
 ipaserver/install/service.py|   44 ++-
 4 files changed, 74 insertions(+), 36 deletions(-)

diff --git a/install/tools/ipa-adtrust-install 
b/install/tools/ipa-adtrust-install
index 
6678018e6346d75d5042894cfb833d38079d3f21..f367d5b2b516bd411bce9275ff299eb3ffdf6bf9
 100755
--- a/install/tools/ipa-adtrust-install
+++ b/install/tools/ipa-adtrust-install
@@ -37,8 +37,6 @@ log_file_name = /var/log/ipaserver-install.log
 
 def parse_options():
 parser = IPAOptionParser(version=version.VERSION)
-parser.add_option(-p, --ds-password, dest=dm_password,
-  sensitive=True, help=directory manager password)
 parser.add_option(-d, --debug, dest=debug, action=store_true,
   default=False, help=print debugging information)
 parser.add_option(--ip-address, dest=ip_address,
@@ -86,6 +84,30 @@ def read_netbios_name(netbios_default):
 
 return netbios_name
 
+def ensure_kerberos_admin_rights(api):
+try:
+ctx = krbV.default_context()
+ccache = ctx.default_ccache()
+principal = ccache.principal()
+api.Backend.ldap2.connect(ccache.name)
+user = api.Command.user_show(unicode(principal[0]))['result']
+group = api.Command.group_show(u'admins')['result']
+api.Backend.ldap2.disconnect()
+if not (user['uid'][0] in group['member_user'] and
+group['cn'][0] in user['memberof_group']):
+raise errors.RequirementError(name='admins group membership')
+except Exception, e:
+error_messages = dict(
+   ACIError = Outdated Kerberos credentials. Use kdestroy and kinit 
to update your ticket,
+   Krb5Error = Must have Kerberos credentials to setup AD trusts on 
server,
+   RequirementError = Must have administrative privileges to setup AD 
trusts on server
+)
+name = type(e).__name__
+if name in error_messages:
+sys.exit(error_messages[name])
+else:
+sys.exit(Unrecognized error during check of admin rights: %s\n%s 
% (name, str(e)))
+
 def main():
 safe_options, options = parse_options()
 
@@ -128,6 +150,8 @@ def main():
 api.bootstrap(**cfg)
 api.finalize()
 
+ensure_kerberos_admin_rights(api)
+
 if adtrustinstance.ipa_smb_conf_exists():
 if not options.unattended:
 while True:
@@ -194,9 +218,8 @@ def main():
 if not options.unattended and ( not netbios_name or 

Re: [Freeipa-devel] [PATCH] 0065 Improve output validation

2012-06-26 Thread Rob Crittenden

Petr Viktorin wrote:

In my patch 62 I fixed output validation. Since that patch was rejected,
I'm submitting the fix separately.

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


ACK, pushed to master

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH] 0065 Use ldapi with krb5kdc

2011-01-20 Thread JR Aquino
NACK.

Please retest this... I'm not sure how it is related, but I receive an
error during the make rpm process:

Traceback (most recent call last):
  File ./makeapi, line 27, in module
from ipalib import *
  File 
/usr/src/freeipa/rpmbuild/BUILD/freeipa-2.0.0GITb9ad279/ipalib/__init__.py
, line 878, in module
from frontend import Command, LocalOrRemote
  File 
/usr/src/freeipa/rpmbuild/BUILD/freeipa-2.0.0GITb9ad279/ipalib/frontend.py
, line 36, in module
from ipapython.version import API_VERSION
  File 
/usr/src/freeipa/rpmbuild/BUILD/freeipa-2.0.0GITb9ad279/ipapython/version.
py, line 25, in module
NUM_VERSION=200
NameError: name '__NUM_VERSION__' is not defined
make[1]: *** [version-update] Error 1
make[1]: Leaving directory
`/usr/src/freeipa/rpmbuild/BUILD/freeipa-2.0.0GITb9ad279'
error: Bad exit status from /var/tmp/rpm-tmp.315pIJ (%build)


RPM build errors:
Bad exit status from /var/tmp/rpm-tmp.315pIJ (%build)
make: *** [rpms] Error 1



On 1/19/11 4:11 PM, Simo Sorce sso...@redhat.com wrote:


Long ago we decided to use the ldapi socket to let the KDC access the
ldap data in order to avoid comunication over the network (even if it
is 127.0.0.1).

This patch finally implements that. Although beware that this patch
will need you to either create custom policy or to set selinux in
permissive mode until the new policy lands in fedora land.

Bugs have been opened and I think the policy has already landed in
rawhide.

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH] 0065 Use ldapi with krb5kdc

2011-01-20 Thread JR Aquino
On 1/20/11 10:11 AM, Rob Crittenden rcrit...@redhat.com wrote:

JR Aquino wrote:
 NACK.

 Please retest this... I'm not sure how it is related, but I receive an
 error during the make rpm process:

 Traceback (most recent call last):
File ./makeapi, line 27, inmodule
  from ipalib import *
File
 
/usr/src/freeipa/rpmbuild/BUILD/freeipa-2.0.0GITb9ad279/ipalib/__init__.
py
 , line 878, inmodule
  from frontend import Command, LocalOrRemote
File
 
/usr/src/freeipa/rpmbuild/BUILD/freeipa-2.0.0GITb9ad279/ipalib/frontend.
py
 , line 36, inmodule
  from ipapython.version import API_VERSION
File
 
/usr/src/freeipa/rpmbuild/BUILD/freeipa-2.0.0GITb9ad279/ipapython/versio
n.
 py, line 25, inmodule
  NUM_VERSION=200
 NameError: name '__NUM_VERSION__' is not defined
 make[1]: *** [version-update] Error 1
 make[1]: Leaving directory
 `/usr/src/freeipa/rpmbuild/BUILD/freeipa-2.0.0GITb9ad279'
 error: Bad exit status from /var/tmp/rpm-tmp.315pIJ (%build)


 RPM build errors:
  Bad exit status from /var/tmp/rpm-tmp.315pIJ (%build)
 make: *** [rpms] Error 1

This error is unrelated though I'm unsure what is broken. The first
thing the build should do is run the version-update target which will do
substitutions in ipapython/version.py.in into ipapython/version.py. It
seems that didn't happen or is otherwise broke. Can you see if
version-update is being called by make?

rob

Thank you for catching that Rob!

This was unrelated.  Did a full remove and a new clone.

Patch works correctly.

ACK



___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH] 0065 Use ldapi with krb5kdc

2011-01-20 Thread Simo Sorce
On Thu, 20 Jan 2011 19:24:59 +
JR Aquino jr.aqu...@citrix.com wrote:

 Patch works correctly.
 
 ACK

thanks,
pushed to master.

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel