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
_______________________________________________ nettle-bugs mailing list [email protected] http://lists.lysator.liu.se/mailman/listinfo/nettle-bugs
