Defend against deserialization-based attacks (#2150)

* Defend against deserialization-based attacks

Added the `SafeObjectInputStream` class that defends attacks using
malformed serialized data, including remote code execution and
denial-of-service attacks.

Started using the new class to handle EPP resource VKeys and
PendingDeposits, which are passed across credential-boundaries: between
TaskQueue and AppEngine server, and between AppEngine server and the RDE
pipeline on GCE. Note that the wireformat of VKeys do not change,
therefore existing tasks sitting in the TaskQueue are not affected.

Also removed an unused class: JaxbFragment.

core/src/main/java/google/registry/beam/rde/RdePipeline.java

@@ -27,6 +27,10 @@ import static google.registry.beam.rde.RdePipeline.TupleTags.REVISION_ID;
 import static google.registry.beam.rde.RdePipeline.TupleTags.SUPERORDINATE_DOMAINS;
 import static google.registry.model.reporting.HistoryEntryDao.RESOURCE_TYPES_TO_HISTORY_TYPES;
 import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
+import static google.registry.util.SafeSerializationUtils.safeDeserializeCollection;
+import static google.registry.util.SafeSerializationUtils.serializeCollection;
+import static google.registry.util.SerializeUtils.decodeBase64;
+import static google.registry.util.SerializeUtils.encodeBase64;
 import static org.apache.beam.sdk.values.TypeDescriptors.kvs;
 
 import com.google.common.collect.ImmutableList;
@@ -65,11 +69,7 @@ import google.registry.rde.PendingDeposit.PendingDepositCoder;
 import google.registry.rde.RdeMarshaller;
 import google.registry.util.UtilsModule;
 import google.registry.xml.ValidationMode;
-import java.io.ByteArrayInputStream;
-import java.io.ByteArrayOutputStream;
 import java.io.IOException;
-import java.io.ObjectInputStream;
-import java.io.ObjectOutputStream;
 import java.io.Serializable;
 import java.util.HashSet;
 import javax.inject.Inject;
@@ -658,14 +658,8 @@ public class RdePipeline implements Serializable {
    */
   @SuppressWarnings("unchecked")
   static ImmutableSet<PendingDeposit> decodePendingDeposits(String encodedPendingDeposits) {
-    try (ObjectInputStream ois =
-        new ObjectInputStream(
-            new ByteArrayInputStream(
-                BaseEncoding.base64Url().omitPadding().decode(encodedPendingDeposits)))) {
-      return (ImmutableSet<PendingDeposit>) ois.readObject();
-    } catch (IOException | ClassNotFoundException e) {
-      throw new IllegalArgumentException("Unable to parse encoded pending deposit map.", e);
-    }
+    return ImmutableSet.copyOf(
+        safeDeserializeCollection(PendingDeposit.class, decodeBase64(encodedPendingDeposits)));
   }
 
   /**
@@ -674,12 +668,7 @@ public class RdePipeline implements Serializable {
    */
   public static String encodePendingDeposits(ImmutableSet<PendingDeposit> pendingDeposits)
       throws IOException {
-    try (ByteArrayOutputStream baos = new ByteArrayOutputStream()) {
-      ObjectOutputStream oos = new ObjectOutputStream(baos);
-      oos.writeObject(pendingDeposits);
-      oos.flush();
-      return BaseEncoding.base64Url().omitPadding().encode(baos.toByteArray());
-    }
+    return encodeBase64(serializeCollection(pendingDeposits));
   }
 
   public static void main(String[] args) throws IOException, ClassNotFoundException {

core/src/main/java/google/registry/persistence/VKey.java

@@ -16,6 +16,8 @@ package google.registry.persistence;
 
 import static com.google.common.collect.ImmutableMap.toImmutableMap;
 import static google.registry.util.PreconditionsUtils.checkArgumentNotNull;
+import static google.registry.util.SafeSerializationUtils.safeDeserialize;
+import static google.registry.util.SerializeUtils.decodeBase64;
 import static java.util.function.Function.identity;
 
 import com.google.common.base.Joiner;
@@ -97,7 +99,7 @@ public class VKey<T> extends ImmutableObject implements Serializable {
       throw new IllegalArgumentException(
           String.format("\"%s\" missing from the string: %s", LOOKUP_KEY, keyString));
     }
-    return VKey.create(classType, SerializeUtils.parse(Serializable.class, kvs.get(LOOKUP_KEY)));
+    return VKey.create(classType, safeDeserialize(decodeBase64(kvs.get(LOOKUP_KEY))));
   }
 
   /** Returns the type of the entity. */

core/src/main/java/google/registry/rde/PendingDeposit.java

@@ -14,18 +14,23 @@
 
 package google.registry.rde;
 
+import static com.google.common.base.Preconditions.checkState;
+
 import com.google.auto.value.AutoValue;
 import google.registry.model.common.Cursor.CursorType;
 import google.registry.model.rde.RdeMode;
 import java.io.IOException;
 import java.io.InputStream;
+import java.io.ObjectInputStream;
+import java.io.ObjectOutputStream;
+import java.io.ObjectStreamException;
 import java.io.OutputStream;
 import java.io.Serializable;
+import java.util.Optional;
 import javax.annotation.Nullable;
 import org.apache.beam.sdk.coders.AtomicCoder;
 import org.apache.beam.sdk.coders.BooleanCoder;
 import org.apache.beam.sdk.coders.NullableCoder;
-import org.apache.beam.sdk.coders.SerializableCoder;
 import org.apache.beam.sdk.coders.StringUtf8Coder;
 import org.apache.beam.sdk.coders.VarIntCoder;
 import org.joda.time.DateTime;
@@ -35,6 +40,12 @@ import org.joda.time.Duration;
  * Container representing a single RDE or BRDA XML escrow deposit that needs to be created.
  *
  * <p>There are some {@code @Nullable} fields here because Optionals aren't Serializable.
+ *
+ * <p>Note that this class is serialized in two ways: by Beam pipelines using custom serialization
+ * mechanism and the {@code Coder} API, and by Java serialization when passed as command-line
+ * arguments (see {@code RdePipeline#decodePendingDeposits}). The latter requires safe
+ * deserialization because the data crosses credential boundaries (See {@code
+ * SafeObjectInputStream}).
  */
 @AutoValue
 public abstract class PendingDeposit implements Serializable {
@@ -95,11 +106,61 @@ public abstract class PendingDeposit implements Serializable {
 
   PendingDeposit() {}
 
+  /**
+   * Specifies that {@link SerializedForm} be used for {@code SafeObjectInputStream}-compatible
+   * custom-serialization of {@link AutoValue_PendingDeposit the AutoValue implementation class}.
+   *
+   * <p>This method is package-protected so that the AutoValue implementation class inherits this
+   * behavior.
+   *
+   * <p>This method leverages {@link PendingDepositCoder} to serializes an instance. However, it is
+   * not invoked in Beam pipelines.
+   */
+  Object writeReplace() throws ObjectStreamException {
+    return new SerializedForm(this);
+  }
+
+  /**
+   * Proxy for custom-serialization of {@link PendingDeposit}. This is necessary because the actual
+   * class to be (de)serialized is the generated AutoValue implementation. See also {@link
+   * #writeReplace}.
+   *
+   * <p>This class leverages {@link PendingDepositCoder} to safely deserializes an instance.
+   * However, it is not used in Beam pipelines.
+   */
+  private static class SerializedForm implements Serializable {
+
+    private static final long serialVersionUID = 3141095605225904433L;
+
+    private PendingDeposit value;
+
+    private SerializedForm(PendingDeposit value) {
+      this.value = value;
+    }
+
+    private void writeObject(ObjectOutputStream os) throws IOException {
+      checkState(value != null, "Non-null value expected for serialization.");
+      PendingDepositCoder.INSTANCE.encode(value, os);
+    }
+
+    private void readObject(ObjectInputStream is) throws IOException, ClassNotFoundException {
+      checkState(value == null, "Non-null value unexpected for deserialization.");
+      this.value = PendingDepositCoder.INSTANCE.decode(is);
+    }
+
+    @SuppressWarnings("unused")
+    private Object readResolve() throws ObjectStreamException {
+      return this.value;
+    }
+  }
+
   /**
    * A deterministic coder for {@link PendingDeposit} used during a GroupBy transform.
    *
-   * <p>We cannot use a {@link SerializableCoder} directly because it does not guarantee
-   * determinism, which is required by GroupBy.
+   * <p>We cannot use a {@code SerializableCoder} directly for two reasons: the default
+   * serialization does not guarantee determinism, which is required by GroupBy in Beam; and the
+   * default deserialization is not robust against deserialization-based attacks (See {@code
+   * SafeObjectInputStream} for more information).
    */
   public static class PendingDepositCoder extends AtomicCoder<PendingDeposit> {
 
@@ -117,10 +178,15 @@ public abstract class PendingDeposit implements Serializable {
     public void encode(PendingDeposit value, OutputStream outStream) throws IOException {
       BooleanCoder.of().encode(value.manual(), outStream);
       StringUtf8Coder.of().encode(value.tld(), outStream);
-      SerializableCoder.of(DateTime.class).encode(value.watermark(), outStream);
-      SerializableCoder.of(RdeMode.class).encode(value.mode(), outStream);
-      NullableCoder.of(SerializableCoder.of(CursorType.class)).encode(value.cursor(), outStream);
-      NullableCoder.of(SerializableCoder.of(Duration.class)).encode(value.interval(), outStream);
+      StringUtf8Coder.of().encode(value.watermark().toString(), outStream);
+      StringUtf8Coder.of().encode(value.mode().name(), outStream);
+      NullableCoder.of(StringUtf8Coder.of())
+          .encode(
+              Optional.ofNullable(value.cursor()).map(CursorType::name).orElse(null), outStream);
+      NullableCoder.of(StringUtf8Coder.of())
+          .encode(
+              Optional.ofNullable(value.interval()).map(Duration::toString).orElse(null),
+              outStream);
       NullableCoder.of(StringUtf8Coder.of()).encode(value.directoryWithTrailingSlash(), outStream);
       NullableCoder.of(VarIntCoder.of()).encode(value.revision(), outStream);
     }
@@ -130,10 +196,14 @@ public abstract class PendingDeposit implements Serializable {
       return new AutoValue_PendingDeposit(
           BooleanCoder.of().decode(inStream),
           StringUtf8Coder.of().decode(inStream),
-          SerializableCoder.of(DateTime.class).decode(inStream),
-          SerializableCoder.of(RdeMode.class).decode(inStream),
-          NullableCoder.of(SerializableCoder.of(CursorType.class)).decode(inStream),
-          NullableCoder.of(SerializableCoder.of(Duration.class)).decode(inStream),
+          DateTime.parse(StringUtf8Coder.of().decode(inStream)),
+          RdeMode.valueOf(StringUtf8Coder.of().decode(inStream)),
+          Optional.ofNullable(NullableCoder.of(StringUtf8Coder.of()).decode(inStream))
+              .map(CursorType::valueOf)
+              .orElse(null),
+          Optional.ofNullable(NullableCoder.of(StringUtf8Coder.of()).decode(inStream))
+              .map(Duration::parse)
+              .orElse(null),
           NullableCoder.of(StringUtf8Coder.of()).decode(inStream),
           NullableCoder.of(VarIntCoder.of()).decode(inStream));
     }

core/src/main/java/google/registry/xjc/JaxbFragment.java

@@ -1,105 +0,0 @@
-// Copyright 2017 The Nomulus Authors. All Rights Reserved.
-//
-// Licensed 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 google.registry.xjc;
-
-import static java.nio.charset.StandardCharsets.UTF_8;
-
-import google.registry.xml.XmlException;
-import java.io.ByteArrayInputStream;
-import java.io.ByteArrayOutputStream;
-import java.io.IOException;
-import java.io.ObjectInputStream;
-import java.io.ObjectOutputStream;
-import java.io.Serializable;
-
-/**
- * JAXB element wrapper for java object serialization.
- *
- * Instances of {@link JaxbFragment} wrap a non-serializable JAXB element instance, and provide
- * hooks into the java object serialization process that allow the elements to be safely
- * marshalled and unmarshalled using {@link ObjectOutputStream} and {@link ObjectInputStream},
- * respectively.
- */
-public class JaxbFragment<T> implements Serializable {
-
-  private static final long serialVersionUID = 5651243983008818813L;
-
-  private T instance;
-
-  /** Stores a JAXB element in a {@link JaxbFragment} */
-  public static <T> JaxbFragment<T> create(T object) {
-    JaxbFragment<T> fragment = new JaxbFragment<>();
-    fragment.instance = object;
-    return fragment;
-  }
-
-  /** Serializes a JAXB element into xml bytes. */
-  private static <T> byte[] freezeInstance(T instance) throws IOException {
-    try {
-      ByteArrayOutputStream bout = new ByteArrayOutputStream();
-      XjcXmlTransformer.marshalLenient(instance, bout, UTF_8);
-      return bout.toByteArray();
-    } catch (XmlException e) {
-      throw new IOException(e);
-    }
-  }
-
-  /** Deserializes a JAXB element from xml bytes. */
-  private static <T> T unfreezeInstance(byte[] instanceData, Class<T> instanceType)
-      throws IOException {
-    try {
-      ByteArrayInputStream bin = new ByteArrayInputStream(instanceData);
-      return XjcXmlTransformer.unmarshal(instanceType, bin);
-    } catch (XmlException e) {
-      throw new IOException(e);
-    }
-  }
-
-  /**
-   * Retrieves the JAXB element that is wrapped by this fragment.
-   */
-  public T getInstance() {
-    return instance;
-  }
-
-  @Override
-  public String toString() {
-    try {
-      return new String(freezeInstance(instance), UTF_8);
-    } catch (IOException e) {
-      throw new RuntimeException(e);
-    }
-  }
-
-  private void writeObject(ObjectOutputStream out) throws IOException {
-    // write instanceType, then instanceData
-    out.writeObject(instance.getClass());
-    out.writeObject(freezeInstance(instance));
-  }
-
-  @SuppressWarnings("unchecked")
-  private void readObject(ObjectInputStream in) throws IOException {
-    // read instanceType, then instanceData
-    Class<T> instanceType;
-    byte[] instanceData;
-    try {
-      instanceType = (Class<T>) in.readObject();
-      instanceData = (byte[]) in.readObject();
-    } catch (ClassNotFoundException e) {
-      throw new RuntimeException(e);
-    }
-    instance = unfreezeInstance(instanceData, instanceType);
-  }
-}

core/src/test/java/google/registry/rde/PendingDepositTest.java

@@ -0,0 +1,59 @@
+// Copyright 2023 The Nomulus Authors. All Rights Reserved.
+//
+// Licensed 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 google.registry.rde;
+
+import static com.google.common.truth.Truth.assertThat;
+import static google.registry.model.common.Cursor.CursorType.RDE_STAGING;
+import static google.registry.model.rde.RdeMode.FULL;
+import static google.registry.util.SafeSerializationUtils.safeDeserialize;
+import static google.registry.util.SerializeUtils.deserialize;
+import static google.registry.util.SerializeUtils.serialize;
+
+import org.joda.time.DateTime;
+import org.joda.time.Duration;
+import org.junit.jupiter.api.Test;
+
+/** Unit tests for {@link PendingDeposit}. */
+public class PendingDepositTest {
+  private final DateTime now = DateTime.parse("2000-01-01TZ");
+
+  PendingDeposit pendingDeposit =
+      PendingDeposit.create("soy", now, FULL, RDE_STAGING, Duration.standardDays(1));
+
+  PendingDeposit manualPendingDeposit =
+      PendingDeposit.createInManualOperation("soy", now, FULL, "/", null);
+
+  @Test
+  void deserialize_normalDeposit_success() {
+    assertThat(deserialize(PendingDeposit.class, serialize(pendingDeposit)))
+        .isEqualTo(pendingDeposit);
+  }
+
+  @Test
+  void deserialize_manualDeposit_success() {
+    assertThat(deserialize(PendingDeposit.class, serialize(manualPendingDeposit)))
+        .isEqualTo(manualPendingDeposit);
+  }
+
+  @Test
+  void safeDeserialize_normalDeposit_success() {
+    assertThat(safeDeserialize(serialize(pendingDeposit))).isEqualTo(pendingDeposit);
+  }
+
+  @Test
+  void safeDeserialize_manualDeposit_success() {
+    assertThat(safeDeserialize(serialize(manualPendingDeposit))).isEqualTo(manualPendingDeposit);
+  }
+}

core/src/test/java/google/registry/xjc/JaxbFragmentTest.java

@@ -1,56 +0,0 @@
-// Copyright 2017 The Nomulus Authors. All Rights Reserved.
-//
-// Licensed 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 google.registry.xjc;
-
-import static com.google.common.truth.Truth.assertThat;
-import static google.registry.testing.TestDataHelper.loadFile;
-import static java.nio.charset.StandardCharsets.UTF_8;
-
-import google.registry.xjc.rdehost.XjcRdeHostElement;
-import java.io.ByteArrayInputStream;
-import java.io.ByteArrayOutputStream;
-import java.io.InputStream;
-import java.io.ObjectInputStream;
-import java.io.ObjectOutputStream;
-import org.junit.jupiter.api.Test;
-
-/** Unit tests for {@link JaxbFragment}. */
-class JaxbFragmentTest {
-
-  private static final String HOST_FRAGMENT = loadFile(XjcObjectTest.class, "host_fragment.xml");
-
-  /** Verifies that a {@link JaxbFragment} can be serialized and deserialized successfully. */
-  @SuppressWarnings("unchecked")
-  @Test
-  void testJavaSerialization() throws Exception {
-    // Load rdeHost xml fragment into a jaxb object, wrap it, marshal, unmarshal, verify host.
-    // The resulting host name should be "ns1.example1.test", from the original xml fragment.
-    try (InputStream source = new ByteArrayInputStream(HOST_FRAGMENT.getBytes(UTF_8))) {
-      // Load xml
-      JaxbFragment<XjcRdeHostElement> hostFragment =
-          JaxbFragment.create(XjcXmlTransformer.unmarshal(XjcRdeHostElement.class, source));
-      // Marshal
-      ByteArrayOutputStream bout = new ByteArrayOutputStream();
-      new ObjectOutputStream(bout).writeObject(hostFragment);
-      // Unmarshal
-      ObjectInputStream in = new ObjectInputStream(new ByteArrayInputStream(bout.toByteArray()));
-      JaxbFragment<XjcRdeHostElement> restoredHostFragment =
-          (JaxbFragment<XjcRdeHostElement>) in.readObject();
-      // Verify host name
-      assertThat(restoredHostFragment.getInstance().getValue().getName())
-          .isEqualTo("ns1.example1.test");
-    }
-  }
-}