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

Thu Sep 17 13:01:47 UTC 2015

On 09/17/2015 11:37 AM, Jan Cholasta wrote:
> 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.
>

ACK

master:
* 2964b019d93b33b9703e6e26c8ca6fc28509ba64 baseldap: make subtree 
deletion optional in LDAPDelete
* d396913e9c0578fa68847b84e44a4f0dd916fbfd vault: add vault container 
commands
* 5cf46b89364111b54172682283a6362bb82db9a6 vault: set owner to current 
user on container creation
* d3503043c47a1adc139688776341dc86b7085448 vault: update access control
* 0dfcf1d9db4b297791e3784588bf23cc0ac8d2ee vault: add permissions and 
administrator privilege
* 5137478fb8bba16d9cbecba53983c893dc0884d5 install: support KRA update
ipa-4-2:
* b3932055c6ad0c4032790530c4776a5c7262cd8c baseldap: make subtree 
deletion optional in LDAPDelete
* ad7325d08c432d77b048910d37be2bd8e13cb162 vault: add vault container 
commands
* 78f890620b8cae286a03b26e28a0bee4ea49b8f1 vault: set owner to current 
user on container creation
* b9615c89cd07a6bd2907686c03b2ea11e40c76bf vault: update access control
* 500e0d152cf1611db47c775f3fbc5b72e1522da0 vault: add permissions and 
administrator privilege
* b1587bf2d8072d76df52ea0c7480f146cbb8c933 install: support KRA update

-- 
Petr Vobornik