Wonderfall / dockerfiles

Discontinued. Fork at your will.
Creative Commons Zero v1.0 Universal
392 stars 170 forks source link

[nextcloud] instanceid should be generated non-deterministically #105

Open arno01 opened 7 years ago

arno01 commented 7 years ago

Hello @Wonderfall ,

I have found that the instanceid is being deterministically generated:

The instanceid is playing an important role when it comes to the data encryption and probably also token generation, (maybe more? I haven't checked the whole code). For more details please refer to https://github.com/arno01/ocdec/issues/7#issuecomment-274296441

nextcloud/11.0-armhf/setup.sh:instanceid=oc$(echo $PRIMARY_HOSTNAME | sha1sum | fold -w 10 | head -n 1)
nextcloud/11.0/setup.sh:instanceid=oc$(echo $PRIMARY_HOSTNAME | sha1sum | fold -w 10 | head -n 1)
nextcloud/daily-armhf/setup.sh:instanceid=oc$(echo $PRIMARY_HOSTNAME | sha1sum | fold -w 10 | head -n 1)
nextcloud/daily/setup.sh:instanceid=oc$(echo $PRIMARY_HOSTNAME | sha1sum | fold -w 10 | head -n 1)

The 2nd problem is that PRIMARY_HOSTNAME is always empty, so that way each clean run gives the same instanceid:

instanceid=oc$(echo $PRIMARY_HOSTNAME | sha1sum | fold -w 10 | head -n 1)

1st run $ docker run --rm -ti --entrypoint sh wonderfall/nextcloud -c "echo oc$(echo $PRIMARY_HOSTNAME | sha1sum | fold -w 10 | head -n 1)"
ocadc83b19e7
2nd run $ docker run --rm -ti --entrypoint sh wonderfall/nextcloud -c "echo oc$(echo $PRIMARY_HOSTNAME | sha1sum | fold -w 10 | head -n 1)"
ocadc83b19e7

See below how nextcloud actually does this:

instanceid

/**
 * This is a unique identifier for your Nextcloud installation, created
 * automatically by the installer. This example is for documentation only,
 * and you should never use it because it will not work. A valid ``instanceid``
 * is created when you install Nextcloud.
 *
 * 'instanceid' => 'd3c944a9a',
 */
'instanceid' => '',
    /**
     * get an id unique for this instance
     *
     * @return string
     */
    public static function getInstanceId() {
        $id = \OC::$server->getSystemConfig()->getValue('instanceid', null);
        if (is_null($id)) {
            // We need to guarantee at least one letter in instanceid so it can be used as the session_name
            $id = 'oc' . \OC::$server->getSecureRandom()->generate(10, \OCP\Security\ISecureRandom::CHAR_LOWER.\OCP\Security\ISecureRandom::CHAR_DIGITS);
            \OC::$server->getSystemConfig()->setValue('instanceid', $id);
        }
        return $id;
    }

The good point is that secret is properly generated, when it comes to the data encryption.. Though, I am not sure about when it comes to the session tokens, etc...

Probably the users who already use the aforementioned non-unique instanceid should be informed somehow and change it (by decrypting, reinstalling, re-encrypting...).

Thanks for addressing to this issue.

Wonderfall commented 7 years ago

Thanks for your discovery, I should have paid attention to this. I don't have much time but I'll try to fix this as soon as I can (and provide some documentation about it but as I have a lot of exams, this is not my priority unfortunately).

Martinius commented 7 years ago

I am not sure, but I think this problem could be easily solved by mounting /dev/random and /dev/urandom as read only to the docker containers. Probably a good idea to do this to all used containers (like nginx). But I am not an expert on this topic.

arno01 commented 7 years ago

@Martinius that has nothing to do with the /dev/*random.

tblaschke commented 7 years ago

As a quick fix we could just use a hash of the current time, grep the middle and hash it again. ~No need to mount /dev/random.~

date +"%s" | sha1sum | fold -w 14 | head -2 | tail -1 | sha1sum | fold -w 10 | head -1

Edit:

I was too fast. I've seen that you mentioned the instanceid is used for all sorts of encryption within Nextcloud. Then there is really no way around using /dev/random and get the 10 bytes. However, this could increase the setup time a lot if the container doesn't create a lot of entropy. Using /dev/urandom would basically generate numbers as the hashed time approach.

bratkartoffel commented 7 years ago

I suggest using /dev/random directly, without sha1sum. This way you'll also get chars outside of [a-f]. oc$( tr -dc 'a-z0-9' </dev/random | head -c 10 )

arno01 commented 7 years ago

@tblaschke using date +"%s" is good for uniqueness (to never hit the same value again), but is very bad for the security as knowing the approximate installation date/month or even year makes it very easy to guess the instanceid which has to be secret.

@bratkartoffel using /dev/random makes thing go super slow. Use /dev/urandom instead. It is CSPRNG in the recent Linux kernels, please introduce yourself with https://www.2uo.de/myths-about-urandom/ ; and as of extending the range to [a-f], not sure if it will break things. Maybe there is a reason why Nextcloud generates within [a-f].

Actually, I do not see a problem why not using the same instanceid generator as used in the Nextcloud itself https://github.com/nextcloud/server/blob/0fa49db77049712c6ecd249921235efd9776318b/lib/private/legacy/util.php#L1131

As a handy alternative, I would advice using the OpenSSL's rand (which leverages HW CSPRNG), e.g. : echo "oc$(openssl rand -hex 6 |head -c10)" but some believe it can be insecure https://en.wikipedia.org/wiki/RdRand#Reception on some platforms.

bratkartoffel commented 7 years ago

@arno01 nextcloud uses "\OCP\Security\ISecureRandom::CHAR_LOWER" and "\OCP\Security\ISecureRandom::CHAR_DIGITS". CHAR_LOWER means every character, as you can see here: const CHAR_LOWER = 'abcdefghijklmnopqrstuvwxyz';

+1 for /dev/urandom or openssl

arno01 commented 7 years ago

@bratkartoffel this means we can use [a-z] range. Thanks for looking into it.

anthraxx commented 5 years ago

@arno01 @Wonderfall what happened to this ticket? is this resolved?