Hi Stats, One additional comment.
2011/11/30 Yasuo Ohgaki <yohg...@ohgaki.net>: > Hi Stats, > > 2011/11/30 Stas Malyshev <smalys...@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: >> >> 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. 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 > >> 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 -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
