From c36087dc93fdafc993c3b0b0cd05ef1b3b713cff Mon Sep 17 00:00:00 2001 From: Juan Celhay Date: Tue, 28 Jul 2026 12:00:55 -0400 Subject: [PATCH] generate servertrids using securerandom behind feature flag (#3163) --- .../flows/ServerTridProviderImpl.java | 31 +++- .../registry/model/common/FeatureFlag.java | 5 +- .../flows/ServerTridProviderImplTest.java | 151 ++++++++++++++++++ .../sql/schema/db-schema.sql.generated | 2 +- 4 files changed, 185 insertions(+), 4 deletions(-) create mode 100644 core/src/test/java/google/registry/flows/ServerTridProviderImplTest.java diff --git a/core/src/main/java/google/registry/flows/ServerTridProviderImpl.java b/core/src/main/java/google/registry/flows/ServerTridProviderImpl.java index b94e4e17d..c1ef6aa44 100644 --- a/core/src/main/java/google/registry/flows/ServerTridProviderImpl.java +++ b/core/src/main/java/google/registry/flows/ServerTridProviderImpl.java @@ -15,20 +15,38 @@ package google.registry.flows; import static com.google.common.primitives.Longs.BYTES; +import static google.registry.model.common.FeatureFlag.FeatureName.USE_RANDOM_SERVER_TRID; +import static google.registry.persistence.transaction.TransactionManagerFactory.tm; +import com.google.common.annotations.VisibleForTesting; import com.google.common.io.BaseEncoding; +import google.registry.model.common.FeatureFlag; import jakarta.inject.Inject; import java.nio.ByteBuffer; +import java.security.NoSuchAlgorithmException; +import java.security.SecureRandom; import java.util.UUID; import java.util.concurrent.atomic.AtomicLong; -/** A server Trid provider that generates globally incrementing UUIDs. */ +/** A server Trid provider that generates transaction IDs. */ public class ServerTridProviderImpl implements ServerTridProvider { private static final String SERVER_ID = getServerId(); private static final AtomicLong idCounter = new AtomicLong(); - @Inject public ServerTridProviderImpl() {} + @VisibleForTesting + static final ThreadLocal secureRandom = + ThreadLocal.withInitial( + () -> { + try { + return SecureRandom.getInstance("DRBG"); + } catch (NoSuchAlgorithmException e) { + throw new RuntimeException(e); + } + }); + + @Inject + public ServerTridProviderImpl() {} /** Creates a unique id for this server instance, as a base64 encoded UUID. */ private static String getServerId() { @@ -42,6 +60,15 @@ public class ServerTridProviderImpl implements ServerTridProvider { @Override public String createServerTrid() { + if (tm().reTransact(() -> FeatureFlag.isActiveNow(USE_RANDOM_SERVER_TRID))) { + // The server TRID can be at most 64 characters. We generate 24 random bytes + // (192 bits), which base64url-encodes without padding to 32 characters. + // This provides an unpredictable TRID that does not leak pod identity or + // command volume. + byte[] randomBytes = new byte[24]; + secureRandom.get().nextBytes(randomBytes); + return BaseEncoding.base64Url().omitPadding().encode(randomBytes); + } // The server id can be at most 64 characters. The SERVER_ID is at most 22 characters (128 // bits in base64), plus the dash. That leaves 41 characters, so we just append the counter in // hex. diff --git a/core/src/main/java/google/registry/model/common/FeatureFlag.java b/core/src/main/java/google/registry/model/common/FeatureFlag.java index 2569e59f6..f3f30f2f7 100644 --- a/core/src/main/java/google/registry/model/common/FeatureFlag.java +++ b/core/src/main/java/google/registry/model/common/FeatureFlag.java @@ -87,7 +87,10 @@ public class FeatureFlag extends ImmutableObject implements Buildable { PROHIBIT_CONTACT_OBJECTS_ON_LOGIN(FeatureStatus.INACTIVE), /** If we're prohibiting insecure algorithms as detailed by RFC 9904. */ - FORBID_INSECURE_ALGORITHMS_RFC_9904(FeatureStatus.INACTIVE); + FORBID_INSECURE_ALGORITHMS_RFC_9904(FeatureStatus.INACTIVE), + + /** If we're using secure random base64 encoded server TRIDs. */ + USE_RANDOM_SERVER_TRID(FeatureStatus.INACTIVE); private final FeatureStatus defaultStatus; diff --git a/core/src/test/java/google/registry/flows/ServerTridProviderImplTest.java b/core/src/test/java/google/registry/flows/ServerTridProviderImplTest.java new file mode 100644 index 000000000..e646f6c3f --- /dev/null +++ b/core/src/test/java/google/registry/flows/ServerTridProviderImplTest.java @@ -0,0 +1,151 @@ +// Copyright 2026 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.flows; + +import static com.google.common.truth.Truth.assertThat; +import static google.registry.model.common.FeatureFlag.FeatureName.USE_RANDOM_SERVER_TRID; +import static google.registry.model.common.FeatureFlag.FeatureStatus.ACTIVE; +import static google.registry.testing.DatabaseHelper.persistResource; +import static google.registry.util.DateTimeUtils.START_INSTANT; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.mock; + +import com.google.common.collect.ImmutableSortedMap; +import google.registry.model.common.FeatureFlag; +import google.registry.model.common.FeatureFlag.FeatureStatus; +import google.registry.persistence.transaction.JpaTestExtensions; +import google.registry.persistence.transaction.JpaTestExtensions.JpaIntegrationTestExtension; +import java.security.SecureRandom; +import java.time.Instant; +import java.util.regex.Pattern; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +/** Unit tests for {@link ServerTridProviderImpl}. */ +class ServerTridProviderImplTest { + + @RegisterExtension + final JpaIntegrationTestExtension jpa = + new JpaTestExtensions.Builder().buildIntegrationTestExtension(); + + @AfterEach + void tearDown() { + ServerTridProviderImpl.secureRandom.remove(); + } + + @Test + void testCreateServerTrid_flagInactive_generatesLegacyFormat() { + ServerTridProviderImpl provider = new ServerTridProviderImpl(); + String trid1 = provider.createServerTrid(); + String trid2 = provider.createServerTrid(); + + assertThat(trid1).contains("-"); + assertThat(trid2).contains("-"); + assertThat(trid1).isNotEqualTo(trid2); + } + + @Test + void testCreateServerTrid_flagActive_generatesCorrectFormat() { + persistResource( + new FeatureFlag.Builder() + .setFeatureName(USE_RANDOM_SERVER_TRID) + .setStatusMap( + ImmutableSortedMap.naturalOrder() + .put(START_INSTANT, ACTIVE) + .build()) + .build()); + + SecureRandom mockSecureRandom = mock(SecureRandom.class); + + // Mock secureRandom to return a deterministic sequence of bytes: 0, 1, 2, ..., 23 + doAnswer( + invocation -> { + byte[] bytes = invocation.getArgument(0); + for (int i = 0; i < bytes.length; i++) { + bytes[i] = (byte) i; + } + return null; + }) + .when(mockSecureRandom) + .nextBytes(any(byte[].class)); + + ServerTridProviderImpl.secureRandom.set(mockSecureRandom); + ServerTridProviderImpl provider = new ServerTridProviderImpl(); + String trid = provider.createServerTrid(); + + String expectedTrid = "AAECAwQFBgcICQoLDA0ODxAREhMUFRYX"; + + Pattern tridPattern = Pattern.compile("^[A-Za-z0-9_-]{32}$"); + assertThat(trid).matches(tridPattern); + assertThat(trid.length()).isAtMost(64); + assertThat(trid).isEqualTo(expectedTrid); + } + + @Test + void testCreateServerTrid_flagActive_withMaxByteValues() { + persistResource( + new FeatureFlag.Builder() + .setFeatureName(USE_RANDOM_SERVER_TRID) + .setStatusMap( + ImmutableSortedMap.naturalOrder() + .put(START_INSTANT, ACTIVE) + .build()) + .build()); + + SecureRandom mockSecureRandom = mock(SecureRandom.class); + + // Mock secureRandom to return all 0xFF bytes + doAnswer( + invocation -> { + byte[] bytes = invocation.getArgument(0); + for (int i = 0; i < bytes.length; i++) { + bytes[i] = (byte) 0xFF; + } + return null; + }) + .when(mockSecureRandom) + .nextBytes(any(byte[].class)); + + ServerTridProviderImpl.secureRandom.set(mockSecureRandom); + ServerTridProviderImpl provider = new ServerTridProviderImpl(); + String trid = provider.createServerTrid(); + + String expectedTrid = "________________________________"; + assertThat(trid).isEqualTo(expectedTrid); + } + + @Test + void testCreateServerTrid_flagActive_realInitializationWorks() { + persistResource( + new FeatureFlag.Builder() + .setFeatureName(USE_RANDOM_SERVER_TRID) + .setStatusMap( + ImmutableSortedMap.naturalOrder() + .put(START_INSTANT, ACTIVE) + .build()) + .build()); + + ServerTridProviderImpl provider = new ServerTridProviderImpl(); + String trid1 = provider.createServerTrid(); + String trid2 = provider.createServerTrid(); + + Pattern tridPattern = Pattern.compile("^[A-Za-z0-9_-]{32}$"); + assertThat(trid1).matches(tridPattern); + assertThat(trid2).matches(tridPattern); + assertThat(trid1).isNotEqualTo(trid2); + } +} diff --git a/db/src/main/resources/sql/schema/db-schema.sql.generated b/db/src/main/resources/sql/schema/db-schema.sql.generated index 870bfff1b..7e484566f 100644 --- a/db/src/main/resources/sql/schema/db-schema.sql.generated +++ b/db/src/main/resources/sql/schema/db-schema.sql.generated @@ -333,7 +333,7 @@ ); create table "FeatureFlag" ( - feature_name text not null check ((feature_name in ('TEST_FEATURE','FEE_EXTENSION_1_DOT_0_IN_PROD','MINIMUM_DATASET_CONTACTS_OPTIONAL','MINIMUM_DATASET_CONTACTS_PROHIBITED','INCLUDE_PENDING_DELETE_DATE_FOR_DOMAINS','PROHIBIT_CONTACT_OBJECTS_ON_LOGIN','FORBID_INSECURE_ALGORITHMS_RFC_9904'))), + feature_name text not null check ((feature_name in ('TEST_FEATURE','FEE_EXTENSION_1_DOT_0_IN_PROD','MINIMUM_DATASET_CONTACTS_OPTIONAL','MINIMUM_DATASET_CONTACTS_PROHIBITED','INCLUDE_PENDING_DELETE_DATE_FOR_DOMAINS','PROHIBIT_CONTACT_OBJECTS_ON_LOGIN','FORBID_INSECURE_ALGORITHMS_RFC_9904','USE_RANDOM_SERVER_TRID'))), status hstore not null, primary key (feature_name) );