Safety paranoia, login script

ways to make a login script as safe as possible

[punisa]

I constructed a login script for our employees.
Everything works smooth, but considering that we are dealing with sensitive company info I started researching deeper into security.
Once reading about all theories of hacking and stealing session ids and so on, I'm totally paranoid : P

If you don't mind, I'll attach my current login page, just to see if you guys spot any major flaws. I'd be very grateful for any ideas you may have !

As you see I've improvised a lot, hoping to boost my security. But I'm not entirely sure if I did any good work.

I've also read about session IDs and them being stored in some temp directory. I'll be honest, I have no clue what they are and where they are stored : (
Also I've read that it would be wise to check user-agent before login? Again I'm clueless on this..

Anyway, here is my code so far:
(I've also added small comments in the code for you)

my login page:

<?php session_start();
if (isset($_SESSION['username'])) {
//output stuff goes here...
}
else {
echo 'not logged in:
<form method="post" >
username:<input type="text" name="username" maxlength="255" value="">
<br>
password:<input type="password" name="password" maxlength="50">
<input type="submit" class="submit" name="action" value="Login">
</form>';
}

my login script:
(I placed it above my root folder and gave it a "funny" name, it is not called login.php lol)

<?php
// somewhere I've read about brute force cracking password, thus I've put a 1 second sleep before processing info. Good or useless idea?
sleep(1);
define('SQL_HOST','localhost');
define('SQL_USER','username');
define('SQL_PASS','longpassssword');
define('SQL_DB','companybase');
$conn = mysql_connect(SQL_HOST, SQL_USER, SQL_PASS)
or die('Could not connect to the database; ' . mysql_error());
mysql_select_db(SQL_DB, $conn)
or die('Could not select database; ' . mysql_error());
function redirect($url) {
if (!headers_sent()) {
header('Location: http://' . $_SERVER['HTTP_HOST'] .
dirname($_SERVER['PHP_SELF']) . '/' . $url);
} else {
die('Could not redirect; Headers already sent (output).');
}
}
if (isset($_REQUEST['action'])) {
switch ($_REQUEST['action']) {
case 'Login':
if (isset($_POST['username'])
and isset($_POST['password']))
{
// added this function to check if any BAD characters are in there
function letters($string) {
$eregi = eregi_replace("([A-Z0-9]+)","",$string);
if(empty($eregi)){
return true;
}
return false;
}
$keywords = $_POST['password'];
if(letters($keywords)) {
$go1 = "ok";
}
else{
$go1 = "no";
}
$keywords = $_POST['username'];
if(letters($keywords)) {
$go2 = "ok";
}
else{
$go2 = "no";
}
// if both check went well...
if (($go1=="ok")&&($go2=="ok")){
$pass=$_POST['password'];
$salt='a looooooooooooooong salt goes here';
$salted_md5=md5($salt.$pass);
$sql = "SELECT * FROM login WHERE username='" . $_POST['username'] . "' AND password='" . $salted_md5 . "'";
$result = mysql_query($sql, $conn)
or die('Could not look up user information; ' .
mysql_error());
if ($row = mysql_fetch_array($result)) {
session_start();
$_SESSION['username'] = $row['username'];
$_SESSION['realname'] = $row['realname'];
$_SESSION['level'] = $row['level'];
}
}
}
redirect('mainpanel.php');
break;
case 'Logout':
session_start();
session_unset();
session_destroy();
redirect('mainpanel.php');
break;
}
}
?>

Thanks for reading ! : D

[blang]

I've also read about session IDs and them being stored in some temp directory. I'll be honest, I have no clue what they are and where they are stored

Check your php.ini config file. If you're on a UNIX like server platform (Linux,BSD,MacOSX), then you'll probably find session data stored in the /tmp directory. If you're on the Windows platform, your session data will probably be stored in the user's "temp" directory (i.e. the user that is running the httpd process), or perhaps C:\Windows\Temp or another directory defined in the php.ini config. They're just plain ASCII files that store all the session data in plain text, and usually start with "sess_" followed by the MD5 hash session id value (or SHA1 value if you've selected that).

Being that you're not sure where the session data is even stored, you may not know much more about the server configuration. If you're on a "shared server" hosting account, then the session data is typically in the /tmp or /var/tmp directories, and available to anyone that has an account with that host (on that server) to look at your session data. BE SURE YOU DO NOT STORE ANY SENSITIVE DATA IN THE SESSION.

// somewhere I've read about brute force cracking password, 
// thus I've put a 1 second sleep before processing info. 
// Good or useless idea? 
sleep(1);

I'm sorry to say it's pretty much useless. That will literally slow the process by 1 second, but that's it. If you want to protect against brute force logins, you can build in a mechanism into the session data and database that checks how many times this specific user has attempted to log on. Throttle their attempts after let's say 5 tries in a certain amount of time, and either make them wait an hour, 24 hours or indefinitely until they are forced to change a password via their email address, depending on how strict you want to make the process.

define('SQL_HOST','localhost'); 
define('SQL_USER','username'); 
define('SQL_PASS','longpassssword'); 
define('SQL_DB','companybase'); 
$conn = mysql_connect(SQL_HOST, SQL_USER, SQL_PASS) 
or die('Could not connect to the database; ' . mysql_error()); 
mysql_select_db(SQL_DB, $conn) 
or die('Could not select database; ' . mysql_error());  

Take all the database connection code and push it into a file outside the web root. While you're at it, find a way to log errors or at least avoid showing them to the user. Exposing database information to the user is a great way to help them crack your system.

// added this function to check if any BAD characters are in there 
function letters($string) { 
$eregi = eregi_replace("([A-Z0-9]+)","",$string); 
if(empty($eregi)){ 
return true; 
} 
return false; 
} 

This wrapper function should be replaced by a single call to ctype_alnum() [us2.php.net].

// if both check went well... 
if (($go1=="ok")&&($go2=="ok")){  

...and this conditional can then be simply...

if ( ctype_alnum($_POST['username']) 
 && ctype_alnum($_POST['password']) ) { 

$sql = "SELECT * FROM login WHERE username='" . $_POST['username'] . "' AND password='" . $salted_md5 . "'"; 
$result = mysql_query($sql, $conn) 
or die('Could not look up user information; ' . 
mysql_error());  

More opportunities to make the script more efficient and less exposed to attack. First off, don't use SELECT ALL ('*'), use SELECT COUNT(*) and simply retrieve a single value, the count of the matching records. There should obviously be only 1 that match the login data. If there is 0, bad login. If there is more than 1, bad application and database design. There's really not much need to pull data out of the database at this point, we're just authenticating the login. If you find you really need data, you can use a statement to use a COUNT() and also retrieve record data, or make a second (small) SELECT statement. For example, you pull down ALL records and only use 3 columns of information, 1 of which we already know (the username value).

Secondly, take out that call to mysql_error() reporting the error output from the login attempt. This is the worst place to allow the user to see what causes the script to error out, i.e. column names that store the login info.

The use of a salted hash is always a good idea, but I might go with SHA256 or a stronger hashing mechanism.

Oh, and session_start() should simply be at the top of the script, since you use it in either event (login or logout).

[blang]

Also I've read that it would be wise to check user-agent before login? Again I'm clueless on this.

Missed this on the first pass through. The user-agent string can be spoofed, so this is also pretty much useless. IP addresses, unless you're on a controlled LAN, are useless. Using JavaScript to control login attempts is useless (as it can be circumvented or just plain turned off).

[punisa]

Hey Blang, thank you so much for your detailed input! You really showed me some stuff I was never aware of. For example I never knew that adding mysql_error() part could be a bad thing.
I'll remove it right away.
Also I never heard of ctype_alnum() function, thanks : )

You also mention that I never should store any sensitive data in my session. What kind of data do you mean?

As far as I know, I store these variables:
$_SESSION['username'] (eg marc_black)
$_SESSION['realname'] (eg Marc Black)
$_SESSION['level'] (eg 3)

This data is just rudimentary way to show certain elements on site, like "welcome Marc Black" etc.
Dunno If any other info is being exposed without me declaring it, hope not : )

[londrum]

if it's for your employees on some kind of intranet, then maybe you could tie it down to specific IPs.
you could even give the company's browsers specific user_agent strings -- which are simple enough to change yourself. if you make them something unguessable then the chances of someone outside the company logging in will be remote.

[blang]

You also mention that I never should store any sensitive data in my session. What kind of data do you mean?

As far as I know, I store these variables:
$_SESSION['username'] (eg marc_black)
$_SESSION['realname'] (eg [/quMarc Black)
$_SESSION['level'] (eg 3)

Well, usernames and "real names" aren't too bad, but again, if you're on a shared server, exposing details such as these could be a bad thing (especially if your post-login authentication uses the username). What I prefer is to have a `login` table, separate from the `user` table, that stores login data. For example, when the authentication process is complete, create a token (maybe the user's ID value, username, and some random value passed through MD5) and store it in the `login` table and in the session. The `login` table also stores the `user`.`ID` value (assuming you have such a field) along with whatever other pertinent data (last visit / login time, etc). Then you can link back to the `user` table on each subsequent page request to retrieve the username, "real name", "level" value, etc.