Please Critique my PHP authentication efforts

Question

After posting this a while back, I decided to create my own Registration / Authentication capability in PHP. I'd love anyone to point out the flaws / opportunities for improvement, particularly around what's stored in the session...

The logical flow is:

1 - User Registers using email as username, a "site name" which then forms part of any url which they will have access to and a password of at least 6 characters which must contain letters and numbers (I know this could be stronger)

2 - Provided the user and site are unique, I then store both of those, along with a randomly generated string (salt) in a row in the auth table in my database. I then take the users password, concatenate the salt to it, and store an md5 hash of this salted password in the same database row

3 - When a user then logs in, I take the password she's entered and concatenate the salt to it, create an md5 hash of that, and compare it to what I have stored in the database - if they match, the user has entered the right password, and their username is written to the session

4 - On every request, I use the username stored in the session to query the database and read the site name associated with this user. I then compare this to the site name in the url itself, and if they match I set a variable which is accessible to the rest or of the script (not a global variable, it's just readable by my controller which decides if a user can see a particular page) if the two site names don't match, the user is redirected back to login

My concern is could someone write to the session, and thus be able to access peoples pages if they know the username they signed up with? How would you go about preventing this?

Before anyone accuses me of negligence by the way this is a personal project for learning - I'm not exposing any clients data!

Answer 1

1. Why 6 characters? Make it bigger and require a minimum of 6 (or more) characters. There is no excuse for limiting the number of characters in the password to 6.

2. Put more than the user name in the session. But to do this safely, you must change the salt every login:

A- From the login page: Take name and password verify with existing salt. If valid update the user table salt and password with a new salt (you have the password from the user so you can md5 it and the salt again). Write the md5 of the password to the session. B- From any other page: compare the user and hashed password against the database. If they don't match, redirect to the login page.

The flaw with this idea is the user cannot maintain logins on multiple machines/browsers.

Your registration system needs work to. How do you know the email address is valid? How do you know the user registering owns the email address? You must send email to the address containing a link back to your site which must be clicked before you allow the account access to anything. Otherwise someone can sign up under someone else's email address make fraudulent claims as that person or just cause your site to spam that person getting your site shut down.

You also might want to investigate CAPTCHA to limit scripted registrations.

Answer 2

Short answer: looks good!

Long answer:

1. Yes, you should allow stronger/longer passwords, but otherwise this is ok. Just make sure you accept only valid url-part characters (I'd stick with the PCRE word characters) for the site name. However, a "validate this email" system would be appropriate, to make sure the registrant actually controls the provided email address.
2. Looks good. I like sha1 better than md5, though.
3. As long as your sessions are safe, you can store more than just the username (which would be a fairly expensive lookup to SQL for every page request - go ahead and store their PK).
4. Looks good, but should be adjusted per my comments in #3

To make sure you are handling your session security properly, check the guide on PHPSec.

Answer 3

• Limit the velocity on logins. Log each attempt for a user and lock them out after so many failed attempts. As the failed attempts mount up, make the lock out longer and longer.

• Add a salt in your code that is static and use it in combination with the salt from the database. Then if you db gets hacked, they still don't have the salt from the code. This salt can't / shouldn't change.

• Can users retrieve passwords / reset lockouts? you will need to collect challenge questions and answers.

• When users reset their passwords, do they have to know the original one?

If this a secure site, or just a site that tracks someone. I know of sites that you can take your cookie from machine to machine to login. It always remembers you, but is just a forum so the potential for trouble is low.

Why salt the code as well as the database? Once your database is hacked your site is toast. However seeing users tend to use the same password on many sites, no sense in helping hack everybody's property. If they get your code too then the royal screwing happens, but lets put up as many barriers as we can.

Security through obscurity is dumb, but many layers of security can help.

Regarding putting username in session Hash the username, url and a salt. Store that in the database and in the session. use that as authentication, if that isn't valid dump them to the login. they can't copy the cookie to another site, they won't be exposing their username as much, and it eliminates a query.

You can even regenerate that salted value every X pages views and store in the session to expire it and make stealing it less useful over time. You then would store two salts in your database. One for the password, one for the authentication session value.