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

[Arno Wagner <arno@wagner.name>]

On Thu, Jul 29, 2010 at 02:01:52PM +0200, Milan Broz wrote:
> 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).

Well, noting is ever perfect, so avoiding strong dependencies is
definitely a good idea.

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

I like that for security resons, due to increased clarity
about what is used and lower complexity.
 
> 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.

I agree.

> - 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.)

Uniform tests are definitely a very good idea.

> - 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.)

I agree. Which algorithms are supported by all backend and 
which require a specific backend will also become a FAQ item ;-)

We could run a poll here for a month or so about what people 
consider must-have. 

> 
> 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.

I would support that. Seems like a good idea.

Arno
-- 
Arno Wagner, Dr. sc. techn., Dipl. Inform., CISSP -- Email: arno@wagner.name 
GnuPG:  ID: 1E25338F  FP: 0C30 5782 9D93 F785 E79C  0296 797F 6B50 1E25 338F
----
Cuddly UI's are the manifestation of wishful thinking. -- Dylan Evans

If it's in the news, don't worry about it.  The very definition of 
"news" is "something that hardly ever happens." -- Bruce Schneier