I figured out the mistake, I guess. The cookie was getting expired and wasn't getting saved to the file. I added --keep-session-cookies to WGET_OPTIONS, now I can see the cookies being saved in the file.
In the test, I am removing the first 4 lines of the cookie file (which WGET generates) and then saving it again. Since no cookie should exist (in this case), I am comparing it to an empty file using the ExpectedFiles hook. Without the patch (0003), the test shouldn't pass, as the secure cookie is set. Although there is an error raised "Error: Contents of mycookies.wget do not match." in the test log, the test suit doesn't fail explicitly. Is this supposed to happen? After applying the patch (0003), the error is no longer raised in the log. Patches and test log are attached. Thank you, Kushagra
Setting --no-config (noconfig) to 1 Setting --keep-session-cookies (keepsessioncookies) to 1 Setting --save-cookies (savecookies) to mycookies.wget DEBUG output created by Wget 1.17.1.13-c190-dirty on linux-gnu. URI encoding = 'ANSI_X3.4-1968' converted 'http://127.0.0.1:34880/File1' (ANSI_X3.4-1968) -> 'http://127.0.0.1:34880/File1' (UTF-8) Converted file name 'File1' (UTF-8) -> 'File1' (ANSI_X3.4-1968) --2016-02-11 02:44:34-- http://127.0.0.1:34880/File1 Connecting to 127.0.0.1:34880... connected. Created socket 3. Releasing 0x0000000001064810 (new refcount 0). Deleting unused 0x0000000001064810. ---request begin--- GET /File1 HTTP/1.1 User-Agent: Wget/1.17.1.13-c190-dirty (linux-gnu) Accept: */* Accept-Encoding: identity Host: 127.0.0.1:34880 Connection: Keep-Alive ---request end--- HTTP request sent, awaiting response... ---response begin--- HTTP/1.1 200 OK Server: BaseHTTP/0.6 Python/3.4.0 Date: Wed, 10 Feb 2016 21:14:34 GMT content-type: text/plain content-length: 75 set-cookie: sess-id=0213; path=/; secure ---response end--- 200 OK Stored cookie 127.0.0.1 34880 / <session> <secure> [expiry none] sess-id 0213 Registered socket 3 for persistent reuse. Length: 75 [text/plain] Saving to: 'File1' 0K 100% 13.6M=0s 2016-02-11 02:44:34 (13.6 MB/s) - 'File1' saved [75/75] Saving cookies to mycookies.wget. Done saving cookies. 127.0.0.1 - - [11/Feb/2016 02:44:34] "GET /File1 HTTP/1.1" 200 - --- Actual +++ Expected @@ -1,44 +0,0 @@ -1 -2 -7 -. -0 -. -0 -. -1 -: -3 -4 -8 -8 -0 - -F -A -L -S -E - -/ - -T -R -U -E - -0 - -s -e -s -s -- -i -d - -0 -2 -1 -3 - Running Test Reject Secure Cookie /home/kush/Desktop/wget/src/wget --debug --no-config --keep-session-cookies --save-cookies=mycookies.wget http://127.0.0.1:34880/File1 ['/home/kush/Desktop/wget/src/wget', '--debug', '--no-config', '--keep-session-cookies', '--save-cookies=mycookies.wget', 'http://127.0.0.1:34880/File1'] Error: Contents of mycookies.wget do not match.
From 47fe57a1724293f93342b4350fba02a96c44bb1b Mon Sep 17 00:00:00 2001 From: kush789 <[email protected]> Date: Thu, 11 Feb 2016 02:51:49 +0530 Subject: [PATCH 2/2] Added test, rejects setting a secure cookie over a non secure connection. --- testenv/Makefile.am | 1 + testenv/Test-reject-secure-cookie.py | 72 ++++++++++++++++++++++++++++++++++++ 2 files changed, 73 insertions(+) create mode 100644 testenv/Test-reject-secure-cookie.py diff --git a/testenv/Makefile.am b/testenv/Makefile.am index 370c404..084d435 100644 --- a/testenv/Makefile.am +++ b/testenv/Makefile.am @@ -68,6 +68,7 @@ if HAVE_PYTHON3 Test--spider-r.py \ Test--rejected-log.py \ Test-redirect-crash.py \ + Test-reject-secure-cookie.py \ Test-reserved-chars.py \ Test-condget.py \ $(METALINK_TESTS) diff --git a/testenv/Test-reject-secure-cookie.py b/testenv/Test-reject-secure-cookie.py new file mode 100644 index 0000000..9b10df2 --- /dev/null +++ b/testenv/Test-reject-secure-cookie.py @@ -0,0 +1,72 @@ +#!/usr/bin/env python3 +from sys import exit +from test.http_test import HTTPTest +from test.base_test import HTTP +from misc.wget_file import WgetFile + +""" + This test ensures that a secure-only cookie is rejected over an insecure + connection +""" +TEST_NAME = "Reject Secure Cookie" +############# File Definitions ############################################### +File1 = """All happy families are alike; +Each unhappy family is unhappy in its own way""" +File2 = "" +# Expected cookie file should be empty + +File1_rules = { + "SendHeader" : { + "Set-Cookie" : "sess-id=0213; path=/; secure" + } +} + +A_File = WgetFile ("File1", File1, rules=File1_rules) +B_File = WgetFile ("mycookies.wget", File2) + +WGET_OPTIONS = "--keep-session-cookies --save-cookies=mycookies.wget" +WGET_URLS = [["File1"]] + +Servers = [HTTP] + +Files = [[A_File]] + +ExpectedReturnCode = 0 + +def postfunc(): + cookie_file_content = [] + + with open("mycookies.wget", 'r') as fp: + cookie_file_content = fp.readlines() + + cookie_file_content = cookie_file_content[4:] + # Removing first four lines (comments generated by wget) + + with open("mycookies.wget", 'w') as fp: + for line in cookie_file_content: + fp.write(line) + + return [A_File, B_File] + +################ Pre and Post Test Hooks ##################################### +pre_test = { + "ServerFiles" : Files, +} +test_options = { + "WgetCommands" : WGET_OPTIONS, + "Urls" : WGET_URLS +} +post_test = { + "ExpectedFiles" : postfunc, + "ExpectedRetcode" : ExpectedReturnCode +} + +err = HTTPTest ( + name=TEST_NAME, + pre_hook=pre_test, + test_params=test_options, + post_hook=post_test, + protocols=Servers +).begin () + +exit (err) -- 1.9.1
From b6a9290559d3a5caff4d1f8b4b9daf6ebfab0429 Mon Sep 17 00:00:00 2001 From: kush789 <[email protected]> Date: Thu, 11 Feb 2016 02:49:07 +0530 Subject: [PATCH 1/2] Updated expected_files hook to allow passing a function to generate expected fs. --- testenv/conf/expected_files.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/testenv/conf/expected_files.py b/testenv/conf/expected_files.py index 5362771..c9a333f 100644 --- a/testenv/conf/expected_files.py +++ b/testenv/conf/expected_files.py @@ -17,7 +17,10 @@ files are found, else returns gracefully. @hook() class ExpectedFiles: def __init__(self, expected_fs): - self.expected_fs = expected_fs + if callable(expected_fs): + self.expected_fs = expected_fs() + else: + self.expected_fs = expected_fs @staticmethod def gen_local_fs_snapshot(): -- 1.9.1
From f765337085bac0986c9f20dca96d484d55759f45 Mon Sep 17 00:00:00 2001 From: kush789 <[email protected]> Date: Fri, 29 Jan 2016 13:42:06 +0530 Subject: [PATCH] Added recommendations from draft-West --- src/cookies.c | 69 +++++++++++++++++++++++++++++++++++++++++++++++++++-------- src/cookies.h | 5 ++++- src/http.c | 2 +- 3 files changed, 65 insertions(+), 11 deletions(-) diff --git a/src/cookies.c b/src/cookies.c index 81ecfa5..b2bfb54 100644 --- a/src/cookies.c +++ b/src/cookies.c @@ -122,6 +122,13 @@ struct cookie { same domain. */ }; +static int +find_chains_of_host (struct cookie_jar *jar, const char *host, + struct cookie *dest[]); + +static int +count_char (const char *string, char chr); + #define PORT_ANY (-1) /* Allocate and return a new, empty cookie structure. */ @@ -350,7 +357,8 @@ discard_matching_cookie (struct cookie_jar *jar, struct cookie *cookie) filled. */ static struct cookie * -parse_set_cookie (const char *set_cookie, bool silent) +parse_set_cookie (const char *set_cookie, enum url_scheme scheme, + bool silent) { const char *ptr = set_cookie; struct cookie *cookie = cookie_new (); @@ -439,8 +447,21 @@ parse_set_cookie (const char *set_cookie, bool silent) } else if (TOKEN_IS (name, "secure")) { - /* ignore value completely */ - cookie->secure = 1; + if (scheme == SCHEME_HTTP) + { + /* Deleting cookie since secure only flag is set but connection + is not secure. */ + if (!silent) + logprintf (LOG_NOTQUIET, + _("Trying to create secure only cookie, but connection is not secure.\nSet-Cookie : %s\n"), + quotearg_style (escape_quoting_style, set_cookie)); + delete_cookie (cookie); + return NULL; + } + else + /* Ignore value completely since secure is a value-less + attribute*/ + cookie->secure = 1; } /* else: Ignore unrecognized attribute. */ } @@ -461,6 +482,7 @@ parse_set_cookie (const char *set_cookie, bool silent) return NULL; } + #undef TOKEN_IS #undef TOKEN_NON_EMPTY @@ -707,9 +729,12 @@ check_path_match (const char *cookie_path, const char *path) void cookie_handle_set_cookie (struct cookie_jar *jar, const char *host, int port, - const char *path, const char *set_cookie) + const char *path, enum url_scheme scheme, + const char *set_cookie) { - struct cookie *cookie; + struct cookie *cookie, *old_cookie; + struct cookie **chains; + int chain_count, i; cookies_now = time (NULL); /* Wget's paths don't begin with '/' (blame rfc1808), but cookie @@ -717,7 +742,7 @@ cookie_handle_set_cookie (struct cookie_jar *jar, simply prepend slash to PATH. */ PREPEND_SLASH (path); - cookie = parse_set_cookie (set_cookie, false); + cookie = parse_set_cookie (set_cookie, scheme, false); if (!cookie) goto out; @@ -767,6 +792,29 @@ cookie_handle_set_cookie (struct cookie_jar *jar, } } + if ((cookie->secure == 0) && (scheme == SCHEME_HTTP)) + { + /* If an old cookie exists such that the all of the following are + true, then discard the new cookie. + + - The "domain" domain-matches the domain of the new cookie + - The "name" matches the "name" of the new cookie + - Secure-only flag of old cookie is set */ + + chains = alloca_array (struct cookie *, 1 + count_char (host, '.')); + chain_count = find_chains_of_host (jar, host, chains); + + for (i = 0; i < chain_count; i++) + for (old_cookie = chains[i]; old_cookie; old_cookie = old_cookie->next) + { + if (!cookie_expired_p(old_cookie) + && !(old_cookie->domain_exact + && 0 != strcasecmp (host, old_cookie->domain)) + && 0 == strcasecmp(old_cookie->attr, cookie->attr) + && 1 == old_cookie->secure) + goto out; + } + } /* Now store the cookie, or discard an existing cookie, if discarding was requested. */ @@ -795,9 +843,12 @@ count_char (const char *string, char chr) { const char *p; int count = 0; - for (p = string; *p; p++) - if (*p == chr) - ++count; + p = string; + while ((p = strchr(p, chr)) != NULL) + { + ++count; + p++; + } return count; } diff --git a/src/cookies.h b/src/cookies.h index b4281e1..d565b09 100644 --- a/src/cookies.h +++ b/src/cookies.h @@ -31,13 +31,16 @@ as that of the covered work. */ #ifndef COOKIES_H #define COOKIES_H +#include "url.h" + struct cookie_jar; struct cookie_jar *cookie_jar_new (void); void cookie_jar_delete (struct cookie_jar *); void cookie_handle_set_cookie (struct cookie_jar *, const char *, int, - const char *, const char *); + const char *, enum url_scheme, + const char *); char *cookie_header (struct cookie_jar *, const char *, int, const char *, bool); diff --git a/src/http.c b/src/http.c index 8916d2b..f7d42b2 100644 --- a/src/http.c +++ b/src/http.c @@ -3276,7 +3276,7 @@ gethttp (struct url *u, struct http_stat *hs, int *dt, struct url *proxy, { char *set_cookie; BOUNDED_TO_ALLOCA (scbeg, scend, set_cookie); cookie_handle_set_cookie (wget_cookie_jar, u->host, u->port, - u->path, set_cookie); + u->path, u->scheme, set_cookie); } } -- 1.9.1
