cookies: leave secure cookies alone

Only allow secure origins to be able to write cookies with the
'secure' flag set. This reduces the risk of non-secure origins
to influence the state of secure origins. This implements IETF
Internet-Draft draft-ietf-httpbis-cookie-alone-01 which updates
RFC6265.

Closes #2956
Reviewed-by: Daniel Stenberg <daniel@haxx.se>

11 files changed, 148 insertions, 43 deletions

diff --git a/docs/HTTP-COOKIES.md b/docs/HTTP-COOKIES.md
index a1b283454..66e39d232 100644
--- a/docs/HTTP-COOKIES.md
+++ b/docs/HTTP-COOKIES.md
@@ -18,7 +18,9 @@
   original [Netscape spec from 1994](https://curl.haxx.se/rfc/cookie_spec.html).
 
   In 2011, [RFC6265](https://www.ietf.org/rfc/rfc6265.txt) was finally
-  published and details how cookies work within HTTP.
+  published and details how cookies work within HTTP. In 2017, an update was
+  [drafted](https://tools.ietf.org/html/draft-ietf-httpbis-cookie-alone-01)
+  to deprecate modification of 'secure' cookies from non-secure origins.
 
 ## Cookies saved to disk
 
diff --git a/docs/TODO b/docs/TODO
index f7fd722a8..e0d8ed68f 100644
--- a/docs/TODO
+++ b/docs/TODO
@@ -73,7 +73,6 @@
  5.5 auth= in URLs
  5.6 Refuse "downgrade" redirects
  5.7 QUIC
- 5.8 Leave secure cookies alone
 
  6. TELNET
  6.1 ditch stdin
@@ -605,13 +604,6 @@
  implemented. This, to allow other projects to benefit from the work and to
  thus broaden the interest and chance of others to participate.
 
-5.8 Leave secure cookies alone
-
- Non-secure origins (HTTP sites) should not be allowed to set or modify
- cookies with the 'secure' property:
-
- https://tools.ietf.org/html/draft-ietf-httpbis-cookie-alone-01
-
 
 6. TELNET
 
diff --git a/lib/cookie.c b/lib/cookie.c
index 3dc85ee5c..bc0ab0dfe 100644
--- a/lib/cookie.c
+++ b/lib/cookie.c
@@ -433,9 +433,10 @@ Curl_cookie_add(struct Curl_easy *data,
                 bool noexpire, /* if TRUE, skip remove_expired() */
                 char *lineptr,   /* first character of the line */
                 const char *domain, /* default domain */
-                const char *path)   /* full path used when this cookie is set,
+                const char *path,   /* full path used when this cookie is set,
                                        used to get default path for the cookie
                                        unless set */
+                bool secure)  /* TRUE if connection is over secure origin */
 {
   struct Cookie *clist;
   struct Cookie *co;
@@ -546,8 +547,20 @@ Curl_cookie_add(struct Curl_easy *data,
           /* this was a "<name>=" with no content, and we must allow
              'secure' and 'httponly' specified this weirdly */
           done = TRUE;
-          if(strcasecompare("secure", name))
-            co->secure = TRUE;
+          /*
+           * secure cookies are only allowed to be set when the connection is
+           * using a secure protocol, or when the cookie is being set by
+           * reading from file
+           */
+          if(strcasecompare("secure", name)) {
+            if(secure || !c->running) {
+              co->secure = TRUE;
+            }
+            else {
+              badcookie = TRUE;
+              break;
+            }
+          }
           else if(strcasecompare("httponly", name))
             co->httponly = TRUE;
           else if(sep)
@@ -831,7 +844,13 @@ Curl_cookie_add(struct Curl_easy *data,
         fields++; /* add a field and fall down to secure */
         /* FALLTHROUGH */
       case 3:
-        co->secure = strcasecompare(ptr, "TRUE")?TRUE:FALSE;
+        co->secure = FALSE;
+        if(strcasecompare(ptr, "TRUE")) {
+          if(secure || c->running)
+            co->secure = TRUE;
+          else
+            badcookie = TRUE;
+        }
         break;
       case 4:
         if(curlx_strtoofft(ptr, NULL, 10, &co->expires))
@@ -929,9 +948,31 @@ Curl_cookie_add(struct Curl_easy *data,
         /* the domains were identical */
 
         if(clist->spath && co->spath) {
-          if(strcasecompare(clist->spath, co->spath)) {
-            replace_old = TRUE;
+          if(clist->secure && !co->secure) {
+            size_t cllen;
+            const char *sep;
+
+            /*
+             * A non-secure cookie may not overlay an existing secure cookie.
+             * For an existing cookie "a" with path "/login", refuse a new
+             * cookie "a" with for example path "/login/en", while the path
+             * "/loginhelper" is ok.
+             */
+
+            sep = strchr(clist->spath + 1, '/');
+
+            if(sep)
+              cllen = sep - clist->spath;
+            else
+              cllen = strlen(clist->spath);
+
+            if(strncasecompare(clist->spath, co->spath, cllen)) {
+              freecookie(co);
+              return NULL;
+            }
           }
+          else if(strcasecompare(clist->spath, co->spath))
+            replace_old = TRUE;
           else
             replace_old = FALSE;
         }
@@ -1103,7 +1144,7 @@ struct CookieInfo *Curl_cookie_init(struct Curl_easy *data,
       while(*lineptr && ISBLANK(*lineptr))
         lineptr++;
 
-      Curl_cookie_add(data, c, headerline, TRUE, lineptr, NULL, NULL);
+      Curl_cookie_add(data, c, headerline, TRUE, lineptr, NULL, NULL, TRUE);
     }
     free(line); /* free the line buffer */
     remove_expired(c); /* run this once, not on every cookie */
diff --git a/lib/cookie.h b/lib/cookie.h
index a9f90ca71..3ee457c62 100644
--- a/lib/cookie.h
+++ b/lib/cookie.h
@@ -7,7 +7,7 @@
  *                            | (__| |_| |  _ <| |___
  *                             \___|\___/|_| \_\_____|
  *
- * Copyright (C) 1998 - 2017, Daniel Stenberg, <daniel@haxx.se>, et al.
+ * Copyright (C) 1998 - 2018, Daniel Stenberg, <daniel@haxx.se>, et al.
  *
  * This software is licensed as described in the file COPYING, which
  * you should have received as part of this distribution. The terms
@@ -85,7 +85,8 @@ struct Curl_easy;
 struct Cookie *Curl_cookie_add(struct Curl_easy *data,
                                struct CookieInfo *, bool header, bool noexpiry,
                                char *lineptr,
-                               const char *domain, const char *path);
+                               const char *domain, const char *path,
+                               bool secure);
 
 struct Cookie *Curl_cookie_getlist(struct CookieInfo *, const char *,
                                    const char *, bool);
diff --git a/lib/http.c b/lib/http.c
index 345100f6c..0a3e46243 100644
--- a/lib/http.c
+++ b/lib/http.c
@@ -3873,7 +3873,9 @@ CURLcode Curl_http_readwrite_headers(struct Curl_easy *data,
                          here, or else use real peer host name. */
                       conn->allocptr.cookiehost?
                       conn->allocptr.cookiehost:conn->host.name,
-                      data->state.up.path);
+                      data->state.up.path,
+                      (conn->handler->protocol&CURLPROTO_HTTPS)?
+                      TRUE:FALSE);
       Curl_share_unlock(data, CURL_LOCK_DATA_COOKIE);
     }
 #endif
diff --git a/lib/setopt.c b/lib/setopt.c
index 1627aba6d..01a890b39 100644
--- a/lib/setopt.c
+++ b/lib/setopt.c
@@ -803,12 +803,12 @@ CURLcode Curl_vsetopt(struct Curl_easy *data, CURLoption option,
         if(checkprefix("Set-Cookie:", argptr))
           /* HTTP Header format line */
           Curl_cookie_add(data, data->cookies, TRUE, FALSE, argptr + 11, NULL,
-                          NULL);
+                          NULL, TRUE);
 
         else
           /* Netscape format line */
           Curl_cookie_add(data, data->cookies, FALSE, FALSE, argptr, NULL,
-                          NULL);
+                          NULL, TRUE);
 
         Curl_share_unlock(data, CURL_LOCK_DATA_COOKIE);
         free(argptr);
diff --git a/tests/data/Makefile.inc b/tests/data/Makefile.inc
index dd38f8964..250aa2004 100644
--- a/tests/data/Makefile.inc
+++ b/tests/data/Makefile.inc
@@ -176,7 +176,7 @@ test1533 test1534 test1535 test1536 test1537 test1538 \
 test1540 \
 test1550 test1551 test1552 test1553 test1554 test1555 test1556 test1557 \
 \
-test1560 \
+test1560 test1561 \
 \
 test1590 \
 test1600 test1601 test1602 test1603 test1604 test1605 test1606 test1607 \
diff --git a/tests/data/test1155 b/tests/data/test1155
index 9bf325460..3db824d58 100644
--- a/tests/data/test1155
+++ b/tests/data/test1155
@@ -14,7 +14,7 @@ cookies
 HTTP/1.1 200 OK

 Date: Thu, 09 Nov 2010 14:49:00 GMT

 Content-Length: 0

-Set-Cookie: domain=value;secure;path=/
+Set-Cookie: domain=value;path=/
 

 </data>
 </reply>
@@ -48,7 +48,7 @@ Accept: */*
 # https://curl.haxx.se/docs/http-cookies.html
 # This file was generated by libcurl! Edit at your own risk.
 
-127.0.0.1	FALSE	/	TRUE	0	domain	value
+127.0.0.1	FALSE	/	FALSE	0	domain	value
 </file>
 </verify>
 </testcase>
diff --git a/tests/data/test1561 b/tests/data/test1561
new file mode 100644
index 000000000..356dc94e4
--- /dev/null
+++ b/tests/data/test1561
@@ -0,0 +1,86 @@
+<testcase>
+<info>
+<keywords>
+HTTPS
+HTTP
+HTTP GET
+cookies
+cookiejar
+HTTP replaced headers
+</keywords>
+</info>
+
+# Server-side
+<reply>
+<data1>
+HTTP/1.1 200 OK
+Date: Thu, 09 Nov 2010 14:49:00 GMT
+Server: test-server/fake
+Set-Cookie: super=secret; domain=example.com; path=/1561; secure;
+Set-Cookie: supersuper=secret; domain=example.com; path=/1561/login/; secure;
+Content-Length: 7
+
+nomnom
+</data1>
+<data2>
+HTTP/1.1 200 OK
+Date: Thu, 09 Nov 2010 14:49:00 GMT
+Server: test-server/fake
+Set-Cookie: super=secret; domain=example.com; path=/1561; httponly;
+Set-Cookie: super=secret; domain=example.com; path=/1561/; httponly;
+Set-Cookie: super=secret; domain=example.com; path=/15; httponly;
+Set-Cookie: public=yes; domain=example.com; path=/foo;
+Set-Cookie: supersuper=secret; domain=example.com; path=/1561/login/en;
+Set-Cookie: supersuper=secret; domain=example.com; path=/1561/login;
+Set-Cookie: secureoverhttp=yes; domain=example.com; path=/1561; secure;
+Content-Length: 7
+
+nomnom
+</data2>
+</reply>
+
+# Client-side
+<client>
+<features>
+SSL
+</features>
+<server>
+http
+https
+</server>
+<name>
+HTTP
+</name>
+<command>
+-k https://%HOSTIP:%HTTPSPORT/15610001 -L -c log/jar1561.txt -H "Host: www.example.com"  http://%HOSTIP:%HTTPPORT/15610002 -L -c log/jar1561.txt -H "Host: www.example.com"
+</command>
+</client>
+<verify>
+<strip>
+^User-Agent:.*
+</strip>
+<protocol>
+GET /15610001 HTTP/1.1

+Host: www.example.com

+User-Agent: curl/7.62.0-DEV

+Accept: */*

+

+GET /15610002 HTTP/1.1

+Host: www.example.com

+User-Agent: curl/7.62.0-DEV

+Accept: */*

+

+</protocol>
+<file name="log/jar1561.txt" mode="text">
+# Netscape HTTP Cookie File
+# https://curl.haxx.se/docs/http-cookies.html
+# This file was generated by libcurl! Edit at your own risk.
+
+.example.com	TRUE	/foo	FALSE	0	public	yes
+.example.com	TRUE	/1561/login/	TRUE	0	supersuper	secret
+#HttpOnly_.example.com	TRUE	/15	FALSE	0	super	secret
+</file>
+
+</verify>
+
+</testcase>
diff --git a/tests/data/test31 b/tests/data/test31
index 78f3766e9..58398c55d 100644
--- a/tests/data/test31
+++ b/tests/data/test31
@@ -100,7 +100,6 @@ Accept: */*
 # https://curl.haxx.se/docs/http-cookies.html
 # This file was generated by libcurl! Edit at your own risk.
 
-127.0.0.1	FALSE	/we/want/	TRUE	0	securewithspace	after
 127.0.0.1	FALSE	/we/want/	FALSE	0	prespace	yes before
 127.0.0.1	FALSE	/we/want/	FALSE	0	withspaces2	before equals
 127.0.0.1	FALSE	/we/want/	FALSE	0	withspaces	yes  within and around
@@ -108,28 +107,11 @@ Accept: */*
 #HttpOnly_127.0.0.1	FALSE	/silly/	FALSE	0	magic	yessir
 127.0.0.1	FALSE	/we/want/	FALSE	2054030187	nodomain	value
 127.0.0.1	FALSE	/	FALSE	0	partmatch	present
-#HttpOnly_127.0.0.1	FALSE	/p4/	TRUE	0	httpandsec8	myvalue9
-#HttpOnly_127.0.0.1	FALSE	/p4/	TRUE	0	httpandsec7	myvalue8
-#HttpOnly_127.0.0.1	FALSE	/p4/	TRUE	0	httpandsec6	myvalue7
-#HttpOnly_127.0.0.1	FALSE	/p4/	TRUE	0	httpandsec5	myvalue6
-#HttpOnly_127.0.0.1	FALSE	/p4/	TRUE	0	httpandsec4	myvalue5
-#HttpOnly_127.0.0.1	FALSE	/p4/	TRUE	0	httpandsec3	myvalue4
-#HttpOnly_127.0.0.1	FALSE	/p4/	TRUE	0	httpandsec2	myvalue3
-#HttpOnly_127.0.0.1	FALSE	/p4/	TRUE	0	httpandsec	myvalue2
 #HttpOnly_127.0.0.1	FALSE	/p4/	FALSE	0	httponly	myvalue1
 #HttpOnly_127.0.0.1	FALSE	/p4/	FALSE	0	httpo4	value4
 #HttpOnly_127.0.0.1	FALSE	/p3/	FALSE	0	httpo3	value3
 #HttpOnly_127.0.0.1	FALSE	/p2/	FALSE	0	httpo2	value2
 #HttpOnly_127.0.0.1	FALSE	/p1/	FALSE	0	httpo1	value1
-127.0.0.1	FALSE	/secure9/	TRUE	0	secure	very1
-127.0.0.1	FALSE	/secure8/	TRUE	0	sec8value	secure8
-127.0.0.1	FALSE	/secure7/	TRUE	0	sec7value	secure7
-127.0.0.1	FALSE	/secure6/	TRUE	0	sec6value	secure6
-127.0.0.1	FALSE	/secure5/	TRUE	0	sec5value	secure5
-127.0.0.1	FALSE	/secure4/	TRUE	0	sec4value	secure4
-127.0.0.1	FALSE	/secure3/	TRUE	0	sec3value	secure3
-127.0.0.1	FALSE	/secure2/	TRUE	0	sec2value	secure2
-127.0.0.1	FALSE	/secure1/	TRUE	0	sec1value	secure1
 127.0.0.1	FALSE	/overwrite	FALSE	0	overwrite	this2
 127.0.0.1	FALSE	/silly/	FALSE	0	ismatch	this
 </file>
diff --git a/tests/data/test61 b/tests/data/test61
index 784163fa9..2709f5112 100644
--- a/tests/data/test61
+++ b/tests/data/test61
@@ -65,7 +65,6 @@ Accept: */*
 # https://curl.haxx.se/docs/http-cookies.html
 # This file was generated by libcurl! Edit at your own risk.
 
-.foo.com	TRUE	/moo	TRUE	0	test3	maybe
 .host.foo.com	TRUE	/we/want/	FALSE	2054030187	test2	yes
 #HttpOnly_.foo.com	TRUE	/we/want/	FALSE	2054030187	test	yes
 </file>