On 04/01/2018 09:42 PM, Florian Weimer wrote:
> * Zack Weinberg:
>
>> On Sun, Apr 1, 2018 at 2:54 PM, Florian Weimer <fweimer@redhat.com> wrote:
>>> On 03/15/2018 11:03 PM, Florian Weimer wrote:
>>>>
>>>> For some reason, libidn2 does not fail the conversion, but uses the
>>>> replacement character '?' for unencodable characters. This will have to be
>>>> fixed in libidn2 eventually.>
>>>
>>> This has now been fixed in upstream libidn2.
>>
>> I don't think I can comment usefully on any of the code changes but I
>> have some concerns about the behavior in some of the failure cases.
>> Most importantly, I don't understand exactly what we do if the
>> application calls getaddrinfo, passing AI_IDN, with a name that would
>> be changed by punycoding it, but libidn2 is not available. The NEWS
>> makes it sound like we might transmit a raw non-ASCII name to the DNS
>> server in that case, and I think we shouldn't do that.
>
> Only very few applications currently use AI_IDN. This means that if
> the user enters such a name with a non-AI_IDN application, exactly the
> same thing will happen. That's why I thought the fallback behavior
> made sense.
>
> It's probably more of a performance optimization to change this: We
> could check the name for non-ASCII characters (and backslashes,
> because interesting things happen with them) and load and call into
> libidn2 only if there are such non-ASCII characters.
Here's a patch implementing this. I'd like to commit this separately
because it is slightly less well-tested than the rest.
>> I also wonder if we shouldn't somehow detect and refuse to use
>> versions of libidn2 without all the necessary bugfixes.
>
> The ??? transcoding bug is probably something approaching a security
> bug, but I'm not sure if it can actually be triggered in a UTF-8
> locale. (The point is that for getnameinfo and AI_CANONIDN, the ?
> replacement character is introduced *after* the meta-character checks
> in nss_dns, and pathname expansion in the shell could result in all
> kinds of unwanted characters.) The other bugs are really benign, and
> there's no evidence that they have been encountered in the wild.
It also papers over part of the transcoding bug because it will
recognize invalid multi-byte sequences and not pass them to libidn2.
Thanks,
Florian
Subject: [PATCH] getaddrinfo: Check for IDNA name before calling into libidn2
To: libc-alpha@sourceware.org
This avoids unnecessary loading of libidn2, and sending
unencoded non-ASCII names over the network.
2018-04-04 Florian Weimer <fweimer@redhat.com>
* inet/Makefile (tests-static, tests-internal): Add
tst-idna_name_classify.
(LOCALES): Generate locales for tests.
(tst-idna_name_classify.out): Depend on generated locales.
* inet/idna_name_classify.c: New file.
* inet/idna.c (__idna_to_dns_encoding): Call __idna_name_classify
before encoding. Do not send unencoded non-ASCII names over the
network.
* inet/net-internal.h (enum idna_name_classification): Define.
(__idna_name_classify): Declare.
* inet/tst-idna_name_classify.c: New file.
@@ -28,11 +28,13 @@ Major new features:
* IDN domain names in getaddrinfo and getnameinfo now use the system libidn2
library if installed. If libidn2 is not available, internationalized
domain names are not encoded or decoded even if the AI_IDN or NI_IDN flags
- are passed to getaddrinfo or getnameinfo. Flags which used to change the
- IDN encoding and decoding behavior (AI_IDN_ALLOW_UNASSIGNED,
- AI_IDN_USE_STD3_ASCII_RULES, NI_IDN_ALLOW_UNASSIGNED,
- NI_IDN_USE_STD3_ASCII_RULES) have been deprecated. They no longer have
- any effect. (libidn2 allows yet-unassigned characters by default.)
+ are passed to getaddrinfo or getnameinfo. (getaddrinfo calls with
+ non-ASCII names and AI_IDN will fail with an encoding error.) Flags which
+ used to change the IDN encoding and decoding behavior
+ (AI_IDN_ALLOW_UNASSIGNED, AI_IDN_USE_STD3_ASCII_RULES,
+ NI_IDN_ALLOW_UNASSIGNED, NI_IDN_USE_STD3_ASCII_RULES) have been
+ deprecated. They no longer have any effect. (libidn2 allows
+ yet-unassigned characters by default.)
Deprecated and removed features, and other changes affecting compatibility:
@@ -45,7 +45,7 @@ routines := htonl htons \
in6_addr getnameinfo if_index ifaddrs inet6_option \
getipv4sourcefilter setipv4sourcefilter \
getsourcefilter setsourcefilter inet6_opt inet6_rth \
- inet6_scopeid_pton deadline idna
+ inet6_scopeid_pton deadline idna idna_name_classify
aux := check_pf check_native ifreq
@@ -59,12 +59,20 @@ tests := htontest test_ifindex tst-ntoa tst-ether_aton tst-network \
tests-static += tst-deadline
tests-internal += tst-deadline
+# tst-idna_name_classify must be linked statically because it tests
+# internal functionality.
+tests-static += tst-idna_name_classify
+tests-internal += tst-idna_name_classify
+
# tst-inet6_scopeid_pton also needs internal functions but does not
# need to be linked statically.
tests-internal += tst-inet6_scopeid_pton
include ../Rules
+LOCALES := en_US.UTF-8 en_US.ISO-8859-1
+include ../gen-locales.mk
+
ifeq ($(have-thread-library),yes)
CFLAGS-gethstbyad_r.c += -fexceptions
@@ -103,3 +111,5 @@ endif
ifeq ($(build-static-nss),yes)
CFLAGS += -DSTATIC_NSS
endif
+
+$(objpfx)tst-idna_name_classify.out: $(gen-locales)
@@ -112,11 +112,29 @@ gai_strdup (const char *name, char **result)
int
__idna_to_dns_encoding (const char *name, char **result)
{
+ switch (__idna_name_classify (name))
+ {
+ case idna_name_ascii:
+ /* Nothing to convert. */
+ return gai_strdup (name, result);
+ case idna_name_nonascii:
+ /* Encoding needed. Handled below. */
+ break;
+ case idna_name_nonascii_backslash:
+ case idna_name_encoding_error:
+ return EAI_IDN_ENCODE;
+ case idna_name_memory_error:
+ return EAI_MEMORY;
+ case idna_name_error:
+ return EAI_SYSTEM;
+ }
+
struct functions *functions = get_functions ();
if (functions == NULL)
- /* Forward the domain name to the NSS module unencoded. It may be
- able to apply its own encoding to find an answer. */
- return gai_strdup (name, result);
+ /* We report this as an encoding error (assuming that libidn2 is
+ not installed), although the root cause may be a temporary
+ error condition due to resource shortage. */
+ return EAI_IDN_ENCODE;
char *ptr = NULL;
__typeof__ (functions->lookup_ul) fptr = functions->lookup_ul;
#ifdef PTR_DEMANGLE
new file mode 100644
@@ -0,0 +1,75 @@
+/* Classify a domain name for IDNA purposes.
+ Copyright (C) 2018 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+#include <errno.h>
+#include <inet/net-internal.h>
+#include <stdbool.h>
+#include <string.h>
+#include <wchar.h>
+
+enum idna_name_classification
+__idna_name_classify (const char *name)
+{
+ mbstate_t mbs;
+ memset (&mbs, 0, sizeof (mbs));
+ const char *p = name;
+ const char *end = p + strlen (p) + 1;
+ bool nonascii = false;
+ bool backslash = false;
+ while (true)
+ {
+ wchar_t wc;
+ size_t result = mbrtowc (&wc, p, end - p, &mbs);
+ if (result == 0)
+ /* NUL terminator was reached. */
+ break;
+ else if (result == (size_t) -2)
+ /* Incomplete trailing multi-byte character. This is an
+ encoding error becaue we received the full name. */
+ return idna_name_encoding_error;
+ else if (result == (size_t) -1)
+ {
+ /* Other error, including EILSEQ. */
+ if (errno == EILSEQ)
+ return idna_name_encoding_error;
+ else if (errno == ENOMEM)
+ return idna_name_memory_error;
+ else
+ return idna_name_error;
+ }
+ else
+ {
+ /* A wide character was decoded. */
+ p += result;
+ if (wc == L'\\')
+ backslash = true;
+ else if (wc > 127)
+ nonascii = true;
+ }
+ }
+
+ if (nonascii)
+ {
+ if (backslash)
+ return idna_name_nonascii_backslash;
+ else
+ return idna_name_nonascii;
+ }
+ else
+ return idna_name_ascii;
+}
@@ -40,6 +40,22 @@ int __idna_from_dns_encoding (const char *name, char **result);
libc_hidden_proto (__idna_from_dns_encoding)
+/* Return value of __idna_name_classify below. */
+enum idna_name_classification
+{
+ idna_name_ascii, /* No non-ASCII characters. */
+ idna_name_nonascii, /* Non-ASCII characters, no backslash. */
+ idna_name_nonascii_backslash, /* Non-ASCII characters with backslash. */
+ idna_name_encoding_error, /* Decoding error. */
+ idna_name_memory_error, /* Memory allocation failure. */
+ idna_name_error, /* Other error during decoding. Check errno. */
+};
+
+/* Check the specified name for non-ASCII characters and backslashes
+ or encoding errors. */
+enum idna_name_classification __idna_name_classify (const char *name)
+ attribute_hidden;
+
/* Deadline handling for enforcing timeouts.
Code should call __deadline_current_time to obtain the current time
new file mode 100644
@@ -0,0 +1,73 @@
+/* Test IDNA name classification.
+ Copyright (C) 2018 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+#include <inet/net-internal.h>
+#include <locale.h>
+#include <stdio.h>
+#include <support/check.h>
+
+static void
+locale_insensitive_tests (void)
+{
+ TEST_COMPARE (__idna_name_classify (""), idna_name_ascii);
+ TEST_COMPARE (__idna_name_classify ("abc"), idna_name_ascii);
+ TEST_COMPARE (__idna_name_classify (".."), idna_name_ascii);
+ TEST_COMPARE (__idna_name_classify ("\001abc\177"), idna_name_ascii);
+ TEST_COMPARE (__idna_name_classify ("\\065bc"), idna_name_ascii);
+}
+
+static int
+do_test (void)
+{
+ puts ("info: C locale tests");
+ locale_insensitive_tests ();
+ TEST_COMPARE (__idna_name_classify ("abc\200def"),
+ idna_name_encoding_error);
+ TEST_COMPARE (__idna_name_classify ("abc\200\\def"),
+ idna_name_encoding_error);
+ TEST_COMPARE (__idna_name_classify ("abc\377def"),
+ idna_name_encoding_error);
+
+ puts ("info: en_US.ISO-8859-1 locale tests");
+ if (setlocale (LC_CTYPE, "en_US.ISO-8859-1") == 0)
+ FAIL_EXIT1 ("setlocale for en_US.ISO-8859-1: %m\n");
+ locale_insensitive_tests ();
+ TEST_COMPARE (__idna_name_classify ("abc\200def"), idna_name_nonascii);
+ TEST_COMPARE (__idna_name_classify ("abc\377def"), idna_name_nonascii);
+ TEST_COMPARE (__idna_name_classify ("abc\\\200def"),
+ idna_name_nonascii_backslash);
+ TEST_COMPARE (__idna_name_classify ("abc\200\\def"),
+ idna_name_nonascii_backslash);
+
+ puts ("info: en_US.UTF-8 locale tests");
+ if (setlocale (LC_CTYPE, "en_US.UTF-8") == 0)
+ FAIL_EXIT1 ("setlocale for en_US.UTF-8: %m\n");
+ locale_insensitive_tests ();
+ TEST_COMPARE (__idna_name_classify ("abc\xc3\x9f""def"), idna_name_nonascii);
+ TEST_COMPARE (__idna_name_classify ("abc\\\xc3\x9f""def"),
+ idna_name_nonascii_backslash);
+ TEST_COMPARE (__idna_name_classify ("abc\xc3\x9f\\def"),
+ idna_name_nonascii_backslash);
+ TEST_COMPARE (__idna_name_classify ("abc\200def"), idna_name_encoding_error);
+ TEST_COMPARE (__idna_name_classify ("abc\xc3""def"), idna_name_encoding_error);
+ TEST_COMPARE (__idna_name_classify ("abc\xc3"), idna_name_encoding_error);
+
+ return 0;
+}
+
+#include <support/test-driver.c>
@@ -37,12 +37,15 @@ gai_tests (void)
"canonname: non-idn.example\n"
"address: STREAM/TCP 192.0.2.110 80\n");
+ /* This gets passed over the network to the server, so it will
+ result in an NXDOMAIN error. */
check_ai (NAEMCHEN ".example", 0,
"error: Name or service not known\n");
+ /* Due to missing libidn2, this fails inside getaddrinfo. */
check_ai (NAEMCHEN ".example", AI_IDN,
- "error: Name or service not known\n");
+ "error: Parameter string not correctly encoded\n");
check_ai (NAEMCHEN ".example", AI_IDN | AI_CANONNAME | AI_CANONIDN,
- "error: Name or service not known\n");
+ "error: Parameter string not correctly encoded\n");
/* Non-IDN CNAME. */
check_ai ("with.cname.example", 0,