Re: [dm-crypt] [PATCH] support OpenSSL's libcrypto and make libgcrypt optional

[Milan Broz <mbroz@redhat.com>]

On 07/28/2010 10:33 AM, Jussi.Laako@nokia.com wrote:
> In some embedded environments, like in some where MeeGo is used, it is
> needed to reduce number of packages with duplicate functionality in order to
> save flash space. Thus we have created a patch to add optional support for
> OpenSSL's libcrypto as well as make libgcrypt optional, making it possible
> to build cryptsetup in environments lacking either one. Or to support both,
> if that makes any sense.

Hi,

I intentionally removed plugin interface and [lib]cryptsetup now depends
on libgcrypt.

Well, since that decision I found some problems in gcrypt (which upstream
refused to fix) so seems there is now yet another reason to support more
crypto backends. (It should support gcrypt, nss, openssl at least).

But I do not want to use runtime plugin architecture but compile time
decision only.

So for you patch:
- seems you are reverting some configure options which are not needed,
I think it is enough to have someting like --with-crypto=[gcrypt,openssl,nss]
or so.

- setup_backend is not needed, it will always depend on device-mapper
(did I miss anything? it is not used even in your patch...just defined there)

- using #ifdef is not ideal, mainly in crypto code - it is easy to break
algorithm when using wrong defines. It need properly define crypto backend
callbacks and switch only its implementations. And include tests
(I have already PBKDF2 testvectors test, that should be used for all backends.)

- I think openssl backend function should not duplicate all the code
for every algorithm

- you added some "sync" calls to test - is it another problem?
(should not be needed at all)

- there is still hardcoded gcrypt logic on some places (including api-test),
so it still links to gcrypt, all regression tests must run with all supported
backends. Probably some basic list of must-have algorithms should be defined
for crypto backends. (e.g. I am using whirlpool hash for testing, not all crypto
backend support it currently.)


So, in general, I agree we should support more crypto backends.

Are you ok with supporting this in compile time only (so there is always
only one backend in compiled binaries - depends on distro preference)?

If so, I'll add this to my TODO list for next versions.

Thanks,
Milan