Add DS validation to match Cloud DNS (#1487)

* Add DS validation to match Cloud DNS * Add checks to flows * Add some flow tests * Add tests for DomainCreateFlow * Add tests for UpdateDomainCommand * Fix docs test * Small fixes * Remove builder from tests
2025-07-30 22:46:26 +02:00 · 2022-02-01 15:25:00 -05:00 · 2022-02-01 15:25:00 -05:00 · d5db6c16bc
commit d5db6c16bc
parent c1ad06afd1
18 changed files with 445 additions and 47 deletions
--- a/core/src/main/java/google/registry/flows/domain/DomainCreateFlow.java
+++ b/core/src/main/java/google/registry/flows/domain/DomainCreateFlow.java
@ -169,6 +169,7 @@ import org.joda.time.Duration;
 * @error {@link DomainFlowUtils.FeesMismatchException}
 * @error {@link DomainFlowUtils.FeesRequiredDuringEarlyAccessProgramException}
 * @error {@link DomainFlowUtils.FeesRequiredForPremiumNameException}
+ * @error {@link DomainFlowUtils.InvalidDsRecordException}
 * @error {@link DomainFlowUtils.InvalidIdnDomainLabelException}
 * @error {@link DomainFlowUtils.InvalidPunycodeException}
 * @error {@link DomainFlowUtils.InvalidTcnIdChecksumException}
--- a/core/src/main/java/google/registry/flows/domain/DomainFlowUtils.java
+++ b/core/src/main/java/google/registry/flows/domain/DomainFlowUtils.java
@ -129,6 +129,7 @@ import google.registry.model.tld.label.ReservedList;
 import google.registry.model.tmch.ClaimsListDao;
 import google.registry.persistence.VKey;
 import google.registry.tldconfig.idn.IdnLabelValidator;
+import google.registry.tools.DigestType;
 import google.registry.util.Idn;
 import java.math.BigDecimal;
 import java.util.Collection;
@ -144,6 +145,7 @@ import org.joda.money.CurrencyUnit;
 import org.joda.money.Money;
 import org.joda.time.DateTime;
 import org.joda.time.Duration;
+import org.xbill.DNS.DNSSEC.Algorithm;

 /** Static utility functions for domain flows. */
 public class DomainFlowUtils {
@ -293,13 +295,46 @@ public class DomainFlowUtils {

  /** Check that the DS data that will be set on a domain is valid. */
  static void validateDsData(Set<DelegationSignerData> dsData) throws EppException {
-    if (dsData != null && dsData.size() > MAX_DS_RECORDS_PER_DOMAIN) {
-      throw new TooManyDsRecordsException(
-          String.format(
-              "A maximum of %s DS records are allowed per domain.", MAX_DS_RECORDS_PER_DOMAIN));
+    if (dsData != null) {
+      if (dsData.size() > MAX_DS_RECORDS_PER_DOMAIN) {
+        throw new TooManyDsRecordsException(
+            String.format(
+                "A maximum of %s DS records are allowed per domain.", MAX_DS_RECORDS_PER_DOMAIN));
+      }
+      // TODO(sarahbot@): Add signature length verification
+      ImmutableList<DelegationSignerData> invalidAlgorithms =
+          dsData.stream()
+              .filter(ds -> !validateAlgorithm(ds.getAlgorithm()))
+              .collect(toImmutableList());
+      if (!invalidAlgorithms.isEmpty()) {
+        throw new InvalidDsRecordException(
+            String.format(
+                "Domain contains DS record(s) with an invalid algorithm wire value: %s",
+                invalidAlgorithms));
+      }
+      ImmutableList<DelegationSignerData> invalidDigestTypes =
+          dsData.stream()
+              .filter(ds -> !DigestType.fromWireValue(ds.getDigestType()).isPresent())
+              .collect(toImmutableList());
+      if (!invalidDigestTypes.isEmpty()) {
+        throw new InvalidDsRecordException(
+            String.format(
+                "Domain contains DS record(s) with an invalid digest type: %s",
+                invalidDigestTypes));
+      }
    }
  }

+  public static boolean validateAlgorithm(int alg) {
+    if (alg > 255 || alg < 0) {
+      return false;
+    }
+    // Algorithms that are reserved or unassigned will just return a string representation of their
+    // integer wire value.
+    String algorithm = Algorithm.string(alg);
+    return !algorithm.equals(Integer.toString(alg));
+  }
+
  /** We only allow specifying years in a period. */
  static Period verifyUnitIsYears(Period period) throws EppException {
    if (!checkNotNull(period).getUnit().equals(Period.Unit.YEARS)) {
@ -1217,6 +1252,13 @@ public class DomainFlowUtils {
    }
  }

+  /** Domain has an invalid DS record. */
+  static class InvalidDsRecordException extends ParameterValuePolicyErrorException {
+    public InvalidDsRecordException(String message) {
+      super(message);
+    }
+  }
+
  /** Domain name is under tld which doesn't exist. */
  static class TldDoesNotExistException extends ParameterValueRangeErrorException {
    public TldDoesNotExistException(String tld) {
--- a/core/src/main/java/google/registry/flows/domain/DomainUpdateFlow.java
+++ b/core/src/main/java/google/registry/flows/domain/DomainUpdateFlow.java
@ -114,6 +114,7 @@ import org.joda.time.DateTime;
 * @error {@link DomainFlowUtils.EmptySecDnsUpdateException}
 * @error {@link DomainFlowUtils.FeesMismatchException}
 * @error {@link DomainFlowUtils.FeesRequiredForNonFreeOperationException}
+ * @error {@link DomainFlowUtils.InvalidDsRecordException}
 * @error {@link DomainFlowUtils.LinkedResourcesDoNotExistException}
 * @error {@link DomainFlowUtils.LinkedResourceInPendingDeleteProhibitsOperationException}
 * @error {@link DomainFlowUtils.MaxSigLifeChangeNotSupportedException}
--- a/core/src/main/java/google/registry/tools/DigestType.java
+++ b/core/src/main/java/google/registry/tools/DigestType.java
@ -0,0 +1,58 @@
+// Copyright 2022 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.tools;
+
+import java.util.Optional;
+
+/**
+ * Enumerates the DNSSEC digest types for use with Delegation Signer records.
+ *
+ * <p>This also enforces the set of types that are valid for use with Cloud DNS. Customers cannot
+ * create DS records containing any other digest type.
+ *
+ * <p>The complete list can be found here:
+ * https://www.iana.org/assignments/ds-rr-types/ds-rr-types.xhtml
+ */
+public enum DigestType {
+  SHA1(1),
+  SHA256(2),
+  // Algorithm number 3 is GOST R 34.11-94 and is deliberately NOT SUPPORTED.
+  // This algorithm was reviewed by ise-crypto and deemed academically broken (b/207029800).
+  // In addition, RFC 8624 specifies that this algorithm MUST NOT be used for DNSSEC delegations.
+  // TODO(sarhabot@): Add note in Cloud DNS code to notify the Registry of any new changes to
+  // supported digest types.
+  SHA384(4);
+
+  private final int wireValue;
+
+  DigestType(int wireValue) {
+    this.wireValue = wireValue;
+  }
+
+  /** Fetches a DigestType enumeration constant by its IANA assigned value. */
+  public static Optional<DigestType> fromWireValue(int wireValue) {
+    for (DigestType alg : DigestType.values()) {
+      if (alg.getWireValue() == wireValue) {
+        return Optional.of(alg);
+      }
+    }
+    return Optional.empty();
+  }
+
+  /** Fetches a value in the range [0, 255] that encodes this DS digest type on the wire. */
+  public int getWireValue() {
+    return wireValue;
+  }
+}
--- a/core/src/main/java/google/registry/tools/DsRecord.java
+++ b/core/src/main/java/google/registry/tools/DsRecord.java
@ -16,6 +16,7 @@ package google.registry.tools;

 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.collect.ImmutableList.toImmutableList;
+import static google.registry.util.PreconditionsUtils.checkArgumentPresent;

 import com.beust.jcommander.IStringConverter;
 import com.google.auto.value.AutoValue;
@ -25,6 +26,7 @@ import com.google.common.base.Splitter;
 import com.google.common.io.BaseEncoding;
 import com.google.template.soy.data.SoyListData;
 import com.google.template.soy.data.SoyMapData;
+import google.registry.flows.domain.DomainFlowUtils;
 import java.util.List;

@AutoValue
@ -46,6 +48,15 @@ abstract class DsRecord {
        "digest should be even-lengthed hex, but is %s (length %s)",
        digest,
        digest.length());
+    checkArgumentPresent(
+        DigestType.fromWireValue(digestType),
+        String.format("DS record uses an unrecognized digest type: %d", digestType));
+
+    if (!DomainFlowUtils.validateAlgorithm(alg)) {
+      throw new IllegalArgumentException(
+          String.format("DS record uses an unrecognized algorithm: %d", alg));
+    }
+
    return new AutoValue_DsRecord(keyTag, alg, digestType, digest);
  }

--- a/core/src/test/java/google/registry/flows/domain/DomainCreateFlowTest.java
+++ b/core/src/test/java/google/registry/flows/domain/DomainCreateFlowTest.java
@ -106,6 +106,7 @@ import google.registry.flows.domain.DomainFlowUtils.FeeDescriptionParseException
 import google.registry.flows.domain.DomainFlowUtils.FeesMismatchException;
 import google.registry.flows.domain.DomainFlowUtils.FeesRequiredDuringEarlyAccessProgramException;
 import google.registry.flows.domain.DomainFlowUtils.FeesRequiredForPremiumNameException;
+import google.registry.flows.domain.DomainFlowUtils.InvalidDsRecordException;
 import google.registry.flows.domain.DomainFlowUtils.InvalidIdnDomainLabelException;
 import google.registry.flows.domain.DomainFlowUtils.InvalidPunycodeException;
 import google.registry.flows.domain.DomainFlowUtils.InvalidTcnIdChecksumException;
@ -1002,6 +1003,22 @@ class DomainCreateFlowTest extends ResourceFlowTestCase<DomainCreateFlow, Domain
    assertAboutEppExceptions().that(thrown).marshalsToXml();
  }

+  @TestOfyAndSql
+  void testFailure_secDnsInvalidDigestType() throws Exception {
+    setEppInput("domain_create_dsdata_bad_digest_types.xml");
+    persistContactsAndHosts();
+    EppException thrown = assertThrows(InvalidDsRecordException.class, this::runFlow);
+    assertAboutEppExceptions().that(thrown).marshalsToXml();
+  }
+
+  @TestOfyAndSql
+  void testFailure_secDnsInvalidAlgorithm() throws Exception {
+    setEppInput("domain_create_dsdata_bad_algorithms.xml");
+    persistContactsAndHosts();
+    EppException thrown = assertThrows(InvalidDsRecordException.class, this::runFlow);
+    assertAboutEppExceptions().that(thrown).marshalsToXml();
+  }
+
  @TestOfyAndSql
  void testFailure_wrongExtension() {
    setEppInput("domain_create_wrong_extension.xml");
--- a/core/src/test/java/google/registry/flows/domain/DomainUpdateFlowTest.java
+++ b/core/src/test/java/google/registry/flows/domain/DomainUpdateFlowTest.java
@ -72,6 +72,7 @@ import google.registry.flows.domain.DomainFlowUtils.DuplicateContactForRoleExcep
 import google.registry.flows.domain.DomainFlowUtils.EmptySecDnsUpdateException;
 import google.registry.flows.domain.DomainFlowUtils.FeesMismatchException;
 import google.registry.flows.domain.DomainFlowUtils.FeesRequiredForNonFreeOperationException;
+import google.registry.flows.domain.DomainFlowUtils.InvalidDsRecordException;
 import google.registry.flows.domain.DomainFlowUtils.LinkedResourceInPendingDeleteProhibitsOperationException;
 import google.registry.flows.domain.DomainFlowUtils.LinkedResourcesDoNotExistException;
 import google.registry.flows.domain.DomainFlowUtils.MaxSigLifeChangeNotSupportedException;
@ -122,7 +123,7 @@ import org.junit.jupiter.api.extension.RegisterExtension;
 class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow, DomainBase> {

  private static final DelegationSignerData SOME_DSDATA =
-      DelegationSignerData.create(1, 2, 3, base16().decode("0123"));
+      DelegationSignerData.create(1, 2, 2, base16().decode("0123"));
  private static final ImmutableMap<String, String> OTHER_DSDATA_TEMPLATE_MAP =
      ImmutableMap.of(
          "KEY_TAG", "12346",
@ -536,7 +537,7 @@ class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow, Domain
        "domain_update_dsdata_add.xml",
        ImmutableSet.of(SOME_DSDATA),
        ImmutableSet.of(SOME_DSDATA),
-        ImmutableMap.of("KEY_TAG", "1", "ALG", "2", "DIGEST_TYPE", "3", "DIGEST", "0123"));
+        ImmutableMap.of("KEY_TAG", "1", "ALG", "2", "DIGEST_TYPE", "2", "DIGEST", "0123"));
  }

  @TestOfyAndSql
@ -556,8 +557,8 @@ class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow, Domain
        "domain_update_dsdata_add.xml",
        ImmutableSet.of(SOME_DSDATA),
        ImmutableSet.of(
-            SOME_DSDATA, DelegationSignerData.create(12346, 2, 3, base16().decode("0123"))),
-        ImmutableMap.of("KEY_TAG", "12346", "ALG", "2", "DIGEST_TYPE", "3", "DIGEST", "0123"));
+            SOME_DSDATA, DelegationSignerData.create(12346, 2, 2, base16().decode("0123"))),
+        ImmutableMap.of("KEY_TAG", "12346", "ALG", "2", "DIGEST_TYPE", "2", "DIGEST", "0123"));
  }

  @TestOfyAndSql
@ -565,8 +566,8 @@ class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow, Domain
    doSecDnsSuccessfulTest(
        "domain_update_dsdata_add.xml",
        ImmutableSet.of(SOME_DSDATA),
-        ImmutableSet.of(SOME_DSDATA, DelegationSignerData.create(1, 8, 3, base16().decode("0123"))),
-        ImmutableMap.of("KEY_TAG", "1", "ALG", "8", "DIGEST_TYPE", "3", "DIGEST", "0123"));
+        ImmutableSet.of(SOME_DSDATA, DelegationSignerData.create(1, 8, 2, base16().decode("0123"))),
+        ImmutableMap.of("KEY_TAG", "1", "ALG", "8", "DIGEST_TYPE", "2", "DIGEST", "0123"));
  }

  @TestOfyAndSql
@ -583,15 +584,15 @@ class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow, Domain
    doSecDnsSuccessfulTest(
        "domain_update_dsdata_add.xml",
        ImmutableSet.of(SOME_DSDATA),
-        ImmutableSet.of(SOME_DSDATA, DelegationSignerData.create(1, 2, 3, base16().decode("4567"))),
-        ImmutableMap.of("KEY_TAG", "1", "ALG", "2", "DIGEST_TYPE", "3", "DIGEST", "4567"));
+        ImmutableSet.of(SOME_DSDATA, DelegationSignerData.create(1, 2, 2, base16().decode("4567"))),
+        ImmutableMap.of("KEY_TAG", "1", "ALG", "2", "DIGEST_TYPE", "2", "DIGEST", "4567"));
  }

  @TestOfyAndSql
  void testSuccess_secDnsAddToMaxRecords() throws Exception {
    ImmutableSet.Builder<DelegationSignerData> builder = new ImmutableSet.Builder<>();
    for (int i = 0; i < 7; ++i) {
-      builder.add(DelegationSignerData.create(i, 2, 3, new byte[] {0, 1, 2}));
+      builder.add(DelegationSignerData.create(i, 2, 2, new byte[] {0, 1, 2}));
    }
    ImmutableSet<DelegationSignerData> commonDsData = builder.build();

@ -643,7 +644,7 @@ class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow, Domain
  void testSuccess_secDnsAddRemoveToMaxRecords() throws Exception {
    ImmutableSet.Builder<DelegationSignerData> builder = new ImmutableSet.Builder<>();
    for (int i = 0; i < 7; ++i) {
-      builder.add(DelegationSignerData.create(i, 2, 3, new byte[] {0, 1, 2}));
+      builder.add(DelegationSignerData.create(i, 2, 2, new byte[] {0, 1, 2}));
    }
    ImmutableSet<DelegationSignerData> commonDsData = builder.build();

@ -813,11 +814,69 @@ class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow, Domain
        MaxSigLifeChangeNotSupportedException.class, "domain_update_maxsiglife.xml");
  }

+  @TestOfyAndSql
+  void testFailure_secDnsInvalidDigestType() throws Exception {
+    setEppInput("domain_update_dsdata_add.xml", OTHER_DSDATA_TEMPLATE_MAP);
+    persistResource(
+        newDomainBase(getUniqueIdFromCommand())
+            .asBuilder()
+            .setDsData(ImmutableSet.of(DelegationSignerData.create(1, 2, 3, new byte[] {0, 1, 2})))
+            .build());
+    EppException thrown = assertThrows(InvalidDsRecordException.class, this::runFlow);
+    assertAboutEppExceptions().that(thrown).marshalsToXml();
+  }
+
+  @TestOfyAndSql
+  void testFailure_secDnsMultipleInvalidDigestTypes() throws Exception {
+    setEppInput("domain_update_dsdata_add.xml", OTHER_DSDATA_TEMPLATE_MAP);
+    persistResource(
+        newDomainBase(getUniqueIdFromCommand())
+            .asBuilder()
+            .setDsData(
+                ImmutableSet.of(
+                    DelegationSignerData.create(1, 2, 3, new byte[] {0, 1, 2}),
+                    DelegationSignerData.create(2, 2, 6, new byte[] {0, 1, 2})))
+            .build());
+    EppException thrown = assertThrows(InvalidDsRecordException.class, this::runFlow);
+    assertThat(thrown).hasMessageThat().contains("digestType=3");
+    assertThat(thrown).hasMessageThat().contains("digestType=6");
+    assertAboutEppExceptions().that(thrown).marshalsToXml();
+  }
+
+  @TestOfyAndSql
+  void testFailure_secDnsInvalidAlgorithm() throws Exception {
+    setEppInput("domain_update_dsdata_add.xml", OTHER_DSDATA_TEMPLATE_MAP);
+    persistResource(
+        newDomainBase(getUniqueIdFromCommand())
+            .asBuilder()
+            .setDsData(ImmutableSet.of(DelegationSignerData.create(1, 99, 2, new byte[] {0, 1, 2})))
+            .build());
+    EppException thrown = assertThrows(InvalidDsRecordException.class, this::runFlow);
+    assertAboutEppExceptions().that(thrown).marshalsToXml();
+  }
+
+  @TestOfyAndSql
+  void testFailure_secDnsMultipleInvalidAlgorithms() throws Exception {
+    setEppInput("domain_update_dsdata_add.xml", OTHER_DSDATA_TEMPLATE_MAP);
+    persistResource(
+        newDomainBase(getUniqueIdFromCommand())
+            .asBuilder()
+            .setDsData(
+                ImmutableSet.of(
+                    DelegationSignerData.create(1, 998, 2, new byte[] {0, 1, 2}),
+                    DelegationSignerData.create(2, 99, 2, new byte[] {0, 1, 2})))
+            .build());
+    EppException thrown = assertThrows(InvalidDsRecordException.class, this::runFlow);
+    assertThat(thrown).hasMessageThat().contains("algorithm=998");
+    assertThat(thrown).hasMessageThat().contains("algorithm=99");
+    assertAboutEppExceptions().that(thrown).marshalsToXml();
+  }
+
  @TestOfyAndSql
  void testFailure_secDnsTooManyDsRecords() throws Exception {
    ImmutableSet.Builder<DelegationSignerData> builder = new ImmutableSet.Builder<>();
    for (int i = 0; i < 8; ++i) {
-      builder.add(DelegationSignerData.create(i, 2, 3, new byte[] {0, 1, 2}));
+      builder.add(DelegationSignerData.create(i, 2, 2, new byte[] {0, 1, 2}));
    }

    setEppInput("domain_update_dsdata_add.xml", OTHER_DSDATA_TEMPLATE_MAP);
--- a/core/src/test/java/google/registry/tools/CreateDomainCommandTest.java
+++ b/core/src/test/java/google/registry/tools/CreateDomainCommandTest.java
@ -50,7 +50,7 @@ class CreateDomainCommandTest extends EppToolCommandTestCase<CreateDomainCommand
        "--admins=crr-admin",
        "--techs=crr-tech",
        "--password=2fooBAR",
-        "--ds_records=1 2 3 abcd,4 5 6 EF01",
+        "--ds_records=1 2 2 abcd,4 5 1 EF01",
        "--ds_records=60485 5  2  D4B7D520E7BB5F0F67674A0CCEB1E3E0614B93C4F9E99B8383F6A1E4469DA50A",
        "example.tld");
    eppVerifier.verifySent("domain_create_complete.xml");
@ -66,7 +66,7 @@ class CreateDomainCommandTest extends EppToolCommandTestCase<CreateDomainCommand
        "--admins=crr-admin",
        "--techs=crr-tech",
        "--password=2fooBAR",
-        "--ds_records=1 2 3 abcd,4 5 6 EF01",
+        "--ds_records=1 2 2 abcd,4 5 1 EF01",
        "--ds_records=60485 5  2  D4B7D520E7BB5F0F67674A0CCEB1E3E0614B93C4F9E99B8383F6A1E4469DA50A",
        "example.tld");
    eppVerifier.verifySent("domain_create_complete.xml");
@ -314,6 +314,38 @@ class CreateDomainCommandTest extends EppToolCommandTestCase<CreateDomainCommand
    assertThat(thrown).hasMessageThat().contains("--period");
  }

+  @Test
+  void testFailure_invalidDigestType() {
+    IllegalArgumentException thrown =
+        assertThrows(
+            IllegalArgumentException.class,
+            () ->
+                runCommandForced(
+                    "--client=NewRegistrar",
+                    "--registrant=crr-admin",
+                    "--admins=crr-admin",
+                    "--techs=crr-tech",
+                    "--ds_records=1 2 3 abcd",
+                    "example.tld"));
+    assertThat(thrown).hasMessageThat().isEqualTo("DS record uses an unrecognized digest type: 3");
+  }
+
+  @Test
+  void testFailure_invalidAlgorithm() {
+    IllegalArgumentException thrown =
+        assertThrows(
+            IllegalArgumentException.class,
+            () ->
+                runCommandForced(
+                    "--client=NewRegistrar",
+                    "--registrant=crr-admin",
+                    "--admins=crr-admin",
+                    "--techs=crr-tech",
+                    "--ds_records=1 999 4 abcd",
+                    "example.tld"));
+    assertThat(thrown).hasMessageThat().isEqualTo("DS record uses an unrecognized algorithm: 999");
+  }
+
  @Test
  void testFailure_dsRecordsNot4Parts() {
    IllegalArgumentException thrown =
--- a/core/src/test/java/google/registry/tools/UpdateDomainCommandTest.java
+++ b/core/src/test/java/google/registry/tools/UpdateDomainCommandTest.java
@ -86,12 +86,12 @@ class UpdateDomainCommandTest extends EppToolCommandTestCase<UpdateDomainCommand
        "--add_admins=crr-admin2",
        "--add_techs=crr-tech2",
        "--add_statuses=serverDeleteProhibited",
-        "--add_ds_records=1 2 3 abcd,4 5 6 EF01",
+        "--add_ds_records=1 2 2 abcd,4 5 1 EF01",
        "--remove_nameservers=ns3.zdns.google,ns4.zdns.google",
        "--remove_admins=crr-admin1",
        "--remove_techs=crr-tech1",
        "--remove_statuses=serverHold",
-        "--remove_ds_records=7 8 9 12ab,6 5 4 34CD",
+        "--remove_ds_records=7 8 1 12ab,6 5 4 34CD",
        "--registrant=crr-admin",
        "--password=2fooBAR",
        "example.tld");
@ -106,12 +106,12 @@ class UpdateDomainCommandTest extends EppToolCommandTestCase<UpdateDomainCommand
        "--add_admins=crr-admin2",
        "--add_techs=crr-tech2",
        "--add_statuses=serverDeleteProhibited",
-        "--add_ds_records=1 2 3 abcd,4 5 6 EF01",
+        "--add_ds_records=1 2 2 abcd,4 5 1 EF01",
        "--remove_nameservers=ns[3-4].zdns.google",
        "--remove_admins=crr-admin1",
        "--remove_techs=crr-tech1",
        "--remove_statuses=serverHold",
-        "--remove_ds_records=7 8 9 12ab,6 5 4 34CD",
+        "--remove_ds_records=7 8 1 12ab,6 5 4 34CD",
        "--registrant=crr-admin",
        "--password=2fooBAR",
        "example.tld");
@ -128,12 +128,12 @@ class UpdateDomainCommandTest extends EppToolCommandTestCase<UpdateDomainCommand
        "--add_admins=crr-admin2",
        "--add_techs=crr-tech2",
        "--add_statuses=serverDeleteProhibited",
-        "--add_ds_records=1 2 3 abcd,4 5 6 EF01",
+        "--add_ds_records=1 2 2 abcd,4 5 1 EF01",
        "--remove_nameservers=ns[3-4].zdns.google",
        "--remove_admins=crr-admin1",
        "--remove_techs=crr-tech1",
        "--remove_statuses=serverHold",
-        "--remove_ds_records=7 8 9 12ab,6 5 4 34CD",
+        "--remove_ds_records=7 8 1 12ab,6 5 4 34CD",
        "--registrant=crr-admin",
        "--password=2fooBAR",
        "example.tld",
@ -186,7 +186,7 @@ class UpdateDomainCommandTest extends EppToolCommandTestCase<UpdateDomainCommand
        "--add_admins=crr-admin2",
        "--add_techs=crr-tech2",
        "--add_statuses=serverDeleteProhibited",
-        "--add_ds_records=1 2 3 abcd,4 5 6 EF01",
+        "--add_ds_records=1 2 2 abcd,4 5 1 EF01",
        "example.tld");
    eppVerifier.verifySent("domain_update_add.xml");
  }
@ -199,7 +199,7 @@ class UpdateDomainCommandTest extends EppToolCommandTestCase<UpdateDomainCommand
        "--remove_admins=crr-admin1",
        "--remove_techs=crr-tech1",
        "--remove_statuses=serverHold",
-        "--remove_ds_records=7 8 9 12ab,6 5 4 34CD",
+        "--remove_ds_records=7 8 1 12ab,6 5 4 34CD",
        "example.tld");
    eppVerifier.verifySent("domain_update_remove.xml");
  }
@ -277,8 +277,7 @@ class UpdateDomainCommandTest extends EppToolCommandTestCase<UpdateDomainCommand

  @TestOfyAndSql
  void testSuccess_setDsRecords() throws Exception {
-    runCommandForced(
-        "--client=NewRegistrar", "--ds_records=1 2 3 abcd,4 5 6 EF01", "example.tld");
+    runCommandForced("--client=NewRegistrar", "--ds_records=1 2 2 abcd,4 5 1 EF01", "example.tld");
    eppVerifier.verifySent("domain_update_set_ds_records.xml");
  }

@ -286,7 +285,7 @@ class UpdateDomainCommandTest extends EppToolCommandTestCase<UpdateDomainCommand
  void testSuccess_setDsRecords_withUnneededClear() throws Exception {
    runCommandForced(
        "--client=NewRegistrar",
-        "--ds_records=1 2 3 abcd,4 5 6 EF01",
+        "--ds_records=1 2 2 abcd,4 5 1 EF01",
        "--clear_ds_records",
        "example.tld");
    eppVerifier.verifySent("domain_update_set_ds_records.xml");
@ -630,6 +629,28 @@ class UpdateDomainCommandTest extends EppToolCommandTestCase<UpdateDomainCommand
                + "you cannot use the add_statuses and remove_statuses flags.");
  }

+  @TestOfyAndSql
+  void testFailure_invalidDsRecordAlgorithm() {
+    IllegalArgumentException thrown =
+        assertThrows(
+            IllegalArgumentException.class,
+            () ->
+                runCommandForced(
+                    "--client=NewRegistrar", "--add_ds_records=1 299 2 abcd", "example.tld"));
+    assertThat(thrown).hasMessageThat().isEqualTo("DS record uses an unrecognized algorithm: 299");
+  }
+
+  @TestOfyAndSql
+  void testFailure_invalidDsRecordDigestType() {
+    IllegalArgumentException thrown =
+        assertThrows(
+            IllegalArgumentException.class,
+            () ->
+                runCommandForced(
+                    "--client=NewRegistrar", "--add_ds_records=1 2 3 abcd", "example.tld"));
+    assertThat(thrown).hasMessageThat().isEqualTo("DS record uses an unrecognized digest type: 3");
+  }
+
  @TestOfyAndSql
  void testFailure_provideDsRecordsAndAddDsRecords() {
    IllegalArgumentException thrown =
@ -638,8 +659,8 @@ class UpdateDomainCommandTest extends EppToolCommandTestCase<UpdateDomainCommand
            () ->
                runCommandForced(
                    "--client=NewRegistrar",
-                    "--add_ds_records=1 2 3 abcd",
-                    "--ds_records=4 5 6 EF01",
+                    "--add_ds_records=1 2 2 abcd",
+                    "--ds_records=4 5 1 EF01",
                    "example.tld"));
    assertThat(thrown)
        .hasMessageThat()
@ -656,8 +677,8 @@ class UpdateDomainCommandTest extends EppToolCommandTestCase<UpdateDomainCommand
            () ->
                runCommandForced(
                    "--client=NewRegistrar",
-                    "--remove_ds_records=7 8 9 12ab",
-                    "--ds_records=4 5 6 EF01",
+                    "--remove_ds_records=7 8 1 12ab",
+                    "--ds_records=4 5 1 EF01",
                    "example.tld"));
    assertThat(thrown)
        .hasMessageThat()
@ -674,7 +695,7 @@ class UpdateDomainCommandTest extends EppToolCommandTestCase<UpdateDomainCommand
            () ->
                runCommandForced(
                    "--client=NewRegistrar",
-                    "--add_ds_records=1 2 3 abcd",
+                    "--add_ds_records=1 2 2 abcd",
                    "--clear_ds_records",
                    "example.tld"));
    assertThat(thrown)
@ -692,7 +713,7 @@ class UpdateDomainCommandTest extends EppToolCommandTestCase<UpdateDomainCommand
            () ->
                runCommandForced(
                    "--client=NewRegistrar",
-                    "--remove_ds_records=7 8 9 12ab",
+                    "--remove_ds_records=7 8 1 12ab",
                    "--clear_ds_records",
                    "example.tld"));
    assertThat(thrown)
--- a/core/src/test/resources/google/registry/flows/domain/domain_create_dsdata_bad_algorithms.xml
+++ b/core/src/test/resources/google/registry/flows/domain/domain_create_dsdata_bad_algorithms.xml
@ -0,0 +1,77 @@
+<?xml version="1.0" encoding="UTF-8" standalone="no"?>
+<epp xmlns="urn:ietf:params:xml:ns:epp-1.0"
+    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance">
+  <command>
+    <create>
+      <domain:create
+          xmlns:domain="urn:ietf:params:xml:ns:domain-1.0">
+        <domain:name>example.tld</domain:name>
+        <domain:period unit="y">2</domain:period>
+        <domain:ns>
+          <domain:hostObj>ns1.example.net</domain:hostObj>
+          <domain:hostObj>ns2.example.net</domain:hostObj>
+        </domain:ns>
+        <domain:registrant>jd1234</domain:registrant>
+        <domain:contact type="admin">sh8013</domain:contact>
+        <domain:contact type="tech">sh8013</domain:contact>
+        <domain:authInfo>
+          <domain:pw>2fooBAR</domain:pw>
+        </domain:authInfo>
+      </domain:create>
+    </create>
+    <extension>
+      <secDNS:create
+          xmlns:secDNS="urn:ietf:params:xml:ns:secDNS-1.1">
+        <secDNS:dsData>
+          <secDNS:keyTag>12345</secDNS:keyTag>
+          <secDNS:alg>99</secDNS:alg>
+          <secDNS:digestType>1</secDNS:digestType>
+          <secDNS:digest>49FD46E6C4B45C55D4AC</secDNS:digest>
+        </secDNS:dsData>
+        <secDNS:dsData>
+          <secDNS:keyTag>12346</secDNS:keyTag>
+          <secDNS:alg>3</secDNS:alg>
+          <secDNS:digestType>1</secDNS:digestType>
+          <secDNS:digest>49FD46E6C4B45C55D4AC</secDNS:digest>
+        </secDNS:dsData>
+        <secDNS:dsData>
+          <secDNS:keyTag>12347</secDNS:keyTag>
+          <secDNS:alg>3</secDNS:alg>
+          <secDNS:digestType>1</secDNS:digestType>
+          <secDNS:digest>49FD46E6C4B45C55D4AC</secDNS:digest>
+        </secDNS:dsData>
+        <secDNS:dsData>
+          <secDNS:keyTag>12348</secDNS:keyTag>
+          <secDNS:alg>3</secDNS:alg>
+          <secDNS:digestType>1</secDNS:digestType>
+          <secDNS:digest>49FD46E6C4B45C55D4AC</secDNS:digest>
+        </secDNS:dsData>
+        <secDNS:dsData>
+          <secDNS:keyTag>12349</secDNS:keyTag>
+          <secDNS:alg>98</secDNS:alg>
+          <secDNS:digestType>1</secDNS:digestType>
+          <secDNS:digest>49FD46E6C4B45C55D4AC</secDNS:digest>
+        </secDNS:dsData>
+        <secDNS:dsData>
+          <secDNS:keyTag>12350</secDNS:keyTag>
+          <secDNS:alg>3</secDNS:alg>
+          <secDNS:digestType>1</secDNS:digestType>
+          <secDNS:digest>49FD46E6C4B45C55D4AC</secDNS:digest>
+        </secDNS:dsData>
+        <secDNS:dsData>
+          <secDNS:keyTag>12351</secDNS:keyTag>
+          <secDNS:alg>3</secDNS:alg>
+          <secDNS:digestType>1</secDNS:digestType>
+          <secDNS:digest>49FD46E6C4B45C55D4AC</secDNS:digest>
+        </secDNS:dsData>
+        <secDNS:dsData>
+          <secDNS:keyTag>12352</secDNS:keyTag>
+          <secDNS:alg>3</secDNS:alg>
+          <secDNS:digestType>1</secDNS:digestType>
+          <secDNS:digest>49FD46E6C4B45C55D4AC</secDNS:digest>
+        </secDNS:dsData>
+      </secDNS:create>
+    </extension>
+    <clTRID>ABC-12345</clTRID>
+  </command>
+</epp>
--- a/core/src/test/resources/google/registry/flows/domain/domain_create_dsdata_bad_digest_types.xml
+++ b/core/src/test/resources/google/registry/flows/domain/domain_create_dsdata_bad_digest_types.xml
@ -0,0 +1,77 @@
+<?xml version="1.0" encoding="UTF-8" standalone="no"?>
+<epp xmlns="urn:ietf:params:xml:ns:epp-1.0"
+    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance">
+  <command>
+    <create>
+      <domain:create
+          xmlns:domain="urn:ietf:params:xml:ns:domain-1.0">
+        <domain:name>example.tld</domain:name>
+        <domain:period unit="y">2</domain:period>
+        <domain:ns>
+          <domain:hostObj>ns1.example.net</domain:hostObj>
+          <domain:hostObj>ns2.example.net</domain:hostObj>
+        </domain:ns>
+        <domain:registrant>jd1234</domain:registrant>
+        <domain:contact type="admin">sh8013</domain:contact>
+        <domain:contact type="tech">sh8013</domain:contact>
+        <domain:authInfo>
+          <domain:pw>2fooBAR</domain:pw>
+        </domain:authInfo>
+      </domain:create>
+    </create>
+    <extension>
+      <secDNS:create
+          xmlns:secDNS="urn:ietf:params:xml:ns:secDNS-1.1">
+        <secDNS:dsData>
+          <secDNS:keyTag>12345</secDNS:keyTag>
+          <secDNS:alg>3</secDNS:alg>
+          <secDNS:digestType>100</secDNS:digestType>
+          <secDNS:digest>49FD46E6C4B45C55D4AC</secDNS:digest>
+        </secDNS:dsData>
+        <secDNS:dsData>
+          <secDNS:keyTag>12346</secDNS:keyTag>
+          <secDNS:alg>3</secDNS:alg>
+          <secDNS:digestType>3</secDNS:digestType>
+          <secDNS:digest>49FD46E6C4B45C55D4AC</secDNS:digest>
+        </secDNS:dsData>
+        <secDNS:dsData>
+          <secDNS:keyTag>12347</secDNS:keyTag>
+          <secDNS:alg>3</secDNS:alg>
+          <secDNS:digestType>1</secDNS:digestType>
+          <secDNS:digest>49FD46E6C4B45C55D4AC</secDNS:digest>
+        </secDNS:dsData>
+        <secDNS:dsData>
+          <secDNS:keyTag>12348</secDNS:keyTag>
+          <secDNS:alg>3</secDNS:alg>
+          <secDNS:digestType>1</secDNS:digestType>
+          <secDNS:digest>49FD46E6C4B45C55D4AC</secDNS:digest>
+        </secDNS:dsData>
+        <secDNS:dsData>
+          <secDNS:keyTag>12349</secDNS:keyTag>
+          <secDNS:alg>3</secDNS:alg>
+          <secDNS:digestType>1</secDNS:digestType>
+          <secDNS:digest>49FD46E6C4B45C55D4AC</secDNS:digest>
+        </secDNS:dsData>
+        <secDNS:dsData>
+          <secDNS:keyTag>12350</secDNS:keyTag>
+          <secDNS:alg>3</secDNS:alg>
+          <secDNS:digestType>1</secDNS:digestType>
+          <secDNS:digest>49FD46E6C4B45C55D4AC</secDNS:digest>
+        </secDNS:dsData>
+        <secDNS:dsData>
+          <secDNS:keyTag>12351</secDNS:keyTag>
+          <secDNS:alg>3</secDNS:alg>
+          <secDNS:digestType>1</secDNS:digestType>
+          <secDNS:digest>49FD46E6C4B45C55D4AC</secDNS:digest>
+        </secDNS:dsData>
+        <secDNS:dsData>
+          <secDNS:keyTag>12352</secDNS:keyTag>
+          <secDNS:alg>3</secDNS:alg>
+          <secDNS:digestType>1</secDNS:digestType>
+          <secDNS:digest>49FD46E6C4B45C55D4AC</secDNS:digest>
+        </secDNS:dsData>
+      </secDNS:create>
+    </extension>
+    <clTRID>ABC-12345</clTRID>
+  </command>
+</epp>
--- a/core/src/test/resources/google/registry/tools/server/domain_create_complete.xml
+++ b/core/src/test/resources/google/registry/tools/server/domain_create_complete.xml
@ -25,13 +25,13 @@
        <secDNS:dsData>
          <secDNS:keyTag>1</secDNS:keyTag>
          <secDNS:alg>2</secDNS:alg>
-          <secDNS:digestType>3</secDNS:digestType>
+          <secDNS:digestType>2</secDNS:digestType>
          <secDNS:digest>ABCD</secDNS:digest>
        </secDNS:dsData>
        <secDNS:dsData>
          <secDNS:keyTag>4</secDNS:keyTag>
          <secDNS:alg>5</secDNS:alg>
-          <secDNS:digestType>6</secDNS:digestType>
+          <secDNS:digestType>1</secDNS:digestType>
          <secDNS:digest>EF01</secDNS:digest>
        </secDNS:dsData>
        <secDNS:dsData>
--- a/core/src/test/resources/google/registry/tools/server/domain_update_add.xml
+++ b/core/src/test/resources/google/registry/tools/server/domain_update_add.xml
@ -22,13 +22,13 @@
          <secDNS:dsData>
            <secDNS:keyTag>1</secDNS:keyTag>
            <secDNS:alg>2</secDNS:alg>
-            <secDNS:digestType>3</secDNS:digestType>
+            <secDNS:digestType>2</secDNS:digestType>
            <secDNS:digest>ABCD</secDNS:digest>
          </secDNS:dsData>
          <secDNS:dsData>
            <secDNS:keyTag>4</secDNS:keyTag>
            <secDNS:alg>5</secDNS:alg>
-            <secDNS:digestType>6</secDNS:digestType>
+            <secDNS:digestType>1</secDNS:digestType>
            <secDNS:digest>EF01</secDNS:digest>
          </secDNS:dsData>
        </secDNS:add>
--- a/core/src/test/resources/google/registry/tools/server/domain_update_complete.xml
+++ b/core/src/test/resources/google/registry/tools/server/domain_update_complete.xml
@ -37,7 +37,7 @@
          <secDNS:dsData>
            <secDNS:keyTag>7</secDNS:keyTag>
            <secDNS:alg>8</secDNS:alg>
-            <secDNS:digestType>9</secDNS:digestType>
+            <secDNS:digestType>1</secDNS:digestType>
            <secDNS:digest>12AB</secDNS:digest>
          </secDNS:dsData>
          <secDNS:dsData>
@ -51,13 +51,13 @@
          <secDNS:dsData>
            <secDNS:keyTag>1</secDNS:keyTag>
            <secDNS:alg>2</secDNS:alg>
-            <secDNS:digestType>3</secDNS:digestType>
+            <secDNS:digestType>2</secDNS:digestType>
            <secDNS:digest>ABCD</secDNS:digest>
          </secDNS:dsData>
          <secDNS:dsData>
            <secDNS:keyTag>4</secDNS:keyTag>
            <secDNS:alg>5</secDNS:alg>
-            <secDNS:digestType>6</secDNS:digestType>
+            <secDNS:digestType>1</secDNS:digestType>
            <secDNS:digest>EF01</secDNS:digest>
          </secDNS:dsData>
        </secDNS:add>
--- a/core/src/test/resources/google/registry/tools/server/domain_update_complete_abc.xml
+++ b/core/src/test/resources/google/registry/tools/server/domain_update_complete_abc.xml
@ -37,7 +37,7 @@
          <secDNS:dsData>
            <secDNS:keyTag>7</secDNS:keyTag>
            <secDNS:alg>8</secDNS:alg>
-            <secDNS:digestType>9</secDNS:digestType>
+            <secDNS:digestType>1</secDNS:digestType>
            <secDNS:digest>12AB</secDNS:digest>
          </secDNS:dsData>
          <secDNS:dsData>
@ -51,13 +51,13 @@
          <secDNS:dsData>
            <secDNS:keyTag>1</secDNS:keyTag>
            <secDNS:alg>2</secDNS:alg>
-            <secDNS:digestType>3</secDNS:digestType>
+            <secDNS:digestType>2</secDNS:digestType>
            <secDNS:digest>ABCD</secDNS:digest>
          </secDNS:dsData>
          <secDNS:dsData>
            <secDNS:keyTag>4</secDNS:keyTag>
            <secDNS:alg>5</secDNS:alg>
-            <secDNS:digestType>6</secDNS:digestType>
+            <secDNS:digestType>1</secDNS:digestType>
            <secDNS:digest>EF01</secDNS:digest>
          </secDNS:dsData>
        </secDNS:add>
--- a/core/src/test/resources/google/registry/tools/server/domain_update_remove.xml
+++ b/core/src/test/resources/google/registry/tools/server/domain_update_remove.xml
@ -21,7 +21,7 @@
          <secDNS:dsData>
            <secDNS:keyTag>7</secDNS:keyTag>
            <secDNS:alg>8</secDNS:alg>
-            <secDNS:digestType>9</secDNS:digestType>
+            <secDNS:digestType>1</secDNS:digestType>
            <secDNS:digest>12AB</secDNS:digest>
          </secDNS:dsData>
          <secDNS:dsData>
--- a/core/src/test/resources/google/registry/tools/server/domain_update_set_ds_records.xml
+++ b/core/src/test/resources/google/registry/tools/server/domain_update_set_ds_records.xml
@ -16,13 +16,13 @@
          <secDNS:dsData>
            <secDNS:keyTag>1</secDNS:keyTag>
            <secDNS:alg>2</secDNS:alg>
-            <secDNS:digestType>3</secDNS:digestType>
+            <secDNS:digestType>2</secDNS:digestType>
            <secDNS:digest>ABCD</secDNS:digest>
          </secDNS:dsData>
          <secDNS:dsData>
            <secDNS:keyTag>4</secDNS:keyTag>
            <secDNS:alg>5</secDNS:alg>
-            <secDNS:digestType>6</secDNS:digestType>
+            <secDNS:digestType>1</secDNS:digestType>
            <secDNS:digest>EF01</secDNS:digest>
          </secDNS:dsData>
        </secDNS:add>
--- a/docs/flows.md
+++ b/docs/flows.md
@ -398,6 +398,7 @@ An EPP flow that creates a new domain resource.
    *   The fee description passed in the transform command matches multiple fee
        types.
    *   The fee description passed in the transform command cannot be parsed.
+    *   Domain has an invalid DS record.
    *   Domain name starts with xn-- but is not a valid IDN.
    *   The specified trademark validator is not supported.
    *   Domain labels cannot begin with a dash.
@ -810,6 +811,7 @@ statuses are updated at once.
 *   2306
    *   Cannot add and remove the same value.
    *   More than one contact for a given role is not allowed.
+    *   Domain has an invalid DS record.
    *   Missing type attribute for contact.
    *   The secDNS:all element must have value 'true' if present.
    *   Too many DS records set on a domain.