[PHP] My Method Of A Secure Login System

[Breakthrough]

I came up with a method I'm sure is in use somewhere else, but I was just wondering if any of you either used this before, or know of any flaws with this method. Right now I'm working on creating a content management system, and wanted to create the most secure method to login over HTTP.

What I came up with is this:

1. User requests the login page

2. Server generates a random string, sets it in $_SESSION

3. This string is sent to the client's browser in a hidden <input /> tag.

4. When the user logs in, two things happen: First, the SHA1 value of their password is calculated. This value is then appended to the random string from the hidden input, and the SHA1 value of that entire string is taken again. (So the string looks like this - password = sha1(sha1(password) + rand_string) - this is all done client-side, with JavaScript.

5. Now the server has three inputs from $_POST - the username, the double-encrypted password, and the random string. The server first locates the administrator username on the user table, then takes the password (stored as an SHA1 value), and adds the random string to it, and gets the SHA1 value (similar to what was done in JavaScript above). Then it's a simple comparison.

6. For some additional security, the value of the random string is also checked against the $_SESSION variable. Each login attempt clears this variable as well - so the user must continually request the form each time.

Thoughts?

[the evn show]

  Quote

2. Server generates a random string, sets it in $_SESSION

3. This string is sent to the client's browser in a hidden <input /> tag.

6. For some additional security, the value of the random string is also checked against the $_SESSION variable. Each login attempt clears this variable as well - so the user must continually request the form each time.

This is good. It would be easy to extend to do things like requiring a captcha after 5 failed attempts and locking the account after 10 failures.

  Breakthrough said:

4. When the user logs in, two things happen: First, the SHA1 value of their password is calculated. This value is then appended to the random string from the hidden input, and the SHA1 value of that entire string is taken again. (So the string looks like this - password = sha1(sha1(password) + rand_string) - this is all done client-side, with JavaScript.

This part is bad: it doesn't add any real computational complexity when attempting a brute force attack - in fact it may even allow an attacker to reduce the computational intensity. Lets assume you "cap" passwords at a maximum 100 characters in length -- larger would allow for an even bigger key space, but this will do. Let's imagine a user that has a strong password like: "?ber-STRONG#985@Expos?!"

Not knowing what the maximum length of password is allowed to be you have to check the entire range passwords from 0 to 100 characters and each password can contain any of 1000 easily typed UTF characters.

If you hash this and then hash it again you drastically reduce the key space to exactly 40 characters in length containing one of 36 characters (a-z0-9).

Lastly, due to the way you plan to store the password (as an sha1 hash) and the way you're handling authentication (comparing that sha1 hash + string to the user sending you an sha1 hash + string) you are in effect storing it as plain text.

Consider me as an attacker and I've managed to gain access to your database.

• You send me the form, send me a random string, set your session variable.
  
• I decide to log in as the administrator. I read the name/password-hash pair from my copy of the database.
  
• I read your hidden from the hidden input field and runecho "require 'digest/sha1'; puts Digest::SHA1.hexdigest('password'+'secretstring')" | /usr/bin/rubyi>
  
• I HTTP post 'administrator' + 3bb7c55100dd2f4087614311c9c75bfed1e413e6 + 'secretstring' to your login form.
  
• Your server does it's authentication: look up the user, read the hash, add the hash to the string, hash the result, compare that to what I sent:
  
• they match and I get access. No chance for failure, no brute force necessary.
  

  Quote

Thoughts?

It's over complicated.

You started with fairly basic challenge/response but you've over engineered it and accidently reduced it's effectiveness, first by reducing the number of passwords an attacker would try, second by doing extra unnecessary computations operations (which could allow for an easier denial of service by hammering the login server).

Finally, while the "actual" password isn't stored - an attack based on having access to the username/password-hash pairs would be as effective as if you'd just stored plain text password. There's some protection for users who might use the same password on your site as they would on another but no protection from an attacker trying to gain access without really caring what passwords are.

I think your goal was to protect the user's password during authentication while it is in transit between you and the server. For example a user logging in at a Starbucks shouldn't have to worry about the prick in the corner sniffing traffic. While you do accomplish that much it won't offer protection from an attacker attempting to gain unauthorized access when they're able to sniff the HTTP traffic. Imagine we're in a coffee shop and you're logging in to admin your site while I listen-in over in the corner.

You log in and I listen to all the traffic. I won't be able to reverse the login process so I can't just run into the form and mash in your name/password. What I can do is wait for you to send anything to your server and just read your session ID from the cookie or URL (depending on how you encode it) - copy that, and then parade around as you. Once authentication is finished I don't need your password, just the token that says I'm authenticated which you'll send in plain text with every single page request.

Obviously my access will be short-lived (once you log out the session will no longer be valid) but during that time I could use my access to do things like change your password or alter the e-mail address associated with the account. Then I can come back later, reset your password if necessary, and then log in on my own terms.

I'm sure I've missed some things but I think the above is reason enough to consider some serious changes.suggestionsb>

• Pair back the unnecessary hashing: you'll get your keyspace back to "really big" and an attacker with access to your database will still have to brute-force his way in.
  
• Stop worrying about sending things in plain text: it only offers marginal protection anyway (none for you, some for the user)
  
• Use HTTP+SSL to secure your authentication process.
  
  • You'll be leaning on security technology with thousands of eyes looking for flaws which means it'll be much better tested than your own code.
    
  • You'll gain access to optimizations (ie: hardware ssl kit) that aren't available to PHP.
    
  • All traffic will be encrypted including usernames and your secret string so you don't leak parts of the authentication information (users tend to recycle usernames just like they do passwords)
    
  • Your session id will be encrypted which will help protect against session fixation attacks
    
  • Authentication will work on systems that don't support javascript (firefox+noscript users) or that have flakey javascript implementations (smart phones)
    

[Breakthrough]

Thanks for your reply, evn.

There is a lot I just learned in regards to PHP security from what you've shown me... And I did assume throughout this that my database has not been comprimised.

You were correct in stating that I was trying to protect the user's password (as well as the SHA1 hash of the password) during transit... I wasn't as aware, though, of session hijacking being such a big issue - some Googling quickly proved me wrong.

My first ideas were to generate a different session ID on each page load by the logged in user - however, all an attacker would have to do is take the session ID and load a different page before the user could.

I have a shared web host, and nowhere does it say anything about SSL... Is it possible to implement myself, or is it something my host has to do?

[the evn show]

  Quote

And I did assume throughout this that my database has not been comprimised.

That's a bit of a dangerous assumption to make: it's the reason you store hashed passwords instead of plain text versions.

Obviously if an attacker has read/write access to arbitrary fields then all bets are off but you can still plan around mitigating the damage of accidently leaking information. Maybe there's a bug in PHP or MySQL that allows attackers to read data they shouldn't, maybe your code will be vulnerable to SQL Injection: either way it's best to act-as-if people have read access. Good security practices will work in both environments.

  Quote

I have a shared web host, and nowhere does it say anything about SSL... Is it possible to implement myself, or is it something my host has to do?

If you're paying for hosting then I'd surprised if they didn't support it, check the FAQs as they should cover what is need to set it up. For development you can just make your own keys (your browser will raise an alert about untrusted certificate root authority when you do this, but it's fine for building). When you go to deploy to your production server you'll probably want certificates signed by a trusted root - those start at about $20/year. Getting your certificate signed by a trusted root repository means that users won't get that annoying "we can't verify the authenticity of this site, do you want to continue" dialog but otherwise offers no improved security.

If your users don't care about that dialog (ie: corporate users connecting to a private company site) then you can save your money.

[Soyabean]

If your seriously considering making a login secure I would defo consider getting SSL alot of websites are going that way now not just for online purchasing but login into forums and other things. If your designing a new cms you make it login as SSL to change profiles and other what not things then switch from https to http for general use. As the above poster said get a SSL cert there not expensive and please for the sake of GOD do not use a shared CERT its more troubles than its worth.

[Breakthrough]

Theoretically, then, Neowin users are susceptable to these session hijacks as well?

[the evn show]

  Breakthrough said:

Theoretically, then, Neowin users are susceptable to these session hijacks as well?

Yes, it's trivial.

[primexx]

  evn. said:

If you're paying for hosting then I'd surprised if they didn't support it, check the FAQs as they should cover what is need to set it up. For development you can just make your own keys (your browser will raise an alert about untrusted certificate root authority when you do this, but it's fine for building). When you go to deploy to your production server you'll probably want certificates signed by a trusted root - those start at about $20/year. Getting your certificate signed by a trusted root repository means that users won't get that annoying "we can't verify the authenticity of this site, do you want to continue" dialog but otherwise offers no improved security.

hmm I was under the impression that it cost a lot more than that. Where do you get one for $20 a year?

thanks

[deactivated01032015]

Can someone take a look at my php login code and check for any serious flaws?

I only have little experience in PHP programming and security is still something I have to get to.

Tnx.

<?php
	error_reporting(E_ALL);

	session_start();

	if(!get_magic_quotes_gpc())
	{
		$username = strval(mysql_escape_string($_POST['username']));
		$password = strval(mysql_escape_string($_POST['password']));
	}
	else
	{
		$username = strval($_POST['username']);
		$password = strval($_POST['password']);
	}

	require_once("data.php");

	$errors = array();
	$validation = false;

	mysql_pconnect($host, $user, $password) or die("Failed to connect!");
	mysql_select_db($database) or die("Failed to select!");

	if($username == '')
	{
		$errors[] = 'Username missing!';
		$validation = true;
	}
	if($password == '')
	{
		$errors[] = 'Password missing!';
		$validation = true;
	}

	if($validation)
	{
		$_SESSION['ERRORS'] = $errors;
		session_write_close();
		header("location: login.php");
		exit();
	}

	$sql = "SELECT * FROM users WHERE username = '$username' AND password = '".sha1($_POST['password'])."'";
	$result = mysql_query($sql);

	if($result)
	{
		if(mysql_num_rows($result) == 1)
		{
			session_regenerate_id();
			$user = mysql_fetch_assoc($result);
			$_SESSION['ID'] = $user['id'];
			session_write_close();
			header("location: panel.php");
			exit();
		}
		else
		{
			header("location: login.php");
			exit();
		}
	}
	else
	{
		die("Query failed!");
	}
?>

[Breakthrough]

Firstly, these two lines - wrong $password variable?

$password = strval($_POST['password']);
[...]
mysql_pconnect($host, $user, $password) or die("Failed to connect!");

Second, you don't filter any of the user input in the username. The password is fine - you only use the sha1 value. The username is sent to the MySQL query unfiltered though...

Say I put in the following in the username field:

administrator' OR 1=1 #

1=1 is always true, so I will be logged in as the administrator.

See the following pages for how to prevent this:

http://us3.php.net/manual/en/security.data...l-injection.php

http://www.tizag.com/mysqlTutorial/mysql-p...l-injection.php

A simple way to overcome this is to only allow alphanumeric characters in a username. If a username has any characters not in the alphabet or numbers, reject it before allowing it to be queried. The following link is also a useful read.

http://en.wikibooks.org/wiki/PHP_Programming/SQL_Injection

[deactivated01032015]

I basically thought that mysql_escape_string part solves SQL injection.

$username = strval(mysql_escape_string($_POST['username']));

[XerXis]

  Quote

Consider me as an attacker and I've managed to gain access to your database

In that case it already is game over as they could just as easily change the password hash (assuming the attacker has write permissions, which is probable, or has access to the stored procedures writing to the table), so not a valid concern. I agree about the rest though, the method doesn't add any security whatsoever. If you really want a secure login, go the https way. Otherwise, you data will always be vulnerable to sniffing or man in the middle attacks.

Edited August 12, 2009 by XerXis

[XerXis]

  blu3f1sh said:

I basically thought that mysql_escape_string part solves SQL injection.

$username = strval(mysql_escape_string($_POST['username']));

mysql_escape_string has some holes in it, you should use mysql_real_escape_string.

Better yet, stop using the deprecated mysql extension and starting mysqli. If you do that you can use parametrized queries which are:

a) much more orderly and clear to read, even after coming back to your code while you havn't seen it for a few months (believe me, you'll find out what i'm talking about)

b) automatically escaped according to the used charset, which is something mysql_escape_string doesn't

another thing: do NOT rely on magic quotes, disable it in your htaccess file, it has the same security flaws as mysql_escape_string (failing to escape multibyte chars)

[the evn show]

  primexx said:

hmm I was under the impression that it cost a lot more than that. Where do you get one for $20 a year?

thanks

I typo'd and meant $200: that's the fee places like geotrust, thawte, or network solutions changes.

Having said that, IIRC godaddy is selling them for well under $20/year and claim 99% recognition.

  Quote

I basically thought that mysql_escape_string part solves SQL injection.

That can cover some of the more obvious problems (<input typename='admin"; /*'>) but logical errors can still leave you vulnerable to leaking information. A surprisingly common example is a profile update form that isn't anal enough about permission checking. consider a profile form

name: <input text>
password: <input password>
<? if user.admin  ?>
authority <select admin|moderator|memeber|guest>
<? end ?>
<submit>

That's not too out of the ordinary.

If the function it submits to does something like

if http[referrer] = 'mysite.com' {

form_user = new user /* make a new user so we can have the accessors sanitize variables for us */

/* nil values aren't modified by our update routine so we set null to things the user hasn't changed */
form_user.name = post[name] != form_user.name ? post[name] : null
form_user.password = post[password] != null ? hash_password(post[password]) : null
form_user.authority = post[authority] != null ? get_authority_id(post[authority]) : null

logged_in_user = find_user_by_id(session[user_id])  /* so that users can't edit other people */
paramaters_hash = [
  name => form_user.name,
  password => form_user.password,
  authority = form_user.authority,
  profile_last_modified => time(now) /* log the last change so we can see when people edit stuff */
]
update_user_by_id_with_hash(logged_in_user, paramaters_hash)

set_notice('Your profile has been updated')
} else {
  set_warning('you're not allowed to update profiles from a form not hosted on this site')
}

Seems pretty sane and I've seen something like that a few times from people that are too trusting of the user. They look at the above, see that every value is sanitized by their user class, they see that the update_user function properly escapes all values before issuing the query, they see that we pull the ID of the user to update from the session so that they can't just pass id as a part of the query string and edit somebody else, they make sure the form is submitted from their own site and not one I made up and then assume everything is fine.

The obvious problem is that there isn't sufficient checking of permissions on what field a user is allowed to modify. There's the faulty checking of http referrer to try and prevent xss. You can't trust referrer because the user is in control of it and can change it to anything they like, but even if they couldn't you can still just add an <select |admin|moderator|member|guest> field to the form in the browser using safari/firefox/ie's debugging tools.

This is clearly a trivialized example but it's the sort of thing that trips up people trying to write secure code.

All it takes is one minor oversight (in this case not checking logged_in_user.authority = admin before changing form_user.authority) and you could end up leaking information. Once somebody becomes an administrator they might use your CMS to modify a page to do something like "select * from users;" and then dump that do a page. If they're clever they might even delete that page after they've printed it and then they would restore themselves to normal member status. Who would be the wiser?

edit

This sort of thing slips past due to the way people test. Typically they'll have an couple of accounts with different levels of authority. They'll log in a user: try to break the name / password field, make sure they don't see an option to change their authority, make sure that the password doesn't change accidently, and then log out.

They'll come back in with admin - do the same, make sure they can promote themselves down and that when they do they aren't given the option to go back up. They think they're testing the code but it's actually just testing the UI.

Proper automated tests make it much easier to catch that sort of stuff but I've never noticed those to be a big deal for the PHP community.

Edited August 12, 2009 by evn.

[Breakthrough]

evn.: Wonderful points. I don't think I'm going to go the SSL route, though - I just know to be extremely careful when I'm on unencrypted wifi or untrusted networks.

XerXis: I understand the methodology for using the new MySQLi extensions, but how much of a performance overhead is there? A lot (NOT ALL) of object-oriented stuff (e.g. classes) has a lot of overhead compared to procedural programming styles. I faintly remember seeing somewhere that using the MySQL extension vs MySQLi yielded slightly higher performance with PHP, but I don't know how old that was...

[Breakthrough]

Oh, one more thing guys I just stumbled upon:

http://www.w3schools.com/PHP/php_ref_filter.asp

Would those be useful for any user-input before placing the string in a database query? I'm not too familiar with how PHP filters work, but I'm sure that they could be extremely useful in quickly determining if values are what they should be...

[the evn show]

  Breakthrough said:

XerXis: I understand the methodology for using the new MySQLi extensions, but how much of a performance overhead is there? A lot (NOT ALL) of object-oriented stuff (e.g. classes) has a lot of overhead compared to procedural programming styles.

The penalty for method dispatch is orders of magnitude lighter than the hit you're taking for the interpreter (which applies to both). For the majority of tasks the performance penalty from running stuff over the net is going to dwarf even the penalty of using an interpreted language.

some lazy googling turns up results that show the difference is sub-20% (the threshold I'd need to cross before I considered something else). Why so high? Because no matter what you're doing, when performance matters you're going to be caching as aggressively as possible and that means you're not going to be hitting the database 75%+ of the time.

That 20% performance difference only applies to the 25% of the time that you are actually hitting the DB and .25*.2 translates to a 5% difference to the application (give or take depending on cache efficiency). IMO Donald Knuth had it pegged when he said "We should forget about small efficiencies, say about 97% of the time: premature optimization is the root of all evil."

[TonyLock]

So then what would be the best way to encrypt/encode passwords into the database then?

[+theblazingangel MVC]

You've got three options:

1) plain text

2) sha1/md5 hash

3) encryption

Plain text shouldn't ever be used! Should any of the contents of your user table be exposed, by a flaw in your code, an sql error displayed to the user, someone looking over your shoulder as you do maintenance on the database, etc, you've got a serious problem! Also, for user confidence, it's nice that even you and your staff don't know the passwords of your users - users tend to use the same passwords on many sites!

A sha1 hash or encrypted copy of the password is much better. One concern i have with encryption is that it is possible to decrypt, whereas with a sha1 hash, that's impossible! With a sha1 hash it's best to store it with a random salt, that way if two or more people use the same password, anyone looking at the hashes would be none the wiser! - This means that an attacker can't obtain the password from a weaker target to compromise a stronger one, it also means that they can't keep changing their own password to see if there hash matches that of another account!

An example of a sha1 hash implementation:

-----

Creating a hash of a new password:

$salt = substr(md5(uniqid(rand(), true)), 0, 9); //9 is the length of the salt string! (max=32!)

$passwordHash = $salt . sha1($salt . $password);

To check a password, first query the database for the username, and return the password hash. Grab the 9 character salt from the beginning of the existing hash, use this along with the user supplied password: $tempPasswordHash = $salt . sha1($salt . $suppliedPassword); If $tempPasswordHash matches the full hash obtained from the database, the supplied password was correct!

To maintain a login as people browse the website, some people simply store the user' username and password in session variables upon successful login, and recheck it on every page. Personally i don't like this, I prefer generating a 'token', a random sha1 hash, storing it in a field in the user table, and in a session variable, and using that for identification instead! I don't believe it is any less secure, infact more-so since the user's password is not being stored anywhere but the database! It also has the advantage (or possibly disadvantage depending on how you see it), of allowing only one login to an account at a time! - A second login would replace the token and invalidate the first login! Although it's certainly possible to code around this "limitation" if you needed to.

-----

As for recovery of passwords, the sha1 hash cannot be decrypted to send the user their password, however personally i believe this to be a good thing! I don't think decrypting a users' password and sending it to them in an unencrypted email is very wise considering the user may have used it on other sites! How i do it: When a recovery is requested, you should generate a random ID for the request, (like sha1(uniqid(microtime()) . $userID);). Store this along with a timestamp, and the userID in a 'confirmations' table. Send a link containing the confirmationID and userID in an email to the user. If the user follows the link, your script should find the entry in the confirmations table, and check that the timestamp is no more than say 24hrs old. If that was successful, consider it a valid confirmation, generate a random password (e.g. substr(sha1(uniqid(microtime())), 0, 8);), run it through your password hashing routine, to salt and hash it (probably allow a new salt to be generated!), replace the user's password hash with this new one, mark the account to force the user to change there password upon next login, send a copy of this new temporary (unhashed) password to them via email.

There are two avenues of attack here that i can think of, 1) someone guessing the confirmation key. Hopefully a randomly generated 40 char sha1 hash is good enough (evn?), plus we've added the userID into the mix, so that an attacker needs a matching confirmationID and userID pair to make a successful guess! 2) The email with the confirmation link is not encrypted, but i'm not sure that you can do anything about that since most users haven't even heard of pgp...

It's also very important to protect the email address associated with an account! Imagine an attacker got into someones account, if they are able to change the email address, providing the account does not belong to someone with direct database access, game over, the people in charge of the website may not be able to know who the real user is! - They can't just change the email address associated with an account on a whim, just because someone emailed them to say that someone took their account!

The best way i believe of implementing a change email option in a 'user account' section is: 1) require the user be logged in for all of the following stages, 2) require entry of the user' password, along with the new email address, 3) create a confirmation ID, and store the new email address along side it in the 'confirmations' table, 4) send the confirmation email to the current email address, 5) once confirmed, delete the confirmation and send a new confirmation to the new address (to make sure it's valid to make recovery possible!), 6) once that's confirmed, change the user's email address.

If you use the same 'confirmations' for multiple types of confirmations, e.g. password recovery, and change of email, it might be a good idea to throw 'action' into the mix too, so an attacker has to guess a matching confirmationID, and userID, and actionName! I do this, though i never send the action as part of the confirmation url, the scripts use it, so a password recovery script will only find confirmations for password recovery attempts in the confirmations table!

There are many things to think about to create a truly secure site, many of which have been covered already. Simple things like SQL injection XSS and CSRF, to the stuff i've written above, to session fixation attacks, to obscure things like: when a login fails, don't say specifically why! Avoid LIMIT clauses on certain SQL statements, shorter execution times may give things away - e.g. on login, when searching for a valid username, with a LIMIT clause if the username does not exist, it will go through every record, whereas if the username does exist, the query could complete much faster! Remove the limit clause so that it goes through the entire user table every time, so there is no difference in timings! If you display the number of queries executed on the bottom of the page, or script execution time, beware that these might give something away!

Also, never forget that ID's in urls can be changed! There was a big security problem found in an important UK health sector web application (i forget what it was for exactly) not long ago, that allowed user's to access data from other users, simply by changing the userID specified in the url on a certain page, because no checking was done! Never forget this, it is such a basic mistake!

SSL (https) encrytion is always a very important factor! If you use it btw, using it for the login submit only, only prevents someone from getting your login credentials in transmission, if you do not use it on every single other page the user visits while logged in, it is still possible to hi-jack the user' current session!

[the evn show]

The above post is excellent. For as often as I need to implement username/password authentication from scratch I tend to follow the same approach. Popular development frameworks should either bundle or have libraries that implement that are readily available.

This post strays a bit beyond simple authentication and into more general account security concepts towards the end.

  theblazingangel said:

To check a password, first query the database for the username, and return the password hash. Grab the 9 character salt from the beginning of the existing hash, use this along with the user supplied password: $tempPasswordHash = $salt . sha1($salt . $suppliedPassword); If $tempPasswordHash matches the full hash obtained from the database, the supplied password was correct!

One additional benefit is that by salting hashes you make dictionary attacks much more difficult. When presented with a collection of plain SHA/MD5 hashes my first method of attack would be to simply take each hash and compare it to a table of known string/hash pairs. This approach is extremely fast when it works but it relies on the fact that most users have "predictable" passwords: things like normal words ("password), word+single digit (commonly #1 as in "password1"), digit+word ("1password") or word+punctuation(! is common as in "password!").

By salting the password with random characters you reduce the effectiveness of a dictionary based attack. Knowing some information about the algorithm used to salt a password (first 8 characters of the md5 hash of the date the account was created) can allow an attacker some advantage. Still this approach is very valuable and should render dictionary-based attacks ineffective for all but the most well funded and dedicated attackers (like governments). Such a well funded/dedicated attacker is far more likely to just club the intended victim over the head with a pipe wrench to get the information they want.

  Quote

I prefer generating a 'token', a random sha1 hash, storing it in a field in the user table, and in a session variable, and using that for identification instead! I don't believe it is any less secure, infact more-so since the user's password is not being stored anywhere but the database!

Correct. It also becomes impossible to "guess" the token for future log-ins even if I manage to compromise one.

IE: If I sniff the authentication routine and capture a login token at a coffee shop, when you log in tonight it will be different and I will no longer be able to use it as the basis of a session fixation attack.

  Quote

There are two avenues of attack here that i can think of, 1) someone guessing the confirmation key. Hopefully a randomly generated 40 char sha1 hash is good enough (evn?)

Absolutely. There are some attacks on SHA1 that reduce the complexity of attacks but for the most part SHA1 is good enough for the sort of thing we're using it for. IMO a better avenue of attack would be at the random number generators on the server but in order to exploit this you'd need intimate access to the server. Somebody with that sort of access already compromised the system so it's not a huge concern.

  Quote

It's also very important to protect the email address associated with an account! Imagine an attacker got into someones account, if they are able to change the email address, providing the account does not belong to someone with direct database access, game over, the people in charge of the website may not be able to know who the real user is! - They can't just change the email address associated with an account on a whim, just because someone emailed them to say that someone took their account!

The best way i believe of implementing a change email option in a 'user account' section is: 1) require the user be logged in for all of the following stages, 2) require entry of the user' password, along with the new email address, 3) create a confirmation ID, and store the new email address along side it in the 'confirmations' table, 4) send the confirmation email to the current email address, 5) once confirmed, delete the confirmation and send a new confirmation to the new address (to make sure it's valid to make recovery possible!), 6) once that's confirmed, change the user's email address.

I agree up to the point of requiring a password to change a user's e-mail address. The steps after that are a complication that IMO are more likely to do harm than good.

Consider the use case:

• A user has an account and has not forgot his or her password.
  
• That user changed ISPs or mail providers. (ie: they were a .Mac/MobileMe subscriber and have decided that $100/year isn't worth the money. They have changed jobs or re-branded their business and no longer have access to the old domain name).
  
• The user has a new mail provider and wants to update their account.
  
• The user logs into the site, chooses update profile, re-enters their password and changes the e-mail address.
  
• The user can no longer the "stage" 1 confirmation mail and the update process stalls.
  

At this point admin staff would be required to get involved. It'd be unlikely that a site administrator could tell the difference between a legitimate account update request and a fraudulent one made by an attacker with access to the user's password so no real benefit would be offered (admin either says "no, can't verify you" or "sure thing boss" to everyone).

Given that people rarely change e-mail providers unless they have a very good reason?typically losing access to the old one?I don't think this is a good decision.

  Quote

If you use the same 'confirmations' for multiple types of confirmations, e.g. password recovery, and change of email, it might be a good idea to throw 'action' into the mix too, so an attacker has to guess a matching confirmationID, and userID, and actionName! I do this, though i never send the action as part of the confirmation url, the scripts use it, so a password recovery script will only find confirmations for password recovery attempts in the confirmations table!

Some problems I can see here:

• how many confirmation-required actions are you likely to have? 2 or 3 would be exceptional - so an attacker would be able to guess.
  
• We're trying to prevent against unauthorized password changes: we can safely assume the attacker can guess the 'action' field because he or she is the one initiating the action.
  
• Protecting against "intercepting" the confirmation emails without actually initiating the action has limited value because the attacker would need to have access to the victims e-mail address in order to launch this attack and that's enough to invalidate our methods.
  

In light of the above, I think the UI sacrifices outweigh any possible benefit. The major loss is that your e-mails can't contain links with text like "complete the password reset procedure" and that the dialog for resetting a password gains additional complexity (a field to select the action). There is also a fair bit of complexity involved in creating all of this (extra code, extra e-mails to design/write, extra database fields) and that's a lot of overhead for very little if any gain.

IMO adding complexity to a feature used almost exclusively by frustrated users is something that we should avoid unless we have very good reason for it.

[+theblazingangel MVC]

  evn. said:

The above post is excellent.

Thankyou! Yours too! :)

  evn. said:

One additional benefit is that by salting hashes you make dictionary attacks much more difficult.

Good point!

  evn. said:

I agree up to the point of requiring a password to change a user's e-mail address. The steps after that are a complication that IMO are more likely to do harm than good.

...

Given that people rarely change e-mail providers unless they have a very good reason?typically losing access to the old one?I don't think this is a good decision.

Until recently i would not have assumed loosing access to an old email account to have been a common problem, I assumed that everyone ultimately linked everything back to an account provided by their ISP, either directly, or via another email account that is then linked back, and that you can always regain access to the base email account by proving your identity to the provider. However the recent post here by someone with a free aol mail account was enlightning, so is your suggestion that an email account provided by an employer might have been used. I might have to agree with you that a confirmation email to the original address might be harmful. I'd still stand by the second confirmation though - to the new address - what if they changed the email to an invalid address, then later lost access requiring password recovery, which is then not possible.

  evn. said:

Some problems I can see here:

• how many confirmation-required actions are you likely to have? 2 or 3 would be exceptional - so an attacker would be able to guess.
  
• We're trying to prevent against unauthorized password changes: we can safely assume the attacker can guess the 'action' field because he or she is the one initiating the action.
  
• Protecting against "intercepting" the confirmation emails without actually initiating the action has limited value because the attacker would need to have access to the victims e-mail address in order to launch this attack and that's enough to invalidate our methods.
  

In light of the above, I think the UI sacrifices outweigh any possible benefit. The major loss is that your e-mails can't contain links with text like "complete the password reset procedure" and that the dialog for resetting a password gains additional complexity (a field to select the action). There is also a fair bit of complexity involved in creating all of this (extra code, extra e-mails to design/write, extra database fields) and that's a lot of overhead for very little if any gain.

IMO adding complexity to a feature used almost exclusively by frustrated users is something that we should avoid unless we have very good reason for it.

I made a mistake in that part of my post, the attacker should never see the 'action' parameter, nor anyone else. The action parameter should only be used internally by the scripts, so confirmation entries in the confirmations table are only ever looked at by the relevant script.

Thus, should a user say have both a email confirmation and a password recovery confirmation in the table for whatever reason, neither with too old a timestamp, and an attacker tries to guess confirmations (perhaps some idiot trying to cause havoc rather than a real attacker), with say the password recovery script, by submitting url's like www.example.com/passrecovery.php?confID=9f2c3270b56a2efe27de6b7600918b9180973846&uid=34, the uid and the actionID mean that there is only one possible valid entry in the table that they could hit on with this page, per uid!

It doesn't really offer much additional "protection", and the point of it was not really to help with preventing unauthorized password changes, it is really just to help limit the damage idiots could possibly cause, and collateral damage an attacker might cause! Should an attacker target a particular user, while guessing confirmationID's, the uid and actionID prevent accidental confirmation unrelated entries in the table belonging to other users, or scripts!

I'm not sure where you got "a field to select the action" from, perhaps that was a misunderstanding?

[the evn show]

  Quote

I assumed that everyone ultimately linked everything back to an account provided by their ISP

Different circles of typical users I guess. The people I'd have to build something like this for would be employees of companies using a service on behalf of that company (ie: campaignmonitor.com). While best practices would be to have a generic account (it_services@example.com) I've learned from experience that most places don't do that.

  theblazingangel said:

I'd still stand by the second confirmation though - to the new address - what if they changed the email to an invalid address, then later lost access requiring password recovery, which is then not possible.

I can agree to that. Ensuring a valid new address is pretty standard practice and it does make sense to confirm an address if you're going to be making critical functionality depend on being able send email to a user.

  Quote

Should an attacker target a particular user, while guessing confirmationID's, the uid and actionID prevent accidental confirmation unrelated entries in the table belonging to other users, or scripts!

I think I'm missing something obvious here, but I'm not seeing the benefit.

Can we assume the following table?

Users:
---------
name  varchar(50)
email varchar(100)
password_hash varchar(40)
password_salt varchar(8)
confirmation_sent timestamp()
confirmation_code varchar(40)
etc.

(you might choose to split out confirmation related into another table linked back to the user via the user's id, but the queries would be similar but with joins on user_id & confirmation_id).

User requests a password reset:

• Check cofirmation_sent timestamp: if it's in the last day or two then fail with a message saying "You're already trying to reset/change private data. Please wait a while for the confirmation code to arrive"
  
• generate a confirmation code: first few characters of hash(current_time + user's salt). Update user table inserting code and updating the timestamp for confirmation. send the confirmation code to the user
  
• user receives message saying "go to example.com/reset_password?confirm=confirmcodehere"
  
• user visits the link, enters their user id, submits the form.
  
• Reset password form operates a select where user_id=<submitted id> and confirm_code=<submitted confirm> and confirm_set is in the last couple of days.
  
• No user found? fail with uninformative error like "the user id and confirmation pair is invalid."
  
• do standard-issue password reset stuff
  

I'm not sure how you'd use the recorded action to make the above more difficult to mess with.

  Quote

I'm not sure where you got "a field to select the action" from, perhaps that was a misunderstanding?

Yeah: the way I read it you were proposing that any action that required confirmation would require at least three pieces of info:

• User ID
  
• Confirmation Code
  
• the action to be performed (reset password, change email, etc).
  

The user already knows their ID, the user is mailed the confirmation code, and we expected them to remember that 30 seconds ago they were trying to reset their password so they could end up at an appropriate page to complete the action. Needless to say the only interaction I could think of was needlessly complex.

Now that I understand what you intended: I withdraw that comment.

[firey]

XD I thought mine was secure.. the way I did mine.

#1) Remove any spaces in the user or pass (Would break any potential passed in queries)

#2) I just used the built in sql PASSWORD() Function to encrypt my passwords.

#3) Removed any special characters, etc

And instead of pulling back any user info when a person logs in I simply do a row select count the results, if it is a 0 login failed, if it is a 1 then my query:

Select * from user where userLogin = $_POST['username'] and userPass = Password($_POST['password'])

Can anyone see any potential flaws with that? Assuming I have checks throughout each page where passing in a query is stopped?

[the evn show]

  firey said:

#1) Remove any spaces in the user or pass (Would break any potential passed in queries)

#3) Removed any special characters, etc

These two points are bad - they reduce the complexity of the password making an attack orders of magnitude easier.

I'd also be concerned if something as simple as a space could break my queries: you're not doing something silly like $q = "select * from users where name='" . $_post['name'] ?etc.

are you?

Consider the passwordOv!  ?*&r(B- --l o^"wn 1}!/i> - it's pretty damn strong: 34 characters long, mix of letters, symbols, utf characters. Realistically you're not going to be able to crack that with a brute force attempt. Your filtering is going to convert that into "OverBlown" which is a standard word in a password dictionary and that could be compromised in very little time.

  Quote

#2) I just used the built in sql PASSWORD() Function to encrypt my passwords.

I'm not sure what you're using for a database but it's not portable. MySQL has changed the hashing algorithm a number of times and there's no way to be sure that your current db software is going to be using one compatible with whatever you might replace your database with in the future.

Lastly, the sql password() function is a simple unsalted hash so it's susceptible to rainbow table based attacks -- especially considering the earlier steps you took to reduce the possible key-space.

  Quote

And instead of pulling back any user info when a person logs in I simply do a row select count the results, if it is a 0 login failed, if it is a 1 then my query:

The more complex versions we discussed are necessary because we're advocating the use of a salted hash. Any sane application design would have the query abstracted by an appropriate function. IE: if you're using the model-view-controller pattern then the User object will have a class method like find_user_by_name_and_password(name,password) and returns a single user.

  Quote

Assuming I have checks throughout each page where passing in a query is stopped?

That's assumed by default, but that doesn't mean you can't consider leaking information. For example error logs on your server might contain password hashes visible to 3rd parties (other people hosting on your server) or to end users if things end up misconfigured to display detailed errors.

When designing something that requires a strong level of security. Assume your algorithms and database are out in the open: what have you don't to prevent somebody with that information from attacking your software. Hiding things make it more difficult for an attacker but it's not enough.

[+theblazingangel MVC]

  evn. said:

I think I'm missing something obvious here, but I'm not seeing the benefit.

Can we assume the following table?

Users:
---------
name  varchar(50)
email varchar(100)
password_hash varchar(40)
password_salt varchar(8)
confirmation_sent timestamp()
confirmation_code varchar(40)
etc.

(you might choose to split out confirmation related into another table linked back to the user via the user's id, but the queries would be similar but with joins on user_id & confirmation_id).

User requests a password reset:

• Check cofirmation_sent timestamp: if it's in the last day or two then fail with a message saying "You're already trying to reset/change private data. Please wait a while for the confirmation code to arrive"
  
• generate a confirmation code: first few characters of hash(current_time + user's salt). Update user table inserting code and updating the timestamp for confirmation. send the confirmation code to the user
  
• user receives message saying "go to example.com/reset_password?confirm=confirmcodehere"
  
• user visits the link, enters their user id, submits the form.
  
• Reset password form operates a select where user_id=<submitted id> and confirm_code=<submitted confirm> and confirm_set is in the last couple of days.
  
• No user found? fail with uninformative error like "the user id and confirmation pair is invalid."
  
• do standard-issue password reset stuff
  

I'm not sure how you'd use the recorded action to make the above more difficult to mess with.

Ah, well in your case you only support one confirmation at a time, so you're right, storing the action wouldn't help, except maybe to make sure only the correct script handles it (if you have more than one script making them) - one may have stricter rules on time limits maybe, i dunno...

In my case, i had a separate table for storing confirmations, and supported more than one at a time, in which case i think it could be beneficial! I should point out that i've not yet ever done this professionally, and therefore my experience only comes from projects of my own. I've only ever built a login system once, for a large project of mine, which i never had the time to actually finish, so i did not get around to figuring out whether supporting multiple confirmations was actually needed!

[TonyLock]

Has anyone looked into thunder table attacks?

http://blog.crackpassword.com/2009/05/thunder-tables/