Hi Stas,

On Tue, Dec 22, 2015 at 3:22 PM, Stanislav Malyshev <smalys...@gmail.com> wrote:
> Hi!
>
>> I would like to restart better session management for PHP 7.1.
>>
>> https://wiki.php.net/rfc/precise_session_management
>
> I've read the RFC and I have some questions and comments:
>
> 1. I do not see why old session being active is a problem when you
> regenerate. You write "Attacker may abuse stolen session forever." - I'm
> not sure what abuse is meant. If the old session was unauthenticated,
> then you can not abuse it in any meaningful way. If the old session was
> authenticated, then a) you already lost b) regenerate is not needed,
> since regenerate happens when crossing security border and c) sessions
> can expire, so it won't be forever anyway, unless you maintain it, but
> even then application can easily implement limit on session length in
> userspace.

The old session data may be stolen by suffering/XSS/etc.
session_regenerate_id() is used to mitigate risks of stolen
"valid/authenticated"
session IDs. Good web apps should renew session ID periodically.  Therefore,

a) Session could be authenticated/valid.
b) Periodic session ID renewal is needed for better security.
c) Attacker may keep accessing stolen session to avoid GC.

Users can implement the same feature. I've been advocating this for years, but
it's not common even in Japan.

> 2. I do not see why you need to know specifically when the session is
> deleted. Also, I'm not sure what you mean by "invalid session".

Old session ID is the invalid one. New session ID that is generated by
session_regenerate_id()
is valid one.

Please note that old session ID is valid until TTL also.

> 3. "Stealing session ID is easy regardless of HTTPS" - I certainly
> disagree with this. Is that were easy, every application in PHP using
> sessions for authentication (which is close to all of them ever using
> authentication) would be trivially vulnerable. As I do not see any
> reports of such mass security failure happening, it must not be as easy
> as you claim. Also, "search the internet to find the proof of what I am
> saying" is not a good argument in an RFC. It's your argument, so it's
> your job to search the internet and present the proof. If it's very
> easy, then it should not even take long.

Recent browsers became stricter than before for TLS/SSL connections.
It's not trivial as few years ago, but it's still possible intercept TLS/SSL
connections. e.g. Enterprise network admins sniff their TLS/SSL traffic.

Currently, there is no chance to know if there is stolen sessions. New
session module give a chance to know possible risk by raising error
for invalid/obsolete/old session accesses.

BTW, if attacker such as colleague in office may directly see the cookie
value of a client/browser. If web apps do not renew session ID, the colleague
may abuse the session as long as session ID is valid/authenticated.

> 4. The bugs you quote are:
> - 69127 - unclear what is going on there, but looks like you understand
> what is going on, could you explain on the bug or here?

It's race in browsers.

Try test code that I've posted. Browser developers might implement proper
locks to avoid race, but they may not yet. Even if firefox/chrome uses proper
locks, we may not rely on them as RFC does not require such cookie
management.

https://bugs.php.net/bug.php?id=69127

-------------------------------------------------------------------------------
[2015-09-19 23:41 UTC] yohg...@php.net

Procedure to reproduce this issue

test.php
--------------
<?php
ob_start();
session_start();
echo "<pre>";

var_dump(session_id(),
$_SESSION['v']++);
session_regenerate_id(true);
var_dump(session_id());
?>
--------------

Start CLI server
$ php -S 127.0.0.1:8888

Access test.php and press F5 few minutes
http://127.0.0.1:8888/test.php

You'll see counter value ($_SESSION['v']) is resetted sometimes.

PHP 7.0 git + Fedora 22 + Chrome 45.0.2454.93 (64-bit) : Easy to
reproduce. Thousands of requests are enough.
PHP 7.0 git + Fedora 22 + Firefox 40.0.3 : Very hard to reproduce.
Tens of thousands of requests are required.

CLI server process requests one by one. The reason why there is
difference would be how browser locks cookie data. It seems Chrome
locking is more lazy or no locks at all. (BTW, even if browser locks
cookie strictly, lost packet/etc could cause lost session. Therefore,
"eventually consistent" approach is required for reliable HTTP session
management.)

Let me know if you(anyone) could reproduce the bug or not by this
procedure - PHP version, OS name/version, Browser name/version and
easiness/hardness of reproducibility.
-------------------------------------------------------------------------------

> - 68063 - easily fixable by rejecting empty ID< but it is a trivial
> matter, since practical importance of this seems to be nil

https://bugs.php.net/bug.php?id=68063

The root cause of session ID being NULL string is browser race.

When session_regenerate_id(TRUE) is called and session.use_strict_mode=1,
session module may generates new session IDs (multiple IDs since browsers
use multiple requests) in a short period.

This causes browser race condition and results in null session ID cookie
randomly/rarely. My client observed this only a few in a millions of requests.

session.use_strict_mode=0 cannot be a mitigation nor resolution. Allowing
use of uninitialized session will open door for abusing "unchangeable cookie".
i.e. Cookie can be undeletable/unchangeable by combination of path/domain/
httponly/secure attributes. Those who need higher security for web must use
session.use_strict_mode=1. Otherwise, attacker may steal active session
forever by exploiting unchangeable cookie and adoptive session ID management.

> - 70584 - unclear what is going on here, but doesn't seem to be related
> to anything discussed above, at least. Maybe caused by the new code,
> according to 70013

https://bugs.php.net/bug.php?id=70584

Since the reporter does not give feedback if he/she uses
session_regenerate_id(true)
or not, I cannot tell if it relates to https://bugs.php.net/bug.php?id=70013.

However, if it related to 70013, "we observed the complete loss of all
$_SESSION variables occurring about 1 to 10 times/day" wouldn't be possible.
70013 is a bug that reference to $_SESSION loose the reference _always_.


> They are from 2015 and 2014, so I don't see how any of these are "known
> more than 10 years" - could you explain what you mean by that?

Deleting old session data causes problems. I thought it was due to unstable
network used to be and problem wouldn't occur under stable TCP network.
This discussion popped up on occasions and the first one was very long time
ago. However, I cannot be sure when it was. I don't mind at all drop this
phrase. It does not worthwhile researching records. And my memory could
be wrong.

> 5. I don't like magic session variables too much, though if they will be
> implemented transparently from the user, it might be ok.

Good to hear!

> 6. Session TTLs are very dependent on application nature - for some,
> it's ok to have sessions live nearly forever, for others, sessions may
> live only minutes. Also, some applications may count it from session
> start, others from session last use, others have more complex criteria.
> Trying to do one-fits-all solution is not good here. While having the
> date when session was initiated may be useful and would not be hurtful
> for most scenarios, messing with sessions - such as deleting them - when
> app developer does not expect it may not be a good idea.

This proposal gives developers full control of session life time, so developers
may choose proper values for their needs.

I think most developers expect old session data will be deleted depending
on session.gc_maxlifetime. However, this assumption is not true. e.g.
attacker may keep session alive by simply accessing stolen session ID.

>
> 7. Not sure I understand what ttl_update is for. If we already have last
> access time (which we already do, AFAIK) and session start time (which
> is your proposal) - why do we need anything else?

If I update TTL timestamp always, lazy_write would not work at all.
I need to lower write frequency lazy_write to work.

>
> 8. "Therefore, there should be GC API for cron task for instance." - I
> am not sure what are you meaning here. Could you describe the scenario
> where such function would be useful and how exactly would it be used?
> What people that do not have cron functionality or access to it should do?

GC is performed at session_start() by probability. GC could be long and
some user may experience very slow response on occasion. To avoid this,
developers may set up cron task for removing obsolete sessions.

For users do not have access to cron or like, they may leave probability
based GC as it is now.

>
> 9. "Raise error/exception for invalid access" - I do not think it is
> clear what is "invalid access". Does it mean access with non-existing
> session ID would raise an error? That would definitely break almost
> every application using sessions (since now they all rely on PHP
> treating expired session the same as if there were no session, thus
> triggering standard "not authenticated" workflow).
>

Does it mean access with non-existing session ID would raise an error?

No. Access to non-existing session ID is handled by session.use_strict_mode=1
already and does not raise error at all. It creates new session ID and
uses it simply.

"Raise error/exception for invalid access"

I mean "Raise error/exception for accessing session data that should not
be accessed anymore." The invalid session is session ID that is obsoleted by
session_regenerate_id(). Access to old/obsolete session 300 seconds after
session_regenerate_id() is invalid in most cases.

However, unstable network (e.g. wireless network) may cause "invalid access"
in rare cases. Example scenario is

 1. Client:  Browser accesses to web site
 2. Server: PHP's session_regenerate_id() is called.
    2.1. Internal php_session_reset_id() which sends new session ID
cookie is called.
    2.2. PHP sends new session ID.
    2.3. Network connection is lost (Subway, elevator, etc)
 3. Client: Browser does not get new session ID and keep using old session ID.
 4. Client: Browser sends old session ID which is obsolete in the server.

This proposal allows to resend new session ID once after new session ID is sent.
(60 seconds later up to old session TTL which is 300 seconds, currently) This
mitigates risk of lost session and invalid session access above
scenario or like.
Risks of abuse increase a little in return, though. Even if this
scenario is rare, it
seems it happens.

Thank you for your comment.
I hope I explained well.

Regards,

--
Yasuo Ohgaki
yohg...@ohgaki.net

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Reply via email to