[libsoup/wip/tingping/secure-cookies] Implement strict secure cookies

[Patrick Griffis <pgriffis src gnome org>]

commit 04d85e1b523f183f1c4f4d8e973a094f66cdf238
Author: Patrick Griffis <pgriffis igalia com>
Date:   Thu Feb 28 16:26:40 2019 -0500

    Implement strict secure cookies
    
    This prevents insecure origins from creating or modifying secure cookies.
    
    https://tools.ietf.org/html/draft-ietf-httpbis-cookie-alone-01

 docs/reference/libsoup-2.4-sections.txt |   1 +
 libsoup/soup-cookie-jar.c               | 156 +++++++++++++++++++-------------
 libsoup/soup-cookie-jar.h               |   5 +
 tests/cookies-test.c                    |  44 +++++++++
 4 files changed, 144 insertions(+), 62 deletions(-)
---
diff --git a/docs/reference/libsoup-2.4-sections.txt b/docs/reference/libsoup-2.4-sections.txt
index 354c0783..ecbcc954 100644
--- a/docs/reference/libsoup-2.4-sections.txt
+++ b/docs/reference/libsoup-2.4-sections.txt
@@ -951,6 +951,7 @@ soup_cookie_jar_set_cookie_with_first_party
 <SUBSECTION>
 soup_cookie_jar_add_cookie
 soup_cookie_jar_add_cookie_with_first_party
+soup_cookie_jar_add_cookie_full
 soup_cookie_jar_delete_cookie
 soup_cookie_jar_all_cookies
 <SUBSECTION>
diff --git a/libsoup/soup-cookie-jar.c b/libsoup/soup-cookie-jar.c
index b2b78909..1f9e23e9 100644
--- a/libsoup/soup-cookie-jar.c
+++ b/libsoup/soup-cookie-jar.c
@@ -433,21 +433,61 @@ soup_cookie_jar_get_cookie_list (SoupCookieJar *jar, SoupURI *uri, gboolean for_
        return get_cookies (jar, uri, for_http, TRUE);
 }
 
+static const char *
+normalize_cookie_domain (const char *domain)
+{
+       /* Trim any leading dot if present to transform the cookie
+         * domain into a valid hostname.
+         */
+       if (domain != NULL && domain[0] == '.')
+               return domain + 1;
+       return domain;
+}
+
+static gboolean
+incoming_cookie_is_third_party (SoupCookie *cookie, SoupURI *first_party)
+{
+       const char *normalized_cookie_domain;
+       const char *cookie_base_domain;
+       const char *first_party_base_domain;
+
+       if (first_party == NULL || first_party->host == NULL)
+               return TRUE;
+
+       normalized_cookie_domain = normalize_cookie_domain (cookie->domain);
+       cookie_base_domain = soup_tld_get_base_domain (normalized_cookie_domain, NULL);
+       if (cookie_base_domain == NULL)
+               cookie_base_domain = cookie->domain;
+
+       first_party_base_domain = soup_tld_get_base_domain (first_party->host, NULL);
+       if (first_party_base_domain == NULL)
+               first_party_base_domain = first_party->host;
+       return !soup_host_matches_host (cookie_base_domain, first_party_base_domain);
+}
+
 /**
- * soup_cookie_jar_add_cookie:
+ * soup_cookie_jar_add_cookie_full:
  * @jar: a #SoupCookieJar
  * @cookie: (transfer full): a #SoupCookie
+ * @uri: (nullable): the URI setting the cookie
+ * @first_party: (nullable): the URI for the main document
  *
  * Adds @cookie to @jar, emitting the 'changed' signal if we are modifying
  * an existing cookie or adding a valid new cookie ('valid' means
  * that the cookie's expire date is not in the past).
  *
+ * @first_party will be used to reject cookies coming from third party
+ * resources in case such a security policy is set in the @jar.
+ *
+ * @uri will be used to reject setting or overwriting secure cookies
+ * from insecure origins. %NULL is treated as secure.
+ * 
  * @cookie will be 'stolen' by the jar, so don't free it afterwards.
  *
- * Since: 2.26
+ * Since: 2.68
  **/
 void
-soup_cookie_jar_add_cookie (SoupCookieJar *jar, SoupCookie *cookie)
+soup_cookie_jar_add_cookie_full (SoupCookieJar *jar, SoupCookie *cookie, SoupURI *uri, SoupURI *first_party)
 {
        SoupCookieJarPrivate *priv;
        GSList *old_cookies, *oc, *last = NULL;
@@ -464,12 +504,33 @@ soup_cookie_jar_add_cookie (SoupCookieJar *jar, SoupCookie *cookie)
        }
 
        priv = soup_cookie_jar_get_instance_private (jar);
+
+       if (first_party != NULL) {
+               if (priv->accept_policy == SOUP_COOKIE_JAR_ACCEPT_NEVER ||
+                    (priv->accept_policy == SOUP_COOKIE_JAR_ACCEPT_NO_THIRD_PARTY &&
+                    incoming_cookie_is_third_party (cookie, first_party))) {
+                       soup_cookie_free (cookie);
+                       return;
+               }
+       }
+
+       /* Cannot set a secure cookie over http */
+       if (uri != NULL && !soup_uri_is_https (uri, NULL) && soup_cookie_get_secure (cookie)) {
+               soup_cookie_free (cookie);
+               return;
+       }
+
        old_cookies = g_hash_table_lookup (priv->domains, cookie->domain);
        for (oc = old_cookies; oc; oc = oc->next) {
                old_cookie = oc->data;
                if (!strcmp (cookie->name, old_cookie->name) &&
                    !g_strcmp0 (cookie->path, old_cookie->path)) {
-                       if (cookie->expires && soup_date_is_past (cookie->expires)) {
+                       if (soup_cookie_get_secure (oc->data) && uri != NULL && !soup_uri_is_https (uri, 
NULL)) {
+                               /* We do not allow overwriting secure cookies from an insecure origin
+                                * https://tools.ietf.org/html/draft-ietf-httpbis-cookie-alone-01
+                                */
+                               soup_cookie_free (cookie);
+                       } else if (cookie->expires && soup_date_is_past (cookie->expires)) {
                                /* The new cookie has an expired date,
                                 * this is the way the the server has
                                 * of telling us that we have to
@@ -510,36 +571,23 @@ soup_cookie_jar_add_cookie (SoupCookieJar *jar, SoupCookie *cookie)
        soup_cookie_jar_changed (jar, NULL, cookie);
 }
 
-static const char *
-normalize_cookie_domain (const char *domain)
-{
-       /* Trim any leading dot if present to transform the cookie
-         * domain into a valid hostname.
-         */
-       if (domain != NULL && domain[0] == '.')
-               return domain + 1;
-       return domain;
-}
-
-static gboolean
-incoming_cookie_is_third_party (SoupCookie *cookie, SoupURI *first_party)
+/**
+ * soup_cookie_jar_add_cookie:
+ * @jar: a #SoupCookieJar
+ * @cookie: (transfer full): a #SoupCookie
+ *
+ * Adds @cookie to @jar, emitting the 'changed' signal if we are modifying
+ * an existing cookie or adding a valid new cookie ('valid' means
+ * that the cookie's expire date is not in the past).
+ *
+ * @cookie will be 'stolen' by the jar, so don't free it afterwards.
+ *
+ * Since: 2.26
+ **/
+void
+soup_cookie_jar_add_cookie (SoupCookieJar *jar, SoupCookie *cookie)
 {
-       const char *normalized_cookie_domain;
-       const char *cookie_base_domain;
-       const char *first_party_base_domain;
-
-       if (first_party == NULL || first_party->host == NULL)
-               return TRUE;
-
-       normalized_cookie_domain = normalize_cookie_domain (cookie->domain);
-       cookie_base_domain = soup_tld_get_base_domain (normalized_cookie_domain, NULL);
-       if (cookie_base_domain == NULL)
-               cookie_base_domain = cookie->domain;
-
-       first_party_base_domain = soup_tld_get_base_domain (first_party->host, NULL);
-       if (first_party_base_domain == NULL)
-               first_party_base_domain = first_party->host;
-       return !soup_host_matches_host (cookie_base_domain, first_party_base_domain);
+       soup_cookie_jar_add_cookie_full (jar, cookie, NULL, NULL);
 }
 
 /**
@@ -557,30 +605,17 @@ incoming_cookie_is_third_party (SoupCookie *cookie, SoupURI *first_party)
  *
  * @cookie will be 'stolen' by the jar, so don't free it afterwards.
  *
+ * For secure cookies to work properly you may want to use
+ * soup_cookie_jar_add_cookie_full().
+ *
  * Since: 2.40
  **/
 void
 soup_cookie_jar_add_cookie_with_first_party (SoupCookieJar *jar, SoupURI *first_party, SoupCookie *cookie)
 {
-       SoupCookieJarPrivate *priv;
-
-       g_return_if_fail (SOUP_IS_COOKIE_JAR (jar));
        g_return_if_fail (first_party != NULL);
-       g_return_if_fail (cookie != NULL);
 
-       priv = soup_cookie_jar_get_instance_private (jar);
-       if (priv->accept_policy == SOUP_COOKIE_JAR_ACCEPT_NEVER) {
-               soup_cookie_free (cookie);
-               return;
-       }
-
-       if (priv->accept_policy == SOUP_COOKIE_JAR_ACCEPT_ALWAYS ||
-           !incoming_cookie_is_third_party (cookie, first_party)) {
-               /* will steal or free soup_cookie */
-               soup_cookie_jar_add_cookie (jar, cookie);
-       } else {
-               soup_cookie_free (cookie);
-       }
+       soup_cookie_jar_add_cookie_full (jar, cookie, NULL, first_party);
 }
 
 /**
@@ -623,7 +658,7 @@ soup_cookie_jar_set_cookie (SoupCookieJar *jar, SoupURI *uri,
        soup_cookie = soup_cookie_parse (cookie, uri);
        if (soup_cookie) {
                /* will steal or free soup_cookie */
-               soup_cookie_jar_add_cookie (jar, soup_cookie);
+               soup_cookie_jar_add_cookie_full (jar, soup_cookie, uri, NULL);
        }
 }
 
@@ -658,8 +693,9 @@ soup_cookie_jar_set_cookie_with_first_party (SoupCookieJar *jar,
                return;
 
        soup_cookie = soup_cookie_parse (cookie, uri);
-       if (soup_cookie)
-               soup_cookie_jar_add_cookie_with_first_party (jar, first_party, soup_cookie);
+       if (soup_cookie) {
+               soup_cookie_jar_add_cookie_full (jar, soup_cookie, uri, first_party);
+       }
 }
 
 static void
@@ -668,20 +704,16 @@ process_set_cookie_header (SoupMessage *msg, gpointer user_data)
        SoupCookieJar *jar = user_data;
        SoupCookieJarPrivate *priv = soup_cookie_jar_get_instance_private (jar);
        GSList *new_cookies, *nc;
+       SoupURI *first_party, *uri;
 
        if (priv->accept_policy == SOUP_COOKIE_JAR_ACCEPT_NEVER)
                return;
 
        new_cookies = soup_cookies_from_response (msg);
-       for (nc = new_cookies; nc; nc = nc->next) {
-               SoupURI *first_party = soup_message_get_first_party (msg);
-               
-               if ((priv->accept_policy == SOUP_COOKIE_JAR_ACCEPT_NO_THIRD_PARTY &&
-                    !incoming_cookie_is_third_party (nc->data, first_party)) ||
-                   priv->accept_policy == SOUP_COOKIE_JAR_ACCEPT_ALWAYS)
-                       soup_cookie_jar_add_cookie (jar, nc->data);
-               else
-                       soup_cookie_free (nc->data);
+       first_party = soup_message_get_first_party (msg);
+       uri = soup_message_get_uri (msg);
+       for (nc = new_cookies; nc; nc = nc->next) {             
+               soup_cookie_jar_add_cookie_full (jar, g_steal_pointer (&nc->data), uri, first_party);
        }
        g_slist_free (new_cookies);
 }
diff --git a/libsoup/soup-cookie-jar.h b/libsoup/soup-cookie-jar.h
index d9c6850e..d3ee4f23 100644
--- a/libsoup/soup-cookie-jar.h
+++ b/libsoup/soup-cookie-jar.h
@@ -75,6 +75,11 @@ SOUP_AVAILABLE_IN_2_40
 void                      soup_cookie_jar_add_cookie_with_first_party (SoupCookieJar             *jar,
                                                                       SoupURI                   *first_party,
                                                                       SoupCookie                *cookie);
+SOUP_AVAILABLE_IN_2_68
+void                      soup_cookie_jar_add_cookie_full             (SoupCookieJar             *jar,
+                                                                       SoupCookie                *cookie,
+                                                                      SoupURI                   *uri,
+                                                                      SoupURI                   
*first_party);
 SOUP_AVAILABLE_IN_2_26
 void                      soup_cookie_jar_delete_cookie               (SoupCookieJar             *jar,
                                                                       SoupCookie                *cookie);
diff --git a/tests/cookies-test.c b/tests/cookies-test.c
index f2fcc63f..8161ce1b 100644
--- a/tests/cookies-test.c
+++ b/tests/cookies-test.c
@@ -209,6 +209,49 @@ do_cookies_subdomain_policy_test (void)
        g_object_unref (jar);
 }
 
+static void
+do_cookies_strict_secure_test (void)
+{
+       SoupCookieJar *jar;
+       GSList *cookies;
+       SoupURI *insecure_uri;
+       SoupURI *secure_uri;
+
+       insecure_uri = soup_uri_new ("http://gnome.org";);
+       secure_uri = soup_uri_new ("https://gnome.org";);
+       jar = soup_cookie_jar_new ();
+
+       /* Set a cookie from secure origin */
+       soup_cookie_jar_set_cookie (jar, secure_uri, "1=foo; secure");
+       cookies = soup_cookie_jar_all_cookies (jar);
+       g_assert_cmpint (g_slist_length (cookies), ==, 1);
+       g_assert_cmpstr (soup_cookie_get_value(cookies->data), ==, "foo");
+       g_slist_free_full (cookies, (GDestroyNotify)soup_cookie_free);
+
+       /* Do not allow an insecure origin to overwrite a secure cookie */
+       soup_cookie_jar_set_cookie (jar, insecure_uri, "1=bar");
+       cookies = soup_cookie_jar_all_cookies (jar);
+       g_assert_cmpint (g_slist_length (cookies), ==, 1);
+       g_assert_cmpstr (soup_cookie_get_value(cookies->data), ==, "foo");
+       g_slist_free_full (cookies, (GDestroyNotify)soup_cookie_free);
+
+       /* Secure can only be set by from secure origin */
+       soup_cookie_jar_set_cookie (jar, insecure_uri, "2=foo; secure");
+       cookies = soup_cookie_jar_all_cookies (jar);
+       g_assert_cmpint (g_slist_length (cookies), ==, 1);
+       g_slist_free_full (cookies, (GDestroyNotify)soup_cookie_free);
+
+       /* But we can make one for another path */
+       soup_cookie_jar_set_cookie (jar, insecure_uri, "1=foo; path=/foo");
+       cookies = soup_cookie_jar_all_cookies (jar);
+       g_assert_cmpint (g_slist_length (cookies), ==, 2);
+       g_slist_free_full (cookies, (GDestroyNotify)soup_cookie_free);
+
+       soup_uri_free (insecure_uri);
+       soup_uri_free (secure_uri);
+       g_object_unref (jar);
+}
+
 /* FIXME: moar tests! */
 static void
 do_cookies_parsing_test (void)
@@ -361,6 +404,7 @@ main (int argc, char **argv)
        g_test_add_func ("/cookies/parsing/no-path-null-origin", do_cookies_parsing_nopath_nullorigin);
        g_test_add_func ("/cookies/get-cookies/empty-host", do_get_cookies_empty_host_test);
        g_test_add_func ("/cookies/remove-feature", do_remove_feature_test);
+       g_test_add_func ("/cookies/secure-cookies", do_cookies_strict_secure_test);
 
        ret = g_test_run ();