Add optional, but on by default, check to ensure that IdP session cookie comes from...
authorlajoie <lajoie@ab3bd59b-922f-494d-bb5f-6f0a3c29deca>
Fri, 19 Sep 2008 12:04:23 +0000 (12:04 +0000)
committerlajoie <lajoie@ab3bd59b-922f-494d-bb5f-6f0a3c29deca>
Fri, 19 Sep 2008 12:04:23 +0000 (12:04 +0000)
git-svn-id: https://subversion.switch.ch/svn/shibboleth/java-idp/branches/REL_2@2764 ab3bd59b-922f-494d-bb5f-6f0a3c29deca

doc/RELEASE-NOTES.txt
src/main/java/edu/internet2/middleware/shibboleth/idp/authn/AuthenticationEngine.java
src/main/java/edu/internet2/middleware/shibboleth/idp/session/IdPSessionFilter.java
src/main/java/edu/internet2/middleware/shibboleth/idp/session/Session.java
src/main/java/edu/internet2/middleware/shibboleth/idp/session/impl/SessionImpl.java
src/main/java/edu/internet2/middleware/shibboleth/idp/session/impl/SessionManagerImpl.java

index 103573c..29659d1 100644 (file)
@@ -25,4 +25,5 @@ Changes in Release 2.1.0
 [SIDP-214] - Installer needs to put (at least) bcprov onto the calsspath before it runs ant
 [SIDP-222] - Template engine used by LDAP and database connectors throw an NPE on startup
 [SIDP-224] - Add version information in library JAR manifest and provide command line tool to view it
 [SIDP-214] - Installer needs to put (at least) bcprov onto the calsspath before it runs ant
 [SIDP-222] - Template engine used by LDAP and database connectors throw an NPE on startup
 [SIDP-224] - Add version information in library JAR manifest and provide command line tool to view it
+[SIDP-225] - Credential theft vulnerability in login.jsp
 [SIDP-226] - Cross site scripting vulnerability
\ No newline at end of file
 [SIDP-226] - Cross site scripting vulnerability
\ No newline at end of file
index 43f9e84..94ea968 100644 (file)
@@ -17,6 +17,7 @@
 package edu.internet2.middleware.shibboleth.idp.authn;
 
 import java.io.IOException;
 package edu.internet2.middleware.shibboleth.idp.authn;
 
 import java.io.IOException;
+import java.security.GeneralSecurityException;
 import java.security.NoSuchAlgorithmException;
 import java.security.Principal;
 import java.util.ArrayList;
 import java.security.NoSuchAlgorithmException;
 import java.security.Principal;
 import java.util.ArrayList;
@@ -27,6 +28,8 @@ import java.util.Map;
 import java.util.Set;
 import java.util.Map.Entry;
 
 import java.util.Set;
 import java.util.Map.Entry;
 
+import javax.crypto.Mac;
+import javax.crypto.SecretKey;
 import javax.security.auth.Subject;
 import javax.servlet.RequestDispatcher;
 import javax.servlet.ServletConfig;
 import javax.security.auth.Subject;
 import javax.servlet.RequestDispatcher;
 import javax.servlet.ServletConfig;
@@ -42,6 +45,7 @@ import org.opensaml.common.impl.SecureRandomIdentifierGenerator;
 import org.opensaml.saml2.core.AuthnContext;
 import org.opensaml.util.storage.ExpiringObject;
 import org.opensaml.util.storage.StorageService;
 import org.opensaml.saml2.core.AuthnContext;
 import org.opensaml.util.storage.ExpiringObject;
 import org.opensaml.util.storage.StorageService;
+import org.opensaml.xml.util.Base64;
 import org.opensaml.xml.util.DatatypeHelper;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.opensaml.xml.util.DatatypeHelper;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -222,6 +226,12 @@ public class AuthenticationEngine extends HttpServlet {
             HttpServletResponse httpResponse) {
         LOG.debug("Returning control to profile handler at: {}", loginContext.getProfileHandlerURL());
         httpRequest.setAttribute(LoginContext.LOGIN_CONTEXT_KEY, loginContext);
             HttpServletResponse httpResponse) {
         LOG.debug("Returning control to profile handler at: {}", loginContext.getProfileHandlerURL());
         httpRequest.setAttribute(LoginContext.LOGIN_CONTEXT_KEY, loginContext);
+        
+        // Cleanup this cookie
+        Cookie lcKeyCookie = new Cookie(LOGIN_CONTEXT_KEY_NAME, "");
+        lcKeyCookie.setMaxAge(0);
+        httpResponse.addCookie(lcKeyCookie);
+        
         forwardRequest(loginContext.getProfileHandlerURL(), httpRequest, httpResponse);
     }
 
         forwardRequest(loginContext.getProfileHandlerURL(), httpRequest, httpResponse);
     }
 
@@ -703,8 +713,23 @@ public class AuthenticationEngine extends HttpServlet {
             Session userSession) {
         httpRequest.setAttribute(Session.HTTP_SESSION_BINDING_ATTRIBUTE, userSession);
 
             Session userSession) {
         httpRequest.setAttribute(Session.HTTP_SESSION_BINDING_ATTRIBUTE, userSession);
 
+        String remoteAddress = httpRequest.getRemoteAddr();
+        String sessionId = userSession.getSessionID();
+        
+        String signature = null;
+        SecretKey signingKey = userSession.getSessionSecretKey();
+        try {
+            Mac mac = Mac.getInstance("HmacSHA256");
+            mac.init(signingKey);
+            mac.update(remoteAddress.getBytes());
+            mac.update(sessionId.getBytes());
+            signature = Base64.encodeBytes(mac.doFinal());
+        } catch (GeneralSecurityException e) {
+            LOG.error("Unable to compute signature over session cookie material", e);
+        }
+
         LOG.debug("Adding IdP session cookie to HTTP response");
         LOG.debug("Adding IdP session cookie to HTTP response");
-        Cookie sessionCookie = new Cookie(IDP_SESSION_COOKIE_NAME, userSession.getSessionID());
+        Cookie sessionCookie = new Cookie(IDP_SESSION_COOKIE_NAME, remoteAddress + "|" + sessionId + "|" + signature);
 
         String contextPath = httpRequest.getContextPath();
         if (DatatypeHelper.isEmpty(contextPath)) {
 
         String contextPath = httpRequest.getContextPath();
         if (DatatypeHelper.isEmpty(contextPath)) {
index 1b6fd29..27cd93e 100644 (file)
 package edu.internet2.middleware.shibboleth.idp.session;
 
 import java.io.IOException;
 package edu.internet2.middleware.shibboleth.idp.session;
 
 import java.io.IOException;
+import java.security.GeneralSecurityException;
 
 
+import javax.crypto.Mac;
+import javax.crypto.SecretKey;
 import javax.servlet.Filter;
 import javax.servlet.FilterChain;
 import javax.servlet.FilterConfig;
 import javax.servlet.Filter;
 import javax.servlet.FilterChain;
 import javax.servlet.FilterConfig;
@@ -28,6 +31,7 @@ import javax.servlet.http.Cookie;
 import javax.servlet.http.HttpServletRequest;
 
 import org.joda.time.DateTime;
 import javax.servlet.http.HttpServletRequest;
 
 import org.joda.time.DateTime;
+import org.opensaml.xml.util.Base64;
 import org.opensaml.xml.util.DatatypeHelper;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.opensaml.xml.util.DatatypeHelper;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -44,6 +48,9 @@ public class IdPSessionFilter implements Filter {
     /** Class Logger. */
     private final Logger log = LoggerFactory.getLogger(IdPSessionFilter.class);
 
     /** Class Logger. */
     private final Logger log = LoggerFactory.getLogger(IdPSessionFilter.class);
 
+    /** Whether the client must always come back from the same address. */
+    private boolean consistentAddress;
+
     /** IdP session manager. */
     private SessionManager<Session> sessionManager;
 
     /** IdP session manager. */
     private SessionManager<Session> sessionManager;
 
@@ -57,16 +64,13 @@ public class IdPSessionFilter implements Filter {
             ServletException {
         HttpServletRequest httpRequest = (HttpServletRequest) request;
 
             ServletException {
         HttpServletRequest httpRequest = (HttpServletRequest) request;
 
-        Session idpSession = null;
-        Cookie idpSessionCookie = getIdPSessionCookie(httpRequest);
-        if (idpSessionCookie != null) {
-            idpSession = sessionManager.getSession(idpSessionCookie.getValue());
-            if (idpSession != null) {
-                log.trace("Updating IdP session activity time and adding session object to the request");
-                idpSession.setLastActivityInstant(new DateTime());
-                MDC.put("idpSessionId", idpSession.getSessionID());
-                httpRequest.setAttribute(Session.HTTP_SESSION_BINDING_ATTRIBUTE, idpSession);
-            }
+        Cookie sessionCookie = getIdPSessionCookie(httpRequest);
+        Session idpSession = validateCookie(sessionCookie, httpRequest);
+        if (idpSession != null) {
+            log.trace("Updating IdP session activity time and adding session object to the request");
+            idpSession.setLastActivityInstant(new DateTime());
+            MDC.put("idpSessionId", idpSession.getSessionID());
+            httpRequest.setAttribute(Session.HTTP_SESSION_BINDING_ATTRIBUTE, idpSession);
         }
 
         filterChain.doFilter(request, response);
         }
 
         filterChain.doFilter(request, response);
@@ -80,18 +84,25 @@ public class IdPSessionFilter implements Filter {
         }
 
         sessionManager = (SessionManager<Session>) filterConfig.getServletContext().getAttribute(sessionManagerId);
         }
 
         sessionManager = (SessionManager<Session>) filterConfig.getServletContext().getAttribute(sessionManagerId);
+
+        String consistentAddressParam = filterConfig.getInitParameter("ensureConsistentClientAddress");
+        if (DatatypeHelper.isEmpty(consistentAddressParam)) {
+            consistentAddress = true;
+        } else {
+            consistentAddress = Boolean.parseBoolean(consistentAddressParam);
+        }
     }
 
     /**
      * Gets the IdP session cookie from the current request, if the user currently has a session.
      * 
     }
 
     /**
      * Gets the IdP session cookie from the current request, if the user currently has a session.
      * 
-     * @param request current HTTP request
+     * @param httpRequest current HTTP request
      * 
      * @return the user's current IdP session cookie, if they have a current session, otherwise null
      */
      * 
      * @return the user's current IdP session cookie, if they have a current session, otherwise null
      */
-    protected Cookie getIdPSessionCookie(HttpServletRequest request) {
+    protected Cookie getIdPSessionCookie(HttpServletRequest httpRequest) {
         log.trace("Attempting to retrieve IdP session cookie.");
         log.trace("Attempting to retrieve IdP session cookie.");
-        Cookie[] requestCookies = request.getCookies();
+        Cookie[] requestCookies = httpRequest.getCookies();
 
         if (requestCookies != null) {
             for (Cookie requestCookie : requestCookies) {
 
         if (requestCookies != null) {
             for (Cookie requestCookie : requestCookies) {
@@ -102,7 +113,57 @@ public class IdPSessionFilter implements Filter {
             }
         }
 
             }
         }
 
-        log.trace("No IdP session cookie sent by the client.");
         return null;
     }
         return null;
     }
+
+    /**
+     * Validates the given session cookie against the associated session.
+     * 
+     * @param sessionCookie the session cookie
+     * @param httpRequest the current HTTP request
+     * 
+     * @return the session against which the cookie was validated
+     */
+    protected Session validateCookie(Cookie sessionCookie, HttpServletRequest httpRequest) {
+        if (sessionCookie == null) {
+            return null;
+        }
+
+        // index 0: remote address
+        // index 1: session ID
+        // index 2: Base64(HMAC(index 0 + index 1))
+        String[] valueComponents = sessionCookie.getValue().split("\\|");
+
+        if (consistentAddress) {
+            if (!httpRequest.getRemoteAddr().equals(valueComponents[0])) {
+                log.error("Client sent a cookie from addres {} but the cookie was issued to address {}", httpRequest
+                        .getRemoteAddr(), valueComponents[0]);
+                return null;
+            }
+        }
+
+        Session userSession = sessionManager.getSession(valueComponents[1]);
+
+        if (userSession != null) {
+            SecretKey signingKey = userSession.getSessionSecretKey();
+            try {
+                Mac mac = Mac.getInstance("HmacSHA256");
+                mac.init(signingKey);
+                mac.update(valueComponents[0].getBytes());
+                mac.update(valueComponents[1].getBytes());
+                byte[] signature = mac.doFinal();
+
+                if (!DatatypeHelper.safeEquals(valueComponents[2], Base64.encodeBytes(signature))) {
+                    log.error("Session cookie signature did not match, the session cookie has been tampered with");
+                    return null;
+                }
+            } catch (GeneralSecurityException e) {
+                log.error("Unable to computer over session cookie material", e);
+            }
+        } else {
+            log.debug("No session associated with session ID {} - session must have timed out",
+                            valueComponents[1]);
+        }
+        return userSession;
+    }
 }
\ No newline at end of file
 }
\ No newline at end of file
index 2eabc95..24e63b8 100644 (file)
@@ -18,6 +18,8 @@ package edu.internet2.middleware.shibboleth.idp.session;
 
 import java.util.Map;
 
 
 import java.util.Map;
 
+import javax.crypto.SecretKey;
+
 /**
  * Session information for user logged into the IdP.
  */
 /**
  * Session information for user logged into the IdP.
  */
@@ -25,6 +27,13 @@ public interface Session extends edu.internet2.middleware.shibboleth.common.sess
 
     /** Name of the HTTP request attribute to which a users IdP session is bound. */
     public static final String HTTP_SESSION_BINDING_ATTRIBUTE = "ShibbolethIdPSession";
 
     /** Name of the HTTP request attribute to which a users IdP session is bound. */
     public static final String HTTP_SESSION_BINDING_ATTRIBUTE = "ShibbolethIdPSession";
+    
+    /**
+     * A secret key associated with this session.
+     * 
+     * @return secret key associated with this session
+     */
+    public SecretKey getSessionSecretKey();
 
     /**
      * Gets the methods by which the user has authenticated to the IdP.
 
     /**
      * Gets the methods by which the user has authenticated to the IdP.
index a923ab0..5c6dae0 100644 (file)
@@ -19,6 +19,8 @@ package edu.internet2.middleware.shibboleth.idp.session.impl;
 import java.util.HashMap;
 import java.util.Map;
 
 import java.util.HashMap;
 import java.util.Map;
 
+import javax.crypto.SecretKey;
+
 import edu.internet2.middleware.shibboleth.common.session.impl.AbstractSession;
 import edu.internet2.middleware.shibboleth.idp.session.AuthenticationMethodInformation;
 import edu.internet2.middleware.shibboleth.idp.session.ServiceInformation;
 import edu.internet2.middleware.shibboleth.common.session.impl.AbstractSession;
 import edu.internet2.middleware.shibboleth.idp.session.AuthenticationMethodInformation;
 import edu.internet2.middleware.shibboleth.idp.session.ServiceInformation;
@@ -29,6 +31,9 @@ public class SessionImpl extends AbstractSession implements Session {
 
     /** Serial version UID. */
     private static final long serialVersionUID = 2927868242208211623L;
 
     /** Serial version UID. */
     private static final long serialVersionUID = 2927868242208211623L;
+    
+    /** Secret key associated with the session. */
+    private SecretKey sessionKey;
 
     /** The list of methods used to authenticate the user. */
     private HashMap<String, AuthenticationMethodInformation> authnMethods;
 
     /** The list of methods used to authenticate the user. */
     private HashMap<String, AuthenticationMethodInformation> authnMethods;
@@ -40,14 +45,22 @@ public class SessionImpl extends AbstractSession implements Session {
      * Constructor.
      * 
      * @param sessionId ID of the session
      * Constructor.
      * 
      * @param sessionId ID of the session
+     * @param key a secret key to associate with the session
      * @param timeout inactivity timeout for the session in milliseconds
      */
      * @param timeout inactivity timeout for the session in milliseconds
      */
-    public SessionImpl(String sessionId, long timeout) {
+    public SessionImpl(String sessionId, SecretKey key, long timeout) {
         super(sessionId, timeout);
 
         super(sessionId, timeout);
 
+        sessionKey = key;
+        
         authnMethods = new HashMap<String, AuthenticationMethodInformation>();
         servicesInformation = new HashMap<String, ServiceInformation>();
     }
         authnMethods = new HashMap<String, AuthenticationMethodInformation>();
         servicesInformation = new HashMap<String, ServiceInformation>();
     }
+    
+    /** {@inheritDoc} */
+    public SecretKey getSessionSecretKey() {
+        return sessionKey;
+    }
 
     /** {@inheritDoc} */
     public Map<String, AuthenticationMethodInformation> getAuthenticationMethods() {
 
     /** {@inheritDoc} */
     public Map<String, AuthenticationMethodInformation> getAuthenticationMethods() {
index 33ce367..fe6da01 100644 (file)
 
 package edu.internet2.middleware.shibboleth.idp.session.impl;
 
 
 package edu.internet2.middleware.shibboleth.idp.session.impl;
 
+import java.security.NoSuchAlgorithmException;
 import java.security.SecureRandom;
 import java.util.List;
 import java.util.Vector;
 
 import java.security.SecureRandom;
 import java.util.List;
 import java.util.Vector;
 
+import javax.crypto.KeyGenerator;
+
 import org.apache.commons.ssl.util.Hex;
 import org.joda.time.DateTime;
 import org.opensaml.util.storage.ExpiringObject;
 import org.apache.commons.ssl.util.Hex;
 import org.joda.time.DateTime;
 import org.opensaml.util.storage.ExpiringObject;
@@ -49,6 +52,9 @@ public class SessionManagerImpl implements SessionManager<Session>, ApplicationC
     /** Spring context used to publish login and logout events. */
     private ApplicationContext appCtx;
 
     /** Spring context used to publish login and logout events. */
     private ApplicationContext appCtx;
 
+    /** Generator used to create secret keys associated with the session. */
+    private KeyGenerator secretKeyGen;
+
     /** Number of random bits within a session ID. */
     private final int sessionIDSize = 32;
 
     /** Number of random bits within a session ID. */
     private final int sessionIDSize = 32;
 
@@ -74,6 +80,12 @@ public class SessionManagerImpl implements SessionManager<Session>, ApplicationC
         sessionStore = storageService;
         partition = "session";
         sessionLifetime = lifetime;
         sessionStore = storageService;
         partition = "session";
         sessionLifetime = lifetime;
+
+        try {
+            secretKeyGen = KeyGenerator.getInstance("AES");
+        } catch (NoSuchAlgorithmException e) {
+            log.error("AES key generation is not supported", e);
+        }
     }
 
     /**
     }
 
     /**
@@ -101,7 +113,7 @@ public class SessionManagerImpl implements SessionManager<Session>, ApplicationC
         prng.nextBytes(sid);
         String sessionID = Hex.encode(sid);
 
         prng.nextBytes(sid);
         String sessionID = Hex.encode(sid);
 
-        Session session = new SessionImpl(sessionID, sessionLifetime);
+        Session session = new SessionImpl(sessionID, secretKeyGen.generateKey(), sessionLifetime);
         SessionManagerEntry sessionEntry = new SessionManagerEntry(session, sessionLifetime);
         sessionStore.put(partition, sessionID, sessionEntry);
 
         SessionManagerEntry sessionEntry = new SessionManagerEntry(session, sessionLifetime);
         sessionStore.put(partition, sessionID, sessionEntry);
 
@@ -120,7 +132,7 @@ public class SessionManagerImpl implements SessionManager<Session>, ApplicationC
 
         MDC.put("idpSessionId", sessionID);
 
 
         MDC.put("idpSessionId", sessionID);
 
-        Session session = new SessionImpl(sessionID, sessionLifetime);
+        Session session = new SessionImpl(sessionID, secretKeyGen.generateKey(), sessionLifetime);
         SessionManagerEntry sessionEntry = new SessionManagerEntry(session, sessionLifetime);
         sessionStore.put(partition, sessionID, sessionEntry);
         log.trace("Created session {}", sessionID);
         SessionManagerEntry sessionEntry = new SessionManagerEntry(session, sessionLifetime);
         sessionStore.put(partition, sessionID, sessionEntry);
         log.trace("Created session {}", sessionID);
@@ -178,14 +190,14 @@ public class SessionManagerImpl implements SessionManager<Session>, ApplicationC
 
     /** {@inheritDoc} */
     public void onApplicationEvent(ApplicationEvent event) {
 
     /** {@inheritDoc} */
     public void onApplicationEvent(ApplicationEvent event) {
-        if(event instanceof AddEntryEvent){
-            AddEntryEvent addEvent = (AddEntryEvent)event;
-            if(addEvent.getValue() instanceof SessionManagerEntry){
+        if (event instanceof AddEntryEvent) {
+            AddEntryEvent addEvent = (AddEntryEvent) event;
+            if (addEvent.getValue() instanceof SessionManagerEntry) {
                 SessionManagerEntry sessionEntry = (SessionManagerEntry) addEvent.getValue();
                 appCtx.publishEvent(new LoginEvent(sessionEntry.getSession()));
             }
         }
                 SessionManagerEntry sessionEntry = (SessionManagerEntry) addEvent.getValue();
                 appCtx.publishEvent(new LoginEvent(sessionEntry.getSession()));
             }
         }
-        
+
         if (event instanceof RemoveEntryEvent) {
             RemoveEntryEvent removeEvent = (RemoveEntryEvent) event;
             if (removeEvent.getValue() instanceof SessionManagerEntry) {
         if (event instanceof RemoveEntryEvent) {
             RemoveEntryEvent removeEvent = (RemoveEntryEvent) event;
             if (removeEvent.getValue() instanceof SessionManagerEntry) {
@@ -207,7 +219,7 @@ public class SessionManagerImpl implements SessionManager<Session>, ApplicationC
     /** {@inheritDoc} */
     public void setApplicationContext(ApplicationContext applicationContext) {
         ApplicationContext rootContext = applicationContext;
     /** {@inheritDoc} */
     public void setApplicationContext(ApplicationContext applicationContext) {
         ApplicationContext rootContext = applicationContext;
-        while(rootContext.getParent() != null){
+        while (rootContext.getParent() != null) {
             rootContext = rootContext.getParent();
         }
         appCtx = rootContext;
             rootContext = rootContext.getParent();
         }
         appCtx = rootContext;