Adding sha256 and sha512 to C?

Adding sha256 and sha512 to C?

[sand]

Leo (sdl.web at gmail.com) recently added the gnulib "sha1" function to
Emacs' C code.  The gnulib crypto library also has sha256 and sha512
functions; is there any objection to adding those as well?  Researchers
discovered better-than-brute-force attacks against SHA-1 back in 2005
(http://www.schneier.com/blog/archives/2005/02/cryptanalysis_o.html).
Because of this, in 2006 the NIST recommended that application and
protocol designers switch to SHA-2 family hash functions by 2011
(http://csrc.nist.gov/groups/ST/hash/policy.html).  Providing C
versions of the SHA-2 functions will make it easier for developers to
use the those stronger functions in new applications.

Thanks,

Derek

-- 
Derek Upham
sand@blarg.net

Re: Adding sha256 and sha512 to C?

[Paul Eggert]

On 05/27/11 20:18, sand@blarg.net wrote:
> The gnulib crypto library also has sha256 and sha512
> functions; is there any objection to adding those as well?

Sounds good, but rather than continue to add crypto functions
wouldn't it be better to have a single function parameterized by the
algorithm name?  Something like the following signature:

 (crypto-hash-function ALGORITHM OBJECT
    &optional START END CODING-SYSTEM NOERROR BINARY)

Then, we could implement the existing functions this way:

(defun md5 (object &optional start end coding-system noerror)
   (crypto-hash-function 'md5 object start end coding-system noerror nil))

(define sha1 (object &optional start end binary)
   (crypto-hash-function 'sha1 object start end nil nil binary))

Re: Adding sha256 and sha512 to C?

[Eli Zaretskii]

> Date: Fri, 27 May 2011 20:58:38 -0700
> From: Paul Eggert <eggert@cs.ucla.edu>
> Cc: emacs-devel@gnu.org
> 
> On 05/27/11 20:18, sand@blarg.net wrote:
> > The gnulib crypto library also has sha256 and sha512
> > functions; is there any objection to adding those as well?
> 
> Sounds good, but rather than continue to add crypto functions
> wouldn't it be better to have a single function parameterized by the
> algorithm name?

I agree with Paul: the API he proposes makes much more sense.

Re: Adding sha256 and sha512 to C?

[Stefan Monnier]

>> > The gnulib crypto library also has sha256 and sha512
>> > functions; is there any objection to adding those as well?
>> Sounds good, but rather than continue to add crypto functions
>> wouldn't it be better to have a single function parameterized by the
>> algorithm name?
> I agree with Paul: the API he proposes makes much more sense.

I'm not sue the CODING-SYSTEM argument is a good idea.  Other than that,
I wouldn't object, although really I don't see the advantage either.
I'd argue you could define:

  (defalias 'crypto-hash-function #'apply)

and still call (crypto-hash-function 'md5 object start end).


        Stefan

Re: Adding sha256 and sha512 to C?

[Leo]

[-- Attachment #1: Type: text/plain, Size: 87 bytes --]

Could people comment on the attached patch which implements sha-2?
Thanks in advance.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: sha.diff --]
[-- Type: text/x-diff, Size: 6464 bytes --]

=== modified file 'src/deps.mk'
--- src/deps.mk	2011-05-24 08:22:58 +0000
+++ src/deps.mk	2011-06-11 05:29:51 +0000
@@ -284,8 +284,8 @@
 floatfns.o: floatfns.c syssignal.h lisp.h globals.h $(config_h)
 fns.o: fns.c commands.h lisp.h $(config_h) frame.h buffer.h character.h \
    keyboard.h keymap.h window.h $(INTERVALS_H) coding.h ../lib/md5.h \
-   ../lib/sha1.h blockinput.h atimer.h systime.h xterm.h ../lib/unistd.h \
-   globals.h
+   ../lib/sha1.h ../lib/sha256.h ../lib/sha512.h blockinput.h atimer.h \
+   systime.h xterm.h ../lib/unistd.h globals.h
 print.o: print.c process.h frame.h window.h buffer.h keyboard.h character.h \
    lisp.h globals.h $(config_h) termchar.h $(INTERVALS_H) msdos.h termhooks.h \
    blockinput.h atimer.h systime.h font.h charset.h coding.h ccl.h \

=== modified file 'src/fns.c'
--- src/fns.c	2011-06-07 01:39:26 +0000
+++ src/fns.c	2011-06-11 05:41:20 +0000
@@ -4538,21 +4538,21 @@
 
 \f
 /************************************************************************
-			     MD5 and SHA1
+			MD5, SHA-1, and SHA-2
  ************************************************************************/
 
 #include "md5.h"
 #include "sha1.h"
+#include "sha256.h"
+#include "sha512.h"
 
 /* Convert a possibly-signed character to an unsigned character.  This is
    a bit safer than casting to unsigned char, since it catches some type
    errors that the cast doesn't.  */
 static inline unsigned char to_uchar (char ch) { return ch; }
 
-/* TYPE: 0 for md5, 1 for sha1. */
-
 static Lisp_Object
-crypto_hash_function (int type, Lisp_Object object, Lisp_Object start, Lisp_Object end, Lisp_Object coding_system, Lisp_Object noerror, Lisp_Object binary)
+crypto_hash_function (void (*hash_func) (const char *buffer, size_t len, void *resblock), EMACS_UINT digest_size, Lisp_Object object, Lisp_Object start, Lisp_Object end, Lisp_Object coding_system, Lisp_Object noerror, Lisp_Object binary)
 {
   int i;
   EMACS_INT size;
@@ -4562,6 +4562,7 @@
   register EMACS_INT b, e;
   register struct buffer *bp;
   EMACS_INT temp;
+  char *digest;
   Lisp_Object res=Qnil;
 
   if (STRINGP (object))
@@ -4733,46 +4734,24 @@
 	object = code_convert_string (object, coding_system, Qnil, 1, 0, 0);
     }
 
-  switch (type)
+  digest = xmalloc(digest_size * sizeof(char));
+
+  hash_func(SSDATA (object) + start_byte,
+	    SBYTES (object) - (size_byte - end_byte),
+	    digest);
+
+  if (NILP(binary))
     {
-    case 0:			/* MD5 */
-      {
-	char digest[16];
-	md5_buffer (SSDATA (object) + start_byte,
-		    SBYTES (object) - (size_byte - end_byte),
-		    digest);
-
-	if (NILP (binary))
-	  {
-	    char value[33];
-	    for (i = 0; i < 16; i++)
-	      sprintf (&value[2 * i], "%02x", to_uchar (digest[i]));
-	    res = make_string (value, 32);
-	  }
-	else
-	  res = make_string (digest, 16);
-	break;
-      }
-
-    case 1:			/* SHA1 */
-      {
-	char digest[20];
-	sha1_buffer (SSDATA (object) + start_byte,
-		     SBYTES (object) - (size_byte - end_byte),
-		     digest);
-	if (NILP (binary))
-	  {
-	    char value[41];
-	    for (i = 0; i < 20; i++)
-	      sprintf (&value[2 * i], "%02x", to_uchar (digest[i]));
-	    res = make_string (value, 40);
-	  }
-	else
-	  res = make_string (digest, 20);
-	break;
-      }
+      char *value = xmalloc((2 * digest_size + 1) * sizeof(char));
+      for (i = 0; i < digest_size; i++)
+	sprintf (&value[2 * i], "%02x", to_uchar (digest[i]));
+      res = make_string(value, digest_size * 2);
+      xfree(value);
     }
+  else
+    res = make_string(digest, digest_size);
 
+  xfree(digest);
   return res;
 }
 
@@ -4805,7 +4784,7 @@
 guesswork fails.  Normally, an error is signaled in such case.  */)
   (Lisp_Object object, Lisp_Object start, Lisp_Object end, Lisp_Object coding_system, Lisp_Object noerror)
 {
-  return crypto_hash_function (0, object, start, end, coding_system, noerror, Qnil);
+  return crypto_hash_function ((void *) &md5_buffer, MD5_DIGEST_SIZE, object, start, end, coding_system, noerror, Qnil);
 }
 
 DEFUN ("sha1", Fsha1, Ssha1, 1, 4, 0,
@@ -4817,7 +4796,39 @@
 form.  */)
      (Lisp_Object object, Lisp_Object start, Lisp_Object end, Lisp_Object binary)
 {
-  return crypto_hash_function (1, object, start, end, Qnil, Qnil, binary);
+  return crypto_hash_function ((void *) &sha1_buffer, SHA1_DIGEST_SIZE, object, start, end, Qnil, Qnil, binary);
+}
+
+DEFUN ("sha224", Fsha224, Ssha224, 1, 4, 0,
+       doc: /* Return the SHA-224 (Secure Hash Algorithm) of an OBJECT.
+See `sha1' for explanation of the arguments.  */)
+  (Lisp_Object object, Lisp_Object start, Lisp_Object end, Lisp_Object binary)
+{
+  return crypto_hash_function ((void *) &sha224_buffer, SHA224_DIGEST_SIZE, object, start, end, Qnil, Qnil, binary);
+}
+
+DEFUN ("sha256", Fsha256, Ssha256, 1, 4, 0,
+       doc: /* Return the SHA-256 (Secure Hash Algorithm) of an OBJECT.
+See `sha1' for explanation of the arguments.  */)
+  (Lisp_Object object, Lisp_Object start, Lisp_Object end, Lisp_Object binary)
+{
+  return crypto_hash_function ((void *) &sha256_buffer, SHA256_DIGEST_SIZE, object, start, end, Qnil, Qnil, binary);
+}
+
+DEFUN ("sha384", Fsha384, Ssha384, 1, 4, 0,
+       doc: /* Return the SHA-384 (Secure Hash Algorithm) of an OBJECT.
+See `sha1' for explanation of the arguments.  */)
+  (Lisp_Object object, Lisp_Object start, Lisp_Object end, Lisp_Object binary)
+{
+  return crypto_hash_function ((void *) &sha384_buffer, SHA384_DIGEST_SIZE, object, start, end, Qnil, Qnil, binary);
+}
+
+DEFUN ("sha512", Fsha512, Ssha512, 1, 4, 0,
+       doc: /* Return the SHA-512 (Secure Hash Algorithm) of an OBJECT.
+See `sha1' for explanation of the arguments.  */)
+  (Lisp_Object object, Lisp_Object start, Lisp_Object end, Lisp_Object binary)
+{
+  return crypto_hash_function ((void *) &sha512_buffer, SHA512_DIGEST_SIZE, object, start, end, Qnil, Qnil, binary);
 }
 
 \f
@@ -4993,6 +5004,10 @@
   defsubr (&Sbase64_decode_string);
   defsubr (&Smd5);
   defsubr (&Ssha1);
+  defsubr (&Ssha224);
+  defsubr (&Ssha256);
+  defsubr (&Ssha384);
+  defsubr (&Ssha512);
   defsubr (&Slocale_info);
 }
 

=== modified file 'src/makefile.w32-in'
--- src/makefile.w32-in	2011-05-31 17:03:24 +0000
+++ src/makefile.w32-in	2011-06-11 05:30:23 +0000
@@ -869,6 +869,8 @@
 	$(EMACS_ROOT)/nt/inc/sys/time.h \
 	$(EMACS_ROOT)/lib/md5.h \
 	$(EMACS_ROOT)/lib/sha1.h \
+	$(EMACS_ROOT)/lib/sha256.h \
+	$(EMACS_ROOT)/lib/sha512.h \
 	$(LISP_H) \
 	$(SRC)/atimer.h \
 	$(SRC)/blockinput.h \

Re: Adding sha256 and sha512 to C?

[Eli Zaretskii]

> From: Leo <sdl.web@gmail.com>
> Date: Sat, 11 Jun 2011 13:43:00 +0800
> Cc: Eli Zaretskii <eliz@gnu.org>, Paul Eggert <eggert@cs.ucla.edu>,
> 	sand@blarg.net, emacs-devel@gnu.org
> 
> Could people comment on the attached patch which implements sha-2?

Why 3 different functions?  Why not one?

Re: Adding sha256 and sha512 to C?

[Leo]

On 2011-06-11 16:00 +0800, Eli Zaretskii wrote:
[snipped 5 lines]
>> Could people comment on the attached patch which implements sha-2?
>
> Why 3 different functions?  Why not one?

How do you mean?

Leo

Re: Adding sha256 and sha512 to C?

[Eli Zaretskii]

> From: Leo <sdl.web@gmail.com>
> Cc: eggert@cs.ucla.edu,  emacs-devel@gnu.org,  monnier@iro.umontreal.ca,  sand@blarg.net
> Date: Sat, 11 Jun 2011 20:37:36 +0800
> 
> On 2011-06-11 16:00 +0800, Eli Zaretskii wrote:
> [snipped 5 lines]
> >> Could people comment on the attached patch which implements sha-2?
> >
> > Why 3 different functions?  Why not one?
> 
> How do you mean?

Like this:

 (sha2 OBJECT &optional BEG END BINARY ALGORITHM)

where ALGORITHM can be sha-224, sha-256, etc.

(We could have ALGORITHM the 3rd argument, if keeping a signature
compatible with sha1 is not important.)

Re: Adding sha256 and sha512 to C?

[Paul Eggert]

On 06/11/11 08:24, Eli Zaretskii wrote:

> Like this:
> 
>  (sha2 OBJECT &optional BEG END BINARY ALGORITHM)
> 
> where ALGORITHM can be sha-224, sha-256, etc.
> 
> (We could have ALGORITHM the 3rd argument, if keeping a signature
> compatible with sha1 is not important.)

Or better yet:

 (crypto-hash-function ALGORITHM OBJECT
    &optional START END NOERROR BINARY)

as was discussed in <http://lists.gnu.org/archive/html/emacs-devel/2011-05/msg00872.html>
and the ensuing thread.  The above suggestion removes
the previously-proposed CODING-SYSTEM argument that Stefan
didn't think was needed, which means that people who want
md5's CODING-SYSTEM feature would have to invoke the md5
function directly.

The advantage of this approach is that we don't pollute
the Lisp namespace with one function name per algorithm.

Some other comments:

Please modify GNULIB_MODULES in Makefile.in to reflect
the new dependencies.

No need to use EMACS_UINT for digest_size.  Please use
plain 'int'.  We prefer to not use unsigned types due
to problems when they're used in comparisons.

No need to use sizeof (char).  It is always 1.

Please write "F (ARGS)" rather than "F(ARGS)".

When creating the digest the code should use make_unibyte_string
rather than make_string.  (This comment applies
also to the original.)

Please don't cast function pointers to void * and back, as
that defeats the purpose of the C type checking.  Instead,
just use the unvarnished C types.

Re: Adding sha256 and sha512 to C?

[Juanma Barranquero]

On Sat, Jun 11, 2011 at 18:02, Paul Eggert <eggert@cs.ucla.edu> wrote:

> The advantage of this approach is that we don't pollute
> the Lisp namespace with one function name per algorithm.

I think generalization is a good argument, but "don't pollute the
namespace" is not. Interesting, widely used cryptographically secure
hash functions (which, I suppose, are the target of
crypto-hash-function) are introduced at a rate of one every X years, X
>> 1. (md5 is from 1992, sha-1 is from 1995, sha-2 is from 2001).

 I'm pretty sure we "pollute the namespace" with all kind of functiosn
at a rate several orders of magnitude bigger than that ;-)

    Juanma

Re: Adding sha256 and sha512 to C?

[YAMAMOTO Mitsuharu]

>>>>> On Sat, 11 Jun 2011 09:02:36 -0700, Paul Eggert <eggert@cs.ucla.edu> said:

> When creating the digest the code should use make_unibyte_string
> rather than make_string.  (This comment applies also to the
> original.)

And perhaps I would allocate a Lisp string with make_uninit_string and
then store the result directly into its contents rather than xmalloc -
make_unibyte_string - xfree, if the size is known in advance.

				     YAMAMOTO Mitsuharu
				mituharu@math.s.chiba-u.ac.jp

Re: Adding sha256 and sha512 to C?

[Leo]

[-- Attachment #1: Type: text/plain, Size: 2329 bytes --]

On 2011-06-12 00:02 +0800, Paul Eggert wrote:
> On 06/11/11 08:24, Eli Zaretskii wrote:
>
>> Like this:
>> 
>>  (sha2 OBJECT &optional BEG END BINARY ALGORITHM)
>> 
>> where ALGORITHM can be sha-224, sha-256, etc.
>> 
>> (We could have ALGORITHM the 3rd argument, if keeping a signature
>> compatible with sha1 is not important.)
>
> Or better yet:
>
>  (crypto-hash-function ALGORITHM OBJECT
>     &optional START END NOERROR BINARY)

crypto-hash-function is very long to type.

How about export this to elisp:

(sha OBJECT &optional START END BINARY ALGORITHM)

where ALGORITHM can be 1 (default), 224, 256, 384, 512, and make sha1
obsolete? In a sense we unify all SHA functions and leave MD5 as is.

Please review the attached patch. Thank you.

> as was discussed in <http://lists.gnu.org/archive/html/emacs-devel/2011-05/msg00872.html>
> and the ensuing thread.  The above suggestion removes
> the previously-proposed CODING-SYSTEM argument that Stefan
> didn't think was needed, which means that people who want
> md5's CODING-SYSTEM feature would have to invoke the md5
> function directly.
>
> The advantage of this approach is that we don't pollute
> the Lisp namespace with one function name per algorithm.
>
> Some other comments:

Many thanks for the comments.

> Please modify GNULIB_MODULES in Makefile.in to reflect
> the new dependencies.

This was done in the original patch, no?

> No need to use EMACS_UINT for digest_size.  Please use
> plain 'int'.  We prefer to not use unsigned types due
> to problems when they're used in comparisons.

fixed.

> No need to use sizeof (char).  It is always 1.

I get rid of xmalloc as suggested.

> Please write "F (ARGS)" rather than "F(ARGS)".

Hopefully fixed.

> When creating the digest the code should use make_unibyte_string
> rather than make_string.  (This comment applies
> also to the original.)
>
> Please don't cast function pointers to void * and back, as
> that defeats the purpose of the C type checking.  Instead,
> just use the unvarnished C types.

fixed.

On 2011-06-12 08:34 +0800, YAMAMOTO Mitsuharu wrote:
[snipped 6 lines]
> And perhaps I would allocate a Lisp string with make_uninit_string and
> then store the result directly into its contents rather than xmalloc -
> make_unibyte_string - xfree, if the size is known in advance.

Thanks.

Leo

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: sha-2.diff --]
[-- Type: text/x-diff, Size: 6872 bytes --]

=== modified file 'lisp/subr.el'
--- lisp/subr.el	2011-06-02 18:04:44 +0000
+++ lisp/subr.el	2011-06-12 12:08:48 +0000
@@ -1053,6 +1053,7 @@
 (define-obsolete-function-alias 'show-buffer 'set-window-buffer "22.1")
 (define-obsolete-function-alias 'eval-current-buffer 'eval-buffer "22.1")
 (define-obsolete-function-alias 'string-to-int 'string-to-number "22.1")
+(define-obsolete-function-alias 'sha1 'sha "24.1")
 
 (make-obsolete 'forward-point "use (+ (point) N) instead." "23.1")
 

=== modified file 'src/deps.mk'
--- src/deps.mk	2011-05-24 08:22:58 +0000
+++ src/deps.mk	2011-06-11 05:29:51 +0000
@@ -284,8 +284,8 @@
 floatfns.o: floatfns.c syssignal.h lisp.h globals.h $(config_h)
 fns.o: fns.c commands.h lisp.h $(config_h) frame.h buffer.h character.h \
    keyboard.h keymap.h window.h $(INTERVALS_H) coding.h ../lib/md5.h \
-   ../lib/sha1.h blockinput.h atimer.h systime.h xterm.h ../lib/unistd.h \
-   globals.h
+   ../lib/sha1.h ../lib/sha256.h ../lib/sha512.h blockinput.h atimer.h \
+   systime.h xterm.h ../lib/unistd.h globals.h
 print.o: print.c process.h frame.h window.h buffer.h keyboard.h character.h \
    lisp.h globals.h $(config_h) termchar.h $(INTERVALS_H) msdos.h termhooks.h \
    blockinput.h atimer.h systime.h font.h charset.h coding.h ccl.h \

=== modified file 'src/fns.c'
--- src/fns.c	2011-06-07 01:39:26 +0000
+++ src/fns.c	2011-06-12 12:32:07 +0000
@@ -4538,21 +4538,23 @@
 
 \f
 /************************************************************************
-			     MD5 and SHA1
+			MD5, SHA-1, and SHA-2
  ************************************************************************/
 
 #include "md5.h"
 #include "sha1.h"
+#include "sha256.h"
+#include "sha512.h"
 
 /* Convert a possibly-signed character to an unsigned character.  This is
    a bit safer than casting to unsigned char, since it catches some type
    errors that the cast doesn't.  */
 static inline unsigned char to_uchar (char ch) { return ch; }
 
-/* TYPE: 0 for md5, 1 for sha1. */
+/* ALGORITHM: 0 for md5, 1 for sha1, 224 for sha224 etc. */
 
 static Lisp_Object
-crypto_hash_function (int type, Lisp_Object object, Lisp_Object start, Lisp_Object end, Lisp_Object coding_system, Lisp_Object noerror, Lisp_Object binary)
+crypto_hash_function (int algorithm, Lisp_Object object, Lisp_Object start, Lisp_Object end, Lisp_Object coding_system, Lisp_Object noerror, Lisp_Object binary)
 {
   int i;
   EMACS_INT size;
@@ -4562,7 +4564,9 @@
   register EMACS_INT b, e;
   register struct buffer *bp;
   EMACS_INT temp;
-  Lisp_Object res=Qnil;
+  int digest_size;
+  void *(*hash_func) (const char *, size_t, void *);
+  Lisp_Object digest;
 
   if (STRINGP (object))
     {
@@ -4733,47 +4737,51 @@
 	object = code_convert_string (object, coding_system, Qnil, 1, 0, 0);
     }
 
-  switch (type)
-    {
-    case 0:			/* MD5 */
-      {
-	char digest[16];
-	md5_buffer (SSDATA (object) + start_byte,
-		    SBYTES (object) - (size_byte - end_byte),
-		    digest);
-
-	if (NILP (binary))
-	  {
-	    char value[33];
-	    for (i = 0; i < 16; i++)
-	      sprintf (&value[2 * i], "%02x", to_uchar (digest[i]));
-	    res = make_string (value, 32);
-	  }
-	else
-	  res = make_string (digest, 16);
-	break;
-      }
-
-    case 1:			/* SHA1 */
-      {
-	char digest[20];
-	sha1_buffer (SSDATA (object) + start_byte,
-		     SBYTES (object) - (size_byte - end_byte),
-		     digest);
-	if (NILP (binary))
-	  {
-	    char value[41];
-	    for (i = 0; i < 20; i++)
-	      sprintf (&value[2 * i], "%02x", to_uchar (digest[i]));
-	    res = make_string (value, 40);
-	  }
-	else
-	  res = make_string (digest, 20);
-	break;
-      }
-    }
-
-  return res;
+  switch (algorithm)
+    {
+    case 0:
+      digest_size = MD5_DIGEST_SIZE;
+      hash_func	  = &md5_buffer;
+      break;
+    case 1:
+      digest_size = SHA1_DIGEST_SIZE;
+      hash_func	  = &sha1_buffer;
+      break;
+    case 224:
+      digest_size = SHA224_DIGEST_SIZE;
+      hash_func	  = &sha224_buffer;
+      break;
+    case 256:
+      digest_size = SHA256_DIGEST_SIZE;
+      hash_func	  = &sha256_buffer;
+      break;
+    case 384:
+      digest_size = SHA384_DIGEST_SIZE;
+      hash_func	  = &sha384_buffer;
+      break;
+    case 512:
+      digest_size = SHA512_DIGEST_SIZE;
+      hash_func	  = &sha512_buffer;
+      break;
+    default:
+      error ("Invalid ALGORITHM argument");
+    }
+
+  digest = make_uninit_string (digest_size);
+
+  hash_func (SSDATA (object) + start_byte,
+             SBYTES (object) - (size_byte - end_byte),
+             SSDATA (digest));
+
+  if (NILP (binary))
+    {
+      Lisp_Object value = make_uninit_string (2 * digest_size);
+      for (i = 0; i < digest_size; i++)
+	sprintf (&SSDATA (value)[2 * i], "%02x", to_uchar (SSDATA (digest)[i]));
+      return value;
+    }
+  else
+    return digest;
 }
 
 DEFUN ("md5", Fmd5, Smd5, 1, 5, 0,
@@ -4808,18 +4816,26 @@
   return crypto_hash_function (0, object, start, end, coding_system, noerror, Qnil);
 }
 
-DEFUN ("sha1", Fsha1, Ssha1, 1, 4, 0,
-       doc: /* Return the SHA-1 (Secure Hash Algorithm) of an OBJECT.
-
-OBJECT is either a string or a buffer.  Optional arguments START and
-END are character positions specifying which portion of OBJECT for
-computing the hash.  If BINARY is non-nil, return a string in binary
-form.  */)
-     (Lisp_Object object, Lisp_Object start, Lisp_Object end, Lisp_Object binary)
+DEFUN ("sha", Fsha, Ssha, 1, 5, 0,
+       doc: /* Return the SHA (Secure Hash Algorithm) checksums of an OBJECT.
+
+ALGORITHM is an integer specifying the algorithm number. It can be 1
+(default), 224, 256, 384 or 512 for SHA-1, SHA-224, SHA-256, SHA-384
+or SHA-512, respectively.
+
+OBJECT is either a string or a buffer.
+
+Optional arguments START and END are character positions specifying
+which portion of OBJECT for computing the hash.  If BINARY is non-nil,
+return a string in binary form.  */)
+  (Lisp_Object object, Lisp_Object start, Lisp_Object end, Lisp_Object binary, Lisp_Object algorithm)
 {
-  return crypto_hash_function (1, object, start, end, Qnil, Qnil, binary);
+  if (NILP (algorithm))
+    XSETFASTINT (algorithm, 1);
+
+  CHECK_NATNUM (algorithm);
+  return crypto_hash_function (XFASTINT (algorithm), object, start, end, Qnil, Qnil, binary);
 }
-
 \f
 void
 syms_of_fns (void)
@@ -4992,7 +5008,7 @@
   defsubr (&Sbase64_encode_string);
   defsubr (&Sbase64_decode_string);
   defsubr (&Smd5);
-  defsubr (&Ssha1);
+  defsubr (&Ssha);
   defsubr (&Slocale_info);
 }
 

=== modified file 'src/makefile.w32-in'
--- src/makefile.w32-in	2011-05-31 17:03:24 +0000
+++ src/makefile.w32-in	2011-06-11 05:30:23 +0000
@@ -869,6 +869,8 @@
 	$(EMACS_ROOT)/nt/inc/sys/time.h \
 	$(EMACS_ROOT)/lib/md5.h \
 	$(EMACS_ROOT)/lib/sha1.h \
+	$(EMACS_ROOT)/lib/sha256.h \
+	$(EMACS_ROOT)/lib/sha512.h \
 	$(LISP_H) \
 	$(SRC)/atimer.h \
 	$(SRC)/blockinput.h \

Re: Adding sha256 and sha512 to C?

[Thien-Thi Nguyen]

() Leo <sdl.web@gmail.com>
() Sun, 12 Jun 2011 21:03:19 +0800

   crypto-hash-function is very long to type.

What does typing have to do w/ Emacs?  :-)

I think more important than the name itself is the
name's prefix, to be shared w/ other related funcs / vars.
For that, "crypto" seems about as good as any.

Re: Adding sha256 and sha512 to C?

[Deniz Dogan]

On 2011-06-12 15:03, Leo wrote:
> crypto-hash-function is very long to type.
>

Not as long as `thing-at-point-bounds-of-list-at-point'!

Re: Adding sha256 and sha512 to C?

[Richard Riley]

Deniz Dogan <deniz@dogan.se> writes:

> On 2011-06-12 15:03, Leo wrote:
>> crypto-hash-function is very long to type.
>>
>
> Not as long as `thing-at-point-bounds-of-list-at-point'!
>
>
>

Thank the lord for auto-complete.

Re: Adding sha256 and sha512 to C?

[Paul Eggert]

On 06/12/11 06:03, Leo wrote:

> (sha OBJECT &optional START END BINARY ALGORITHM)
> 
> where ALGORITHM can be 1 (default), 224, 256, 384, 512, and make sha1
> obsolete? In a sense we unify all SHA functions and leave MD5 as is.

That's better, thanks, but I still have two qualms.  First, the name
"sha" is confusing at the Emacs Lisp level: it feels too much like
"ash".  It's not like programmers will be using crypto functions in
every expression; their names need not be *that* short.  How about the
name "secure-hash" instead?  That's pretty short.

Second, naming algorithms via bit counts doesn't sound
forward-looking.  SHA-3 is likely to have a 512-bit variant, for
example.  How about using atoms to name the algorithms, e.g., SHA-1,
SHA-224, SHA-256, etc.?  This is more likely to be robust after SHA-3
comes out, not to mention SHA-4 etc.

+      hash_func	  = &md5_buffer;

There's no need for the "&" here, or in similar assignments to
hash_func.  (And there's no need for multiple spaces before the "=".)

+  digest = make_uninit_string (digest_size);
...
+      Lisp_Object value = make_uninit_string (2 * digest_size);

There's no need to call make_uninit_string twice, as only one
string is being returned.  Any temporary buffer for the digest can
be put into the C stack.  Or, perhaps better, use the same
uninitialized string for both the binary digest and the text
digest, and run the binary-to-text loop backwards (and without
using sprintf) so that the loop doesn't stomp on its own work.
Something like this:

      unsigned char *p = SDATA (digest);
      for (i = digest_size - 1; i >= 0; i--)
	{
	  static char const hexdigit[16] = "0123456789abcdef";
	  int p_i = p[i];
	  p[2 * i] = hexdigit[p_i >> 4];
	  p[2 * i + 1] = hexdigit[p_i & 0xf];
	}

The text-vs-binary checksum thing seems to be enough of a hassle that
perhaps it should be pulled out into a separate function, rather than
as a flag to the sha/secure-hash function.  That is, secure-hash could
always return the text form, and if someone wants a binary form they
could call the text-to-binary converter.

Won't there need to be changes to the Emacs Lisp reference manual, and
to NEWS?

Re: Adding sha256 and sha512 to C?

[Leo]

[-- Attachment #1: Type: text/plain, Size: 2197 bytes --]

Sorry for the delay.

On 2011-06-13 06:37 +0800, Paul Eggert wrote:
> That's better, thanks, but I still have two qualms.  First, the name
> "sha" is confusing at the Emacs Lisp level: it feels too much like
> "ash".  It's not like programmers will be using crypto functions in
> every expression; their names need not be *that* short.  How about the
> name "secure-hash" instead?  That's pretty short.

Sounds good.

> Second, naming algorithms via bit counts doesn't sound
> forward-looking.  SHA-3 is likely to have a 512-bit variant, for
> example.  How about using atoms to name the algorithms, e.g., SHA-1,
> SHA-224, SHA-256, etc.?  This is more likely to be robust after SHA-3
> comes out, not to mention SHA-4 etc.
>
> +      hash_func	  = &md5_buffer;
>
> There's no need for the "&" here, or in similar assignments to
> hash_func.  (And there's no need for multiple spaces before the "=".)
>
> +  digest = make_uninit_string (digest_size);
> ...
> +      Lisp_Object value = make_uninit_string (2 * digest_size);
>
> There's no need to call make_uninit_string twice, as only one
> string is being returned.  Any temporary buffer for the digest can
> be put into the C stack.  Or, perhaps better, use the same
> uninitialized string for both the binary digest and the text
> digest, and run the binary-to-text loop backwards (and without
> using sprintf) so that the loop doesn't stomp on its own work.
> Something like this:
>
>       unsigned char *p = SDATA (digest);
>       for (i = digest_size - 1; i >= 0; i--)
> 	{
> 	  static char const hexdigit[16] = "0123456789abcdef";
> 	  int p_i = p[i];
> 	  p[2 * i] = hexdigit[p_i >> 4];
> 	  p[2 * i + 1] = hexdigit[p_i & 0xf];
> 	}

Thanks for the suggestion.

> The text-vs-binary checksum thing seems to be enough of a hassle that
> perhaps it should be pulled out into a separate function, rather than
> as a flag to the sha/secure-hash function.  That is, secure-hash could
> always return the text form, and if someone wants a binary form they
> could call the text-to-binary converter.
>
> Won't there need to be changes to the Emacs Lisp reference manual, and
> to NEWS?

Will take care of the NEWS entry when committing.

Leo


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: sha-2.diff --]
[-- Type: text/x-diff, Size: 7995 bytes --]

=== modified file 'lisp/subr.el'
--- lisp/subr.el	2011-06-15 17:30:41 +0000
+++ lisp/subr.el	2011-06-19 16:06:58 +0000
@@ -2600,6 +2600,14 @@
 	(get-char-property (1- (field-end pos)) 'field)
       raw-field)))
 
+(defun sha1 (object &optional start end binary)
+  "Return the SHA1 (Secure Hash Algorithm) of an OBJECT.
+OBJECT is either a string or a buffer.  Optional arguments START and
+END are character positions specifying which portion of OBJECT for
+computing the hash.  If BINARY is non-nil, return a string in binary
+form."
+  (secure-hash 'sha1 object start end binary))
+
 \f
 ;;;; Support for yanking and text properties.
 

=== modified file 'src/deps.mk'
--- src/deps.mk	2011-05-24 08:22:58 +0000
+++ src/deps.mk	2011-06-19 15:46:13 +0000
@@ -284,8 +284,8 @@
 floatfns.o: floatfns.c syssignal.h lisp.h globals.h $(config_h)
 fns.o: fns.c commands.h lisp.h $(config_h) frame.h buffer.h character.h \
    keyboard.h keymap.h window.h $(INTERVALS_H) coding.h ../lib/md5.h \
-   ../lib/sha1.h blockinput.h atimer.h systime.h xterm.h ../lib/unistd.h \
-   globals.h
+   ../lib/sha1.h ../lib/sha256.h ../lib/sha512.h blockinput.h atimer.h \
+   systime.h xterm.h ../lib/unistd.h globals.h
 print.o: print.c process.h frame.h window.h buffer.h keyboard.h character.h \
    lisp.h globals.h $(config_h) termchar.h $(INTERVALS_H) msdos.h termhooks.h \
    blockinput.h atimer.h systime.h font.h charset.h coding.h ccl.h \

=== modified file 'src/fns.c'
--- src/fns.c	2011-06-17 15:18:54 +0000
+++ src/fns.c	2011-06-19 15:46:13 +0000
@@ -51,6 +51,8 @@
 static Lisp_Object Qwidget_type;
 static Lisp_Object Qcodeset, Qdays, Qmonths, Qpaper;
 
+static Lisp_Object Qmd5, Qsha1, Qsha224, Qsha256, Qsha384, Qsha512;
+
 static int internal_equal (Lisp_Object , Lisp_Object, int, int);
 
 #ifndef HAVE_UNISTD_H
@@ -4550,21 +4552,18 @@
 
 \f
 /************************************************************************
-			     MD5 and SHA1
+			MD5, SHA-1, and SHA-2
  ************************************************************************/
 
 #include "md5.h"
 #include "sha1.h"
-
-/* Convert a possibly-signed character to an unsigned character.  This is
-   a bit safer than casting to unsigned char, since it catches some type
-   errors that the cast doesn't.  */
-static inline unsigned char to_uchar (char ch) { return ch; }
-
-/* TYPE: 0 for md5, 1 for sha1. */
+#include "sha256.h"
+#include "sha512.h"
+
+/* ALGORITHM is a symbol: md5, sha1, sha224 and so on. */
 
 static Lisp_Object
-crypto_hash_function (int type, Lisp_Object object, Lisp_Object start, Lisp_Object end, Lisp_Object coding_system, Lisp_Object noerror, Lisp_Object binary)
+secure_hash (Lisp_Object algorithm, Lisp_Object object, Lisp_Object start, Lisp_Object end, Lisp_Object coding_system, Lisp_Object noerror, Lisp_Object binary)
 {
   int i;
   EMACS_INT size;
@@ -4574,7 +4573,11 @@
   register EMACS_INT b, e;
   register struct buffer *bp;
   EMACS_INT temp;
-  Lisp_Object res=Qnil;
+  int digest_size;
+  void *(*hash_func) (const char *, size_t, void *);
+  Lisp_Object digest;
+
+  CHECK_SYMBOL (algorithm);
 
   if (STRINGP (object))
     {
@@ -4745,47 +4748,61 @@
 	object = code_convert_string (object, coding_system, Qnil, 1, 0, 0);
     }
 
-  switch (type)
-    {
-    case 0:			/* MD5 */
-      {
-	char digest[16];
-	md5_buffer (SSDATA (object) + start_byte,
-		    SBYTES (object) - (size_byte - end_byte),
-		    digest);
-
-	if (NILP (binary))
-	  {
-	    char value[33];
-	    for (i = 0; i < 16; i++)
-	      sprintf (&value[2 * i], "%02x", to_uchar (digest[i]));
-	    res = make_string (value, 32);
-	  }
-	else
-	  res = make_string (digest, 16);
-	break;
-      }
-
-    case 1:			/* SHA1 */
-      {
-	char digest[20];
-	sha1_buffer (SSDATA (object) + start_byte,
-		     SBYTES (object) - (size_byte - end_byte),
-		     digest);
-	if (NILP (binary))
-	  {
-	    char value[41];
-	    for (i = 0; i < 20; i++)
-	      sprintf (&value[2 * i], "%02x", to_uchar (digest[i]));
-	    res = make_string (value, 40);
-	  }
-	else
-	  res = make_string (digest, 20);
-	break;
-      }
-    }
-
-  return res;
+  if (EQ (algorithm, Qmd5))
+    {
+      digest_size = MD5_DIGEST_SIZE;
+      hash_func	  = md5_buffer;
+    }
+  else if (EQ (algorithm, Qsha1))
+    {
+      digest_size = SHA1_DIGEST_SIZE;
+      hash_func	  = sha1_buffer;
+    }
+  else if (EQ (algorithm, Qsha224))
+    {
+      digest_size = SHA224_DIGEST_SIZE;
+      hash_func	  = sha224_buffer;
+    }
+  else if (EQ (algorithm, Qsha256))
+    {
+      digest_size = SHA256_DIGEST_SIZE;
+      hash_func	  = sha256_buffer;
+    }
+  else if (EQ (algorithm, Qsha384))
+    {
+      digest_size = SHA384_DIGEST_SIZE;
+      hash_func	  = sha384_buffer;
+    }
+  else if (EQ (algorithm, Qsha512))
+    {
+      digest_size = SHA512_DIGEST_SIZE;
+      hash_func	  = sha512_buffer;
+    }
+  else
+    error ("Invalid algorithm arg: %s", SDATA (Fsymbol_name (algorithm)));
+
+  /* allocate 2 times the size of digest_size so that it can be
+     re-used to hold the hexified value */
+  digest = make_uninit_string (digest_size * 2);
+
+  hash_func (SSDATA (object) + start_byte,
+	     SBYTES (object) - (size_byte - end_byte),
+	     SSDATA (digest));
+
+  if (NILP (binary))
+    {
+      unsigned char *p = SDATA (digest);
+      for (i = digest_size - 1; i >= 0; i--)
+	{
+	  static char const hexdigit[16] = "0123456789abcdef";
+	  int p_i = p[i];
+	  p[2 * i] = hexdigit[p_i >> 4];
+	  p[2 * i + 1] = hexdigit[p_i & 0xf];
+	}
+      return digest;
+    }
+  else
+    return make_unibyte_string (SDATA (digest), digest_size);
 }
 
 DEFUN ("md5", Fmd5, Smd5, 1, 5, 0,
@@ -4817,25 +4834,31 @@
 guesswork fails.  Normally, an error is signaled in such case.  */)
   (Lisp_Object object, Lisp_Object start, Lisp_Object end, Lisp_Object coding_system, Lisp_Object noerror)
 {
-  return crypto_hash_function (0, object, start, end, coding_system, noerror, Qnil);
+  return secure_hash (Qmd5, object, start, end, coding_system, noerror, Qnil);
 }
 
-DEFUN ("sha1", Fsha1, Ssha1, 1, 4, 0,
-       doc: /* Return the SHA-1 (Secure Hash Algorithm) of an OBJECT.
-
-OBJECT is either a string or a buffer.  Optional arguments START and
-END are character positions specifying which portion of OBJECT for
-computing the hash.  If BINARY is non-nil, return a string in binary
-form.  */)
-     (Lisp_Object object, Lisp_Object start, Lisp_Object end, Lisp_Object binary)
+DEFUN ("secure-hash", Fsecure_hash, Ssecure_hash, 2, 5, 0,
+       doc: /* Return the secure hash of an OBJECT.
+ALGORITHM is a symbol: md5, sha1, sha224, sha256, sha384 or sha512.
+OBJECT is either a string or a buffer.
+Optional arguments START and END are character positions specifying
+which portion of OBJECT for computing the hash.  If BINARY is non-nil,
+return a string in binary form.  */)
+  (Lisp_Object algorithm, Lisp_Object object, Lisp_Object start, Lisp_Object end, Lisp_Object binary)
 {
-  return crypto_hash_function (1, object, start, end, Qnil, Qnil, binary);
+  return secure_hash (algorithm, object, start, end, Qnil, Qnil, binary);
 }
-
 \f
 void
 syms_of_fns (void)
 {
+  DEFSYM (Qmd5,    "md5");
+  DEFSYM (Qsha1,   "sha1");
+  DEFSYM (Qsha224, "sha224");
+  DEFSYM (Qsha256, "sha256");
+  DEFSYM (Qsha384, "sha384");
+  DEFSYM (Qsha512, "sha512");
+
   /* Hash table stuff.  */
   Qhash_table_p = intern_c_string ("hash-table-p");
   staticpro (&Qhash_table_p);
@@ -5004,7 +5027,7 @@
   defsubr (&Sbase64_encode_string);
   defsubr (&Sbase64_decode_string);
   defsubr (&Smd5);
-  defsubr (&Ssha1);
+  defsubr (&Ssecure_hash);
   defsubr (&Slocale_info);
 }
 

=== modified file 'src/makefile.w32-in'
--- src/makefile.w32-in	2011-06-12 02:48:18 +0000
+++ src/makefile.w32-in	2011-06-19 15:46:13 +0000
@@ -867,6 +867,8 @@
 	$(EMACS_ROOT)/nt/inc/sys/time.h \
 	$(EMACS_ROOT)/lib/md5.h \
 	$(EMACS_ROOT)/lib/sha1.h \
+	$(EMACS_ROOT)/lib/sha256.h \
+	$(EMACS_ROOT)/lib/sha512.h \
 	$(LISP_H) \
 	$(SRC)/atimer.h \
 	$(SRC)/blockinput.h \

Re: Adding sha256 and sha512 to C?

[Leo]

On 2011-05-28 11:58 +0800, Paul Eggert wrote:
> Sounds good, but rather than continue to add crypto functions
> wouldn't it be better to have a single function parameterized by the
> algorithm name?  Something like the following signature:
>
>  (crypto-hash-function ALGORITHM OBJECT
>     &optional START END CODING-SYSTEM NOERROR BINARY)

I like this too.

> Then, we could implement the existing functions this way:
>
> (defun md5 (object &optional start end coding-system noerror)
>    (crypto-hash-function 'md5 object start end coding-system noerror nil))
>
> (define sha1 (object &optional start end binary)
>    (crypto-hash-function 'sha1 object start end nil nil binary))

I think we should have a consistent signature for all these functions. I
propose:

  (OBJECT &optional START END CODING-SYSTEM BINARY)

omitting the NOERROR arg to make the functions easier to call.

Leo

Re: Adding sha256 and sha512 to C?

[Paul Eggert]

On 05/28/11 21:22, Leo wrote:
> I think we should have a consistent signature for all these functions.

The idea of changing the signatures for md5 and sha1 got shot down,
though, due to backward compatibility concerns.
<http://lists.gnu.org/archive/html/emacs-devel/2011-05/msg00818.html>

Perhaps we can revisit that idea later, after crypto-hash-function
is added.  It is a separate issue, and can be treated separately.