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

Fri Aug 2 15:50:21 UTC 2013

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.

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-akrivoka-0052-02-Add-ipa-advise-plugins-for-legacy-clients.patch
Type: text/x-patch
Size: 11549 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20130802/bae2a2b2/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-akrivoka-0053-Enable-running-API-commands-in-ipa-advise-plugins.patch
Type: text/x-patch
Size: 1993 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20130802/bae2a2b2/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-akrivoka-0054-Add-new-command-compat-is-enabled.patch
Type: text/x-patch
Size: 3209 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20130802/bae2a2b2/attachment-0002.bin>