I think this is a valid point.

Well, there is still no 'session timeout after n minutes of 
inactivity' feature in horde, so not really.

But I've given up arguing since i didn't perceive the discussion as 
constructive anymore. We solved the problem by deploying tmpwatch on 
the session directory, so honestly I don't care too much...

Is everybody happy now with the current situation/setup?

Thanks for your extensive answer. I learned a lot.

So the only question that remains is that now cookie-lifetime is equal 
to session-timeout due to

https://github.com/horde/horde/commit/722b1912b37d2ece4d991193d157cf7179cbee0c

So this means that if I want the user to be logged out after she 
closes her browser, I need to set it to 0 and to have a user logged 
out *some* time (x seconds after she closed her tab) I need to 
configure the x seconds in the gc_maxlifetime. Right? This is not that 
obvious from how things are currently explained, but I think I got now 
the picture.

Many years ago, I was retained by a large company that wanted to 
further adapt Horde (mostly IMP at the time) to enterprise-level 
performance.  At the beginning of this work, we looked at the 
bottlenecks of the system.  Two things stood out (there was more than 
two... but these were the first time items on the list and pertinent 
for the present discussion):

1) c-client's performance was atrocious
2) session retrieval/saving took up way too much resources for any 
given request

Admittedly, the former was the much bigger performance issue than the 
latter (2-3 times).  Hence Horde_Imap_Client was born.  But I digress...

As for the latter, we were seeing things like an internal network, 
solely for purposes of interacting with the session storage, being 
completely swamped.  This was because of several factors:

1) We were using session storage as a general cache.  So session sizes 
were way too large.  This was remedied by moving large data items out 
of the session.
2) Along with 1, session data on the wire could be large - 50-100 KB 
may not sound like much, but on every access and with 1,000s of active 
users, it adds up.  An easy way to reduce the load was implementing 
lzf compression of session data.
3) unserializing is not free.  Pretty much dependent on session size (see 1)

...and, important for our current discussion:

4) serializing is not free.  Dependent on 1, but this action is 
completely avoidable if session data does not change in a request.
5) See number 2 above - session data then needs to be sent out on the 
wire... again.
6) There is no guarantee of the performance of the session storage 
backend.  For people who use SQL databases to store session data, 
write times and propagation across a cluster can take forever (one of 
the reasons session data is NOT RECOMMENDED for SQL databases).  Not 
to mention that theses SQL databases may have a caching component in 
front of it - e.g. memcache.  So *2* systems potentially need to be 
updated on every session write

What we learned is that by preventing/limiting session writes, 
performance was vastly improved.

And from a theoretical perspective of how session data is supposed to 
be used, this makes sense.  Session data is there to store things like 
authentication/configuration information.  These are things that 
normally occur once per application upon initial authentication.   
Now... session storage is flexible enough to allow data to be stored 
in it at a later time, but this should be avoided as a 
practice/performance consideration as much as possible.
Exactly.  Not only would you lose the performance advantage, but you 
lose the performance advantage WITHOUT ANY ADDITIONAL FUNCTIONALITY 
THAT DOESN'T ALREADY EXIST.

Additionally, I think you are mistaking session expiration as a 
front-line security mechanism.  It isn't.  Obviously, leaving sessions 
active forever is a bad idea.  But the simple fact that a session is 
slightly aged in and of itself is NOT an appreciable security risk.

As I have mentioned before, and Jan repeated, I have yet to hear how 
something like a 30 minute timed session that is not actually removed 
until 45 minutes is in actual security risk.
See above.

That's why I have to laugh when I see people post on the PHP 
manual/other random internet site and act like session storage is 
"free".  That is so near-sighted, it is comical.  Yes, it works on a 
single-user site.  But expecting that to work on any halfway loaded 
installation with any sort of speed is sort of like expecting an 
sqlite database to be able to scale up to the size of serving all 
Google search requests.

Almost. We have different views on whether in story 3, the session 
must timeout 31 minutes after the last activity, or "some time" after 
30 minutes of inactivity. We think the latter is sufficient too.
Correct, and this is what has been done since ever.
Correct and already implemented this way.
Yes, but so is it if using PHP's session garbage collection.
I can't tell about the cost of storing the session if not necessary, 
but Michael has put a lot of work into only writing to the session if 
really necessary, so I trust him that there are very good reason to 
not write to the session on every request.
As a compromise I could imagine only doing this on explicit demand by 
the administrator, per configuration, at the expense of performance 
penalty. But I don't really see this being necessary.
It's not purely random, but it's the administrator's responsibility to 
tweak the PHP GC settings so that sessions will be cleaned up in a 
timeframe acceptable for his security requirements and, more 
important, matching his site's traffic.
I'm with Michael that it's perfectly fine to not being able to set the 
session timeout on the minute, but rely on PHP's session garbage 
collection which exact purpose it to kill outdated sessions. If it 
doesn't do this quick enough for you, and you accept the additional 
performance penalty (which you do, see above), set a higher probability.
I'll leave this to Michael to explain why he has such a strong opinion 
about this.

So may I contribute my 2c here? To be honest I didn't look at the 
code, I'd ratjer like to formulate my expectations to how I would love 
to see  session expiry working.

Sidenote: For the time being it's irrelevant to me whether we 
distinguish between real-user activity or mailbox polling. I think 
this is a seperate issue. More on that later.

So I'd like to have horde this way:

1. cookie_timeout: When I close the browser the cookie should expire 
(means cookie_timeout should be 0). This means that when I'm using a 
normal browser that respects this setting I should not anymore be 
logged in, when I access the webmail the next time I start the 
browser. This is quite a crucial setting as it is the most efficient 
way to address forgotten logouts on shared computers. If I set 
cookie_timeout to something bigger thant 0, my cookie should live that 
long and also survive browser restarts and so on.

2. max_session_lifetime: This should be the maximum time I want users 
to be able to be logged in. Users are forced to log in again after 
that time. No matter how much activity happens within their session. 
Main reasons for that is to enforce users to run their login tasks.

3.  session_timeout: This should be the maximum time a session should 
live without any further requests. This means for example, if this is 
set to 30min. and I close my webmail tab (but not the whole browser) 
and try access webmail after 31 minutes again I need to log in again.

As I mentioned above for simplicity I do not want to distinguish 
currently between automated/background polls and real user 
interaction. This means that for point 3, if I leave my webmail tab 
open, I will be logged in also after 35 minutes. But still if I close 
my tab and so no polling happens I am logged out. Which is something 
that you, Michael, completely miss in comment #8.

Do we agree on these user stories or did I miss something, get 
something wrong?

Implementation brainstorming (as I said I have no idea how it is done 
currently):

Point 1 is a setting the timeout value of the cookie. That's not a big 
deal: copy setting value to cookie value.

Point 2: If I record the time the session got created and compare that 
value with max_session_lifetime each time I validate the session, I 
will perfectly know when I have to not accept the session anymore.

Point 3: If I record the time of the current request in the session 
and on the next request I check if that value is more than 
session_timeout ago and hence do not accept the session as valid, if 
it's beyond that time, I also have user story 3 implemented. Right?

So point 1 is unrelated to point 2 & 3 and hence an independent 
setting. Implementing point 2 means to store the creation time and 
compare it on each request. Point 3 is to store on each request the 
current time and compare the previous value to the current time. I 
don't expect this to be that expensive as I expect horde to write much 
more things on each request and session storage is usually something 
that should be written quickly.

The first answer in 
http://stackoverflow.com/questions/1236374/session-timeouts-in-php-best-practices provides an in my opinion a pretty good explanation how something like that should look like and why you can't count on gc_maxlifetime. The only thing in my opinion gc is for, is to garbage collect sessions that did not get closed by the user itself (read user logged out).  As mentioned before the gc probability can be quite low and so sessions can live much longer than expected. And well if longer would only be 5 minutes it would probably not be a that big issue, but if it's 2 to 3 times longer than what is set in session_timeout it's simply not working anymore as one would expect. Also there is no way to make that more controllable, it's purely 
random.

I didn't yet get why it should be a bad idea to write to the session 
on each request. The only thing I can think of is that the write 
operation is expensive. But I think horde is probably doing much more 
expensive operations during a request than updating a session. But I'm 
not an expert.

no, after your changes, this config option now sets the gc_timeout 
*and* cookie timeout to always the same value.

if i want to set gc_timeout to n seconds but cookie timeout to 0 (like 
you suggest) then i'm now deprived of any option to do that. In a 
typical setup (as many other webservices deploy) the session timeout 
would be ~30mins and the maximal session lifetime ~2h. so the attack 
window quadruples if you remove the possibility to set gc_timeout 
independently.
I already explained that i'm not talking about 5 mins, but several 
more. so i don't see an argument here. additionally, by coupling 
session and cookie timeout, this discussion becomes obsolete, so why 
do you argue about 5 mins, when there is no usable timeout mechanism 
and max_time is enforced to the second already.
why do you just say that, when every other webmail software disagrees 
with you?
why should it take time to get the session id? this assumption is 
completely bogus. i guess the most likely attack vector is someone not 
closing the browser on a public client (e.g. closing tab or walking 
away) therefore it is crucial to minimize the attack window. the 
attack window starts when the user makes his last action and ends when 
the session timeouts. it should not be > 20 minutes in my opinion.
then i would consider this a bug to the session timeout mechanism, 
since it should detect real user action vs. automatic refresh.
ok i'm repeating myself. this is not true, there is a common threat 
model (not closing the browser) and a simple technical solution 
(timeout a session some minutes after the last real user activity) and 
almost all reasonable webservices which have some sort of a login 
system do exactly that.
then consider all the situations....

Not to mention the idea of a session "timeout" being the last time you 
accessed a server is a dangerous concept.  If using something like 
dynamic IMP, your session will NEVER time out.  So your proposal 
actually opens up additional security holes.

The only way to correctly "timeout" a session is to implement a time 
limit AT THE TIME OF THE INITIAL AUTHENTICATION.  This is what we 
provide via the max_time configuration option.  Anything else might 
help in certain situations (e.g. a single user system) but will hurt 
in other situations (a single user system where the user never closes 
their browser).

Huh?  How does this make things worse?  This doesn't affect session 
timeouts.  This only affects COOKIE timeouts.

You obviously can't enforce session timeouts on the browser side.

The gc code is in there to ensure there is SOME way of enforcing a 
timeout and/or cookies are not being used.  But that code will be 
removed if we make the change to the default value of max_time like I 
already proposed.
Because people apparently want to be able to close their browser and 
then reopen and not lose their cookies.  I don't agree with it, but 
the configuration option has existed for awhile (and was essentially 
unused otherwise).

You have yet to explain HOW it is a "security issue" when, for 
example, a session lasts 35 minutes and the session timeout value is 
actually 30 minutes.  What about those extra 5 minutes makes it a 
"security issue"?

We don't guarantee a session will automatically timeout at 30 minutes 
and 1 second, and why would we?  Session timeouts are not (and should 
not) be an exact value.  Session timeouts are there to prevent a 
SINGLE attack vector: someone manages to obtain your session 
credentials/ID (the assumption being that this takes time) and can 
then use this to access an unexpired session at some point in the 
future.  Having a session persist 5-10 minutes beyond its timeout 
value does not materially affect/change this vector.

Those links you provided are not helpful.  You absolutely do NOT want 
to be setting/changing a "timestamp" in your session every page 
access.  Yikes.

oh now i see the new commits.

as far as i can tell, they make the problem worse, as they combine 
cookie lifetime and gc_maxlifetime into one config setting. so now i 
cannot even get the weak security properties of setting 
gc_maxlifetime, since it also affects cookie lifetime.

and as the comment in the config file correctly states, setting cookie 
lifetime to "a non-zero value is NOT RECOMMENDED [...] this will most 
certainly not work the way you want/think it should"

so why even provide this option??

I disagree strongly with the comments. Horde has no reliable session 
inactivity timeout mechanism and this needs to be addressed. How can 
you argue not to fix a security issue, because its hard to implement??

horde currently relies solely on gc_maxlifetime to discard inactive 
sessions, which is not reliable!

see e.g. 
http://stackoverflow.com/questions/1236374/session-timeouts-in-php-best-practices

Do Michael's latest commits close this ticket?

done:
https://github.com/horde/horde/pull/40

Currently horde relies only on session.gc_maxlifetime to enforce session
timeouts.

Especially on systems where gc_probability needs to be low for
performance reasons or on low traffic servers this is a serious
problem, since sessions might be significantly longer valid, than it was
intended by the admin by setting conf['session']['timeout'].

Therefore we should always check the last modification of the
session and deny authentication if the session should have timeouted.

still working on a patch...