Re: On undoing a forced push

2015-06-10 Thread brian m. carlson
On Wed, Jun 10, 2015 at 09:43:34AM +0700, Duy Nguyen wrote:
 On Tue, Jun 9, 2015 at 10:00 PM, brian m. carlson
 sand...@crustytoothpaste.net wrote:
  You've increased this by 20, but you're adding 40 characters to the
  strcpy.  Are you sure that's enough?
 
  Also, you might consider writing this in terms of GIT_SHA1_HEXSZ, as it
  will be more obvious that this depends on that value.  If you don't now,
  I will later.
 
 It's a demonstration patch and I didn't pay much attention. I think
 converting this quickref to strbuf may be better though, when you
 convert this file to object_id.

Yeah, I didn't realize until after the fact that it was only supposed to
be a demo.  I agree that strbuf might be a better idea.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187


signature.asc
Description: Digital signature


Re: On undoing a forced push

2015-06-09 Thread Sitaram Chamarty
On 06/09/2015 05:42 PM, Duy Nguyen wrote:
 From a thread on Hacker News. It seems that if a user does not have
 access to the remote's reflog and accidentally forces a push to a ref,
 how does he recover it? In order to force push again to revert it
 back, he would need to know the remote's old SHA-1. Local reflog does
 not help because remote refs are not updated during a push.
 
 This patch prints the latest SHA-1 before the forced push in full. He
 then can do
 
 git push remote +old-sha1:ref
 
 He does not even need to have the objects that old-sha1 refers
 to. We could simply push an empty pack and the the remote will happily
 accept the force, assuming garbage collection has not happened. But
 that's another and a little more complex patch.

If I am not mistaken, we actively prevent people from downloading an
unreferenced SHA (such as would happen if you overwrote refs that
contained sensitive information like passwords).

Wouldn't allowing the kind of push you just described, require negating
that protection?

--
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: On undoing a forced push

2015-06-09 Thread Sitaram Chamarty
On 06/09/2015 07:55 PM, Jeff King wrote:
 On Tue, Jun 09, 2015 at 07:36:20PM +0530, Sitaram Chamarty wrote:
 
 This patch prints the latest SHA-1 before the forced push in full. He
 then can do

 git push remote +old-sha1:ref

 He does not even need to have the objects that old-sha1 refers
 to. We could simply push an empty pack and the the remote will happily
 accept the force, assuming garbage collection has not happened. But
 that's another and a little more complex patch.

 If I am not mistaken, we actively prevent people from downloading an
 unreferenced SHA (such as would happen if you overwrote refs that
 contained sensitive information like passwords).

 Wouldn't allowing the kind of push you just described, require negating
 that protection?
 
 No, this has always worked. If you have write access to a repository,
 you can fetch anything from it with this trick. Even if we blocked this,
 there are other ways to leak information. For instance, I can push up
 objects that are similar to the target object, claim to have the
 target object, and then hope git will make a delta between my similar
 object and the target. Iterate on the similar object and you can
 eventually figure out what is in the target object.

aah ok; I must have mis-remembered something.  Thanks!


--
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: On undoing a forced push

2015-06-09 Thread brian m. carlson
On Tue, Jun 09, 2015 at 07:12:21PM +0700, Duy Nguyen wrote:
 diff --git a/transport.c b/transport.c
 index f080e93..6bd6a64 100644
 --- a/transport.c
 +++ b/transport.c
 @@ -657,16 +657,17 @@ static void print_ok_ref_status(struct ref *ref, int 
 porcelain)
   [new branch]),
   ref, ref-peer_ref, NULL, porcelain);
   else {
 - char quickref[84];
 + char quickref[104];

You've increased this by 20, but you're adding 40 characters to the
strcpy.  Are you sure that's enough?

Also, you might consider writing this in terms of GIT_SHA1_HEXSZ, as it
will be more obvious that this depends on that value.  If you don't now,
I will later.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187


signature.asc
Description: Digital signature


Re: On undoing a forced push

2015-06-09 Thread Jeff King
On Tue, Jun 09, 2015 at 07:36:20PM +0530, Sitaram Chamarty wrote:

  This patch prints the latest SHA-1 before the forced push in full. He
  then can do
  
  git push remote +old-sha1:ref
  
  He does not even need to have the objects that old-sha1 refers
  to. We could simply push an empty pack and the the remote will happily
  accept the force, assuming garbage collection has not happened. But
  that's another and a little more complex patch.
 
 If I am not mistaken, we actively prevent people from downloading an
 unreferenced SHA (such as would happen if you overwrote refs that
 contained sensitive information like passwords).
 
 Wouldn't allowing the kind of push you just described, require negating
 that protection?

No, this has always worked. If you have write access to a repository,
you can fetch anything from it with this trick. Even if we blocked this,
there are other ways to leak information. For instance, I can push up
objects that are similar to the target object, claim to have the
target object, and then hope git will make a delta between my similar
object and the target. Iterate on the similar object and you can
eventually figure out what is in the target object.

This is one of the reasons we do not share objects between public and
private forks at GitHub.

-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: On undoing a forced push

2015-06-09 Thread Johannes Schindelin
Hi,

On 2015-06-09 16:06, Sitaram Chamarty wrote:
 On 06/09/2015 05:42 PM, Duy Nguyen wrote:
 From a thread on Hacker News. It seems that if a user does not have
 access to the remote's reflog and accidentally forces a push to a ref,
 how does he recover it? In order to force push again to revert it
 back, he would need to know the remote's old SHA-1. Local reflog does
 not help because remote refs are not updated during a push.

 This patch prints the latest SHA-1 before the forced push in full. He
 then can do

 git push remote +old-sha1:ref

 He does not even need to have the objects that old-sha1 refers
 to. We could simply push an empty pack and the the remote will happily
 accept the force, assuming garbage collection has not happened. But
 that's another and a little more complex patch.
 
 If I am not mistaken, we actively prevent people from downloading an
 unreferenced SHA (such as would happen if you overwrote refs that
 contained sensitive information like passwords).
 
 Wouldn't allowing the kind of push you just described, require negating
 that protection?

I believe that to be the case.

Sorry to chime in so late in the discussion, but I think that the 
`--force-with-lease` option is what you are looking for. It allows you to 
force-push *but only* if the forced push would overwrite the ref we expect, 
i.e. (simplified, but you get the idea) `git push --force-with-lease remote 
ref` will *only* succeed if the remote's ref agrees with the local 
`refs/remotes/remote/ref`.

If you use `--force-with-lease`, you simply cannot force-forget anything on the 
remote side that you cannot undo (because you have everything locally you need 
to undo it).

Ciao,
Johannes
--
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: On undoing a forced push

2015-06-09 Thread Stefan Beller
On Tue, Jun 9, 2015 at 9:29 AM, Johannes Schindelin
johannes.schinde...@gmx.de wrote:
 Hi,

 On 2015-06-09 16:06, Sitaram Chamarty wrote:
 On 06/09/2015 05:42 PM, Duy Nguyen wrote:
 From a thread on Hacker News. It seems that if a user does not have
 access to the remote's reflog and accidentally forces a push to a ref,
 how does he recover it? In order to force push again to revert it
 back, he would need to know the remote's old SHA-1. Local reflog does
 not help because remote refs are not updated during a push.

 This patch prints the latest SHA-1 before the forced push in full. He
 then can do

 git push remote +old-sha1:ref

 He does not even need to have the objects that old-sha1 refers
 to. We could simply push an empty pack and the the remote will happily
 accept the force, assuming garbage collection has not happened. But
 that's another and a little more complex patch.

 If I am not mistaken, we actively prevent people from downloading an
 unreferenced SHA (such as would happen if you overwrote refs that
 contained sensitive information like passwords).

 Wouldn't allowing the kind of push you just described, require negating
 that protection?

 I believe that to be the case.

 Sorry to chime in so late in the discussion, but I think that the 
 `--force-with-lease` option is what you are looking for. It allows you to 
 force-push *but only* if the forced push would overwrite the ref we expect, 
 i.e. (simplified, but you get the idea) `git push --force-with-lease remote 
 ref` will *only* succeed if the remote's ref agrees with the local 
 `refs/remotes/remote/ref`.

Yeah that was my first thought as well. It's unfortunate that
--force-with-lease is not as well known though (it wasn't there first,
so many people picked it up and it's good enough to not pickup other
--force-with-foo options).

Maybe we should add the option receive.denyNonFastForwards =
onlyWithLease instead?

Thanks,
Stefan


 If you use `--force-with-lease`, you simply cannot force-forget anything on 
 the remote side that you cannot undo (because you have everything locally you 
 need to undo it).

 Ciao,
 Johannes
 --
 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
--
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: On undoing a forced push

2015-06-09 Thread Duy Nguyen
On Tue, Jun 9, 2015 at 11:29 PM, Johannes Schindelin
johannes.schinde...@gmx.de wrote:
 Sorry to chime in so late in the discussion, but I think that the 
 `--force-with-lease` option is what you are looking for. It allows you to 
 force-push *but only* if the forced push would overwrite the ref we expect, 
 i.e. (simplified, but you get the idea) `git push --force-with-lease remote 
 ref` will *only* succeed if the remote's ref agrees with the local 
 `refs/remotes/remote/ref`.

 If you use `--force-with-lease`, you simply cannot force-forget anything on 
 the remote side that you cannot undo (because you have everything locally you 
 need to undo it).

Yeah I recall Junio did something about pushes.. I was about to
suggest that we promote force-with-lease to default --force and
current --force becomes --force --force. But there's this from commit
2233ad4 (Merge branch 'jc/push-cas' - 2013-09-09) that makes me
hesitate

The logic to choose the default implemented here is fragile
(e.g. git fetch after seeing a failure will update the
remote-tracking branch and will make the next push pass,
defeating the safety pretty easily).  It is suitable only for the
simplest workflows, and it may hurt users more than it helps them.

Either way I still want to provide an escape hatch for --force as it's
good to reduce the number of unrecoverable operations down.
-- 
Duy
--
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: On undoing a forced push

2015-06-09 Thread Duy Nguyen
On Tue, Jun 9, 2015 at 10:00 PM, brian m. carlson
sand...@crustytoothpaste.net wrote:
 On Tue, Jun 09, 2015 at 07:12:21PM +0700, Duy Nguyen wrote:
 diff --git a/transport.c b/transport.c
 index f080e93..6bd6a64 100644
 --- a/transport.c
 +++ b/transport.c
 @@ -657,16 +657,17 @@ static void print_ok_ref_status(struct ref *ref, int 
 porcelain)
   [new branch]),
   ref, ref-peer_ref, NULL, porcelain);
   else {
 - char quickref[84];
 + char quickref[104];

 You've increased this by 20, but you're adding 40 characters to the
 strcpy.  Are you sure that's enough?

 Also, you might consider writing this in terms of GIT_SHA1_HEXSZ, as it
 will be more obvious that this depends on that value.  If you don't now,
 I will later.

It's a demonstration patch and I didn't pay much attention. I think
converting this quickref to strbuf may be better though, when you
convert this file to object_id.
-- 
Duy
--
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: On undoing a forced push

2015-06-09 Thread Matthieu Moy
Duy Nguyen pclo...@gmail.com writes:

 From a thread on Hacker News. It seems that if a user does not have
 access to the remote's reflog and accidentally forces a push to a ref,
 how does he recover it? In order to force push again to revert it
 back, he would need to know the remote's old SHA-1. Local reflog does
 not help because remote refs are not updated during a push.

More precisely, the remote-tracking ref is updated, and so is its
reflog, but the reflog entry usually does not help, because it documents
the old and new sha1 of the remote-tracking ref, not of the remote ref
itself. Typically, if a coworker pushed some code that I did not pull,
and I force-push to the same branch, my reflog won't have the sha1 of my
coworker's code.

 This patch prints the latest SHA-1 before the forced push in full.

Sounds like a good idea to me.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
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