On 06/28/2012 01:09 PM, Martin Kosek wrote:
> On 06/28/2012 12:19 PM, Sumit Bose wrote:
>> On Thu, Jun 28, 2012 at 09:52:14AM +0200, Martin Kosek wrote:
>>> On 06/27/2012 06:38 PM, Alexander Bokovoy wrote:
>>>> On Wed, 27 Jun 2012, Sumit Bose wrote:
>>>>> On Wed, Jun 13, 2012 at 12:37:49PM +0200, Sumit Bose wrote:
>>>>>> On Wed, Jun 13, 2012 at 12:26:43PM +0200, Sumit Bose wrote:
>>>>>>> On Mon, Jun 11, 2012 at 05:46:17PM +0300, Alexander Bokovoy wrote:
>>>>>>>> On Thu, 07 Jun 2012, Sumit Bose wrote:
>>>>>>>>> On Fri, Mar 23, 2012 at 01:52:34PM +0100, Sumit Bose wrote:
>>>>>>>>>> Hi,
>>>>>>>>>>
>>>>>>>>>> these two patches introduce a new extended operation to the IPA
>>>>>>>>>> server
>>>>>>>>>> which can be used by clients in the IPA domain to obtain information
>>>>>>>>>> about users and groups from trusted domains. Currently this exop is
>>>>>>>>>> used
>>>>>>>>>> by the sssd sub-domain patch to map user names from a trusted AD
>>>>>>>>>> domain
>>>>>>>>>> to a SID and back. There is also some code for other kind of requests
>>>>>>>>>> which might become useful in future, e.g. with trusted IPA domain.
>>>>>>>>>>
>>>>>>>>>> I added some unit test and added check for the check unit test
>>>>>>>>>> framework
>>>>>>>>>> for C (http://check.sourceforge.net/) which is used by sssd as well.
>>>>>>>>>> I
>>>>>>>>>> modified the spec file that the test is run during the build of the
>>>>>>>>>> packages. I hope this is ok.
>>>>>>>>>>
>>>>>>>>>> The patches depend on the idmap library patch which was ACKed
>>>>>>>>>> recently
>>>>>>>>>> on sssd-devel and as mentioned before the sub-domain patches on
>>>>>>>>>> sssd-devel can only be fully tested with an IPA server which has
>>>>>>>>>> these
>>>>>>>>>> patches applied.
>>>>>>>>>>
>>>>>>>>>> Since Alexander is currently rewriting parts of the
>>>>>>>>>> ipa-adtrust-install
>>>>>>>>>> utility I stand back from adding activation code for the exop to
>>>>>>>>>> ipa-adtrust-install and will send a patch when Alexander's changes
>>>>>>>>>> are
>>>>>>>>>> available. So currently extdom-extop-conf.ldif has to be loaded
>>>>>>>>>> manually
>>>>>>>>>> after replacing $SUFFIX to activate the new exop.
>>>>>>>>>>
>>>>>>>>>> bye,
>>>>>>>>>> Sumit
>>>>>>>>>
>>>>>>>>> Please find a rebased version of the patches which work on top of
>>>>>>>>> Alexander's latest series of patches. The patches now also contain the
>>>>>>>>> loading of extdom-extop-conf.ldif and the activation of winbind.
>>>>>>>> Thanks for the rebase.
>>>>>>>>
>>>>>>>> Few comments.
>>>>>>>>
>>>>>>>> 1.The extdom plugin should support IDMAP_BOTH. We do provide user
>>>>>>>> private
>>>>>>>> groups so in our case it should be viewed as preferred output. Thus you
>>>>>>>> would need to add new response type to cover this case.
>>>>>>>
>>>>>>> Currently the plugin only uses winbind to map SIDs to names and back and
>>>>>>> in the returned user data the user private groups are already respected
>>>>>>> by setting the GID to the UID. On the client side sssd handles the
>>>>>>> trusted domains a mpg (magic private group) domains.
>>>>>>>
>>>>>>>>
>>>>>>>> 2. I have tried to look at the plugin description from point of view of
>>>>>>>> a system administrator and I failed to understand what it does:
>>>>>>>>> +#define IPA_EXTDOM_PLUGIN_NAME "ipa-extdom-extop"
>>>>>>>>> +#define IPA_EXTDOM_FEATURE_DESC "IPA EXTDOM ID mapper"
>>>>>>>>> +#define IPA_EXTDOM_PLUGIN_DESC "IPA EXTDOM ID mapper Extended
>>>>>> Operation plugin"
>>>>>>>>
>>>>>>>> In the ipa-extdom-extop-conf.ldif you have following description:
>>>>>>>>> +nsslapd-plugindescription: Support resolving IDs in trusted domains
>>>>>>>>> to
>>>>>> names and back
>>>>>>>> Probably it is better to reuse the same description in
>>>>>> IPA_EXTDOM_PLUGIN_DESC?
>>>>>>>>
>>>>>>>> This is a minor point but EXTDOM itself is vague. Maybe we should be
>>>>>> more clear
>>>>>>>> and call it 'IPA trusted domain ID mapper' as it really limits itself
>>>>>>>> to
>>>>>>>> only trusted domains? We don't dispatch winbind request if the domain
>>>>>>>> is
>>>>>>>> not found in our list of trusted domains.
>>>>>>>
>>>>>>> I have updated the descriptions. I prefer the EXTDOM prefix because
>>>>>>> there might be future use cases where we might want to get some data
>>>>>>> from other domains without trust. But I'm happy to change it if you like
>>>>>>> a different prefix better.
>>>>>>>
>>>>>>>>
>>>>>>>> 3. Could you please define the oid in ipa_extdom.h so that it could be
>>>>>>>> useful for client code as well?
>>>>>>>>> +#define EXOP_EXTDOM_OID "2.16.840.1.113730.3.8.10.4"
>>>>>>>
>>>>>>> done
>>>>>>>
>>>>>>> New version attached.
>>>>>>
>>>>>> ah. sorry, forgot to squash in some changes.
>>>>>>
>>>>>> Additionally I moved the binary to the freeipa-server-trust-ad package
>>>>>> to avoid additional dependencies in the freeipa-server package.
>>>>>>
>>>>>> bye,
>>>>>> Sumit
>>>>>>
>>>>>>>
>>>>>>>>
>>>>>>>> 4. Do we have 'check' tool in RHEL6?
>>>>>>>
>>>>>>> yes, current version is check-0.9.8-1.1.el6
>>>>>>>
>>>>>>> Thank you for the review.
>>>>>>>
>>>>>>> bye,
>>>>>>> Sumit
>>>>>>>> --
>>>>>>>> / Alexander Bokovoy
>>>>>
>>>>> rebased version with some changes by Alexander attached.
>>>> ACK from my side. Works in tests I've run.
>>>
>>> Patch 17 pushed to master.
>>>
>>> Patch 18 does not apply. I also have one question related to this patch:
>>
>> a rebased version is attached.
>>
>>>
>>> We added a winbind service to ADTRUSTInstance which is now being configured
>>> as
>>> a part of ipa-adtrust-install. To make this cleaner, we may want to write
>>> winbind's own service.Service class which would do the necessary
>>> configuration
>>> and could be also better expanded in the future.
>>
>> Currently none of the configuration steps are done exclusively for
>> winbind, e.g. winbind will use the same credential as the smbd to access
>> the directory server. I would agree to create an class for winbind if it
>> turns out that we have to add special winbind options, but for now we
>> only need to start the winbind process.
>
> Ok, lets keep current setup and expand when needed. I also did few
> installation
> tests with your patch, everything worked fine.
>
> So ACK #2, pushed to master.
>
> Martin
There was a missing Requires for libsss_idmap on freeipa-server-trust-ad
package.
Fixed and pushed as a one-liner (attached).
Martin
>From 28623cc8bb69c95e87b46f7667d8d7aa0b6e181f Mon Sep 17 00:00:00 2001
From: Martin Kosek <mko...@redhat.com>
Date: Thu, 28 Jun 2012 13:47:27 +0200
Subject: [PATCH] Add missing libsss_idmap Requires on freeipa-server-trust-ad
---
freeipa.spec.in | 1 +
1 file changed, 1 insertion(+)
diff --git a/freeipa.spec.in b/freeipa.spec.in
index 72ac0e7a2ad139aba1fd7b4ecc94f961a6743ab4..f7b115202bc8086ba26b25fbe1848fb4ad1fec2a 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -206,6 +206,7 @@ Requires: %{name}-server = %version-%release
Requires: python-crypto
Requires: samba4-python
Requires: samba4
+Requires: libsss_idmap
%description server-trust-ad
Cross-realm trusts with Active Directory in IPA require working Samba 4 installation.
--
1.7.10.2
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
