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

Investigate argon2 issue (Termux/Raspberry Pi) #4422

Closed
jsjoeio opened this issue Nov 1, 2021 · 41 comments · Fixed by #4733
Closed

Investigate argon2 issue (Termux/Raspberry Pi) #4422

jsjoeio opened this issue Nov 1, 2021 · 41 comments · Fixed by #4733
Assignees
Milestone

Comments

@jsjoeio
Copy link
Contributor

jsjoeio commented Nov 1, 2021

We've had numerous reports of argon2 causing issues on Raspberry Pi and Linux (see here). We need to investigate argon2 (repo here) to see if it's possible to build from source on those devices.

I have a Pixel 2 with Termux so I can test there. Or we can spin up a dev environment on Coder that mimics Termux (Alpine Linux I think) or Raspberry Pi and test there.

If we can get argon2 working, then we'll be able to unblock both Termux and Pi users of code-server.

Special thanks to @im-coder-lg for all their help debugging this!

@jsjoeio jsjoeio added the needs-investigation This issue needs to be further investigated label Nov 1, 2021
@jsjoeio jsjoeio added this to the 3.12.2 - improve iPad UX milestone Nov 1, 2021
@jsjoeio jsjoeio self-assigned this Nov 1, 2021
@im-coder-lg
Copy link
Contributor

I kinda feel like Termux is a combination of FreeBSD and Debian since it includes apt and pkg package managers. But the Argon2 error is affecting the entire NPM package.
Yesterday, I was exploring my Raspberry Pi that has MATE, and an idea flashed in my mind. In one of the discussions, @Slodziak190q told to install code-server's 3.10.0 version and that worked successfully, providing that it took some time. It works now, and it included a hashed password there.

Wait a minute, let's go a minute back. What if Argon2's 0.28.0 release has the error?

I just had a fantastic idea! @jsjoeio can you tell me when did Argon2 get implemented, before 3.10.0 or after 3.10.0? If it's after that, then why can't we use the older hashing module? Because, I observed, the password was hashed in my Pi, and I changed it, but it was hashed. If Argon2 was implemented before 3.10.0, we need to use an older version of Argon2, that might work, right?

@im-coder-lg
Copy link
Contributor

Can we create an x86 installer for code-server?

@jsjoeio
Copy link
Contributor Author

jsjoeio commented Nov 2, 2021

I just had a fantastic idea! @jsjoeio can you tell me when did Argon2 get implemented, before 3.10.0 or after 3.10.0? If it's after that, then why can't we use the older hashing module? Because, I observed, the password was hashed in my Pi, and I changed it, but it was hashed. If Argon2 was implemented before 3.10.0, we need to use an older version of Argon2, that might work, right?

Hmm...possibly! For hashing, we used sha256 which I think came from crypto which is built-in/native to Node. I added argon2 in this PR which came out in 3.11.0. That explains why 3.10.0 works.

It's supposed to be more secure because it requires more computational effort for the password hashing. I still think we need to look more into argon2 to fix this.

@jwannebo
Copy link

jwannebo commented Nov 3, 2021

I appear to have picked the worst time to try code-server out on Termux, but I may have a lead on the argon issue. I unzipped the pre-built package in my Termux install, then changed the launch script to use the system node, and got this error:

~/.../code-server-3.12.0-linux-arm64/bin $ ./code-server
node:internal/modules/cjs/loader:1183
  return process.dlopen(module, path.toNamespacedPath(filename));
                 ^

Error: dlopen failed: library "libstdc++.so.6" not found: needed by /data/data/com.termux/files/home/code/code-server-3.12.0-linux-arm64/node_modules/argon2/lib/binding/napi-v3/argon2.node in namespace (default)
    at Object.Module._extensions..node (node:internal/modules/cjs/loader:1183:18)
    at Module.load (node:internal/modules/cjs/loader:981:32)
    at Function.Module._load (node:internal/modules/cjs/loader:822:12)
    at Module.require (node:internal/modules/cjs/loader:1005:19)
    at require (node:internal/modules/cjs/helpers:102:18)
    at Object.<anonymous> (/data/data/com.termux/files/home/code/code-server-3.12.0-linux-arm64/node_modules/argon2/argon2.js:9:56)
    at Module._compile (node:internal/modules/cjs/loader:1101:14)
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1153:10)
    at Module.load (node:internal/modules/cjs/loader:981:32)
    at Function.Module._load (node:internal/modules/cjs/loader:822:12) {
  code: 'ERR_DLOPEN_FAILED'
}

I can confirm there's no libstdc++.so of any flavor in the libs folder, build-essentials omitted it, and gcc isn't on pkg to install it.

Edit: I just extracted 3.10 with the script edited in the same way, and was able to launch it ok.

@jsjoeio
Copy link
Contributor Author

jsjoeio commented Nov 3, 2021

Thanks for the notes @jwannebo! Right now we're recommending people install on Termux via yarn/npm but I know there are also Node version errors.

3.10 doesn't have issues because argon2 was adding in 3.11.0 I believe. Thanks for confirming though!

jsjoeio referenced this issue Nov 8, 2021
nodejs installs v16 which is not compatible.  nodejs-lts currently uses v14.
@im-coder-lg
Copy link
Contributor

Any updates? Should we open a ticket at Argon2 and mention this issue there?

@im-coder-lg
Copy link
Contributor

im-coder-lg commented Nov 25, 2021

I just hopped onto the Argon2 repo and discovered something. Issue 317, ranisalt/node-argon2(don't want to mention it here because it might cause problems, idk but @jsjoeio what do you say? Linking our related Argon2 problems here?
Title: node-argon2 version 0.28.2 uses a vulnerable strip-ANSI package
We use 0.28.2, right? Might be related.
Issue 278, 276 and 305 on node-argon2 seems kinda related.
278's title: Installing fails on Windows(is this related?)
276's title: Broken argon2.node on RHEL7(Red Hat Enterprise Linux, I think it's similar to Fedora, I think it uses dnf)
305: Build fails on Node 16.4 ARM M1(this seems too related except the Node version, what processor does your Mac run on @jsjoeio?)
If you want to link them, feel free, but also I think we must open a ticket at Argon2 and get help from the maintainers there.

@jsjoeio
Copy link
Contributor Author

jsjoeio commented Nov 29, 2021

@im-coder-lg I don't have any updates yet but once we get 4.0.0 out, we can prioritize this!

Thanks for linking those node-argon2 issues. We may have to switch away from node-argon2 but I don't want to make any decisions until we can investigate this further.

@jsjoeio
Copy link
Contributor Author

jsjoeio commented Jan 3, 2022

node-rs now publishes an argon2 package. Might help here: https://github.com/napi-rs/node-rs/tree/main/packages/argon2

@yisibl
Copy link

yisibl commented Jan 4, 2022

Yes, @node-rs/argon2 https://github.com/napi-rs/node-rs/tree/main/packages/argon2 relies on the power of napi-rs to solve cross-platform compatibility issues.

Feel free to try and give feedback!

@im-coder-lg
Copy link
Contributor

@jsjoeio can we test this on a separate branch? You can try building it(also can you tell hardware requirements for building code-server?) and sending a build to me over here(aarch64 please) and I'll test on Raspberry Pi with Ubuntu 21.10, what do you think? Also @yisibl can you tell us how to implement this over here?

@jsjoeio
Copy link
Contributor Author

jsjoeio commented Jan 4, 2022

Yes, @node-rs/argon2 https://github.com/napi-rs/node-rs/tree/main/packages/argon2 relies on the power of napi-rs to solve cross-platform compatibility issues.

Amazing!

Feel free to try and give feedback!

Will do! I wonder if it's as simple as replacing argon2 with @node-rs/argon2

@jsjoeio
Copy link
Contributor Author

jsjoeio commented Jan 4, 2022

@jsjoeio can we test this on a separate branch?

@im-coder-lg go for it!

You can try building it(also can you tell hardware requirements for building code-server?) and sending a build to me over here(aarch64 please) and I'll test on Raspberry Pi with Ubuntu 21.10, what do you think

I don't have a ton of bandwidth but if you give me some steps I can follow quickly, I can try building and sending to you!

@im-coder-lg
Copy link
Contributor

I don't have a ton of bandwidth but if you give me some steps I can follow quickly, I can try building and sending to you!

Wait wdym? Every build is done on a server? Any hardware requirements?
And what does WiFi have to do in building?

go for it!

I don't know how to implement this yet, all of my devices are 4-8 GB. I have a nice bandwidth but no cloud server :( but if someone with a gaming-level/business-level computer could help us, that's be cool since a gaming machine would have atleast a minimum of 16 GB and a maximum of 32/64 GB of RAM, building would work flawlessly.

@yisibl
Copy link

yisibl commented Jan 5, 2022

@jsjoeio What other packages besides node-argon2 depend on glibc?

The only requirement to use the standalone release is glibc >= 2.17 and
glibcxx >= v3.4.18 on Linux

code-server/docs/install.md

Lines 123 to 124 in 3d99998

The only requirement to use the standalone release is `glibc` >= 2.17 and
`glibcxx` >= v3.4.18 on Linux (for macOS, there is no minimum system

@yisibl
Copy link

yisibl commented Jan 5, 2022

I wonder if it's as simple as replacing argon2 with @node-rs/argon2

Yes, the migration is very simple and the API is almost identical.

const { hash, verify, Algorithm } = require('@node-rs/argon2')

export const hashPassword = async (password) => {
  try {
    return await hash(password, {
        algorithm: Algorithm.Argon2id, // Default Value
    })
  } catch (error) {
    console.log(error)
    return ''
  }
}

@im-coder-lg
Copy link
Contributor

im-coder-lg commented Jan 5, 2022

@yisibl thanks for the code! @jsjoeio let's replace and test it? Maybe on a fork than here, what's your idea?

but if you give me some steps I can follow quickly, I can try building and sending to you!

@jsjoeio posted here

Hmm... What's your computer's RAM? It has to be kinda like 8-16 GB, that'd do some good. If you have a powerhouse machine with 16+ GB of memory, you could try building it on that. Are you telling that it's hard for you to clone the repo, that's why you said 'low bandwidth'?
Maybe if that's the case, you could try using Gitpod or GitHub Codespaces, but I'd choose Gitpod since you need to pay for Codespaces. Gitpod has 6 GB of RAM(unsure but expected is 6 GB from neofetch), 50 ms meaning ultra-fast speed and it's universal, meaning if you disconnect from your computer, you just log in to the dashboard on other devices and visit the workspace again. How's this idea too?
PS: Gitpod is Ubuntu but Homebrew is added too.

@im-coder-lg
Copy link
Contributor

So, I started a Gitpod workspace and you added support for it?! That's extremely cool!

@im-coder-lg
Copy link
Contributor

Errors:

Cannot find module '/workspace/code-server/vendor/modules/code-oss-dev/out/bootstrap-node' Require stack: - /workspace/code-server/out/node/util.js - /workspace/code-server/out/node/cli.js - /workspace/code-server/out/node/entry.js

@im-coder-lg
Copy link
Contributor

It's a file error. I will have to get that code.

@im-coder-lg
Copy link
Contributor

it's not here, but if we add that code, it might work.

@jsjoeio
Copy link
Contributor Author

jsjoeio commented Jan 5, 2022

@jsjoeio What other packages besides node-argon2 depend on glibc?

I believe it's just node-argon2 🤔 I looked at the VS Code requirements and don't see glibc so must just be that.

Yes, the migration is very simple and the API is almost identical.

Easy then!

@jsjoeio
Copy link
Contributor Author

jsjoeio commented Jan 5, 2022

Hmm... What's your computer's RAM? It has to be kinda like 8-16 GB, that'd do some good. If you have a powerhouse machine with 16+ GB of memory, you could try building it on that. Are you telling that it's hard for you to clone the repo, that's why you said 'low bandwidth'?

Sorry, let me work with you on this in the next sprint or so! I have some other pressing things I need to do first, but when I start on this, I'll ping you and we can do it together :)

@im-coder-lg
Copy link
Contributor

When's it starting? If it's next week, maybe after 20th, I'll be decked up with everything for the test.

@jsjoeio
Copy link
Contributor Author

jsjoeio commented Jan 7, 2022

@im-coder-lg I'm going to put together v4.0.2 milestone and start adding issues today. If you want, I can add it to that and that allocate some bandwidth to be around if you have questions while you're testing!

@im-coder-lg
Copy link
Contributor

im-coder-lg commented Jan 8, 2022

Well, idk if I will have doubts, all I know of the tarball here is that you unzip it and execute the binary. If it works, I wil tell here. If it doesn't, I will try debugging more to get some insight on this.

@jsjoeio
Copy link
Contributor Author

jsjoeio commented Jan 10, 2022

Sounds good! So I guess then what we need to do is:

  1. open new branch
  2. replace the package with @node-rs/argon2
  3. fix any local errors
  4. when CI builds the release-packages, have you download the tarball for Raspberry Pi/Termux and test

@im-coder-lg
Copy link
Contributor

Just noticed this, when are we gonna start? I'm a teenager and my holidays start on Thursday(Wednesday for you) so you could make the branch at that time, do the development and build and hopefully, I will test this by Sat/Sun IST.

@jsjoeio
Copy link
Contributor Author

jsjoeio commented Jan 10, 2022

I'll see if I can get to this tomorrow or Wednesday which should give you enough time to help. Thank you for helping on your holidays!

@im-coder-lg
Copy link
Contributor

I'll see if I can get to this tomorrow or Wednesday which should give you enough time to help.

👍

Thank you for helping on your holidays!

No worries! I like something to keep me occupied during my free time. What'd the branch name be?

@im-coder-lg
Copy link
Contributor

im-coder-lg commented Jan 11, 2022

Also(unrelated topic coming up!), I went to see the README, it has an extra comma beside the Coder link, it seems wrong to me, can you please inspect it? Also, since the Coder GitHub org changed the name from cdr to coder, you need to change this line in docs/MAINTAINING.md.

@im-coder-lg
Copy link
Contributor

im-coder-lg commented Jan 11, 2022

Also, we need to update all the cdr links to coder. Do you mind if I do this?

@jsjoeio
Copy link
Contributor Author

jsjoeio commented Jan 12, 2022

Also, we need to update all the cdr links to coder. Do you mind if I do this?

@im-coder-lg that would be fantastic!

@jsjoeio
Copy link
Contributor Author

jsjoeio commented Jan 12, 2022

What'd the branch name be?

Let me see if I can work on this this afternoon and get back to you!

@jsjoeio
Copy link
Contributor Author

jsjoeio commented Jan 12, 2022

Alright I have a PR up. Once CI finishes, there will be a release-packages artifact that you can download and test: https://github.com/coder/code-server/actions/runs/1690093340

I'll add steps to the PR description for how to test.

@im-coder-lg
Copy link
Contributor

alr is the artifact ready?

@im-coder-lg
Copy link
Contributor

It's the new argon2's error.

@jsjoeio
Copy link
Contributor Author

jsjoeio commented Jan 13, 2022

@im-coder-lg not yet. We'll have to check that PR again. I had to rerun the end-to-end tests

@im-coder-lg
Copy link
Contributor

I think this is happening again, any ideas on a fix?

@jsjoeio
Copy link
Contributor Author

jsjoeio commented Aug 8, 2022

@im-coder-lg can you open a new issue? 🙏🏼

@im-coder-lg
Copy link
Contributor

Yeah, will do so. But I want to get more info, so I'll post it by tonight(take IST, perhaps 8 pm here).

@im-coder-lg im-coder-lg mentioned this issue Aug 9, 2022
4 tasks
@code-asher code-asher removed the needs-investigation This issue needs to be further investigated label Jul 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants