From 88cf9dec060e8c3f1705a4e41a30e283bc3c8b1b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Tue, 10 Jul 2018 14:34:35 +0200 Subject: [PATCH] Improve error handling in idn_ace_to_locale() While idn2_to_unicode_8zlz() takes a 'flags' argument, it is ignored and thus cannot be used to perform IDN checks on the output string. The bug in libidn2 versions before 2.0.5 was not that a call to idn2_to_unicode_8zlz() with certain flags set did not cause IDN checks to be performed. The bug was that idn2_to_unicode_8zlz() did not check whether a conversion can be performed between UTF-8 and the current locale's character encoding. In other words, with libidn2 version 2.0.5+, if the current locale's character encoding is ASCII, then idn2_to_unicode_8zlz() will fail when it is passed any Punycode string which decodes to a non-ASCII string, even if it is a valid IDNA2008 name. Rework idn_ace_to_locale() so that invalid IDNA2008 names are properly and consistently detected for all libidn2 versions and locales. Update the "idna" system test accordingly. Add checks for processing a server response containing Punycode which decodes to an invalid IDNA2008 name. Fix invalid subtest description. (cherry picked from commit 7fe0f00a3bf31dc26ee3dc8777e3076546b83990) (cherry picked from commit 4fdee34a0b0ad142182914327a1bc53144b90ed1) --- bin/dig/dighost.c | 72 ++++++++++++++++++++++++------- bin/tests/system/idna/tests.sh | 77 +++++++++++----------------------- 2 files changed, 82 insertions(+), 67 deletions(-) diff --git a/bin/dig/dighost.c b/bin/dig/dighost.c index d46379ddc2..26528af3cc 100644 --- a/bin/dig/dighost.c +++ b/bin/dig/dighost.c @@ -4812,26 +4812,70 @@ idn_locale_to_ace(const char *from, char *to, size_t tolen) { static isc_result_t idn_ace_to_locale(const char *from, char *to, size_t tolen) { int res; - char *tmp_str = NULL; + char *utf8_src, *tmp_str = NULL; - res = idn2_to_unicode_8zlz(from, &tmp_str, - IDN2_NONTRANSITIONAL|IDN2_NFC_INPUT); + /* + * We need to: + * + * 1) check whether 'from' is a valid IDNA2008 name, + * 2) if it is, output it in the current locale's character encoding. + * + * Unlike idn2_to_ascii_*(), idn2_to_unicode_*() functions are unable + * to perform IDNA2008 validity checks. Thus, we need to decode any + * Punycode in 'from', check if the resulting name is a valid IDNA2008 + * name, and only once we ensure it is, output that name in the current + * locale's character encoding. + * + * We could just use idn2_to_unicode_8zlz() + idn2_to_ascii_lz(), but + * then we would not be able to universally tell invalid names and + * character encoding errors apart (if the current locale uses ASCII + * for character encoding, the former function would fail even for a + * valid IDNA2008 name, as long as it contained any non-ASCII + * character). Thus, we need to take a longer route. + * + * First, convert 'from' to UTF-8, ignoring the current locale. + */ + res = idn2_to_unicode_8z8z(from, &utf8_src, 0); + if (res != IDN2_OK) { + fatal("Bad ACE string '%s' (%s), use +noidnout", + from, idn2_strerror(res)); + } - if (res == IDN2_OK) { - /* check the length */ - if (strlen(tmp_str) >= tolen) { - debug("encoded ASC string is too long"); - idn2_free(tmp_str); - return ISC_R_FAILURE; - } + /* + * Then, check whether decoded 'from' is a valid IDNA2008 name. + */ + res = idn2_to_ascii_8z(utf8_src, NULL, IDN2_NONTRANSITIONAL); + if (res != IDN2_OK) { + fatal("'%s' is not a legal IDNA2008 name (%s), use +noidnout", + from, idn2_strerror(res)); + } - (void) strlcpy(to, tmp_str, tolen); + /* + * Finally, try converting the decoded 'from' into the current locale's + * character encoding. + */ + res = idn2_to_unicode_8zlz(utf8_src, &tmp_str, 0); + if (res != IDN2_OK) { + fatal("Cannot represent '%s' in the current locale (%s), " + "use +noidnout or a different locale", + from, idn2_strerror(res)); + } + + /* + * Free the interim conversion result. + */ + idn2_free(utf8_src); + + /* check the length */ + if (strlen(tmp_str) >= tolen) { + debug("encoded ASC string is too long"); idn2_free(tmp_str); - return ISC_R_SUCCESS; + return (ISC_R_FAILURE); } - fatal("'%s' is not a legal IDN name (%s), use +noidnout", from, idn2_strerror(res)); - return ISC_R_FAILURE; + (void) strlcpy(to, tmp_str, tolen); + idn2_free(tmp_str); + return (ISC_R_SUCCESS); } #endif /* WITH_IDN_OUT_SUPPORT */ #endif /* WITH_LIBIDN2 */ diff --git a/bin/tests/system/idna/tests.sh b/bin/tests/system/idna/tests.sh index 3a9b91b442..6637bf6828 100644 --- a/bin/tests/system/idna/tests.sh +++ b/bin/tests/system/idna/tests.sh @@ -136,46 +136,6 @@ idna_fail() { status=`expr $status + $ret` } -# Check if current version of libidn2 is >= a given version -# -# This requires that: -# a) "pkg-config" exists on the system -# b) The libidn2 installed has an associated ".pc" file -# c) The system sort command supports "-V" -# -# $1 - Minimum version required -# -# Returns: -# 0 - Version check is OK, libidn2 at required version or greater. -# 1 - Version check was made, but libidn2 not at required version. -# 2 - Could not carry out version check - -libidn_version_check() { - ret=2 - if [ -n "`command -v pkg-config`" ]; then - version=`pkg-config --modversion --silence-errors libidn2` - if [ -n "$version" ]; then - # Does the sort command have a "-V" flag on this system? - sort -V 2>&1 > /dev/null << . -. - if [ $? -eq 0 ]; then - # Sort -V exists. Sort the IDN version and the minimum version - # required. If the IDN version is greater than or equal to that - # version, it will appear last in the list. - last_version=`printf "%s\n" $version $1 | sort -V | tail -1` - if [ "$version" = "$last_version" ]; then - ret=0 - else - ret=1 - fi - fi - fi - fi - - return $ret -} - - # Function to check that case is preserved for an all-ASCII label. # # Without IDNA support, case-preservation is the expected behavior. @@ -310,16 +270,7 @@ idna_enabled_test() { text="Checking fake A-label" idna_fail "$text" "" "xn--ahahah" idna_test "$text" "+noidnin +noidnout" "xn--ahahah" "xn--ahahah." - - # Owing to issues with libdns, the next test will fail for versions of - # libidn earlier than 2.0.5. For this reason, get the version (if - # available) and compare with 2.0.5. - libidn_version_check 2.0.5 - if [ $? -ne 0 ]; then - echo_i "Skipping fake A-label +noidnin +idnout test (libidn2 version issues)" - else - idna_test "$text" "+noidnin +idnout" "xn--ahahah" "xn--ahahah." - fi + idna_fail "$text" "+noidnin +idnout" "xn--ahahah" idna_fail "$text" "+idnin +noidnout" "xn--ahahah" idna_fail "$text" "+idnin +idnout" "xn--ahahah" @@ -327,7 +278,7 @@ idna_enabled_test() { # BIND rejects such labels: with +idnin label="xn--xflod18hstflod18hstflod18hstflod18hstflod18hstflod18-1iejjjj" - text="Checking punycode label shorter than minimum valid length" + text="Checking punycode label longer than maximum valid length" idna_fail "$text" "" "$label" idna_fail "$text" "+noidnin +noidnout" "$label" idna_fail "$text" "+noidnin +idnout" "$label" @@ -337,7 +288,7 @@ idna_enabled_test() { - # Tests of a valid unicode string but an invalid U-label + # Tests of a valid unicode string but an invalid U-label (input) # # Symbols are not valid IDNA names. # @@ -347,12 +298,32 @@ idna_enabled_test() { # # The +[no]idnout options should not have any effect on the test. - text="Checking invalid U-label" + text="Checking invalid input U-label" idna_fail "$text" "" "🧦.com" idna_test "$text" "+noidnin +noidnout" "🧦.com" "\240\159\167\166.com." idna_test "$text" "+noidnin +idnout" "🧦.com" "\240\159\167\166.com." idna_fail "$text" "+idnin +noidnout" "🧦.com" idna_fail "$text" "+idnin +idnout" "🧦.com" + + # Tests of a valid unicode string but an invalid U-label (output) + # + # Symbols are not valid IDNA names. + # + # Note that an invalid U-label is accepted even when +idnin is in effect + # because "xn--19g" is valid Punycode. + # + # +noidnout: "dig" should send the ACE string to the server and display the + # returned qname. + # +idnout: "dig" should generate an error. + # + # The +[no]idnin options should not have any effect on the test. + + text="Checking invalid output U-label" + idna_fail "$text" "" "xn--19g" + idna_test "$text" "+noidnin +noidnout" "xn--19g" "xn--19g." + idna_fail "$text" "+noidnin +idnout" "xn--19g" + idna_test "$text" "+idnin +noidnout" "xn--19g" "xn--19g." + idna_fail "$text" "+idnin +idnout" "xn--19g" } -- 2.20.1