Merge pull request #2248 from greenbone/tls-cert-escaping

Fix: Escape control chars in UTF-8 TLS cert DNs

src/gmp.c

@@ -11498,6 +11498,7 @@ handle_get_alerts (gmp_parser_t *gmp_parser, GError **error)
               if (certificate && strcmp (certificate, "")
                   && get_certificate_info ((gchar*)certificate,
                                            strlen (certificate),
+                                           TRUE,
                                            &activation_time,
                                            &expiration_time,
                                            &md5_fingerprint,
@@ -12345,6 +12346,7 @@ handle_get_credentials (gmp_parser_t *gmp_parser, GError **error)
 
           get_certificate_info (cert,
                                 -1,
+                                TRUE,
                                 &activation_time,
                                 &expiration_time,
                                 &md5_fingerprint,
@@ -16505,6 +16507,7 @@ handle_get_scanners (gmp_parser_t *gmp_parser, GError **error)
 
               get_certificate_info (scanner_iterator_ca_pub (&scanners),
                                     -1,
+                                    TRUE,
                                     &activation_time,
                                     &expiration_time,
                                     &md5_fingerprint,
@@ -16559,6 +16562,7 @@ handle_get_scanners (gmp_parser_t *gmp_parser, GError **error)
 
               get_certificate_info (scanner_iterator_key_pub (&scanners),
                                     -1,
+                                    TRUE,
                                     &activation_time,
                                     &expiration_time,
                                     &md5_fingerprint,
@@ -17143,6 +17147,7 @@ handle_get_settings (gmp_parser_t *gmp_parser, GError **error)
 
           get_certificate_info (setting_iterator_value (&settings),
                                 -1,
+                                TRUE,
                                 &activation_time,
                                 &expiration_time,
                                 &md5_fingerprint,
@@ -19935,6 +19940,7 @@ gmp_xml_handle_end_element (/* unused */ GMarkupParseContext* context,
 
                   get_certificate_info (ldap_cacert,
                                         -1,
+                                        TRUE,
                                         &activation_time,
                                         &expiration_time,
                                         &md5_fingerprint,

src/manage.c

@@ -362,6 +362,7 @@ truncate_private_key (const gchar* private_key)
  *
  * @param[in]  certificate        The certificate to get data from.
  * @param[in]  certificate_len    Length of certificate, -1: null-terminated
+ * @param[in]  escape_dns         Whether to escape control characters in DNs.
  * @param[out] activation_time    Pointer to write activation time to.
  * @param[out] expiration_time    Pointer to write expiration time to.
  * @param[out] md5_fingerprint    Pointer for newly allocated MD5 fingerprint.
@@ -376,6 +377,7 @@ truncate_private_key (const gchar* private_key)
  */
 int
 get_certificate_info (const gchar* certificate, gssize certificate_len,
+                      gboolean escape_dns,
                       time_t* activation_time, time_t* expiration_time,
                       gchar** md5_fingerprint, gchar **sha256_fingerprint,
                       gchar **subject, gchar** issuer, gchar **serial,
@@ -518,7 +520,13 @@ get_certificate_info (const gchar* certificate, gssize certificate_len,
           buffer = g_malloc (buffer_size);
           gnutls_x509_crt_get_dn (gnutls_cert, buffer, &buffer_size);
 
-          *subject = buffer;
+          if (escape_dns)
+            {
+              *subject = strescape_check_utf8 (buffer, NULL);
+              g_free (buffer);
+            }
+          else
+            *subject = buffer;
         }
 
       if (issuer)
@@ -529,7 +537,13 @@ get_certificate_info (const gchar* certificate, gssize certificate_len,
           buffer = g_malloc (buffer_size);
           gnutls_x509_crt_get_issuer_dn (gnutls_cert, buffer, &buffer_size);
 
-          *issuer = buffer;
+          if (escape_dns)
+            {
+              *issuer = strescape_check_utf8 (buffer, NULL);
+              g_free (buffer);
+            }
+          else
+            *issuer = buffer;
         }
 
       if (serial)

src/manage.h

@@ -170,6 +170,7 @@ truncate_private_key (const gchar*);
 int
 get_certificate_info (const gchar *,
                       gssize,
+                      gboolean,
                       time_t *,
                       time_t *,
                       gchar **,

src/manage_migrators.c

@@ -780,6 +780,7 @@ migrate_212_to_213 ()
 
       get_certificate_info ((gchar*)certificate,
                             certificate_size,
+                            FALSE,
                             NULL,   /* activation_time */
                             NULL,   /* expiration_time */
                             NULL,   /* md5_fingerprint */
@@ -851,9 +852,9 @@ make_tls_certificate_214 (user_t owner,
   quoted_certificate_b64
     = certificate_b64 ? sql_quote (certificate_b64) : NULL;
   quoted_subject_dn
-    = subject_dn ? sql_ascii_escape_and_quote (subject_dn) : NULL;
+    = subject_dn ? sql_ascii_escape_and_quote (subject_dn, NULL) : NULL;
   quoted_issuer_dn
-    = issuer_dn ? sql_ascii_escape_and_quote (issuer_dn) : NULL;
+    = issuer_dn ? sql_ascii_escape_and_quote (issuer_dn, NULL) : NULL;
   quoted_md5_fingerprint
     = md5_fingerprint ? sql_quote (md5_fingerprint) : NULL;
   quoted_sha256_fingerprint
@@ -1086,6 +1087,7 @@ migrate_213_to_214 ()
           /* Try extracting the data directly from the certificate */
           get_certificate_info ((gchar*)certificate,
                                 certificate_size,
+                                FALSE,
                                 &activation_time,
                                 &expiration_time,
                                 &md5_fingerprint,

src/manage_sql.c

@@ -7317,7 +7317,8 @@ validate_tippingpoint_data (alert_method_t method, const gchar *name,
           int ret;
           gnutls_x509_crt_fmt_t crt_fmt;
 
-          ret = get_certificate_info (*data, strlen(*data), NULL, NULL, NULL,
+          ret = get_certificate_info (*data, strlen(*data), FALSE,
+                                      NULL, NULL, NULL,
                                       NULL, NULL, NULL, NULL, &crt_fmt);
           if (ret || crt_fmt != GNUTLS_X509_FMT_PEM)
             {
@@ -27441,6 +27442,7 @@ print_report_host_tls_certificates_xml (report_host_t report_host,
 
       get_certificate_info ((gchar*)certificate,
                             certificate_size,
+                            TRUE,
                             &activation_time,
                             &expiration_time,
                             &md5_fingerprint,

src/manage_sql_tls_certificates.c

@@ -585,9 +585,9 @@ make_tls_certificate (const char *name,
   quoted_sha256_fingerprint
     = sql_quote (sha256_fingerprint ? sha256_fingerprint : "");
   quoted_subject_dn
-    = sql_ascii_escape_and_quote (subject_dn ? subject_dn : "");
+    = sql_ascii_escape_and_quote (subject_dn ? subject_dn : "", NULL);
   quoted_issuer_dn
-    = sql_ascii_escape_and_quote (issuer_dn ? issuer_dn : "");
+    = sql_ascii_escape_and_quote (issuer_dn ? issuer_dn : "", NULL);
   quoted_serial
     = sql_quote (serial ? serial : "");
 
@@ -725,6 +725,7 @@ make_tls_certificate_from_base64 (const char *name,
 
   ret = get_certificate_info (certificate_decoded,
                               certificate_len,
+                              FALSE,
                               &activation_time,
                               &expiration_time,
                               &md5_fingerprint,
@@ -1558,6 +1559,7 @@ add_tls_certificates_from_report_host (report_host_t report_host,
 
       get_certificate_info ((gchar*)certificate,
                             certificate_size,
+                            FALSE,
                             &activation_time,
                             &expiration_time,
                             &md5_fingerprint,
@@ -1725,6 +1727,8 @@ cleanup_tls_certificate_encoding ()
   int changes = 0;
   iterator_t iterator;
 
+  // Clean up names that are not valid UTF-8
+
   init_iterator (&iterator,
                  "SELECT id, subject_dn, issuer_dn"
                  " FROM tls_certificates"
@@ -1743,8 +1747,10 @@ cleanup_tls_certificate_encoding ()
       if (g_utf8_validate (subject_dn, -1, NULL) == FALSE
           || g_utf8_validate (issuer_dn, -1, NULL) == FALSE)
         {
-          gchar *quoted_subject_dn = sql_ascii_escape_and_quote (subject_dn);
-          gchar *quoted_issuer_dn = sql_ascii_escape_and_quote (issuer_dn);
+          gchar *quoted_subject_dn 
+            = sql_ascii_escape_and_quote (subject_dn, NULL);
+          gchar *quoted_issuer_dn 
+            = sql_ascii_escape_and_quote (issuer_dn, NULL);
 
           sql ("UPDATE tls_certificates"
                " SET subject_dn = '%s', issuer_dn = '%s'"
@@ -1757,5 +1763,36 @@ cleanup_tls_certificate_encoding ()
         }
     }
   cleanup_iterator (&iterator);
+
+  // Clean up control characters in remaining UTF-8 DNs
+
+  init_iterator (&iterator,
+                 "SELECT id, subject_dn, issuer_dn"
+                 " FROM tls_certificates"
+                 " WHERE subject_dn ~ '[\\x01-\\x1F\\x7F]'"
+                 "   OR issuer_dn ~ '[\\x01-\\x1F\\x7F]'");
+
+  while (next (&iterator))
+    {
+      tls_certificate_t tls_certificate;
+      const char *subject_dn, *issuer_dn;
+
+      tls_certificate = iterator_int64 (&iterator, 0);
+      subject_dn = iterator_string (&iterator, 1);
+      issuer_dn = iterator_string (&iterator, 2);
+
+      gchar *quoted_subject_dn = sql_ascii_escape_and_quote (subject_dn, NULL);
+      gchar *quoted_issuer_dn = sql_ascii_escape_and_quote (issuer_dn, NULL);
+
+      sql ("UPDATE tls_certificates"
+            " SET subject_dn = '%s', issuer_dn = '%s'"
+            " WHERE id = %llu",
+            quoted_subject_dn, quoted_issuer_dn, tls_certificate);
+      changes ++;
+
+      g_free (quoted_subject_dn);
+      g_free (quoted_issuer_dn);
+    }
+
   return changes;
 }

src/sql.c

@@ -150,38 +150,30 @@ sql_quote (const char* string)
 }
 
 /**
- * @brief Quotes a string for use in SQL statements, also ASCII escaping it
- *         if it is not valid UTF-8.
+ * @brief Quotes a string for use in SQL statements, also ASCII escaping it.
  *
- * @param[in]  string  String to quote, has to be \\0 terminated.
+ * The ASCII escaping excludes characters 0x80 - 0xFF for valid UTF-8 strings
+ * and includes them otherwise.
+ *
+ * @param[in]  string       String to quote, has to be \\0 terminated.
+ * @param[in]  exceptions   Optional exceptions for the escaping.
  *
  * @return Freshly allocated, quoted string. Free with g_free.
  */
 gchar*
-sql_ascii_escape_and_quote (const char* string)
+sql_ascii_escape_and_quote (const char* string, const char* exceptions)
 {
+  gchar *escaped_string;
   gchar *quoted_string;
 
   assert (string);
 
   if (string == NULL)
-    {
-      return NULL;
-    }
-  else if (g_utf8_validate (string, -1, NULL))
-    {
-      // Quote valid UTF-8 without ASCII escaping
-      quoted_string = sql_quote (string);
-    }
-  else
-    {
-      // Assume invalid UTF-8 uses a different, unknown encoding and
-      //  ASCII-escape it.
-      gchar *escaped_string;
-      escaped_string = g_strescape (string, "");
-      quoted_string = sql_quote (escaped_string);
-      g_free (escaped_string);
-    }
+    return NULL;
+
+  escaped_string = strescape_check_utf8 (string, exceptions);
+  quoted_string = sql_quote (escaped_string);
+  g_free (escaped_string);
 
   return quoted_string;
 }

src/sql.h

@@ -80,7 +80,7 @@ gchar *
 sql_quote (const char *);
 
 gchar *
-sql_ascii_escape_and_quote (const char *);
+sql_ascii_escape_and_quote (const char *, const char *);
 
 gchar *
 sql_insert (const char *);

src/utils.c

@@ -770,6 +770,73 @@ is_uuid (const char *uuid)
   return 1;
 }
 
+
+/* Strings. */
+
+/**
+ * @brief Escape a string with exceptions for UTF-8 if it is valid UTF-8.
+ *
+ * If the string is valid UTF-8, characters in the range 0x80 - 0xFF
+ * are excluded so non-ASCII UTF-8 characters and any characters in the
+ * extra_exceptions are not escaped.
+ * This leaves the characters 0x01 - 0x1F and 0x7F to be escaped if they are
+ * not in the given extra_exceptions.
+ *
+ * For strings that are not valid UTF-8 all characters in the ranges
+ *
+ * @param[in]  str               The string to escape.
+ * @param[in]  extra_exceptions  Extra exc.
+ *
+ * @return Newly allocated escaped copy of the string.
+ */
+gchar *
+strescape_check_utf8 (const char *str, const char *extra_exceptions)
+{
+  if (g_utf8_validate (str, -1, NULL))
+    return strescape_without_utf8 (str, extra_exceptions);
+  else
+    return g_strescape (str, extra_exceptions);
+}
+
+/**
+ * @brief Escape control characters in a string with exceptions for UTF-8.
+ *
+ * Characters in the range 0x80 - 0xFF are excluded so non-ASCII UTF-8
+ * characters are not escaped.
+ * This leaves the characters 0x01 - 0x1F and 0x7F to be escaped if they are
+ * not in the given extra_exceptions.
+ *
+ * @param[in]  str               The string to escape.
+ * @param[in]  extra_exceptions  Extra exceptions to the escaping.
+ *
+ * @return Newly allocated escaped copy of the string.
+ */
+gchar *
+strescape_without_utf8 (const char *str, const char *extra_exceptions)
+{
+  static const char *base_exceptions =
+    "\200\201\202\203\204\205\206\207\210\211\212\213\214\215\216\217"
+    "\220\221\222\223\224\225\226\227\230\231\232\233\234\235\236\237"
+    "\240\241\242\243\244\245\246\247\250\251\252\253\254\255\256\257"
+    "\260\261\262\263\264\265\266\267\270\271\272\273\274\275\276\277"
+    "\300\301\302\303\304\305\306\307\310\311\312\313\314\315\316\317"
+    "\320\321\322\323\324\325\326\327\330\331\332\333\334\335\336\337"
+    "\340\341\342\343\344\345\346\347\350\351\352\353\354\355\356\357"
+    "\360\361\362\363\364\365\366\367\370\371\372\373\374\375\376\377";
+  gchar *exceptions = NULL;
+  gchar *escaped;
+
+  if (extra_exceptions && strcmp (extra_exceptions, ""))
+    {
+      exceptions = g_strconcat (base_exceptions,
+                                extra_exceptions ? extra_exceptions : "",
+                                NULL);
+    }
+  escaped = g_strescape (str, exceptions ? exceptions : base_exceptions);
+  g_free (exceptions);
+  return escaped;
+}
+
 
 /* XML. */
 

src/utils.h

@@ -88,6 +88,12 @@ lockfile_locked (const gchar *);
 int
 is_uuid (const char *);
 
+gchar *
+strescape_check_utf8 (const char *, const char *);
+
+gchar *
+strescape_without_utf8 (const char *, const char *);
+
 int
 parse_xml_file (const gchar *, entity_t *);