Skip to content

Commit

Permalink
[MGPG-105] Make possible backward compatibility (#74)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
cstamas authored Mar 5, 2024
1 parent 6a94f3a commit 9a73f90
Show file tree
Hide file tree
Showing 6 changed files with 183 additions and 6 deletions.
2 changes: 2 additions & 0 deletions pgp-keys-map.list
Original file line number Diff line number Diff line change
Expand Up @@ -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
22 changes: 22 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,28 @@ under the License.
<version>2.9.0</version>
<type>pom</type>
</dependency>
<dependency>
<groupId>org.codehaus.plexus</groupId>
<artifactId>plexus-sec-dispatcher</artifactId>
<version>2.0</version>
<exclusions>
<exclusion>
<groupId>*</groupId>
<artifactId>*</artifactId>
</exclusion>
</exclusions>
</dependency>
<dependency>
<groupId>org.codehaus.plexus</groupId>
<artifactId>plexus-cipher</artifactId>
<version>2.0</version>
<exclusions>
<exclusion>
<groupId>*</groupId>
<artifactId>*</artifactId>
</exclusion>
</exclusions>
</dependency>

<dependency>
<groupId>org.junit.jupiter</groupId>
Expand Down
132 changes: 128 additions & 4 deletions src/main/java/org/apache/maven/plugins/gpg/AbstractGpgMojo.java
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 "
Expand All @@ -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);
Expand All @@ -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
Expand All @@ -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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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));
}
}

0 comments on commit 9a73f90

Please sign in to comment.