diff --git a/core/src/main/java/google/registry/flows/domain/DomainFlowUtils.java b/core/src/main/java/google/registry/flows/domain/DomainFlowUtils.java index 1efab8370..5857d398e 100644 --- a/core/src/main/java/google/registry/flows/domain/DomainFlowUtils.java +++ b/core/src/main/java/google/registry/flows/domain/DomainFlowUtils.java @@ -301,7 +301,6 @@ public class DomainFlowUtils { String.format( "A maximum of %s DS records are allowed per domain.", MAX_DS_RECORDS_PER_DOMAIN)); } - // TODO(sarahbot@): Add signature length verification ImmutableList invalidAlgorithms = dsData.stream() .filter(ds -> !validateAlgorithm(ds.getAlgorithm())) @@ -322,6 +321,20 @@ public class DomainFlowUtils { "Domain contains DS record(s) with an invalid digest type: %s", invalidDigestTypes)); } + ImmutableList digestsWithInvalidDigestLength = + dsData.stream() + .filter( + ds -> + DigestType.fromWireValue(ds.getDigestType()).isPresent() + && (ds.getDigest().length + != DigestType.fromWireValue(ds.getDigestType()).get().getBytes())) + .collect(toImmutableList()); + if (!digestsWithInvalidDigestLength.isEmpty()) { + throw new InvalidDsRecordException( + String.format( + "Domain contains DS record(s) with an invalid digest length: %s", + digestsWithInvalidDigestLength)); + } } } diff --git a/core/src/main/java/google/registry/tools/DigestType.java b/core/src/main/java/google/registry/tools/DigestType.java index 251399461..9fab50054 100644 --- a/core/src/main/java/google/registry/tools/DigestType.java +++ b/core/src/main/java/google/registry/tools/DigestType.java @@ -26,19 +26,21 @@ import java.util.Optional; * https://www.iana.org/assignments/ds-rr-types/ds-rr-types.xhtml */ public enum DigestType { - SHA1(1), - SHA256(2), + SHA1(1, 20), + SHA256(2, 32), // 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); + SHA384(4, 48); private final int wireValue; + private final int bytes; - DigestType(int wireValue) { + DigestType(int wireValue, int bytes) { this.wireValue = wireValue; + this.bytes = bytes; } /** Fetches a DigestType enumeration constant by its IANA assigned value. */ @@ -55,4 +57,9 @@ public enum DigestType { public int getWireValue() { return wireValue; } + + /** Returns the expected length in bytes of the signature. */ + public int getBytes() { + return bytes; + } } diff --git a/core/src/main/java/google/registry/tools/DsRecord.java b/core/src/main/java/google/registry/tools/DsRecord.java index fb619d51f..10e594940 100644 --- a/core/src/main/java/google/registry/tools/DsRecord.java +++ b/core/src/main/java/google/registry/tools/DsRecord.java @@ -51,6 +51,11 @@ abstract class DsRecord { checkArgumentPresent( DigestType.fromWireValue(digestType), String.format("DS record uses an unrecognized digest type: %d", digestType)); + if (DigestType.fromWireValue(digestType).get().getBytes() + != BaseEncoding.base16().decode(digest).length) { + throw new IllegalArgumentException( + String.format("DS record has an invalid digest length: %s", digest)); + } if (!DomainFlowUtils.validateAlgorithm(alg)) { throw new IllegalArgumentException( diff --git a/core/src/test/java/google/registry/flows/domain/DomainCreateFlowTest.java b/core/src/test/java/google/registry/flows/domain/DomainCreateFlowTest.java index 3738c7558..f9de9fc12 100644 --- a/core/src/test/java/google/registry/flows/domain/DomainCreateFlowTest.java +++ b/core/src/test/java/google/registry/flows/domain/DomainCreateFlowTest.java @@ -856,7 +856,8 @@ class DomainCreateFlowTest extends ResourceFlowTestCase { private static final DelegationSignerData SOME_DSDATA = - DelegationSignerData.create(1, 2, 2, base16().decode("0123")); + DelegationSignerData.create( + 1, + 2, + 2, + base16().decode("9F86D081884C7D659A2FEAA0C55AD015A3BF4F1B2B0B822CD15D6C15B0F00A08")); private static final ImmutableMap OTHER_DSDATA_TEMPLATE_MAP = ImmutableMap.of( "KEY_TAG", "12346", "ALG", "3", "DIGEST_TYPE", "1", - "DIGEST", "38EC35D5B3A34B44C39B"); + "DIGEST", "A94A8FE5CCB19BA61C4C0873D391E987982FBBD3"); private ContactResource sh8013Contact; private ContactResource mak21Contact; @@ -523,9 +527,17 @@ class DomainUpdateFlowTest extends ResourceFlowTestCase builder = new ImmutableSet.Builder<>(); for (int i = 0; i < 7; ++i) { - builder.add(DelegationSignerData.create(i, 2, 2, new byte[] {0, 1, 2})); + builder.add( + DelegationSignerData.create( + i, + 2, + 2, + base16().decode("9F86D081884C7D659A2FEAA0C55AD015A3BF4F1B2B0B822CD15D6C15B0F00A08"))); } ImmutableSet commonDsData = builder.build(); @@ -613,7 +721,10 @@ class DomainUpdateFlowTest extends ResourceFlowTestCase builder = new ImmutableSet.Builder<>(); for (int i = 0; i < 7; ++i) { - builder.add(DelegationSignerData.create(i, 2, 2, new byte[] {0, 1, 2})); + builder.add( + DelegationSignerData.create( + i, + 2, + 2, + base16().decode("9F86D081884C7D659A2FEAA0C55AD015A3BF4F1B2B0B822CD15D6C15B0F00A08"))); } ImmutableSet commonDsData = builder.build(); @@ -664,13 +784,19 @@ class DomainUpdateFlowTest extends ResourceFlowTestCase + runCommandForced( + "--client=NewRegistrar", + "--registrant=crr-admin", + "--admins=crr-admin", + "--techs=crr-tech", + "--ds_records=1 2 1 abcd", + "example.tld")); + assertThat(thrown).hasMessageThat().isEqualTo("DS record has an invalid digest length: ABCD"); + } + @Test void testFailure_invalidAlgorithm() { IllegalArgumentException thrown = @@ -341,7 +359,9 @@ class CreateDomainCommandTest extends EppToolCommandTestCase runCommandForced( - "--client=NewRegistrar", "--add_ds_records=1 299 2 abcd", "example.tld")); + "--client=NewRegistrar", + "--add_ds_records=1 299 2" + + " D4B7D520E7BB5F0F67674A0CCEB1E3E0614B93C4F9E99B8383F6A1E4469DA50A", + "example.tld")); assertThat(thrown).hasMessageThat().isEqualTo("DS record uses an unrecognized algorithm: 299"); } @@ -647,10 +663,29 @@ class UpdateDomainCommandTest extends EppToolCommandTestCase runCommandForced( - "--client=NewRegistrar", "--add_ds_records=1 2 3 abcd", "example.tld")); + "--client=NewRegistrar", + "--add_ds_records=1 2 3" + + " D4B7D520E7BB5F0F67674A0CCEB1E3E0614B93C4F9E99B8383F6A1E4469DA50A", + "example.tld")); assertThat(thrown).hasMessageThat().isEqualTo("DS record uses an unrecognized digest type: 3"); } + @TestOfyAndSql + void testFailure_invalidDigestLength() { + IllegalArgumentException thrown = + assertThrows( + IllegalArgumentException.class, + () -> + runCommandForced( + "--client=NewRegistrar", + "--registrant=crr-admin", + "--admins=crr-admin", + "--techs=crr-tech", + "--ds_records=1 2 1 abcd", + "example.tld")); + assertThat(thrown).hasMessageThat().isEqualTo("DS record has an invalid digest length: ABCD"); + } + @TestOfyAndSql void testFailure_provideDsRecordsAndAddDsRecords() { IllegalArgumentException thrown = @@ -659,8 +694,9 @@ class UpdateDomainCommandTest extends EppToolCommandTestCase runCommandForced( "--client=NewRegistrar", - "--add_ds_records=1 2 2 abcd", - "--ds_records=4 5 1 EF01", + "--add_ds_records=1 2 2" + + " D4B7D520E7BB5F0F67674A0CCEB1E3E0614B93C4F9E99B8383F6A1E4469DA50A", + "--ds_records=4 5 1 A94A8FE5CCB19BA61C4C0873D391E987982FBBD3", "example.tld")); assertThat(thrown) .hasMessageThat() @@ -677,8 +713,8 @@ class UpdateDomainCommandTest extends EppToolCommandTestCase runCommandForced( "--client=NewRegistrar", - "--remove_ds_records=7 8 1 12ab", - "--ds_records=4 5 1 EF01", + "--remove_ds_records=7 8 1 A94A8FE5CCB19BA61C4C0873D391E987982FBBD3", + "--ds_records=4 5 1 A94A8FE5CCB19BA61C4C0873D391E987982FBBD3", "example.tld")); assertThat(thrown) .hasMessageThat() @@ -695,7 +731,8 @@ class UpdateDomainCommandTest extends EppToolCommandTestCase runCommandForced( "--client=NewRegistrar", - "--add_ds_records=1 2 2 abcd", + "--add_ds_records=1 2 2" + + " D4B7D520E7BB5F0F67674A0CCEB1E3E0614B93C4F9E99B8383F6A1E4469DA50A", "--clear_ds_records", "example.tld")); assertThat(thrown) @@ -713,7 +750,7 @@ class UpdateDomainCommandTest extends EppToolCommandTestCase runCommandForced( "--client=NewRegistrar", - "--remove_ds_records=7 8 1 12ab", + "--remove_ds_records=7 8 1 A94A8FE5CCB19BA61C4C0873D391E987982FBBD3", "--clear_ds_records", "example.tld")); assertThat(thrown) diff --git a/core/src/test/resources/google/registry/flows/domain/domain_create_dsdata_8_records.xml b/core/src/test/resources/google/registry/flows/domain/domain_create_dsdata_8_records.xml index 15a8ee041..82048ba80 100644 --- a/core/src/test/resources/google/registry/flows/domain/domain_create_dsdata_8_records.xml +++ b/core/src/test/resources/google/registry/flows/domain/domain_create_dsdata_8_records.xml @@ -26,49 +26,49 @@ 12345 3 1 - 49FD46E6C4B45C55D4AC + A94A8FE5CCB19BA61C4C0873D391E987982FBBD3 12346 3 1 - 49FD46E6C4B45C55D4AC + A94A8FE5CCB19BA61C4C0873D391E987982FBBD3 12347 3 1 - 49FD46E6C4B45C55D4AC + A94A8FE5CCB19BA61C4C0873D391E987982FBBD3 12348 3 1 - 49FD46E6C4B45C55D4AC + A94A8FE5CCB19BA61C4C0873D391E987982FBBD3 12349 3 1 - 49FD46E6C4B45C55D4AC + A94A8FE5CCB19BA61C4C0873D391E987982FBBD3 12350 3 1 - 49FD46E6C4B45C55D4AC + A94A8FE5CCB19BA61C4C0873D391E987982FBBD3 12351 3 1 - 49FD46E6C4B45C55D4AC + A94A8FE5CCB19BA61C4C0873D391E987982FBBD3 12352 3 1 - 49FD46E6C4B45C55D4AC + A94A8FE5CCB19BA61C4C0873D391E987982FBBD3 diff --git a/core/src/test/resources/google/registry/flows/domain/domain_create_dsdata_no_maxsiglife.xml b/core/src/test/resources/google/registry/flows/domain/domain_create_dsdata_no_maxsiglife.xml index 0855dfead..1028636f7 100644 --- a/core/src/test/resources/google/registry/flows/domain/domain_create_dsdata_no_maxsiglife.xml +++ b/core/src/test/resources/google/registry/flows/domain/domain_create_dsdata_no_maxsiglife.xml @@ -26,7 +26,7 @@ 12345 3 1 - 49FD46E6C4B45C55D4AC + A94A8FE5CCB19BA61C4C0873D391E987982FBBD3 diff --git a/core/src/test/resources/google/registry/flows/domain/domain_update_dsdata_add_rem.xml b/core/src/test/resources/google/registry/flows/domain/domain_update_dsdata_add_rem.xml index 84bcf027f..dae4d973e 100644 --- a/core/src/test/resources/google/registry/flows/domain/domain_update_dsdata_add_rem.xml +++ b/core/src/test/resources/google/registry/flows/domain/domain_update_dsdata_add_rem.xml @@ -16,7 +16,7 @@ 12345 3 1 - 38EC35D5B3A34B33C99B + A94A8FE5CCB19BA61C4C0873D391E987982FBBD3 @@ -24,7 +24,7 @@ 12346 3 1 - 38EC35D5B3A34B44C39B + A94A8FE5CCB19BA61C4C0873D391E987982FBBD3 diff --git a/core/src/test/resources/google/registry/flows/domain/domain_update_dsdata_add_rem_same.xml b/core/src/test/resources/google/registry/flows/domain/domain_update_dsdata_add_rem_same.xml index 254aaa336..5fce45746 100644 --- a/core/src/test/resources/google/registry/flows/domain/domain_update_dsdata_add_rem_same.xml +++ b/core/src/test/resources/google/registry/flows/domain/domain_update_dsdata_add_rem_same.xml @@ -16,7 +16,7 @@ 12345 3 1 - 38EC35D5B3A34B33C99B + A94A8FE5CCB19BA61C4C0873D391E987982FBBD3 @@ -24,7 +24,7 @@ 12345 3 1 - 38EC35D5B3A34B33C99B + A94A8FE5CCB19BA61C4C0873D391E987982FBBD3 diff --git a/core/src/test/resources/google/registry/flows/domain/domain_update_dsdata_rem.xml b/core/src/test/resources/google/registry/flows/domain/domain_update_dsdata_rem.xml index 605e627eb..47a6c4c10 100644 --- a/core/src/test/resources/google/registry/flows/domain/domain_update_dsdata_rem.xml +++ b/core/src/test/resources/google/registry/flows/domain/domain_update_dsdata_rem.xml @@ -16,7 +16,7 @@ 12346 3 1 - 38EC35D5B3A34B44C39B + A94A8FE5CCB19BA61C4C0873D391E987982FBBD3 diff --git a/core/src/test/resources/google/registry/flows/domain_update_dsdata_add.xml b/core/src/test/resources/google/registry/flows/domain_update_dsdata_add.xml index 820b19c2a..4efd89f77 100644 --- a/core/src/test/resources/google/registry/flows/domain_update_dsdata_add.xml +++ b/core/src/test/resources/google/registry/flows/domain_update_dsdata_add.xml @@ -16,7 +16,7 @@ 12346 3 1 - 38EC35D5B3A34B44C39B + A94A8FE5CCB19BA61C4C0873D391E987982FBBD3 diff --git 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 index bcbb2ac39..f6491e830 100644 --- 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 @@ -26,13 +26,13 @@ 1 2 2 - ABCD + 9F86D081884C7D659A2FEAA0C55AD015A3BF4F1B2B0B822CD15D6C15B0F00A08 4 5 1 - EF01 + A94A8FE5CCB19BA61C4C0873D391E987982FBBD3 60485 diff --git 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 index aaab20984..02f8cc3d3 100644 --- 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 @@ -23,13 +23,13 @@ 1 2 2 - ABCD + D4B7D520E7BB5F0F67674A0CCEB1E3E0614B93C4F9E99B8383F6A1E4469DA50A 4 5 1 - EF01 + A94A8FE5CCB19BA61C4C0873D391E987982FBBD3 diff --git 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 index ae3aa2d0f..c414b1139 100644 --- 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 @@ -38,13 +38,13 @@ 7 8 1 - 12AB + A94A8FE5CCB19BA61C4C0873D391E987982FBBD3 6 5 4 - 34CD + 768412320F7B0AA5812FCE428DC4706B3CAE50E02A64CAA16A782249BFE8EFC4B7EF1CCB126255D196047DFEDF17A0A9 @@ -52,13 +52,13 @@ 1 2 2 - ABCD + 9F86D081884C7D659A2FEAA0C55AD015A3BF4F1B2B0B822CD15D6C15B0F00A08 4 5 1 - EF01 + A94A8FE5CCB19BA61C4C0873D391E987982FBBD3 diff --git 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 index 9b0966b5e..77ed598ad 100644 --- 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 @@ -38,13 +38,13 @@ 7 8 1 - 12AB + A94A8FE5CCB19BA61C4C0873D391E987982FBBD3 6 5 4 - 34CD + 768412320F7B0AA5812FCE428DC4706B3CAE50E02A64CAA16A782249BFE8EFC4B7EF1CCB126255D196047DFEDF17A0A9 @@ -52,13 +52,13 @@ 1 2 2 - ABCD + 9F86D081884C7D659A2FEAA0C55AD015A3BF4F1B2B0B822CD15D6C15B0F00A08 4 5 1 - EF01 + A94A8FE5CCB19BA61C4C0873D391E987982FBBD3 diff --git 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 index 1a99325c4..a9a58f636 100644 --- 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 @@ -22,13 +22,13 @@ 7 8 1 - 12AB + A94A8FE5CCB19BA61C4C0873D391E987982FBBD3 6 5 4 - 34CD + 768412320F7B0AA5812FCE428DC4706B3CAE50E02A64CAA16A782249BFE8EFC4B7EF1CCB126255D196047DFEDF17A0A9 diff --git 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 index 04f301350..d8c6000f3 100644 --- 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 @@ -17,13 +17,13 @@ 1 2 2 - ABCD + D4B7D520E7BB5F0F67674A0CCEB1E3E0614B93C4F9E99B8383F6A1E4469DA50A 4 5 1 - EF01 + A94A8FE5CCB19BA61C4C0873D391E987982FBBD3 diff --git a/core/src/test/resources/google/registry/tools/server/uniform_rapid_suspension.xml b/core/src/test/resources/google/registry/tools/server/uniform_rapid_suspension.xml index 646038354..99536a3ad 100644 --- a/core/src/test/resources/google/registry/tools/server/uniform_rapid_suspension.xml +++ b/core/src/test/resources/google/registry/tools/server/uniform_rapid_suspension.xml @@ -31,7 +31,7 @@ 1 1 1 - ABCD + A94A8FE5CCB19BA61C4C0873D391E987982FBBD3 diff --git a/core/src/test/resources/google/registry/tools/server/uniform_rapid_suspension_with_client_hold.xml b/core/src/test/resources/google/registry/tools/server/uniform_rapid_suspension_with_client_hold.xml index 6a6c58eae..3ef3a2559 100644 --- a/core/src/test/resources/google/registry/tools/server/uniform_rapid_suspension_with_client_hold.xml +++ b/core/src/test/resources/google/registry/tools/server/uniform_rapid_suspension_with_client_hold.xml @@ -32,7 +32,7 @@ 1 1 1 - ABCD + A94A8FE5CCB19BA61C4C0873D391E987982FBBD3