View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0004465 | mantisbt | security | public | 2004-09-02 05:16 | 2014-09-23 18:05 |
Reporter | hugopedersen | Assigned To | dregad | ||
Priority | normal | Severity | feature | Reproducibility | always |
Status | closed | Resolution | fixed | ||
Platform | Mantis | OS | Bugtracker | OS Version | 0.19.0rc1 |
Target Version | 1.2.10 | Fixed in Version | 1.2.10 | ||
Summary | 0004465: Turn off 'save login' feature | ||||
Description | I think it would be nice to be able to configure that the 'save login' feature should be enabled or disabled. | ||||
Tags | patch | ||||
Attached Files | login_page.1.52.4.1.6.2+.php.patch (689 bytes)
--- login_page.php 24 Jul 2006 01:22:25 -0000 1.52.4.1.6.2 +++ login_page.php 31 Jan 2007 18:08:50 -0000 @@ -97,14 +97,19 @@ <input type="password" name="password" size="16" maxlength="32" /> </td> </tr> -<tr class="row-1"> - <td class="category"> - <?php echo lang_get( 'save_login' ) ?> - </td> - <td> - <input type="checkbox" name="perm_login" /> - </td> -</tr> +<?php + if ( ON == config_get( 'allow_save_login' ) ) { + ?> <tr class="row-1"> + <td class="category"> + <?php echo lang_get( 'save_login' ) ?> + </td> + <td> + <input type="checkbox" name="perm_login" /> + </td> + </tr> + <?php + } +?> login_page.1.52.4.1.6.2.php (6,887 bytes)
<?php # Mantis - a php based bugtracking system # Copyright (C) 2000 - 2002 Kenzaburo Ito - kenito@300baud.org # Copyright (C) 2002 - 2004 Mantis Team - mantisbt-dev@lists.sourceforge.net # This program is distributed under the terms and conditions of the GPL # See the README and LICENSE files for details # -------------------------------------------------------- # $Id: login_page.php,v 1.52.4.1.6.2 2006/07/24 01:22:25 thraxisp Exp $ # -------------------------------------------------------- # Login page POSTs results to login.php # Check to see if the user is already logged in require_once( 'core.php' ); if ( auth_is_user_authenticated() && !current_user_is_anonymous() ) { print_header_redirect( config_get( 'default_home_page' ) ); } $f_error = gpc_get_bool( 'error' ); $f_cookie_error = gpc_get_bool( 'cookie_error' ); $f_return = gpc_get_string( 'return', '' ); # Check for HTTP_AUTH. HTTP_AUTH is handled in login.php if ( HTTP_AUTH == config_get( 'login_method' ) ) { $t_uri = "login.php"; if ( !$f_return && ON == config_get( 'allow_anonymous_login' ) ) { $t_uri = "login_anon.php"; } if ( $f_return ) { $t_uri .= "?return=" . urlencode( $f_return ); } print_header_redirect( $t_uri ); exit; } html_page_top1(); html_page_top2a(); echo '<br /><div align="center">'; # Display short greeting message # echo lang_get( 'login_page_info' ) . '<br />'; # Only echo error message if error variable is set if ( $f_error ) { echo '<font color="red">' . lang_get( 'login_error' ) . '</font>'; } if ( $f_cookie_error ) { echo lang_get( 'login_cookies_disabled' ) . '<br />'; } echo '</div>'; ?> <!-- Login Form BEGIN --> <br /> <div align="center"> <form name="login_form" method="post" action="login.php"> <table class="width50" cellspacing="1"> <tr> <td class="form-title"> <?php if ( !is_blank( $f_return ) ) { ?> <input type="hidden" name="return" value="<?php echo string_html_specialchars( $f_return ) ?>" /> <?php } echo lang_get( 'login_title' ) ?> </td> <td class="right"> <?php if ( ON == config_get( 'allow_anonymous_login' ) ) { print_bracket_link( 'login_anon.php', lang_get( 'login_anonymously' ) ); } ?> </td> </tr> <tr class="row-1"> <td class="category" width="25%"> <?php echo lang_get( 'username' ) ?> </td> <td width="75%"> <input type="text" name="username" size="32" maxlength="32" /> </td> </tr> <tr class="row-2"> <td class="category"> <?php echo lang_get( 'password' ) ?> </td> <td> <input type="password" name="password" size="16" maxlength="32" /> </td> </tr> <?php if ( ON == config_get( 'allow_save_login' ) ) { ?> <tr class="row-1"> <td class="category"> <?php echo lang_get( 'save_login' ) ?> </td> <td> <input type="checkbox" name="perm_login" /> </td> </tr> <?php } ?> <tr> <td class="center" colspan="2"> <input type="submit" class="button" value="<?php echo lang_get( 'login_button' ) ?>" /> </td> </tr> </table> </form> </div> <?php PRINT '<br /><div align="center">'; print_signup_link(); PRINT ' '; print_lost_password_link(); PRINT '</div>'; # # Do some checks to warn administrators of possible security holes. # Since this is considered part of the admin-checks, the strings are not translated. # # Warning, if plain passwords are selected if ( config_get( 'login_method' ) === PLAIN ) { echo '<div class="warning" align="center">'; echo '<p><font color="red"><strong>WARNING:</strong> Plain password authentication is used, this will expose your passwords to administrators.</font></p>'; echo '</div>'; } # Generate a warning if administrator/root is valid. $t_admin_user_id = user_get_id_by_name( 'administrator' ); if ( $t_admin_user_id !== false ) { if ( user_is_enabled( $t_admin_user_id ) && auth_does_password_match( $t_admin_user_id, 'root' ) ) { echo '<div class="warning" align="center">'; echo '<p><font color="red"><strong>WARNING:</strong> You should disable the default "administrator" account or change its password.</font></p>'; echo '</div>'; } } # Check if the admin directory is available and is readable. $t_admin_dir = dirname( __FILE__ ) . DIRECTORY_SEPARATOR . 'admin' . DIRECTORY_SEPARATOR; if ( is_dir( $t_admin_dir ) && is_readable( $t_admin_dir ) ) { echo '<div class="warning" align="center">', "\n"; echo '<p><font color="red"><strong>WARNING:</strong> Admin directory should be removed.</font></p>', "\n"; echo '</div>', "\n"; # since admin directory and db_upgrade lists are available check for missing db upgrades # Check for db upgrade for versions < 1.0.0 using old upgrader $t_db_version = config_get( 'database_version' , 0 ); # if db version is 0, we haven't moved to new installer. if ( $t_db_version == 0 ) { if ( db_table_exists( config_get( 'mantis_upgrade_table' ) ) ) { $query = "SELECT COUNT(*) from " . config_get( 'mantis_upgrade_table' ) . ";"; $result = db_query( $query ); if ( db_num_rows( $result ) < 1 ) { $t_upgrade_count = 0; } else { $t_upgrade_count = (int)db_result( $result ); } } else { $t_upgrade_count = 0; } if ( $t_upgrade_count > 0 ) { # table exists, check for number of updates require_once( 'admin/upgrade_inc.php' ); $t_upgrades_reqd = $upgrade_set->count_items(); } else { $t_upgrades_reqd = 1000; # arbitrarily large number to force an upgrade } if ( ( $t_upgrade_count != $t_upgrades_reqd ) && ( $t_upgrade_count != ( $t_upgrades_reqd + 10 ) ) ) { # there are 10 optional data escaping fixes that may be present echo '<div class="warning" align="center">'; echo '<p><font color="red"><strong>WARNING:</strong> The database structure may be out of date. Please upgrade <a href="admin/upgrade.php">here</a> before logging in.</font></p>'; echo '</div>'; } } # Check for db upgrade for versions > 1.0.0 using new installer and schema require_once( 'admin/schema.php' ); $t_upgrades_reqd = sizeof( $upgrade ) - 1; if ( ( 0 < $t_db_version ) && ( $t_db_version != $t_upgrades_reqd ) ) { echo '<div class="warning" align="center">'; echo '<p><font color="red"><strong>WARNING:</strong> The database structure may be out of date. Please upgrade <a href="admin/install.php">here</a> before logging in.</font></p>'; echo '</div>'; } } ?> <!-- Autofocus JS --> <?php if ( ON == config_get( 'use_javascript' ) ) { ?> <script type="text/javascript" language="JavaScript"> <!-- window.document.login_form.username.focus(); // --> </script> <?php } ?> <?php html_page_bottom1a( __FILE__ ) ?> 0001-Fix-4465-Add-config-to-disable-save-login-feature.patch (1,964 bytes)
From 7f3d743d2c606612b601bf28e40480f7917668d3 Mon Sep 17 00:00:00 2001 From: Damien Regad <damien.regad@merckgroup.com> Date: Fri, 23 Mar 2012 15:32:54 +0100 Subject: [PATCH] Fix #4465: Add config to disable 'save login' feature To increase security, the administrator may want to prevent users from using a 'permanent' cookie, thus forcing them to authenticate each time they start a new session. The new config option 'g_allow_permanent_cookie' enables this. --- config_defaults_inc.php | 9 +++++++++ login_page.php | 9 ++++++++- 2 files changed, 17 insertions(+), 1 deletions(-) diff --git a/config_defaults_inc.php b/config_defaults_inc.php index 8c5df3d..f03f1f2 100644 --- a/config_defaults_inc.php +++ b/config_defaults_inc.php @@ -304,6 +304,15 @@ */ $g_max_lost_password_in_progress_count = 3; + /** + * Allow users to opt for a 'permanent' cookie when logging in + * Controls the display of the 'Remember my login in this browser' checkbox + * on the login page + * @see $g_cookie_time_length + * @global int $g_allow_permanent_cookie + */ + $g_allow_permanent_cookie = ON; + /*************************** * MantisBT Email Settings * ***************************/ diff --git a/login_page.php b/login_page.php index 9b5edf0..c7b65dc 100644 --- a/login_page.php +++ b/login_page.php @@ -139,6 +139,9 @@ <input type="password" name="password" size="32" maxlength="<?php echo auth_get_password_max_size(); ?>" /> </td> </tr> +<?php + if( ON == config_get( 'allow_permanent_cookie' ) ) { +?> <tr class="row-1"> <td class="category"> <?php echo lang_get( 'save_login' ) ?> @@ -147,7 +150,11 @@ <input type="checkbox" name="perm_login" <?php echo ( $f_perm_login ? 'checked="checked" ' : '' ) ?>/> </td> </tr> -<?php if ( $t_session_validation ) { ?> +<?php + } + + if ( $t_session_validation ) { +?> <tr class="row-2"> <td class="category"> <?php echo lang_get( 'secure_session' ) ?> -- 1.7.5.4 | ||||
I think this problem also exists for installations that are more widely used. Would just adding a warning help? |
|
I have done some modification on my own setup that allows me to set a config option to disable this option. I have less than no experiance in PHP but I managed to get it working. It may be a solution to warn the user when he/she has selected the option to save the password. |
|
Yup, we could definitely make use of that feature here. We have a large user-base and use the LDAP authentication feature, and we've love to just tighten up the login security just that little bit more. I'll see if I can knock together a patch to make use of a '$g_allow_save_login' config option or something like that. |
|
I've attached the modified version of login_page.php rev 1.52.4.1.6.2 (from Mantis version 1.0.6) and the corresponding patch, which I've been using successfully here for a few months now. Very simple fix; all it needs is something like the following in your config_inc file. --- save login ----------------Setting to disable the 'save login' feature.$g_allow_save_login = OFF; Please give it a try and let me know if it works for you. |
|
Please test attached patch. |
|
@dregad, if you want to introduce this option you should add this also to $g_global_settings in config_defaults_inc.php. |
|
Since it's a global setting relating to security, I guess your remark makes sense. I'll add it. |
|
@dregad, have also a look at function config_is_private in config_api.php |
|
I am not sure. Does it really make sense to deny the SOAP API access to the contents of this new variable? It does not in itself contain any sensitive information, just the fact that users are allowed to use the "permanent" cookie. You can easily obtain the same information just by displaying the login page. |
|
Another topic: |
|
I guess we should bounce that off grangeway (as original author of config_is_private function) and rombert. I'll write to the mailing list later as time allows.
Good catch, thanks ! Fixed in https://github.com/dregad/mantisbt/tree/fix-4465 |
|
Feedback from grangeway on config_is_private: 22:43.33 <Paul24> I think we'd concluded stuff that is a security issue Which confirms what I was thinking. So I'll just take the opportunity to better document the function in the code (PHPdoc) and leave it at that unless someone objects. |
|
Changes pushed to github. |
|
Marking as 'acknowledged' not resolved/closed to track that change gets ported to master-2.0.x branch |
|
MantisBT: master 766a7d38 2012-03-23 03:32 Details Diff |
Fix 0004465: Add config to disable 'save login' feature To increase security, the administrator may want to prevent users from using a 'permanent' cookie, thus forcing them to authenticate each time they start a new session. The new config option 'g_allow_permanent_cookie' enables this. Porting to master branch of the following 1.2.x commits: - 56986173ea3a2da12345acecb11afc7ab374696a - 6a9f3a810dca575cc230808b780a80946acdcb73 - 568ee14305c4c6935b18137032d13de097841286 |
Affected Issues 0004465 |
|
mod - config_defaults_inc.php | Diff File | ||
mod - login.php | Diff File | ||
mod - login_page.php | Diff File | ||
MantisBT: master-1.2.x 56986173 2012-03-23 03:32 Details Diff |
Fix 0004465: Add config to disable 'save login' feature To increase security, the administrator may want to prevent users from using a 'permanent' cookie, thus forcing them to authenticate each time they start a new session. The new config option 'g_allow_permanent_cookie' enables this. |
Affected Issues 0004465 |
|
mod - config_defaults_inc.php | Diff File | ||
mod - login_page.php | Diff File | ||
MantisBT: master-1.2.x 6a9f3a81 2012-03-27 01:44 Details Diff |
Add 'allow_permanent_cookie' to g_global_settings Also reflowed the array definition to avoid long lines Fixes 0004465 |
Affected Issues 0004465 |
|
mod - config_defaults_inc.php | Diff File | ||
MantisBT: master-1.2.x 568ee143 2012-03-28 23:24 Details Diff |
Prevent setting permanent cookie using hand-crafted login.php This commit prevents hand-crafted calls to login.php from setting a permanent cookie when its use is disabled via allow_permanent_cookie setting. Thanks to Roland Becker for catching this. Fixes 0004465 |
Affected Issues 0004465 |
|
mod - login.php | Diff File |