From ab515cb3521dc27e690f93815bb8b34ec7bcd418 Mon Sep 17 00:00:00 2001 From: guyben Date: Tue, 11 Apr 2017 19:38:14 -0700 Subject: [PATCH] Replace KeystoreKeyring with KmsKeystore comparison Replace KeystoreKeyring with ComparatorKeyring between KeystoreKeyring and KmsKeystore. In the opensource version, will replace DummyKeyring with KmsKeyring directly. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=152893767 --- .../keyring/api/ComparatorKeyring.java | 12 ++++++- .../registry/keyring/kms/KeyringModule.java | 34 +++++++++++++++++++ java/google/registry/module/backend/BUILD | 1 + .../module/backend/BackendComponent.java | 6 ++-- java/google/registry/module/frontend/BUILD | 1 + .../module/frontend/FrontendComponent.java | 6 ++-- java/google/registry/module/tools/BUILD | 1 + .../registry/module/tools/ToolsComponent.java | 6 ++-- java/google/registry/tools/BUILD | 1 - .../tools/EscrowDepositEncryptor.java | 9 ++--- .../registry/tools/GhostrydeCommand.java | 12 ++++--- .../registry/tools/RegistryToolComponent.java | 6 ++-- .../util/ComparingInvocationHandler.java | 18 +++++----- .../EncryptEscrowDepositCommandTest.java | 4 +-- .../registry/tools/GhostrydeCommandTest.java | 5 +-- .../util/ComparingInvocationHandlerTest.java | 8 ++--- 16 files changed, 94 insertions(+), 36 deletions(-) create mode 100644 java/google/registry/keyring/kms/KeyringModule.java diff --git a/java/google/registry/keyring/api/ComparatorKeyring.java b/java/google/registry/keyring/api/ComparatorKeyring.java index 8b28ad55a..1fc1bc792 100644 --- a/java/google/registry/keyring/api/ComparatorKeyring.java +++ b/java/google/registry/keyring/api/ComparatorKeyring.java @@ -21,6 +21,8 @@ import google.registry.util.ComparingInvocationHandler; import google.registry.util.FormattingLogger; import java.io.IOException; +import java.io.PrintWriter; +import java.io.StringWriter; import java.lang.reflect.Method; import java.util.Arrays; import java.util.Objects; @@ -43,7 +45,7 @@ import org.bouncycastle.openpgp.PGPPublicKey; *

If both keyrings threw exceptions, there is no check whether the exeptions are the same. The * assumption is that an error happened in both, but they might report that error differently. */ -final class ComparatorKeyring extends ComparingInvocationHandler { +public final class ComparatorKeyring extends ComparingInvocationHandler { @VisibleForTesting static final FormattingLogger logger = FormattingLogger.getLoggerForCallerClass(); @@ -98,6 +100,14 @@ final class ComparatorKeyring extends ComparingInvocationHandler { return super.stringifyResult(method, a); } + @Override + protected String stringifyThrown(Method method, Throwable throwable) { + StringWriter stringWriter = new StringWriter(); + PrintWriter printWriter = new PrintWriter(stringWriter); + throwable.printStackTrace(printWriter); + return String.format("%s\nStack trace:\n%s", throwable.toString(), stringWriter.toString()); + } + // .equals implementation for PGP types. @VisibleForTesting diff --git a/java/google/registry/keyring/kms/KeyringModule.java b/java/google/registry/keyring/kms/KeyringModule.java new file mode 100644 index 000000000..317c4fd3e --- /dev/null +++ b/java/google/registry/keyring/kms/KeyringModule.java @@ -0,0 +1,34 @@ +// 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 dagger.Module; +import dagger.Provides; + +import google.registry.keyring.api.Keyring; + +import javax.inject.Singleton; + +/** Dagger module for {@link Keyring} */ +@Module +public final class KeyringModule { + + @Provides + @Singleton + // TODO(b/35810650): return kmsKeyring directly once comparison period is over. + public static Keyring provideKeyring(KmsKeyring kmsKeyring) { + return kmsKeyring; + } +} diff --git a/java/google/registry/module/backend/BUILD b/java/google/registry/module/backend/BUILD index be891abeb..980c1f8aa 100644 --- a/java/google/registry/module/backend/BUILD +++ b/java/google/registry/module/backend/BUILD @@ -23,6 +23,7 @@ java_library( "//java/google/registry/gcs", "//java/google/registry/groups", "//java/google/registry/keyring/api", + "//java/google/registry/keyring/kms", "//java/google/registry/mapreduce", "//java/google/registry/model", "//java/google/registry/monitoring/metrics", diff --git a/java/google/registry/module/backend/BackendComponent.java b/java/google/registry/module/backend/BackendComponent.java index 437e9cab5..0d76bfe25 100644 --- a/java/google/registry/module/backend/BackendComponent.java +++ b/java/google/registry/module/backend/BackendComponent.java @@ -24,8 +24,9 @@ import google.registry.gcs.GcsServiceModule; import google.registry.groups.DirectoryModule; import google.registry.groups.GroupsModule; import google.registry.groups.GroupssettingsModule; -import google.registry.keyring.api.DummyKeyringModule; import google.registry.keyring.api.KeyModule; +import google.registry.keyring.kms.KeyringModule; +import google.registry.keyring.kms.KmsModule; import google.registry.module.backend.BackendRequestComponent.BackendRequestComponentModule; import google.registry.monitoring.metrics.MetricReporter; import google.registry.monitoring.whitebox.StackdriverModule; @@ -56,7 +57,6 @@ import javax.inject.Singleton; DatastoreServiceModule.class, DirectoryModule.class, DriveModule.class, - DummyKeyringModule.class, GcsServiceModule.class, GoogleCredentialModule.class, GroupsModule.class, @@ -64,6 +64,8 @@ import javax.inject.Singleton; JSchModule.class, Jackson2Module.class, KeyModule.class, + KeyringModule.class, + KmsModule.class, ModulesServiceModule.class, SpreadsheetServiceModule.class, StackdriverModule.class, diff --git a/java/google/registry/module/frontend/BUILD b/java/google/registry/module/frontend/BUILD index bfe6b9658..a140ebd64 100644 --- a/java/google/registry/module/frontend/BUILD +++ b/java/google/registry/module/frontend/BUILD @@ -13,6 +13,7 @@ java_library( "//java/google/registry/dns", "//java/google/registry/flows", "//java/google/registry/keyring/api", + "//java/google/registry/keyring/kms", "//java/google/registry/monitoring/metrics", "//java/google/registry/monitoring/whitebox", "//java/google/registry/rdap", diff --git a/java/google/registry/module/frontend/FrontendComponent.java b/java/google/registry/module/frontend/FrontendComponent.java index 00e80b503..7aed016e6 100644 --- a/java/google/registry/module/frontend/FrontendComponent.java +++ b/java/google/registry/module/frontend/FrontendComponent.java @@ -19,8 +19,9 @@ import google.registry.braintree.BraintreeModule; import google.registry.config.RegistryConfig.ConfigModule; import google.registry.flows.ServerTridProviderModule; import google.registry.flows.custom.CustomLogicFactoryModule; -import google.registry.keyring.api.DummyKeyringModule; import google.registry.keyring.api.KeyModule; +import google.registry.keyring.kms.KeyringModule; +import google.registry.keyring.kms.KmsModule; import google.registry.module.frontend.FrontendRequestComponent.FrontendRequestComponentModule; import google.registry.monitoring.metrics.MetricReporter; import google.registry.monitoring.whitebox.StackdriverModule; @@ -46,11 +47,12 @@ import javax.inject.Singleton; ConfigModule.class, ConsoleConfigModule.class, CustomLogicFactoryModule.class, - DummyKeyringModule.class, FrontendMetricsModule.class, FrontendRequestComponentModule.class, Jackson2Module.class, KeyModule.class, + KeyringModule.class, + KmsModule.class, ModulesServiceModule.class, ServerTridProviderModule.class, StackdriverModule.class, diff --git a/java/google/registry/module/tools/BUILD b/java/google/registry/module/tools/BUILD index 57eb1e8e7..818c6f0bc 100644 --- a/java/google/registry/module/tools/BUILD +++ b/java/google/registry/module/tools/BUILD @@ -15,6 +15,7 @@ java_library( "//java/google/registry/gcs", "//java/google/registry/groups", "//java/google/registry/keyring/api", + "//java/google/registry/keyring/kms", "//java/google/registry/loadtest", "//java/google/registry/mapreduce", "//java/google/registry/monitoring/whitebox", diff --git a/java/google/registry/module/tools/ToolsComponent.java b/java/google/registry/module/tools/ToolsComponent.java index 60e804084..1518e7801 100644 --- a/java/google/registry/module/tools/ToolsComponent.java +++ b/java/google/registry/module/tools/ToolsComponent.java @@ -23,8 +23,9 @@ import google.registry.gcs.GcsServiceModule; import google.registry.groups.DirectoryModule; import google.registry.groups.GroupsModule; import google.registry.groups.GroupssettingsModule; -import google.registry.keyring.api.DummyKeyringModule; import google.registry.keyring.api.KeyModule; +import google.registry.keyring.kms.KeyringModule; +import google.registry.keyring.kms.KmsModule; import google.registry.module.tools.ToolsRequestComponent.ToolsRequestComponentModule; import google.registry.request.Modules.AppIdentityCredentialModule; import google.registry.request.Modules.DatastoreServiceModule; @@ -50,13 +51,14 @@ import javax.inject.Singleton; DatastoreServiceModule.class, DirectoryModule.class, DriveModule.class, - DummyKeyringModule.class, GcsServiceModule.class, GoogleCredentialModule.class, GroupsModule.class, GroupssettingsModule.class, Jackson2Module.class, KeyModule.class, + KeyringModule.class, + KmsModule.class, ModulesServiceModule.class, ServerTridProviderModule.class, SystemClockModule.class, diff --git a/java/google/registry/tools/BUILD b/java/google/registry/tools/BUILD index ac9384488..b2725773a 100644 --- a/java/google/registry/tools/BUILD +++ b/java/google/registry/tools/BUILD @@ -52,7 +52,6 @@ java_library( "//java/google/registry/request", "//java/google/registry/request:modules", "//java/google/registry/security", - "//java/google/registry/tldconfig/idn", "//java/google/registry/tmch", "//java/google/registry/tools/params", "//java/google/registry/tools/server", diff --git a/java/google/registry/tools/EscrowDepositEncryptor.java b/java/google/registry/tools/EscrowDepositEncryptor.java index c53f84c0d..c1f025ed7 100644 --- a/java/google/registry/tools/EscrowDepositEncryptor.java +++ b/java/google/registry/tools/EscrowDepositEncryptor.java @@ -38,6 +38,7 @@ import java.io.OutputStream; import java.nio.file.Files; import java.nio.file.Path; import javax.inject.Inject; +import javax.inject.Provider; import org.bouncycastle.bcpg.ArmoredOutputStream; import org.bouncycastle.openpgp.PGPException; import org.bouncycastle.openpgp.PGPKeyPair; @@ -54,8 +55,8 @@ final class EscrowDepositEncryptor { @Inject RydePgpFileOutputStreamFactory pgpFileFactory; @Inject RydePgpSigningOutputStreamFactory pgpSigningFactory; @Inject RydeTarOutputStreamFactory tarFactory; - @Inject @Key("rdeSigningKey") PGPKeyPair rdeSigningKey; - @Inject @Key("rdeReceiverKey") PGPPublicKey rdeReceiverKey; + @Inject @Key("rdeSigningKey") Provider rdeSigningKey; + @Inject @Key("rdeReceiverKey") Provider rdeReceiverKey; @Inject EscrowDepositEncryptor() {} /** Creates a {@code .ryde} and {@code .sig} file, provided an XML deposit file. */ @@ -68,12 +69,12 @@ final class EscrowDepositEncryptor { Path rydePath = outdir.resolve(name + ".ryde"); Path sigPath = outdir.resolve(name + ".sig"); Path pubPath = outdir.resolve(tld + ".pub"); - PGPKeyPair signingKey = rdeSigningKey; + PGPKeyPair signingKey = rdeSigningKey.get(); try (OutputStream rydeOutput = Files.newOutputStream(rydePath); RydePgpSigningOutputStream signLayer = pgpSigningFactory.create(rydeOutput, signingKey)) { try (RydePgpEncryptionOutputStream encryptLayer = - pgpEncryptionFactory.create(signLayer, rdeReceiverKey); + pgpEncryptionFactory.create(signLayer, rdeReceiverKey.get()); RydePgpCompressionOutputStream compressLayer = pgpCompressionFactory.create(encryptLayer); RydePgpFileOutputStream fileLayer = diff --git a/java/google/registry/tools/GhostrydeCommand.java b/java/google/registry/tools/GhostrydeCommand.java index ffe9d8e32..24c9a6a97 100644 --- a/java/google/registry/tools/GhostrydeCommand.java +++ b/java/google/registry/tools/GhostrydeCommand.java @@ -23,6 +23,7 @@ import com.beust.jcommander.Parameters; import com.google.common.io.ByteStreams; import google.registry.keyring.api.KeyModule.Key; import google.registry.rde.Ghostryde; +import google.registry.tools.Command.RemoteApiCommand; import google.registry.tools.params.PathParameter; import java.io.IOException; import java.io.InputStream; @@ -32,6 +33,7 @@ import java.nio.file.Path; import java.nio.file.Paths; import java.nio.file.attribute.FileTime; import javax.inject.Inject; +import javax.inject.Provider; import org.bouncycastle.openpgp.PGPException; import org.bouncycastle.openpgp.PGPPrivateKey; import org.bouncycastle.openpgp.PGPPublicKey; @@ -39,7 +41,7 @@ import org.joda.time.DateTime; /** Command to encrypt/decrypt {@code .ghostryde} files. */ @Parameters(separators = " =", commandDescription = "Encrypt/decrypt a ghostryde file.") -final class GhostrydeCommand implements Command { +final class GhostrydeCommand implements RemoteApiCommand { @Parameter( names = {"-e", "--encrypt"}, @@ -71,11 +73,11 @@ final class GhostrydeCommand implements Command { @Inject @Key("rdeStagingEncryptionKey") - PGPPublicKey rdeStagingEncryptionKey; + Provider rdeStagingEncryptionKey; @Inject @Key("rdeStagingDecryptionKey") - PGPPrivateKey rdeStagingDecryptionKey; + Provider rdeStagingDecryptionKey; @Override public final void run() throws Exception { @@ -93,7 +95,7 @@ final class GhostrydeCommand implements Command { : output; try (OutputStream out = Files.newOutputStream(outFile); Ghostryde.Encryptor encryptor = - ghostryde.openEncryptor(out, rdeStagingEncryptionKey); + ghostryde.openEncryptor(out, rdeStagingEncryptionKey.get()); Ghostryde.Compressor kompressor = ghostryde.openCompressor(encryptor); Ghostryde.Output ghostOutput = ghostryde.openOutput(kompressor, input.getFileName().toString(), @@ -106,7 +108,7 @@ final class GhostrydeCommand implements Command { private void runDecrypt() throws IOException, PGPException { try (InputStream in = Files.newInputStream(input); Ghostryde.Decryptor decryptor = - ghostryde.openDecryptor(in, rdeStagingDecryptionKey); + ghostryde.openDecryptor(in, rdeStagingDecryptionKey.get()); Ghostryde.Decompressor decompressor = ghostryde.openDecompressor(decryptor); Ghostryde.Input ghostInput = ghostryde.openInput(decompressor)) { Path outFile = Files.isDirectory(output) diff --git a/java/google/registry/tools/RegistryToolComponent.java b/java/google/registry/tools/RegistryToolComponent.java index 6233938c3..a08856490 100644 --- a/java/google/registry/tools/RegistryToolComponent.java +++ b/java/google/registry/tools/RegistryToolComponent.java @@ -19,12 +19,13 @@ import google.registry.config.RegistryConfig.ConfigModule; import google.registry.dns.writer.VoidDnsWriterModule; import google.registry.dns.writer.clouddns.CloudDnsWriterModule; import google.registry.dns.writer.dnsupdate.DnsUpdateWriterModule; -import google.registry.keyring.api.DummyKeyringModule; import google.registry.keyring.api.KeyModule; +import google.registry.keyring.kms.KeyringModule; import google.registry.keyring.kms.KmsModule; import google.registry.rde.RdeModule; import google.registry.request.Modules.AppIdentityCredentialModule; import google.registry.request.Modules.DatastoreServiceModule; +import google.registry.request.Modules.GoogleCredentialModule; import google.registry.request.Modules.Jackson2Module; import google.registry.request.Modules.ModulesServiceModule; import google.registry.request.Modules.URLFetchServiceModule; @@ -55,9 +56,10 @@ import javax.inject.Singleton; DefaultRequestFactoryModule.class, DefaultRequestFactoryModule.RequestFactoryModule.class, DnsUpdateWriterModule.class, - DummyKeyringModule.class, + GoogleCredentialModule.class, Jackson2Module.class, KeyModule.class, + KeyringModule.class, KmsModule.class, ModulesServiceModule.class, RdeModule.class, diff --git a/java/google/registry/util/ComparingInvocationHandler.java b/java/google/registry/util/ComparingInvocationHandler.java index b1b031ec2..ff42efca7 100644 --- a/java/google/registry/util/ComparingInvocationHandler.java +++ b/java/google/registry/util/ComparingInvocationHandler.java @@ -117,7 +117,7 @@ public abstract class ComparingInvocationHandler implements InvocationHandler * @param actual the exception thrown by a call to method for the "actual" implementation * @param second the exception thrown by a call to method for the "second" implementation */ - protected boolean compareException( + protected boolean compareThrown( @SuppressWarnings("unused") Method method, Throwable actual, Throwable second) { @@ -133,10 +133,10 @@ public abstract class ComparingInvocationHandler implements InvocationHandler * @param method the method whose return value is given * @param exception the exception thrown by a call to method */ - protected String stringifyException( + protected String stringifyThrown( @SuppressWarnings("unused") Method method, - Throwable exception) { - return exception.toString(); + Throwable throwable) { + return throwable.toString(); } @Override @@ -159,26 +159,26 @@ public abstract class ComparingInvocationHandler implements InvocationHandler // First compare the two implementations' result, and log any differences: if (actualException != null && secondException != null) { - if (!compareException(method, actualException, secondException)) { + if (!compareThrown(method, actualException, secondException)) { log( method, String.format( "Both implementations threw, but got different exceptions! '%s' vs '%s'", - stringifyException(method, actualException), - stringifyException(method, secondException))); + stringifyThrown(method, actualException), + stringifyThrown(method, secondException))); } } else if (actualException != null) { log( method, String.format( "Only actual implementation threw exception: %s", - stringifyException(method, actualException))); + stringifyThrown(method, actualException))); } else if (secondException != null) { log( method, String.format( "Only second implementation threw exception: %s", - stringifyException(method, secondException))); + stringifyThrown(method, secondException))); } else { // Neither threw exceptions - we compare the results if (!compareResults(method, actualResult, secondResult)) { diff --git a/javatests/google/registry/tools/EncryptEscrowDepositCommandTest.java b/javatests/google/registry/tools/EncryptEscrowDepositCommandTest.java index 0e425b262..fba169fc3 100644 --- a/javatests/google/registry/tools/EncryptEscrowDepositCommandTest.java +++ b/javatests/google/registry/tools/EncryptEscrowDepositCommandTest.java @@ -50,8 +50,8 @@ public class EncryptEscrowDepositCommandTest res.pgpFileFactory = new RydePgpFileOutputStreamFactory(Providers.of(1024)); res.pgpSigningFactory = new RydePgpSigningOutputStreamFactory(); res.tarFactory = new RydeTarOutputStreamFactory(); - res.rdeReceiverKey = new FakeKeyringModule().get().getRdeReceiverKey(); - res.rdeSigningKey = new FakeKeyringModule().get().getRdeSigningKey(); + res.rdeReceiverKey = Providers.of(new FakeKeyringModule().get().getRdeReceiverKey()); + res.rdeSigningKey = Providers.of(new FakeKeyringModule().get().getRdeSigningKey()); return res; } diff --git a/javatests/google/registry/tools/GhostrydeCommandTest.java b/javatests/google/registry/tools/GhostrydeCommandTest.java index 65b3545cb..38c1f1fea 100644 --- a/javatests/google/registry/tools/GhostrydeCommandTest.java +++ b/javatests/google/registry/tools/GhostrydeCommandTest.java @@ -23,6 +23,7 @@ import google.registry.rde.Ghostryde.DecodeResult; import google.registry.testing.BouncyCastleProviderRule; import google.registry.testing.FakeKeyringModule; import google.registry.testing.InjectRule; +import google.registry.testing.Providers; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; @@ -67,8 +68,8 @@ public class GhostrydeCommandTest extends CommandTestCase { public void before() throws Exception { keyring = new FakeKeyringModule().get(); command.ghostryde = new Ghostryde(1024); - command.rdeStagingDecryptionKey = keyring.getRdeStagingDecryptionKey(); - command.rdeStagingEncryptionKey = keyring.getRdeStagingEncryptionKey(); + command.rdeStagingDecryptionKey = Providers.of(keyring.getRdeStagingDecryptionKey()); + command.rdeStagingEncryptionKey = Providers.of(keyring.getRdeStagingEncryptionKey()); } @Test diff --git a/javatests/google/registry/util/ComparingInvocationHandlerTest.java b/javatests/google/registry/util/ComparingInvocationHandlerTest.java index ce6dfcad6..489672438 100644 --- a/javatests/google/registry/util/ComparingInvocationHandlerTest.java +++ b/javatests/google/registry/util/ComparingInvocationHandlerTest.java @@ -94,12 +94,12 @@ public class ComparingInvocationHandlerTest { return super.stringifyResult(method, a); } - @Override protected boolean compareException(Method method, Throwable a, Throwable b) { - return exceptionEqualsResult && super.compareException(method, a, b); + @Override protected boolean compareThrown(Method method, Throwable a, Throwable b) { + return exceptionEqualsResult && super.compareThrown(method, a, b); } - @Override protected String stringifyException(Method method, Throwable a) { - return String.format("testException(%s)", super.stringifyException(method, a)); + @Override protected String stringifyThrown(Method method, Throwable a) { + return String.format("testException(%s)", super.stringifyThrown(method, a)); } }