From 88cf9dec060e8c3f1705a4e41a30e283bc3c8b1b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= <michal@isc.org>
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