[CVE-2020-1732] Adjust JASPIC integration to create a new ServerAuthM…

…odule for each request.

impl/src/main/java/org/glassfish/soteria/mechanisms/jaspic/DefaultAuthConfigProvider.java

@@ -17,6 +17,7 @@
 package org.glassfish.soteria.mechanisms.jaspic;
 
 import java.util.Map;
+import java.util.function.Function;
 
 import javax.security.auth.callback.CallbackHandler;
 import javax.security.auth.message.AuthException;
@@ -30,23 +31,23 @@
 /**
  * This class functions as a kind of factory-factory for {@link ServerAuthConfig} instances, which are by themselves factories
  * for {@link ServerAuthContext} instances, which are delegates for the actual {@link ServerAuthModule} (SAM) that we're after.
- * 
+ *
  * @author Arjan Tijms
  */
 public class DefaultAuthConfigProvider implements AuthConfigProvider {
 
     private static final String CALLBACK_HANDLER_PROPERTY_NAME = "authconfigprovider.client.callbackhandler";
 
     private Map<String, String> providerProperties;
-    private ServerAuthModule serverAuthModule;
+    private Function<CallbackHandler, ServerAuthModule> serverAuthModuleFactory;
 
-    public DefaultAuthConfigProvider(ServerAuthModule serverAuthModule) {
-        this.serverAuthModule = serverAuthModule;
+    public DefaultAuthConfigProvider(Function<CallbackHandler, ServerAuthModule> serverAuthModuleFactory) {
+        this.serverAuthModuleFactory = serverAuthModuleFactory;
     }
 
     /**
      * Constructor with signature and implementation that's required by API.
-     * 
+     *
      * @param properties provider properties
      * @param factory the auth config factory
      */
@@ -71,8 +72,9 @@ public DefaultAuthConfigProvider(Map<String, String> properties, AuthConfigFacto
     @Override
     public ServerAuthConfig getServerAuthConfig(String layer, String appContext, CallbackHandler handler) throws AuthException,
         SecurityException {
+
         return new DefaultServerAuthConfig(layer, appContext, handler == null ? createDefaultCallbackHandler() : handler,
-            providerProperties, serverAuthModule);
+            providerProperties, serverAuthModuleFactory.apply(handler));
     }
 
     @Override
@@ -89,7 +91,7 @@ public void refresh() {
      * Creates a default callback handler via the system property "authconfigprovider.client.callbackhandler", as seemingly
      * required by the API (API uses wording "may" create default handler). TODO: Isn't
      * "authconfigprovider.client.callbackhandler" JBoss specific?
-     * 
+     *
      * @return
      * @throws AuthException
      */

impl/src/main/java/org/glassfish/soteria/mechanisms/jaspic/DefaultServerAuthConfig.java

@@ -29,7 +29,7 @@
 /**
  * This class functions as a kind of factory for {@link ServerAuthContext} instances, which are delegates for the actual
  * {@link ServerAuthModule} (SAM) that we're after.
- * 
+ *
  * @author Arjan Tijms
  */
 public class DefaultServerAuthConfig implements ServerAuthConfig {

impl/src/main/java/org/glassfish/soteria/mechanisms/jaspic/DefaultServerAuthContext.java

@@ -34,7 +34,7 @@
  * <p>
  * Since this simple example only has a single SAM, we delegate directly to that one. Note that this {@link ServerAuthContext}
  * and the {@link ServerAuthModule} (SAM) share a common base interface: {@link ServerAuth}.
- * 
+ *
  * @author Arjan Tijms
  */
 public class DefaultServerAuthContext implements ServerAuthContext {

impl/src/main/java/org/glassfish/soteria/mechanisms/jaspic/HttpBridgeServerAuthModule.java

@@ -49,17 +49,17 @@
  */
 public class HttpBridgeServerAuthModule implements ServerAuthModule {
 
-        private CallbackHandler handler;
+        private final CallbackHandler handler;
         private final Class<?>[] supportedMessageTypes = new Class[] { HttpServletRequest.class, HttpServletResponse.class };
         private final CDIPerRequestInitializer cdiPerRequestInitializer;
 
-        public HttpBridgeServerAuthModule(CDIPerRequestInitializer cdiPerRequestInitializer) {
+        public HttpBridgeServerAuthModule(CDIPerRequestInitializer cdiPerRequestInitializer, CallbackHandler handler) {
             this.cdiPerRequestInitializer = cdiPerRequestInitializer;
+            this.handler = handler;
         }
 
         @Override
         public void initialize(MessagePolicy requestPolicy, MessagePolicy responsePolicy, CallbackHandler handler, @SuppressWarnings("rawtypes") Map options) throws AuthException {
-            this.handler = handler;
             // options not supported.
         }
 

impl/src/main/java/org/glassfish/soteria/mechanisms/jaspic/Jaspic.java

@@ -23,7 +23,6 @@
 import java.security.AccessController;
 import java.security.Principal;
 import java.security.PrivilegedAction;
-import java.util.List;
 import java.util.Set;
 
 import javax.security.enterprise.AuthenticationStatus;
@@ -43,42 +42,44 @@
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 
+import org.glassfish.soteria.cdi.spi.CDIPerRequestInitializer;
+
 /**
  * A set of utility methods for using the JASPIC API
- * 
+ *
  * @author Arjan Tijms
  *
  */
 public final class Jaspic {
-	
+
 	public static final String IS_AUTHENTICATION = "org.glassfish.soteria.security.message.request.authentication";
 	public static final String IS_AUTHENTICATION_FROM_FILTER = "org.glassfish.soteria.security.message.request.authenticationFromFilter";
 	public static final String IS_SECURE_RESPONSE = "org.glassfish.soteria.security.message.request.secureResponse";
 	public static final String IS_REFRESH = "org.glassfish.soteria.security.message.request.isRefresh";
 	public static final String DID_AUTHENTICATION = "org.glassfish.soteria.security.message.request.didAuthentication";
-	
+
 	public static final String AUTH_PARAMS = "org.glassfish.soteria.security.message.request.authParams";
-	
+
 	public static final String LOGGEDIN_USERNAME = "org.glassfish.soteria.security.message.loggedin.username";
 	public static final String LOGGEDIN_ROLES = "org.glassfish.soteria.security.message.loggedin.roles";
 	public static final String LAST_AUTH_STATUS = "org.glassfish.soteria.security.message.authStatus";
-	
+
 	public static final String CONTEXT_REGISTRATION_ID = "org.glassfish.soteria.security.message.registrationId";
-	
+
 	// Key in the MessageInfo Map that when present AND set to true indicated a protected resource is being accessed.
 	// When the resource is not protected, GlassFish omits the key altogether. WebSphere does insert the key and sets
 	// it to false.
 	private static final String IS_MANDATORY = "javax.security.auth.message.MessagePolicy.isMandatory";
 	private static final String REGISTER_SESSION = "javax.servlet.http.registerSession";
 
 	private Jaspic() {}
-	
+
 	public static boolean authenticate(HttpServletRequest request, HttpServletResponse response, AuthenticationParameters authParameters) {
 		try {
 		    // JASPIC 1.1 does not have any way to distinguish between a
 		    // SAM called at start of a request or following request#authenticate.
 		    // See https://java.net/jira/browse/JASPIC_SPEC-5
-		    
+
 		    // We now add this as a request attribute instead, but should better
 		    // be the MessageInfo map
 			request.setAttribute(IS_AUTHENTICATION, true);
@@ -101,10 +102,10 @@ public static AuthenticationParameters getAuthParameters(HttpServletRequest requ
 		if (authParameters == null) {
 			authParameters = new AuthenticationParameters();
 		}
-		
+
 		return authParameters;
 	}
-	
+
 	public static void logout(HttpServletRequest request, HttpServletResponse response) {
 		try {
 			request.logout();
@@ -131,21 +132,21 @@ public Void run() {
 	public static boolean isRegisterSession(MessageInfo messageInfo) {
 		return Boolean.valueOf((String)messageInfo.getMap().get(REGISTER_SESSION));
 	}
-	
+
 	public static boolean isProtectedResource(MessageInfo messageInfo) {
 		return Boolean.valueOf((String) messageInfo.getMap().get(IS_MANDATORY));
 	}
-	
+
 	@SuppressWarnings("unchecked")
 	public static void setRegisterSession(MessageInfo messageInfo, String username, Set<String> roles) {
 		messageInfo.getMap().put("javax.servlet.http.registerSession", TRUE.toString());
-		
+
 		HttpServletRequest request = (HttpServletRequest) messageInfo.getRequestMessage();
 		request.setAttribute(LOGGEDIN_USERNAME, username);
 		// TODO: check for existing roles and add
 		request.setAttribute(LOGGEDIN_ROLES, roles);
 	}
-	
+
 	public static boolean isAuthenticationRequest(HttpServletRequest request) {
 		return TRUE.equals(request.getAttribute(IS_AUTHENTICATION));
 	}
@@ -202,70 +203,70 @@ public static AuthStatus fromAuthenticationStatus(AuthenticationStatus authentic
 	            throw new IllegalStateException("Unhandled status:" + authenticationStatus.name());
 	    }
 	}
-	
+
 	/**
 	 * Should be called when the callback handler is used with the intention that an actual
 	 * user is going to be authenticated (as opposed to using the handler for the "do nothing" protocol
 	 * which uses the unauthenticated identity).
-	 * 
+	 *
 	 * @param request The involved HTTP servlet request.
-	 * 
+	 *
 	 */
 	public static void setDidAuthentication(HttpServletRequest request) {
 		request.setAttribute(DID_AUTHENTICATION, TRUE);
 	}
-	
+
 	/**
 	 * Gets the app context ID from the servlet context.
-	 * 
+	 *
 	 * <p>
 	 * The app context ID is the ID that JASPIC associates with the given application.
 	 * In this case that given application is the web application corresponding to the
 	 * ServletContext.
-	 * 
+	 *
 	 * @param context the servlet context for which to obtain the JASPIC app context ID
 	 * @return the app context ID for the web application corresponding to the given context
 	 */
 	public static String getAppContextID(ServletContext context) {
 		return context.getVirtualServerName() + " " + context.getContextPath();
 	}
-	
+
 	/**
 	 * Registers a server auth module as the one and only module for the application corresponding to
 	 * the given servlet context.
-	 * 
+	 *
 	 * <p>
 	 * This will override any other modules that have already been registered, either via proprietary
 	 * means or using the standard API.
-	 * 
-	 * @param serverAuthModule the server auth module to be registered
+	 *
+	 * @param cdiPerRequestInitializer A {@link CDIPerRequestInitializer} to pass to the {@link ServerAuthModule}
 	 * @param servletContext the context of the app for which the module is registered
 	 * @return A String identifier assigned by an underlying factory corresponding to an underlying factory-factory-factory registration
 	 */
-	public static String registerServerAuthModule(ServerAuthModule serverAuthModule, ServletContext servletContext) {
-		
+	public static String registerServerAuthModule(CDIPerRequestInitializer cdiPerRequestInitializer, ServletContext servletContext) {
+
 	    // Register the factory-factory-factory for the SAM
 	    String registrationId = AccessController.doPrivileged(new PrivilegedAction<String>() {
 	        public String run() {
 	            return AuthConfigFactory.getFactory().registerConfigProvider(
-	                    new DefaultAuthConfigProvider(serverAuthModule),
-	                    "HttpServlet", 
-	                    getAppContextID(servletContext), 
+	                    new DefaultAuthConfigProvider((CallbackHandler handler) -> new HttpBridgeServerAuthModule(cdiPerRequestInitializer, handler)),
+	                    "HttpServlet",
+	                    getAppContextID(servletContext),
 	                    "Default single SAM authentication config provider");
 	        }
 	    });
-		
+
 		// Remember the registration ID returned by the factory, so we can unregister the JASPIC module when the web module
 		// is undeployed. JASPIC being the low level API that it is won't do this automatically.
 		servletContext.setAttribute(CONTEXT_REGISTRATION_ID, registrationId);
-		
+
 		return registrationId;
 	}
-	
+
 	/**
 	 * Deregisters the server auth module (and encompassing wrappers/factories) that was previously registered via a call
 	 * to registerServerAuthModule.
-	 * 
+	 *
 	 * @param servletContext the context of the app for which the module is deregistered
 	 */
 	public static void deregisterServerAuthModule(ServletContext servletContext) {
@@ -278,6 +279,6 @@ public Boolean run() {
 			});
 		}
 	}
-	
-	
+
+
 }

impl/src/main/java/org/glassfish/soteria/servlet/SamRegistrationInstaller.java

@@ -39,80 +39,80 @@
 import org.glassfish.soteria.mechanisms.jaspic.Jaspic;
 
 /**
- * If an HttpAuthenticationMechanism implementation has been found on the classpath, this 
+ * If an HttpAuthenticationMechanism implementation has been found on the classpath, this
  * initializer installs a bridge SAM that delegates the validateRequest, secureResponse and
  * cleanSubject methods from the SAM to the HttpAuthenticationMechanism.
- * 
+ *
  * <p>
  * The bridge SAM uses <code>CDI.current()</code> to obtain the HttpAuthenticationMechanism, therefore
  * fully enabling CDI in the implementation of that interface.
- * 
+ *
  * @author Arjan Tijms
  *
  */
 public class SamRegistrationInstaller implements ServletContainerInitializer, ServletContextListener {
-    
+
     private static final Logger logger =  Logger.getLogger(SamRegistrationInstaller.class.getName());
 
     @Override
     public void onStartup(Set<Class<?>> c, ServletContext ctx) throws ServletException {
 
         // Obtain a reference to the CdiExtension that was used to see if
         // there's an enabled bean
-        
+
         CDI<Object> cdi;
         try {
             cdi = CDI.current();
-            
+
             if (logger.isLoggable(INFO)) {
                 String version = getClass().getPackage().getImplementationVersion();
                 logger.log(INFO, "Initializing Soteria {0} for context ''{1}''", new Object[]{version, ctx.getContextPath()});
             }
-            
+
         } catch (IllegalStateException e) {
-            // On GlassFish 4.1.1/Payara 4.1.1.161 CDI is not initialized (org.jboss.weld.Container#initialize is not called), 
+            // On GlassFish 4.1.1/Payara 4.1.1.161 CDI is not initialized (org.jboss.weld.Container#initialize is not called),
             // and calling CDI.current() will throw an exception. It's no use to continue then.
             // TODO: Do we need to find out *why* the default module does not have CDI initialized?
             logger.log(FINEST, "CDI not available for app context id: " + Jaspic.getAppContextID(ctx), e);
-            
+
             return;
         }
-        
+
         CdiExtension cdiExtension = cdi.select(CdiExtension.class).get();
 
         if (cdiExtension.isHttpAuthenticationMechanismFound()) {
 
             // A SAM must be registered at this point, since the programmatically added
             // Listener is for some reason restricted (not allow) from calling
             // getVirtualServerName. At this point we're still allowed to call this.
-            
+
             // TODO: Ask the Servlet EG to address this? Is there any ground for this restriction???
-            
+
             CDIPerRequestInitializer cdiPerRequestInitializer = null;
-            
+
             if (!isEmpty(System.getProperty("wlp.server.name"))) {
                 // Hardcode server check for now. TODO: design/implement proper service loader/SPI for this
                 cdiPerRequestInitializer = new LibertyCDIPerRequestInitializer();
                 logger.log(INFO, "Running on Liberty - installing CDI request scope activator");
             }
-            
-            registerServerAuthModule(new HttpBridgeServerAuthModule(cdiPerRequestInitializer), ctx);
-          
+
+            registerServerAuthModule(cdiPerRequestInitializer, ctx);
+
             // Add a listener so we can process the context destroyed event, which is needed
             // to de-register the SAM correctly.
             ctx.addListener(this);
         }
 
     }
-    
+
     @Override
     public void contextInitialized(ServletContextEvent sce) {
        // noop
     }
-    
+
     @Override
     public void contextDestroyed(ServletContextEvent sce) {
         deregisterServerAuthModule(sce.getServletContext());
     }
-    
+
 }