[Freeipa-devel] [PATCH] Password vault

[Alexander Bokovoy]

On Tue, 02 Jun 2015, Endi Sukma Dewata wrote:
>Please take a look at the new patch.
>
>On 6/2/2015 10:05 AM, Martin Kosek wrote:
>>>>4) In the vault-archive forward method, you use "pki" module. However,
>>>>this module will be only available on FreeIPA PKI-powered servers and
>>>>not on FreeIPA clients - so this will not work unless freeipa-client
>>>>gets a dependency on pki-base - which is definitely not something we
>>>>want...
>>>
>>>In my opinion it should be fine to require pki-base on the client because it
>>>contains just the client library, unless you have other concerns? Any
>>>objections to having pki-nss and pki-cryptography dependencies on the client?
>>>
>>>Even if we can change the client code not to depend on "pki" module, since in
>>>this framework the client and server code are written in the same plugin, the
>>>"import pki" still cannot be removed since it's still needed by the server
>>>code, and I don't think conditional import is a good programming practice.
>>
>>I have major concerns here. Look at the different between installing
>>"freeipa-client" and "freeipa-client + pki-base" on my F21:
>>
>>~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>$ sudo yum install freeipa-client
>>...
>>Install  1 Package (+4 Dependent packages)
>>
>>Total download size: 2.6 M
>>Installed size: 14 M
>>~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>$ sudo yum install freeipa-client pki-base
>>...
>>Install  2 Packages (+288 Dependent packages)
>>
>>Total download size: 160 M
>>Installed size: 235 M
>>~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>
>>This is obviously a no-go for client. The conditional import is smaller concern
>>that big dependency growth on the client. We do them in trust plugin for
>>example and it works fine (though I agree it is not ideal programming practice).
>>
>>IMO, we should limit new freeipa-client dependencies only to
>>python-cryptography (or also python-nss in the worst case, in case
>>python-cryptography is not enough) - there should be no pki dependencies at
>>all, these should be only on the server side.
>
>OK. I opened a ticket to split the pki-base into separate Python and 
>Java packages:
>https://fedorahosted.org/pki/ticket/1399
>
>For now in this patch I added conditional imports for pki.account and 
>pki.key which are needed to access KRA on the server side. I removed 
>dependency on pki.crypto on the client side and replaced it with 
>direct python-nss code.
>
>On 6/2/2015 1:40 PM, Simo Sorce wrote:
>> I have coded in python (jwcrypto)
>> support for some key wrapping not yet available in python-cryptography
>> and can lend you the code as needed.
>
>Thanks. I'll get back to you when it's time to add support for 
>python-cryptography in KRA:
>https://fedorahosted.org/pki/ticket/1400
>
>On 6/2/2015 10:16 AM, Alexander Bokovoy wrote:
>>Yes, please use conditional import here, it is perfectly valid use case
>>for the client side.
>
>On 6/2/2015 1:40 PM, Simo Sorce wrote:
>> conditional import is just fine
>
>The conditional imports that I've seen usually are used for importing 
>different versions of the same module, which I think is acceptable 
>because the dependency always exists. In the vault case we're 
>selectively importing a module depending on where the code runs. I 
>think that is bad because it adds complexity and it's easy to make 
>mistakes. Any code that depends on that module would have to be (a) 
>guarded:
>
>  if pki_is_loaded:
>      ... call pki ...
>
>or (b) used in a method that's only called if the module is loaded:
>
>  def do_something(self): # runs only on server
>      ... call pki ...
>
>The (a) is similar to #ifdef's which should be avoidable using OOD, 
>and in (b) we may inadvertently call a wrong method indirectly. I 
>think ideally the client and server code should be in separate files 
>(so they can be deployed separately too), but the framework doesn't 
>seem to allow that.
This exactly the case we have to use here and we are using that in
trusts case as well -- some code has to run on server only and shouldn't
cause to install Samba related packages on the client. This is because
IPA client is actually using the same IPA plugins that server uses, to
have access to the API calls metadata and client-side callbacks are
defined in the same place where server-side callbacks are. It is IPA
framework design, so we have to use what we have.

In other words, it is not necessarily an evil under conditions we are
dealing with.

-- 
/ Alexander Bokovoy