* Re: SHA1 collisions found
From: Junio C Hamano @ 2017-02-23 17:02 UTC (permalink / raw)
To: Joey Hess; +Cc: Git Mailing List
In-Reply-To: <20170223164306.spg2avxzukkggrpb@kitenet.net>
On Thu, Feb 23, 2017 at 8:43 AM, Joey Hess <id@joeyh.name> wrote:
>
> Since we now have collisions in valid PDF files, collisions in valid git
> commit and tree objects are probably able to be constructed.
That may be true, but
https://public-inbox.org/git/Pine.LNX.4.58.0504291221250.18901@ppc970.osdl.org/
^ permalink raw reply
* Re: [PATCH] http(s): automatically try NTLM authentication first
From: Johannes Schindelin @ 2017-02-23 17:08 UTC (permalink / raw)
To: Jeff King
Cc: Junio C Hamano, David Turner, git@vger.kernel.org,
sandals@crustytoothpaste.net, Eric Sunshine
In-Reply-To: <20170222213410.iak43asq775tzr42@sigill.intra.peff.net>
Hi Peff,
On Wed, 22 Feb 2017, Jeff King wrote:
> On Wed, Feb 22, 2017 at 01:16:33PM -0800, Junio C Hamano wrote:
>
> > David Turner <David.Turner@twosigma.com> writes:
> >
> > > Always, no. For failed authentication (or authorization),
> > > apparently, yes. I tested this by setting the variable to false
> > > and then true, and trying to Push to a github repository which I
> > > didn't have write access to, with both an empty username
> > > (https://@:github.com/...) and no username (http://github.com/...).
> > > I ran this under GIT_CURL_VERBOSE=1 and I saw two 401 responses in
> > > the "http.emptyauth=true" case and one in the false case. I also
> > > tried with a repo that I did have access to (first configuring the
> > > necessary tokens for HTTPS push access), and saw two 401 responses
> > > in *both* cases.
> >
> > Thanks; that matches my observation. I do not think we care about
> > an extra roundtrip for the failure case, but as long as we do not
> > increase the number of roundtrip in the normal case, we can declare
> > that this is an improvement. I am not quite sure where that extra
> > 401 comes from in the normal case, and that might be an indication
> > that we already are doing something wrong, though.
>
> This patch drops the useless probe request:
>
> diff --git a/http.c b/http.c
> index 943e630ea..7b4c2db86 100644
> --- a/http.c
> +++ b/http.c
> @@ -1663,6 +1663,9 @@ static int http_request(const char *url,
> curlinfo_strbuf(slot->curl, CURLINFO_EFFECTIVE_URL,
> options->effective_url);
>
> + if (results.auth_avail == CURLAUTH_BASIC)
> + http_auth_methods = CURLAUTH_BASIC;
> +
> curl_slist_free_all(headers);
> strbuf_release(&buf);
>
>
> but setting http.emptyauth adds back in the useless request. I think
> that could be fixed by skipping the empty-auth thing when
> http_auth_methods does not have CURLAUTH_NEGOTIATE in it (or perhaps
> other methods need it to, so maybe skip it if _just_ BASIC is set).
>
> I suspect the patch above could probably be generalized as:
>
> /* cut out methods we know the server doesn't support */
> http_auth_methods &= results.auth_avail;
>
> and let curl figure it out from there.
Maybe this patch (or a variation thereof) would also be able to fix this
problem with the patch:
https://github.com/git-for-windows/git/issues/1034
Short version: for certain servers (that do *not* advertise Negotiate),
setting emptyauth to true will result in a failed fetch, without letting
the user type in their credentials.
Ciao,
Johannes
^ permalink raw reply
* Re: SHA1 collisions found
From: David Lang @ 2017-02-23 17:00 UTC (permalink / raw)
To: Joey Hess; +Cc: git
In-Reply-To: <20170223164306.spg2avxzukkggrpb@kitenet.net>
On Thu, 23 Feb 2017, Joey Hess wrote:
> https://shattered.io/static/shattered.pdf
> https://freedom-to-tinker.com/2017/02/23/rip-sha-1/
>
> IIRC someone has been working on parameterizing git's SHA1 assumptions
> so a repository could eventually use a more secure hash. How far has
> that gotten? There are still many "40" constants in git.git HEAD.
>
> In the meantime, git commit -S, and checks that commits are signed,
> seems like the only way to mitigate against attacks such as
> the ones described in the threads at
> https://joeyh.name/blog/sha-1/ and
> https://joeyh.name/blog/entry/size_of_the_git_sha1_collision_attack_surface/
>
> Since we now have collisions in valid PDF files, collisions in valid git
> commit and tree objects are probably able to be constructed.
keep in mind that there is a huge difference between
creating a collision between two documents you create, both of which contain a
huge amount of arbitrary binary data that can be changed at will without
affecting the results
and
creating a collision betwen an existing document that someone else created and a
new document that is also valid C code without huge amounts of binary in it.
So, it's not time to panic, but it is one more push to make the changes to
support something else.
David Lang
^ permalink raw reply
* Re: SHA1 collisions found
From: Junio C Hamano @ 2017-02-23 17:18 UTC (permalink / raw)
To: Joey Hess; +Cc: Git Mailing List
In-Reply-To: <CAPc5daVZ79WWKSw76kxHgDra9a7fSR1AibZa_pvK9aUuuVawLQ@mail.gmail.com>
On Thu, Feb 23, 2017 at 9:02 AM, Junio C Hamano <gitster@pobox.com> wrote:
> On Thu, Feb 23, 2017 at 8:43 AM, Joey Hess <id@joeyh.name> wrote:
>>
>> Since we now have collisions in valid PDF files, collisions in valid git
>> commit and tree objects are probably able to be constructed.
>
> That may be true, but
> https://public-inbox.org/git/Pine.LNX.4.58.0504291221250.18901@ppc970.osdl.org/
IOW, we want to continue the work to switch from SHA-1, but today's announcement
does not fundamentally change anything and we do not panic.
^ permalink raw reply
* Re: SHA1 collisions found
From: Linus Torvalds @ 2017-02-23 17:19 UTC (permalink / raw)
To: Joey Hess; +Cc: Git Mailing List
In-Reply-To: <20170223164306.spg2avxzukkggrpb@kitenet.net>
On Thu, Feb 23, 2017 at 8:43 AM, Joey Hess <id@joeyh.name> wrote:
>
> IIRC someone has been working on parameterizing git's SHA1 assumptions
> so a repository could eventually use a more secure hash. How far has
> that gotten? There are still many "40" constants in git.git HEAD.
I don't think you'd necessarily want to change the size of the hash.
You can use a different hash and just use the same 160 bits from it.
> Since we now have collisions in valid PDF files, collisions in valid git
> commit and tree objects are probably able to be constructed.
I haven't seen the attack yet, but git doesn't actually just hash the
data, it does prepend a type/length field to it. That usually tends to
make collision attacks much harder, because you either have to make
the resulting size the same too, or you have to be able to also edit
the size field in the header.
pdf's don't have that issue, they have a fixed header and you can
fairly arbitrarily add silent data to the middle that just doesn't get
shown.
So pdf's make for a much better attack vector, exactly because they
are a fairly opaque data format. Git has opaque data in some places
(we hide things in commit objects intentionally, for example, but by
definition that opaque data is fairly secondary.
Put another way: I doubt the sky is falling for git as a source
control management tool. Do we want to migrate to another hash? Yes.
Is it "game over" for SHA1 like people want to say? Probably not.
I haven't seen the attack details, but I bet
(a) the fact that we have a separate size encoding makes it much
harder to do on git objects in the first place
(b) we can probably easily add some extra sanity checks to the opaque
data we do have, to make it much harder to do the hiding of random
data that these attacks pretty much always depend on.
Linus
^ permalink raw reply
* Re: SHA1 collisions found
From: David Lang @ 2017-02-23 17:12 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Joey Hess, Git Mailing List
In-Reply-To: <CAPc5daVZ79WWKSw76kxHgDra9a7fSR1AibZa_pvK9aUuuVawLQ@mail.gmail.com>
On Thu, 23 Feb 2017, Junio C Hamano wrote:
> On Thu, Feb 23, 2017 at 8:43 AM, Joey Hess <id@joeyh.name> wrote:
>>
>> Since we now have collisions in valid PDF files, collisions in valid git
>> commit and tree objects are probably able to be constructed.
>
> That may be true, but
> https://public-inbox.org/git/Pine.LNX.4.58.0504291221250.18901@ppc970.osdl.org/
>
it doesn't help that the Google page on this explicitly says that this shows
that it's possible to create two different git repos that have the same hash but
different contents.
https://shattered.it/
How is GIT affected?
GIT strongly relies on SHA-1 for the identification and integrity checking of
all file objects and commits. It is essentially possible to create two GIT
repositories with the same head commit hash and different contents, say a benign
source code and a backdoored one. An attacker could potentially selectively
serve either repository to targeted users. This will require attackers to
compute their own collision.
David Lang
^ permalink raw reply
* Re: SHA1 collisions found
From: Joey Hess @ 2017-02-23 17:35 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List
In-Reply-To: <CAPc5daVZ79WWKSw76kxHgDra9a7fSR1AibZa_pvK9aUuuVawLQ@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1602 bytes --]
Junio C Hamano wrote:
> On Thu, Feb 23, 2017 at 8:43 AM, Joey Hess <id@joeyh.name> wrote:
> >
> > Since we now have collisions in valid PDF files, collisions in valid git
> > commit and tree objects are probably able to be constructed.
>
> That may be true, but
> https://public-inbox.org/git/Pine.LNX.4.58.0504291221250.18901@ppc970.osdl.org/
That's about someone replacing an valid object in Linus's repository
with an invalid random blob they found that collides. This SHA1
break doesn't allow generating such a blob anyway. Linus is right,
that's an impractical attack.
Attacks using this SHA1 break will look something more like:
* I push a "bad" object to a repo on github I set up under a
pseudonym.
* I publish a "good" object in a commit and convince the maintainer to
merge it.
* I wait for the maintainer to push to github.
* I wait for github to deduplicate and hope they'll replace the good
object with the bad one I pre-uploaded, thus silently changing the
content of the good commit the maintainer reviewed and pushed.
* The bad object is pulled from github and deployed.
* The maintainer still has the good object. They may not notice the bad
object is out there for a long time.
Of course, it doesn't need to involve Github, and doesn't need to
rely on internal details of their deduplication[1];
that only let me publish the bad object under a psydonym.
--
see shy jo
[1] Which I'm only guessing about, but now that we have colliding
objects, we can upload them to different repos and see if such
dedupication happens.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: SHA1 collisions found
From: Linus Torvalds @ 2017-02-23 17:29 UTC (permalink / raw)
To: Joey Hess; +Cc: Git Mailing List
In-Reply-To: <CA+55aFxJGDpJXqpcoPnwvzcn_fB-zaggj=w7P2At-TOt4buOqw@mail.gmail.com>
On Thu, Feb 23, 2017 at 9:19 AM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> I don't think you'd necessarily want to change the size of the hash.
> You can use a different hash and just use the same 160 bits from it.
Side note: I do believe that in practice you should just change the
size of the hash too, I'm just saying that the size of the hash and
the choice of the hash algorithm are independent issues.
So you *could* just use something like SHA3-256, but then pick the
first 160 bits.
Realistically, changing the few hardcoded sizes internally in git is
likely the least problem in switching hashes.
So what you'd probably do is switch to a 256-bit hash, use that
internally and in the native git database, and then by default only
*show* the hash as a 40-character hex string (kind of like how we
already abbreviate things in many situations).
That way tools around git don't even see the change unless passed in
some special "--full-hash" argument (or "--abbrev=64" or whatever -
the default being that we abbreviate to 40).
Linus
^ permalink raw reply
* Re: SHA1 collisions found
From: Linus Torvalds @ 2017-02-23 17:52 UTC (permalink / raw)
To: Joey Hess; +Cc: Junio C Hamano, Git Mailing List
In-Reply-To: <20170223173547.qljypk7sdqi37oha@kitenet.net>
On Thu, Feb 23, 2017 at 9:35 AM, Joey Hess <id@joeyh.name> wrote:
>
> Attacks using this SHA1 break will look something more like:
We don't actually know what the break is, but it's likely that you
can't actually do what you think you can do:
> * I push a "bad" object to a repo on github I set up under a
> pseudonym.
> * I publish a "good" object in a commit and convince the maintainer to
> merge it.
It's not clear that the "good" object can be anything sane.
What you describe pretty much already requires a pre-image attack,
which the new attack is _not_.
The new attack doesn't have a controlled "good" case, you need two
different objects that both have "near-collision" blocks in the
middle. I don't know what the format of those near-collision blocks
are, but it's a big problem.
You blithely just say "I create a good object". It's not that simple.
If it was, this would be a pre-image attack.
So basically, the attack needs some kind of random binary garbage in
*both* objects in the middle.
That's why pdf's are the classic model for showing these attacks: it's
easy to insert garbage in the middle of a pdf that is invisible.
In a psf, you can just define a bitmap that you don't use for printing
- but you can use them to then make a decision about what to print -
making the printed version of the pdf look radically different in ways
that are not so much _directly_ about the invisible block itself.
Linus
^ permalink raw reply
* Re: SHA1 collisions found
From: Joey Hess @ 2017-02-23 18:10 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Git Mailing List
In-Reply-To: <CA+55aFxJGDpJXqpcoPnwvzcn_fB-zaggj=w7P2At-TOt4buOqw@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1699 bytes --]
Linus Torvalds wrote:
> I haven't seen the attack yet, but git doesn't actually just hash the
> data, it does prepend a type/length field to it. That usually tends to
> make collision attacks much harder, because you either have to make
> the resulting size the same too, or you have to be able to also edit
> the size field in the header.
I have some sha1 collisions (and other fun along these lines) in
https://github.com/joeyh/supercollider
That includes two files with the same SHA and size, which do get
different blobs thanks to the way git prepends the header to the
content.
joey@darkstar:~/tmp/supercollider>sha1sum bad.pdf good.pdf
d00bbe65d80f6d53d5c15da7c6b4f0a655c5a86a bad.pdf
d00bbe65d80f6d53d5c15da7c6b4f0a655c5a86a good.pdf
joey@darkstar:~/tmp/supercollider>git ls-tree HEAD
100644 blob ca44e9913faf08d625346205e228e2265dd12b65 bad.pdf
100644 blob 5f90b67523865ad5b1391cb4a1c010d541c816c1 good.pdf
While appending identical data to these colliding files does generate
other collisions, prepending data does not.
It would cost 6500 CPU years + 100 GPU years to generate valid colliding
git objects using the methods of the paper's authors. That might be cost
effective if it helped get a backdoor into eg, the kernel.
> (b) we can probably easily add some extra sanity checks to the opaque
> data we do have, to make it much harder to do the hiding of random
> data that these attacks pretty much always depend on.
For example, git fsck does warn about a commit message with opaque
data hidden after a NUL. But, git show/merge/pull give no indication
that something funky is going on when working with such commits.
--
see shy jo
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: SHA1 collisions found
From: David Lang @ 2017-02-23 17:52 UTC (permalink / raw)
To: Joey Hess; +Cc: Junio C Hamano, Git Mailing List
In-Reply-To: <20170223173547.qljypk7sdqi37oha@kitenet.net>
On Thu, 23 Feb 2017, Joey Hess wrote:
> Junio C Hamano wrote:
>> On Thu, Feb 23, 2017 at 8:43 AM, Joey Hess <id@joeyh.name> wrote:
>>>
>>> Since we now have collisions in valid PDF files, collisions in valid git
>>> commit and tree objects are probably able to be constructed.
>>
>> That may be true, but
>> https://public-inbox.org/git/Pine.LNX.4.58.0504291221250.18901@ppc970.osdl.org/
>
> That's about someone replacing an valid object in Linus's repository
> with an invalid random blob they found that collides. This SHA1
> break doesn't allow generating such a blob anyway. Linus is right,
> that's an impractical attack.
>
> Attacks using this SHA1 break will look something more like:
>
> * I push a "bad" object to a repo on github I set up under a
> pseudonym.
> * I publish a "good" object in a commit and convince the maintainer to
> merge it.
> * I wait for the maintainer to push to github.
> * I wait for github to deduplicate and hope they'll replace the good
> object with the bad one I pre-uploaded, thus silently changing the
> content of the good commit the maintainer reviewed and pushed.
> * The bad object is pulled from github and deployed.
> * The maintainer still has the good object. They may not notice the bad
> object is out there for a long time.
>
> Of course, it doesn't need to involve Github, and doesn't need to
> rely on internal details of their deduplication[1];
> that only let me publish the bad object under a psydonym.
read that e-mail again, it covers the case where a central server gets a blob
replaced in it.
tricking a maintainerinto accepting a file that contains huge amounts of binary
data in it is going to be a non-trivial task, and even after you trick them into
accepting one bad file, you then need to replace the file they accepted with a
new one (breaking into github or assuming that github is putting both files into
the same repo, both of which are fairly unlikely)
David Lang
^ permalink raw reply
* Re: SHA1 collisions found
From: Joey Hess @ 2017-02-23 18:21 UTC (permalink / raw)
To: Linus Torvalds, Git Mailing List
In-Reply-To: <CA+55aFzFEpi1crykZ33r9f7BsvLt_kiB-CHXOkuCAX=fd4BU-w@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 591 bytes --]
Linus Torvalds wrote:
> What you describe pretty much already requires a pre-image attack,
> which the new attack is _not_.
>
> It's not clear that the "good" object can be anything sane.
Generate a regular commit object; use the entire commit object + NUL as the
chosen prefix, and use the identical-prefix collision attack to generate
the colliding good/bad objects.
(The size in git's object header is a minor complication. Set the size
field to something sufficiently large, and then pad out the colliding
objects to that size once they're generated.)
--
see shy jo
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: SHA1 collisions found
From: Linus Torvalds @ 2017-02-23 18:29 UTC (permalink / raw)
To: Joey Hess; +Cc: Git Mailing List
In-Reply-To: <20170223181018.ns4vyosgzmuoyiva@kitenet.net>
On Thu, Feb 23, 2017 at 10:10 AM, Joey Hess <id@joeyh.name> wrote:
>
> It would cost 6500 CPU years + 100 GPU years to generate valid colliding
> git objects using the methods of the paper's authors. That might be cost
> effective if it helped get a backdoor into eg, the kernel.
I still think it also needs to be interesting enough data, not just
random noise that is then trivial to find with automated tools.
Because for the kernel, it's not just that an attacker needs to do the
CPU time. Yes, first he needs the technical resources to just do just
the attack and create the situation you described.
But then he *also* needs to build up the social capital to get the end
result pulled into the tree (ie if he depends on the hidden spaces, he
needs somebody to actually do a git pull, not just apply a patch).
.. and if we then have a tool that then finds the problem trivially
(ie "git fsck"), he's not only wasted all those technical resources,
he's also burned his identity.
>> (b) we can probably easily add some extra sanity checks to the opaque
>> data we do have, to make it much harder to do the hiding of random
>> data that these attacks pretty much always depend on.
>
> For example, git fsck does warn about a commit message with opaque
> data hidden after a NUL. But, git show/merge/pull give no indication
> that something funky is going on when working with such commits.
I do agree that we might want to do some of the fsck checks
particularly at fetch time. That's when doing checks is both relevant
and cheap.
So we could do the opaque data checks, but we could/should probably
also add the attack pattern ("disturbance vectors") checks.
And the thing is, adding those checks is really cheap, and basically
makes the whole attack vector pointless against git.
Because unlike some "signing a pdf" attack, git doesn't fundamentally
depend on the SHA1 as some kind of absolute security. If we have the
minimal machinery in git to just notice the attack, the attack
essentially goes away. Attackers can waste infinite amounts of CPU
time, and if it's cheap for us to notice, it completely disarms all
that attack work.
Again, I'm not arguing that people shouldn't work on extending git to
a new (and bigger) hash. I think that's a no-brainer, and we do want
to have a path to eventually move towards SHA3-256 or whatever.
But I'm very definitely arguing that the current attack doesn't
actually sound like it really even _matters_, because it should be so
easy to mitigate against.
Linus
^ permalink raw reply
* Re: SHA1 collisions found
From: Junio C Hamano @ 2017-02-23 18:38 UTC (permalink / raw)
To: Joey Hess; +Cc: Linus Torvalds, Git Mailing List
In-Reply-To: <20170223181018.ns4vyosgzmuoyiva@kitenet.net>
Joey Hess <id@joeyh.name> writes:
> For example, git fsck does warn about a commit message with opaque
> data hidden after a NUL. But, git show/merge/pull give no indication
> that something funky is going on when working with such commits.
Would
$ git config transfer.fsckobjects true
help?
^ permalink raw reply
* Re: SHA1 collisions found
From: Linus Torvalds @ 2017-02-23 18:40 UTC (permalink / raw)
To: Joey Hess; +Cc: Git Mailing List
In-Reply-To: <20170223182147.hbsyxsmyijgkqu75@kitenet.net>
On Thu, Feb 23, 2017 at 10:21 AM, Joey Hess <id@joeyh.name> wrote:
> Linus Torvalds wrote:
>> What you describe pretty much already requires a pre-image attack,
>> which the new attack is _not_.
>>
>> It's not clear that the "good" object can be anything sane.
>
> Generate a regular commit object; use the entire commit object + NUL as the
> chosen prefix, and use the identical-prefix collision attack to generate
> the colliding good/bad objects.
So I agree with you that we need to make git check for the opaque
data. I think I was the one who brought that whole argument up.
But even then, what you describe doesn't work. What you describe just
replaces the opaque data - that git doesn't actually *use*, and that
nobody sees - with another piece of opaque data.
You also need to make the non-opaque data of the bad object be
something that actually encodes valid git data with interesting hashes
in it (for the parent/tree/whatever pointers).
So you don't have just that "chosen prefix". You actually need to also
fill in some very specific piece of data *in* the attack parts itself.
And you need to do this in the exact same size (because that's part of
the prefix), etc etc.
So I think it's challenging.
... and then we can discover it trivially.
Ok, so "git fsck" right now takes a couple of minutes for me and I
don't actually run it very often (I used to run it religiously back in
the days), but afaik kernel.org actually runs it nightly. So it's
pretty much "trivially discoverable" - imagine spending thousands of
CPU-hours and lots of social capital to get an attack in, and then the
next night the kernel.org fsck complains about the strange commit you
added?
Linus
^ permalink raw reply
* Re: SHA1 collisions found
From: Jeff King @ 2017-02-23 18:46 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Joey Hess, Git Mailing List
In-Reply-To: <CA+55aFxckeEW1ePcebrgG4iN4Lp62A2vU6tA=xnSDC_BnKQiCQ@mail.gmail.com>
On Thu, Feb 23, 2017 at 10:40:48AM -0800, Linus Torvalds wrote:
> > Generate a regular commit object; use the entire commit object + NUL as the
> > chosen prefix, and use the identical-prefix collision attack to generate
> > the colliding good/bad objects.
>
> So I agree with you that we need to make git check for the opaque
> data. I think I was the one who brought that whole argument up.
We do already.
> But even then, what you describe doesn't work. What you describe just
> replaces the opaque data - that git doesn't actually *use*, and that
> nobody sees - with another piece of opaque data.
>
> You also need to make the non-opaque data of the bad object be
> something that actually encodes valid git data with interesting hashes
> in it (for the parent/tree/whatever pointers).
>
> So you don't have just that "chosen prefix". You actually need to also
> fill in some very specific piece of data *in* the attack parts itself.
> And you need to do this in the exact same size (because that's part of
> the prefix), etc etc.
It's not an identical prefix, but I think collision attacks generally
are along the lines of selecting two prefixes followed by garbage, and
then mutating the garbage on both sides. That would "work" in this case
(modulo the fact that git would complain about the NUL).
I haven't read the paper yet to see if that is the case here, though.
A related case is if you could stick a "cruft ...." header at the end of
the commit headers, and mutate its value (avoiding newlines). fsck
doesn't complain about that.
-Peff
^ permalink raw reply
* [PATCH] upload-pack: report "not our ref" to client
From: Jonathan Tan @ 2017-02-23 18:43 UTC (permalink / raw)
To: git; +Cc: Jonathan Tan, gitster
In-Reply-To: <xmqqo9xtajcu.fsf@gitster.mtv.corp.google.com>
Make upload-pack report "not our ref" errors to the client as an "ERR" line.
(If not, the client would be left waiting for a response when the server is
already dead.)
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
Thanks, here is the independent patch.
upload-pack.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/upload-pack.c b/upload-pack.c
index 7597ba340..ffb028d62 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -822,9 +822,13 @@ static void receive_needs(void)
use_include_tag = 1;
o = parse_object(sha1_buf);
- if (!o)
+ if (!o) {
+ packet_write_fmt(1,
+ "ERR upload-pack: not our ref %s",
+ sha1_to_hex(sha1_buf));
die("git upload-pack: not our ref %s",
sha1_to_hex(sha1_buf));
+ }
if (!(o->flags & WANTED)) {
o->flags |= WANTED;
if (!((allow_unadvertised_object_request & ALLOW_ANY_SHA1) == ALLOW_ANY_SHA1
--
2.11.0.483.g087da7b7c-goog
^ permalink raw reply related
* Re: [PATCH v2] send-email: only allow one address per body tag
From: Junio C Hamano @ 2017-02-23 18:53 UTC (permalink / raw)
To: Matthieu Moy; +Cc: Johan Hovold, Jeff King, Kevin Daudt, Larry Finger, git
In-Reply-To: <vpqo9xxkqqo.fsf@anie.imag.fr>
Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
> Johan Hovold <johan@kernel.org> writes:
>
>> --- a/git-send-email.perl
>> +++ b/git-send-email.perl
>> @@ -1563,7 +1563,7 @@ foreach my $t (@files) {
>> # Now parse the message body
>> while(<$fh>) {
>> $message .= $_;
>> - if (/^(Signed-off-by|Cc): (.*)$/i) {
>> + if (/^(Signed-off-by|Cc): ([^>]*>?)/i) {
>
> I think this is acceptable, but this doesn't work with trailers like
>
> Cc: "Some > Body" <Some.Body@example.com>
>
> A proper management of this kind of weird address should be doable by
> reusing the regexp parsing "..." in parse_mailbox:
>
> my $re_quote = qr/"(?:[^\"\\]|\\.)*"/;
>
> So the final regex would look like
>
> if (/^(Signed-off-by|Cc): (([^>]*|"(?:[^\"\\]|\\.)*")>?)/i) {
>
> I don't think that should block the patch inclusion, but it may be worth
> considering.
>
> Anyway, thanks for the patch!
Somehow this fell off the radar. So your reviewed-by: and then
we'll cook this in 'next' for a while?
Thanks.
^ permalink raw reply
* Re: SHA1 collisions found
From: Joey Hess @ 2017-02-23 18:31 UTC (permalink / raw)
To: Linus Torvalds, Git Mailing List
In-Reply-To: <20170223182147.hbsyxsmyijgkqu75@kitenet.net>
[-- Attachment #1: Type: text/plain, Size: 886 bytes --]
Joey Hess wrote:
> Linus Torvalds wrote:
> > What you describe pretty much already requires a pre-image attack,
> > which the new attack is _not_.
> >
> > It's not clear that the "good" object can be anything sane.
>
> Generate a regular commit object; use the entire commit object + NUL as the
> chosen prefix, and use the identical-prefix collision attack to generate
> the colliding good/bad objects.
>
> (The size in git's object header is a minor complication. Set the size
> field to something sufficiently large, and then pad out the colliding
> objects to that size once they're generated.)
Sorry! While that would work, it's a useless attack because the good and bad
commit objects still point to the same tree.
It would be interesting to have such colliding objects, to see what beaks,
but probably not worth $75k to generate them.
--
see shy jo
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH] http(s): automatically try NTLM authentication first
From: Junio C Hamano @ 2017-02-23 19:06 UTC (permalink / raw)
To: Johannes Schindelin
Cc: Jeff King, David Turner, git@vger.kernel.org,
sandals@crustytoothpaste.net, Eric Sunshine
In-Reply-To: <alpine.DEB.2.20.1702231806340.3767@virtualbox>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> On Wed, 22 Feb 2017, Jeff King wrote:
>> This patch drops the useless probe request:
> ...
>> but setting http.emptyauth adds back in the useless request. I think
>> that could be fixed by skipping the empty-auth thing when
>> http_auth_methods does not have CURLAUTH_NEGOTIATE in it (or perhaps
>> other methods need it to, so maybe skip it if _just_ BASIC is set).
>>
>> I suspect the patch above could probably be generalized as:
>>
>> /* cut out methods we know the server doesn't support */
>> http_auth_methods &= results.auth_avail;
>>
>> and let curl figure it out from there.
>
> Maybe this patch (or a variation thereof) would also be able to fix this
> problem with the patch:
>
> https://github.com/git-for-windows/git/issues/1034
>
> Short version: for certain servers (that do *not* advertise Negotiate),
> setting emptyauth to true will result in a failed fetch, without letting
> the user type in their credentials.
The issue described in that page looks rather serious.
I believe that a "variation" has become the first part of a
two-patch series that appear in the downthread from here. Perhaps
you can ask them to test it out (or even better if you have a setup
you can easily test against yourself)?
^ permalink raw reply
* Re: SHA1 collisions found
From: Jeff King @ 2017-02-23 18:42 UTC (permalink / raw)
To: Joey Hess; +Cc: Linus Torvalds, Git Mailing List
In-Reply-To: <20170223182147.hbsyxsmyijgkqu75@kitenet.net>
On Thu, Feb 23, 2017 at 02:21:47PM -0400, Joey Hess wrote:
> Linus Torvalds wrote:
> > What you describe pretty much already requires a pre-image attack,
> > which the new attack is _not_.
> >
> > It's not clear that the "good" object can be anything sane.
>
> Generate a regular commit object; use the entire commit object + NUL as the
> chosen prefix, and use the identical-prefix collision attack to generate
> the colliding good/bad objects.
FWIW, git-fsck complains about those (and transfer.fsck rejects them):
$ (git cat-file commit HEAD; printf '\0more stuff') |
git hash-object -w --stdin -t commit
ecb2e5165c184f9025cb4c49d8f75901f4830354
$ git fsck
warning in commit ecb2e5165c184f9025cb4c49d8f75901f4830354: nulInCommit: NUL byte in the commit object body
So as long as either your "good" or "evil" commit has binary junk in it,
you are likely to be noticed (not everybody turns on transfer.fsck, but
GitHub does).
-Peff
^ permalink raw reply
* Re: SHA1 collisions found
From: Linus Torvalds @ 2017-02-23 19:09 UTC (permalink / raw)
To: Jeff King; +Cc: Joey Hess, Git Mailing List
In-Reply-To: <20170223184637.xr74k42vc6y2pmse@sigill.intra.peff.net>
On Thu, Feb 23, 2017 at 10:46 AM, Jeff King <peff@peff.net> wrote:
>>
>> So I agree with you that we need to make git check for the opaque
>> data. I think I was the one who brought that whole argument up.
>
> We do already.
I'm aware of the fsck checks, but I have to admit I wasn't aware of
'transfer.fsckobjects'. I should turn that on myself.
Or maybe git should just turn it on by default? At least the
per-object fsck costs should be essentially free compared to the
network costs when you just apply them to the incoming objects.
I also do think that it would be good to check for the disturbance
vectors at receive time (and fsck). Not necessarily interesting during
normal operations.
And in particular, while the *kernel* doesn't generally have critical
opaque blobs, other projects do. Things like firmware images etc are
open to attack, and crazy people put ISO images in repositories etc.
So I don't think this discussion should focus exclusively on the git metadata.
It is likely much easier to replace a binary blob than it is to
replace a commit or tree (or a source file that has to go through a
compiler). And for many projects, that would be a bad thing.
> It's not an identical prefix, but I think collision attacks generally
> are along the lines of selecting two prefixes followed by garbage, and
> then mutating the garbage on both sides. That would "work" in this case
> (modulo the fact that git would complain about the NUL).
I think this particular attack depended on an actual identical prefix,
but I didn't go back to the paper and check.
But the attacks tend to very much depend on particular input bit
patterns that have very particular effects on the resulting
intermediate hash, and those bit patterns are specific to the hash and
known.
So a very powerful defense is to just look for those bit patterns in
the objects, and just warn about them. Those patterns don't tend to
exist in normal inputs anyway, but particularly if you just warn, it's
a heads-ups that "ok, something iffy is going on"
And as mentioned, a cheap "something iffy is going on" thing is
basically a death sentence to SCM attacks.
The whole _point_ of an SCM is that it isn't about a one-time event,
but about continuous history. That also fundamentally means that a
successful attack needs to work over time, and not be detectable.
In contrast, many other uses of hashes are "one-time" events. If you
use a hash to validate a single piece of data from a source that you
wouldn't otherwise trust, it's a one-time "all or nothing" trust
situation.
And the attack surface is very different for those "one-time" vs
"trust over time" cases. If you can get a bank to trust a session one
time, you can empty a bank account and live on a paradise island for
the rest of your life. It doesn't matter if it gets detected or not
after-the-fact.
But if you can fool a SCM one time, insert your code, and it gets
detected next week, you didn't actually do anything useful. You only
burned yourself.
See the difference? One-time vs having a continual interaction makes a
*fundamntal* difference in game theory.
Linus
^ permalink raw reply
* Re: [PATCH] http(s): automatically try NTLM authentication first
From: Junio C Hamano @ 2017-02-23 19:11 UTC (permalink / raw)
To: Jeff King
Cc: brian m. carlson, David Turner, git@vger.kernel.org,
Johannes Schindelin, Eric Sunshine
In-Reply-To: <20170222234246.wjp3567vesdusiaf@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Wed, Feb 22, 2017 at 11:34:19PM +0000, brian m. carlson wrote:
>
>> Browsers usually disable this feature by default, as it basically will
>> attempt to authenticate to any site that sends a 401. For Kerberos
>> against a malicious site, the user will either not have a valid ticket
>> for that domain, or the user's Kerberos server will refuse to provide a
>> ticket to pass to the server, so there's no security risk involved.
>>
>> I'm unclear how SPNEGO works with NTLM, so I can't speak for the
>> security of it. From what I understand of NTLM and from RFC 4559, it
>> consists of a shared secret. I'm unsure what security measures are in
>> place to not send that to an untrusted server.
>>
>> As far as Kerberos, this is a desirable feature to have enabled, with
>> little downside. I just don't know about the security of the NTLM part,
>> and I don't think we should take this patch unless we're sure we know
>> the consequences of it.
>
> Hmm. That would be a problem with my proposed patch 2 then, too, if only
> because it turns the feature on by default in more places.
>
> If it _is_ dangerous to turn on all the time, I'd think we should
> consider warning people in the http.emptyauth documentation.
I presume that we have finished discussing the security
ramification, and if I am not mistaken the conclusion was that it
could leak information if we turned on emptyAuth unconditionally
when talking to a wrong server, and that the documentation needs an
update to recommend those who use emptyAuth because they want to
talk to Negotiate servers to use the http.<site>.emptyAuth form,
limited to such servers, not a more generic http.emptyAuth, to avoid
information leakage?
If that is the case, let's take your 1/2 in the near-by thread
without 2/2 (auto-enable emptyAuth) for now, as Dscho seems to have
a case that may be helped by it.
^ permalink raw reply
* Re: SHA1 collisions found
From: Morten Welinder @ 2017-02-23 19:13 UTC (permalink / raw)
To: Joey Hess; +Cc: Linus Torvalds, Git Mailing List
In-Reply-To: <20170223183105.joxtpbut4wcqfbtu@kitenet.net>
The attack seems to generate two 64-bytes blocks, one quarter of which
is repeated data. (Table-1 in the paper.)
Assuming the result of that is evenly distributed and that bytes are
independent, we can estimate the chances that the result is NUL-free
as (255/256)^192 = 47% and the probability that the result is NUL and
newline free as (254/256)^192 = 22%. Clearly one should not rely of
NULs or newlines to save the day. On the other hand, the chances of
an ascii result is something like (95/256)^192 = 10^-83.
The actual collision in the paper has no newline, but it does have a NUL.
M.
On Thu, Feb 23, 2017 at 1:31 PM, Joey Hess <id@joeyh.name> wrote:
> Joey Hess wrote:
>> Linus Torvalds wrote:
>> > What you describe pretty much already requires a pre-image attack,
>> > which the new attack is _not_.
>> >
>> > It's not clear that the "good" object can be anything sane.
>>
>> Generate a regular commit object; use the entire commit object + NUL as the
>> chosen prefix, and use the identical-prefix collision attack to generate
>> the colliding good/bad objects.
>>
>> (The size in git's object header is a minor complication. Set the size
>> field to something sufficiently large, and then pad out the colliding
>> objects to that size once they're generated.)
>
> Sorry! While that would work, it's a useless attack because the good and bad
> commit objects still point to the same tree.
>
> It would be interesting to have such colliding objects, to see what beaks,
> but probably not worth $75k to generate them.
>
> --
> see shy jo
^ permalink raw reply
* usability bug: git-gui: keyboard shortcuts don't operate correctly on multi-file selections
From: peter fargas @ 2017-02-23 19:09 UTC (permalink / raw)
To: git
Ctrl+T/Ctrl+U add/remove only one file, not the whole selection - used
to work. Neither are access keys for menu underlined (Ease of access
center > underline keyboard shortcuts is on), so there is no way to
effectively work with keyb only.
git-gui verison 0.21 GITGUI
git version 2.11.1.windows.1
Tcl/Tk 8.6.6
64-bit installer
Windows 7 Professional
Originally posted at
https://github.com/git/git-scm.com/issues/939#issuecomment-276024341
peter.fargas @
informatik-handwerk.de
0176 / 458 67 358
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox