Re: [PATCHv3 0/5] verify-commit: verify commit signatures
Jeff King writes:
> On Mon, Jun 23, 2014 at 10:52:38AM -0700, Junio C Hamano wrote:
>
>> > The one thing that does give me pause is that we do not seem to have any
>> > way of accessing mergetag signatures. We should perhaps stop and think
>> > for a second about how we might expose those (and whether it would fit
>> > into the "git-verify-{commit,tag}" paradigm). I am tempted to say that
>> > "git verify-tag" on a commit should verify the mergetag (right now it
>> > would simply be an error). But I haven't though that hard on it.
>>
>> I agree that "verify-commit" that lives next to "verify-tag" is fine
>> and does not have to wait for a unified "verify" that may not even
>> be a good idea, but if we were to verify the mergetags in one of
>> these "verify-$OBJECTTYPE" commands, I would think "verify-commit"
>> should be the one to check them, for the simple reason that they
>> appear in "commit" objects, not in "tag" objects.
>
> My thinking was the opposite: it is a signature on a tag, but that
> signature happens to be stuffed into a commit object. But I do not have
> a strong feeling either way.
Well, the whole point of storing mergetag inside commit objects is
so that these transient "please pull, here is a tag to prove you
that it is from me" tags do not have to be kept in the history;
hence people who are following along only see commits and not tags.
The signature being sent via a signed tag from the requestor to the
integrator is merely an implementation detail after the mergetag is
recorded and when people would want to verify them.
So...
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv3 0/5] verify-commit: verify commit signatures
On Mon, Jun 23, 2014 at 10:52:38AM -0700, Junio C Hamano wrote:
> > The one thing that does give me pause is that we do not seem to have any
> > way of accessing mergetag signatures. We should perhaps stop and think
> > for a second about how we might expose those (and whether it would fit
> > into the "git-verify-{commit,tag}" paradigm). I am tempted to say that
> > "git verify-tag" on a commit should verify the mergetag (right now it
> > would simply be an error). But I haven't though that hard on it.
>
> I agree that "verify-commit" that lives next to "verify-tag" is fine
> and does not have to wait for a unified "verify" that may not even
> be a good idea, but if we were to verify the mergetags in one of
> these "verify-$OBJECTTYPE" commands, I would think "verify-commit"
> should be the one to check them, for the simple reason that they
> appear in "commit" objects, not in "tag" objects.
My thinking was the opposite: it is a signature on a tag, but that
signature happens to be stuffed into a commit object. But I do not have
a strong feeling either way.
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv3 0/5] verify-commit: verify commit signatures
Jeff King writes:
> On Mon, Jun 23, 2014 at 09:05:46AM +0200, Michael J Gruber wrote:
>
>> This incorporates all remarks about the test coding guidelines and
>> rearranging the series.
>>
>> Open questions:
>> - There was some debate about (optionally) verifying more than what
>> git-verify-{commit,tag} currently do, or going for a generic git-verify
>> command.
>> The former would require both to be changed (in order to treat similar cases
>> similarly),
>> the latter would need a deprecation for git-verify-tag.
>
> I think that a potential "git verify" doesn't need to block this series,
> per the logic I gave elsewhere.
>
> The one thing that does give me pause is that we do not seem to have any
> way of accessing mergetag signatures. We should perhaps stop and think
> for a second about how we might expose those (and whether it would fit
> into the "git-verify-{commit,tag}" paradigm). I am tempted to say that
> "git verify-tag" on a commit should verify the mergetag (right now it
> would simply be an error). But I haven't though that hard on it.
I agree that "verify-commit" that lives next to "verify-tag" is fine
and does not have to wait for a unified "verify" that may not even
be a good idea, but if we were to verify the mergetags in one of
these "verify-$OBJECTTYPE" commands, I would think "verify-commit"
should be the one to check them, for the simple reason that they
appear in "commit" objects, not in "tag" objects.
I would imagine that I would not mind too much if "verify-tag"
delegated the verification to "verify-commit" automatically when it
is given a commit object, but for a command fairly low-level to be
useful for scripting, such a DWIMmage may be too unexpected and make
them unnecessarily unreliable. Using scripts that want strictness
(and who in the right mind that wants to verify things do not want
strictness?) would need to "cat-file -t" upfront to switch on the
object type.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv3 0/5] verify-commit: verify commit signatures
On Mon, Jun 23, 2014 at 09:05:46AM +0200, Michael J Gruber wrote:
> This incorporates all remarks about the test coding guidelines and
> rearranging the series.
>
> Open questions:
> - There was some debate about (optionally) verifying more than what
> git-verify-{commit,tag} currently do, or going for a generic git-verify
> command.
> The former would require both to be changed (in order to treat similar cases
> similarly),
> the latter would need a deprecation for git-verify-tag.
I think that a potential "git verify" doesn't need to block this series,
per the logic I gave elsewhere.
The one thing that does give me pause is that we do not seem to have any
way of accessing mergetag signatures. We should perhaps stop and think
for a second about how we might expose those (and whether it would fit
into the "git-verify-{commit,tag}" paradigm). I am tempted to say that
"git verify-tag" on a commit should verify the mergetag (right now it
would simply be an error). But I haven't though that hard on it.
I don't think implementation of that needs to be a blocker for this
series, but I'd rather see at least a vague plan so that we do not paint
ourselves into a corner.
> Michael J Gruber (5):
> gpg-interface: provide clear helper for struct signature_check
> gpg-interface: provide access to the payload
> verify-commit: scriptable commit signature verification
> t7510: exit for loop with test result
> t7510: test verify-commit
I didn't see anything objectionable here. I think you may want to rebase
on top of jk/pretty-G-format-fixes. That makes your patch 4 redundant,
and your patch 5 will probably need a few minor updates to match
cleanups in the surrounding code.
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv3 0/5] verify-commit: verify commit signatures
This incorporates all remarks about the test coding guidelines and
rearranging the series.
Open questions:
- There was some debate about (optionally) verifying more than what
git-verify-{commit,tag} currently do, or going for a generic git-verify command.
The former would require both to be changed (in order to treat similar cases
similarly),
the latter would need a deprecation for git-verify-tag.
- I haven't looked yet at what happened over the weekend.
Michael J Gruber (5):
gpg-interface: provide clear helper for struct signature_check
gpg-interface: provide access to the payload
verify-commit: scriptable commit signature verification
t7510: exit for loop with test result
t7510: test verify-commit
Documentation/git-verify-commit.txt | 28 +++
Makefile| 1 +
builtin.h | 1 +
builtin/merge.c | 5 +-
builtin/verify-commit.c | 93 +
command-list.txt| 1 +
commit.c| 1 +
git.c | 1 +
gpg-interface.c | 14 ++
gpg-interface.h | 2 +
pretty.c| 3 +-
t/t7510-signed-commit.sh| 24 --
12 files changed, 165 insertions(+), 9 deletions(-)
create mode 100644 Documentation/git-verify-commit.txt
create mode 100644 builtin/verify-commit.c
--
2.0.0.426.g37dbf84
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
