From 342632d925458e3e4c85ed18e92babc52cfb31dd Mon Sep 17 00:00:00 2001 From: Timo Pollmeier Date: Wed, 3 Jul 2024 15:51:15 +0200 Subject: [PATCH] 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 1587fe6e31..196cfdfd09 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, @@ -12346,6 +12347,7 @@ handle_get_credentials (gmp_parser_t *gmp_parser, GError **error) get_certificate_info (cert, -1, + TRUE, &activation_time, &expiration_time, &md5_fingerprint, @@ -16506,6 +16508,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, @@ -16560,6 +16563,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, @@ -17144,6 +17148,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, @@ -19936,6 +19941,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 0af8a54860..c3e368bd56 100644 --- a/src/manage.c +++ b/src/manage.c @@ -352,6 +352,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. @@ -366,6 +367,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, @@ -508,7 +510,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) @@ -519,7 +527,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 9e7bbbce92..f6f9c7c3fd 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 3053fd2fe7..78f7eadf4f 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 c9f1a68d8e..dd04bc069c 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 e10ca729dd..c21ab3a6c4 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 0f08b56e14..41efb4688e 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 9c7ceb7cc9..6073209676 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 1054dc0ca9..8dbf325167 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 *);