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
This commit is contained in:
mcilwain 2019-01-28 11:13:15 -08:00 committed by Ben McIlwain
parent 0130f91830
commit f0c677b18b
17 changed files with 65 additions and 62 deletions

View file

@ -35,9 +35,9 @@ public final class InMemoryKeyring implements Keyring {
private final String rdeSshClientPrivateKey; private final String rdeSshClientPrivateKey;
private final String icannReportingPassword; private final String icannReportingPassword;
private final String safeBrowsingAPIKey; private final String safeBrowsingAPIKey;
private final String marksdbDnlLogin; private final String marksdbDnlLoginAndPassword;
private final String marksdbLordnPassword; private final String marksdbLordnPassword;
private final String marksdbSmdrlLogin; private final String marksdbSmdrlLoginAndPassword;
private final String jsonCredential; private final String jsonCredential;
public InMemoryKeyring( public InMemoryKeyring(
@ -50,9 +50,9 @@ public final class InMemoryKeyring implements Keyring {
String rdeSshClientPrivateKey, String rdeSshClientPrivateKey,
String icannReportingPassword, String icannReportingPassword,
String safeBrowsingAPIKey, String safeBrowsingAPIKey,
String marksdbDnlLogin, String marksdbDnlLoginAndPassword,
String marksdbLordnPassword, String marksdbLordnPassword,
String marksdbSmdrlLogin, String marksdbSmdrlLoginAndPassword,
String jsonCredential) { String jsonCredential) {
checkArgument(PgpHelper.isSigningKey(rdeSigningKey.getPublicKey()), checkArgument(PgpHelper.isSigningKey(rdeSigningKey.getPublicKey()),
"RDE signing key must support signing: %s", rdeSigningKey.getKeyID()); "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.rdeSshClientPrivateKey = checkNotNull(rdeSshClientPrivateKey, "rdeSshClientPrivateKey");
this.icannReportingPassword = checkNotNull(icannReportingPassword, "icannReportingPassword"); this.icannReportingPassword = checkNotNull(icannReportingPassword, "icannReportingPassword");
this.safeBrowsingAPIKey = checkNotNull(safeBrowsingAPIKey, "safeBrowsingAPIKey"); this.safeBrowsingAPIKey = checkNotNull(safeBrowsingAPIKey, "safeBrowsingAPIKey");
this.marksdbDnlLogin = checkNotNull(marksdbDnlLogin, "marksdbDnlLogin"); this.marksdbDnlLoginAndPassword =
checkNotNull(marksdbDnlLoginAndPassword, "marksdbDnlLoginAndPassword");
this.marksdbLordnPassword = checkNotNull(marksdbLordnPassword, "marksdbLordnPassword"); this.marksdbLordnPassword = checkNotNull(marksdbLordnPassword, "marksdbLordnPassword");
this.marksdbSmdrlLogin = checkNotNull(marksdbSmdrlLogin, "marksdbSmdrlLogin"); this.marksdbSmdrlLoginAndPassword =
checkNotNull(marksdbSmdrlLoginAndPassword, "marksdbSmdrlLoginAndPassword");
this.jsonCredential = checkNotNull(jsonCredential, "jsonCredential"); this.jsonCredential = checkNotNull(jsonCredential, "jsonCredential");
} }
@ -130,8 +132,8 @@ public final class InMemoryKeyring implements Keyring {
} }
@Override @Override
public String getMarksdbDnlLogin() { public String getMarksdbDnlLoginAndPassword() {
return marksdbDnlLogin; return marksdbDnlLoginAndPassword;
} }
@Override @Override
@ -140,8 +142,8 @@ public final class InMemoryKeyring implements Keyring {
} }
@Override @Override
public String getMarksdbSmdrlLogin() { public String getMarksdbSmdrlLoginAndPassword() {
return marksdbSmdrlLogin; return marksdbSmdrlLoginAndPassword;
} }
@Override @Override

View file

@ -55,9 +55,9 @@ public final class KeyModule {
} }
@Provides @Provides
@Key("marksdbDnlLogin") @Key("marksdbDnlLoginAndPassword")
static Optional<String> provideMarksdbDnlLogin(Keyring keyring) { static Optional<String> provideMarksdbDnlLoginAndPassword(Keyring keyring) {
return Optional.ofNullable(emptyToNull(keyring.getMarksdbDnlLogin())); return Optional.ofNullable(emptyToNull(keyring.getMarksdbDnlLoginAndPassword()));
} }
@Provides @Provides
@ -67,9 +67,9 @@ public final class KeyModule {
} }
@Provides @Provides
@Key("marksdbSmdrlLogin") @Key("marksdbSmdrlLoginAndPassword")
static Optional<String> provideMarksdbSmdrlLogin(Keyring keyring) { static Optional<String> provideMarksdbSmdrlLoginAndPassword(Keyring keyring) {
return Optional.ofNullable(emptyToNull(keyring.getMarksdbSmdrlLogin())); return Optional.ofNullable(emptyToNull(keyring.getMarksdbSmdrlLoginAndPassword()));
} }
@Provides @Provides

View file

@ -129,7 +129,7 @@ public interface Keyring extends AutoCloseable {
* *
* @see google.registry.tmch.TmchDnlAction * @see google.registry.tmch.TmchDnlAction
*/ */
String getMarksdbDnlLogin(); String getMarksdbDnlLoginAndPassword();
/** /**
* Returns password for TMCH MarksDB HTTP server LORDN interface. * Returns password for TMCH MarksDB HTTP server LORDN interface.
@ -143,7 +143,7 @@ public interface Keyring extends AutoCloseable {
* *
* @see google.registry.tmch.TmchSmdrlAction * @see google.registry.tmch.TmchSmdrlAction
*/ */
String getMarksdbSmdrlLogin(); String getMarksdbSmdrlLoginAndPassword();
/** /**
* Returns the credentials for a service account on the Google AppEngine project downloaded from * Returns the credentials for a service account on the Google AppEngine project downloaded from

View file

@ -139,7 +139,7 @@ public class KmsKeyring implements Keyring {
} }
@Override @Override
public String getMarksdbDnlLogin() { public String getMarksdbDnlLoginAndPassword() {
return getString(StringKeyLabel.MARKSDB_DNL_LOGIN_STRING); return getString(StringKeyLabel.MARKSDB_DNL_LOGIN_STRING);
} }
@ -149,7 +149,7 @@ public class KmsKeyring implements Keyring {
} }
@Override @Override
public String getMarksdbSmdrlLogin() { public String getMarksdbSmdrlLoginAndPassword() {
return getString(StringKeyLabel.MARKSDB_SMDRL_LOGIN_STRING); return getString(StringKeyLabel.MARKSDB_SMDRL_LOGIN_STRING);
} }

View file

@ -104,7 +104,7 @@ public final class KmsUpdater {
return setString(password, ICANN_REPORTING_PASSWORD_STRING); return setString(password, ICANN_REPORTING_PASSWORD_STRING);
} }
public KmsUpdater setMarksdbDnlLogin(String login) { public KmsUpdater setMarksdbDnlLoginAndPassword(String login) {
return setString(login, MARKSDB_DNL_LOGIN_STRING); return setString(login, MARKSDB_DNL_LOGIN_STRING);
} }
@ -112,7 +112,7 @@ public final class KmsUpdater {
return setString(password, MARKSDB_LORDN_PASSWORD_STRING); return setString(password, MARKSDB_LORDN_PASSWORD_STRING);
} }
public KmsUpdater setMarksdbSmdrlLogin(String login) { public KmsUpdater setMarksdbSmdrlLoginAndPassword(String login) {
return setString(login, MARKSDB_SMDRL_LOGIN_STRING); return setString(login, MARKSDB_SMDRL_LOGIN_STRING);
} }

View file

@ -112,9 +112,9 @@ public final class Marksdb {
} }
} }
byte[] fetch(URL url, Optional<String> login) throws IOException { byte[] fetch(URL url, Optional<String> loginAndPassword) throws IOException {
HTTPRequest req = new HTTPRequest(url, GET, validateCertificate().setDeadline(60d)); HTTPRequest req = new HTTPRequest(url, GET, validateCertificate().setDeadline(60d));
setAuthorizationHeader(req, login); setAuthorizationHeader(req, loginAndPassword);
HTTPResponse rsp = fetchService.fetch(req); HTTPResponse rsp = fetchService.fetch(req);
if (rsp.getResponseCode() != SC_OK) { if (rsp.getResponseCode() != SC_OK) {
throw new UrlFetchException("Failed to fetch from MarksDB", req, rsp); throw new UrlFetchException("Failed to fetch from MarksDB", req, rsp);
@ -122,16 +122,17 @@ public final class Marksdb {
return rsp.getContent(); return rsp.getContent();
} }
List<String> fetchSignedCsv(Optional<String> login, String csvPath, String sigPath) List<String> fetchSignedCsv(Optional<String> loginAndPassword, String csvPath, String sigPath)
throws IOException, SignatureException, PGPException { 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; String csvUrl = tmchMarksdbUrl + csvPath;
byte[] csv = fetch(new URL(csvUrl), login); byte[] csv = fetch(new URL(csvUrl), loginAndPassword);
logFetchedBytes(csvUrl, csv); logFetchedBytes(csvUrl, csv);
String sigUrl = tmchMarksdbUrl + sigPath; String sigUrl = tmchMarksdbUrl + sigPath;
byte[] sig = fetch(new URL(sigUrl), login); byte[] sig = fetch(new URL(sigUrl), loginAndPassword);
logFetchedBytes(sigUrl, sig); logFetchedBytes(sigUrl, sig);
pgpVerifySignature(csv, sig, marksdbPublicKey); pgpVerifySignature(csv, sig, marksdbPublicKey);

View file

@ -42,7 +42,7 @@ public final class TmchDnlAction implements Runnable {
private static final String DNL_SIG_PATH = "/dnl/dnl-latest.sig"; private static final String DNL_SIG_PATH = "/dnl/dnl-latest.sig";
@Inject Marksdb marksdb; @Inject Marksdb marksdb;
@Inject @Key("marksdbDnlLogin") Optional<String> marksdbDnlLogin; @Inject @Key("marksdbDnlLoginAndPassword") Optional<String> marksdbDnlLoginAndPassword;
@Inject TmchDnlAction() {} @Inject TmchDnlAction() {}
/** Synchronously fetches latest domain name list and saves it to Datastore. */ /** Synchronously fetches latest domain name list and saves it to Datastore. */
@ -50,7 +50,7 @@ public final class TmchDnlAction implements Runnable {
public void run() { public void run() {
List<String> lines; List<String> lines;
try { 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) { } catch (SignatureException | IOException | PGPException e) {
throw new RuntimeException(e); throw new RuntimeException(e);
} }

View file

@ -42,7 +42,7 @@ public final class TmchSmdrlAction implements Runnable {
private static final String SMDRL_SIG_PATH = "/smdrl/smdrl-latest.sig"; private static final String SMDRL_SIG_PATH = "/smdrl/smdrl-latest.sig";
@Inject Marksdb marksdb; @Inject Marksdb marksdb;
@Inject @Key("marksdbSmdrlLogin") Optional<String> marksdbSmdrlLogin; @Inject @Key("marksdbSmdrlLoginAndPassword") Optional<String> marksdbSmdrlLoginAndPassword;
@Inject TmchSmdrlAction() {} @Inject TmchSmdrlAction() {}
/** Synchronously fetches latest signed mark revocation list and saves it to Datastore. */ /** 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() { public void run() {
List<String> lines; List<String> lines;
try { 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) { } catch (SignatureException | IOException | PGPException e) {
throw new RuntimeException(e); throw new RuntimeException(e);
} }

View file

@ -74,14 +74,14 @@ final class GetKeyringSecretCommand implements CommandWithRemoteApi {
case JSON_CREDENTIAL: case JSON_CREDENTIAL:
out.write(KeySerializer.serializeString(keyring.getJsonCredential())); out.write(KeySerializer.serializeString(keyring.getJsonCredential()));
break; break;
case MARKSDB_DNL_LOGIN: case MARKSDB_DNL_LOGIN_AND_PASSWORD:
out.write(KeySerializer.serializeString(keyring.getMarksdbDnlLogin())); out.write(KeySerializer.serializeString(keyring.getMarksdbDnlLoginAndPassword()));
break; break;
case MARKSDB_LORDN_PASSWORD: case MARKSDB_LORDN_PASSWORD:
out.write(KeySerializer.serializeString(keyring.getMarksdbLordnPassword())); out.write(KeySerializer.serializeString(keyring.getMarksdbLordnPassword()));
break; break;
case MARKSDB_SMDRL_LOGIN: case MARKSDB_SMDRL_LOGIN_AND_PASSWORD:
out.write(KeySerializer.serializeString(keyring.getMarksdbSmdrlLogin())); out.write(KeySerializer.serializeString(keyring.getMarksdbSmdrlLoginAndPassword()));
break; break;
case RDE_RECEIVER_PUBLIC_KEY: case RDE_RECEIVER_PUBLIC_KEY:
out.write(KeySerializer.serializePublicKey(keyring.getRdeReceiverKey())); out.write(KeySerializer.serializePublicKey(keyring.getRdeReceiverKey()));

View file

@ -71,14 +71,14 @@ final class UpdateKmsKeyringCommand implements CommandWithRemoteApi {
case JSON_CREDENTIAL: case JSON_CREDENTIAL:
kmsUpdater.setJsonCredential(deserializeString(input)); kmsUpdater.setJsonCredential(deserializeString(input));
break; break;
case MARKSDB_DNL_LOGIN: case MARKSDB_DNL_LOGIN_AND_PASSWORD:
kmsUpdater.setMarksdbDnlLogin(deserializeString(input)); kmsUpdater.setMarksdbDnlLoginAndPassword(deserializeString(input));
break; break;
case MARKSDB_LORDN_PASSWORD: case MARKSDB_LORDN_PASSWORD:
kmsUpdater.setMarksdbLordnPassword(deserializeString(input)); kmsUpdater.setMarksdbLordnPassword(deserializeString(input));
break; break;
case MARKSDB_SMDRL_LOGIN: case MARKSDB_SMDRL_LOGIN_AND_PASSWORD:
kmsUpdater.setMarksdbSmdrlLogin(deserializeString(input)); kmsUpdater.setMarksdbSmdrlLoginAndPassword(deserializeString(input));
break; break;
case RDE_RECEIVER_PUBLIC_KEY: case RDE_RECEIVER_PUBLIC_KEY:
kmsUpdater.setRdeReceiverPublicKey(deserializePublicKey(input)); kmsUpdater.setRdeReceiverPublicKey(deserializePublicKey(input));

View file

@ -26,9 +26,9 @@ public enum KeyringKeyName {
BRDA_SIGNING_PUBLIC_KEY, BRDA_SIGNING_PUBLIC_KEY,
ICANN_REPORTING_PASSWORD, ICANN_REPORTING_PASSWORD,
JSON_CREDENTIAL, JSON_CREDENTIAL,
MARKSDB_DNL_LOGIN, MARKSDB_DNL_LOGIN_AND_PASSWORD,
MARKSDB_LORDN_PASSWORD, MARKSDB_LORDN_PASSWORD,
MARKSDB_SMDRL_LOGIN, MARKSDB_SMDRL_LOGIN_AND_PASSWORD,
RDE_RECEIVER_PUBLIC_KEY, RDE_RECEIVER_PUBLIC_KEY,
RDE_SIGNING_KEY_PAIR, RDE_SIGNING_KEY_PAIR,
RDE_SIGNING_PUBLIC_KEY, RDE_SIGNING_PUBLIC_KEY,

View file

@ -137,12 +137,12 @@ public class KmsKeyringTest {
} }
@Test @Test
public void test_getMarksdbDnlLogin() { public void test_getMarksdbDnlLoginAndPassword() {
saveCleartextSecret("marksdb-dnl-login-string"); 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 @Test
@ -155,12 +155,12 @@ public class KmsKeyringTest {
} }
@Test @Test
public void test_getMarksdbSmdrlLogin() { public void test_getMarksdbSmdrlLoginAndPassword() {
saveCleartextSecret("marksdb-smdrl-login-string"); 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");
} }

View file

@ -50,7 +50,7 @@ public class KmsUpdaterTest {
@Test @Test
public void test_setMultipleSecrets() { public void test_setMultipleSecrets() {
updater updater
.setMarksdbDnlLogin("value1") .setMarksdbDnlLoginAndPassword("value1")
.setIcannReportingPassword("value2") .setIcannReportingPassword("value2")
.setJsonCredential("value3") .setJsonCredential("value3")
.update(); .update();
@ -110,8 +110,8 @@ public class KmsUpdaterTest {
} }
@Test @Test
public void test_setMarksdbDnlLogin() { public void test_setMarksdbDnlLoginAndPassword() {
updater.setMarksdbDnlLogin("value1").update(); updater.setMarksdbDnlLoginAndPassword("value1").update();
verifySecretAndSecretRevisionWritten( verifySecretAndSecretRevisionWritten(
"marksdb-dnl-login-string", "marksdb-dnl-login-string/foo", getCiphertext("value1")); "marksdb-dnl-login-string", "marksdb-dnl-login-string/foo", getCiphertext("value1"));
@ -128,8 +128,8 @@ public class KmsUpdaterTest {
} }
@Test @Test
public void test_setMarksdbSmdrlLogin() { public void test_setMarksdbSmdrlLoginAndPassword() {
updater.setMarksdbSmdrlLogin("value1").update(); updater.setMarksdbSmdrlLoginAndPassword("value1").update();
verifySecretAndSecretRevisionWritten( verifySecretAndSecretRevisionWritten(
"marksdb-smdrl-login-string", "marksdb-smdrl-login-string/foo", getCiphertext("value1")); "marksdb-smdrl-login-string", "marksdb-smdrl-login-string/foo", getCiphertext("value1"));

View file

@ -52,9 +52,9 @@ public final class FakeKeyringModule {
loadBytes(FakeKeyringModule.class, "pgp-private-keyring-registry.asc"); loadBytes(FakeKeyringModule.class, "pgp-private-keyring-registry.asc");
private static final String ICANN_REPORTING_PASSWORD = "yolo"; private static final String ICANN_REPORTING_PASSWORD = "yolo";
private static final String SAFE_BROWSING_API_KEY = "a/b_c"; 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_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"; private static final String JSON_CREDENTIAL = "json123";
@Provides @Provides
@ -111,8 +111,8 @@ public final class FakeKeyringModule {
} }
@Override @Override
public String getMarksdbSmdrlLogin() { public String getMarksdbSmdrlLoginAndPassword() {
return MARKSDB_SMDRL_LOGIN; return MARKSDB_SMDRL_LOGIN_AND_PASSWORD;
} }
@Override @Override
@ -121,8 +121,8 @@ public final class FakeKeyringModule {
} }
@Override @Override
public String getMarksdbDnlLogin() { public String getMarksdbDnlLoginAndPassword() {
return MARKSDB_DNL_LOGIN; return MARKSDB_DNL_LOGIN_AND_PASSWORD;
} }
@Override @Override

View file

@ -37,7 +37,7 @@ import org.mockito.Mock;
@RunWith(JUnit4.class) @RunWith(JUnit4.class)
public class TmchActionTestCase { 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"; static final String MARKSDB_URL = "http://127.0.0.1/love";
@Rule public final AppEngineRule appEngine = AppEngineRule.builder().withDatastore().build(); @Rule public final AppEngineRule appEngine = AppEngineRule.builder().withDatastore().build();

View file

@ -31,7 +31,7 @@ public class TmchDnlActionTest extends TmchActionTestCase {
private TmchDnlAction newTmchDnlAction() { private TmchDnlAction newTmchDnlAction() {
TmchDnlAction action = new TmchDnlAction(); TmchDnlAction action = new TmchDnlAction();
action.marksdb = marksdb; action.marksdb = marksdb;
action.marksdbDnlLogin = Optional.of(MARKSDB_LOGIN); action.marksdbDnlLoginAndPassword = Optional.of(MARKSDB_LOGIN_AND_PASSWORD);
return action; return action;
} }

View file

@ -33,7 +33,7 @@ public class TmchSmdrlActionTest extends TmchActionTestCase {
private TmchSmdrlAction newTmchSmdrlAction() { private TmchSmdrlAction newTmchSmdrlAction() {
TmchSmdrlAction action = new TmchSmdrlAction(); TmchSmdrlAction action = new TmchSmdrlAction();
action.marksdb = marksdb; action.marksdb = marksdb;
action.marksdbSmdrlLogin = Optional.of("username:password"); action.marksdbSmdrlLoginAndPassword = Optional.of("username:password");
return action; return action;
} }