diff --git a/java/google/registry/rde/Ghostryde.java b/java/google/registry/rde/Ghostryde.java index 433a42f5c..1dc4d9d86 100644 --- a/java/google/registry/rde/Ghostryde.java +++ b/java/google/registry/rde/Ghostryde.java @@ -16,7 +16,6 @@ package google.registry.rde; import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; -import static com.google.common.base.Preconditions.checkState; import static java.nio.charset.StandardCharsets.US_ASCII; import static java.nio.charset.StandardCharsets.UTF_8; import static org.bouncycastle.bcpg.CompressionAlgorithmTags.ZLIB; @@ -24,7 +23,6 @@ import static org.bouncycastle.bcpg.SymmetricKeyAlgorithmTags.AES_128; import static org.bouncycastle.jce.provider.BouncyCastleProvider.PROVIDER_NAME; import static org.bouncycastle.openpgp.PGPLiteralData.BINARY; -import com.google.common.flogger.FluentLogger; import com.google.common.io.ByteStreams; import com.google.common.io.Closer; import google.registry.util.ImprovedInputStream; @@ -37,6 +35,8 @@ import java.io.OutputStream; import java.security.NoSuchAlgorithmException; import java.security.ProviderException; import java.security.SecureRandom; +import java.util.Optional; +import java.util.stream.Collectors; import javax.annotation.CheckReturnValue; import javax.annotation.Nullable; import javax.annotation.WillNotClose; @@ -47,11 +47,9 @@ import org.bouncycastle.openpgp.PGPEncryptedDataList; import org.bouncycastle.openpgp.PGPException; import org.bouncycastle.openpgp.PGPLiteralData; import org.bouncycastle.openpgp.PGPLiteralDataGenerator; -import org.bouncycastle.openpgp.PGPObjectFactory; import org.bouncycastle.openpgp.PGPPrivateKey; import org.bouncycastle.openpgp.PGPPublicKey; import org.bouncycastle.openpgp.PGPPublicKeyEncryptedData; -import org.bouncycastle.openpgp.bc.BcPGPObjectFactory; import org.bouncycastle.openpgp.operator.bc.BcPublicKeyDataDecryptorFactory; import org.bouncycastle.openpgp.operator.bc.BcPublicKeyKeyEncryptionMethodGenerator; import org.bouncycastle.openpgp.operator.jcajce.JcePGPDataEncryptorBuilder; @@ -125,8 +123,6 @@ import org.joda.time.DateTime; */ public final class Ghostryde { - private static final FluentLogger logger = FluentLogger.forEnclosingClass(); - /** Size of the buffer used by the intermediate streams. */ static final int BUFFER_SIZE = 64 * 1024; @@ -388,30 +384,35 @@ public final class Ghostryde { private static ImprovedInputStream openDecryptor( @WillNotClose InputStream input, PGPPrivateKey privateKey) throws IOException, PGPException { checkNotNull(privateKey, "privateKey"); - PGPObjectFactory fact = new BcPGPObjectFactory(checkNotNull(input, "input")); - PGPEncryptedDataList ciphertexts = pgpCast(fact.nextObject(), PGPEncryptedDataList.class); - checkState(ciphertexts.size() > 0); - if (ciphertexts.size() > 1) { - logger.atWarning().log("crypts.size() is %d (should be 1)", ciphertexts.size()); - } - PGPPublicKeyEncryptedData cyphertext = - pgpCast(ciphertexts.get(0), PGPPublicKeyEncryptedData.class); - if (cyphertext.getKeyID() != privateKey.getKeyID()) { - throw new PGPException(String.format( - "Message was encrypted for keyid %x but ours is %x", - cyphertext.getKeyID(), privateKey.getKeyID())); + PGPEncryptedDataList ciphertextList = + PgpUtils.readSinglePgpObject(input, PGPEncryptedDataList.class); + // Go over all the possible decryption keys, and look for the one that has our key ID. + Optional cyphertext = + PgpUtils.stream(ciphertextList, PGPPublicKeyEncryptedData.class) + .filter(ciphertext -> ciphertext.getKeyID() == privateKey.getKeyID()) + .findAny(); + // If we can't find one with our key ID, then we can't decrypt the file! + if (!cyphertext.isPresent()) { + String keyIds = + PgpUtils.stream(ciphertextList, PGPPublicKeyEncryptedData.class) + .map(ciphertext -> Long.toHexString(ciphertext.getKeyID())) + .collect(Collectors.joining(",")); + throw new PGPException( + String.format( + "Message was encrypted for keyids [%s] but ours is %x", + keyIds, privateKey.getKeyID())); } // We want an input stream that also verifies ciphertext wasn't corrupted or tampered with when // the stream is closed. return new ImprovedInputStream( "GhostrydeDecryptor", - cyphertext.getDataStream(new BcPublicKeyDataDecryptorFactory(privateKey))) { + cyphertext.get().getDataStream(new BcPublicKeyDataDecryptorFactory(privateKey))) { @Override protected void onClose() throws IOException { if (USE_INTEGRITY_PACKET) { try { - if (!cyphertext.verify()) { + if (!cyphertext.get().verify()) { throw new PGPException("ghostryde integrity check failed: possible tampering D:"); } } catch (PGPException e) { @@ -437,8 +438,7 @@ public final class Ghostryde { @CheckReturnValue private static ImprovedInputStream openDecompressor(@WillNotClose InputStream input) throws IOException, PGPException { - PGPObjectFactory fact = new BcPGPObjectFactory(checkNotNull(input, "input")); - PGPCompressedData compressed = pgpCast(fact.nextObject(), PGPCompressedData.class); + PGPCompressedData compressed = PgpUtils.readSinglePgpObject(input, PGPCompressedData.class); return new ImprovedInputStream("GhostrydeDecompressor", compressed.getDataStream()); } @@ -457,21 +457,7 @@ public final class Ghostryde { @CheckReturnValue private static ImprovedInputStream openPgpFileInputStream(@WillNotClose InputStream input) throws IOException, PGPException { - PGPObjectFactory fact = new BcPGPObjectFactory(checkNotNull(input, "input")); - PGPLiteralData literal = pgpCast(fact.nextObject(), PGPLiteralData.class); + PGPLiteralData literal = PgpUtils.readSinglePgpObject(input, PGPLiteralData.class); return new ImprovedInputStream("GhostrydePgpFileInputStream", literal.getDataStream()); } - - /** Safely extracts an object from an OpenPGP message. */ - private static T pgpCast(@Nullable Object object, Class expect) throws PGPException { - if (object == null) { - throw new PGPException(String.format( - "Expected %s but out of objects", expect.getSimpleName())); - } - if (!expect.isAssignableFrom(object.getClass())) { - throw new PGPException(String.format( - "Expected %s but got %s", expect.getSimpleName(), object.getClass().getSimpleName())); - } - return expect.cast(object); - } } diff --git a/java/google/registry/rde/PgpUtils.java b/java/google/registry/rde/PgpUtils.java new file mode 100644 index 000000000..1a076d4a6 --- /dev/null +++ b/java/google/registry/rde/PgpUtils.java @@ -0,0 +1,65 @@ +// Copyright 2018 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.rde; + +import static com.google.common.base.Preconditions.checkNotNull; +import static com.google.common.base.Preconditions.checkState; + +import com.google.common.collect.Streams; +import java.io.IOException; +import java.io.InputStream; +import java.util.stream.Stream; +import javax.annotation.Nullable; +import org.bouncycastle.openpgp.PGPEncryptedDataList; +import org.bouncycastle.openpgp.PGPObjectFactory; +import org.bouncycastle.openpgp.jcajce.JcaPGPObjectFactory; + +/** Utilities that help us deal with Pgp objects. */ +final class PgpUtils { + + private PgpUtils() {} + + /** Safely extracts an object from an OpenPGP message. */ + static T pgpCast(@Nullable Object object, Class expect) { + checkNotNull(object, "PGP error: expected %s but out of objects", expect.getSimpleName()); + checkState( + expect.isAssignableFrom(object.getClass()), + "PGP error: expected %s but got %s", + expect.getSimpleName(), + object.getClass().getSimpleName()); + return expect.cast(object); + } + + static T readSinglePgpObject(InputStream input, Class expect) { + try { + PGPObjectFactory fact = new JcaPGPObjectFactory(input); + return PgpUtils.pgpCast(fact.nextObject(), expect); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + + /** + * Stream the "list" that is PGPEncryptedDataList. + * + *

PGPEncryptedDataList is apparently very "legacy". It's not actually a list, and although it + * implements "Iterable", there's no explicit type given. This requires multiple casts, which is + * ugly. Moving the casts to a dedicated function makes it all clearer. + */ + static Stream stream(PGPEncryptedDataList ciphertexts, Class expect) { + return Streams.stream((Iterable) ciphertexts) + .map(ciphertext -> PgpUtils.pgpCast(ciphertext, expect)); + } +} diff --git a/javatests/google/registry/rde/GhostrydeTest.java b/javatests/google/registry/rde/GhostrydeTest.java index 319f0ef06..aed8374cd 100644 --- a/javatests/google/registry/rde/GhostrydeTest.java +++ b/javatests/google/registry/rde/GhostrydeTest.java @@ -229,7 +229,8 @@ public class GhostrydeTest { }); assertThat(thrown) .hasMessageThat() - .contains("Message was encrypted for keyid a59c132f3589a1d5 but ours is c9598c84ec70b9fd"); + .contains( + "Message was encrypted for keyids [a59c132f3589a1d5] but ours is c9598c84ec70b9fd"); } @Test