[evolution-data-server/gnome-43] IMAPX: Hide complete requests in debug logs for some sensitive commands

[Milan Crha <mcrha src gnome org>]

commit e6966343e0bb11bbcef730f8e14e1553ab9ee330
Author: Milan Crha <mcrha redhat com>
Date:   Tue Sep 27 15:28:51 2022 +0200

    IMAPX: Hide complete requests in debug logs for some sensitive commands
    
    Sensitive commands like LOGIN or AUTHENTICATE should not be shown
    in the logs, to avoid accidental private data disclosure. These had
    been hidden in the most cases, but not all.

 src/camel/providers/imapx/camel-imapx-logger.c | 111 +++++++++++++++++++------
 src/camel/providers/imapx/camel-imapx-logger.h |   5 +-
 src/camel/providers/imapx/camel-imapx-server.c |  28 ++++++-
 src/camel/providers/imapx/camel-imapx-server.h |   4 +
 4 files changed, 121 insertions(+), 27 deletions(-)
---
diff --git a/src/camel/providers/imapx/camel-imapx-logger.c b/src/camel/providers/imapx/camel-imapx-logger.c
index 287628ffb..4fca615b9 100644
--- a/src/camel/providers/imapx/camel-imapx-logger.c
+++ b/src/camel/providers/imapx/camel-imapx-logger.c
@@ -35,11 +35,13 @@
 
 struct _CamelIMAPXLoggerPrivate {
        gchar prefix;
+       GWeakRef server_weakref;
 };
 
 enum {
        PROP_0,
-       PROP_PREFIX
+       PROP_PREFIX,
+       PROP_SERVER
 };
 
 /* Forward Declarations */
@@ -62,6 +64,19 @@ imapx_logger_set_prefix (CamelIMAPXLogger *logger,
        logger->priv->prefix = prefix;
 }
 
+static void
+imapx_logger_set_server (CamelIMAPXLogger *logger,
+                        CamelIMAPXServer *server)
+{
+       g_weak_ref_set (&logger->priv->server_weakref, server);
+}
+
+static CamelIMAPXServer *
+imapx_logger_dup_server (CamelIMAPXLogger *logger)
+{
+       return g_weak_ref_get (&logger->priv->server_weakref);
+}
+
 static void
 imapx_logger_set_property (GObject *object,
                            guint property_id,
@@ -74,6 +89,11 @@ imapx_logger_set_property (GObject *object,
                                CAMEL_IMAPX_LOGGER (object),
                                g_value_get_schar (value));
                        return;
+               case PROP_SERVER:
+                       imapx_logger_set_server (
+                               CAMEL_IMAPX_LOGGER (object),
+                               g_value_get_object (value));
+                       return;
        }
 
        G_OBJECT_WARN_INVALID_PROPERTY_ID (object, property_id, pspec);
@@ -92,11 +112,45 @@ imapx_logger_get_property (GObject *object,
                                camel_imapx_logger_get_prefix (
                                CAMEL_IMAPX_LOGGER (object)));
                        return;
+               case PROP_SERVER:
+                       g_value_take_object (
+                               value,
+                               imapx_logger_dup_server (
+                               CAMEL_IMAPX_LOGGER (object)));
+                       return;
        }
 
        G_OBJECT_WARN_INVALID_PROPERTY_ID (object, property_id, pspec);
 }
 
+static void
+imapx_logger_finalize (GObject *object)
+{
+       CamelIMAPXLogger *self = CAMEL_IMAPX_LOGGER (object);
+
+       g_weak_ref_clear (&self->priv->server_weakref);
+
+       G_OBJECT_CLASS (camel_imapx_logger_parent_class)->finalize (object);
+}
+
+static gboolean
+imapx_logger_discard_logging (CamelIMAPXLogger *logger,
+                             const gchar **out_replace_text)
+{
+       CamelIMAPXServer *server;
+       gboolean res;
+
+       server = imapx_logger_dup_server (logger);
+       if (!server)
+               return FALSE;
+
+       res = camel_imapx_server_should_discard_logging (server, out_replace_text);
+
+       g_object_unref (server);
+
+       return res;
+}
+
 static GConverterResult
 imapx_logger_convert (GConverter *converter,
                       gconstpointer inbuf,
@@ -108,12 +162,12 @@ imapx_logger_convert (GConverter *converter,
                       gsize *bytes_written,
                       GError **error)
 {
-       CamelIMAPXLoggerPrivate *priv;
+       CamelIMAPXLogger *logger;
        GConverterResult result;
        gsize min_size;
-       const gchar *login_start;
+       const gchar *replace_text = NULL;
 
-       priv = CAMEL_IMAPX_LOGGER (converter)->priv;
+       logger = CAMEL_IMAPX_LOGGER (converter);
 
        min_size = MIN (inbuf_size, outbuf_size);
 
@@ -121,21 +175,11 @@ imapx_logger_convert (GConverter *converter,
                memcpy (outbuf, inbuf, min_size);
        *bytes_read = *bytes_written = min_size;
 
-       login_start = g_strstr_len (outbuf, min_size, " LOGIN ");
-       if (login_start > (const gchar *) outbuf) {
-               const gchar *space = g_strstr_len (outbuf, min_size, " ");
-
-               if (space == login_start) {
-                       camel_imapx_debug (
-                               io, priv->prefix, "I/O: '%.*s ...'\n",
-                               (gint) (login_start - ((const gchar *) outbuf) + 6), (gchar *) outbuf);
-               } else {
-                       /* To print the command the other way */
-                       login_start = NULL;
-               }
-       }
-
-       if (!login_start) {
+       if (imapx_logger_discard_logging (logger, &replace_text)) {
+               camel_imapx_debug (
+                       io, logger->priv->prefix, "I/O: %s...\n",
+                       replace_text ? replace_text : "");
+       } else {
                /* Skip ending '\n' '\r'; it may sometimes show wrong data,
                   when the input is divided into wrong chunks, but it will
                   usually work as is needed, no extra new-lines in the log */
@@ -143,7 +187,7 @@ imapx_logger_convert (GConverter *converter,
                        min_size--;
 
                camel_imapx_debug (
-                       io, priv->prefix, "I/O: '%.*s'\n",
+                       io, logger->priv->prefix, "I/O: '%.*s'\n",
                        (gint) min_size, (gchar *) outbuf);
        }
 
@@ -171,6 +215,7 @@ camel_imapx_logger_class_init (CamelIMAPXLoggerClass *class)
        object_class = G_OBJECT_CLASS (class);
        object_class->set_property = imapx_logger_set_property;
        object_class->get_property = imapx_logger_get_property;
+       object_class->finalize = imapx_logger_finalize;
 
        g_object_class_install_property (
                object_class,
@@ -183,6 +228,18 @@ camel_imapx_logger_class_init (CamelIMAPXLoggerClass *class)
                        G_PARAM_READWRITE |
                        G_PARAM_CONSTRUCT_ONLY |
                        G_PARAM_STATIC_STRINGS));
+
+       g_object_class_install_property (
+               object_class,
+               PROP_SERVER,
+               g_param_spec_object (
+                       "server",
+                       "CamelIMAPXServer",
+                       NULL,
+                       CAMEL_TYPE_IMAPX_SERVER,
+                       G_PARAM_READWRITE |
+                       G_PARAM_CONSTRUCT_ONLY |
+                       G_PARAM_STATIC_STRINGS));
 }
 
 static void
@@ -196,26 +253,32 @@ static void
 camel_imapx_logger_init (CamelIMAPXLogger *logger)
 {
        logger->priv = camel_imapx_logger_get_instance_private (logger);
+       g_weak_ref_init (&logger->priv->server_weakref, NULL);
 }
 
 /**
  * camel_imapx_logger_new:
  * @prefix: a prefix character
+ * @server: (nullable): a CamelIMAPXServer, or %NULL
  *
  * Creates a new #CamelIMAPXLogger.  Each output line generated by the
  * logger will have a prefix string that includes the @prefix character
  * to distinguish it from other active loggers.
  *
+ * The @server can hint to discard logging for certain commands.
+ *
  * Returns: a #CamelIMAPXLogger
  *
  * Since: 3.12
  **/
 GConverter *
-camel_imapx_logger_new (gchar prefix)
+camel_imapx_logger_new (gchar prefix,
+                       CamelIMAPXServer *server)
 {
-       return g_object_new (
-               CAMEL_TYPE_IMAPX_LOGGER,
-               "prefix", prefix, NULL);
+       return g_object_new (CAMEL_TYPE_IMAPX_LOGGER,
+               "prefix", prefix,
+               "server", server,
+               NULL);
 }
 
 /**
diff --git a/src/camel/providers/imapx/camel-imapx-logger.h b/src/camel/providers/imapx/camel-imapx-logger.h
index 3f2fa74e8..411fe050f 100644
--- a/src/camel/providers/imapx/camel-imapx-logger.h
+++ b/src/camel/providers/imapx/camel-imapx-logger.h
@@ -20,6 +20,8 @@
 
 #include <gio/gio.h>
 
+#include "camel-imapx-server.h"
+
 /* Standard GObject macros */
 #define CAMEL_TYPE_IMAPX_LOGGER \
        (camel_imapx_logger_get_type ())
@@ -67,7 +69,8 @@ struct _CamelIMAPXLoggerClass {
 };
 
 GType          camel_imapx_logger_get_type     (void) G_GNUC_CONST;
-GConverter *   camel_imapx_logger_new          (gchar prefix);
+GConverter *   camel_imapx_logger_new          (gchar prefix,
+                                                CamelIMAPXServer *server);
 gchar          camel_imapx_logger_get_prefix   (CamelIMAPXLogger *logger);
 
 G_END_DECLS
diff --git a/src/camel/providers/imapx/camel-imapx-server.c b/src/camel/providers/imapx/camel-imapx-server.c
index 2ed68cff5..e5645e251 100644
--- a/src/camel/providers/imapx/camel-imapx-server.c
+++ b/src/camel/providers/imapx/camel-imapx-server.c
@@ -2678,7 +2678,7 @@ imapx_server_set_streams (CamelIMAPXServer *is,
                GInputStream *temp_stream;
 
                /* The logger produces debugging output. */
-               logger = camel_imapx_logger_new (is->priv->tagprefix);
+               logger = camel_imapx_logger_new (is->priv->tagprefix, NULL);
                input_stream = g_converter_input_stream_new (
                        input_stream, logger);
                g_clear_object (&logger);
@@ -2695,7 +2695,7 @@ imapx_server_set_streams (CamelIMAPXServer *is,
 
        if (output_stream != NULL) {
                /* The logger produces debugging output. */
-               logger = camel_imapx_logger_new (is->priv->tagprefix);
+               logger = camel_imapx_logger_new (is->priv->tagprefix, is);
                output_stream = g_converter_output_stream_new (
                        output_stream, logger);
                g_clear_object (&logger);
@@ -7503,3 +7503,27 @@ camel_imapx_server_ref_current_command (CamelIMAPXServer *is)
 
        return command;
 }
+
+gboolean
+camel_imapx_server_should_discard_logging (CamelIMAPXServer *is,
+                                          const gchar **out_replace_text)
+{
+       gboolean discard = FALSE;
+
+       g_return_val_if_fail (CAMEL_IS_IMAPX_SERVER (is), FALSE);
+       g_return_val_if_fail (out_replace_text != NULL, FALSE);
+
+       COMMAND_LOCK (is);
+
+       if (imapx_server_has_current_command (is, CAMEL_IMAPX_JOB_AUTHENTICATE)) {
+               discard = TRUE;
+               *out_replace_text = "AUTHENTICATE";
+       } else if (imapx_server_has_current_command (is, CAMEL_IMAPX_JOB_LOGIN)) {
+               discard = TRUE;
+               *out_replace_text = "LOGIN";
+       }
+
+       COMMAND_UNLOCK (is);
+
+       return discard;
+}
diff --git a/src/camel/providers/imapx/camel-imapx-server.h b/src/camel/providers/imapx/camel-imapx-server.h
index c6e0b2537..80cba7084 100644
--- a/src/camel/providers/imapx/camel-imapx-server.h
+++ b/src/camel/providers/imapx/camel-imapx-server.h
@@ -302,6 +302,10 @@ const CamelIMAPXUntaggedRespHandlerDesc *
                                                (CamelIMAPXServer *is,
                                                 const gchar *untagged_response,
                                                 const CamelIMAPXUntaggedRespHandlerDesc *desc);
+gboolean       camel_imapx_server_should_discard_logging
+                                               (CamelIMAPXServer *is,
+                                                const gchar **out_replace_text);
+
 G_END_DECLS
 
 #endif /* CAMEL_IMAPX_SERVER_H */