[Freeipa-devel] [PATCH] 0052 Add ipa-advise plugins for legacy clients

[Alexander Bokovoy]

On Fri, 02 Aug 2013, Ana Krivokapic wrote:
>On 08/01/2013 04:13 PM, Alexander Bokovoy wrote:
>> Hi!
>>
>> On Thu, 01 Aug 2013, Ana Krivokapic wrote:
>>> Hello,
>
>Thanks Alexander for the quick review!
>
>>>
>>> This patch adds ipa-advise plugins to help configure legacy clients for access
>>> to trusted domain resources. For more details, please read the commit message.
>>> Plugins are currently named "config-redhat-sssd-before-1-9" and
>>> "config-generic-sssd-before-1-9"; suggestions for better names are welcome.
>>>
>>> Plugin content heavily inspired by
>>> https://fedoraproject.org/wiki/QA:Testcase_freeipa_use_legacy_sssd_to_give_access_to_trusted_domain_users.
>>>
>> I think it is a good start. Comments inline.
>>
>>> https://fedorahosted.org/freeipa/ticket/3671
>>> ---
>>> install/share/Makefile.am                  |   2 +
>>> install/share/pam.conf.template            |  22 ++++++
>>> install/share/sssd.conf.template           |  12 +++
>> I would imagine we would have multiple plugins that need their own
>> templates for pam.conf/sssd.conf. What about introducing
>>  to avoid conflicts?
>>
>> In this case you use the same templates for both plugins so you might
>> have <name> as 'legacy', for example.
>>
>> Another way is to have plugin name in the template, e.g.
>> legacy.sssd.conf.template.
>
>Done. I opted for the install/share/advise/<name>/*.template option. The changes
>are in the updated patch 52.
>
>>
>>> +class config_redhat_sssd_before_1_9(Advice):
>>> +    """
>>> +    Legacy client configuration for Red Hat based platforms.
>>> +    """
>>> +
>>> +    description = ('Instructions for configuring a system with an old version '
>>> +                   'of SSSD (1.5-1.8) as a FreeIPA client. This set of '
>>> +                   'instructions is targeted for platforms that include '
>>> +                   'the authconfig utility, which are all Red Hat based '
>>> +                   'platforms.')
>> You need to check that Schema Compatibility plugin is configured to
>> serve trusted domain users and groups.
>>
>> We have two trees:
>> dn: cn=groups,cn=Schema Compatibility,cn=plugins,cn=config
>> dn: cn=users,cn=Schema Compatibility,cn=plugins,cn=config
>>
>> In both of the trees there should be
>>    schema-compat-lookup-sssd: <user|group>
>>
>> attribute, with the value according to the tree (i.e. user for
>> cn=users).
>>
>> If not, then suggest to run 'ipa-adtrust-install --enable-compat=true' on the
>> IPA server.
>
>Done. I added a new API command 'compat-is-enabled' (similar to
>'adtrust-is-enabled') to facilitate checking whether the Schema Compatibility
>plugin is configured. 'compat-is-enabled' is called from the ipa-advise plugin
>and the suggestion to run 'ipa-adtrust-install --enable-compat' is printed as
>the first piece of advice, when appropriate.
>
>Patch 54 adds the new API command 'compat-is-enabled', while patch 53 is a small
>fix which enables IPA API commands to be run from the ipa-advise plugins.
>
>>
>>> +
>>> +    def get_info(self):
>>> +        self.log.comment('Install the sssd and authconfig packages via yum')
>>> +        self.log.command('yum install -y sssd authconfig\n')
>> You are using 'wget' below, it might make sense to add it into the above
>> line too.
>
>Fixed in patch 52.
>
>>
>>> +
>>> +        self.log.comment('Download the CA certificate of the IPA server')
>>> +        self.log.command('mkdir -p -m 755 /etc/openldap/cacerts')
>>> +        self.log.command('wget http://%s/ipa/config/ca.crt -O '
>>> +                         '/etc/openldap/cacerts/ipa.crt\n' % api.env.host)
>>> +
>>> +        self.log.comment('Generate hashes for the openldap library')
>>> +        self.log.command('cacertdir_rehash /etc/openldap/cacerts/\n')
>>> +
>>> +        self.log.comment('Use the authconfig to configure nsswitch.conf '
>>> +                         'and the PAM stack')
>>> +        self.log.command('authconfig --updateall --enablesssd '
>>> +                         '--enablesssdauth\n')
>>> +
>>> +        self.log.comment('Configure SSSD')
>>> +        self.log.command('cat > /etc/sssd/sssd.conf << EOF \n'
>>> +                         '%s\nEOF' % generate_sssd_conf())
>>> +        self.log.command('chmod 0600 /etc/sssd/sssd.conf\n')
>>> +
>>> +        self.log.comment('Start SSSD')
>>> +        self.log.command('service sssd start')
>> Would it make sense to also add instructions to restore SELinux context
>> (if needed)? I'm not sure, just throwing the idea for consideration.
>
>I am not sure about this either so I will wait for more opinions about this.
>
>>
>> Same comments go for the second plugin.
>>
>
>I also refactored the plugin a bit (added a new base class to avoid code
>duplication).
>
>Updated patches are attached. Patch 52 depends on patches 53 and 54.
One small comment:

I've refactored slapi-nis code to make it more generic and references to
sssd in the configuration options went away, so please change this part
too:

>+        attr = users_entry.get('schema-compat-lookup-sssd')
to 
+        attr = users_entry.get('schema-compat-lookup-nsswitch')

>+        if not attr or 'user' not in attr:
>+            return dict(result=False)
>+
>+        try:
>+            groups_entry = ldap.get_entry(groups_dn)
>+        except errors.NotFound:
>+            return dict(result=False)
>+
>+        attr = groups_entry.get('schema-compat-lookup-sssd')
same here.

It needs my patch 0112 too -- it changes ipa-adtrust-install to write
proper configuration options to slapi-nis configs.
-- 
/ Alexander Bokovoy