Inklings-io / selfauth

self-hosted auth_endpoint using simple login mechanism
Creative Commons Zero v1.0 Universal
81 stars 14 forks source link

proper secret generation in setup #6

Closed sknebel closed 7 years ago

sknebel commented 7 years ago

Hashing a timestamp is not a reliable way to generate secrets: there just is not enough entropy in them. If you tell me that you just yesterday set up selfauth, I only have to try 86400 different timestamps to find the secret.

PHP only got a cryptographic random number generator with version 7, but there are several other options:

For the current approach, where the setup tells the user what to put in the file, we could generate the secret client-side using the WebCrypto API.

Server-side (relevant for e.g. #3), https://github.com/paragonie/random_compat provides an (MIT licensed) polyfill.

dissolve commented 7 years ago

Agreed, this definitely needs to be improved.

I rather like the idea of having the servers being able to write the file for you. I do not want to require php7 yet as it's not always available. Perhaps using webcrypto client side and sending it in the Post. I would like to include as few dependencies as possible with it, but this could work if there is a hosted solution.

At the very least it should include __DIR__ and proper Rand () so an attacker would need to know the entire path on the host as well. It wouldn't be perfect, but certainly an improvement.

Also you would need to know exactly which day self auth was installed on to only have 86400 guesses.

aaronpk commented 7 years ago

The mt_rand function is available, http://php.net/mt_rand although it explicitly says it should not be used for cryptographic secret generation. However, it does point to several alternatives. You could check for the presence of the following functions in this order, and use mt_rand as a worst-case fallback:

Zegnat commented 7 years ago

I find it funny that we have a reached a state where we trust the client more with the generation of secrets than the server our code runs on.

I think we would want to steer clear of the fallback hell that is contained in the random_compat polyfill. But something like @aaronpk’s list would be acceptable. Except maybe mt_rand?

This comes down to if you want to be easy or secure. You can’t have both ways. Either you create cryptographically secure secrets and demand certain methods to be available on the server (e.g. openssl_*) or you allow for non-secure secrets so you can fallback to mt_rand/rand and not put demands on the server.

dissolve commented 7 years ago

this was added to setup.php.

Does everyone think this is good enough?

sknebel commented 7 years ago

(comment rescinded)

Zegnat commented 7 years ago

Closing this.

Short of completely reimplementing random_compat and all its fallbacks this is probably as good as it gets.