mpi: Make gcry_mpi_print work with negative zeroes.

* mpi/mpicoder.c (gcry_mpi_print): Take care of negative zero.
(gcry_mpi_aprint): Allocate at least 1 byte.
* tests/t-convert.c: New.
* tests/Makefile.am (TESTS): Add t-convert.
--

Reported-by: Christian Fuchs
Signed-off-by: Werner Koch <wk@gnupg.org>

3 files changed, 188 insertions, 10 deletions

diff --git a/mpi/mpicoder.c b/mpi/mpicoder.c
index aca3710e..a9259228 100644
--- a/mpi/mpicoder.c
+++ b/mpi/mpicoder.c
@@ -1,6 +1,7 @@
 /* mpicoder.c  -  Coder for the external representation of MPIs
  * Copyright (C) 1998, 1999, 2000, 2001, 2002, 2003
  *               2008 Free Software Foundation, Inc.
+ * Copyright (C) 2013 g10 Code GmbH
  *
  * This file is part of Libgcrypt.
  *
@@ -178,7 +179,8 @@ mpi_fromstr (gcry_mpi_t val, const char *str)
 /* Dump the value of A in a format suitable for debugging to
    Libgcrypt's logging stream.  Note that one leading space but no
    trailing space or linefeed will be printed.  It is okay to pass
-   NULL for A. */
+   NULL for A.  Note that this function prints the sign as it is used
+   internally and won't map -0 to 0. */
 void
 gcry_mpi_dump (const gcry_mpi_t a)
 {
@@ -548,10 +550,21 @@ gcry_mpi_print (enum gcry_mpi_format format,
   unsigned int nbits = mpi_get_nbits (a);
   size_t len;
   size_t dummy_nwritten;
+  int negative;
 
   if (!nwritten)
     nwritten = &dummy_nwritten;
 
+  /* Libgcrypt does no always care to set clear the sign if the value
+     is 0.  For printing this is a bit of a surprise, in particular
+     because if some of the formats don't support negative numbers but
+     should be able to print a zero.  Thus we need this extra test
+     for a negative number.  */
+  if (a->sign && _gcry_mpi_cmp_ui (a, 0))
+    negative = 1;
+  else
+    negative = 0;
+
   len = buflen;
   *nwritten = 0;
   if (format == GCRYMPI_FMT_STD)
@@ -560,16 +573,20 @@ gcry_mpi_print (enum gcry_mpi_format format,
       int extra = 0;
       unsigned int n;
 
-      if (a->sign)
+      if (negative)
         return gcry_error (GPG_ERR_INTERNAL); /* Can't handle it yet. */
 
       tmp = _gcry_mpi_get_buffer (a, &n, NULL);
       if (!tmp)
         return gpg_error_from_syserror ();
+
+      /* If the high bit of the returned buffer is set, we need to
+         print an extra leading 0x00 so that the output is interpreted
+         as a positive number.  */
       if (n && (*tmp & 0x80))
         {
           n++;
-          extra=1;
+          extra = 1;
 	}
 
       if (buffer && n > len)
@@ -597,6 +614,7 @@ gcry_mpi_print (enum gcry_mpi_format format,
       /* Note:  We ignore the sign for this format.  */
       /* FIXME: for performance reasons we should put this into
 	 mpi_aprint because we can then use the buffer directly.  */
+
       if (buffer && n > len)
         return gcry_error (GPG_ERR_TOO_SHORT);
       if (buffer)
@@ -617,7 +635,7 @@ gcry_mpi_print (enum gcry_mpi_format format,
       unsigned int n = (nbits + 7)/8;
 
       /* The PGP format can only handle unsigned integers.  */
-      if( a->sign )
+      if (negative)
         return gcry_error (GPG_ERR_INV_ARG);
 
       if (buffer && n+2 > len)
@@ -646,7 +664,7 @@ gcry_mpi_print (enum gcry_mpi_format format,
       int extra = 0;
       unsigned int n;
 
-      if (a->sign)
+      if (negative)
         return gcry_error (GPG_ERR_INTERNAL); /* Can't handle it yet.  */
 
       tmp = _gcry_mpi_get_buffer (a, &n, NULL);
@@ -694,7 +712,7 @@ gcry_mpi_print (enum gcry_mpi_format format,
       if (!n || (*tmp & 0x80))
         extra = 2;
 
-      if (buffer && 2*n + extra + !!a->sign + 1 > len)
+      if (buffer && 2*n + extra + negative + 1 > len)
         {
           gcry_free(tmp);
           return gcry_error (GPG_ERR_TOO_SHORT);
@@ -703,7 +721,7 @@ gcry_mpi_print (enum gcry_mpi_format format,
         {
           unsigned char *s = buffer;
 
-          if (a->sign)
+          if (negative)
             *s++ = '-';
           if (extra)
             {
@@ -724,7 +742,7 @@ gcry_mpi_print (enum gcry_mpi_format format,
 	}
       else
         {
-          *nwritten = 2*n + extra + !!a->sign + 1;
+          *nwritten = 2*n + extra + negative + 1;
 	}
       gcry_free (tmp);
       return 0;
@@ -752,7 +770,7 @@ gcry_mpi_aprint (enum gcry_mpi_format format,
   if (rc)
     return rc;
 
-  *buffer = mpi_is_secure(a) ? gcry_malloc_secure (n) : gcry_malloc (n);
+  *buffer = mpi_is_secure(a) ? gcry_malloc_secure (n?n:1) : gcry_malloc (n?n:1);
   if (!*buffer)
     return gpg_error_from_syserror ();
   rc = gcry_mpi_print( format, *buffer, n, &n, a );
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 871e32b7..35bcd60b 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -18,7 +18,7 @@
 
 ## Process this file with automake to produce Makefile.in
 
-TESTS = version t-mpi-bit t-mpi-point prime basic \
+TESTS = version t-convert t-mpi-bit t-mpi-point prime basic \
         mpitests tsexp keygen pubkey hmac keygrip fips186-dsa aeswrap \
 	curves t-kdf pkcs1v2 random dsa-rfc6979
 
diff --git a/tests/t-convert.c b/tests/t-convert.c
new file mode 100644
index 00000000..b7f97fa3
--- /dev/null
+++ b/tests/t-convert.c
@@ -0,0 +1,160 @@
+/* t-convert.c  - Tests for mpi print and scna functions
+ * Copyright (C) 2013 g10 Code GmbH
+ *
+ * This file is part of Libgcrypt.
+ *
+ * Libgcrypt is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as
+ * published by the Free Software Foundation; either version 2.1 of
+ * the License, or (at your option) any later version.
+ *
+ * Libgcrypt is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifdef HAVE_CONFIG_H
+# include <config.h>
+#endif
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <assert.h>
+#include <stdarg.h>
+
+#include "../src/gcrypt-int.h"
+
+#define PGM "t-mpi-point"
+
+static const char *wherestr;
+static int verbose;
+static int debug;
+static int error_count;
+
+
+#define xmalloc(a)    gcry_xmalloc ((a))
+#define xcalloc(a,b)  gcry_xcalloc ((a),(b))
+#define xfree(a)      gcry_free ((a))
+#define pass() do { ; } while (0)
+
+static void
+show (const char *format, ...)
+{
+  va_list arg_ptr;
+
+  if (!verbose)
+    return;
+  fprintf (stderr, "%s: ", PGM);
+  va_start (arg_ptr, format);
+  vfprintf (stderr, format, arg_ptr);
+  va_end (arg_ptr);
+}
+
+static void
+fail (const char *format, ...)
+{
+  va_list arg_ptr;
+
+  fflush (stdout);
+  fprintf (stderr, "%s: ", PGM);
+  if (wherestr)
+    fprintf (stderr, "%s: ", wherestr);
+  va_start (arg_ptr, format);
+  vfprintf (stderr, format, arg_ptr);
+  va_end (arg_ptr);
+  error_count++;
+}
+
+static void
+die (const char *format, ...)
+{
+  va_list arg_ptr;
+
+  fflush (stdout);
+  fprintf (stderr, "%s: ", PGM);
+  if (wherestr)
+    fprintf (stderr, "%s: ", wherestr);
+  va_start (arg_ptr, format);
+  vfprintf (stderr, format, arg_ptr);
+  va_end (arg_ptr);
+  exit (1);
+}
+
+
+/* Check that mpi_print does not return a negative zero.  */
+static void
+negative_zero (void)
+{
+  gpg_error_t err;
+  gcry_mpi_t a;
+  char *buf;
+  void *bufaddr = &buf;
+  struct { const char *name; enum gcry_mpi_format format; } fmts[] =
+    {
+      { "STD", GCRYMPI_FMT_STD },
+      { "PGP", GCRYMPI_FMT_PGP },
+      { "SSH", GCRYMPI_FMT_SSH },
+      { "HEX", GCRYMPI_FMT_HEX },
+      { "USG", GCRYMPI_FMT_USG },
+      { NULL, 0 }
+    };
+  int i;
+
+  a = gcry_mpi_new (0);
+  for (i=0; fmts[i].name; i++)
+    {
+      err = gcry_mpi_aprint (fmts[i].format, bufaddr, NULL, a);
+      if (err)
+        fail ("error printing a zero as %s: %s\n",
+              fmts[i].name,gpg_strerror (err) );
+      else
+        gcry_free (buf);
+    }
+
+  /* With the current version of libgcrypt the next two statements
+     should set a to -0. */
+  gcry_mpi_sub_ui (a, a, 1);
+  gcry_mpi_add_ui (a, a, 1);
+
+  for (i=0; fmts[i].name; i++)
+    {
+      err = gcry_mpi_aprint (fmts[i].format, bufaddr, NULL, a);
+      if (err)
+        fail ("error printing a negative zero as %s: %s\n",
+              fmts[i].name,gpg_strerror (err) );
+      else
+        gcry_free (buf);
+    }
+
+  gcry_mpi_release (a);
+}
+
+
+
+
+int
+main (int argc, char **argv)
+{
+  if (argc > 1 && !strcmp (argv[1], "--verbose"))
+    verbose = 1;
+  else if (argc > 1 && !strcmp (argv[1], "--debug"))
+    verbose = debug = 1;
+
+  if (!gcry_check_version (GCRYPT_VERSION))
+    die ("version mismatch\n");
+
+  gcry_control (GCRYCTL_DISABLE_SECMEM, 0);
+  gcry_control (GCRYCTL_ENABLE_QUICK_RANDOM, 0);
+  if (debug)
+    gcry_control (GCRYCTL_SET_DEBUG_FLAGS, 1u, 0);
+  gcry_control (GCRYCTL_INITIALIZATION_FINISHED, 0);
+
+  negative_zero ();
+
+  show ("All tests completed. Errors: %d\n", error_count);
+  return error_count ? 1 : 0;
+}