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

password encryption #326

Closed
gregtampa opened this issue Sep 24, 2020 · 2 comments · Fixed by #346
Closed

password encryption #326

gregtampa opened this issue Sep 24, 2020 · 2 comments · Fixed by #346

Comments

@gregtampa
Copy link

why is sha1 being used to store the password and not sha256?

@strugee
Copy link
Contributor

strugee commented Jan 26, 2021

Neither SHA1 nor SHA256 are suitable for storing passwords because they are too fast and often even hardware-accelerated. Additionally these algorithms on their own do not salt passwords.

At minimum bcrypt should be used. scrypt would be even better.

This was referenced Jan 27, 2021
strugee added a commit to strugee/lxdui that referenced this issue Feb 5, 2021
SHA1 passwords are recognized and automatically migrated.

It would be better to automatically bcrypt the existing SHA hashes, but
it would be more complicated to do that on startup and handle all the
possible login scenarios.

Fixes AdaptiveScale#326
strugee added a commit to strugee/lxdui that referenced this issue Feb 5, 2021
SHA1 passwords are recognized and automatically migrated.

It would be better to automatically bcrypt the existing SHA hashes, but
it would be more complicated to do that on startup and handle all the
possible login scenarios.

Fixes AdaptiveScale#326
strugee added a commit to strugee/lxdui that referenced this issue Feb 5, 2021
SHA1 passwords are recognized and automatically migrated.

It would be better to automatically bcrypt the existing SHA hashes, but
it would be more complicated to do that on startup and handle all the
possible login scenarios.

Fixes AdaptiveScale#326
@strugee
Copy link
Contributor

strugee commented Feb 7, 2021

The security bug described here is a type of Sensitive Data Exposure, which the OWASP Top 10 list currently ranks as the third most common type of security vulnerability in web application software. However, this issue has been open for slightly over four months without acknowledgement from LXDUI developers. Additionally, I submitted a proposed fix for this bug 7 days ago (see #346), which has also gone without any response, including after a reminder.

I completely understand and sympathize with an inability to respond to ordinary Pull Requests and issues in a timely manner. We are all extremely busy, and time to spend on (unpaid!) open source is limited. Hell, I myself have probably been guilty of letting PRs pile up more times than not. However, there needs to be some prioritization of security issues, because the longer these exist - especially reported publicly in an issue tracker like this - the more likely it becomes that malicious attackers are making use of these problems.

Therefore, for anyone subscribed to this issue and wanting to deploy a fix to this security bug, I have prepared a modified version of LXDUI 2.1.3 with the fix applied (as well as several others). You can find the release here, signed with my personal GPG key. Do not deploy it without reading the release notes, because they contain important information about compatibility with previous LXDUI releases as well as future LXDUI releases.

I want to emphasize that this is NOT a fork of LXDUI - I'm only taking this step because of the importance of this security issue. I really hope that the security-related PRs applied to this release are merged into LXDUI upstream soon, which would completely remove the need for this alternative release. Furthermore, forks are a serious matter and I do not want to splinter development effort or LXDUI's userbase, especially because LXDUI's developers have clearly put so much time and energy into the project whereas I have not.

If anyone has questions, feel free to ask. You should also consider backing up your configuration (taking care to put auth.conf somewhere safe) before deploying this due to #347.

ailegion added a commit that referenced this issue Jul 1, 2021
SHA1 passwords are recognized and automatically migrated.

It would be better to automatically bcrypt the existing SHA hashes, but
it would be more complicated to do that on startup and handle all the
possible login scenarios.

Fixes #326

Co-authored-by: Ajdin Idrizi <[email protected]>
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.

2 participants