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.

3 files changed, 66 insertions, 7 deletions

diff --git a/libsoup/soup-hsts-enforcer.c b/libsoup/soup-hsts-enforcer.c
index 82c3b9d6..8d0e9e54 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,21 @@ 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 794b4d8d..5036b912 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,
@@ -195,7 +196,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 aba7859c..0f916e4b 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 ();