Merge pull request #865 from timopollmeier/result_nvts-fix-9.0

Fix result_nvt for new OSP and slave results (9.0)

CHANGELOG.md

@@ -30,6 +30,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 - MODIFY_USER saves comment when COMMENT is empty [#842](https://github.com/greenbone/gvmd/pull/842)
 - Fix result diff generation to ignore white space in delta reports [#861](https://github.com/greenbone/gvmd/pull/861)
 - Fix resource type checks for permissions [#863](https://github.com/greenbone/gvmd/pull/863)
+- Fix result_nvt for new OSP and slave results [#865](https://github.com/greenbone/gvmd/pull/865)
 
 ### Removed
 - Remove 1.3.6.1.4.1.25623.1.0.90011 from Discovery config (9.0) [#847](https://github.com/greenbone/gvmd/pull/847)

INSTALL.md

@@ -411,6 +411,11 @@ supported values for `<name>` are:
   using the DELETE_REPORT_FORMAT GMP command, for example after a built-in
   report format has been removed.
 
+- `cleanup-result-nvts`
+
+  This cleans up results with missing result_nvt entries which can result
+  in filters and overrides not working properly.
+
 - `cleanup-result-severities`
 
   This cleans up results with no severity by assigning the default

doc/gvmd.8

@@ -110,7 +110,7 @@ Modify user's password and exit.
 Modify user's password and exit.
 .TP
 \fB--optimize=\fINAME\fB\f1
-Run an optimization: vacuum, analyze, cleanup-config-prefs, cleanup-port-names, cleanup-report-formats, cleanup-result-severities, cleanup-schedule-times, migrate-relay-sensors, rebuild-report-cache or update-report-cache.
+Run an optimization: vacuum, analyze, cleanup-config-prefs, cleanup-port-names, cleanup-report-formats, cleanup-result-nvts, cleanup-result-severities, cleanup-schedule-times, migrate-relay-sensors, rebuild-report-cache or update-report-cache.
 .TP
 \fB--osp-vt-update=\fISCANNER-SOCKET\fB\f1
 Unix socket for OSP NVT update. Defaults to the path of the 'OpenVAS Default' scanner if it is an absolute path.

doc/gvmd.8.xml

@@ -251,7 +251,7 @@ Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
       <p><opt>--optimize=<arg>NAME</arg></opt></p>
       <optdesc>
         <p>Run an optimization: vacuum, analyze, cleanup-config-prefs,
-           cleanup-port-names, cleanup-report-formats,
+           cleanup-port-names, cleanup-report-formats, cleanup-result-nvts,
            cleanup-result-severities, cleanup-schedule-times,
            migrate-relay-sensors, rebuild-report-cache
            or update-report-cache.</p>

doc/gvmd.html

@@ -234,7 +234,7 @@ <h2>Options</h2>
       <p><b>--optimize=<em>NAME</em></b></p>
 
         <p>Run an optimization: vacuum, analyze, cleanup-config-prefs,
-           cleanup-port-names, cleanup-report-formats,
+           cleanup-port-names, cleanup-report-formats, cleanup-result-nvts,
            cleanup-result-severities, cleanup-schedule-times,
            migrate-relay-sensors, rebuild-report-cache
            or update-report-cache.</p>

src/gvmd.c

@@ -1777,7 +1777,7 @@ gvmd (int argc, char** argv)
         { "optimize", '\0', 0, G_OPTION_ARG_STRING,
           &optimize,
           "Run an optimization: vacuum, analyze, cleanup-config-prefs,"
-          " cleanup-port-names, cleanup-report-formats,"
+          " cleanup-port-names, cleanup-report-formats, cleanup-result-nvts"
           " cleanup-result-severities, cleanup-schedule-times,"
           " migrate-relay-sensors, rebuild-report-cache or"
           " update-report-cache.",

src/manage.h

@@ -1510,6 +1510,9 @@ result_iterator_has_cert_bunds (iterator_t*);
 int
 result_iterator_has_dfn_certs (iterator_t*);
 
+int
+cleanup_result_nvts ();
+
 void
 init_report_host_iterator (iterator_t*, report_t, const char *, report_host_t);
 

src/manage_sql.c

@@ -21122,11 +21122,14 @@ make_osp_result (task_t task, const char *host, const char *hostname,
   result_nvt_notice (quoted_nvt);
   sql ("INSERT into results"
        " (owner, date, task, host, hostname, port, nvt,"
-       "  nvt_version, severity, type, qod, qod_type, description, uuid)"
+       "  nvt_version, severity, type, qod, qod_type, description, uuid,"
+       "  result_nvt)"
        " VALUES (NULL, m_now(), %llu, '%s', '%s', '%s', '%s',"
-       "         '%s', '%s', '%s', %d, '', '%s', make_uuid ());",
+       "         '%s', '%s', '%s', %d, '', '%s', make_uuid (),"
+       "         (SELECT id FROM result_nvts WHERE nvt = '%s'));",
        task, host ?: "", quoted_hostname, quoted_port, quoted_nvt,
-       nvt_revision ?: "", result_severity ?: "0", type, qod, quoted_desc);
+       nvt_revision ?: "", result_severity ?: "0", type, qod, quoted_desc,
+       quoted_nvt);
   g_free (result_severity);
   g_free (nvt_revision);
   g_free (quoted_desc);
@@ -25108,6 +25111,70 @@ result_iterator_has_dfn_certs (iterator_t* iterator)
   return iterator_int (iterator, GET_ITERATOR_COLUMN_COUNT + 28);
 }
 
+/**
+ * @brief Check if the result_nvts are assigned to result
+ *
+ * @return 0 success, -1 error
+ */
+int
+cleanup_result_nvts ()
+{
+  iterator_t affected_iter;
+  GArray *affected;
+  int index;
+
+  g_debug ("%s: Creating missing result_nvts entries", __func__);
+  sql ("INSERT INTO result_nvts (nvt)"
+       " SELECT DISTINCT nvt FROM results ON CONFLICT (nvt) DO NOTHING;");
+
+  // Get affected reports with overrides
+  affected = g_array_new (TRUE, TRUE, sizeof (report_t));
+  init_iterator (&affected_iter,
+                 "SELECT DISTINCT report FROM results"
+                 " WHERE (result_nvt IS NULL"
+                 "        OR report NOT IN"
+                 "           (SELECT report FROM result_nvt_reports"
+                 "             WHERE result_nvt IS NOT NULL))"
+                 "   AND nvt IN (SELECT nvt FROM overrides);");
+  while (next (&affected_iter))
+    {
+      report_t report;
+      report = iterator_int64 (&affected_iter, 0);
+      g_array_append_val (affected, report);
+    }
+  cleanup_iterator(&affected_iter);
+
+  g_debug ("%s: Adding missing result_nvt values to results", __func__);
+  sql ("UPDATE results"
+       " SET result_nvt"
+       "       = (SELECT id FROM result_nvts"
+       "           WHERE result_nvts.nvt = results.nvt)"
+       " WHERE result_nvt IS NULL");
+
+  g_debug ("%s: Cleaning up NULL result_nvt_reports entries", __func__);
+  sql ("DELETE FROM result_nvt_reports WHERE result_nvt IS NULL;");
+
+  g_debug ("%s: Adding missing result_nvt_reports entries", __func__);
+  sql ("INSERT INTO result_nvt_reports (report, result_nvt)"
+       " SELECT DISTINCT report, result_nvts.id FROM results"
+       "   JOIN result_nvts ON result_nvts.nvt = results.nvt"
+       "  WHERE report NOT IN (SELECT report FROM result_nvt_reports"
+       "                       WHERE result_nvt IS NOT NULL)");
+
+  // Re-cache affected reports with overrides
+  for (index = 0; index < affected->len; index++)
+    {
+      report_t report;
+      report = g_array_index (affected, report_t, index);
+      g_debug ("%s: Updating cache of affected report %llu",
+               __func__, report);
+      report_cache_counts (report, 0, 1, NULL);
+    }
+  g_array_free (affected, TRUE);
+
+  return 0;
+}
+
 /**
  * @brief Initialise a host iterator.
  *
@@ -49225,7 +49292,8 @@ set_slave_commit_size (int new_commit_size)
 /**
  * @brief Buffer a result to be inserted.
  *
- * @param[in]  buffer       Buffer to store SQL.
+ * @param[in]  results_buffer     Buffer to store results SQL.
+ * @param[in]  result_nvts_buffer Buffer to store result_nvts SQL.
  * @param[in]  task         The task associated with the result.
  * @param[in]  host         Host IP address.
  * @param[in]  hostname     Hostname.
@@ -49236,21 +49304,26 @@ set_slave_commit_size (int new_commit_size)
  * @param[in]  report       Report that result belongs to.
  * @param[in]  owner        Owner of report.
  *
- * @return A result descriptor for the new result, 0 if error.
+ * @return 0 success, -1 error.
  */
-static result_t
-buffer_insert (GString *buffer, task_t task, const char* host,
+static int
+buffer_insert (GString *results_buffer, GString *result_nvts_buffer,
+               task_t task, const char* host,
                const char *hostname, const char* port, const char* nvt,
                const char* type, const char* description,
                report_t report, user_t owner)
 {
+  static int db_server_version_num = 0;
   gchar *nvt_revision, *severity;
   gchar *quoted_hostname, *quoted_descr, *quoted_qod_type;
   int qod, first;
   nvt_t nvt_id = 0;
 
   assert (report);
 
+  if (db_server_version_num == 0)
+    db_server_version_num = sql_server_version ();
+
   if (nvt && strcmp (nvt, "") && (find_nvt (nvt, &nvt_id) || nvt_id <= 0))
     {
       g_warning ("NVT '%s' not found. Result not created", nvt);
@@ -49308,17 +49381,38 @@ buffer_insert (GString *buffer, task_t task, const char* host,
   quoted_hostname = sql_quote (hostname ? hostname : "");
   quoted_descr = sql_quote (description ?: "");
   result_nvt_notice (nvt);
-  first = (strlen (buffer->str) == 0);
+  first = (strlen (results_buffer->str) == 0);
 
   if (first)
-    g_string_append (buffer,
-                     "INSERT into results"
-                     " (owner, date, task, host, hostname, port,"
-                     "  nvt, nvt_version, severity, type,"
-                     "  description, uuid, qod, qod_type, result_nvt,"
-                     "  report)"
-                     " VALUES");
-  g_string_append_printf (buffer,
+    {
+      g_string_append (results_buffer,
+                       "INSERT into results"
+                       " (owner, date, task, host, hostname, port,"
+                       "  nvt, nvt_version, severity, type,"
+                       "  description, uuid, qod, qod_type, result_nvt,"
+                       "  report)"
+                       " VALUES");
+      if (db_server_version_num >= 90500)
+        g_string_append (result_nvts_buffer,
+                        "INSERT INTO result_nvts (nvt) VALUES ");
+    }
+
+  if (db_server_version_num < 90500)
+    g_string_append_printf
+        (result_nvts_buffer,
+         "INSERT into result_nvts (nvt)"
+         " SELECT '%s' WHERE NOT EXISTS (SELECT * FROM result_nvts"
+         "                               WHERE nvt = '%s');\n",
+         nvt,
+         nvt);
+  else
+    g_string_append_printf
+        (result_nvts_buffer,
+         "%s ('%s')",
+         first ? "" : ",",
+         nvt);
+
+  g_string_append_printf (results_buffer,
                           "%s"
                           " (%llu, m_now (), %llu, '%s', '%s', '%s',"
                           "  '%s', '%s', '%s', '%s',"
@@ -49343,21 +49437,33 @@ buffer_insert (GString *buffer, task_t task, const char* host,
 /**
  * @brief Run INSERT for update_from_slave.
  *
- * @param[in]   buffer  Buffer.
- * @param[in]   report  Report.
+ * @param[in]  results_buffer     Buffer of results SQL.
+ * @param[in]  result_nvts_buffer Buffer of result_nvts SQL.
+ * @param[in]  report             Report.
  */
 static void
-update_from_slave_insert (GString *buffer, report_t report)
+update_from_slave_insert (GString *results_buffer, GString *result_nvts_buffer,
+                          report_t report)
 {
-  if (buffer && strlen (buffer->str))
+  if (result_nvts_buffer && strlen (result_nvts_buffer->str))
+    {
+      if (sql_server_version () >= 90500)
+        g_string_append (result_nvts_buffer,
+                         " ON CONFLICT (nvt) DO NOTHING;");
+
+      sql ("%s", result_nvts_buffer->str);
+      g_string_truncate (result_nvts_buffer, 0);
+    }
+
+  if (results_buffer && strlen (results_buffer->str))
     {
       if (report)
         {
           iterator_t ids;
 
-          g_string_append (buffer, " RETURNING id;");
+          g_string_append (results_buffer, " RETURNING id;");
 
-          init_iterator (&ids, "%s", buffer->str);
+          init_iterator (&ids, "%s", results_buffer->str);
           while (next (&ids))
             report_add_result_for_buffer (report, iterator_int64 (&ids, 0));
           cleanup_iterator (&ids);
@@ -49372,9 +49478,9 @@ update_from_slave_insert (GString *buffer, report_t report)
                report, report);
         }
       else
-        sql ("%s", buffer->str);
+        sql ("%s", results_buffer->str);
 
-      g_string_truncate (buffer, 0);
+      g_string_truncate (results_buffer, 0);
     }
 }
 
@@ -49395,7 +49501,7 @@ update_from_slave (task_t task, entity_t get_report, entity_t *report,
   entity_t entity, host, start;
   entities_t results, hosts, entities;
   int current_commit_size;
-  GString *buffer;
+  GString *results_buffer, *result_nvts_buffer;
   user_t owner;
 
   entity = entity_child (get_report, "report");
@@ -49468,7 +49574,8 @@ update_from_slave (task_t task, entity_t get_report, entity_t *report,
   sql_begin_immediate ();
   results = entity->entities;
   current_commit_size = 0;
-  buffer = g_string_new ("");
+  results_buffer = g_string_new ("");
+  result_nvts_buffer = g_string_new ("");
   while ((entity = first_entity (results)))
     {
       if (strcmp (entity_name (entity), "result") == 0)
@@ -49501,7 +49608,8 @@ update_from_slave (task_t task, entity_t get_report, entity_t *report,
           if (description == NULL)
             goto rollback_fail;
 
-          buffer_insert (buffer,
+          buffer_insert (results_buffer,
+                         result_nvts_buffer,
                          task,
                          entity_text (result_host),
                          hostname ? entity_text (hostname) : "",
@@ -49515,7 +49623,9 @@ update_from_slave (task_t task, entity_t get_report, entity_t *report,
           current_commit_size++;
           if (slave_commit_size && current_commit_size >= slave_commit_size)
             {
-              update_from_slave_insert (buffer, global_current_report);
+              update_from_slave_insert (results_buffer,
+                                        result_nvts_buffer,
+                                        global_current_report);
               sql_commit ();
               sql_begin_immediate ();
               current_commit_size = 0;
@@ -49525,8 +49635,11 @@ update_from_slave (task_t task, entity_t get_report, entity_t *report,
         }
       results = next_entities (results);
     }
-  update_from_slave_insert (buffer, global_current_report);
-  g_string_free (buffer, TRUE);
+  update_from_slave_insert (results_buffer,
+                            result_nvts_buffer,
+                            global_current_report);
+  g_string_free (results_buffer, TRUE);
+  g_string_free (result_nvts_buffer, TRUE);
   sql_commit ();
 
   sql_begin_immediate ();
@@ -65254,6 +65367,21 @@ manage_optimize (GSList *log_config, const gchar *database, const gchar *name)
                                       " %d alert(s).",
                                       alert_changes);
     }
+  else if (strcasecmp (name, "cleanup-result-nvts") == 0)
+    {
+      sql_begin_immediate ();
+
+      if (cleanup_result_nvts ())
+        {
+          sql_rollback();
+          fprintf (stderr, "Clean-up of result_nvts failed.\n");
+          return 1;
+        }
+
+      sql_commit ();
+
+      success_text = g_strdup_printf ("Optimized: Cleaned up result_nvts.");
+    }
   else if (strcasecmp (name, "cleanup-result-severities") == 0)
     {
       int missing_severity_changes = 0;

src/sql.h

@@ -31,6 +31,9 @@
 
 /* Helpers. */
 
+int
+sql_server_version ();
+
 const char *
 sql_schema ();
 

src/sql_pg.c

@@ -87,6 +87,17 @@ static PGconn *conn = NULL;
 
 /* Helpers. */
 
+/**
+ * @brief Get the server version number.
+ * 
+ * @return The version as an integer.
+ */
+int
+sql_server_version ()
+{
+  return sql_int ("SELECT current_setting ('server_version_num')::integer;");
+}
+
 /**
  * @brief Get main schema name.
  *