23

I know this topic has been discussed a lot, but I have a few specific questions still not answered. For example:

// **PREVENTING SESSION HIJACKING**
// Prevents javascript XSS attacks aimed to steal the session ID
ini_set('session.cookie_httponly', 1);

// Adds entropy into the randomization of the session ID, as PHP's random number
// generator has some known flaws
ini_set('session.entropy_file', '/dev/urandom');

// Uses a strong hash
ini_set('session.hash_function', 'whirlpool');

// **PREVENTING SESSION FIXATION**
// Session ID cannot be passed through URLs
ini_set('session.use_only_cookies', 1);

// Uses a secure connection (HTTPS) if possible
ini_set('session.cookie_secure', 1);

session_start();

// If the user is already logged
if (isset($_SESSION['uid'])) {
    // If the IP or the navigator doesn't match with the one stored in the session
    // there's probably a session hijacking going on

    if ($_SESSION['ip'] !== getIp() || $_SESSION['user_agent_id'] !== getUserAgentId()) {
        // Then it destroys the session
        session_unset();
        session_destroy();

        // Creates a new one
        session_regenerate_id(true); // Prevent's session fixation
        session_id(sha1(uniqid(microtime())); // Sets a random ID for the session
    }
} else {
    session_regenerate_id(true); // Prevent's session fixation
    session_id(sha1(uniqid(microtime())); // Sets a random ID for the session
    // Set the default values for the session
    setSessionDefaults();
    $_SESSION['ip'] = getIp(); // Saves the user's IP
    $_SESSION['user_agent_id'] = getUserAgentId(); // Saves the user's navigator
}

So, my questions are

  • do the ini_set's provide enough security?
  • is it okay to save the user's IP and navigator and then check it every time the page is loaded to detect a session hijack? Could this be problematic in any way?
  • is the use of session_regenerate_id() correct?
  • is the use of session_id() correct?
federico-t
  • 12,014
  • 19
  • 67
  • 111
  • 2
    All the overblown randomness stuff is probably the least relevant part of security, since the widest security holes are usually somewhere else entirely. You only need to regenerate the session ID when the login is successful, too. – Kerrek SB Dec 07 '11 at 17:07
  • Well, it can't hurt. Also, a main reason I'm doing this is because I don't have accounts in my website, just anonymous navigation. So, there's no such thing as a login. Would it be better _not_ to regenerate the session ID every time? – federico-t Dec 07 '11 at 17:10
  • 1
    There's no unique answer. The first thing is to regenerate on login. Beyond that is kinda overkill. Somehow, that session regeneration could be helpful in certain situations. Example: if you manage some kind of transactions (saving posts, uploading files, changing data somewhere behind the app) it helps because you could track if the user performed a history(-1) and got an old request sent again. – Alfabravo Dec 07 '11 at 17:23
  • 1
    regarding regenearation: regenerate *any* time the privileges of the user change. Whilst this includes regenerating when crossing the login / logout barrier, it also should include any time the user is granted or revoked *any* kind of applicational permission or role change. – Cheekysoft Dec 07 '11 at 17:32
  • Don't consider it complete, but you may find this useful as a tick-list http://stackoverflow.com/questions/8318944/did-i-use-session-variable-safely/8324014#8324014 – Cheekysoft Dec 07 '11 at 17:35
  • I thought `session_regenerate_id()` produces a random id based on your INI settings for session ids. Why then, would you need to call `session_id(sha1(uniqid(microtime())))` to set a new id, when `session_regenerate_id()` sets one for you? http://bit.ly/uL2Pc6 – Avicinnian Dec 07 '11 at 18:06

1 Answers1

18

Your configuration is awesome. You definitely read up on how to lock down php sessions. However this line of code negates a lot of the protection provided by your php configuration: session_id(sha1(uniqid(microtime()));

This is a particularly awful method of generating a session id. Based on your configurations you are generating the session id from /dev/urandom which is a awesome entropy pool. This is going to be a lot more random than uniqid() which is already mostly a timestamp, adding another timestamp to this mix doesn't help at all. Remove this line of code, asap.

Checking the IP address is problematic, ip addresses change for legitimate reasons, such as if the user is behind a load balancer or TOR. The user agent check is pointless, it is like having a GET variable like ?is_hacker=False, if the attacker has the session id they probably have the user agent, and if they don't this value is really easy to brute force.

rook
  • 66,304
  • 38
  • 162
  • 239
  • 1
    Great answer.. And I agree, the IP can be genuinly dynamic and the navigator info can be easily faked if you already know the session ID, but the more security the better (unless it spends a lot of resources, which in this particular case it doesn't). However, I'm probably going to change `$_SESSION['ip'] !== getIp() || $_SESSION['user_agent_id'] !== getUserAgentId()` into `$_SESSION['ip'] !== getIp() && $_SESSION['user_agent_id'] !== getUserAgentId()`. If BOTH conditions are met, then certainly something weird is going on. Anyways thanks for your answer! – federico-t Dec 07 '11 at 18:30
  • @John Doe yeah thats not a bad idea. If you detect any fowl play like that you might just want to destroy the session to prevent the attacker from trying to spoof his user agent. If he messes up once, then don't give him another chance. – rook Dec 07 '11 at 18:34
  • Exactly! if he was a genuine user he wouldn't be messing with the user agent in the first place, so it's safe to destroy his session – federico-t Dec 07 '11 at 18:37
  • @John Doe you also should read up on CSRF, aka "Session Riding" – rook Dec 07 '11 at 18:58
  • 2
    You can also use the first two sets of the IP address xxx.xxx. Less safe, but safer than nothing and less problems on large uni's and corporates. – Sanne Feb 20 '13 at 23:21
  • @Sanne Or you could incorporate your method with the one above to create a stronger barrier. – Anonymous Penguin Mar 29 '14 at 03:19
  • Why is a more random session ID a bad idea? Is it collisions? – Anonymous Penguin Mar 29 '14 at 03:22
  • @federicot some notes here that Opera browser allows quite openly for the user agent to supply a different user agent. And from the discussion here I would imagine that as IP and user agent are unsuitable to be kept as an authorisation token then a token string should be generated on login and stored in a database as well as in the session, and if they don't compare, then boot that user out. – Martin Mar 01 '16 at 09:57
  • @Martin This still won't address the problem of session riding, such as; XSS, CSRF, Click Jacking, and other SOP abuses. – rook Mar 02 '16 at 02:11