From 9a73f900f6882c2037031b77b828900b6dff9a71 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Tue, 5 Mar 2024 10:04:51 +0100 Subject: [PATCH] [MGPG-105] Make possible backward compatibility (#74) Back out a bit from original MGPG-105 PR that _fails_ if best practices not followed. Introduce a "bestPractice" boolean configuration flag. By default, plugin enforces best practice (= `true`), and will refuse to operate if best practices violated. Still, user can explicitly configure value `false`, when plugin regains "old way" (unsecure) mode of secret configuration, but plugin will warn. --- https://issues.apache.org/jira/browse/MGPG-105 --- pgp-keys-map.list | 2 + pom.xml | 22 +++ .../maven/plugins/gpg/AbstractGpgMojo.java | 132 +++++++++++++++++- .../plugins/gpg/GpgSignAttachedMojo.java | 2 +- .../plugins/gpg/SignAndDeployFileMojo.java | 2 +- .../plugins/gpg/it/GpgSignArtifactIT.java | 29 ++++ 6 files changed, 183 insertions(+), 6 deletions(-) diff --git a/pgp-keys-map.list b/pgp-keys-map.list index c507a97..dcaa0b7 100644 --- a/pgp-keys-map.list +++ b/pgp-keys-map.list @@ -28,6 +28,8 @@ org.junit.platform:junit-platform-commons = 0xFF6E2C001948C5F2F38B0CC385911F425E org.opentest4j:opentest4j = 0xFF6E2C001948C5F2F38B0CC385911F425EC61B51 org.apache.maven.resolver = 0x522CA055B326A636D833EF6A0551FD3684FCBBB7 org.apache.maven.shared:maven-invoker = 0x84789D24DF77A32433CE1F079EB80E92EB2135B1 +org.codehaus.plexus:plexus-cipher = 0x6A814B1F869C2BBEAB7CB7271A2A1C94BDE89688 org.codehaus.plexus:plexus-classworlds = 0xB91AB7D2121DC6B0A61AA182D7742D58455ECC7C org.codehaus.plexus:plexus-component-annotations = 0xFA77DCFEF2EE6EB2DEBEDD2C012579464D01C06A org.codehaus.plexus:plexus-utils = 0xF254B35617DC255D9344BCFA873A8E86B4372146 +org.codehaus.plexus:plexus-sec-dispatcher = 0x2BE13D052E9AA567D657D9791FD507154FB9BA39 \ No newline at end of file diff --git a/pom.xml b/pom.xml index c547723..3aa46eb 100644 --- a/pom.xml +++ b/pom.xml @@ -137,6 +137,28 @@ under the License. 2.9.0 pom + + org.codehaus.plexus + plexus-sec-dispatcher + 2.0 + + + * + * + + + + + org.codehaus.plexus + plexus-cipher + 2.0 + + + * + * + + + org.junit.jupiter diff --git a/src/main/java/org/apache/maven/plugins/gpg/AbstractGpgMojo.java b/src/main/java/org/apache/maven/plugins/gpg/AbstractGpgMojo.java index 691834d..91009f8 100644 --- a/src/main/java/org/apache/maven/plugins/gpg/AbstractGpgMojo.java +++ b/src/main/java/org/apache/maven/plugins/gpg/AbstractGpgMojo.java @@ -27,6 +27,11 @@ import org.apache.maven.plugin.MojoFailureException; import org.apache.maven.plugins.annotations.Component; import org.apache.maven.plugins.annotations.Parameter; +import org.apache.maven.project.MavenProject; +import org.apache.maven.settings.Server; +import org.apache.maven.settings.Settings; +import org.sonatype.plexus.components.sec.dispatcher.SecDispatcher; +import org.sonatype.plexus.components.sec.dispatcher.SecDispatcherException; /** * @author Benjamin Bentmann @@ -248,14 +253,47 @@ public abstract class AbstractGpgMojo extends AbstractMojo { @Component protected MavenSession session; + // === Deprecated stuff + + /** + * Switch to lax plugin enforcement of "best practices". If set to {@code false}, plugin will retain all the + * backward compatibility regarding getting secrets (but will warn). By default, plugin enforces "best practices" + * and in such cases plugin fails. + * + * @since 3.2.0 + * @deprecated + */ + @Deprecated + @Parameter(property = "gpg.bestPractices", defaultValue = "true") + private boolean bestPractices; + + /** + * Current user system settings for use in Maven. + * + * @since 1.6 + * @deprecated + */ + @Deprecated + @Parameter(defaultValue = "${settings}", readonly = true, required = true) + private Settings settings; + + /** + * Maven Security Dispatcher. + * + * @since 1.6 + * @deprecated + */ + @Deprecated + @Component + private SecDispatcher secDispatcher; + @Override public final void execute() throws MojoExecutionException, MojoFailureException { if (skip) { // We're skipping the signing stuff return; } - if ((passphrase != null && !passphrase.trim().isEmpty()) - || (passphraseServerId != null && !passphraseServerId.trim().isEmpty())) { + if (bestPractices && (isNotBlank(passphrase) || isNotBlank(passphraseServerId))) { // Stop propagating worst practices: passphrase MUST NOT be in any file on disk throw new MojoFailureException( "Do not store passphrase in any file (disk or SCM repository), rely on GnuPG agent or provide passphrase in " @@ -267,7 +305,19 @@ public final void execute() throws MojoExecutionException, MojoFailureException protected abstract void doExecute() throws MojoExecutionException, MojoFailureException; - protected AbstractGpgSigner newSigner() throws MojoFailureException { + private void logBestPracticeWarning(String source) { + getLog().warn(""); + getLog().warn("W A R N I N G"); + getLog().warn(""); + getLog().warn("Do not store passphrase in any file (disk or SCM repository),"); + getLog().warn("instead rely on GnuPG agent in interactive sessions, or provide passphrase in "); + getLog().warn(passphraseEnvName + " environment variable for batch mode."); + getLog().warn(""); + getLog().warn("Sensitive content loaded from " + source); + getLog().warn(""); + } + + protected AbstractGpgSigner newSigner(MavenProject mavenProject) throws MojoFailureException { AbstractGpgSigner signer; if (GpgSigner.NAME.equals(this.signer)) { signer = new GpgSigner(executable); @@ -294,10 +344,32 @@ protected AbstractGpgSigner newSigner() throws MojoFailureException { signer.setLockMode(lockMode); signer.setArgs(gpgArguments); + // "new way": env prevails String passphrase = (String) session.getRepositorySession().getConfigProperties().get("env." + passphraseEnvName); - if (passphrase != null) { + if (isNotBlank(passphrase)) { signer.setPassPhrase(passphrase); + } else if (!bestPractices) { + // "old way": mojo config + passphrase = this.passphrase; + if (isNotBlank(passphrase)) { + logBestPracticeWarning("Mojo configuration"); + signer.setPassPhrase(passphrase); + } else { + // "old way": serverId + settings + passphrase = loadGpgPassphrase(); + if (isNotBlank(passphrase)) { + logBestPracticeWarning("settings.xml"); + signer.setPassPhrase(passphrase); + } else { + // "old way": project properties + passphrase = getPassphrase(mavenProject); + if (isNotBlank(passphrase)) { + logBestPracticeWarning("Project properties"); + signer.setPassPhrase(passphrase); + } + } + } } // gpg signer: always failed if no passphrase and no agent and not interactive: retain this behavior @@ -310,4 +382,56 @@ protected AbstractGpgSigner newSigner() throws MojoFailureException { return signer; } + + private boolean isNotBlank(String string) { + return string != null && !string.trim().isEmpty(); + } + + // Below is attic, to be thrown out + + @Deprecated + private static final String GPG_PASSPHRASE = "gpg.passphrase"; + + @Deprecated + private String loadGpgPassphrase() throws MojoFailureException { + if (isNotBlank(passphrase)) { + Server server = settings.getServer(passphraseServerId); + if (server != null) { + if (isNotBlank(server.getPassphrase())) { + try { + return secDispatcher.decrypt(server.getPassphrase()); + } catch (SecDispatcherException e) { + throw new MojoFailureException("Unable to decrypt gpg passphrase", e); + } + } + } + } + return null; + } + + @Deprecated + public String getPassphrase(MavenProject project) { + String pass = null; + if (project != null) { + pass = project.getProperties().getProperty(GPG_PASSPHRASE); + if (pass == null) { + MavenProject prj2 = findReactorProject(project); + pass = prj2.getProperties().getProperty(GPG_PASSPHRASE); + } + } + if (project != null) { + findReactorProject(project).getProperties().setProperty(GPG_PASSPHRASE, pass); + } + return pass; + } + + @Deprecated + private MavenProject findReactorProject(MavenProject prj) { + if (prj.getParent() != null + && prj.getParent().getBasedir() != null + && prj.getParent().getBasedir().exists()) { + return findReactorProject(prj.getParent()); + } + return prj; + } } diff --git a/src/main/java/org/apache/maven/plugins/gpg/GpgSignAttachedMojo.java b/src/main/java/org/apache/maven/plugins/gpg/GpgSignAttachedMojo.java index 55ae88a..b72d387 100644 --- a/src/main/java/org/apache/maven/plugins/gpg/GpgSignAttachedMojo.java +++ b/src/main/java/org/apache/maven/plugins/gpg/GpgSignAttachedMojo.java @@ -82,7 +82,7 @@ protected void doExecute() throws MojoExecutionException, MojoFailureException { // Sign collected files and attach all the signatures // ---------------------------------------------------------------------------- - AbstractGpgSigner signer = newSigner(); + AbstractGpgSigner signer = newSigner(project); signer.setOutputDirectory(ascDirectory); signer.setBuildDirectory(new File(project.getBuild().getDirectory())); signer.setBaseDirectory(project.getBasedir()); diff --git a/src/main/java/org/apache/maven/plugins/gpg/SignAndDeployFileMojo.java b/src/main/java/org/apache/maven/plugins/gpg/SignAndDeployFileMojo.java index 623e3e3..26eb671 100644 --- a/src/main/java/org/apache/maven/plugins/gpg/SignAndDeployFileMojo.java +++ b/src/main/java/org/apache/maven/plugins/gpg/SignAndDeployFileMojo.java @@ -349,7 +349,7 @@ protected void doExecute() throws MojoExecutionException, MojoFailureException { } // sign all - AbstractGpgSigner signer = newSigner(); + AbstractGpgSigner signer = newSigner(null); signer.setOutputDirectory(ascDirectory); signer.setBaseDirectory(new File("").getAbsoluteFile()); diff --git a/src/test/java/org/apache/maven/plugins/gpg/it/GpgSignArtifactIT.java b/src/test/java/org/apache/maven/plugins/gpg/it/GpgSignArtifactIT.java index 69160ed..0a71c02 100644 --- a/src/test/java/org/apache/maven/plugins/gpg/it/GpgSignArtifactIT.java +++ b/src/test/java/org/apache/maven/plugins/gpg/it/GpgSignArtifactIT.java @@ -23,6 +23,7 @@ import java.util.Collection; import org.apache.maven.shared.invoker.InvocationRequest; +import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.MethodSource; @@ -80,4 +81,32 @@ void testPlacementOfArtifactInOutputDirectory(String pomPath, String expectedFil Arrays.sort(expectedFiles); assertEquals(Arrays.toString(expectedFiles), Arrays.toString(outputFiles)); } + + @Test + void testWorstPracticesStillWork() throws Exception { + // given + final File pomFile = InvokerTestUtils.getTestResource("/it/sign-release-in-same-dir/pom.xml"); + final InvocationRequest request = + InvokerTestUtils.createRequest(pomFile, mavenUserSettings, gpgHome, "gpg", false); + request.addArg("-Dgpg.bestPractices=false"); + request.addArg("-Dgpg.passphrase=TEST"); + + final File integrationTestRootDirectory = new File(pomFile.getParent()); + final File expectedOutputDirectory = new File(integrationTestRootDirectory + "/target/tarballs/"); + + // when + InvokerTestUtils.executeRequest(request, mavenHome, localRepository); + + // then + assertTrue(expectedOutputDirectory.isDirectory()); + + String[] outputFiles = expectedOutputDirectory.list(); + assertNotNull(outputFiles); + + String[] expectedFiles = + new String[] {"sign-release-in-same-dir-1.0.jar", "sign-release-in-same-dir-1.0.jar.asc"}; + Arrays.sort(outputFiles); + Arrays.sort(expectedFiles); + assertEquals(Arrays.toString(expectedFiles), Arrays.toString(outputFiles)); + } }