Volume XXII, number 279Tuesday, October 6, 2026Latest message 47 minutes ago

The Git List

News and archive of git@vger.kernel.org, since April 2005

patchgpg-interface: still print ssh signatures when allowed signers file is not set

4 messages between Jun 25, 2026 and Jul 10, 2026, from Grayson Tinker, Junio C Hamano.

Plain Markdown or JSON for tools and agents. Diffs are folded; open one to read it.

Grayson TinkerJun 25, 2026, 19:43 UTC on lore

"show-signature" errors when the allowed signers file is not configured, which means that the user can't see the key that the ref was signed with without creating and configuring the file. Change the logic so that the file is only used when configured, and so the signature status is always displayed.

Example of previous output:
```
error: gpg.ssh.allowedSignersFile needs to be configured and exist for ssh signature verification
commit b437db5ddc38ebda223bbae2087eee90a7b1c6e2 (HEAD -> master)
No signature
Author: Grayson Tinker <graysontinker@gmail.com>
```
Example of new output:
```
commit b437db5ddc38ebda223bbae2087eee90a7b1c6e2 (HEAD -> master)
hint: Configure gpg.ssh.allowedSignersFile for automatic principal matching
Good "git" signature with ED25519-SK key SHA256:yTU4KFs/g6MY7biDSlVStB63Gi1rCKg7dOFDXbe0yuw
Author: Grayson Tinker <graysontinker@gmail.com>
```
Signed-off-by: Grayson Tinker <graysontinker@gmail.com>
---
 gpg-interface.c | 42 +++++++++++++++++++++++-------------------
 1 file changed, 23 insertions(+), 19 deletions(-)
Show changes to gpg-interface.c +23 −19
diff --git a/gpg-interface.c b/gpg-interface.c
index dafd5371fa..ef3e1a0aa0 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -1,6 +1,7 @@
 #define USE_THE_REPOSITORY_VARIABLE
 
 #include "git-compat-util.h"
+#include "advice.h"
 #include "commit.h"
 #include "config.h"
 #include "date.h"
@@ -480,11 +481,6 @@ static int verify_ssh_signed_buffer(struct signature_check *sigc,
 		.local = 1,
 	};
 
-	if (!ssh_allowed_signers) {
-		error(_("gpg.ssh.allowedSignersFile needs to be configured and exist for ssh signature verification"));
-		return -1;
-	}
-
 	buffer_file = mks_tempfile_t(".git_vtag_tmpXXXXXX");
 	if (!buffer_file)
 		return error_errno(_("could not create temporary file"));
@@ -500,22 +496,26 @@ static int verify_ssh_signed_buffer(struct signature_check *sigc,
 		strbuf_addf(&verify_time, "-Overify-time=%s",
 			show_date(sigc->payload_timestamp, 0, verify_date_mode));
 
-	/* Find the principal from the signers */
-	strvec_pushl(&ssh_keygen.args, fmt->program,
-		     "-Y", "find-principals",
-		     "-f", ssh_allowed_signers,
-		     "-s", buffer_file->filename.buf,
-		     verify_time.buf,
-		     NULL);
-	ret = pipe_command(&ssh_keygen, NULL, 0, &ssh_principals_out, 0,
-			   &ssh_principals_err, 0);
-	if (ret && strstr(ssh_principals_err.buf, "usage:")) {
-		error(_("ssh-keygen -Y find-principals/verify is needed for ssh signature verification (available in openssh version 8.2p1+)"));
-		goto out;
+	if (ssh_allowed_signers) {
+		/* Find the principal from the signers */
+		strvec_pushl(&ssh_keygen.args, fmt->program,
+				"-Y", "find-principals",
+				"-f", ssh_allowed_signers,
+				"-s", buffer_file->filename.buf,
+				verify_time.buf,
+				NULL);
+		ret = pipe_command(&ssh_keygen, NULL, 0, &ssh_principals_out, 0,
+				&ssh_principals_err, 0);
+		if (ret && strstr(ssh_principals_err.buf, "usage:")) {
+			error(_("ssh-keygen -Y find-principals/verify is needed for ssh signature verification (available in openssh version 8.2p1+)"));
+			goto out;
+		}
 	}
-	if (ret || !ssh_principals_out.len) {
+
+	if (!ssh_allowed_signers || ret || !ssh_principals_out.len) {
 		/*
-		 * We did not find a matching principal in the allowedSigners
+		 * We did not find a matching principal in the allowedSigners,
+		 * or no allowedSigners file was configured
 		 * Check without validation
 		 */
 		child_process_init(&ssh_keygen);
@@ -528,6 +528,10 @@ static int verify_ssh_signed_buffer(struct signature_check *sigc,
 		pipe_command(&ssh_keygen, sigc->payload, sigc->payload_len,
 				   &ssh_keygen_out, 0, &ssh_keygen_err, 0);
 
+		if (!ssh_allowed_signers) {
+			advise(_("Configure gpg.ssh.allowedSignersFile for automatic principal matching\n"));
+		}
+
 		/*
 		 * Fail on unknown keys
 		 * we still call check-novalidate to display the signature info

-- 
2.54.0
Grayson TinkerJul 9, 2026, 23:50 UTC in reply to Grayson Tinker on lore

Re: [PATCH] gpg-interface: still print ssh signatures when allowed signers file is not set

Friendly ping on this :)
Junio C HamanoJul 10, 2026, 01:59 UTC in reply to Grayson Tinker on lore

Re: [PATCH] gpg-interface: still print ssh signatures when allowed signers file is not set

Grayson Tinker <graysontinker@gmail.com> writes:
Show 20 quoted lines
> "show-signature" errors when the allowed signers file is not configured,
> which means that the user can't see the key that the ref was signed with
> without creating and configuring the file. Change the logic so that the file
> is only used when configured, and so the signature status is always displayed.
>
> Example of previous output:
> ```
> error: gpg.ssh.allowedSignersFile needs to be configured and exist for ssh signature verification
> commit b437db5ddc38ebda223bbae2087eee90a7b1c6e2 (HEAD -> master)
> No signature
> Author: Grayson Tinker <graysontinker@gmail.com>
> ```
>
> Example of new output:
> ```
> commit b437db5ddc38ebda223bbae2087eee90a7b1c6e2 (HEAD -> master)
> hint: Configure gpg.ssh.allowedSignersFile for automatic principal matching
> Good "git" signature with ED25519-SK key SHA256:yTU4KFs/g6MY7biDSlVStB63Gi1rCKg7dOFDXbe0yuw
> Author: Grayson Tinker <graysontinker@gmail.com>
> ```

While I haven't closely looked at the parts of this patch that I did not quote here, this specific section caught my eye:

Show 7 quoted lines
> @@ -528,6 +528,10 @@ static int verify_ssh_signed_buffer(struct signature_check *sigc,
>  		pipe_command(&ssh_keygen, sigc->payload, sigc->payload_len,
>  				   &ssh_keygen_out, 0, &ssh_keygen_err, 0);
>  
> +		if (!ssh_allowed_signers) {
> +			advise(_("Configure gpg.ssh.allowedSignersFile for automatic principal matching\n"));
> +		}

If a user runs 'git log --show-signature -100', they will be spammed with this message 100 times. Because it bypasses the advise_if_enabled() mechanism, there is no way for them to disable it.

Since I don't use SSH signing, I'm curious: how common or useful is it to run log --show-signature without allowedSignersFile configured? If it serves no purpose at all, then perhaps this warning is acceptable, as users would have to configure the variable to get any utility out of the command.

However, doesn't cryptographic verification still provide value on its own? Even without allowedSignersFile, the signature at least guarantees the commit content hasn't been modified since it was signed, even if the signer's identity remains unverified. If some users rely on this purely cryptographic validation, they probably won't want to maintain an allowed signers file, and they would definitely want a way to squelch this repetitive advice. 

Thanks.
Grayson TinkerJul 10, 2026, 03:42 UTC in reply to Junio C Hamano on lore

Re: [PATCH] gpg-interface: still print ssh signatures when allowed signers file is not set

On Thu, Jul 9, 2026 at 6:59 PM Junio C Hamano <gitster@pobox.com> wrote:
Show 5 quoted lines
>
> If a user runs 'git log --show-signature -100', they will be spammed
> with this message 100 times.  Because it bypasses the
> advise_if_enabled() mechanism, there is no way for them to disable
> it.

The downside to using advise_if_enabled is that this single line would get turned into three, with a newline in the middle, which would decently disrupt the view of the log until the hint is disabled. I'm not sure what the best solution is here; I lean towards keeping it as is to reduce the overall noise level, but perhaps those who use this feature more would disagree.

(The previous message was also printed every time, FWIW. So at the very least this isn't worse behavior.)

I'll make the hint disableable if you'd prefer.
Show 7 quoted lines
> However, doesn't cryptographic verification still provide value on
> its own?  Even without allowedSignersFile, the signature at least
> guarantees the commit content hasn't been modified since it was
> signed, even if the signer's identity remains unverified.  If some
> users rely on this purely cryptographic validation, they probably
> won't want to maintain an allowed signers file, and they would
> definitely want a way to squelch this repetitive advice.

Allowing for this usecase was exactly the intent of this patch; I am this type of user and had some annoyance with this.

Thanks!

Back to recent threads