[Freeipa-devel] [PATCH 0141] ipatests: Check for legacy_client attribute presence
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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