[libsoup/hsts] Ensure that hostnames in the HSTS enforcer are IDNA-canonicalized

[Claudio Saavedra <csaavedra src gnome org>]

commit c2420c87b64953696544b20cc0db54e51acdc14b
Author: Claudio Saavedra <csaavedra igalia com>
Date:   Wed Sep 26 18:01:37 2018 +0300

    Ensure that hostnames in the HSTS enforcer are IDNA-canonicalized
    
    The specification requires hostnames to be canonicalized before they
    are processed by clients. Do this, add a comment about it to the
    documentation, and add a couple of tests that mix ASCII-encoded
    hostnames and IDNA-canonicalized ones.

 libsoup/soup-hsts-enforcer.c | 25 +++++++++++++++++++++----
 libsoup/soup-hsts-policy.c   | 17 ++++++++++++++---
 tests/hsts-test.c            | 30 ++++++++++++++++++++++++++++++
 3 files changed, 65 insertions(+), 7 deletions(-)
---
diff --git a/libsoup/soup-hsts-enforcer.c b/libsoup/soup-hsts-enforcer.c
index 67ed6eb3..bd206418 100644
--- a/libsoup/soup-hsts-enforcer.c
+++ b/libsoup/soup-hsts-enforcer.c
@@ -6,8 +6,6 @@
  * Copyright (C) 2017, 2018 Metrological Group B.V.
  */
 
-/* TODO Use only internationalized domain names */
-
 #ifdef HAVE_CONFIG_H
 #include <config.h>
 #endif
@@ -496,6 +494,7 @@ preprocess_request (SoupHSTSEnforcer *enforcer, SoupMessage *msg)
        SoupURI *uri;
        const char *scheme;
        const char *host;
+       char *canonicalized = NULL;
 
        uri = soup_message_get_uri (msg);
        host = soup_uri_get_host (uri);
@@ -505,12 +504,18 @@ preprocess_request (SoupHSTSEnforcer *enforcer, SoupMessage *msg)
 
        scheme = soup_uri_get_scheme (uri);
        if (scheme == SOUP_URI_SCHEME_HTTP) {
-               if (soup_hsts_enforcer_must_enforce_secure_transport (enforcer, host)) {
+               if (g_hostname_is_ascii_encoded (host)) {
+                       canonicalized = g_hostname_to_unicode (host);
+                       if (!canonicalized)
+                               return;
+               }
+               if (soup_hsts_enforcer_must_enforce_secure_transport (enforcer, canonicalized? canonicalized 
: host)) {
                        rewrite_message_uri_to_https (msg);
                        g_signal_connect (msg, "starting",
                                          G_CALLBACK (on_sts_known_host_message_starting),
                                          enforcer);
                }
+               g_free (canonicalized);
        } else if (scheme == SOUP_URI_SCHEME_HTTPS) {
                soup_message_add_header_handler (msg, "got-headers",
                                                 "Strict-Transport-Security",
@@ -605,8 +610,20 @@ gboolean
 soup_hsts_enforcer_has_valid_policy (SoupHSTSEnforcer *hsts_enforcer,
                                     const char *domain)
 {
+       char *canonicalized = NULL;
+       gboolean retval;
+
        g_return_val_if_fail (SOUP_IS_HSTS_ENFORCER (hsts_enforcer), FALSE);
        g_return_val_if_fail (domain != NULL, FALSE);
 
-       return SOUP_HSTS_ENFORCER_GET_CLASS (hsts_enforcer)->has_valid_policy (hsts_enforcer, domain);
+       if (g_hostname_is_ascii_encoded (domain)) {
+               canonicalized = g_hostname_to_unicode (domain);
+               g_return_val_if_fail (canonicalized, FALSE);
+       }
+
+       retval = SOUP_HSTS_ENFORCER_GET_CLASS (hsts_enforcer)->has_valid_policy (hsts_enforcer, canonicalized 
? canonicalized : domain);
+
+       g_free (canonicalized);
+
+       return retval;
 }
diff --git a/libsoup/soup-hsts-policy.c b/libsoup/soup-hsts-policy.c
index 5395a3b5..6b74753e 100644
--- a/libsoup/soup-hsts-policy.c
+++ b/libsoup/soup-hsts-policy.c
@@ -38,8 +38,9 @@
  *
  * An HTTP Strict Transport Security policy.
  *
- * @domain give the host or domain that this policy belongs to and applies
- * on.
+ * @domain represents the host that this policy applies to. The domain
+ * must be IDNA-canonicalized. soup_hsts_policy_new() and related methods
+ * will do this for you.
  *
  * @max_age contains the 'max-age' value from the Strict Transport
  * Security header and indicates the time to live of this policy,
@@ -197,7 +198,17 @@ soup_hsts_policy_new_full (const char *domain,
        g_return_val_if_fail (is_hostname_valid (domain), NULL);
 
        policy = g_slice_new0 (SoupHSTSPolicy);
-       policy->domain = g_strdup (domain);
+
+       if (g_hostname_is_ascii_encoded (domain)) {
+               policy->domain = g_hostname_to_unicode (domain);
+               if (!policy->domain) {
+                       g_slice_free (SoupHSTSPolicy, policy);
+                       return NULL;
+               }
+       } else {
+               policy->domain = g_strdup (domain);
+       }
+
        policy->max_age = max_age;
        policy->expires = expires;
        policy->include_subdomains = include_subdomains;
diff --git a/tests/hsts-test.c b/tests/hsts-test.c
index 075037b7..489a8bc7 100644
--- a/tests/hsts-test.c
+++ b/tests/hsts-test.c
@@ -432,6 +432,35 @@ do_hsts_session_policy_test (void)
        g_object_unref (enforcer);
 }
 
+static void
+on_idna_test_enforcer_changed (SoupHSTSEnforcer *enforcer, SoupHSTSPolicy *old, SoupHSTSPolicy *new, 
gpointer data)
+{
+       /* If NULL, then instead of replacing we're adding a new
+        * policy and somewhere we're failing to canonicalize a hostname. */
+       g_assert_nonnull (old);
+       g_assert_cmpstr (old->domain, ==, new->domain);
+       /*  Domains should not have punycoded segments at this point. */
+       g_assert_false (g_hostname_is_ascii_encoded (old->domain));
+}
+
+static void
+do_hsts_idna_addresses_test (void)
+{
+       SoupHSTSEnforcer *enforcer = soup_hsts_enforcer_new ();
+
+       soup_hsts_enforcer_set_session_policy (enforcer, "áéí.com", FALSE);
+       soup_hsts_enforcer_set_session_policy (enforcer, "xn--6scagyk0fc4c.in", FALSE);
+
+       g_assert_true (soup_hsts_enforcer_has_valid_policy (enforcer, "xn--1caqm.com"));
+
+       g_signal_connect (enforcer, "changed", G_CALLBACK (on_idna_test_enforcer_changed), NULL);
+
+       soup_hsts_enforcer_set_session_policy (enforcer, "xn--1caqm.com", TRUE);
+       soup_hsts_enforcer_set_session_policy (enforcer, "ನೆನಪಿರಲಿ.in", TRUE);
+
+       g_object_unref (enforcer);
+}
+
 int
 main (int argc, char **argv)
 {
@@ -473,6 +502,7 @@ main (int argc, char **argv)
        g_test_add_func ("/hsts/ip-address", do_hsts_ip_address_test);
        g_test_add_func ("/hsts/utf8-address", do_hsts_utf8_address_test);
        g_test_add_func ("/hsts/session-policy", do_hsts_session_policy_test);
+       g_test_add_func ("/hsts/idna-addresses", do_hsts_idna_addresses_test);
 
        ret = g_test_run ();