development/git
development/git
1
0
Fork 0
mirror of https://github.com/git/git synced 2024-07-05 00:58:49 +00:00
Code Issues Projects Releases Wiki Activity

verify_signed_buffer: use pipe_command

Browse Source 
This is shorter and should make the function easier to
follow. But more importantly, it removes the possibility of
any deadlocks based on reading or writing to gpg.

It's not clear if such a deadlock is possible in practice.

We do write the whole payload before reading anything, so we
could deadlock there. However, in practice gpg will need to
read our whole input to verify the signature, so it will
drain our payload first. It could write an error to stderr
before reading, but it's unlikely that such an error
wouldn't be followed by it immediately exiting, or that the
error would actually be larger than a pipe buffer.

On the writing side, we drain stderr (with the
human-readable output) in its entirety before reading stdout
(with the status-fd data). Running strace on "gpg --verify"
does show interleaved output on the two descriptors:

  write(2, "gpg: ", 5)                    = 5
  write(2, "Signature made Thu 16 Jun 2016 0"..., 73) = 73
  write(1, "[GNUPG:] SIG_ID tQw8KGcs9rBfLvAj"..., 66) = 66
  write(1, "[GNUPG:] GOODSIG 69808639F9430ED"..., 60) = 60
  write(2, "gpg: ", 5)                    = 5
  write(2, "Good signature from \"Jeff King <"..., 47) = 47
  write(2, "\n", 1)                       = 1
  write(2, "gpg: ", 5)                    = 5
  write(2, "                aka \"Jeff King <"..., 49) = 49
  write(2, "\n", 1)                       = 1
  write(1, "[GNUPG:] VALIDSIG C49CE24156AF08"..., 135) = 135
  write(1, "[GNUPG:] TRUST_ULTIMATE\n", 24) = 24

The second line written to stdout there contains the
signer's UID, which can be arbitrarily long. If it fills the
pipe buffer, then gpg would block writing to its stdout,
while we are blocked trying to read its stderr.

In practice, GPG seems to limit UIDs to 2048 bytes, so
unless your pipe buffer size is quite small, or unless gpg
does not enforce the limit under some conditions, this seems
unlikely in practice.

Still, it is not hard for us to be cautious and just use
pipe_command.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This commit is contained in:
Jeff King 2016-06-17 19:38:52 -04:00 committed by Junio C Hamano
parent 96335bcf4d
commit 0d2b664efd
1 changed files with 3 additions and 19 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

22
gpg-interface.c
View File

@ -229,29 +229,13 @@ int verify_signed_buffer(const char *payload, size_t payload_size,
"--status-fd=1",
"--verify", temp.filename.buf, "-",
NULL);
gpg.in = -1;
gpg.out = -1;
if (gpg_output)
gpg.err = -1;
if (start_command(&gpg)) {
delete_tempfile(&temp);
return error(_("could not run gpg."));
}
sigchain_push(SIGPIPE, SIG_IGN);
write_in_full(gpg.in, payload, payload_size);
close(gpg.in);
if (gpg_output) {
strbuf_read(gpg_output, gpg.err, 0);
close(gpg.err);
}
if (!gpg_status)
gpg_status = &buf;
strbuf_read(gpg_status, gpg.out, 0);
close(gpg.out);
ret = finish_command(&gpg);
sigchain_push(SIGPIPE, SIG_IGN);
ret = pipe_command(&gpg, payload, payload_size,
gpg_status, 0, gpg_output, 0);
sigchain_pop(SIGPIPE);
delete_tempfile(&temp);