[MGPG-105] [MGPG-108] Make plugin backward compat and update site and doco (#77)

Document the latest changes. But also implement Java changes related to agent usage and back compat. Now we distinguish really (and option is un-deprecated): * `useAgent` * `interactive` Means to provide secret needed for signing: |flag|agent pinentry|agent cached|env variable| |---|---|---|---| |`useAgent && interactive` | ✔️ | ✔️ | ✔️ | |`useAgent && !interactive` | ❌ | ✔️ | ✔️ | |`!useAgent && interactive` | ❌ | ❌ | ✔️ | |`!useAgent && !interactive` | ❌ | ❌ | ✔️ | Finally, `!bestPractices` provides existing "pass in passphrase as property" mode as well. As first really means "can we talk to the agent" and second means "can agent pop up pinentry dialogue" for both signers. In fact, this was the case already in `GpgSigner`, but `BcSigner` conflated the two. As it turns out, `gpg-agent` also supports "non interactive" password caching that now both signers make use of. --- https://issues.apache.org/jira/browse/MGPG-108 https://issues.apache.org/jira/browse/MGPG-105

Author: cstamas

src/main/java/org/apache/maven/plugins/gpg/AbstractGpgMojo.java

@@ -51,11 +51,13 @@ public abstract class AbstractGpgMojo extends AbstractMojo {
  private String agentSocketLocations;
 
  /**
- * BC Signer only: The path of the exported key in TSK format, and probably passphrase protected. If relative,
- * the file is resolved against Maven local repository root.
+ * BC Signer only: The path of the exported key in
+ * <a href="https://openpgp.dev/book/private_keys.html#transferable-secret-key-format">TSK format</a>,
+ * and may be passphrase protected. If relative, the file is resolved against user home directory.
  * <p>
- * <em>Note: it is not recommended to have sensitive files on disk or SCM repository, this mode is more to be used
- * in local environment (workstations) or for testing purposes.</em>
+ * <em>Note: it is not recommended to have sensitive files checked into SCM repository. Key file should reside on
+ * developer workstation, outside of SCM tracked repository. For CI-like use cases you should set the
+ * key material as env variable instead.</em>
  *
  * @since 3.2.0
  */
@@ -71,9 +73,11 @@ public abstract class AbstractGpgMojo extends AbstractMojo {
  private String keyFingerprint;
 
  /**
- * BC Signer only: The env variable name where the GnuPG key is set. The default value is {@code MAVEN_GPG_KEY}.
+ * BC Signer only: The env variable name where the GnuPG key is set.
  * To use BC Signer you must provide GnuPG key, as it does not use GnuPG home directory to extract/find the
- * key (while it does use GnuPG Agent to ask for password in interactive mode).
+ * key (while it does use GnuPG Agent to ask for password in interactive mode). The key should be in
+ * <a href="https://openpgp.dev/book/private_keys.html#transferable-secret-key-format">TSK format</a> and may
+ * be passphrase protected.
  *
  * @since 3.2.0
  */
@@ -82,16 +86,16 @@ public abstract class AbstractGpgMojo extends AbstractMojo {
 
  /**
  * BC Signer only: The env variable name where the GnuPG key fingerprint is set, if the provided keyring contains
- * multiple keys. The default value is {@code MAVEN_GPG_KEY_FINGERPRINT}.
+ * multiple keys.
  *
  * @since 3.2.0
  */
  @Parameter(property = "gpg.keyFingerprintEnvName", defaultValue = DEFAULT_ENV_MAVEN_GPG_FINGERPRINT)
  private String keyFingerprintEnvName;
 
  /**
- * The env variable name where the GnuPG passphrase is set. The default value is {@code MAVEN_GPG_PASSPHRASE}.
- * This is the recommended way to pass passphrase for signing in batch mode execution of Maven.
+ * The env variable name where the GnuPG passphrase is set. This is the recommended way to pass passphrase
+ * for signing in batch mode execution of Maven.
  *
  * @since 3.2.0
  */
@@ -109,23 +113,25 @@ public abstract class AbstractGpgMojo extends AbstractMojo {
 
  /**
  * The passphrase to use when signing. If not given, look up the value under Maven
- * settings using server id at 'passphraseServerKey' configuration. <em>Do not use this parameter, if set, the
- * plugin will fail. Passphrase should be provided only via gpg-agent (interactive) or via env variable
- * (non-interactive).</em>
+ * settings using server id at 'passphraseServerKey' configuration. <em>Do not use this parameter, it leaks
+ * sensitive data. Passphrase should be provided only via gpg-agent or via env variable.
+ * If parameter {@link #bestPractices} set to {@code true}, plugin fails when this parameter is configured.</em>
  *
- * @deprecated Do not use this configuration, plugin will fail if set.
+ * @deprecated Do not use this configuration, it may leak sensitive information. Rely on gpg-agent or env
+ * variables instead.
  **/
  @Deprecated
  @Parameter(property = "gpg.passphrase")
  private String passphrase;
 
  /**
- * Server id to lookup the passphrase under Maven settings. <em>Do not use this parameter, if set, the
- * plugin will fail. Passphrase should be provided only via gpg-agent (interactive) or via env variable
- * (non-interactive).</em>
+ * Server id to lookup the passphrase under Maven settings. <em>Do not use this parameter, it leaks
+ * sensitive data. Passphrase should be provided only via gpg-agent or via env variable.
+ * If parameter {@link #bestPractices} set to {@code true}, plugin fails when this parameter is configured.</em>
  *
  * @since 1.6
- * @deprecated Do not use this configuration, plugin will fail if set.
+ * @deprecated Do not use this configuration, it may leak sensitive information. Rely on gpg-agent or env
+ * variables instead.
  **/
  @Deprecated
  @Parameter(property = "gpg.passphraseServerId")
@@ -138,23 +144,22 @@ public abstract class AbstractGpgMojo extends AbstractMojo {
  private String keyname;
 
  /**
- * GPG Signer only: Passes <code>--use-agent</code> or <code>--no-use-agent</code> to gpg. If using an agent, the
- * passphrase is optional as the agent will provide it. For gpg2, specify true as --no-use-agent was removed in
- * gpg2 and doesn't ask for a passphrase anymore. Deprecated, and better to rely on session "interactive" setting
- * (if interactive, agent will be used, otherwise not).
- *
- * @deprecated
+ * All signers: whether gpg-agent is allowed to be used or not. If enabled, passphrase is optional, as agent may
+ * provide it. Have to be noted, that in "batch" mode, gpg-agent will be prevented to pop up pinentry
+ * dialogue, hence best is to "prime" the agent caches beforehand.
+ * <p>
+ * GPG Signer: Passes <code>--use-agent</code> or <code>--no-use-agent</code> option to gpg if it is version 2.1
+ * or older. Otherwise, will use an agent. In non-interactive mode gpg options are appended with
+ * <code>--pinentry-mode error</code>, preventing gpg agent to pop up pinentry dialogue. Agent will be able to
+ * hand over only cached passwords.
+ * <p>
+ * BC Signer: Allows signer to communicate with gpg agent. In non-interactive mode it uses
+ * <code>--no-ask</code> option with the <code>GET_PASSPHRASE</code> function. Agent will be able to hand over
+ * only cached passwords.
  */
- @Deprecated
  @Parameter(property = "gpg.useagent", defaultValue = "true")
  private boolean useAgent;
 
- /**
- * Detect is session interactive or not.
- */
- @Parameter(defaultValue = "${settings.interactiveMode}", readonly = true)
- private boolean interactive;
-
  /**
  * GPG Signer only: The path to the GnuPG executable to use for artifact signing. Defaults to either "gpg" or
  * "gpg.exe" depending on the operating system.
@@ -182,7 +187,7 @@ public abstract class AbstractGpgMojo extends AbstractMojo {
  * ‘private-keys-v1.d’ directory below the GnuPG home directory.
  *
  * @since 1.2
- * @deprecated
+ * @deprecated Obsolete option since GnuPG 2.1 version.
  */
  @Deprecated
  @Parameter(property = "gpg.secretKeyring")
@@ -198,7 +203,7 @@ public abstract class AbstractGpgMojo extends AbstractMojo {
  * ‘pubring.kbx’ file below the GnuPG home directory.
  *
  * @since 1.2
- * @deprecated
+ * @deprecated Obsolete option since GnuPG 2.1 version.
  */
  @Deprecated
  @Parameter(property = "gpg.publicKeyring")
@@ -224,7 +229,7 @@ public abstract class AbstractGpgMojo extends AbstractMojo {
  private boolean skip;
 
  /**
- * Sets the arguments to be passed to gpg. Example:
+ * GPG Signer only: Sets the arguments to be passed to gpg. Example:
  *
  * <pre>
  * &lt;gpgArguments&gt;
@@ -256,32 +261,32 @@ public abstract class AbstractGpgMojo extends AbstractMojo {
  // === 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.
+ * Switch to improve plugin enforcement of "best practices". If set to {@code false}, plugin retains all the
+ * backward compatibility regarding getting secrets (but will warn). If set to {@code true}, plugin will fail
+ * if any "bad practices" regarding sensitive data handling are detected. By default, plugin remains backward
+ * compatible (this flag is {@code false}). Somewhere in the future, when this parameter enabling transitioning
+ * from older plugin versions is removed, the logic using this flag will be modified like it is set to {@code true}.
+ * It is warmly advised to configure this parameter to {@code true} and migrate project and user environment
+ * regarding how sensitive information is stored.
  *
  * @since 3.2.0
- * @deprecated
  */
- @Deprecated
- @Parameter(property = "gpg.bestPractices", defaultValue = "true")
+ @Parameter(property = "gpg.bestPractices", defaultValue = "false")
  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 Provides quasi-encryption, should be avoided.
  */
  @Deprecated
  @Component
@@ -310,7 +315,7 @@ private void logBestPracticeWarning(String source) {
  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("instead rely on GnuPG agent or provide passphrase in ");
  getLog().warn(passphraseEnvName + " environment variable for batch mode.");
  getLog().warn("");
  getLog().warn("Sensitive content loaded from " + source);
@@ -334,7 +339,7 @@ protected AbstractGpgSigner newSigner(MavenProject mavenProject) throws MojoFail
  }
 
  signer.setLog(getLog());
- signer.setInteractive(interactive);
+ signer.setInteractive(settings.isInteractiveMode());
  signer.setKeyName(keyname);
  signer.setUseAgent(useAgent);
  signer.setHomeDirectory(homedir);
@@ -371,13 +376,6 @@ protected AbstractGpgSigner newSigner(MavenProject mavenProject) throws MojoFail
  }
  }
  }
-
- // gpg signer: always failed if no passphrase and no agent and not interactive: retain this behavior
- // bc signer: it is optimistic, will fail during prepare() only IF key is passphrase protected
- if (GpgSigner.NAME.equals(this.signer) && null == passphrase && !useAgent && !interactive) {
- throw new MojoFailureException("Cannot obtain passphrase in batch mode");
- }
-
  signer.prepare();
 
  return signer;
@@ -419,7 +417,7 @@ public String getPassphrase(MavenProject project) {
  pass = prj2.getProperties().getProperty(GPG_PASSPHRASE);
  }
  }
- if (project != null) {
+ if (project != null && pass != null) {
  findReactorProject(project).getProperties().setProperty(GPG_PASSPHRASE, pass);
  }
  return pass;

src/main/java/org/apache/maven/plugins/gpg/BcSigner.java

@@ -34,6 +34,7 @@
 import java.time.ZoneId;
 import java.util.Arrays;
 import java.util.List;
+import java.util.Locale;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
@@ -71,11 +72,6 @@ public class BcSigner extends AbstractGpgSigner {
  public static final String NAME = "bc";
 
  public interface Loader {
- /**
- * Returns {@code true} if this loader requires user interactivity.
- */
- boolean isInteractive();
-
  /**
  * Returns the key ring material, or {@code null}.
  */
@@ -93,17 +89,12 @@ default byte[] loadKeyFingerprint(RepositorySystemSession session) throws IOExce
  /**
  * Returns the key password, or {@code null}.
  */
- default char[] loadPassword(RepositorySystemSession session, long keyId) throws IOException {
+ default char[] loadPassword(RepositorySystemSession session, byte[] fingerprint) throws IOException {
  return null;
  }
  }
 
  public final class GpgEnvLoader implements Loader {
- @Override
- public boolean isInteractive() {
- return false;
- }
-
  @Override
  public byte[] loadKeyRingMaterial(RepositorySystemSession session) {
  String keyMaterial = (String) session.getConfigProperties().get("env." + keyEnvName);
@@ -134,16 +125,13 @@ public final class GpgConfLoader implements Loader {
  */
  private static final long MAX_SIZE = 5 * 1024 + 1L;
 
- @Override
- public boolean isInteractive() {
- return false;
- }
-
  @Override
  public byte[] loadKeyRingMaterial(RepositorySystemSession session) throws IOException {
  Path keyPath = Paths.get(keyFilePath);
  if (!keyPath.isAbsolute()) {
- keyPath = session.getLocalRepository().getBasedir().toPath().resolve(keyPath);
+ keyPath = Paths.get(System.getProperty("user.home"))
+ .resolve(keyPath)
+ .toAbsolutePath();
  }
  if (Files.isRegularFile(keyPath)) {
  if (Files.size(keyPath) < MAX_SIZE) {
@@ -171,27 +159,33 @@ public byte[] loadKeyFingerprint(RepositorySystemSession session) {
 
  public final class GpgAgentPasswordLoader implements Loader {
  @Override
- public boolean isInteractive() {
- return true;
- }
-
- @Override
- public char[] loadPassword(RepositorySystemSession session, long keyId) throws IOException {
+ public char[] loadPassword(RepositorySystemSession session, byte[] fingerprint) throws IOException {
+ if (!useAgent) {
+ return null;
+ }
  List<String> socketLocations = Arrays.stream(agentSocketLocations.split(","))
  .filter(s -> s != null && !s.isEmpty())
  .collect(Collectors.toList());
  for (String socketLocation : socketLocations) {
  try {
- return load(keyId, Paths.get(System.getProperty("user.home"), socketLocation))
- .toCharArray();
+ Path socketLocationPath = Paths.get(socketLocation);
+ if (!socketLocationPath.isAbsolute()) {
+ socketLocationPath = Paths.get(System.getProperty("user.home"))
+ .resolve(socketLocationPath)
+ .toAbsolutePath();
+ }
+ String pw = load(fingerprint, socketLocationPath);
+ if (pw != null) {
+ return pw.toCharArray();
+ }
  } catch (SocketException e) {
  // try next location
  }
  }
  return null;
  }
 
- private String load(long keyId, Path socketPath) throws IOException {
+ private String load(byte[] fingerprint, Path socketPath) throws IOException {
  try (AFUNIXSocket sock = AFUNIXSocket.newInstance()) {
  sock.connect(AFUNIXSocketAddress.of(socketPath));
  try (BufferedReader in = new BufferedReader(new InputStreamReader(sock.getInputStream()));
@@ -210,23 +204,43 @@ private String load(long keyId, Path socketPath) throws IOException {
  os.flush();
  expectOK(in);
  }
- String hexKeyId = Long.toHexString(keyId & 0xFFFFFFFFL);
+ String hexKeyFingerprint = Hex.toHexString(fingerprint);
+ String displayFingerprint = hexKeyFingerprint.toUpperCase(Locale.ROOT);
  // https://unix.stackexchange.com/questions/71135/how-can-i-find-out-what-keys-gpg-agent-has-cached-like-how-ssh-add-l-shows-yo
- String instruction = "GET_PASSPHRASE " + hexKeyId + " " + "Passphrase+incorrect"
- + " GnuPG+Key+Passphrase Enter+passphrase+for+encrypted+GnuPG+key+" + hexKeyId
+ String instruction = "GET_PASSPHRASE "
+ + (!isInteractive ? "--no-ask " : "")
+ + hexKeyFingerprint
+ + " "
+ + "X "
+ + "GnuPG+Passphrase "
+ + "Please+enter+the+passphrase+to+unlock+the+OpenPGP+secret+key+with+fingerprint:+"
+ + displayFingerprint
  + "+to+use+it+for+signing+Maven+Artifacts\n";
  os.write((instruction).getBytes());
  os.flush();
- return new String(Hex.decode(expectOK(in).trim()));
+ String pw = mayExpectOK(in);
+ if (pw != null) {
+ return new String(Hex.decode(pw.trim()));
+ }
+ return null;
  }
  }
  }
 
- private String expectOK(BufferedReader in) throws IOException {
+ private void expectOK(BufferedReader in) throws IOException {
  String response = in.readLine();
  if (!response.startsWith("OK")) {
  throw new IOException("Expected OK but got this instead: " + response);
  }
+ }
+
+ private String mayExpectOK(BufferedReader in) throws IOException {
+ String response = in.readLine();
+ if (response.startsWith("ERR")) {
+ return null;
+ } else if (!response.startsWith("OK")) {
+ throw new IOException("Expected OK/ERR but got this instead: " + response);
+ }
  return response.substring(Math.min(response.length(), 3));
  }
  }
@@ -265,7 +279,6 @@ public String signerName() {
  public void prepare() throws MojoFailureException {
  try {
  List<Loader> loaders = Stream.of(new GpgEnvLoader(), new GpgConfLoader(), new GpgAgentPasswordLoader())
- .filter(l -> this.isInteractive || !l.isInteractive())
  .collect(Collectors.toList());
 
  byte[] keyRingMaterial = null;
@@ -327,7 +340,7 @@ public void prepare() throws MojoFailureException {
  final boolean keyPassNeeded = secretKey.getKeyEncryptionAlgorithm() != SymmetricKeyAlgorithmTags.NULL;
  if (keyPassNeeded && keyPassword == null) {
  for (Loader loader : loaders) {
- keyPassword = loader.loadPassword(session, secretKey.getKeyID());
+ keyPassword = loader.loadPassword(session, secretKey.getFingerprint());
  if (keyPassword != null) {
  break;
  }