[Freeipa-devel] [PATCH 0141] ipatests: Check for legacy_client attribute presence

2014-01-20 Thread Tomas Babej
Hi,

When legacy client tests fail during IPA installation, the legacy
client test produces an additional misleading error
(the real cause is reported as well). This happens due the fact
that we try to cleanup host that was not yet defined. We need to
check for this attribute being defined before unapplying fixes there.

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

From 1b4d06b1b26a1ddbecb1f458fc35e3f600f67d0b Mon Sep 17 00:00:00 2001
From: Tomas Babej tba...@redhat.com
Date: Mon, 20 Jan 2014 09:28:26 +0100
Subject: [PATCH] ipatests: Check for legacy_client attribute presence if
 unapplying fixes

When legacy client tests fail during IPA installation, the legacy
client test produces an additional misleading error
(the real cause is reported as well). This happens due the fact
that we try to cleanup host that was not yet defined. We need to
check for this attribute being defined before unapplying fixes there.

https://fedorahosted.org/freeipa/ticket/4124
---
 ipatests/test_integration/test_legacy_clients.py | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/ipatests/test_integration/test_legacy_clients.py b/ipatests/test_integration/test_legacy_clients.py
index 72b7ff4927a16bb914e96b5d5f64cc6da35ba98c..6df5bea2675c10dcc91840f3a4b33b8afaa991e9 100644
--- a/ipatests/test_integration/test_legacy_clients.py
+++ b/ipatests/test_integration/test_legacy_clients.py
@@ -233,7 +233,11 @@ class BaseTestLegacyClient(trust_tests.TestEnforcedPosixADTrust):
 def uninstall(cls):
 cls.master.run_command(['ipa', 'user-del', 'disabledipauser'],
 raiseonerr=False)
-tasks.unapply_fixes(cls.legacy_client)
+
+# Also unapply fixes on the legacy client, if defined
+if hasattr(cls, legacy_client):
+tasks.unapply_fixes(cls.legacy_client)
+
 super(BaseTestLegacyClient, cls).uninstall()
 
 
-- 
1.8.4.2

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

[Freeipa-devel] [PATCH 0142] ipatests: Remove sudo calls from tasks

2014-01-20 Thread Tomas Babej
Hi,

Sudo calls are not necessary since we log in as a root. Additionally,
sudo requires tty in default configuration, which is not acquired
when using OpenSSH transport.

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

Tomas
From 39d032af375dde5a76da44821bc59eda7ed1fcc4 Mon Sep 17 00:00:00 2001
From: Tomas Babej tba...@redhat.com
Date: Mon, 20 Jan 2014 09:41:32 +0100
Subject: [PATCH] ipatests: Remove sudo calls from tasks

Sudo calls are not necessary since we log in as a root. Additionally,
sudo requires tty in default configuration, which is not acquired
when using OpenSSH transport.

https://fedorahosted.org/freeipa/ticket/4125
---
 ipatests/test_integration/tasks.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ipatests/test_integration/tasks.py b/ipatests/test_integration/tasks.py
index cee54768312c9cf0d79b9137f134af718b0a3b5f..72196914f6d27cd46dd84eef15b3d1dd60aacdf3 100644
--- a/ipatests/test_integration/tasks.py
+++ b/ipatests/test_integration/tasks.py
@@ -412,8 +412,8 @@ def sync_time(host, server):
 leaves ntpd stopped.
 
 
-host.run_command(['sudo', 'systemctl', 'stop', 'ntpd'])
-host.run_command(['sudo', 'ntpdate', server.hostname])
+host.run_command(['systemctl', 'stop', 'ntpd'])
+host.run_command(['ntpdate', server.hostname])
 
 
 def connect_replica(master, replica):
-- 
1.8.4.2

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

Re: [Freeipa-devel] [PATCH] 0135 resolve SIDs to names in group-show for external members

2014-01-20 Thread Martin Kosek
On 01/17/2014 01:26 PM, Sumit Bose wrote:
 On Fri, Jan 17, 2014 at 01:02:18PM +0100, Petr Vobornik wrote:
 On 17.1.2014 12:27, Sumit Bose wrote:
 On Fri, Jan 17, 2014 at 12:09:03PM +0100, Martin Kosek wrote:
 On 01/17/2014 11:50 AM, Sumit Bose wrote:
 On Fri, Jan 17, 2014 at 11:49:18AM +0200, Alexander Bokovoy wrote:
 On Thu, 16 Jan 2014, Alexander Bokovoy wrote:
 Hi,

 when group contains external members, they are specified using SIDs. Use
 trust-resolve command to convert them back on group-show.

 https://bugzilla.redhat.com/show_bug.cgi?id=1054391
 Sumit found omission on name translation. New patch is attached.

 --
 / Alexander Bokovoy

 Patch now works as expected and python code looks good to me, so ACK.
 It would be nice if anyone else can check the python code before
 committing the patch.

 bye,
 Sumit

 Sumit, did you also test Web UI? We should check how it works there, we 
 may no
 longer need to call trust-resolve internally there given it was changed on
 server side.

 If not, Petr1 plans to check that now.

 sorry, no, I didn't check it.

 bye,
 Sumit


 Martin

 On my test system trust-resolve command is somehow broken. It
 doesn't return any names; therefore I was not able to test
 Alexander's patch properly.

 Anyway, attached patch removes the functionality from Web UI.
 
 WebUI still translates the SIDs here, so ACK.
 
 bye,
 Sumit

Thanks. Pushed both Web UI and Alexander's group-show patch to master, ipa-3-3
(I had to rebase Petr's patch there a little).

Martin

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


Re: [Freeipa-devel] [PATCH 0141] ipatests: Check for legacy_client attribute presence

2014-01-20 Thread Tomas Babej
Yep, 100% bug coverage.

Updated patch attached.

On 01/20/2014 09:33 AM, Tomas Babej wrote:
 Hi,

 When legacy client tests fail during IPA installation, the legacy
 client test produces an additional misleading error
 (the real cause is reported as well). This happens due the fact
 that we try to cleanup host that was not yet defined. We need to
 check for this attribute being defined before unapplying fixes there.

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



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

From b3d105a30b671173532f5b9d6e9b55c4250f545d Mon Sep 17 00:00:00 2001
From: Tomas Babej tba...@redhat.com
Date: Mon, 20 Jan 2014 09:28:26 +0100
Subject: [PATCH] ipatests: Check for legacy_client attribute presence if
 unapplying fixes

When legacy client tests fail during IPA installation, the legacy
client test produces an additional misleading error
(the real cause is reported as well). This happens due the fact
that we try to cleanup host that was not yet defined. We need to
check for this attribute being defined before unapplying fixes there.

https://fedorahosted.org/freeipa/ticket/4124
---
 ipatests/test_integration/test_legacy_clients.py | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/ipatests/test_integration/test_legacy_clients.py b/ipatests/test_integration/test_legacy_clients.py
index 72b7ff4927a16bb914e96b5d5f64cc6da35ba98c..6bbe54b32778b9185d523b00c9c87fd76bd15d53 100644
--- a/ipatests/test_integration/test_legacy_clients.py
+++ b/ipatests/test_integration/test_legacy_clients.py
@@ -233,7 +233,11 @@ class BaseTestLegacyClient(trust_tests.TestEnforcedPosixADTrust):
 def uninstall(cls):
 cls.master.run_command(['ipa', 'user-del', 'disabledipauser'],
 raiseonerr=False)
-tasks.unapply_fixes(cls.legacy_client)
+
+# Also unapply fixes on the legacy client, if defined
+if hasattr(cls, 'legacy_client'):
+tasks.unapply_fixes(cls.legacy_client)
+
 super(BaseTestLegacyClient, cls).uninstall()
 
 
-- 
1.8.4.2

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

Re: [Freeipa-devel] [PATCH] 0136 ipa-adtrust-install configure host netbios name by default

2014-01-20 Thread Martin Kosek
On 01/17/2014 01:13 PM, Alexander Bokovoy wrote:
 https://fedorahosted.org/freeipa/ticket/4116

Works fine. ACK, pushed to master, ipa-3-3.

Martin

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


Re: [Freeipa-devel] [PATCH] 210 Allow SAN in IPA certificate profile

2014-01-20 Thread Jan Cholasta

On 17.1.2014 11:39, Jan Cholasta wrote:

On 10.1.2014 13:34, Martin Kosek wrote:

On 01/09/2014 04:49 PM, Simo Sorce wrote:

On Thu, 2014-01-09 at 10:44 -0500, Rob Crittenden wrote:

Martin Kosek wrote:

On 01/09/2014 03:12 PM, Simo Sorce wrote:



Also maybe we should allow admins to bypass the need to have an
actual
object to represent the alt name ?


I'd rather not. This would allow a rogue admin to create a cert for
www.google.com. Sure, they could also create a host for that but
forcing
them to add more entries increases the chances of them getting caught
doing it.


They can remove the host right after they create a cert, I honestly do
not think this is a valid concern. If your admin is rouge he can already
take full ownership of your infrastructure in many ways, preventing
setting a name in a cert doesn't really make a difference IMO.

However I would be ok to limit this to some new Security Admin/CA
Admin role that is not assigned by default.

Simo.



Ok, let's reach some conclusion here. I would really like to not defer
this
feature for too long, it is quite wanted. Would creating new virtual
operation
Request certificate with SAN make the situation better? It would not
be so
difficult to do, the check_access function can already access virtual
operation
name as a parameter, we just need to call it.


Why don't we treat SAN hostnames the same way as the subject hostname?
The way I see it, with SAN the only difference is that there is a set of
hostnames instead of just a single hostname, so maybe we should support
requesting a certificate for a set of hosts/services instead of just a
single host/service.

As far as authorization is concerned, currently you can request a
certificate for a single host/service, if you have the Request
certificate permission and write access to the host/service entry. With
multiple hosts/services, you would be able to request a certificate if
you have the Request certificate permission and write access to *all*
of the host/certificate entries you are requesting the certificate for.

Effectively this means that cert-request would accept multiple
principals instead of single principal and the automatic revocation code
in cert-request, host-del and service-del would take into account that a
single certificate might be assigned to multiple entities.



See attachment for patch which implements the above design.

--
Jan Cholasta
From bb95b3afd6786adc04aa4cd5ac114fbace964907 Mon Sep 17 00:00:00 2001
From: Jan Cholasta jchol...@redhat.com
Date: Mon, 20 Jan 2014 10:59:08 +0100
Subject: [PATCH] Support SAN in cert-request.

https://fedorahosted.org/freeipa/ticket/3977
---
 API.txt|   2 +-
 VERSION|   3 +-
 ipalib/plugins/cert.py | 191 +++--
 3 files changed, 107 insertions(+), 89 deletions(-)

diff --git a/API.txt b/API.txt
index a6c3aed..015d829 100644
--- a/API.txt
+++ b/API.txt
@@ -479,7 +479,7 @@ command: cert_request
 args: 1,4,1
 arg: File('csr', cli_name='csr_file')
 option: Flag('add', autofill=True, default=False)
-option: Str('principal')
+option: Str('principal+')
 option: Str('request_type', autofill=True, default=u'pkcs10')
 option: Str('version?', exclude='webui')
 output: Output('result', type 'dict', None)
diff --git a/VERSION b/VERSION
index 5ce16b5..c9f06d1 100644
--- a/VERSION
+++ b/VERSION
@@ -89,4 +89,5 @@ IPA_DATA_VERSION=2010061412
 #  #
 
 IPA_API_VERSION_MAJOR=2
-IPA_API_VERSION_MINOR=72
+IPA_API_VERSION_MINOR=73
+# Last change: jcholast - SAN certificate requests
diff --git a/ipalib/plugins/cert.py b/ipalib/plugins/cert.py
index 762f13b..d343f99 100644
--- a/ipalib/plugins/cert.py
+++ b/ipalib/plugins/cert.py
@@ -249,7 +249,7 @@ class cert_request(VirtualCommand):
 operation=request certificate
 
 takes_options = (
-Str('principal',
+Str('principal+',
 label=_('Principal'),
 doc=_('Service principal for this certificate (e.g. HTTP/test.example.com)'),
 ),
@@ -301,13 +301,7 @@ class cert_request(VirtualCommand):
 ),
 )
 
-def execute(self, csr, **kw):
-ldap = self.api.Backend.ldap2
-principal = kw.get('principal')
-add = kw.get('add')
-request_type = kw.get('request_type')
-service = None
-
+def execute(self, csr, **options):
 
 Access control is partially handled by the ACI titled
 'Hosts can modify service userCertificate'. This is for the case
@@ -324,73 +318,101 @@ class cert_request(VirtualCommand):
 if not bind_principal.startswith('host/'):
 self.check_access()
 
-# FIXME: add support for subject alt name
-
-# Ensure that the hostname in the CSR matches the principal
-subject_host = get_csr_hostname(csr)
-if not subject_host:
-raise 

Re: [Freeipa-devel] [PATCHES] 225-230 Drop support for the legacy LDAP API

2014-01-20 Thread Petr Viktorin

On 01/14/2014 11:31 AM, Jan Cholasta wrote:

On 10.1.2014 16:02, Petr Viktorin wrote:

On 01/07/2014 01:54 PM, Jan Cholasta wrote:

On 16.12.2013 14:45, Petr Viktorin wrote:

On 12/16/2013 10:22 AM, Jan Cholasta wrote:

On 13.12.2013 15:16, Petr Viktorin wrote:

On 12/10/2013 04:05 PM, Jan Cholasta wrote:

Hi,

I believe the time has come to drop support for the legacy (dn,
entry_attrs) tuple API and move to the new LDAPEntry API
exclusively.
The attached patches convert existing code which uses the old API to
the
new API and remove backward compatibility code from the ipaldap
module.

Note that there are still some functions/methods which accept
separate
dn and entry_attrs arguments, they will be adapted to LDAPEntry
later.

Honza


The first N-1 patches can be tested,acked,pushed independently,
right?


Yes.


If that's the case, ACK for 225


Pushed that one to master, 5 more to go.
bc3f3381c6bf0b4941889b775025a60f56318551



226 needs a rebase.

227: in install/tools/ipa-adtrust-install:

+entry_attrs = conn.make_entry(
+dn,
+objectclass=['top', 'pkiuser', 'nscontainer'],
+usercertificate=cert)
+conn.add_entry(entry_attrs)

Shouldn't it be `usercertificate=[cert]` now?  Similarly in ra_cert, and
in ipa-server-install with ipacertificatesubjectbase

Otherwise this looks good.

228: in ipaserver/install/plugins/update_idranges.py, again we should
use lists

Otherwise it looks good

229: ACK



Rebased and fixed everything, updated patches attached.


Here, patch 226 breaks tests for selinuxusermap_enable/disable, at 
least. The EmptyModlist and AlreadyActive/AlreadyInactive error is no 
longer raised, since the previous entry state is no longer retrieved.


--
Petr³

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


Re: [Freeipa-devel] [PATCH 0141] ipatests: Check for legacy_client attribute presence

2014-01-20 Thread Petr Viktorin

On 01/20/2014 10:31 AM, Tomas Babej wrote:

Yep, 100% bug coverage.

Updated patch attached.

On 01/20/2014 09:33 AM, Tomas Babej wrote:

Hi,

When legacy client tests fail during IPA installation, the legacy
client test produces an additional misleading error
(the real cause is reported as well). This happens due the fact
that we try to cleanup host that was not yet defined. We need to
check for this attribute being defined before unapplying fixes there.

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


This and 0141 look good, but I don't yet have the setup to test them -- 
after all these tests are still being developed.

If they pass in your environment we should push as one-liners.

--
Petr³

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


Re: [Freeipa-devel] [PATCH 0142] ipatests: Remove sudo calls from tasks

2014-01-20 Thread Petr Viktorin

On 01/20/2014 09:43 AM, Tomas Babej wrote:

Hi,

Sudo calls are not necessary since we log in as a root. Additionally,
sudo requires tty in default configuration, which is not acquired
when using OpenSSH transport.

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

Tomas


ACK, pushed to:
master: 5403648afd7dde69bf774f5dbdd2d07873ec3f84
ipa-3-3: dc1a1189e18c5d579570506bdf839f18aa94db6b


--
Petr³

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


Re: [Freeipa-devel] [PATCH 0141] ipatests: Check for legacy_client attribute presence

2014-01-20 Thread Petr Viktorin

On 01/20/2014 02:22 PM, Petr Viktorin wrote:

On 01/20/2014 10:31 AM, Tomas Babej wrote:

Yep, 100% bug coverage.

Updated patch attached.

On 01/20/2014 09:33 AM, Tomas Babej wrote:

Hi,

When legacy client tests fail during IPA installation, the legacy
client test produces an additional misleading error
(the real cause is reported as well). This happens due the fact
that we try to cleanup host that was not yet defined. We need to
check for this attribute being defined before unapplying fixes there.

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


This and 0141 look good, but I don't yet have the setup to test them --
after all these tests are still being developed.
If they pass in your environment we should push as one-liners.


Pushed to:
master: 2adfaa3a9bd94707d9cc78f9fb6bd85d53477db2
ipa-3-3: cfaaeb9dadbb30cea0b1306f065c485d2dd82974


--
Petr³

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


Re: [Freeipa-devel] [PATCH] 0137 ipasam: remove child domains before removing trust

2014-01-20 Thread Martin Kosek
On 01/20/2014 03:49 PM, Alexander Bokovoy wrote:
 Hi!
 
 Make sure we delete child domains before removing the trust itself as
 LDAP protocol does not allow removing non-leaf objects.
 
 This has non-obvious effect -- old code did remove cross-realm
 principals and then removed trust object. However, for trusts with child
 domains the trust domain object was not removed as LDAP server prevents
 removing non-leaf objects. It resulted in the object still existing but
 cross-realm principals missing. The trust is thus non-functioning. This
 situation can be triggered with a second 'ipa trust-add' call.
 
 Fix the code by removing child domains first and then remove the forest
 root trusted domain object.
 
 https://fedorahosted.org/freeipa/ticket/4126

Thanks for the patch! I did not test, I am just thinking about this search:

+
+   rc = smbldap_search(ldap_state-smbldap_state, dn, scope, filter, NULL, 
0,
result);
+   TALLOC_FREE(filter);
+

 - shouldn't you search with SCOPE_ONELEVEL given we do not dive deeper anyway?
- shouldn't we search with filter (objectclass=ipaNTTrustedDomain) just to
make sure we do not delete anything we do not want to be deleted? For example
if the function gets a wrong DN, we may want to make sure we don't delete the
whole DIT

Additionally, I think we should add few DEBUG messages, so that in debug log we
see we are doing this deletion.

Martin

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


Re: [Freeipa-devel] [PATCH] 0137 ipasam: remove child domains before removing trust

2014-01-20 Thread Alexander Bokovoy

On Mon, 20 Jan 2014, Martin Kosek wrote:

On 01/20/2014 03:49 PM, Alexander Bokovoy wrote:

Hi!

Make sure we delete child domains before removing the trust itself as
LDAP protocol does not allow removing non-leaf objects.

This has non-obvious effect -- old code did remove cross-realm
principals and then removed trust object. However, for trusts with child
domains the trust domain object was not removed as LDAP server prevents
removing non-leaf objects. It resulted in the object still existing but
cross-realm principals missing. The trust is thus non-functioning. This
situation can be triggered with a second 'ipa trust-add' call.

Fix the code by removing child domains first and then remove the forest
root trusted domain object.

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


Thanks for the patch! I did not test, I am just thinking about this search:

+
+   rc = smbldap_search(ldap_state-smbldap_state, dn, scope, filter, NULL, 
0,
result);
+   TALLOC_FREE(filter);
+

- shouldn't you search with SCOPE_ONELEVEL given we do not dive deeper anyway?

No. We need to remove dn but to remove it we need to remove everything
under it. Thus, we don't care what is there, since whole dn
(cn=TRUSTNAME,cn=ad,cn=trusts,$SUFFIX) will be deleted anyway.


- shouldn't we search with filter (objectclass=ipaNTTrustedDomain) just to
make sure we do not delete anything we do not want to be deleted? For example
if the function gets a wrong DN, we may want to make sure we don't delete the
whole DIT

We should delete everything under 'dn' which is 
cn=TRUSTNAME,cn=ad,cn=trusts,$SUFFIX


Additionally, I think we should add few DEBUG messages, so that in debug log we
see we are doing this deletion.

We'll see them at level 5 anyway because of smbldap_delete():
[2014/01/20 17:14:02.965144,  5, pid=5111, effective(87440, 87440), 
real(87440, 0)] ../source3/lib/smbldap.c:1535(smbldap_delete)
  smbldap_delete: dn = 
[cn=ad12y.ad12x.weald.vda.li,cn=ad12x.weald.vda.li,cn=ad,cn=trusts,dc=ipa,dc=weald,dc=vda,dc=li]
[2014/01/20 17:14:03.034982,  5, pid=5111, effective(87440, 87440), 
real(87440, 0)] ../source3/lib/smbldap.c:1535(smbldap_delete)
  smbldap_delete: dn = 
[cn=ad12x.weald.vda.li,cn=ad,cn=trusts,dc=ipa,dc=weald,dc=vda,dc=li]

I don't think we need to add more.
--
/ Alexander Bokovoy

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


[Freeipa-devel] [Trusts] Admin enforcing POSIX range when it's not being detected

2014-01-20 Thread Tomas Babej
Hey!

Let us discuss a which behaviour we should take with trust-add command.

Currently, if you run:

$ ipa trust-add --type ad host
Range type (POSIX or non-POSIX) is being detected automatically.

However, if you run:

$ ipa trust-add --type ad host --range-type=ipa-ad-trust-posix
You override the detection of the SFU support on the AD. This is not a
problem
when you have a AD with POSIX support, and you try to enforce a
non-posix range type.

However, if you *think* you have SFU up and running, but you don't (or
we just can't
access the information for whatever reason), you end up enforcing POSIX
range type
while not defining any of the expected attributes.

Currently, not defining base_id simply means you will have one generated
from SID.
So you end up with a posix range like this one:

 Range name: ADPOSIX.QE_id_range
 First Posix ID of the range: *28040*
 Number of IDs in the range: 20
 First RID of the corresponding RID range: 0
 Domain SID of the trusted domain: S-1-5-21-365534-3880942204-3419777279
 Range type: Active Directory trust range *with POSIX attributes*

The question is, what position should we take?

1.) Are we going to stick with the defaults estabilished by AD?
(base_id: 1)
2.) Or are we going to bail out in this case and report an error?

Tomas

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


Re: [Freeipa-devel] [PATCH] 210 Allow SAN in IPA certificate profile

2014-01-20 Thread Simo Sorce
On Mon, 2014-01-20 at 11:07 +0100, Jan Cholasta wrote:
 On 17.1.2014 11:39, Jan Cholasta wrote:
  On 10.1.2014 13:34, Martin Kosek wrote:
  On 01/09/2014 04:49 PM, Simo Sorce wrote:
  On Thu, 2014-01-09 at 10:44 -0500, Rob Crittenden wrote:
  Martin Kosek wrote:
  On 01/09/2014 03:12 PM, Simo Sorce wrote:
 
  Also maybe we should allow admins to bypass the need to have an
  actual
  object to represent the alt name ?
 
  I'd rather not. This would allow a rogue admin to create a cert for
  www.google.com. Sure, they could also create a host for that but
  forcing
  them to add more entries increases the chances of them getting caught
  doing it.
 
  They can remove the host right after they create a cert, I honestly do
  not think this is a valid concern. If your admin is rouge he can already
  take full ownership of your infrastructure in many ways, preventing
  setting a name in a cert doesn't really make a difference IMO.
 
  However I would be ok to limit this to some new Security Admin/CA
  Admin role that is not assigned by default.
 
  Simo.
 
 
  Ok, let's reach some conclusion here. I would really like to not defer
  this
  feature for too long, it is quite wanted. Would creating new virtual
  operation
  Request certificate with SAN make the situation better? It would not
  be so
  difficult to do, the check_access function can already access virtual
  operation
  name as a parameter, we just need to call it.
 
  Why don't we treat SAN hostnames the same way as the subject hostname?
  The way I see it, with SAN the only difference is that there is a set of
  hostnames instead of just a single hostname, so maybe we should support
  requesting a certificate for a set of hosts/services instead of just a
  single host/service.
 
  As far as authorization is concerned, currently you can request a
  certificate for a single host/service, if you have the Request
  certificate permission and write access to the host/service entry. With
  multiple hosts/services, you would be able to request a certificate if
  you have the Request certificate permission and write access to *all*
  of the host/certificate entries you are requesting the certificate for.
 
  Effectively this means that cert-request would accept multiple
  principals instead of single principal and the automatic revocation code
  in cert-request, host-del and service-del would take into account that a
  single certificate might be assigned to multiple entities.
 
 
 See attachment for patch which implements the above design.

Hi Jan, I am a bit too far removed from the code to fully understand the
change just from the diff.

Could you please add a short explanation in the commit message, a bout
what the code does now differently than it did before.
For example although I understand the checks you do on subjectname
+subjectaltname I do not know where the principals come from or why you
have a comment that says principals must all be of the same service
type.

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] 0137 ipasam: remove child domains before removing trust

2014-01-20 Thread Sumit Bose
On Mon, Jan 20, 2014 at 05:18:30PM +0200, Alexander Bokovoy wrote:
 On Mon, 20 Jan 2014, Martin Kosek wrote:
 On 01/20/2014 03:49 PM, Alexander Bokovoy wrote:
 Hi!
 
 Make sure we delete child domains before removing the trust itself as
 LDAP protocol does not allow removing non-leaf objects.
 
 This has non-obvious effect -- old code did remove cross-realm
 principals and then removed trust object. However, for trusts with child
 domains the trust domain object was not removed as LDAP server prevents
 removing non-leaf objects. It resulted in the object still existing but
 cross-realm principals missing. The trust is thus non-functioning. This
 situation can be triggered with a second 'ipa trust-add' call.
 
 Fix the code by removing child domains first and then remove the forest
 root trusted domain object.
 
 https://fedorahosted.org/freeipa/ticket/4126
 
 Thanks for the patch! I did not test, I am just thinking about this search:
 
 +
 +rc = smbldap_search(ldap_state-smbldap_state, dn, scope, filter, NULL, 
 0,
 result);
 +TALLOC_FREE(filter);
 +
 
 - shouldn't you search with SCOPE_ONELEVEL given we do not dive deeper 
 anyway?
 No. We need to remove dn but to remove it we need to remove everything
 under it. Thus, we don't care what is there, since whole dn
 (cn=TRUSTNAME,cn=ad,cn=trusts,$SUFFIX) will be deleted anyway.
 
 - shouldn't we search with filter (objectclass=ipaNTTrustedDomain) just to
 make sure we do not delete anything we do not want to be deleted? For example
 if the function gets a wrong DN, we may want to make sure we don't delete the
 whole DIT
 We should delete everything under 'dn' which is 
 cn=TRUSTNAME,cn=ad,cn=trusts,$SUFFIX

The user samba uses to bind to LDAP should not have the privileges to
remove the whole tree. So in the worst case it will just remove all
trusts.

I agree with Alexander that we should remove everything below the DN of
the forest root. The original idea behind putting the other domains in
the forest below was that be removing the sub-tree all information about
the trust was gone (except for the idranges, but that's a different
story).

bye,
Sumit

 
 Additionally, I think we should add few DEBUG messages, so that in debug log 
 we
 see we are doing this deletion.
 We'll see them at level 5 anyway because of smbldap_delete():
 [2014/01/20 17:14:02.965144,  5, pid=5111, effective(87440, 87440), 
 real(87440, 0)] ../source3/lib/smbldap.c:1535(smbldap_delete)
   smbldap_delete: dn = 
 [cn=ad12y.ad12x.weald.vda.li,cn=ad12x.weald.vda.li,cn=ad,cn=trusts,dc=ipa,dc=weald,dc=vda,dc=li]
 [2014/01/20 17:14:03.034982,  5, pid=5111, effective(87440, 87440), 
 real(87440, 0)] ../source3/lib/smbldap.c:1535(smbldap_delete)
   smbldap_delete: dn = 
 [cn=ad12x.weald.vda.li,cn=ad,cn=trusts,dc=ipa,dc=weald,dc=vda,dc=li]
 
 I don't think we need to add more.
 -- 
 / Alexander Bokovoy
 
 ___
 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] 0137 ipasam: remove child domains before removing trust

2014-01-20 Thread Martin Kosek
On 01/20/2014 04:42 PM, Sumit Bose wrote:
 On Mon, Jan 20, 2014 at 05:18:30PM +0200, Alexander Bokovoy wrote:
 On Mon, 20 Jan 2014, Martin Kosek wrote:
 On 01/20/2014 03:49 PM, Alexander Bokovoy wrote:
 Hi!

 Make sure we delete child domains before removing the trust itself as
 LDAP protocol does not allow removing non-leaf objects.

 This has non-obvious effect -- old code did remove cross-realm
 principals and then removed trust object. However, for trusts with child
 domains the trust domain object was not removed as LDAP server prevents
 removing non-leaf objects. It resulted in the object still existing but
 cross-realm principals missing. The trust is thus non-functioning. This
 situation can be triggered with a second 'ipa trust-add' call.

 Fix the code by removing child domains first and then remove the forest
 root trusted domain object.

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

 Thanks for the patch! I did not test, I am just thinking about this search:

 +
 +   rc = smbldap_search(ldap_state-smbldap_state, dn, scope, filter, NULL, 
 0,
 result);
 +   TALLOC_FREE(filter);
 +

 - shouldn't you search with SCOPE_ONELEVEL given we do not dive deeper 
 anyway?
 No. We need to remove dn but to remove it we need to remove everything
 under it. Thus, we don't care what is there, since whole dn
 (cn=TRUSTNAME,cn=ad,cn=trusts,$SUFFIX) will be deleted anyway.

 - shouldn't we search with filter (objectclass=ipaNTTrustedDomain) just to
 make sure we do not delete anything we do not want to be deleted? For 
 example
 if the function gets a wrong DN, we may want to make sure we don't delete 
 the
 whole DIT
 We should delete everything under 'dn' which is 
 cn=TRUSTNAME,cn=ad,cn=trusts,$SUFFIX
 
 The user samba uses to bind to LDAP should not have the privileges to
 remove the whole tree. So in the worst case it will just remove all
 trusts.
 
 I agree with Alexander that we should remove everything below the DN of
 the forest root. The original idea behind putting the other domains in
 the forest below was that be removing the sub-tree all information about
 the trust was gone (except for the idranges, but that's a different
 story).
 
 bye,
 Sumit

Ok, makes sense. I have no conceptual objection then.

Thanks,
Martin

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


Re: [Freeipa-devel] [PATCH] 451 Hide trust-resolve command

2014-01-20 Thread Alexander Bokovoy

On Fri, 17 Jan 2014, Martin Kosek wrote:

We do not need to expose a public FreeIPA specific interface to resolve
SIDs to names. The interface is only used internally to resolve SIDs
when external group members are listed. Additionally, the command interface
is not prepared for regular user and can give rather confusing results.

Hide it from CLI. The API itself is still accessible and compatible with
older clients.

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

ACK, works fine for me. Web UI shows external members resolved.


--
/ Alexander Bokovoy

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


Re: [Freeipa-devel] [PATCH] 210 Allow SAN in IPA certificate profile

2014-01-20 Thread Jan Cholasta

On 20.1.2014 16:36, Simo Sorce wrote:

On Mon, 2014-01-20 at 11:07 +0100, Jan Cholasta wrote:

On 17.1.2014 11:39, Jan Cholasta wrote:

On 10.1.2014 13:34, Martin Kosek wrote:

On 01/09/2014 04:49 PM, Simo Sorce wrote:

On Thu, 2014-01-09 at 10:44 -0500, Rob Crittenden wrote:

Martin Kosek wrote:

On 01/09/2014 03:12 PM, Simo Sorce wrote:



Also maybe we should allow admins to bypass the need to have an
actual
object to represent the alt name ?


I'd rather not. This would allow a rogue admin to create a cert for
www.google.com. Sure, they could also create a host for that but
forcing
them to add more entries increases the chances of them getting caught
doing it.


They can remove the host right after they create a cert, I honestly do
not think this is a valid concern. If your admin is rouge he can already
take full ownership of your infrastructure in many ways, preventing
setting a name in a cert doesn't really make a difference IMO.

However I would be ok to limit this to some new Security Admin/CA
Admin role that is not assigned by default.

Simo.



Ok, let's reach some conclusion here. I would really like to not defer
this
feature for too long, it is quite wanted. Would creating new virtual
operation
Request certificate with SAN make the situation better? It would not
be so
difficult to do, the check_access function can already access virtual
operation
name as a parameter, we just need to call it.


Why don't we treat SAN hostnames the same way as the subject hostname?
The way I see it, with SAN the only difference is that there is a set of
hostnames instead of just a single hostname, so maybe we should support
requesting a certificate for a set of hosts/services instead of just a
single host/service.

As far as authorization is concerned, currently you can request a
certificate for a single host/service, if you have the Request
certificate permission and write access to the host/service entry. With
multiple hosts/services, you would be able to request a certificate if
you have the Request certificate permission and write access to *all*
of the host/certificate entries you are requesting the certificate for.

Effectively this means that cert-request would accept multiple
principals instead of single principal and the automatic revocation code
in cert-request, host-del and service-del would take into account that a
single certificate might be assigned to multiple entities.



See attachment for patch which implements the above design.


Hi Jan, I am a bit too far removed from the code to fully understand the
change just from the diff.

Could you please add a short explanation in the commit message, a bout
what the code does now differently than it did before.


Done.


For example although I understand the checks you do on subjectname
+subjectaltname I do not know where the principals come from or why you
have a comment that says principals must all be of the same service
type.


Principals come from the --principal option, which can have multiple 
values with the patch.


The service name constraint is there so that there is a 1-to-1 mapping 
between principals and hostnames in the request (both subject and SAN). 
Without this you would be able to request a certificate for completely 
unrelated services and I was not sure if it's OK to allow that, since 
the use case here is load balancing (right?)




Simo.




Updated patch attached.

--
Jan Cholasta
From ce8a650b6bd8a3d1c353d31c70fc7edb0af7f1c0 Mon Sep 17 00:00:00 2001
From: Jan Cholasta jchol...@redhat.com
Date: Mon, 20 Jan 2014 10:59:08 +0100
Subject: [PATCH] Support requests with SAN in cert-request.

The command now accepts multiple values for the principal option. This makes
it possible to request a certificate for a service load-balanced on multiple
hosts. Each principal's hostname must match a hostname in the request, either
in the subject or a SAN, and all principals must have the same service name.

https://fedorahosted.org/freeipa/ticket/3977
---
 API.txt|   2 +-
 VERSION|   3 +-
 ipalib/plugins/cert.py | 191 +++--
 3 files changed, 107 insertions(+), 89 deletions(-)

diff --git a/API.txt b/API.txt
index a6c3aed..015d829 100644
--- a/API.txt
+++ b/API.txt
@@ -479,7 +479,7 @@ command: cert_request
 args: 1,4,1
 arg: File('csr', cli_name='csr_file')
 option: Flag('add', autofill=True, default=False)
-option: Str('principal')
+option: Str('principal+')
 option: Str('request_type', autofill=True, default=u'pkcs10')
 option: Str('version?', exclude='webui')
 output: Output('result', type 'dict', None)
diff --git a/VERSION b/VERSION
index 5ce16b5..c9f06d1 100644
--- a/VERSION
+++ b/VERSION
@@ -89,4 +89,5 @@ IPA_DATA_VERSION=2010061412
 #  #
 
 IPA_API_VERSION_MAJOR=2
-IPA_API_VERSION_MINOR=72
+IPA_API_VERSION_MINOR=73
+# Last change: jcholast - SAN 

Re: [Freeipa-devel] [PATCH] 543 Trust domains Web UI

2014-01-20 Thread Alexander Bokovoy

On Fri, 17 Jan 2014, Petr Vobornik wrote:

Note: this version of the patch is especially prepared for ipa-3-3 branch.

Add Web UI counterpart of following CLI commands:

* trust-fetch-domains Refresh list of the domains associated with the trust
* trustdomain-del Remove infromation about the domain associated with 
the trust.
* trustdomain-disable Disable use of IPA resources by the domain of 
the trust

* trustdomain-enable Allow use of IPA resources by the domain of the trust
* trustdomain-find Search domains of the trust

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

ACK functionally, everything works for me.

I wonder if you could make UI a bit smarter and prevent enable/disable
actions for the forest root trusted domain. Right now selecting it for
'disable' will show you an error telling that disabling root domain is
not possible.


--
/ Alexander Bokovoy

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


Re: [Freeipa-devel] [PATCH] 451 Hide trust-resolve command

2014-01-20 Thread Martin Kosek

On 01/20/2014 05:42 PM, Alexander Bokovoy wrote:

On Fri, 17 Jan 2014, Martin Kosek wrote:

We do not need to expose a public FreeIPA specific interface to resolve
SIDs to names. The interface is only used internally to resolve SIDs
when external group members are listed. Additionally, the command interface
is not prepared for regular user and can give rather confusing results.

Hide it from CLI. The API itself is still accessible and compatible with
older clients.

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

ACK, works fine for me. Web UI shows external members resolved.


Pushed to master, ipa-3-3.

Martin

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


Re: [Freeipa-devel] [PATCH] 210 Allow SAN in IPA certificate profile

2014-01-20 Thread Simo Sorce
On Mon, 2014-01-20 at 17:49 +0100, Jan Cholasta wrote:
 On 20.1.2014 16:36, Simo Sorce wrote:
  On Mon, 2014-01-20 at 11:07 +0100, Jan Cholasta wrote:
  On 17.1.2014 11:39, Jan Cholasta wrote:
  On 10.1.2014 13:34, Martin Kosek wrote:
  On 01/09/2014 04:49 PM, Simo Sorce wrote:
  On Thu, 2014-01-09 at 10:44 -0500, Rob Crittenden wrote:
  Martin Kosek wrote:
  On 01/09/2014 03:12 PM, Simo Sorce wrote:
 
  Also maybe we should allow admins to bypass the need to have an
  actual
  object to represent the alt name ?
 
  I'd rather not. This would allow a rogue admin to create a cert for
  www.google.com. Sure, they could also create a host for that but
  forcing
  them to add more entries increases the chances of them getting caught
  doing it.
 
  They can remove the host right after they create a cert, I honestly do
  not think this is a valid concern. If your admin is rouge he can already
  take full ownership of your infrastructure in many ways, preventing
  setting a name in a cert doesn't really make a difference IMO.
 
  However I would be ok to limit this to some new Security Admin/CA
  Admin role that is not assigned by default.
 
  Simo.
 
 
  Ok, let's reach some conclusion here. I would really like to not defer
  this
  feature for too long, it is quite wanted. Would creating new virtual
  operation
  Request certificate with SAN make the situation better? It would not
  be so
  difficult to do, the check_access function can already access virtual
  operation
  name as a parameter, we just need to call it.
 
  Why don't we treat SAN hostnames the same way as the subject hostname?
  The way I see it, with SAN the only difference is that there is a set of
  hostnames instead of just a single hostname, so maybe we should support
  requesting a certificate for a set of hosts/services instead of just a
  single host/service.
 
  As far as authorization is concerned, currently you can request a
  certificate for a single host/service, if you have the Request
  certificate permission and write access to the host/service entry. With
  multiple hosts/services, you would be able to request a certificate if
  you have the Request certificate permission and write access to *all*
  of the host/certificate entries you are requesting the certificate for.
 
  Effectively this means that cert-request would accept multiple
  principals instead of single principal and the automatic revocation code
  in cert-request, host-del and service-del would take into account that a
  single certificate might be assigned to multiple entities.
 
 
  See attachment for patch which implements the above design.
 
  Hi Jan, I am a bit too far removed from the code to fully understand the
  change just from the diff.
 
  Could you please add a short explanation in the commit message, a bout
  what the code does now differently than it did before.
 
 Done.
 
  For example although I understand the checks you do on subjectname
  +subjectaltname I do not know where the principals come from or why you
  have a comment that says principals must all be of the same service
  type.
 
 Principals come from the --principal option, which can have multiple 
 values with the patch.
 
 The service name constraint is there so that there is a 1-to-1 mapping 
 between principals and hostnames in the request (both subject and SAN). 
 Without this you would be able to request a certificate for completely 
 unrelated services and I was not sure if it's OK to allow that, since 
 the use case here is load balancing (right?)
 
 
  Simo.
 
 
 
 Updated patch attached.
 

Sorry have to NACK.

I was confused by the code so asked on IRC:
simo so if I have subectname = one.ipa.lan and altname = two.ipa.net
then the certificate is anchored to both HTTP/one.ipa.net  AND
HTTP/two.ipa.net ?
jcholast yep
simo and what happens when I create other.ipa.net with altname
two.ipa.net ?
simo does HTTP/two.ipa.net now have 2 certificates anchored ?
jcholast the old certificate gets revoked and removed from
HTTP/one.ipa.net and HTTP/two.ipa.net, then the new certificate is
issued and anchored to HTTP/two.ipa.net and HTTP/other.ipa.net

This defeats the purpose of having altnames.

There are 2 reasons to have altnames:
1. just add aliases that are not shared by any other service
2. have a common alias between multiple service to allow loadbalancing

(1) will not be affected, but (2) would be impossible, because as soon
as I try to create the cert for one of the other nodes to be balanced
the previous nodes get their certificates revoked, which defeats the
whole point of creating them with a common altname.

Simo.

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

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