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

Valitov Alexander Alexander.Valitov at digia.com
Fri Jul 30 13:46:11 CEST 2010


Hi,

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

I added 'sync' because otherwise sometimes in tests this command:

  # dmsetup remove volumeName

failed to remove volume and I got this error:

  device-mapper: remove ioctl failed: Device or resource busy

Initially I added 'sleep 1' before 'dmsetup', but then decided to replace it with 'sync'.
'sync' also did the trick. So I kept it.
I didn't try to figure out what's the problem with 'dmsetup remove'

Best Regards,
  Alexander Valitov
________________________________________
From: Milan Broz [mbroz at redhat.com]
Sent: 29 July 2010 15:01
To: Jussi.Laako at nokia.com
Cc: agk at redhat.com; dm-crypt at saout.de; Valitov Alexander
Subject: Re: [PATCH] support OpenSSL's libcrypto and make libgcrypt optional

On 07/28/2010 10:33 AM, Jussi.Laako at 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


More information about the dm-crypt mailing list