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

Wed Sep 9 08:52:44 UTC 2015

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?

>
> 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
>

-- 
Jan Cholasta