Re: [Cfrg] I-D Action:draft-kiyomoto-kcipher2-02.txt

[Joachim Strömbergson <[e-mail]>]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Aloha!

I've looked at the draft, the code and tested it using the clang+llvm
compiler. Here are some comments. Most things are minor nits.

* I would recommend that you change the name of the code from CamelCase
and esp from ANaiveKCipher2Impl to something more readable. How about
kcipher2_simple, kcipher2_basic or similarly. Naive has a bit of a bad
ring to it.


* Is CamelCase the naming convention used by you? I'm quite certain it
is generally not for C code and makes it a bit harder to read. And
readibility and understandability is the point of this code.


* Move the S_box, INIT and NORMAL definitions to the .c-file. They are
internal to the cipher, not part of the API. A small name thing: In the
draft it is S-box, in the code S_box. Almost the same, just a detail.


* You get extra points for presenting internal state values during
execution of the test vectors. Very usable, good!


* There are two minor errors in the code, probably simple copy crimes.
KCipher2.c:201:51: warning: data argument not used by format string
[-Wformat-extra-args]
    printf("\nFinally, after the Init() call:\n", i); // for debugging
           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~  ^

KCipher2.c:380:39: warning: data argument not used by format string
[-Wformat-extra-args]
        printf(" are as follows: \n", j);
               ~~~~~~~~~~~~~~~~~~~~~  ^


* I really like the comment structure of the code with headers
describing what is done, what is being changed etc.


* I would change the declarations to use definitions in <stdint.h>
instead of "unsigned int", "unisgned char". This makes more clear what
bit widths are used.


* Add a license/copyright header to the .h-file too.


* Question: What is the license for KCipher2? The code points to the
draft which reiterates the IETF copyright. Basically this states that
the code is under simplified BSD license. (And you should probably add
such license in the code to make it clearer)

But what I would like to see, both in the draft and in the code is
something that clarifies the KDDI R&D Laboratories, Inc stand on the
license for the KCipher2. Is if free for non commercial, is there patent
claims related to the cipher etc?

Compare to the statement for CAST-128:
http://tools.ietf.org/html/rfc2144#section-3


* In the draft, how about calling the shift registers "LFSR"? They are
normal linear shift registers as far as I can see and IMHO LFSR is the
more common term.


That is all for now.

- -- 
Med vänlig hälsning, Yours

Joachim Strömbergson - Alltid i harmonisk svängning.
========================================================================
Kryptoblog - IT-säkerhet på svenska
http://www.strombergson.com/kryptoblog
========================================================================
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (Darwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk2v4lcACgkQZoPr8HT30QHniwCfSrEXnY8Qiwd/eJVSfhG9YT/E
AYsAoJoaOXABtSnnlo6q8KAspm2IglQV
=wzJg
-----END PGP SIGNATURE-----