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

Joachim Strömbergson <Joachim@Strombergson.com> Thu, 21 April 2011 07:53 UTC

Return-Path: <Joachim@Strombergson.com>
X-Original-To: cfrg@ietfc.amsl.com
Delivered-To: cfrg@ietfc.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfc.amsl.com (Postfix) with ESMTP id D646DE0741 for <cfrg@ietfc.amsl.com>; Thu, 21 Apr 2011 00:53:00 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -2.699
X-Spam-Level:
X-Spam-Status: No, score=-2.699 tagged_above=-999 required=5 tests=[BAYES_00=-2.599, J_CHICKENPOX_54=0.6, MIME_8BIT_HEADER=0.3, RCVD_IN_DNSWL_LOW=-1]
Received: from mail.ietf.org ([208.66.40.236]) by localhost (ietfc.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id rEEe+s7NKmst for <cfrg@ietfc.amsl.com>; Thu, 21 Apr 2011 00:52:59 -0700 (PDT)
Received: from susano.oderland.com (susano.oderland.com [91.201.63.143]) by ietfc.amsl.com (Postfix) with ESMTP id 387A0E0692 for <cfrg@irtf.org>; Thu, 21 Apr 2011 00:52:58 -0700 (PDT)
Received: from 2.67.227.87.static.g-sn.siw.siwnet.net ([87.227.67.2] helo=snabbis.local) by susano.oderland.com with esmtpsa (TLSv1:AES256-SHA:256) (Exim 4.69) (envelope-from <Joachim@Strombergson.com>) id 1QCoh6-0001oG-Bc; Thu, 21 Apr 2011 09:52:56 +0200
Message-ID: <4DAFE257.4080602@Strombergson.com>
Date: Thu, 21 Apr 2011 09:52:55 +0200
From: =?UTF-8?B?Sm9hY2hpbSBTdHLDtm1iZXJnc29u?= <Joachim@Strombergson.com>
Organization: Kryptologik
User-Agent: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.2.15) Gecko/20110303 Lightning/1.0b2 Thunderbird/3.1.9
MIME-Version: 1.0
To: Wook Shin <wookshin@kddilabs.jp>
References: <20110413010001.17531.11616.idtracker@ietfc.amsl.com> <op.vtur72zepzjggh@ohpato-t61.sec.kddilabs.jp>
In-Reply-To: <op.vtur72zepzjggh@ohpato-t61.sec.kddilabs.jp>
X-Enigmail-Version: 1.1.1
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
X-AntiAbuse: This header was added to track abuse, please include it with any abuse report
X-AntiAbuse: Primary Hostname - susano.oderland.com
X-AntiAbuse: Original Domain - irtf.org
X-AntiAbuse: Originator/Caller UID/GID - [47 12] / [47 12]
X-AntiAbuse: Sender Address Domain - Strombergson.com
Cc: "cfrg@irtf.org" <cfrg@irtf.org>
Subject: Re: [Cfrg] I-D Action:draft-kiyomoto-kcipher2-02.txt
X-BeenThere: cfrg@irtf.org
X-Mailman-Version: 2.1.12
Precedence: list
Reply-To: Joachim@Strombergson.com
List-Id: Crypto Forum Research Group <cfrg.irtf.org>
List-Unsubscribe: <http://www.irtf.org/mailman/options/cfrg>, <mailto:cfrg-request@irtf.org?subject=unsubscribe>
List-Archive: <http://www.irtf.org/mail-archive/web/cfrg>
List-Post: <mailto:cfrg@irtf.org>
List-Help: <mailto:cfrg-request@irtf.org?subject=help>
List-Subscribe: <http://www.irtf.org/mailman/listinfo/cfrg>, <mailto:cfrg-request@irtf.org?subject=subscribe>
X-List-Received-Date: Thu, 21 Apr 2011 07:53:01 -0000

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