This is an automated email from the ASF dual-hosted git repository.
pjfanning pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/pekko.git
The following commit(s) were added to refs/heads/main by this push:
new 347264aa0f fix: check the remote deployment allow list before
deserializing constructor args (#3493)
347264aa0f is described below
commit 347264aa0fa82bedc87250fbc76765107f0d2780
Author: PJ Fanning <[email protected]>
AuthorDate: Mon Aug 31 22:01:03 2026 +0100
fix: check the remote deployment allow list before deserializing
constructor args (#3493)
Motivation:
DaemonMsgCreateSerializer.fromBinary loaded the peer-named actor class and
then
deserialized every constructor argument - with peer-chosen serializer ids
and
manifests - before RemoteSystemDaemon consulted the remote deployment allow
list. For a class the allow list rejects, that deserialization is attack
surface
taken on for a deployment that is refused moments later, which weakens the
guarantee the allow list is credited with.
Modification:
Extract RemoteDeploymentAllowList, holding the two existing config keys and
the
class-name comparison, and use it from both RemoteSystemDaemon and
DaemonMsgCreateSerializer so the two cannot drift. The serializer now
checks the
allow list immediately after resolving the actor class and before
deserializing
any argument, logging at error with LogMarker.Security and the same
exception
RemoteSystemDaemon raises.
Result:
With the allow list enabled, a rejected class is refused before any
peer-supplied argument is deserialized. No new configuration, and no change
when the allow list is off, which is the default.
Tests:
- sbt "remote/testOnly
org.apache.pekko.remote.serialization.DaemonMsgCreateSerializerAllowListSpec
org.apache.pekko.remote.serialization.DaemonMsgCreateSerializerAllowListDisabledSpec"
- new, incl. a counting serializer asserting args are not deserialized for a
rejected class
- sbt "remote/testOnly
org.apache.pekko.remote.classic.RemoteDeploymentAllowListSpec
org.apache.pekko.remote.serialization.DaemonMsgCreateSerializerAllowJavaSerializationSpec
org.apache.pekko.remote.serialization.DaemonMsgCreateSerializerNoJavaSerializationSpec"
- existing specs pass unchanged
- sbt "remote/mimaReportBinaryIssues" - no issues
References:
Refs #3478
---
.../org/apache/pekko/remote/RemoteDaemon.scala | 50 +++++--
.../serialization/DaemonMsgCreateSerializer.scala | 26 ++++
.../DaemonMsgCreateSerializerAllowListSpec.scala | 146 +++++++++++++++++++++
3 files changed, 212 insertions(+), 10 deletions(-)
diff --git a/remote/src/main/scala/org/apache/pekko/remote/RemoteDaemon.scala
b/remote/src/main/scala/org/apache/pekko/remote/RemoteDaemon.scala
index 34ac5b41e5..16885a6394 100644
--- a/remote/src/main/scala/org/apache/pekko/remote/RemoteDaemon.scala
+++ b/remote/src/main/scala/org/apache/pekko/remote/RemoteDaemon.scala
@@ -17,6 +17,7 @@ import java.util.concurrent.ConcurrentHashMap
import scala.annotation.tailrec
import scala.collection.immutable
+import scala.jdk.CollectionConverters._
import scala.util.control.NonFatal
import org.apache.pekko
@@ -42,11 +43,14 @@ import pekko.actor.Identify
import pekko.actor.SelectChildName
import pekko.actor.SelectChildPattern
import pekko.actor.SelectParent
+import pekko.annotation.InternalApi
import pekko.dispatch.sysmsg.{ DeathWatchNotification, SystemMessage, Watch }
import pekko.dispatch.sysmsg.Unwatch
import pekko.event.{ AddressTerminatedTopic, LogMarker, MarkerLoggingAdapter }
import pekko.util.Switch
+import com.typesafe.config.Config
+
/**
* INTERNAL API
*/
@@ -83,13 +87,8 @@ private[pekko] class RemoteSystemDaemon(
private val parent2children = new ConcurrentHashMap[ActorRef, Set[ActorRef]]
- private val allowListEnabled =
system.settings.config.getBoolean("pekko.remote.deployment.enable-allow-list")
- private val remoteDeploymentAllowList: immutable.Set[String] = {
- import scala.jdk.CollectionConverters._
- if (allowListEnabled)
-
system.settings.config.getStringList("pekko.remote.deployment.allowed-actor-classes").asScala.toSet
- else Set.empty
- }
+ private val allowList = RemoteDeploymentAllowList(system.settings.config)
+ private def allowListEnabled = allowList.enabled
@tailrec private def addChildParentNeedsWatch(parent: ActorRef, child:
ActorRef): Boolean =
parent2children.get(parent) match {
@@ -176,12 +175,11 @@ private[pekko] class RemoteSystemDaemon(
log.debug("does not accept deployments (untrusted) for [{}]",
path) // TODO add security marker?
case DaemonMsgCreate(props, deploy, path, supervisor) if
allowListEnabled =>
- val name = props.clazz.getCanonicalName
- if (remoteDeploymentAllowList.contains(name))
+ if (allowList.isAllowed(props.clazz))
doCreateActor(message, props, deploy, path, supervisor)
else {
val ex =
- new
NotAllowedClassRemoteDeploymentAttemptException(props.actorClass(),
remoteDeploymentAllowList)
+ new
NotAllowedClassRemoteDeploymentAttemptException(props.actorClass(),
allowList.allowedClassNames)
log.error(
LogMarker.Security,
ex,
@@ -284,6 +282,38 @@ private[pekko] class RemoteSystemDaemon(
}
+/**
+ * INTERNAL API
+ *
+ * The remote deployment allow list, shared by `RemoteSystemDaemon` and
+ * `DaemonMsgCreateSerializer` so that both read the same configuration and
apply the
+ * same class-name comparison.
+ */
+@InternalApi
+private[pekko] object RemoteDeploymentAllowList {
+ final val EnableAllowListPath = "pekko.remote.deployment.enable-allow-list"
+ final val AllowedActorClassesPath =
"pekko.remote.deployment.allowed-actor-classes"
+
+ def apply(config: Config): RemoteDeploymentAllowList = {
+ val enabled = config.getBoolean(EnableAllowListPath)
+ val allowed =
+ if (enabled) config.getStringList(AllowedActorClassesPath).asScala.toSet
+ else immutable.Set.empty[String]
+ new RemoteDeploymentAllowList(enabled, allowed)
+ }
+}
+
+/** INTERNAL API */
+@InternalApi
+private[pekko] final class RemoteDeploymentAllowList(
+ val enabled: Boolean,
+ val allowedClassNames: immutable.Set[String]) {
+
+ /** True when the allow list is disabled, or the class is listed on it. */
+ def isAllowed(actorClass: Class[?]): Boolean =
+ !enabled || allowedClassNames.contains(actorClass.getCanonicalName)
+}
+
/** INTERNAL API */
final class NotAllowedClassRemoteDeploymentAttemptException(illegal: Class[?],
allowedClassNames: immutable.Set[String])
extends RuntimeException(
diff --git
a/remote/src/main/scala/org/apache/pekko/remote/serialization/DaemonMsgCreateSerializer.scala
b/remote/src/main/scala/org/apache/pekko/remote/serialization/DaemonMsgCreateSerializer.scala
index e619eb29ec..d956d110cf 100644
---
a/remote/src/main/scala/org/apache/pekko/remote/serialization/DaemonMsgCreateSerializer.scala
+++
b/remote/src/main/scala/org/apache/pekko/remote/serialization/DaemonMsgCreateSerializer.scala
@@ -20,9 +20,12 @@ import util.{ Failure, Success }
import org.apache.pekko
import pekko.actor.{ Deploy, ExtendedActorSystem, NoScopeGiven, Props, Scope }
+import pekko.event.{ LogMarker, Logging }
import pekko.protobufv3.internal.ByteString
import pekko.remote.ByteStringUtils
import pekko.remote.DaemonMsgCreate
+import pekko.remote.NotAllowedClassRemoteDeploymentAttemptException
+import pekko.remote.RemoteDeploymentAllowList
import pekko.remote.WireFormats.{ DaemonMsgCreateData, DeployData, PropsData }
import pekko.routing.{ NoRouter, RouterConfig }
import pekko.serialization.{ BaseSerializer, SerializationExtension,
SerializerWithStringManifest }
@@ -43,6 +46,9 @@ private[pekko] final class DaemonMsgCreateSerializer(val
system: ExtendedActorSy
import ProtobufSerializer.serializeActorRef
private lazy val serialization = SerializationExtension(system)
+ private lazy val log = Logging.withMarker(system,
classOf[DaemonMsgCreateSerializer])
+
+ private val allowList = RemoteDeploymentAllowList(system.settings.config)
override val includeManifest: Boolean = false
@@ -174,6 +180,12 @@ private[pekko] final class DaemonMsgCreateSerializer(val
system: ExtendedActorSy
import scala.jdk.CollectionConverters._
val protoProps = proto.getProps
val actorClass =
system.dynamicAccess.getClassFor[AnyRef](protoProps.getClazz).get
+ // Check the allow list before deserializing the constructor arguments
below. Those
+ // arguments are peer-supplied and are deserialized with peer-chosen
serializer ids and
+ // manifests, so for a class the allow list would reject that work is
attack surface
+ // taken on for a deployment that will be refused anyway.
`RemoteSystemDaemon` performs
+ // the same check when it handles the message; this one only makes it
earlier.
+ checkAllowedActorClass(actorClass)
val args: Vector[AnyRef] =
// message from a newer node always contains serializer ids and
possibly a string manifest for each position
if (protoProps.getSerializerIdsCount > 0) {
@@ -206,6 +218,20 @@ private[pekko] final class DaemonMsgCreateSerializer(val
system: ExtendedActorSy
supervisor = deserializeActorRef(system, proto.getSupervisor))
}
+ private def checkAllowedActorClass(actorClass: Class[?]): Unit =
+ if (!allowList.isAllowed(actorClass)) {
+ val ex = new NotAllowedClassRemoteDeploymentAttemptException(actorClass,
allowList.allowedClassNames)
+ // Logged at error with the exception, matching `RemoteSystemDaemon`, so
that the
+ // security signal is the same wherever the deployment is refused.
+ log.error(
+ LogMarker.Security,
+ ex,
+ "Received command to create remote Actor, but class [{}] is not
allow-listed! " +
+ "Rejected before deserializing the constructor arguments.",
+ actorClass.getName)
+ throw ex
+ }
+
private def serialize(any: Any): (Int, Boolean, String, Array[Byte]) = {
val m = any.asInstanceOf[AnyRef]
val serializer = serialization.findSerializerFor(m)
diff --git
a/remote/src/test/scala/org/apache/pekko/remote/serialization/DaemonMsgCreateSerializerAllowListSpec.scala
b/remote/src/test/scala/org/apache/pekko/remote/serialization/DaemonMsgCreateSerializerAllowListSpec.scala
new file mode 100644
index 0000000000..60a0f6c91d
--- /dev/null
+++
b/remote/src/test/scala/org/apache/pekko/remote/serialization/DaemonMsgCreateSerializerAllowListSpec.scala
@@ -0,0 +1,146 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.pekko.remote.serialization
+
+import java.nio.charset.StandardCharsets.UTF_8
+import java.util.concurrent.atomic.AtomicInteger
+
+import scala.annotation.nowarn
+
+import org.apache.pekko
+import pekko.actor.Actor
+import pekko.actor.Deploy
+import pekko.actor.Props
+import pekko.remote.DaemonMsgCreate
+import pekko.remote.NotAllowedClassRemoteDeploymentAttemptException
+import pekko.serialization.SerializationExtension
+import pekko.serialization.SerializerWithStringManifest
+import pekko.testkit.PekkoSpec
+
+object DaemonMsgCreateSerializerAllowListSpec {
+
+ trait EmptyActor extends Actor {
+ def receive = Actor.emptyBehavior
+ }
+
+ class AllowedActor(@nowarn("msg=never used") arg: MarkerArg) extends
EmptyActor
+ class NotAllowedActor(@nowarn("msg=never used") arg: MarkerArg) extends
EmptyActor
+ class SupervisorActor extends EmptyActor
+
+ final case class MarkerArg(value: String)
+
+ /** Counts how often a constructor argument is actually deserialized. */
+ val deserializeCount = new AtomicInteger(0)
+
+ class MarkerArgSerializer extends SerializerWithStringManifest {
+ override def identifier: Int = 987654
+ override def manifest(o: AnyRef): String = "M"
+ override def toBinary(o: AnyRef): Array[Byte] =
o.asInstanceOf[MarkerArg].value.getBytes(UTF_8)
+ override def fromBinary(bytes: Array[Byte], manifest: String): AnyRef = {
+ deserializeCount.incrementAndGet()
+ MarkerArg(new String(bytes, UTF_8))
+ }
+ }
+}
+
+class DaemonMsgCreateSerializerAllowListSpec
+ extends PekkoSpec(s"""
+ pekko.remote.deployment {
+ enable-allow-list = on
+ allowed-actor-classes = [
+
"org.apache.pekko.remote.serialization.DaemonMsgCreateSerializerAllowListSpec.AllowedActor"
+ ]
+ }
+ pekko.actor {
+ serializers.marker-arg =
"${classOf[DaemonMsgCreateSerializerAllowListSpec.MarkerArgSerializer].getName}"
+ serialization-bindings {
+
"org.apache.pekko.remote.serialization.DaemonMsgCreateSerializerAllowListSpec$$MarkerArg"
= marker-arg
+ }
+ }
+ """) {
+
+ import DaemonMsgCreateSerializerAllowListSpec._
+
+ private val ser = SerializationExtension(system)
+ private val supervisor = system.actorOf(Props[SupervisorActor](),
"supervisor")
+
+ private def daemonMsgCreate(actorClass: Class[?]): DaemonMsgCreate =
+ DaemonMsgCreate(
+ props = Props(actorClass, MarkerArg("payload")),
+ deploy = Deploy(),
+ path = "foo",
+ supervisor = supervisor)
+
+ "DaemonMsgCreateSerializer with the remote deployment allow list enabled"
must {
+
+ "deserialize a DaemonMsgCreate for an allow-listed class" in {
+ val bytes = ser.serialize(daemonMsgCreate(classOf[AllowedActor])).get
+ val got = ser.deserialize(bytes,
classOf[DaemonMsgCreate]).get.asInstanceOf[DaemonMsgCreate]
+ got.props.clazz should ===(classOf[AllowedActor])
+ got.props.args should ===(Seq(MarkerArg("payload")))
+ }
+
+ "reject a DaemonMsgCreate for a class that is not allow-listed" in {
+ val bytes = ser.serialize(daemonMsgCreate(classOf[NotAllowedActor])).get
+ val ex = intercept[NotAllowedClassRemoteDeploymentAttemptException] {
+ ser.deserialize(bytes, classOf[DaemonMsgCreate]).get
+ }
+ ex.getMessage should include("NotAllowedActor")
+ }
+
+ "not deserialize the constructor arguments of a rejected class" in {
+ val bytes = ser.serialize(daemonMsgCreate(classOf[NotAllowedActor])).get
+ deserializeCount.set(0)
+ intercept[NotAllowedClassRemoteDeploymentAttemptException] {
+ ser.deserialize(bytes, classOf[DaemonMsgCreate]).get
+ }
+ withClue("peer-supplied constructor arguments must not be deserialized
for a rejected class") {
+ deserializeCount.get should ===(0)
+ }
+ }
+ }
+}
+
+class DaemonMsgCreateSerializerAllowListDisabledSpec extends PekkoSpec(s"""
+ pekko.actor {
+ serializers.marker-arg =
"${classOf[DaemonMsgCreateSerializerAllowListSpec.MarkerArgSerializer].getName}"
+ serialization-bindings {
+
"org.apache.pekko.remote.serialization.DaemonMsgCreateSerializerAllowListSpec$$MarkerArg"
= marker-arg
+ }
+ }
+ """) {
+
+ import DaemonMsgCreateSerializerAllowListSpec._
+
+ private val ser = SerializationExtension(system)
+ private val supervisor = system.actorOf(Props[SupervisorActor](),
"supervisor")
+
+ "DaemonMsgCreateSerializer with the allow list disabled (the default)" must {
+
+ "deserialize any actor class, as before" in {
+ val msg = DaemonMsgCreate(
+ props = Props(classOf[NotAllowedActor], MarkerArg("payload")),
+ deploy = Deploy(),
+ path = "foo",
+ supervisor = supervisor)
+ val got = ser.deserialize(ser.serialize(msg).get,
classOf[DaemonMsgCreate]).get.asInstanceOf[DaemonMsgCreate]
+ got.props.clazz should ===(classOf[NotAllowedActor])
+ got.props.args should ===(Seq(MarkerArg("payload")))
+ }
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
