Let gcry_pk_decrypt in non-raw mode return a verbatim buffer.
authorWerner Koch <wk@gnupg.org>
Tue, 31 May 2011 09:08:12 +0000 (11:08 +0200)
committerWerner Koch <wk@gnupg.org>
Tue, 31 May 2011 09:08:12 +0000 (11:08 +0200)
The non-raw modes of gcry_pk_decrypt (i.e. pkcs1 or oaep un-padding)
are new and thus we can still change the semantics.

They now return a verbatim buffer and not anything which internally
has been interpreted as a signed integer.  In raw mode we still stick
to the old semantics which is usually fine because it is mostly used
with pkcs#1 padding and that guarantees that the return value may
never be interpreted as a signed MPI or shorted due to block type used
as the second byte.

cipher/ChangeLog
cipher/pubkey.c

index 7fda3dc..c50ce55 100644 (file)
@@ -1,3 +1,9 @@
+2011-05-31  Werner Koch  <wk@g10code.com>
+
+       * pubkey.c (pkcs1_encode_for_signature, oaep_decode): Change
+       return value from one MPI to a buffer.
+       (gcry_pk_decrypt): Adjust for this change.
+
 2011-05-30  Werner Koch  <wk@g10code.com>
 
        * pubkey.c (pkcs1_decode_for_encryption): Change handling of
index b1b53f6..0d8b082 100644 (file)
@@ -875,18 +875,19 @@ pkcs1_encode_for_encryption (gcry_mpi_t *r_result, unsigned int nbits,
 
 /* Decode a plaintext in VALUE assuming pkcs#1 block type 2 padding.
    NBITS is the size of the secret key.  On success the result is
-   stored as a new MPI at R_RESULT.  On error the value at R_RESULT is
-   undefined.  */
+   stored as a newly allocated buffer at R_RESULT and its valid length at
+   R_RESULTLEN.  On error NULL is stored at R_RESULT.  */
 static gcry_err_code_t
-pkcs1_decode_for_encryption (gcry_mpi_t *r_result, unsigned int nbits,
-                            gcry_mpi_t value)
+pkcs1_decode_for_encryption (unsigned char **r_result, size_t *r_resultlen,
+                             unsigned int nbits, gcry_mpi_t value)
 {
-  gcry_err_code_t rc = 0;
   gcry_error_t err;
   unsigned char *frame = NULL;
   size_t nframe = (nbits+7) / 8;
   size_t n;
 
+  *r_result = NULL;
+
   if ( !(frame = gcry_malloc_secure (nframe)))
     return gpg_err_code_from_syserror ();
 
@@ -930,15 +931,17 @@ pkcs1_decode_for_encryption (gcry_mpi_t *r_result, unsigned int nbits,
     }
   n++; /* Skip the zero byte.  */
 
-  err = gcry_mpi_scan (r_result, GCRYMPI_FMT_USG, &frame[n], nframe - n, NULL);
-  if (err)
-    rc = gcry_err_code (err);
-  else if (DBG_CIPHER)
-    log_mpidump ("value extracted from PKCS#1 block type 2 encoded data",
-                *r_result);
-  gcry_free (frame);
+  /* To avoid an extra allocation we reuse the frame buffer.  The only
+     caller of this function will anyway free the result soon.  */
+  memmove (frame, frame + n, nframe - n);
+  *r_result = frame;
+  *r_resultlen = nframe - n;
 
-  return rc;
+  if (DBG_CIPHER)
+    log_printhex ("value extracted from PKCS#1 block type 2 encoded data:",
+                  *r_result, *r_resultlen);
+
+  return 0;
 }
 
 
@@ -1144,7 +1147,8 @@ oaep_encode (gcry_mpi_t *r_result, unsigned int nbits, int algo,
 }
 
 static gcry_err_code_t
-oaep_decode (gcry_mpi_t *r_result, unsigned int nbits, int algo,
+oaep_decode (unsigned char **r_result, size_t *r_resultlen,
+             unsigned int nbits, int algo,
              gcry_mpi_t value, const unsigned char *label, size_t labellen)
 {
   gcry_err_code_t rc = 0;
@@ -1155,6 +1159,8 @@ oaep_decode (gcry_mpi_t *r_result, unsigned int nbits, int algo,
   gcry_md_hd_t hd;
   size_t n;
 
+  *r_result = NULL;
+
   if ( !(frame = gcry_malloc_secure (nframe)))
     return gpg_err_code_from_syserror ();
 
@@ -1225,14 +1231,18 @@ oaep_decode (gcry_mpi_t *r_result, unsigned int nbits, int algo,
     }
 
   n++;
-  err = gcry_mpi_scan (r_result, GCRYMPI_FMT_USG, &frame[n], nframe - n, NULL);
-  if (err)
-    rc = gcry_err_code (err);
-  else if (DBG_CIPHER)
-    log_mpidump ("value extracted from OAEP encoded data", *r_result);
-  gcry_free (frame);
 
-  return rc;
+  /* To avoid an extra allocation we reuse the frame buffer.  The only
+     caller of this function will anyway free the result soon.  */
+  memmove (frame, frame + n, nframe - n);
+  *r_result = frame;
+  *r_resultlen = nframe - n;
+
+  if (DBG_CIPHER)
+    log_printhex ("value extracted from OAEP encoded data:",
+                  *r_result, *r_resultlen);
+
+  return 0;
 }
 
 /* Internal function.   */
@@ -2259,12 +2269,18 @@ gcry_pk_encrypt (gcry_sexp_t *r_ciph, gcry_sexp_t s_data, gcry_sexp_t s_pkey)
    s_skey = <key-as-defined-in-sexp_to_key>
    r_plain= Either an incomplete S-expression without the parentheses
             or if the flags list is used (even if empty) a real S-expression:
-            (value PLAIN).
+            (value PLAIN).  In raw mode (or no flags given) the returned value
+            is to be interpreted as a signed MPI, thus it may have an extra
+            leading zero octet even if not included in the original data.
+            With pkcs1 or oaep decoding enabled the returned value is a
+            verbatim octet string.
  */
 gcry_error_t
 gcry_pk_decrypt (gcry_sexp_t *r_plain, gcry_sexp_t s_data, gcry_sexp_t s_skey)
 {
-  gcry_mpi_t *skey = NULL, *data = NULL, plain = NULL, unpad = NULL;
+  gcry_mpi_t *skey = NULL, *data = NULL, plain = NULL;
+  unsigned char *unpad = NULL;
+  size_t unpadlen = 0;
   int modern, flags;
   struct pk_encoding_ctx ctx;
   gcry_err_code_t rc;
@@ -2297,37 +2313,44 @@ gcry_pk_decrypt (gcry_sexp_t *r_plain, gcry_sexp_t s_data, gcry_sexp_t s_skey)
   switch (ctx.encoding)
     {
     case PUBKEY_ENC_PKCS1:
-      rc = pkcs1_decode_for_encryption (&unpad, gcry_pk_get_nbits (s_skey),
-                                       plain);
-      if (rc)
-       goto leave;
+      rc = pkcs1_decode_for_encryption (&unpad, &unpadlen,
+                                        gcry_pk_get_nbits (s_skey), plain);
       mpi_free (plain);
-      plain = unpad;
+      plain = NULL;
+      if (!rc)
+        rc = gcry_err_code (gcry_sexp_build (r_plain, NULL, "(value %b)",
+                                             (int)unpadlen, unpad));
       break;
+
     case PUBKEY_ENC_OAEP:
-      rc = oaep_decode (&unpad, gcry_pk_get_nbits (s_skey), ctx.hash_algo,
+      rc = oaep_decode (&unpad, &unpadlen,
+                        gcry_pk_get_nbits (s_skey), ctx.hash_algo,
                        plain, ctx.label, ctx.labellen);
-      if (rc)
-       goto leave;
       mpi_free (plain);
-      plain = unpad;
+      plain = NULL;
+      if (!rc)
+        rc = gcry_err_code (gcry_sexp_build (r_plain, NULL, "(value %b)",
+                                             (int)unpadlen, unpad));
       break;
+
     default:
+      /* Raw format.  For backward compatibility we need to assume a
+         signed mpi by using the sexp format string "%m".  */
+      rc = gcry_err_code (gcry_sexp_build
+                          (r_plain, NULL, modern? "(value %m)" : "%m", plain));
       break;
     }
 
-  if (gcry_sexp_build (r_plain, NULL, modern? "(value %m)" : "%m", plain))
-    BUG ();
-
  leave:
+  gcry_free (unpad);
+
   if (skey)
     {
       release_mpi_array (skey);
       gcry_free (skey);
     }
 
-  if (plain)
-    mpi_free (plain);
+  mpi_free (plain);
 
   if (data)
     {