[Freeipa-devel] [PATCH 019] Asymmetric vault: validate public key in client

[Petr Vobornik]

On 08/13/2015 12:38 PM, Christian Heimes wrote:
> On 2015-08-13 12:10, Petr Vobornik wrote:
>> On 07/23/2015 08:38 PM, Christian Heimes wrote:
>>> The ipa vault commands now load the public keys in order to verify them.
>>> The validation also prevents a user from accidentally sending her
>>> private keys to the server. The patch fixes #5142 and #5142.
>>>
>>> $ ./ipa vault-add AsymmetricVault --desc "Asymmetric vault" --type
>>> asymmetric --public-key-file mykey.pem
>>> ipa: ERROR: invalid 'ipavaultpublickey': Invalid or unsupported vault
>>> public key: Could not unserialize key data.
>>>
>>> https://fedorahosted.org/freeipa/ticket/5142
>>> https://fedorahosted.org/freeipa/ticket/5143
>>>
>>
>> ACK as fix for 5142.
>>
>> I don't think that it fixes 5143. The traceback is fixed therefore 5143
>> doesn't occur but if there was other traceback raised by
>> `self.api.Command.vault_archive(*args, **opts)` then the vault added in
>> `response = self.api.Command.vault_add_internal(*args, **options)` would
>> be still created.
>
> Yes, that is correct. There aren't any arguments that can lead to an
> exception. The arguments are either already validated by vault_add() or
> don't raise an error.
>
> Of course there are plenty of opportunities errors. The connection to
> the IPA or LDAP server could fail, NSS DB could be missing and so on.
> How should we handle an error in vault_archive? Is there another way
> then to delete the new vault all along?
>
> try:
>      self.api.Command.vault_archive(*args, **opts)
> except Exception:
>      log_error()
>      self.api.Command.vault_del(*args, **opts)
>      report_error()
>
> Christian
>

Imho this is the way. But it may fail because of the same root cause as 
vault_archive.

That said I don't see #5142 as a priority and would defer it.
-- 
Petr Vobornik