views:

669

answers:

6

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!

+1  A: 

If anyone is watching the registration form submission then they have the information you're concerned they'll figure out. First step: Use HTTPS for your forms.

As for the encrypted db stuff, I'll let someone else take issue with it. :-)

Nerdling
+8  A: 
  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.

jmucchiello
Thanks Joe - Re 6 charachters, it is already at least 6 rather than exactly 6, I'll edit to clarify. Re writing the password to the session, is the idea to write somethign different each time, so that stolen session data is only good until a re login?
Rob Y
Yes. Otherwise someone can reuse the session forever (or until the user changes his password. Some would say the value should change every page but then you need to store the raw password in the session. That makes less sense to me.
jmucchiello
+3  A: 

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.

Peter Bailey
+3  A: 
  • 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.

MrChrister
I have to say, although what you say in your 2nd bullet is true, once the database has been hacked the game is kinda up by then, no? =P
Peter Bailey
Retrive password - a new one is emailed to users address and is forced to change at next loginUsers do need to know existing p/w to change
Rob Y
Updated my thoughts about salting the code
MrChrister
There's some good stuff in this answer too.
jmucchiello
A: 

If I get the flow right, then if I know your username and site name, go to the site name and give you a cookie with the username (since sessions, after all, are saved as cookies), I'm in - no password needed? Isn't that a flaw?

Liorsion
I've looked at this, and as far as I can see, the session ID is stored as a cookie, but none of the variables in it?
Rob Y
Your 4th point said that "On every request, I use the username stored in the session" (which can bo forged) and then compare this to the site name in the url itself. So if I forge them both - I'm in.
Liorsion
+1  A: 

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?

I'm not sure what you mean. How would somebody "write to the session"? The session is [usually] a file stored on the server that the client cannot view or modify.

If you are concerned about session hijacking (aka fixation), then you need to enable SSL and disable session IDs in the URL string. (See php.ini's session.use_only_cookies)

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

This sounds like the biggest concern. What does the site name look like? How exactly are you matching it against the URL? With a regex?

Some more details on this aspect would be good, because if your access control is flawed then the quality of your authentication doesn't matter too much.

mehaase