Re: [VOTE] Proposed API change to the Manager interface
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
- 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
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
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]