KEYCLOAK-10162 Usage of ObjectInputStream without checking the object types

Co-authored-by: mposolda <mposolda@gmail.com>

adapters/oidc/servlet-filter/src/main/java/org/keycloak/adapters/servlet/OIDCFilterSessionStore.java

@@ -17,6 +17,7 @@
 
 package org.keycloak.adapters.servlet;
 
+import org.keycloak.KeycloakPrincipal;
 import org.keycloak.KeycloakSecurityContext;
 import org.keycloak.adapters.AdapterTokenStore;
 import org.keycloak.adapters.KeycloakDeployment;
@@ -26,10 +27,14 @@ import org.keycloak.adapters.RequestAuthenticator;
 import org.keycloak.adapters.spi.HttpFacade;
 import org.keycloak.adapters.spi.KeycloakAccount;
 import org.keycloak.adapters.spi.SessionIdMapper;
+import org.keycloak.common.util.DelegatingSerializationFilter;
 
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletRequestWrapper;
 import javax.servlet.http.HttpSession;
+
+import java.io.IOException;
+import java.io.ObjectInputStream;
 import java.io.Serializable;
 import java.security.Principal;
 import java.util.Set;
@@ -157,6 +162,17 @@ public class OIDCFilterSessionStore extends FilterSessionStore implements Adapte
         public RefreshableKeycloakSecurityContext getKeycloakSecurityContext() {
             return securityContext;
         }
+
+        private void readObject(ObjectInputStream in) throws IOException, ClassNotFoundException {
+            DelegatingSerializationFilter.builder()
+                    .addAllowedClass(OIDCFilterSessionStore.SerializableKeycloakAccount.class)
+                    .addAllowedClass(RefreshableKeycloakSecurityContext.class)
+                    .addAllowedClass(KeycloakSecurityContext.class)
+                    .addAllowedClass(KeycloakPrincipal.class)
+                    .setFilter(in);
+
+            in.defaultReadObject();
+        }
     }
 
     @Override

adapters/oidc/tomcat/tomcat-core/src/main/java/org/keycloak/adapters/tomcat/CatalinaSessionTokenStore.java

@@ -20,13 +20,17 @@ package org.keycloak.adapters.tomcat;
 import org.apache.catalina.Session;
 import org.apache.catalina.connector.Request;
 import org.apache.catalina.realm.GenericPrincipal;
+import org.keycloak.KeycloakPrincipal;
 import org.keycloak.KeycloakSecurityContext;
 import org.keycloak.adapters.AdapterTokenStore;
 import org.keycloak.adapters.KeycloakDeployment;
 import org.keycloak.adapters.OidcKeycloakAccount;
 import org.keycloak.adapters.RefreshableKeycloakSecurityContext;
 import org.keycloak.adapters.RequestAuthenticator;
+import org.keycloak.common.util.DelegatingSerializationFilter;
 
+import java.io.IOException;
+import java.io.ObjectInputStream;
 import java.io.Serializable;
 import java.security.Principal;
 import java.util.Set;
@@ -163,6 +167,17 @@ public class CatalinaSessionTokenStore extends CatalinaAdapterSessionStore imple
         public RefreshableKeycloakSecurityContext getKeycloakSecurityContext() {
             return securityContext;
         }
+
+        private void readObject(ObjectInputStream in) throws IOException, ClassNotFoundException {
+            DelegatingSerializationFilter.builder()
+                    .addAllowedClass(CatalinaSessionTokenStore.SerializableKeycloakAccount.class)
+                    .addAllowedClass(RefreshableKeycloakSecurityContext.class)
+                    .addAllowedClass(KeycloakSecurityContext.class)
+                    .addAllowedClass(KeycloakPrincipal.class)
+                    .setFilter(in);
+
+            in.defaultReadObject();
+        }
     }
 
     @Override

common/src/main/java/org/keycloak/common/util/DelegatingSerializationFilter.java

@@ -0,0 +1,230 @@
+/*
+ * Copyright 2020 Red Hat, Inc. and/or its affiliates
+ * and other contributors as indicated by the @author tags.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+package org.keycloak.common.util;
+
+import org.jboss.logging.Logger;
+
+import java.io.ObjectInputStream;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+
+public class DelegatingSerializationFilter {
+    private static final Logger LOG = Logger.getLogger(DelegatingSerializationFilter.class.getName());
+
+    private static final SerializationFilterAdapter serializationFilterAdapter = isJava6To8() ? createOnJava6To8Adapter() : createOnJavaAfter8Adapter();
+
+    private static boolean isJava6To8() {
+        List<String> olderVersions = Arrays.asList("1.6", "1.7", "1.8");
+        return olderVersions.contains(System.getProperty("java.specification.version"));
+    }
+
+    private DelegatingSerializationFilter() {
+    }
+
+    public static DelegatingSerializationFilter.FilterPatternBuilder builder() {
+        return new DelegatingSerializationFilter.FilterPatternBuilder();
+    }
+
+    private void setFilter(ObjectInputStream ois, String filterPattern) {
+        LOG.debug("Using: " + serializationFilterAdapter.getClass().getSimpleName());
+
+        if (serializationFilterAdapter.getObjectInputFilter(ois) == null) {
+            serializationFilterAdapter.setObjectInputFilter(ois, filterPattern);
+        }
+    }
+
+    interface SerializationFilterAdapter {
+
+        Object getObjectInputFilter(ObjectInputStream ois);
+
+        void setObjectInputFilter(ObjectInputStream ois, String filterPattern);
+    }
+
+    private static SerializationFilterAdapter createOnJava6To8Adapter() {
+        try {
+            ClassLoader cl = Thread.currentThread().getContextClassLoader();
+            Class<?> objectInputFilterClass = cl.loadClass("sun.misc.ObjectInputFilter");
+            Class<?> objectInputFilterConfigClass = cl.loadClass("sun.misc.ObjectInputFilter$Config");
+            Method getObjectInputFilter = objectInputFilterConfigClass.getDeclaredMethod("getObjectInputFilter", ObjectInputStream.class);
+            Method setObjectInputFilter = objectInputFilterConfigClass.getDeclaredMethod("setObjectInputFilter", ObjectInputStream.class, objectInputFilterClass);
+            Method createFilter = objectInputFilterConfigClass.getDeclaredMethod("createFilter", String.class);
+            LOG.info("Using OnJava6To8 serialization filter adapter");
+            return new OnJava6To8(getObjectInputFilter, setObjectInputFilter, createFilter);
+        } catch (ClassNotFoundException | NoSuchMethodException e) {
+            // This can happen for older JDK updates.
+            LOG.warn("Could not configure SerializationFilterAdapter. For better security, it is highly recommended to upgrade to newer JDK version update!");
+            LOG.warn("For the Java 7, the recommended update is at least 131 (1.7.0_131 or newer). For the Java 8, the recommended update is at least 121 (1.8.0_121 or newer).");
+            LOG.warn("Error details", e);
+            return new EmptyFilterAdapter();
+        }
+    }
+
+    private static SerializationFilterAdapter createOnJavaAfter8Adapter() {
+        try {
+            ClassLoader cl = Thread.currentThread().getContextClassLoader();
+            Class<?> objectInputFilterClass = cl.loadClass("java.io.ObjectInputFilter");
+            Class<?> objectInputFilterConfigClass = cl.loadClass("java.io.ObjectInputFilter$Config");
+            Class<?> objectInputStreamClass = cl.loadClass("java.io.ObjectInputStream");
+            Method getObjectInputFilter = objectInputStreamClass.getDeclaredMethod("getObjectInputFilter");
+            Method setObjectInputFilter = objectInputStreamClass.getDeclaredMethod("setObjectInputFilter", objectInputFilterClass);
+            Method createFilter = objectInputFilterConfigClass.getDeclaredMethod("createFilter", String.class);
+            LOG.info("Using OnJavaAfter8 serialization filter adapter");
+            return new OnJavaAfter8(getObjectInputFilter, setObjectInputFilter, createFilter);
+        } catch (ClassNotFoundException | NoSuchMethodException e) {
+            // This can happen for older JDK updates.
+            LOG.warn("Could not configure SerializationFilterAdapter. For better security, it is highly recommended to upgrade to newer JDK version update!");
+            LOG.warn("Error details", e);
+            return new EmptyFilterAdapter();
+        }
+    }
+
+    // If codebase stays on Java 8 for a while you could use Java 8 classes directly without reflection
+    static class OnJava6To8 implements SerializationFilterAdapter {
+
+        private final Method getObjectInputFilterMethod;
+        private final Method setObjectInputFilterMethod;
+        private final Method createFilterMethod;
+
+        private OnJava6To8(Method getObjectInputFilterMethod, Method setObjectInputFilterMethod, Method createFilterMethod) {
+            this.getObjectInputFilterMethod = getObjectInputFilterMethod;
+            this.setObjectInputFilterMethod = setObjectInputFilterMethod;
+            this.createFilterMethod = createFilterMethod;
+        }
+
+        public Object getObjectInputFilter(ObjectInputStream ois) {
+            try {
+                return getObjectInputFilterMethod.invoke(null, ois);
+            } catch (IllegalAccessException | InvocationTargetException e) {
+                LOG.warn("Could not read ObjectFilter from ObjectInputStream: " + e.getMessage());
+                return null;
+            }
+        }
+
+        public void setObjectInputFilter(ObjectInputStream ois, String filterPattern) {
+            try {
+                Object objectFilter = createFilterMethod.invoke(null, filterPattern);
+                setObjectInputFilterMethod.invoke(null, ois, objectFilter);
+            } catch (IllegalAccessException | InvocationTargetException e) {
+                LOG.warn("Could not set ObjectFilter: " + e.getMessage());
+            }
+        }
+    }
+
+
+    static class EmptyFilterAdapter implements SerializationFilterAdapter {
+
+        @Override
+        public Object getObjectInputFilter(ObjectInputStream ois) {
+            return null;
+        }
+
+        @Override
+        public void setObjectInputFilter(ObjectInputStream ois, String filterPattern) {
+
+        }
+
+    }
+
+
+    // If codebase moves to Java 9+ could use Java 9+ classes directly without reflection and keep the old variant with reflection
+    static class OnJavaAfter8 implements SerializationFilterAdapter {
+
+        private final Method getObjectInputFilterMethod;
+        private final Method setObjectInputFilterMethod;
+        private final Method createFilterMethod;
+
+        private OnJavaAfter8(Method getObjectInputFilterMethod, Method setObjectInputFilterMethod, Method createFilterMethod) {
+            this.getObjectInputFilterMethod = getObjectInputFilterMethod;
+            this.setObjectInputFilterMethod = setObjectInputFilterMethod;
+            this.createFilterMethod = createFilterMethod;
+        }
+
+        public Object getObjectInputFilter(ObjectInputStream ois) {
+            try {
+                return getObjectInputFilterMethod.invoke(ois);
+            } catch (IllegalAccessException | InvocationTargetException e) {
+                LOG.warn("Could not read ObjectFilter from ObjectInputStream: " + e.getMessage());
+                return null;
+            }
+        }
+
+        public void setObjectInputFilter(ObjectInputStream ois, String filterPattern) {
+            try {
+                Object objectFilter = createFilterMethod.invoke(ois, filterPattern);
+                setObjectInputFilterMethod.invoke(ois, objectFilter);
+            } catch (IllegalAccessException | InvocationTargetException e) {
+                LOG.warn("Could not set ObjectFilter: " + e.getMessage());
+            }
+        }
+    }
+
+
+    public static class FilterPatternBuilder {
+
+        private Set<Class> classes = new HashSet<>();
+        private Set<String> patterns = new HashSet<>();
+
+        public FilterPatternBuilder() {
+            // Add "java.util" package by default (contains all the basic collections)
+            addAllowedPattern("java.util.*");
+        }
+
+        /**
+         * This is used when the caller of this method can't use the {@link #addAllowedClass(Class)}. For example because the
+         * particular is private or it is not available at the compile time. Or when adding the whole package like "java.util.*"
+         *
+         * @param pattern
+         * @return
+         */
+        public FilterPatternBuilder addAllowedPattern(String pattern) {
+            this.patterns.add(pattern);
+            return this;
+        }
+
+        public FilterPatternBuilder addAllowedClass(Class javaClass) {
+            this.classes.add(javaClass);
+            return this;
+        }
+
+        @Override
+        public String toString() {
+            StringBuilder builder = new StringBuilder();
+
+            for (Class javaClass : classes) {
+                builder.append(javaClass.getName()).append(";");
+            }
+            for (String pattern : patterns) {
+                builder.append(pattern).append(";");
+            }
+
+            builder.append("!*");
+
+            return builder.toString();
+        }
+
+        public void setFilter(ObjectInputStream ois) {
+            DelegatingSerializationFilter filter = new DelegatingSerializationFilter();
+            String filterPattern = this.toString();
+            filter.setFilter(ois, filterPattern);
+        }
+    }
+}

common/src/main/java/org/keycloak/common/util/KerberosSerializationUtils.java

@@ -19,15 +19,16 @@ package org.keycloak.common.util;
 
 import org.ietf.jgss.GSSCredential;
 
+import javax.security.auth.kerberos.KerberosPrincipal;
 import javax.security.auth.kerberos.KerberosTicket;
 import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
-import java.io.ObjectInput;
 import java.io.ObjectInputStream;
 import java.io.ObjectOutput;
 import java.io.ObjectOutputStream;
 import java.io.Serializable;
+import java.net.InetAddress;
 
 /**
  * Provides serialization/deserialization of kerberos {@link org.ietf.jgss.GSSCredential}, so it can be transmitted from auth-server to the application
@@ -109,9 +110,15 @@ public class KerberosSerializationUtils {
     private static Object deserialize(String serialized) throws ClassNotFoundException, IOException {
         byte[] bytes = Base64.decode(serialized);
         ByteArrayInputStream bis = new ByteArrayInputStream(bytes);
-        ObjectInput in = null;
+        ObjectInputStream in = null;
         try {
             in = new ObjectInputStream(bis);
+            DelegatingSerializationFilter.builder()
+                    .addAllowedClass(KerberosTicket.class)
+                    .addAllowedClass(KerberosPrincipal.class)
+                    .addAllowedClass(InetAddress.class)
+                    .addAllowedPattern("javax.security.auth.kerberos.KeyImpl")
+                    .setFilter(in);
             return in.readObject();
         } finally {
             try {

core/pom.xml

@@ -65,6 +65,11 @@
             <groupId>com.fasterxml.jackson.core</groupId>
             <artifactId>jackson-databind</artifactId>
         </dependency>
+        <dependency>
+            <groupId>org.jboss.logging</groupId>
+            <artifactId>jboss-logging</artifactId>
+            <scope>provided</scope>
+        </dependency>
         <dependency>
             <groupId>junit</groupId>
             <artifactId>junit</artifactId>

core/src/main/java/org/keycloak/KeycloakPrincipal.java

@@ -17,6 +17,10 @@
 
 package org.keycloak;
 
+import org.keycloak.common.util.DelegatingSerializationFilter;
+
+import java.io.IOException;
+import java.io.ObjectInputStream;
 import java.io.Serializable;
 import java.security.Principal;
 
@@ -63,4 +67,13 @@ public class KeycloakPrincipal<T extends KeycloakSecurityContext> implements Pri
     public String toString() {
         return name;
     }
+
+    private void readObject(ObjectInputStream in) throws IOException, ClassNotFoundException {
+        DelegatingSerializationFilter.builder()
+                .addAllowedClass(KeycloakPrincipal.class)
+                .addAllowedClass(KeycloakSecurityContext.class)
+                .setFilter(in);
+
+        in.defaultReadObject();
+    }
 }

core/src/main/java/org/keycloak/KeycloakSecurityContext.java

@@ -18,6 +18,7 @@
 package org.keycloak;
 
 import org.keycloak.common.util.Base64Url;
+import org.keycloak.common.util.DelegatingSerializationFilter;
 import org.keycloak.representations.AccessToken;
 import org.keycloak.representations.IDToken;
 import org.keycloak.util.JsonSerialization;
@@ -85,6 +86,9 @@ public class KeycloakSecurityContext implements Serializable {
     }
 
     private void readObject(ObjectInputStream in) throws IOException, ClassNotFoundException {
+        DelegatingSerializationFilter.builder()
+                .addAllowedClass(KeycloakSecurityContext.class)
+                .setFilter(in);
         in.defaultReadObject();
 
         token = parseToken(tokenString, AccessToken.class);

testsuite/integration-arquillian/HOW-TO-RUN.md

@@ -66,6 +66,9 @@ Analogically, there is the same behaviour for JBoss based app server as for auth
 
     -Dapp.server.debug.port=$PORT
     -Dapp.server.debug.suspend=y
+    
+When you are debugging cluster adapter tests (For example OIDCAdapterClusterTest) you may use ports 7901 and 7902 for the app
+server nodes. Tests are usually using 2 cluster adapter nodes.    
 
 ## Testsuite logging
 

testsuite/integration-arquillian/test-apps/servlets/src/main/java/org/keycloak/testsuite/adapter/servlet/SessionServlet.java

@@ -25,6 +25,7 @@ import javax.servlet.http.HttpServletResponse;
 import javax.servlet.http.HttpSession;
 import java.io.IOException;
 import java.io.PrintWriter;
+import java.io.Serializable;
 
 /**
  * @author <a href="mailto:mposolda@redhat.com">Marek Posolda</a>
@@ -40,17 +41,21 @@ public class SessionServlet extends HttpServlet {
         }
 
         String counter;
+        String counterWrapperValue;
         if (req.getRequestURI().endsWith("/donotincrease")) {
             counter = getCounter(req);
+            counterWrapperValue = getCounterWrapper(req);
         } else {
             counter = increaseAndGetCounter(req);
+            counterWrapperValue = increaseAndGetCounterWrapper(req);
         }
 
         resp.setContentType("text/html");
         PrintWriter pw = resp.getWriter();
         pw.printf("<html><head><title>%s</title></head><body>", "Session Test");
-        pw.printf("Counter=%s", counter);
-        pw.printf("Node name=%s", System.getProperty("jboss.node.name", "property not specified"));
+        pw.printf("Counter=%s<br>", counter);
+        pw.printf("CounterWrapper=%s<br>", counterWrapperValue);
+        pw.printf("Node name=%s<br>", System.getProperty("jboss.node.name", "property not specified"));
         pw.print("</body></html>");
         pw.flush();
 
@@ -69,4 +74,34 @@ public class SessionServlet extends HttpServlet {
         session.setAttribute("counter", counter);
         return String.valueOf(counter);
     }
+
+    private String getCounterWrapper(HttpServletRequest req) {
+        HttpSession session = req.getSession();
+        return String.valueOf(session.getAttribute("counterWrapper"));
+    }
+
+    private String increaseAndGetCounterWrapper(HttpServletRequest req) {
+        HttpSession session = req.getSession();
+        CounterWrapper counter = (CounterWrapper)session.getAttribute("counterWrapper");
+        counter = (counter == null) ? new CounterWrapper() : counter.increase();
+        session.setAttribute("counterWrapper", counter);
+        return String.valueOf(counter);
+    }
+
+
+    // This is just to test that custom class can be added as an attribute to the HttpSession
+    public static class CounterWrapper implements Serializable {
+
+        private int counter = 1;
+
+        @Override
+        public String toString() {
+            return String.valueOf(counter);
+        }
+
+        public CounterWrapper increase() {
+            counter = counter + 1;
+            return this;
+        }
+    }
 }

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/cluster/OIDCAdapterClusterTest.java

@@ -43,6 +43,7 @@ import org.keycloak.testsuite.adapter.AbstractAdapterClusteredTest;
 import org.keycloak.testsuite.adapter.page.SessionPortalDistributable;
 import org.keycloak.testsuite.adapter.servlet.SessionServlet;
 import org.keycloak.testsuite.arquillian.annotation.AppServerContainer;
+import org.keycloak.testsuite.util.WaitUtils;
 import org.keycloak.testsuite.utils.arquillian.ContainerConstants;
 import org.keycloak.testsuite.auth.page.AuthRealm;
 import org.keycloak.testsuite.auth.page.login.OIDCLogin;
@@ -150,7 +151,7 @@ public class OIDCAdapterClusterTest extends AbstractAdapterClusteredTest {
                     driver.navigate().to(appUrl + "/donotincrease");
                     waitForPageToLoad();
 
-                    return driver.getPageSource().contains("Counter=" + expectedCount);
+                    return driver.getPageSource().contains("Counter=" + expectedCount) && driver.getPageSource().contains("CounterWrapper=" + expectedCount);
                 });
     }
 
@@ -163,7 +164,9 @@ public class OIDCAdapterClusterTest extends AbstractAdapterClusteredTest {
         driver.navigate().to(appUrl);
         waitForPageToLoad();
 
-        assertThat(driver.getPageSource(), containsString("Counter=" + expectedCount));
-        assertThat(driver.getPageSource(), containsString("Node name=" + hostToPointToName));
+        String pageSource = driver.getPageSource();
+        assertThat(pageSource, containsString("Counter=" + expectedCount));
+        assertThat(pageSource, containsString("CounterWrapper=" + expectedCount));
+        assertThat(pageSource, containsString("Node name=" + hostToPointToName));
     }
 }

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/cluster/OIDCFilterAdapterClusterTest.java

@@ -0,0 +1,35 @@
+/*
+ * Copyright 2020 Red Hat, Inc. and/or its affiliates
+ * and other contributors as indicated by the @author tags.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+package org.keycloak.testsuite.adapter.servlet.cluster;
+
+import org.keycloak.testsuite.arquillian.annotation.AppServerContainer;
+import org.keycloak.testsuite.utils.annotation.UseServletFilter;
+import org.keycloak.testsuite.utils.arquillian.ContainerConstants;
+
+/**
+ * @author <a href="mailto:mposolda@redhat.com">Marek Posolda</a>
+ */
+@AppServerContainer(ContainerConstants.APP_SERVER_WILDFLY_CLUSTER)
+@AppServerContainer(ContainerConstants.APP_SERVER_WILDFLY_DEPRECATED_CLUSTER)
+@AppServerContainer(ContainerConstants.APP_SERVER_EAP_CLUSTER)
+@AppServerContainer(ContainerConstants.APP_SERVER_EAP6_CLUSTER)
+@UseServletFilter(filterName = "oidc-filter", filterClass = "org.keycloak.adapters.servlet.KeycloakOIDCFilter",
+        filterDependency = "org.keycloak:keycloak-servlet-filter-adapter", skipPattern = "/error.html")
+public class OIDCFilterAdapterClusterTest extends OIDCAdapterClusterTest {
+}