# [RFC] Strict Session - Thread: https://externals.io/message/56677 - Original archive: https://news-web.php.net/php.internals/56677 - Started by Yasuo Ohgaki on 2011-11-30T07:59:02+00:00 - Messages: 4 - Reply by email to internals@lists.php.net - More for agents: search with https://externals.io/search.md?q={query} · JSON API at https://externals.io/api · documentation at https://externals.io/llms.txt --- ## #56677 — Yasuo Ohgaki — 2011-11-30T07:59:02+00:00 URL: https://externals.io/message/56677#56677 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 --- ## #56678 — Stas Malyshev — 2011-11-30T08:38:56+00:00 URL: https://externals.io/message/56677#56678 · In reply to: #56677 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: 1. Why we need separate validate call in the API? Can't we just do the checks in open/read? 2. 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? 3. Why replacing php_session_create_id with custom functions doing the same in each standard module? 4. 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 --- ## #56681 — Yasuo Ohgaki — 2011-11-30T10:35:43+00:00 URL: https://externals.io/message/56677#56681 · In reply to: #56678 Hi Stats, 2011/11/30 Stas Malyshev : > > […] > > > I've checked out the RFC and the patch, and I have couple of notes: > > 1. 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. > 2. 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. > 3. 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. > 4. 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 --- ## #56682 — Yasuo Ohgaki — 2011-11-30T10:49:05+00:00 URL: https://externals.io/message/56677#56682 · In reply to: #56681 Hi Stats, One additional comment. 2011/11/30 Yasuo Ohgaki : > Hi Stats, > > 2011/11/30 Stas Malyshev : > > […] > > 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 > > > […] > > 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. > > > […] > > 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. > > > […] > > 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