-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Enable HPKP/expect-CT for brave domains #767
Comments
Can you describe a test case to verify this is working correctly once it is done? |
https://pinning-test.badssl.com/ should show cert error there are 2 subissues here:
(2) is arguably more important, so we do it in browser-laptop. we don't do (1) because Chrome expressed that embedders should be careful with it. i would prefer we do both in brave-core, or at least (2) |
for beta, we can just make sure the machinery is in place to enable pinning/expect-CT (which might involve just doing #1) and then actually do the pinning in the release milestone when we know the final hostnames of brave-important services. |
I am actually a bit concerned about pinning sites that chrome pins by default. If the certs or pins change for any chrome pinned domains we will have to release a hot-fix for the domains to work. |
https://pinning-test.badssl.com/ test fails. Per above comment this is not expected. I have reported a new issue #1214. |
@btlechowski pinning is only enabled for brave domains. See here: https://github.com/brave/brave-core/blob/74fb2a164cdd780a8f074597aabae3eecaf45a24/chromium_src/net/tools/transport_security_state_generator/input_file_parsers.cc#L311 We decided against enabling pinning for all domains due to web-compat reasons: #767 (comment) |
@jumde thanks for the response. I was referring to the testcase from #767 (comment). Since https://pinning-test.badssl.com/ is not a valid test case how should we test this feature? |
Test Plan
For the negative test case, I think it makes sense to spin up a dummy brave domain with SSL cert not part of the brave pins. Thoughts? cc: @diracdeltas @tomlowenthal @w0ts0n |
Verification Passed on
Verified passed with
|
@jumde good idea, please set that up |
Just created an issue for devops. cc: @w0ts0n @RyanJarv https://github.com/brave/devops/issues/313 |
From: #13 (comment)
Check:
enable_static_pins_
andenable_static_expect_ct_
innet/http/transport_security_state.cc
The text was updated successfully, but these errors were encountered: