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

[Petr Vobornik]

On 08/13/2015 02:12 PM, Christian Heimes wrote:
> On 2015-08-13 14:05, Petr Vobornik wrote:
>> 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.
>
> I'd still like to see my patch for #5142 in RHEL, too. It prevents
> accidental exposure of private keys, too. In the test case the test
> uploads his private keys to the server. FreeIPA should not leak a user's
> private key. My patch prevents that, too.
>

Sorry, wrong number. I meant defer #5143. The patch for #5142 is OK.

Pushed to:
master: e4dff25838f7a2342779851bd68460080d77683b
ipa-4-2: 06d68b447ff62b64a3a6e4a3c565fcf406a457ec
-- 
Petr Vobornik