Class for creating save sessions in PHP

Question

Is the following class well-built and secure or am I missing something? Any improvement ideas welcome.

/*
 * This class is for generating and using one single save session.
 */ 
final class Session {

    private static $name;
    private static $force_ssl_cookie;
    private static $destroy_unset;
    private static $max_inactivity;

    private static function checkInit() {
        if(!isset(self::$name)) {
            throw new Exception("Cannot use Session class without calling setSessionName once first.");
        }

        if(!isset(self::$force_ssl_cookie)) {
            throw new Exception("Cannot use Session class without calling setForceSSLCookie once first.");
        }

        if(!isset(self::$destroy_unset)) {
            throw new Exception("Cannot use Session class without calling setSessionDestroyUnset once first.");
        }

        if(!isset(self::$max_inactivity)) {
            throw new Exception("Cannot use Session class without calling setMaxInactivity once first.");
        }
    }

    public static function setName($name) {
        if(isset(self::$name)) {
            throw new Exception("Session class misuse: A session name has already been set.");
        }
        self::$name = $name;
    }

    public static function getName() {
        return self::$name;
    }

    public static function setForceSSLCookie($force_ssl_cookie) {
        if(isset(self::$force_ssl_cookie)) {
            throw new Exception("Session class misuse: The parameter force_ssl_cookie has already been set.");
        }
        self::$force_ssl_cookie = $force_ssl_cookie;
    }

    public static function setDestroyUnset($destroy_unset) {
        if(!is_array($destroy_unset)) {
            throw new Exception("Session class misuse: The parameter session_destroy_unset must be an array.");
        }

        if(isset(self::$destroy_unset)) {
            throw new Exception("Session class misuse: The parameter session_destroy_unset has already been set.");
        }
        self::$destroy_unset = $destroy_unset;
    }

    public static function setMaxInactivity($max_inactivity) {
        if(isset(self::$max_inactivity)) {
            throw new Exception("Session class misuse: The parameter max_inactivity has already been set.");
        }
        self::$max_inactivity = $max_inactivity;
    }

    /*
     * Destroy the current session and start a new one.
     */
    public static function restart() {
        self::destroyAbsolute();
        self::start();
    }

    /**
     * Secure start of a session.
     * 
     * @return type
     */
    public static function start() {

        self::checkInit();

        session_name(self::$name);
        session_set_cookie_params(0, COOKIEPATH, null, self::$force_ssl_cookie, true);

        $fresh = false;

        // no session existing, make a new session
        if(!isset($_COOKIE[self::$name])) {
            // replace the Session ID
            session_id(self::getSaveSessionId());
            $fresh = true;
        }

        session_start();

        // If the session was already existing, client_ip must match, the session must still be active etc.
        if (!$fresh) {
            // If this client ip does not match the ip of the current client, this might be an attack --> destroy session
            if (self::getClientIP() === $_SESSION["client_ip"] && self::getClientAgent() === $_SESSION['client_agent']) {
                if(self::checkStillActive()) {
                    return true;
                }
            } else {
                self::destroyAbsolute();
                header('Location: '.$_SERVER['REQUEST_URI']); 
                die;
            }
        } else {
            // if we have to create a new session, we do it in a secure, self-defined way
            self::setClientIP();
            self::setClientAgent();
            $_SESSION['last_activity'] = time();
            return true;
        }
    }

    // creates a new, secure session id (MUST be called BEFORE session_start())
    public static function getSaveSessionId() {
        self::checkInit();
        $hash_time = md5(microtime());
        $hash_ip = md5($_SERVER["REMOTE_ADDR"]);
        $hash_space = sha1(disk_free_space(getcwd()));
        return sha1($hash_time . $hash_ip . $hash_space);
    }


    /**
     * Secure instant destruction of session. Must be called after session_start !
     */
    public static function destroyAbsolute() {

        self::checkInit();

        session_name(self::$name);
        session_set_cookie_params(0, COOKIEPATH, null, self::$force_ssl_cookie, true);

        if(session_id()) {
            if (isset($_COOKIE[session_name()])) {
                setcookie(session_name(), "", time() - 42000, COOKIEPATH);
            }
            unset($_COOKIE[session_name()]);
            session_destroy();
        }
    }

    // retrieve the current session id
    public static function getCurrentId() {
            self::checkInit();
            session_name(self::$name);
            return session_id();
    }

    /**
     * Destroys a session with a specific ID.
     * 
     * @param type $sessid: Session ID.
     */
    public static function destroySpecific($sessid) {
        if($sessid && !empty($sessid)) {
            session_name(self::$name);
            session_id($sessid);
            // session_start();
            foreach(self::$destroy_unset as $uns) {
                unset($_SESSION[$uns]);
            }
            session_destroy();
        }
    }

    public static function checkStillActive() {

        self::checkInit();

        // make sure that a user was not inactive for too long
        if(intval($_SESSION['last_activity']) < time()-self::$max_inactivity) { //have we expired?
            self::destroyAbsolute();
            header('Location: '.$_SERVER['REQUEST_URI']); 
            die;
        } else { //if we haven't expired:
            $_SESSION['last_activity'] = time(); //this was the moment of last activity.
            return true;
        }
    }

    // sets the session IP of the current client
    private static function setClientIP() {
            if (isset($_SERVER["HTTP_X_FORWARDED_FOR"])) {
                    $_SESSION["client_ip"] = md5($_SERVER["HTTP_X_FORWARDED_FOR"]);
            } else {
                    $_SESSION["client_ip"] = md5($_SERVER["REMOTE_ADDR"]);
            }
    }

    // retrieves the client IP of the session owner
    private static function getClientIP() {
            if (isset($_SERVER["HTTP_X_FORWARDED_FOR"])) {
                    return md5($_SERVER["HTTP_X_FORWARDED_FOR"]);
            } else {
                    return md5($_SERVER["REMOTE_ADDR"]);
            }
    }

    private static function setClientAgent() {
        $_SESSION['client_agent'] = md5($_SERVER['HTTP_USER_AGENT']);
    }

    private static function getClientAgent() {
        return md5($_SERVER['HTTP_USER_AGENT']);
    }

}

Example of usage:

Session::setName("SESSION-mysession");
Session::setForceSSLCookie($force_ssl_cookie);
Session::setMaxInactivity(PHP_INT_MAX); // no problem so far
Session::setDestroyUnset(array('example'));

// create a new session, if no session is initialized
Session::start();

Answer 1

My first question with regards to your implementation is whether you should truly be setting some of the configuration values like session name, use of SSL cookie, inactivity timeout, etc. at runtime.

Do you really need to change these things at runtime, or should these be derived from application configuration and/or be set as class constants? If you derive these items from configuration, you can probably move away from repeated calls to checkInit() inside most every method, as you already know that the configuration is already set up the way you want it to be.

---

Stylistically, I question the mixed used of snake case and camel case in your class. I would think all code within the class should be consistent (though I know PHP as a language is not very consistent in this regard).

---

I don't understand why you are hashing many of the values that you are storing on the session class. This basically renders them unusable by your code, if you wanted to, for example, get authoritative IP address being associated with the session somewhere else in your code. The hash is meaningless to any calling code. Data stored in $_SESSION is secure (at least assuming your overall infrastructure including file directory, database, or other persistence medium for session data storage is also secure).

---

I don't understand the logic about trying to only destroy certain session variables when destroying a session. I would eliminate all that and just do unset($_SESSION).

---

If you are trying to implement a class-based interface to working with PHP sessions, I would think you would need to offer a complete set of session functionality, such that the code writer is not doing some operations via this class and other operations by directly calling PHP's session_* functions. For example, you give no provision for regenerating session id's (something that should take place after any authorization transition like a user login). By not supporting the wider session_* functions, you have, in essence, limited your ability to actually work with sessions securely.

---

Why create your own custom session id's using session_id()? This will mean that you will resend cookie to client every time they load a page, even if the session id has not changed (a side-effect of calling session_id() with explicit value passed). If you want to specify something other than default hashing algorithm, you should probably do so by changing your session.hash_function configuration (either at application level or at runtime).

---

You should consider moving routing decisions out of this class if you want to make this class more re-usable (i.e. across different applications). Allow the caller to make routing decisions. The public methods on this class should return appropriate information to allow the caller to make those routing decisions.

---

Read this document - PHP Session Management Basics