[Freeipa-devel] Notes and questions for fine-grained read permissions

[Petr Viktorin]

On 09/06/2013 04:41 PM, Rob Crittenden wrote:
> Petr Viktorin wrote:
>> On 09/06/2013 03:59 PM, Rob Crittenden wrote:
>>> Petr Viktorin wrote:
>>>> On 09/06/2013 02:46 PM, Rob Crittenden wrote:
>>>>> Martin Kosek wrote:
>>>>>> On 09/05/2013 07:48 PM, Rob Crittenden wrote:
>>>>>>> Petr Viktorin wrote:
[...]
>>>>>>>> # P.S.
>>>>>>>>
>>>>>>>> I believe that we should strive to put our info about default
>>>>>>>> permissions, containers, settings, and the schema for each
>>>>>>>> plugin in
>>>>>>>> the
>>>>>>>> actual plugin module, rather than it all being split across several
>>>>>>>> ldif/update files. This would make this data more manageable,
>>>>>>>> introspectable and consistent, expose dependencies between plugins,
>>>>>>>> and
>>>>>>>> make it possible to actually write self-contained plugins.
>>>>>>>> This should be done when the time comes for a new version of the
>>>>>>>> ldap-updater.
>>>> [...]
>>>>>
>>>>> Well, my alarm bells are going off. This would require some
>>>>> significant
>>>>> review each and every time any object is updated. Consider how many
>>>>> times API.txt is overlooked, and it has no security implications.
>>>>
>>>> Consider how many times the ldif or update files were overlooked.
>>>> API.txt is checked during every build, whereas ldif/update files not,
>>>> and are tedious to check.
>>>>
>>>> We can of course add a similar file for ACIs, in fact I was planning to
>>>> do so. Any changes can be then reviewed both in the source and (like
>>>> now) in the resulting ldif. Also, the information would be in one
>>>> place,
>>>> rather than in ldif (which doesn't apply on upgrades) and in update
>>>> files.
>>>> I believe it would actually be better for security.
>>>
>>> The plan at the time updates were added was to move absolutely
>>> everything out of ldif and into updates. It just never happened.
>>
>> Good to know. Is it still the plan? Do I only need to change the update
>> files?
>
> It would be my preference. It goes beyond only changing one set of
> files. The existing ldif that duplicate things need to be deprecated. We
> can't get to a zero-ldif install, but it can be reduced significantly.

Thanks, I'll keep it in mind.

>>>>> I can't say I'm a fan of programmatic ACI generation.
>>>>
>>>> Well, I'm sure not going to write the ACIs for this feature by hand. It
>>>> would help I could actually polish, commit and integrate the script
>>>> that
>>>> generates them.
>>>> In my book, programmatic generation is much better than copy+paste.
>>>>
>>>
>>> On the other hand, it becomes very easy to just rely on it and not
>>> closely examine the output.
>>
>> That is a question of checking the changes. Frankly I don't see that
>> much difference between not checking after updates and not checking
>> after copy+pasting.
>> If nothing, the existence of an ACI.txt should remind us that security
>> is at stake.
>> Also, if the objectclasses change this will alert us that ACIs need
>> changing as well.
>>
>
> Well, there was little copy+pasting before. The majority of the current
> ACIs were generated by permission-add.
>
> The difference is that ACI changes may be automatically spawned by an
> update to the plugin. Right now the risk is that attributes won't be
> added to a write list, so there is limited potential damage. It is just
> annoying. If instead it will potentially grant both read and write
> access the scope of harm grows.

We have two cases here:

1. A core IPA plugin is changed.
In this case the change is reflected in our ACI.txt file and vetted by 
developers.

2. A third-party plugin is changed.
Currently third-party plugins needed to either change core IPA update 
files (which I doubt plugin authors will want to do), or have their own 
mechanism for granting permissions (yes, plugins can be bent to do 
anything, including installing their own updates).
It would actually be more secure if we provided a tested mechanism to them.


Also, read ACIs that would get deployed on install and then *only* 
changed by the user would mean that we can either:
- Never add new hidden/password attributes, for (targetattr != ...), or
- Never add new normal attributes, for (targetattr == ...),
- Or (as we IMO do now) assume admins will manually update anything they 
have changed.

-- 
Petr³