Affects: \5.3 and above

In #25379 there has been a discussion about Java serialization and security vulnerabilities. Some statements provided by the contributor Juergen Hoeller (jhoeller) have led to the decision to deprecate all binary protocols including the HTTPInvoker, which we currently use.

It seems that the main author Michael Bazos (mbazos) has had strong doubts about the idea of deprecation initially. Finally Juergen has convinced him according to the thread there.

After significant research I came to the conclusion that Juergens’s opinion is indeed shared by many others on Stackoverflow.

But at the same time it turned out that most of these writings lacked proper architectural argumentation. The writings we found contained no listings of the possible trade-offs addressing different real-world security contexts or other solid argumentation. So these did not really help us to make a guess on the arguments Juergen has had in mind.

As soon as Juergen writes down his arguments in detail I will be happy to get into a discussion with him.

Personally I stick to the very open mindset Microsoft shows these days.

Though in the past they have been “the evil ones” they nowadays are open to Open Source, Linux and in the meantime even to Java.

They explicitly allow for binary serialization of the objects exchanged via REST APIs for performance reasons:

“Some services MAY also have special performance needs that require a different format, such as a binary protocol.”

Please see the Microsoft REST API Guidelines: https://github.com/microsoft/api-guidelines/blob/vNext/Guidelines.md

Additionally there is a pretty inexpensive fix of the vulnerabilities possible.

So my intention is to convince you to take back the deprecation and instead add some deserialization blacklisting code to Spring framework.

I can of course contribute the code needed to the Spring source code, remove the deprecations and issue a pull request, if you want me to.

In the first step I would like to get Michael’s opinion if possible. If Michael as the main contributor on this subject does not agree on my ideas, the time I spend would be completely useless.

Rough concept for a fix

Basic idea: Jackson allows for deserializing arbitrary Java classes as well, thus has the same general problem as described in https://security.snyk.io/vuln/SNYK-JAVA-COMFASTERXMLJACKSONCORE-72448 and provides a working solution accepted by the Spring community and by Sonatype.

The Jackson developers have solved this issue by implementing a blacklisting logic for classes for deserialization.

I have written a piece of example serialization/deserialization code which shows how the reuse of all current and future Jackson serialization vulnerability fixes is possible.

I can provide or contribute it, but do not yet know what is the best way. I have added it below as an own text chapter under the heading “Example code”.

The target context for our architecture

Our deployment target is a heavily secured production environment in the medical area. Man-in-the-middle attacks are not possible at all by running all services in a de facto "demilitarized zone" with absolutely no APIs or HTML pages being accessible from the World Wide Web.

If there would be a security breach allowing somebody to take control of parts of the inner network then the possibility of an MIM attack by Java serialization means is definitely one of the smallest of the problems we would have then.

I know another mid-size company following the same model. So this seems to be not as rare as one might think.

For our main customer performance has always been a big issue since years.

Security has not been any issue since years cause it is reliably ensured by other means.

Arguments regarding Juergen’s claims

The “old school” statement

Juergen writes: "Such a style of remoting is old school and not recommendable for many reasons these days."

Due to the shortness of this sentence there are no concrete arguments contained. So I can only answer with similar generality:

Many “old school” things still exist in real life software development for good reasons. Often they are still the current "state of the art". Often they are simply the best economical choice by the architect for a certain team of developers.

What I want to say: This “old school” thing is no argument on its own. As said initially I am happy to write answers to Juergen’s arguments as soon as I read them.

The Oracle deprecating Java serialization claim

Juergen writes: "Also, the Oracle JDK team intends to deprecate serialization in some form; we can easily anticipate that here."

We could not find a valid confirmation on that on the web.

For sure there are individuals working for Oracle having this opinion. But there is no official decision in this sense according to our research.

I am a C# developer as well and I think Oracle cannot afford to deprecate Java serialization in the future due to the ongoing competitition of programming languages. There are existing scenarios not only for APIs but also for other things like persisting cached objects, deep cloning large objects and so on where it outperforms the existing JSON alternatives by far while being very easy to handle.

Besides Java serialization seems currently to be in real use even in Spring framework itself.

Each class in the Jackson blacklist like "org.springframework.aop.config.MethodLocatingFactoryBean" must have a reason to implement "java.io.Serializable". The only reason I can imagine is their usage by Java serialization in real scenarios. Otherwise the "implements Serializable" statement in the class code would be a bug, should be deleted and the class can be completely removed from the blacklist.

While writing this, the argumentation sounds too easy to me. If it is not right, please feel free to correct me.

Example code with re-use of Jacksons SubTypeValidator

package org.springframework.binaryprotocols;

import com.fasterxml.jackson.databind.DeserializationContext;
import com.fasterxml.jackson.databind.JavaType;
import com.fasterxml.jackson.databind.cfg.DeserializerFactoryConfig;
import com.fasterxml.jackson.databind.deser.BeanDeserializerFactory;
import com.fasterxml.jackson.databind.deser.DefaultDeserializationContext;
import com.fasterxml.jackson.databind.exc.InvalidDefinitionException;
import com.fasterxml.jackson.databind.introspect.BasicBeanDescription;
import com.fasterxml.jackson.databind.jsontype.impl.SubTypeValidator;
import com.fasterxml.jackson.databind.type.TypeFactory;

import java.io.*;

public class JacksonBasedDeserializationFilter implements ObjectInputFilter
{
    // this main method will fail intentionally to show that the Jackson blacklisting is
    // successfully invoked while using the Java serialization   
    public static void main(String[] args) throws IOException, ClassNotFoundException
    {
        Serializable objectToSerialize = new TestClass();
        ByteArrayOutputStream byteArrayOutputStream = new ByteArrayOutputStream();
        ObjectOutputStream objectOutputStream = new ObjectOutputStream(byteArrayOutputStream);
        objectOutputStream.writeObject(objectToSerialize);
        objectOutputStream.close();
        byte[] serializedObject = byteArrayOutputStream.toByteArray();
        ByteArrayInputStream byteArrayInputStream = new ByteArrayInputStream(serializedObject);
        ObjectInputStream objectInputStream = new ObjectInputStream(byteArrayInputStream);
        objectInputStream.setObjectInputFilter(new JacksonBasedDeserializationFilter());
        Serializable deserializedObject = (Serializable) objectInputStream.readObject();
    }

    @Override
    public Status checkInput(FilterInfo filterInfo)
    {
        String canonicalName = filterInfo.serialClass().getCanonicalName();
        try
        {
            DeserializationContext deserializationContext = new DefaultDeserializationContext.Impl(new BeanDeserializerFactory(new DeserializerFactoryConfig()));
            JavaType javaType = TypeFactory.defaultInstance().constructFromCanonical(canonicalName);
            SubTypeValidator.instance().validateSubType(deserializationContext, javaType, BasicBeanDescription.forOtherUse(null, javaType, null));
        }
        catch (InvalidDefinitionException e)
        {
            return Status.REJECTED;
        }
        catch (Exception e)
        {
            return Status.UNDECIDED;
        }

        // To allow additional filter configuration by the system property/security property "jdk.serialFilter" the value Status.UNDECIDED is returned instead of Status.ALLOWED
        return Status.UNDECIDED;
    }

    public static class TestClass implements Serializable
    {
        public TestClass()
        {
        }

        private String name = "Any instance name";
        // javax.swing.JTextPane is part of the deserialization blacklist in Jackson for whatever reason.
        // Thus, the following line leads to an exception when running the main method above.
        // If you uncomment this, there will be no exception anymore.
        private javax.swing.JTextPane jTextPane = new javax.swing.JTextPane();
    }
}

Comment From: bclozel

Thanks @erikliebelt for your feedback. The short answer is: our decision still stands and it's unfortunately too late.

It seems that the main author Michael Bazos (mbazos) has had strong doubts about the idea of deprecation initially. Finally Juergen has convinced him according to the thread there.

I don't think Michael really cared about HTTPInvoker itself, but rather about a way to get rid of CVE alarm bells going off because of #24434. This incident was just an additional data point for the Framework team to decide to deprecate this.

Personally I stick to the very open mindset Microsoft shows these days. They explicitly allow for binary serialization of the objects exchanged via REST APIs for performance reasons:

You seem to conflate Java deserialization and binary protocols in general. Spring Framework officially supports binary deserialization protocols over REST, such as protobuf and binary JSON "Smile".

Juergen writes: "Also, the Oracle JDK team intends to deprecate serialization in some form; we can easily anticipate that here." We could not find a valid confirmation on that on the web. For sure there are individuals working for Oracle having this opinion. But there is no official decision in this sense according to our research.

Even if no JEP is officially out Mark Reinhold gave a pretty detailed explanation on that in 2018; he might be an individual working for Oracle, but as Chief artchitect of the Java platform, we're taking this statement quite seriously.

I think that the proposal you're describing in your comment is already pretty well covered by the Java platform itself. Over the years, several features were added to further lock this feature down. This is all explained in details on the official secure coding guidelines for Java.

Finally, this feature has been deprecated officially in 2020 with the 5.3.0 release, and removed altogether in 6.0.0-RC1. Walking this back now would go against the Java team's opinion, the Java community's sentiment, the Spring team's opinion and would also go against years of consistent management of our API.

At this point, I would suggest considering options for your future transition to Spring Framework 6.0, looking at our support timeline. There are many well supported, high performance binary protocols supported by the Spring community. You can still choose to keep using Java deserialization, but this first deprecation+removal is in our opinion a warning sign for Java developers that the community is moving away from such approaches.

Comment From: erikliebelt

lock

Thank you for answering so fast and for adding these interesting details for further reading. So we will take a look on the two alternatives you have mentioned. Besides performance the more intuitive automatic passing through of (business) exceptions was a big argument for Spring Boot with HTTPInvoker compared to regular Spring Boot with REST and JSON in an evaluation we did. This is an advantages only for pure Java to Java communications, I know, but this is exactly this special case that we have here.