PHP session fixation

Question

I'm trying to implement a system of restricted access. Right now I'm focusing on the "session fixation". I am concerned about the following aspects:

1. Control of a "fingerprint" of the user created by mixing UserAgent, IPAddress and a salt key. If the fingerprint were to change, destroy the user's session.

2. Using the same session id for a certain number of times, after which regeneration session id.

3. Regenerate the session id every time change the level of user authentication

What I want to know is whether my approach is right and if it is safe enough. I apologize if the code is long, I tried to remove all the unnecessary parts.

I do not want a code review, but the opinions about it ... particularly if the mine is a safe approach.

---

<?php
class oLogonUser 
{
    const SESSION_LIMIT = 10;
    const SESSION_SALT  = 'AS86F(,sa)8as7d+/234N&&"$£%';

    function __construct()
    {
        // init session
        $this->InitSession();

        // check if user logged
        if( $this->isUserLogged() )
        {
            // Check if finger print is valid
            if( !$this->isValidFingerPrint() )
                $this->LogOutUser(); // log out
        }
    }

    private function InitSession()
    {
        // check if session already start
        if( session_id() == '' )
            session_start();

        // Set user's fingerprint
        $this->setUserFingerPrint();

        // Session counter
        if( isset($_SESSION['UserSessionCounter']) )
            $_SESSION['UserSessionCounter'] = (int)$_SESSION['UserSessionCounter'] + 1;
        else 
            $_SESSION['UserSessionCounter'] = 0;

        // if session counter exeed limit, regenerate session id
        if( $_SESSION['UserSessionCounter'] > self::SESSION_LIMIT )
        {
            $_SESSION['UserSessionCounter'] = 0;
            session_regenerate_id();
        }
    }

    private function isValidFingerPrint()
    {
        // checking if the user fingerprint is the same as that stored in session
        if( isset($_SESSION['UserFingerPrint']) )
            return ( $_SESSION['UserFingerPrint'] === $this->getUserFingerPrint() );

        return false;
    }

    private function getUserFingerPrint()
    {
        // return user fingerprint
        return md5($_SERVER['REMOTE_ADDR'] . $_SERVER['HTTP_USER_AGENT'] . self::SESSION_SALT );
    }

    private function setUserFingerPrint()
    {
        // store in session user fingerprint
        if( !isset($_SESSION['UserFingerPrint']) )
            $_SESSION['UserFingerPrint'] = $this->getUserFingerPrint();
    }

    public function LogOnUser($UserName, $Password)
    {
        //NB: in fact control the credentials in the database
        if( $UserName == 'test' && $Password == 'test' )
            $this->setUserLogged();
    }

    public function LogOutUser($UserName, $Password)
    {
        // destroy user session
        session_destroy();
        session_regenerate_id();
    }

    private function setUserLogged()
    {
        session_regenerate_id();

        // set user logged
        $_SESSION['UserLogged'] = TRUE;
    }

    public function isUserLogged()
    {
        // check if user is logged
        return (isset($_SESSION['UserLogged']) && $_SESSION['UserLogged'] == TRUE );
    }
}

// Test it 
$objLogonUser = new oLogonUser();

if( !$objLogonUser->isUserLogged() )
    $objLogonUser->LogOnUser('test', 'test');

echo session_id() . '<BR />';

print_r($_SESSION);

Answer 1

Preferably, you would generate a random salt. Add a function to generate random salt, then be sure to call it in your constructor. If a user discovers you salt, that is a major security hole right now.

In my opinion, it would be best to separate this class into a separate file for easier use in multiple situations. Also, a separate file with useful functions like generating salt could be useful.

Your code is relatively secure, but you need a random salt generator. Otherwise, your code looks good.