OpenNebula / one

The open source Cloud & Edge Computing Platform bringing real freedom to your Enterprise Cloud 🚀
http://opennebula.io
Apache License 2.0
1.23k stars 478 forks source link

Outdated password hashing used (SHA1) #2899

Open niekbosch opened 5 years ago

niekbosch commented 5 years ago

Vulnerability Details When using the core authentication driver, passwords are hashed using the SHA1 hashing algorithm. This is not considered secure for a long time now. During our security audit, this was considered one of the more important problems.

To Reproduce See the code in 'src/um/User.cc', line 346:

password = one_util::sha1_digest(passwd);

Version Completed for affected components

Additional context Our security auditor gave this recommendation:

Use a hashing algorithm specifically designed for storing passwords like Argon2, PBKDF2, bcrypt or scrypt. More information can be found here: https://www.owasp.org/index.php/Password_Storage_Cheat_Sheet

Progress Status

niekbosch commented 5 years ago

I appreciate the work, especially how there is no explicit database conversion needed.

However, although the move to sha256 improves hashing a bit, this is still not a very secure solution. The first problem I found with both this new implementation and the previous one, is that no salting is being used. You can revert a password back to its plain text format with a simple tool like this: https://crackstation.net/

With the above in mind, also remember that for some reason, you can look at your own password hash. What's more, group admins can also look at password hashes of all users in their groups. That's a considerable security risk, especially when combined with the above tool!

Furthermore, like I described in the original issue body, security auditors advise to use much stronger algorithms. This blog (first hit when googling 'is sha 256 secure') gives some more background: https://dusted.codes/sha-256-is-not-a-secure-password-hashing-algorithm

rsmontero commented 5 years ago

Thanks for the feedback @niekbosch !!!!

So yes you are right, the password driver was initially thought for trusted environments and rely on other auth drivers (x509, ssh keys...) when a stronger security is required.

Nevertheless I agree that OpenNebula could benefit from improving the password security. In order to address your both concerns, we can follow a unix approach:

  1. Split the auth data, and move the passwords to a "shadow" table in the database that can be accessed only by oneadmin. Group admins will not have access to the shadow passwords
  2. Add a salt to the passwords, again same way as UNIX

What do you think?

jaypif commented 5 years ago

Hi all,

I am also really interested by this concern as secure environment are really important. Currently, other auth drivers (x509, ssh keys, ...) are not supported when we want to use XML RPC calls. Only Servers (sunstone of EC2) are able to manage this. Is there a plan to support stronger secure method also for XML RPC ? That is a must have if you want to use Infra as Code tool via public interface.

Thank you

al3xhh commented 5 years ago

Hello @jaypif

The XMLRPC supports all authentication methods, for example CLI is using those authentication methods, it just matter of setting the token. Could you elaborate more on your case so we can see what we should do?

Thanks!

jaypif commented 5 years ago

Hello @al3xhh

That's a good news because I just read this page: http://docs.opennebula.org/5.8/deployment/authentication_setup/external_auth.html#how-should-i-read-this-chapter and it is not very explicit that x509 is also supported for API. Maybe the doc is outdated ?

Thank you

al3xhh commented 5 years ago

Yes, you are right @jaypif that is outdated, we will update it, thank you so much for noticing that!

niekbosch commented 5 years ago

Sorry for being quiet, this moved a bit below on my priority list.

I believe that best practices are:

I believe the whole idea behind a readable password field was so you could use it to store an external identifier in case you use the 'proxy'. In that case, the password field in the database is not used, so it is available for other usage. And with a proxy authenticating the user, you often get an identifier (eg. through SAML) that you need to map to an OpenNebula user. However, to me, it feels like leakage of implementation details (the fact that you re-use a table field) at best. That fact that you have to use 'search password', where you actually want to 'search remote identifier' is a failure in abstracting away the implementation. And it is a security risk to use the password field, because of exactly this issue; people being able to read the hashed passwords.

Anyway, I don't want to sound to negative about the proposed solutions, I am just trying to follow best practices. I really do appreciate the time you take to investigate this issue.

Thanks for the feedback @niekbosch !!!!

So yes you are right, the password driver was initially thought for trusted environments and rely on other auth drivers (x509, ssh keys...) when a stronger security is required.

Nevertheless I agree that OpenNebula could benefit from improving the password security. In order to address your both concerns, we can follow a unix approach:

1. Split the auth data, and move the passwords to a "shadow" table in the database that can be accessed only by oneadmin. Group admins will not have access to the shadow passwords

2. Add a salt to the passwords, again same way as UNIX

What do you think?