From bb3c4c5de24a6732e6f13f96935eea8cb1392c04 Mon Sep 17 00:00:00 2001 From: "pauljensen@chromium.org" Date: Fri, 13 Jul 2012 19:27:48 +0000 Subject: Account for server vs host clock skew in cookie expiration times. When setting a cookie's expiration time in the cookie store we need to take into account any difference between the HTTP server and the host machine's real time clock. BUG=135131 TEST=net_unittests --gtest_filter=CookieMonster/CookieStoreTest/0.TestCookieDeletion Review URL: https://chromiumcodereview.appspot.com/10692137 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@146616 0039d316-1c4b-4281-b951-d872f2087c98 --- net/cookies/cookie_monster.cc | 20 +++++++++++++++----- net/cookies/cookie_options.h | 13 ++++++++++++- net/cookies/cookie_store_unittest.h | 27 +++++++++++++++++++++++++++ 3 files changed, 54 insertions(+), 6 deletions(-) (limited to 'net/cookies') diff --git a/net/cookies/cookie_monster.cc b/net/cookies/cookie_monster.cc index e44ba18..9851c9a 100644 --- a/net/cookies/cookie_monster.cc +++ b/net/cookies/cookie_monster.cc @@ -236,7 +236,9 @@ std::string CanonPath(const GURL& url, const ParsedCookie& pc) { return CanonPathWithString(url, path_string); } -Time CanonExpiration(const ParsedCookie& pc, const Time& current) { +Time CanonExpiration(const ParsedCookie& pc, + const Time& current, + const Time& server_time) { // First, try the Max-Age attribute. uint64 max_age = 0; if (pc.HasMaxAge() && @@ -250,8 +252,11 @@ Time CanonExpiration(const ParsedCookie& pc, const Time& current) { } // Try the Expires attribute. - if (pc.HasExpires()) - return CookieMonster::ParseCookieTime(pc.Expires()); + if (pc.HasExpires()) { + // Adjust for clock skew between server and host. + return current + (CookieMonster::ParseCookieTime(pc.Expires()) - + server_time); + } // Invalid or no expiration, persistent cookie. return Time(); @@ -1888,6 +1893,11 @@ bool CookieMonster::SetCookieWithCreationTimeAndOptions( creation_time = CurrentTime(); last_time_seen_ = creation_time; } + Time server_time; + if (options.has_server_time()) + server_time = options.server_time(); + else + server_time = creation_time; // Parse the cookie. ParsedCookie pc(cookie_line); @@ -1913,7 +1923,7 @@ bool CookieMonster::SetCookieWithCreationTimeAndOptions( pc.MACAlgorithm() : std::string(); scoped_ptr cc; - Time cookie_expires = CanonExpiration(pc, creation_time); + Time cookie_expires = CanonExpiration(pc, creation_time, server_time); cc.reset(new CanonicalCookie(url, pc.Name(), pc.Value(), cookie_domain, cookie_path, mac_key, mac_algorithm, @@ -2375,7 +2385,7 @@ CookieMonster::CanonicalCookie::CanonicalCookie(const GURL& url, secure_(pc.IsSecure()), httponly_(pc.IsHttpOnly()) { if (pc.HasExpires()) - expiry_date_ = CanonExpiration(pc, creation_date_); + expiry_date_ = CanonExpiration(pc, creation_date_, creation_date_); else SetSessionCookieExpiryTime(); diff --git a/net/cookies/cookie_options.h b/net/cookies/cookie_options.h index 8370702..ed5e2ef 100644 --- a/net/cookies/cookie_options.h +++ b/net/cookies/cookie_options.h @@ -15,15 +15,26 @@ class CookieOptions { // - reading operations will not return httponly cookies. // - writing operations will not write httponly cookies. CookieOptions() - : exclude_httponly_(true) { + : exclude_httponly_(true), + server_time_() { } void set_exclude_httponly() { exclude_httponly_ = true; } void set_include_httponly() { exclude_httponly_ = false; } bool exclude_httponly() const { return exclude_httponly_; } + // |server_time| indicates what the server sending us the Cookie thought the + // current time was when the cookie was produced. This is used to adjust for + // clock skew between server and host. + void set_server_time(const base::Time& server_time) { + server_time_ = server_time; + } + bool has_server_time() const { return !server_time_.is_null(); } + base::Time server_time() const { return server_time_; } + private: bool exclude_httponly_; + base::Time server_time_; }; } // namespace net diff --git a/net/cookies/cookie_store_unittest.h b/net/cookies/cookie_store_unittest.h index be34c4a..592d52a 100644 --- a/net/cookies/cookie_store_unittest.h +++ b/net/cookies/cookie_store_unittest.h @@ -147,6 +147,17 @@ class CookieStoreTest : public testing::Test { return callback.result(); } + bool SetCookieWithServerTime(CookieStore* cs, + const GURL& url, + const std::string& cookie_line, + const base::Time& server_time) { + CookieOptions options; + if (!CookieStoreTestTraits::supports_http_only) + options.set_include_httponly(); + options.set_server_time(server_time); + return SetCookieWithOptions(cs, url, cookie_line, options); + } + bool SetCookie(CookieStore* cs, const GURL& url, const std::string& cookie_line) { @@ -702,6 +713,22 @@ TYPED_TEST_P(CookieStoreTest, TestCookieDeletion) { std::string(kValidCookieLine) + "; expires=Mon, 18-Apr-22 22:50:13 GMT")); this->MatchCookieLines("A=B", this->GetCookies(cs, this->url_google_)); + // Check that it is not deleted with significant enough clock skew. + base::Time server_time; + EXPECT_TRUE(base::Time::FromString("Sun, 17-Apr-1977 22:50:13 GMT", + &server_time)); + EXPECT_TRUE(this->SetCookieWithServerTime( + cs, this->url_google_, + std::string(kValidCookieLine) + + "; expires=Mon, 18-Apr-1977 22:50:13 GMT", + server_time)); + this->MatchCookieLines("A=B", this->GetCookies(cs, this->url_google_)); + + // Create a persistent cookie. + EXPECT_TRUE(this->SetCookie(cs, this->url_google_, + std::string(kValidCookieLine) + + "; expires=Mon, 18-Apr-22 22:50:13 GMT")); + this->MatchCookieLines("A=B", this->GetCookies(cs, this->url_google_)); // Delete it via Expires, with a unix epoch of 0. EXPECT_TRUE(this->SetCookie(cs, this->url_google_, std::string(kValidCookieLine) + -- cgit v1.1