[Freeipa-devel] [PATCHES] 0552-0554 Upgrading write permissions

[Martin Kosek]

On 05/26/2014 04:44 PM, Petr Viktorin wrote:
> On 05/22/2014 03:07 PM, Petr Viktorin wrote:
>> Hello,
>> Here I start upgrading  the existing default permissions to the new
>> Managed style.
>>
>> https://fedorahosted.org/freeipa/ticket/4346
>>
>> The patches rely on my patch 0551
>> (https://fedorahosted.org/freeipa/ticket/4349)
>> You may run into what seems to be a 389 bug. If you get a "Midair
>> Collision" (NO_SUCH_ATTRIBUTE) error, restart the DS and try running
>> ipa-ldap-updater again. I'm working with Ludwig on this one.
>>
>>
>>
>> The operation is now described at
>> http://www.freeipa.org/page/V4/Managed_Read_permissions#Replacing_legacy_default_permissions
>>
>>
>>
>> If there user has modified an old default permission, a warning is
>> logged the replacement permission is not added/updated. The user needs
>> to evaluate the situation: either update the old permission to match the
>> original default, or remove the old permission, and then run
>> ipa-ldap-updater will create the new one.
>> Is bailing out the right thing to do if the old entry was modified?

Forcing user to remove old permission and create new one seems as a too much
work to me. After the upgrade, we need to be sure that the managed permissions
is there.

What is the problem of having both 2 permissions in the DS? The old modified
permission and the new system managed one? As we are dealing with allow
permissions, having 2 of them should be harmless.

>> It could be possible to parse the permission, figure out the changes the
>> user made, and apply them to the new one, but that seems like too much
>> guesswork to me.

Maybe we could do the same we do with managed permissions upgrades? Only allow
differences in the list of attributes? I am thinking that people could hotfix
missing attributes at permissions themselves (like adding description to
sudorule permission), this would lead to duplicate permissions later.

What we could do when old ACI differs only in allowed attributes is to compare
it to defaults and set whitelist and blacklist attributes of the new managed
permission. Then we can safely delete the old ACI (with warning).

If you think this is too much work, we can keep the old behavior and just add
duplicate ACI.

>> On the other hand, my approach has a downside as well: if the
>> 'memberallowcmd' attribute was removed from 'Modify Sudo rule', there's
>> now no way to upgrade while allowing access but keeping that attribute
>> off-limits, short of writing deny a ACI by hand. How big a problem is
>> this? It might be worth it to create a special tool that upgrades a
>> single permission and allows setting the excluded/included attributes
>> explicitly.

This problem would be removed with my approach proposed above.

>> There are some interesting scenarios to think about with respect to
>> upgrades and user changes:
>>
>> * Upgrade to old version, e.g.
>>    - have IPA 3.2 master, IPA 3.2 replica
>>    - upgrade master to 4.0 (old permissions are updated)
>>    - then upgrade replica to 3.3 (old permissions are added again!)
>>
>> This is AFAIK not supported but it does happen.
>> We can't change old IPA versions, so any upgrade to a pre-4.0 IPA will
>> always add the old permissions, but with this patch, a subsequent
>> upgrade to 4.0+, or running a 4.0+ ipa-ldap-update, will remove the old
>> permissions again.

Hm, I think this is the best option we have. We should warn about this behavior
in our release notes though.

>> Tied to that is another scenario:
>>
>> * Re-create permissions with old names
>>    - have IPA 4.0 master
>>    - Create a permission named 'Modify Sudo rule'
>>    - Upgrade to IPA 4.1
>>
>> Here we need to make sure the new permission is *not* removed, because a
>> new 'Modify Sudo rule' permission is no longer special in any way. To
>> ensure this the updater only removes old-style permissions.

Right, we can decide based on objectclasses - whether permissionsv2 OC is there
or not.

>>
>> One thing that can happen when 4.0 masters are still mixed with 3.x is
>> that an old permission named 'Modify Sudo rule' is added on the old
>> server. Any update to 4.0+ will remove that.
>> Old-style default permissions were sorta-kinda managed by IPA itself
>> anyway, so users should expect this. We should still point it out in the
>> docs though, since I expect some users to start messing with the
>> permissions before upgrading all of the infrastructure to 4.0.

+1, I would just point out that behavior in the release notes.

>> The second patch upgrades sudorule permissions, this server as an
>> example of how the  will work.
>> The third patch fixes https://fedorahosted.org/freeipa/ticket/4344
> 
> The user read permissions patches had a conflict with these; attaching rebased
> version.

Now the actual review
552.2: worked fine for me. Some updates will probably be needed though, based
on the discussion above.

553.2:

1) Why should we bother specifying ipapermdefaultattr for "add" ACIs? Looks
like a noop to me, it was also never part of our add ACIs.

I tried to strip that down to just "description" and I was still able to add a
whole new SUDO rule. Ludwig, is that correct - does DS ignore (should it?)
targetattr part of add ACI?

2) You stated 'System: Modify Sudo rule' as "add" ACI, making it ineffective.
Privileged user still cannot update all SUDO rule attributes.

Besides that, the ACIs were working fine.

Martin