From 34e3be8ae9d5f15b13ac78a1a036ba20b7c0f3d1 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Mon, 23 Sep 2024 15:21:58 +0200 Subject: [PATCH 1/7] [MGPG-136] Windows passphrase corruption This is shot in a dark, but should make possible to at least eliminate (if not even figure out) the issue on Windows with passphrase. --- https://issues.apache.org/jira/browse/MGPG-136 --- .../maven/plugins/gpg/AbstractGpgMojo.java | 21 +++++++++++++++++++ .../maven/plugins/gpg/AbstractGpgSigner.java | 6 ++++++ .../apache/maven/plugins/gpg/GpgSigner.java | 17 +++++++++++---- 3 files changed, 40 insertions(+), 4 deletions(-) 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 a99ce6c..1dcfafb 100644 --- a/src/main/java/org/apache/maven/plugins/gpg/AbstractGpgMojo.java +++ b/src/main/java/org/apache/maven/plugins/gpg/AbstractGpgMojo.java @@ -270,6 +270,26 @@ public abstract class AbstractGpgMojo extends AbstractMojo { @Parameter(property = "gpg.bestPractices", defaultValue = "false") private boolean bestPractices; + /** + * Whether to append the passphrase with LF character or not, as on some systems and some GPG executable combinations + * lack of this character may cause some issues. Since 3.2.0 this character was appended, but as + * {@link System#lineSeparator()} which in turn causes issues in binary handling of stdin (as I assume GPG does), + * since on windows this is {@code "\r\n"} (while on Unix-relatives is only {@code "\n"}, and GPG handles only + * {@code "\n"}. This may lead to "corruption" of passphrase with one extra {@code "\r"} character. This parameter + * affects ONLY the GPG signer, not the BC signer. + *

+ * By default, this parameter is {@code null} and plugin will behave as pre-3.2.7 versions. If this parameter is + * explicitly set to {@code true}, it will append passphrase with {@code "\n"} character ONLY (as opposed to pre-3.2.7 + * behaviour where it used {@link System#lineSeparator()} and used {@code "\r\n"}. If this parameter is + * explicitly set to {@code false}, the passphrase is used as-is, nothing is appended. + * + * @since 3.2.7 + * @see MGPG-99 + * @see MGPG-136 + */ + @Parameter(property = "gpg.passphraseLf") + private Boolean passphraseLf; + /** * Current user system settings for use in Maven. * @@ -345,6 +365,7 @@ protected AbstractGpgSigner newSigner(MavenProject mavenProject) throws MojoFail signer.setPublicKeyring(publicKeyring); signer.setLockMode(lockMode); signer.setArgs(gpgArguments); + signer.setPassphraseLf(passphraseLf); // "new way": env prevails String passphrase = diff --git a/src/main/java/org/apache/maven/plugins/gpg/AbstractGpgSigner.java b/src/main/java/org/apache/maven/plugins/gpg/AbstractGpgSigner.java index 0c1a9a4..86f051d 100644 --- a/src/main/java/org/apache/maven/plugins/gpg/AbstractGpgSigner.java +++ b/src/main/java/org/apache/maven/plugins/gpg/AbstractGpgSigner.java @@ -46,6 +46,8 @@ public abstract class AbstractGpgSigner { protected String passphrase; + protected Boolean passphraseLf; + private File outputDir; private File buildDir; @@ -98,6 +100,10 @@ public void setPassPhrase(String s) { passphrase = s; } + public void setPassphraseLf(Boolean b) { + this.passphraseLf = b; + } + public void setOutputDirectory(File out) { outputDir = out; } diff --git a/src/main/java/org/apache/maven/plugins/gpg/GpgSigner.java b/src/main/java/org/apache/maven/plugins/gpg/GpgSigner.java index 98d5331..d41ec04 100644 --- a/src/main/java/org/apache/maven/plugins/gpg/GpgSigner.java +++ b/src/main/java/org/apache/maven/plugins/gpg/GpgSigner.java @@ -112,11 +112,20 @@ protected void generateSignatureForFile(File file, File signature) throws MojoEx cmd.createArg().setValue("--passphrase-fd"); cmd.createArg().setValue("0"); - // Prepare the input stream which will be used to pass the passphrase to the executable - if (!passphrase.endsWith(System.lineSeparator())) { - in = new ByteArrayInputStream((passphrase + System.lineSeparator()).getBytes()); + // obey passphraseLf + if (passphraseLf != null) { + if (passphraseLf) { + in = new ByteArrayInputStream((passphrase + '\n').getBytes()); + } else { + in = new ByteArrayInputStream((passphrase).getBytes()); + } } else { - in = new ByteArrayInputStream(passphrase.getBytes()); + // Prepare the input stream which will be used to pass the passphrase to the executable + if (!passphrase.endsWith(System.lineSeparator())) { + in = new ByteArrayInputStream((passphrase + System.lineSeparator()).getBytes()); + } else { + in = new ByteArrayInputStream(passphrase.getBytes()); + } } } From 217de842f3f53422b0e2495247420d3eebe84b53 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Tue, 24 Sep 2024 11:54:01 +0200 Subject: [PATCH 2/7] Make new behaviour the fixed LF (instead of CRLF) append --- .../maven/plugins/gpg/AbstractGpgMojo.java | 16 +++++----------- .../maven/plugins/gpg/AbstractGpgSigner.java | 4 ++-- .../org/apache/maven/plugins/gpg/GpgSigner.java | 15 +++------------ 3 files changed, 10 insertions(+), 25 deletions(-) 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 1dcfafb..902b126 100644 --- a/src/main/java/org/apache/maven/plugins/gpg/AbstractGpgMojo.java +++ b/src/main/java/org/apache/maven/plugins/gpg/AbstractGpgMojo.java @@ -272,23 +272,17 @@ public abstract class AbstractGpgMojo extends AbstractMojo { /** * Whether to append the passphrase with LF character or not, as on some systems and some GPG executable combinations - * lack of this character may cause some issues. Since 3.2.0 this character was appended, but as - * {@link System#lineSeparator()} which in turn causes issues in binary handling of stdin (as I assume GPG does), - * since on windows this is {@code "\r\n"} (while on Unix-relatives is only {@code "\n"}, and GPG handles only - * {@code "\n"}. This may lead to "corruption" of passphrase with one extra {@code "\r"} character. This parameter - * affects ONLY the GPG signer, not the BC signer. + * lack of this character may cause GPG to not detected passphrase on STDIN. Since 3.2.0 it was appended. + * This parameter affects ONLY the GPG signer, not the BC signer. *

- * By default, this parameter is {@code null} and plugin will behave as pre-3.2.7 versions. If this parameter is - * explicitly set to {@code true}, it will append passphrase with {@code "\n"} character ONLY (as opposed to pre-3.2.7 - * behaviour where it used {@link System#lineSeparator()} and used {@code "\r\n"}. If this parameter is - * explicitly set to {@code false}, the passphrase is used as-is, nothing is appended. + * By default, this parameter is {@code true} to retain same behaviour as before. * * @since 3.2.7 * @see MGPG-99 * @see MGPG-136 */ - @Parameter(property = "gpg.passphraseLf") - private Boolean passphraseLf; + @Parameter(property = "gpg.passphraseLf", defaultValue = "true") + private boolean passphraseLf; /** * Current user system settings for use in Maven. diff --git a/src/main/java/org/apache/maven/plugins/gpg/AbstractGpgSigner.java b/src/main/java/org/apache/maven/plugins/gpg/AbstractGpgSigner.java index 86f051d..4344db7 100644 --- a/src/main/java/org/apache/maven/plugins/gpg/AbstractGpgSigner.java +++ b/src/main/java/org/apache/maven/plugins/gpg/AbstractGpgSigner.java @@ -46,7 +46,7 @@ public abstract class AbstractGpgSigner { protected String passphrase; - protected Boolean passphraseLf; + protected boolean passphraseLf; private File outputDir; @@ -100,7 +100,7 @@ public void setPassPhrase(String s) { passphrase = s; } - public void setPassphraseLf(Boolean b) { + public void setPassphraseLf(boolean b) { this.passphraseLf = b; } diff --git a/src/main/java/org/apache/maven/plugins/gpg/GpgSigner.java b/src/main/java/org/apache/maven/plugins/gpg/GpgSigner.java index d41ec04..c3573f6 100644 --- a/src/main/java/org/apache/maven/plugins/gpg/GpgSigner.java +++ b/src/main/java/org/apache/maven/plugins/gpg/GpgSigner.java @@ -113,19 +113,10 @@ protected void generateSignatureForFile(File file, File signature) throws MojoEx cmd.createArg().setValue("0"); // obey passphraseLf - if (passphraseLf != null) { - if (passphraseLf) { - in = new ByteArrayInputStream((passphrase + '\n').getBytes()); - } else { - in = new ByteArrayInputStream((passphrase).getBytes()); - } + if (passphraseLf && !passphrase.endsWith("\n")) { + in = new ByteArrayInputStream((passphrase + "\n").getBytes()); } else { - // Prepare the input stream which will be used to pass the passphrase to the executable - if (!passphrase.endsWith(System.lineSeparator())) { - in = new ByteArrayInputStream((passphrase + System.lineSeparator()).getBytes()); - } else { - in = new ByteArrayInputStream(passphrase.getBytes()); - } + in = new ByteArrayInputStream((passphrase).getBytes()); } } From 92373efa30eaae2adab0ed87ed798573916897be Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Tue, 24 Sep 2024 11:55:33 +0200 Subject: [PATCH 3/7] Fix Javadox --- .../java/org/apache/maven/plugins/gpg/AbstractGpgMojo.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 902b126..aa4b50a 100644 --- a/src/main/java/org/apache/maven/plugins/gpg/AbstractGpgMojo.java +++ b/src/main/java/org/apache/maven/plugins/gpg/AbstractGpgMojo.java @@ -272,8 +272,8 @@ public abstract class AbstractGpgMojo extends AbstractMojo { /** * Whether to append the passphrase with LF character or not, as on some systems and some GPG executable combinations - * lack of this character may cause GPG to not detected passphrase on STDIN. Since 3.2.0 it was appended. - * This parameter affects ONLY the GPG signer, not the BC signer. + * lack of this character may cause GPG to not detect passphrase on STDIN. Since 3.2.0 it was always appended, unless + * passphrase itself ended with line separator. This parameter affects ONLY the GPG signer, not the BC signer. *

* By default, this parameter is {@code true} to retain same behaviour as before. * From 446185769a44305aa7dc93fc25f2a424f36c5d9b Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Tue, 24 Sep 2024 11:56:39 +0200 Subject: [PATCH 4/7] Remove extra braces --- src/main/java/org/apache/maven/plugins/gpg/GpgSigner.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/apache/maven/plugins/gpg/GpgSigner.java b/src/main/java/org/apache/maven/plugins/gpg/GpgSigner.java index c3573f6..21ca193 100644 --- a/src/main/java/org/apache/maven/plugins/gpg/GpgSigner.java +++ b/src/main/java/org/apache/maven/plugins/gpg/GpgSigner.java @@ -116,7 +116,7 @@ protected void generateSignatureForFile(File file, File signature) throws MojoEx if (passphraseLf && !passphrase.endsWith("\n")) { in = new ByteArrayInputStream((passphrase + "\n").getBytes()); } else { - in = new ByteArrayInputStream((passphrase).getBytes()); + in = new ByteArrayInputStream(passphrase.getBytes()); } } From 68dc78647b5af4ddd79a0179a5fc34fb0d067cb7 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Tue, 24 Sep 2024 11:57:12 +0200 Subject: [PATCH 5/7] more doco fixes --- src/main/java/org/apache/maven/plugins/gpg/GpgSigner.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/apache/maven/plugins/gpg/GpgSigner.java b/src/main/java/org/apache/maven/plugins/gpg/GpgSigner.java index 21ca193..1d0686f 100644 --- a/src/main/java/org/apache/maven/plugins/gpg/GpgSigner.java +++ b/src/main/java/org/apache/maven/plugins/gpg/GpgSigner.java @@ -112,7 +112,8 @@ protected void generateSignatureForFile(File file, File signature) throws MojoEx cmd.createArg().setValue("--passphrase-fd"); cmd.createArg().setValue("0"); - // obey passphraseLf + // Prepare the input stream which will be used to pass the passphrase to the executable + // but obey passphraseLf: append LF if asked for if (passphraseLf && !passphrase.endsWith("\n")) { in = new ByteArrayInputStream((passphrase + "\n").getBytes()); } else { From f3b5391134a2adcd43c15c9bb4e742ee67130dfe Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Tue, 24 Sep 2024 12:40:37 +0200 Subject: [PATCH 6/7] Rename the parameter --- .../apache/maven/plugins/gpg/AbstractGpgMojo.java | 14 +++++++------- .../maven/plugins/gpg/AbstractGpgSigner.java | 6 +++--- .../org/apache/maven/plugins/gpg/GpgSigner.java | 6 +++--- 3 files changed, 13 insertions(+), 13 deletions(-) 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 aa4b50a..1db45d3 100644 --- a/src/main/java/org/apache/maven/plugins/gpg/AbstractGpgMojo.java +++ b/src/main/java/org/apache/maven/plugins/gpg/AbstractGpgMojo.java @@ -271,18 +271,18 @@ public abstract class AbstractGpgMojo extends AbstractMojo { private boolean bestPractices; /** - * Whether to append the passphrase with LF character or not, as on some systems and some GPG executable combinations - * lack of this character may cause GPG to not detect passphrase on STDIN. Since 3.2.0 it was always appended, unless - * passphrase itself ended with line separator. This parameter affects ONLY the GPG signer, not the BC signer. + * Whether to terminate the passphrase with LF character or not, as on some systems and some GPG executable combinations + * lack of trailing LF may cause GPG to not detect passphrase on STDIN. Since 3.2.0 it was always appended, unless + * passphrase itself ended with it. This parameter affects ONLY the GPG signer, not the BC signer. *

- * By default, this parameter is {@code true} to retain same behaviour as before. + * By default, this parameter is {@code true}. * * @since 3.2.7 * @see MGPG-99 * @see MGPG-136 */ - @Parameter(property = "gpg.passphraseLf", defaultValue = "true") - private boolean passphraseLf; + @Parameter(property = "gpg.terminatePassphrase", defaultValue = "true") + private boolean terminatePassphrase; /** * Current user system settings for use in Maven. @@ -359,7 +359,7 @@ protected AbstractGpgSigner newSigner(MavenProject mavenProject) throws MojoFail signer.setPublicKeyring(publicKeyring); signer.setLockMode(lockMode); signer.setArgs(gpgArguments); - signer.setPassphraseLf(passphraseLf); + signer.setTerminatePassphrase(terminatePassphrase); // "new way": env prevails String passphrase = diff --git a/src/main/java/org/apache/maven/plugins/gpg/AbstractGpgSigner.java b/src/main/java/org/apache/maven/plugins/gpg/AbstractGpgSigner.java index 4344db7..56004ae 100644 --- a/src/main/java/org/apache/maven/plugins/gpg/AbstractGpgSigner.java +++ b/src/main/java/org/apache/maven/plugins/gpg/AbstractGpgSigner.java @@ -46,7 +46,7 @@ public abstract class AbstractGpgSigner { protected String passphrase; - protected boolean passphraseLf; + protected boolean terminatePassphrase; private File outputDir; @@ -100,8 +100,8 @@ public void setPassPhrase(String s) { passphrase = s; } - public void setPassphraseLf(boolean b) { - this.passphraseLf = b; + public void setTerminatePassphrase(boolean b) { + this.terminatePassphrase = b; } public void setOutputDirectory(File out) { diff --git a/src/main/java/org/apache/maven/plugins/gpg/GpgSigner.java b/src/main/java/org/apache/maven/plugins/gpg/GpgSigner.java index 1d0686f..d63bd5f 100644 --- a/src/main/java/org/apache/maven/plugins/gpg/GpgSigner.java +++ b/src/main/java/org/apache/maven/plugins/gpg/GpgSigner.java @@ -112,9 +112,9 @@ protected void generateSignatureForFile(File file, File signature) throws MojoEx cmd.createArg().setValue("--passphrase-fd"); cmd.createArg().setValue("0"); - // Prepare the input stream which will be used to pass the passphrase to the executable - // but obey passphraseLf: append LF if asked for - if (passphraseLf && !passphrase.endsWith("\n")) { + // Prepare the STDIN stream which will be used to pass the passphrase to the executable + // but obey terminatePassphrase: append LF if asked for + if (terminatePassphrase && !passphrase.endsWith("\n")) { in = new ByteArrayInputStream((passphrase + "\n").getBytes()); } else { in = new ByteArrayInputStream(passphrase.getBytes()); From e03759639ee69ee865dafb4732ecefe19eda30c1 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Tue, 24 Sep 2024 12:48:32 +0200 Subject: [PATCH 7/7] Make mention of pre 3.2.7 termination --- .../java/org/apache/maven/plugins/gpg/AbstractGpgMojo.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) 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 1db45d3..e1c9a23 100644 --- a/src/main/java/org/apache/maven/plugins/gpg/AbstractGpgMojo.java +++ b/src/main/java/org/apache/maven/plugins/gpg/AbstractGpgMojo.java @@ -273,7 +273,9 @@ public abstract class AbstractGpgMojo extends AbstractMojo { /** * Whether to terminate the passphrase with LF character or not, as on some systems and some GPG executable combinations * lack of trailing LF may cause GPG to not detect passphrase on STDIN. Since 3.2.0 it was always appended, unless - * passphrase itself ended with it. This parameter affects ONLY the GPG signer, not the BC signer. + * passphrase itself ended with it. Note: before 3.2.7 the "line separator" was used for termination, that on + * other hand caused issues on Windows, where line separator is CRLF while GPG handles LF only. + * This parameter affects ONLY the GPG signer, not the BC signer. *

* By default, this parameter is {@code true}. *