From 0926f3489dbc93a2e91ec8607a0ceb5d4d8a53e3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
Date: Fri, 21 Jul 2017 07:51:07 -0400
Subject: [PATCH] resolved: make sure idn2 conversions are roundtrippable
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

While working on the gateway→_gateway conversion, I noticed that
libidn2 strips the leading underscore in some names.
https://gitlab.com/libidn/libidn2/issues/30 was resolved in
https://gitlab.com/libidn/libidn2/commit/05d753ea69e2308cd02436d0511f4b844071dc79,
which disabled "STD3 ASCII rules" by default, i.e. disabled stripping
of underscores. So the situation is that with previously released libidn2
versions we would get incorrect behaviour, and once new libidn2 is released,
we should be OK.

Let's implement a simple test which checks that the name survives the
roundtrip, and if it doesn't, skip IDN resolution. Under old libidn2 this will
fail in more cases, and under new libidn2 in fewer, but should be the right
thing to do also under new libidn2.
---
 src/shared/dns-domain.c    | 29 ++++++++++++++++++++++++++---
 src/test/test-dns-domain.c |  6 ++++++
 2 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/src/shared/dns-domain.c b/src/shared/dns-domain.c
index 12c4d65dd3..139d286af8 100644
--- a/src/shared/dns-domain.c
+++ b/src/shared/dns-domain.c
@@ -1274,15 +1274,38 @@ int dns_name_apply_idna(const char *name, char **ret) {
 
 #if defined(HAVE_LIBIDN2)
         int r;
+        _cleanup_free_ char *t = NULL;
 
         assert(name);
         assert(ret);
 
-        r = idn2_lookup_u8((uint8_t*) name, (uint8_t**) ret,
+        r = idn2_lookup_u8((uint8_t*) name, (uint8_t**) &t,
                            IDN2_NFC_INPUT | IDN2_NONTRANSITIONAL);
-        if (r == IDN2_OK)
+        log_debug("idn2_lookup_u8: %s → %s", name, t);
+        if (r == IDN2_OK) {
+                if (!startswith(name, "xn--")) {
+                        _cleanup_free_ char *s = NULL;
+
+                        r = idn2_to_unicode_8z8z(t, &s, 0);
+                        if (r != IDN2_OK) {
+                                log_debug("idn2_to_unicode_8z8z(\"%s\") failed: %d/%s",
+                                          t, r, idn2_strerror(r));
+                                return 0;
+                        }
+
+                        if (!streq_ptr(name, s)) {
+                                log_debug("idn2 roundtrip failed: \"%s\" → \"%s\" → \"%s\", ignoring.",
+                                          name, t, s);
+                                return 0;
+                        }
+                }
+
+                *ret = t;
+                t = NULL;
                 return 1; /* *ret has been written */
-        log_debug("idn2_lookup_u8(\"%s\") failed: %s", name, idn2_strerror(r));
+        }
+
+        log_debug("idn2_lookup_u8(\"%s\") failed: %d/%s", name, r, idn2_strerror(r));
         if (r == IDN2_2HYPHEN)
                 /* The name has two hypens — forbidden by IDNA2008 in some cases */
                 return 0;
diff --git a/src/test/test-dns-domain.c b/src/test/test-dns-domain.c
index 11cf0b1f0b..cbd2d1e656 100644
--- a/src/test/test-dns-domain.c
+++ b/src/test/test-dns-domain.c
@@ -652,6 +652,12 @@ static void test_dns_name_apply_idna(void) {
         test_dns_name_apply_idna_one("föö.bär.", ret, "xn--f-1gaa.xn--br-via");
         test_dns_name_apply_idna_one("xn--f-1gaa.xn--br-via", ret, "xn--f-1gaa.xn--br-via");
 
+        test_dns_name_apply_idna_one("_443._tcp.fedoraproject.org", ret2,
+                                     "_443._tcp.fedoraproject.org");
+        test_dns_name_apply_idna_one("_443", ret2, "_443");
+        test_dns_name_apply_idna_one("gateway", ret, "gateway");
+        test_dns_name_apply_idna_one("_gateway", ret2, "_gateway");
+
         test_dns_name_apply_idna_one("r3---sn-ab5l6ne7.googlevideo.com", ret2,
                                      ret2 ? "r3---sn-ab5l6ne7.googlevideo.com" : "");
 }