1

IMPORTANT

After following the advice in the answers below, the client was able to log-in without any problems but did not attempt to actually navigate the secured pages. When he attempted to do so later, he was returned to log-in as before with the "Please log in" error. After much head scratching, something incredibly simple came to mind - the client was accessing the site with http://www.example.com/admin and everything in the login script was redirecting to http://example.com, so the session cookie that it was looking for was set for another domain. This also explains why he had problems logging in the first time but not subsequent times - the script redirected him to the log-in form without the www.

A quick fix was to write a .htaccess file to remove the www, problem solved. Of course this could also be handled within the login script, which I will improve for future use.

ORIGINAL POST

I develop PHP and MySQL based sites with a home brew CMS and log-in system. My CMS is unique to every client and it has been quite the crowd pleaser - unfortunately the same is not true of my log-in system. The following is a long post but I need to cover the details to try and find a solution. Bear with me..

The system is fairly straight forward, if not a bit hefty. Each user has a salted hash stored in a MySQL table, alongside the SALT. When the user logs in, their SALT is retrieved and the submitted password becomes a salted hash.

If the submitted salted hash matches the one stored in the table, the user is authenticated. Details such as their name, last IP address, and account level (3 levels on most sites) are stored in an array assigned to a session variable. They are then redirected to the landing page of the restricted site to which they logged in (Members Only or Admin/CMS).

Secured pages include a smaller auth.php file that checks to see if the session variable containing their details is present. If not, they are redirected to that site's log-in form with an error message that reads "Please log-in." If it is present, they are allowed to continue and the details stored in the array are assigned to variables.

The problem that many users has reported is that they often need to log-in several times to keep from being bounced back to the log-in form with the "Please log-in" error message, or they navigate to another page in the secure site and randomly get bounced back to login with the same error. So, the session variable seems to either not be getting set or it is being cleared for some reason during normal use of the site.

The first problem has NEVER happened to me - over a multitude of devices and networks - and I have witnessed it at a client's office using their laptop. I had them connect to my mobile hotspot and there was no change. However, they were able to log in without any problems using my laptop and my hotspot connection. Unfortunately, I was not able to connect to their network using my laptop, so that variable could not be ruled out.

*NOTE - * I forgot to mention initially that the system works normally for problem clients after they have logged in two or three times with the correct credentials. Subsequent log-in attempts while their browser remains open tend to execute without problems thereafter. Also, the log-in page destroys the session.

Here is the code for each stage, starting with the log-in script:

login.php

<?php
putenv("TZ=US/Eastern");

if (array_key_exists('site', $_POST)) {
    $authenticate = new loginUser($_POST['username'], $_POST['password'], $_POST['site'], $_SERVER['REMOTE_ADDR']);
}
//Authenticate and log-in
class loginUser {
    private $memDB, $username, $password, $site, $ip_address;

     //Clean input variables
    private function clean($str) {
        $str = @trim($str);
        if(get_magic_quotes_gpc()) {
            $str = stripslashes($str);
        }
        return $str;
    }
    //Construct variables
    function __construct($username, $password, $site, $ip_address) {
    session_start();
        $this->memDB = new PDO('mysql:host=localhost;dbname=exampleDB', 'exampleUser', 'examplePassword');
        $this->username = $this->clean($username);
        $this->password = $this->clean($password);
        $this->site = $site;
        $this->ip_address = $ip_address;
    $this->authUser();
    }
    //Validate username
    private function validateUsername($username) {

        $checkUsername = $this->memDB->prepare("SELECT COUNT(*) FROM accounts WHERE username = ?");
        $checkUsername->execute(array($username));
        return $checkUsername->fetchColumn();
    }
    //Obtain and set account details
    private function accountDetails() {

        $fetchAccountDetails = $this->memDB->prepare("SELECT id, name_f, name_l, ipAddr, lastLogin, accountLevel, isActive 
        FROM accounts WHERE username = ?");
        $fetchAccountDetails->execute(array($this->username));
        $accountDetails = $fetchAccountDetails->fetch();
        $this->updateLogin();
        return $accountDetails;
    }
    //Update last login details
    private function updateLogin() {

        $updateLogin = $this->memDB->prepare("UPDATE accounts SET ipAddr = ?, lastLogin = DATE_ADD(NOW(), INTERVAL 1 HOUR) WHERE username = ?");
        $updateLogin->execute(array($this->ip_address, $this->username));
    }
    public function authUser() {

        $loginErr = array(); //Array for holding login error message
        $loginErrFlag = false; //Boolean for error
        //Validate submitted $_POST elements
        if (!$this->username) {
            $loginErr[] = "Username missing";
            $loginErrFlag = true;
        }
        if (!$this->password) {
            $loginErr[] = "Password missing";
            $loginErrFlag = true;
        }
        if ($this->username && $this->validateUsername($this->username) == 0) {
            $loginErr[] = "Username invalid";
            $loginErrFlag = true;
        }
        if (!$loginErrFlag) {
            //Fetch the password and SALT to compare to entered password
            $validatePW = $this->memDB->prepare("SELECT password, salt FROM accounts WHERE username = ? LIMIT 1");
            $validatePW->execute(array($this->username));
            $passwordResult = $validatePW->fetch();
            $dbPW = $passwordResult['password'];
            $dbSalt = $passwordResult['salt'];
            //Compare entered password to SALT + hash
            $hashPW = hash('sha512', $dbSalt . $this->password);
            if ($hashPW === $dbPW) {
                //Logged in
                $_SESSION['CVFD-USER-DETAILS'] = $this->accountDetails();
                //Redirect to secure landing page for log-in origin (Members or Admin)
                //Adding SID is a recent attempt to handle log-in problems
                header("Location: http://example.com/$this->site/$this->site-main.php?" . SID);
                //session_write_close() was here but was removed
                exit();
            } else {
                //Password invalid
                $loginErr[] = "Please check your password and try again";
                $_SESSION['CVFD_LOGIN_ERR'] = $loginErr;
                //Redirect to the log-in for the origin
                header("Location: http://example.com/$this->site");
        session_write_close();
                exit();
            }
        } else {
            $_SESSION['CVFD_LOGIN_ERR'] = $loginErr;
            header("Location: http://example.com/$this->site");
            session_write_close();
            exit();
        }

    }
}
?>

auth.php

<?php
session_start();
if (!isset($_SESSION['CVFD-USER-DETAILS']) || $_SESSION['CVFD-USER-DETAILS'] == '') {
    //Not logged in
    $_SESSION['CVFD_LOGIN_ERR'] = array('Please login');
    header('Location: http://example.com/members');
    session_write_close();
    exit();
} else {
    $userDetails = $_SESSION['CVFD-USER-DETAILS']; //Assign user details array to variable
    //Check to see if account is active
    $accountStatus = $userDetails['isActive'];
    $accountLevel = $userDetails['accountLevel'];
    if ($accountStatus == 0) {
        //Account is not yet active (pending Admin activation)
        $_SESSION['CVFD_LOGIN_ERR'] = array('Your account is suspended or pending activation');
        header('Location: http://example.com/members');
        session_write_close();
        exit();
    } else {
        $CVFDFirstName = $userDetails['name_f'];
        $CVFDLastName = $userDetails['name_l'];
        $CVFDLastLogin = date("m/d/Y H:i:s", strtotime($userDetails['lastLogin']));
        $CVFDAccountLevel = $userDetails['accountLevel'];
        $CVFDIPAddr = $userDetails['ipAddr'];
    }
}
?>

Here is how the auth.php is included in secure files-

<?php
if (substr_count($_SERVER['HTTP_ACCEPT_ENCODING'], 'gzip')) ob_start("ob_gzhandler"); else ob_start();
require($_SERVER['DOCUMENT_ROOT'] . '/members/includes/handlers/handler.auth.php');

Any help would be appreciated. Quite a mystery..

Thanks!

NightMICU
  • 9,000
  • 30
  • 89
  • 121
  • So you can't reproduce the problem at all? – MrCode Oct 05 '12 at 15:42
  • From my devices and networks, no. The second problem - randomly getting spit back to the log-in during use of the CMS - happens to me occasionally. Most clients use IE and Windows, although the one that has complained the most has had the problem on his iPhone, iPad, Windows NT laptop, and using both IE 9 and Chrome. The problem occurs both when he is home and at work on all devices. – NightMICU Oct 05 '12 at 15:46
  • I also confirmed that cookies were enabled throughout his devices and browsers. I witnessed the problem in person. An important point just came to mind, though - adding to original question – NightMICU Oct 05 '12 at 15:47
  • What's the timeout on your sessions (I can't remember the exact configuration directive)? – slugonamission Oct 05 '12 at 15:54
  • And whats the session handler? Default file system? – ficuscr Oct 05 '12 at 15:54
  • @slugonamission - whatever the default would be. I have not set this. ficuscr - Default – NightMICU Oct 05 '12 at 15:58
  • http://www.php.net/manual/en/function.session-write-close.php#86791 – Steve's a D Oct 05 '12 at 16:03
  • Session regen also has the added benefit to improve security by cutting down on chance of session fixation. Good practice in general when re-authenticating/elevating privileges. Worth a try. Also, might want to check your disks. Make sure your session writes are not taking some inordinate amount of time. – ficuscr Oct 05 '12 at 16:05
  • @ficuscr - this is in a shared hosting environment (Host Gator in the case of the biggest problem client), so I do not have control over server performance.. – NightMICU Oct 05 '12 at 16:08

3 Answers3

1

The way I do login system is to just use the session id, rather than storing anything in the session itself. When a user logs in their hashed user agent data, their session ID, their user id (corresponding to a users table) and an expiry time is put into a table, often called "active_users", I then had a logged in head file included in every admin restricted page that starts the session, retrieves the users session ID and checks to see whether that session ID is in the active users table and whether the user being checked against has the same user agent data, the expiry time is not surpassed. If nothing is returned from that query they're not logged in and are bounced out.

That's how most login systems I make work and I haven't had any problems.

EM-Creations
  • 4,195
  • 4
  • 40
  • 56
  • Interesting. So, for my application, I would create a new table with their user ID, session ID, and expiration time. Then, cross check the Members table with every page view using the auth.php to retrieve their details and make sure they had the correct account level for that site? It's a lot of MySQL queries but I suppose they're pretty fast to do for something simple like that.. not dealing with more than 30 users on average – NightMICU Oct 05 '12 at 16:05
  • Just thinking more about this. It seems that the session is being destroyed somehow, causing this bug. Wouldn't it be likely that the session ID is changing as well when this happens? – NightMICU Oct 05 '12 at 16:13
  • You shouldn't use the user-agent to identify a session. It can change between requests. Aside from that, it doesn't offer any additional security because if an attacker has sniffed someone's session ID then they almost certainly will have the user-agent too. – MrCode Oct 05 '12 at 16:23
  • I don't just use the user-agent to identify a session. It's just an additional loop to jump through. I don't rely upon it. – EM-Creations Oct 06 '12 at 08:23
0

The one thing that jumps out at me is the following:

header('Location: http://example.com/members');
session_write_close();
exit();

I would place the session_write_close() call before the header('location ...')

Are any 'headers already sent' errors showing up in your logs?

Other thing that comes to mind is some AJAX race condition. Any async calls going on with login pages?

ficuscr
  • 6,975
  • 2
  • 32
  • 52
  • Valid point. However, the only place that appears in the code at the moment is in auth.php and only if the session variable is missing or if they are not authorized to view the site (wrong account level). They are wrapped in conditional statements. Would that still be a problem? As for the errors, I have not seen any. – NightMICU Oct 05 '12 at 16:00
  • I think this is the problem, I would get rid of `session_write_close()` completely and see what happens. If you don't expect that code to execute very often then you could have a bug. You could add a log there and monitor it. – MrCode Oct 05 '12 at 16:03
  • Okay I will give that a shot.. again, though, this is only called conditionally. Would that still be a problem? – NightMICU Oct 05 '12 at 16:06
  • Probably not be it at all then. Again, just the one thing that stood out. And no ajax, hmmm. Might need to write some logging code to try and collect more details, see what path can cause this to happen and check everything when it does. – ficuscr Oct 05 '12 at 16:06
  • Everything seems to work right up until the auth.php page is required by the secure landing page. And the resulting error only occurs when the session variable has not been set.. or has somehow been cleared. And what I really don't understand is why it only seems to be happening to a small amount of users – NightMICU Oct 05 '12 at 16:09
  • 1
    As @ficuscr pointed out, you call `session_write_close()` after the header is sent. Because of that, `session_write_close()` is not guaranteed to complete before the script is aborted. I think you will have some browsers/networks completing the call, and some not because as soon as the browser receives the `loaction` header, it will disconnect and cause your PHP to abort. – MrCode Oct 05 '12 at 16:19
  • @MrCode, again, this is only called conditionally if the `$_SESSION['CVFD-USER-DETAILS` session variable is missing or empty. In this case, it is setting a session variable with an error message and sending the user back to the login page. That part of it is working just fine - it's when that session variable SHOULD be set and appears not to be that is the problem. But I will remove it anyway to see if the problem continues.. – NightMICU Oct 05 '12 at 16:21
  • Why did you remove the session_write_close from `($hashPW === $dbPW)` block? Think you need to ensure that session write are getting completed before the next page (redirect) is loaded. Also, use some better, stricter checks. `$_SESSION['CVFD-USER-DETAILS'] == ''` Do you use `unset()` to clear it? Is it ever set to an empty string? Evaluate for real potential values. – ficuscr Oct 05 '12 at 16:29
  • I removed the `session_write_close()` from that block just to rule it out as a cause of the problem. And it may have been, since I believe it was called AFTER the header redirect. Will investigate that further. Also a good point on stricter checks – NightMICU Oct 05 '12 at 17:06
  • Selected as the correct answer since I cannot be sure if the code previously had the session_write_close() AFTER the header redirect in the original login.php file. The system appears to be working normally after removing session_write_close() – NightMICU Oct 05 '12 at 17:45
0

Success! Still need to narrow down exactly what change resulted in the problem going away, but the client reports that he no longer has problems logging in.

The biggest change that immediately comes to mind was removing session_write_close() just about everywhere. It may have been placed AFTER the header redirect in parts of the code, or just having it present may have been the cause. I will experiment with placing it before the redirect.

Thanks to all for your suggestions

NightMICU
  • 9,000
  • 30
  • 89
  • 121