Skip to content
This repository was archived by the owner on Jan 4, 2019. It is now read-only.

HSTS Fingerprinting #527

Closed
wants to merge 1 commit into from
Closed

HSTS Fingerprinting #527

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 31 additions & 1 deletion patches/master_patch.patch
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1844,6 +1844,36 @@ index a433afd0217830b49ddb3381ad55afeae2381840..ddcca749666f956c6317676dbcdd48da
DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
}

diff --git a/net/url_request/url_request_http_job.cc b/net/url_request/url_request_http_job.cc
index da7a19865989404f3147ca3e29a13f7f29f175d4..47fa22638f7450367c9014d2f2fa70f1659d2f15 100644
--- a/net/url_request/url_request_http_job.cc
+++ b/net/url_request/url_request_http_job.cc
@@ -415,8 +415,10 @@ void URLRequestHttpJob::NotifyBeforeSendHeadersCallback(
}

void URLRequestHttpJob::NotifyHeadersComplete() {
- DCHECK(!response_info_);
+ std::string first_party_host = request_->site_for_cookies().host();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we care about this warning? https://cs.chromium.org/chromium/src/net/url_request/url_request.h?q=site_for_cookies&sq=package:chromium&l=270

site_for_cookies is used for firstPartyUrl in webRequest also; i don't exactly remember why we decided it wasn't an issue there

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even in this case we are using site_for_cookies to figure out the first party url.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

according to https://tools.ietf.org/html/draft-ietf-httpbis-cookie-same-site-00 section 2.1, site for cookies is not the same as top-level site in a few cases:

  1. in a nested cross-origin iframe, the site-for-cookies is ''
  2. for shared workers belonging to documents with different origins, the site-for-cookies is ''

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i guess it doesn't matter in this case since we only use the first party host to check if it is the same as the request host (which would not be empty in the cases above): https://github.com/brave/muon/pull/527/files#diff-85ff0d997b1a2f089546f7b2823b0ad7R1869

+ std::string request_host = request_->url().host();

+ DCHECK(!response_info_);
response_info_ = transaction_->GetResponseInfo();

// Save boolean, as we'll need this info at destruction time, and filters may
@@ -426,8 +428,11 @@ void URLRequestHttpJob::NotifyHeadersComplete() {
if (!is_cached_content_ && throttling_entry_.get())
throttling_entry_->UpdateWithResponse(GetResponseCode());

- // The ordering of these calls is not important.
- ProcessStrictTransportSecurityHeader();
+ if (first_party_host == request_host) {
+ // The ordering of these calls is not important.
+ ProcessStrictTransportSecurityHeader();
+ }
+
ProcessPublicKeyPinsHeader();
ProcessExpectCTHeader();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think we should also move ProcessPublicKeyPinsHeader and ProcessExpectCTHeader into the if (first_party_host == request_host) {... block since they can be used for similar tracking

#if BUILDFLAG(ENABLE_REPORTING)
diff --git a/net/url_request/url_request_job.h b/net/url_request/url_request_job.h
index 389315a25d7da30d71b59e3803ab795bc4c18e1c..79797fe7e674f9f6d2a65de542f262a945454f16 100644
--- a/net/url_request/url_request_job.h
Expand All @@ -1870,7 +1900,7 @@ index 023555bcd3b56fd12c3319096deff07fa9854388..5b75bfaa858cead732a701029f45f549

bool ShouldSelectReplacement() const {
diff --git a/third_party/WebKit/Source/core/exported/WebViewImpl.cpp b/third_party/WebKit/Source/core/exported/WebViewImpl.cpp
index b73e62a0420f77db2874260a6f5e141264018cb8..c0f24ad844cb53eb813e4dc6ad7f8eef20ca7fc4 100644
index 9a59d4f569de1ea8f5cd84849e313b4512576043..8e12398d8a153f9710650e35f21b1f5b9de83dd3 100644
--- a/third_party/WebKit/Source/core/exported/WebViewImpl.cpp
+++ b/third_party/WebKit/Source/core/exported/WebViewImpl.cpp
@@ -3237,6 +3237,7 @@ void WebViewImpl::DidCloseContextMenu() {
Expand Down