[svn] gcry - r1369 - in trunk: . mpi random tests

svn author wk cvs at cvs.gnupg.org
Fri Dec 5 09:46:02 CET 2008


Author: wk
Date: 2008-12-05 09:46:01 +0100 (Fri, 05 Dec 2008)
New Revision: 1369

Modified:
   trunk/TODO
   trunk/mpi/ChangeLog
   trunk/mpi/mpicoder.c
   trunk/random/random-csprng.c
   trunk/tests/benchmark.c
Log:
Fixed error cases in mpicoder.
Documentation cleanups.


Modified: trunk/mpi/ChangeLog
===================================================================
--- trunk/mpi/ChangeLog	2008-12-04 16:24:23 UTC (rev 1368)
+++ trunk/mpi/ChangeLog	2008-12-05 08:46:01 UTC (rev 1369)
@@ -1,5 +1,15 @@
-2008-12-04  Werner Koch  <wk at g10code.com>
+2008-12-05  Werner Koch  <wk at g10code.com>
 
+	* mpicoder.c (mpi_read_from_buffer): Do not bail out if the mpi is
+	larger than the buffer (potential problem).  Do not print error
+	messages.
+	(mpi_fromstr): Return an error instead of hitting an assert.
+	(gcry_mpi_scan) <PGP>: Fix potential double free problem.
+	(gcry_mpi_scan) <HEX>: Fix potential memory leak.
+	(do_get_buffer): Return NULL on memory allocation failure.
+	(gcry_mpi_print): Check result of do_get_buffer.
+	(gcry_mpi_aprint): Return error on a memory allocation failure.
+
 	* mpicoder.c: Re-indent.
 
 2008-12-03  Werner Koch  <wk at g10code.com>

Modified: trunk/TODO
===================================================================
--- trunk/TODO	2008-12-04 16:24:23 UTC (rev 1368)
+++ trunk/TODO	2008-12-05 08:46:01 UTC (rev 1369)
@@ -36,9 +36,6 @@
   collectros need to run that bunch of Unix utilities we don't waste
   their precious results.
 
-* mpi_print does not use secure memory
-  for internal variables.
-
 * Add OAEP
 
 * gcryptrnd.c

Modified: trunk/mpi/mpicoder.c
===================================================================
--- trunk/mpi/mpicoder.c	2008-12-04 16:24:23 UTC (rev 1368)
+++ trunk/mpi/mpicoder.c	2008-12-05 08:46:01 UTC (rev 1369)
@@ -1,6 +1,6 @@
 /* mpicoder.c  -  Coder for the external representation of MPIs
- * Copyright (C) 1998, 1999, 2000, 2001, 2002,
- *               2003 Free Software Foundation, Inc.
+ * Copyright (C) 1998, 1999, 2000, 2001, 2002, 2003
+ *               2008 Free Software Foundation, Inc.
  *
  * This file is part of Libgcrypt.
  *
@@ -43,12 +43,12 @@
   nbits = buffer[0] << 8 | buffer[1];
   if ( nbits > MAX_EXTERN_MPI_BITS )
     {
-      log_error ("mpi too large (%u bits)\n", nbits);
+/*       log_debug ("mpi too large (%u bits)\n", nbits); */
       goto leave;
     }
   else if( !nbits ) 
     {
-      log_error ("an mpi of size 0 is not allowed\n");
+/*       log_debug ("an mpi of size 0 is not allowed\n"); */
       goto leave;
     }
   buffer += 2;
@@ -56,7 +56,7 @@
 
   nbytes = (nbits+7) / 8;
   nlimbs = (nbytes+BYTES_PER_MPI_LIMB-1) / BYTES_PER_MPI_LIMB;
-  val = secure? mpi_alloc_secure (nlimbs) : mpi_alloc( nlimbs );
+  val = secure? mpi_alloc_secure (nlimbs) : mpi_alloc (nlimbs);
   i = BYTES_PER_MPI_LIMB - nbytes % BYTES_PER_MPI_LIMB;
   i %= BYTES_PER_MPI_LIMB;
   j= val->nlimbs = nlimbs;
@@ -67,7 +67,12 @@
       for (; i < BYTES_PER_MPI_LIMB; i++ ) 
         {
           if ( ++nread > *ret_nread )
-            log_bug ("mpi larger than buffer");
+            {
+/*               log_debug ("mpi larger than buffer"); */
+              mpi_free (val);
+              val = NULL;
+              goto leave;
+            }
           a <<= 8;
           a |= *buffer++;
 	}
@@ -82,7 +87,7 @@
 
 
 /****************
- * Make an mpi from a hex character string.
+ * Fill the mpi VAL from the hex string in STR.
  */
 static int
 mpi_fromstr (gcry_mpi_t val, const char *str)
@@ -130,9 +135,17 @@
           else
             c1 = *str++;
 
-          gcry_assert (c1);
+          if (!c1)
+            {
+              mpi_clear (val);
+              return 1;  /* Error.  */
+	    }
           c2 = *str++;
-          gcry_assert (c2);
+          if (!c2)
+            {
+              mpi_clear (val);
+              return 1;  /* Error.  */
+	    }
           if ( c1 >= '0' && c1 <= '9' )
             c = c1 - '0';
           else if ( c1 >= 'a' && c1 <= 'f' )
@@ -216,9 +229,10 @@
 
 /* Return an allocated buffer with the MPI (msb first).  NBYTES
    receives the length of this buffer.  Caller must free the return
-   string.  This function does return a 0 byte buffer with NBYTES set
+   string.  This function returns an allocated buffer with NBYTES set
    to zero if the value of A is zero.  If sign is not NULL, it will be
-   set to the sign of the A.  */
+   set to the sign of the A.  On error NULL is returned and ERRNO set
+   appropriately.  */
 static unsigned char *
 do_get_buffer (gcry_mpi_t a, unsigned int *nbytes, int *sign, int force_secure)
 {
@@ -232,8 +246,10 @@
 
   *nbytes = a->nlimbs * BYTES_PER_MPI_LIMB;
   n = *nbytes? *nbytes:1; /* Allocate at least one byte.  */
-  p = buffer = force_secure || mpi_is_secure(a) ? gcry_xmalloc_secure(n)
-						  : gcry_xmalloc(n);
+  p = buffer = (force_secure || mpi_is_secure(a))? gcry_malloc_secure (n)
+						 : gcry_malloc (n);
+  if (!buffer)
+    return NULL;
 
   for (i=a->nlimbs-1; i >= 0; i--)
     {
@@ -399,7 +415,7 @@
 	}
       else
         mpi_free(a);
-      return gcry_error (GPG_ERR_NO_ERROR);
+      return 0;
     }
   else if (format == GCRYMPI_FMT_USG)
     {
@@ -428,9 +444,12 @@
           mpi_normalize (a);
           *ret_mpi = a;
 	}
-      else
-        mpi_free(a);
-      return gcry_error (a ? GPG_ERR_NO_ERROR : GPG_ERR_INV_OBJ);
+      else if (a)
+        {
+          mpi_free(a);
+          a = NULL;
+        }
+      return a? 0 : gcry_error (GPG_ERR_INV_OBJ);
     }
   else if (format == GCRYMPI_FMT_SSH)
     {
@@ -464,14 +483,14 @@
 	}
       if (nscanned)
         *nscanned = n+4;
-	if (ret_mpi)
-          {
-	    mpi_normalize ( a );
-	    *ret_mpi = a;
-          }
-	else
-          mpi_free(a);
-	return 0;
+      if (ret_mpi)
+        {
+          mpi_normalize ( a );
+          *ret_mpi = a;
+        }
+      else
+        mpi_free(a);
+      return 0;
     }
   else if (format == GCRYMPI_FMT_HEX)
     {
@@ -481,7 +500,10 @@
 
       a = secure? mpi_alloc_secure (0) : mpi_alloc(0);
       if (mpi_fromstr (a, (const char *)buffer))
-        return gcry_error (GPG_ERR_INV_OBJ);
+        {
+          mpi_free (a);
+          return gcry_error (GPG_ERR_INV_OBJ);
+        }
       if (ret_mpi) 
         {
           mpi_normalize ( a );
@@ -526,6 +548,8 @@
         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 (n && (*tmp & 0x80))
         {
           n++;
@@ -564,6 +588,8 @@
           unsigned char *tmp;
 
           tmp = _gcry_mpi_get_buffer (a, &n, NULL);
+          if (!tmp)
+            return gpg_error_from_syserror ();
           memcpy (buffer, tmp, n);
           gcry_free (tmp);
 	}
@@ -590,6 +616,8 @@
           s[1] = nbits;
           
           tmp = _gcry_mpi_get_buffer (a, &n, NULL);
+          if (!tmp)
+            return gpg_error_from_syserror ();
           memcpy (s+2, tmp, n);
           gcry_free (tmp);
 	}
@@ -606,6 +634,8 @@
         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 (n && (*tmp & 0x80))
         {
           n++;
@@ -643,6 +673,8 @@
       unsigned int n = 0;
       
       tmp = _gcry_mpi_get_buffer (a, &n, NULL);
+      if (!tmp)
+        return gpg_error_from_syserror ();
       if (!n || (*tmp & 0x80))
         extra = 2;
 
@@ -704,7 +736,9 @@
   if (rc)
     return rc;
 
-  *buffer = mpi_is_secure(a) ? gcry_xmalloc_secure( n ) : gcry_xmalloc( n );
+  *buffer = mpi_is_secure(a) ? gcry_malloc_secure (n) : gcry_malloc (n);
+  if (!*buffer)
+    return gpg_error_from_syserror ();
   rc = gcry_mpi_print( format, *buffer, n, &n, a );
   if (rc)
     {

Modified: trunk/random/random-csprng.c
===================================================================
--- trunk/random/random-csprng.c	2008-12-04 16:24:23 UTC (rev 1368)
+++ trunk/random/random-csprng.c	2008-12-05 08:46:01 UTC (rev 1369)
@@ -1,4 +1,4 @@
-/* random-csprng.c - CSPRNG style random number generator (libgcrypt clssic)
+/* random-csprng.c - CSPRNG style random number generator (libgcrypt classic)
  * Copyright (C) 1998, 2000, 2001, 2002, 2003, 2004, 2005, 2006,
  *               2007, 2008  Free Software Foundation, Inc.
  *
@@ -20,9 +20,15 @@
 
 /*
    This random number generator is modelled after the one described in
-   Peter Gutmann's paper: "Software Generation of Practically Strong
-   Random Numbers". See also chapter 6 in his book "Cryptographic
-   Security Architecture", New York, 2004, ISBN 0-387-95387-6.
+   Peter Gutmann's 1998 Usenix Security Symposium paper: "Software
+   Generation of Practically Strong Random Numbers".  See also chapter
+   6 in his book "Cryptographic Security Architecture", New York,
+   2004, ISBN 0-387-95387-6.
+
+   Note that the acronym CSPRNG stands for "Continuously Seeded
+   PseudoRandom Number Generator" as used in Peter's implementation of
+   the paper and not only for "Cryptographically Secure PseudoRandom
+   Number Generator".
  */
 
 

Modified: trunk/tests/benchmark.c
===================================================================
--- trunk/tests/benchmark.c	2008-12-04 16:24:23 UTC (rev 1368)
+++ trunk/tests/benchmark.c	2008-12-05 08:46:01 UTC (rev 1369)
@@ -640,7 +640,7 @@
       fflush (stdout);
 
       err = gcry_sexp_build (&key_spec, NULL,
-                             gcry_control (GCRYCTL_FIPS_MODE_P, 0)
+                             gcry_fips_mode_active ()
                              ? "(genkey (RSA (nbits %d)))"
                              : "(genkey (RSA (nbits %d)(transient-key)))",
                              p_sizes[testno]);




More information about the Gnupg-commits mailing list