Skip to content

Commit dde4107

Browse files
authored
Forbid SHA-1 digests as part of RFC 9904 changes (#3069)
We can't change digest types that are already in the database but that's fine (since we just store them as integers). But we forbid them as part of domain creates/updates.
1 parent 0030645 commit dde4107

28 files changed

Lines changed: 368 additions & 149 deletions

core/src/main/java/google/registry/flows/domain/DomainFlowUtils.java

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import static com.google.common.collect.Sets.intersection;
2424
import static com.google.common.collect.Sets.union;
2525
import static google.registry.bsa.persistence.BsaLabelUtils.isLabelBlocked;
26+
import static google.registry.model.common.FeatureFlag.FeatureName.FORBID_INSECURE_ALGORITHMS_RFC_9904;
2627
import static google.registry.model.domain.Domain.MAX_REGISTRATION_YEARS;
2728
import static google.registry.model.domain.token.AllocationToken.TokenType.REGISTER_BSA;
2829
import static google.registry.model.tld.Tld.TldState.GENERAL_AVAILABILITY;
@@ -73,6 +74,7 @@
7374
import google.registry.model.billing.BillingBase.Flag;
7475
import google.registry.model.billing.BillingBase.Reason;
7576
import google.registry.model.billing.BillingRecurrence;
77+
import google.registry.model.common.FeatureFlag;
7678
import google.registry.model.domain.Domain;
7779
import google.registry.model.domain.DomainCommand.Create;
7880
import google.registry.model.domain.DomainCommand.CreateOrUpdate;
@@ -341,17 +343,24 @@ static void validateDsData(Set<DomainDsData> dsData) throws EppException {
341343
}
342344
ImmutableList<DomainDsData> invalidAlgorithms =
343345
dsData.stream()
344-
.filter(ds -> !validateAlgorithm(ds.getAlgorithm()))
346+
.filter(ds -> algorithmIsInvalid(ds.getAlgorithm()))
345347
.collect(toImmutableList());
346348
if (!invalidAlgorithms.isEmpty()) {
347349
throw new InvalidDsRecordException(
348350
String.format(
349351
"Domain contains DS record(s) with an invalid algorithm wire value: %s",
350352
invalidAlgorithms));
351353
}
354+
boolean forbidInsecureTypes = FeatureFlag.isActiveNow(FORBID_INSECURE_ALGORITHMS_RFC_9904);
352355
ImmutableList<DomainDsData> invalidDigestTypes =
353356
dsData.stream()
354-
.filter(ds -> DigestType.fromWireValue(ds.getDigestType()).isEmpty())
357+
.filter(
358+
ds -> {
359+
Optional<DigestType> digestType = DigestType.fromWireValue(ds.getDigestType());
360+
return digestType
361+
.map(type -> forbidInsecureTypes && !type.isAllowedInRfc9904())
362+
.orElse(true);
363+
})
355364
.collect(toImmutableList());
356365
if (!invalidDigestTypes.isEmpty()) {
357366
throw new InvalidDsRecordException(
@@ -376,14 +385,14 @@ static void validateDsData(Set<DomainDsData> dsData) throws EppException {
376385
}
377386
}
378387

379-
public static boolean validateAlgorithm(int alg) {
388+
public static boolean algorithmIsInvalid(int alg) {
380389
if (alg > 255 || alg < 0) {
381-
return false;
390+
return true;
382391
}
383392
// Algorithms that are reserved or unassigned will just return a string representation of their
384393
// integer wire value.
385394
String algorithm = Algorithm.string(alg);
386-
return !algorithm.equals(Integer.toString(alg));
395+
return algorithm.equals(Integer.toString(alg));
387396
}
388397

389398
/** We only allow specifying years in a period. */

core/src/main/java/google/registry/model/common/FeatureFlag.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,10 @@ public enum FeatureName {
8484
INCLUDE_PENDING_DELETE_DATE_FOR_DOMAINS(FeatureStatus.INACTIVE),
8585

8686
/** If we're prohibiting the inclusion of the contact object URI on login. */
87-
PROHIBIT_CONTACT_OBJECTS_ON_LOGIN(FeatureStatus.INACTIVE);
87+
PROHIBIT_CONTACT_OBJECTS_ON_LOGIN(FeatureStatus.INACTIVE),
88+
89+
/** If we're prohibiting insecure algorithms as detailed by RFC 9904. */
90+
FORBID_INSECURE_ALGORITHMS_RFC_9904(FeatureStatus.INACTIVE);
8891

8992
private final FeatureStatus defaultStatus;
9093

core/src/main/java/google/registry/tools/DigestType.java

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,21 +29,26 @@
2929
* https://www.iana.org/assignments/ds-rr-types/ds-rr-types.xhtml
3030
*/
3131
public enum DigestType {
32-
SHA1(1, 20),
33-
SHA256(2, 32),
32+
// Algorithm number 1 is SHA-1 and will be is deliberately NOT SUPPORTED.
33+
// RFC 9904 specifies that this algorithm MUST NOT be used for DNSSEC delegations.
34+
// This prohibition is gated behind a feature flag.
35+
SHA1(1, 20, false),
36+
SHA256(2, 32, true),
3437
// Algorithm number 3 is GOST R 34.11-94 and is deliberately NOT SUPPORTED.
3538
// This algorithm was reviewed by ise-crypto and deemed academically broken (b/207029800).
3639
// In addition, RFC 8624 specifies that this algorithm MUST NOT be used for DNSSEC delegations.
3740
// TODO(sarhabot@): Add note in Cloud DNS code to notify the Registry of any new changes to
3841
// supported digest types.
39-
SHA384(4, 48);
42+
SHA384(4, 48, true);
4043

4144
private final int wireValue;
4245
private final int bytes;
46+
private final boolean allowedInRfc9904;
4347

44-
DigestType(int wireValue, int bytes) {
48+
DigestType(int wireValue, int bytes, boolean allowedInRfc9904) {
4549
this.wireValue = wireValue;
4650
this.bytes = bytes;
51+
this.allowedInRfc9904 = allowedInRfc9904;
4752
}
4853

4954
private static final ImmutableMap<Integer, DigestType> WIRE_VALUE_TO_DIGEST_TYPE =
@@ -63,4 +68,9 @@ public int getWireValue() {
6368
public int getBytes() {
6469
return bytes;
6570
}
71+
72+
/** Whether this digest type is supported as of RFC 9904. */
73+
public boolean isAllowedInRfc9904() {
74+
return allowedInRfc9904;
75+
}
6676
}

core/src/main/java/google/registry/tools/DsRecord.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ private static DsRecord create(int keyTag, int alg, int digestType, String diges
4646
String.format("DS record has an invalid digest length: %s", digest));
4747
}
4848

49-
if (!DomainFlowUtils.validateAlgorithm(alg)) {
49+
if (DomainFlowUtils.algorithmIsInvalid(alg)) {
5050
throw new IllegalArgumentException(
5151
String.format("DS record uses an unrecognized algorithm: %d", alg));
5252
}

core/src/test/java/google/registry/flows/domain/DomainCreateFlowTest.java

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import static google.registry.model.billing.BillingBase.Flag.SUNRISE;
2525
import static google.registry.model.billing.BillingBase.RenewalPriceBehavior.NONPREMIUM;
2626
import static google.registry.model.billing.BillingBase.RenewalPriceBehavior.SPECIFIED;
27+
import static google.registry.model.common.FeatureFlag.FeatureName.FORBID_INSECURE_ALGORITHMS_RFC_9904;
2728
import static google.registry.model.domain.fee.Fee.FEE_EXTENSION_URIS;
2829
import static google.registry.model.domain.token.AllocationToken.TokenType.BULK_PRICING;
2930
import static google.registry.model.domain.token.AllocationToken.TokenType.DEFAULT_PROMO;
@@ -150,6 +151,8 @@
150151
import google.registry.model.billing.BillingBase.RenewalPriceBehavior;
151152
import google.registry.model.billing.BillingEvent;
152153
import google.registry.model.billing.BillingRecurrence;
154+
import google.registry.model.common.FeatureFlag;
155+
import google.registry.model.common.FeatureFlag.FeatureStatus;
153156
import google.registry.model.domain.Domain;
154157
import google.registry.model.domain.DomainHistory;
155158
import google.registry.model.domain.GracePeriod;
@@ -794,7 +797,11 @@ void testSuccess_secDns() throws Exception {
794797
.that(domain)
795798
.hasExactlyDsData(
796799
DomainDsData.create(
797-
12345, 3, 1, base16().decode("A94A8FE5CCB19BA61C4C0873D391E987982FBBD3"))
800+
12345,
801+
3,
802+
2,
803+
base16()
804+
.decode("D4B7D520E7BB5F0F67674A0CCEB1E3E0614B93C4F9E99B8383F6A1E4469DA50A"))
798805
.cloneWithDomainRepoId(domain.getRepoId()));
799806
}
800807

@@ -957,6 +964,38 @@ void testFailure_secDnsInvalidDigestType() throws Exception {
957964
assertAboutEppExceptions().that(thrown).marshalsToXml();
958965
}
959966

967+
@Test
968+
void testFailure_secDnsSha1DigestType() throws Exception {
969+
setEppInput("domain_create_dsdata_sha1.xml");
970+
persistHosts();
971+
DatabaseHelper.persistResource(
972+
new FeatureFlag.Builder()
973+
.setFeatureName(FORBID_INSECURE_ALGORITHMS_RFC_9904)
974+
.setStatusMap(ImmutableSortedMap.of(START_INSTANT, FeatureStatus.ACTIVE))
975+
.build());
976+
EppException thrown = assertThrows(InvalidDsRecordException.class, this::runFlow);
977+
assertAboutEppExceptions().that(thrown).marshalsToXml();
978+
}
979+
980+
@Test
981+
void testSuccess_secDnsSha1_flagInactive() throws Exception {
982+
setEppInput("domain_create_dsdata_sha1.xml");
983+
persistHosts();
984+
DatabaseHelper.persistResource(
985+
new FeatureFlag.Builder()
986+
.setFeatureName(FORBID_INSECURE_ALGORITHMS_RFC_9904)
987+
.setStatusMap(ImmutableSortedMap.of(START_INSTANT, FeatureStatus.INACTIVE))
988+
.build());
989+
doSuccessfulTest("tld");
990+
Domain domain = reloadResourceByForeignKey();
991+
assertAboutDomains()
992+
.that(domain)
993+
.hasExactlyDsData(
994+
DomainDsData.create(
995+
12345, 3, 1, base16().decode("49FD46E6C4B45C55D4AC49FD46E6C4B45C55D4AC"))
996+
.cloneWithDomainRepoId(domain.getRepoId()));
997+
}
998+
960999
@Test
9611000
void testFailure_secDnsInvalidAlgorithm() throws Exception {
9621001
setEppInput("domain_create_dsdata_bad_algorithms.xml");

0 commit comments

Comments
 (0)