Re: Some fixes to GPG signature verification in pkg_install

[Pierre Pronchery <khorben%defora.org@localhost>]

On 28/05/2015 14:33, Pierre Pronchery wrote:
>>> 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).
> 
> It is not instantly obvious from the warn(3) manpage that it uses stdio,
> but it probably does; I can correct this in another version of the patch
> in any case.

Here is is; is it good enough to commit?

I see at least parts of this as an improvement.

HTH,
-- 
khorben

commit 37f1a098d2719b82596fe53774d2e5d2984fbec6
Author: Pierre Pronchery <khorben%defora.org@localhost>
Date:   Fri May 29 15:37:08 2015 +0200

    Avoid using stdio while in a vfork() child

diff --git a/external/bsd/pkg_install/dist/lib/gpgsig.c b/external/bsd/pkg_install/dist/lib/gpgsig.c
index 1768084..1c9e132 100644
--- a/external/bsd/pkg_install/dist/lib/gpgsig.c
+++ b/external/bsd/pkg_install/dist/lib/gpgsig.c
@@ -6,6 +6,7 @@
 #if HAVE_SYS_CDEFS_H
 #include <sys/cdefs.h>
 #endif
+#include <errno.h>
 
 __RCSID("$NetBSD: gpgsig.c,v 1.1.1.2 2009/08/06 16:55:27 joerg Exp $");
 
@@ -60,6 +61,7 @@ verify_signature(const char *input, size_t input_len, const char *keyring,
 	pid_t child;
 	int fd[2], status;
 	int error = 0;
+	char const * err_msg;
 
 	if (pipe(fd) == -1) {
 		warn("Cannot create input pipes");
@@ -77,7 +79,8 @@ verify_signature(const char *input, size_t input_len, const char *keyring,
 		close(fd[1]);
 		close(STDIN_FILENO);
 		if (dup2(fd[0], STDIN_FILENO) == -1) {
-			warn("Cannot redirect stdin of GPG process");
+			err_msg = "Cannot redirect stdin of GPG process\n";
+			write(STDERR_FILENO, err_msg, strlen(err_msg));
 			_exit(2);
 		}
 		close(fd[0]);
@@ -97,7 +100,12 @@ verify_signature(const char *input, size_t input_len, const char *keyring,
 		*argvp = NULL;
 
 		execvp(gpg_cmd, __UNCONST(argv));
-		warn("%s", gpg_cmd);
+		/* we would call warn(gpg_cmd) but stdio is not allowed */
+		write(STDERR_FILENO, gpg_cmd, strlen(gpg_cmd));
+		write(STDERR_FILENO, ": ", 2);
+		err_msg = strerror(errno);
+		write(STDERR_FILENO, err_msg, strlen(err_msg));
+		write(STDERR_FILENO, "\n", 1);
 		_exit(2);
 	}
 	close(fd[0]);