Hi all,
I though I've better to start new thread, since I changed the status
to "Under Discussion".
This is RFC for making PHP session strict.
https://wiki.php.net/rfc/strict_sessions
I'll implement DoS protection later, since current patch pretty well
tested and suitable for PHP 5.4/5.3, too.
Any comments?
--
Yasuo Ohgaki
yohgaki@ohgaki.net
Hi!
Hi all,
I though I've better to start new thread, since I changed the status
to "Under Discussion".This is RFC for making PHP session strict.
https://wiki.php.net/rfc/strict_sessions
I'll implement DoS protection later, since current patch pretty well
tested and suitable for PHP 5.4/5.3, too.
I've checked out the RFC and the patch, and I have couple of notes:
-
Why we need separate validate call in the API? Can't we just do the
checks in open/read? -
Very restrictive limits on session key values don't look useful for
me - I know some custom solutions use characters beyond alphanumerics in
session IDs. Of course it can be worked around with encoding, etc. - but
what does it add? -
Why replacing php_session_create_id with custom functions doing the
same in each standard module? -
I'm not feeling very comfortable getting such a big change (API
change, logic change, etc.) with unknown effects this late in 5.4. I'd
much better prefer doing it in 5.4.1 but API change doesn't really allow
that either.
--
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
(408)454-6900 ext. 227
Hi Stats,
2011/11/30 Stas Malyshev smalyshev@sugarcrm.com:
I though I've better to start new thread, since I changed the status
to "Under Discussion".This is RFC for making PHP session strict.
https://wiki.php.net/rfc/strict_sessions
I'll implement DoS protection later, since current patch pretty well
tested and suitable for PHP 5.4/5.3, too.I've checked out the RFC and the patch, and I have couple of notes:
- Why we need separate validate call in the API? Can't we just do the
checks in open/read?
open is used for creating session if does not exists. read cannot be
failed if there is session data, so separate API is needed to check if
it's a initialized one or not.
- Very restrictive limits on session key values don't look useful for me -
I know some custom solutions use characters beyond alphanumerics in session
IDs. Of course it can be worked around with encoding, etc. - but what does
it add?
It mainly for precaution for unknown security risks. Allowing special
chars often became cause of security incidents. It may be ok for allow
more chars or just denying lower special chars may be sufficient.
By the way, I'm planning to write security check module that checks
all inputs at once. It may detect session ID cookie as possibly
malicious input, but users may ignore anyway.
- Why replacing php_session_create_id with custom functions doing the same
in each standard module?
Bundled ps_module is now using PS_MOD_SID/PS_MOD_FUNCS_SID instead of
PS_MOD/PS_MOD_FUNCS. In order to use validate function, use of
PS_MOD_SID/PS_MOD_FUNCS_SID is required. As you can see, bundled
ps_modules are just calling internal session ID creation function
which has already been there.
PS_MOD_SID/PS_MOD_FUNCS_SID may implement it's own session ID
generation algorithms for whatever reasons. For example, it allows
ps_module writers to give special meaning to session IDs if they want.
We can force to use internal session ID creation function, though.
- I'm not feeling very comfortable getting such a big change (API change,
logic change, etc.) with unknown effects this late in 5.4. I'd much better
prefer doing it in 5.4.1 but API change doesn't really allow that either.
For ps_module writers, they still can use old API. User module may use
new API (i.e. new parameters for session ID creation and session ID
validation function), but they still may use old way, too. I would
prefer to have this patch for 5.4.0, but I don't insist. I've waited
for more than 6 years already, so it does not matter much :)
--
Yasuo Ohgaki
Hi Stats,
One additional comment.
2011/11/30 Yasuo Ohgaki yohgaki@ohgaki.net:
Hi Stats,
2011/11/30 Stas Malyshev smalyshev@sugarcrm.com:
I though I've better to start new thread, since I changed the status
to "Under Discussion".This is RFC for making PHP session strict.
https://wiki.php.net/rfc/strict_sessions
I'll implement DoS protection later, since current patch pretty well
tested and suitable for PHP 5.4/5.3, too.I've checked out the RFC and the patch, and I have couple of notes:
- Why we need separate validate call in the API? Can't we just do the
checks in open/read?open is used for creating session if does not exists. read cannot be
failed if there is session data, so separate API is needed to check if
it's a initialized one or not.
It is possible to make open always initialize with new session ID if
there is not exiting data. In this case, PS_MOD/PS_MOD_FUNCS macro can
be used. If we do this, users cannot know if ps_modules is new one
that prevents adoption or not, especially for third party ps_modules.
If API is the same, it cannot distinguish if ps_module supports
validation or not.
Those who are using user save handlers should rewrite open function so
that it validates session ID and regenerate session ID using
session_regenerate_id()
. It would nicer if we provide API for
validation and there will be less mistakes in user save handlers.
--
Yasuo Ohgaki
- Very restrictive limits on session key values don't look useful for me -
I know some custom solutions use characters beyond alphanumerics in session
IDs. Of course it can be worked around with encoding, etc. - but what does
it add?It mainly for precaution for unknown security risks. Allowing special
chars often became cause of security incidents. It may be ok for allow
more chars or just denying lower special chars may be sufficient.By the way, I'm planning to write security check module that checks
all inputs at once. It may detect session ID cookie as possibly
malicious input, but users may ignore anyway.
- Why replacing php_session_create_id with custom functions doing the same
in each standard module?Bundled ps_module is now using PS_MOD_SID/PS_MOD_FUNCS_SID instead of
PS_MOD/PS_MOD_FUNCS. In order to use validate function, use of
PS_MOD_SID/PS_MOD_FUNCS_SID is required. As you can see, bundled
ps_modules are just calling internal session ID creation function
which has already been there.PS_MOD_SID/PS_MOD_FUNCS_SID may implement it's own session ID
generation algorithms for whatever reasons. For example, it allows
ps_module writers to give special meaning to session IDs if they want.We can force to use internal session ID creation function, though.
- I'm not feeling very comfortable getting such a big change (API change,
logic change, etc.) with unknown effects this late in 5.4. I'd much better
prefer doing it in 5.4.1 but API change doesn't really allow that either.For ps_module writers, they still can use old API. User module may use
new API (i.e. new parameters for session ID creation and session ID
validation function), but they still may use old way, too. I would
prefer to have this patch for 5.4.0, but I don't insist. I've waited
for more than 6 years already, so it does not matter much :)--
Yasuo Ohgaki