Skip to content

Commit

Permalink
Fix: Escape control chars in UTF-8 TLS cert DNs
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
timopollmeier committed Jul 3, 2024
1 parent b8f5561 commit 342632d
Show file tree
Hide file tree
Showing 9 changed files with 148 additions and 23 deletions.
6 changes: 6 additions & 0 deletions src/gmp.c
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
18 changes: 16 additions & 2 deletions src/manage.c
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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,
Expand Down Expand Up @@ -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);

Check warning on line 516 in src/manage.c

View check run for this annotation

Codecov / codecov/patch

src/manage.c#L515-L516

Added lines #L515 - L516 were not covered by tests
}
else
*subject = buffer;

Check warning on line 519 in src/manage.c

View check run for this annotation

Codecov / codecov/patch

src/manage.c#L519

Added line #L519 was not covered by tests
}

if (issuer)
Expand All @@ -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);

Check warning on line 533 in src/manage.c

View check run for this annotation

Codecov / codecov/patch

src/manage.c#L532-L533

Added lines #L532 - L533 were not covered by tests
}
else
*issuer = buffer;

Check warning on line 536 in src/manage.c

View check run for this annotation

Codecov / codecov/patch

src/manage.c#L536

Added line #L536 was not covered by tests
}

if (serial)
Expand Down
1 change: 1 addition & 0 deletions src/manage.h
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,7 @@ truncate_private_key (const gchar*);
int
get_certificate_info (const gchar *,
gssize,
gboolean,
time_t *,
time_t *,
gchar **,
Expand Down
2 changes: 2 additions & 0 deletions src/manage_migrators.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down Expand Up @@ -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,
Expand Down
4 changes: 3 additions & 1 deletion src/manage_sql.c
Original file line number Diff line number Diff line change
Expand Up @@ -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,

Check warning on line 7320 in src/manage_sql.c

View check run for this annotation

Codecov / codecov/patch

src/manage_sql.c#L7320

Added line #L7320 was not covered by tests
NULL, NULL, NULL,
NULL, NULL, NULL, NULL, &crt_fmt);
if (ret || crt_fmt != GNUTLS_X509_FMT_PEM)
{
Expand Down Expand Up @@ -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,
Expand Down
35 changes: 35 additions & 0 deletions src/manage_sql_tls_certificates.c
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -1757,5 +1761,36 @@ cleanup_tls_certificate_encoding ()
}
}
cleanup_iterator (&iterator);

// Clean up control characters in remaining UTF-8 DNs

init_iterator (&iterator,

Check warning on line 1767 in src/manage_sql_tls_certificates.c

View check run for this annotation

Codecov / codecov/patch

src/manage_sql_tls_certificates.c#L1767

Added line #L1767 was not covered by tests
"SELECT id, subject_dn, issuer_dn"
" FROM tls_certificates"
" WHERE subject_dn ~ '[\\x01-\\x1F\\x7F]'"
" OR issuer_dn ~ '[\\x01-\\x1F\\x7F]'");

while (next (&iterator))

Check warning on line 1773 in src/manage_sql_tls_certificates.c

View check run for this annotation

Codecov / codecov/patch

src/manage_sql_tls_certificates.c#L1773

Added line #L1773 was not covered by tests
{
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);

Check warning on line 1780 in src/manage_sql_tls_certificates.c

View check run for this annotation

Codecov / codecov/patch

src/manage_sql_tls_certificates.c#L1778-L1780

Added lines #L1778 - L1780 were not covered by tests

gchar *quoted_subject_dn = sql_ascii_escape_and_quote (subject_dn);
gchar *quoted_issuer_dn = sql_ascii_escape_and_quote (issuer_dn);

Check warning on line 1783 in src/manage_sql_tls_certificates.c

View check run for this annotation

Codecov / codecov/patch

src/manage_sql_tls_certificates.c#L1782-L1783

Added lines #L1782 - L1783 were not covered by tests

sql ("UPDATE tls_certificates"

Check warning on line 1785 in src/manage_sql_tls_certificates.c

View check run for this annotation

Codecov / codecov/patch

src/manage_sql_tls_certificates.c#L1785

Added line #L1785 was not covered by tests
" SET subject_dn = '%s', issuer_dn = '%s'"
" WHERE id = %llu",
quoted_subject_dn, quoted_issuer_dn, tls_certificate);
changes ++;

Check warning on line 1789 in src/manage_sql_tls_certificates.c

View check run for this annotation

Codecov / codecov/patch

src/manage_sql_tls_certificates.c#L1789

Added line #L1789 was not covered by tests

g_free (quoted_subject_dn);
g_free (quoted_issuer_dn);

Check warning on line 1792 in src/manage_sql_tls_certificates.c

View check run for this annotation

Codecov / codecov/patch

src/manage_sql_tls_certificates.c#L1791-L1792

Added lines #L1791 - L1792 were not covered by tests
}

return changes;
}
32 changes: 12 additions & 20 deletions src/sql.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Check warning on line 172 in src/sql.c

View check run for this annotation

Codecov / codecov/patch

src/sql.c#L172

Added line #L172 was not covered by tests

escaped_string = strescape_check_utf8 (string, NULL);
quoted_string = sql_quote (escaped_string);
g_free (quoted_string);

Check warning on line 176 in src/sql.c

View check run for this annotation

Codecov / codecov/patch

src/sql.c#L174-L176

Added lines #L174 - L176 were not covered by tests

return quoted_string;
}
Expand Down
67 changes: 67 additions & 0 deletions src/utils.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Check warning on line 793 in src/utils.c

View check run for this annotation

Codecov / codecov/patch

src/utils.c#L793

Added line #L793 was not covered by tests
{
if (g_utf8_validate (str, 0, NULL))
return strescape_without_utf8 (str, extra_exceptions);

Check warning on line 796 in src/utils.c

View check run for this annotation

Codecov / codecov/patch

src/utils.c#L796

Added line #L796 was not covered by tests
else
return g_strescape (str, extra_exceptions);

Check warning on line 798 in src/utils.c

View check run for this annotation

Codecov / codecov/patch

src/utils.c#L798

Added line #L798 was not covered by tests
}

/**
* @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)

Check warning on line 815 in src/utils.c

View check run for this annotation

Codecov / codecov/patch

src/utils.c#L815

Added line #L815 was not covered by tests
{
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;

Check warning on line 826 in src/utils.c

View check run for this annotation

Codecov / codecov/patch

src/utils.c#L826

Added line #L826 was not covered by tests
gchar *escaped;

if (extra_exceptions && strcmp (extra_exceptions, ""))
{
exceptions = g_strconcat (base_exceptions,

Check warning on line 831 in src/utils.c

View check run for this annotation

Codecov / codecov/patch

src/utils.c#L831

Added line #L831 was not covered by tests
extra_exceptions ? extra_exceptions : "",
NULL);
}
escaped = g_strescape (str, exceptions ? exceptions : base_exceptions);
g_free (exceptions);
return escaped;

Check warning on line 837 in src/utils.c

View check run for this annotation

Codecov / codecov/patch

src/utils.c#L835-L837

Added lines #L835 - L837 were not covered by tests
}


/* XML. */

Expand Down
6 changes: 6 additions & 0 deletions src/utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 *);

Expand Down

0 comments on commit 342632d

Please sign in to comment.