Jackson whitelist for deserialization of unbound class, #26910

This commit is contained in:
Patrik Nordwall 2019-06-05 12:31:02 +02:00
parent 5d22e1e20d
commit c62f428d51
5 changed files with 60 additions and 9 deletions

View file

@ -270,6 +270,18 @@ That type of migration must be configured with the old class name as key. The ac
@@snip [config](/akka-serialization-jackson/src/test/scala/doc/akka/serialization/jackson/SerializationDocSpec.scala) { #migrations-conf-rename }
### Remove from serialization-bindings
When a class is not used for serialization any more it can be removed from `serialization-bindings` but to still
allow deserialization it must then be listed in the `whitelist-class-prefix` configuration. This is useful for example
during rolling update with serialization changes, or when reading old stored data. It can also be used
when changing from Jackson serializer to another serializer (e.g. Protobuf) and thereby changing the serialization
binding, but it should still be possible to deserialize old data with Jackson.
@@snip [config](/akka-serialization-jackson/src/test/scala/doc/akka/serialization/jackson/SerializationDocSpec.scala) { #whitelist-class-prefix }
It's a list of class names or prefixes of class names.
## Jackson Modules
The following Jackson modules are enabled by default:

View file

@ -66,6 +66,11 @@ akka.serialization.jackson {
FAIL_ON_UNKNOWN_PROPERTIES = off
}
# Additional classes that are allowed even if they are not defined in `serialization-bindings`.
# This is useful when a class is not used for serialization any more and therefore removed
# from `serialization-bindings`, but should still be possible to deserialize.
whitelist-class-prefix = []
# Specific settings for jackson-json binding can be defined in this section to
# override the settings in 'akka.serialization.jackson'
jackson-json {}

View file

@ -158,6 +158,10 @@ import com.fasterxml.jackson.dataformat.smile.SmileFactory
}
}
private val blacklist: GadgetClassBlacklist = new GadgetClassBlacklist
private val whitelistClassPrefix = {
import scala.collection.JavaConverters._
conf.getStringList("whitelist-class-prefix").asScala.toVector
}
// This must lazy otherwise it will deadlock the ActorSystem creation
private lazy val serialization = SerializationExtension(system)
@ -286,7 +290,7 @@ import com.fasterxml.jackson.dataformat.smile.SmileFactory
val warnMsg = s"Can't serialize/deserialize object of type [${clazz.getName}] in [${getClass.getName}]. " +
"Only classes that are whitelisted are allowed for security reasons. " +
"Configure whitelist with akka.actor.serialization-bindings or " +
"akka.serialization.jackson.whitelist-packages."
"akka.serialization.jackson.whitelist-class-prefix."
log.warning(LogMarker.Security, warnMsg)
throw new IllegalArgumentException(warnMsg)
}
@ -299,30 +303,38 @@ import com.fasterxml.jackson.dataformat.smile.SmileFactory
* used for selecting serializer.
* Here we use `serialization-bindings` also and more importantly when deserializing (fromBinary)
* to check that the manifest class is of a known (registered) type.
* The drawback of using `serialization-bindings` for this is that an old class can't be removed
* from `serialization-bindings` when it's not used for serialization but still used for
* deserialization (e.g. rolling update with serialization changes). It's also
* not possible to change a binding from a JacksonSerializer to another serializer (e.g. protobuf)
*
* If and old class is removed from `serialization-bindings` when it's not used for serialization
* but still used for deserialization (e.g. rolling update with serialization changes) it can
* be allowed by specifying in `whitelist-class-prefix`.
*
* That is also possible when changing a binding from a JacksonSerializer to another serializer (e.g. protobuf)
* and still bind with the same class (interface).
* If this is too limiting we can add another config property as an additional way to
* whitelist classes that are not bound to this serializer with serialization-bindings.
*/
private def isInWhitelist(clazz: Class[_]): Boolean = {
isBoundToJacksonSerializer(clazz) || isInWhitelistClassPrefix(clazz.getName)
}
private def isBoundToJacksonSerializer(clazz: Class[_]): Boolean = {
try {
// The reason for using isInstanceOf rather than `eq this` is to allow change of
// serializizer within the Jackson family, but we don't trust other serializers
// because they might be bound to open-ended interfaces like java.io.Serializable.
val boundSerializer = serialization.serializerFor(clazz)
boundSerializer.isInstanceOf[JacksonSerializer] ||
// FIXME this is probably not needed when we have the more flexible configuration in place and
// can bind use the plain JacksonJsonSerializer for the old serializerId
// to support rolling updates in Lagom we also trust the binding to the Lagom 1.5.x JacksonJsonSerializer,
// which is named OldJacksonJsonSerializer in Lagom 1.6.x
// FIXME maybe make this configurable, but I don't see any other usages than for Lagom?
boundSerializer.getClass.getName == "com.lightbend.lagom.internal.jackson.OldJacksonJsonSerializer"
} catch {
case NonFatal(_) => false // not bound
}
}
private def isInWhitelistClassPrefix(className: String): Boolean =
whitelistClassPrefix.exists(className.startsWith)
/**
* Check that serialization-bindings are not configured with open-ended interfaces,
* like java.lang.Object, bound to this serializer.

View file

@ -82,6 +82,8 @@ object ScalaTestMessages {
// not defined in JsonSubTypes
final case class Cockroach(name: String) extends Animal
final case class OldCommandNotInBindings(name: String)
}
class ScalaTestEventMigration extends JacksonMigration {
@ -273,6 +275,17 @@ class JacksonJsonSerializerSpec extends JacksonSerializerSpec("jackson-json") {
json should ===(expected)
}
}
"allow deserialization of classes in configured whitelist-class-prefix" in {
val json = """{"name":"abc"}"""
val old = SimpleCommand("abc")
val serializer = serializerFor(old)
val expected = OldCommandNotInBindings("abc")
deserializeFromJsonString(json, serializer.identifier, serializer.manifest(expected)) should ===(expected)
}
}
}
@ -294,6 +307,7 @@ abstract class JacksonSerializerSpec(serializerName: String)
"akka.serialization.jackson.JavaTestMessages$$TestMessage" = $serializerName
}
}
akka.serialization.jackson.whitelist-class-prefix = ["akka.serialization.jackson.ScalaTestMessages$$OldCommand"]
""")))
with WordSpecLike
with Matchers

View file

@ -38,7 +38,7 @@ object SerializationDocSpec {
val configMigrationRenamClass = """
#//#migrations-conf-rename
akka.serialization.jackson.migrations {
"com.myservice.event.OrederAdded" = "com.myservice.event.OrderPlacedMigration"
"com.myservice.event.OrderAdded" = "com.myservice.event.OrderPlacedMigration"
}
#//#migrations-conf-rename
"""
@ -111,5 +111,13 @@ object SerializationDocSpec {
}
#//#date-time
"""
val configWhitelist = """
#//#whitelist-class-prefix
akka.serialization.jackson.whitelist-class-prefix =
["com.myservice.event.OrderAdded", "com.myservice.command"]
#//#whitelist-class-prefix
"""
}
// FIXME add real tests for the migrations, see EventMigrationTest.java in Lagom