From 63c55177e50cc790b9bee7f14334b0296718a9e9 Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Thu, 22 May 2025 19:50:27 +0200 Subject: [PATCH 1/6] gpg-interface: simplify ssh fingerprint parsing In "gpg-interface.c", the 'parse_ssh_output()' function takes a 'struct signature_check *sigc' argument and populates many members of this 'sigc' using information parsed from 'sigc->output' which contains the ouput of an `ssh-keygen -Y ...` command that was used to verify an SSH signature. When it populates 'sigc->fingerprint' though, it uses `xstrdup(strstr(line, "key ") + 4)` while `strstr(line, "key ")` has already been computed a few lines above and is already available in the `key` variable. Let's simplify this. Signed-off-by: Christian Couder --- gpg-interface.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gpg-interface.c b/gpg-interface.c index 0896458de5a..e7af82d123e 100644 --- a/gpg-interface.c +++ b/gpg-interface.c @@ -431,7 +431,7 @@ static void parse_ssh_output(struct signature_check *sigc) key = strstr(line, "key "); if (key) { - sigc->fingerprint = xstrdup(strstr(line, "key ") + 4); + sigc->fingerprint = xstrdup(key + 4); sigc->key = xstrdup(sigc->fingerprint); } else { /* -- GitLab From 2ba54d6ee0645ba3d2fc7d279bc9534ba18ca310 Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Fri, 23 May 2025 13:59:54 +0200 Subject: [PATCH 2/6] gpg-interface: use left shift to define GPG_VERIFY_* In "gpg-interface.h", the definitions of the GPG_VERIFY_* boolean flags are currently using 1, 2 and 4 while we often prefer the bitwise left shift operator, `<<`, for that purpose to make it clearer that they are boolean. Let's use the left shift operator here too. Let's also fix an indent issue with "4" while at it. Signed-off-by: Christian Couder --- gpg-interface.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/gpg-interface.h b/gpg-interface.h index e09f12e8d04..9a32dd6ce87 100644 --- a/gpg-interface.h +++ b/gpg-interface.h @@ -3,9 +3,9 @@ struct strbuf; -#define GPG_VERIFY_VERBOSE 1 -#define GPG_VERIFY_RAW 2 -#define GPG_VERIFY_OMIT_STATUS 4 +#define GPG_VERIFY_VERBOSE (1<<0) +#define GPG_VERIFY_RAW (1<<1) +#define GPG_VERIFY_OMIT_STATUS (1<<2) enum signature_trust_level { TRUST_UNDEFINED, -- GitLab From 8724a5669761cf172a7a05da8d13374aeaff9c56 Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Fri, 23 May 2025 10:48:46 +0200 Subject: [PATCH 3/6] doc/verify-commit: update and improve the whole doc The documentation of the `git verify-commit` commands currently looks very outdated and minimal. Especially it has the following issues: - It only talks about verifying GPG signatures while the command actually supports verifying other signatures like SSH ones. - It's not clear what the exit code of the command is. - It talks about the `...` arguments only as "SHA-1 identifiers" while SHA-256 as well as any committish is actually supported. Let's fix all those issues by updating and improving the whole documentation. Signed-off-by: Christian Couder --- Documentation/git-verify-commit.adoc | 36 ++++++++++++++++++++++++---- 1 file changed, 31 insertions(+), 5 deletions(-) diff --git a/Documentation/git-verify-commit.adoc b/Documentation/git-verify-commit.adoc index aee4c40eac4..6a208a0c2a2 100644 --- a/Documentation/git-verify-commit.adoc +++ b/Documentation/git-verify-commit.adoc @@ -3,7 +3,7 @@ git-verify-commit(1) NAME ---- -git-verify-commit - Check the GPG signature of commits +git-verify-commit - Check the signature of commits SYNOPSIS -------- @@ -12,20 +12,46 @@ SYNOPSIS DESCRIPTION ----------- -Validates the GPG signature created by 'git commit -S'. +Validates the cryptographic signature of commits. This is typically +a GPG signature created by 'git commit -S', but other signature +formats like SSH may also be verified depending on Git configuration +(see linkgit:git-config[1] and the `gpg.format` option). + +By default, the command prints human-readable verification results to +standard error. + +EXIT STATUS +----------- +If all the specified commits are successfully verified and their +signatures are good and trusted according to the configured trust +requirements, the command exits with 0. + +If any commit fails verification (e.g., due to a bad signature, a +missing or untrusted key), if a specified object cannot be found or is +not a commit object, or if another error occurs during verification, +the command exits with a non-zero status. OPTIONS ------- --raw:: - Print the raw gpg status output to standard error instead of the normal - human-readable output. + Print the raw signature verification status output to standard + error instead of the normal human-readable output. The format + of this output is specific to the signature format being used. -v:: --verbose:: Print the contents of the commit object before validating it. ...:: - SHA-1 identifiers of Git commit objects. + Commit objects to verify. Can be specified using any format + accepted by linkgit:git-rev-parse[1]. + +SEE ALSO +-------- +linkgit:git-commit[1], +linkgit:git-config[1], +linkgit:git-verify-tag[1], +linkgit:git-log[1] GIT --- -- GitLab From fedcc9350e54c8e5cbaab3e2c546772578a1bc0a Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Sat, 3 May 2025 10:55:11 +0200 Subject: [PATCH 4/6] gpg-interface: extract hash algorithm from signature status output When using GPG/GPGSM to verify OpenPGP/X.509 signatures, the verification result (good/bad/etc.), signer, and key fingerprint are extracted from the output, but not the specific hash algorithm (e.g., "sha1", "sha256") reported by GPG as having been used for the signature itself. Let's improve the `gpg-interface` parsing logic to capture this information. This information can be useful for Git commands or external tools that process signature information. For example, it could be used when displaying signature verification results to users or when working with various signature formats in tools like fast-export and fast-import. GPG provides the hash algorithm ID used for the signature within its machine-readable status output, specifically in the fields following the `VALIDSIG` and `ERRSIG` keywords, as documented in GnuPG's `doc/DETAILS`. The implementation follows RFC 4880 (OpenPGP Message Format) section 9.4 for the mapping between hash algorithm IDs and their corresponding names. Signed-off-by: Christian Couder --- gpg-interface.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++++ gpg-interface.h | 4 +++ 2 files changed, 78 insertions(+) diff --git a/gpg-interface.c b/gpg-interface.c index e7af82d123e..15687ede438 100644 --- a/gpg-interface.c +++ b/gpg-interface.c @@ -153,6 +153,7 @@ void signature_check_clear(struct signature_check *sigc) FREE_AND_NULL(sigc->key); FREE_AND_NULL(sigc->fingerprint); FREE_AND_NULL(sigc->primary_key_fingerprint); + FREE_AND_NULL(sigc->sig_algo); } /* An exclusive status -- only one of them can appear in output */ @@ -221,6 +222,65 @@ static int parse_gpg_trust_level(const char *level, return 1; } +/* See RFC 4880: OpenPGP Message Format, section 9.4. Hash Algorithms */ +static struct sigcheck_gpg_hash_algo { + const char *id; + const char *name; +} sigcheck_gpg_hash_algo[] = { + { "1", "md5" }, /* deprecated */ + { "2", "sha1" }, /* mandatory */ + { "3", "ripemd160" }, + { "8", "sha256" }, + { "9", "sha384" }, + { "10", "sha512" }, + { "11", "sha224" }, +}; + +static const char *lookup_gpg_hash_algo(const char *algo_id) +{ + if (!algo_id) + return NULL; + + for (size_t i = 0; i < ARRAY_SIZE(sigcheck_gpg_hash_algo); i++) { + if (!strcmp(sigcheck_gpg_hash_algo[i].id, algo_id)) + return sigcheck_gpg_hash_algo[i].name; + } + + return NULL; +} + +static char *extract_gpg_hash_algo(const char *args_start, + const char *line_end, + int field_index) +{ + const char *p = args_start; + int current_field = 0; + char *result = NULL; + + while (p < line_end && current_field < field_index) { + /* Skip to the end of the current field */ + while (p < line_end && *p != ' ') + p++; + /* Skip spaces to get to the start of the next field */ + while (p < line_end && *p == ' ') { + p++; + current_field++; + } + } + + if (p < line_end && current_field == field_index) { + /* Found start of the target field */ + const char *algo_id_end = strchrnul(p, ' '); + char *algo_id = xmemdupz(p, algo_id_end - p); + const char *hash_algo = lookup_gpg_hash_algo(algo_id); + if (hash_algo) + result = xstrdup(hash_algo); + free(algo_id); + } + + return result; +} + static void parse_gpg_output(struct signature_check *sigc) { const char *buf = sigc->gpg_status; @@ -242,6 +302,18 @@ static void parse_gpg_output(struct signature_check *sigc) /* Iterate over all search strings */ for (size_t i = 0; i < ARRAY_SIZE(sigcheck_gpg_status); i++) { if (skip_prefix(line, sigcheck_gpg_status[i].check, &line)) { + + /* Do we have hash algorithm? */ + if (!sigc->sig_algo) { + const char *line_end = strchrnul(line, '\n'); + if (!strcmp(sigcheck_gpg_status[i].check, "VALIDSIG ")) + /* Hash algorithm is the 8th field in VALIDSIG */ + sigc->sig_algo = extract_gpg_hash_algo(line, line_end, 7); + else if (!strcmp(sigcheck_gpg_status[i].check, "ERRSIG ")) + /* Hash algorithm is the 3rd field in ERRSIG */ + sigc->sig_algo = extract_gpg_hash_algo(line, line_end, 2); + } + /* * GOODSIG, BADSIG etc. can occur only once for * each signature. Therefore, if we had more @@ -323,6 +395,7 @@ static void parse_gpg_output(struct signature_check *sigc) } } } + return; error: @@ -332,6 +405,7 @@ static void parse_gpg_output(struct signature_check *sigc) FREE_AND_NULL(sigc->fingerprint); FREE_AND_NULL(sigc->signer); FREE_AND_NULL(sigc->key); + FREE_AND_NULL(sigc->sig_algo); } static int verify_gpg_signed_buffer(struct signature_check *sigc, diff --git a/gpg-interface.h b/gpg-interface.h index 9a32dd6ce87..2b7701ca2cf 100644 --- a/gpg-interface.h +++ b/gpg-interface.h @@ -42,6 +42,10 @@ struct signature_check { char *key; char *fingerprint; char *primary_key_fingerprint; + + /* hash algo for GPG/GPGSM, key type for SSH */ + char *sig_algo; + enum signature_trust_level trust_level; }; -- GitLab From 088685ce2ec5e0aa70ee2a049c047a9d376ce799 Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Tue, 6 May 2025 09:54:07 +0200 Subject: [PATCH 5/6] gpg-interface: extract SSH key type from signature status output A previous commit extracted the hash algorithm from GPG/GPGSM signature status output and stored it in a new 'sig_algo' member of 'struct signature_check'. For SSH signatures, it's more interesting and easier to extract the key type (like "RSA", "ECDSA", "Ed25519", ...) rather than the hash algorithm which often depends on the key type. For example "Ed25519" has SHA-512 integrated into its design, and "ECDSA" and "RSA" are typically used with SHA-256. Let's improve the `gpg-interface` parsing logic to capture the SSH key type when parsing the SSH signature status output. Similarly as the hash algorithm for GPG/GPGSM signatures, this information can be useful for Git commands or external tools that process signature information. For example, it could be used when displaying signature verification results to users or when working with various signature formats in tools like fast-export and fast-import. As they share a common usage, we also store the SSH key type in the new 'sig_algo' member of 'struct signature_check'. Signed-off-by: Christian Couder --- gpg-interface.c | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/gpg-interface.c b/gpg-interface.c index 15687ede438..182e579769d 100644 --- a/gpg-interface.c +++ b/gpg-interface.c @@ -456,11 +456,27 @@ static int verify_gpg_signed_buffer(struct signature_check *sigc, return ret; } +static char *extract_ssh_key_type(const char *type_start, const char *type_end) +{ + if (!type_end || type_end <= type_start) + return NULL; + + /* Back up over any spaces before " key " */ + while (type_end > type_start && *(type_end - 1) == ' ') + type_end--; + + if (type_end <= type_start) + return NULL; + + return xmemdupz(type_start, type_end - type_start); +} + static void parse_ssh_output(struct signature_check *sigc) { const char *line, *principal, *search; char *to_free; char *key = NULL; + const char *after_last_with = NULL; /* * ssh-keygen output should be: @@ -485,8 +501,10 @@ static void parse_ssh_output(struct signature_check *sigc) principal = line; do { search = strstr(line, " with "); - if (search) + if (search) { line = search + 1; + after_last_with = search + 6; + } } while (search != NULL); if (line == principal) goto cleanup; @@ -499,6 +517,7 @@ static void parse_ssh_output(struct signature_check *sigc) /* Valid signature, but key unknown */ sigc->result = 'G'; sigc->trust_level = TRUST_UNDEFINED; + after_last_with = line; } else { goto cleanup; } @@ -507,6 +526,9 @@ static void parse_ssh_output(struct signature_check *sigc) if (key) { sigc->fingerprint = xstrdup(key + 4); sigc->key = xstrdup(sigc->fingerprint); + + if (after_last_with) + sigc->sig_algo = extract_ssh_key_type(after_last_with, key); } else { /* * Output did not match what we expected -- GitLab From 649fb04ae6eb06a2455a4e06f364db7b1f427240 Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Thu, 22 May 2025 14:21:01 +0200 Subject: [PATCH 6/6] verify-commit: add a --summary flag The current outputs, with `--raw`, `--verbose` or by default, from `git verify-commit` are all quite verbose and do not make it easy to quickly assess signature status. Let's add a new `--summary` option to `git verify-commit` that prints a concise, one-line summary of the signature verification to standard output. This compact format is useful for scripts and tools that need to quickly parse signature verification results, while still being human-readable. Signed-off-by: Christian Couder --- Documentation/git-verify-commit.adoc | 17 ++++++++++++++++- builtin/verify-commit.c | 4 +++- gpg-interface.c | 11 +++++++++++ gpg-interface.h | 6 ++++++ t/t7510-signed-commit.sh | 24 ++++++++++++++++++++++++ t/t7528-signed-commit-ssh.sh | 28 ++++++++++++++++++++++++++++ 6 files changed, 88 insertions(+), 2 deletions(-) diff --git a/Documentation/git-verify-commit.adoc b/Documentation/git-verify-commit.adoc index 6a208a0c2a2..fb038ae0cf2 100644 --- a/Documentation/git-verify-commit.adoc +++ b/Documentation/git-verify-commit.adoc @@ -8,7 +8,7 @@ git-verify-commit - Check the signature of commits SYNOPSIS -------- [verse] -'git verify-commit' [-v | --verbose] [--raw] ... +'git verify-commit' [-v | --verbose] [--raw] [--summary] ... DESCRIPTION ----------- @@ -38,6 +38,21 @@ OPTIONS error instead of the normal human-readable output. The format of this output is specific to the signature format being used. +--summary:: + Print a one-line human-readable summary of the signature check + to standard output in the format: `STATUS FORMAT ALGORITHM`. ++ +STATUS is the result character (e.g., G, B, E, U, N, ...) shown by the +"%G?" pretty format specifier. See the "Pretty Formats" section in +linkgit:git-log[1]. ++ +FORMAT indicates the signature format (`openpgp`, `x509`, or `ssh`) or +`?` if unknown. ++ +ALGORITHM is the hash algorithm used for GPG/GPGSM signatures +(e.g. `sha1`, `sha256`, ...), or the key type for SSH signatures +(`RSA`, `ECDSA`, `ED25519`, ...), or `?` if unknown. + -v:: --verbose:: Print the contents of the commit object before validating it. diff --git a/builtin/verify-commit.c b/builtin/verify-commit.c index 5f749a30daf..54b5b7d360b 100644 --- a/builtin/verify-commit.c +++ b/builtin/verify-commit.c @@ -14,7 +14,7 @@ #include "gpg-interface.h" static const char * const verify_commit_usage[] = { - N_("git verify-commit [-v | --verbose] [--raw] ..."), + N_("git verify-commit [-v | --verbose] [--raw] [--summary] ..."), NULL }; @@ -27,6 +27,7 @@ static int run_gpg_verify(struct commit *commit, unsigned flags) ret = check_commit_signature(commit, &signature_check); print_signature_buffer(&signature_check, flags); + print_signature_summary(&signature_check, flags); signature_check_clear(&signature_check); return ret; @@ -60,6 +61,7 @@ int cmd_verify_commit(int argc, const struct option verify_commit_options[] = { OPT__VERBOSE(&verbose, N_("print commit contents")), OPT_BIT(0, "raw", &flags, N_("print raw gpg status output"), GPG_VERIFY_RAW), + OPT_BIT(0, "summary", &flags, N_("print concise signature verification summary"), GPG_VERIFY_SUMMARY), OPT_END() }; diff --git a/gpg-interface.c b/gpg-interface.c index 182e579769d..fc198715c4c 100644 --- a/gpg-interface.c +++ b/gpg-interface.c @@ -153,6 +153,7 @@ void signature_check_clear(struct signature_check *sigc) FREE_AND_NULL(sigc->key); FREE_AND_NULL(sigc->fingerprint); FREE_AND_NULL(sigc->primary_key_fingerprint); + FREE_AND_NULL(sigc->format_name); FREE_AND_NULL(sigc->sig_algo); } @@ -756,6 +757,8 @@ int check_signature(struct signature_check *sigc, if (!fmt) die(_("bad/incompatible signature '%s'"), signature); + sigc->format_name = xstrdup(fmt->name); + if (parse_payload_metadata(sigc)) return 1; @@ -782,6 +785,14 @@ void print_signature_buffer(const struct signature_check *sigc, unsigned flags) fputs(output, stderr); } +void print_signature_summary(const struct signature_check *sigc, unsigned flags) +{ + if (flags & GPG_VERIFY_SUMMARY) + printf("%c %s %s\n", sigc->result, + sigc->format_name ? sigc->format_name : "?", + sigc->sig_algo ? sigc->sig_algo : "?"); +} + size_t parse_signed_buffer(const char *buf, size_t size) { size_t len = 0; diff --git a/gpg-interface.h b/gpg-interface.h index 2b7701ca2cf..a9565757f64 100644 --- a/gpg-interface.h +++ b/gpg-interface.h @@ -6,6 +6,7 @@ struct strbuf; #define GPG_VERIFY_VERBOSE (1<<0) #define GPG_VERIFY_RAW (1<<1) #define GPG_VERIFY_OMIT_STATUS (1<<2) +#define GPG_VERIFY_SUMMARY (1<<3) enum signature_trust_level { TRUST_UNDEFINED, @@ -43,6 +44,9 @@ struct signature_check { char *fingerprint; char *primary_key_fingerprint; + /* "openpgp", "x509", "ssh" */ + char *format_name; + /* hash algo for GPG/GPGSM, key type for SSH */ char *sig_algo; @@ -95,5 +99,7 @@ int check_signature(struct signature_check *sigc, const char *signature, size_t slen); void print_signature_buffer(const struct signature_check *sigc, unsigned flags); +void print_signature_summary(const struct signature_check *sigc, + unsigned flags); #endif diff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh index 39677e859ab..47f40862f3c 100755 --- a/t/t7510-signed-commit.sh +++ b/t/t7510-signed-commit.sh @@ -232,6 +232,30 @@ test_expect_success GPG2 'bare signature' ' test_cmp expect actual ' +test_expect_success GPG 'verify signatures with --summary' ' + # GPG-signed commit + git verify-commit --summary sixth-signed >actual && + test_grep "^G openpgp sha1" actual && + + # Non-signed commit + test_must_fail git verify-commit --summary seventh-unsigned >actual 2>&1 && + test_grep "^N ? ?" actual && + + # Trusted signature with alternate key (hash used might depend on the OS) + git verify-commit --summary eighth-signed-alt >actual && + test_grep -E "^G openpgp sha(256|512)" actual && + + # Bad signature + test_must_fail git verify-commit --summary $(cat forged1.commit) >actual 2>err && + test_grep "^B openpgp ?" actual +' + +test_expect_success GPG '--summary and --raw work together' ' + git verify-commit --summary --raw sixth-signed >actual 2>err && + test_grep "^G openpgp sha1" actual && + test_grep "GOODSIG" err +' + test_expect_success GPG 'show good signature with custom format' ' cat >expect <<-\EOF && G diff --git a/t/t7528-signed-commit-ssh.sh b/t/t7528-signed-commit-ssh.sh index 065f7806362..3d0e7d78592 100755 --- a/t/t7528-signed-commit-ssh.sh +++ b/t/t7528-signed-commit-ssh.sh @@ -277,6 +277,34 @@ test_expect_success GPGSSH 'detect fudged signature with NUL' ' ! grep "${GPGSSH_GOOD_SIGNATURE_TRUSTED}" actual2 ' +test_expect_success GPGSSH 'verify-commit --summary outputs format and key type for SSH signatures' ' + test_config gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" && + + # SSH-signed commit with ED25519 key + git verify-commit --summary sixth-signed >actual && + test_grep "^G ssh ED25519" actual && + + # SSH-signed commit with ECDSA key + git verify-commit --summary thirteenth-signed-ecdsa >actual && + test_grep "^G ssh ECDSA" actual && + + # Non-signed commit + test_must_fail git verify-commit --summary seventh-unsigned >actual 2>&1 && + test_grep "^N ? ?" actual && + + # Bad signature + test_must_fail git verify-commit --summary $(cat forged1.commit) >actual 2>err && + test_grep "^B ssh ?" actual +' + +test_expect_success GPGSSH '--summary and --raw work together' ' + test_config gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" && + + git verify-commit --summary --raw sixth-signed >actual 2>err && + test_grep "^G ssh ED25519" actual && + test_grep "${GPGSSH_GOOD_SIGNATURE_TRUSTED}" err +' + test_expect_success GPGSSH 'amending already signed commit' ' test_config gpg.format ssh && test_config user.signingkey "${GPGSSH_KEY_PRIMARY}" && -- GitLab