Hi! > CREATED is used to determine if the session should be renewed. > i.e. session_regenerate_id()
This looks like something that belongs in userspace. > UPDATED is used to determine if the session is expired or not. > "UPDATED+ttl < now" is expired. We already have GC mechanism that does that, why duplicate it? > For most save handlers, gc's timestamping is not precise enough for > gc. gc timestamp is updated even for obsolete session also. It cannot > replace UPDATED timestamp. I'm sorry, I don't understand it - why it's not precise enough? It's a regular unix timestamp AFAIK, so it's as precise as the rest of timestamps everywhere. I am also not sure what you mean by "obsolete session" and why updating its timestamp is a problem. > Browser may not get new session ID always due to bad network, etc. > e.g. Server send new session ID cookie, but connection is lost, then > browser will continue to use old session cookie ID. This is one of the > cause of random lost sessions. I don't think we should try to fix bad network in PHP. If you didn't get new session ID, then it's lost and the user will get a new login screen and will log in again. That's what happens when you have bad network. We shouldn't add complexity into PHP to fix bad networks. > My patch try to resend new session ID once for this case. This will > reduce number of lost sessions close to zero, hopefully. What if you have bad network twice? If it's bad, then it's bad. Can happen more than once. > It's not hard expiration. I think this should be done in user space. > > UPDATED will be updated by specified frequency(ttl_update). > Session expiration is checked "UPDATED + ttl < now". In this case, we already have gc lifetime parameter. > Users must regenerate session ID for security reasons. Even if > periodic session ID regeneration is mandatory for security, there > are many codes that don't do this. I think this belongs in userspace. And I don't see it as mandatory - for some applications, it's perfectly fine to keep long sessions. For others, it's not. > Old session IDs are needed because users may use session IDs > for security protections. CSRF protection key is common usage. > e.g. $csrf_key = sha1(session_id() . $mysecret) something like > that. That seems to be bringing a particular implementation of a particular software into core. This does not look like a good design. If your CSRF handling requires keeping old IDs, by all means keep them, but in core nothing requires that. >> 6. Changes like "Use strict mode by default (disallow stealing session >> forever). Use httponly cookie by default. Use SHA1 as hash (and use 5 >> bits for hashed value string for better compatibility)" are not >> described further and not clear how they are connected to the rest - is >> it necessary for the rest of the RFC to work? Why? > > This RFC target is precise (~= secure) session management. This > is the reason why these are included. (As well as session_id() > accepts any chars, but we agreed to work on this in another RFC) It would still be good to explain why those must be defaults and why current defaults are not fine. I can see strict mode by default being ok. httponly cookie is questionable, as it may break some JS apps that may need access to session cookie. But we may put it to a vote and see what people think. For hash function, I don't see how it matters even a little bit, given that session IDs are random. > If you see the patch, there are many phpt changes, but code and > logic is rather straight forward. Many phpt changes - if those are changes to existing phpt - usually means a lot of BC breakage. That is exactly what I am concerned with. There's no point in releasing new version which nobody would use because it breaks all their applications. > I've already have/will have 3 RFC for session. > This one, session_id() and user space serialize handler. > https://github.com/php/php-src/pull/1732 > I would like not to have too many RFCs for session. In this case, discussing so many unrelated changes in one RFC is worse, IMO, since it is hugely distracting and very hard to keep track of, and very easy to forget something. Unless there is a framework that unites those changes, but so far I didn't see it in the RFC, I just saw a lot of independent changes. > As you can see from patches, the more I have RFC, the more works for > phpt adjustment are required _separately_. Besides writing and managing > RFC, it consumes my time a lot. That's why I suggested to do a focused effort instead of doing too many things at once. > I'm certain that changes proposed by this RFC are mandatory or almost > mandatory for better security and session stability. Unfortunately, I am not sure yet it was proven that they are mandatory. Many of them pertain to very specific scenarios of very specific implementations and as such, would be much better done in userspace. -- Stas Malyshev smalys...@gmail.com -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
