New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Keycloak-10162] Usage of ObjectInputStream without checking the object types #7053
Conversation
|
With newer JDKs we would also need to swap calls to |
|
@douglaspalmer how about this: package demo;
import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
import java.io.ObjectInputStream;
import java.io.ObjectOutputStream;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.util.logging.Level;
import java.util.logging.Logger;
public class SerialFilterDemo {
public static void main(String[] args) throws Exception {
Logger.getLogger(DelegatingSerializationFilter.class.getName()).setLevel(Level.FINEST);
ByteArrayOutputStream out = new ByteArrayOutputStream();
ObjectOutputStream oos = new ObjectOutputStream(out);
oos.writeObject("hello");
ObjectInputStream ois = new ObjectInputStream(new ByteArrayInputStream(out.toByteArray()));
JavaSerializationFilter serializationFilter = new DelegatingSerializationFilter();
serializationFilter.setFilter(ois, "org.keycloak.KeycloakSecurityContext;!*");
// Java 8
// System.out.println(sun.misc.ObjectInputFilter.Config.getObjectInputFilter(ois));
// Java 9+
// System.out.println(ois.getObjectInputFilter());
}
interface JavaSerializationFilter {
void setFilter(ObjectInputStream ois, String filterPattern);
}
static class DelegatingSerializationFilter implements JavaSerializationFilter {
private static final Logger LOG = Logger.getLogger(DelegatingSerializationFilter.class.getName());
private static final SerializationFilterAdapter serializationFilterAdapter = isJava8() ? new OnJava8() : new OnJavaAfter8();
private static boolean isJava8() {
return "1.8".equals(System.getProperty("java.specification.version"));
}
@Override
public void setFilter(ObjectInputStream ois, String filterPattern) {
LOG.info("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);
}
// If codebase stays on Java 8 for a while you could use Java 8 classes directly without reflection
static class OnJava8 implements SerializationFilterAdapter {
private static final Method getObjectInputFilterMethod;
private static final Method setObjectInputFilterMethod;
private static final Method createFilterMethod;
static {
Method getObjectInputFilter;
Method setObjectInputFilter;
Method createFilter;
try {
ClassLoader cl = Thread.currentThread().getContextClassLoader();
Class<?> objectInputFilterClass = cl.loadClass("sun.misc.ObjectInputFilter");
Class<?> objectInputFilterConfigClass = cl.loadClass("sun.misc.ObjectInputFilter$Config");
getObjectInputFilter = objectInputFilterConfigClass.getDeclaredMethod("getObjectInputFilter", ObjectInputStream.class);
setObjectInputFilter = objectInputFilterConfigClass.getDeclaredMethod("setObjectInputFilter", ObjectInputStream.class, objectInputFilterClass);
createFilter = objectInputFilterConfigClass.getDeclaredMethod("createFilter", String.class);
} catch (ClassNotFoundException | NoSuchMethodException e) {
LOG.warning("Could not configure SerializationFilterAdapter: " + e.getMessage());
getObjectInputFilter = null;
setObjectInputFilter = null;
createFilter = null;
}
getObjectInputFilterMethod = getObjectInputFilter;
setObjectInputFilterMethod = setObjectInputFilter;
createFilterMethod = createFilter;
}
public Object getObjectInputFilter(ObjectInputStream ois) {
try {
return getObjectInputFilterMethod.invoke(null, ois);
} catch (IllegalAccessException | InvocationTargetException e) {
LOG.warning("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.warning("Could not set ObjectFilter: " + e.getMessage());
}
}
}
// 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 static final Method getObjectInputFilterMethod;
private static final Method setObjectInputFilterMethod;
private static final Method createFilterMethod;
static {
Method getObjectInputFilter;
Method setObjectInputFilter;
Method createFilter;
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");
getObjectInputFilter = objectInputStreamClass.getDeclaredMethod("getObjectInputFilter");
setObjectInputFilter = objectInputStreamClass.getDeclaredMethod("setObjectInputFilter", objectInputFilterClass);
createFilter = objectInputFilterConfigClass.getDeclaredMethod("createFilter", String.class);
} catch (ClassNotFoundException | NoSuchMethodException e) {
LOG.warning("Could not configure SerializationFilterAdapter: " + e.getMessage());
getObjectInputFilter = null;
setObjectInputFilter = null;
createFilter = null;
}
getObjectInputFilterMethod = getObjectInputFilter;
setObjectInputFilterMethod = setObjectInputFilter;
createFilterMethod = createFilter;
}
public Object getObjectInputFilter(ObjectInputStream ois) {
try {
return getObjectInputFilterMethod.invoke(ois);
} catch (IllegalAccessException | InvocationTargetException e) {
LOG.warning("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.warning("Could not set ObjectFilter: " + e.getMessage());
}
}
}
}
} |
2e34cd4
to
bcbf3da
Compare
|
Thanks @thomasdarimont, I've incorporated your code into my PR. |
|
@douglaspalmer Thanks for the PR. I am fine with the codebase itself. However as I pointed, it will be good to run the pipeline to make sure there are not regressions. I have some concerns especially about:
|
|
@douglaspalmer did you manage to talk with @tkyjovsk? As we approach the next release, I believe we need to wrap this up next week. |
|
In case this is not fixable for ancient JDKs, one could print a big warning on startup, with a hint for upgrading the used Java Version, if an old JVM is detected. |
|
@douglaspalmer Hi, I've finished testing the PR. I didn't find any problem with the server but there is an issue with EAP6 client adapter and JDK 7 and 8 (independent on OS and JDK vendor). See details in the JIRA. The POMs in this PR are still on |
bcbf3da
to
1cbfce6
Compare
1cbfce6
to
5821e37
Compare
|
FYI. I've sent another PR from this branch #7138 , which adds some more test fixes ( OIDCAdapterClusterTest with various JDKs) |
|
@douglaspalmer I believe this needs a rebase. |
|
@douglaspalmer it needs a rebase. |
|
@abstractj Closing as this was super-seded by other PR #7138 . See the email for more details |
|
@douglaspalmer |
|
@zywj which JDK / version are you using? |
|
@thomasdarimont I have tried OracleJDK 8/11 and OpenJDK 8-hotspot , OpenJDK 8-openj9. |
|
@zywj can you check with a debugger which exception is thrown in |
|
@thomasdarimont |
|
@zywj There is some work in progress on the issue related to this. We may have more updates regarding this next week... |
|
@mposolda thanks! |
|
@zywj it seems that the line with the Could you try running your app with a JDK greater than 11? |
|
Close but no cigar... the check was present in Java 11, then gone in 12,13,14 and was reintroduced in 15. |
|
There is the System property A potential workaround for the problem could be checking the internal state of the import org.jboss.logging.Logger;
import java.io.ObjectInputStream;
import java.lang.reflect.Field;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.lang.reflect.Modifier;
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 boolean JDK_SERIAL_SET_FILTER_AFTER_READ_ALLOWED = Boolean.getBoolean("jdk.serialSetFilterAfterRead");
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.debugf("Using: %s", 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");
if (JDK_SERIAL_SET_FILTER_AFTER_READ_ALLOWED) {
return new OnJava6To8(getObjectInputFilter, setObjectInputFilter, createFilter, null);
} else {
// Before setting the ObjectInputFilter, we need to check the internal state of the ObjectInputStream to avoid exceptions if the stream was already used before.
Field totalObjectRefsField = null;
try {
// this field is checked in the setInternalObjectFilterMethod
// lookup totalObjectRefsField to guard ObjectInputFilter configuration
totalObjectRefsField = ObjectInputStream.class.getDeclaredField("totalObjectRefs");
makeAccessible(totalObjectRefsField);
} catch (NoSuchFieldException e) {
LOG.debug("Could not find totalObjectRefs field in ObjectInputStream", e);
}
return new OnJava6To8(getObjectInputFilter, setObjectInputFilter, createFilter, totalObjectRefsField);
}
} 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");
if (JDK_SERIAL_SET_FILTER_AFTER_READ_ALLOWED) {
return new OnJavaAfter8(getObjectInputFilter, setObjectInputFilter, createFilter, null);
} else {
// Before setting the ObjectInputFilter, we need to check the internal state of the ObjectInputStream to avoid exceptions if the stream was already used before.
Field totalObjectRefsField = null;
try {
// this field is checked in the setObjectFilterMethod in JDK 9-11 and 15-16+? but not in 12-14
// lookup totalObjectRefsField to guard ObjectInputFilter configuration
totalObjectRefsField = ObjectInputStream.class.getDeclaredField("totalObjectRefs");
makeAccessible(totalObjectRefsField);
} catch (NoSuchFieldException e) {
LOG.debug("Could not find totalObjectRefs field in ObjectInputStream", e);
}
return new OnJavaAfter8(getObjectInputFilter, setObjectInputFilter, createFilter, totalObjectRefsField);
}
} 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();
}
}
private static void makeAccessible(Field field) {
if ((!Modifier.isPublic(field.getModifiers()) ||
!Modifier.isPublic(field.getDeclaringClass().getModifiers()) ||
Modifier.isFinal(field.getModifiers())) && !field.isAccessible()) {
field.setAccessible(true);
}
}
static class EmptyFilterAdapter implements SerializationFilterAdapter {
@Override
public Object getObjectInputFilter(ObjectInputStream ois) {
return null;
}
@Override
public void setObjectInputFilter(ObjectInputStream ois, String filterPattern) {
// NOOP
}
}
// 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 final Field totalObjectRefsField;
private OnJava6To8(Method getObjectInputFilterMethod, Method setObjectInputFilterMethod, Method createFilterMethod, Field totalObjectRefsField) {
this.getObjectInputFilterMethod = getObjectInputFilterMethod;
this.setObjectInputFilterMethod = setObjectInputFilterMethod;
this.createFilterMethod = createFilterMethod;
this.totalObjectRefsField = totalObjectRefsField;
}
public Object getObjectInputFilter(ObjectInputStream ois) {
try {
return getObjectInputFilterMethod.invoke(null, ois);
} catch (IllegalAccessException | InvocationTargetException e) {
LOG.warnf("Could not read ObjectFilter from ObjectInputStream: %s", e.getMessage());
return null;
}
}
public void setObjectInputFilter(ObjectInputStream ois, String filterPattern) {
try {
// Avoid causing an exception when setting the filter to an ObjectInputStream which has already been read from.
// workaround for bug "filter cannot be set after an object has been read"
// https://github.com/keycloak/keycloak/pull/7053#issuecomment-701193325
if (totalObjectRefsField != null) {
Long totalObjectRefs = (Long) totalObjectRefsField.get(ois);
if (totalObjectRefs != null && totalObjectRefs > 0L) {
LOG.debugf("Skip configuring ObjectInputFilter for ObjectInputStream to avoid Exception after object has been read");
return;
}
}
Object objectFilter = createFilterMethod.invoke(null, filterPattern);
setObjectInputFilterMethod.invoke(null, ois, objectFilter);
} catch (IllegalAccessException | InvocationTargetException e) {
LOG.warnf("Could not set ObjectFilter: %s", e.getMessage());
}
}
}
// 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 final Field totalObjectRefsField;
private OnJavaAfter8(Method getObjectInputFilterMethod, Method setObjectInputFilterMethod, Method createFilterMethod, Field totalObjectRefsField) {
this.getObjectInputFilterMethod = getObjectInputFilterMethod;
this.setObjectInputFilterMethod = setObjectInputFilterMethod;
this.createFilterMethod = createFilterMethod;
this.totalObjectRefsField = totalObjectRefsField;
}
public Object getObjectInputFilter(ObjectInputStream ois) {
try {
return getObjectInputFilterMethod.invoke(ois);
} catch (IllegalAccessException | InvocationTargetException e) {
LOG.warnf("Could not read ObjectFilter from ObjectInputStream: %s", e.getMessage());
return null;
}
}
public void setObjectInputFilter(ObjectInputStream ois, String filterPattern) {
try {
// Avoid causing an exception when setting the filter to an ObjectInputStream which has already been read from.
// workaround for bug "filter cannot be set after an object has been read"
// This happens in Java 9-11 and from 15-16
// https://github.com/keycloak/keycloak/pull/7053#issuecomment-701193325
if (totalObjectRefsField != null) {
Long totalObjectRefs = (Long) totalObjectRefsField.get(ois);
if (totalObjectRefs != null && totalObjectRefs > 0L) {
LOG.debugf("Skip configuring ObjectInputFilter for ObjectInputStream to avoid Exception after object has been read");
return;
}
}
Object objectFilter = createFilterMethod.invoke(ois, filterPattern);
setObjectInputFilterMethod.invoke(ois, objectFilter);
} catch (IllegalAccessException | InvocationTargetException e) {
LOG.warnf("Could not set ObjectFilter: %s", 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);
}
}
}This can be tested with the following program which works for JDK 8 - JDK 16: import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.io.ObjectInputStream;
import java.io.ObjectOutputStream;
import java.io.Serializable;
public class KeycloakDelegatingSerializationBug {
public static void main(String[] args) throws Exception{
// System.setSecurityManager(new SecurityManager());
transport();
transport();
}
private static void transport() throws IOException, ClassNotFoundException {
ByteArrayOutputStream baos = new ByteArrayOutputStream();
ObjectOutputStream oos = new ObjectOutputStream(baos);
oos.writeObject(new Data());
oos.writeObject(new Data());
ObjectInputStream ois = new ObjectInputStream(new ByteArrayInputStream(baos.toByteArray()));
Object result = ois.readObject();
System.out.println(result);
DelegatingSerializationFilter.builder().addAllowedPattern("*").setFilter(ois);
result = ois.readObject();
System.out.println(result);
}
public static class Data implements Serializable {
String s = "foo";
int i = 42;
public Data() {
}
@Override
public String toString() {
return "Data{" +
"s='" + s + '\'' +
", i=" + i +
'}';
}
}
} |
|
@zywj could you try to run your app with: |
|
@thomasdarimont thanks. i'll try it tomorrow |
|
@thomasdarimont |
|
This exception indicates that you are trying to deserialize types, which are not allowed by the class / package patterns configured in the Could you find out what is missing from the allowed classes, package patterns and tell us? If you find the missing class you could try to shadow the class I think we need a way to configure the DelegatingSerializationFilter via a system property |
|
@zywj @thomasdarimont It seems the later issue is already reported here: https://issues.redhat.com/browse/KEYCLOAK-14871. @zywj Could you please check this: #7471 solved the |
|
@thomasdarimont Could you please open a new PR with your solution for the |
|
@mhajas I could create a new PR, but I think my solution is more of a hack than a real solution... I think the following "fix" would be better:
For the record here are the changes I did to the |
|
@mhajas Sorry, I don't know how to test your code. Do I need to compile keycloak source code?
@thomasdarimont Yes. It can be reproduced in 15. So I think the keycloak might has a bug. |
@thomasdarimont I think we could do both. Add the note to the documentation. And add your workaround to the DelegatingSerilizationFilter, however, I would change the debug log to warning for the
Sorry, it is my fault. Can you try with this keycloak-core jar file? I uploaded updated jar file here: https://issues.redhat.com/browse/KEYCLOAK-14871. |
|
@mhajas I think documenting this and mentioning the option with the System property is the only sensible thing we can do. Peeking into the internals of I recommend to "fix" this with a section in the Keycloak documentation (and perhaps a reference in the release notes). |
|
@thomasdarimont If I understand correctly, currently the exception is ignored as well right? There is just this message |
|
Adding |
|
It happens after introducing spring-session. Using Keycloak 18.0.2 |


I have an issue with this PR which is why it is currently a draft PR.
This code is JDK 8 specific. JDK 9 introduced java.io.ObjectInputFilter. The filter was then backported to JDK 8 as sun.misc.ObjectInputFilter. How can we maintain JDK compatibility when the package name is different between versions?