Project

General

Profile

Bug #65

When a site fails to load, for example due to its IP address not being found, the injected value with settings remains in the URL

Added by koszko about 2 years ago. Updated about 2 years ago.

Status:
Closed
Priority:
Normal
Assignee:
-
Start date:
07/26/2021
Due date:
% Done:

100%

Estimated time:

History

#1

Updated by koszko about 2 years ago

We should investigate if we can use Set-Cookie header instead of URL for policy smuggling

EDIT: Looks very promising. Under Chromium at least. Cookie value can be checked synchronously at document_start. What's more, deleting the cookie then prevents it from being sent to server in Coookie: request header in connections originating from this site.

#2

Updated by jahoti about 2 years ago

Sounds like a winner (and much safer than dealing with the URL fragment)! That said, is there any way to deal with a page trying to set a cookie with the same key as we use?

#3

Updated by koszko about 2 years ago

  • % Done changed from 0 to 90

Sounds like a winner (and much safer than dealing with the URL fragment)!

It is indeed way more convenient. Safer? Not sure. With the old method it was very unlikely the smuggled value would somehow leak to the server. The only bugs were related to convenience. Now there is real danger cookie will not get deleted for some reason and will get sent to server. Anyway, I think this approach is sufficient for now :)

That said, is there any way to deal with a page trying to set a cookie with the same key as we use?

I made key contain the signature. Signature is derived from nonce, so it should be OK.

I also dropped all the conditionals that made code execute under Firefox only. The code works just as well when there's no header caching in place. Plus who knows when Chrome devs decide to do the same thing with caching? As long as header operations don't slow things down too much, it can be like this :)

It's on my branch for now. I am yet to test under Mozilla

EDIT: Newest commit on my branch restores compatibility with IceCat 60. Testing on other browsers still welcome :)

#4

Updated by jahoti about 2 years ago

Now there is real danger cookie will not get deleted for some reason and will get sent to server. Anyway, I think this approach is sufficient for now :)

Ah, good point- that will be something to look at eventually.

I also dropped all the conditionals that made code execute under Firefox only. The code works just as well when there's no header caching in place. Plus who knows when Chrome devs decide to do the same thing with caching? As long as header operations don't slow things down too much, it can be like this :)

Indeed, and anything that reduces the amount of browser specific code is a positive :)!

It's on my branch for now. I am yet to test under Mozilla

EDIT: Newest commit on my branch restores compatibility with IceCat 60. Testing on other browsers still welcome :)

I'll have a look soon!

#5

Updated by jahoti about 2 years ago

EDIT: Newest commit on my branch restores compatibility with IceCat 60. Testing on other browsers still welcome :)

It most works on LibreWolf and Tor Browser too! However, there is a small bug in content/main.js; the nonce variable is no longer set, yet the inject_csp function still uses it (and it's still declared).

#6

Updated by koszko about 2 years ago

Thanks for pointing out. I'll fix it together with some bigger changes for issue 15 https://hachettebugs.koszko.org/issues/15

#7

Updated by koszko about 2 years ago

Now there is real danger cookie will not get deleted for some reason and will get sent to server. Anyway, I think this approach is sufficient for now :)

Ah, good point- that will be something to look at eventually.

We could use webRequest to remove our cookies from request headers in case they happen to get there

#8

Updated by koszko about 2 years ago

We could use webRequest to remove our cookies from request headers in case they happen to get there

Committed to `koszko-smuggle-policy' branch

#9

Updated by koszko about 2 years ago

  • Status changed from New to Closed
  • % Done changed from 90 to 100

Merged to master

Also available in: Atom PDF