Poor password security

[phpnode]

There are a number of serious issues with the way orient currently stores passwords:

1. User passwords are stored as direct hashes. This means:

   • I can google for a hash and often retrieve the original password.
   • I can use rainbow tables to crack common passwords extremely efficiently.
   • I can use a GPU to bruteforce password hashes with relative ease.
   • If two or more users share the same password, I recover them all at once.

   this is very very very bad.. At the very least we need to introduce per user salts which would defeat the easiest of the attacks, but the correct solution is to use scrypt or bcrypt here. It is possible to fix this without requiring users to change their passwords or having to wait until they next log in, see this post: http://blog.jgc.org/2012/06/one-way-to-fix-your-rubbish-password.html

2. I can execute a query that retrieves passwords. There are no circumstances where it actually makes sense to be able to retrieve a user's hashed password. It definitely makes no sense for an arbitrary user to be able to retrieve another user's password. Yet at the moment it's as simple as:

   SELECT name, password FROM OUser;

   and as far as I can tell, it's not actually possible to restrict this using the current permissions system.

   Orient currently supports a readonly restriction for properties, I propose we add a writeonly restriction too and apply it to OUser.password. Such properties should never be returned from queries or record load commands.

3. System passwords are stored in plaintext. While Orient generates a strong random root password on first build, it's stored as plain text in orient's configuration file. If I can find a way to read the configuration file, I can access every record in the system, even if I have no further access. The solution is to generate a strong password on first build, present it to the user for them to remember, and store the strong hash (bcrypt, scrypt) in the config file. So now even if I can read the config file, I can't use the credentials to actually login to the database.

[phpnode]

@laa would argue that at least the first point is a critical bug :)

[lvca]

Thanks for this issue. I agree with you this is something important to consider. Do you know a secure way to encrypt passwords bundled with JDK? This because we're very careful with adding external jars.

[phpnode]

@lvca I'm not aware of one but then I'm not a Java developer. I would look at what the popular web frameworks are using for this, as those will probably have received the most attention. It looks like play framework uses this - http://www.mindrot.org/projects/jBCrypt/

[phpnode]

@lvca if you're completely against adding an external library, the following pseudo-code provides a reasonable alternative solution, but bcrypt/scrypt are much more appropriate:

const ITERATIONS = 10000;
const MAX_PASSWORD_LENGTH = 1024;

function hashPassword (password) {
  var salt = cypto.randomBytes(32); // presuming a secure PRNG
  return saltPassword(salt, password);
}

function comparePassword (hashed, password) {
  var salt = hashed.slice(0, 32);
  return constantTimeEquals(hashed, saltPassword(salt, password));
}

function saltPassword (salt, password) {
  var hashed = sha256(salt + password);
  for (i = 0; i < ITERATIONS; i++) {
    hashed = sha256(salt + hashed);
  }
  return salt + hashed;
}

// string comparison which doesn't leak timing information
// Java probably has a built in alternative, use it!
function constantTimeEquals(a, b) {
  var equal = true;
  if (a.length !== b.length) {
    equal = false;
  }
  for (i = 0; i < MAX_PASSWORD_LENGTH; i++) {
    if (a.charAt(i) !== b.charAt(i)) {
      equal = false;
    }
  }
  return equal;
}

[ruckc]

Couldn't hooks be extended to support masking properties of documents being returned? This would assist with data masking of sensitive fields.

[phpnode]

@ruckc yes, and I think that's an important feature too, but it's slightly different from this, and likely going to be slower than having a separate writeonly field which is directly accessible from Java - I think orient could do both.

[lvca]

I found this that can be used without any additional jars with PBKDF2 algorithm:

http://www.javacodegeeks.com/2012/05/secure-password-storage-donts-dos-and.html

WDYT?

[enisher]

@lvca :+1: seems it as resistant to brute force as bcrypt and bundled with JDK

[phpnode]

@lvca @enisher looks fine I think, it's much better than the current situation definitely. I notice that the array comparison in there is not constant time, but since it's comparing hashes I'm not sure it actually matters in practise.

[enisher]

@phpnode Generally it is O(n), but it compare hashes which are fixed length. So it is O(1). I don't see any problems there.

[phpnode]

@enisher sorry, constant time is probably the wrong phrase. What I mean is that it will bail out on the first element of the array that doesn't match, if this was comparing the password itself, it would leak timing information and an attacker could use it to reveal the password. However, since this code is comparing hashes, and not passwords, it probably doesn't matter in practise, because it's unlikely that an attacker can create inputs that hash to a certain value.

[young-druid]

This article covers encryption topic quite well.

[lvca]

@young-druid, very well article about it, thanks.

[phpnode]

@lvca with the new token support this issue becomes much more severe, because people can/will now start using OUser for auth in their apps, and they will presume a certain level of security for their passwords.

This is a horrible, horrible security flaw, the passwords might as well be stored in plain text. Yet it's cheap and easy to fix, can this go into 2.0 please?

[lvca]

@phpnode I'd like to put this in 2.0, but we already have too many things to close. We can do that for 2.1 that will have a very short release cycle.

[hho]

This is security-relevant and has now been open for more than a year… :worried:

It is also a duplicate of #1229, which has been open for almost 3 years.

[lvca]

Part 3) has been fixed in #2811.

[lvca]

Part 1) has been implemented: #1229.

[lvca]

Part 2) has ben fixed by allowing only to "admin" the permission to access to OUser and ORole clusters.