From 4d1a94320d506060e546e7c7cb1e0092fad04d49 Mon Sep 17 00:00:00 2001 From: Timo Pollmeier Date: Wed, 3 Jul 2024 15:51:15 +0200 Subject: [PATCH 1/4] Fix: Escape control chars in UTF-8 TLS cert DNs ASCII control characters (0x01 - 0x1F and 0x7F) are now escaped in the DNs of TLS certficates even they are valid UTF-8. Characters in the range 0x80 - 0xFF are escaped if the DN is not valid UTF-8. This character escaping for TLS certificates is applied more consistently. This addresses issues with XML parsers, e.g. ones used by report formats, not being able to process control characters. --- src/gmp.c | 6 +++ src/manage.c | 18 ++++++++- src/manage.h | 1 + src/manage_migrators.c | 2 + src/manage_sql.c | 4 +- src/manage_sql_tls_certificates.c | 35 ++++++++++++++++ src/sql.c | 32 ++++++--------- src/utils.c | 67 +++++++++++++++++++++++++++++++ src/utils.h | 6 +++ 9 files changed, 148 insertions(+), 23 deletions(-) diff --git a/src/gmp.c b/src/gmp.c index f98cc69f6..6e8840e2e 100644 --- a/src/gmp.c +++ b/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, diff --git a/src/manage.c b/src/manage.c index 1465ca279..5e4f754db 100644 --- a/src/manage.c +++ b/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) diff --git a/src/manage.h b/src/manage.h index b60939d56..891b3a0c7 100644 --- a/src/manage.h +++ b/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 **, diff --git a/src/manage_migrators.c b/src/manage_migrators.c index 3053fd2fe..78f7eadf4 100644 --- a/src/manage_migrators.c +++ b/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 */ @@ -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, diff --git a/src/manage_sql.c b/src/manage_sql.c index 100030655..961911a40 100644 --- a/src/manage_sql.c +++ b/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, diff --git a/src/manage_sql_tls_certificates.c b/src/manage_sql_tls_certificates.c index e10ca729d..c21ab3a6c 100644 --- a/src/manage_sql_tls_certificates.c +++ b/src/manage_sql_tls_certificates.c @@ -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" @@ -1757,5 +1761,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); + gchar *quoted_issuer_dn = sql_ascii_escape_and_quote (issuer_dn); + + 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; } diff --git a/src/sql.c b/src/sql.c index 0f08b56e1..41efb4688 100644 --- a/src/sql.c +++ b/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) { + 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, NULL); + quoted_string = sql_quote (escaped_string); + g_free (quoted_string); return quoted_string; } diff --git a/src/utils.c b/src/utils.c index 38455f2c4..a4b5dfb08 100644 --- a/src/utils.c +++ b/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, 0, 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. */ diff --git a/src/utils.h b/src/utils.h index 8c3e53ff9..10fcf59d8 100644 --- a/src/utils.h +++ b/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 *); From e50c2c8adc3df26b0f7c6eec222749c14979a84c Mon Sep 17 00:00:00 2001 From: Timo Pollmeier Date: Wed, 3 Jul 2024 17:29:40 +0200 Subject: [PATCH 2/4] Fix UTF-8 validation in strescape_check_utf8 --- src/utils.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils.c b/src/utils.c index a4b5dfb08..5db841129 100644 --- a/src/utils.c +++ b/src/utils.c @@ -792,7 +792,7 @@ is_uuid (const char *uuid) gchar * strescape_check_utf8 (const char *str, const char *extra_exceptions) { - if (g_utf8_validate (str, 0, NULL)) + if (g_utf8_validate (str, -1, NULL)) return strescape_without_utf8 (str, extra_exceptions); else return g_strescape (str, extra_exceptions); From c61cf01dab771e2e66d2f28acca153f1e6091bf4 Mon Sep 17 00:00:00 2001 From: Timo Pollmeier Date: Wed, 3 Jul 2024 17:30:13 +0200 Subject: [PATCH 3/4] Add tests for strescape_check_utf8 --- src/utils_tests.c | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/src/utils_tests.c b/src/utils_tests.c index 2a252a19d..ad6b8aa3f 100644 --- a/src/utils_tests.c +++ b/src/utils_tests.c @@ -109,6 +109,42 @@ Ensure (utils, gvm_sleep_sleep_for_1) assert_that (timespec_subtract (&end, &start), is_greater_than (NANOSECONDS - 1)); } +Ensure (utils, strescape_check_utf_8_no_exceptions) +{ + const char *utf8_input = "Äöü\n123\\UTF-8\x04"; + const char *utf8_expected = "Äöü\\n123\\\\UTF-8\\004"; + const char *cp850_input = "\x8E\x94\x81\n123\\CP850\x04"; + const char *cp850_expected = "\\216\\224\\201\\n123\\\\CP850\\004"; + + assert_that (g_utf8_validate (utf8_input, -1, NULL), is_true); + gchar *output = strescape_check_utf8 (utf8_input, NULL); + assert_that (output, is_equal_to_string (utf8_expected)); + g_free (output); + + assert_that (g_utf8_validate (cp850_input, -1, NULL), is_false); + output = strescape_check_utf8 (cp850_input, NULL); + assert_that (output, is_equal_to_string (cp850_expected)); + g_free (output); +} + +Ensure (utils, strescape_check_utf_8_with_exceptions) +{ + const char *utf8_input = "Äöü\n123\\UTF-8\x04"; + const char *utf8_expected = "Äöü\n123\\\\UTF-8\\004"; + const char *cp850_input = "\x8E\x94\x81\n123\\CP850\x04"; + const char *cp850_expected = "\\216\\224\\201\n123\\\\CP850\\004"; + + assert_that (g_utf8_validate (utf8_input, -1, NULL), is_true); + gchar *output = strescape_check_utf8 (utf8_input, "\t\n\r"); + assert_that (output, is_equal_to_string (utf8_expected)); + g_free (output); + + assert_that (g_utf8_validate (cp850_input, -1, NULL), is_false); + output = strescape_check_utf8 (cp850_input, "\t\n\r"); + assert_that (output, is_equal_to_string (cp850_expected)); + g_free (output); +} + /* Test suite. */ int @@ -128,6 +164,9 @@ main (int argc, char **argv) add_test_with_context (suite, utils, parse_iso_time_tz_with_z); add_test_with_context (suite, utils, parse_iso_time_tz_with_fallback_tz); add_test_with_context (suite, utils, parse_iso_time_tz_variants); + + add_test_with_context (suite, utils, strescape_check_utf_8_no_exceptions); + add_test_with_context (suite, utils, strescape_check_utf_8_with_exceptions); if (argc > 1) return run_single_test (suite, argv[1], create_text_reporter ()); From b19ad67d5176de863c97e1dafd64819a9069c663 Mon Sep 17 00:00:00 2001 From: Timo Pollmeier Date: Tue, 23 Jul 2024 09:21:04 +0200 Subject: [PATCH 4/4] Fix sql_ascii_escape_and_quote Added the missing exceptions params and fixed the g_free of escaped_string. --- src/manage_migrators.c | 4 ++-- src/manage_sql_tls_certificates.c | 14 ++++++++------ src/sql.c | 6 +++--- src/sql.h | 2 +- 4 files changed, 14 insertions(+), 12 deletions(-) diff --git a/src/manage_migrators.c b/src/manage_migrators.c index 78f7eadf4..a9e118528 100644 --- a/src/manage_migrators.c +++ b/src/manage_migrators.c @@ -852,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 diff --git a/src/manage_sql_tls_certificates.c b/src/manage_sql_tls_certificates.c index c21ab3a6c..e51f33eee 100644 --- a/src/manage_sql_tls_certificates.c +++ b/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 : ""); @@ -1747,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'" @@ -1779,8 +1781,8 @@ cleanup_tls_certificate_encoding () subject_dn = iterator_string (&iterator, 1); issuer_dn = iterator_string (&iterator, 2); - 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'" diff --git a/src/sql.c b/src/sql.c index 41efb4688..efae4de7b 100644 --- a/src/sql.c +++ b/src/sql.c @@ -161,7 +161,7 @@ sql_quote (const char* string) * @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; @@ -171,9 +171,9 @@ sql_ascii_escape_and_quote (const char* string) if (string == NULL) return NULL; - escaped_string = strescape_check_utf8 (string, NULL); + escaped_string = strescape_check_utf8 (string, exceptions); quoted_string = sql_quote (escaped_string); - g_free (quoted_string); + g_free (escaped_string); return quoted_string; } diff --git a/src/sql.h b/src/sql.h index b0a225eff..08103a959 100644 --- a/src/sql.h +++ b/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 *);