Skip to content
This repository has been archived by the owner on Nov 6, 2023. It is now read-only.

Add upgrade-insecure-requests to responseHeaders in HTTP Nowhere Mode #14600

Merged
merged 5 commits into from
Feb 15, 2018
Merged

Add upgrade-insecure-requests to responseHeaders in HTTP Nowhere Mode #14600

Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0fa804f
Add upgrade-insecure-requests to responseHeaders in HTTP Nowhere Mode…
Feb 11, 2018
0d1f114
When extension is disabled, do nothing
Feb 11, 2018
1e06bf1
If CSP already existed, modify it insteads
Feb 13, 2018
e4b17e8
Exclude .onion requests from being upgraded
Feb 14, 2018
9541530
Return responseHeaders only if it is modified
Feb 14, 2018
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
41 changes: 40 additions & 1 deletion chromium/background.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -497,11 +497,46 @@ function onErrorOccurred(details) {
}
}

/**
* handle webrequest.onHeadersReceived, insert upgrade-insecure-requests directive
* @param details details for the chrome.webRequest (see chrome doc)
*/
function onHeadersReceived(details) {
if (isExtensionEnabled && httpNowhereOn) {
Copy link
Member

Choose a reason for hiding this comment

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

There may be a wild edge case here. In

https-everywhere/chromium/background.js

Lines 247 to 253 in 2808747

const shouldCancel = httpNowhereOn &&
uri.protocol === 'http:' &&
uri.hostname.slice(-6) !== '.onion' &&
uri.hostname !== 'localhost' &&
!/^127(\.[0-9]{1,3}){3}$/.test(canonical_host) &&
!/^0\.0\.0\.0$/.test(canonical_host) &&
uri.hostname !== '[::1]';
, we specifically allow .onion URLs that do not have HTTPS in HTTP Nowhere mode. If a .onion URL serves only one page in HTTPS, and the rest are over HTTP, landing on that page with this logic will cause the client to make unreachable upgraded requests over HTTPS.

As I said, it's a pretty far-fetched edge case (why would a .onion site have a TLS configuration like that?) but it is possible, and probably worth accounting for.

Copy link
Collaborator Author

@cschanaj cschanaj Feb 14, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

I agree. Since .onion requests are encrypted anyway, IMHO we shall exclude them from the HTTPS upgrades, see e4b17e8

Copy link

Choose a reason for hiding this comment

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

I think requests to non-.onion websites should still be upgraded if they were made from .onion website.

// Do not upgrade the .onion requests in HTTP Nowhere Mode,
// See https://github.com/EFForg/https-everywhere/pull/14600#discussion_r168072480
const uri = new URL(details.url);
if (uri.hostname.slice(-6) == '.onion') {
return {responseHeaders: details.responseHeaders};
Copy link

Choose a reason for hiding this comment

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

return {};

}

for (const idx in details.responseHeaders) {
Copy link

Choose a reason for hiding this comment

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

Don't use a for loop, use find function.

if (details.responseHeaders[idx].name.match(/Content-Security-Policy/i)) {
// Existing CSP headers found
const value = details.responseHeaders[idx].value;

// Prepend if no upgrade-insecure-requests directive exists
if (!value.match(/upgrade-insecure-requests/i)) {
details.responseHeaders[idx].value = "upgrade-insecure-requests; " + value;
}
return {responseHeaders: details.responseHeaders};
}
}

// CSP headers not found
const upgradeInsecureRequests = {
name: 'Content-Security-Policy',
value: 'upgrade-insecure-requests'
}
details.responseHeaders.push(upgradeInsecureRequests);
}
return {responseHeaders: details.responseHeaders};
}

// Registers the handler for requests
// See: https://github.com/EFForg/https-everywhere/issues/10039
chrome.webRequest.onBeforeRequest.addListener(onBeforeRequest, {urls: ["*://*/*"]}, ["blocking"]);


// Try to catch redirect loops on URLs we've redirected to HTTPS.
chrome.webRequest.onBeforeRedirect.addListener(onBeforeRedirect, {urls: ["https://*/*"]});

Expand All @@ -511,9 +546,13 @@ chrome.webRequest.onCompleted.addListener(onCompleted, {urls: ["*://*/*"]});
// Cleanup redirectCounter if neccessary
chrome.webRequest.onErrorOccurred.addListener(onErrorOccurred, {urls: ["*://*/*"]})

// Insert upgrade-insecure-requests directive in httpNowhere mode
chrome.webRequest.onHeadersReceived.addListener(onHeadersReceived, {urls: ["https://*/*"]}, ["blocking", "responseHeaders"]);

// Listen for cookies set/updated and secure them if applicable. This function is async/nonblocking.
chrome.cookies.onChanged.addListener(onCookieChanged);


/**
* disable switch Planner
* @param tabId the Tab to disable for
Expand Down