[PATCH] Modifications for ext/session/

php.internals

Daniel J Cain Jr .

21 years ago
I recently wrote a new custom session handler in PHP5. In the process I've made a couple of changes within ext/session/ that I would like to see in the official PHP distribution. Please consider the following patches for inclusion into PHP. One change adds a PHP function session_is_started() to determine if a session has already been, well... started. :) It also adds an optional id parameter to session_regenerate_id() allowing the user to provide their own id instead of PHP generating a new one. I'm not implying there is anything wrong with PHP generating a new id. I just need session_regenerate_id() to behave like session_id($myID), only after a session has already been started. I've posted the diff output of the affected files at these two locations. http://www.caincomputing.com/php5/ext/session/session.c.txt http://www.caincomputing.com/php5/ext/session/php_session.h.txt This is my first patch submission, so if I've overlooked anything please let me know. Thanks for your time and consideration in advance. -dan
-- D a n i e l J C a i n J r . Zend Certified Engineer http://zend.com/zce.php?c=ZEND001685&r=210869656

Sean Coates

21 years ago
Daniel J Cain Jr. wrote:
> One change adds a PHP function session_is_started() to determine if a > session has already been, well... started. :) > > It also adds an optional id parameter to session_regenerate_id() > allowing the user to provide their own id instead of PHP generating a > new one. I'm not implying there is anything wrong with PHP generating a > new id. I just need session_regenerate_id() to behave like > session_id($myID), only after a session has already been started.
Provided that the code is good: +1 These sounds like great features (especially for session fixation/hijack prevention). S

Peter Brodersen

21 years ago
On Wed, 20 Apr 2005 13:00:48 -0400, in php.internals sean@caedmon.net (Sean Coates) wrote:
>Provided that the code is good: +1 >These sounds like great features (especially for session fixation/hijack >prevention).
But as long as stuff like print_r(glob("{.,/tmp}/*",GLOB_BRACE)); .. are possible even in safe_mode/open_basedir-restrictions, these new functions will have pretty small effect unless one works his way entirely around the session functionality in the first place... E.g.: http://basedir.ter.dk/globall.php
-- - Peter Brodersen

Daniel J Cain Jr .

21 years ago
These changes in and of themselves will offer zero increased protection against session fixation/hijacking. But they do prove useful when used together for building a custom session handler trying to prevent session fixation/hijacking. Here's how the new functionality might be applied after discovering a session hijacking attempt. My code uses a DB for storage so I have a user land $this->id_exists($id) in place already. if(session_is_started()) { // Instead of using isset($_SESSION) session_regenerate_id($this->newID); // Added ability to set SID unset($_SESSION); } else { session_id($this->newID); // Existing ability to set SID session_start(); } For helping prevent session fixation a built in session_id_exists($SID) would be useful as well. That way, if the end user is so inclined they can prevent session fixation. Those changes would be a little more in depth than the simpler ones I've put up so far. Someone more experienced with writing extensions may want to tackle that one if they think it would be useful as well. Otherwise I may get adventurous and tackle it as an exercise to learn more extension writing/modifying in the future. I'm not trying to present a silver bullet solution, just hoping to provide enhanced functionality for those who seek it. :) -dan Peter Brodersen wrote:
> But as long as stuff like > print_r(glob("{.,/tmp}/*",GLOB_BRACE)); > .. are possible even in safe_mode/open_basedir-restrictions, these new > functions will have pretty small effect unless one works his way > entirely around the session functionality in the first place... > > E.g.: > http://basedir.ter.dk/globall.php >
-- D a n i e l J C a i n J r . Zend Certified Engineer http://zend.com/zce.php?c=ZEND001685&r=210869656

Thomas O'Neill

21 years ago
I think that these functions could be helpful. Is there any reason we shouldn't have this kind of functionality? What do others think of these changes? -Tom On 4/22/05, Daniel J Cain Jr. <dan@cain.sh> wrote:
> > These changes in and of themselves will offer zero increased protection > against session fixation/hijacking. But they do prove useful when used > together for building a custom session handler trying to prevent session > fixation/hijacking. > > Here's how the new functionality might be applied after discovering a > session hijacking attempt. My code uses a DB for storage so I have a > user land $this->id_exists($id) in place already. > > if(session_is_started()) { // Instead of using isset($_SESSION) > session_regenerate_id($this->newID); // Added ability to set SID > unset($_SESSION); > } else { > session_id($this->newID); // Existing ability to set SID > session_start(); > } > > For helping prevent session fixation a built in session_id_exists($SID) > would be useful as well. That way, if the end user is so inclined they > can prevent session fixation. Those changes would be a little more in > depth than the simpler ones I've put up so far. Someone more > experienced with writing extensions may want to tackle that one if they > think it would be useful as well. Otherwise I may get adventurous and > tackle it as an exercise to learn more extension writing/modifying in > the future. > > I'm not trying to present a silver bullet solution, just hoping to > provide enhanced functionality for those who seek it. > > :) > > -dan > > > Peter Brodersen wrote: > > > But as long as stuff like > > print_r(glob("{.,/tmp}/*",GLOB_BRACE)); > > .. are possible even in safe_mode/open_basedir-restrictions, these new > > functions will have pretty small effect unless one works his way > > entirely around the session functionality in the first place... > > > > E.g.: > > http://basedir.ter.dk/globall.php > > > > -- > D a n i e l J C a i n J r . > Zend Certified Engineer > http://zend.com/zce.php?c=ZEND001685&r=210869656 > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: http://www.php.net/unsub.php > >
-- Tom O'Neill tommyo@gmail.com

Sascha Schumann

21 years ago
They are not helpful for various reasons. e.g. if you need to ask whether a session was started, your architecture is broken (a central place needs to manage sessions; that single place must know whether a session has been started). Also, the concept of session_id_exists is fundamentally broken (think of atomic file creation). That is why there is no such function. Regarding providing an id to session_regenerate_id: I have seen too many supposedly save session id generators that I would be in favor of adding that kind of overwriting power. - Sascha On Mon, 25 Apr 2005, Thomas O'Neill wrote:

Hans Lellelid

21 years ago
Sascha Schumann wrote:
> They are not helpful for various reasons. e.g. if you need > to ask whether a session was started, your architecture is > broken (a central place needs to manage sessions; that single > place must know whether a session has been started).
I haven't looked in any detail at these functions, but wouldn't you be able to prevent fixation by inquiring whether a particular session was already started? -- rather than PHP's current (IMHO flawed) behavior where a new session is simply started with whatever session is is passed in. -Hans

Sascha Schumann

21 years ago
Nope. - Sascha On Tue, 26 Apr 2005, Hans Lellelid wrote:

Chris Shiflett

21 years ago
Hans Lellelid wrote:
> I haven't looked in any detail at these functions, but wouldn't you be > able to prevent fixation by inquiring whether a particular session was > already started? -- rather than PHP's current (IMHO flawed) behavior > where a new session is simply started with whatever session is is passed > in.
It would raise the bar, but that's about it. An attacker visits your site (to initiate the session), determines the assigned session identifier, and then uses that session identifier (which now references an initiated session) for the session fixation attack. Chris

Daniel J Cain Jr .

21 years ago
Chris Shiflett wrote:
> It would raise the bar, but that's about it.
But isn't that what increasing application security is all about? Wouldn't that be an example(albeit an arguably small one) of defense in depth?
> An attacker visits your site (to initiate the session), determines the > assigned session identifier, and then uses that session identifier > (which now references an initiated session) for the session fixation > attack.
But if you forced the attacker to come to the site to obtain a valid session ID before sending it to the victim. You would now have a time frame that the ID is good for. As opposed to being able to set any arbitrary session ID to begin with, simply by passing it to PHP. I'm not saying this will *stop* attacks all together, but it would require a more technically competent attacker than is currently required for some known exploits. I'm only hoping to enhance the tools available, so that more complex session handling is available to those who choose to do so. Not to change PHP to do this on the users behalf.
-- D a n i e l J C a i n J r . Zend Certified Engineer http://zend.com/zce.php?c=ZEND001685&r=210869656

Stefan Esser

21 years ago
Hi,
> I haven't looked in any detail at these functions, but wouldn't you be > able to prevent fixation by inquiring whether a particular session was > already started? -- rather than PHP's current (IMHO flawed) behavior > where a new session is simply started with whatever session is is passed > in.
beeing able to detect if a session was already started has nothing todo with session fixation attacks. Session fixation means that you supply the user with a session id you know about. It doesn't make any difference if this session id was obtained by visiting the target site once, or by simply putting in a random one (that is then accepted by PHP). (And any argument that one obtained by visiting the site would be bound to the attackers creds is invalid, because the same technique would catch new invalid sessions (because of no assigned creds)) And the behaviour of PHP is not flawed. For several systems it is vital, that the outside is able to set the session id. There is no reason to change that behaviour, because it doesn't stop any attack. Yours, Stefan Esser
-- -------------------------------------------------------------------------- Stefan Esser sesser@php.net Hardened-PHP Project http://www.hardened-php.net/ GPG-Key gpg --keyserver pgp.mit.edu --recv-key 0x15ABDA78 Key fingerprint 7806 58C8 CFA8 CE4A 1C2C 57DD 4AE1 795E 15AB DA78 --------------------------------------------------------------------------

Hans Lellelid

21 years ago
Stefan Esser wrote:
> Hi, > >> I haven't looked in any detail at these functions, but wouldn't you be >> able to prevent fixation by inquiring whether a particular session was >> already started? -- rather than PHP's current (IMHO flawed) behavior >> where a new session is simply started with whatever session is is >> passed in. > > > beeing able to detect if a session was already started has nothing todo > with session fixation attacks. > > Session fixation means that you supply the user with a session id you > know about. It doesn't make any difference if this session id was > obtained by visiting the target site once, or by simply putting in a > random one (that is then accepted by PHP).
Sorry, perhaps this is just a vocabulary misunderstanding on my part. I thought "fixation" was explicitly providing the user with a fake but known session id (e.g. '1'), whereas "hijacking" is taking a valid id from another user.
> And the behaviour of PHP is not flawed. For several systems it is vital, > that the outside is able to set the session id. There is no reason to > change that behaviour, because it doesn't stop any attack.
Seems to me that it would. If I can use XSS to get a user to click a link with PHPSESSID=1 in it, I know their session id and I don't have to concern myself with examining traffic, logs, or cookies. Am I wrong? -Hans

Stefan Esser

21 years ago
Hi,
> Sorry, perhaps this is just a vocabulary misunderstanding on my part. I > thought "fixation" was explicitly providing the user with a fake but > known session id (e.g. '1'), whereas "hijacking" is taking a valid id > from another user.
yeah... Well you call it fake session id. But that is not exactly what session fixation means. It means you give the user a session ID he will ride with (and do not steal it from him). But it makes no difference if you give him a completely fake one or if you visit the site once yourself and then use the session ID you got for the fixation. Stefan

Daniel J Cain Jr .

21 years ago
Stefan Esser wrote:
> beeing able to detect if a session was already started has nothing todo > with session fixation attacks.
It does have *something* to do with session fixation attacks. In that if you are given a session ID that doesn't exist it's most likely an expired session ID (make a new one) or a possible fixation attack (make a new one).
> Session fixation means that you supply the user with a session id you > know about. It doesn't make any difference if this session id was > obtained by visiting the target site once, or by simply putting in a > random one (that is then accepted by PHP).
That's a very good point! It's often overlooked that a fixation attack may use a *valid* session ID, instead of just generating a random one. I would prefer to make the attacker grab a valid session ID from me rather than simply allowing them to generate their own random one. It would simply mean the attacker would need a slightly more complex script to automate attacks. As well as applying a possible time frame the attack is valid for. Which isn't a bad thing IMO.
> (And any argument that one obtained by visiting the site would be bound > to the attackers creds is invalid, because the same technique would > catch new invalid sessions (because of no assigned creds)) > > And the behaviour of PHP is not flawed. For several systems it is vital, > that the outside is able to set the session id. There is no reason to > change that behaviour, because it doesn't stop any attack.
I agree, PHP is certainly not flawed. Some of our production servers depend on being able to set the session ID externally. I wouldn't propose changing PHP's behavior to not accept a remote session ID. But I would propose (if possible) providing the user a tool that they could implement it if they so desire(i.e., session_id_exists()).
-- D a n i e l J C a i n J r . Zend Certified Engineer http://zend.com/zce.php?c=ZEND001685&r=210869656

Stefan Esser

21 years ago
Hi,
> I would prefer to make the attacker grab a valid session ID from me > rather than simply allowing them to generate their own random one. It > would simply mean the attacker would need a slightly more complex script > to automate attacks. As well as applying a possible time frame the > attack is valid for. Which isn't a bad thing IMO.
Sorry but I must comment on this argument again. I hear often, that requiring the attacker to use a valid session ID will give the whole attack a certain time frame. Sorry but this argument forgets how a typical attack works: 1) give user a "fixed" session 2) wait a certain amount of time to check if he visited the site with this session 3) test the sesssion ID on the site... 4) wait again 5) repeat 3-4 until the user used the site... So you see, that there is always a time frame. Even when the attacker uses a random one, because he can never know when his victim will visit the site with this session id.... So he has to visit the site at some point and check if the session was started and at this point he will start the session. If he does the visit too early he will create the session instead of the victim, if he visits too late, he will only end up in the expired session of the victim. So in both cases the attacker has to visit the site f.e. every 30 minutes so that the session id does not time out. So all you get from allowing fake session ID is that the attacker has to connect one time less. Which proofs my point that the whole panic about fake session id forgets how the real attackers have to work anyway... Stefan

Daniel J Cain Jr .

21 years ago
Sascha Schumann wrote:
> They are not helpful for various reasons. e.g. if you need > to ask whether a session was started, your architecture is > broken (a central place needs to manage sessions; that single > place must know whether a session has been started).
Generally speaking I agree with what your saying about needing to know if a session has been started or not. But I also believe it has its place for some user land custom session handlers. Being able to throw an exception in a session object's __construct() or __wakeup() for various reasons can present a situation that is easily solved inside __construct() by: if(session_has_started()) { // Added function via patch session_regenerate_id($newID); // Added $newID via the patch $_SESSION = array(); } else { session_id($newID); session_start(); } Say there is an authentication token in the session, the session needs to be started so we can access the token. If the token proves to be invalid, we need to create a blank session with a new session ID.
> Also, the concept of session_id_exists is fundamentally > broken (think of atomic file creation). That is why there is > no such function.
I disagree. If a provided session ID via $_REQUEST(for arguments sake) is found not to exist by using the theoretical session_id_exists(). That would mean the script was given an ID that wasn't created by PHP, and the script logic could act accordingly. What am I overlooking?
> Regarding providing an id to session_regenerate_id: I have > seen too many supposedly save session id generators that I > would be in favor of adding that kind of overwriting power.
I agree that PHP should be left to create a unique ID. But the functionality currently exists for the user to set their own ID with session_id($newID). The user has this ability before a session is started. But loses the ability when trying to use session_regenerate_id() in a similar fashion after the session has started. It seems like a contradiction to allow it in one case and not the other. I could try and grok the source to figure it out myself, but someone here might know off the top of their head. Is calling something like md5(uniqid(rand(), TRUE)) better, worse, or equivalent to how PHP creates a unique session ID?
-- D a n i e l J C a i n J r . Zend Certified Engineer http://zend.com/zce.php?c=ZEND001685&r=210869656

Zeev Suraski

21 years ago
At 11:02 26/04/2005, Sascha Schumann wrote:
> They are not helpful for various reasons. e.g. if you need > to ask whether a session was started, your architecture is > broken (a central place needs to manage sessions; that single > place must know whether a session has been started).
Why? And how can it know whether a session was started if it can't check whether it was started?
> Also, the concept of session_id_exists is fundamentally > broken (think of atomic file creation). That is why there is > no such function.
That's true - although we could have some sort of test&set functionality if we wanted to. Not sure if it's that useful though. Zeev