Commit Graph

25 Commits

Author SHA1 Message Date
SZEDER Gábor
2f3cbcd8c5 tests: make forging GPG signed commits and tags more robust
A couple of test scripts create forged GPG signed commits or tags to
check that such forgery can't fool various git commands' signature
verification.  All but one of those test scripts are prone to
occasional failures because the forgery creates a bogus GPG signature,
and git commands error out with an unexpected error message, e.g.
"Commit deadbeef does not have a GPG signature" instead of "...  has a
bad GPG signature".

't5573-pull-verify-signatures.sh', 't7510-signed-commit.sh' and
't7612-merge-verify-signatures.sh' create forged signed commits like
this:

  git commit -S -m "bad on side" &&
  git cat-file commit side-bad >raw &&
  sed -e "s/bad/forged bad/" raw >forged &&
  git hash-object -w -t commit forged >forged.commit

On rare occasions the given pattern occurs not only in the commit
message but in the GPG signature as well, and after it's replaced in
the signature the resulting signature becomes invalid, GPG will report
CRC error and that it couldn't find any signature, which will then
ultimately cause the test failure.

Since in all three cases the pattern to be replaced during the forgery
is the first word of the commit message's subject line, and since the
GPG signature in the commit object is indented by a space, let's just
anchor those patterns to the beginning of the line to prevent this
issue.

The test script 't7030-verify-tag.sh' creates a forged signed tag
object in a similar way by replacing the pattern "seventh", but the
GPG signature in tag objects is not indented by a space, so the above
solution is not applicable in this case.  However, in the tag object
in question the pattern "seventh" occurs not only in the tag message
but in the 'tag' header as well.  To create a forged tag object it's
sufficient to replace only one of the two occurences, so modify the
sed script to limit the pattern to the 'tag' header (i.e. a line
beginning with "tag ", which, because of the space character, can
never occur in the base64-encoded GPG signature).

Note that the forgery in 't7004-tag.sh' is not affected by this issue:
while 't7004' does create a forged signed tag kind of the same way,
it replaces "signed-tag" in the tag object, which, because of the '-'
character, can never occur in the base64-encoded GPG signarute.

Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-06-11 10:19:03 -07:00
SZEDER Gábor
9dd39821e3 t7510-signed-commit: use 'test_must_fail'
The two tests 'detect fudged signature' and 'detect fudged signature
with NUL' in 't7510-signed-commit.sh' check that 'git verify-commit'
errors out when encountering a forged commit, but they do so by
running

  ! git verify-commit ...

Use 'test_must_fail' instead, because that would catch potential
unexpected errors like a segfault as well.

Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-06-11 10:19:03 -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
Mehul Jain
fce04c3ca6 log: add log.showSignature configuration variable
Users may want to always use "--show-signature" while using git-log and
related commands.

When log.showSignature is set to true, git-log and related commands will
behave as if "--show-signature" was given to them.

Note that this config variable is meant to affect git-log, git-show,
git-whatchanged and git-reflog. Other commands like git-format-patch,
git-rev-list are not to be affected by this config variable.

Signed-off-by: Mehul Jain <mehul.jain2029@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-06-24 13:01:13 -07:00
Junio C Hamano
50b26f5612 Merge branch 'jc/commit-tree-ignore-commit-gpgsign'
"git commit-tree" plumbing command required the user to always sign
its result when the user sets the commit.gpgsign configuration
variable, which was an ancient mistake.  Rework "git rebase" that
relied on this mistake so that it reads commit.gpgsign and pass (or
not pass) the -S option to "git commit-tree" to keep the end-user
expectation the same, while teaching "git commit-tree" to ignore
the configuration variable.  This will stop requiring the users to
sign commit objects used internally as an implementation detail of
"git stash".

* jc/commit-tree-ignore-commit-gpgsign:
  commit-tree: do not pay attention to commit.gpgsign
2016-05-13 13:18:27 -07:00
Junio C Hamano
6694856153 commit-tree: do not pay attention to commit.gpgsign
ba3c69a9 (commit: teach --gpg-sign option, 2011-10-05) introduced a
"signed commit" by teaching the --[no]-gpg-sign option and the
commit.gpgsign configuration variable to various commands that
create commits.

Teaching these to "git commit" and "git merge", both of which are
end-user facing Porcelain commands, was perfectly fine.  Allowing
the plumbing "git commit-tree" to suddenly change the behaviour to
surprise the scripts by paying attention to commit.gpgsign was not.

Among the in-tree scripts, filter-branch, quiltimport, rebase and
stash are the commands that run "commit-tree".  If any of these
wants to allow users to always sign every single commit, they should
offer their own configuration (e.g. "filterBranch.gpgsign") with an
option to disable signing (e.g. "git filter-branch --no-gpgsign").

Ignoring commit.gpgsign option _obviously_ breaks the backward
compatibility, but it is easy to follow the standard pattern in
scripts to honor whatever configuration variable they choose to
follow.  E.g.

	case $(git config --bool commit.gpgsign) in
	true) sign=-S ;;
	*) sign= ;;
	esac &&
	git commit-tree $sign ...whatever other args...

Do so to make sure that "git rebase" keeps paying attention to the
configuration variable, which unfortunately is a documented mistake.

Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-05-03 10:59:25 -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
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
8e98e5f27a verify-commit: add test for exit status on untrusted signature
verify-tag exits successfully if the signature is good but the key is
untrusted.  verify-commit exits unsuccessfully.  This divergence in
behavior is unexpected and unwanted.  Since verify-tag existed earlier,
add a failing test to have verify-commit share verify-tag's behavior.

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
Junio C Hamano
30db51a3fe Merge branch 'jk/test-chain-lint'
People often forget to chain the commands in their test together
with &&, leaving a failure from an earlier command in the test go
unnoticed.  The new GIT_TEST_CHAIN_LINT mechanism allows you to
catch such a mistake more easily.

* jk/test-chain-lint: (36 commits)
  t9001: drop save_confirm helper
  t0020: use test_* helpers instead of hand-rolled messages
  t: simplify loop exit-code status variables
  t: fix some trivial cases of ignored exit codes in loops
  t7701: fix ignored exit code inside loop
  t3305: fix ignored exit code inside loop
  t0020: fix ignored exit code inside loops
  perf-lib: fix ignored exit code inside loop
  t6039: fix broken && chain
  t9158, t9161: fix broken &&-chain in git-svn tests
  t9104: fix test for following larger parents
  t4104: drop hand-rolled error reporting
  t0005: fix broken &&-chains
  t7004: fix embedded single-quotes
  t0050: appease --chain-lint
  t9001: use test_when_finished
  t4117: use modern test_* helpers
  t6034: use modern test_* helpers
  t1301: use modern test_* helpers
  t0020: use modern test_* helpers
  ...
2015-03-26 11:57:14 -07:00
Junio C Hamano
4c24385e80 Merge branch 'mg/verify-commit'
Workarounds for certain build of GPG that triggered false breakage
in a test.

* mg/verify-commit:
  t7510: do not fail when gpg warns about insecure memory
2015-03-20 13:11:51 -07:00
Jeff King
99094a7ad4 t: fix trivial &&-chain breakage
These are tests which are missing a link in their &&-chain,
but during a setup phase. We may fail to notice failure in
commands that build the test environment, but these are
typically not expected to fail at all (but it's still good
to double-check that our test environment is what we
expect).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-03-20 10:20:14 -07:00
Kyle J. McKay
3f88c1b524 t7510: do not fail when gpg warns about insecure memory
Depending on how gpg was built, it may issue the following
message to stderr when run:

  Warning: using insecure memory!

When the test is collecting gpg output it is therefore not
enough to just match on a "gpg: " prefix it must also match
on a "Warning: " prefix wherever it needs to match lines
that have been produced by gpg.

Signed-off-by: Kyle J. McKay <mackyle@gmail.com>
Acked-by: Michael J Gruber <git@drmicha.warpmail.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-03-10 15:25:22 -07:00
Junio C Hamano
39177c7f18 Merge branch 'mg/verify-commit'
Add 'verify-commit' to be used in a way similar to 'verify-tag' is
used.  Further work on verifying the mergetags might be needed.

* mg/verify-commit:
  t7510: test verify-commit
  t7510: exit for loop with test result
  verify-commit: scriptable commit signature verification
  gpg-interface: provide access to the payload
  gpg-interface: provide clear helper for struct signature_check
2014-07-10 11:27:34 -07:00
Jeff King
958b2eb26c move "%G" format test from t7510 to t6006
The final test in t7510 checks that "--format" placeholders
that look similar to GPG placeholders (but that we don't
actually understand) are passed through. That test was
placed in t7510, since the other GPG placeholder tests are
there. However, it does not have a GPG prerequisite, because
it is not actually checking any signed commits.

This causes the test to erroneously fail when gpg is not
installed on a system, however. Not because we need signed
commits, but because we need _any_ commit to run "git log".
If we don't have gpg installed, t7510 doesn't create any
commits at all.

We can fix this by moving the test into t6006. This is
arguably a better place anyway, because it is where we test
most of the other placeholders (we do not test GPG
placeholders there because of the infrastructure needed to
make signed commits).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-06-25 15:01:06 -07:00
Michael J Gruber
8e92c2cf37 t7510: test verify-commit
This mixes the "git verify-commit" tests in with the "git show
--show-signature" tests, to keep the tests more readable.

The tests already mix in the "call show" tests with the "verify" tests.
So in case of a test beakage, a '-v' run would be needed to reveal the
exact point of breakage anyway.

Additionally, test the actual output of "git verify-commit" and "git
show --show-signature" and compare to "git cat-file".

Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-06-23 15:50:31 -07:00
Michael J Gruber
0f109c92b0 t7510: exit for loop with test result
t7510 uses for loops in a subshell, which need to make sure that the test
returns with the appropriate error code from within the loop.

Restructure the loops as the usual && chains with a single point of
"exit 1" at the end of the loop to make this clearer.

Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-06-23 15:50:31 -07:00
Jeff King
aa4b78d483 pretty: avoid reading past end-of-string with "%G"
If the user asks for --format=%G with nothing else, we
correctly realize that "%G" is not a valid placeholder (it
should be "%G?", "%GK", etc). But we still tell the
strbuf_expand code that we consumed 2 characters, causing it
to jump over the trailing NUL and output garbage.

This also fixes the case where "%GX" would be consumed (and
produce no output). In other cases, we pass unrecognized
placeholders through to the final string.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-06-17 13:41:41 -07:00
Jeff King
06ca0f45a0 t7510: check %G* pretty-format output
We do not check these along with the other pretty-format
placeholders in t6006, because we need signed commits to
make them interesting. t7510 has such commits, and can
easily exercise them in addition to the regular
--show-signature code path.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-06-17 13:41:39 -07:00
Jeff King
4baf839fe0 t7510: test a commit signed by an unknown key
We tested both good and bad signatures, but not ones made
correctly but with a key for which we have no trust.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-06-17 13:41:28 -07:00
Michael J Gruber
7b1732c116 t7510: use consistent &&-chains in loop
We check multiple commits in a loop. Because we want to
break out of the loop if any single iteration fails, we use
a subshell/exit like:

  (
	for i in $stuff
	do
		do-something $i || exit 1
	done
  )

However, we are inconsistent in our loop body. Some commands
get their own "|| exit 1", and others try to chain to the
next command with "&&", like:

  X &&
  Y || exit 1
  Z || exit 1

This is a little hard to read and follow, because X and Y
are treated differently for no good reason. But much worse,
the second loop follows a similar pattern and gets it wrong.
"Y" is expected to fail, so we use "&& exit 1", giving us:

  X &&
  Y && exit 1
  Z || exit 1

That gets the test for X wrong (we do not exit unless both X
fails and Y unexpectedly succeeds, but we would want to exit
if _either_ is wrong). We can write this clearly and
correctly by consistently using "&&", followed by a single
"|| exit 1", and negating Y with "!" (as we would in a
normal &&-chain). Like:

  X &&
  ! Y &&
  Z || exit 1

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-06-17 13:39:52 -07:00
Jeff King
526d56e072 t7510: stop referring to master in later tests
Our setup creates a sequence of commits, each with its own
tag. However, we sometimes refer to "seventh-signed" as
"master". This works, since it is at the tip of the created
branch, but is brittle if new tests need to add more
commits. Let's use its tag name to be unambiguous.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-06-17 13:39:12 -07:00
Nicolas Vigier
4b8d14b4c5 test the commit.gpgsign config option
The tests are checking that :

- when commit.gpgsign is true, "git commit" creates signed commits

- when commit.gpgsign is false, "git commit" creates unsigned commits

- when commit.gpgsign is true, "git commit --no-gpg-sign" creates
  unsigned commits

- when commit.gpgsign is true, "git rebase -f" creates signed commits

Signed-off-by: Nicolas Vigier <boklm@mars-attacks.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-02-24 14:51:35 -08:00
Junio C Hamano
c871a1d17b commit --amend -S: strip existing gpgsig headers
Any existing commit signature was made against the contents of the old
commit, including its committer date that is about to change, and will
become invalid by amending it.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2012-01-05 13:02:26 -08:00
Junio C Hamano
247503f28f test "commit -S" and "log --show-signature"
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2011-11-12 22:27:38 -08:00