From 3fa56dec45f98c80b9e7fef59913e59bc8cd602a Mon Sep 17 00:00:00 2001 From: Weimin Yu Date: Tue, 1 Jun 2021 15:28:22 -0400 Subject: [PATCH] Make keyring use SecretManager as sole storage (#1185) * Make keyring use SecretManager as sole storage The Keyring will only use the SecretManager as storage. Accesses to the Datastore are removed. Also consolidated KmsKeyringTest into KmsKeyingUpdaterTest. The latter is left with its original name to facilitate code reviews. It will be renamed in planned cleanups. Additional cleanup is left for a future PR. These include: - Remove KmsConnection and its associated injection modules - Remove KmsSecretRevision from SQL schema and code - Rename relevant files to more appropriate names. --- .../registry/keyring/kms/KmsKeyring.java | 56 +--- .../registry/keyring/kms/KmsUpdater.java | 54 +--- .../keyring/kms/FakeKmsConnection.java | 45 --- .../registry/keyring/kms/KmsKeyringTest.java | 187 ------------ .../registry/keyring/kms/KmsTestHelper.java | 2 +- .../registry/keyring/kms/KmsUpdaterTest.java | 267 ++++++++---------- 6 files changed, 118 insertions(+), 493 deletions(-) delete mode 100644 core/src/test/java/google/registry/keyring/kms/FakeKmsConnection.java delete mode 100644 core/src/test/java/google/registry/keyring/kms/KmsKeyringTest.java diff --git a/core/src/main/java/google/registry/keyring/kms/KmsKeyring.java b/core/src/main/java/google/registry/keyring/kms/KmsKeyring.java index 26e906081..82e9d800b 100644 --- a/core/src/main/java/google/registry/keyring/kms/KmsKeyring.java +++ b/core/src/main/java/google/registry/keyring/kms/KmsKeyring.java @@ -16,24 +16,12 @@ package google.registry.keyring.kms; import static com.google.common.base.CaseFormat.LOWER_HYPHEN; import static com.google.common.base.CaseFormat.UPPER_UNDERSCORE; -import static com.google.common.base.Preconditions.checkState; -import static google.registry.model.common.EntityGroupRoot.getCrossTldKey; -import static google.registry.model.ofy.ObjectifyService.auditedOfy; -import static google.registry.persistence.transaction.TransactionManagerFactory.tm; -import com.google.common.flogger.FluentLogger; -import com.googlecode.objectify.Key; -import google.registry.config.RegistryConfig.Config; import google.registry.keyring.api.KeySerializer; import google.registry.keyring.api.Keyring; import google.registry.keyring.api.KeyringException; -import google.registry.model.server.KmsSecret; -import google.registry.model.server.KmsSecretRevision; -import google.registry.model.server.KmsSecretRevisionSqlDao; import google.registry.privileges.secretmanager.KeyringSecretStore; import java.io.IOException; -import java.util.Arrays; -import java.util.Optional; import javax.inject.Inject; import org.bouncycastle.openpgp.PGPException; import org.bouncycastle.openpgp.PGPKeyPair; @@ -47,11 +35,9 @@ import org.bouncycastle.openpgp.PGPPublicKey; * @see Google Cloud Key Management Service * Documentation */ -// TODO(2021-06-01): rename this class to SecretManagerKeyring and delete KmsSecretRevision +// TODO(2021-07-01): rename this class to SecretManagerKeyring and delete KmsSecretRevision public class KmsKeyring implements Keyring { - private static final FluentLogger logger = FluentLogger.forEnclosingClass(); - /** Key labels for private key secrets. */ enum PrivateKeyLabel { BRDA_SIGNING_PRIVATE, @@ -92,13 +78,10 @@ public class KmsKeyring implements Keyring { } } - private final KmsConnection kmsConnection; private final KeyringSecretStore secretStore; @Inject - KmsKeyring( - @Config("defaultKmsConnection") KmsConnection kmsConnection, KeyringSecretStore secretStore) { - this.kmsConnection = kmsConnection; + KmsKeyring(KeyringSecretStore secretStore) { this.secretStore = secretStore; } @@ -201,44 +184,11 @@ public class KmsKeyring implements Keyring { return getKeyPair(keyLabel).getPrivateKey(); } - private byte[] getDecryptedDataFromDatastore(String keyName) { - String encryptedData; - if (tm().isOfy()) { - KmsSecret secret = - auditedOfy().load().key(Key.create(getCrossTldKey(), KmsSecret.class, keyName)).now(); - checkState(secret != null, "Requested secret '%s' does not exist.", keyName); - encryptedData = auditedOfy().load().key(secret.getLatestRevision()).now().getEncryptedValue(); - } else { - Optional revision = - tm().transact(() -> KmsSecretRevisionSqlDao.getLatestRevision(keyName)); - checkState(revision.isPresent(), "Requested secret '%s' does not exist.", keyName); - encryptedData = revision.get().getEncryptedValue(); - } - - try { - return kmsConnection.decrypt(keyName, encryptedData); - } catch (Exception e) { - return new byte[0]; - } - } - - private byte[] getDataFromSecretStore(String keyName) { + private byte[] getDecryptedData(String keyName) { try { return secretStore.getSecret(keyName); } catch (Exception e) { throw new KeyringException("Failed to retrieve secret for " + keyName, e); } } - - private byte[] getDecryptedData(String keyName) { - byte[] dsData = getDecryptedDataFromDatastore(keyName); - byte[] secretStoreData = getDataFromSecretStore(keyName); - - if (Arrays.equals(dsData, secretStoreData)) { - logger.atInfo().log("Values for %s in Datastore and Secret Manager match.", keyName); - return secretStoreData; - } - logger.atWarning().log("Values for %s in Datastore and Secret Manager do not match.", keyName); - return secretStoreData; - } } diff --git a/core/src/main/java/google/registry/keyring/kms/KmsUpdater.java b/core/src/main/java/google/registry/keyring/kms/KmsUpdater.java index 816a68e55..fcc3411a0 100644 --- a/core/src/main/java/google/registry/keyring/kms/KmsUpdater.java +++ b/core/src/main/java/google/registry/keyring/kms/KmsUpdater.java @@ -32,18 +32,13 @@ 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.persistence.transaction.TransactionManagerFactory.tm; import static google.registry.util.PreconditionsUtils.checkArgumentNotNull; -import com.google.common.collect.ImmutableMap; import com.google.common.flogger.FluentLogger; -import google.registry.config.RegistryConfig.Config; import google.registry.keyring.api.KeySerializer; import google.registry.keyring.kms.KmsKeyring.PrivateKeyLabel; import google.registry.keyring.kms.KmsKeyring.PublicKeyLabel; import google.registry.keyring.kms.KmsKeyring.StringKeyLabel; -import google.registry.model.server.KmsSecret; -import google.registry.model.server.KmsSecretRevision; import google.registry.privileges.secretmanager.KeyringSecretStore; import java.io.IOException; import java.util.HashMap; @@ -62,14 +57,11 @@ import org.bouncycastle.openpgp.PGPPublicKey; public final class KmsUpdater { private static final FluentLogger logger = FluentLogger.forEnclosingClass(); - private final KmsConnection kmsConnection; private final KeyringSecretStore secretStore; private final HashMap secretValues; @Inject - public KmsUpdater( - @Config("defaultKmsConnection") KmsConnection kmsConnection, KeyringSecretStore secretStore) { - this.kmsConnection = kmsConnection; + public KmsUpdater(KeyringSecretStore secretStore) { this.secretStore = secretStore; // Use LinkedHashMap to preserve insertion order on update() to simplify testing and debugging @@ -150,24 +142,6 @@ public final class KmsUpdater { + "Please check the status of Secret Manager and re-run the command.", e); } - - // TODO(2021-06-01): remove the writes to Datastore - persistEncryptedValues(encryptValues(secretValues)); - } - - /** - * Encrypts updated secrets using KMS. If the configured {@code KeyRing} or {@code CryptoKey} - * associated with a secret doesn't exist, they will first be created. - * - * @see google.registry.config.RegistryConfigSettings.Kms - */ - private ImmutableMap encryptValues(Map keyValues) { - ImmutableMap.Builder encryptedValues = new ImmutableMap.Builder<>(); - for (Map.Entry entry : keyValues.entrySet()) { - String secretName = entry.getKey(); - encryptedValues.put(secretName, kmsConnection.encrypt(secretName, entry.getValue())); - } - return encryptedValues.build(); } private KmsUpdater setString(String key, StringKeyLabel stringKeyLabel) { @@ -195,32 +169,6 @@ public final class KmsUpdater { return this; } - /** - * Persists encrypted secrets to Datastore as {@link KmsSecretRevision} entities and makes them - * primary. {@link KmsSecret} entities point to the latest {@link KmsSecretRevision}. - * - *

The changes are committed transactionally; if an error occurs, all existing {@link - * KmsSecretRevision} entities will remain primary. - */ - private static void persistEncryptedValues( - final ImmutableMap encryptedValues) { - tm().transact( - () -> { - for (Map.Entry entry : encryptedValues.entrySet()) { - String secretName = entry.getKey(); - EncryptResponse revisionData = entry.getValue(); - - KmsSecretRevision secretRevision = - new KmsSecretRevision.Builder() - .setEncryptedValue(revisionData.ciphertext()) - .setKmsCryptoKeyVersionName(revisionData.cryptoKeyVersionName()) - .setParent(secretName) - .build(); - tm().putAll(secretRevision, KmsSecret.create(secretName, secretRevision)); - } - }); - } - private void setSecret(String secretName, byte[] value) { checkArgument(!secretValues.containsKey(secretName), "Attempted to set %s twice", secretName); secretValues.put(secretName, value); diff --git a/core/src/test/java/google/registry/keyring/kms/FakeKmsConnection.java b/core/src/test/java/google/registry/keyring/kms/FakeKmsConnection.java deleted file mode 100644 index 5508b417f..000000000 --- a/core/src/test/java/google/registry/keyring/kms/FakeKmsConnection.java +++ /dev/null @@ -1,45 +0,0 @@ -// Copyright 2017 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.keyring.kms; - -import com.google.common.io.BaseEncoding; -import org.bouncycastle.util.Arrays; - -class FakeKmsConnection implements KmsConnection { - - FakeKmsConnection() {} - - /** - * Returns a dummy {@link EncryptResponse}. - * - *

The "encrypted value" in the response is the provided value reversed and then base64-encoded - * and the name of the cryptoKeyVersion is {@code cryptoKeyName + "/foo"}. - */ - @Override - public EncryptResponse encrypt(String cryptoKeyName, byte[] plaintext) { - return EncryptResponse.create( - BaseEncoding.base64().encode(Arrays.reverse(plaintext)), cryptoKeyName + "/foo"); - } - - /** - * Returns a "decrypted" plaintext. - * - *

The plaintext is the encodedCiphertext base64-decoded and then reversed. - */ - @Override - public byte[] decrypt(String cryptoKeyName, String encodedCiphertext) { - return Arrays.reverse(BaseEncoding.base64().decode(encodedCiphertext)); - } -} diff --git a/core/src/test/java/google/registry/keyring/kms/KmsKeyringTest.java b/core/src/test/java/google/registry/keyring/kms/KmsKeyringTest.java deleted file mode 100644 index b00ba18fc..000000000 --- a/core/src/test/java/google/registry/keyring/kms/KmsKeyringTest.java +++ /dev/null @@ -1,187 +0,0 @@ -// Copyright 2017 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.keyring.kms; - -import static com.google.common.truth.Truth.assertThat; -import static google.registry.persistence.transaction.TransactionManagerFactory.tm; - -import google.registry.keyring.api.KeySerializer; -import google.registry.model.server.KmsSecret; -import google.registry.model.server.KmsSecretRevision; -import google.registry.privileges.secretmanager.FakeSecretManagerClient; -import google.registry.privileges.secretmanager.KeyringSecretStore; -import google.registry.testing.AppEngineExtension; -import google.registry.testing.BouncyCastleProviderExtension; -import google.registry.testing.DualDatabaseTest; -import google.registry.testing.TestOfyAndSql; -import org.bouncycastle.openpgp.PGPKeyPair; -import org.bouncycastle.openpgp.PGPPrivateKey; -import org.bouncycastle.openpgp.PGPPublicKey; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.extension.RegisterExtension; - -/** Unit tests for {@link KmsKeyring}. */ -@DualDatabaseTest -class KmsKeyringTest { - - @RegisterExtension - final BouncyCastleProviderExtension bouncy = new BouncyCastleProviderExtension(); - - @RegisterExtension - final AppEngineExtension appEngine = - AppEngineExtension.builder().withDatastoreAndCloudSql().build(); - - private KmsKeyring keyring; - private KeyringSecretStore fakeSecretStore = - new KeyringSecretStore(new FakeSecretManagerClient()); - - @BeforeEach - void beforeEach() { - keyring = new KmsKeyring(new FakeKmsConnection(), fakeSecretStore); - } - - @TestOfyAndSql - void test_getRdeSigningKey() throws Exception { - saveKeyPairSecret("rde-signing-public", "rde-signing-private"); - PGPKeyPair rdeSigningKey = keyring.getRdeSigningKey(); - assertThat(KeySerializer.serializeKeyPair(rdeSigningKey)) - .isEqualTo(KeySerializer.serializeKeyPair(KmsTestHelper.getKeyPair())); - } - - @TestOfyAndSql - void test_getRdeStagingEncryptionKey() throws Exception { - savePublicKeySecret("rde-staging-public"); - PGPPublicKey rdeStagingEncryptionKey = keyring.getRdeStagingEncryptionKey(); - assertThat(rdeStagingEncryptionKey.getFingerprint()) - .isEqualTo(KmsTestHelper.getPublicKey().getFingerprint()); - } - - @TestOfyAndSql - void test_getRdeStagingDecryptionKey() throws Exception { - savePrivateKeySecret("rde-staging-private"); - savePublicKeySecret("rde-staging-public"); - - PGPPrivateKey rdeStagingDecryptionKey = keyring.getRdeStagingDecryptionKey(); - PGPPublicKey rdeStagingEncryptionKey = keyring.getRdeStagingEncryptionKey(); - PGPKeyPair keyPair = new PGPKeyPair(rdeStagingEncryptionKey, rdeStagingDecryptionKey); - - assertThat(KeySerializer.serializeKeyPair(keyPair)) - .isEqualTo(KeySerializer.serializeKeyPair(KmsTestHelper.getKeyPair())); - } - - @TestOfyAndSql - void test_getRdeReceiverKey() throws Exception { - savePublicKeySecret("rde-receiver-public"); - PGPPublicKey rdeReceiverKey = keyring.getRdeReceiverKey(); - assertThat(rdeReceiverKey.getFingerprint()) - .isEqualTo(KmsTestHelper.getPublicKey().getFingerprint()); - } - - @TestOfyAndSql - void test_getBrdaSigningKey() throws Exception { - saveKeyPairSecret("brda-signing-public", "brda-signing-private"); - PGPKeyPair brdaSigningKey = keyring.getBrdaSigningKey(); - assertThat(KeySerializer.serializeKeyPair(brdaSigningKey)) - .isEqualTo(KeySerializer.serializeKeyPair(KmsTestHelper.getKeyPair())); - } - - @TestOfyAndSql - void test_getBrdaReceiverKey() throws Exception { - savePublicKeySecret("brda-receiver-public"); - PGPPublicKey brdaReceiverKey = keyring.getBrdaReceiverKey(); - assertThat(brdaReceiverKey.getFingerprint()) - .isEqualTo(KmsTestHelper.getPublicKey().getFingerprint()); - } - - @TestOfyAndSql - void test_getRdeSshClientPublicKey() { - saveCleartextSecret("rde-ssh-client-public-string"); - String rdeSshClientPublicKey = keyring.getRdeSshClientPublicKey(); - assertThat(rdeSshClientPublicKey).isEqualTo("rde-ssh-client-public-stringmoo"); - } - - @TestOfyAndSql - void test_getRdeSshClientPrivateKey() { - saveCleartextSecret("rde-ssh-client-private-string"); - String rdeSshClientPrivateKey = keyring.getRdeSshClientPrivateKey(); - assertThat(rdeSshClientPrivateKey).isEqualTo("rde-ssh-client-private-stringmoo"); - } - - @TestOfyAndSql - void test_getIcannReportingPassword() { - saveCleartextSecret("icann-reporting-password-string"); - String icannReportingPassword = keyring.getIcannReportingPassword(); - assertThat(icannReportingPassword).isEqualTo("icann-reporting-password-stringmoo"); - } - - @TestOfyAndSql - void test_getMarksdbDnlLoginAndPassword() { - saveCleartextSecret("marksdb-dnl-login-string"); - String marksdbDnlLoginAndPassword = keyring.getMarksdbDnlLoginAndPassword(); - assertThat(marksdbDnlLoginAndPassword).isEqualTo("marksdb-dnl-login-stringmoo"); - } - - @TestOfyAndSql - void test_getMarksdbLordnPassword() { - saveCleartextSecret("marksdb-lordn-password-string"); - String marksdbLordnPassword = keyring.getMarksdbLordnPassword(); - assertThat(marksdbLordnPassword).isEqualTo("marksdb-lordn-password-stringmoo"); - } - - @TestOfyAndSql - void test_getMarksdbSmdrlLoginAndPassword() { - saveCleartextSecret("marksdb-smdrl-login-string"); - String marksdbSmdrlLoginAndPassword = keyring.getMarksdbSmdrlLoginAndPassword(); - assertThat(marksdbSmdrlLoginAndPassword).isEqualTo("marksdb-smdrl-login-stringmoo"); - } - - @TestOfyAndSql - void test_getJsonCredential() { - saveCleartextSecret("json-credential-string"); - String jsonCredential = keyring.getJsonCredential(); - assertThat(jsonCredential).isEqualTo("json-credential-stringmoo"); - } - - private void persistSecret(String secretName, byte[] secretValue) { - KmsConnection kmsConnection = new FakeKmsConnection(); - - KmsSecretRevision secretRevision = - new KmsSecretRevision.Builder() - .setEncryptedValue(kmsConnection.encrypt(secretName, secretValue).ciphertext()) - .setKmsCryptoKeyVersionName(KmsTestHelper.DUMMY_CRYPTO_KEY_VERSION) - .setParent(secretName) - .build(); - KmsSecret secret = KmsSecret.create(secretName, secretRevision); - tm().transact(() -> tm().putAll(secretRevision, secret)); - fakeSecretStore.createOrUpdateSecret(secretName, secretValue); - } - - private void saveCleartextSecret(String secretName) { - persistSecret(secretName, KeySerializer.serializeString(secretName + "moo")); - } - - private void savePublicKeySecret(String publicKeyName) throws Exception { - persistSecret(publicKeyName, KeySerializer.serializePublicKey(KmsTestHelper.getPublicKey())); - } - - private void savePrivateKeySecret(String privateKeyName) throws Exception { - persistSecret(privateKeyName, KeySerializer.serializeKeyPair(KmsTestHelper.getKeyPair())); - } - - private void saveKeyPairSecret(String publicKeyName, String privateKeyName) throws Exception { - savePublicKeySecret(publicKeyName); - savePrivateKeySecret(privateKeyName); - } -} diff --git a/core/src/test/java/google/registry/keyring/kms/KmsTestHelper.java b/core/src/test/java/google/registry/keyring/kms/KmsTestHelper.java index 8ed95946e..bd8281023 100644 --- a/core/src/test/java/google/registry/keyring/kms/KmsTestHelper.java +++ b/core/src/test/java/google/registry/keyring/kms/KmsTestHelper.java @@ -26,7 +26,7 @@ import org.bouncycastle.openpgp.bc.BcPGPSecretKeyRing; import org.bouncycastle.openpgp.operator.bc.BcPBESecretKeyDecryptorBuilder; import org.bouncycastle.openpgp.operator.bc.BcPGPDigestCalculatorProvider; -/** Stores dummy values for test use in {@link KmsUpdaterTest} and {@link KmsKeyringTest}. */ +/** Stores dummy values for test use in {@link KmsUpdaterTest}. */ final class KmsTestHelper { static final String DUMMY_CRYPTO_KEY_VERSION = "cheeseburger"; diff --git a/core/src/test/java/google/registry/keyring/kms/KmsUpdaterTest.java b/core/src/test/java/google/registry/keyring/kms/KmsUpdaterTest.java index 224db7b53..db8ce0fba 100644 --- a/core/src/test/java/google/registry/keyring/kms/KmsUpdaterTest.java +++ b/core/src/test/java/google/registry/keyring/kms/KmsUpdaterTest.java @@ -15,226 +15,185 @@ package google.registry.keyring.kms; import static com.google.common.truth.Truth.assertThat; -import static google.registry.model.common.EntityGroupRoot.getCrossTldKey; -import static google.registry.persistence.transaction.TransactionManagerFactory.tm; -import com.googlecode.objectify.Key; import google.registry.keyring.api.KeySerializer; -import google.registry.model.server.KmsSecret; -import google.registry.model.server.KmsSecretRevision; -import google.registry.model.server.KmsSecretRevisionSqlDao; -import google.registry.persistence.VKey; import google.registry.privileges.secretmanager.FakeSecretManagerClient; import google.registry.privileges.secretmanager.KeyringSecretStore; -import google.registry.testing.AppEngineExtension; import google.registry.testing.BouncyCastleProviderExtension; -import google.registry.testing.DualDatabaseTest; -import google.registry.testing.TestOfyAndSql; import java.io.IOException; +import java.nio.charset.StandardCharsets; import org.bouncycastle.openpgp.PGPKeyPair; import org.bouncycastle.openpgp.PGPPublicKey; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; -/** Unit tests for {@link KmsUpdater} */ -@DualDatabaseTest +/** Unit tests for {@link KmsKeyring} and {@link KmsUpdater} */ +// TODO(2021-07-01): Rename this class along with KmsKeyring public class KmsUpdaterTest { - @RegisterExtension - public final AppEngineExtension appEngine = - AppEngineExtension.builder().withDatastoreAndCloudSql().build(); - @RegisterExtension public final BouncyCastleProviderExtension bouncy = new BouncyCastleProviderExtension(); + private KeyringSecretStore secretStore; private KmsUpdater updater; + private KmsKeyring keyring; @BeforeEach void beforeEach() { - updater = - new KmsUpdater( - new FakeKmsConnection(), new KeyringSecretStore(new FakeSecretManagerClient())); + secretStore = new KeyringSecretStore(new FakeSecretManagerClient()); + updater = new KmsUpdater(secretStore); + keyring = new KmsKeyring(secretStore); } - @TestOfyAndSql - void test_setMultipleSecrets() { + @Test + void setAndReadMultiple() { + String secretPrefix = "setAndReadMultiple_"; updater - .setMarksdbDnlLoginAndPassword("value1") - .setIcannReportingPassword("value2") - .setJsonCredential("value3") + .setMarksdbDnlLoginAndPassword(secretPrefix + "marksdb") + .setIcannReportingPassword(secretPrefix + "icann") + .setJsonCredential(secretPrefix + "json") .update(); - verifySecretAndSecretRevisionWritten( - "marksdb-dnl-login-string", - "marksdb-dnl-login-string/foo", - getCiphertext("value1")); - verifySecretAndSecretRevisionWritten( - "icann-reporting-password-string", - "icann-reporting-password-string/foo", - getCiphertext("value2")); - verifySecretAndSecretRevisionWritten( - "json-credential-string", "json-credential-string/foo", getCiphertext("value3")); + assertThat(keyring.getMarksdbDnlLoginAndPassword()).isEqualTo(secretPrefix + "marksdb"); + assertThat(keyring.getIcannReportingPassword()).isEqualTo(secretPrefix + "icann"); + assertThat(keyring.getJsonCredential()).isEqualTo(secretPrefix + "json"); + + verifyPersistedSecret("marksdb-dnl-login-string", secretPrefix + "marksdb"); + verifyPersistedSecret("icann-reporting-password-string", secretPrefix + "icann"); + verifyPersistedSecret("json-credential-string", secretPrefix + "json"); } - @TestOfyAndSql - void test_setBrdaReceiverKey() throws Exception { - updater.setBrdaReceiverPublicKey(KmsTestHelper.getPublicKey()).update(); + @Test + void brdaReceiverKey() throws Exception { + PGPPublicKey publicKey = KmsTestHelper.getPublicKey(); + updater.setBrdaReceiverPublicKey(publicKey).update(); - verifySecretAndSecretRevisionWritten( - "brda-receiver-public", - "brda-receiver-public/foo", - getCiphertext(KmsTestHelper.getPublicKey())); + assertThat(keyring.getBrdaReceiverKey().getFingerprint()).isEqualTo(publicKey.getFingerprint()); + verifyPersistedSecret("brda-receiver-public", serializePublicKey(publicKey)); } - @TestOfyAndSql - void test_setBrdaSigningKey() throws Exception { - updater.setBrdaSigningKey(KmsTestHelper.getKeyPair()).update(); + @Test + void brdaSigningKey() throws Exception { + PGPKeyPair keyPair = KmsTestHelper.getKeyPair(); + updater.setBrdaSigningKey(keyPair).update(); - verifySecretAndSecretRevisionWritten( - "brda-signing-private", - "brda-signing-private/foo", - getCiphertext(KmsTestHelper.getKeyPair())); - verifySecretAndSecretRevisionWritten( - "brda-signing-public", - "brda-signing-public/foo", - getCiphertext(KmsTestHelper.getPublicKey())); + assertThat(serializeKeyPair(keyring.getBrdaSigningKey())).isEqualTo(serializeKeyPair(keyPair)); + verifyPersistedSecret("brda-signing-private", serializeKeyPair(KmsTestHelper.getKeyPair())); + verifyPersistedSecret("brda-signing-public", serializePublicKey(KmsTestHelper.getPublicKey())); } - @TestOfyAndSql - void test_setIcannReportingPassword() { - updater.setIcannReportingPassword("value1").update(); + @Test + void icannReportingPassword() { + String secret = "icannReportingPassword"; + updater.setIcannReportingPassword(secret).update(); - verifySecretAndSecretRevisionWritten( - "icann-reporting-password-string", - "icann-reporting-password-string/foo", - getCiphertext("value1")); + assertThat(keyring.getIcannReportingPassword()).isEqualTo(secret); + verifyPersistedSecret("icann-reporting-password-string", secret); } - @TestOfyAndSql - void test_setJsonCredential() { - updater.setJsonCredential("value1").update(); + @Test + void jsonCredential() { + String secret = "jsonCredential"; + updater.setJsonCredential(secret).update(); - verifySecretAndSecretRevisionWritten( - "json-credential-string", "json-credential-string/foo", getCiphertext("value1")); + assertThat(keyring.getJsonCredential()).isEqualTo(secret); + verifyPersistedSecret("json-credential-string", secret); } - @TestOfyAndSql - void test_setMarksdbDnlLoginAndPassword() { - updater.setMarksdbDnlLoginAndPassword("value1").update(); + @Test + void marksdbDnlLoginAndPassword() { + String secret = "marksdbDnlLoginAndPassword"; + updater.setMarksdbDnlLoginAndPassword(secret).update(); - verifySecretAndSecretRevisionWritten( - "marksdb-dnl-login-string", "marksdb-dnl-login-string/foo", getCiphertext("value1")); + assertThat(keyring.getMarksdbDnlLoginAndPassword()).isEqualTo(secret); + verifyPersistedSecret("marksdb-dnl-login-string", secret); } - @TestOfyAndSql - void test_setMarksdbLordnPassword() { - updater.setMarksdbLordnPassword("value1").update(); + @Test + void marksdbLordnPassword() { + String secret = "marksdbLordnPassword"; + updater.setMarksdbLordnPassword(secret).update(); - verifySecretAndSecretRevisionWritten( - "marksdb-lordn-password-string", - "marksdb-lordn-password-string/foo", - getCiphertext("value1")); + assertThat(keyring.getMarksdbLordnPassword()).isEqualTo(secret); + verifyPersistedSecret("marksdb-lordn-password-string", secret); } - @TestOfyAndSql - void test_setMarksdbSmdrlLoginAndPassword() { - updater.setMarksdbSmdrlLoginAndPassword("value1").update(); + @Test + void marksdbSmdrlLoginAndPassword() { + String secret = "marksdbSmdrlLoginAndPassword"; + updater.setMarksdbSmdrlLoginAndPassword(secret).update(); - verifySecretAndSecretRevisionWritten( - "marksdb-smdrl-login-string", "marksdb-smdrl-login-string/foo", getCiphertext("value1")); + assertThat(keyring.getMarksdbSmdrlLoginAndPassword()).isEqualTo(secret); + verifyPersistedSecret("marksdb-smdrl-login-string", secret); } - @TestOfyAndSql - void test_setRdeReceiverKey() throws Exception { - updater.setRdeReceiverPublicKey(KmsTestHelper.getPublicKey()).update(); + @Test + void rdeReceiverKey() throws Exception { + PGPPublicKey publicKey = KmsTestHelper.getPublicKey(); + updater.setRdeReceiverPublicKey(publicKey).update(); - verifySecretAndSecretRevisionWritten( - "rde-receiver-public", - "rde-receiver-public/foo", - getCiphertext( - KeySerializer.serializePublicKey(KmsTestHelper.getPublicKey()))); + assertThat(keyring.getRdeReceiverKey().getFingerprint()).isEqualTo(publicKey.getFingerprint()); + verifyPersistedSecret("rde-receiver-public", serializePublicKey(KmsTestHelper.getPublicKey())); } - @TestOfyAndSql - void test_setRdeSigningKey() throws Exception { - updater.setRdeSigningKey(KmsTestHelper.getKeyPair()).update(); + @Test + void rdeSigningKey() throws Exception { + PGPKeyPair keyPair = KmsTestHelper.getKeyPair(); + updater.setRdeSigningKey(keyPair).update(); - verifySecretAndSecretRevisionWritten( - "rde-signing-private", - "rde-signing-private/foo", - getCiphertext(KmsTestHelper.getKeyPair())); - verifySecretAndSecretRevisionWritten( - "rde-signing-public", - "rde-signing-public/foo", - getCiphertext(KmsTestHelper.getPublicKey())); + assertThat(serializeKeyPair(keyring.getRdeSigningKey())).isEqualTo(serializeKeyPair(keyPair)); + + verifyPersistedSecret("rde-signing-private", serializeKeyPair(keyPair)); + verifyPersistedSecret("rde-signing-public", serializePublicKey(keyPair.getPublicKey())); } - @TestOfyAndSql - void test_setRdeSshClientPrivateKey() { - updater.setRdeSshClientPrivateKey("value1").update(); + @Test + void rdeSshClientPrivateKey() { + String secret = "rdeSshClientPrivateKey"; + updater.setRdeSshClientPrivateKey(secret).update(); - verifySecretAndSecretRevisionWritten( - "rde-ssh-client-private-string", - "rde-ssh-client-private-string/foo", - getCiphertext("value1")); + assertThat(keyring.getRdeSshClientPrivateKey()).isEqualTo(secret); + verifyPersistedSecret("rde-ssh-client-private-string", secret); } - @TestOfyAndSql - void test_setRdeSshClientPublicKey() { - updater.setRdeSshClientPublicKey("value1").update(); + @Test + void rdeSshClientPublicKey() { + String secret = "rdeSshClientPublicKey"; + updater.setRdeSshClientPublicKey(secret).update(); - verifySecretAndSecretRevisionWritten( - "rde-ssh-client-public-string", - "rde-ssh-client-public-string/foo", - getCiphertext("value1")); + assertThat(keyring.getRdeSshClientPublicKey()).isEqualTo(secret); + verifyPersistedSecret("rde-ssh-client-public-string", secret); } - @TestOfyAndSql - void test_setRdeStagingKey() throws Exception { - updater.setRdeStagingKey(KmsTestHelper.getKeyPair()).update(); + @Test + void rdeStagingKey() throws Exception { + PGPKeyPair keyPair = KmsTestHelper.getKeyPair(); + updater.setRdeStagingKey(keyPair).update(); - verifySecretAndSecretRevisionWritten( - "rde-staging-private", - "rde-staging-private/foo", - getCiphertext(KmsTestHelper.getKeyPair())); - verifySecretAndSecretRevisionWritten( - "rde-staging-public", - "rde-staging-public/foo", - getCiphertext(KmsTestHelper.getPublicKey())); + assertThat(serializePublicKey(keyring.getRdeStagingEncryptionKey())) + .isEqualTo(serializePublicKey(keyPair.getPublicKey())); + // Since we do not have dedicated tools to compare private keys, we leverage key-pair + // serialization util to compare private keys. + assertThat( + serializeKeyPair( + new PGPKeyPair( + keyring.getRdeStagingEncryptionKey(), keyring.getRdeStagingDecryptionKey()))) + .isEqualTo(serializeKeyPair(keyPair)); + verifyPersistedSecret("rde-staging-private", serializeKeyPair(keyPair)); + verifyPersistedSecret("rde-staging-public", serializePublicKey(KmsTestHelper.getPublicKey())); } - private static void verifySecretAndSecretRevisionWritten( - String secretName, String expectedCryptoKeyVersionName, String expectedEncryptedValue) { - KmsSecretRevision secretRevision; - if (tm().isOfy()) { - KmsSecret secret = - tm().loadByKey( - VKey.createOfy( - KmsSecret.class, Key.create(getCrossTldKey(), KmsSecret.class, secretName))); - assertThat(secret).isNotNull(); - secretRevision = - tm().loadByKey(VKey.createOfy(KmsSecretRevision.class, secret.getLatestRevision())); - } else { - secretRevision = - tm().transact(() -> KmsSecretRevisionSqlDao.getLatestRevision(secretName).get()); - } - assertThat(secretRevision.getKmsCryptoKeyVersionName()).isEqualTo(expectedCryptoKeyVersionName); - assertThat(secretRevision.getEncryptedValue()).isEqualTo(expectedEncryptedValue); + private void verifyPersistedSecret(String secretName, String expectedPlainTextValue) { + assertThat(new String(secretStore.getSecret(secretName), StandardCharsets.UTF_8)) + .isEqualTo(expectedPlainTextValue); } - private static String getCiphertext(byte[] plaintext) { - return new FakeKmsConnection().encrypt("blah", plaintext).ciphertext(); + private static String serializePublicKey(PGPPublicKey publicKey) throws IOException { + return new String(KeySerializer.serializePublicKey(publicKey), StandardCharsets.UTF_8); } - private static String getCiphertext(String plaintext) { - return getCiphertext(KeySerializer.serializeString(plaintext)); - } - - private static String getCiphertext(PGPPublicKey publicKey) throws IOException { - return getCiphertext(KeySerializer.serializePublicKey(publicKey)); - } - - private static String getCiphertext(PGPKeyPair keyPair) throws Exception { - return getCiphertext(KeySerializer.serializeKeyPair(keyPair)); + private static String serializeKeyPair(PGPKeyPair keyPair) throws Exception { + return new String(KeySerializer.serializeKeyPair(keyPair), StandardCharsets.UTF_8); } }