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

librewolf: remove with-app-basename flag that exposes LibreWolf in the user agent string when resistFingerprinting is disabled #321083

Merged
merged 1 commit into from
Jul 18, 2024

Conversation

cinnamonpancake
Copy link
Contributor

Description of changes

Due to a quirk in Firefox itself, the --with-app-basename flag causes its argument to replace Firefox in the user agent string (if resistFingerprinting is disabled, as it cloaks the UA string) which hurts privacy by making users much easier to fingerprint, as well as hurting website compatibility (even preventing users from being able to install extensions from the Mozilla add-ons store, as it thinks the user is not on a Firefox-based browser, as mentioned in #260488)

The general.useragent.compatMode.firefox setting can be used to add Firefox to the user agent string, however it is still disabled by default, and it also doesn't remove LibreWolf from the string, which still severely hurts privacy.

Removing the flag doesn't seem to have any other significant effect, and it is no longer included in upstream LibreWolf for similar reasons. https://codeberg.org/librewolf/source/src/commit/d766054b586a28b4ddb5ec69d344886397da1137/assets/mozconfig.new

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.11 Release Notes (or backporting 23.11 and 24.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@superherointj superherointj changed the title Remove redundant build flag in LibreWolf that causes "Firefox" to be replaced with "LibreWolf" in the user agent string if resistFingerprinting is disabled, hurting privacy and web compatibility librewolf: remove redundant build flag in LibreWolf that causes "Firefox" to be replaced with "LibreWolf" in the user agent string if resistFingerprinting is disabled, hurting privacy and web compatibility Jun 19, 2024
@superherointj superherointj changed the title librewolf: remove redundant build flag in LibreWolf that causes "Firefox" to be replaced with "LibreWolf" in the user agent string if resistFingerprinting is disabled, hurting privacy and web compatibility librewolf: remove redundant build flag in LibreWolf that causes firefox to be replaced with LibreWolf in the user agent string if resistFingerprinting is disabled Jun 19, 2024
@superherointj superherointj changed the title librewolf: remove redundant build flag in LibreWolf that causes firefox to be replaced with LibreWolf in the user agent string if resistFingerprinting is disabled librewolf: remove with-app-basename flag that causes firefox to be replaced with LibreWolf in the user agent string if resistFingerprinting is disabled Jun 19, 2024
@superherointj superherointj changed the title librewolf: remove with-app-basename flag that causes firefox to be replaced with LibreWolf in the user agent string if resistFingerprinting is disabled librewolf: remove with-app-basename flag that exposes LibreWolf in the user agent string if resistFingerprinting is disabled Jun 19, 2024
@superherointj
Copy link
Contributor

superherointj commented Jun 19, 2024

Please fix commit title.

It must start with package name. Then, :. Only then, the description.

librewolf: remove with-app-basename flag that exposes LibreWolf in the user agent string when resistFingerprinting is disabled

You can add additional description to the body of commit message. (But not the title.)

@superherointj superherointj changed the title librewolf: remove with-app-basename flag that exposes LibreWolf in the user agent string if resistFingerprinting is disabled librewolf: remove with-app-basename flag that exposes LibreWolf in the user agent string when resistFingerprinting is disabled Jun 19, 2024
@cinnamonpancake
Copy link
Contributor Author

ah, i apologize

@superherointj
Copy link
Contributor

ah, i apologize

Commit message is still not right...

@cinnamonpancake
Copy link
Contributor Author

sorry, i had to go do some stuff
is it alright now?

@superherointj
Copy link
Contributor

is it alright now?

No. Use:

package-name: short description

(replace both)

@cinnamonpancake
Copy link
Contributor Author

i apologize for all of that, i'm still kinda new to all this
should be good now

@superherointj
Copy link
Contributor

superherointj commented Jun 20, 2024

i apologize for all of that, i'm still kinda new to all this

Read the docs. There are contribution guidelines. And they are helpful.
In case you don't, you'll be wasting reviewers time reminding you of basic things.
Which only detracts from what you are trying to accomplish.

should be good now

Then, you should add as reviewers the meta.maintainers of package. (ping if you can't)

https://github.com/NixOS/nixpkgs/blob/dc018d967b98c6b558e0c815015af949e722e2bf/pkgs/applications/networking/browsers/librewolf/default.nix#L20C43-L20C60

(I just added them as reviewers for you.)


On commit message, I still think that commit message is far too long. And it's not fully in lowercase.

I'd suggest removal of:

hurting privacy and web compatibility

But still, as it is now, it is better still.


I won't be reviewing your PR further because I'm not a maintainer of this package.
And I don't have opinions on matter.

…e user agent string when resistFingerprinting is disabled
@cinnamonpancake
Copy link
Contributor Author

thanks for the feedback, will try to read more into the docs next time

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review/3032/4266

@superherointj
Copy link
Contributor

@cinnamonpancake I suggest you to use git history for the files you are touching and find new reviewers.

Copy link
Member

@squalus squalus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the mozconfig that upstream actually uses to build: https://codeberg.org/librewolf/source/src/branch/main/assets/mozconfig.new. The app-basename option is not present there. So I agree that the app-basename option should not be present in the Nix build.

  • Tested a build without the app-basename flag on NixOS x86_64-linux
  • Verified that setting app-basename produces the undesirable User-Agent behavior when resistFingerprinting is off
  • Verified that removing app-basename fixes the undesirable User-Agent behavior when resistFingerprinting is off

The gentoo build has the same problem: https://codeberg.org/librewolf/gentoo/src/commit/c3d6a2e1eb59f5651b54782ef259b47b8b2c4847/www-client/librewolf/librewolf-128.0_p2.ebuild#L835

The arch build does not have the problem: https://codeberg.org/librewolf/arch/src/commit/88e990dbefccb277b40cd722962adcd92baca447/PKGBUILD#L129

Related:

Thanks for the contribution and sorry for the review delay.

@squalus
Copy link
Member

squalus commented Jul 18, 2024

Fixes #260488

@superherointj superherointj merged commit 7dccae1 into NixOS:master Jul 18, 2024
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants