BaseXdb / basex

BaseX Main Repository.
http://basex.org
BSD 3-Clause "New" or "Revised" License
680 stars 267 forks source link

Revise User Management #882

Closed dirkk closed 9 years ago

dirkk commented 10 years ago

Seeing https://github.com/BaseXdb/basex/commit/102296dbeeead7a2ef309b4ade5874f7d3b7c431 reminded me once again, that we are still using MD5 without salt.

MD5 is officially insecure and I would vote to replace it with SHA512 with salts. Problem is, that the existing passwords would be invalid when upgrading. However, we could maybe use the MD5 encrypted password as password for SHA512, that would enable us to "upgrade" all passwords without user interaction. The only downside would be, that we would still need to encrypt each password with MD5, followed by SHA512, which seems to be a bit of a waste. However, performance shouldn't be an issue here.

JensErat commented 10 years ago

I don't see an issue in doing some "silent upgrade path": Just store new passwords with a better function. For authentication, compare with the whatever-hashed value, if it fails try hashing the user's password with MD5, if it fails fail the authentication.

Anyway, I'd go for doing things right and use MCF for storing password hashes. You will realize by looking at the first character that a hash isn't stored as MDF, and can use MD5 as fallback. Using MDF, future "upgrade paths" are completely painless.

This will probably also be helpful for supporting any kind of external authentication in future.

dirkk commented 10 years ago

MCF sounds good to me. +1 from me

The problem I see is that all currently stored passwords will remain MD5 forever (unless the password is changed). This can be ok, of course, as it is a design decision. I see your proposal as the nicer design, but are hesitant because users may not update their passwords. We could at least put some major warnings somewhere, so that users are aware of the problem.

JensErat commented 10 years ago

Print a list of users to "upgrade" each time BaseX starts up with 7.9, and deny with 8.0.

ChristianGruen commented 10 years ago

+1 for your suggestions… I agree that we could surely switch to a new algorithm with 8.0. And it’s currently a mystery anyway how BaseX users are to be handled in the servlet environment…

Maybe we can have a joint discussion on the most appropriate algorithm and proceeding in near future (possibly aligned with the next hackathon)?

Regarding MCF: is it easily implementable, or would we need to resort to external libraries?

JensErat commented 10 years ago

MCF: In the end just means tokenizing on $s and a little bit of pattern matching for selecting the appropriate hashing algorithm.

micheee commented 10 years ago

:+1: I like MCFs approach, but maybe we should limit ourselves to supporting only those algorithms that Java supports ootb.

LeoWoerteler commented 10 years ago
ChristianGruen commented 10 years ago

;) Thanks. Ok, I see, it’s only about conventions. So I think that, for now, SHA-256 with salts would be a viable choice.

ChristianGruen commented 10 years ago

I have enhanced the scope of this issue to “Revise User Management”.

ChristianGruen commented 10 years ago

We’ll need to bring MCF together with our cram-md5 authentication… Ideas anyone?

ChristianGruen commented 10 years ago

I think that, unfortunately, we cannot switch to MCF without breaking all our client bindings. However, our HTTP authentication could easily be updated. We should additionally have a look at the digest authentication issue (#847).

ChristianGruen commented 10 years ago

Another side note: It may be worthwhile to bring database and http session-based authentication together.

ChristianGruen commented 9 years ago

I think I have found a way to incrementally switch to the new user management:

With BaseX 8.0, user credentials and permissions will be stored in XML. If CRAM-MD5 is not required, the MD5 hashes can be manually removed from a permission file.

If we find a good alternative for CRAM-MD5, and if we can ensure that the existing language bindings can be rewritten without too much hassle, we can completely remove the storage of MD5 hashes.

dirkk commented 9 years ago

How does this new permission XML schema look like? I would like to add that I wouldn't strictly restrict it to SHA-256 in the XML, but instead use a schema along the lines of

<password hash="sha256">23456789</password>

so to easily replace SHA-256 in the future. So basically the same what MCF does, just in XML instead of a plain text file. Also, the salt should be stored somewhere, so actually something like

<password algorithm="sha256">
  <salt>mysalt</salt>
  <hash>23456789</hash>
</password>

is probably more useful.

To get rid of CRAM-MD5 we could at least switch to CRAM-SHA256 (not sure this is an official name), i.e. simply switching the hashing algorithm. This seems to me the least possible effort, as it just requires the exchange of the hashing algorithm for the language bindings. Of course, this doesn't solve the theoretical weaknesses of CRAM in general. But to solve this, we would have to switch to a different authentication schema, which would require this to be adapted for all language bindings.

ChristianGruen commented 9 years ago

Thanks, Dirk, for your comments.

How does this new permission XML schema look like?

Probably like this:

<!-- global permissions (.basexperm) -->
<users>
  <user name='John' perm='none'>
    <http-digest>...</http-digest>
    <salt>...</salt>
    <md5>....</md5>
    <salted-sha256>...</salted-sha256>
  </user>
</users>

<!-- local permissions in db directory (prm.basex) -->
<users>
  <user name='John' perm='read'/>
</users>

To get rid of CRAM-MD5 we could at least switch to CRAM-SHA256

I thought about that as well, but sha256 itself is not much safer without salts. Next, we would lose backwards-compatibility, and we would additionally need to store sha256 hashes, which would get obsolete once we switch the algorithm again.

I currently see the following solutions:

1. Status Quo (cram-md5):

↗ no problems with existing bindings ↘ known weaknesses ↘ plain md5 hashes would continue to be stored

2. Improved cram-md5:

• client sends username to the server • server sends timestamp and salt • client sends salted password, additionally hashed with timestamp • server computes match, using salted-sha256 stored in db

↗ safer, because we don't need to stored unsalted passwords anymore ↘ backward-compatibility hard to achieve (both client- and server-side) ↘ deviates from standard

3. http-digest

↗ well-documented, already used in HTTP context ↘ generally more complex (but still straightforward to implement) ↘ HTTP overhead, which would need to be induced in all client bindings

4. Simplified digest authentication:

• client connects to server without sending data (same as cram-md5) • server sends realm and nonce (nonce could be timestamp of cram-md5) • client sends http-digest (md5(username:realm:password)) • server computes match, using http-digest stored in db

↗ http-digest hash can be reused; easy to implement ↗ backward-compatible (both client- and server-side), as order of sent data packages is identical ↘ deviates from standard (because of missing HTTP environment)

5. SCRAM

↗ well-documented, supposedly easier to implement than http-digest ↘ I simply know too less about it (certificates needed? etc.) ↘ no backward-compatibility

The more I think about it, the more I tend to Solution 4. It is still using md5, but as the hash includes username and realm as input, rainbow tables don't represent a problem; and we are not endangered by collision attacks neither. – Opinions?

dirkk commented 9 years ago

Regarding the XML schema I would suggest storing a salt for each password individually. First, because a salt belongs to a password, not to the user itself. Second, and more importantly, this would mean reusing the same salt for hashing all passwords, which I think is an unsafe practice (although it might not matter here much for just two hashes, but if we add potentially more it might be a more practical problem. I see the problem more because the security community seems to dislike reusing salts, so I think we shouldn't do it as a matter of principle).

As for the authentication discussion: I think it is a complex topic. I personally would prefer 5, simply because it is a standard and especially in security-related tasks I am a big fan of following an existing standard as much as possible, as there are many side-effects and possible attack vectors we might not have though about. However, I see your points regarding solution 4 and think it would be a feasible solution.

ChristianGruen commented 9 years ago

Would it take much time to implement SCRAM? What would be the major obstacles in your opinion?

dirkk commented 9 years ago

Well, the server part no, we could pretty much take my implementation lying around somewhere. The hard part will be adapting the language bindings, because I have no experience in roundabout 3 out of 4 programming languages we support as bindings...

ChristianGruen commented 9 years ago

What about the Java client?

dirkk commented 9 years ago

This shouldn't be too difficult. It's just different messages in a different order than right now, but nothing really complicated.

ChristianGruen commented 9 years ago

Solution 4 was chosen, because it does not break compatibility (updated clients can still talk to old older server instances). However, md5 hashes will not be stored anymore. As a result, older clients will not be able to talk to BaseX 8 and upwards.

The XML syntax was adapted as follows:

<users>
  <user name="John" perm="none">
    <password algorithm="salted-sha256">
      <salt>...</salt>
      <hash>...</hash>
    </password>
    <password algorithm="digest">
      <hash>...</hash>
    </password>
  </user>
</users>
ChristianGruen commented 9 years ago

More infos: see sec branch.

ChristianGruen commented 9 years ago

Finalized: