tech-pkg archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
Re: Some fixes to GPG signature verification in pkg_install
Hi there,
On 05/30/15 16:10, Pierre Pronchery wrote:
> [...]
I have updated the patch again (attached here). It does not use
strerror(3) in vfork(2) context anymore either.
HTH,
--
khorben
diff --git a/external/bsd/pkg_install/dist/lib/gpgsig.c b/external/bsd/pkg_install/dist/lib/gpgsig.c
index 8091d4a..9c60e10 100644
--- a/external/bsd/pkg_install/dist/lib/gpgsig.c
+++ b/external/bsd/pkg_install/dist/lib/gpgsig.c
@@ -52,28 +52,35 @@ __RCSID("$NetBSD: gpgsig.c,v 1.1.1.2 2009/08/06 16:55:27 joerg Exp $");
#include "lib.h"
-static void
+static int
verify_signature(const char *input, size_t input_len, const char *keyring,
const char *detached_signature)
{
const char *argv[8], **argvp;
pid_t child;
int fd[2], status;
+ int error = 0;
+ char const * err_msg;
- if (pipe(fd) == -1)
- err(EXIT_FAILURE, "cannot create input pipes");
+ if (pipe(fd) == -1) {
+ warn("Cannot create input pipes");
+ return -1;
+ }
child = vfork();
- if (child == -1)
- err(EXIT_FAILURE, "cannot fork GPG process");
+ if (child == -1) {
+ warn("Cannot fork GPG process");
+ close(fd[0]);
+ close(fd[1]);
+ return -1;
+ }
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);
- _exit(255);
+ err_msg = "Cannot redirect stdin of GPG process\n";
+ write(STDERR_FILENO, err_msg, strlen(err_msg));
+ _exit(2);
}
close(fd[0]);
argvp = argv;
@@ -92,23 +99,38 @@ verify_signature(const char *input, size_t input_len, const char *keyring,
*argvp = NULL;
execvp(gpg_cmd, __UNCONST(argv));
- _exit(255);
+ /* we would call warn(gpg_cmd) but stdio is not allowed */
+ write(STDERR_FILENO, gpg_cmd, strlen(gpg_cmd));
+ err_msg = ": Could not execute GPG\n";
+ write(STDERR_FILENO, err_msg, strlen(err_msg));
+ _exit(2);
}
close(fd[0]);
- if (write(fd[1], input, input_len) != (ssize_t)input_len)
- errx(EXIT_FAILURE, "Short read from GPG");
+ if (waitpid(child, &status, WNOHANG) != 0) {
+ warnx("GPG could not verify the signature");
+ close(fd[0]);
+ close(fd[1]);
+ return 1;
+ }
+ /* XXX dies on SIGPIPE if the child exited in the meantime (race) */
+ if (write(fd[1], input, input_len) != (ssize_t)input_len) {
+ warnx("Short read from GPG");
+ error = 1;
+ }
close(fd[1]);
- waitpid(child, &status, 0);
- if (status)
- errx(EXIT_FAILURE, "GPG could not verify the signature");
+ if (waitpid(child, &status, 0) == -1
+ || !WIFEXITED(status)
+ || WEXITSTATUS(status)) {
+ warnx("GPG could not verify the signature");
+ error = 1;
+ }
+ return error;
}
int
inline_gpg_verify(const char *content, size_t len, const char *keyring)
{
- verify_signature(content, len, keyring, NULL);
-
- return 0;
+ return verify_signature(content, len, keyring, NULL);
}
int
@@ -119,6 +141,7 @@ detached_gpg_verify(const char *content, size_t len,
const char *tmpdir;
char *tempsig;
ssize_t ret;
+ int error = 0;
if (gpg_cmd == NULL) {
warnx("GPG variable not set, failing signature check");
@@ -132,26 +155,34 @@ detached_gpg_verify(const char *content, size_t len,
fd = mkstemp(tempsig);
if (fd == -1) {
warnx("Creating temporary file for GPG signature failed");
+ free(tempsig);
return -1;
}
while (signature_len) {
ret = write(fd, signature, signature_len);
- if (ret == -1)
- err(EXIT_FAILURE, "Write to GPG failed");
- if (ret == 0)
- errx(EXIT_FAILURE, "Short write to GPG");
+ if (ret == -1) {
+ warn("Write to GPG failed");
+ error = 1;
+ break;
+ }
+ if (ret == 0) {
+ warnx("Short write to GPG");
+ error = 1;
+ break;
+ }
signature_len -= ret;
signature += ret;
}
- verify_signature(content, len, keyring, tempsig);
+ if (error == 0)
+ error = verify_signature(content, len, keyring, tempsig);
unlink(tempsig);
close(fd);
free(tempsig);
- return 0;
+ return error;
}
int
@@ -163,18 +194,33 @@ detached_gpg_sign(const char *content, size_t len, char **sig, size_t *sig_len,
int fd_in[2], fd_out[2], status;
size_t allocated;
ssize_t ret;
+ int error = 0;
- if (gpg_cmd == NULL)
- errx(EXIT_FAILURE, "GPG variable not set");
+ if (gpg_cmd == NULL) {
+ warnx("GPG variable not set");
+ return 1;
+ }
- if (pipe(fd_in) == -1)
- err(EXIT_FAILURE, "cannot create input pipes");
- if (pipe(fd_out) == -1)
- err(EXIT_FAILURE, "cannot create output pipes");
+ if (pipe(fd_in) == -1) {
+ warn("Cannot create input pipes");
+ return 1;
+ }
+ if (pipe(fd_out) == -1) {
+ warn("Cannot create output pipes");
+ close(fd_in[0]);
+ close(fd_in[1]);
+ return 1;
+ }
child = fork();
- if (child == -1)
- err(EXIT_FAILURE, "cannot fork GPG process");
+ if (child == -1) {
+ warn("Cannot fork GPG process");
+ close(fd_in[0]);
+ close(fd_in[1]);
+ close(fd_out[0]);
+ close(fd_out[1]);
+ return 1;
+ }
if (child == 0) {
close(fd_in[1]);
close(STDIN_FILENO);
@@ -182,7 +228,7 @@ detached_gpg_sign(const char *content, size_t len, char **sig, size_t *sig_len,
static const char err_msg[] =
"cannot redirect stdin of GPG process\n";
write(STDERR_FILENO, err_msg, sizeof(err_msg) - 1);
- _exit(255);
+ _exit(2);
}
close(fd_in[0]);
@@ -192,7 +238,7 @@ detached_gpg_sign(const char *content, size_t len, char **sig, size_t *sig_len,
static const char err_msg[] =
"cannot redirect stdout of GPG process\n";
write(STDERR_FILENO, err_msg, sizeof(err_msg) - 1);
- _exit(255);
+ _exit(2);
}
close(fd_out[1]);
@@ -216,11 +262,14 @@ detached_gpg_sign(const char *content, size_t len, char **sig, size_t *sig_len,
*argvp = NULL;
execvp(gpg_cmd, __UNCONST(argv));
- _exit(255);
+ warn("%s", gpg_cmd);
+ _exit(2);
}
close(fd_in[0]);
- if (write(fd_in[1], content, len) != (ssize_t)len)
- errx(EXIT_FAILURE, "Short read from GPG");
+ if (write(fd_in[1], content, len) != (ssize_t)len) {
+ warnx("Short read from GPG");
+ error = 1;
+ }
close(fd_in[1]);
allocated = 1024;
@@ -240,9 +289,12 @@ detached_gpg_sign(const char *content, size_t len, char **sig, size_t *sig_len,
close(fd_out[0]);
- waitpid(child, &status, 0);
- if (status)
- errx(EXIT_FAILURE, "GPG could not create signature");
+ if (waitpid(child, &status, 0) == -1
+ || !WIFEXITED(status)
+ || WEXITSTATUS(status)) {
+ warnx("GPG could not create signature");
+ error = 1;
+ }
- return 0;
+ return error;
}
diff --git a/external/bsd/pkg_install/dist/lib/vulnerabilities-file.c b/external/bsd/pkg_install/dist/lib/vulnerabilities-file.c
index e183689..ddc55e1 100644
--- a/external/bsd/pkg_install/dist/lib/vulnerabilities-file.c
+++ b/external/bsd/pkg_install/dist/lib/vulnerabilities-file.c
@@ -115,7 +115,8 @@ verify_signature(const char *input, size_t input_len)
"At least GPG or CERTIFICATE_ANCHOR_PKGVULN "
"must be configured");
if (gpg_cmd != NULL)
- inline_gpg_verify(input, input_len, gpg_keyring_pkgvuln);
+ if (inline_gpg_verify(input, input_len, gpg_keyring_pkgvuln))
+ exit(EXIT_FAILURE);
if (certs_pkg_vulnerabilities != NULL)
verify_signature_pkcs7(input);
}
Home |
Main Index |
Thread Index |
Old Index