From 16e1927a4e8dcf9c8d69a41451a7fbf66a130fdd Mon Sep 17 00:00:00 2001 From: Lai Jiang Date: Wed, 22 Jun 2022 11:03:41 -0400 Subject: [PATCH] Only use GPG2 in tests (#1676) GPG1 is deprecated and stuck in v1.4 from 2018. GPG2 is recommended. We only use the GPG binary in tests and when the host system has both versions it causes problems because we hardcode the GPG import command in GpgSystemCommandExension to use the binary named "gpg", which could be linked to either GPG1 or GPG2, causing the other test to fail when the version of GPG that runs in tests is incompatible with the version of GPG that imports the keys. With this PR we only support GPG2 from now on. --- .../registry/rde/BrdaCopyActionTest.java | 9 +++++---- .../rde/GhostrydeGpgIntegrationTest.java | 17 +++++++---------- .../registry/rde/RdeUploadActionTest.java | 5 +++-- .../registry/rde/RydeGpgIntegrationTest.java | 18 ++++++++---------- .../testing/GpgSystemCommandExtension.java | 5 +++-- release/builder/build.sh | 2 ++ 6 files changed, 28 insertions(+), 28 deletions(-) diff --git a/core/src/test/java/google/registry/rde/BrdaCopyActionTest.java b/core/src/test/java/google/registry/rde/BrdaCopyActionTest.java index 8b37c5cd3..3e35fa299 100644 --- a/core/src/test/java/google/registry/rde/BrdaCopyActionTest.java +++ b/core/src/test/java/google/registry/rde/BrdaCopyActionTest.java @@ -20,6 +20,7 @@ import static google.registry.model.common.Cursor.CursorType.BRDA; import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import static google.registry.testing.DatabaseHelper.createTld; import static google.registry.testing.DatabaseHelper.persistResource; +import static google.registry.testing.GpgSystemCommandExtension.GPG_BINARY; import static google.registry.testing.SystemInfo.hasCommand; import static java.nio.charset.StandardCharsets.UTF_8; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -152,14 +153,14 @@ public class BrdaCopyActionTest { @ParameterizedTest @ValueSource(strings = {"", "job-name/"}) void testRun_rydeFormat(String prefix) throws Exception { - assumeTrue(hasCommand("gpg --version")); + assumeTrue(hasCommand(GPG_BINARY + " --version")); runAction(prefix); File rydeTmp = new File(gpg.getCwd(), "ryde"); Files.write(gcsUtils.readBytesFrom(RYDE_FILE), rydeTmp); Process pid = gpg.exec( - "gpg", + GPG_BINARY, "--list-packets", "--ignore-mdc-error", "--keyid-format", @@ -200,7 +201,7 @@ public class BrdaCopyActionTest { @ParameterizedTest @ValueSource(strings = {"", "job-name/"}) void testRun_rydeSignature(String prefix) throws Exception { - assumeTrue(hasCommand("gpg --version")); + assumeTrue(hasCommand(GPG_BINARY + " --version")); runAction(prefix); File rydeTmp = new File(gpg.getCwd(), "ryde"); @@ -208,7 +209,7 @@ public class BrdaCopyActionTest { Files.write(gcsUtils.readBytesFrom(RYDE_FILE), rydeTmp); Files.write(gcsUtils.readBytesFrom(SIG_FILE), sigTmp); - Process pid = gpg.exec("gpg", "--verify", sigTmp.toString(), rydeTmp.toString()); + Process pid = gpg.exec(GPG_BINARY, "--verify", sigTmp.toString(), rydeTmp.toString()); String stderr = slurp(pid.getErrorStream()); assertWithMessage(stderr).that(pid.waitFor()).isEqualTo(0); assertThat(stderr).contains("Good signature"); diff --git a/core/src/test/java/google/registry/rde/GhostrydeGpgIntegrationTest.java b/core/src/test/java/google/registry/rde/GhostrydeGpgIntegrationTest.java index efa8c2578..59fcc2b16 100644 --- a/core/src/test/java/google/registry/rde/GhostrydeGpgIntegrationTest.java +++ b/core/src/test/java/google/registry/rde/GhostrydeGpgIntegrationTest.java @@ -16,6 +16,7 @@ package google.registry.rde; import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertWithMessage; +import static google.registry.testing.GpgSystemCommandExtension.GPG_BINARY; import static google.registry.testing.SystemInfo.hasCommand; import static java.nio.charset.StandardCharsets.UTF_8; import static org.junit.jupiter.api.Assumptions.assumeTrue; @@ -52,8 +53,6 @@ class GhostrydeGpgIntegrationTest { RdeTestData.loadBytes("pgp-public-keyring.asc"), RdeTestData.loadBytes("pgp-private-keyring-registry.asc")); - // TODO(b/236723363) add in "gpg2" once we figure out why it's broken - private static final ImmutableList COMMANDS = ImmutableList.of("gpg"); private static final ImmutableList CONTENTS = ImmutableList.of( "(◕‿◕)", @@ -64,18 +63,16 @@ class GhostrydeGpgIntegrationTest { @SuppressWarnings("unused") static Stream provideTestCombinations() { Stream.Builder stream = Stream.builder(); - for (String command : COMMANDS) { - for (String content : CONTENTS) { - stream.add(Arguments.of(command, content)); - } + for (String content : CONTENTS) { + stream.add(Arguments.of(content)); } return stream.build(); } @ParameterizedTest @MethodSource("provideTestCombinations") - void test(String command, String content) throws Exception { - assumeTrue(hasCommand(command + " --version")); + void test(String content) throws Exception { + assumeTrue(hasCommand(GPG_BINARY + " --version")); Keyring keyring = new FakeKeyringModule().get(); PGPPublicKey publicKey = keyring.getRdeStagingEncryptionKey(); File file = new File(gpg.getCwd(), "love.gpg"); @@ -86,7 +83,7 @@ class GhostrydeGpgIntegrationTest { ghostrydeEncoder.write(data); } - Process pid = gpg.exec(command, "--list-packets", "--keyid-format", "long", file.getPath()); + Process pid = gpg.exec(GPG_BINARY, "--list-packets", "--keyid-format", "long", file.getPath()); String stdout = CharStreams.toString(new InputStreamReader(pid.getInputStream(), UTF_8)); String stderr = CharStreams.toString(new InputStreamReader(pid.getErrorStream(), UTF_8)); assertWithMessage(stderr).that(pid.waitFor()).isEqualTo(0); @@ -96,7 +93,7 @@ class GhostrydeGpgIntegrationTest { assertThat(stdout).contains("name=\"" + Ghostryde.INNER_FILENAME + "\""); assertThat(stderr).contains("encrypted with 2048-bit RSA key, ID A59C132F3589A1D5"); - pid = gpg.exec(command, "--use-embedded-filename", file.getPath()); + pid = gpg.exec(GPG_BINARY, "--use-embedded-filename", file.getPath()); stderr = CharStreams.toString(new InputStreamReader(pid.getErrorStream(), UTF_8)); assertWithMessage(stderr).that(pid.waitFor()).isEqualTo(0); File dataFile = new File(gpg.getCwd(), Ghostryde.INNER_FILENAME); diff --git a/core/src/test/java/google/registry/rde/RdeUploadActionTest.java b/core/src/test/java/google/registry/rde/RdeUploadActionTest.java index 8d6dc520a..5c53a5f3a 100644 --- a/core/src/test/java/google/registry/rde/RdeUploadActionTest.java +++ b/core/src/test/java/google/registry/rde/RdeUploadActionTest.java @@ -24,6 +24,7 @@ import static google.registry.persistence.transaction.TransactionManagerFactory. import static google.registry.testing.DatabaseHelper.createTld; import static google.registry.testing.DatabaseHelper.persistResource; import static google.registry.testing.DatabaseHelper.persistSimpleResource; +import static google.registry.testing.GpgSystemCommandExtension.GPG_BINARY; import static google.registry.testing.SystemInfo.hasCommand; import static java.nio.charset.StandardCharsets.UTF_8; import static org.joda.time.Duration.standardDays; @@ -440,7 +441,7 @@ public class RdeUploadActionTest { @TestOfyAndSql void testRunWithLock_producesValidSignature() throws Exception { - assumeTrue(hasCommand("gpg --version")); + assumeTrue(hasCommand(GPG_BINARY + " --version")); int port = sftpd.serve("user", "password", folder); URI uploadUrl = URI.create(String.format("sftp://user:password@localhost:%d/", port)); DateTime stagingCursor = DateTime.parse("2010-10-18TZ"); @@ -451,7 +452,7 @@ public class RdeUploadActionTest { // identical to the ones sent over SFTP. Process pid = gpg.exec( - "gpg", + GPG_BINARY, "--verify", new File(folder, "tld_2010-10-17_full_S1_R0.sig").toString(), new File(folder, "tld_2010-10-17_full_S1_R0.ryde").toString()); diff --git a/core/src/test/java/google/registry/rde/RydeGpgIntegrationTest.java b/core/src/test/java/google/registry/rde/RydeGpgIntegrationTest.java index ac5b06366..d95a0c643 100644 --- a/core/src/test/java/google/registry/rde/RydeGpgIntegrationTest.java +++ b/core/src/test/java/google/registry/rde/RydeGpgIntegrationTest.java @@ -16,6 +16,7 @@ package google.registry.rde; import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertWithMessage; +import static google.registry.testing.GpgSystemCommandExtension.GPG_BINARY; import static google.registry.testing.SystemInfo.hasCommand; import static java.nio.charset.StandardCharsets.UTF_8; import static org.junit.jupiter.api.Assumptions.assumeTrue; @@ -60,8 +61,6 @@ public class RydeGpgIntegrationTest { private final FakeKeyringModule keyringFactory = new FakeKeyringModule(); - // TODO(b/236723363) add in "gpg2" once we figure out why it's broken - private static final ImmutableList COMMANDS = ImmutableList.of("gpg"); private static final ImmutableList CONTENTS = ImmutableList.of( "(◕‿◕)", @@ -71,20 +70,18 @@ public class RydeGpgIntegrationTest { static Stream provideTestCombinations() { Stream.Builder stream = Stream.builder(); - for (String command : COMMANDS) { for (String content : CONTENTS) { - stream.add(Arguments.of(command, content)); - } + stream.add(Arguments.of(content)); } return stream.build(); } @ParameterizedTest @MethodSource("provideTestCombinations") - void test(String command, String content) throws Exception { + void test(String content) throws Exception { final String filename = "sloth"; assumeTrue(hasCommand("tar")); - assumeTrue(hasCommand(command + " --version")); + assumeTrue(hasCommand(GPG_BINARY + " --version")); Keyring keyring = keyringFactory.get(); PGPKeyPair signingKey = keyring.getRdeSigningKey(); @@ -125,7 +122,7 @@ public class RydeGpgIntegrationTest { { Process pid = gpg.exec( - command, + GPG_BINARY, "--list-packets", "--ignore-mdc-error", "--keyid-format", @@ -170,7 +167,7 @@ public class RydeGpgIntegrationTest { // gpg: Good signature from logger.atInfo().log("Running GPG to verify signature..."); { - Process pid = gpg.exec(command, "--verify", sigFile.toString(), rydeFile.toString()); + Process pid = gpg.exec(GPG_BINARY, "--verify", sigFile.toString(), rydeFile.toString()); String stderr = slurp(pid.getErrorStream()); assertWithMessage(stderr).that(pid.waitFor()).isEqualTo(0); assertThat(stderr).contains("Good signature"); @@ -189,7 +186,8 @@ public class RydeGpgIntegrationTest { logger.atInfo().log("Running GPG to extract tar..."); { Process pid = - gpg.exec(command, "--use-embedded-filename", "--ignore-mdc-error", rydeFile.toString()); + gpg.exec( + GPG_BINARY, "--use-embedded-filename", "--ignore-mdc-error", rydeFile.toString()); String stderr = slurp(pid.getErrorStream()); assertWithMessage(stderr).that(pid.waitFor()).isEqualTo(0); } diff --git a/core/src/test/java/google/registry/testing/GpgSystemCommandExtension.java b/core/src/test/java/google/registry/testing/GpgSystemCommandExtension.java index c0cbe2d4f..6ee36c90e 100644 --- a/core/src/test/java/google/registry/testing/GpgSystemCommandExtension.java +++ b/core/src/test/java/google/registry/testing/GpgSystemCommandExtension.java @@ -47,6 +47,7 @@ public final class GpgSystemCommandExtension implements BeforeEachCallback, Afte private static final FluentLogger logger = FluentLogger.forEnclosingClass(); private static final File DEV_NULL = new File("/dev/null"); private static final String TEMP_FILE_PREFIX = "gpgtest"; + public static final String GPG_BINARY = "gpg2"; private File cwd = DEV_NULL; private File conf = DEV_NULL; @@ -105,7 +106,7 @@ public final class GpgSystemCommandExtension implements BeforeEachCallback, Afte "PATH=" + System.getenv("PATH"), "GNUPGHOME=" + conf.getAbsolutePath(), }; - Process pid = exec("gpg", "--import"); + Process pid = exec(GPG_BINARY, "--import"); publicKeyring.copyTo(pid.getOutputStream()); pid.getOutputStream().close(); int returnValue = pid.waitFor(); @@ -114,7 +115,7 @@ public final class GpgSystemCommandExtension implements BeforeEachCallback, Afte .that(returnValue) .isEqualTo(0); - pid = exec("gpg", "--allow-secret-key-import", "--import"); + pid = exec(GPG_BINARY, "--allow-secret-key-import", "--import"); privateKeyring.copyTo(pid.getOutputStream()); pid.getOutputStream().close(); returnValue = pid.waitFor(); diff --git a/release/builder/build.sh b/release/builder/build.sh index 5852c57ff..73c1402d0 100755 --- a/release/builder/build.sh +++ b/release/builder/build.sh @@ -19,6 +19,8 @@ apt-get install locales -y locale-gen en_US.UTF-8 apt-get install apt-utils gnupg -y apt-get upgrade -y +# Install GPG2 (in case it was not included) +apt-get install gnupg2 -y # Install Java apt-get install openjdk-11-jdk-headless -y # Install Python