Хешування паролів з використанням password_hash() та перевірка з password_verify() #54

chief-j · 2017-07-01T05:25:17Z

Fixes #50

konfuciusu · 2017-07-01T10:50:38Z

library/includes/ucp/register.php

@@ -232,16 +232,16 @@
                    } elseif ($new_pass != $cfm_pass) {
                        $errors[] = $lang['CHOOSE_PASS_ERR'];
                    }
-                    $db_data['user_password'] = md5(md5($new_pass));
+					$db_data['user_password'] = password_hash(md5($new_pass), PASSWORD_DEFAULT);


на Толоці ми зараз додатково використовуємо

$options = [ 'cost' => 12, ];

щоб не послабити нові паролі, думаю, варто додати. Єдине, не зовсім зрозуміло, як воно буде себе поводити, коли bcrypt переведуть на щось інше. Щоб уникнути двозначності і сюрпризів, я би тоді також замінив PASSWORD_DEFAULT на PASSWORD_BCRYPT

konfuciusu · 2017-07-01T10:51:00Z

library/includes/ucp/register.php

                }

                if ($mode == 'register') {
                    if (empty($new_pass)) {
-                        $errors[] = $lang['CHOOSE_PASS'];
+					$errors[] = $lang['CHOOSE_PASS'];


треба забрати

Це я прибрати не можу. Не буде повідомлення про не введений пароль при реєстрації

konfuciusu · 2017-07-01T10:53:09Z

src/Legacy/Common/User.php

-                if (!$userdata['username'] || !$userdata['user_password'] || $userdata['user_id'] == GUEST_UID || md5(md5($password)) !== $userdata['user_password'] || !$userdata['user_active']) {
+			$userdata = DB()->fetch_row($sql);
+            if (password_verify(md5($password), $userdata['user_password'])) {
+                if (!$userdata['username'] || !$userdata['user_password'] || $userdata['user_id'] == GUEST_UID /*|| md5(md5($password)) !== $userdata['user_password']*/ || !$userdata['user_active']) {


закоментований код треба забрати

konfuciusu · 2017-07-01T19:49:10Z

library/includes/ucp/activate.php

@@ -42,7 +42,7 @@
    if ($row['user_active'] && trim($row['user_actkey']) == '') {
        bb_die($lang['ALREADY_ACTIVATED']);
    } elseif ((trim($row['user_actkey']) == trim($_GET['act_key'])) && (trim($row['user_actkey']) != '')) {
-        $sql_update_pass = ($row['user_newpasswd'] != '') ? ", user_password = '" . md5(md5($row['user_newpasswd'])) . "', user_newpasswd = ''" : '';
+        $sql_update_pass = ($row['user_newpasswd'] != '') ? ", user_password = '" . password_hash(md5($row['user_newpasswd']), PASSWORD_DEFAULT) . "', user_newpasswd = ''" : '';


тут теж тоді має бути bcrypt і cost 12

konfuciusu · 2017-07-01T19:50:32Z

library/includes/ucp/register.php

                }

                if ($mode == 'register') {
                    if (empty($new_pass)) {
-                        $errors[] = $lang['CHOOSE_PASS'];
+						$errors[] = $lang['CHOOSE_PASS'];


замініть на пробіли

konfuciusu · 2017-07-01T19:51:14Z

src/Legacy/Common/User.php

 				  AND user_active = 1
 				  AND user_id != " . GUEST_UID . "
 				LIMIT 1
 			";

-            if ($userdata = DB()->fetch_row($sql)) {
-                if (!$userdata['username'] || !$userdata['user_password'] || $userdata['user_id'] == GUEST_UID || md5(md5($password)) !== $userdata['user_password'] || !$userdata['user_active']) {
+			$userdata = DB()->fetch_row($sql);


замініть на пробіли

Зверху таби стоять

konfuciusu · 2017-07-01T19:52:01Z

library/includes/ucp/register.php

@@ -232,16 +232,17 @@
                    } elseif ($new_pass != $cfm_pass) {
                        $errors[] = $lang['CHOOSE_PASS_ERR'];
                    }
-                    $db_data['user_password'] = md5(md5($new_pass));
+					$options = ['cost' => 12,];
+					$db_data['user_password'] = password_hash(md5($new_pass), PASSWORD_BCRYPT, $options);


на цих рядках таби, а решта файлу з пробілами. Замініть на пробіли

konfuciusu · 2017-07-01T19:53:08Z

install/sql/mysql.sql

@@ -1116,7 +1116,7 @@ CREATE TABLE `bb_users` (
 INSERT INTO `bb_users` (`user_id`, `user_active`, `username`, `user_password`, `user_session_time`, `user_lastvisit`, `user_last_ip`, `user_regdate`, `user_reg_ip`, `user_level`, `user_posts`, `user_timezone`, `user_lang`, `user_new_privmsg`, `user_unread_privmsg`, `user_last_privmsg`, `user_opt`, `user_rank`, `avatar_ext_id`, `user_gender`, `user_birthday`, `user_email`, `user_skype`, `user_twitter`, `user_icq`, `user_website`, `user_from`, `user_sig`, `user_occ`, `user_interests`, `user_actkey`, `user_newpasswd`, `autologin_id`, `user_newest_pm_id`, `user_points`, `tpl_name`) VALUES
 (-746, 0, 'bot', 'd41d8cd98f00b204e9800998ecf8427e', 0, 0, '0', 0, '0', 0, 0, 0.00, '', 0, 0, 0, 144, 0, 0, 0, '0000-00-00', 'bot@torrentpier.me', '', '', '', '', '', '', '', '', '', '', '', 0, 0.00, 'default'),
 (-1, 0, 'Guest', 'd41d8cd98f00b204e9800998ecf8427e', 0, 0, '0', 0, '0', 0, 0, 0.00, '', 0, 0, 0, 0, 0, 0, 0, '0000-00-00', '', '', '', '', '', '', '', '', '', '', '', '', 0, 0.00, 'default'),
-(2, 1, 'admin', 'c3284d0f94606de1fd2af172aba15bf3', 0, 0, 'c0a86301', 0, '0', 1, 1, 2.00, '', 0, 0, 0, 304, 1, 0, 0, '0000-00-00', 'admin@torrentpier.me', '', '', '', '', '', '', '', '', '', '', 'XCbkm1SmP1GB', 0, 0.00, 'default');
+(2, 1, 'admin', '$2y$10$fjuYdcS0DmOdPsY/LXliq.zQsp3bUR2D.Jw8w5w3HUswBvGgkLSRi', 0, 0, 'c0a86301', 0, '0', 1, 1, 2.00, '', 0, 0, 0, 304, 1, 0, 0, '0000-00-00', 'admin@torrentpier.me', '', '', '', '', '', '', '', '', '', '', 'XCbkm1SmP1GB', 0, 0.00, 'default');


для цілісності, пароль бажано перехешувати з cost 12

chief-j · 2017-07-02T05:13:11Z

Перевірити на працездатність зараз не можу. Теоретично, все буде працювати

konfuciusu · 2017-07-02T11:09:31Z

library/config.php

@@ -33,6 +33,9 @@

 $bb_cfg = [];

+//Алгоритм хешування
+$bb_cfg['passhash_cost'] = ['cost' => 12,];


Плюс, коли це вже масив, то краще перейменувати в

$bb_cfg['passhash_opts'] = ['cost' => 12,];

konfuciusu · 2017-07-02T11:10:01Z

package.json

@@ -1,12 +0,0 @@
-{


випадково видалений файл, треба повернути

konfuciusu · 2017-07-02T21:48:46Z

package.json

@@ -7,6 +7,7 @@
  "author": "Exile <admin@torrentpier.me>",
  "license": "MIT",
  "dependencies": {
-    "jquery": "^3.2.1"
+    "jquery": "^3.2.1",
+    "semantic-ui": "^2.2.10"


це зайве тут, для semantic треба створити окрему гілку

yukoff · 2017-07-18T05:35:57Z

install/sql/mysql.sql

@@ -1074,7 +1074,7 @@ CREATE TABLE `bb_users` (
  `user_id` mediumint(8) NOT NULL,
  `user_active` tinyint(1) NOT NULL DEFAULT 1,
  `username` varchar(25) NOT NULL DEFAULT '',
-  `user_password` varchar(32) CHARACTER SET utf8 COLLATE utf8_bin NOT NULL DEFAULT '',
+  `user_password` varchar(128) CHARACTER SET utf8 COLLATE utf8_bin NOT NULL DEFAULT '',


Навіщо 128? З PASSWORD_BCRYPT завжди вертає 60 символів http://php.net/manual/function.password-hash.php

yukoff · 2017-08-23T00:39:27Z

src/Legacy/Common/User.php

-            if ($userdata = DB()->fetch_row($sql)) {
-                if (!$userdata['username'] || !$userdata['user_password'] || $userdata['user_id'] == GUEST_UID || md5(md5($password)) !== $userdata['user_password'] || !$userdata['user_active']) {
+			$userdata = DB()->fetch_row($sql);
+            if (password_verify(md5($password), $userdata['user_password'])) {


краще

if ($userdata && password_verify(md5($password), $userdata['user_password'])) {

yukoff · 2017-08-23T04:04:29Z

Я перебазував на поточний master та дещо поправив (ну і зробив squash, щоб зменшити кількість комітів). @chief-j, постав тут теж прапорець "Allow edits from maintainers".

chief-j · 2017-08-23T11:08:50Z

Gоставив

yukoff · 2017-08-23T16:01:01Z

@chief-j Дякую

yukoff

Треба буде змержити разом з #74

konfuciusu reviewed Jul 1, 2017

View reviewed changes

konfuciusu reviewed Jul 2, 2017

View reviewed changes

chief-j mentioned this pull request Jul 15, 2017

Покращення хешу паролів #50

Open

yukoff added the in progress label Jul 16, 2017

chief-j mentioned this pull request Jul 17, 2017

Додав користувачів #72

Closed

yukoff added needs review and removed in progress labels Jul 17, 2017

yukoff assigned chief-j Jul 17, 2017

yukoff added pr and removed pr labels Jul 18, 2017

yukoff requested changes Aug 22, 2017

View reviewed changes

yukoff reviewed Aug 23, 2017

View reviewed changes

yukoff added a commit to yukoff/toloka that referenced this pull request Aug 23, 2017

Follow up review comments on hurtom#54

2573c49

yukoff added a commit to yukoff/toloka that referenced this pull request Aug 23, 2017

Follow up review comments on hurtom#54

3944aaf

yukoff added a commit to yukoff/toloka that referenced this pull request Aug 23, 2017

Follow up review comments on hurtom#54

0ae0ae2

yukoff force-pushed the password_hash branch from b0bfc7c to 0ae0ae2 Compare August 23, 2017 15:44

yukoff changed the title ~~Хешування хешованих паролів в алгоритмі password_hash~~ Хешування паролів з password_hash Aug 23, 2017

yukoff changed the title ~~Хешування паролів з password_hash~~ Хешування паролів з використанням password_hash() та перевірка з password_verify() Aug 23, 2017

yukoff previously approved these changes Aug 23, 2017

View reviewed changes

yukoff mentioned this pull request Aug 24, 2017

[WIP] Реалізація міграцій БД з використанням doctrine migrations #74

Open

42 tasks

chief-j and others added 4 commits August 24, 2017 06:45

Crypt passwords by password_hash

6cd1875

Update mysql.sql

4228966

Minor change

54df81d

Follow up review comments on hurtom#54

0f5c0f8

yukoff dismissed their stale review via 0f5c0f8 August 24, 2017 03:47

yukoff force-pushed the password_hash branch from 0ae0ae2 to 0f5c0f8 Compare August 24, 2017 03:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Хешування паролів з використанням password_hash() та перевірка з password_verify() #54

Хешування паролів з використанням password_hash() та перевірка з password_verify() #54

chief-j commented Jul 1, 2017 •

edited by yukoff

Loading

konfuciusu Jul 1, 2017

konfuciusu Jul 1, 2017

chief-j Jul 1, 2017

konfuciusu Jul 1, 2017

konfuciusu Jul 1, 2017

konfuciusu Jul 1, 2017

konfuciusu Jul 1, 2017

chief-j Jul 2, 2017

konfuciusu Jul 1, 2017

konfuciusu Jul 1, 2017

chief-j commented Jul 2, 2017

konfuciusu Jul 2, 2017

yukoff Aug 22, 2017

konfuciusu Jul 2, 2017

konfuciusu Jul 2, 2017

yukoff Jul 18, 2017

yukoff Aug 23, 2017 •

edited

Loading

yukoff commented Aug 23, 2017

chief-j commented Aug 23, 2017

yukoff commented Aug 23, 2017 •

edited

Loading

yukoff left a comment

Хешування паролів з використанням password_hash() та перевірка з password_verify() #54

Are you sure you want to change the base?

Хешування паролів з використанням password_hash() та перевірка з password_verify() #54

Conversation

chief-j commented Jul 1, 2017 • edited by yukoff Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chief-j commented Jul 2, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yukoff Aug 23, 2017 • edited Loading

Choose a reason for hiding this comment

yukoff commented Aug 23, 2017

chief-j commented Aug 23, 2017

yukoff commented Aug 23, 2017 • edited Loading

yukoff left a comment

Choose a reason for hiding this comment

chief-j commented Jul 1, 2017 •

edited by yukoff

Loading

yukoff Aug 23, 2017 •

edited

Loading

yukoff commented Aug 23, 2017 •

edited

Loading