core: Blank out the plaintext after decryption failure.

* src/data.h (data_prop_t): New enum.
(struct gpgme_data): Add field propidx.
* src/data.c (property_t): New.
(property_table, property_table_size, property_table_lock): New.
(insert_into_property_table): New.
(remove_from_property_table): New.
(_gpgme_data_get_dserial): New.
(_gpgme_data_set_prop): New.
(_gpgme_data_get_prop): New.
(_gpgme_data_new): Connect new object to property_table.
(_gpgme_data_release): Remove from property_table.
(gpgme_data_read): With DATA_PROP_BLANKOUT set don't fill the buffer.
* src/data-mem.c (gpgme_data_release_and_get_mem): Likewise.
* src/decrypt.c (struct op_data): Add field plaintext_dserial.
(_gpgme_op_decrypt_init_result): Add arg plaintext and init new field.
(_gpgme_decrypt_status_handler): Set DATA_PROP_BLANKOUT on decryption
failure.
(_gpgme_decrypt_start): Pass PLAIN to the init function.
* src/decrypt-verify.c (decrypt_verify_start): Ditto.
* configure.ac: Check for stdint.h and bail out if uint64_t is not
available.
--

This is a best effort feature to not output plaintext after a
decryption failure (e.g. due to no or broken authenticated
encryption).  It always work when using a memory object and reading it
after the decryption but it can't work reliable when the user is
reading from the data object while the decryption process is still
running.

This is quite a large change because the data objects and the context
objects are allowed to be owned by different threads.  Thus a
synchronization is needed and we do this with a global table of all
data objects to which the context objects can do soft-linking via a
unique data object serial number.

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

configure.ac

@@ -537,7 +537,7 @@ AM_CONDITIONAL(RUN_G13_TESTS, test "$run_g13_test" = "yes")
 
 
 # Checks for header files.
-AC_CHECK_HEADERS_ONCE([locale.h sys/select.h sys/uio.h argp.h
+AC_CHECK_HEADERS_ONCE([locale.h sys/select.h sys/uio.h argp.h stdint.h
                        unistd.h sys/time.h sys/types.h sys/stat.h])
 
 
@@ -548,6 +548,15 @@ AC_SYS_LARGEFILE
 AC_TYPE_OFF_T
 AC_TYPE_UINTPTR_T
 
+# We require uint64_t
+if test "$ac_cv_header_stdint_h" != yes; then
+   AC_MSG_ERROR([[
+***
+*** No stdint.h and thus no uint64_t type.  Can't build this library.
+***]])
+fi
+
+
 # A simple compile time check in gpgme.h for GNU/Linux systems that
 # prevents a file offset bits mismatch between gpgme and the application.
 NEED__FILE_OFFSET_BITS=0

src/data-mem.c

@@ -224,7 +224,10 @@ gpgme_data_new_from_mem (gpgme_data_t *r_dh, const char *buffer,
 char *
 gpgme_data_release_and_get_mem (gpgme_data_t dh, size_t *r_len)
 {
+  gpg_error_t err;
   char *str = NULL;
+  size_t len;
+  int blankout;
 
   TRACE_BEG1 (DEBUG_DATA, "gpgme_data_release_and_get_mem", dh,
 	      "r_len=%p", r_len);
@@ -236,10 +239,22 @@ gpgme_data_release_and_get_mem (gpgme_data_t dh, size_t *r_len)
       return NULL;
     }
 
+  err = _gpgme_data_get_prop (dh, 0, DATA_PROP_BLANKOUT, &blankout);
+  if (err)
+    {
+      gpgme_data_release (dh);
+      TRACE_ERR (err);
+      return NULL;
+    }
+
   str = dh->data.mem.buffer;
+  len = dh->data.mem.length;
+  if (blankout && len)
+    len = 1;
+
   if (!str && dh->data.mem.orig_buffer)
     {
-      str = malloc (dh->data.mem.length);
+      str = malloc (len);
       if (!str)
 	{
 	  int saved_err = gpg_error_from_syserror ();
@@ -247,15 +262,22 @@ gpgme_data_release_and_get_mem (gpgme_data_t dh, size_t *r_len)
 	  TRACE_ERR (saved_err);
 	  return NULL;
 	}
-      memcpy (str, dh->data.mem.orig_buffer, dh->data.mem.length);
+      if (blankout)
+        memset (str, 0, len);
+      else
+        memcpy (str, dh->data.mem.orig_buffer, len);
     }
   else
-    /* Prevent mem_release from releasing the buffer memory.  We must
+    {
-       not fail from this point.  */
+      if (blankout && len)
+        *str = 0;
+      /* Prevent mem_release from releasing the buffer memory.  We
+       * must not fail from this point.  */
       dh->data.mem.buffer = NULL;
+    }
 
   if (r_len)
-    *r_len = dh->data.mem.length;
+    *r_len = len;
 
   gpgme_data_release (dh);
 

src/data.c

@@ -28,6 +28,7 @@
 #endif
 #include <errno.h>
 #include <string.h>
+#include <assert.h>
 
 #include "gpgme.h"
 #include "data.h"
@@ -36,10 +37,269 @@
 #include "priv-io.h"
 #include "debug.h"
 
+
+/* The property table which has an entry for each active data object.
+ * The data object itself uses an index into this table and the table
+ * has a pointer back to the data object.  All access to that table is
+ * controlled by the property_table_lock.
+ *
+ * We use a separate table instead of linking all data objects
+ * together for faster locating properties of the data object using
+ * the data objects serial number.  We use 64 bit for the serial
+ * number which is good enough to create a new data object every
+ * nanosecond for more than 500 years.  Thus no wrap around will ever
+ * happen.
+ */
+struct property_s
+{
+  gpgme_data_t dh;   /* The data objcet or NULL if the slot is not used.  */
+  uint64_t dserial;  /* The serial number of the data object.  */
+  struct {
+    unsigned int blankout : 1;  /* Void the held data.  */
+  } flags;
+};
+typedef struct property_s *property_t;
+
+static property_t property_table;
+static unsigned int property_table_size;
+DEFINE_STATIC_LOCK (property_table_lock);
+
+
+
+/* Insert the newly created data object DH into the property table and
+ * store the index of it at R_IDX.  An error code is returned on error
+ * and the table is not changed.  */
+static gpg_error_t
+insert_into_property_table (gpgme_data_t dh, unsigned int *r_idx)
+{
+  static uint64_t last_dserial;
+  gpg_error_t err;
+  unsigned int idx;
+
+  LOCK (property_table_lock);
+  if (!property_table)
+    {
+      property_table_size = 10;
+      property_table = calloc (property_table_size, sizeof *property_table);
+      if (!property_table)
+        {
+          err = gpg_error_from_syserror ();
+          goto leave;
+        }
+    }
+
+  /* Find an empty slot.  */
+  for (idx = 0; idx < property_table_size; idx++)
+    if (!property_table[idx].dh)
+      break;
+  if (!(idx < property_table_size))
+    {
+      /* No empty slot found.  Enlarge the table.  */
+      property_t newtbl;
+      unsigned int newsize;
+
+      newsize = property_table_size + 10;
+      if ((newsize * sizeof *property_table)
+          < (property_table_size * sizeof *property_table))
+        {
+          err = gpg_error (GPG_ERR_ENOMEM);
+          goto leave;
+        }
+      newtbl = realloc (property_table, newsize * sizeof *property_table);
+      if (!newtbl)
+        {
+          err = gpg_error_from_syserror ();
+          goto leave;
+        }
+      property_table = newtbl;
+      for (idx = property_table_size; idx < newsize; idx++)
+        property_table[idx].dh = NULL;
+      idx = property_table_size;
+      property_table_size = newsize;
+    }
+
+  /* Slot found. */
+  property_table[idx].dh = dh;
+  property_table[idx].dserial = ++last_dserial;
+  *r_idx = idx;
+  err = 0;
+
+ leave:
+  UNLOCK (property_table_lock);
+  return err;
+}
+
+
+/* Remove the data object at PROPIDX from the table.  DH is only used
+ * for cross checking.  */
+static void
+remove_from_property_table (gpgme_data_t dh, unsigned int propidx)
+{
+  LOCK (property_table_lock);
+  assert (property_table);
+  assert (propidx < property_table_size);
+  assert (property_table[propidx].dh == dh);
+  property_table[propidx].dh = NULL;
+  UNLOCK (property_table_lock);
+}
+
+
+/* Return the data object's serial number for handle DH.  This is a
+ * unique serial number for each created data object.  */
+uint64_t
+_gpgme_data_get_dserial (gpgme_data_t dh)
+{
+  uint64_t dserial;
+  unsigned int idx;
+
+  if (!dh)
+    return 0;
+
+  idx = dh->propidx;
+  LOCK (property_table_lock);
+  assert (property_table);
+  assert (idx < property_table_size);
+  assert (property_table[idx].dh == dh);
+  dserial = property_table[idx].dserial;
+  UNLOCK (property_table_lock);
+
+  return dserial;
+}
+
+
+/* Set an internal property of a data object.  The data object may
+ * either be identified by the usual DH or by using the data serial
+ * number DSERIAL.  */
+gpg_error_t
+_gpgme_data_set_prop (gpgme_data_t dh, uint64_t dserial,
+                      data_prop_t name, int value)
+{
+  gpg_error_t err = 0;
+  int idx;
+  TRACE_BEG3 (DEBUG_DATA, "gpgme_data_set_prop", dh,
+	      "dserial=%llu %lu=%d",
+              (unsigned long long)dserial,
+              (unsigned long)name, value);
+
+  LOCK (property_table_lock);
+  if ((!dh && !dserial) || (dh && dserial))
+    {
+      err = gpg_error (GPG_ERR_INV_VALUE);
+      goto leave;
+    }
+  if (dh) /* Lookup via handle.  */
+    {
+      idx = dh->propidx;
+      assert (property_table);
+      assert (idx < property_table_size);
+      assert (property_table[idx].dh == dh);
+    }
+  else /* Lookup via DSERIAL.  */
+    {
+      if (!property_table)
+        {
+          err = gpg_error (GPG_ERR_NOT_FOUND);
+          goto leave;
+        }
+      for (idx = 0; idx < property_table_size; idx++)
+        if (property_table[idx].dh && property_table[idx].dserial == dserial)
+          break;
+      if (!(idx < property_table_size))
+        {
+          err = gpg_error (GPG_ERR_NOT_FOUND);
+          goto leave;
+        }
+    }
+
+  switch (name)
+    {
+    case DATA_PROP_NONE: /* Nothing to to do.  */
+      break;
+    case DATA_PROP_BLANKOUT:
+      property_table[idx].flags.blankout = !!value;
+      break;
+
+    default:
+      err = gpg_error (GPG_ERR_UNKNOWN_NAME);
+      break;
+    }
+
+ leave:
+  UNLOCK (property_table_lock);
+  return TRACE_ERR (err);
+}
+
+
+/* Get an internal property of a data object.  This is the counter
+ * part to _gpgme_data_set_property.  The value of the property is
+ * stored at R_VALUE.  On error 0 is stored at R_VALUE.  */
+gpg_error_t
+_gpgme_data_get_prop (gpgme_data_t dh, uint64_t dserial,
+                      data_prop_t name, int *r_value)
+{
+  gpg_error_t err = 0;
+  int idx;
+  TRACE_BEG2 (DEBUG_DATA, "gpgme_data_get_prop", dh,
+	      "dserial=%llu %lu",
+              (unsigned long long)dserial,
+              (unsigned long)name);
+
+  *r_value = 0;
+
+  LOCK (property_table_lock);
+  if ((!dh && !dserial) || (dh && dserial))
+    {
+      err = gpg_error (GPG_ERR_INV_VALUE);
+      goto leave;
+    }
+  if (dh) /* Lookup via handle.  */
+    {
+      idx = dh->propidx;
+      assert (property_table);
+      assert (idx < property_table_size);
+      assert (property_table[idx].dh == dh);
+    }
+  else /* Lookup via DSERIAL.  */
+    {
+      if (!property_table)
+        {
+          err = gpg_error (GPG_ERR_NOT_FOUND);
+          goto leave;
+        }
+      for (idx = 0; idx < property_table_size; idx++)
+        if (property_table[idx].dh && property_table[idx].dserial == dserial)
+          break;
+      if (!(idx < property_table_size))
+        {
+          err = gpg_error (GPG_ERR_NOT_FOUND);
+          goto leave;
+        }
+    }
+
+  switch (name)
+    {
+    case DATA_PROP_NONE: /* Nothing to to do.  */
+      break;
+    case DATA_PROP_BLANKOUT:
+      *r_value = property_table[idx].flags.blankout;
+      break;
+
+    default:
+      err = gpg_error (GPG_ERR_UNKNOWN_NAME);
+      break;
+    }
+
+ leave:
+  UNLOCK (property_table_lock);
+  return TRACE_ERR (err);
+}
+
+
 
 gpgme_error_t
 _gpgme_data_new (gpgme_data_t *r_dh, struct _gpgme_data_cbs *cbs)
 {
+  gpgme_error_t err;
   gpgme_data_t dh;
 
   if (!r_dh)
@@ -56,6 +316,13 @@ _gpgme_data_new (gpgme_data_t *r_dh, struct _gpgme_data_cbs *cbs)
 
   dh->cbs = cbs;
 
+  err = insert_into_property_table (dh, &dh->propidx);
+  if (err)
+    {
+      free (dh);
+      return err;
+    }
+
   *r_dh = dh;
   return 0;
 }
@@ -67,11 +334,13 @@ _gpgme_data_release (gpgme_data_t dh)
   if (!dh)
     return;
 
+  remove_from_property_table (dh, dh->propidx);
   if (dh->file_name)
     free (dh->file_name);
   free (dh);
 }
 
+
 
 /* Read up to SIZE bytes into buffer BUFFER from the data object with
    the handle DH.  Return the number of characters read, 0 on EOF and
@@ -80,6 +349,7 @@ gpgme_ssize_t
 gpgme_data_read (gpgme_data_t dh, void *buffer, size_t size)
 {
   gpgme_ssize_t res;
+  int blankout;
   TRACE_BEG2 (DEBUG_DATA, "gpgme_data_read", dh,
 	      "buffer=%p, size=%u", buffer, size);
 
@@ -93,9 +363,16 @@ gpgme_data_read (gpgme_data_t dh, void *buffer, size_t size)
       gpg_err_set_errno (ENOSYS);
       return TRACE_SYSRES (-1);
     }
+
+  if (_gpgme_data_get_prop (dh, 0, DATA_PROP_BLANKOUT, &blankout)
+      || blankout)
+    res = 0;
+  else
+    {
       do
         res = (*dh->cbs->read) (dh, buffer, size);
       while (res < 0 && errno == EINTR);
+    }
 
   return TRACE_SYSRES (res);
 }

src/data.h

@@ -29,6 +29,7 @@
 # include <sys/types.h>
 #endif
 #include <limits.h>
+#include <stdint.h>
 
 #include "gpgme.h"
 
@@ -73,6 +74,7 @@ struct gpgme_data
 {
   struct _gpgme_data_cbs *cbs;
   gpgme_data_encoding_t encoding;
+  unsigned int propidx;  /* Index into the property table.  */
 
 #ifdef PIPE_BUF
 #define BUFFER_SIZE PIPE_BUF
@@ -89,7 +91,7 @@ struct gpgme_data
   /* File name of the data object.  */
   char *file_name;
 
-  /* Hint on the to be expected toatl size of the data.  */
+  /* Hint on the to be expected total size of the data.  */
   gpgme_off_t size_hint;
 
   union
@@ -130,7 +132,28 @@ struct gpgme_data
   } data;
 };
 
+
+/* The data property types.  */
+typedef enum
+  {
+    DATA_PROP_NONE = 0,   /* Dummy property. */
+    DATA_PROP_BLANKOUT    /* Do not return the held data.  */
+  } data_prop_t;
+
+
 
+/* Return the data object's serial number for handle DH.  */
+uint64_t _gpgme_data_get_dserial (gpgme_data_t dh);
+
+/* Set an internal property of a data object.  */
+gpg_error_t _gpgme_data_set_prop (gpgme_data_t dh, uint64_t dserial,
+                                  data_prop_t name, int value);
+
+/* Get an internal property of a data object.  */
+gpg_error_t _gpgme_data_get_prop (gpgme_data_t dh, uint64_t dserial,
+                                  data_prop_t name, int *r_value);
+
+/* Create a new data object.  */
 gpgme_error_t _gpgme_data_new (gpgme_data_t *r_dh,
 			       struct _gpgme_data_cbs *cbs);
 
@@ -143,4 +166,5 @@ int _gpgme_data_get_fd (gpgme_data_t dh);
 /* Get the size-hint value for DH or 0 if not available.  */
 gpgme_off_t _gpgme_data_get_size_hint (gpgme_data_t dh);
 
+
 #endif	/* DATA_H */

src/decrypt-verify.c

@@ -58,7 +58,7 @@ decrypt_verify_start (gpgme_ctx_t ctx, int synchronous,
   if (err)
     return err;
 
-  err = _gpgme_op_decrypt_init_result (ctx);
+  err = _gpgme_op_decrypt_init_result (ctx, plain);
   if (err)
     return err;
 

src/decrypt.c

@@ -32,7 +32,7 @@
 #include "util.h"
 #include "context.h"
 #include "ops.h"
-
+#include "data.h"
 
 
 typedef struct
@@ -69,6 +69,9 @@ typedef struct
      This makes appending new invalid signers painless while
      preserving the order.  */
   gpgme_recipient_t *last_recipient_p;
+
+  /* The data object serial number of the plaintext.  */
+  uint64_t plaintext_dserial;
 } *op_data_t;
 
 
@@ -419,6 +422,14 @@ _gpgme_decrypt_status_handler (void *priv, gpgme_status_code_t code,
 
     case GPGME_STATUS_DECRYPTION_FAILED:
       opd->failed = 1;
+      /* Tell the data object that it shall not return any data.  We
+       * use the serial number because the data object may be owned by
+       * another thread.  We also don't check for an error because it
+       * is possible that the data object has already been destroyed
+       * and we are then not interested in returning an error.  */
+      if (!ctx->ignore_mdc_error)
+        _gpgme_data_set_prop (NULL, opd->plaintext_dserial,
+                              DATA_PROP_BLANKOUT, 1);
       break;
 
     case GPGME_STATUS_ERROR:
@@ -508,7 +519,7 @@ decrypt_status_handler (void *priv, gpgme_status_code_t code, char *args)
 
 
 gpgme_error_t
-_gpgme_op_decrypt_init_result (gpgme_ctx_t ctx)
+_gpgme_op_decrypt_init_result (gpgme_ctx_t ctx, gpgme_data_t plaintext)
 {
   gpgme_error_t err;
   void *hook;
@@ -521,6 +532,7 @@ _gpgme_op_decrypt_init_result (gpgme_ctx_t ctx)
     return err;
 
   opd->last_recipient_p = &opd->result.recipients;
+  opd->plaintext_dserial = _gpgme_data_get_dserial (plaintext);
   return 0;
 }
 
@@ -538,7 +550,7 @@ _gpgme_decrypt_start (gpgme_ctx_t ctx, int synchronous,
   if (err)
     return err;
 
-  err = _gpgme_op_decrypt_init_result (ctx);
+  err = _gpgme_op_decrypt_init_result (ctx, plain);
   if (err)
     return err;
 

src/ops.h

@@ -85,7 +85,8 @@ gpgme_error_t _gpgme_verify_status_handler (void *priv,
 
 
 /* From decrypt.c.  */
-gpgme_error_t _gpgme_op_decrypt_init_result (gpgme_ctx_t ctx);
+gpgme_error_t _gpgme_op_decrypt_init_result (gpgme_ctx_t ctx,
+                                             gpgme_data_t plaintext);
 gpgme_error_t _gpgme_decrypt_status_handler (void *priv,
 					     gpgme_status_code_t code,
 					     char *args);