"Remember me" cookie stores plaintext password

[dreadnaut]

Originally reported on Google Code with ID 170

Which is never a good thing to do. We could store a hash instead (SYSTEMPASSWORDENCRYPTED
in the code, although not really encrypted), which is relatively better. Then, instead
of checking 

`hash(<password from cookie> + <salt from cookie>) == <hash server-side>`

we can compare

`<hash from cookie> == hash(<password from config> + <salt from cookie>)`

Alternatively, we can also withhold the salt, and compare

`<hash from cookie> == hash(<password from config> + <salt from config>)`

Attached is a patch for the first behaviour, just to show how small the change would
be.

Reported by dreadnaut on 2013-01-21 19:05:01

<hr> * Attachment: noplain-r318.diff

[dreadnaut]

Sorry, forget the second part, about using the salt from config. Salt must change every
time we don't find it stored on the client, otherwise it's useless.

Reported by dreadnaut on 2013-01-21 19:07:53

[dreadnaut]

Rebased patch to r332. Can anyone double check it?

Reported by dreadnaut on 2013-02-09 17:39:41 - Status changed: Started

<hr> * Attachment: noplain-r332.diff

[Christopher Kramer]

Thanks, I'll check your patch.

Reported by crazy4chrissi on 2013-02-09 17:44:46

[Christopher Kramer]

Two things I noticed so far:
- salt is generated outside the Authorisation class. This means we have a global assumption
(that some salt is generated). We should better put salt-management inside the class
to decouple it (the rest of phpLiteAdmin doesn't use the salt).
- the salt is pretty short. Why do we use such a short salt? I don't see any good reason.

Those aren't problems introduced by your patch. But I just noticed those while having
a look at our Authorisation code.

Reported by crazy4chrissi on 2013-02-09 18:11:33

[Christopher Kramer]

Another thing, while we are at it: Why not using httpOnly cookies? Would be a great
help against XSS/cookie attacks.
Only thing is PHP 5.2 would be required to use setcookie(), or we'd need to set cookies
manually using header(). I am not sure which PHP version we currently require. I guess
we probably already require 5.2 anyway, but I'm not sure.

Reported by crazy4chrissi on 2013-02-09 18:47:03

[Christopher Kramer]

Okay: Please commit your patch. The other points I mentioned actually are different
issues for which I will open separate issues.

Reported by crazy4chrissi on 2013-02-09 21:18:13

[dreadnaut]

Thanks for looking at this. Patch committed, I'll head to the other issues in the next
days.

Reported by dreadnaut on 2013-02-10 01:33:09 - Status changed: Fixed