2

this is what I'm doing currently to create sessions on login page.

if($count==1) {

   $_SESSION['username'] = $username;
   $_SESSION['password'] = $password;
}

i know this is very basic and I need to protect the user sessions. Can u provide me some basic tips. If you could edit the code and write the secure one, it would be great. Thanks.

Currently, I am using the email address as session username.

sarthak
  • 299
  • 1
  • 7
  • 16

3 Answers3

6

Ask your self this question:

  • Why am I storing the password when the username is unique in the database

After you have answered that you should of come to the conclusion that its pointless, you can either store the username or the user id in the session when it comes to login systems.

How login systems tend to work is that the user sends the username password from a form to the server where its validated, during the validation process you select the user from from the database where username = post_username.

If there is no rows found the user does not exists so you can directly send output at that point, if the user does exist you then compare the password with the post_password.

the reason why we specifically select the row by just the username is that you should be incorporating some sort of hashing system to add extra security.

if you stored the password as (password + hash) which would be a new string, you would also store just the hash aswell, thus if a user is found then you can create a hash from (post_password + db_hash) and check to see if its the same as the db_password.

this way if your database gets leaked somehow your users credentials are more secure.

once the user has been validated you would store the user id within the session, and then on every page load you can check if the id is within the session and if it is the user is currently logged in and you can select the users data by SELECT * FROM users WHERE id = session_id.

This should get you started.

RobertPitt
  • 56,863
  • 21
  • 114
  • 161
1

Common practice is to check the user name and password against the database, then on success store just the user id in the session. Then later, to see if a person is logged in or authorized, you check that user id stored in the session. Though, the session variables are only visible to the server unless you've done something horribly wrong. So its not horrible or insecure but its basically unnecessary.

Edit

Removed bit about cookies, could cause confusion.

profitphp
  • 8,104
  • 2
  • 28
  • 21
  • why can't I simply store the username instead of an id? I am not using a cookie. – sarthak Jan 14 '11 at 21:26
  • @sarthak: because it's generally better and faster to query a table by its primary key instead of a group of another columns. If you're using this session for authentication, it implies you're also using a cookie, otherwise it doesn't make sense. – jweyrich Jan 14 '11 at 21:32
  • 1
    You could, but I wouldn't unless its your primary key, and you usually want an INT type for your primary key. – profitphp Jan 14 '11 at 21:32
  • @jweyrich, the username is unique, it can be primary, it resembles the same factors as an interger based id. you have no valid reason to store the id over the username as they both have the same properties bar string vs int, in fact by using an id aswell as a username your using more storage as you have an extra column in your database for no real good reason. – RobertPitt Jan 14 '11 at 21:37
  • @profitphp: storing the user ID directly on the cookie is an invitation for an impersonation attack and a possible privilege escalation. Be aware that users are able to change the ID present on the cookie, and thus login as someone else's account. – jweyrich Jan 14 '11 at 21:38
  • @robertpitt Sure you CAN, but its more efficient for sorts and comparisons if you use an INT for the primary key. – profitphp Jan 14 '11 at 21:40
  • @jweyrich, your user id is `298054`, the fact that SO does not hide this integer is the fact that it really makes no difference, 90% of sites how the user id. – RobertPitt Jan 14 '11 at 21:41
  • @RobertPitt: any non-duplicate field has potential to become a primary key, but this doesn't imply they should. And my point about better performance is still valid. – jweyrich Jan 14 '11 at 21:41
  • @jweyrich I didnt say to log them in with it, its just so it remembers who they are. – profitphp Jan 14 '11 at 21:41
  • @profitphp: it would still be susceptible. You can change its value at any time. I think it would be good question for [security](http://security.stackexchange.com), if such doesn't already exist. – jweyrich Jan 14 '11 at 21:44
  • The primary key will be a clustered index and thus search for a users details via their username will be very quick, It will stop duplicate usernames from appearing, You don't have to worry about using two different peices of information (username or id).. It will make writing code much easier because of not having to lookup two bits of information. – RobertPitt Jan 14 '11 at 21:45
  • @jweyrich and accomplish what? Its still going to make you login. I guess you could store the username instead of the id. Because they could just put in any number, and be able to pull out usernames. – profitphp Jan 14 '11 at 21:45
  • @jweyrich It doesn't matter if you can change it any time, its not used ever, except when they first go to the login screen, and it prefills the username. You'd never use it for authentication. – profitphp Jan 14 '11 at 21:49
  • @profitphp: if you're not going to use it after the login, then it serves no purpose and therefore shouldn't exist. – jweyrich Jan 14 '11 at 21:50
  • @jwetrich, please do not take my comments as argumental, I just think that `ID Vs Username` should be respected as a valid debate: see http://stackoverflow.com/questions/4911/asp-net-authentication-user-name-vs-user-id – RobertPitt Jan 14 '11 at 21:52
  • @RobertPitt: I doubt any implementation would not need to fetch other columns as well, or even `JOIN` against other tables (think of permissions). Are you going to `JOIN` using the `username` column? Even if it's indexed, I wouldn't do that because user might change his username and you'd have to update various tables. – jweyrich Jan 14 '11 at 21:53
  • @jweyrich it serves a purpose, its so when they leave for a week, and come back, it remembers who they are. Ever seen a checkbox that says 'remember me' on a website? Thats how it works. Except it would be better to store a username instead of an ID, so there is no potential for info leak. – profitphp Jan 14 '11 at 21:53
  • @profitphp: "remembe me" without auto-login makes no sense to me. Does it to you? – jweyrich Jan 14 '11 at 21:55
  • usernames should not really be updated, its the display name that would usually get modified. – RobertPitt Jan 14 '11 at 21:55
  • @jweyrich ya, saves you from having to remember your stupid username for some random site you rarely use. And hopefully you remember the password when you see the username already in the box. Plenty of sites have just that, 2 boxes, one says remember me, one says keep me logged in. – profitphp Jan 14 '11 at 21:56
  • @profitphp: I wouldn't bother implementing such thing, but if you think it's useful and worth the time, I'm okay with it. All good. – jweyrich Jan 14 '11 at 21:59
  • @jwerich https://secure.newegg.com/NewMyAccount/AccountLogin.aspx They dont even have the auto login option. Just something that pre-fills the login box. Its not just my retarded idea, hehe. And i'm with you about an int being the primary key. – profitphp Jan 14 '11 at 22:00
  • @RobertPitt: I agree that in this point of view it doesn't matter what is going to be the PK (username or id), but didn't you ever wanted to change your login (not display name) at some site? I guess I'll stop here, not by lack of arguments, but this is becoming a rather extensive discussion with no real gain. I'll take one last comment from each one, if you wish so. – jweyrich Jan 14 '11 at 22:02
  • Ya, that got a little out of hand, i was contemplating removing the post since it didnt even get voted. – profitphp Jan 14 '11 at 22:05
  • Your point is valid in regards to changing the username, but thats a very small percentage of users who would need to change there username. i think integer based PK is ok but has uneeded overhead, but Gaines in keeping the username modifiable, but to be honest thats probably the only valid reason why you should have id over username based PK. peace and thanks for the debate :) – RobertPitt Jan 14 '11 at 22:08
  • @RobertPitt: Sure, I enjoyed. Good arguments from your side. I also liked the related question you pointed. Thank you! Peace. – jweyrich Jan 14 '11 at 22:14
1
/*
    SecureSession class
    Written by Vagharshak Tozalakyan <vagh@armdex.com>
    Released under GNU Public License
*/

class SecureSession {
    // Include browser name in fingerprint?
    var $check_browser = true;
    // How many numbers from IP use in fingerprint?
    var $check_ip_blocks = 0;
    // Control word - any word you want.
    var $secure_word = 'random_string_here';
    // Regenerate session ID to prevent fixation attacks?
    var $regenerate_id = true;

    // Call this when init session.
    function Open()
    {
        $_SESSION['ss_fprint'] = $this->_Fingerprint();
        $this->_RegenerateId();
    }

    // Call this to check session.
    function Check()
    {
        $this->_RegenerateId();
        return (isset($_SESSION['ss_fprint'])

        && $_SESSION['ss_fprint'] == $this->_Fingerprint());
    }

    function Destroy()
    {
        // Unset all of the session variables.
        $_SESSION = array();

        // If it's desired to kill the session, also delete the session cookie.
        // Note: This will destroy the session, and not just the session data!
        if (isset($_COOKIE[session_name()])) {
            setcookie(session_name(), '', time()-42000, '/');
        }

        // Finally, destroy the session.
        session_destroy();
    }


    // Internal function. Returns MD5 from fingerprint.
    function _Fingerprint()
    {
        $fingerprint = $this->secure_word;
        if ($this->check_browser)
            $fingerprint .= $_SERVER['HTTP_USER_AGENT'];

        if ($this->check_ip_blocks)
        {
            $num_blocks = abs(intval($this->check_ip_blocks));

            if ($num_blocks > 4)
                $num_blocks = 4;

            $blocks = explode('.', $_SERVER['REMOTE_ADDR']);

            for ($i=0; $i<$num_blocks; $i++)
            {
                $fingerprint .= $blocks[$i] . '.';
            }
        }
        return md5($fingerprint);
    }

    // Internal function. Regenerates session ID if possible.
    function _RegenerateId()
    {
        if ($this->regenerate_id && function_exists('session_regenerate_id'))
            session_regenerate_id();

    }
}
dqhendricks
  • 19,030
  • 11
  • 50
  • 83
  • handly gnu class for creating, checking, and destroying secure sessions. – dqhendricks Jan 14 '11 at 21:35
  • If I want to set a time for logout then how to do it ? Like my website user admin should logout after 15 minutes of inactivity, how to set this logout time in this class ? – sqlchild Feb 14 '15 at 08:34