git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: Elijah Newren <newren@gmail.com>
Cc: Junio C Hamano <gitster@pobox.com>,
	Luke Shumaker <lukeshu@lukeshu.com>,
	Git Mailing List <git@vger.kernel.org>,
	Luke Shumaker <lukeshu@datawire.io>, Jeff King <peff@peff.net>
Subject: Re: [RFC PATCH] fast-export, fast-import: Let tags specify an internal name
Date: Thu, 22 Apr 2021 10:54:23 +0200	[thread overview]
Message-ID: <871rb23dj4.fsf@evledraar.gmail.com> (raw)
In-Reply-To: <CABPp-BF373j2BbyTgTJbKzDP9Y5R2jZVNqWeOqLtypdz6VZRMQ@mail.gmail.com>


On Wed, Apr 21 2021, Elijah Newren wrote:

> On Wed, Apr 21, 2021 at 11:54 AM Junio C Hamano <gitster@pobox.com> wrote:
>>
>> Elijah Newren <newren@gmail.com> writes:
>>
>> > On Tue, Apr 20, 2021 at 2:40 PM Junio C Hamano <gitster@pobox.com> wrote:
>> >> ...
>> >> +The `<refname>` is prefixed with `refs/tags/` when stored
>> >>  in Git, so importing the CVS branch symbol `RELENG-1_0-FINAL` would
>> >> -use just `RELENG-1_0-FINAL` for `<name>`, and fast-import will write the
>> >> +use just `RELENG-1_0-FINAL` for `<refname>`, and fast-import will write the
>> >>  corresponding ref as `refs/tags/RELENG-1_0-FINAL`.
>> >
>> > Going on a slight tangent since you didn't introduce this, but since
>> > you're modifying this exact documentation...
>> >
>> > I hate the assumed "refs/tags/" prefix.  Especially since ...
>> > ... The special casing reminds me of the ref-updated hook in
>> > gerrit
>>
>> Gerrit and fast-import?  What is common is Shawn, perhaps ;-)?
>
> :-)  To be fair, though, given the number of things he created for us,
> it's inevitable there'd be a few small things causing problems and
> these are small potatoes in the big scheme of things.  ref-updated was
> fixed years ago, and it sounds like Luke is about to fix the tag
> prefix assumption for us.
>
>> > broken given the fact that the name inside the tag didn't match the
>> > name of the actual ref.  (To be honest, though, I was never sure why
>> > the name of the tag was recorded inside the tag itself.)
>>
>> The name of the tag and the name of the object has to be together
>> for a signature over it to have any meaning, no?
>
> Oh, I guess if you treat the signature as affirming that not only do
> you like the object but that it has a particular nickname, then yes
> you'd need both.  I had always viewed a signed tag as an affirmation
> that the object was good/tested/verified/whatever, and viewed the
> nickname of that good object as ancillary.  I have to admit to not
> using signed tags much, though.

The current behavior leaves the door open to an attack where say git has
a security point-release v2.31.2, and you have my hostile repo as a
remote, and I've sneakily replaced v2.31.2 in that repo with the object
pointed-to by v2.31.1.

You "update" (but not really), verify v2.31.2 with Junio's GPG key,
which is all correctly signed content. But the tag name isn't what you
expected, and thus you don't get the security fix, I use this
information to attack you.

This already unplausible but hypothetical attack was sort-of-maybe
plausible before my 0bc8d71b99e (fetch: stop clobbering existing tags
without --force, 2018-08-31).

That was released with v2.20.0, before that I could more easily sneak
such a tag into your repo knowing that you were doing a "git fetch
--all" and had my evil git.git clone[1] on github.com evil remote. Now
that's unlikely to happen, in practice the "fetch --all" happens in
order, you'll have your "origin" remote first in the file (it's the way
git config adds them), and will get the good tag first.

Hrm, I suppose with --jobs and a race condition that might not always be
true. Aside from this mostly imaginary issue maybe having --jobs be
deterministic (i.e. "fetch content in parallel, apply ref updates in
sequence") might be a good idea..

Anyway, getting back on point since no release of git has cared about
the "tag" field I'd be inclined to say that we should explicitly
document that we don't care, and perhaps document this caveat.

1. Disclosure: I know of no actual evilness except a bunch of crappy WIP
   code in my git.git fork on github.

  reply	other threads:[~2021-04-22  8:54 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-20 19:05 [RFC PATCH] fast-export, fast-import: Let tags specify an internal name Luke Shumaker
2021-04-20 21:40 ` Junio C Hamano
2021-04-21  8:18   ` Ævar Arnfjörð Bjarmason
2021-04-21 16:17     ` Luke Shumaker
2021-04-21 16:59     ` Junio C Hamano
2021-04-21 18:34     ` Elijah Newren
2021-04-21 18:48       ` Luke Shumaker
2021-04-21 19:24         ` Elijah Newren
2021-04-22  8:41       ` Ævar Arnfjörð Bjarmason
2021-04-21 18:41   ` Elijah Newren
2021-04-21 18:54     ` Junio C Hamano
2021-04-21 19:32       ` Elijah Newren
2021-04-22  8:54         ` Ævar Arnfjörð Bjarmason [this message]
2021-04-22 19:37           ` Elijah Newren
2021-04-21  8:03 ` Ævar Arnfjörð Bjarmason
2021-04-21 16:34   ` Luke Shumaker
2021-04-21 17:26     ` Luke Shumaker
2021-04-21 18:26     ` Elijah Newren
2021-04-21 17:48   ` Junio C Hamano
2021-04-23 16:47 ` Luke Shumaker

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=871rb23dj4.fsf@evledraar.gmail.com \
    --to=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=lukeshu@datawire.io \
    --cc=lukeshu@lukeshu.com \
    --cc=newren@gmail.com \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).