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

Petr Viktorin pviktori at redhat.com
Tue Jun 3 12:41:16 UTC 2014


On 05/28/2014 04:36 PM, Petr Viktorin wrote:
> On 05/28/2014 04:27 PM, Petr Viktorin wrote:
>> On 05/27/2014 04:20 PM, Martin Kosek wrote:
>>> 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.
>>
>> Note that this only happens if the user changed the permissions, so we
>> need to be extra careful to respect their wishes.
>>
>>> 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.
>>
>> The new one could be granting too much access, the admin might have
>> wanted to restrict the defaults.
>>
>>
>>>>> 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.
>>
>> Having duplicate permissions would be possible, after all they have a
>> different name. However I'd expect that most people would still want to
>> delete the old ones, rather than letting them hide among user-defined
>> permissions.
>>
>>>>> 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.
>>
>> Simo, I hazily remember discussing that we should only allow specific
>> attributes on add, otherwise users can add entries with any extra
>> objectclasses and attributes. Did we come to a conclusion?
>> I might have confused targetattr with targetattrfilter in my notes;
>> since I see targetarr is ineffective.
>
> OK, this was just me confused. As Ludwig told me,
>> for adding an entry you need add rights for the entry and write rights
>> for each attribute, so in the add aci the targetattrs are irrelevant.
> so I'll remove them from the add ACI.
>
>>> 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.
>>
>> Duh. I've been staring at this too long.
>>
>>> Besides that, the ACIs were working fine.

The attached version looks at the old permission in LDAP and if it 
differs from the old default only in the targetattrs, it transplants the 
difference to the new managed permission.

There is a lot of logging here so if something didn't work the way you 
expected, at least you'll know what happened.

When there were multiple defaults, i.e. IPA added/removed some 
attributes in some version: the new managed permission's attributes will 
be applied, so upgrades from both very old and not-so-old versions 
should "do the right thing".

If the old permission differs in something else than targetattrs, an 
error is logged (this will show up in yum update output), and as before 
the new managed permission is not created. The user now has 3 options to 
fix this:
- Delete the old permission, then run ipa-ldap-updater to create the new 
default.
- Modify the old permission on an *old* system to match the old default, 
possibly with targetattr changes, then run a *new* ipa-ldap-updater to 
convert that to the new default
- Modify the old permission on a *new* system, which changes it to a V2 
permission, then run ipa-ldap-updater to create the new default, ending 
up with both permissions.

The distinction betwen the last two is subtle and error-prone, but
1) I don't see a better way, considering that future upgrades need to 
work well (in IPA 4.0+ a user-created permission named "Add Sudo Rule" 
has no special status)
2) I'm hoping that people didn't modify the old default permissions that 
much; if they did they should have felt some pain already -- I don't 
think the update system in 3.x would handle such changes wery well

Apply my patch 0565 before trying these out.


Some testing tips:
- Create 3.x master and replica
- Upgrade master RPMs with these patches
- to add old permissions, run ipa-ldap-updater on the replica
- to simulate an upgrade, run ipa-ldap-updater on the master
- to delete a managed permission:
$ curl -v -H Content-Type:application/json -H Accept:applicaton/json 
--negotiate -u : --delegation always --cacert /etc/ipa/ca.crt -X POST -H 
referer:https://`hostname`/ipa/json -d '{"method": "permission_del", 
"params":[["$PERMISSION_NAME"],{"force":true}], "id":0 }' 
https://`hostname`/ipa/json
- be careful where you run permission-mod; on 4.0 it will convert the 
permission to V2

-- 
Petr³

-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pviktori-0552.3-Add-mechanism-for-updating-permissions-to-managed.patch
Type: text/x-patch
Size: 9320 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20140603/036053c1/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pviktori-0553.3-Convert-Sudo-rule-default-permissions-to-managed.patch
Type: text/x-patch
Size: 6660 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20140603/036053c1/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pviktori-0554.3-Add-missing-attributes-to-Modify-Sudo-rule-permissio.patch
Type: text/x-patch
Size: 1711 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20140603/036053c1/attachment-0002.bin>


More information about the Freeipa-devel mailing list