Skip to content

Commit

Permalink
Make it easier to disable Java serialization, #22283
Browse files Browse the repository at this point in the history 
* one single config option to disable it completely
* improve security documentation

Co-authored-by: Patrik Nordwall <patrik.nordwall@gmail.com>
  • Loading branch information
@ktoso @patriknw
ktoso and patriknw committed Feb 10, 2017
1 parent 0b3b86e commit cc6561b
Show file tree
Hide file tree
Showing 19 changed files with 830 additions and 170 deletions.
91 changes: 85 additions & 6 deletions akka-actor-tests/src/test/scala/akka/serialization/SerializationSetupSpec.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,26 +4,30 @@
package akka.serialization

import akka.actor.setup.ActorSystemSetup
import akka.actor.{ ActorSystem, BootstrapSetup }
import akka.testkit.AkkaSpec
import akka.actor.{ ActorSystem, BootstrapSetup, ExtendedActorSystem, Terminated }
import akka.testkit.{ AkkaSpec, TestKit, TestProbe }
import com.typesafe.config.ConfigFactory

import scala.concurrent.duration._

class ConfigurationDummy
class ProgrammaticDummy
case class ProgrammaticJavaDummy()
case class SerializableDummy() // since case classes are serializable

object SerializationSetupSpec {

val testSerializer = new TestSerializer

val serializationSettings = SerializationSetup { _ ⇒
List(
SerializerDetails("test", testSerializer, List(classOf[ProgrammaticDummy]))
)
SerializerDetails("test", testSerializer, List(classOf[ProgrammaticDummy])))
}
val bootstrapSettings = BootstrapSetup(None, Some(ConfigFactory.parseString("""
akka {
actor {
serialize-messages = off
serialization-bindings {
"akka.serialization.ConfigurationDummy" = test
}
Expand All @@ -32,11 +36,20 @@ object SerializationSetupSpec {
""")), None)
val actorSystemSettings = ActorSystemSetup(bootstrapSettings, serializationSettings)

val noJavaSerializationSystem = ActorSystem("SerializationSettingsSpec" + "NoJavaSerialization", ConfigFactory.parseString(
"""
akka {
actor {
allow-java-serialization = off
}
}
""".stripMargin))
val noJavaSerializer = new DisabledJavaSerializer(noJavaSerializationSystem.asInstanceOf[ExtendedActorSystem])

}

class SerializationSetupSpec extends AkkaSpec(
ActorSystem("SerializationSettingsSpec", SerializationSetupSpec.actorSystemSettings)
) {
ActorSystem("SerializationSettingsSpec", SerializationSetupSpec.actorSystemSettings)) {

import SerializationSetupSpec._

Expand All @@ -54,4 +67,70 @@ class SerializationSetupSpec extends AkkaSpec(

}

// This is a weird edge case, someone creating a JavaSerializer manually and using it in a system means
// that they'd need a different actor system to be able to create it... someone MAY pick a system with
// allow-java-serialization=on to create the SerializationSetup and use that SerializationSetup
// in another system with allow-java-serialization=off
val addedJavaSerializationSettings = SerializationSetup { _ ⇒
List(
SerializerDetails("test", testSerializer, List(classOf[ProgrammaticDummy])),
SerializerDetails("java-manual", new JavaSerializer(system.asInstanceOf[ExtendedActorSystem]), List(classOf[ProgrammaticJavaDummy])))
}
val addedJavaSerializationProgramaticallyButDisabledSettings = BootstrapSetup(None, Some(ConfigFactory.parseString("""
akka {
loglevel = debug
actor {
allow-java-serialization = off
}
}
""")), None)

val addedJavaSerializationViaSettingsSystem =
ActorSystem("addedJavaSerializationSystem", ActorSystemSetup(addedJavaSerializationProgramaticallyButDisabledSettings, addedJavaSerializationSettings))

"Disabling java serialization" should {

"throw if passed system to JavaSerializer has allow-java-serialization = off" in {
intercept[DisabledJavaSerializer.JavaSerializationException] {
new JavaSerializer(noJavaSerializationSystem.asInstanceOf[ExtendedActorSystem])
}.getMessage should include("akka.actor.allow-java-serialization = off")

intercept[DisabledJavaSerializer.JavaSerializationException] {
SerializationExtension(addedJavaSerializationViaSettingsSystem).findSerializerFor(new ProgrammaticJavaDummy).toBinary(new ProgrammaticJavaDummy)
}
}

"have replaced java serializer" in {
val p = TestProbe()(addedJavaSerializationViaSettingsSystem) // only receiver has the serialization disabled

p.ref ! new ProgrammaticJavaDummy
SerializationExtension(system).findSerializerFor(new ProgrammaticJavaDummy).toBinary(new ProgrammaticJavaDummy)
// should not receive this one, it would have been java serialization!
p.expectNoMsg(100.millis)

p.ref ! new ProgrammaticDummy
p.expectMsgType[ProgrammaticDummy]
}

"disable java serialization also for incoming messages if serializer id usually would have found the serializer" in {
val ser1 = SerializationExtension(system)
val msg = SerializableDummy()
val bytes = ser1.serialize(msg).get
val serId = ser1.findSerializerFor(msg).identifier
ser1.findSerializerFor(msg).includeManifest should ===(false)

val ser2 = SerializationExtension(noJavaSerializationSystem)
ser2.findSerializerFor(new SerializableDummy) should ===(noJavaSerializer)
ser2.serializerByIdentity(serId) should ===(noJavaSerializer)
intercept[DisabledJavaSerializer.JavaSerializationException] {
ser2.deserialize(bytes, serId, "").get
}
}
}

override def afterTermination(): Unit = {
TestKit.shutdownActorSystem(noJavaSerializationSystem)
TestKit.shutdownActorSystem(addedJavaSerializationViaSettingsSystem)
}

}
28 changes: 25 additions & 3 deletions akka-actor/src/main/resources/reference.conf
View file
Original file line number Diff line number Diff line change
Expand Up @@ -567,7 +567,28 @@ akka {
# enable WARN logging of misconfigured routers
router-misconfiguration = off
}


# SECURITY BEST-PRACTICE is to disable java serialization for its multiple
# known attack surfaces.
#
# This setting is a short-cut to
# - using DisabledJavaSerializer instead of JavaSerializer
# - enable-additional-serialization-bindings = on
#
# Completely disable the use of `akka.serialization.JavaSerialization` by the
# Akka Serialization extension, instead DisabledJavaSerializer will
# be inserted which will fail explicitly if attempts to use java serialization are made.
#
# The log messages emitted by such serializer SHOULD be be treated as potential
# attacks which the serializer prevented, as they MAY indicate an external operator
# attempting to send malicious messages intending to use java serialization as attack vector.
# The attempts are logged with the SECURITY marker.
#
# Please note that this option does not stop you from manually invoking java serialization
#
# The default value for this might be changed to off in future versions of Akka.
allow-java-serialization = on

# Entries for pluggable serializers and their bindings.
serializers {
java = "akka.serialization.JavaSerializer"
Expand All @@ -589,13 +610,14 @@ akka {
# Set this to on to enable serialization-bindings define in
# additional-serialization-bindings. Those are by default not included
# for backwards compatibility reasons. They are enabled by default if
# akka.remote.artery.enabled=on.
# akka.remote.artery.enabled=on or if akka.actor.allow-java-serialization=off.
enable-additional-serialization-bindings = off

# Additional serialization-bindings that are replacing Java serialization are
# defined in this section and not included by default for backwards compatibility
# reasons. They can be enabled with enable-additional-serialization-bindings=on.
# They are enabled by default if akka.remote.artery.enabled=on.
# They are enabled by default if akka.remote.artery.enabled=on or if
# akka.actor.allow-java-serialization=off.
additional-serialization-bindings {
}

Expand Down
1 change: 1 addition & 0 deletions akka-actor/src/main/scala/akka/actor/Actor.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ final case class Terminated private[akka] (@BeanProperty actor: ActorRef)(
@BeanProperty val existenceConfirmed: Boolean,
@BeanProperty val addressTerminated: Boolean)
extends AutoReceivedMessage with PossiblyHarmful with DeadLetterSuppression
with NoSerializationVerificationNeeded // local message, the remote one is DeathWatchNotification

/**
* INTERNAL API
Expand Down
3 changes: 3 additions & 0 deletions akka-actor/src/main/scala/akka/actor/ActorSystem.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,9 @@ object ActorSystem {
final val CreationTimeout: Timeout = Timeout(config.getMillisDuration("akka.actor.creation-timeout"))
final val UnstartedPushTimeout: Timeout = Timeout(config.getMillisDuration("akka.actor.unstarted-push-timeout"))

final val AllowJavaSerialization: Boolean = getBoolean("akka.actor.allow-java-serialization")
final val EnableAdditionalSerializationBindings: Boolean =
!AllowJavaSerialization || getBoolean("akka.actor.enable-additional-serialization-bindings")
final val SerializeAllMessages: Boolean = getBoolean("akka.actor.serialize-messages")
final val SerializeAllCreators: Boolean = getBoolean("akka.actor.serialize-creators")

Expand Down
76 changes: 58 additions & 18 deletions akka-actor/src/main/scala/akka/serialization/Serialization.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,12 @@ object Serialization {
val defaultBindings = config.getConfig("akka.actor.serialization-bindings")
val bindings =
if (config.getBoolean("akka.actor.enable-additional-serialization-bindings") ||
config.hasPath("akka.remote.artery.enabled") && config.getBoolean("akka.remote.artery.enabled"))
!config.getBoolean("akka.actor.allow-java-serialization") ||
config.hasPath("akka.remote.artery.enabled") && config.getBoolean("akka.remote.artery.enabled")) {
defaultBindings.withFallback(config.getConfig("akka.actor.additional-serialization-bindings"))
else defaultBindings
} else {
defaultBindings
}
configToMap(bindings)
}

Expand Down Expand Up @@ -97,6 +100,8 @@ class Serialization(val system: ExtendedActorSystem) extends Extension {
import Serialization._

val settings = new Settings(system.settings.config)
val AllowJavaSerialization: Boolean = system.settings.AllowJavaSerialization

private[this] val _log = Logging.withMarker(system, getClass.getName)
val log: LoggingAdapter = _log
private val manifestCache = new AtomicReference[Map[String, Option[Class[_]]]](Map.empty[String, Option[Class[_]]])
Expand Down Expand Up @@ -222,22 +227,27 @@ class Serialization(val system: ExtendedActorSystem) extends Extension {
(possibilities forall (_._1 isAssignableFrom possibilities(0)._1)) ||
(possibilities forall (_._2 == possibilities(0)._2))

val ser = bindings filter { _._1 isAssignableFrom clazz } match {
case Seq() ⇒
throw new NotSerializableException("No configured serialization-bindings for class [%s]" format clazz.getName)
case possibilities ⇒
if (!unique(possibilities))
_log.warning(LogMarker.Security, "Multiple serializers found for " + clazz + ", choosing first: " + possibilities)
possibilities(0)._2
val ser = {
bindings.filter {
case (c, _) ⇒ c isAssignableFrom clazz
} match {
case immutable.Seq() ⇒
throw new NotSerializableException("No configured serialization-bindings for class [%s]" format clazz.getName)
case possibilities ⇒
if (!unique(possibilities))
_log.warning(LogMarker.Security, "Multiple serializers found for " + clazz + ", choosing first: " + possibilities)
possibilities(0)._2
}
}

serializerMap.putIfAbsent(clazz, ser) match {
case null ⇒
if (shouldWarnAboutJavaSerializer(clazz, ser)) {
_log.warning(LogMarker.Security, "Using the default Java serializer for class [{}] which is not recommended because of " +
"performance implications. Use another serializer or disable this warning using the setting " +
"'akka.actor.warn-about-java-serializer-usage'", clazz.getName)
}
log.debug("Using serializer[{}] for message [{}]", ser.getClass.getName, clazz.getName)
log.debug("Using serializer [{}] for message [{}]", ser.getClass.getName, clazz.getName)
ser
case some ⇒ some
}
Expand All @@ -248,18 +258,34 @@ class Serialization(val system: ExtendedActorSystem) extends Extension {
* Tries to load the specified Serializer by the fully-qualified name; the actual
* loading is performed by the system’s [[akka.actor.DynamicAccess]].
*/
def serializerOf(serializerFQN: String): Try[Serializer] =
system.dynamicAccess.createInstanceFor[Serializer](serializerFQN, List(classOf[ExtendedActorSystem] → system)) recoverWith {
case _: NoSuchMethodException ⇒ system.dynamicAccess.createInstanceFor[Serializer](serializerFQN, Nil)
def serializerOf(serializerFQN: String): Try[Serializer] = {
// We override each instantiation of the JavaSerializer with the "disabled" serializer which will log warnings if used.
val fqn =
if (!system.settings.AllowJavaSerialization && serializerFQN == classOf[JavaSerializer].getName) {
log.debug("Replacing JavaSerializer with DisabledJavaSerializer, " +
"due to `akka.actor.allow-java-serialization = off`.")
classOf[DisabledJavaSerializer].getName
} else serializerFQN

system.dynamicAccess.createInstanceFor[Serializer](fqn, List(classOf[ExtendedActorSystem] → system)) recoverWith {
case _: NoSuchMethodException ⇒
system.dynamicAccess.createInstanceFor[Serializer](fqn, Nil)
}
}

/**
* Programmatically defined serializers
*/
private val serializerDetails =
system.settings.setup.get[SerializationSetup] match {
private val serializerDetails: immutable.Seq[SerializerDetails] =
(system.settings.setup.get[SerializationSetup] match {
case None ⇒ Vector.empty
case Some(setting) ⇒ setting.createSerializers(system)
}) collect {
case det: SerializerDetails if isDisallowedJavaSerializer(det.serializer) ⇒
log.debug("Replacing JavaSerializer with DisabledJavaSerializer, " +
"due to `akka.actor.allow-java-serialization = off`.")
SerializerDetails(det.alias, new DisabledJavaSerializer(system), det.useFor)
case det ⇒ det
}

/**
Expand All @@ -268,7 +294,9 @@ class Serialization(val system: ExtendedActorSystem) extends Extension {
*/
private val serializers: Map[String, Serializer] = {
val fromConfig = for ((k: String, v: String) ← settings.Serializers) yield k → serializerOf(v).get
fromConfig ++ serializerDetails.map(d ⇒ d.alias → d.serializer)
val result = fromConfig ++ serializerDetails.map(d ⇒ d.alias → d.serializer)
ensureOnlyAllowedSerializers(result.map { case (_, ser) ⇒ ser }(collection.breakOut))
result
}

/**
Expand All @@ -285,7 +313,15 @@ class Serialization(val system: ExtendedActorSystem) extends Extension {
detail.useFor.map(clazz ⇒ clazz → detail.serializer)
}

sort(fromConfig ++ fromSettings)
val result = sort(fromConfig ++ fromSettings)
ensureOnlyAllowedSerializers(result.map { case (_, ser) ⇒ ser }(collection.breakOut))
result
}

private def ensureOnlyAllowedSerializers(iter: Iterator[Serializer]): Unit = {
if (!system.settings.AllowJavaSerialization) {
require(iter.forall(!isDisallowedJavaSerializer(_)), "Disallowed JavaSerializer binding.")
}
}

// com.google.protobuf serialization binding is only used if the class can be loaded,
Expand Down Expand Up @@ -349,6 +385,10 @@ class Serialization(val system: ExtendedActorSystem) extends Extension {
private val isJavaSerializationWarningEnabled = settings.config.getBoolean("akka.actor.warn-about-java-serializer-usage")
private val isWarningOnNoVerificationEnabled = settings.config.getBoolean("akka.actor.warn-on-no-serialization-verification")

private def isDisallowedJavaSerializer(serializer: Serializer): Boolean = {
serializer.isInstanceOf[JavaSerializer] && !system.settings.AllowJavaSerialization
}

private def shouldWarnAboutJavaSerializer(serializedClass: Class[_], serializer: Serializer) = {

def suppressWarningOnNonSerializationVerification(serializedClass: Class[_]) = {
Expand All @@ -357,7 +397,7 @@ class Serialization(val system: ExtendedActorSystem) extends Extension {
}

isJavaSerializationWarningEnabled &&
serializer.isInstanceOf[JavaSerializer] &&
(serializer.isInstanceOf[JavaSerializer] || serializer.isInstanceOf[DisabledJavaSerializer]) &&
!serializedClass.getName.startsWith("akka.") &&
!serializedClass.getName.startsWith("java.lang.") &&
!suppressWarningOnNonSerializationVerification(serializedClass)
Expand Down

0 comments on commit cc6561b

Please sign in to comment.