Re: Strong Crypto Support for GRUB2

[Simon Peter <dn.tlp@gmx.net>]

Hi Robert,

thanks for the in-depth look at my patch. My comments below.

> > +#ifndef GET_UINT32_BE
> > +#define GET_UINT32_BE(n,b,i)                            \
> > +{                                                       \
> > +    (n) = ( (uint32) (b)[(i)    ] << 24 )        \
> > +        | ( (uint32) (b)[(i) + 1] << 16 )        \
> > +        | ( (uint32) (b)[(i) + 2] <<  8 )        \
> > +        | ( (uint32) (b)[(i) + 3]       );       \
> > +}
> Doesn't follow GCS indentation style in a number of places.  I would
> suggest using the indent(1) tool on it.

Any specific options that I shall use?

> > +GRUB_MOD_INIT(crypto)
> > +{
> > +  (void)mod;			/* To stop warning. */
> > +  grub_crypto_cipher_register(&grub_cipher_none);
> > +  grub_crypto_cipher_register(&grub_hash_none);
> > +}
> Which warning was that?

Actually, I copied that line verbatim from hello.c, the GRUB hello
world module. :) It seems that warning is long gone.

> > diff -u -r -x CVS -N grub2-orig/crypto/rmd160.c
> > grub2/crypto/rmd160.c --- grub2-orig/crypto/rmd160.c
> > 1970-01-01 01:00:00.000000000 +0100 +++
> > grub2/crypto/rmd160.c	2007-09-02 00:52:57.000000000 +0200 @@
> > -0,0 +1,430 @@ +/*
> > + * 2007-09-01: Modified for GNU GRUB by Simon Peter
> > <dn.tlp@gmx.net>.
> > + */
> > +/********************************************************************
> > \
> > + *
> > + *      FILE:     rmd160.c
> > + *
> > + *      CONTENTS: A sample C-implementation of the RIPEMD-160
> > + *                hash-function.
> > + *      TARGET:   any computer with an ANSI C compiler
> > + *
> > + *      AUTHOR:   Antoon Bosselaers, ESAT-COSIC
> > + *      DATE:     1 March 1996
> > + *      VERSION:  1.0
> > + *
> > + *      Copyright (c) Katholieke Universiteit Leuven
> > + *      1996, All Rights Reserved
> > + *
> > + *  Conditions for use of the RIPEMD-160 Software
> > + *
> > + *  The RIPEMD-160 software is freely available for use under the
> > terms and
> > + *  conditions described hereunder, which shall be deemed to be
> > accepted by
> > + *  any user of the software and applicable on any use of the
> > software:
> > + * 
> > + *  1. K.U.Leuven Department of Electrical Engineering-ESAT/COSIC
> > shall for
> > + *     all purposes be considered the owner of the RIPEMD-160
> > software and of
> > + *     all copyright, trade secret, patent or other intellectual
> > property
> > + *     rights therein.
> > + *  2. The RIPEMD-160 software is provided on an "as is" basis
> > without
> > + *     warranty of any sort, express or implied. K.U.Leuven makes
> > no
> > + *     representation that the use of the software will not
> > infringe any
> > + *     patent or proprietary right of third parties. User will
> > indemnify
> > + *     K.U.Leuven and hold K.U.Leuven harmless from any claims or
> > liabilities
> > + *     which may arise as a result of its use of the software. In
> > no
> > + *     circumstances K.U.Leuven R&D will be held liable for any
> > deficiency,
> > + *     fault or other mishappening with regard to the use or
> > performance of
> > + *     the software.
> > + *  3. User agrees to give due credit to K.U.Leuven in scientific
> > publications 
> > + *     or communications in relation with the use of the
> > RIPEMD-160 software 
> > + *     as follows: RIPEMD-160 software written by Antoon
> > Bosselaers, 
> > + *     available at
> > http://www.esat.kuleuven.be/~cosicart/ps/AB-9601/.
> > + *
> > +
> > \********************************************************************/
> 3) doesn't look GPL-compatible.  As for 1), note the author is
> claiming ownership of any patents that might be covered by this
> code.  GPL compatibility aside, I'm not sure what the consequences of
> accepting the license would be (could it lead to someone
> acknowledging K.U.Leuven as the owner of their own patents?), but it
> looks dangerous.

Interesting, as RIPEMD is known to be one of the most open and
unencumbered hash functions (see http://en.wikipedia.org/wiki/RIPEMD).
There are no patents covering the code. :)

> > +enum grub_cipher_type
> > +  {
> > +    GRUB_CIPHER_TYPE_NONE = 0,
> > +    GRUB_CIPHER_TYPE_CIPHER = 1,
> > +    GRUB_CIPHER_TYPE_HASH = 2
> > +  };
> Wasn't the point of using enum to avoid hardcoding these numbers? :-)

Woops. I thought you guys were doing the same and that's why I did it.
I reverted that (leaving NONE = 0 intact).

I'm going to post another patch with your comments implemented, after I
have your reply (I need to know what to pass to indent(1)).

Simon