View Issue Details

IDProjectCategoryView StatusLast Update
0008336mantisbtsecuritypublic2007-10-04 01:37
Reportervboctor Assigned Tojreese  
PrioritylowSeverityminorReproducibilityN/A
Status closedResolutionfixed 
Product Version1.1.0a4 
Fixed in Version1.1.0rc1 
Summary0008336: Require re-login for high impact tasks
Description

In SourceForge users can setup a 'Remember Me' flag in order not to need to login everytime. However, for certain operations users are asked to re-login. For example, if an admin is just viewing or reporting an issue, the remember password works, however, if the user is creating a user or deleting a project, then the admin should be asked to re-login.

TagsNo tags attached.
Attached Files
mantis_reauth_2007-09-20.patch (5,319 bytes)   
diff -urN --exclude=CVS --exclude=.svn --exclude='.git*' --exclude='*.swp' --exclude=config_inc.php mantis-cvs/core/authentication_api.php mantis/core/authentication_api.php
--- mantis-cvs/core/authentication_api.php	2007-09-08 19:21:00.000000000 -0400
+++ mantis/core/authentication_api.php	2007-09-20 10:41:22.000000000 -0400
@@ -127,6 +127,7 @@
 
 		# set the cookies
 		auth_set_cookies( $t_user_id, $p_perm_login );
+		auth_set_tokens( $t_user_id );
 
 		return true;
 	}
@@ -405,6 +406,123 @@
 		return $t_cookie;
 	}
 
+	#===================================
+	# Re-Authentication Tokens
+	#===================================
+
+	/**
+	 * Set authentication tokens for secure session.
+	 * @param integer User ID
+	 */
+	function auth_set_tokens( $p_user_id ) {
+		$t_auth_token = token_get( TOKEN_AUTHENTICATED, $p_user_id );
+		if ( null == $t_auth_token ) {
+			token_set( TOKEN_AUTHENTICATED, true, TOKEN_EXPIRY_AUTHENTICATED, $p_user_id );
+		} else {
+			token_touch( $t_auth_token['id'], TOKEN_EXPIRY_AUTHENTICATED );
+		}
+	}
+
+	/**
+	 * Check for authentication tokens, and display re-authentication page if needed.
+	 * Currently, if using BASIC or HTTP authentication methods, or if logged in anonymously, 
+	 * this function will always "authenticate" the user (do nothing).
+	 */
+	function auth_reauthenticate() {
+		if ( BASIC_AUTH == config_get( 'login_method' ) ||
+				HTTP_AUTH == config_get( 'login_method' ) ) {
+			return true;
+		}
+
+		$t_auth_token = token_get( TOKEN_AUTHENTICATED );
+		if ( null != $t_auth_token ) {
+			token_touch( $t_auth_token['id'], TOKEN_EXPIRY_AUTHENTICATED );
+			return true;
+		} else {
+			$t_anon_account = config_get( 'anonymous_account' );
+			$t_anon_allowed = config_get( 'allow_anonymous_login' );
+
+			$t_user_id = auth_get_current_user_id();
+			$t_username = user_get_field( $t_user_id, 'username' );
+
+			# check for anonymous login
+			if ( ON == $t_anon_allowed && $t_anon_account == $t_username ) {
+				return true;
+			}
+	
+			return auth_reauthenticate_page( $t_user_id, $t_username );
+		}
+	}
+
+	/**
+	 * Generate the intermediate authentication page.
+	 * @param integer User ID
+	 * @param string Username
+	 */
+	function auth_reauthenticate_page( $p_user_id, $p_username ) {
+		$t_error = false;
+
+		if ( true == gpc_get_bool( '_authenticate' ) ) {
+			$f_password     = gpc_get_string( 'password', '' );
+					    
+			if ( auth_attempt_login( $p_username, $f_password ) ) {
+				auth_set_tokens( $p_user_id );
+				return true;
+			} else {
+				$t_error = true;
+			}
+		}
+
+		html_page_top1();
+		html_page_top2();
+
+?>
+<div align="center">
+<p>
+<?php 
+		echo lang_get( 'reauthenticate_message' ); 
+		if ( $t_error != false ) {
+			echo '<br/><font color="red">',lang_get( 'login_error' ),'</font>';
+		}
+?>
+</p>
+<form method="post" action="<?php echo $_SERVER['PHP_SELF']; ?>">
+
+<?php
+		print_hidden_inputs( gpc_strip_slashes( $_POST ) );
+		print_hidden_inputs( gpc_strip_slashes( $_GET ) );
+?>
+
+<input type="hidden" name="_authenticate" value="1" />
+
+<table class="width50 center">
+<tr>
+	<td class="form-title"><?php echo lang_get( 'reauthenticate_title' ); ?></td>
+</tr>
+
+<tr class="row-1">
+	<td class="category"><?php echo lang_get( 'username' ); ?></td>
+	<td><input type="text" disabled="disabled" size="32" maxlength="32" value="<?php echo $p_username; ?>" /></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>
+
+<tr>
+	<td class="center" colspan="2"><input type="submit" class="button" value="Authenticate" /></td>
+</tr>
+</table>
+
+</form>
+</div>
+
+		<?php
+		html_page_bottom1();
+
+		exit;
+	}
 
 	#===================================
 	# Data Access
diff -urN --exclude=CVS --exclude=.svn --exclude='.git*' --exclude='*.swp' --exclude=config_inc.php mantis-cvs/core/constant_inc.php mantis/core/constant_inc.php
--- mantis-cvs/core/constant_inc.php	2007-09-18 09:06:23.000000000 -0400
+++ mantis/core/constant_inc.php	2007-09-20 10:42:53.000000000 -0400
@@ -361,9 +361,12 @@
 	define( 'TOKEN_FILTER',			1 );
 	define( 'TOKEN_GRAPH',			2 );
 	define( 'TOKEN_LAST_VISITED',	3 );
+	define( 'TOKEN_AUTHENTICATED',	4 );
 	define( 'TOKEN_USER',			1000 );
 
+	# token expirations
 	define( 'TOKEN_EXPIRY', 		60*60 ); # Default expiration of 60 minutes ( 3600 seconds )
+	define( 'TOKEN_EXPIRY_AUTHENTICATED', 5*60 );
 
 	# config types
 	define( 'CONFIG_TYPE_INT', 1 );
diff -urN --exclude=CVS --exclude=.svn --exclude='.git*' --exclude='*.swp' --exclude=config_inc.php mantis-cvs/lang/strings_english.txt mantis/lang/strings_english.txt
--- mantis-cvs/lang/strings_english.txt	2007-09-18 09:06:26.000000000 -0400
+++ mantis/lang/strings_english.txt	2007-09-18 11:46:58.000000000 -0400
@@ -84,6 +84,8 @@
 $s_bug_relationships = 'Relationships';
 $s_empty_password_sure_msg = 'The user has an empty password.  Are you sure that is what you want?';
 $s_empty_password_button = 'Use Empty Password';
+$s_reauthenticate_title = 'Authenticate';
+$s_reauthenticate_message = 'You are visting a secure page, and your secure session has expired.  Please authenticate yourself to continue.';
 
 $s_duplicate_of = "duplicate of";
 $s_has_duplicate = "has duplicate";
mantis_reauth_2007-09-20.patch (5,319 bytes)   

Activities

jreese

jreese

2007-09-06 11:10

reporter   ~0015581

I think this could be generalized to encompass other items, like bug 0008335. Instead of changing the system to request a password at the same time, simply require re-login when entering the My Account or Manage Users/Projects pages.

Going further, I think it would be best to extend the authentication API to add a "verify login" procedure, which would request re-login, and then create a special cookie (or some more secure flag on the server side) that expires within 10 minutes (or some other short value), and in future executions of the procedure, it would update the cookie to last another ten minutes, etc.

This would allow any "sensitive" page to simply call a single function, which would act similar to the helper_ensure_confirmed() function to either display the re-login page or update the cookie. Then the user would only need to re-verify login for Manage pages once that cookie has expired, while allowing prolonged usage without constant badgering for authentication.

vboctor

vboctor

2007-09-07 04:54

manager   ~0015588

This is inline with what I had in mind. Following are some comments:

  • Although this is a step in covering 0008335, I think we should still implement the fix for 0008335. We should implement the common pattern for password change which is more standard and secure.

  • We should add a configuration variable for the new cookie and another one to control the lifetime of the cookie. The default can be 5 minutes or whatever.

  • The lifetime of the new cookie will be extended on both normal and sensitive pages? i.e. standard session expiry logic. If we renew on all pages, then we have to be careful of the auto-refresh pages like View Issues page.

jreese

jreese

2007-09-07 07:19

reporter   ~0015591

Upon further thinking and review, two thoughts:

The lifetime should only be refreshed when visiting pages that require the re-authentication process. This will go a long way to preventing security breaches due to leaving browsers open. This is just like the method 'sudo' on Linux machines works to allow continuous use of admin privileges; after five minutes of non-use, a user must re-authenticate before proceeding again.

We should not use a cookie, since those can be manipulated and forged. This should be some sort of addition to the user table, a new relational table, or should be stored in some sort of persistent hash. Regardless, it definitely needs to be server side, otherwise a devious user could simply recreate the appropriate cookie to permanently bypass the need for re-authentication.

vboctor

vboctor

2007-09-07 07:54

manager   ~0015593

We can use a token for the authentication (see core/token_api.php), this will keep it 100% server side and will require no database changes.

I agree that extending the lifetime of this token should be based on visiting pages that trigger the privileged authentication.

jreese

jreese

2007-09-07 09:28

reporter   ~0015595

I'm going to go ahead and start on this today. I'll start by cleaning/finishing up the Token API, and then move to adding functions to the Auth API for requesting re-login, and then I'll move to adding re-auth requests from all "sensitive" pages, which for now, I will assume to be any "Manage ..." type of page.

jreese

jreese

2007-09-20 11:04

reporter   ~0015675

Patch 'mantis_reauth_2007-09-20.patch' created from CVS Head on Thursday, Sep. 20th.

This adds all the API functions, constants, and strings necessary to implement this feature. It does not include any of the per-page calls that will actually enforce the re-authentication.

I'm not sure how to handle BASIC_AUTH or HTTP_AUTH login methods, so I have it automatically bypass re-authentication for those login methods, and the same for when using the anonymous user account. Does it even make sense to have basic/http re-authentication necessary?

Anyways, I have tested the patch as is, and it works great, so I am ready to commit it as soon as we decide how to handle basic/http authentication. Then I would just need to follow-up with a patch to all the manage_* pages to enforce re-authentication.

jreese

jreese

2007-09-22 19:14

reporter   ~0015694

After some more testing, I went ahead and committed the patch to CVS. For the time being, the code will not break for anyone using basic/http auth, it will simply ignore the need for re-authentication. If we decide on a proper way to handle this situation, I have no problem implementing it.

I will be patching the manage_* pages in the next two or three days.

jreese

jreese

2007-09-25 19:54

reporter   ~0015714

Code committed to CVS.

  • Added per-page reauthentication to all manage_* pages.
  • Cleaned up numerous pages with excess <?php ?> tags.

Related Changesets

MantisBT: master 820b74fd

2007-09-22 18:51

jreese


Details Diff
For Issue 0008336: Added Auth API code for per-page re-authentication.

git-svn-id: http://mantisbt.svn.sourceforge.net/svnroot/mantisbt/trunk@4588 <a class="text" href="/?p=mantisbt.git;a=object;h=f5dc347c">f5dc347c</a>-c33d-0410-90a0-b07cc1902cb9
Affected Issues
0008336
mod - core/constant_inc.php Diff File
mod - core/authentication_api.php Diff File
mod - lang/strings_english.txt Diff File

MantisBT: master bb4f40f3

2007-09-25 19:52

jreese


Details Diff
Fix 0008336: Require re-login for high impact tasks.
- Added per-page reauthentication to all manage_* pages.
- Cleaned up numerous pages with excess <?php ?> tags.

git-svn-id: http://mantisbt.svn.sourceforge.net/svnroot/mantisbt/trunk@4593 <a class="text" href="/?p=mantisbt.git;a=object;h=f5dc347c">f5dc347c</a>-c33d-0410-90a0-b07cc1902cb9
Affected Issues
0008336
mod - manage_user_create_page.php Diff File
mod - manage_user_update.php Diff File
mod - manage_proj_cat_add.php Diff File
mod - manage_proj_user_remove.php Diff File
mod - manage_user_proj_delete.php Diff File
mod - manage_user_delete.php Diff File
mod - manage_config_revert.php Diff File
mod - manage_proj_custom_field_add_existing.php Diff File
mod - manage_user_proj_add.php Diff File
mod - manage_custom_field_proj_add.php Diff File
mod - manage_custom_field_create.php Diff File
mod - manage_user_create.php Diff File
mod - manage_proj_subproj_add.php Diff File
mod - manage_config_workflow_page.php Diff File
mod - manage_config_email_page.php Diff File
mod - manage_config_work_threshold_page.php Diff File
mod - manage_proj_cat_delete.php Diff File
mod - manage_config_workflow_set.php Diff File
mod - manage_proj_edit_page.php Diff File
mod - manage_proj_ver_update.php Diff File
mod - manage_proj_subproj_delete.php Diff File
mod - manage_config_email_set.php Diff File
mod - manage_proj_delete.php Diff File
mod - manage_proj_create.php Diff File
mod - manage_proj_update.php Diff File
mod - manage_user_edit_page.php Diff File
mod - manage_config_work_threshold_set.php Diff File
mod - manage_user_prune.php Diff File
mod - manage_user_reset.php Diff File
mod - manage_proj_cat_edit_page.php Diff File
mod - manage_custom_field_delete.php Diff File
mod - manage_proj_user_add.php Diff File
mod - manage_proj_custom_field_copy.php Diff File
mod - manage_proj_user_copy.php Diff File
mod - manage_user_page.php Diff File
mod - manage_proj_ver_add.php Diff File
mod - manage_proj_ver_delete.php Diff File
mod - manage_custom_field_update.php Diff File
mod - manage_proj_create_page.php Diff File
mod - manage_proj_ver_copy.php Diff File
mod - manage_proj_page.php Diff File
mod - manage_proj_cat_copy.php Diff File
mod - manage_proj_ver_edit_page.php Diff File
mod - manage_custom_field_page.php Diff File
mod - manage_custom_field_edit_page.php Diff File
mod - manage_proj_cat_update.php Diff File
mod - manage_proj_custom_field_remove.php Diff File