Skip to content
This repository was archived by the owner on May 31, 2022. It is now read-only.

RedisTokenStore will break when upgrading spring-security-core #662

Open
efenderbosch opened this issue Dec 17, 2015 · 28 comments
Open

RedisTokenStore will break when upgrading spring-security-core #662

efenderbosch opened this issue Dec 17, 2015 · 28 comments

Comments

@efenderbosch
Copy link

We just encountered this when upgrading to Spring Boot 1.3.0.
Spring Boot 1.2.4 uses security core 3.2.7 but Spring Boot 1.3.0 has upgraded to 4.0.3.

Caused by: java.io.InvalidClassException: org.springframework.security.core.authority.SimpleGrantedAuthority; local class incompatible: stream classdesc serialVersionUID = 320, local class serialVersionUID = 400

This happens because the default serialization strategy is JdkSerializationStrategy and SimpleGrantedAuthority gets its serialVersionUID from SpringSecurityCoreVersion.SERIAL_VERSION_UID. I'm sure other objects in the tree have also had their serialVersionUID changed from 320 to 400 as well.

I'm not sure what to do about this. I guess just a warning to users when they upgrade? We are likely going to have to flush all our token related Redis keys when we deploy our services based on Spring Boot 1.3.0.

We are also considering implementing a Kyro based serialization strategy to avoid this problem in the future.

@ricardoMogg
Copy link

did you find any solution for this?

@efenderbosch
Copy link
Author

I think a key prefix was added recently. The prefix could be used to keep v3 serialized objects and v4 serialized objects separate. I would effectively expire all tokens, though.

@ricardoMogg
Copy link

Cool! You really saved me some hours with this. Thank you!

@sustainablepace
Copy link

We have run into the same problem updating from Spring Boot 1.2.8 to 1.3.3.

Our "solution" is to implement a custom JSON serializer that is independent of changes in the SERIAL_VERSION_UID.

However, is anything done to prevent this problem with JdkSerialization in future updates of Spring Security? We can't afford to invalidate thousands of access tokens, but would love to continue working with an uncustomized Spring Boot stack.

Any feedback is welcome, thanks.

@jgrandja
Copy link
Contributor

@dsyer What are your thoughts on this issue? Should the default RedisTokenStoreSerializationStrategy implementation be a JSON serializer instead of a JDK serializer?

@efenderbosch
Copy link
Author

I tried writing a JSON serializer, but polymorphism was a problem. I got pretty far in to a Jackson Module to handle it, but it just wasn't quite right.

Something like Kryo would probably work pretty easily, however.

@dsyer dsyer added this to the 2.1.0 milestone May 13, 2016
@narup
Copy link

narup commented Sep 20, 2016

I got this exception when I upgraded my Spring boot 1.3.1 to 1.4 Is this related? I haven't been able to upgrade to 1.4 at the moment since I am not sure how to resolve it.

@dsyer
Copy link
Contributor

dsyer commented Sep 20, 2016

Probably. You don't really have a choice but to deal with it. Either delete all the tokens and let users generate new ones. Or converts them from the old to the new format in batch.

@narup
Copy link

narup commented Sep 20, 2016

Interesting. Thanks @dsyer

@jgrandja jgrandja modified the milestones: 2.1.1, 2.1.0 Mar 3, 2017
@maksimu
Copy link

maksimu commented May 25, 2017

Probably. You don't really have a choice but to deal with it. Either delete all the tokens and let users generate new ones. Or converts them from the old to the new format in batch.

What would be the best way to convert them in a batch? Do I have to have two version of the same lib. to be installed in order to convert from one to another?

@jgrandja jgrandja modified the milestones: 2.1.1, 2.1.2 May 29, 2017
@jgrandja jgrandja modified the milestones: 2.1.2, 2.2.0.M1 Jun 29, 2017
@jgrandja jgrandja modified the milestones: 2.2.0.RC1, 2.2.0 Jul 14, 2017
@jgrandja jgrandja removed this from the 2.2.1 milestone Sep 18, 2017
@jp-silva
Copy link

jp-silva commented Jan 9, 2019

  • when upgrading, is deleting all the old tokens still the current solution ?
  • is increasing the support for serialising into JSON on the plans for a future release?

@aschatten
Copy link
Contributor

aschatten commented Jan 10, 2019

@dsyer The only difference inSimpleGrantedAuthority between 4.2.10.RELEASE and 5.0.9.RELEASE is just this:

@@ -39,10 +39,12 @@
 		this.role = role;
 	}
 
+	@Override
 	public String getAuthority() {
 		return role;
 	}
 
+	@Override
 	public boolean equals(Object obj) {
 		if (this == obj) {
 			return true;
@@ -55,10 +57,12 @@
 		return false;
 	}
 
+	@Override
 	public int hashCode() {
 		return this.role.hashCode();
 	}
 
+	@Override
 	public String toString() {
 		return this.role;
 	}

According to Java Object Serialization Specification this change is compatible and the version should remain the same. Otherwise, in order to upgrade all the tokens must be flushed or perform a complicated migration between 2 versions, which is completely unnecessary in this case.

@aschatten
Copy link
Contributor

aschatten commented Jan 11, 2019

Following up on the discussion from https://gitter.im/spring-projects/spring-security?at=5c37972683c7e377654b3771:

There are multiple github tickets that discuss this issue. I am not sure which one I should move the discussion to. In OAuth project besides this one, there are couple more: #662, #665, #1547
Original change that caused the issue is this one: https://jira.spring.io/browse/SEC-1709 (spring-projects/spring-security#1945) – the root cause was that the same version of the class had different serialVersionUid in different JVMs, but after the fix set the version to the same constant across the entire library, it has been changing with every minor version. Here @rwinch stated that these objects were not supposed to be serialized between multiple versions. Then it means that even minor version upgrade is backwards incompatible, if this is the intention, then these objects should not be serialized and stored at all. More issues in Spring Security project here: spring-projects/spring-security#3918, spring-projects/spring-security#3737.

It looks like going forward the best course of action is to serialize using other formats. @fhanik mentioned JSON. I wonder if JSON or Kryo is considered to be a default format in the future?

@ufosaga
Copy link

ufosaga commented Jan 16, 2019

As I mentioned in #1547, save serialized java class object is a horrible way, it makes very hard to migrate. I have to deploy a migrating service using the old version. Hope we can switch to JSON or other proper format in future version.

@neale-bpm
Copy link

@jgrandja @dsyer What do you propose for zero downtime deployments on this?

@mkopylec
Copy link

mkopylec commented Oct 10, 2019

IMO the source cause of all the issues with serialization in spring-security is that spring-security forces developers to deal with object serialization. This is because spring-security sets those objects in the HTTP session as attributes. As objects I mean: OAuth2ClientContext, AuthenticationException, CsrfToken and so on. Those objects have no common structure that allows serialization tools to properly serialize and deserialize them, f.e. no-arg constructor. Another problem with those objects is that they are not backword compatible. Additionally, even if those objects don't change between versions, they are still not backword compatible because the SpringSecurityCoreVersion.SERIAL_VERSION_UID changes. This successfully prevents developers from upgrading spring-security's version, because it will likely break the already deployed application.

Instead of relaying on object session attributes, spring-security should rely on primitive/wrapper/string session attributes.

@OrangeDog

This comment has been minimized.

@juliusspencer
Copy link

Hi, I have the same issue trying to migrate from 2.0.9 to 2.1.6 of spring-boot:

java.lang.IllegalArgumentException: java.io.InvalidClassException: org.springframework.security.core.authority.SimpleGrantedAuthority; local class incompatible: stream classdesc serialVersionUID = 500, local class serialVersionUID = 510

Is there any article that details how to get handle this with a number of users who have Oauth access and refresh tokens?

@OrangeDog
Copy link
Contributor

@juliusspencer in theory, since the classes are actually identical, if you can modify the serialVersionUID in the binary data then it should work.

@neale-bpm
Copy link

neale-bpm commented Jan 6, 2020 via email

@OrangeDog
Copy link
Contributor

@neale-bpm which exactly is "that method"?

@neale-bpm
Copy link

For us, override JdbcTokenStore's deserialisation:

    @Override
    protected OAuth2Authentication deserializeAuthentication(byte[] authentication) {
            return deserializeIgnoringVersionUid(authentication);
    }

where deserializeIgnoringVersionUid uses an ObjectInputStream subclass that overrides the readClassDescriptor() method with an copy that ignores the UID.

@juliusspencer
Copy link

Thanks for the help @neale-bpm. Do I need to create my own subclass of JdbcTokenStore, set the serialVersionUID to the original value and supply that in the tokenStore Bean?

@juliusspencer
Copy link

Just to follow up currently I'm just using the following:

        @Bean
	public TokenStore tokenStore() {
		return new JdbcTokenStore(dataSource);
	}

@azdragon2
Copy link

azdragon2 commented Mar 11, 2020

@juliusspencer Yeah, I followed his recommendation and it worked for me on Spring Boot 2.2.4. Create your own subclass of JdbcTokenStore, override the deserializeAuthentication method. And use this:

public static <T> T deserializeIgnoringVersionUid(byte[] byteArray)
	{
		ObjectInputStream oip = null;
		try
		{
			oip = new YourOwnConfigurableObjectInputStream(new ByteArrayInputStream(byteArray), Thread.currentThread().getContextClassLoader());
			@SuppressWarnings("unchecked")
			T result = (T) oip.readObject();
			return result;
		}
		catch (IOException e)
		{
			throw new IllegalArgumentException(e);
		}
		catch (ClassNotFoundException e)
		{
			throw new IllegalArgumentException(e);
		}
		finally
		{
			if (oip != null)
			{
				try
				{
					oip.close();
				}
				catch (IOException e)
				{
					// eat it
				}
			}
		}
	}

And then for YourOwnConfigurableObjectInputStream, this guy's answer works perfectly:
https://stackoverflow.com/questions/1816559/make-java-runtime-ignore-serialversionuids

@DevDengChao
Copy link

I think there should be an OAuth2AuthenticationJackson2Deserializer.

@OrangeDog
Copy link
Contributor

@XieEDeHeiShou none of the classes involved are beans, and cannot be deserialized with Jackson.

@DevDengChao
Copy link

DevDengChao commented Apr 9, 2020

Can these code meet (skip de-serialization gap && readability of access/refresh token)'s requirement ?

import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;
import lombok.extern.slf4j.Slf4j;
import org.springframework.data.redis.serializer.SerializationException;
import org.springframework.lang.NonNull;
import org.springframework.security.oauth2.provider.OAuth2Authentication;
import org.springframework.security.oauth2.provider.token.store.redis.JdkSerializationStrategy;
import org.springframework.security.oauth2.provider.token.store.redis.StandardStringSerializationStrategy;

import java.io.*;
import java.nio.charset.StandardCharsets;

@Slf4j
public class JacksonRedisTokenStoreSerializationStrategy extends StandardStringSerializationStrategy {

    @NonNull
    private final ObjectMapper mapper;
    @NonNull
    private final JdkSerializationStrategy jdkSerializationStrategy;

    public JacksonRedisTokenStoreSerializationStrategy() {
        this(new ObjectMapper());
    }

    public JacksonRedisTokenStoreSerializationStrategy(@NonNull ObjectMapper mapper) {
        this.mapper = mapper;
        jdkSerializationStrategy = new JdkSerializationStrategy();
    }

    @Override
    protected <T> T deserializeInternal(byte[] bytes, Class<T> clazz) {
        if (OAuth2Authentication.class.equals(clazz)) {
            try {
                log.debug("Deserializing input into OAuth2Authentication");
                //noinspection unchecked
                return (T) new DecompressibleInputStream(new ByteArrayInputStream(bytes)).readObject();
            } catch (Exception e) {
                log.warn("Unable to deserialize input into OAuth2Authentication object with DecompressibleInputStream");
                return jdkSerializationStrategy.deserialize(bytes, clazz);
            }
        }

        try {
            return mapper.readValue(bytes, clazz);
        } catch (IOException e) {
            String msg = "Unable to deserialize " + new String(bytes, StandardCharsets.UTF_8) + " into " + clazz.getName();
            throw new SerializationException(msg, e);
        }
    }

    @Override
    protected byte[] serializeInternal(Object object) {
        if (object instanceof OAuth2Authentication) {
            log.debug("Serializing OAuth2Authentication");
            return jdkSerializationStrategy.serialize(object);
        }
        try {
            return mapper.writeValueAsBytes(object);
        } catch (JsonProcessingException e) {
            throw new SerializationException("Unable to serialize " + object.getClass().getName(), e);
        }
    }

    @Slf4j
    private static class DecompressibleInputStream extends ObjectInputStream {

        public DecompressibleInputStream(InputStream in) throws IOException {
            super(in);
        }

        @Override
        protected ObjectStreamClass readClassDescriptor() throws IOException, ClassNotFoundException {
            ObjectStreamClass resultClassDescriptor = super.readClassDescriptor(); // initially streams descriptor
            Class<?> localClass; // the class in the local JVM that this descriptor represents.
            try {
                localClass = Class.forName(resultClassDescriptor.getName());
            } catch (ClassNotFoundException e) {
                log.error("No local class for " + resultClassDescriptor.getName(), e);
                return resultClassDescriptor;
            }
            ObjectStreamClass localClassDescriptor = ObjectStreamClass.lookup(localClass);
            if (localClassDescriptor != null) { // only if class implements serializable
                final long localSUID = localClassDescriptor.getSerialVersionUID();
                final long streamSUID = resultClassDescriptor.getSerialVersionUID();
                if (streamSUID != localSUID) { // check for serialVersionUID mismatch.
                    String s = "Overriding serialized class version mismatch: local serialVersionUID = " + localSUID +
                            " stream serialVersionUID = " + streamSUID;
                    Exception e = new InvalidClassException(s);
                    log.error("Potentially Fatal Deserialization Operation.", e);
                    resultClassDescriptor = localClassDescriptor; // Use local class descriptor for deserialization
                }
            }
            return resultClassDescriptor;
        }
    }
}

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

No branches or pull requests