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

Check Certificate Transparency and reporting #1554

Closed
uazo opened this issue Nov 9, 2021 · 8 comments
Closed

Check Certificate Transparency and reporting #1554

uazo opened this issue Nov 9, 2021 · 8 comments

Comments

@uazo
Copy link
Collaborator

uazo commented Nov 9, 2021

Is your feature request related to privacy?

I don't know yet.

Is there a patch available for this feature somewhere?

No

Describe the solution you would like

I found other reports sent:

ref https://github.com/GoogleChrome/CertificateTransparency/blob/master/ct_policy.md

Describe alternatives you have considered

none for now

@csagan5
Copy link
Contributor

csagan5 commented Nov 9, 2021

I don't know yet.

See https://source.chromium.org/chromium/chromium/src/+/main:chrome/browser/prefetch/prefetch_proxy/prefetch_proxy_tab_helper.cc;l=1652;drc=b6e40f8ab247ea209bcecff994386a70a4169e9d;bpv=1;bpt=1

  // Explicitly disallow network service features which could cause a privacy
  // leak.
  context_params->enable_certificate_reporting = false;
  context_params->enable_expect_ct_reporting = false;
  context_params->enable_domain_reliability = false;

We should probably have all of them off.

I found other reports sent:

ref https://github.com/GoogleChrome/CertificateTransparency/blob/master/ct_policy.md

I am going to need proof of this before any patch or change. Can you verify that reports are actually being sent from current Bromite?

@uazo
Copy link
Collaborator Author

uazo commented Nov 10, 2021

  // Explicitly disallow network service features which could cause a privacy
  // leak.

incredible... when is the windows version :) ?

Can you verify that reports are actually being sent from current Bromite?

Sure. I haven't seen anything about it yet, just marked.

@uazo
Copy link
Collaborator Author

uazo commented Nov 23, 2021

Currently all three features are disabled:

  • certificate_reporting and expect_ct_reporting, linked to the Certificate Transparency functionality (https://certificate.transparency.dev)
    disabled by default, although BUILDFLAG(IS_CT_SUPPORTED)=true because the logic is underneath BUILDFLAG(GOOGLE_CHROME_BRANDING) && defined(OFFICIAL_BUILD)
    and the motivation seems this:
 // Certificate Transparency is only enabled if:
 // - base::GetBuildTime() is deterministic to the source (OFFICIAL_BUILD)
 // - The build in reliably updatable (GOOGLE_CHROME_BRANDING)

 // Static pinning is only enabled for official builds to make sure that
 // others don't end up with pins that cannot be easily updated.

google is considering reactivating the feature in future versions (through kCertificateTransparencyAndroid, active since commit
https://source.chromium.org/chromium/chromium/src/+/c88ab85c077282cd0cbc0b0444a0d490c4461975 of 1 Nov),
so sooner or later we should decide what to do.
reporting is not "formally" deactivated, but, on balance, since the CT is deactivated, there is no way to send any report except through chrome://net-internals

therefore, apart from the external reports to be deactivated, Certificate Transparency seems to me a positive thing to activate.

Some technical documents in this regard:

Technically it works like this:

  • From a static json (components/certificate_transparency/data/log_list.json) at compile time it is generated a list of values containing the CTLogs (file ./out/bromite/gen/components/certificate_transparency/data/log_list-inc.cc)
  • The list of control pins can be found in transport_security_state_static.pins and generates ./out/bromite/gen/net/http/transport_security_state_static.h
  • The report urls are taken from the same list

you find the various generators in ./net/http/BUILD.gn (https://source.chromium.org/chromium/chromium/src/+/main:net/tools/transport_security_state_generator/README.md)

I provide you with the patch I made to check the code, then I modify you https://github.com/uazo/superpatch to allow you to study it if you want without having to apply it.
EDIT: use this link
If we decide to activate the ct without report, I will prepare a pull, however the idea is to activate it with a flag because I imagine it could give some problems to users who use the proxy with https decryption.

To check, you can use the developer tools, with active cts you have this:
image

I hope I have not written incorrect things, and for my english, you know, it is the fault of the google translator :)

@uazo uazo changed the title Check "Expect-CT reporting for Certificate Transparency reporting" Check Certificate Transparency and reporting Nov 23, 2021
@csagan5
Copy link
Contributor

csagan5 commented Dec 2, 2021

  // Explicitly disallow network service features which could cause a privacy
  // leak.

incredible... when is the windows version :) ?

Doesn't this code apply to all OSes?

Currently all three features are disabled:

Thanks for checking this.

reporting is not "formally" deactivated

So our patch about enable_reporting does not cover these?

so sooner or later we should decide what to do.

We have to disable them all.

  • domain_reliability, already disabled by the patch ungoogled-chromium-Disable-domain-reliability.patch
    being deactivated, I have not checked anything, but if you tell me that you are not sure of the function I do it.

I am certain that there is no way that the browser can identify "reliable domains" (e.g. Google-owned domains).

therefore, apart from the external reports to be deactivated, Certificate Transparency seems to me a positive thing to activate.

I was trying to convey the same in the discussions of #995

Some technical documents in this regard:

Technically it works like this:

  • From a static json (components/certificate_transparency/data/log_list.json) at compile time it is generated a list of values containing the CTLogs (file ./out/bromite/gen/components/certificate_transparency/data/log_list-inc.cc)
  • The list of control pins can be found in transport_security_state_static.pins and generates ./out/bromite/gen/net/http/transport_security_state_static.h
  • The report urls are taken from the same list

Yes, I am aware of how this works; thanks for the recap, might be useful for others too.

I provide you with the patch I made to check the code, then I modify you https://github.com/uazo/superpatch to allow you to study it if you want without having to apply it.
EDIT: use this link
If we decide to activate the ct without report, I will prepare a pull, however the idea is to activate it with a flag because I imagine it could give some problems to users who use the proxy with https decryption.

Yes, I believe we should have this functionality and the update will happen as new Bromite releases are made. Most times there is a release within 10 weeks so this should not be a problem; we can study in future how to update this more often if needed (in case there is a big certificates revocation like it happened in the past for Symantec etc, it could be an useful feature).

As for users proxying HTTPS: they would already have to enable the user certificates flag, doesn't that cover their use case already?

@uazo
Copy link
Collaborator Author

uazo commented Dec 2, 2021

// Explicitly disallow network service features which could cause a privacy
// leak.

in my opinion it's more prefetch (== data saver) which is anti-privacy, since it runs on external servers.
maybe it is due to some behavior related to this.

Doesn't this code apply to all OSes?

which code?
i meant we should think about doing bromite for windows seriously.

So our patch about enable_reporting does not cover these?

no, for chromium they are two different things.

We have to disable them all.

if we decide to activate the CT, we will deactivate the reporting

As for users proxying HTTPS: they would already have to enable the user certificates flag, doesn't that cover their use case already?

I don't know, it must be verified, in my opinion not, as that is a hack of the code
I would start putting a CT flag toggle on/off, also because I don't have a full environment to test proxies yet

@uazo
Copy link
Collaborator Author

uazo commented Jan 22, 2022

@csagan5
Copy link
Contributor

csagan5 commented Mar 6, 2022

// Explicitly disallow network service features which could cause a privacy
// leak.

in my opinion it's more prefetch (== data saver) which is anti-privacy, since it runs on external servers.
maybe it is due to some behavior related to this.

Data saver is on its way out, finally.

Doesn't this code apply to all OSes?

which code? i meant we should think about doing bromite for windows seriously.

I think ungoogled-chromium (by @Eloston & others) with is covering that pretty well; I would rather not. But in future if/when I release Bromite build tooling we could consider extending to build ungoogled-chromium with it as well.

We have to disable them all.

if we decide to activate the CT, we will deactivate the reporting

I see it is already like this in your patch? e.g. the CT report is disabled, so CT works only in "passive" mode.

if you want, a patch is available
https://github.com/uazo/bromite/blob/master-v97/build/patches/Enable-Certificate-Transparency.patch

I will add your patch.

@uazo
Copy link
Collaborator Author

uazo commented Mar 8, 2022

fixed in 99.0.4844.55

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.

3 participants