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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

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

yisibl commented Jan 13, 2022

Brooooooklyn commented Jan 13, 2022

yisibl commented Jan 13, 2022

im-coder-lg commented Jan 13, 2022

im-coder-lg commented Jan 13, 2022

im-coder-lg commented Jan 13, 2022 •

edited

Loading

Brooooooklyn commented Jan 13, 2022

jsjoeio commented Jan 13, 2022

jsjoeio commented Jan 13, 2022

im-coder-lg commented Jan 14, 2022

jsjoeio commented Jan 14, 2022

jsjoeio commented Jan 14, 2022

jsjoeio commented Jan 14, 2022

jsjoeio commented Jan 14, 2022

im-coder-lg commented Jan 14, 2022

jsjoeio commented Jan 14, 2022

jsjoeio commented Jan 14, 2022

im-coder-lg commented Jan 15, 2022

im-coder-lg commented Jan 15, 2022

im-coder-lg commented Jan 15, 2022

im-coder-lg commented Jan 15, 2022

im-coder-lg left a comment

jsjoeio commented Jan 18, 2022

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

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

Conversation

jsjoeio commented Jan 12, 2022 • edited Loading

How to Test

Tested on

Notes

Resources

codecov bot commented Jan 12, 2022 • edited Loading

Codecov Report

github-actions bot commented Jan 13, 2022 • edited Loading

im-coder-lg commented Jan 13, 2022

yisibl commented Jan 13, 2022

Brooooooklyn commented Jan 13, 2022

yisibl commented Jan 13, 2022

im-coder-lg commented Jan 13, 2022

im-coder-lg commented Jan 13, 2022

im-coder-lg commented Jan 13, 2022 • edited Loading

Brooooooklyn commented Jan 13, 2022

jsjoeio commented Jan 13, 2022

jsjoeio commented Jan 13, 2022

im-coder-lg commented Jan 14, 2022

jsjoeio commented Jan 14, 2022

jsjoeio commented Jan 14, 2022

jsjoeio commented Jan 14, 2022

jsjoeio commented Jan 14, 2022

Termux

im-coder-lg commented Jan 14, 2022

jsjoeio commented Jan 14, 2022

jsjoeio commented Jan 14, 2022

im-coder-lg commented Jan 15, 2022

im-coder-lg commented Jan 15, 2022

im-coder-lg commented Jan 15, 2022

im-coder-lg commented Jan 15, 2022

im-coder-lg left a comment

Choose a reason for hiding this comment

jsjoeio commented Jan 18, 2022

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