Thanks for the quick review Niels!

On 03/17/2011 03:56 AM, Niels Möller wrote:

A typo:

The typos and stupidly-broken hmac_data() convenience function have been fixed and pushed. I also added a unit test for hmac_data() within t/03-hmac.t which would have caught the mistake in the first place.

: COPYRIGHT AND LICENSE : Copyright (c) Daniel Kahn Gillmor Crypt::Nettle is free software, you : may redistribute it and/or modify it under the same terms as Perl : itself.

The GPL/LGPL license of the nettle library itself may apply to perl programs using these bindings. I don't know if it's customary to document this in a bit more detail?

I welcome suggestions for improved text. I agree that the intersections of the various licenses can be a bit confusing.

You include arctwo algorithms twice in the algorithm list.

That's what i get for copying/pasting from the docs :P

Section 6.2.11 of http://www.lysator.liu.se/~nisse/nettle/nettle.html actually lists them twice.

Maybe you should exclude serpent until the recently discovered interoperability problems are sorted out?

I'd prefer to expose the functions exposed by the underlying library if possible. If there are incompatibilities, we should be catching them in the test suite (though my test suite for ciphers only covers aes and cast and camellia at the moment).

How do you deal with algorithms with a large number of possible key sizes? Maybe it would be better to view, e.g., aes and arcfour as just two algorithm, and let the size of the given key imply the keysize?

hm, this is an interesting idea. I'll have to think about how to implement that, but i think it's doable.

The $is_encrypt flag to new seems a bit awkward. Maybe it would be easier with

my $ctx = new ($algo, $mode) /* Possibly with $mode defaulting to ecb?, and not allowed at all for stream ciphers. */

$ctx->set_encrypt_key($key, $iv) /* $iv optional and required when applicable */ $ctx->set_decrypt_key($key, $iv)

: process($data)

My problem with this is that i then have to handle the case where the user invokes process() without having remembered to set a key. I agree that $is_encrypt seems a little bit clumsy (and it's irrelevant for many of the ciphers), but i think the fact that it's readable mitigates things somewhat.

I think the requirement that the length is a multiple of the block size needs to be relaxed a bit. For CTR mode, one should allow a partial block for the last call. And *maybe* for all calls (with an internal block buffer to let CTR work like a stream cipher), even if that's not how nettle's ctr mode support works.

I'm actually not enforcing any of these constraints in the perl code -- they'll just crop up if the user passes the wrong data down to the library underneath.

Maybe you should think about how to add gcm support. Which is a bit more complicated, with both per-key state and per-message state, and additional inputs and outputs.

interesting -- is GCM part of nettle itself, or do you think i should implement it in the perl wrapper? I didn't see any mention of GCM in the online docs.

How do you query if a cipher is a block or a stream cipher? block_size() returning 0?

yep, that's it at the moment.

Let me know what other feedback you have,

--dkg

On 03/17/2011 05:29 AM, Niels Möller wrote:

Daniel Kahn Gillmor dkg@fifthhorseman.net writes:

...

My problem with this is that i then have to handle the case where the user invokes process() without having remembered to set a key.

Can't you just raise some error ?

Sure, but that introduces a different kind of ugliness -- i'm not sure that trading off this ugliness for the other one is worthwhile, though i'm willing to be convinced.

You'd need to have some flag to remember if it's been initialized, but you need that anyway for the is_encrypt method, right?

i'm not sure i see the parallel. is_encrypt() says whether the Crypt::Nettle::Cipher object was initialized as an encrypting cipher or a decrypting cipher.

...

I'm actually not enforcing any of these constraints in the perl code -- they'll just crop up if the user passes the wrong data down to the library underneath.

That seems a bit dangerous. I thought the principle was that it shouldn't be easy to write perl code which triggers some assertion failure in some C routine.

Yes, that would be ideal. I think the right way to go in the long term (as noted in the BUGS section of Cipher.pm) is to add internal buffering for process() calls so the user can pass arbitrarily-sized data to the object.

The extra fiddly bits with this arrangement are:

0) process_in_place() is still brittle: you won't be able to call it at all if the internal buffer isn't on a clean block boundary, and you also won't be able to call it with arbitrary-sized data.

1) the data retrieved from process() isn't guaranteed to be the same size as the data fed in.

2) i'll need to introduce a finish() call that handles some sort of padding and emits the final data.

It's a bunch more bookkeeping internally, but it does seem like it would let the user treat the block ciphers as something approximating a stream cipher without having to think much about it, which would be nice.

I think getting RSA working is higher priority for me at the moment, but i'll certainly keep this suggestion on my plate.

It's in the CVS version of Nettle, but not yet in any release. Maybe most of the discussion was private rather than on this list?

When a new version is released, i'll be happy to update the perl bindings to enable access to the new features :)

Is the revision control for Nettle publicly visible?

--dkg

On 03/17/2011 05:50 AM, Daniel Kahn Gillmor wrote:

I think getting RSA working is higher priority for me at the moment,

And now it's done -- I just wrapped up Crypt::Nettle::Yarrow and Crypt::Nettle::RSA! I'm about ready to tag this thing as Crypt::Nettle version 0.1. I'll try to do a CPAN upload shortly if i can figure that out.

If anyone can run the test suite and report back, i'd appreciate it.

git clone git://lair.fifthhorseman.net/~dkg/libcrypt-nettle-perl cd libcrypt-nettle-perl perl Makefile.PL make make test

Niels, this is clearly a derivative work of Nettle. I'm happy to put whatever free license you think is appropriate on it. Is GPL-2+ OK?

Regards,

--dkg

On 03/18/2011 01:55 AM, Niels Möller wrote:

Daniel Kahn Gillmor dkg@fifthhorseman.net writes:

...

Niels, this is clearly a derivative work of Nettle. I'm happy to put whatever free license you think is appropriate on it. Is GPL-2+ OK?

If by "GPL-2+" you mean GPL version 2 or (user's option) any later version, that's fine with me.

yes, this is what i mean.

As you know, changing the Nettle license to (some version of) the LPGL is planned. When that happens, it would make some sense for the perl bindings to follow, even if sticking to the GPL is still a perfectly acceptable option.

Sure, I'd be happy to follow the licensing changes.

--dkg

Hi Simon--

On 03/17/2011 04:45 AM, Simon Josefsson wrote:

Don't forget to add RSA blinding, otherwise it may be vulnerable in the real world. I wish Nettle supported this natively, RSA is not generally safe without it.

Thanks for this suggestion -- i'm not sure that the perl bindings are the right place to do this, though. Do other Nettle language bindings handle RSA blinding? I'd rather have the perl bindings stay fairly close to the underlying C library.

My understanding is that RSA blinding is a countermeasure against timing attacks, and that it introduces a new dependency on some sort of RNG (though perhaps a weak one?) to parts of the process that wouldn't otherwise need it. I'd certainly prefer to have that handled within the lower-level library if possible, though i wouldn't mind creating and handing in a yarrow context for each of these operations.

--dkg

On Thu, Mar 17, 2011 at 10:35 AM, Niels Möller nisse@lysator.liu.se wrote:

Daniel Kahn Gillmor dkg@fifthhorseman.net writes:

...

My understanding is that RSA blinding is a countermeasure against timing attacks, and that it introduces a new dependency on some sort of RNG (though perhaps a weak one?) to parts of the process that wouldn't otherwise need it.

I confess I don't remember the details of why blinding is desirable. Does it improve hiding of the key, message, or both?

Actually RSA is has pretty much limited utility without blinding since retrieving the RSA private key from a web server has been shown practical since 2003 and attacks were known since 1996 (Kocher). gnutls implements blinding over nettle's functions. You might add a warning on the documentation of nettle's functions.

The papers discussion the attacks: * Timing Attacks on Implementations of Diffie-Hellman, RSA, DSS, and Other Systems by Kocher (1996) * Remote timing attacks are practical by Boneh and Brumley * Improving Brumley and Boneh Timing Attack on Unprotected SSL Implementations

regards, Nikos