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

[Martin Kosek]

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

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

1) Uncatched exception

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

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

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

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

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


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

Martin