[Freeipa-devel] [PATCH] Password vault

[Jan Cholasta]

Dne 15.6.2015 v 09:22 Jan Cholasta napsal(a):
> Dne 10.6.2015 v 08:13 Martin Kosek napsal(a):
>> On 06/09/2015 11:13 PM, Endi Sukma Dewata wrote:
>>> Please take a look at the attached patch to add symmetric &
>>> asymmetric vaults.
>>> Some comments about the patch:
>
> I think it would be better to use a new attribute type which inherits
> from ipaPublicKey (ipaVaultPublicKey?) rather than ipaPublicKey directly
> for assymetric vault public keys, so that assymetric public key and
> escrow public key are on the same level and you can still use
> ipaPublicKey to refer to either one:
>
>      ipaPublicKey
>          ipaVaultPublicKey
>          ipaEscrowPublicKey
>
>      ( 2.16.840.1.113730.3.8.18.2.? NAME 'ipaVaultPublicKey' DESC
> 'Assymetric vault public key as DER-encoded SubjectPublicKeyInfo (RFC
> 5280)' SUP ipaPublicKey EQUALITY octetStringMatch SYNTAX
> 1.3.6.1.4.1.1466.115.121.1.40 X-ORIGIN 'IPA v4.2' )
>      ( 2.16.840.1.113730.3.8.18.2.3 NAME 'ipaEscrowPublicKey' DESC 'IPA
> escrow public key as DER-encoded SubjectPublicKeyInfo (RFC 5280)' SUP
> ipaPublicKey EQUALITY octetStringMatch SYNTAX
> 1.3.6.1.4.1.1466.115.121.1.40 X-ORIGIN 'IPA v4.2' )
>

BTW what is the reason for that vault type is idendified by ipaVaultType 
attribute rather than object class?

>>>
>>> 1. The vault_add was split into a client-side vault_add and server-side
>>> vault_add_internal since the parameters are different (i.e. public
>>> key file and
>>> future escrow-related params). Since vault_add inherits from Local all
>>> non-primary-key attributes have to be added explicitly.
>
> The split is not really necessary, since the only difference is the
> public_key_file option, which exists only because of the lack of proper
> file support in the framework. This is a different situation from
> vault_{archive,retrieve}, which has two different sets of options on
> client and server side. Escrow adds only ipaescrowpublickey and
> escrow_public_key_file, right? If yes, we can safely keep the command in
> a single piece.
>
>>>
>>> 2. Since the vault_archive_internal inherits from Update, it accepts
>>> all non
>>> primary-key attributes automatically. This is incorrect since we
>>> don't want to
>>> update these parameters during archival. Can this behavior be
>>> overridden?
>
> Inherit from PKQuery instead (don't forget to add "has_output =
> output.standard_entry").
>
> BTW the correct solution would be to have a separate object and commands
> for vault data (e.g. vaultdata object, vault_archive -> vaultdata_mod,
> vault_retrieve -> vauldata_show), then we wouldn't have to deal with
> mixing vault attributes with vault data and could use proper crud base
> classes.
>
>>
>> Just for the record, this changes API, right? It would be better to
>> have this
>> in Alpha planned for this week. Not a blocker for Alpha though, we can
>> give
>> warning that the internal API may change before GA.


-- 
Jan Cholasta