Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable "Conversion Measurement API" #1531

Closed
uazo opened this issue Nov 1, 2021 · 30 comments · Fixed by #1583
Closed

Disable "Conversion Measurement API" #1531

uazo opened this issue Nov 1, 2021 · 30 comments · Fixed by #1583

Comments

@uazo
Copy link
Collaborator

uazo commented Nov 1, 2021

Is your feature request related to privacy?

Yes

Is there a patch available for this feature somewhere?

No

Describe the solution you would like

I think that patch is incomplete: there are other types of reports not covered by that patch like QueueAccessReport or QueueNavigationReport

Describe alternatives you have considered

Fix directly this or the other side of the mojo call

@uazo
Copy link
Collaborator Author

uazo commented Nov 1, 2021

I apologize if I'm clogging up the repo with my requests, but I'm checking this and a few things come up to check

@csagan5
Copy link
Contributor

csagan5 commented Nov 1, 2021

There is no way to disable everything about reporting?

@csagan5 csagan5 changed the title Fix Allow-building-without-enable_reporting.patch Some type of reports (QueueAccessReport or QueueNavigationReport) are not covered by enable_reporting patch Nov 2, 2021
@uazo
Copy link
Collaborator Author

uazo commented Nov 4, 2021

I checked the code, there are two different types of reports, those related to navigation and those related to the "Conversion Measurement API" currently under origin trials.
The mojom calls are very similar and this has led me to confusion: the patch in question correctly disables the former as it is, possibly, if one day they remove the buildflag, you can act here CanSendReports.
the last type of report I am still verifying

@uazo uazo changed the title Some type of reports (QueueAccessReport or QueueNavigationReport) are not covered by enable_reporting patch Disable "Conversion Measurement API" Nov 4, 2021
@csagan5
Copy link
Contributor

csagan5 commented Nov 4, 2021

Conversion measurement is perhaps related to RLZ? That is something disabled long ago (since first version of Bromite)

@uazo
Copy link
Collaborator Author

uazo commented Nov 4, 2021

Conversion measurement is perhaps related to RLZ?

no, it seems worse to me.
from a first reading it seems that redirects are basically changed to ping via that api, but I'm still checking.

@uazo
Copy link
Collaborator Author

uazo commented Nov 6, 2021

so, here the situation is a bit more complicated and I hope my bad English is not in the way.

first of all the "Conversion Measurement API" is part of the developments linked to the same series of the privacy-sandbox features source, the next will be Fledge.
here one of the few public documents I have found linked in the code and which explains the use by web developers in the fastest way.

deactivation is possible although I am undecided on the correct point where to insert it.
points can be

  • via flag/switch/enabled_features which completely removes the function also on the javascript side (for now) as well as the detection always on the javascript side
  • leaving everything as it is now but removing the POST call (here or maybe better here)

in the latter case we have the advantage that the function is detectable by javascript (then looking like chrome, resuming this), but no report will ever be sent. and considering there is no constraint (see https://github.com/WICG/conversion-measurement-api/ issue 244) on when to send, they may never notice the difference.

There is no way to disable everything about reporting?

I'm noticing more and more that actually all the parts that communicate with the outside are marked by a NetworkTrafficAnnotationTag which makes me think that a proposed solution could really close any present and future unauthorized communication.
java side there are no annotations but we could use the bytecode processor to modify/track the external calls by looking for HttpURLConnection.getOutputStream(), @wchen342 you say that is something possible?

from a first reading it seems that redirects are basically changed to ping via that api, but I'm still checking.

no, that's not quite the case. the code eliminates the redirect only for calls to the special url /.well-known/attribution-reporting/trigger-attribution

@uazo
Copy link
Collaborator Author

uazo commented Nov 6, 2021

the code eliminates the redirect only for calls to the special url

ah, the bad thing is that not even the subresource filter would have been able to block it, because the check is placed at the end (which is another thing to check, makes me think there are urls that cannot be never blocked)

https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/loader/base_fetch_context.cc;l=663;drc=917a2b5c9c82f96f685692202fd44725b02f6e98;bpv=1;bpt=1

@uazo
Copy link
Collaborator Author

uazo commented Nov 7, 2021

AttributionReportingProvider must also be disabled (via ChromeFeatureList.APP_TO_WEB_ATTRIBUTION actually disabled), since it is possible for android applications to send report requests to the browser via intent.

@csagan5
Copy link
Contributor

csagan5 commented Nov 7, 2021

deactivation is possible although I am undecided on the correct point where to insert it. points can be

  • via flag/switch/enabled_features which completely removes the function also on the javascript side (for now) as well as the detection always on the javascript side
  • leaving everything as it is now but removing the POST call (here or maybe better here)

in the latter case we have the advantage that the function is detectable by javascript (then looking like chrome, resuming this), but no report will ever be sent. and considering there is no constraint (see https://github.com/WICG/conversion-measurement-api/ issue 244) on when to send, they may never notice the difference.

We should disable the flag/switch/enabled_feature; there is no intent in Bromite to look like Chrome/Chromium anyways. It is a separate browser, and frankly those are no more browsers but something else.

There is no way to disable everything about reporting?

I'm noticing more and more that actually all the parts that communicate with the outside are marked by a NetworkTrafficAnnotationTag which makes me think that a proposed solution could really close any present and future unauthorized communication.

See my reply there:

Before doing this we should survey what are the traffic annotations and if there is an use-case for enabling/disabling some. If not, this would not be necessary.

It is a good idea to study the existing traffic annotations and trace them back to the feature or code that causes them, but blocking undesired functionality should be based on disabled features and code changes not just on filtering at the network annotation level.

ah, the bad thing is that not even the subresource filter would have been able to block it, because the check is placed at the end (which is another thing to check, makes me think there are urls that cannot be never blocked)

https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/loader/base_fetch_context.cc;l=663;drc=917a2b5c9c82f96f685692202fd44725b02f6e98;bpv=1;bpt=1

Thank you for digging this out; are all these "features" something which has been added recently?

As for the subresource filter: that is only for ads. As mentioned earlier we should block the undesired functionality at code level, possibly by not even compiling it.

AttributionReportingProvider must also be disabled (via ChromeFeatureList.APP_TO_WEB_ATTRIBUTION actually disabled), since it is possible for android applications to send report requests to the browser via intent.

Yes, it should be disabled as well.

@uazo
Copy link
Collaborator Author

uazo commented Nov 7, 2021

We should disable the flag/switch/enabled_feature; there is no intent in Bromite to look like Chrome/Chromium anyways. It is a separate browser

the only doubt I ask is about the future.
considering that google intends to disable thirty party cookies permanently, my concern is that websites will one day identify bromite and block the display as it happens today when they identify an incognito session.

..Chromium.. and frankly those are no more browsers but something else

and to think that I am using it in windows, I feel bad...

It is a good idea to study the existing traffic annotations and trace them back to the feature or code that causes them, but blocking undesired functionality should be based on disabled features and code changes not just on filtering at the network annotation level.

certainly, any deactivation must be verified

are all these "features" something which has been added recently?

it all seems related to the privacy sandbox, see https://github.com/google/ads-privacy

As for the subresource filter: that is only for ads.

in my opinion it must be considered as a blocking method decided by the user, among other things also the only one existing in bromite, regardless of how it was born.

Yes, it should be disabled as well.

Sure, although I think, being a nominal intent, it should be called explicitly.
that is, who has bromite and chrome installed, the chrome intent will be invoked, certainly not that of bromite (I think).

among other things I also found this https://github.com/WICG/floc/blob/main/README.md#qualifying-users-for-whom-a-cohort-will-be-logged-with-their-sync-data

@wchen342
Copy link
Collaborator

wchen342 commented Nov 7, 2021

java side there are no annotations but we could use the bytecode processor to modify/track the external calls by looking for HttpURLConnection.getOutputStream(), @wchen342 you say that is something possible?

You can, but I don't really get why you need to do that since you already have patching ability. The reason we use ASM is avoiding patching as much as possible.

@uazo
Copy link
Collaborator Author

uazo commented Nov 7, 2021

You can,

Perfect

but I don't really get why you need to do that since you already have patching ability.

to track them and automatically report new ones.

The reason we use ASM is avoiding patching as much as possible.

yes, I saw, nice idea, even if it seems dirty too much then the code (but I have no experience in this regard).

@wchen342
Copy link
Collaborator

wchen342 commented Nov 7, 2021

to track them and automatically report new ones.

From a software engineering point of view, I think if you want to use asm for that it is better done as some kind of unit test, rather than doing it directly because the latter is more error prone. But that requires you have a running test first, so yeah, you need to trade off here.

@uazo
Copy link
Collaborator Author

uazo commented Nov 7, 2021

it is better done as some kind of unit test,

you mean inside a c.i.? yes I mean just that.
if you mean real tests I have thought about it and I have also tried, but I think that the development of the unit tests in the strict sense does not go very well. the substantial problem I should solve is not so much if my tests work, but if their code (once they have modified it) calls mine. and with unit tests I would not have this guarantee.
in fact I am trying with the instrumentation tests, but I find them very complex to do by hand, unless you use the android studio espresso test record. but here the problem is that I develop on windows, compile on linux via vscode remote, all inside now in a remote docker container, and i haven't tried to transfer the android gradle project yet and see if it works (in windows).
I am also trying an alternative way, which would be to not activate any test but to drive the apk from the outside, using CulebraTester, but in addition to compiling it I did not go ahead. if this solution worked maybe it would allow anyone to make tests without necessarily knowing any programming language.

rather than doing it directly because the latter is more error prone

I'm sorry, I don't understand. it would only to report it, it shouldn't change anything.
or is asm not to be reliable?

But that requires you have a running test first, so yeah, you need to trade off here.

yes, the problem here is that there are some patches that break the build of their tests, although they would be of little interest to me, because they have to run mine, not theirs. here an example of result of same tests i write.
i should do a gn/apk with just my tests, it probably becomes the way I will go if the other ways don't work.

@csagan5
Copy link
Contributor

csagan5 commented Nov 8, 2021

We should disable the flag/switch/enabled_feature; there is no intent in Bromite to look like Chrome/Chromium anyways. It is a separate browser

the only doubt I ask is about the future. considering that google intends to disable thirty party cookies permanently, my concern is that websites will one day identify bromite and block the display as it happens today when they identify an incognito session.

Let them do that and we will find another strategy; no point implementing something now for that future scenario, you are trying to optimise just for the time window needed to find a new strategy.

As for the subresource filter: that is only for ads.

in my opinion it must be considered as a blocking method decided by the user, among other things also the only one existing in bromite, regardless of how it was born.

The browser should not initiate connections without user consent; we will block those with patches, not with the subresource filter.

Yes, it should be disabled as well.

Sure, although I think, being a nominal intent, it should be called explicitly. that is, who has bromite and chrome installed, the chrome intent will be invoked, certainly not that of bromite (I think).

among other things I also found this https://github.com/WICG/floc/blob/main/README.md#qualifying-users-for-whom-a-cohort-will-be-logged-with-their-sync-data

We just need to remove/not compile the code that Bromite does not use anyways.

You can create a testsuite to "sample" the outgoing traffic but personally I prefer reading the CHANGELOGs and feature development history directly.

@uazo
Copy link
Collaborator Author

uazo commented Nov 9, 2021

Let them do that and we will find another strategy; no point implementing something now for that future scenario, you are trying to optimise just for the time window needed to find a new strategy.

okay, let's try this.

The browser should not initiate connections without user consent; we will block those with patches, not with the subresource filter.

I don't completely agree, but the goal is the same.
among other things I have seen that there are headers in the calls that cannot be blocked, and guess what? some those aimed at google servers... (ref https://chromium-review.googlesource.com/c/chromium/src/+/2129787 but my english is bad ..)

Would you help me understand if there is any justification for this? that is, is it normal that some headers do not pass through the cors filter? in the code is everything that is placed in cors_exempt_header_list

among other things I also found this https://github.com/WICG/floc/blob/main/README.md#qualifying-users-for-whom-a-cohort-will-be-logged-with-their-sync-data
We just need to remove/not compile the code that Bromite does not use anyways.

sure, bromite doesn't care, but substantially the floc data of the users, sent via sync, will be used by google to produce an aggregate report that will be granted to the websites, if I have not misunderstood
https://github.com/WICG/conversion-measurement-api/blob/main/SERVICE.md

@uazo
Copy link
Collaborator Author

uazo commented Nov 9, 2021

java side there are no annotations but we could use the bytecode processor to modify/track the external calls by looking for HttpURLConnection.getOutputStream()

just to correct myself, it seems like they are introducing it for java too
https://source.chromium.org/chromium/chromium/src/+/main:net/android/java/src/org/chromium/net/NetworkTrafficAnnotationTag.java;l=23;drc=f78106f1fb37b3aa9e660993c62e645ea92f34f1;bpv=1;bpt=0

@csagan5
Copy link
Contributor

csagan5 commented Nov 9, 2021

among other things I have seen that there are headers in the calls that cannot be blocked, and guess what? some those aimed at google servers... (ref https://chromium-review.googlesource.com/c/chromium/src/+/2129787 but my english is bad ..)

I think domain-specific exceptions are covered by ungoogled-chromium-Disable-Google-host-detection.patch and by the automated domain substitution patch?

Any proof that Bromite is currently sending any special header to Google domains? (It is not, as far as I know)

Would you help me understand if there is any justification for this? that is, is it normal that some headers do not pass through the cors filter? in the code is everything that is placed in cors_exempt_header_list

Without a list of the headers it is impossible to make a guess, and it is a bit difficult to extract it from the Gerrit code review. The headers seem related to Google Apps integrations (X-GoogApps-Allowed-Domain). The last question on that review wonders about the same, but it is unanswered.

@csagan5
Copy link
Contributor

csagan5 commented Nov 9, 2021

among other things I also found this https://github.com/WICG/floc/blob/main/README.md#qualifying-users-for-whom-a-cohort-will-be-logged-with-their-sync-data
We just need to remove/not compile the code that Bromite does not use anyways.

sure, bromite doesn't care, but substantially the floc data of the users, sent via sync, will be used by google to produce an aggregate report that will be granted to the websites, if I have not misunderstood https://github.com/WICG/conversion-measurement-api/blob/main/SERVICE.md

It cares, otherwise it would not exist; but other than existing I do not think much else can be done for this further privacy land-grab; what do you suggest Bromite should do other than not running FLoC?

java side there are no annotations but we could use the bytecode processor to modify/track the external calls by looking for HttpURLConnection.getOutputStream()

just to correct myself, it seems like they are introducing it for java too https://source.chromium.org/chromium/chromium/src/+/main:net/android/java/src/org/chromium/net/NetworkTrafficAnnotationTag.java;l=23;drc=f78106f1fb37b3aa9e660993c62e645ea92f34f1;bpv=1;bpt=0

Feel free to use and peruse the network annotations to discover about new privacy violations.

@uazo
Copy link
Collaborator Author

uazo commented Nov 10, 2021

Without a list of the headers it is impossible to make a guess

I try to retrieve the list

what do you suggest Bromite should do other than not running FLoC?

I don't know, I was thinking about it too
maybe force disable android backup and force dbs in memory rather than disk.
or make default/preferred the always incognito mode, you don't know how many things are skipped in that mode!

among other things, reading the code, I came up with ways to understand if you are browsing incognito, even if it seems strange to me that they have not already been exploited (so I'm probably wrong). I'll try this too.

Feel free to use and peruse the network annotations to discover about new privacy violations.

Yes I will.

@wchen342
Copy link
Collaborator

because they have to run mine, not theirs. here an example of result of same tests i write.

That's the right approach IMO. I had a quick look at your workflow file and I think it is pretty neat. You shall try polish it a little more and I may also use something similar in the future. Making tests is on my TODO list after I finish refurbishing the whole building process.
Instrument tests are harder to write and run but if you need to test some interactions/UI then you shall use them. The principle of unit tests are that the tested part shall be able to work correctly standalone, thus "unit" test.

@uazo
Copy link
Collaborator Author

uazo commented Nov 10, 2021

You shall try polish it a little more

yes I know... there is also a vulnerability that I already know but I don't know how to fix ...
the problem is that the containers are deleted after use and this does not allow me to use docker cp.

Making tests is on my TODO list after I finish refurbishing the whole building process.

ok, I'm keeping an eye on you! :)

The principle of unit tests are that the tested part shall be able to work correctly standalone, thus "unit" test.

yes, but I apply the principle to my code, that is to the parts that I modify, but I don't know how the chromium team modifies its code, so my -unit- tests could be successful, but together with theirs eventually instead no.

@csagan5
Copy link
Contributor

csagan5 commented Nov 15, 2021

consider that it is currently active in bromite.

@uazo can the upstream servers receive anything, considering that there are no API keys in Bromite?

@csagan5 csagan5 reopened this Nov 15, 2021
@uazo
Copy link
Collaborator Author

uazo commented Nov 15, 2021

can the upstream servers receive anything

the recipient of the reports is not upstream, but the url which is defined by the javascript call (or by attributes in a tags or by window.open()).

@csagan5
Copy link
Contributor

csagan5 commented Nov 15, 2021

can the upstream servers receive anything

the recipient of the reports is not upstream, but the url which is defined by the javascript call (or by attributes in a tags or by window.open()).

And in this case no key-signing would happen?

@uazo
Copy link
Collaborator Author

uazo commented Nov 15, 2021

And in this case no key-signing would happen?

if the question is if there is any signature in the sent payload (a simple json), no, no signature.

@csagan5
Copy link
Contributor

csagan5 commented Nov 15, 2021

Sounds like a wet dream for advertisers.

@uazo
Copy link
Collaborator Author

uazo commented Nov 15, 2021

there is a nice public report that declares a net loss of earnings without third party cookies, and so they transform the browser to remedy.
and I don't tell you how it is fledge !! I advise you to read the specifications...

@csagan5
Copy link
Contributor

csagan5 commented Nov 15, 2021

Sounds like a desperate move; I have read the summary of all these techs you listed here but it was quite some time ago, they were not fully detailed/formed.

@csagan5
Copy link
Contributor

csagan5 commented Nov 17, 2021

Fixed in 95.0.4638.79.

@csagan5 csagan5 closed this as completed Nov 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants