Re: Some fixes to GPG signature verification in pkg_install

[Joerg Sonnenberger <joerg%britannica.bec.de@localhost>]

On Thu, May 28, 2015 at 12:52:25AM +0200, Pierre Pronchery wrote:
> 2/4: error codes
>      Ensures error codes from child processes are below 126 (2 instead
>      of 255) for failed executions attempts of the GPG binary.

Why?

> diff --git a/external/bsd/pkg_install/dist/lib/gpgsig.c b/external/bsd/pkg_install/dist/lib/gpgsig.c
> index 645d04c..ae21f3f 100644
> --- a/external/bsd/pkg_install/dist/lib/gpgsig.c
> +++ b/external/bsd/pkg_install/dist/lib/gpgsig.c
> @@ -92,6 +92,7 @@ verify_signature(const char *input, size_t input_len, const char *keyring,
>  		*argvp = NULL;
>  
>  		execvp(gpg_cmd, __UNCONST(argv));
> +		warn("%s", gpg_cmd);
>  		_exit(2);
>  	}
>  	close(fd[0]);
> @@ -216,6 +217,7 @@ detached_gpg_sign(const char *content, size_t len, char **sig, size_t *sig_len,
>  		*argvp = NULL;
>  
>  		execvp(gpg_cmd, __UNCONST(argv));
> +		warn("%s", gpg_cmd);
>  		_exit(2);
>  	}
>  	close(fd_in[0]);
> 

This part is wrong. You must not use stdio in a vfork'd child (and it is
generally bad to do it in a fork'd child either).

>  	if (child == 0) {
>  		close(fd[1]);
>  		close(STDIN_FILENO);
>  		if (dup2(fd[0], STDIN_FILENO) == -1) {
> -			static const char err_msg[] =
> -			    "Cannot redirect stdin of GPG process\n";
> -			write(STDERR_FILENO, err_msg, sizeof(err_msg) - 1);
> +			warn("Cannot redirect stdin of GPG process");
>  			_exit(2);
>  		}
>  		close(fd[0]);

Same. Generally, I think this is the wrong approach. If you want to
improve the temp file logic, drive gpg as filter and avoid them in first
place. Libarchive has an implementation of the necessary logic.

Joerg