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

Secure Preferences is not really secure #76

Open
uazo opened this issue May 12, 2023 · 5 comments
Open

Secure Preferences is not really secure #76

uazo opened this issue May 12, 2023 · 5 comments
Labels
enhancement New feature or request It would be nice to have it It would be nice to have it, in my opinion working on it working on it

Comments

@uazo
Copy link
Owner

uazo commented May 12, 2023

currently the use of secure preferences is not the same as that used in chrome, since there is no seed in chromium.

#if BUILDFLAG(GOOGLE_CHROME_BRANDING)
  seed = std::string(ui::ResourceBundle::GetSharedInstance().GetRawDataResource(
      IDR_PREF_HASH_SEED_BIN));
#endif

https://source.chromium.org/chromium/chromium/src/+/refs/heads/main:chrome/browser/prefs/chrome_pref_service_factory.cc;l=29

adding a seed in an open source code, however, would not serve security purposes. by the way, the method for extracting that value from the pak is already public.
In addition, there are few preferences put under control, see kTrackedPrefs list and Local State doesn't really seem to be under control.

so technically any application could modify the contents of those files

@uazo
Copy link
Owner Author

uazo commented May 14, 2023

so, no WinRT api available but I found this Issue 1333461: add application bound encryption primitives for chrome unfortunately it is a wip.

quite strange, in chrome there is no elevation service, but the value os_crypt.app_bound_fixed_data exists, whereas in brave there is the service but not the exe file!

https://source.chromium.org/chromium/chromium/src/+/main:chrome/elevation_service/elevator.cc

In any case, the idea could be to exploit that method by digitally signing the executables and verifying the signature with the public key. I am a little disturbed by the use of hell com objects.

The aim is to protect config from other applications on the machine. Whereas to protect executables I should use the msi installer.
I also saw that there is a new flag blocking the reading of cookie file during use, which should be activated if you enable data deletion on shutdown.

I should also start adding at least the proxy configuration to the secure prefs.

@uazo
Copy link
Owner Author

uazo commented Jun 2, 2023

the idea is to use the seed

  • not make it public but integrated into the build process (how?)
  • don't put it in resources but inline in the code

the disadvantage is that I will lose all the settings
see also message_encrypter.cc

@uazo
Copy link
Owner Author

uazo commented Jul 18, 2023

not make it public but integrated into the build process

I found a way, but it breaks the possibility of having a reproducible build.
On the other hand, I will be able to make public the patch that activates cromite because the idea is to create a site that can verify that the browser installed is the one generated by this repo.

the disadvantage is that I will lose all the settings

no, it does not.
meanwhile, in android it is not active. in windows it presents a message that allows the user to reset everything but it is not mandatory.
this is fine for now, considering that the change will impact current installations.

@uazo uazo transferred this issue from uazo/bromite-buildtools Jul 21, 2023
@uazo uazo added the enhancement New feature or request label Jul 21, 2023
@uazo uazo added the It would be nice to have it It would be nice to have it, in my opinion label Oct 21, 2023
@uazo uazo added the working on it working on it label Dec 24, 2023
@AJolly
Copy link

AJolly commented Feb 21, 2024

Subscribing incase this changes, it makes it harder to clone/backup/move user profiles.

@uazo
Copy link
Owner Author

uazo commented Jun 12, 2024

fix position of cromite_pref_hash_seed_bin

that parameter should only be active for desktop platforms, erroneously it is only active for android (which does not use it).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request It would be nice to have it It would be nice to have it, in my opinion working on it working on it
Projects
None yet
Development

No branches or pull requests

2 participants