[Freeipa-devel] [PATCH] 0057 Only allow root to run update plugins

[Petr Viktorin]

On 06/05/2012 06:53 PM, Petr Viktorin wrote:
> On 06/05/2012 04:18 PM, Rob Crittenden wrote:
>> Petr Viktorin wrote:
>>> On 06/05/2012 03:00 PM, Rob Crittenden wrote:
>>>> Petr Viktorin wrote:
>>>>> On 06/05/2012 10:06 AM, Martin Kosek wrote:
>>>>>> On Mon, 2012-06-04 at 11:51 -0400, Simo Sorce wrote:
>>>>>>> On Mon, 2012-06-04 at 17:22 +0200, Petr Viktorin wrote:
>>>>>>>> An update plugin needed root privileges, and aborted the update
>>>>>>>> if an
>>>>>>>> ordinary user user ran it.
>>>>>>>> With this patch the plugin is skipped with a warning in that case.
>>>>>>>>
>>>>>>>> https://fedorahosted.org/freeipa/ticket/2621
>>>>>>>
>>>>>>> Hi Petr,
>>>>>>> I am not sure I like the proposed solution.
>>>>>>>
>>>>>>> If there is a legitimate reason to run this plugin as non-root (eg
>>>>>>> admin
>>>>>>> user) then you should change the connection part to try to use
>>>>>>> GSSAPI
>>>>>>> auth over ldap when non-root, not just throw a warning.
>>>>>>>
>>>>>>> If there is no reason for anyone but root to run this script then we
>>>>>>> should just abort if not root IMO.
>>>>>>>
>>>>>>> Simo.
>>>>>>>
>>>>>>
>>>>>> I would keep this script runable for root users only. Regularly, this
>>>>>> should not be run manually but as a part of RPM update which is
>>>>>> done by
>>>>>> root. It is being run manually only when something is broken anyway
>>>>>> and
>>>>>> I am not convinced that non-root users should be involved in such
>>>>>> recovery.
>>>>>>
>>>>>> Martin
>>>>>>
>>>>>
>>>>> Thanks for the advice. The attached patch only allows root to run
>>>>> ipa-ldap-updater.
>>>>
>>>> NACK. It is very handy for developers to be able to run
>>>> ipa-ldap-updater
>>>> to test update files.
>>>>
>>>> rob
>>>
>>> Developers can run it as root, I don't see a problem here.
>>
>> I'd really rather not. This does nothing requiring root permissions,
>> it's all done over LDAP. I'd rather trade not running some plugins than
>> always requiring root.
>>
>> rob
>>
>
> Thanks for info on how the tool is used. I looked into it deeper.
> The proper fix would be to use the ldap2 backend here, instead of the
> IPAdmin. That's ticket 2660, and it'll be quite a lot of work to get
> ReplicationManager and tools that depend on that ported.
>
>
> But, I think it makes sense to require root if (and only if) plugins are
> run. Justification below. Would that work for your use case?
>
>
> There are currently three modes ipa-ldap-updater can run in:
> 1) --upgrade (needs root, runs plugins)
> 2) no --upgrade, either no files specified or --plugins (doesn't need
> root, runs plugins)
> 3) no --upgrade, specific files specified without --plugins (doesn't
> need root, doesn't run plugins)
>
> I propose to make mode 2 require root.
>
> There are two major uses of the script: install/upgrade (first two
> modes), and a developer testing update files (third or possibly second
> mode). Install/upgrade is always run as root, and the developer usually
> doesn't need to run the plugins (if they do, they should run as root
> anyway, so that some (parts of) plugins aren't skipped).
>
> Some of the plugins ask to restart the DS. Without root privileges, the
> restart (but not the rest of the plugin) is skipped. I think this is
> just asking for trouble.
> Some plugins (or parts of plugins) don't need root, but I don't think
> singling these out and testing both cases is worth the effort.
>
>

The attached patch that implements the above. I re-ordered the code a 
bit to put the checks before the DM password prompt, so you don't enter 
the password only to find out you had to use sudo or different options.

-- 
Petr³
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pviktori-0057-03-Only-allow-root-to-run-update-plugins.patch
Type: text/x-patch
Size: 3758 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20120606/c0292e04/attachment.bin>