refactor: migrate from argon2 -> @node-rs/argon2 #4733

jsjoeio · 2022-01-12T21:55:41Z

This migrates our hashing password implementation from using argon2 to @node-rs/argon2.

How to Test

Wait for CI to finish
Download release-packages from Artifacts
Unzip and then run cd into release-packages
Run tar -xzf code-server* based on your arch
Generate an argon2 password or use this one: "$argon2i$v=19$m=4096,t=3,p=1$cecR+o8tshB6mKzPBUG7fw$w2XFt+Ezn+CjRGTwptsjH3yj72htGLhMc3WdPs7wIOk" (equals "password")
edit your config.yaml and add hashed-password: "$argon2i$v=19$m=4096,t=3,p=1$cecR+o8tshB6mKzPBUG7fw$w2XFt+Ezn+CjRGTwptsjH3yj72htGLhMc3WdPs7wIOk"
Run ./code-server*/bin/code-server
Login with password: "password"
Everything should work

Tested on

macos @jsjoeio
linux @jsjoeio
Termux @jsjoeio
Raspberry Pi @im-coder-lg

Notes

At first, I ran into some issues with tests because I was overriding process.platform and that caused issues with any test files that required @node-rs/argon2. @code-asher had a good idea to fix this: refactor getEnvPaths() to pass in process.platform. That fixed it.

Resources

src/node/util.ts

ci/build/build-standalone-release.sh

ci/build/npm-postinstall.sh

src/node/util.ts

codecov · 2022-01-12T23:03:18Z

Codecov Report

Merging #4733 (c02b98a) into main (2752d95) will decrease coverage by 0.04%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #4733      +/-   ##
==========================================
- Coverage   69.23%   69.18%   -0.05%     
==========================================
  Files          29       29              
  Lines        1638     1652      +14     
  Branches      341      363      +22     
==========================================
+ Hits         1134     1143       +9     
- Misses        428      432       +4     
- Partials       76       77       +1

Impacted Files	Coverage Δ
src/node/util.ts	`81.28% <100.00%> (+0.10%)`	⬆️
src/node/main.ts	`50.00% <0.00%> (-0.54%)`	⬇️
src/node/entry.ts	`0.00% <0.00%> (ø)`
src/common/util.ts	`100.00% <0.00%> (ø)`
src/node/wrapper.ts	`0.00% <0.00%> (ø)`
src/node/cli.ts	`83.58% <0.00%> (+0.06%)`	⬆️
src/node/app.ts	`98.07% <0.00%> (+0.07%)`	⬆️
src/node/routes/index.ts	`81.25% <0.00%> (+0.19%)`	⬆️
src/node/routes/errors.ts	`83.33% <0.00%> (+0.57%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2752d95...c02b98a. Read the comment docs.

src/node/util.ts

test/unit/node/util.test.ts

github-actions · 2022-01-13T00:43:20Z

✨ Coder.com for PR #4733 deployed! It will be updated on every commit.

Host: https://codercom-41c2zfdl1-codercom.vercel.app/docs/code-server
Last deploy status: success
Commit: c02b98a
Workflow status: https://github.com/coder/code-server/actions/runs/1714050309

im-coder-lg · 2022-01-13T02:34:01Z

@yisibl any ideas why the artifact failed?

yisibl · 2022-01-13T02:36:05Z

@yisibl any ideas why the artifact failed?

What is the actual glibc version in Docker?

Brooooooklyn · 2022-01-13T06:29:01Z

@yisibl any ideas why the artifact failed?

@node-rs/argon2 doesn't support GLIBC_2.17 for now. I'm implementing the support in napi-rs/napi-rs#1025

And I'll draft a patch release after NAPI-RS toolchain supported the lower GLIBC on the Linux platform.

yisibl · 2022-01-13T07:02:09Z

Yeah, I see GLIBC_2.17, let's wait for the next version.

im-coder-lg · 2022-01-13T11:05:19Z

Actually, idk if I could test on Termux, since whenever I tried it it didn't work. But I will try 3.9.3 and try learning how to do that.

im-coder-lg · 2022-01-13T11:09:42Z

Uh oh... My Termux isn't working. From the beginning, I haven't able to get the mirrors updated, there isn't a sources.list file in my /etc dir. Idk what happened to my phone, maybe it's because I have an older phone.

im-coder-lg · 2022-01-13T11:25:05Z

Wait we are supposed to use @napi-rs/argon2 right? Why's it node-rs?

Brooooooklyn · 2022-01-13T14:42:21Z

Upgrade to @node-rs/argon2@1.0.5 to solve the GLIBC problem with centos:7

jsjoeio · 2022-01-13T17:06:32Z

@Brooooooklyn will do - thank you for the quick patch!

@yisibl - thank you for the review and feedback!

@im-coder-lg no worries, I will test on Termux

jsjoeio · 2022-01-13T17:30:22Z

This should fix the audit/code scanning CI failing (I'll rebase once merged): #4742

im-coder-lg · 2022-01-14T04:24:42Z

Testing this now!

jsjoeio · 2022-01-14T16:40:45Z

the total size of the build artifacts are nearing 2 GB. How are you all managing it?

😅 I wish I had a good answer for that...

jsjoeio · 2022-01-14T17:53:04Z

@code-asher brought up a good point. our e2e tests run on Linux so those are passing which means this is good on Linux.

jsjoeio · 2022-01-14T19:20:31Z

@im-coder-lg any updates?

jsjoeio · 2022-01-14T19:21:45Z

Termux

Previously, installing the project would fail on Termux due to argon2 issues. From what I can tell, it seems as though those problems are resolved with this PR!

I couldn't successfully downgrade Node to v14 on Termux so code-server wouldn't actually run. So assuming someone can downgrade, it should work.

im-coder-lg · 2022-01-14T20:22:38Z

I'll just test again. Since I didn't install Node 14 before running, I got into errors. I will try again and hopefully succeed.

jsjoeio · 2022-01-14T20:30:04Z

I'll just test again. Since I didn't install Node 14 before running, I got into errors. I will try again and hopefully succeed.

No worries at all. What I did was upload the npm-package to Google Drive, then download directly on my device, unzip/untar and then yarn in that (yarn install --production) and was able to get it to work.

jsjoeio · 2022-01-14T21:42:06Z

Here's a link to the npm-package.zip if you want to download: https://drive.google.com/file/d/1VyB0i4GMf9yAo8n_DU-YkfPGRKnzb9Nf/view?usp=sharing

im-coder-lg · 2022-01-15T05:24:04Z

Heads up: I'll be testing two architectures, the first one is AARCH64 aka. ARM64 and then ARMV7L which is integrated to Raspberry Pi OS Bullseye.

im-coder-lg · 2022-01-15T06:05:39Z

LOLZ, all I needed to test was the package.tar.gz tarball? I couldn't unzip the ZIP file, maybe it's due to that.

im-coder-lg · 2022-01-15T06:21:47Z

This absolutely works. I am testing this once again with the hashed password and maybe again on armv7l.

im-coder-lg · 2022-01-15T06:44:09Z

I can say this works successfully, although 4.0.1 is still in development, some simple errors pop up.
Logged in, opened folders, tested the typing, everything works.
Maybe now I'll try a simple webserver. Docusaurus v2?

im-coder-lg

We can merge! LGTM!

package.json

test/package.json

jsjoeio · 2022-01-18T17:34:10Z

@im-coder-lg thanks so much for helping test!!! I will merge this once CI passes

This reverts commit 723469a.

This reverts part of the changes introduced in refactor: migrate from argon2 -> @node-rs/argon2 (#4733) Switching to @node-rs/argon2 introduced bugs that we couldn't solve due to limitations in npm. see here #4804 (comment)

* revert: partial revert of 723469a This reverts part of the changes introduced in refactor: migrate from argon2 -> @node-rs/argon2 (#4733) Switching to @node-rs/argon2 introduced bugs that we couldn't solve due to limitations in npm. see here #4804 (comment)

* chore(deps): replace argon2 w/@node-rs/argon2 * refactor: clean up hashPassword functions * refactor(util): pass in process.platform * fix: use correct settings for test-extension Before, it was running into errors with an @types package. Now, we're correctly running `tsc` so it picks up our `tsconfig.json` and we're telling TypeScript to not typecheck our lib and exclude `node_modules`

* revert: partial revert of 723469a This reverts part of the changes introduced in refactor: migrate from argon2 -> @node-rs/argon2 (coder#4733) Switching to @node-rs/argon2 introduced bugs that we couldn't solve due to limitations in npm. see here coder#4804 (comment)

jsjoeio self-assigned this Jan 12, 2022

jsjoeio had a problem deploying to CI January 12, 2022 21:55 Failure

jsjoeio had a problem deploying to CI January 12, 2022 22:17 Failure

jsjoeio force-pushed the jsjoeio-argon-change branch from e52eb94 to d796af5 Compare January 12, 2022 22:41

jsjoeio had a problem deploying to CI January 12, 2022 22:41 Failure

jsjoeio commented Jan 12, 2022

View reviewed changes

src/node/util.ts Show resolved Hide resolved

ci/build/build-standalone-release.sh Outdated Show resolved Hide resolved

ci/build/npm-postinstall.sh Outdated Show resolved Hide resolved

src/node/util.ts Show resolved Hide resolved

jsjoeio force-pushed the jsjoeio-argon-change branch from d796af5 to 698c46f Compare January 12, 2022 22:53

jsjoeio had a problem deploying to CI January 12, 2022 22:53 Failure

jsjoeio marked this pull request as ready for review January 12, 2022 22:54

jsjoeio requested a review from a team January 12, 2022 22:54

jsjoeio mentioned this pull request Jan 12, 2022

Investigate argon2 issue (Termux/Raspberry Pi) #4422

Closed

code-asher reviewed Jan 12, 2022

View reviewed changes

src/node/util.ts Outdated Show resolved Hide resolved

test/unit/node/util.test.ts Outdated Show resolved Hide resolved

jsjoeio force-pushed the jsjoeio-argon-change branch from 698c46f to e888281 Compare January 12, 2022 23:33

jsjoeio had a problem deploying to CI January 12, 2022 23:33 Failure

jsjoeio force-pushed the jsjoeio-argon-change branch from e888281 to 02944b3 Compare January 13, 2022 17:27

jsjoeio had a problem deploying to CI January 13, 2022 17:27 Failure

jsjoeio had a problem deploying to CI January 13, 2022 18:07 Failure

chore(deps): replace argon2 w/@node-rs/argon2

029f8f9

jsjoeio requested a review from code-asher January 14, 2022 19:26

code-asher approved these changes Jan 14, 2022

View reviewed changes

im-coder-lg approved these changes Jan 17, 2022

View reviewed changes

package.json Outdated Show resolved Hide resolved

test/package.json Outdated Show resolved Hide resolved

Merge branch 'main' into jsjoeio-argon-change

c02b98a

jsjoeio temporarily deployed to CI January 18, 2022 17:34 Inactive

jsjoeio merged commit 723469a into main Jan 18, 2022

jsjoeio deleted the jsjoeio-argon-change branch January 18, 2022 23:13

benz0li mentioned this pull request Jan 28, 2022

[Bug]: Cannot find module '@node-rs/argon2-linux-arm64-gnu' #4804

Closed

3 tasks

jsjoeio mentioned this pull request Feb 1, 2022

feat: add test for hash with empty string #4814

Merged

jsjoeio added a commit that referenced this pull request Feb 4, 2022

Revert "refactor: migrate from argon2 -> @node-rs/argon2 (#4733)"

3c636bd

This reverts commit 723469a.

jsjoeio added a commit that referenced this pull request Feb 4, 2022

revert: "refactor: migrate from argon2 -> @node-rs/argon2 (#4733)"

5a827e7

This reverts commit 723469a.

jsjoeio mentioned this pull request Feb 4, 2022

revert: @node-rs/argon2 -> node-argon2 #4829

Merged

im-coder-lg mentioned this pull request Aug 9, 2022

[Bug]: Argon2 Again #5430

Closed

4 tasks

refactor: migrate from argon2 -> @node-rs/argon2 #4733

refactor: migrate from argon2 -> @node-rs/argon2 #4733

Uh oh!

Conversation

jsjoeio commented Jan 12, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

How to Test

Tested on

Notes

Resources

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov bot commented Jan 12, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Jan 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

im-coder-lg commented Jan 13, 2022

Uh oh!

yisibl commented Jan 13, 2022

Uh oh!

Brooooooklyn commented Jan 13, 2022

Uh oh!

yisibl commented Jan 13, 2022

Uh oh!

im-coder-lg commented Jan 13, 2022

Uh oh!

im-coder-lg commented Jan 13, 2022

Uh oh!

im-coder-lg commented Jan 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Brooooooklyn commented Jan 13, 2022

Uh oh!

jsjoeio commented Jan 13, 2022

Uh oh!

jsjoeio commented Jan 13, 2022

Uh oh!

im-coder-lg commented Jan 14, 2022

Uh oh!

jsjoeio commented Jan 14, 2022

Uh oh!

jsjoeio commented Jan 14, 2022

Uh oh!

jsjoeio commented Jan 14, 2022

Uh oh!

jsjoeio commented Jan 14, 2022

Termux

Uh oh!

im-coder-lg commented Jan 14, 2022

Uh oh!

jsjoeio commented Jan 14, 2022

Uh oh!

jsjoeio commented Jan 14, 2022

Uh oh!

im-coder-lg commented Jan 15, 2022

Uh oh!

im-coder-lg commented Jan 15, 2022

Uh oh!

im-coder-lg commented Jan 15, 2022

Uh oh!

im-coder-lg commented Jan 15, 2022

Uh oh!

im-coder-lg left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

jsjoeio commented Jan 18, 2022

Uh oh!

Reviewers

Assignees

Labels

jsjoeio commented Jan 12, 2022 •

edited

Loading

codecov bot commented Jan 12, 2022 •

edited

Loading

github-actions bot commented Jan 13, 2022 •

edited

Loading

im-coder-lg commented Jan 13, 2022 •

edited

Loading