Re: [PATCH v7 4/5] merge/pull Check for untrusted good GPG signatures

[John Keeping <john@keeping.me.uk>]

On Sun, Mar 31, 2013 at 04:33:57PM +0200, Sebastian Götte wrote:
> When --verify-signatures is specified, abort the merge in case a good
> GPG signature from an untrusted key is encountered.
> 
> Signed-off-by: Sebastian Götte <jaseg@physik-pool.tu-berlin.de>
> ---
>  Documentation/merge-options.txt    |   4 ++--
>  builtin/merge.c                    |   2 ++
>  commit.c                           |  13 ++++++++-----
>  commit.h                           |  10 +++++-----
>  gpg-interface.h                    |   1 +
>  t/lib-gpg/pubring.gpg              | Bin 1164 -> 2359 bytes
>  t/lib-gpg/random_seed              | Bin 600 -> 600 bytes
>  t/lib-gpg/secring.gpg              | Bin 1237 -> 3734 bytes
>  t/lib-gpg/trustdb.gpg              | Bin 1280 -> 1360 bytes
>  t/t7612-merge-verify-signatures.sh |   9 +++++++++
>  10 files changed, 27 insertions(+), 12 deletions(-)
> 
> diff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt
> index 31f1067..a0f022b 100644
> --- a/Documentation/merge-options.txt
> +++ b/Documentation/merge-options.txt
> @@ -85,8 +85,8 @@ option can be used to override --squash.
>  
>  --verify-signatures::
>  --no-verify-signatures::
> -	Verify that the commits being merged have good GPG signatures and abort the
> -	merge in case they do not.
> +	Verify that the commits being merged have good and trusted GPG signatures
> +	and abort the merge in case they do not.
>  
>  --summary::
>  --no-summary::
> diff --git a/builtin/merge.c b/builtin/merge.c
> index 7a33d03..752e3a9 100644
> --- a/builtin/merge.c
> +++ b/builtin/merge.c
> @@ -1248,6 +1248,8 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
>  			switch(signature_check.result){
>  				case 'G':
>  					break;
> +				case 'U':
> +					die(_("Commit %s has a good, untrusted GPG signature allegedly by %s."), hex, signature_check.signer);
>  				case 'B':
>  					die(_("Commit %s has a bad GPG signature allegedly by %s."), hex, signature_check.signer);
>  				default: /* 'N' */
> diff --git a/commit.c b/commit.c
> index eda7f90..bb2d9ad 100644
> --- a/commit.c
> +++ b/commit.c
> @@ -1029,6 +1029,8 @@ static struct {
>  } sigcheck_gpg_status[] = {
>  	{ 'G', "[GNUPG:] GOODSIG " },
>  	{ 'B', "[GNUPG:] BADSIG " },
> +	{ 'U', "[GNUPG:] TRUST_NEVER" },
> +	{ 'U', "[GNUPG:] TRUST_UNDEFINED" },
>  };
>  
>  static void parse_gpg_output(struct signature_check *sigc)
> @@ -1050,11 +1052,12 @@ static void parse_gpg_output(struct signature_check *sigc)
>  			found += strlen(sigcheck_gpg_status[i].check);
>  		}
>  		sigc->result = sigcheck_gpg_status[i].result;
> -		sigc->key = xmemdupz(found, 16);
> -		found += 17;
> -		next = strchrnul(found, '\n');
> -		sigc->signer = xmemdupz(found, next - found);
> -		break;
> +		if (sigc->result != 'U') {

This could use a comment; we know now that only GOODSIG and BADSIG
are followed by a signature, but someone looking at this code in the
future will probably appreciate an explanation.

> +			sigc->key = xmemdupz(found, 16);
> +			found += 17;
> +			next = strchrnul(found, '\n');
> +			sigc->signer = xmemdupz(found, next - found);
> +		}
>  	}
>  }
>