From f0c677b18b39d84013071bad95714b866b40e3c5 Mon Sep 17 00:00:00 2001 From: mcilwain Date: Mon, 28 Jan 2019 11:13:15 -0800 Subject: [PATCH] Rename DNL and SMDRL "login" to "loginAndPassword" They are passed around in the format username:password, whereas just saying "login" implies it's just a username and not necessarily also a secret password. Putting password in the variable name makes it obvious what this is and reduces the likelihood of anyone ever logging it or otherwise using it inappropriately. Note that this does not require data migrations as the actual key used to store the data in KMS remains unchanged. This is a follow-up to [] ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=231253964 --- .../registry/keyring/api/InMemoryKeyring.java | 22 ++++++++++--------- .../registry/keyring/api/KeyModule.java | 12 +++++----- java/google/registry/keyring/api/Keyring.java | 4 ++-- .../registry/keyring/kms/KmsKeyring.java | 4 ++-- .../registry/keyring/kms/KmsUpdater.java | 4 ++-- java/google/registry/tmch/Marksdb.java | 13 ++++++----- java/google/registry/tmch/TmchDnlAction.java | 4 ++-- .../google/registry/tmch/TmchSmdrlAction.java | 4 ++-- .../tools/GetKeyringSecretCommand.java | 8 +++---- .../tools/UpdateKmsKeyringCommand.java | 8 +++---- .../registry/tools/params/KeyringKeyName.java | 4 ++-- .../registry/keyring/kms/KmsKeyringTest.java | 12 +++++----- .../registry/keyring/kms/KmsUpdaterTest.java | 10 ++++----- .../registry/testing/FakeKeyringModule.java | 12 +++++----- .../registry/tmch/TmchActionTestCase.java | 2 +- .../registry/tmch/TmchDnlActionTest.java | 2 +- .../registry/tmch/TmchSmdrlActionTest.java | 2 +- 17 files changed, 65 insertions(+), 62 deletions(-) diff --git a/java/google/registry/keyring/api/InMemoryKeyring.java b/java/google/registry/keyring/api/InMemoryKeyring.java index d2bcf8775..b49b0f763 100644 --- a/java/google/registry/keyring/api/InMemoryKeyring.java +++ b/java/google/registry/keyring/api/InMemoryKeyring.java @@ -35,9 +35,9 @@ public final class InMemoryKeyring implements Keyring { private final String rdeSshClientPrivateKey; private final String icannReportingPassword; private final String safeBrowsingAPIKey; - private final String marksdbDnlLogin; + private final String marksdbDnlLoginAndPassword; private final String marksdbLordnPassword; - private final String marksdbSmdrlLogin; + private final String marksdbSmdrlLoginAndPassword; private final String jsonCredential; public InMemoryKeyring( @@ -50,9 +50,9 @@ public final class InMemoryKeyring implements Keyring { String rdeSshClientPrivateKey, String icannReportingPassword, String safeBrowsingAPIKey, - String marksdbDnlLogin, + String marksdbDnlLoginAndPassword, String marksdbLordnPassword, - String marksdbSmdrlLogin, + String marksdbSmdrlLoginAndPassword, String jsonCredential) { checkArgument(PgpHelper.isSigningKey(rdeSigningKey.getPublicKey()), "RDE signing key must support signing: %s", rdeSigningKey.getKeyID()); @@ -73,9 +73,11 @@ public final class InMemoryKeyring implements Keyring { this.rdeSshClientPrivateKey = checkNotNull(rdeSshClientPrivateKey, "rdeSshClientPrivateKey"); this.icannReportingPassword = checkNotNull(icannReportingPassword, "icannReportingPassword"); this.safeBrowsingAPIKey = checkNotNull(safeBrowsingAPIKey, "safeBrowsingAPIKey"); - this.marksdbDnlLogin = checkNotNull(marksdbDnlLogin, "marksdbDnlLogin"); + this.marksdbDnlLoginAndPassword = + checkNotNull(marksdbDnlLoginAndPassword, "marksdbDnlLoginAndPassword"); this.marksdbLordnPassword = checkNotNull(marksdbLordnPassword, "marksdbLordnPassword"); - this.marksdbSmdrlLogin = checkNotNull(marksdbSmdrlLogin, "marksdbSmdrlLogin"); + this.marksdbSmdrlLoginAndPassword = + checkNotNull(marksdbSmdrlLoginAndPassword, "marksdbSmdrlLoginAndPassword"); this.jsonCredential = checkNotNull(jsonCredential, "jsonCredential"); } @@ -130,8 +132,8 @@ public final class InMemoryKeyring implements Keyring { } @Override - public String getMarksdbDnlLogin() { - return marksdbDnlLogin; + public String getMarksdbDnlLoginAndPassword() { + return marksdbDnlLoginAndPassword; } @Override @@ -140,8 +142,8 @@ public final class InMemoryKeyring implements Keyring { } @Override - public String getMarksdbSmdrlLogin() { - return marksdbSmdrlLogin; + public String getMarksdbSmdrlLoginAndPassword() { + return marksdbSmdrlLoginAndPassword; } @Override diff --git a/java/google/registry/keyring/api/KeyModule.java b/java/google/registry/keyring/api/KeyModule.java index d4282e28d..dbadf2d16 100644 --- a/java/google/registry/keyring/api/KeyModule.java +++ b/java/google/registry/keyring/api/KeyModule.java @@ -55,9 +55,9 @@ public final class KeyModule { } @Provides - @Key("marksdbDnlLogin") - static Optional provideMarksdbDnlLogin(Keyring keyring) { - return Optional.ofNullable(emptyToNull(keyring.getMarksdbDnlLogin())); + @Key("marksdbDnlLoginAndPassword") + static Optional provideMarksdbDnlLoginAndPassword(Keyring keyring) { + return Optional.ofNullable(emptyToNull(keyring.getMarksdbDnlLoginAndPassword())); } @Provides @@ -67,9 +67,9 @@ public final class KeyModule { } @Provides - @Key("marksdbSmdrlLogin") - static Optional provideMarksdbSmdrlLogin(Keyring keyring) { - return Optional.ofNullable(emptyToNull(keyring.getMarksdbSmdrlLogin())); + @Key("marksdbSmdrlLoginAndPassword") + static Optional provideMarksdbSmdrlLoginAndPassword(Keyring keyring) { + return Optional.ofNullable(emptyToNull(keyring.getMarksdbSmdrlLoginAndPassword())); } @Provides diff --git a/java/google/registry/keyring/api/Keyring.java b/java/google/registry/keyring/api/Keyring.java index ff6b51cc4..5b44db049 100644 --- a/java/google/registry/keyring/api/Keyring.java +++ b/java/google/registry/keyring/api/Keyring.java @@ -129,7 +129,7 @@ public interface Keyring extends AutoCloseable { * * @see google.registry.tmch.TmchDnlAction */ - String getMarksdbDnlLogin(); + String getMarksdbDnlLoginAndPassword(); /** * Returns password for TMCH MarksDB HTTP server LORDN interface. @@ -143,7 +143,7 @@ public interface Keyring extends AutoCloseable { * * @see google.registry.tmch.TmchSmdrlAction */ - String getMarksdbSmdrlLogin(); + String getMarksdbSmdrlLoginAndPassword(); /** * Returns the credentials for a service account on the Google AppEngine project downloaded from diff --git a/java/google/registry/keyring/kms/KmsKeyring.java b/java/google/registry/keyring/kms/KmsKeyring.java index e8968784f..17245206e 100644 --- a/java/google/registry/keyring/kms/KmsKeyring.java +++ b/java/google/registry/keyring/kms/KmsKeyring.java @@ -139,7 +139,7 @@ public class KmsKeyring implements Keyring { } @Override - public String getMarksdbDnlLogin() { + public String getMarksdbDnlLoginAndPassword() { return getString(StringKeyLabel.MARKSDB_DNL_LOGIN_STRING); } @@ -149,7 +149,7 @@ public class KmsKeyring implements Keyring { } @Override - public String getMarksdbSmdrlLogin() { + public String getMarksdbSmdrlLoginAndPassword() { return getString(StringKeyLabel.MARKSDB_SMDRL_LOGIN_STRING); } diff --git a/java/google/registry/keyring/kms/KmsUpdater.java b/java/google/registry/keyring/kms/KmsUpdater.java index 5ee1fefb5..970a4eab5 100644 --- a/java/google/registry/keyring/kms/KmsUpdater.java +++ b/java/google/registry/keyring/kms/KmsUpdater.java @@ -104,7 +104,7 @@ public final class KmsUpdater { return setString(password, ICANN_REPORTING_PASSWORD_STRING); } - public KmsUpdater setMarksdbDnlLogin(String login) { + public KmsUpdater setMarksdbDnlLoginAndPassword(String login) { return setString(login, MARKSDB_DNL_LOGIN_STRING); } @@ -112,7 +112,7 @@ public final class KmsUpdater { return setString(password, MARKSDB_LORDN_PASSWORD_STRING); } - public KmsUpdater setMarksdbSmdrlLogin(String login) { + public KmsUpdater setMarksdbSmdrlLoginAndPassword(String login) { return setString(login, MARKSDB_SMDRL_LOGIN_STRING); } diff --git a/java/google/registry/tmch/Marksdb.java b/java/google/registry/tmch/Marksdb.java index a99f3087d..17ae96a91 100644 --- a/java/google/registry/tmch/Marksdb.java +++ b/java/google/registry/tmch/Marksdb.java @@ -112,9 +112,9 @@ public final class Marksdb { } } - byte[] fetch(URL url, Optional login) throws IOException { + byte[] fetch(URL url, Optional loginAndPassword) throws IOException { HTTPRequest req = new HTTPRequest(url, GET, validateCertificate().setDeadline(60d)); - setAuthorizationHeader(req, login); + setAuthorizationHeader(req, loginAndPassword); HTTPResponse rsp = fetchService.fetch(req); if (rsp.getResponseCode() != SC_OK) { throw new UrlFetchException("Failed to fetch from MarksDB", req, rsp); @@ -122,16 +122,17 @@ public final class Marksdb { return rsp.getContent(); } - List fetchSignedCsv(Optional login, String csvPath, String sigPath) + List fetchSignedCsv(Optional loginAndPassword, String csvPath, String sigPath) throws IOException, SignatureException, PGPException { - checkArgument(login.isPresent(), "Cannot fetch from MarksDB without login credentials"); + checkArgument( + loginAndPassword.isPresent(), "Cannot fetch from MarksDB without login credentials"); String csvUrl = tmchMarksdbUrl + csvPath; - byte[] csv = fetch(new URL(csvUrl), login); + byte[] csv = fetch(new URL(csvUrl), loginAndPassword); logFetchedBytes(csvUrl, csv); String sigUrl = tmchMarksdbUrl + sigPath; - byte[] sig = fetch(new URL(sigUrl), login); + byte[] sig = fetch(new URL(sigUrl), loginAndPassword); logFetchedBytes(sigUrl, sig); pgpVerifySignature(csv, sig, marksdbPublicKey); diff --git a/java/google/registry/tmch/TmchDnlAction.java b/java/google/registry/tmch/TmchDnlAction.java index 353b5a773..787b7a4c8 100644 --- a/java/google/registry/tmch/TmchDnlAction.java +++ b/java/google/registry/tmch/TmchDnlAction.java @@ -42,7 +42,7 @@ public final class TmchDnlAction implements Runnable { private static final String DNL_SIG_PATH = "/dnl/dnl-latest.sig"; @Inject Marksdb marksdb; - @Inject @Key("marksdbDnlLogin") Optional marksdbDnlLogin; + @Inject @Key("marksdbDnlLoginAndPassword") Optional marksdbDnlLoginAndPassword; @Inject TmchDnlAction() {} /** Synchronously fetches latest domain name list and saves it to Datastore. */ @@ -50,7 +50,7 @@ public final class TmchDnlAction implements Runnable { public void run() { List lines; try { - lines = marksdb.fetchSignedCsv(marksdbDnlLogin, DNL_CSV_PATH, DNL_SIG_PATH); + lines = marksdb.fetchSignedCsv(marksdbDnlLoginAndPassword, DNL_CSV_PATH, DNL_SIG_PATH); } catch (SignatureException | IOException | PGPException e) { throw new RuntimeException(e); } diff --git a/java/google/registry/tmch/TmchSmdrlAction.java b/java/google/registry/tmch/TmchSmdrlAction.java index 0ddfd4ca2..ef46f757d 100644 --- a/java/google/registry/tmch/TmchSmdrlAction.java +++ b/java/google/registry/tmch/TmchSmdrlAction.java @@ -42,7 +42,7 @@ public final class TmchSmdrlAction implements Runnable { private static final String SMDRL_SIG_PATH = "/smdrl/smdrl-latest.sig"; @Inject Marksdb marksdb; - @Inject @Key("marksdbSmdrlLogin") Optional marksdbSmdrlLogin; + @Inject @Key("marksdbSmdrlLoginAndPassword") Optional marksdbSmdrlLoginAndPassword; @Inject TmchSmdrlAction() {} /** Synchronously fetches latest signed mark revocation list and saves it to Datastore. */ @@ -50,7 +50,7 @@ public final class TmchSmdrlAction implements Runnable { public void run() { List lines; try { - lines = marksdb.fetchSignedCsv(marksdbSmdrlLogin, SMDRL_CSV_PATH, SMDRL_SIG_PATH); + lines = marksdb.fetchSignedCsv(marksdbSmdrlLoginAndPassword, SMDRL_CSV_PATH, SMDRL_SIG_PATH); } catch (SignatureException | IOException | PGPException e) { throw new RuntimeException(e); } diff --git a/java/google/registry/tools/GetKeyringSecretCommand.java b/java/google/registry/tools/GetKeyringSecretCommand.java index 1f11d0da9..63eb0d6d9 100644 --- a/java/google/registry/tools/GetKeyringSecretCommand.java +++ b/java/google/registry/tools/GetKeyringSecretCommand.java @@ -74,14 +74,14 @@ final class GetKeyringSecretCommand implements CommandWithRemoteApi { case JSON_CREDENTIAL: out.write(KeySerializer.serializeString(keyring.getJsonCredential())); break; - case MARKSDB_DNL_LOGIN: - out.write(KeySerializer.serializeString(keyring.getMarksdbDnlLogin())); + case MARKSDB_DNL_LOGIN_AND_PASSWORD: + out.write(KeySerializer.serializeString(keyring.getMarksdbDnlLoginAndPassword())); break; case MARKSDB_LORDN_PASSWORD: out.write(KeySerializer.serializeString(keyring.getMarksdbLordnPassword())); break; - case MARKSDB_SMDRL_LOGIN: - out.write(KeySerializer.serializeString(keyring.getMarksdbSmdrlLogin())); + case MARKSDB_SMDRL_LOGIN_AND_PASSWORD: + out.write(KeySerializer.serializeString(keyring.getMarksdbSmdrlLoginAndPassword())); break; case RDE_RECEIVER_PUBLIC_KEY: out.write(KeySerializer.serializePublicKey(keyring.getRdeReceiverKey())); diff --git a/java/google/registry/tools/UpdateKmsKeyringCommand.java b/java/google/registry/tools/UpdateKmsKeyringCommand.java index b7933bb19..865643e20 100644 --- a/java/google/registry/tools/UpdateKmsKeyringCommand.java +++ b/java/google/registry/tools/UpdateKmsKeyringCommand.java @@ -71,14 +71,14 @@ final class UpdateKmsKeyringCommand implements CommandWithRemoteApi { case JSON_CREDENTIAL: kmsUpdater.setJsonCredential(deserializeString(input)); break; - case MARKSDB_DNL_LOGIN: - kmsUpdater.setMarksdbDnlLogin(deserializeString(input)); + case MARKSDB_DNL_LOGIN_AND_PASSWORD: + kmsUpdater.setMarksdbDnlLoginAndPassword(deserializeString(input)); break; case MARKSDB_LORDN_PASSWORD: kmsUpdater.setMarksdbLordnPassword(deserializeString(input)); break; - case MARKSDB_SMDRL_LOGIN: - kmsUpdater.setMarksdbSmdrlLogin(deserializeString(input)); + case MARKSDB_SMDRL_LOGIN_AND_PASSWORD: + kmsUpdater.setMarksdbSmdrlLoginAndPassword(deserializeString(input)); break; case RDE_RECEIVER_PUBLIC_KEY: kmsUpdater.setRdeReceiverPublicKey(deserializePublicKey(input)); diff --git a/java/google/registry/tools/params/KeyringKeyName.java b/java/google/registry/tools/params/KeyringKeyName.java index 1ca37d910..8b3ecf7be 100644 --- a/java/google/registry/tools/params/KeyringKeyName.java +++ b/java/google/registry/tools/params/KeyringKeyName.java @@ -26,9 +26,9 @@ public enum KeyringKeyName { BRDA_SIGNING_PUBLIC_KEY, ICANN_REPORTING_PASSWORD, JSON_CREDENTIAL, - MARKSDB_DNL_LOGIN, + MARKSDB_DNL_LOGIN_AND_PASSWORD, MARKSDB_LORDN_PASSWORD, - MARKSDB_SMDRL_LOGIN, + MARKSDB_SMDRL_LOGIN_AND_PASSWORD, RDE_RECEIVER_PUBLIC_KEY, RDE_SIGNING_KEY_PAIR, RDE_SIGNING_PUBLIC_KEY, diff --git a/javatests/google/registry/keyring/kms/KmsKeyringTest.java b/javatests/google/registry/keyring/kms/KmsKeyringTest.java index 4a79540ee..98e6da279 100644 --- a/javatests/google/registry/keyring/kms/KmsKeyringTest.java +++ b/javatests/google/registry/keyring/kms/KmsKeyringTest.java @@ -137,12 +137,12 @@ public class KmsKeyringTest { } @Test - public void test_getMarksdbDnlLogin() { + public void test_getMarksdbDnlLoginAndPassword() { saveCleartextSecret("marksdb-dnl-login-string"); - String marksdbDnlLogin = keyring.getMarksdbDnlLogin(); + String marksdbDnlLoginAndPassword = keyring.getMarksdbDnlLoginAndPassword(); - assertThat(marksdbDnlLogin).isEqualTo("marksdb-dnl-login-stringmoo"); + assertThat(marksdbDnlLoginAndPassword).isEqualTo("marksdb-dnl-login-stringmoo"); } @Test @@ -155,12 +155,12 @@ public class KmsKeyringTest { } @Test - public void test_getMarksdbSmdrlLogin() { + public void test_getMarksdbSmdrlLoginAndPassword() { saveCleartextSecret("marksdb-smdrl-login-string"); - String marksdbSmdrlLogin = keyring.getMarksdbSmdrlLogin(); + String marksdbSmdrlLoginAndPassword = keyring.getMarksdbSmdrlLoginAndPassword(); - assertThat(marksdbSmdrlLogin).isEqualTo("marksdb-smdrl-login-stringmoo"); + assertThat(marksdbSmdrlLoginAndPassword).isEqualTo("marksdb-smdrl-login-stringmoo"); } diff --git a/javatests/google/registry/keyring/kms/KmsUpdaterTest.java b/javatests/google/registry/keyring/kms/KmsUpdaterTest.java index fd1f5bbdc..4219468ce 100644 --- a/javatests/google/registry/keyring/kms/KmsUpdaterTest.java +++ b/javatests/google/registry/keyring/kms/KmsUpdaterTest.java @@ -50,7 +50,7 @@ public class KmsUpdaterTest { @Test public void test_setMultipleSecrets() { updater - .setMarksdbDnlLogin("value1") + .setMarksdbDnlLoginAndPassword("value1") .setIcannReportingPassword("value2") .setJsonCredential("value3") .update(); @@ -110,8 +110,8 @@ public class KmsUpdaterTest { } @Test - public void test_setMarksdbDnlLogin() { - updater.setMarksdbDnlLogin("value1").update(); + public void test_setMarksdbDnlLoginAndPassword() { + updater.setMarksdbDnlLoginAndPassword("value1").update(); verifySecretAndSecretRevisionWritten( "marksdb-dnl-login-string", "marksdb-dnl-login-string/foo", getCiphertext("value1")); @@ -128,8 +128,8 @@ public class KmsUpdaterTest { } @Test - public void test_setMarksdbSmdrlLogin() { - updater.setMarksdbSmdrlLogin("value1").update(); + public void test_setMarksdbSmdrlLoginAndPassword() { + updater.setMarksdbSmdrlLoginAndPassword("value1").update(); verifySecretAndSecretRevisionWritten( "marksdb-smdrl-login-string", "marksdb-smdrl-login-string/foo", getCiphertext("value1")); diff --git a/javatests/google/registry/testing/FakeKeyringModule.java b/javatests/google/registry/testing/FakeKeyringModule.java index f0328e6b6..c57f4faa2 100644 --- a/javatests/google/registry/testing/FakeKeyringModule.java +++ b/javatests/google/registry/testing/FakeKeyringModule.java @@ -52,9 +52,9 @@ public final class FakeKeyringModule { loadBytes(FakeKeyringModule.class, "pgp-private-keyring-registry.asc"); private static final String ICANN_REPORTING_PASSWORD = "yolo"; private static final String SAFE_BROWSING_API_KEY = "a/b_c"; - private static final String MARKSDB_DNL_LOGIN = "dnl:yolo"; + private static final String MARKSDB_DNL_LOGIN_AND_PASSWORD = "dnl:yolo"; private static final String MARKSDB_LORDN_PASSWORD = "yolo"; - private static final String MARKSDB_SMDRL_LOGIN = "smdrl:yolo"; + private static final String MARKSDB_SMDRL_LOGIN_AND_PASSWORD = "smdrl:yolo"; private static final String JSON_CREDENTIAL = "json123"; @Provides @@ -111,8 +111,8 @@ public final class FakeKeyringModule { } @Override - public String getMarksdbSmdrlLogin() { - return MARKSDB_SMDRL_LOGIN; + public String getMarksdbSmdrlLoginAndPassword() { + return MARKSDB_SMDRL_LOGIN_AND_PASSWORD; } @Override @@ -121,8 +121,8 @@ public final class FakeKeyringModule { } @Override - public String getMarksdbDnlLogin() { - return MARKSDB_DNL_LOGIN; + public String getMarksdbDnlLoginAndPassword() { + return MARKSDB_DNL_LOGIN_AND_PASSWORD; } @Override diff --git a/javatests/google/registry/tmch/TmchActionTestCase.java b/javatests/google/registry/tmch/TmchActionTestCase.java index cefa7902b..1a9421561 100644 --- a/javatests/google/registry/tmch/TmchActionTestCase.java +++ b/javatests/google/registry/tmch/TmchActionTestCase.java @@ -37,7 +37,7 @@ import org.mockito.Mock; @RunWith(JUnit4.class) public class TmchActionTestCase { - static final String MARKSDB_LOGIN = "lolcat:attack"; + static final String MARKSDB_LOGIN_AND_PASSWORD = "lolcat:attack"; static final String MARKSDB_URL = "http://127.0.0.1/love"; @Rule public final AppEngineRule appEngine = AppEngineRule.builder().withDatastore().build(); diff --git a/javatests/google/registry/tmch/TmchDnlActionTest.java b/javatests/google/registry/tmch/TmchDnlActionTest.java index fdd448ef0..f78c8addd 100644 --- a/javatests/google/registry/tmch/TmchDnlActionTest.java +++ b/javatests/google/registry/tmch/TmchDnlActionTest.java @@ -31,7 +31,7 @@ public class TmchDnlActionTest extends TmchActionTestCase { private TmchDnlAction newTmchDnlAction() { TmchDnlAction action = new TmchDnlAction(); action.marksdb = marksdb; - action.marksdbDnlLogin = Optional.of(MARKSDB_LOGIN); + action.marksdbDnlLoginAndPassword = Optional.of(MARKSDB_LOGIN_AND_PASSWORD); return action; } diff --git a/javatests/google/registry/tmch/TmchSmdrlActionTest.java b/javatests/google/registry/tmch/TmchSmdrlActionTest.java index 68b4145c1..f3733bed9 100644 --- a/javatests/google/registry/tmch/TmchSmdrlActionTest.java +++ b/javatests/google/registry/tmch/TmchSmdrlActionTest.java @@ -33,7 +33,7 @@ public class TmchSmdrlActionTest extends TmchActionTestCase { private TmchSmdrlAction newTmchSmdrlAction() { TmchSmdrlAction action = new TmchSmdrlAction(); action.marksdb = marksdb; - action.marksdbSmdrlLogin = Optional.of("username:password"); + action.marksdbSmdrlLoginAndPassword = Optional.of("username:password"); return action; }