Commit 7a09b52c authored by Daniel Gustafsson's avatar Daniel Gustafsson
Browse files

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: default avatarDaniel Stenberg <daniel@haxx.se>
parent fdc5563b

docs/HTTP-COOKIES.md

+3 −1
Original line number Original line Diff line number Diff line
@@ -18,7 +18,9 @@ 
  original [Netscape spec from 1994](https://curl.haxx.se/rfc/cookie_spec.html).

  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

  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

## Cookies saved to disk

docs/TODO

+0 −8
Original line number Original line Diff line number Diff line
@@ -73,7 +73,6 @@ 
 5.5 auth= in URLs

 5.5 auth= in URLs

 5.6 Refuse "downgrade" redirects

 5.6 Refuse "downgrade" redirects

 5.7 QUIC

 5.7 QUIC

 5.8 Leave secure cookies alone





 6. TELNET

 6. TELNET

 6.1 ditch stdin

 6.1 ditch stdin
@@ -605,13 +604,6 @@ 
 implemented. This, to allow other projects to benefit from the work and to

 implemented. This, to allow other projects to benefit from the work and to

 thus broaden the interest and chance of others to participate.

 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

6. TELNET

lib/cookie.c

+48 −7
Original line number Original line Diff line number Diff line
@@ -433,9 +433,10 @@ Curl_cookie_add(struct Curl_easy *data, 
                bool noexpire, /* if TRUE, skip remove_expired() */

                bool noexpire, /* if TRUE, skip remove_expired() */

                char *lineptr,   /* first character of the line */

                char *lineptr,   /* first character of the line */

                const char *domain, /* default domain */

                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

                                       used to get default path for the cookie

                                       unless set */

                                       unless set */

                bool secure)  /* TRUE if connection is over secure origin */

{

{

  struct Cookie *clist;

  struct Cookie *clist;

  struct Cookie *co;

  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

          /* this was a "<name>=" with no content, and we must allow

             'secure' and 'httponly' specified this weirdly */

             'secure' and 'httponly' specified this weirdly */

          done = TRUE;

          done = TRUE;

          if(strcasecompare("secure", name))

          /*

           * 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;

              co->secure = TRUE;

            }

            else {

              badcookie = TRUE;

              break;

            }

          }

          else if(strcasecompare("httponly", name))

          else if(strcasecompare("httponly", name))

            co->httponly = TRUE;

            co->httponly = TRUE;

          else if(sep)

          else if(sep)
@@ -831,7 +844,13 @@ Curl_cookie_add(struct Curl_easy *data, 
        fields++; /* add a field and fall down to secure */

        fields++; /* add a field and fall down to secure */

        /* FALLTHROUGH */

        /* FALLTHROUGH */

      case 3:

      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;

        break;

      case 4:

      case 4:

        if(curlx_strtoofft(ptr, NULL, 10, &co->expires))

        if(curlx_strtoofft(ptr, NULL, 10, &co->expires))
@@ -929,9 +948,31 @@ Curl_cookie_add(struct Curl_easy *data, 
        /* the domains were identical */

        /* the domains were identical */





        if(clist->spath && co->spath) {

        if(clist->spath && co->spath) {

          if(strcasecompare(clist->spath, co->spath)) {

          if(clist->secure && !co->secure) {

            replace_old = TRUE;

            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

          else

            replace_old = FALSE;

            replace_old = FALSE;

        }

        }
@@ -1103,7 +1144,7 @@ struct CookieInfo *Curl_cookie_init(struct Curl_easy *data, 
      while(*lineptr && ISBLANK(*lineptr))

      while(*lineptr && ISBLANK(*lineptr))

        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 */

    free(line); /* free the line buffer */

    remove_expired(c); /* run this once, not on every cookie */

    remove_expired(c); /* run this once, not on every cookie */

lib/cookie.h

+3 −2
Original line number Original line Diff line number Diff line
@@ -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

 * This software is licensed as described in the file COPYING, which

 * you should have received as part of this distribution. The terms

 * 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 Cookie *Curl_cookie_add(struct Curl_easy *data,

                               struct CookieInfo *, bool header, bool noexpiry,

                               struct CookieInfo *, bool header, bool noexpiry,

                               char *lineptr,

                               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 *,

struct Cookie *Curl_cookie_getlist(struct CookieInfo *, const char *,

                                   const char *, bool);

                                   const char *, bool);

lib/http.c

+3 −1
Original line number Original line Diff line number Diff line
@@ -3873,7 +3873,9 @@ CURLcode Curl_http_readwrite_headers(struct Curl_easy *data, 
                         here, or else use real peer host name. */

                         here, or else use real peer host name. */

                      conn->allocptr.cookiehost?

                      conn->allocptr.cookiehost?

                      conn->allocptr.cookiehost:conn->host.name,

                      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);

      Curl_share_unlock(data, CURL_LOCK_DATA_COOKIE);

    }

    }

#endif

#endif