New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 749059 link

Starred by 7 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug



Sign in to add a comment

Gather metrics on non-secure cookie usage.

Project Member Reported by mkwst@chromium.org, Jul 26 2017

Issue description

https://tools.ietf.org/html/draft-thomson-http-omnomnom-00 is almost certainly too strict to implement as-is, today, but it would be helpful to understand the lay of the land to make informed decisions in the future.
 

Comment 1 by mkwst@chromium.org, Jul 27 2017

Cc: est...@chromium.org
Relatedly, we should also figure out how often cookies are updated without changing their value (e.g. by echoing the `Cookie` request header into a `Set-Cookie` header in the response to update the expiration time).
Project Member

Comment 2 by bugdroid1@chromium.org, Jul 28 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8614988bf1b12c4031f54d421feb7d34a5d71a88

commit 8614988bf1b12c4031f54d421feb7d34a5d71a88
Author: Mike West <mkwst@chromium.org>
Date: Fri Jul 28 10:41:49 2017

Add cookie age and eviction metrics

This patch adds `Cookie.AgeForNonSecureCrossSiteRequest` and
`Cookie.AgeForNonSecureSameSiteRequest`, which record the age
(in days) of the oldest cookie delivered along with a
non-secure cross- or same-site request, respectively. This age
data will give us some insight into the ways in which cookies
are being used in non-secure first- and third-party contexts,
which might help guide decisions about how to shift those
contexts towards TLS.

It also adds an enum value to `Cookie.CookieDeleteEquivalent`,
recording the times where a cookie replaces an existing cookie
with a cookie whose value is identical.

Bug: 749059
Change-Id: Id6276a78e8767f9f2d8fdf647cff1b9ba6200a94
Reviewed-on: https://chromium-review.googlesource.com/586690
Commit-Queue: Mike West <mkwst@chromium.org>
Reviewed-by: Ryan Sleevi <rsleevi@chromium.org>
Reviewed-by: Robert Kaplow <rkaplow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#490354}
[modify] https://crrev.com/8614988bf1b12c4031f54d421feb7d34a5d71a88/net/cookies/cookie_monster.cc
[modify] https://crrev.com/8614988bf1b12c4031f54d421feb7d34a5d71a88/net/cookies/cookie_monster.h
[modify] https://crrev.com/8614988bf1b12c4031f54d421feb7d34a5d71a88/net/cookies/cookie_monster_unittest.cc
[modify] https://crrev.com/8614988bf1b12c4031f54d421feb7d34a5d71a88/net/url_request/url_request_http_job.cc
[modify] https://crrev.com/8614988bf1b12c4031f54d421feb7d34a5d71a88/net/url_request/url_request_unittest.cc
[modify] https://crrev.com/8614988bf1b12c4031f54d421feb7d34a5d71a88/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/8614988bf1b12c4031f54d421feb7d34a5d71a88/tools/metrics/histograms/histograms.xml

Project Member

Comment 3 by bugdroid1@chromium.org, Oct 6 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c4a777b4067adc1cd27adcf66ebde30a6330cc89

commit c4a777b4067adc1cd27adcf66ebde30a6330cc89
Author: Mike West <mkwst@chromium.org>
Date: Fri Oct 06 14:04:20 2017

Inherit creation date for certain overwritten cookies.

When the name, value, domain, and path of a new cookie exactly match
those of an existing cookie, we'll update the new cookie's creation
date to match the existing cookie.

Bug: 749059
Change-Id: I12c7d00341e91fa32bd34bafd3cfee18235d1a2c
Reviewed-on: https://chromium-review.googlesource.com/670620
Reviewed-by: Emily Stark <estark@chromium.org>
Commit-Queue: Mike West <mkwst@chromium.org>
Cr-Commit-Position: refs/heads/master@{#507053}
[modify] https://crrev.com/c4a777b4067adc1cd27adcf66ebde30a6330cc89/net/cookies/canonical_cookie.cc
[modify] https://crrev.com/c4a777b4067adc1cd27adcf66ebde30a6330cc89/net/cookies/canonical_cookie.h
[modify] https://crrev.com/c4a777b4067adc1cd27adcf66ebde30a6330cc89/net/cookies/cookie_monster.cc
[modify] https://crrev.com/c4a777b4067adc1cd27adcf66ebde30a6330cc89/net/cookies/cookie_monster.h
[modify] https://crrev.com/c4a777b4067adc1cd27adcf66ebde30a6330cc89/net/cookies/cookie_monster_unittest.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Mar 21 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6c783dbfdf0af36fe4ec6bbf74f0e3811bf614b8

commit 6c783dbfdf0af36fe4ec6bbf74f0e3811bf614b8
Author: Thiemo Nagel <tnagel@chromium.org>
Date: Wed Mar 21 13:53:30 2018

More cookie metrics

Add per-request metrics for cookie age based on individual cookies (not
just for the oldest cookie in the request) and split by
{secure,non-secure} x {same-site,cross-site}.

Bug: 749059
Change-Id: I0ef5fba95df226da05ea56537984264a0bce5941
Reviewed-on: https://chromium-review.googlesource.com/962601
Commit-Queue: Thiemo Nagel <tnagel@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Reviewed-by: Mike West <mkwst@chromium.org>
Reviewed-by: Ryan Sleevi <rsleevi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#544686}
[modify] https://crrev.com/6c783dbfdf0af36fe4ec6bbf74f0e3811bf614b8/net/url_request/url_request_http_job.cc
[modify] https://crrev.com/6c783dbfdf0af36fe4ec6bbf74f0e3811bf614b8/net/url_request/url_request_unittest.cc
[modify] https://crrev.com/6c783dbfdf0af36fe4ec6bbf74f0e3811bf614b8/tools/metrics/histograms/histograms.xml

Project Member

Comment 5 by bugdroid1@chromium.org, Apr 13 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6aee4ab78c122fd80738594eadb878de2268fe04

commit 6aee4ab78c122fd80738594eadb878de2268fe04
Author: Thiemo Nagel <tnagel@chromium.org>
Date: Fri Apr 13 11:02:10 2018

Clean up TransportSecurityState::GetStaticDomainState()

Only modify the out parameters when there's actual state to be returned.

Also, when PKP lookup fails due to pinset out of bounds (that should not
happen, I guess) the method only returns false if there is no STS state
either.

Bug: 749059
Change-Id: Ida1fe3e60d2eb5c9bbc1625cd4b8b6550f9b716d
Reviewed-on: https://chromium-review.googlesource.com/973610
Reviewed-by: Nick Harper <nharper@chromium.org>
Commit-Queue: Thiemo Nagel <tnagel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550583}
[modify] https://crrev.com/6aee4ab78c122fd80738594eadb878de2268fe04/net/http/transport_security_state.cc
[modify] https://crrev.com/6aee4ab78c122fd80738594eadb878de2268fe04/net/http/transport_security_state.h
[modify] https://crrev.com/6aee4ab78c122fd80738594eadb878de2268fe04/net/http/transport_security_state_unittest.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Apr 17 2018

Labels: merge-merged-testbranch
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6aee4ab78c122fd80738594eadb878de2268fe04

commit 6aee4ab78c122fd80738594eadb878de2268fe04
Author: Thiemo Nagel <tnagel@chromium.org>
Date: Fri Apr 13 11:02:10 2018

Clean up TransportSecurityState::GetStaticDomainState()

Only modify the out parameters when there's actual state to be returned.

Also, when PKP lookup fails due to pinset out of bounds (that should not
happen, I guess) the method only returns false if there is no STS state
either.

Bug: 749059
Change-Id: Ida1fe3e60d2eb5c9bbc1625cd4b8b6550f9b716d
Reviewed-on: https://chromium-review.googlesource.com/973610
Reviewed-by: Nick Harper <nharper@chromium.org>
Commit-Queue: Thiemo Nagel <tnagel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550583}
[modify] https://crrev.com/6aee4ab78c122fd80738594eadb878de2268fe04/net/http/transport_security_state.cc
[modify] https://crrev.com/6aee4ab78c122fd80738594eadb878de2268fe04/net/http/transport_security_state.h
[modify] https://crrev.com/6aee4ab78c122fd80738594eadb878de2268fe04/net/http/transport_security_state_unittest.cc

Project Member

Comment 7 by bugdroid1@chromium.org, Apr 18 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/23e0b326847587e3da819d7dbcdc69f6ab58626f

commit 23e0b326847587e3da819d7dbcdc69f6ab58626f
Author: Thiemo Nagel <tnagel@chromium.org>
Date: Wed Apr 18 12:46:54 2018

Add cookie security metrics

Log the degree of protection against cookie theft, split by 1st/3rd
party.

Includes a bit of TransportSecurityState refactoring (no functional
change) to provide easy access to HSTS lifetime.

Bug: 749059
Change-Id: Ib9ab7cff6a7fb4c65e84d7085e70ef13d5c00ba9
Reviewed-on: https://chromium-review.googlesource.com/978142
Reviewed-by: Nick Harper <nharper@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Reviewed-by: Mike West <mkwst@chromium.org>
Commit-Queue: Thiemo Nagel <tnagel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#551644}
[modify] https://crrev.com/23e0b326847587e3da819d7dbcdc69f6ab58626f/net/BUILD.gn
[modify] https://crrev.com/23e0b326847587e3da819d7dbcdc69f6ab58626f/net/http/transport_security_state.cc
[modify] https://crrev.com/23e0b326847587e3da819d7dbcdc69f6ab58626f/net/http/transport_security_state.h
[modify] https://crrev.com/23e0b326847587e3da819d7dbcdc69f6ab58626f/net/url_request/url_request_http_job.cc
[add] https://crrev.com/23e0b326847587e3da819d7dbcdc69f6ab58626f/net/url_request/url_request_http_job_histogram.h
[modify] https://crrev.com/23e0b326847587e3da819d7dbcdc69f6ab58626f/net/url_request/url_request_unittest.cc
[modify] https://crrev.com/23e0b326847587e3da819d7dbcdc69f6ab58626f/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/23e0b326847587e3da819d7dbcdc69f6ab58626f/tools/metrics/histograms/histograms.xml

Project Member

Comment 8 by bugdroid1@chromium.org, Apr 18 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ad6f7598086fc1632640cb0ec347d4400aa9fd2e

commit ad6f7598086fc1632640cb0ec347d4400aa9fd2e
Author: Mohsen Izadi <mohsen@chromium.org>
Date: Wed Apr 18 15:57:10 2018

Revert "Add cookie security metrics"

This reverts commit 23e0b326847587e3da819d7dbcdc69f6ab58626f.

Reason for revert: Suspected cause of net_unittests failures on Mac-10.10

Original change's description:
> Add cookie security metrics
> 
> Log the degree of protection against cookie theft, split by 1st/3rd
> party.
> 
> Includes a bit of TransportSecurityState refactoring (no functional
> change) to provide easy access to HSTS lifetime.
> 
> Bug: 749059
> Change-Id: Ib9ab7cff6a7fb4c65e84d7085e70ef13d5c00ba9
> Reviewed-on: https://chromium-review.googlesource.com/978142
> Reviewed-by: Nick Harper <nharper@chromium.org>
> Reviewed-by: Ilya Sherman <isherman@chromium.org>
> Reviewed-by: Mike West <mkwst@chromium.org>
> Commit-Queue: Thiemo Nagel <tnagel@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#551644}

TBR=isherman@chromium.org,tnagel@chromium.org,nharper@chromium.org,mkwst@chromium.org

Change-Id: I9e7411956eb5d748b547edcf6aec7f40dde88d8d
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 749059
Reviewed-on: https://chromium-review.googlesource.com/1017141
Reviewed-by: Mohsen Izadi <mohsen@chromium.org>
Commit-Queue: Mohsen Izadi <mohsen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#551698}
[modify] https://crrev.com/ad6f7598086fc1632640cb0ec347d4400aa9fd2e/net/BUILD.gn
[modify] https://crrev.com/ad6f7598086fc1632640cb0ec347d4400aa9fd2e/net/http/transport_security_state.cc
[modify] https://crrev.com/ad6f7598086fc1632640cb0ec347d4400aa9fd2e/net/http/transport_security_state.h
[modify] https://crrev.com/ad6f7598086fc1632640cb0ec347d4400aa9fd2e/net/url_request/url_request_http_job.cc
[delete] https://crrev.com/5973ae788c2e1fae845f0cd33c6ba542cd9d815f/net/url_request/url_request_http_job_histogram.h
[modify] https://crrev.com/ad6f7598086fc1632640cb0ec347d4400aa9fd2e/net/url_request/url_request_unittest.cc
[modify] https://crrev.com/ad6f7598086fc1632640cb0ec347d4400aa9fd2e/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/ad6f7598086fc1632640cb0ec347d4400aa9fd2e/tools/metrics/histograms/histograms.xml

Project Member

Comment 9 by bugdroid1@chromium.org, Apr 18 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0793b9c53a5d4fcbab54949c0ba66a3a904a60e2

commit 0793b9c53a5d4fcbab54949c0ba66a3a904a60e2
Author: Thiemo Nagel <tnagel@chromium.org>
Date: Wed Apr 18 16:57:58 2018

Reland "Add cookie security metrics"

This reverts commit ad6f7598086fc1632640cb0ec347d4400aa9fd2e.

Reason for revert: Reverting the revert because I don't think that the original CL is the culprit for net_unittests failures on Mac as these failures already appeared before the CL landed:

https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Mac10.10%20Tests/31390

Original change's description:
> Revert "Add cookie security metrics"
> 
> This reverts commit 23e0b326847587e3da819d7dbcdc69f6ab58626f.
> 
> Reason for revert: Suspected cause of net_unittests failures on Mac-10.10
> 
> Original change's description:
> > Add cookie security metrics
> > 
> > Log the degree of protection against cookie theft, split by 1st/3rd
> > party.
> > 
> > Includes a bit of TransportSecurityState refactoring (no functional
> > change) to provide easy access to HSTS lifetime.
> > 
> > Bug: 749059
> > Change-Id: Ib9ab7cff6a7fb4c65e84d7085e70ef13d5c00ba9
> > Reviewed-on: https://chromium-review.googlesource.com/978142
> > Reviewed-by: Nick Harper <nharper@chromium.org>
> > Reviewed-by: Ilya Sherman <isherman@chromium.org>
> > Reviewed-by: Mike West <mkwst@chromium.org>
> > Commit-Queue: Thiemo Nagel <tnagel@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#551644}
> 
> TBR=isherman@chromium.org,tnagel@chromium.org,nharper@chromium.org,mkwst@chromium.org
> 
> Change-Id: I9e7411956eb5d748b547edcf6aec7f40dde88d8d
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: 749059
> Reviewed-on: https://chromium-review.googlesource.com/1017141
> Reviewed-by: Mohsen Izadi <mohsen@chromium.org>
> Commit-Queue: Mohsen Izadi <mohsen@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#551698}

TBR=mohsen@chromium.org,isherman@chromium.org,tnagel@chromium.org,nharper@chromium.org,mkwst@chromium.org

Change-Id: Ib3260311c6455b5892619e83352da4780ac466a6
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 749059
Reviewed-on: https://chromium-review.googlesource.com/1016649
Reviewed-by: Thiemo Nagel <tnagel@chromium.org>
Commit-Queue: Thiemo Nagel <tnagel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#551721}
[modify] https://crrev.com/0793b9c53a5d4fcbab54949c0ba66a3a904a60e2/net/BUILD.gn
[modify] https://crrev.com/0793b9c53a5d4fcbab54949c0ba66a3a904a60e2/net/http/transport_security_state.cc
[modify] https://crrev.com/0793b9c53a5d4fcbab54949c0ba66a3a904a60e2/net/http/transport_security_state.h
[modify] https://crrev.com/0793b9c53a5d4fcbab54949c0ba66a3a904a60e2/net/url_request/url_request_http_job.cc
[add] https://crrev.com/0793b9c53a5d4fcbab54949c0ba66a3a904a60e2/net/url_request/url_request_http_job_histogram.h
[modify] https://crrev.com/0793b9c53a5d4fcbab54949c0ba66a3a904a60e2/net/url_request/url_request_unittest.cc
[modify] https://crrev.com/0793b9c53a5d4fcbab54949c0ba66a3a904a60e2/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/0793b9c53a5d4fcbab54949c0ba66a3a904a60e2/tools/metrics/histograms/histograms.xml

Cc: mkwst@chromium.org
Components: Privacy
Labels: Merge-Request-67
Owner: tnagel@chromium.org
Requesting merge to M-67 for [1] and [2]. These are low-risk changes adding cookie metrics. The change has been going out over canary and I've verified that metrics data is being received [3].

[1] https://chromium-review.googlesource.com/973610
[2] https://chromium-review.googlesource.com/978142
[3] https://uma.googleplex.com/p/chrome/histograms/?endDate=20180418&dayCount=1&histograms=Cookie.NetworkSecurity&fixupData=true&showMax=true&filters=isofficial%2Ceq%2CTrue&implicitFilters=isofficial
Project Member

Comment 11 by sheriffbot@chromium.org, Apr 20 2018

Labels: -Merge-Request-67 Merge-Review-67 Hotlist-Merge-Review
This bug requires manual review: Reverts referenced in bugdroid comments after merge request.
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Merge-Review-67 Merge-Approved-67
Approving merge to M67 branch 3396 based on comment #10. Please merge ASAP so we can pick it up for next M67 Dev/Beta Release. Thank you.
Pls merge your change to M67 branch 3396 by 1:00 PM PT, Monday (04/23) so we can pick it up next M67 Dev/Beta release. Thank you.
Project Member

Comment 14 by bugdroid1@chromium.org, Apr 23 2018

Labels: -merge-approved-67 merge-merged-3396
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/423a902a31d4cab0ff4e24ad9faf675c286605f1

commit 423a902a31d4cab0ff4e24ad9faf675c286605f1
Author: Thiemo Nagel <tnagel@chromium.org>
Date: Mon Apr 23 10:02:42 2018

Clean up TransportSecurityState::GetStaticDomainState()

Only modify the out parameters when there's actual state to be returned.

Also, when PKP lookup fails due to pinset out of bounds (that should not
happen, I guess) the method only returns false if there is no STS state
either.

Bug: 749059
Change-Id: Ida1fe3e60d2eb5c9bbc1625cd4b8b6550f9b716d
Reviewed-on: https://chromium-review.googlesource.com/973610
Reviewed-by: Nick Harper <nharper@chromium.org>
Commit-Queue: Thiemo Nagel <tnagel@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#550583}(cherry picked from commit 6aee4ab78c122fd80738594eadb878de2268fe04)
Reviewed-on: https://chromium-review.googlesource.com/1023870
Reviewed-by: Thiemo Nagel <tnagel@chromium.org>
Cr-Commit-Position: refs/branch-heads/3396@{#202}
Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428}
[modify] https://crrev.com/423a902a31d4cab0ff4e24ad9faf675c286605f1/net/http/transport_security_state.cc
[modify] https://crrev.com/423a902a31d4cab0ff4e24ad9faf675c286605f1/net/http/transport_security_state.h
[modify] https://crrev.com/423a902a31d4cab0ff4e24ad9faf675c286605f1/net/http/transport_security_state_unittest.cc

Project Member

Comment 15 by bugdroid1@chromium.org, Apr 23 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/adda82f05a85ab598b2313896270c2dcab8706a1

commit adda82f05a85ab598b2313896270c2dcab8706a1
Author: Thiemo Nagel <tnagel@chromium.org>
Date: Mon Apr 23 10:04:36 2018

Add cookie security metrics

Log the degree of protection against cookie theft, split by 1st/3rd
party.

Includes a bit of TransportSecurityState refactoring (no functional
change) to provide easy access to HSTS lifetime.

Bug: 749059
Change-Id: Ib9ab7cff6a7fb4c65e84d7085e70ef13d5c00ba9
Reviewed-on: https://chromium-review.googlesource.com/978142
Reviewed-by: Nick Harper <nharper@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Reviewed-by: Mike West <mkwst@chromium.org>
Commit-Queue: Thiemo Nagel <tnagel@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#551644}(cherry picked from commit 23e0b326847587e3da819d7dbcdc69f6ab58626f)
Reviewed-on: https://chromium-review.googlesource.com/1023632
Reviewed-by: Thiemo Nagel <tnagel@chromium.org>
Cr-Commit-Position: refs/branch-heads/3396@{#203}
Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428}
[modify] https://crrev.com/adda82f05a85ab598b2313896270c2dcab8706a1/net/BUILD.gn
[modify] https://crrev.com/adda82f05a85ab598b2313896270c2dcab8706a1/net/http/transport_security_state.cc
[modify] https://crrev.com/adda82f05a85ab598b2313896270c2dcab8706a1/net/http/transport_security_state.h
[modify] https://crrev.com/adda82f05a85ab598b2313896270c2dcab8706a1/net/url_request/url_request_http_job.cc
[add] https://crrev.com/adda82f05a85ab598b2313896270c2dcab8706a1/net/url_request/url_request_http_job_histogram.h
[modify] https://crrev.com/adda82f05a85ab598b2313896270c2dcab8706a1/net/url_request/url_request_unittest.cc
[modify] https://crrev.com/adda82f05a85ab598b2313896270c2dcab8706a1/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/adda82f05a85ab598b2313896270c2dcab8706a1/tools/metrics/histograms/histograms.xml

Sign in to add a comment