[Freeipa-devel] [PATCH] 916 vault: add vault container commands

Jan Cholasta jcholast at redhat.com
Thu Sep 17 09:37:24 UTC 2015 

On 14.9.2015 09:44, Jan Cholasta wrote:
> On 9.9.2015 18:39, Petr Vobornik wrote:
>> On 09/09/2015 10:52 AM, Jan Cholasta wrote:
>>> On 8.9.2015 23:06, Petr Vobornik wrote:
>>>> On 09/03/2015 03:18 PM, Jan Cholasta wrote:
>>>>> On 2.9.2015 07:26, Endi Sukma Dewata wrote:
>>>>>> On 9/1/2015 10:22 AM, Simo Sorce wrote:
>>>>>>> On Tue, 2015-09-01 at 17:15 +0200, Petr Vobornik wrote:
>>>>>>>> On 09/01/2015 04:39 PM, Jan Cholasta wrote:
>>>>>>>>> On 1.9.2015 16:26, Jan Cholasta wrote:
>>>>>>>>>> On 26.8.2015 13:22, Petr Vobornik wrote:
>>>>>>>>>>> On 08/25/2015 08:04 PM, Petr Vobornik wrote:
>>>>>>>>>>>> adds commands:
>>>>>>>>>>>> * vaultcontainer-show [--service <service>|--user <user> ]
>>>>>>>>>>>> * vaultcontainer-add-owner
>>>>>>>>>>>>        [--service <service>|--user <user> ]
>>>>>>>>>>>>        [--users <users>]  [--groups <groups>] [--services
>>>>>>>>>>>> <services>]
>>>>>>>>>>>> * vaultcontainer-remove-owner
>>>>>>>>>>>>        [--service <service>|--user <user> ]
>>>>>>>>>>>>        [--users <users>]  [--groups <groups>] [--services
>>>>>>>>>>>> <services>]
>>>>>>>>>>>>
>>>>>>>>>>>> https://fedorahosted.org/freeipa/ticket/5250
>>>>>>>>>>>>
>>>>>>>>>>>> Use cases:
>>>>>>>>>>>> 1. When user/service is deleted, associated vault container
>>>>>>>>>>>> looses
>>>>>>>>>>>> owner. There was no API command to set the owner.
>>>>>>>>>>>> 2. Change owner of container by admin to manage access.
>>>>>>>>>>>>
>>>>>>>>>>>> Show command was added to show current owners.
>>>>>>>>>>>>
>>>>>>>>>>>> Find command was not added, should it be?
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> There is also a design for vault container ownership handling
>>>>>>>>>>> created by
>>>>>>>>>>> Endi - it's for future Vault 2.0.
>>>>>>>>>>>
>>>>>>>>>>> http://www.freeipa.org/page/V4/Password_Vault_2.0#Adding_container_owner
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> This patch has a different API than the proposed - different
>>>>>>>>>>> way of
>>>>>>>>>>> specifying the container. The design page uses path e.g.
>>>>>>>>>>> /users/foobar.
>>>>>>>>>>> This patch uses the same way as vaults e.g. --user=foobar. This
>>>>>>>>>>> means
>>>>>>>>>>> that the implementation in this patch cannot manage ownership of
>>>>>>>>>>> parent
>>>>>>>>>>> vault containers e.g. cn=users,cn=vaults,cn=kra,$SUFFIX.
>>>>>>>>>>>
>>>>>>>>>>> Do we want to go with this approach in 4.2?
>>>>>>>>>>>
>>>>>>>>>>> Attaching also new path which removes setting of owner which
>>>>>>>>>>> doesn't
>>>>>>>>>>> exist so that integrity is OK and that it is consistent with
>>>>>>>>>>> removing of
>>>>>>>>>>> user.
>>>>>>>>>>>
>>>>>>>>>>> Updated patch attached - output fix.
>>>>>>>>>>
>>>>>>>>>> We had a long discussion about this with Petr and we think the
>>>>>>>>>> best
>>>>>>>>>> approach is as follows:
>>>>>>>>>>
>>>>>>>>>>     * Add new "Vault administrators" privilege. Vault
>>>>>>>>>> administrators will
>>>>>>>>>> have unrestricted access to vaults and vault containers,
>>>>>>>>>> including
>>>>>>>>>> the
>>>>>>>>>> power to add/remove owners of vaults and vault containers.
>>>>>>>>>>
>>>>>>>>>>     * Remove the ability of vault owners to add/remove other
>>>>>>>>>> vault
>>>>>>>>>> owners. If vault owner needs to be changed, vault administrator
>>>>>>>>>> has to
>>>>>>>>>> do it. Note that vault owners will still have the ability to
>>>>>>>>>> add/remove
>>>>>>>>>> vault members.
>>>>>>>>>>
>>>>>>>>>>     * When adding new vault container, set owner to the current
>>>>>>>>>> user. If
>>>>>>>>>> vault container owner needs to be changed, vault administrator
>>>>>>>>>> has
>>>>>>>>>> to do
>>>>>>>>>> it.
>>>>>>>>>>
>>>>>>>>>>     * Allow adding vaults and vault containers only if the
>>>>>>>>>> owner is
>>>>>>>>>> set
>>>>>>>>>> to the current user.
>>>>>>>>>>
>>>>>>>>>>     * Introduce commands to modify vault container owner and to
>>>>>>>>>> delete
>>>>>>>>>> vault container, so the administrator has a choice between
>>>>>>>>>> assigning
>>>>>>>>>> ownership or deleting an unowned container.
>>>>>>>>>
>>>>>>>>> Also:
>>>>>>>>>
>>>>>>>>>     * Control access to vault data using an ipaProtectedOperation
>>>>>>>>> ACI.
>>>>>>>>> Users which have read access to "ipaProtectedOperation;accessKRA"
>>>>>>>>> on a
>>>>>>>>> vault can retrieve data from the vault and users which have write
>>>>>>>>> access
>>>>>>>>> to "ipaProtectedOperation;accessKRA" on a vault can archive
>>>>>>>>> data in
>>>>>>>>> the
>>>>>>>>> vault.
>>>>>>>>>
>>>>>>>>> Honza
>>>>>>>>>
>>>>>>>>
>>>>>>>> +1
>>>>>>>>
>>>>>>>> CCing Simo and Endi to check the proposal.
>>>>>>>>
>>>>>>>> And Scott (related to #5216, #5215)
>>>>>>>
>>>>>>> Sounds reasonable to me.
>>>>>>> I can see that allowing owners to hand over vaults w/o admin
>>>>>>> intervention may have some appeal in some use cases, but I also
>>>>>>> see it
>>>>>>> can bring downsides with it, so all in all I think I agree with the
>>>>>>> above points.
>>>>>>>
>>>>>>> Simo.
>>>>>>>
>>>>>>
>>>>>> Not a total objection, but if many people in unrelated groups are
>>>>>> using
>>>>>> vaults, and they are sharing the vaults only with members of each
>>>>>> group,
>>>>>> having to ask a Vault Administrator for each ownership change
>>>>>> sounds a
>>>>>> bit cumbersome. Since the Vault Adminstrator will have access to all
>>>>>> vaults in all groups, only a small number of people can be trusted to
>>>>>> hold that role. If there are many ownership changes the Vault
>>>>>> Administrator will have to handle all those requests, and the vault
>>>>>> users may have to wait until the change is completed.
>>>>>>
>>>>>> If owners are allowed to add others as owners, the vaults will be
>>>>>> pretty
>>>>>> much maintenance free to the admin.
>>>>>
>>>>> Owners can still manage members, which is IMO good enough for
>>>>> sharing a
>>>>> vault with other users.
>>>>>
>>>>>>
>>>>>> Regardless, please update the wiki page to describe the new behavior
>>>>>> when it's implemented:
>>>>>> http://www.freeipa.org/page/V4/Password_Vault_1.1
>>>>>
>>>>> Sure.
>>>>>
>>>>> I have updated and rebased Petr's patch 916.
>>>>>
>>>>> Patch 488 obsoletes Petr's patch 918.
>>>>>
>>>>> Patch for vault data access control is not included, because I was not
>>>>> able to make GER work correctly with
>>>>> "ipaProtectedOperation;accessKRA".
>>>>>
>>>>
>>>> I found 1 major issue(#3), one easy fix(#2), optional(#1) and a
>>>> question
>>>> (#4).
>>>>
>>>> 1. `ipa vaultcontainer-del` doesn't show user/service name. IMHO not a
>>>> blocker.
>>>>
>>>> [pvoborni at vm-063 ~]$ ipa vaultcontainer-del --user=fbar
>>>> --------------------------
>>>> Deleted vault container ""
>>>> --------------------------
>>>
>>> Fixed.
>>>
>>>>
>>>>
>>>> 2. Invalid description of vaultcontainer-show
>>>>    "Display information about a vault."
>>>
>>> Fixed
>>>
>>>>
>>>> 3. Something which needs to be fixed:
>>>>
>>>> Setting password for first vault without a vault container fails(here
>>>> run as vault admin but the same issue is present when it's run as the
>>>> user).
>>>>
>>>> [pvoborni at vm-063 ~]$ ipa vault-add f1 --user=fbar
>>>> New password:
>>>> Verify password:
>>>> ipa: ERROR: Invalid credentials
>>>> [pvoborni at vm-063 ~]$ ipa vault-find --user=fbar
>>>> ---------------
>>>> 1 vault matched
>>>> ---------------
>>>> Vault name: f1
>>>> Type: symmetric
>>>> Vault user: fbar
>>>> ----------------------------
>>>> Number of entries returned 1
>>>> ----------------------------
>>>
>>> Works for me. Are you testing on master or ipa-4-2?
>>
>> I might have not copied a required file during the testing. It works for
>> me now as well.

Petr investigated further and found out that this is caused by 
vaultcontainer-del, which deletes all its child vaults from LDAP, but 
not from KRA. Fixed by disabling subtree delete for vaultcontainer-del.

>>
>>>
>>>>
>>>> Second works:
>>>>
>>>> [pvoborni at vm-063 ~]$ ipa vault-add f2 --user=fbar
>>>> New password:
>>>> Verify password:
>>>> ** Passwords do not match! **
>>>> New password:
>>>> Verify password:
>>>> ----------------
>>>> Added vault "f2"
>>>> ----------------
>>>> Vault name: f2
>>>> Type: symmetric
>>>> Salt: w4tnrjW/Ra2jGS8lI6Frfg==
>>>> Owner users: va
>>>> Vault user: fbar
>>>>
>>>>
>>>>
>>>> [pvoborni at vm-063 ~]$ ipa vault-find --user=fbar
>>>> ----------------
>>>> 2 vaults matched
>>>> ----------------
>>>> Vault name: f1
>>>> Type: symmetric
>>>> Vault user: fbar
>>>>
>>>> Vault name: f2
>>>> Type: symmetric
>>>> Vault user: fbar
>>>> ----------------------------
>>>> Number of entries returned 2
>>>> ----------------------------
>>>>
>>>>
>>>> 4. Q: Should vault container owner delete all its vault?
>>>
>>> I don't know, should it? IMO it shouldn't, at least not by default.
>>>
>>>>
>>>> As fbar when there is a vault without fbar as owner
>>>>
>>>> [root at vm-063 pvoborni]# ipa vaultcontainer-del
>>>> ipa: ERROR: Not allowed on non-leaf entry
>>>>
>>>> when fbar is added as owner to all vaults
>>>>
>>>> [root at vm-063 pvoborni]# ipa vaultcontainer-del
>>>> --------------------------
>>>> Deleted vault container ""
>>>> --------------------------
>>>> [root at vm-063 pvoborni]# ipa vault-add f1
>>>> New password:
>>>> Verify password:
>>>> ipa: ERROR: Invalid credentials
>>>> [root at vm-063 pvoborni]# ipa vault-del f1
>>>> ------------------
>>>> Deleted vault "f1"
>>>> ------------------
>>>> [root at vm-063 pvoborni]# ipa vault-add f1
>>>> New password:
>>>> Verify password:
>>>> ----------------
>>>> Added vault "f1"
>>>> ----------------
>>>> Vault name: f1
>>>> Type: symmetric
>>>> Salt: bkHxRIipkaeX+H/fOnZdBw==
>>>> Owner users: fbar
>>>> Vault user: fbar
>>>>
>>>
>>>
>>
>>
>
> Updated and rebased patches attached.
>
> Added new patch 491 to support KRA updates.

Updated and rebased patches attached.

Added new patch 493 which makes subtree deletion optional in LDAPDelete. 
Apply before the vault container commands patch.

-- 
Jan Cholasta
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-493-baseldap-make-subtree-deletion-optional-in-LDAPDelet.patch
Type: text/x-patch
Size: 1111 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150917/63e69a0c/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pvoborni-0916-4-vault-add-vault-container-commands.patch
Type: text/x-patch
Size: 12818 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150917/63e69a0c/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-488.2-vault-set-owner-to-current-user-on-container-creatio.patch
Type: text/x-patch
Size: 1609 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150917/63e69a0c/attachment-0002.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-489.2-vault-update-access-control.patch
Type: text/x-patch
Size: 6173 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150917/63e69a0c/attachment-0003.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-490.2-vault-add-permissions-and-administrator-privilege.patch
Type: text/x-patch
Size: 11138 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150917/63e69a0c/attachment-0004.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-491.1-install-support-KRA-update.patch
Type: text/x-patch
Size: 14508 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150917/63e69a0c/attachment-0005.bin>

More information about the Freeipa-devel mailing list