From 7a09b52c98ac8d840a8a9907b1a1d9a9e684bcf5 Mon Sep 17 00:00:00 2001 From: Daniel Gustafsson Date: Thu, 13 Dec 2018 09:57:58 +0100 Subject: 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 --- docs/HTTP-COOKIES.md | 4 ++- docs/TODO | 8 ----- lib/cookie.c | 55 +++++++++++++++++++++++++++---- lib/cookie.h | 5 +-- lib/http.c | 4 ++- lib/setopt.c | 4 +-- tests/data/Makefile.inc | 2 +- tests/data/test1155 | 4 +-- tests/data/test1561 | 86 +++++++++++++++++++++++++++++++++++++++++++++++++ tests/data/test31 | 18 ----------- tests/data/test61 | 1 - 11 files changed, 148 insertions(+), 43 deletions(-) create mode 100644 tests/data/test1561 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 "=" 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, , et al. + * Copyright (C) 1998 - 2018, Daniel Stenberg, , 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=/ @@ -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 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 @@ + + + +HTTPS +HTTP +HTTP GET +cookies +cookiejar +HTTP replaced headers + + + +# Server-side + + +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 + + +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 + + + +# Client-side + + +SSL + + +http +https + + +HTTP + + +-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" + + + + +^User-Agent:.* + + +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: */* + + + +# 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 + + + + + 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 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 -- cgit v1.2.3