----- 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 -0000 1.14
> +++
jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/Manager.java
7 Feb 2005 16:49:28 -0000
> @@ -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 <code>null</code>.
> -     *
> +     * id specified will be used as the session id.
> +     * If a new session cannot be created for any reason, return
> +     * <code>null</code>.
> +     *
> +     * @param sessionId The session id which should be used to create the
> +     *  new session; if <code>null</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 -0000 1.18
> +++
jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/connector/Req
uest.java 7 Feb 2005 16:49:29 -0000
> @@ -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 -0000 1.24
> +++
jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/core/Applicat
ionHttpRequest.java 7 Feb 2005 16:49:29 -0000
> @@ -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());
> -                    localSession.setId(other.getId());
> +                    localSession =
> +
context.getManager().createSession(other.getId());
>                  }
>                  if (localSession != null) {
>                      localSession.access();
> Index:
jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/session/Manag
erBase.java
> ===================================================================
> RCS file:
/home/cvs/jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/ses
sion/ManagerBase.java,v
> retrieving revision 1.37
> diff -u -r1.37 ManagerBase.java
> ---
jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/session/Manag
erBase.java 22 Nov 2004 14:50:23 -0000 1.37
> +++
jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/session/Manag
erBase.java 7 Feb 2005 16:49:29 -0000
> @@ -727,15 +727,18 @@
>      /**
>       * 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 <code>null</code>.
> -     *
> +     * id specified will be used as the session id.
> +     * If a new session cannot be created for any reason, return
> +     * <code>null</code>.
> +     *
> +     * @param sessionId The session id which should be used to create the
> +     *  new session; if <code>null</code>, a new session id will be
> +     *  generated
>       * @exception IllegalStateException if a new session cannot be
>       *  instantiated for any reason
>       */
> -    public Session createSession() {
> -
> +    public Session createSession(String sessionId) {
> +
>          // Recycle or create a Session instance
>          Session session = createEmptySession();
>
> @@ -744,15 +747,17 @@
>          session.setValid(true);
>          session.setCreationTime(System.currentTimeMillis());
>          session.setMaxInactiveInterval(this.maxInactiveInterval);
> -        String sessionId = generateSessionId();
> +        if (sessionId == null) {
> +            sessionId = generateSessionId();
> +        }
>          session.setId(sessionId);
>          sessionCounter++;
>
>          return (session);
>
>      }
> -
> -
> +
> +
>      /**
>       * Get a session from the recycled ones or create a new empty one.
>       * The PersistentManager manager does not need to create session data
> Index:
jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/session/Stand
ardManager.java
> ===================================================================
> RCS file:
/home/cvs/jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/ses
sion/StandardManager.java,v
> retrieving revision 1.27
> diff -u -r1.27 StandardManager.java
> ---
jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/session/Stand
ardManager.java 22 Nov 2004 16:35:18 -0000 1.27
> +++
jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/session/Stand
ardManager.java 7 Feb 2005 16:49:29 -0000
> @@ -278,7 +278,7 @@
>       * @exception IllegalStateException if a new session cannot be
>       *  instantiated for any reason
>       */
> -    public Session createSession() {
> +    public Session createSession(String sessionId) {
>
>          if ((maxActiveSessions >= 0) &&
>              (sessions.size() >= maxActiveSessions)) {
> @@ -287,7 +287,7 @@
>                  (sm.getString("standardManager.createSession.ise"));
>          }
>
> -        return (super.createSession());
> +        return (super.createSession(sessionId));
>
>      }
>
> Index:
jakarta-tomcat-catalina/modules/cluster/src/share/org/apache/catalina/cluste
r/session/DeltaManager.java
> ===================================================================
> RCS file:
/home/cvs/jakarta-tomcat-catalina/modules/cluster/src/share/org/apache/catal
ina/cluster/session/DeltaManager.java,v
> retrieving revision 1.36
> diff -u -r1.36 DeltaManager.java
> ---
jakarta-tomcat-catalina/modules/cluster/src/share/org/apache/catalina/cluste
r/session/DeltaManager.java 22 Nov 2004 14:51:18 -0000 1.36
> +++
jakarta-tomcat-catalina/modules/cluster/src/share/org/apache/catalina/cluste
r/session/DeltaManager.java 7 Feb 2005 16:49:30 -0000
> @@ -235,8 +235,8 @@
>       * @exception IllegalStateException if a new session cannot be
>       *  instantiated for any reason
>       */
> -    public Session createSession() {
> -        return createSession(true);
> +    public Session createSession(String sessionId) {
> +        return createSession(sessionId, true);
>      }
>
>
> @@ -247,7 +247,7 @@
>       * @param distribute
>       * @return
>       */
> -    public Session createSession(boolean distribute) {
> +    public Session createSession(String sessionId, boolean distribute) {
>
>        if ((maxActiveSessions >= 0) &&
>            (sessions.size() >= maxActiveSessions)) {
> @@ -258,13 +258,16 @@
>
>        // Recycle or create a Session instance
>        DeltaSession session = getNewDeltaSession();
> -      String sessionId = generateSessionId();
> -
> -      synchronized (sessions) {
> -        while (sessions.get(sessionId) != null) { // Guarantee uniqueness
> -          duplicates++;
> +      if (sessionId == null) {
>            sessionId = generateSessionId();
> -         }
> +
> +          synchronized (sessions) {
> +              while (sessions.get(sessionId) != null) { // Guarantee
uniqueness
> +                  duplicates++;
> +                  sessionId = generateSessionId();
> +              }
> +          }
> +
>        }
>
>        session.setNew(true);
> @@ -849,7 +852,8 @@
>                         if (log.isDebugEnabled())
>                             log.debug("Manager (" + name + ") received
session ("
>                              + msg.getSessionID() + ") created.");
> -                       DeltaSession session =
(DeltaSession)createSession(false);
> +                       DeltaSession session =
> +
(DeltaSession)createSession(msg.getSessionID(), false);
>                         // Q: Why inform all session listener a replicate
node?
>                         session.setId(msg.getSessionID());
>                         session.setNew(false);
>
>


----------------------------------------------------------------------------
----


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



This message is intended only for the use of the person(s) listed above as the 
intended recipient(s), and may contain information that is PRIVILEGED and 
CONFIDENTIAL.  If you are not an intended recipient, you may not read, copy, or 
distribute this message or any attachment. If you received this communication 
in error, please notify us immediately by e-mail and then delete all copies of 
this message and any attachments.

In addition you should be aware that ordinary (unencrypted) e-mail sent through 
the Internet is not secure. Do not send confidential or sensitive information, 
such as social security numbers, account numbers, personal identification 
numbers and passwords, to us via ordinary (unencrypted) e-mail.


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

Reply via email to