This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "FusionForge".
The branch, 6.0 has been updated
via bded8e874e35c60aa1a7a620fb7f4e31f72841e1 (commit)
via 6e945bff5a16c5013cb08dd835f95f9c6a1a23f9 (commit)
via ade24c73539d7f1fa4c50cb71478474185499ba2 (commit)
from 40d8a4ef372ee3c0fa22a50c227db41990367f3a (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
https://scm.fusionforge.org/anonscm/gitweb/?p=fusionforge/fusionforge.git;a=commitdiff;h=bded8e874e35c60aa1a7a620fb7f4e31f72841e1
commit bded8e874e35c60aa1a7a620fb7f4e31f72841e1
Author: Sylvain Beucler <[email protected]>
Date: Thu Nov 12 15:59:57 2015 +0100
account: drop redundant (and insecure) unsalted MD5 password hashes from
the database
diff --git a/src/CHANGES b/src/CHANGES
index 8f151f6..a742cef 100644
--- a/src/CHANGES
+++ b/src/CHANGES
@@ -1,5 +1,6 @@
FusionForge 6.0.4:
* Accounts: do not accept digits-only user and group names, to avoid confusion
with UID/GID in system commands (Inria)
+* Accounts: drop redundant (and insecure) unsalted MD5 password hashes from
the database (Inria)
* MTA-Exim4: restart exim4 on install
* Plugin SCM: fix another race condition when creating project with SCM
selected (Inria)
* Plugin SCM Git: improve user matching when computing stats, support git
.mailmap (Inria)
diff --git a/src/common/include/User.class.php
b/src/common/include/User.class.php
index cd203e7..ddd5376 100644
--- a/src/common/include/User.class.php
+++ b/src/common/include/User.class.php
@@ -407,10 +407,9 @@ class GFUser extends Error {
// if we got this far, it must be good
$confirm_hash =
substr(md5($password1.util_randbytes().microtime()), 0, 16);
db_begin();
- $result = db_query_params('INSERT INTO users
(user_name,user_pw,unix_pw,realname,firstname,lastname,email,add_date,status,confirm_hash,mail_siteupdates,mail_va,language,timezone,unix_box,address,address2,phone,fax,title,ccode,theme_id,tooltips,shell)
- VALUES
($1,$2,$3,$4,$5,$6,$7,$8,$9,$10,$11,$12,$13,$14,$15,$16,$17,$18,$19,$20,$21,$22,$23,$24)',
+ $result = db_query_params('INSERT INTO users
(user_name,unix_pw,realname,firstname,lastname,email,add_date,status,confirm_hash,mail_siteupdates,mail_va,language,timezone,unix_box,address,address2,phone,fax,title,ccode,theme_id,tooltips,shell)
+ VALUES
($1,$2,$3,$4,$5,$6,$7,$8,$9,$10,$11,$12,$13,$14,$15,$16,$17,$18,$19,$20,$21,$22,$23)',
array($unix_name,
- md5($password1),
account_genunixpw($password1),
htmlspecialchars($firstname.' '.$lastname),
htmlspecialchars($firstname),
@@ -850,12 +849,12 @@ Use one below, but make sure it is entered as the single
line.)
* @return string This user's MD5-crypted passwd.
*/
function getMD5Passwd() {
- return $this->data_array['user_pw'];
+ exit(_('MD5 obsoleted'));
}
//Added to be compatible with codendi getUserPw function
function getUserPw() {
- return $this->data_array['user_pw'];
+ exit(_('MD5 obsoleted'));
}
/**
@@ -1394,12 +1393,10 @@ Use one below, but make sure it is entered as the
single line.)
}
db_begin();
- $md5_pw = md5($passwd);
$unix_pw = account_genunixpw($passwd);
- $res = db_query_params('UPDATE users SET user_pw=$1, unix_pw=$2
WHERE user_id=$3',
- array($md5_pw,
- $unix_pw,
+ $res = db_query_params('UPDATE users SET unix_pw=$1 WHERE
user_id=$2',
+ array($unix_pw,
$this->getID()));
if (!$res || db_affected_rows($res) < 1) {
@@ -1433,19 +1430,8 @@ Use one below, but make sure it is entered as the single
line.)
* @return boolean success.
*/
function setMD5Passwd($md5) {
- db_begin();
- if ($md5) {
- $res = db_query_params('UPDATE users SET user_pw=$1
WHERE user_id=$2',
- array($md5, $this->getID()));
-
- if (!$res || db_affected_rows($res) < 1) {
- $this->setError(_('Error: Cannot Change User
Password:').' '.db_error());
- db_rollback();
- return false;
- }
- }
- db_commit();
- return true;
+ exit(_('Error: Cannot Change User Password:').' '._('MD5
obsoleted'));
+ return false;
}
/**
diff --git a/src/common/include/session.php b/src/common/include/session.php
index aa3c7e9..9bd3e38 100644
--- a/src/common/include/session.php
+++ b/src/common/include/session.php
@@ -217,109 +217,66 @@ function
session_check_credentials_in_database($loginname, $passwd, $allowpendin
function session_login_valid_dbonly($loginname, $passwd, $allowpending) {
global $feedback, $userstatus;
- // Try to get the users from the database using user_id and (MD5)
user_pw
+ // Selecting by user_name/email only
if (forge_get_config('require_unique_email')) {
- $res = db_query_params ('SELECT user_id,status,unix_pw FROM
users WHERE (user_name=$1 OR email=$1) AND user_pw=$2',
- array ($loginname,
- md5($passwd))) ;
+ $res = db_query_params ('SELECT user_id,status,unix_pw FROM
users WHERE user_name=$1 OR email=$1',
+ array
($loginname)) ;
} else {
- $res = db_query_params ('SELECT user_id,status,unix_pw FROM
users WHERE user_name=$1 AND user_pw=$2',
- array ($loginname,
- md5($passwd))) ;
+ $res = db_query_params ('SELECT user_id,status,unix_pw FROM
users WHERE user_name=$1',
+ array
($loginname)) ;
}
if (!$res || db_numrows($res) < 1) {
- // No user whose MD5 passwd matches the MD5 of the provided
passwd
- // Selecting by user_name/email only
- if (forge_get_config('require_unique_email')) {
- $res = db_query_params ('SELECT user_id,status,unix_pw
FROM users WHERE user_name=$1 OR email=$1',
- array ($loginname)) ;
- } else {
- $res = db_query_params ('SELECT user_id,status,unix_pw
FROM users WHERE user_name=$1',
- array ($loginname)) ;
- }
- if (!$res || db_numrows($res) < 1) {
- // No user by that name
- $error_msg = _('Invalid Password Or User Name');
- return false;
- } else {
- // There is a user with the provided user_name/email,
but the MD5 passwds do not match
- // We'll have to try checking the (crypt) unix_pw
- $usr = db_fetch_array($res);
- $userstatus = $usr['status'] ;
-
- if (crypt($passwd, $usr['unix_pw']) != $usr['unix_pw'])
{
- // Even the (crypt) unix_pw does not patch
- // This one has clearly typed a bad passwd
- $error_msg = _('Invalid Password Or User Name');
- return false;
- }
- // User exists, (crypt) unix_pw matches
- // Update the (MD5) user_pw and retry authentication
- // It should work, except for status errors
- $res = db_query_params ('UPDATE users SET user_pw=$1
WHERE user_id=$2',
- array (md5($passwd),
- $usr['user_id'])) ;
- return
session_check_credentials_in_database($loginname, $passwd, $allowpending) ;
- }
+ // No user by that name
+ $error_msg = _('Invalid Password Or User Name');
+ return false;
} else {
- // If we're here, then the user has typed a password matching
the (MD5) user_pw
- // Let's check whether it also matches the (crypt) unix_pw
+ // There is a user with the provided user_name/email
+ // Check the (crypt) unix_pw
$usr = db_fetch_array($res);
+ $userstatus = $usr['status'] ;
- if (crypt ($passwd, $usr['unix_pw']) != $usr['unix_pw']) {
- // The (crypt) unix_pw does not match
- if ($usr['unix_pw'] == '') {
- // Empty unix_pw, we'll take the MD5 as
authoritative
- // Update the (crypt) unix_pw and retry
authentication
- // It should work, except for status errors
- $res = db_query_params ('UPDATE users SET
unix_pw=$1 WHERE user_id=$2',
- array
(account_genunixpw($passwd),
-
$usr['user_id'])) ;
- return
session_check_credentials_in_database($loginname, $passwd, $allowpending) ;
- } else {
- // Invalidate (MD5) user_pw, refuse
authentication
- $res = db_query_params ('UPDATE users SET
user_pw=$1 WHERE user_id=$2',
- array ('OUT OF DATE',
-
$usr['user_id'])) ;
- $warning_msg =_('Invalid Password Or User
Name');
- return false;
- }
+ if ($usr['unix_pw'] !== crypt($passwd, $usr['unix_pw'])) {
+ // (crypt) unix_pw does not patch
+ $error_msg = _('Invalid Password Or User Name');
+ return false;
}
+ // User exists, (crypt) unix_pw matches
+ }
- // Yay. The provided password matches both fields in the
database.
- // Let's check the status of this user
- // if allowpending (for verify.php) then allow
- $userstatus = $usr['status'];
- if ($allowpending && ($usr['status'] == 'P')) {
- //1;
- } else {
- if ($usr['status'] == 'S') {
- //acount suspended
- $feedback = _('Account Suspended');
- return false;
- }
- if ($usr['status'] == 'P') {
- //account pending
- $feedback = _('Account Pending');
- return false;
- }
- if ($usr['status'] == 'D') {
- //account deleted
- $feedback = _('Account Deleted');
- return false;
- }
- if ($usr['status'] != 'A') {
- //unacceptable account flag
- $feedback = _('Account Not Active');
- return false;
- }
- }
- // create a new session
- session_set_new(db_result($res, 0, 'user_id'));
+ // Yay. The provided password matches both fields in the database.
+ // Let's check the status of this user
- return true;
+ // if allowpending (for verify.php) then allow
+ $userstatus = $usr['status'];
+ if ($allowpending && ($usr['status'] == 'P')) {
+ //1;
+ } else {
+ if ($usr['status'] == 'S') {
+ //acount suspended
+ $feedback = _('Account Suspended');
+ return false;
+ }
+ if ($usr['status'] == 'P') {
+ //account pending
+ $feedback = _('Account Pending');
+ return false;
+ }
+ if ($usr['status'] == 'D') {
+ //account deleted
+ $feedback = _('Account Deleted');
+ return false;
+ }
+ if ($usr['status'] != 'A') {
+ //unacceptable account flag
+ $feedback = _('Account Not Active');
+ return false;
+ }
}
+ // create a new session
+ session_set_new(db_result($res, 0, 'user_id'));
+
+ return true;
}
/**
diff --git a/src/db/20151112-drop-unsalted-md5.sql
b/src/db/20151112-drop-unsalted-md5.sql
new file mode 100644
index 0000000..c616af0
--- /dev/null
+++ b/src/db/20151112-drop-unsalted-md5.sql
@@ -0,0 +1,2 @@
+-- Drop unsecure copy of unix_pw
+ALTER TABLE users DROP user_pw;
diff --git a/src/post-install.d/db/populate.sh
b/src/post-install.d/db/populate.sh
index cc16c6f..7d13d64 100755
--- a/src/post-install.d/db/populate.sh
+++ b/src/post-install.d/db/populate.sh
@@ -94,9 +94,9 @@ req="SELECT COUNT(*) FROM users WHERE user_name='admin'"
if [ "$(echo $req | su - postgres -c "psql -At $database_name")" != "1" ]; then
psql -h $database_host -p $database_port -U $database_user $database_name
<<EOF >/dev/null
INSERT INTO users (user_name, realname, firstname, lastname, email,
- user_pw, unix_pw, status, theme_id)
+ unix_pw, status, theme_id)
VALUES ('admin', 'Forge Admin', 'Forge', 'Admin',
'[email protected]',
- 'INVALID', 'INVALID', 'A', (SELECT theme_id FROM themes WHERE
dirname='funky'));
+ 'INVALID', 'A', (SELECT theme_id FROM themes WHERE dirname='funky'));
EOF
forge_make_admin admin # set permissions
# Note: no password defined yet
diff --git a/src/www/account/change_pw.php b/src/www/account/change_pw.php
index 003ef70..75c7504 100644
--- a/src/www/account/change_pw.php
+++ b/src/www/account/change_pw.php
@@ -46,7 +46,7 @@ if (getStringFromRequest('submit')) {
$passwd = getStringFromRequest('passwd');
$passwd2 = getStringFromRequest('passwd2');
- if ($u->getMD5Passwd() != md5($old_passwd)) {
+ if ($u->getUnixPasswd() !== crypt($old_passwd, $u->getUnixPasswd())) {
form_release_key(getStringFromRequest('form_key'));
exit_error(_('Old password is incorrect'),'my');
}
diff --git a/src/www/account/index.php b/src/www/account/index.php
index ef85b9e..420ee40 100644
--- a/src/www/account/index.php
+++ b/src/www/account/index.php
@@ -83,16 +83,6 @@ if (getStringFromRequest('submit')) {
}
if ($check) {
-/*
-//needs security audit
- if ($remember_user) {
- // set cookie, expire in 3 months
-
setcookie("sf_user_hash",$u->getID().'_'.substr($u->getMD5Passwd(),0,16),time()+90*24*60*60,'/');
- } else {
- // remove cookie
- setcookie("sf_user_hash",'',0,'/');
- }
-*/
// Refresh page if language or theme changed
$refresh = ($language != $u->getLanguage() || $theme_id !=
$u->getThemeID());
diff --git a/tests/func/10_Site/loginTest.php b/tests/func/10_Site/loginTest.php
index 1c338f7..4489eb6 100644
--- a/tests/func/10_Site/loginTest.php
+++ b/tests/func/10_Site/loginTest.php
@@ -111,8 +111,8 @@ class LoginProcess extends FForge_SeleniumTestCase
$this->type("passwd", FORGE_OTHER_PASSWORD);
$this->type("passwd2", FORGE_OTHER_PASSWORD);
$this->clickAndWait("submit");
- $this->logout();
+ $this->logout();
$this->open( ROOT );
$this->click("link=Log In");
$this->waitForPageToLoad("30000");
@@ -122,6 +122,32 @@ class LoginProcess extends FForge_SeleniumTestCase
$this->waitForPageToLoad("30000");
$this->assertTrue($this->isTextPresent("Forge Admin"));
$this->assertTrue($this->isTextPresent("Log Out"));
+
+ // Test changing password through user account
+ $this->clickAndWait("link=My Account");
+ $this->clickAndWait("link=Change Password");
+ $this->type("old_passwd", "awrongpassword");
+ $this->type("passwd", FORGE_ADMIN_PASSWORD);
+ $this->type("passwd2", FORGE_ADMIN_PASSWORD);
+ $this->clickAndWait("submit");
+ $this->assertTrue($this->isTextPresent("Old password is
incorrect"));
+
+ $this->clickAndWait("link=My Account");
+ $this->clickAndWait("link=Change Password");
+ $this->type("old_passwd", FORGE_OTHER_PASSWORD);
+ $this->type("passwd", FORGE_ADMIN_PASSWORD);
+ $this->type("passwd2", FORGE_ADMIN_PASSWORD);
+ $this->clickAndWait("submit");
+
+ $this->logout();
+ $this->open( ROOT );
+ $this->click("link=Log In");
+ $this->waitForPageToLoad();
+ $this->type("form_loginname", FORGE_ADMIN_USERNAME);
+ $this->type("form_pw", FORGE_ADMIN_PASSWORD);
+ $this->click("login");
+ $this->waitForPageToLoad();
+ $this->assertTrue($this->isTextPresent("Forge Admin"));
$this->assertTrue($this->isTextPresent("Log Out"));
}
}
https://scm.fusionforge.org/anonscm/gitweb/?p=fusionforge/fusionforge.git;a=commitdiff;h=6e945bff5a16c5013cb08dd835f95f9c6a1a23f9
commit 6e945bff5a16c5013cb08dd835f95f9c6a1a23f9
Author: Sylvain Beucler <[email protected]>
Date: Thu Nov 12 15:32:33 2015 +0100
account: don't display printf return value
diff --git a/src/www/account/lostpw.php b/src/www/account/lostpw.php
index 72d1aaa..591769a 100644
--- a/src/www/account/lostpw.php
+++ b/src/www/account/lostpw.php
@@ -71,7 +71,8 @@ if (getStringFromRequest('submit')) {
$HTML->header(array('title'=>_('Lost Password Confirmation')));
- echo '<p>'.printf(_('An email has been sent to the address you
have on file. Follow the instructions in the email to change your account
password.').'</p><p><a href="%s">'._("Home").'</a>', util_make_url
('/')).'</p>';
+ echo '<p>'._('An email has been sent to the address you have on
file. Follow the instructions in the email to change your account
password.').'</p>';
+ printf('<p><a href="%s">'._("Home").'</a></p>', util_make_url
('/'));
$HTML->footer();
exit();
https://scm.fusionforge.org/anonscm/gitweb/?p=fusionforge/fusionforge.git;a=commitdiff;h=ade24c73539d7f1fa4c50cb71478474185499ba2
commit ade24c73539d7f1fa4c50cb71478474185499ba2
Author: Sylvain Beucler <[email protected]>
Date: Thu Nov 12 15:29:53 2015 +0100
testsuite: don't fail if apache is initially stopped - just start it
diff --git a/tests/func/fixtures.sh b/tests/func/fixtures.sh
index 82ec7d6..9b703d7 100755
--- a/tests/func/fixtures.sh
+++ b/tests/func/fixtures.sh
@@ -126,7 +126,7 @@ fi
if [ "$reset" = 1 ]; then
set -e
# Reset connections
- stop_apache --force
+ stop_apache --force || true
service fusionforge-systasksd stop
service postgresql restart
su - postgres -c "dropdb $database" || true
-----------------------------------------------------------------------
Summary of changes:
src/CHANGES | 1 +
src/common/include/User.class.php | 30 ++------
src/common/include/session.php | 139 ++++++++++++----------------------
src/db/20151112-drop-unsalted-md5.sql | 2 +
src/post-install.d/db/populate.sh | 4 +-
src/www/account/change_pw.php | 2 +-
src/www/account/index.php | 10 ---
src/www/account/lostpw.php | 3 +-
tests/func/10_Site/loginTest.php | 28 ++++++-
tests/func/fixtures.sh | 2 +-
10 files changed, 92 insertions(+), 129 deletions(-)
create mode 100644 src/db/20151112-drop-unsalted-md5.sql
hooks/post-receive
--
FusionForge
_______________________________________________
Fusionforge-commits mailing list
[email protected]
http://lists.fusionforge.org/cgi-bin/mailman/listinfo/fusionforge-commits
