Commit Graph

47 Commits

Author SHA1 Message Date
Jeff King
8b44b2be89 gpg-interface: find the last gpg signature line
A signed tag has a detached signature like this:

  object ...
  [...more header...]

  This is the tag body.

  -----BEGIN PGP SIGNATURE-----
  [opaque gpg data]
  -----END PGP SIGNATURE-----

Our parser finds the _first_ line that appears to start a
PGP signature block, meaning we may be confused by a
signature (or a signature-like line) in the actual body.
Let's keep parsing and always find the final block, which
should be the detached signature over all of the preceding
content.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Ben Toews <mastahyeti@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-04-16 14:15:03 +09:00
Jeff King
f68f2dd57f gpg-interface: extract gpg line matching helper
Let's separate the actual line-by-line parsing of signatures
from the notion of "is this a gpg signature line". That will
make it easier to do more refactoring of this loop in future
patches.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Ben Toews <mastahyeti@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-04-16 14:15:03 +09:00
Jeff King
17ef3a421e gpg-interface: fix const-correctness of "eol" pointer
We accidentally shed the "const" of our buffer by passing it
through memchr. Let's fix that, and while we're at it, move
our variable declaration inside the loop, which is the only
place that uses it.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Ben Toews <mastahyeti@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-04-16 14:15:03 +09:00
Jeff King
e6fa6cde5b gpg-interface: use size_t for signature buffer size
Even though our object sizes (from which these buffers would
come) are typically "unsigned long", this is something we'd
like to eventually fix (since it's only 32-bits even on
64-bit Windows). It makes more sense to use size_t when
taking an in-memory buffer.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Ben Toews <mastahyeti@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-04-16 14:15:03 +09:00
Jeff King
f80bee27e3 gpg-interface: modernize function declarations
Let's drop "extern" from our declarations, which brings us
in line with our modern style guidelines. While we're
here, let's wrap some of the overly long lines, and move
docstrings for public functions to their declarations, since
they document the interface.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Ben Toews <mastahyeti@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-04-16 14:15:03 +09:00
Jeff King
1b0eeec3f3 gpg-interface: handle bool user.signingkey
The config handler for user.signingkey does not check for a
boolean value, and thus:

  git -c user.signingkey tag

will segfault. We could fix this and even shorten the code
by using git_config_string(). But our set_signing_key()
helper is used by other code outside of gpg-interface.c, so
we must keep it (and we may as well use it, because unlike
git_config_string() it does not leak when we overwrite an
old value).

Ironically, the handler for gpg.program just below _could_
use git_config_string() but doesn't. But since we're going
to touch that in a future patch, we'll leave it alone for
now. We will add some whitespace and returns in preparation
for adding more config keys, though.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Ben Toews <mastahyeti@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-04-16 14:15:02 +09:00
Jeff King
076aa2cbda tempfile: auto-allocate tempfiles on heap
The previous commit taught the tempfile code to give up
ownership over tempfiles that have been renamed or deleted.
That makes it possible to use a stack variable like this:

  struct tempfile t;

  create_tempfile(&t, ...);
  ...
  if (!err)
          rename_tempfile(&t, ...);
  else
          delete_tempfile(&t);

But doing it this way has a high potential for creating
memory errors. The tempfile we pass to create_tempfile()
ends up on a global linked list, and it's not safe for it to
go out of scope until we've called one of those two
deactivation functions.

Imagine that we add an early return from the function that
forgets to call delete_tempfile(). With a static or heap
tempfile variable, the worst case is that the tempfile hangs
around until the program exits (and some functions like
setup_shallow_temporary rely on this intentionally, creating
a tempfile and then leaving it for later cleanup).

But with a stack variable as above, this is a serious memory
error: the variable goes out of scope and may be filled with
garbage by the time the tempfile code looks at it.  Let's
see if we can make it harder to get this wrong.

Since many callers need to allocate arbitrary numbers of
tempfiles, we can't rely on static storage as a general
solution. So we need to turn to the heap. We could just ask
all callers to pass us a heap variable, but that puts the
burden on them to call free() at the right time.

Instead, let's have the tempfile code handle the heap
allocation _and_ the deallocation (when the tempfile is
deactivated and removed from the list).

This changes the return value of all of the creation
functions. For the cleanup functions (delete and rename),
we'll add one extra bit of safety: instead of taking a
tempfile pointer, we'll take a pointer-to-pointer and set it
to NULL after freeing the object. This makes it safe to
double-call functions like delete_tempfile(), as the second
call treats the NULL input as a noop. Several callsites
follow this pattern.

The resulting patch does have a fair bit of noise, as each
caller needs to be converted to handle:

  1. Storing a pointer instead of the struct itself.

  2. Passing the pointer instead of taking the struct
     address.

  3. Handling a "struct tempfile *" return instead of a file
     descriptor.

We could play games to make this less noisy. For example, by
defining the tempfile like this:

  struct tempfile {
	struct heap_allocated_part_of_tempfile {
                int fd;
                ...etc
        } *actual_data;
  }

Callers would continue to have a "struct tempfile", and it
would be "active" only when the inner pointer was non-NULL.
But that just makes things more awkward in the long run.
There aren't that many callers, so we can simply bite
the bullet and adjust all of them. And the compiler makes it
easy for us to find them all.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-09-06 17:19:54 +09:00
Jeff King
49bd0fc222 tempfile: do not delete tempfile on failed close
When close_tempfile() fails, we delete the tempfile and
reset the fields of the tempfile struct. This makes it
easier for callers to return without cleaning up, but it
also makes this common pattern:

  if (close_tempfile(tempfile))
	return error_errno("error closing %s", tempfile->filename.buf);

wrong, because the "filename" field has been reset after the
failed close. And it's not easy to fix, as in many cases we
don't have another copy of the filename (e.g., if it was
created via one of the mks_tempfile functions, and we just
have the original template string).

Let's drop the feature that a failed close automatically
deletes the file. This puts the burden on the caller to do
the deletion themselves, but this isn't that big a deal.
Callers which do:

  if (write(...) || close_tempfile(...)) {
	delete_tempfile(...);
	return -1;
  }

already had to call delete when the write() failed, and so
aren't affected. Likewise, any caller which just calls die()
in the error path is OK; we'll delete the tempfile during
the atexit handler.

Because this patch changes the semantics of close_tempfile()
without changing its signature, all callers need to be
manually checked and converted to the new scheme. This patch
covers all in-tree callers, but there may be others for
not-yet-merged topics. To catch these, we rename the
function to close_tempfile_gently(), which will attract
compile-time attention to new callers. (Technically the
original could be considered "gentle" already in that it
didn't die() on errors, but this one is even more so).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-09-06 17:19:53 +09:00
Jeff King
45c6b1ed24 always check return value of close_tempfile
If close_tempfile() encounters an error, then it deletes the
tempfile and resets the "struct tempfile". But many code
paths ignore the return value and continue to use the
tempfile. Instead, we should generally treat this the same
as a write() error.

Note that in the postimage of some of these cases our error
message will be bogus after a failed close because we look
at tempfile->filename (either directly or via get_tempfile_path).
But after the failed close resets the tempfile object, this
is guaranteed to be the empty string. That will be addressed
in a future patch (because there are many more cases of the
same problem than just these instances).

Note also in the hunk in gpg-interface.c that it's fine to
call delete_tempfile() in the error path, even if
close_tempfile() failed and already deleted the file. The
tempfile code is smart enough to know the second deletion is
a noop.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-09-06 17:19:53 +09:00
Jeff King
d88ef66051 verify_signed_buffer: prefer close_tempfile() to close()
We do a manual close() on the descriptor provided to us by
mks_tempfile. But this runs contrary to the advice in
tempfile.h, which notes that you should always use
close_tempfile(). Otherwise the descriptor may be reused
without the tempfile object knowing it, and the later call
to delete_tempfile() could close a random descriptor.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-09-06 17:19:52 +09:00
Junio C Hamano
50f03c6676 Merge branch 'ab/free-and-null'
A common pattern to free a piece of memory and assign NULL to the
pointer that used to point at it has been replaced with a new
FREE_AND_NULL() macro.

* ab/free-and-null:
  *.[ch] refactoring: make use of the FREE_AND_NULL() macro
  coccinelle: make use of the "expression" FREE_AND_NULL() rule
  coccinelle: add a rule to make "expression" code use FREE_AND_NULL()
  coccinelle: make use of the "type" FREE_AND_NULL() rule
  coccinelle: add a rule to make "type" code use FREE_AND_NULL()
  git-compat-util: add a FREE_AND_NULL() wrapper around free(ptr); ptr = NULL
2017-06-24 14:28:41 -07:00
Ævar Arnfjörð Bjarmason
88ce3ef636 *.[ch] refactoring: make use of the FREE_AND_NULL() macro
Replace occurrences of `free(ptr); ptr = NULL` which weren't caught by
the coccinelle rule. These fall into two categories:

 - free/NULL assignments one after the other which coccinelle all put
   on one line, which is functionally equivalent code, but very ugly.

 - manually spotted occurrences where the NULL assignment isn't right
   after the free() call.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-06-16 12:44:09 -07:00
Brandon Williams
b2141fc1d2 config: don't include config.h by default
Stop including config.h by default in cache.h.  Instead only include
config.h in those files which require use of the config system.

Signed-off-by: Brandon Williams <bmwill@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-06-15 12:56:22 -07:00
Michael J Gruber
661a180681 gpg-interface: use more status letters
According to gpg2's doc/DETAILS:

    For each signature only one of the codes GOODSIG, BADSIG,
    EXPSIG, EXPKEYSIG, REVKEYSIG or ERRSIG will be emitted.

gpg1 ("classic") behaves the same (although doc/DETAILS differs).

Currently, we parse gpg's status output for GOODSIG, BADSIG and
trust information and translate that into status codes G, B, U, N
for the %G?  format specifier.

git-verify-* returns success in the GOODSIG case only. This is
somewhat in disagreement with gpg, which considers the first 5 of
the 6 above as VALIDSIG, but we err on the very safe side.

Introduce additional status codes E, X, Y, R for ERRSIG, EXPSIG,
EXPKEYSIG, and REVKEYSIG so that a user of %G? gets more information
about the absence of a 'G' on first glance.

Requested-by: Alex <agrambot@gmail.com>
Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-10-12 10:41:59 -07:00
Junio C Hamano
af2b21ec3c Merge branch 'lt/gpg-show-long-key-in-signature-verification-maint' into lt/gpg-show-long-key-in-signature-verification
Linus's original was rebased to apply to the maintenance track just
in case binary distributors that are stuck in the past want to take
it to their older codebase.  Let's merge it up to more modern
codebase that has Peff's gpg-interface clean-up topic that appeared
after Git 2.9 was tagged.

* lt/gpg-show-long-key-in-signature-verification-maint:
  gpg-interface: prefer "long" key format output when verifying pgp signatures
2016-08-16 15:04:13 -07:00
Linus Torvalds
b624a3e67f gpg-interface: prefer "long" key format output when verifying pgp signatures
Yes, gpg2 already uses the long format by default, but most
distributions seem to still have "gpg" be the older 1.x version due to
compatibility reasons.  And older versions of gpg only show the 32-bit
short ID, which is quite insecure.

This doesn't actually matter for the _verification_ itself: if the
verification passes, the pgp signature is good.  But if you don't
actually have the key yet, and want to fetch it, or you want to check
exactly which key was used for verification and want to check it, we
should specify the key with more precision.

In fact, we should preferentially specify the whole key fingerprint, but
gpg doesn't actually support that.  Which is really quite sad.

Showing the "long" format improves things to at least show 64 bits of
the fingerprint.  That's a lot better, even if it's not perfect.

This change the log format for "git log --show-signature" from

    commit 2376d31787
    merged tag 'v2.9.3'
    gpg: Signature made Fri 12 Aug 2016 09:17:59 AM PDT using RSA key ID 96AFE6CB
    gpg: Good signature from "Junio C Hamano <gitster@pobox.com>"
    gpg:                 aka "Junio C Hamano <jch@google.com>"
    gpg:                 aka "Junio C Hamano <junio@pobox.com>"
    Merge: 2807cd7b25 e0c1ceafc5
    Author: Junio C Hamano <gitster@pobox.com>
    Date:   Fri Aug 12 10:02:18 2016 -0700

to

    commit 2376d31787
    merged tag 'v2.9.3'
    gpg: Signature made Fri 12 Aug 2016 09:17:59 AM PDT
    gpg:                using RSA key B0B5E88696AFE6CB
    gpg: Good signature from "Junio C Hamano <gitster@pobox.com>"
    gpg:                 aka "Junio C Hamano <jch@google.com>"
    gpg:                 aka "Junio C Hamano <junio@pobox.com>"
    Merge: 2807cd7b25 e0c1ceafc5
    Author: Junio C Hamano <gitster@pobox.com>
    Date:   Fri Aug 12 10:02:18 2016 -0700

(note the longer key ID, but also the reflowing of the text) and also
changes the format in the merge messages when merging a signed
tag.

If you already use gpg2 (either because it's installed by default, or
because you have set your gpg_program configuration to point to gpg2),
that already used the long format, you'll also see a change: it will now
have the same formatting as gpg 1.x, and the verification string looks
something like

    gpg: Signature made Sun 24 Jul 2016 12:24:02 PM PDT
    gpg:                using RSA key 79BE3E4300411886
    gpg: Good signature from "Linus Torvalds <torvalds@linux-foundation.org>" [ultimate]

where it used to be on one line:

    gpg: Signature made Sun 24 Jul 2016 12:24:02 PM PDT using RSA key ID 79BE3E4300411886
    gpg: Good signature from "Linus Torvalds <torvalds@linux-foundation.org>" [ultimate]

so there is certainly a chance this could break some automated scripting.
But the 32-bit key ID's really are broken. Also note that because of the
differences between gpg-1.x and gpg-2.x, hopefully any scripted key ID
parsing code (if such code exists) is already flexible enough to not care.

This was triggered by the fact that the "evil32" project keys ended up
leaking to the public key servers, so now there are 32-bit aliases for
just about every open source developer that you can easily get by
mistake if you use the 32-bit short ID format.

Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-08-16 15:02:22 -07:00
Michael J Gruber
efee9553a4 gpg-interface: check gpg signature creation status
When we create a signature, it may happen that gpg returns with
"success" but not with an actual detached signature on stdout.

Check for the correct signature creation status to catch these cases
better. Really, --status-fd parsing is the only way to check gpg status
reliably. We do the same for verify already.

Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-06-17 17:03:57 -07:00
Jeff King
0581b54641 sign_buffer: use pipe_command
Similar to the prior commit for verify_signed_buffer, the
motivation here is both to make the code simpler, and to
avoid any possible deadlocks with gpg.

In this case we have the same "write to stdin, then read
from stdout" that the verify case had. This is unlikely to
be a problem in practice, since stdout has the detached
signature, which it cannot compute until it has read all of
stdin (if it were a non-detached signature, that would be a
problem, though).

We don't read from stderr at all currently. However, we will
want to in a future patch, so this also prepares us there
(and in that case gpg _does_ write before reading all of the
input, though again, it is unlikely that a key uid will fill
up a pipe buffer).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-06-17 17:03:57 -07:00
Jeff King
0d2b664efd verify_signed_buffer: use pipe_command
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>
2016-06-17 17:03:57 -07:00
Jeff King
4322353bfb verify_signed_buffer: use tempfile object
We use git_mkstemp to create a temporary file, and try to
clean it up in all exit paths from the function. But that
misses any cases where we die by signal, or by calling die()
in a sub-function. In addition, we missed one of the exit
paths.

Let's convert to using a tempfile object, which handles the
hard cases for us, and add the missing cleanup call. Note
that we would not simply want to rely on program exit to
catch our missed cleanup, as this function may be called
many times in a single program (for the same reason, we use
a static tempfile instead of heap-allocating a new one; that
gives an upper bound on our memory usage).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-06-17 17:03:56 -07:00
Jeff King
c752fcc8e0 verify_signed_buffer: drop pbuf variable
If our caller gave us a non-NULL gpg_status parameter, we
write the gpg status into their strbuf. If they didn't, then
we write it to a temporary local strbuf (since we still need
to look at it).  The variable "pbuf" adds an extra layer of
indirection so that the rest of the function can just access
whichever is appropriate.

However, the name "pbuf" isn't very descriptive, and it's
easy to get confused about what is supposed to be in it
(especially because we are reading both "status" and
"output" from gpg).

Rather than give it a more descriptive name, we can just use
gpg_status as our indirection pointer. Either it points to
the caller's input, or we can point it directly to our
temporary buffer.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-06-17 17:03:56 -07:00
Jeff King
aedb5dc343 gpg-interface: use child_process.args
Our argv allocations are relatively straightforward, but
this avoids us having to manually keep the count up to date
(or create new to-be-replaced slots in the declaration) when
we add new arguments.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-06-17 17:03:55 -07:00
Junio C Hamano
40cfc95856 Merge branch 'nd/error-errno'
The code for warning_errno/die_errno has been refactored and a new
error_errno() reporting helper is introduced.

* nd/error-errno: (41 commits)
  wrapper.c: use warning_errno()
  vcs-svn: use error_errno()
  upload-pack.c: use error_errno()
  unpack-trees.c: use error_errno()
  transport-helper.c: use error_errno()
  sha1_file.c: use {error,die,warning}_errno()
  server-info.c: use error_errno()
  sequencer.c: use error_errno()
  run-command.c: use error_errno()
  rerere.c: use error_errno() and warning_errno()
  reachable.c: use error_errno()
  mailmap.c: use error_errno()
  ident.c: use warning_errno()
  http.c: use error_errno() and warning_errno()
  grep.c: use error_errno()
  gpg-interface.c: use error_errno()
  fast-import.c: use error_errno()
  entry.c: use error_errno()
  editor.c: use error_errno()
  diff-no-index.c: use error_errno()
  ...
2016-05-17 14:38:28 -07:00
Nguyễn Thái Ngọc Duy
ddf362a2a9 gpg-interface.c: use error_errno()
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-05-09 12:29:08 -07:00
Santiago Torres
d281b45d75 builtin/verify-tag.c: ignore SIGPIPE in gpg-interface
The verify_signed_buffer() function may trigger a SIGPIPE when the
GPG child process terminates early (due to a bad keyid, for example)
and Git tries to write to it afterwards.  Previously, ignoring
SIGPIPE was done in builtin/verify-tag.c to avoid this issue.

However, any other caller who wants to call verify_signed_buffer()
would have to do the same.

Use sigchain_push(SIGPIPE, SIG_IGN) in verify_signed_buffer(),
pretty much like in sign_buffer(), so that any caller is not
required to perform this task.

This will avoid possible mistakes by further developers using
verify_signed_buffer().

Signed-off-by: Santiago Torres <santiago@nyu.edu>
Reviewed-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-04-06 09:02:02 -07:00
brian m. carlson
aeff29dd4d verify-commit: add option to print raw gpg status information
verify-commit by default displays human-readable output on standard
error.  However, it can also be useful to get access to the raw gpg
status information, which is machine-readable, allowing automated
implementation of signing policy.  Add a --raw option to make
verify-commit produce the gpg status information on standard error
instead of the human-readable format.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-06-22 14:20:47 -07:00
brian m. carlson
ca194d50b8 gpg: centralize printing signature buffers
The code to handle printing of signature data from a struct
signature_check is very similar between verify-commit and verify-tag.
Place this in a single function.  verify-tag retains its special case
behavior of printing the tag even when no valid signature is found.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-06-22 14:20:47 -07:00
brian m. carlson
434060ec6d gpg: centralize signature check
verify-commit and verify-tag both share a central codepath for verifying
commits: check_signature.  However, verify-tag exited successfully for
untrusted signature, while verify-commit exited unsuccessfully.
Centralize this signature check and make verify-commit adopt the older
verify-tag behavior.  This behavior is more logical anyway, as the
signature is in fact valid, whether or not there's a path of trust to
the author.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-06-22 14:20:46 -07:00
brian m. carlson
a4cc18f293 verify-tag: share code with verify-commit
verify-tag was executing an entirely different codepath than
verify-commit, except for the underlying verify_signed_buffer.  Move
much of the code from check_commit_signature to a generic
check_signature function and adjust both codepaths to call it.

Update verify-tag to explicitly output the signature text, as we now
call verify_signed_buffer with strbufs to catch the output, which
prevents it from being printed automatically.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-06-22 14:20:45 -07:00
Junio C Hamano
fb06b5280e Merge branch 'jc/push-cert'
Allow "git push" request to be signed, so that it can be verified and
audited, using the GPG signature of the person who pushed, that the
tips of branches at a public repository really point the commits
the pusher wanted to, without having to "trust" the server.

* jc/push-cert: (24 commits)
  receive-pack::hmac_sha1(): copy the entire SHA-1 hash out
  signed push: allow stale nonce in stateless mode
  signed push: teach smart-HTTP to pass "git push --signed" around
  signed push: fortify against replay attacks
  signed push: add "pushee" header to push certificate
  signed push: remove duplicated protocol info
  send-pack: send feature request on push-cert packet
  receive-pack: GPG-validate push certificates
  push: the beginning of "git push --signed"
  pack-protocol doc: typofix for PKT-LINE
  gpg-interface: move parse_signature() to where it should be
  gpg-interface: move parse_gpg_output() to where it should be
  send-pack: clarify that cmds_sent is a boolean
  send-pack: refactor inspecting and resetting status and sending commands
  send-pack: rename "new_refs" to "need_pack_data"
  receive-pack: factor out capability string generation
  send-pack: factor out capability string generation
  send-pack: always send capabilities
  send-pack: refactor decision to send update per ref
  send-pack: move REF_STATUS_REJECT_NODELETE logic a bit higher
  ...
2014-10-08 13:05:25 -07:00
Junio C Hamano
d7c67668fe gpg-interface: move parse_signature() to where it should be
Our signed-tag objects set the standard format used by Git to store
GPG-signed payload (i.e. the payload followed by its detached
signature) [*1*], and it made sense to have a helper to find the
boundary between the payload and its signature in tag.c back then.

Newer code added later to parse other kinds of objects that learned
to use the same format to store GPG-signed payload (e.g. signed
commits), however, kept using the helper from the same location.

Move it to gpg-interface; the helper is no longer about signed tag,
but it is how our code and data interact with GPG.

[Reference]
*1* http://thread.gmane.org/gmane.linux.kernel/297998/focus=1383

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-09-15 13:23:20 -07:00
Junio C Hamano
a50e7ca321 gpg-interface: move parse_gpg_output() to where it should be
Earlier, ffb6d7d5 (Move commit GPG signature verification to
commit.c, 2013-03-31) moved this helper that used to be in pretty.c
(i.e. the output code path) to commit.c for better reusability.

It was a good first step in the right direction, but still suffers
from a myopic view that commits will be the only thing we would ever
want to sign---we would actually want to be able to reuse it even
wider.

The function interprets what GPG said; gpg-interface is obviously a
better place.  Move it there.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-09-15 13:23:20 -07:00
René Scharfe
d318027932 run-command: introduce CHILD_PROCESS_INIT
Most struct child_process variables are cleared using memset first after
declaration.  Provide a macro, CHILD_PROCESS_INIT, that can be used to
initialize them statically instead.  That's shorter, doesn't require a
function call and is slightly more readable (especially given that we
already have STRBUF_INIT, ARGV_ARRAY_INIT etc.).

Helped-by: Johannes Sixt <j6t@kdbg.org>
Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-08-20 09:53:37 -07:00
Michael J Gruber
71c214c840 gpg-interface: provide access to the payload
In contrast to tag signatures, commit signatures are put into the
header, that is between the other header parts and commit messages.

Provide access to the commit content sans the signature, which is the
payload that is actually signed. Commit signature verification does the
parsing anyways, and callers may wish to act on or display the commit
object sans the signature.

Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-06-23 15:50:30 -07:00
Michael J Gruber
01e57b5d91 gpg-interface: provide clear helper for struct signature_check
The struct has been growing members whose malloced memory needs to be
freed. Do this with one helper function so that no malloced memory shall
be left unfreed.

Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-06-23 15:50:29 -07:00
Junio C Hamano
0f6875dbe2 Merge branch 'mg/gpg-interface-using-status'
Call "gpg" using the right API when validating the signature on
tags.

* mg/gpg-interface-using-status:
  pretty: make %GK output the signing key for signed commits
  pretty: parse the gpg status lines rather than the output
  gpg_interface: allow to request status return
  log-tree: rely upon the check in the gpg_interface
  gpg-interface: check good signature in a reliable way
2013-03-21 14:02:55 -07:00
Michael J Gruber
9cc4ac8ff1 gpg_interface: allow to request status return
Currently, verify_signed_buffer() returns the user facing output only.

Allow callers to request the status output also.

Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2013-02-14 09:30:04 -08:00
Michael J Gruber
b60b7566c0 gpg-interface: check good signature in a reliable way
Currently, verify_signed_buffer() only checks the return code of gpg,
and some callers implement additional unreliable checks for "Good
signature" in the gpg output meant for the user.

Use the status output instead and parse for a line beinning with
"[GNUPG:] GOODSIG ". This is the only reliable way of checking for a
good gpg signature.

If needed we can change this easily to "[GNUPG:] VALIDSIG " if we want
to take into account the trust model.

Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2013-02-14 09:27:40 -08:00
Junio C Hamano
d2216a4b13 Merge branch 'sb/gpg-plug-fd-leak' into maint
We forgot to close the file descriptor reading from "gpg" output,
killing "git log --show-signature" on a long history.

* sb/gpg-plug-fd-leak:
  gpg: close stderr once finished with it in verify_signed_buffer()
2013-02-07 15:14:54 -08:00
Junio C Hamano
9a1ab9e72f Merge branch 'sb/gpg-i18n'
* sb/gpg-i18n:
  gpg: allow translation of more error messages
2013-02-07 14:41:34 -08:00
Junio C Hamano
15778842bd Merge branch 'sb/gpg-plug-fd-leak'
We forgot to close the file descriptor reading from "gpg" output,
killing "git log --show-signature" on a long history.

* sb/gpg-plug-fd-leak:
  gpg: close stderr once finished with it in verify_signed_buffer()
2013-02-05 16:12:43 -08:00
Stephen Boyd
7dac3f8321 gpg: close stderr once finished with it in verify_signed_buffer()
Failing to close the stderr pipe in verify_signed_buffer() causes
git to run out of file descriptors if there are many calls to
verify_signed_buffer(). An easy way to trigger this is to run

 git log --show-signature --merges | grep "key"

on the linux kernel git repo. Eventually it will fail with

 error: cannot create pipe for gpg: Too many open files
 error: could not run gpg.

Close the stderr pipe so that this can't happen.

Suggested-by: Jeff King <peff@peff.net>
Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2013-01-31 11:10:44 -08:00
Stephen Boyd
4c9a418227 gpg: allow translation of more error messages
Mark these strings for translation so that error messages are
printed in the user's language of choice.

Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2013-01-31 11:10:26 -08:00
Jeff King
f9bc573fda ident: rename IDENT_ERROR_ON_NO_NAME to IDENT_STRICT
Callers who ask for ERROR_ON_NO_NAME are not so much
concerned that the name will be blank (because, after all,
we will fall back to using the username), but rather it is a
check to make sure that low-quality identities do not end up
in things like commit messages or emails (whereas it is OK
for them to end up in things like reflogs).

When future commits add more quality checks on the identity,
each of these callers would want to use those checks, too.
Rather than modify each of them later to add a new flag,
let's refactor the flag.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2012-05-24 17:16:41 -07:00
Junio C Hamano
e3f55e0707 verify_signed_buffer: fix stale comment
The function used to take an integer flag to specify where the output
should go, but these days we supply a strbuf to receive it.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2012-01-05 13:02:26 -08:00
Junio C Hamano
0c5e70f041 gpg-interface: allow use of a custom GPG binary
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2011-11-29 12:30:02 -08:00
Junio C Hamano
2f47eae2a1 Split GPG interface into its own helper library
This mostly moves existing code from builtin/tag.c (for signing)
and builtin/verify-tag.c (for verifying) to a new gpg-interface.c
file to provide a more generic library interface.

 - sign_buffer() takes a payload strbuf, a signature strbuf, and a signing
   key, runs "gpg" to produce a detached signature for the payload, and
   appends it to the signature strbuf. The contents of a signed tag that
   concatenates the payload and the detached signature can be produced by
   giving the same strbuf as payload and signature strbuf.

 - verify_signed_buffer() takes a payload and a detached signature as
   <ptr, len> pairs, and runs "gpg --verify" to see if the payload matches
   the signature. It can optionally capture the output from GPG to allow
   the callers to pretty-print it in a way more suitable for their
   contexts.

"verify-tag" (aka "tag -v") used to save the whole tag contents as if it
is a detached signature, and fed gpg the payload part of the tag. It
relied on gpg to fail when the given tag is not signed but just is
annotated.  The updated run_gpg_verify() function detects the lack of
detached signature in the input, and errors out without bothering "gpg".

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2011-11-04 21:40:25 -07:00