From 28c3869cd2e7d726175149b962de4a6bf835d1c9 Mon Sep 17 00:00:00 2001 From: Johannes Helmold Date: Wed, 8 Sep 2021 10:48:19 +0200 Subject: [PATCH 1/6] Send an error message instead of the fallback report text to gsad. In gmp.c in function handle_get_system_reports (..): Adjusted an erroneous if-statement. In manage.c in function manage_system_report (..): Replaced the fallback report text by the appropriate error message. --- src/gmp.c | 3 ++- src/manage.c | 56 +++++++--------------------------------------------- 2 files changed, 9 insertions(+), 50 deletions(-) diff --git a/src/gmp.c b/src/gmp.c index 8a8f1e5cd..dcd96bebf 100644 --- a/src/gmp.c +++ b/src/gmp.c @@ -16171,6 +16171,7 @@ handle_get_system_reports (gmp_parser_t *gmp_parser, GError **error) (&types, get_system_reports_data->name, get_system_reports_data->slave_id); + switch (ret) { case 1: @@ -16264,7 +16265,7 @@ handle_get_system_reports (gmp_parser_t *gmp_parser, GError **error) "", report_type_iterator_name (&types), report_type_iterator_title (&types), - (ret == 3 ? "txt" : "png"), + (report_ret == 3 ? "txt" : "png"), get_system_reports_data->start_time ? get_system_reports_data->start_time : "", get_system_reports_data->end_time diff --git a/src/manage.c b/src/manage.c index 0b1072849..819948dbb 100644 --- a/src/manage.c +++ b/src/manage.c @@ -4040,16 +4040,8 @@ get_osp_performance_string (scanner_t scanner, int start, int end, opts.titles = g_strdup (titles); error = NULL; - connection_retry = get_scanner_connection_retry (); return_value = osp_get_performance_ext (connection, opts, performance_str, &error); - while (return_value != 0 && connection_retry > 0) - { - sleep(1); - return_value = osp_get_performance_ext (connection, opts, - performance_str, &error); - connection_retry--; - } if (return_value) { @@ -4481,56 +4473,22 @@ manage_system_report (const char *name, const char *duration, || (WIFEXITED (exit_status) == 0) || WEXITSTATUS (exit_status)) { - int ret; - double load[3]; - GError *get_error; - gchar *output; - gsize output_len; GString *buffer; + g_warning ("%s: Failed to create performance graph -- %s", __func__, astderr); g_debug ("%s: gvmcg failed with %d", __func__, exit_status); g_debug ("%s: stdout: %s", __func__, astdout); g_debug ("%s: stderr: %s", __func__, astderr); - g_free (astdout); - g_free (astderr); - g_free (command); - buffer = g_string_new (FALLBACK_SYSTEM_REPORT_HEADER); + buffer = g_string_new (""); - ret = getloadavg (load, 3); - if (ret == 3) - { - g_string_append_printf (buffer, - "Load average for past minute: %.1f\n", - load[0]); - g_string_append_printf (buffer, - "Load average for past 5 minutes: %.1f\n", - load[1]); - g_string_append_printf (buffer, - "Load average for past 15 minutes: %.1f\n", - load[2]); - } - else - g_string_append (buffer, "Error getting load averages.\n"); - - get_error = NULL; - g_file_get_contents ("/proc/meminfo", - &output, - &output_len, - &get_error); - if (get_error) - g_error_free (get_error); - else - { - gchar *safe; - g_string_append (buffer, "\n/proc/meminfo:\n\n"); - safe = g_markup_escape_text (output, strlen (output)); - g_free (output); - g_string_append (buffer, safe); - g_free (safe); - } + g_string_append_printf (buffer, + "Failed to create performance graph: %s", astderr); *report = g_string_free (buffer, FALSE); + g_free (astdout); + g_free (astderr); + g_free (command); return 3; } g_free (astderr); From 0a179264cd0f000023e4f4e8b9f87549ea049d81 Mon Sep 17 00:00:00 2001 From: Johannes Helmold Date: Thu, 9 Sep 2021 16:38:12 +0200 Subject: [PATCH 2/6] Added CHANGELOG entry for error messages from gvmcg. --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index bd94684c6..f5b2b4987 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ### Deprecated ### Removed ### Fixed +- Ensure gvmd sends error messages if gvmcg fails [#1682](https://github.com/greenbone/gvmd/pull/1682) [Unreleased]: https://github.com/greenbone/gvmd/compare/v21.4.3...HEAD From 03c3a1f80877e75c7997193ab8964daaf0c3fe84 Mon Sep 17 00:00:00 2001 From: Johannes Helmold Date: Fri, 17 Sep 2021 09:47:12 +0200 Subject: [PATCH 3/6] Pass error messages from sensor performace reports to gsad. Added the ability to pass error message from the call of "get_osp_performance_string (..)" through to the gsad and further to GSA. Rebuilt the function manage_system_report (..) for that purpose. --- src/manage.c | 227 +++++++++++++++++++++++++++++++++++---------------- 1 file changed, 157 insertions(+), 70 deletions(-) diff --git a/src/manage.c b/src/manage.c index 819948dbb..156cc2651 100644 --- a/src/manage.c +++ b/src/manage.c @@ -3997,18 +3997,18 @@ credential_full_type (const char* abbreviation) * @param[in] titles The end titles for the performance report. * @param[in] performance_str The performance string. * - * @return 0 if successful, 4 could not connect to scanner, - * 6 failed to get performance report, -1 error + * @return 0 if successful, 6 could not connect to scanner or failed to get + * performance report */ static int get_osp_performance_string (scanner_t scanner, int start, int end, - const char *titles, gchar **performance_str) + const char *titles, gchar **performance_str, + gchar **error) { char *host, *ca_pub, *key_pub, *key_priv; int port; osp_connection_t *connection = NULL; osp_get_performance_opts_t opts; - gchar *error; int connection_retry, return_value; host = scanner_host (scanner); @@ -4033,23 +4033,24 @@ get_osp_performance_string (scanner_t scanner, int start, int end, free (key_priv); if (connection == NULL) - return 4; + { + *error = g_strdup("Could not connect to scanner"); + return 6; + } opts.start = start; opts.end = end; opts.titles = g_strdup (titles); - error = NULL; return_value = osp_get_performance_ext (connection, opts, - performance_str, &error); + performance_str, error); if (return_value) { osp_connection_close (connection); - g_warning ("Error getting OSP performance report: %s", error); - g_free (error); + g_warning ("Error getting OSP performance report: %s", *error); g_free (opts.titles); - return 4; + return 6; } osp_connection_close (connection); @@ -4058,6 +4059,59 @@ get_osp_performance_string (scanner_t scanner, int start, int end, return 0; } +/** + * @brief Header for fallback system report. + */ +#define FALLBACK_SYSTEM_REPORT_HEADER \ +"This is the most basic, fallback report. The system can be configured to\n" \ +"produce more powerful reports. Please contact your system administrator\n" \ +"for more information.\n\n" + +static void +get_fallback_report_string(GString *fallback_report) +{ + int ret; + double load[3]; + GError *get_error; + gchar *output; + gsize output_len; + + g_string_append_printf (fallback_report, FALLBACK_SYSTEM_REPORT_HEADER); + + ret = getloadavg (load, 3); + if (ret == 3) + { + g_string_append_printf (fallback_report, + "Load average for past minute: %.1f\n", + load[0]); + g_string_append_printf (fallback_report, + "Load average for past 5 minutes: %.1f\n", + load[1]); + g_string_append_printf (fallback_report, + "Load average for past 15 minutes: %.1f\n", + load[2]); + } + else + g_string_append (fallback_report, "Error getting load averages.\n"); + + get_error = NULL; + g_file_get_contents ("/proc/meminfo", + &output, + &output_len, + &get_error); + if (get_error) + g_error_free (get_error); + else + { + gchar *safe; + g_string_append (fallback_report, "\n/proc/meminfo:\n\n"); + safe = g_markup_escape_text (output, strlen (output)); + g_free (output); + g_string_append (fallback_report, safe); + g_free (safe); + } +} + /** * @brief Command called by get_system_report_types. * gvmcg stands for gvm-create-graphs. @@ -4083,6 +4137,7 @@ get_system_report_types (const char *required_type, gchar ***start, { gchar *astdout = NULL; gchar *astderr = NULL; + gchar *slave_error = NULL; GError *err = NULL; gint exit_status; @@ -4099,10 +4154,14 @@ get_system_report_types (const char *required_type, gchar ***start, return 2; // Assume OSP scanner - ret = get_osp_performance_string (slave, 0, 0, "titles", &astdout); + ret = get_osp_performance_string (slave, 0, 0, "titles", + &astdout, &slave_error); if (ret) - return ret; + { + g_free (slave_error); + return ret; + } } else { @@ -4143,6 +4202,7 @@ get_system_report_types (const char *required_type, gchar ***start, *types = NULL; g_free (astdout); g_free (astderr); + g_free (slave_error); return -1; } *space = '\0'; @@ -4161,6 +4221,7 @@ get_system_report_types (const char *required_type, gchar ***start, *types = type; g_free (astdout); g_free (astderr); + g_free (slave_error); return 0; } type++; @@ -4170,6 +4231,7 @@ get_system_report_types (const char *required_type, gchar ***start, /* Failed to find the single given type. */ g_free (astdout); g_free (astderr); + g_free (slave_error); g_strfreev (*types); return 1; } @@ -4179,6 +4241,7 @@ get_system_report_types (const char *required_type, gchar ***start, g_free (astdout); g_free (astderr); + g_free (slave_error); return 0; } @@ -4271,14 +4334,6 @@ report_type_iterator_title (report_type_iterator_t* iterator) return name + strlen (name) + 1; } -/** - * @brief Header for fallback system report. - */ -#define FALLBACK_SYSTEM_REPORT_HEADER \ -"This is the most basic, fallback report. The system can be configured to\n" \ -"produce more powerful reports. Please contact your system administrator\n" \ -"for more information.\n\n" - /** * @brief Default duration for system reports. */ @@ -4394,8 +4449,7 @@ parse_performance_params (const char *duration, * * @return 0 if successful (including failure to find report), -1 on error, * 2 could not find slave scanner, - * 3 if used the fallback report, 4 could not connect to slave, - * 5 authentication failed, 6 failed to get system report. + * 3 if used the fallback report or got an error message to print */ int manage_system_report (const char *name, const char *duration, @@ -4404,9 +4458,12 @@ manage_system_report (const char *name, const char *duration, { gchar *astdout = NULL; gchar *astderr = NULL; + gchar *slave_error = NULL; GError *err = NULL; + GString *buffer = NULL; gint exit_status; - gchar *command; + gint return_code = 0; + gchar *command = NULL; time_t cmd_param_1, cmd_param_2; int params_count; @@ -4415,6 +4472,8 @@ manage_system_report (const char *name, const char *duration, parse_performance_params (duration, start_time, end_time, &cmd_param_1, &cmd_param_2, ¶ms_count); + *report = NULL; + if (params_count == 0) return manage_system_report ("blank", NULL, NULL, NULL, NULL, report); @@ -4434,66 +4493,91 @@ manage_system_report (const char *name, const char *duration, // only duration time_t now; now = time (NULL); - return get_osp_performance_string (slave, - now - cmd_param_1, - now, - name, - report); + return_code = get_osp_performance_string (slave, + now - cmd_param_1, + now, + name, + report, + &slave_error); } else { // start and end time - return get_osp_performance_string (slave, - cmd_param_1, - cmd_param_2, - name, - report); + return_code = get_osp_performance_string (slave, + cmd_param_1, + cmd_param_2, + name, + report, + &slave_error); } } - - /* For simplicity, it's up to the command to do the base64 encoding. */ - if (params_count == 1) - command = g_strdup_printf ("gvmcg %ld %s", - cmd_param_1, - name); else - command = g_strdup_printf ("gvmcg %ld %ld %s", - cmd_param_1, - cmd_param_2, - name); - - g_debug (" command: %s", command); - - if ((g_spawn_command_line_sync (command, - &astdout, - &astderr, - &exit_status, - &err) - == FALSE) - || (WIFEXITED (exit_status) == 0) - || WEXITSTATUS (exit_status)) { - GString *buffer; + if (!g_find_program_in_path ("gvmcg")) + { + return_code = 7; + } + else + { + /* For simplicity, it's up to the command to do the base64 + * encoding. + */ + if (params_count == 1) + command = g_strdup_printf ("gvmcg %ld %s", + cmd_param_1, + name); + else + command = g_strdup_printf ("gvmcg %ld %ld %s", + cmd_param_1, + cmd_param_2, + name); + + g_debug (" command: %s", command); + + if ((g_spawn_command_line_sync (command, + &astdout, + &astderr, + &exit_status, + &err) + == FALSE) + || (WIFEXITED (exit_status) == 0) + || WEXITSTATUS (exit_status)) + { + return_code = 3; - g_warning ("%s: Failed to create performance graph -- %s", __func__, astderr); - g_debug ("%s: gvmcg failed with %d", __func__, exit_status); - g_debug ("%s: stdout: %s", __func__, astdout); - g_debug ("%s: stderr: %s", __func__, astderr); + g_warning ("%s: Failed to create performance graph -- %s", + __func__, astderr); + g_debug ("%s: gvmcg failed with %d", __func__, exit_status); + g_debug ("%s: stdout: %s", __func__, astdout); + g_debug ("%s: stderr: %s", __func__, astderr); + } + g_free (command); + } + } + if (return_code == 7) + { + buffer = g_string_new (""); + get_fallback_report_string(buffer); + *report = g_string_free (buffer, FALSE); + } + else if (return_code == 3 || return_code == 6) + { buffer = g_string_new (""); - g_string_append_printf (buffer, - "Failed to create performance graph: %s", astderr); - + "Failed to create performance graph: %s", + (return_code == 3 ? astderr : slave_error)); *report = g_string_free (buffer, FALSE); - g_free (astdout); - g_free (astderr); - g_free (command); - return 3; } + g_free (astderr); - g_free (command); - if (astdout == NULL || strlen (astdout) == 0) + g_free (slave_error); + + if (return_code == 6 || return_code == 7) + return_code = 3; + + if ((astdout == NULL || strlen (astdout) == 0) && + *report == NULL) { g_free (astdout); if (strcmp (name, "blank") == 0) @@ -4501,9 +4585,12 @@ manage_system_report (const char *name, const char *duration, return manage_system_report ("blank", NULL, NULL, NULL, NULL, report); } - else + else if (*report == NULL) *report = astdout; - return 0; + else + g_free (astdout); + + return return_code; } From 481daa9373c69cd26ceaec6da87780326cf50572 Mon Sep 17 00:00:00 2001 From: Johannes Helmold Date: Fri, 17 Sep 2021 10:31:45 +0200 Subject: [PATCH 4/6] Added the documentation for a new parameter Added the documentation for the new parameter "error" of function get_osp_performance_string(..). --- src/manage.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/manage.c b/src/manage.c index 68a0f0040..ef2872adb 100644 --- a/src/manage.c +++ b/src/manage.c @@ -4019,6 +4019,7 @@ credential_full_type (const char* abbreviation) * @param[in] end The end time of the performance report. * @param[in] titles The end titles for the performance report. * @param[in] performance_str The performance string. + * @param[in] error The error message text, if any. * * @return 0 if successful, 6 could not connect to scanner or failed to get * performance report From d87a3c24282c73d57f321844ba93417a1a32b7d6 Mon Sep 17 00:00:00 2001 From: Johannes Helmold Date: Fri, 17 Sep 2021 10:41:20 +0200 Subject: [PATCH 5/6] Added documentation for the function get_fallback_report_string () --- src/manage.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/manage.c b/src/manage.c index ef2872adb..e04908f94 100644 --- a/src/manage.c +++ b/src/manage.c @@ -4091,6 +4091,11 @@ get_osp_performance_string (scanner_t scanner, int start, int end, "produce more powerful reports. Please contact your system administrator\n" \ "for more information.\n\n" +/** + * @brief Get the fallback report as a string. + * + * @param[in] fallback_report The string for the fallback report. + */ static void get_fallback_report_string(GString *fallback_report) { From 7af644213197df838829a7adb5ebf1df4099447f Mon Sep 17 00:00:00 2001 From: Johannes Helmold Date: Fri, 17 Sep 2021 11:55:11 +0200 Subject: [PATCH 6/6] Moved some lines of code in function "manage_system_report (..)". --- src/manage.c | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/src/manage.c b/src/manage.c index e04908f94..e067ef028 100644 --- a/src/manage.c +++ b/src/manage.c @@ -4544,6 +4544,9 @@ manage_system_report (const char *name, const char *duration, { if (!g_find_program_in_path ("gvmcg")) { + buffer = g_string_new (""); + get_fallback_report_string(buffer); + *report = g_string_free (buffer, FALSE); return_code = 7; } else @@ -4581,16 +4584,10 @@ manage_system_report (const char *name, const char *duration, g_debug ("%s: stderr: %s", __func__, astderr); } g_free (command); - } + } } - if (return_code == 7) - { - buffer = g_string_new (""); - get_fallback_report_string(buffer); - *report = g_string_free (buffer, FALSE); - } - else if (return_code == 3 || return_code == 6) + if (return_code == 3 || return_code == 6) { buffer = g_string_new (""); g_string_append_printf (buffer,