Add a session wrapper to encrypt the data before storing it on disk

[nickvergessen]

@DeepDiver1975 @LukasReschke @schiesbn

[nickvergessen]

doc block failed

[LukasReschke]

I have voiced my concerns at https://github.com/owncloud/enterprise/issues/518, please document properly in a PHPDoc that this is only obfuscation.

[LukasReschke]

This will make everything explode if a client does not resend the cookies - are we all REALLY aware of this?

This WILL break stuff.

[LukasReschke]

Also does this check here for an empty cookie value? - In that case it will use the instance's secret which may lead to unexpected behaviour. We should hard-fail there rather.

[nickvergessen]

If the cookie does not exist, we will start a new one.

This WILL break stuff.

Executable code please, so I can test it.

[LukasReschke]

@MTRichards @cmonteroluque I have voiced my concerns mainly ignored at https://github.com/owncloud/enterprise/issues/518 – if we add this we need to add a huge documentation in the code and as well in the documentation that this does not provide any further security against malicious administrators. – We shall not add crypto snakeoil without marking it as such, otherwise the world will laugh about us again 🙊

[MTRichards]

I am fine with documenting this appropriately, no issues there at all.

[LukasReschke]

Can we use a full namespace here or rename the class? "Crypto" is somewhat ambiguous as it is used in other contexts as well.

[scrutinizer-notifier]

A new inspection was created.

[nickvergessen]

copied to keep the discussion from @LukasReschke

This will make everything explode if a client does not resend the cookies - are we all REALLY aware of this?
This WILL break stuff.

[nickvergessen]

Executable samples would be nice, so I can test this.

[ghost]

🚀 Test PASSed.🚀

[LukasReschke]

I'm actually wondering if it would be possible to have this happen on base of a cookie that is already handled properly: The PHP session cookie.

Something like:

1. When user authenticates PHP assigns a session with a random session id to the user
2. On the disk store the session values encrypted using a derived key from the session id all data should be encrypted using the original session id value (sent by the user). Such as PBKDF2. – Please do a benchmark for a somewhat sensible and fast solution per default. To somehow at least mitigate the risk of a time-memory trade-off it would make sense to add the username as salt. Far from perfect, yes, but better than nothing.
3. When accessing ownCloud the user sends the random session id to the server which then ownCloud derives the stored session from and uses the sent session id as key.

@nickvergessen What do you think? – From my PoV this should be possible using a custom session handler.

[LukasReschke]

Something like the following - note: simplified!

1. User logs in, gets assigned LAHYqDaIzH
2. ownCloud stores this session as hash_pbkdf2('sha256', 'LAHYqDaIzH', $salt, 20, 20); ($salt is subject to discussion - may be empty as well): a4bc6fa3c81b0b12b2df
3. The data in the session is encrypted with the original session id: LAHYqDaIzH
4. When the user with LAHYqDaIzH accesses oC, it maps this back to a4bc6fa3c81b0b12b2df and decrypts the data with LAHYqDaIzH as key.

This way we have the same level of security (though debatable) with a more stable approach and less bug potential. - Note that above algorithms are just an example :)

[karlitschek]

I agree with @LukasReschke This might be the better and simpler solution

[LukasReschke]

As discussed with @nickvergessen I will hijack this branch and try to implement my suggested approach.

[nickvergessen]

Feel free to take over, since I will be away the next weeks

[LukasReschke]

But hijacking sounds so much cooler than taking over 😢

[LukasReschke]

As discussed with @butonic I will also create a custom callback for create_sid with regards to users that have their system configured wrongly leading to a potential reuse of sessions and thus a data mixup.

[LukasReschke]

Superseded by #17866