Remove SQL credentials from Keyring (#1059)

* Remove SQL credentials from Keyring

Remove SQL credentials from Keyring. SQL credentials will be managed by
an automated system (go/dr-sql-security) and the keyring is no longer a
suitable place to hold them.

Also stopped loading SQL credentials from they keyring for comparison
with those from the secret manager.

core/src/main/java/google/registry/config/RegistryConfig.java

@@ -403,12 +403,6 @@ public final class RegistryConfig {
       return config.cloudSql.jdbcUrl;
     }
 
-    @Provides
-    @Config("cloudSqlUsername")
-    public static String providesCloudSqlUsername(RegistryConfigSettings config) {
-      return config.cloudSql.username;
-    }
-
     @Provides
     @Config("cloudSqlInstanceConnectionName")
     public static String providesCloudSqlInstanceConnectionName(RegistryConfigSettings config) {
@@ -1342,12 +1336,6 @@ public final class RegistryConfig {
       return config.registryTool.clientSecret;
     }
 
-    @Provides
-    @Config("toolsCloudSqlUsername")
-    public static String providesToolsCloudSqlUsername(RegistryConfigSettings config) {
-      return config.registryTool.username;
-    }
-
     @Provides
     @Config("rdapTos")
     public static ImmutableList<String> provideRdapTos(RegistryConfigSettings config) {

core/src/main/java/google/registry/config/RegistryConfigSettings.java

@@ -123,6 +123,7 @@ public class RegistryConfigSettings {
   /** Configuration for Cloud SQL. */
   public static class CloudSql {
     public String jdbcUrl;
+    // TODO(05012021): remove username field after it is removed from all yaml files.
     public String username;
     public String instanceConnectionName;
     public boolean replicateTransactions;
@@ -221,6 +222,7 @@ public class RegistryConfigSettings {
   public static class RegistryTool {
     public String clientId;
     public String clientSecret;
+    // TODO(05012021): remove username field after it is removed from all yaml files.
     public String username;
   }
 

core/src/main/java/google/registry/config/files/default-config.yaml

@@ -225,8 +225,6 @@ cloudSql:
   # If jdbcUrl in this file is moved elsewhere, be sure to move this notice
   # with it until the change is applied.
   jdbcUrl: jdbc:postgresql://localhost
-  # Username for the database user.
-  username: username
   # This name is used by Cloud SQL when connecting to the database.
   instanceConnectionName: project-id:region:instance-id
   # Set this to true to replicate cloud SQL transactions to datastore in the
@@ -447,7 +445,6 @@ registryTool:
   clientId: YOUR_CLIENT_ID
   # OAuth client secret used by the tool.
   clientSecret: YOUR_CLIENT_SECRET
-  username: toolusername
 
 # Configuration options for checking SSL certificates.
 sslCertificateValidation:

core/src/main/java/google/registry/keyring/api/InMemoryKeyring.java

@@ -39,8 +39,6 @@ public final class InMemoryKeyring implements Keyring {
   private final String marksdbLordnPassword;
   private final String marksdbSmdrlLoginAndPassword;
   private final String jsonCredential;
-  private final String cloudSqlPassword;
-  private final String toolsCloudSqlPassword;
 
   public InMemoryKeyring(
       PGPKeyPair rdeStagingKey,
@@ -83,8 +81,6 @@ public final class InMemoryKeyring implements Keyring {
     this.marksdbSmdrlLoginAndPassword =
         checkNotNull(marksdbSmdrlLoginAndPassword, "marksdbSmdrlLoginAndPassword");
     this.jsonCredential = checkNotNull(jsonCredential, "jsonCredential");
-    this.cloudSqlPassword = checkNotNull(cloudSqlPassword, "cloudSqlPassword");
-    this.toolsCloudSqlPassword = checkNotNull(toolsCloudSqlPassword, "toolsCloudSqlPassword");
   }
 
   @Override
@@ -157,16 +153,6 @@ public final class InMemoryKeyring implements Keyring {
     return jsonCredential;
   }
 
-  @Override
-  public String getCloudSqlPassword() {
-    return cloudSqlPassword;
-  }
-
-  @Override
-  public String getToolsCloudSqlPassword() {
-    return toolsCloudSqlPassword;
-  }
-
   /** Does nothing. */
   @Override
   public void close() {}

core/src/main/java/google/registry/keyring/api/KeyModule.java

@@ -36,18 +36,6 @@ public final class KeyModule {
     String value();
   }
 
-  @Provides
-  @Key("cloudSqlPassword")
-  static String providesCloudSqlPassword(Keyring keyring) {
-    return keyring.getCloudSqlPassword();
-  }
-
-  @Provides
-  @Key("toolsCloudSqlPassword")
-  static String providesToolsCloudSqlPassword(Keyring keyring) {
-    return keyring.getToolsCloudSqlPassword();
-  }
-
   @Provides
   @Key("brdaReceiverKey")
   static PGPPublicKey provideBrdaReceiverKey(Keyring keyring) {

core/src/main/java/google/registry/keyring/api/Keyring.java

@@ -28,12 +28,6 @@ import org.bouncycastle.openpgp.PGPPublicKey;
 @ThreadSafe
 public interface Keyring extends AutoCloseable {
 
-  /** Returns the password which is used by App Engine to connect to the Cloud SQL database. */
-  String getCloudSqlPassword();
-
-  /** Returns the password which is used by nomulus tool to connect to the Cloud SQL database. */
-  String getToolsCloudSqlPassword();
-
   /**
    * Returns the key which should be used to sign RDE deposits being uploaded to a third-party.
    *

core/src/main/java/google/registry/keyring/kms/KmsKeyring.java

@@ -72,7 +72,6 @@ public class KmsKeyring implements Keyring {
 
   /** Key labels for string secrets. */
   enum StringKeyLabel {
-    CLOUD_SQL_PASSWORD_STRING,
     SAFE_BROWSING_API_KEY,
     ICANN_REPORTING_PASSWORD_STRING,
     JSON_CREDENTIAL_STRING,
@@ -80,8 +79,7 @@ public class KmsKeyring implements Keyring {
     MARKSDB_LORDN_PASSWORD_STRING,
     MARKSDB_SMDRL_LOGIN_STRING,
     RDE_SSH_CLIENT_PRIVATE_STRING,
-    RDE_SSH_CLIENT_PUBLIC_STRING,
+    RDE_SSH_CLIENT_PUBLIC_STRING;
-    TOOLS_CLOUD_SQL_PASSWORD_STRING;
 
     String getLabel() {
       return UPPER_UNDERSCORE.to(LOWER_HYPHEN, name());
@@ -95,16 +93,6 @@ public class KmsKeyring implements Keyring {
     this.kmsConnection = kmsConnection;
   }
 
-  @Override
-  public String getCloudSqlPassword() {
-    return getString(StringKeyLabel.CLOUD_SQL_PASSWORD_STRING);
-  }
-
-  @Override
-  public String getToolsCloudSqlPassword() {
-    return getString(StringKeyLabel.TOOLS_CLOUD_SQL_PASSWORD_STRING);
-  }
-
   @Override
   public PGPKeyPair getRdeSigningKey() {
     return getKeyPair(PrivateKeyLabel.RDE_SIGNING_PRIVATE);

core/src/main/java/google/registry/keyring/kms/KmsUpdater.java

@@ -24,7 +24,6 @@ import static google.registry.keyring.kms.KmsKeyring.PublicKeyLabel.BRDA_SIGNING
 import static google.registry.keyring.kms.KmsKeyring.PublicKeyLabel.RDE_RECEIVER_PUBLIC;
 import static google.registry.keyring.kms.KmsKeyring.PublicKeyLabel.RDE_SIGNING_PUBLIC;
 import static google.registry.keyring.kms.KmsKeyring.PublicKeyLabel.RDE_STAGING_PUBLIC;
-import static google.registry.keyring.kms.KmsKeyring.StringKeyLabel.CLOUD_SQL_PASSWORD_STRING;
 import static google.registry.keyring.kms.KmsKeyring.StringKeyLabel.ICANN_REPORTING_PASSWORD_STRING;
 import static google.registry.keyring.kms.KmsKeyring.StringKeyLabel.JSON_CREDENTIAL_STRING;
 import static google.registry.keyring.kms.KmsKeyring.StringKeyLabel.MARKSDB_DNL_LOGIN_STRING;
@@ -33,7 +32,6 @@ import static google.registry.keyring.kms.KmsKeyring.StringKeyLabel.MARKSDB_SMDR
 import static google.registry.keyring.kms.KmsKeyring.StringKeyLabel.RDE_SSH_CLIENT_PRIVATE_STRING;
 import static google.registry.keyring.kms.KmsKeyring.StringKeyLabel.RDE_SSH_CLIENT_PUBLIC_STRING;
 import static google.registry.keyring.kms.KmsKeyring.StringKeyLabel.SAFE_BROWSING_API_KEY;
-import static google.registry.keyring.kms.KmsKeyring.StringKeyLabel.TOOLS_CLOUD_SQL_PASSWORD_STRING;
 import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
 import static google.registry.util.PreconditionsUtils.checkArgumentNotNull;
 
@@ -71,10 +69,6 @@ public final class KmsUpdater {
     this.secretValues = new LinkedHashMap<>();
   }
 
-  public KmsUpdater setCloudSqlPassword(String password) {
-    return setString(password, CLOUD_SQL_PASSWORD_STRING);
-  }
-
   public KmsUpdater setRdeSigningKey(PGPKeyPair keyPair) throws IOException, PGPException {
     return setKeyPair(keyPair, RDE_SIGNING_PRIVATE, RDE_SIGNING_PUBLIC);
   }
@@ -107,10 +101,6 @@ public final class KmsUpdater {
     return setString(apiKey, SAFE_BROWSING_API_KEY);
   }
 
-  public KmsUpdater setToolsCloudSqlPassword(String password) {
-    return setString(password, TOOLS_CLOUD_SQL_PASSWORD_STRING);
-  }
-
   public KmsUpdater setIcannReportingPassword(String password) {
     return setString(password, ICANN_REPORTING_PASSWORD_STRING);
   }

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

@@ -31,7 +31,6 @@ import dagger.BindsOptionalOf;
 import dagger.Module;
 import dagger.Provides;
 import google.registry.config.RegistryConfig.Config;
-import google.registry.keyring.kms.KmsKeyring;
 import google.registry.persistence.transaction.CloudSqlCredentialSupplier;
 import google.registry.persistence.transaction.JpaTransactionManager;
 import google.registry.persistence.transaction.JpaTransactionManagerImpl;
@@ -202,18 +201,11 @@ public abstract class PersistenceModule {
   @Singleton
   @AppEngineJpaTm
   static JpaTransactionManager provideAppEngineJpaTm(
-      @Config("cloudSqlUsername") String username,
-      KmsKeyring kmsKeyring,
       SqlCredentialStore credentialStore,
       @PartialCloudSqlConfigs ImmutableMap<String, String> cloudSqlConfigs,
       Clock clock) {
     HashMap<String, String> overrides = Maps.newHashMap(cloudSqlConfigs);
-    validateAndSetCredential(
+    setSqlCredential(credentialStore, new RobotUser(RobotId.NOMULUS), overrides);
-        credentialStore,
-        new RobotUser(RobotId.NOMULUS),
-        overrides,
-        username,
-        kmsKeyring.getCloudSqlPassword());
     return new JpaTransactionManagerImpl(create(overrides), clock);
   }
 
@@ -258,20 +250,13 @@ public abstract class PersistenceModule {
   @Singleton
   @NomulusToolJpaTm
   static JpaTransactionManager provideNomulusToolJpaTm(
-      @Config("toolsCloudSqlUsername") String username,
-      KmsKeyring kmsKeyring,
       SqlCredentialStore credentialStore,
       @PartialCloudSqlConfigs ImmutableMap<String, String> cloudSqlConfigs,
       @CloudSqlClientCredential Credential credential,
       Clock clock) {
     CloudSqlCredentialSupplier.setupCredentialSupplier(credential);
     HashMap<String, String> overrides = Maps.newHashMap(cloudSqlConfigs);
-    validateAndSetCredential(
+    setSqlCredential(credentialStore, new RobotUser(RobotId.TOOL), overrides);
-        credentialStore,
-        new RobotUser(RobotId.TOOL),
-        overrides,
-        username,
-        kmsKeyring.getToolsCloudSqlPassword());
     return new JpaTransactionManagerImpl(create(overrides), clock);
   }
 
@@ -349,35 +334,15 @@ public abstract class PersistenceModule {
     return emf;
   }
 
-  /**
+  /** Configures JPA with the credential from the Secret Manager. */
-   * Verifies that the credential from the Secret Manager matches the one currently in use, and
+  private static void setSqlCredential(
-   * configures JPA with the credential from the Secret Manager.
+      SqlCredentialStore credentialStore, SqlUser sqlUser, Map<String, String> overrides) {
-   *
-   * <p>This is a helper for the transition to the Secret Manager, and will be removed once data and
-   * permissions are properly set up for all projects.
-   */
-  private static void validateAndSetCredential(
-      SqlCredentialStore credentialStore,
-      SqlUser sqlUser,
-      Map<String, String> overrides,
-      String expectedLogin,
-      String expectedPassword) {
     try {
       SqlCredential credential = credentialStore.getCredential(sqlUser);
-      checkState(
-          credential.login().equals(expectedLogin),
-          "Wrong login for %s. Expecting %s, found %s.",
-          sqlUser.geUserName(),
-          expectedLogin,
-          credential.login());
-      checkState(
-          credential.password().equals(expectedPassword),
-          "Wrong password for %s.",
-          sqlUser.geUserName());
       overrides.put(Environment.USER, credential.login());
       overrides.put(Environment.PASS, credential.password());
-      logger.atWarning().log("Credentials in the kerying and the secret manager match.");
     } catch (Throwable e) {
+      // TODO(b/184631990): after SQL becomes primary, throw an exception to fail fast
       logger.atSevere().withCause(e).log("Failed to get SQL credential from Secret Manager");
     }
   }

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

@@ -65,12 +65,6 @@ final class GetKeyringSecretCommand implements CommandWithRemoteApi {
       case BRDA_SIGNING_PUBLIC_KEY:
         out.write(KeySerializer.serializePublicKey(keyring.getBrdaSigningKey().getPublicKey()));
         break;
-      case CLOUD_SQL_PASSWORD:
-        out.write(KeySerializer.serializeString(keyring.getCloudSqlPassword()));
-        break;
-      case TOOLS_CLOUD_SQL_PASSWORD:
-        out.write(KeySerializer.serializeString(keyring.getToolsCloudSqlPassword()));
-        break;
       case ICANN_REPORTING_PASSWORD:
         out.write(KeySerializer.serializeString(keyring.getIcannReportingPassword()));
         break;

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

@@ -65,12 +65,6 @@ final class UpdateKmsKeyringCommand implements CommandWithRemoteApi {
         throw new IllegalArgumentException(
             "Can't update BRDA_SIGNING_PUBLIC_KEY directly."
             + " Must update public and private keys together using BRDA_SIGNING_KEY_PAIR.");
-      case CLOUD_SQL_PASSWORD:
-        kmsUpdater.setCloudSqlPassword(deserializeString(input));
-        break;
-      case TOOLS_CLOUD_SQL_PASSWORD:
-        kmsUpdater.setToolsCloudSqlPassword(deserializeString(input));
-        break;
       case ICANN_REPORTING_PASSWORD:
         kmsUpdater.setIcannReportingPassword(deserializeString(input));
         break;

core/src/main/java/google/registry/tools/params/KeyringKeyName.java

@@ -24,7 +24,6 @@ public enum KeyringKeyName {
   BRDA_RECEIVER_PUBLIC_KEY,
   BRDA_SIGNING_KEY_PAIR,
   BRDA_SIGNING_PUBLIC_KEY,
-  CLOUD_SQL_PASSWORD,
   ICANN_REPORTING_PASSWORD,
   JSON_CREDENTIAL,
   MARKSDB_DNL_LOGIN_AND_PASSWORD,
@@ -37,6 +36,5 @@ public enum KeyringKeyName {
   RDE_SSH_CLIENT_PUBLIC_KEY,
   RDE_STAGING_KEY_PAIR,
   RDE_STAGING_PUBLIC_KEY,
-  SAFE_BROWSING_API_KEY,
+  SAFE_BROWSING_API_KEY
-  TOOLS_CLOUD_SQL_PASSWORD,
 }

core/src/test/java/google/registry/keyring/kms/KmsKeyringTest.java

@@ -48,20 +48,6 @@ class KmsKeyringTest {
     keyring = new KmsKeyring(new FakeKmsConnection());
   }
 
-  @TestOfyAndSql
-  void test_getCloudSqlPassword() {
-    saveCleartextSecret("cloud-sql-password-string");
-    String cloudSqlPassword = keyring.getCloudSqlPassword();
-    assertThat(cloudSqlPassword).isEqualTo("cloud-sql-password-stringmoo");
-  }
-
-  @TestOfyAndSql
-  void test_getToolsCloudSqlPassword() {
-    saveCleartextSecret("tools-cloud-sql-password-string");
-    String toolsCloudSqlPassword = keyring.getToolsCloudSqlPassword();
-    assertThat(toolsCloudSqlPassword).isEqualTo("tools-cloud-sql-password-stringmoo");
-  }
-
   @TestOfyAndSql
   void test_getRdeSigningKey() throws Exception {
     saveKeyPairSecret("rde-signing-public", "rde-signing-private");

core/src/test/java/google/registry/keyring/kms/KmsUpdaterTest.java

@@ -96,24 +96,6 @@ public class KmsUpdaterTest {
         getCiphertext(KmsTestHelper.getPublicKey()));
   }
 
-  @TestOfyAndSql
-  void test_setCloudSqlPassword() {
-    updater.setCloudSqlPassword("value1").update();
-
-    verifySecretAndSecretRevisionWritten(
-        "cloud-sql-password-string", "cloud-sql-password-string/foo", getCiphertext("value1"));
-  }
-
-  @TestOfyAndSql
-  void test_setToolsCloudSqlPassword() {
-    updater.setToolsCloudSqlPassword("value1").update();
-
-    verifySecretAndSecretRevisionWritten(
-        "tools-cloud-sql-password-string",
-        "tools-cloud-sql-password-string/foo",
-        getCiphertext("value1"));
-  }
-
   @TestOfyAndSql
   void test_setIcannReportingPassword() {
     updater.setIcannReportingPassword("value1").update();

core/src/test/java/google/registry/testing/FakeKeyringModule.java

@@ -56,8 +56,6 @@ public final class FakeKeyringModule {
   private static final String MARKSDB_LORDN_PASSWORD = "yolo";
   private static final String MARKSDB_SMDRL_LOGIN_AND_PASSWORD = "smdrl:yolo";
   private static final String JSON_CREDENTIAL = "json123";
-  private static final String CLOUD_SQL_PASSWORD = "cloudsqlpw";
-  private static final String TOOLS_CLOUD_SQL_PASSWORD = "toolscloudsqlpw";
 
   @Provides
   public Keyring get() {
@@ -82,15 +80,6 @@ public final class FakeKeyringModule {
     final String sshPrivate = loadFile(FakeKeyringModule.class, "registry-unittest.id_rsa");
 
     return new Keyring() {
-      @Override
-      public String getCloudSqlPassword() {
-        return CLOUD_SQL_PASSWORD;
-      }
-
-      @Override
-      public String getToolsCloudSqlPassword() {
-        return TOOLS_CLOUD_SQL_PASSWORD;
-      }
 
       @Override
       public PGPPublicKey getRdeStagingEncryptionKey() {