improve my authentication security

aquarius

I'm looking for what I can do to enchant security of my auth

<?php
/*
    Start the PHP Session
*/
session_start();

require_once('db.php');
include('functions.php');


/*
    We will use feyd's SHA256 PHP implementation to support SHA256 (does not require mcrypt enabled).
    Depending on where you get your version ensure no echo() statements are left uncommented out
*/
require_once('sha256.inc.php');

/*
    Generate a Challenge hash using feyd's class
*/
$challenge = SHA256::hash(uniqid(mt_rand(), true));

/*
    All new Challenges are given a 5 minute lifetime. Delete anything older then the current time() value
*/
mysql_query("delete from challenge_record where sess_id = '" . session_id() . "' or timestamp < " . time()) or die("Invalid query: " . mysql_error());

/*
    Store our generated Challenge to the database - and give 5 minutes of life before being invalidated
*/
mysql_query("insert into challenge_record (sess_id, challenge, timestamp) values ('". session_id() ."', '". $challenge ."', ". (time() + 360) .")") or die("Invalid query: " . mysql_error());

/*
   Display form
*/





/* if(isset($_POST['Login']))
{
	if($_POST['username']!='' && $_POST['password']!='' && $_POST['imagename']!='')
	{
		//Use the input username and password and check against 'users' table
		$query = mysql_query('SELECT ID, Username, imageid, imagename, Active FROM users WHERE Username = "'.mysql_real_escape_string($_POST['username']).'" AND  Password = "'.mysql_real_escape_string(md5($_POST['password'])).'" AND imageid = "'.mysql_real_escape_string($_POST['imageid']).'" AND imagename = "'.mysql_real_escape_string($_POST['imagename']).'"');

		if(mysql_num_rows($query) == 1)
		{
			$row = mysql_fetch_assoc($query);
			if($row['Active'] == 1)
			{
				session_start();
				$_SESSION['user_id'] = $row['ID'];
				$_SESSION['logged_in'] = TRUE;
				header("Location: members.php");
			}
			else {
				$error = 'Your membership was not activated. Please open the email that we sent and click on the activation link';
			}
		}
		else {		
			$error = 'Login failed !';		
		}
	}
	else {
		$error = 'Please enter both your username, password and image name to access your account';
	}
} */
?>
<?php if(isset($error)){ echo $error;}?>
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
<!--
    Include a javascript implementation of the SHA256 algorithim
    Download from: http://www.mad-teaparty.com/Chrstph/sha256.html
-->
<title>Untitled Document</title>
<script language="javascript" src="sha256.js" type="text/javascript"></script>
<!--
    Include a javascript function to manipulate our form data, i.e. to generate a Response string, delete
    password and challenge prior to allowing submission. Rem: we don't want to send a plain text password!
-->
<script language="javascript" type="text/javascript">
<!--
  function doChallengeResponse() {
    str = document.myForm.username.value.toLowerCase() + ":" +
    sha256_digest(document.myForm.password.value) + ":" +
    document.myForm.challenge.value;
    document.myForm.password.value = "";
    document.myForm.challenge.value = "";
    document.myForm.response.value = sha256_digest(str);
    return false;
  }
// -->
</script>
<script type="text/javascript">
function SendTo(id){
        //shortening form elements
        var myForm      = document.getElementById('myForm');
        var image_id = document.getElementById('imageid');

    image_id.value = id;

    myForm.submit();            

}
		</script>
</head>
<body>
<div id="container">
  <div id="top">
    <h1>Please complete the form bellow</h1>
  </div>
  <div id="leftSide">
  <fieldset>
  <legend>Login details</legend>
  We have added another layer of security to protect your accounts.
  <div style="clear:both;"></div>
  To login:
  <div style="clear:both;"></div>
  1: Enter your username and password
  <div style="clear:both;"></div>
  2: Enter a name for one of the pictures on the right.
  <div style="clear:both;"></div>
  3: Click on that picture.
  <div style="clear:both;"></div>
  Each time you login after the first time, you will need to enter the same picture name and click on the same picture.</font> <br />
  <div style="clear:both;"></div>
  <?php if(isset($error)){ echo $error;}?>
  <!--
        Our form has 5 fields - but only 3 are submitted. The doChallengeResponse() javascript function
        will generate a Response and set it as the value of 'response'. The same function will also unset
        the value of the 'password' field, and 'challenge' field which we DO NOT want sent!

    The javacript function is called when the user submits the form - see the onsubmit tag...
-->
  <form name="myForm" id="myForm" action="dologin.php" method="post" class="form" onsubmit="doChallengeResponse()">
    <label for="username">Username</label>
    <div class="div_texbox">
      <input name="username" type="text" class="username" id="username" size="32"  value="username" />
    </div>
    <label for="password">Password</label>
    <div class="div_texbox">
      <input name="password" type="password" class="password" id="password" size="32"   value="password" />
    </div>
    <label for="imagename">Image Name</label>
    <div class="div_texbox">
      <input name="imagename" type="text" class="imagename" id="imagename" value="Image Name" />
    </div>
    <div class="clear"></div>
    </fieldset>
    <hr size="1" />
    </div>
    <div id="rightSide">
    <p>Confused? Submit your login by clicking on your picture below.</p>
    <p id="flagbar">
      <!--begin submit login images-->
      <input type='hidden' name='imageid' id='imageid' value='00'/>
      <input type='image' name="Login" src='images/securitypics/01.jpg' name='imageid' id='01' onClick="SendTo(01)" value="Login" />
      <input type='image' name="Login" src='images/securitypics/02.jpg' name='imageid' id='01' onClick="SendTo(02)" value="Login" />
      <input type='image' name="Login" src='images/securitypics/03.jpg' name='imageid' id='03' onClick="SendTo(03)" value="Login" />
      <input type='image' name="Login" src='images/securitypics/04.jpg' name='imageid' id='04' onClick="SendTo(04)" value="Login" />
      <input type='image' name="Login" src='images/securitypics/05.jpg' name='imageid' id='05' onClick="SendTo(05)" value="Login" />
      <input type='image' name="Login" src='images/securitypics/06.jpg' name='imageid' id='06' onClick="SendTo(06)" value="Login" />
      <input type='image' name="Login" src='images/securitypics/07.jpg' name='imageid' id='07' onClick="SendTo(07)" value="Login" />
      <input type='image' name="Login" src='images/securitypics/08.jpg' name='imageid' id='08' onClick="SendTo(08)" value="Login" />
      <input type='image' name="Login" src='images/securitypics/09.jpg' name='imageid' id='09' onClick="SendTo(09)" value="Login" />
      <input type='image' name="Login" src='images/securitypics/10.jpg' name='imageid' id='10' onClick="SendTo(10)" value="Login" />
      <input type='image' name="Login" src='images/securitypics/11.jpg' name='imageid' id='11' onClick="SendTo(11)" value="Login" />
      <input type='image' name="Login" src='images/securitypics/12.jpg' name='imageid' id='12' onClick="SendTo(12)" value="Login" />
      <input type='image' name="Login" src='images/securitypics/13.jpg' name='imageid' id='13' onClick="SendTo(13)" value="Login" />
      <input type='image' name="Login" src='images/securitypics/14.jpg' name='imageid' id='14' onClick="SendTo(14)" value="Login" />
      <input type='image' name="Login" src='images/securitypics/15.jpg' name='imageid' id='15' onClick="SendTo(15)" value="Login" />
      <input type='image' name="Login" src='images/securitypics/16.jpg' name='imageid' id='16' onClick="SendTo(16)" value="Login" />
      <input type='image' name="Login" src='images/securitypics/17.jpg' name='imageid' id='17' onClick="SendTo(17)" value="Login" />
      <input type='image' name="Login" src='images/securitypics/18.jpg' name='imageid' id='18' onClick="SendTo(18)" value="Login" />
      <input type='image' name="Login" src='images/securitypics/19.jpg' name='imageid' id='19' onClick="SendTo(19)" value="Login" />
      <input type='image' name="Login" src='images/securitypics/20.jpg' name='imageid' id='20' onClick="SendTo(20)" value="Login" />
      <input type='image' name="Login" src='images/securitypics/21.jpg' name='imageid' id='21' onClick="SendTo(21)" value="Login" />
      <input type='image' name="Login" src='images/securitypics/22.jpg' name='imageid' id='22' onClick="SendTo(22)" value="Login" />
      <input type='image' name="Login" src='images/securitypics/23.jpg' name='imageid' id='23' onClick="SendTo(23)" value="Login" />
      <input type='image' name="Login" src='images/securitypics/24.jpg' name='imageid' id='24' onClick="SendTo(24)" value="Login" />
      <input type='image' name="Login" src='images/securitypics/25.jpg' name='imageid' id='25' onClick="SendTo(25)" value="Login" />
      <!--end submit login images-->
      <!--
        Insert the Challenge value from the server with a small PHP echo()
    -->
      <input type="hidden" name="challenge" id="challenge" value="<?php echo($challenge); ?>" />
      <!--
        Our 'response' field will be filled by the javascript function once the Response string is generated
    -->
      <input type="hidden" name="response" id="response" value="" />
  </form>
  </p>
</div>
<div class="clear"></div>
</div>
</body>
</html>

aquarius

dologin.php

dologin.php (page login submit form is submitted to) 

[code=php]<?php
error_reporting(E_ALL);

session_start(); 


require_once('db.php');
include('functions.php');

/*
    Get our server stored Challenge from the database
    Rem: ensure we only select Challenges which have not timed out!
*/
$query1 = mysql_query("select challenge from challenge_record where sess_id = '" . session_id() . "' and timestamp > " . time()) or die("Invalid query: " . mysql_error());
/*
    Check we got a matching result
    If this is not so, its most likely the Challenge has timed out - user waited too long to submit form
*/
if(mysql_num_rows($query1) == 0)
{
    header('Location: timedout.php'); //simple file with a die() statement - see the download pack
}

/*
    Fetch the array containing the Challenge
*/
$c_array = mysql_fetch_assoc($query1);

/*
    Execute a query to select User data based on the submitted username imaageid and imagename
    Normally we would use some escaping here - its omitted for clarity (is magic_quotes dependent)
*/
	 if(isset($_POST['Login']))

$query2 = mysql_query('SELECT * FROM users WHERE Username = "'.mysql_real_escape_string($_POST['username']).'" AND imageid = "'.mysql_real_escape_string($_POST['imageid']).'" AND imagename = "'.mysql_real_escape_string($_POST['imagename']).'"');

/*
    Fetch the User data into an associative array
*/
$user = mysql_fetch_assoc($query2);

/*
    We're back to worship at the Altar of Feyd ;)
    Include feyd's PHP sha256 implementation
*/
require_once('sha256.inc.php');

$response_string = strtolower($user['Username']).':'.$user['Password'].':'.$c_array['challenge'];
$expected_response = SHA256::hash($response_string);

/*
    Compare the actual client Response hash against our expected Response hash
    1. If they match, we will authenticate the user
*/	

if($_POST['response'] == $expected_response)
{
				if($user['Active'] == 1)
				{
					$_SESSION['user_id'] = $user['ID'];
					$_SESSION['logged_in'] = TRUE;
					header("Location: members.php");
					echo 'user logged in fine';


}
				else {
					$error = 'Your membership was not activated. Please open the email that we sent and click on the activation link';
					    header('Location: notactive.php'); //simple file with a die() statement - see the download pack
				}
			}

 print_r($_POST);

[/code]

alimadzi

You should consider parameterized queries using PDO, Pear:😃B, ADODB, or one of the other DB layers. You should also turn off all error reporting in a production environment.

MarkR

You should NOT turn off error reporting in a production environment. Only turn off error DISPLAY in a production environment.

Errors will happen in production and it's very important to have them recorded. Obviously showing too much information to the (ab)user could be a security risk, so don't do that.

Don't "turn off all error reporting" in any environment at all - it's thoroughly useless.

Mark

alimadzi

MarkR wrote:
You should NOT turn off error reporting in a production environment. Only turn off error DISPLAY in a production environment.

Errors will happen in production and it's very important to have them recorded. Obviously showing too much information to the (ab)user could be a security risk, so don't do that.

Don't "turn off all error reporting" in any environment at all - it's thoroughly useless.

Mark

Good point, Mark. I should have been clearer. I just read a ZF tutorial that talked about syndicating application errors as an RSS feed. Cool stuff.