Skip to content

bitwarden-desktop: fix browser biometrics#374068

Closed
nyawox wants to merge 2 commits intoNixOS:masterfrom
nyawox:bitwarden-desktop-proxy
Closed

bitwarden-desktop: fix browser biometrics#374068
nyawox wants to merge 2 commits intoNixOS:masterfrom
nyawox:bitwarden-desktop-proxy

Conversation

@nyawox
Copy link
Contributor

@nyawox nyawox commented Jan 15, 2025

Add desktop_proxy to $out/bin
and expose com.8bit.bitwarden.json to $out/lib/mozilla/native-messaging-hosts/com.8bit.bitwarden.json so the user can symlink to their home directory (in the same way as keepassxc)
Should fix #347350
note that if you have firefox sandboxed, you must allow access to ~/.cache/com.bitwarden.desktop/app.bitwarden socket

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/)
  • 25.05 Release Notes (or backporting 24.11 and 25.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.

@nyawox nyawox requested a review from amarshall January 15, 2025 14:01
@github-actions github-actions bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Jan 15, 2025
@Vcele
Copy link

Vcele commented Jan 30, 2025

+1 would be great if this could get merged soon.

Comment on lines +161 to +162
mkdir -p $out/bin
cp -r apps/desktop/desktop_native/target/release/desktop_proxy $out/bin
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
mkdir -p $out/bin
cp -r apps/desktop/desktop_native/target/release/desktop_proxy $out/bin
install -Dm755 -t $out/bin apps/desktop/desktop_native/target/release/desktop_proxy

{
"name": "com.8bit.bitwarden",
"description": "Bitwarden desktop <-> browser bridge",
"path": "@out@/bin/desktop_proxy",
Copy link
Member

Choose a reason for hiding this comment

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

If it is only consumed by firefox in this way, we might put it in paths like $out/libexec/bitwarden to avoid messing up with PATH.

Copy link
Contributor

Choose a reason for hiding this comment

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

When you run current Bitwarden Desktop app in NixOS 24.11 and click on the option to enable browser fingerprint auth, you'll get this error log:

07:38:16.900 › Error generating manifests: Error: Unable to find binary: /nix/store/sbas438bk37a1hm2v46s9kw2i70g0ncm-bitwarden-desktop-2025.1.1/bin/desktop_proxy
07:38:16.909 › Error in browser integration: Error: Unable to find binary: /nix/store/sbas438bk37a1hm2v46s9kw2i70g0ncm-bitwarden-desktop-2025.1.1/bin/desktop_proxy

So, I'd prefer to move it to libexec as you noted. However in that case, somehow we have to tell the app that there's where it'll be. Do you know how to do it?

Copy link
Contributor

Choose a reason for hiding this comment

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

You would have to patch the source code here to return the absolute path

@Aleksanaa
Copy link
Member

Besides these, I hope someone else can test it.

@Plamper
Copy link
Contributor

Plamper commented Mar 5, 2025

I can confirm the native messaging works with this PR. I was able unlock the browser extension using fingerprint. I was however unable to unlock the desktop app using fingerprint, but I think this is unrelated to this PR.

@dshatz
Copy link
Contributor

dshatz commented Mar 11, 2025

I have also tested this and fingerprint unlock is working.

Instructions on how to test for those who use flake.nix.

flake.nix:

{
  inputs = {
    nixpkgs-bitwarden.url = "github:nixos/nixpkgs/6fd0382c80e18f0883709fb5a6bf9f40bc671a62";
    ...
  }
}

configuration.nix:

system.environmentPackages = [
  inputs.nixpkgs-bitwarden.legacyPackages.x86_64-linux.bitwarden-desktop
];

@wegank wegank added the 2.status: merge conflict This PR has merge conflicts with the target branch label Mar 16, 2025
@benley
Copy link
Member

benley commented Mar 17, 2025

Is this blocked on anything, or are you just waiting for a reviewer?

Copy link
Member

@amarshall amarshall left a comment

Choose a reason for hiding this comment

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

Previous review comments should also be addressed. I haven’t tested this, as I don’t use this functionality.

popd

pushd apps/desktop/desktop_native/proxy
cargo build --bin desktop_proxy --release
Copy link
Member

Choose a reason for hiding this comment

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

As far as I can tell there is only one other pkg in nixpkgs that calls cargo build directly like this. We probably at least want to take a few things from cargoBuildHook if it can’t be used. E.g. -j, --offline, and perhaps --target (though this drv is likely broken for cross-compilation, it would be nice to at least not make it worse).

@marie-bnl
Copy link
Contributor

Following dshatz's instructions, it yields the following error (on both commits).

warning: updating lock file '/dotfiles/flake.lock':
• Updated input 'nixpkgs-bitwarden':
    'github:nixos/nixpkgs/6fd0382c80e18f0883709fb5a6bf9f40bc671a62?narHash=sha256-HENAJJESMpPradYjGGgEaRH3gLyB4CeRtjPLe/8bzzA%3D' (2025-01-15)
  → 'github:nixos/nixpkgs/f7bae0c98eefa6cfedfe1f32d0439aaacecd2246?narHash=sha256-QFybfMyUTuCOXmXZI%2BUD3EvBIfJjM8hG9IGXcf9Q9Yg%3D' (2025-01-15)
error: builder for '/nix/store/xbw05pfxb4826daajmv9vhd2gkawgf0f-bitwarden-desktop-2024.12.0.drv' failed with exit code 1;
       last 25 log lines:
       > [Rend] [BABEL] Note: The code generator has deoptimised the styling of /build/source/node_modules/zxcvbn/lib/frequency_lists.js as it exceeds the max of 500KB.
       > [Rend] npm error Lifecycle script `build:renderer` failed with error:
       > [Rend] npm error code 1
       > [Rend] npm error path /build/source/apps/desktop
       > [Rend] npm error workspace @bitwarden/desktop@2024.12.0
       > [Rend] npm error location /build/source/apps/desktop
       > [Rend] npm error command failed
       > [Rend] npm error command sh -c cross-env NODE_ENV=production webpack --config webpack.renderer.js
       > [Rend] npm run build:renderer exited with code 1
       > npm error Lifecycle script `build` failed with error:
       > npm error code 1
       > npm error path /build/source/apps/desktop
       > npm error workspace @bitwarden/desktop@2024.12.0
       > npm error location /build/source/apps/desktop
       > npm error command failed
       > npm error command sh -c concurrently -n Main,Rend,Prel -c yellow,cyan "npm run build:main" "npm run build:renderer" "npm run build:preload"
       >
       > ERROR: `npm build` failed
       >
       > Here are a few things you can try, depending on the error:
       > 1. Make sure your build script (build) exists
       >   If there is none, set `dontNpmBuild = true`.
       > 2. If the error being thrown is something similar to "error:0308010C:digital envelope routines::unsupported", add `NODE_OPTIONS = "--openssl-legacy-provider"` to your derivation
       >   See https://github.com/webpack/webpack/issues/14532 for more information.
       >
       For full logs, run 'nix log /nix/store/xbw05pfxb4826daajmv9vhd2gkawgf0f-bitwarden-desktop-2024.12.0.drv'.
error: 1 dependencies of derivation '/nix/store/sv8yr24rkzsxznzxwqkkfcldisqjiq1x-home-manager-path.drv' failed to build
error: 1 dependencies of derivation '/nix/store/j08mnz72w9r2s0a0mzyjrydhhapznk6p-mozilla-native-messaging-hosts.drv' failed to build
error: 1 dependencies of derivation '/nix/store/43qrmvnj29ippjzbhzhba994kc28r40a-home-manager-generation.drv' failed to build

Copy link
Contributor

@yajo yajo left a comment

Choose a reason for hiding this comment

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

Please could you fix merge conflicts?

{
"name": "com.8bit.bitwarden",
"description": "Bitwarden desktop <-> browser bridge",
"path": "@out@/bin/desktop_proxy",
Copy link
Contributor

Choose a reason for hiding this comment

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

When you run current Bitwarden Desktop app in NixOS 24.11 and click on the option to enable browser fingerprint auth, you'll get this error log:

07:38:16.900 › Error generating manifests: Error: Unable to find binary: /nix/store/sbas438bk37a1hm2v46s9kw2i70g0ncm-bitwarden-desktop-2025.1.1/bin/desktop_proxy
07:38:16.909 › Error in browser integration: Error: Unable to find binary: /nix/store/sbas438bk37a1hm2v46s9kw2i70g0ncm-bitwarden-desktop-2025.1.1/bin/desktop_proxy

So, I'd prefer to move it to libexec as you noted. However in that case, somehow we have to tell the app that there's where it'll be. Do you know how to do it?

cp -r apps/desktop/desktop_native/target/release/desktop_proxy $out/bin

mkdir -p $out/lib/mozilla/native-messaging-hosts
substituteAll ${./firefox-native-messaging-host.json} $out/lib/mozilla/native-messaging-hosts/com.8bit.bitwarden.json
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion: Please could you also add native messaging host support for Chromium-based browsers?

Suggested change
substituteAll ${./firefox-native-messaging-host.json} $out/lib/mozilla/native-messaging-hosts/com.8bit.bitwarden.json
substituteAll ${./chromium-messaging-host.json} $out/etc/chromium/native-messaging-hosts/com.8bit.bitwarden.json

The file ./chromium-messaging-host.json should be something like this:

{
  "name": "com.8bit.bitwarden",
  "description": "Bitwarden desktop <-> browser bridge",
  "path": "@out@/bin/desktop_proxy",
  "type": "stdio",
  "allowed_origins": [
    "chrome-extension://nngceckbapebfimnlniiiahkandclblb/",
    "chrome-extension://hccnnhgbibccigepcmlgppchkpfdophk/",
    "chrome-extension://jbkfoedolllekgbhcbcoahefnbanhhlh/",
    "chrome-extension://ccnckbpmaceehanjmeomladnmlffdjgn/"
  ]
}

Why do this? It would enable home-manager users to add this package to programs.brave.nativeMessagingHosts (or any other supported browser. In contrast, the equivalent files created by the bitwarden desktop app when ticking the host auth and browser integration boxes are only for a limited set of browsers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2.status: merge conflict This PR has merge conflicts with the target branch 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bitwarden-desktop: work with firefox.nativeMessagingHosts