git/gpg-interface.c, branch v2.25.0

gpg-interface: prefer check_signature() for GPG verification

2019-11-30T21:52:35Z

This commit refactors the use of verify_signed_buffer() outside of gpg-interface.c to use check_signature() instead. It also turns verify_signed_buffer() into a file-local function since it's now only invoked internally by check_signature(). There were previously two globally scoped functions used in different parts of Git to perform GPG signature verification: verify_signed_buffer() and check_signature(). Now only check_signature() is used. The verify_signed_buffer() function doesn't guard against duplicate signatures as described by Michał Górny [1]. Instead it only ensures a non-erroneous exit code from GPG and the presence of at least one GOODSIG status field. This stands in contrast with check_signature() that returns an error if more than one signature is encountered. The lower degree of verification makes the use of verify_signed_buffer() problematic if callers don't parse and validate the various parts of the GPG status message themselves. And processing these messages seems like a task that should be reserved to gpg-interface.c with the function check_signature(). Furthermore, the use of verify_signed_buffer() makes it difficult to introduce new functionality that relies on the content of the GPG status lines. Now all operations that does signature verification share a single entry point to gpg-interface.c. This makes it easier to propagate changed or additional functionality in GPG signature verification to all parts of Git, without having odd edge-cases that don't perform the same degree of verification. [1] https://dev.gentoo.org/~mgorny/articles/attack-on-git-signature-verification.html Signed-off-by: Hans Jerry Illikainen Signed-off-by: Junio C Hamano

gpg-interface: limit search for primary key fingerprint

2019-11-23T00:18:40Z

The VALIDSIG status line from GnuPG with --status-fd is documented to have 9 required and 1 optional fields [1]. The final, and optional, field is used to specify the fingerprint of the primary key that made the signature in case it was made by a subkey. However, this field is only available for OpenPGP signatures; not for CMS/X.509. If the VALIDSIG status line does not have the optional 10th field, the current code will continue reading onto the next status line. And this is the case for non-OpenPGP signatures [1]. The consequence is that a subsequent status line may be considered as the "primary key" for signatures that does not have an actual primary key. Limit the search of these 9 or 10 fields to the single line to avoid this problem. If the 10th field is missing, report that there is no primary key fingerprint. [Reference] [1] GnuPG Details, General status codes https://git.gnupg.org/cgi-bin/gitweb.cgi?p=gnupg.git;a=blob;f=doc/DETAILS;h=6ce340e8c04794add995e84308bb3091450bd28f;hb=HEAD#l483 The documentation says: VALIDSIG The args are: - - - - - - - - - - [ ] This status indicates that the signature is cryptographically valid. [...] PRIMARY-KEY-FPR is the fingerprint of the primary key or identical to the first argument. The primary-key-fpr parameter is used for OpenPGP and not available for CMS signatures. [...] Signed-off-by: Hans Jerry Illikainen Signed-off-by: Junio C Hamano

gpg-interface: refactor the free-and-xmemdupz pattern

2019-11-23T00:09:32Z

Introduce a static replace_cstring() function to simplify repeated pattern of free-and-xmemdupz() for GPG status line parsing. This also helps us avoid potential memleaks if parsing of new status lines are introduced in the future. Signed-off-by: Hans Jerry Illikainen Signed-off-by: Junio C Hamano

gpg-interface: do not scan past the end of buffer

2019-07-16T19:15:12Z

If the GPG output ends with trailing blank lines, after skipping them over inside the loop to find the terminating NUL at the end, the loop ends up looking for the next line, starting past the end. Signed-off-by: Steven Roberts Signed-off-by: Junio C Hamano

Merge branch 'mg/gpg-fingerprint'

2018-11-02T15:53:58Z

New "--pretty=format:" placeholders %GF and %GP that show the GPG key fingerprints have been invented. * mg/gpg-fingerprint: gpg-interface.c: obtain primary key fingerprint as well gpg-interface.c: support getting key fingerprint via %GF format gpg-interface.c: use flags to determine key/signer info presence

Merge branch 'mg/gpg-parse-tighten'

2018-11-02T15:53:58Z

Detect and reject a signature block that has more than one GPG signature. * mg/gpg-parse-tighten: gpg-interface.c: detect and reject multiple signatures on commits

gpg-interface.c: obtain primary key fingerprint as well

2018-10-22T23:00:43Z

Obtain the primary key fingerprint off VALIDSIG status message, and expose it via %GP format. Signed-off-by: Michał Górny Signed-off-by: Junio C Hamano

gpg-interface.c: support getting key fingerprint via %GF format

2018-10-22T23:00:09Z

Support processing VALIDSIG status that provides additional information for valid signatures. Use this information to propagate signing key fingerprint and expose it via %GF pretty format. This format can be used to build safer key verification systems that verify the key via complete fingerprint rather than short/long identifier provided by %GK. Signed-off-by: Michał Górny Signed-off-by: Junio C Hamano

gpg-interface.c: use flags to determine key/signer info presence

2018-10-22T22:58:54Z

Replace the logic used to determine whether key and signer information is present to use explicit flags in sigcheck_gpg_status[] array. This is more future-proof, since it makes it possible to add additional statuses without having to explicitly update the conditions. Signed-off-by: Michał Górny Signed-off-by: Junio C Hamano

gpg-interface.c: detect and reject multiple signatures on commits

2018-10-22T03:42:30Z

GnuPG supports creating signatures consisting of multiple signature packets. If such a signature is verified, it outputs all the status messages for each signature separately. However, git currently does not account for such scenario and gets terribly confused over getting multiple *SIG statuses. For example, if a malicious party alters a signed commit and appends a new untrusted signature, git is going to ignore the original bad signature and report untrusted commit instead. However, %GK and %GS format strings may still expand to the data corresponding to the original signature, potentially tricking the scripts into trusting the malicious commit. Given that the use of multiple signatures is quite rare, git does not support creating them without jumping through a few hoops, and finally supporting them properly would require extensive API improvement, it seems reasonable to just reject them at the moment. Signed-off-by: Michał Górny Signed-off-by: Junio C Hamano