Switch to rustcrypto argon2 on desktop

[quexten]

🎟️ Tracking

https://bitwarden.atlassian.net/browse/PM-14185
flathub/com.bitwarden.desktop#222

📔 Objective

Cross-compiling and building desktop in some environments (arm cross compile; flatpak build from source) is made difficult by the native CPP dependency. This PR switches the desktop argon2 implementation to use rustcrypto's argon2 crate (same as the SDK) via desktop_native. Note: The crate currently implements parallelism sequentially, so this change does have a performance impact. In a quick measurement, for the default settings (with parallelism = 4), unlock was 33% as fast as the npm module. However, given that other clients use the same rust implementation via the sdk, this seems like a reasonable trade-off.

📸 Screenshots

⏰ Reminders before review

• Contributor guidelines followed
• All formatters and local linters executed and passed
• Written new unit and / or integration tests where applicable
• Protected functional changes with optionality (feature flags)
• Used internationalization (i18n) for all UI strings
• CI builds passed
• Communicated to DevOps any deployment requirements
• Updated any necessary documentation (Confluence, contributing docs) or informed the documentation team

🦮 Reviewer guidelines

• 👍 (:+1:) or similar for great changes
• 📝 (:memo:) or ℹ️ (:information_source:) for notes or general info
• ❓ (:question:) for questions
• 🤔 (:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion
• 🎨 (:art:) for suggestions / improvements
• ❌ (:x:) or ⚠️ (:warning:) for more significant problems or concerns needing attention
• 🌱 (:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt
• ⛏ (:pick:) for minor or nitpick changes

[dani-garcia]

Changes LGTM, but we should also remove these lines in the electron builder config:

clients/apps/desktop/electron-builder.json

Lines 23 to 26 in e39ab59

	"!**/node_modules/argon2/**/*",
	"**/node_modules/argon2/argon2.cjs",
	"**/node_modules/argon2/package.json",
	"**/node_modules/argon2/build/Release/argon2.node"

[codecov]

Codecov Report

Attention: Patch coverage is 0% with 7 lines in your changes missing coverage. Please review.

Project coverage is 33.51%. Comparing base (853db23) to head (d7db1b9).
Report is 2 commits behind head on main.

✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
...c/app/services/renderer-crypto-function.service.ts	0.00%	4 Missing ⚠️
.../src/platform/main/main-crypto-function.service.ts	0.00%	2 Missing ⚠️
.../node/src/services/node-crypto-function.service.ts	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #11753      +/-   ##
==========================================
- Coverage   33.53%   33.51%   -0.03%     
==========================================
  Files        2886     2892       +6     
  Lines       90152    90229      +77     
  Branches    17135    17128       -7     
==========================================
+ Hits        30235    30239       +4     
- Misses      57527    57599      +72     
- Partials     2390     2391       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[dani-garcia]

I think there are some changes to Cargo.lock that haven't been commited since the addition of the zeroize feature. I've tried to build this branch locally and my Cargo.lock gets updated

[quexten]

Sorry about that, fixed now.

[Thomas-Avery]

Looking good, a few minor things to check out.

[github-actions]

Checkmarx One – Scan Summary & Details – b9532056-58e4-4ec4-9409-533222805065

No New Or Fixed Issues Found

[dani-garcia]

Just noticed one webpack config that still references the old argon2, we probably want to remove it as well:

clients/apps/desktop/webpack.main.js

Line 85 in 5592d64

argon2: "commonjs2 argon2",

[coroiu]

@quexten I'm having issues with an electron update due to argon2 (#11580) which I think we should be able to bypass completely using this PR. Do you think that we could get this merged soon?

[quexten]

It was waiting for QA. Let's test this on main instead, given we have a long window until next rc cut. Enabling automerge.

@coroiu