Re: [VOTE] Proposed API change to the Manager interface

2005-02-10 Thread Remy Maucherat
Rainer Jung wrote:
I have the impression you fixed the jvmRoute in the new Session(-Id) in
ManagerBase, but not in DeltaManager? Even when the context is
distributable one might prefer to stick the session to the node where it
now originates.
You mean the commented out code ? It would need to be cut  pasted to 
the delta manager. I had it originally in both managers.

Furthermore: I did not really understand the use case of emptySessionPath,
The use case is when something abuses cross context, such as a portlet 
container.

but want to indicate 2 problematic areas:
1) If the cookie contains an expired session(-id) then you would allow a
new session to be started with the old id. That is not a naive security
problem, because the session is empty, so authentication has to be done
again etc.
It is still tied to the same user, and as you said the object will be 
brand new. I don't understand why it is a problem.

But there is a more complex threat: Security of webapps relies on the
fact, that session ids are not easily guessable. Once you move into a
situation where a user might use the same session id for a very long time
(in fact different sessions) the id will loose it's security. Normally
it's hard to find out the id of a user session while the session is still
active. And the user can at every time end his session. If he is going to
use the same Id for many sessions you have a much better chance to get
knowledge about the id between sessions and then to just wait for a new
session to begin.
That's what I thought at first, but I actually don't think it's a problem.
There was nothing preventing from using a session for a very long time 
before, depending on the server settings. With the session space (128 
bits) we are using, it does not matter, the ids will be unguessable as 
soon as the random number generator is properly seeded.

And if you really care about security, use SSL ;)
2) There might be problems by putting an arbitrary string coming from an
uncontrolled Cookie and using it as a session id inside tomcat. I don't
know, if there is a way related to cross site scrpting or injection of
malicious code for cookies (of course I know, that there is no scripting
inside Cookies, but atackers are very ingenious nowadays). At least one
should check the string for formal correctness (e.g. containing only
letters and numbers or something similar).
Besides the cross scripting used with a URL, I am not aware of any valid 
cookie which would cause problems. Invalid cookies will not be parsed.

A global session id list is needed to verify the submitted id exists 
(across all contexts). I don't think such a list is worth it, though.

Rémy
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: [VOTE] Proposed API change to the Manager interface

2005-02-09 Thread Rainer Jung
I have the impression you fixed the jvmRoute in the new Session(-Id) in
ManagerBase, but not in DeltaManager? Even when the context is
distributable one might prefer to stick the session to the node where it
now originates.

Furthermore: I did not really understand the use case of emptySessionPath,
but want to indicate 2 problematic areas:

1) If the cookie contains an expired session(-id) then you would allow a
new session to be started with the old id. That is not a naive security
problem, because the session is empty, so authentication has to be done
again etc.

But there is a more complex threat: Security of webapps relies on the
fact, that session ids are not easily guessable. Once you move into a
situation where a user might use the same session id for a very long time
(in fact different sessions) the id will loose it's security. Normally
it's hard to find out the id of a user session while the session is still
active. And the user can at every time end his session. If he is going to
use the same Id for many sessions you have a much better chance to get
knowledge about the id between sessions and then to just wait for a new
session to begin.

2) There might be problems by putting an arbitrary string coming from an
uncontrolled Cookie and using it as a session id inside tomcat. I don't
know, if there is a way related to cross site scrpting or injection of
malicious code for cookies (of course I know, that there is no scripting
inside Cookies, but atackers are very ingenious nowadays). At least one
should check the string for formal correctness (e.g. containing only
letters and numbers or something similar).



-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]



Re: [VOTE] Proposed API change to the Manager interface

2005-02-08 Thread Peter Rossbach
Hmm,
I think that Remy's patch is working correct. But I think we must 
controll the jvmRoute
from those client sessions id's (ManagerBase include this at a comment :-)).
I must check my cluster failover szenarios. I can to that tomorrow.

Peter
Rainer Jung schrieb:
Both Cluster Managers (Delta and SimpleTcpReplication) extend ManagerBase
but override createSession().
Maybe Filip or Peter can check how to make them long-time compatible with
your proposal?
 

Managers extending ManagerBase should work and compile as before with no
   

changes unless they override the createSession method.


-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

 



-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: [VOTE] Proposed API change to the Manager interface

2005-02-08 Thread Remy Maucherat
Peter Rossbach wrote:
Hmm,
I think that Remy's patch is working correct. But I think we must 
controll the jvmRoute
from those client sessions id's (ManagerBase include this at a comment 
:-)).
I must check my cluster failover szenarios. I can to that tomorrow.
Yes, I don't know if it is needed or not, so testing is useful.
Rémy
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: [VOTE] Proposed API change to the Manager interface

2005-02-08 Thread Filip Hanik - Dev
Remy Wrote:
BTW, wouldn't it be better to set the route as a separate cookie, which
would be a lot cleaner ? Was this ever considered ?

That would have been the obvious solution, the jvmRoute must have come from a 
dark basement somewhere :)

Whatever the change you decide on, I'll be happy to implement it and test it 
for the cluster stuff

Filip

- Original Message -
From: Remy Maucherat [EMAIL PROTECTED]
To: Tomcat Developers List tomcat-dev@jakarta.apache.org
Sent: Monday, February 07, 2005 11:34 AM
Subject: Re: [VOTE] Proposed API change to the Manager interface


 @@ -744,15 +747,17 @@
  session.setValid(true);
  session.setCreationTime(System.currentTimeMillis());
  session.setMaxInactiveInterval(this.maxInactiveInterval);
 -String sessionId = generateSessionId();
 +if (sessionId == null) {
 +sessionId = generateSessionId();
 +}

I just noticed my patch needs something for jvmRoute handling
(basically, the session id which is recieved must be edited for the
right route). I'll fix that.

BTW, wouldn't it be better to set the route as a separate cookie, which
would be a lot cleaner ? Was this ever considered ?

Rémy

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]



Re: [VOTE] Proposed API change to the Manager interface

2005-02-08 Thread Rainer Jung
I'm afraid that change will have negative implications: The jvmRoute is 
used to enable routing decisions by balancing components. All these 
components usually support routing either COOKIE-based or URI-based.

In the URI-based case it is more or less the only clean way to include 
the jvmRoute in the jsessionid, since the jsessionid is standardized, so 
all common balancing products know how to handle it.

But then it is pretty common to assume the jsessionid and the cookie to 
have the same value. They are sort of two different ways to transport 
the same session information. So most balancer providers implement the 
routing decision features the same way, independant of the source of the 
session string.

Splitting the jvmRoute from the session id in the cookie case will most 
likely make the situation for all implementers of balancers more complex 
and instable (e.g.: mod_jk).

Of course we would all profit, if some JSR would standardize the way, 
distributed applications exchange routing information with the 
clients/balancers. As long as that's not the case it is very likely, 
that jvmRoute as a suffix of the session id has much better support from 
balancer providers.

Rainer
Filip Hanik - Dev wrote:
Remy Wrote:
BTW, wouldn't it be better to set the route as a separate cookie, which
would be a lot cleaner ? Was this ever considered ?

That would have been the obvious solution, the jvmRoute must have come from a 
dark basement somewhere :)
Whatever the change you decide on, I'll be happy to implement it and test it 
for the cluster stuff
Filip
- Original Message -
From: Remy Maucherat [EMAIL PROTECTED]
To: Tomcat Developers List tomcat-dev@jakarta.apache.org
Sent: Monday, February 07, 2005 11:34 AM
Subject: Re: [VOTE] Proposed API change to the Manager interface

@@ -744,15 +747,17 @@
session.setValid(true);
session.setCreationTime(System.currentTimeMillis());
session.setMaxInactiveInterval(this.maxInactiveInterval);
-String sessionId = generateSessionId();
+if (sessionId == null) {
+sessionId = generateSessionId();
+}

I just noticed my patch needs something for jvmRoute handling
(basically, the session id which is recieved must be edited for the
right route). I'll fix that.
BTW, wouldn't it be better to set the route as a separate cookie, which
would be a lot cleaner ? Was this ever considered ?
Rémy
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: [VOTE] Proposed API change to the Manager interface

2005-02-08 Thread Filip Hanik - Dev
its not at all that complicated.
This is how a big5 load balancer does it.

A) it sets a cookie, and based on the cookie it load balances.
B) if a cookie is not supported, it does a calculation based on the IP address, 
and stays sticky that way.

No need to exchange any info in this scenario and very straight forward, and 
never goes wrong. The only time it goes wrong is for
AOL users who can change gateway between HTTP and HTTPS

Filip

- Original Message -
From: Rainer Jung [EMAIL PROTECTED]
To: Tomcat Developers List tomcat-dev@jakarta.apache.org
Sent: Tuesday, February 08, 2005 10:39 AM
Subject: Re: [VOTE] Proposed API change to the Manager interface


I'm afraid that change will have negative implications: The jvmRoute is
used to enable routing decisions by balancing components. All these
components usually support routing either COOKIE-based or URI-based.

In the URI-based case it is more or less the only clean way to include
the jvmRoute in the jsessionid, since the jsessionid is standardized, so
all common balancing products know how to handle it.

But then it is pretty common to assume the jsessionid and the cookie to
have the same value. They are sort of two different ways to transport
the same session information. So most balancer providers implement the
routing decision features the same way, independant of the source of the
session string.

Splitting the jvmRoute from the session id in the cookie case will most
likely make the situation for all implementers of balancers more complex
and instable (e.g.: mod_jk).

Of course we would all profit, if some JSR would standardize the way,
distributed applications exchange routing information with the
clients/balancers. As long as that's not the case it is very likely,
that jvmRoute as a suffix of the session id has much better support from
balancer providers.

Rainer

Filip Hanik - Dev wrote:

 Remy Wrote:

BTW, wouldn't it be better to set the route as a separate cookie, which
would be a lot cleaner ? Was this ever considered ?


 That would have been the obvious solution, the jvmRoute must have come from a 
 dark basement somewhere :)

 Whatever the change you decide on, I'll be happy to implement it and test it 
 for the cluster stuff

 Filip

 - Original Message -
 From: Remy Maucherat [EMAIL PROTECTED]
 To: Tomcat Developers List tomcat-dev@jakarta.apache.org
 Sent: Monday, February 07, 2005 11:34 AM
 Subject: Re: [VOTE] Proposed API change to the Manager interface



@@ -744,15 +747,17 @@
 session.setValid(true);
 session.setCreationTime(System.currentTimeMillis());
 session.setMaxInactiveInterval(this.maxInactiveInterval);
-String sessionId = generateSessionId();
+if (sessionId == null) {
+sessionId = generateSessionId();
+}


 I just noticed my patch needs something for jvmRoute handling
 (basically, the session id which is recieved must be edited for the
 right route). I'll fix that.

 BTW, wouldn't it be better to set the route as a separate cookie, which
 would be a lot cleaner ? Was this ever considered ?

 Rémy

 -
 To unsubscribe, e-mail: [EMAIL PROTECTED]
 For additional commands, e-mail: [EMAIL PROTECTED]


 -
 To unsubscribe, e-mail: [EMAIL PROTECTED]
 For additional commands, e-mail: [EMAIL PROTECTED]


-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]



Re: [VOTE] Proposed API change to the Manager interface

2005-02-08 Thread Remy Maucherat
Filip Hanik - Dev wrote:
its not at all that complicated.
This is how a big5 load balancer does it.
A) it sets a cookie, and based on the cookie it load balances.
B) if a cookie is not supported, it does a calculation based on the IP address, 
and stays sticky that way.
No need to exchange any info in this scenario and very straight forward, and 
never goes wrong. The only time it goes wrong is for
AOL users who can change gateway between HTTP and HTTPS
I don't think it's that complicated either, but it would still require 
an upgrade of mod_jk to work, so it's probably not an option :(

Rémy
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: [VOTE] Proposed API change to the Manager interface

2005-02-08 Thread Rainer Jung
I don't fully agree: we use F5 too, but:
1) I know many commercial sites (in germany), which do not use Cookies 
(privacy reasons).

2) IP based stickyness does not work very well. In many scenarios you 
will only be able to correctly route most of your clients. If you rely 
on exact routing, IP-based will not suffice.

3) These are the reasons why I assume, that all balancers will support 
routing decisions based on the jsessionid in the URI. So they already 
have to support extracting routing information from the session id string.

So using the same jessionid string in the standard cookie will work in 
most (all ?) situations / for most products.

Example: mod_jk does it like that:
/* Retrieve session id from the cookie or the parameter 
 */
static char *get_sessionid(jk_ws_service_t *s)
{
char *val;
val = get_path_param(s, JK_PATH_SESSION_IDENTIFIER);
if (!val) {
val = get_cookie(s, JK_SESSION_IDENTIFIER);
}
return val;
}

and after that it does no longer matter, where the info comes from.
On the other hand: what's wrong about embedding the jmvRoute as a suffix 
in the session id?

Rainer
Filip Hanik - Dev wrote:
its not at all that complicated.
This is how a big5 load balancer does it.
A) it sets a cookie, and based on the cookie it load balances.
B) if a cookie is not supported, it does a calculation based on the IP address, 
and stays sticky that way.
No need to exchange any info in this scenario and very straight forward, and 
never goes wrong. The only time it goes wrong is for
AOL users who can change gateway between HTTP and HTTPS
Filip
- Original Message -
From: Rainer Jung [EMAIL PROTECTED]
To: Tomcat Developers List tomcat-dev@jakarta.apache.org
Sent: Tuesday, February 08, 2005 10:39 AM
Subject: Re: [VOTE] Proposed API change to the Manager interface
I'm afraid that change will have negative implications: The jvmRoute is
used to enable routing decisions by balancing components. All these
components usually support routing either COOKIE-based or URI-based.
In the URI-based case it is more or less the only clean way to include
the jvmRoute in the jsessionid, since the jsessionid is standardized, so
all common balancing products know how to handle it.
But then it is pretty common to assume the jsessionid and the cookie to
have the same value. They are sort of two different ways to transport
the same session information. So most balancer providers implement the
routing decision features the same way, independant of the source of the
session string.
Splitting the jvmRoute from the session id in the cookie case will most
likely make the situation for all implementers of balancers more complex
and instable (e.g.: mod_jk).
Of course we would all profit, if some JSR would standardize the way,
distributed applications exchange routing information with the
clients/balancers. As long as that's not the case it is very likely,
that jvmRoute as a suffix of the session id has much better support from
balancer providers.
Rainer
Filip Hanik - Dev wrote:

Remy Wrote:

BTW, wouldn't it be better to set the route as a separate cookie, which
would be a lot cleaner ? Was this ever considered ?

That would have been the obvious solution, the jvmRoute must have come from a 
dark basement somewhere :)
Whatever the change you decide on, I'll be happy to implement it and test it 
for the cluster stuff
Filip
- Original Message -
From: Remy Maucherat [EMAIL PROTECTED]
To: Tomcat Developers List tomcat-dev@jakarta.apache.org
Sent: Monday, February 07, 2005 11:34 AM
Subject: Re: [VOTE] Proposed API change to the Manager interface


@@ -744,15 +747,17 @@
   session.setValid(true);
   session.setCreationTime(System.currentTimeMillis());
   session.setMaxInactiveInterval(this.maxInactiveInterval);
-String sessionId = generateSessionId();
+if (sessionId == null) {
+sessionId = generateSessionId();
+}

I just noticed my patch needs something for jvmRoute handling
(basically, the session id which is recieved must be edited for the
right route). I'll fix that.
BTW, wouldn't it be better to set the route as a separate cookie, which
would be a lot cleaner ? Was this ever considered ?
Rémy
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
-
To unsubscribe, e-mail: [EMAIL PROTECTED

Re: [VOTE] Proposed API change to the Manager interface

2005-02-08 Thread Rainer Jung
Upgrading mod_jk would be possible (not too complicated), but I'm afraid
we are going to break other products we don't control the way we do with
mod_jk.
mod_jk is a good example to see, why it's not unlikely that changes
would be needed for other balancers.
Rainer
Remy Maucherat wrote:
Filip Hanik - Dev wrote:
its not at all that complicated.
This is how a big5 load balancer does it.
A) it sets a cookie, and based on the cookie it load balances.
B) if a cookie is not supported, it does a calculation based on the IP 
address, and stays sticky that way.

No need to exchange any info in this scenario and very straight 
forward, and never goes wrong. The only time it goes wrong is for
AOL users who can change gateway between HTTP and HTTPS

I don't think it's that complicated either, but it would still require 
an upgrade of mod_jk to work, so it's probably not an option :(

Rémy
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: [VOTE] Proposed API change to the Manager interface

2005-02-07 Thread Tim Funk
If I read this correctly (big if), the solution could be a security problem. 
If I were a phisher, I might be able to send you an email with a link to that 
would redirect you to a tomcat server with the new configuration in question 
with a special id. If the user were to perform other actions of a 
confidential nature, the attacker could swoop in and impersonate that session.

The phisher could easily be notified that the special session id was used and 
 have a farm of wamr bodies ready to attack.


-Tim
Remy Maucherat wrote:
To address this, I propose simply reusing any session id presented by 
the client. As an option, this could be done only if emptySessionPath is 
true. I have tested it, and it works very well, addressing the problem 
with emptySessionPath=true. I could not think of any security issue 
this would cause (if the client submits a bogus insecure id, then he'll 
be the one being impacted), since the undelying session objects would be 
created and handled as usual. In normal more (emptySessionPath set to 
false), this would save costly session id generation.

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: [VOTE] Proposed API change to the Manager interface

2005-02-07 Thread Remy Maucherat
Tim Funk wrote:
If I read this correctly (big if), the solution could be a security 
problem. If I were a phisher, I might be able to send you an email with 
a link to that would redirect you to a tomcat server with the new 
configuration in question with a special id. If the user were to perform 
other actions of a confidential nature, the attacker could swoop in and 
impersonate that session.

The phisher could easily be notified that the special session id was 
used and  have a farm of wamr bodies ready to attack.
Yes, this could maybe be used like this (maybe we could simply prevent 
URL session ids from being used for this purpose, which would likely 
solve the issue - I hope you can't set a cookie on another host). 
Obviously, if you're not using cookies for session tracking, then the 
emptySessionPath attribute is useless anyway, so there's no bug ;)

The problem is that I see no other acceptable solution (a global list of 
session id, which would be unbounded, would be bad, and introduce lots 
of complexity - the API change might be still needed anyway).

What do you think ?
Rémy
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: [VOTE] Proposed API change to the Manager interface

2005-02-07 Thread Remy Maucherat
Tim Funk wrote:
If I read this correctly (big if), the solution could be a security 
problem. If I were a phisher, I might be able to send you an email with 
a link to that would redirect you to a tomcat server with the new 
configuration in question with a special id. If the user were to perform 
other actions of a confidential nature, the attacker could swoop in and 
impersonate that session.

The phisher could easily be notified that the special session id was 
used and  have a farm of wamr bodies ready to attack.
My patch at the moment is attached to the email for more detailed 
review. It no longer reuses session ids submitted in the URL (very good 
catch).

Rémy
Index: 
jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/Manager.java
===
RCS file: 
/home/cvs/jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/Manager.java,v
retrieving revision 1.14
diff -u -r1.14 Manager.java
--- jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/Manager.java 
7 Sep 2004 20:57:02 -   1.14
+++ jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/Manager.java 
7 Feb 2005 16:49:28 -
@@ -257,6 +257,7 @@
  */
 public void addPropertyChangeListener(PropertyChangeListener listener);
 
+
 /**
  * Get a session from the recycled ones or create a new empty one.
  * The PersistentManager manager does not need to create session data
@@ -264,17 +265,22 @@
  */
 
 public Session createEmptySession();
 
+
 /**
  * Construct and return a new session object, based on the default
  * settings specified by this Manager's properties.  The session
- * id will be assigned by this method, and available via the getId()
- * method of the returned session.  If a new session cannot be created
- * for any reason, return codenull/code.
- *
+ * id specified will be used as the session id.
+ * If a new session cannot be created for any reason, return 
+ * codenull/code.
+ * 
+ * @param sessionId The session id which should be used to create the
+ *  new session; if codenull/code, the session
+ *  id will be assigned by this method, and available via the getId()
+ *  method of the returned session.
  * @exception IllegalStateException if a new session cannot be
  *  instantiated for any reason
  */
-public Session createSession();
+public Session createSession(String sessionId);
 
 
 /**
Index: 
jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/connector/Request.java
===
RCS file: 
/home/cvs/jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/connector/Request.java,v
retrieving revision 1.18
diff -u -r1.18 Request.java
--- 
jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/connector/Request.java
   20 Nov 2004 21:10:47 -  1.18
+++ 
jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/connector/Request.java
   7 Feb 2005 16:49:29 -
@@ -2196,7 +2196,14 @@
   (sm.getString(coyoteRequest.sessionCreateCommitted));
 }
 
-session = manager.createSession();
+// Attempt to reuse session id if one was submitted in a cookie
+// Do not reuse the session id if it is from a URL, to prevent possible
+// phishing attacks
+if (isRequestedSessionIdFromCookie()) {
+session = manager.createSession(getRequestedSessionId());
+} else {
+session = manager.createSession(null);
+}
 
 // Creating a new session cookie based on that session
 if ((session != null)  (getContext() != null)
Index: 
jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/core/ApplicationHttpRequest.java
===
RCS file: 
/home/cvs/jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/core/ApplicationHttpRequest.java,v
retrieving revision 1.24
diff -u -r1.24 ApplicationHttpRequest.java
--- 
jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/core/ApplicationHttpRequest.java
 15 Jan 2005 20:31:21 -  1.24
+++ 
jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/core/ApplicationHttpRequest.java
 7 Feb 2005 16:49:29 -
@@ -529,13 +529,8 @@
 // Ignore
 }
 if (localSession == null  create) {
-localSession = context.getManager().createEmptySession();
-localSession.setNew(true);
-localSession.setValid(true);
-localSession.setCreationTime(System.currentTimeMillis());
-localSession.setMaxInactiveInterval
-(context.getManager().getMaxInactiveInterval());
-

RE: [VOTE] Proposed API change to the Manager interface

2005-02-07 Thread Yoav Shapira
Hi,

However, the problem is that I need to modify a method in the top level 
Manager interface :(

public Session createSession()
must become:
public Session createSession(String sessionId)
(if sessionId is null, a new session id will be generated)

As the createSession() will no longer be used anywhere, and calling it 
in old managers would create bad behavior, I propose removing it rather 
than deprecating it.

Why can't createSession() simply call createSession(null) ?  That way we
could deprecate it and preserve backwards compatibility, at least for a
little while.  I'm uneasy about simply breaking a major interface, one
that's customized often by users.

Other than that, and given Tim's catch and your fix, I'm OK with this.

Yoav


-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]



Re: RE: [VOTE] Proposed API change to the Manager interface

2005-02-07 Thread bounce
Geachte relatie,

Het door u gebruikte e-mailadres is niet meer actief. U kunt uw e-mailbericht 
sturen naar [EMAIL PROTECTED] of dit bericht beantwoorden.

Bedankt voor uw medewerking,

Met vriendelijke groet,

ATP Hypotheken
Het Spoor 40
3994 AK Houten

Tel. 030 750 25 33
Fax. 030 750 25 88
[EMAIL PROTECTED]

 -- DIT IS EEN AUTOMATISCH GEGENEREERD E-MAILBERICHT --



-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]



Re: [VOTE] Proposed API change to the Manager interface

2005-02-07 Thread Remy Maucherat
Yoav Shapira wrote:
Hi,
However, the problem is that I need to modify a method in the top level 
Manager interface :(

public Session createSession()
must become:
public Session createSession(String sessionId)
(if sessionId is null, a new session id will be generated)
As the createSession() will no longer be used anywhere, and calling it 
in old managers would create bad behavior, I propose removing it rather 
than deprecating it.
Why can't createSession() simply call createSession(null) ?  That way we
could deprecate it and preserve backwards compatibility, at least for a
little while.  I'm uneasy about simply breaking a major interface, one
that's customized often by users.
Right, that's what I tried at first. However, session tracking would 
break in mysterious ways whenever emptySessionPath is used if we did 
that. So I considered it was better to remove the method to signal some 
change is needed.

That was my idea, but I'm ok with adding back the method in Manager (and 
ManagerBase) as deprecated if you still think it's better.

Other than that, and given Tim's catch and your fix, I'm OK with this.
Ok.
The adjustment I mentioned in my last email to handle jvmRoute might not 
be needed.

Rémy
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: [VOTE] Proposed API change to the Manager interface

2005-02-07 Thread Yoav Shapira
Hi,

 Right, that's what I tried at first. However, session tracking would 
 break in mysterious ways whenever emptySessionPath is used if we did 
 that. So I considered it was better to remove the method to signal some 
 change is needed.
 
 That was my idea, but I'm ok with adding back the method in Manager (and 
 ManagerBase) as deprecated if you still think it's better.

Yeah, I still think it's better.  Marginally better and obviously not perfect,
but still better.  If you want to add a comment to the old createSession()
version that session tracking might break in mysterious ways if
emptySessionPath is used, that's fine too.  The important thing is we'll
deprecate it and maintain backwards compatibility for at least a few releases /
few months, then we can remove it altogether.  (And I'll be +1 for that, always
preferring a cleaner interface myself)

Yoav


-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]



Re: [VOTE] Proposed API change to the Manager interface

2005-02-07 Thread Bill Barker

- Original Message -
From: Remy Maucherat [EMAIL PROTECTED]
To: Tomcat Developers List tomcat-dev@jakarta.apache.org
Sent: Monday, February 07, 2005 8:57 AM
Subject: Re: [VOTE] Proposed API change to the Manager interface


Tim Funk wrote:
 If I read this correctly (big if), the solution could be a security
 problem. If I were a phisher, I might be able to send you an email with
 a link to that would redirect you to a tomcat server with the new
 configuration in question with a special id. If the user were to perform
 other actions of a confidential nature, the attacker could swoop in and
 impersonate that session.

 The phisher could easily be notified that the special session id was
 used and  have a farm of wamr bodies ready to attack.

My patch at the moment is attached to the email for more detailed
review. It no longer reuses session ids submitted in the URL (very good
catch).


I'd be happier if this was conditional on emptySessionPath=true (or
otherwise could be disabled).  Otherwise I have to trust that the browser
doesn't have some JavaScript and/or IFrame bug that allows a Cookie to be
sent.



Rémy







 Index:
jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/Manager.java
 ===
 RCS file:
/home/cvs/jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/Man
ager.java,v
 retrieving revision 1.14
 diff -u -r1.14 Manager.java
 ---
jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/Manager.java
7 Sep 2004 20:57:02 - 1.14
 +++
jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/Manager.java
7 Feb 2005 16:49:28 -
 @@ -257,6 +257,7 @@
   */
  public void addPropertyChangeListener(PropertyChangeListener
listener);

 +
  /**
   * Get a session from the recycled ones or create a new empty one.
   * The PersistentManager manager does not need to create session data
 @@ -264,17 +265,22 @@
   */
  public Session createEmptySession();

 +
  /**
   * Construct and return a new session object, based on the default
   * settings specified by this Manager's properties.  The session
 - * id will be assigned by this method, and available via the getId()
 - * method of the returned session.  If a new session cannot be
created
 - * for any reason, return codenull/code.
 - *
 + * id specified will be used as the session id.
 + * If a new session cannot be created for any reason, return
 + * codenull/code.
 + *
 + * @param sessionId The session id which should be used to create the
 + *  new session; if codenull/code, the session
 + *  id will be assigned by this method, and available via the getId()
 + *  method of the returned session.
   * @exception IllegalStateException if a new session cannot be
   *  instantiated for any reason
   */
 -public Session createSession();
 +public Session createSession(String sessionId);


  /**
 Index:
jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/connector/Req
uest.java
 ===
 RCS file:
/home/cvs/jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/con
nector/Request.java,v
 retrieving revision 1.18
 diff -u -r1.18 Request.java
 ---
jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/connector/Req
uest.java 20 Nov 2004 21:10:47 - 1.18
 +++
jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/connector/Req
uest.java 7 Feb 2005 16:49:29 -
 @@ -2196,7 +2196,14 @@
(sm.getString(coyoteRequest.sessionCreateCommitted));
  }

 -session = manager.createSession();
 +// Attempt to reuse session id if one was submitted in a cookie
 +// Do not reuse the session id if it is from a URL, to prevent
possible
 +// phishing attacks
 +if (isRequestedSessionIdFromCookie()) {
 +session = manager.createSession(getRequestedSessionId());
 +} else {
 +session = manager.createSession(null);
 +}

  // Creating a new session cookie based on that session
  if ((session != null)  (getContext() != null)
 Index:
jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/core/Applicat
ionHttpRequest.java
 ===
 RCS file:
/home/cvs/jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/cor
e/ApplicationHttpRequest.java,v
 retrieving revision 1.24
 diff -u -r1.24 ApplicationHttpRequest.java
 ---
jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/core/Applicat
ionHttpRequest.java 15 Jan 2005 20:31:21 - 1.24
 +++
jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/core/Applicat
ionHttpRequest.java 7 Feb 2005 16:49:29 -
 @@ -529,13 +529,8

Re: [VOTE] Proposed API change to the Manager interface

2005-02-07 Thread Rainer Jung
Both Cluster Managers (Delta and SimpleTcpReplication) extend ManagerBase
but override createSession().

Maybe Filip or Peter can check how to make them long-time compatible with
your proposal?

 Managers extending ManagerBase should work and compile as before with no
changes unless they override the createSession method.





-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]



Re: [VOTE] Proposed API change to the Manager interface

2005-02-07 Thread Remy Maucherat
Bill Barker wrote:
I'd be happier if this was conditional on emptySessionPath=true (or
otherwise could be disabled).  Otherwise I have to trust that the browser
doesn't have some JavaScript and/or IFrame bug that allows a Cookie to be
sent.
I think it should be safe, but once in a while there's a vulnerability 
allowing javascript access to the cookie store (in IE ;) ). We can 
change that later once it is proven to be safe enough.

Rémy
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]