Re: Bug#325205: please provide sha256sum, sha384sum and sha512sum

[Jim Meyering]

David Madore <address@hidden> wrote:
> Yes, I am willing to do that.

Great.
I'll send you the copyright forms.

>> My requesting a `clean' patch means we're pretty picky,
>> and that the more of the following you can do, the better.
>
> Can I ask you to take a look at the attached patch?  It is not yet
> satisfactory (at least, it needs to be forwardported to the CVS
> version, as I worked against 5.2.1 because I wanted to install this
> ASAP on my Debian boxen), but maybe we can start from this to discuss
> what needs to be done.  It implements sha224, sha256, sha384 and
> sha512.
>
> [Note: need to "chmod +x tests/sha???sum/basic-1" after applying.]
>
> Here are a few things which need to be said about it, and which may be
> concerns:
>
> - sha224 and sha256 (in lib/sha256.c) present no particular
> difficulties.  However, as concerns sha384 and sha512 we have the
> following problem: the algorithm uses 64-bit words.  The attached
> patch (in lib/sha512.c) tries to find a 64-bit unsigned integer type
> basically by copying the code from md5/sha1 and adding a test for
> unsigned long long (and it barfs if it can't find one): however, this
> is probably not portable enough for your goals.  It should be possible

Actually, it may not be a big deal at all.
If there is a reasonable porting target that lacks a 64-bit integral
type, people will report it and we can address the problem then.

Regarding this comment,
  +/* The following contortions are an attempt to use the C preprocessor
  +   to determine an unsigned integral type that is 64 bits wide.  An
  +   alternative approach is to use autoconf's AC_CHECK_SIZEOF macro, but
  +   doing that would require that the configure script compile and *run*
  +   the resulting executable.  Locally running cross-compiled executables
  +   is usually not possible.  */
FWIW, we *can* use compile-only tests to check for type sizes.
We should be able to write a 64-bit analog of m4/uint32_t.m4.
There may already be such a macro.
But for now, you're welcome to go ahead with what you have.
Besides, it'll take at least a couple of weeks to do the copyright
paper exchange.

BTW, it's ok to use #error, now.

> to write an implementation of sha384 and sha512 using only 32-bit
> words, but that would be extremely tedious, so I might be willing to
> do it, but not immediately.

That doesn't seem worthwhile.

> In the meantime, I guess one needs to
> have some configure test determine whether 64-bit words are available
> and, if not, warn the user and skip compiling sha384 and sha512.  Only
> I don't know how to do that, so I'll need some help here.
>
> - Speaking of word sizes, there's a potential alignment problem in the
> common code in src/md5sum.c, which does not seem to check whether the
> result buffer is 32-bit aligned (let alone 64-bit).  This should be
> trivial to fix, if that's an issue: but someone should decide whether
> the fix goes in the library routines (remove alignment constraint) or
> in the md5sum common code.

As far as alignment, I haven't heard of any problems, but maybe
that's just luck, since that buffer is the first automatic
variable declared in main.  You're welcome to fix it in src/md5sum.c.

> - sha1sum does not seem to be documented in texinfo (at least not in
> 5.2.1), so, since I was lazily copying everything I could from sha1, I
> didn't come up with a texinfo doc for sha224 etc.  I'm willing to try
> to document sha1sum along with the rest (but I've never written
> texinfo, I hope that's not a problem).

Thanks!  We still do need texinfo documentation for sha1sum.

> - With respect to the following remark of yours:
>
>> By the way, your e.g., src/sha256sum.c file should look like this:
>>
>>   #include "checksum.h"
>>   int algorithm = ALG_SHA256;
...
> So I'd like to advocate doing things differently, as in my attached
> patch: do away with src/md5.c, src/sha1sum.c and src/checksum.h, and
> simply compile src/md5sum.c six times, with a different compile-time
> flag each time, to provide the six utilities.  This works fine in
> automake, it means recompiling the same source six times but the
> resulting utilities are smaller and do not contain any needless code.
> How does that sound?

That sounds fine, of course.
Obviously I didn't really want 6 copies of such code in each binary :)
The point was to continue to keep everything well factored, as you've done.

> - Lastly, as concerns tests, I used the FIPS-supplied test vectors,
> except that I did not include the "one million a's" test, because I
> don't know how to do this properly with Fetish (we don't really want
> to write a file with one millon a's in it just to test the utility: it
> would be much better to pipe the data, but I don't know the framework
> for that).

These days, I wouldn't worry about creating a 1MB file.
You could use the Fetish (now, in cvs, called Coreutils.pm) framework
with 'a'.1000000 as the input.  Or use the tests/sample template and do
something like this to get the checksum:

  printf %1000000s ' '|sed 's/ /a/' |sha256sum > out