Jump to:

17452 Posts in 4473 Topics by 1971 members

Archive

SilverStripe Forums » Archive » A request for Markus...

Our old forums are still available as a read-only archive.

Moderators: martimiz, Sean, biapar, Willr, Ingo, simon_w

Page: 1
Go to End
Author Topic: 1482 Views
  • Sam
    Avatar
    Administrator
    679 Posts

    A request for Markus... Link to this post

    Hi Markus,

    Could you add support for password encryption?

    I would suggest that you allow for putting something like this in your _config.php

    Security::set_password_encryption('md5');
    Security::set_password_encryption('password');

    You'll notice that the two strings happen to both be mysql function names ;-)

    To aid migration, adding an administrator tool like Security/encryptallpasswords, which would take plaintext passwords in the database and encrypt them according to the current password encrpytion. It would need some magic to prevent it from being called twice, though!

  • Markus
    Avatar
    Google Summer of Code Hacker
    152 Posts

    Re: A request for Markus... Link to this post

    Yes, of course. I can add support for password encryption, the easiest thing would be to add a column PasswordEncrypted or even better PasswordSalt (so we can use a salt) to the member table.
    I would add that because Silverstripe doesn't support transactions and the process of encrypting all passwords can fail or exceed the time limit in the middle of the process.

    I know that those two are MySQL function names, but I don't think that's a good idea to use the MySQL functions for that because it makes the database abstraction even more difficult. I think the easiest and most portable way is to just use the PHP built in functions.

    But implementing this, we get another problem, namely the UI. At the moment in the security tab the passwords are shown in clear text and can be changed. If we add that new feature we have to change also the UI, but I don't know what the best way would be. Since the password entered there has to be encrypted, we have to add a "password change" feature and recovering the old one will be impossible after that change, so we need some other mechanism to use when someone forgot his password. The easiest way would be to generate a new one and mail it to the user which then afterwards can change it again.

    Anyway, I can start implementing the Security::setPasswordEncryption(...) method and the Security/encryptallpasswords action if you give me the OK to do so...

    Another problem: What should happen if someone changes the encryption method? Let’s say from MD5 to SHA1? All passwords are already encrypted according MD5, those can't be converted to SHA1, new ones will be encrypted automatically by SHA1.. I think that will get a big mess.. maybe we need to store also the encryption method if we use something like this.

    I think it would be much easier to fix one encryption algorithm and just make a Security::encryptPasswords(bool). We can use any of the standard available algorithms.. maybe SHA!?

    Tell me what you think!

  • Sam
    Avatar
    Administrator
    679 Posts

    Re: A request for Markus... Link to this post

    I think that despite the issues that arise if someone changed the encryption method, it's worth having the ability to choose how you want them encrpyed. If you're integrating with another system it can be very helpful to use the same encryption method on your CMS as, for example, your phpBB forum.

    That said, there should be a default option so that people who don't have a preference don't need to make an arbitrary decision.

    It's going to be rare that someone wants to change the password encyption mid-stream, and if they do, it's acceptable that they have to mop the mess up themselves. We just need to document this clearly.

    Having a separate Password and PasswordEncrypted fields leads to an unclear situation when you have both fields populated - which do you use? Another option is to have a PasswordEncryption enum field, which is none/password/md5/sha. This has the added benefit of dealing with changing encyption type mid-stream.

  • Markus
    Avatar
    Google Summer of Code Hacker
    152 Posts

    Re: A request for Markus... Link to this post

    > I think that despite the issues that arise if someone changed the encryption
    > method, it's worth having the ability to choose how you want them encrpyed.
    > If you're integrating with another system it can be very helpful to use the
    > same encryption method on your CMS as, for example, your phpBB forum.

    That's a good point! But should we also use a salt? Otherwise the passwords are still very easy attackable with the help of rainbow tables.
    I think with should add the option to use salts.

    > That said, there should be a default option so that people who don't have
    > a preference don't need to make an arbitrary decision.

    OK

    > It's going to be rare that someone wants to change the password encyption
    > mid-stream, and if they do, it's acceptable that they have to mop the mess up
    > themselves. We just need to document this clearly.

    Sounds reasonable

    > Having a separate Password and PasswordEncrypted fields leads to an
    > unclear situation when you have both fields populated - which do you use?

    Maybe I didn't explain that enough.. I thought about a boolean: "Is the password encrypted?" -> bool PasswordEncrypted

    > Another option is to have a PasswordEncryption enum field, which is
    > none/password/md5/sha. This has the added benefit of dealing with
    > changing encyption type mid-stream.

    Hmm... but then the DB needs to know all available algorithms.. but anyway I think that's the best option.

    So just to make sure I understand everything right and I'm going to implement the right things:

    I will implement the following methods:
    - encryptPasswords(bool)
    - setPasswordEncryptionAlgorithm(string algo, bool use_salt)
    (algo is one of the available algorithms determined by hash_algos())

    The passwords will then be encrypted as follows

    $password = (use_salt == true) ? hash($algorithm, $pwd . $salt) : hash($algorithm, $pwd);

    where $salt is a randomly generated salt which is then stored in the Salt column in the member table (empty string or NULL if not used).

    $algorithm will then also be stored in the member table in the column PasswordEncryption

    I'll will on that after finishing the OpenID integration in the backend.
    Please tell me if everything is OK in that way or if I should change some details.

  • Sam
    Avatar
    Administrator
    679 Posts

    Re: A request for Markus... Link to this post

    It all looks good; the only comment I would have is use lowercase_with_underscores() format for the static functions.

    Upon further investigation, I realised that http://doc.silverstripe.com/doku.php?id=coding-conventions wasn't up to date - sorry about the confusion here!

  • Markus
    Avatar
    Google Summer of Code Hacker
    152 Posts

    Re: A request for Markus... Link to this post

    OK, fine. I created a ticket for that and will start to work on that soon.

    1482 Views
Page: 1
Go to Top

Want to know more about the company that brought you SilverStripe? Then check out SilverStripe.com

Comments on this website? Please give feedback.