Project

General

Profile

Feature #603

review of account & password implementation

Added by johanvandyck about 3 years ago. Updated over 2 years ago.

Status:
Resolved
Priority:
Normal
Assignee:
Category:
DomotiGa3
Target version:
Start date:
03/17/2016
Due date:
% Done:

90%

Estimated time:
Resolution:
Fixed

Description

Hi

It all started with long lasting bug https://www.domotiga.nl/issues/575 (user cannot change a password due to a bug)

When having a closer look, the DomotiGa account and password implementation is well done, but outdated and basic. Some obvious features are missing...

1) no mandatory fields like username
2) retype password to avoid mistyping not present
3) no password strenght meter, a decent tool to encourage users to have strong passwords
4) no complex passwords or minimal length constraints
5) no enable / disable account features
6) lock account is basic: fat client close after 3 faulty attemps.
7) db contains by default 2 not-used users.

...but more fundamental:
8) db encoding uses MD5 algo, which is no longer secure. Sha2 should be used
...Nice to have:
9) Two factor authentication

Of course it is easy to say. Let's start!;)

Fase 1 recoding edit user screen:

Soon a first implementation containing 1-2-3-4 is checked into the beta.

Known issues:
- https://www.domotiga.nl/boards/2/topics/6767 (color coding of gui elements)
- https://www.domotiga.nl/boards/2/topics/6768 (regular expressions "spaces")
- When using the buttons of the databrowser, escaping the save function is a little bit tricky. Save will not work when conditions are not met, but DomotiGa will trown a nasty error. A feature of Gambas3 I'm afraid.
--> none of the known issues is a show stopper to my opinion.

I'm deliberately not using the db (yet) to store parameters. It makes the changes easy to add to the current DomotiGa version and the changes 'light'. Only 2 files to replace.

Added: my FEditUsers.class and form. (of DomotiGa 1.0.23 beta, but they should work on 1.0.22 as well)
- copy the files to /domotiga/DomotiGa3/.src/Edit
- compile sh /domotiga/tools/compile.sh
EditUsers Fase1

and Next?

Fase 2: md5 to sha2 + db changes

implementing featurs 5,6,7 and 8.

Fase X:

and Next? not yet known. We will see.

Regards

Johan.

FEditUsers.form (4.22 KB) FEditUsers.form fase1 RC FEditUsers.form johanvandyck, 03/17/2016 10:33 PM
FEditUsers.class (8.35 KB) FEditUsers.class fase1 RC FEditUsers.class johanvandyck, 03/17/2016 10:33 PM
editusers-fase1.jpg (30.8 KB) editusers-fase1.jpg EditUsers Fase1 johanvandyck, 03/17/2016 10:37 PM
fase2-sha256.zip (25.2 KB) fase2-sha256.zip fase2 features 8 and 7. johanvandyck, 03/25/2016 01:33 PM
2556

History

#1 Updated by johanvandyck almost 3 years ago

Hi all

Some update: fase2. I managed to tackle feature 7 and 8: sha256 storage & db changes. The main difficulty was to invent gambas non existing doc around the sha256 encoding.:(

The added files do the job, but do not (yet) convert smoothly from MD5 to sHA256.
usage: copy over the files in the zip to domotiga/Domotiga3/.src and domotiga/Domotiga3/.src/Edit and /domotiga/upgrade/
Excute the db changes first. All passwords are reset to "empty" except admin, which is reset to "admin".
compile and execute...

In order for domotiyii to function, also there the login should be adapted: see https://domotiga.nl/boards/6/topics/6775?r=6779
I summerize: adapt file in /var/www/domotiyii/protected/models/users.php (subdir domotiyii could be left out, depending on your installation) like this:
notice: 8 -> 13 (because salt is enlarged to 13 bits.)
notice: 22 -> 43 (because hash is now 43 bits instead of 22)
notice: MD5 is now $1$ (because crypt lib type of encryption is no longer MD5 but sha256, pointed by "$1$")

* @property string $emailaddress
 */
class Users extends CActiveRecord
{
        /**
         * @return boolean validate user
         */
        public function validatePassword($password)
        {
                if (strlen($this->password) < 11)
                {
                        if($password == $this->password)
                        {
                                return true;
                        } else {
                                return false;
                        }
                }
                return $this->hashPassword($password, substr($this->password, 3, 13)) == $this->password;
        }

        /**
         * @return hashed value
         */

        public function hashPassword($phrase, $salt)
        {
                $md5 = crypt($phrase, "\$5\$" . $salt . "\$");
                $md5 = substr($md5, (43*-1));
                return '$5$' . $salt . $md5;
        }

Goal is to develop the other features and an enlarged splashscreen to ask the user to "change password at login". (= feature 10)
Knowlegde about domotiyii and doc is too little to recode the users screen (and functions like create,delete,...).

I will checking the code in the beta at that time.

Cheers

Johan Van Dyck.

#2 Updated by Alexie almost 3 years ago

  • Status changed from New to In Progress

Thanks for all the work you done until now, i will have a look at the code soon and try to merge it into the BETA branch.

#3 Updated by Alexie almost 3 years ago

  • Status changed from In Progress to Resolved
  • % Done changed from 30 to 90
  • Resolution set to Fixed

Johan, i have committed the "EditUsers" into the BETA branch, after some rework (some internal things in Gambas i had to workaround). It currently only does MD5, but can easily be switched to SHA256/512 now, the controlling code is in the Util.PasswordEncrypt, so not hardcoded as MD5 anymore in DomotiGa (default is still MD5).

I need to have a look into the DomotiYii, otherwise you can't login anymore ... That seems to be a little bit more work, because i am not very familair with the Yii code.

#4 Updated by Alexie almost 3 years ago

BTW: thanks for the hints/tips/suppling code ... always appreciated!

#5 Updated by Alexie almost 3 years ago

At this moment i fixed the User editor in Yii, because it didn't really work. At the moment only MD5 is supported, but i will add SHA256/512 in the near future.

The changes are in the BETA Yii:
https://github.com/DomotiGa/DomotiYii/commit/1a7333dbec7afcd32c339c07e6ca82d59fafb160

#6 Updated by johanvandyck almost 3 years ago

Hi

I analysed the two factor authentication (TFA). I see 2 ways of coding tfa:
  1. 1 use of a api (e.g. authy)

pro: quick programming, easy to implement
pro: sms tfa included
contra: not for free (unless very limited usage)
contra: internet connection needed

  1. 2 own programming

pro: independent of third party
contra: no sample code for gambas.
contra: much programming to do. Gambas has little to no documentation.

Solution:

Best of two worlds: use a open source tfa library and link gambas to the code.
- I choose the python lib ([[https://github.com/pyotp/pyotp.git]])
- I link gambas via shell command to little python script.
pro: quick programming, easy to implement
pro: independent of third party
contra: link to OS shell: python must be installed on the OS. This requires additional skills of the user

tfa features

- for each user a secret is kept in db
- compatible with many clients like authy or google authenticator
- admin's can enable/disable tfa for each user. A regular user can't disable tfa-function.
- admin's can reset (delete) the secret key for a user

open issues

- currently, to setup a QR code is used. To generate this QR code, google is used. (external party n and internet connection needed).
- only available for the full client. I have no knowlegde to implement in the web client domotiyii.

regards
Johan Van Dyck

#7 Updated by johanvandyck almost 3 years ago

Hi

First version of Two Factor Authentication checked in Beta.;)
Now working on
- error handling.
- login procedure where TFA field appears after login. (Now it resides together with username/pwd)
- more elaborated manual
- some small stuff like enable/disable passswords, account lockout, settings in db

Cheers

Johan.

#8 Updated by johanvandyck over 2 years ago

Hey
I'd like to give an update. After the first version is commited to the bete release, I continu working. I made a branch to DomotiGA
Currently in my branch:
- bug: TFA codes starting with one or more "0" are now threaded.
- feature: it is possible to enable/disable an account.
- feature: max possible login of TFA codes is limited to 3
- GUI: the splashscreen is redesigned. TFA is only visible if applicable and after a succesfull authentication

Still to do in order:
- feature: add error handling and feedback to user.
- feature: move parameters to the db (max number of retries, retention time, min password length, min password strength). Not sure yet if I add also a GUI to these parameters.
- bug: hide tfa secret for non admin users in tfa setup screen / account overview screen.
- feature: enhance setup procedure and implement some testing / create wiki pages around security / account / tfa.
- feature: implement retention policy after unsuccefull login
- feature: implement QR code generation by system installed libs instead of google + let user choose between google/system.

note: all above features are for the full client only. The web client of DomotiGa is written in another language and lacks support (of active developpers) or wiki. However, I try:
- web client: adapt web-interface to take only enabled and not locked out accounts into account.
- web client: adapt account overview with new added columns.

Regards

Johan

Also available in: Atom PDF