{"thread":{"id":"44396","subject":"Fetch/push lets a malicious server steal the targets of \"have\" lines","startedAt":"2016-10-28T21:40:11Z","lastAt":"2016-11-14T19:47:17Z","messageCount":24,"participants":["Matt McCutchen","Junio C Hamano","Jeff King","Jon Loeliger"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"305132","messageId":"1477690790.2904.22.camel@mattmccutchen.net","threadId":"44396","inReplyTo":null,"subject":"Fetch/push lets a malicious server steal the targets of \"have\" lines","fromName":"Matt McCutchen","fromEmail":"matt@mattmccutchen.net","sentAt":"2016-10-28T21:39:50Z","receivedAt":"2016-10-28T21:40:11Z","isPatch":false,"sender":{"key":"matt@mattmccutchen.net","avatar":"https://avatars.githubusercontent.com/u/8885753?v=4"},"body":"I was studying the fetch protocol and I realized that in a scenario in\nwhich a client regularly fetches a set of refs from a server and pushes\nthem back without careful scrutiny, the server can steal the targets of\nunrelated refs from the client repository by fabricating its own refs\nto the \"have\" objects specified by the client during the fetch.  This\nis the reverse of attack #1 described in the \"SECURITY\" section of the\ngitnamespaces(7) man page, with the addition that the server doesn't\nhave to know the object IDs in advance.  Is this supposed to be well-\nknown?  I've been using git since 2006 and it was a surprise to me.\n\nHopefully it isn't very common for a user to fetch and push with a\nserver they don't trust to have all the data in their repository.  I\ndon't think I have any such cases myself; I have unfinished work that\nisn't meant for scrutiny by others, but nothing really damaging if it\nwere released to the server.  This attack presents no new risks if a\nuser already runs code fetched from the server in such a way that it\ncan read the repository.  But there might be some users who just review\nembargoed security fixes from multiple sources (or something like that)\nwithout running code themselves, and their security expectations might\nbe violated.\n\nIf my analysis is correct, I'd argue for documenting the issue in a\n\"SECURITY\" section in the git-fetch man page.  Shall I submit a patch?\n\nThanks for your attention.\n\nMatt\n"},{"id":"305134","messageId":"xmqqmvhoxhfp.fsf@gitster.mtv.corp.google.com","threadId":"44396","inReplyTo":"1477690790.2904.22.camel@mattmccutchen.net","subject":"Re: Fetch/push lets a malicious server steal the targets of \"have\" lines","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-10-28T22:00:26Z","receivedAt":"2016-10-28T22:00:34Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matt McCutchen writes:\n\n> I was studying the fetch protocol and I realized that in a scenario in\n> which a client regularly fetches a set of refs from a server and pushes\n> them back without careful scrutiny, the server can steal the targets of\n> unrelated refs from the client repository by fabricating its own refs\n> to the \"have\" objects specified by the client during the fetch.\n\nLet me see if I understood your scenario correctly.\n\nSuppose we start from this history where 'O' are common, your victim\nhas a 'Y' branch with two commits that are private to it, as well as\na 'X' branch on which it has X1 that it previously obtained from the\nserver. On the other hand, the server does not know about Y1 or Y2,\nand it added one commit X2 to the branch 'x' the victim is\nfollowing:\n\n victim server\n\n Y1---Y2 \n / \n ---O---O---X1 ---O---O---X1---X2\n\nThen when victim wants to fetch 'x' from the server, it would say\n\n have X1, have Y2, have Y1, have O\n\nand gets told to shut up by the server who heard enough. The\nhistories on these two parties will then become like this:\n\n\n victim server\n\n Y1---Y2 \n / \n ---O---O---X1---X2 ---O---O---X1---X2\n\nVictim wishes to keep Y1 and Y2 private, but pushes some other\nbranch (perhaps builds X3 on top of X2 and pushes 'x'). On push\nprotocol, the server would lie to the victim that it has Y2 without\nknowing what they are.\n\nIs that how your attack scenario goes?\n"},{"id":"305138","messageId":"1477692961.2904.36.camel@mattmccutchen.net","threadId":"44396","inReplyTo":"xmqqmvhoxhfp.fsf@gitster.mtv.corp.google.com","subject":"Re: Fetch/push lets a malicious server steal the targets of \"have\" lines","fromName":"Matt McCutchen","fromEmail":"matt@mattmccutchen.net","sentAt":"2016-10-28T22:16:01Z","receivedAt":"2016-10-28T22:16:10Z","isPatch":false,"sender":{"key":"matt@mattmccutchen.net","avatar":"https://avatars.githubusercontent.com/u/8885753?v=4"},"body":"On Fri, 2016-10-28 at 15:00 -0700, Junio C Hamano wrote:\n> Let me see if I understood your scenario correctly.\n> \n> Suppose we start from this history where 'O' are common, your victim\n> has a 'Y' branch with two commits that are private to it, as well as\n> a 'X' branch on which it has X1 that it previously obtained from the\n> server.  On the other hand, the server does not know about Y1 or Y2,\n> and it added one commit X2 to the branch 'x' the victim is\n> following:\n> \n>            victim                server\n> \n>              Y1---Y2               \n>             /                      \n>     ---O---O---X1           ---O---O---X1---X2\n> \n> Then when victim wants to fetch 'x' from the server, it would say\n> \n>     have X1, have Y2, have Y1, have O\n> \n> and gets told to shut up by the server who heard enough.  The\n> histories on these two parties will then become like this:\n> \n> \n>            victim                server\n> \n>              Y1---Y2               \n>             /                      \n>     ---O---O---X1---X2      ---O---O---X1---X2\n\nThen the server generates a commit X3 that lists Y2 as a parent, even\nthough it doesn't have Y2, and advances 'x' to X3.  The victim fetches\n'x':\n\n           victim                  server\n\n             Y1---Y2----                      (Y2)\n            /           \\                         \\ \n    ---O---O---X1---X2---X3   ---O---O---X1---X2---X3\n\nThen the server rolls back 'x' to X2:\n\n           victim                  server\n\n             Y1---Y2----\n            /           \\\n    ---O---O---X1---X2---X3   ---O---O---X1---X2\n\nAnd the victim pushes:\n\n           victim                  server\n\n             Y1---Y2----               Y1---Y2----\n            /           \\             /           \\\n    ---O---O---X1---X2---X3   ---O---O---X1---X2---X3\n\nNow the server has the content of Y2.\n\nIf the victim is fetching and pulling a whole \"directory\" of refs, e.g:\n\nfetch: refs/heads/*:refs/remotes/server1/*\npush: refs/heads/for-server1/*:refs/heads/*\n\nthen instead of generating a merge commit, the server can just generate\nanother ref 'xx' pointing to Y2, assuming it can entice the victim to\nset up a corresponding local branch refs/heads/for-server1/xx and push\nit back.  Or if the victim is for some reason just mirroring back and\nforth:\n\nfetch: refs/heads/*:refs/heads/for-server1/*\npush: refs/heads/for-\nserver1/*:refs/heads/*\n\nthen it doesn't have to set up a local branch as separate step.\n\nMatt\n"},{"id":"305147","messageId":"xmqq7f8sx8lg.fsf@gitster.mtv.corp.google.com","threadId":"44396","inReplyTo":"1477692961.2904.36.camel@mattmccutchen.net","subject":"Re: Fetch/push lets a malicious server steal the targets of \"have\" lines","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-10-29T01:11:23Z","receivedAt":"2016-10-29T01:11:32Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matt McCutchen writes:\n\n> Then the server generates a commit X3 that lists Y2 as a parent, even\n> though it doesn't have Y2, and advances 'x' to X3. The victim fetches\n> 'x':\n>\n> victim server\n>\n> Y1---Y2---- (Y2)\n> / \\ \\ \n> ---O---O---X1---X2---X3 ---O---O---X1---X2---X3\n>\n> Then the server rolls back 'x' to X2:\n>\n> victim server\n>\n> Y1---Y2----\n> / \\\n> ---O---O---X1---X2---X3 ---O---O---X1---X2\n\nAh, I see. My immediate reaction is that you can do worse things in\nthe reverse direction compared to this, but your scenario does sound\nbad already.\n\n"},{"id":"305151","messageId":"1477712029.2904.64.camel@mattmccutchen.net","threadId":"44396","inReplyTo":"xmqq7f8sx8lg.fsf@gitster.mtv.corp.google.com","subject":"Re: Fetch/push lets a malicious server steal the targets of \"have\" lines","fromName":"Matt McCutchen","fromEmail":"matt@mattmccutchen.net","sentAt":"2016-10-29T03:33:49Z","receivedAt":"2016-10-29T03:34:55Z","isPatch":false,"sender":{"key":"matt@mattmccutchen.net","avatar":"https://avatars.githubusercontent.com/u/8885753?v=4"},"body":"On Fri, 2016-10-28 at 18:11 -0700, Junio C Hamano wrote:\n> Ah, I see.  My immediate reaction is that you can do worse things in\n> the reverse direction compared to this, but your scenario does sound\n> bad already.\n\nAre you saying that clients connecting to untrusted servers already\nface worse risks that people should know about, so there is no point in\ndocumenting this one?  I guess I don't know about the other risks aside\nfrom accepting a corrupt object, which should be preventable by\nenabling fetch.fsckObjects.  It seems we need either a statement that\nconnecting to untrusted servers is officially unsupported or a\ndescription of the specific risks.\n\nMatt\n"},{"id":"305165","messageId":"20161029133959.kpkohjkku3jgwjql@sigill.intra.peff.net","threadId":"44396","inReplyTo":"1477712029.2904.64.camel@mattmccutchen.net","subject":"Re: Fetch/push lets a malicious server steal the targets of \"have\" lines","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-10-29T13:39:59Z","receivedAt":"2016-10-29T13:40:07Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Oct 28, 2016 at 11:33:49PM -0400, Matt McCutchen wrote:\n\n> On Fri, 2016-10-28 at 18:11 -0700, Junio C Hamano wrote:\n> > Ah, I see.  My immediate reaction is that you can do worse things in\n> > the reverse direction compared to this, but your scenario does sound\n> > bad already.\n> \n> Are you saying that clients connecting to untrusted servers already\n> face worse risks that people should know about, so there is no point in\n> documenting this one?  I guess I don't know about the other risks aside\n> from accepting a corrupt object, which should be preventable by\n> enabling fetch.fsckObjects.  It seems we need either a statement that\n> connecting to untrusted servers is officially unsupported or a\n> description of the specific risks.\n\nI'm not sure I understand how connecting to a remote server to fetch is\na big problem. The server may learn about the existence of particular\nsha1s in your repository, but cannot get their content.\n\nIt's the subsequent push that is a problem.\n\nIn the scenarios you've described, I'm mostly inclined to say that the\nproblem is not git or the protocol itself, but rather lax refspecs.\nYou mentioned earlier:\n\n the server can just generate another ref 'xx' pointing to Y2, assuming\n it can entice the victim to set up a corresponding local branch\n refs/heads/for-server1/xx and push it back. Or if the victim is for\n some reason just mirroring back and forth:\n\nThis sounds a lot like \"I told git to push a bunch of things without\nchecking if they were really secret, and it turned out to push some\nsecret things\". IOW I think the problem is not that the server may lie\nabout what it has, but that the user was not careful about what they\npushed. I dunno. I do not mind making a note in the documentation\nexplaining the implications of a server lying, but the scenarios seem\npretty contrived to me.\n\nA much more interesting one, IMHO, is a server whose receive-pack lies\nabout which objects it has (possibly ones it found out about earlier via\nfetch), which provokes the client to generate deltas against objects the\nserver doesn't have (and thereby leaking information about the base\nobjects).\n\nThat is a problem no matter how careful your refspecs are. I suspect it\nwould be a hard attack to pull off in practice, just because it's going\nto depend heavily on the content of the specific objects, what kinds of\ndeltas you can convince the other side to generate, etc. That might\nmerit a mention in the git-push documentation.\n\n-Peff\n"},{"id":"305166","messageId":"1477757268.1524.20.camel@mattmccutchen.net","threadId":"44396","inReplyTo":"CAPc5daVOxmowdiTU3ScFv6c_BRVEJ+G92gx_AmmKnR-WxUKv-Q@mail.gmail.com","subject":"Re: Fetch/push lets a malicious server steal the targets of \"have\" lines","fromName":"Matt McCutchen","fromEmail":"matt@mattmccutchen.net","sentAt":"2016-10-29T16:07:48Z","receivedAt":"2016-10-29T16:07:59Z","isPatch":false,"sender":{"key":"matt@mattmccutchen.net","avatar":"https://avatars.githubusercontent.com/u/8885753?v=4"},"body":"On Fri, 2016-10-28 at 22:31 -0700, Junio C Hamano wrote:\n> Not sending to the list, where mails from Gmail/phone is known to get\n> rejected.\n\n[I guess I can go ahead and quote this to the list.]\n\n> No. I'm saying that the scenario you gave is bad and people should be\n> taught not to connect to untrustworthy sites.\n\nTo clarify, are you saying:\n\n(1) don't connect to an untrusted server ever (e.g., we don't promise\nthat the server can't execute arbitrary code on the client), or\n\n(2) don't connect to an untrusted server if the client repository has\ndata that needs to be kept secret from the server?\n\nThe fetch/push attack relates only to #2.  If #1, what are the other\nrisks you are thinking of?\n\nMatt\n"},{"id":"305167","messageId":"1477757311.1524.21.camel@mattmccutchen.net","threadId":"44396","inReplyTo":"20161029133959.kpkohjkku3jgwjql@sigill.intra.peff.net","subject":"Re: Fetch/push lets a malicious server steal the targets of \"have\" lines","fromName":"Matt McCutchen","fromEmail":"matt@mattmccutchen.net","sentAt":"2016-10-29T16:08:31Z","receivedAt":"2016-10-29T16:08:40Z","isPatch":false,"sender":{"key":"matt@mattmccutchen.net","avatar":"https://avatars.githubusercontent.com/u/8885753?v=4"},"body":"On Sat, 2016-10-29 at 09:39 -0400, Jeff King wrote:\n> I'm not sure I understand how connecting to a remote server to fetch is\n> a big problem. The server may learn about the existence of particular\n> sha1s in your repository, but cannot get their content.\n> \n> It's the subsequent push that is a problem.\n> \n> In the scenarios you've described, I'm mostly inclined to say that the\n> problem is not git or the protocol itself, but rather lax refspecs.\n> You mentioned earlier:\n> \n>   the server can just generate another ref 'xx' pointing to Y2, assuming\n>   it can entice the victim to set up a corresponding local branch\n>   refs/heads/for-server1/xx and push it back.  Or if the victim is for\n>   some reason just mirroring back and forth:\n> \n> This sounds a lot like \"I told git to push a bunch of things without\n> checking if they were really secret, and it turned out to push some\n> secret things\". IOW I think the problem is not that the server may lie\n> about what it has, but that the user was not careful about what they\n> pushed. I dunno. I do not mind making a note in the documentation\n> explaining the implications of a server lying, but the scenarios seem\n> pretty contrived to me.\n\nLet's focus on the first scenario.  There the user is just pulling and\npushing a master branch.  Are you saying that each time the user pulls,\nthey need to look over all the commits they pulled before pushing them\nback?  I think that's unrealistic, for example, on a busy project with\ncentralized code review or if the user is publishing a project-specific \nmodified version of an upstream library.  The natural user expectation\nis that anything pulled from a public repository is public.\n\nBut let's see what Junio says in the other subthread.\n\n> A much more interesting one, IMHO, is a server whose receive-pack lies\n> about which objects it has (possibly ones it found out about earlier via\n> fetch), which provokes the client to generate deltas against objects the\n> server doesn't have (and thereby leaking information about the base\n> objects).\n> \n> That is a problem no matter how careful your refspecs are. I suspect it\n> would be a hard attack to pull off in practice, just because it's going\n> to depend heavily on the content of the specific objects, what kinds of\n> deltas you can convince the other side to generate, etc. That might\n> merit a mention in the git-push documentation.\n\nSure, if I end up doing a patch, I'll include this.\n\nMatt\n"},{"id":"305169","messageId":"E1c0XaZ-0007Ab-QI@mylo.jdl.com","threadId":"44396","inReplyTo":"xmqq7f8sx8lg.fsf@gitster.mtv.corp.google.com","subject":"Re: Fetch/push lets a malicious server steal the targets of \"have\" lines","fromName":"Jon Loeliger","fromEmail":"jdl@jdl.com","sentAt":"2016-10-29T17:38:39Z","receivedAt":"2016-10-29T18:02:19Z","isPatch":false,"sender":{"key":"jdl@jdl.com","avatar":"https://gravatar.com/avatar/75ce9a10b151acd2c28ec4ab2136dba7b2ff1634530bd04b155981a749d08a64?d=mp&s=160"},"body":"So, like, Junio C Hamano said:\n> Matt McCutchen writes:\n> \n> > Then the server generates a commit X3 that lists Y2 as a parent, even\n> > though it doesn't have Y2, and advances 'x' to X3. The victim fetches\n> > 'x':\n> >\n> > victim server\n> >\n> > Y1---Y2---- (Y2)\n> > / \\ \\ \n> > ---O---O---X1---X2---X3 ---O---O---X1---X2---X3\n> >\n> > Then the server rolls back 'x' to X2:\n> >\n> > victim server\n> >\n> > Y1---Y2----\n> > / \\\n> > ---O---O---X1---X2---X3 ---O---O---X1---X2\n> \n> Ah, I see. My immediate reaction is that you can do worse things in\n> the reverse direction compared to this, but your scenario does sound\n> bad already.\n\nIs there an existing protocol provision, or an extension to\nthe protocol that would allow a distrustful client to say to\nthe server, \"Really, you have Y2? Prove it.\" And expect the\nserver to respond with a SHA1 sequence back to a common SHA\n(in this case the left-most O). If so, a user could designate\nsome branch (Y) as \"sensitive\". Or, a whole repo could be\nso designated and the client then effectivey treats the server\nas a semi-hostile witness.\n\nDunno.\n\njdl\n\n"},{"id":"305170","messageId":"20161029191023.ztrfe76u4gi4l3ci@sigill.intra.peff.net","threadId":"44396","inReplyTo":"1477757311.1524.21.camel@mattmccutchen.net","subject":"Re: Fetch/push lets a malicious server steal the targets of \"have\" lines","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-10-29T19:10:23Z","receivedAt":"2016-10-29T19:10:32Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Oct 29, 2016 at 12:08:31PM -0400, Matt McCutchen wrote:\n\n> Let's focus on the first scenario.  There the user is just pulling and\n> pushing a master branch.  Are you saying that each time the user pulls,\n> they need to look over all the commits they pulled before pushing them\n> back?  I think that's unrealistic, for example, on a busy project with\n> centralized code review or if the user is publishing a project-specific \n> modified version of an upstream library.  The natural user expectation\n> is that anything pulled from a public repository is public.\n\nNo, I'm saying if you are running \"git push foo master\", then you should\nexpect the contents of \"master\" to go to \"foo\". That _could_ have\nsecurity implications if you come up with a sequence of events where\nsecret things made it to \"master\". But it seems to me that \"foo\npreviously lied to you about what it has\" is not the weak link in that\nchain. It is not thinking about what secret things are hitting the\nmaster that you are pushing, no matter how they got there.\n\nI agree there is a potential workflow (that you have laid out) where\nsuch lying can cause an innocent-looking sequence of events to disclose\nthe secret commits. And again, I don't mind a note in the documentation\nmentioning that. I just have trouble believing it's a common one in\npractice.\n\nThe reason I brought up the delta thing, even though it's a much harder\nattack to execute, is that it comes up in much more common workflows,\nlike simply fetching from a private security-sensitive repo into your\n\"main\" public repo (which is an example you brought up, and something I\nknow that I have personally done in the past for git.git).\n\n-Peff\n"},{"id":"305179","messageId":"xmqqy416uvan.fsf@gitster.mtv.corp.google.com","threadId":"44396","inReplyTo":"20161029191023.ztrfe76u4gi4l3ci@sigill.intra.peff.net","subject":"Re: Fetch/push lets a malicious server steal the targets of \"have\" lines","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-10-30T07:53:52Z","receivedAt":"2016-10-30T07:54:01Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> ... It is not thinking about what secret things are hitting the\n> master that you are pushing, no matter how they got there.\n>\n> I agree there is a potential workflow (that you have laid out) where\n> such lying can cause an innocent-looking sequence of events to disclose\n> the secret commits. And again, I don't mind a note in the documentation\n> mentioning that. I just have trouble believing it's a common one in\n> practice.\n\nI'd say I agree with the above. I am not sure how easy people\nemploying common workflows can be tricked into the scenario Matt\npresented, either, but I do not think it would hurt to warn people\nthat they need to be careful not to pull from or push to an\nuntrustworthy place or push things you are not sure that are clean.\n\n> The reason I brought up the delta thing, even though it's a much harder\n> attack to execute, is that it comes up in much more common workflows,\n> like simply fetching from a private security-sensitive repo into your\n> \"main\" public repo (which is an example you brought up, and something I\n> know that I have personally done in the past for git.git).\n\nYup.\n"},{"id":"305180","messageId":"xmqqtwbuuuuy.fsf@gitster.mtv.corp.google.com","threadId":"44396","inReplyTo":"1477757268.1524.20.camel@mattmccutchen.net","subject":"Re: Fetch/push lets a malicious server steal the targets of \"have\" lines","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-10-30T08:03:17Z","receivedAt":"2016-10-30T08:03:28Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matt McCutchen writes:\n\n> On Fri, 2016-10-28 at 22:31 -0700, Junio C Hamano wrote:\n>> Not sending to the list, where mails from Gmail/phone is known to get\n>> rejected.\n>\n> [I guess I can go ahead and quote this to the list.]\n>\n>> No. I'm saying that the scenario you gave is bad and people should be\n>> taught not to connect to untrustworthy sites.\n>\n> To clarify, are you saying:\n>\n> (1) don't connect to an untrusted server ever (e.g., we don't promise\n> that the server can't execute arbitrary code on the client), or\n>\n> (2) don't connect to an untrusted server if the client repository has\n> data that needs to be kept secret from the server?\n\nYou sneaked \"arbitrary code execution\" into the discussion but I do\nnot know where it came from. In any case, \"don't pull from or push\nto untrustworthy place\" would be a common sense advice that would\nmake sense in any scenario ;-)\n\nJust for future reference, when you have ideas/issues that might\nhave possible security ramifications, I'd prefer to see it first\ndiscussed on a private list we created for that exact purpose, until\nwe can assess the impact (if any). Right now MaintNotes says this:\n\n If you think you found a security-sensitive issue and want to disclose\n it to us without announcing it to wider public, please contact us at\n our security mailing list . This is\n a closed list that is limited to people who need to know early about\n vulnerabilities, including:\n\n - people triaging and fixing reported vulnerabilities\n - people operating major git hosting sites with many users\n - people packaging and distributing git to large numbers of people\n\n where these issues are discussed without risk of the information\n leaking out before we're ready to make public announcements.\n\nWe may want to tweak the description from \"disclose it to us\" to\n\"have a discussion on it with us\" (the former makes it sound as if\nthe topic has to be a definite problem, the latter can include an\nidle speculation that may not be realistic attack vector).\n\n"},{"id":"305181","messageId":"xmqqpomiuu8q.fsf@gitster.mtv.corp.google.com","threadId":"44396","inReplyTo":"E1c0XaZ-0007Ab-QI@mylo.jdl.com","subject":"Re: Fetch/push lets a malicious server steal the targets of \"have\" lines","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-10-30T08:16:37Z","receivedAt":"2016-10-30T08:16:46Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jon Loeliger writes:\n\n> Is there an existing protocol provision, or an extension to\n> the protocol that would allow a distrustful client to say to\n> the server, \"Really, you have Y2? Prove it.\"\n\nThere is not, but I do not think it would be an effective solution.\n\nThe issue is not the lack of protocol support, but how to determine\nthat the other side needs such a proof for Y2 but not for other\ncommits. How does your side know what makes Y2 special and why does\nyout side think they should not have Y2?\n\nOnce you know how to determine Y2 is special, that knowledge can be\nused to abort the \"push\" before even starting. When you are pushing\nback the 'master' and that 'master' reaches Y2, which must be kept\nsecret, you shouldn't be pushing that 'master' to them, whether they\nclaim to have Y2 or not.\n\nI think the above is just a different way to say what Peff just said\n(paraphrasing, do not push what is secret).\n"},{"id":"305869","messageId":"1479001205.3471.1.camel@mattmccutchen.net","threadId":"44396","inReplyTo":"xmqqy416uvan.fsf@gitster.mtv.corp.google.com","subject":"[PATCH] fetch/push: document that private data can be leaked","fromName":"Matt McCutchen","fromEmail":"matt@mattmccutchen.net","sentAt":"2016-11-13T01:25:55Z","receivedAt":"2016-11-13T01:40:14Z","isPatch":true,"sender":{"key":"matt@mattmccutchen.net","avatar":"https://avatars.githubusercontent.com/u/8885753?v=4"},"body":"A malicious server may be able to use the fetch and push protocols to\nsteal data from a user's repository that the user did not intend to\nshare, via attacks similar to those described in the gitnamespaces(7)\nman page. Mention this in the git-fetch(1), git-pull(1), and git-push(1)\nman pages and recommend using separate repositories for private data and\ninteraction with untrusted servers.\n\nSigned-off-by: Matt McCutchen \n---\n\nAnd here's a proposed patch. Based on the maint branch, ac84098.\n\n Documentation/fetch-push-security.txt | 9 +++++++++\n Documentation/git-fetch.txt | 2 ++\n Documentation/git-pull.txt | 2 ++\n Documentation/git-push.txt | 2 ++\n 4 files changed, 15 insertions(+)\n create mode 100644 Documentation/fetch-push-security.txt\n\ndiff --git a/Documentation/fetch-push-security.txt b/Documentation/fetch-push-security.txt\nnew file mode 100644\nindex 0000000..00944ed\n--- /dev/null\n+++ b/Documentation/fetch-push-security.txt\n@@ -0,0 +1,9 @@\n+SECURITY\n+--------\n+The fetch and push protocols are not designed to prevent a malicious\n+server from stealing data from your repository that you did not intend to\n+share. The possible attacks are similar to the ones described in the\n+\"SECURITY\" section of linkgit:gitnamespaces[7]. If you have private data\n+that you need to protect from the server, keep it in a separate\n+repository.\n+\ndiff --git a/Documentation/git-fetch.txt b/Documentation/git-fetch.txt\nindex 9e42169..a461b4b 100644\n--- a/Documentation/git-fetch.txt\n+++ b/Documentation/git-fetch.txt\n@@ -192,6 +192,8 @@ The first command fetches the `maint` branch from the repository at\n objects will eventually be removed by git's built-in housekeeping (see\n linkgit:git-gc[1]).\n \n+include::fetch-push-security.txt[]\n+\n BUGS\n ----\n Using --recurse-submodules can only fetch new commits in already checked\ndiff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt\nindex d033b25..0af2de9 100644\n--- a/Documentation/git-pull.txt\n+++ b/Documentation/git-pull.txt\n@@ -237,6 +237,8 @@ If you tried a pull which resulted in complex conflicts and\n would want to start over, you can recover with 'git reset'.\n \n \n+include::fetch-push-security.txt[]\n+\n BUGS\n ----\n Using --recurse-submodules can only fetch new commits in already checked\ndiff --git a/Documentation/git-push.txt b/Documentation/git-push.txt\nindex 47b77e6..5ebef9e 100644\n--- a/Documentation/git-push.txt\n+++ b/Documentation/git-push.txt\n@@ -559,6 +559,8 @@ Commits A and B would no longer belong to a branch with a symbolic name,\n and so would be unreachable. As such, these commits would be removed by\n a `git gc` command on the origin repository.\n \n+include::fetch-push-security.txt[]\n+\n GIT\n ---\n Part of the linkgit:git[1] suite\n-- \n2.7.4\n\n\n"},{"id":"305871","messageId":"1479003016.3471.18.camel@mattmccutchen.net","threadId":"44396","inReplyTo":"xmqqtwbuuuuy.fsf@gitster.mtv.corp.google.com","subject":"Re: Fetch/push lets a malicious server steal the targets of \"have\" lines","fromName":"Matt McCutchen","fromEmail":"matt@mattmccutchen.net","sentAt":"2016-11-13T02:10:16Z","receivedAt":"2016-11-13T02:10:29Z","isPatch":false,"sender":{"key":"matt@mattmccutchen.net","avatar":"https://avatars.githubusercontent.com/u/8885753?v=4"},"body":"On Sun, 2016-10-30 at 01:03 -0700, Junio C Hamano wrote:\n> Matt McCutchen writes:\n> \n> > \n> > On Fri, 2016-10-28 at 22:31 -0700, Junio C Hamano wrote:\n> > > \n> > > Not sending to the list, where mails from Gmail/phone is known to\n> > > get\n> > > rejected.\n> > \n> > [I guess I can go ahead and quote this to the list.]\n> > \n> > > \n> > > No. I'm saying that the scenario you gave is bad and people\n> > > should be\n> > > taught not to connect to untrustworthy sites.\n> > \n> > To clarify, are you saying:\n> > \n> > (1) don't connect to an untrusted server ever (e.g., we don't\n> > promise\n> > that the server can't execute arbitrary code on the client), or\n> > \n> > (2) don't connect to an untrusted server if the client repository\n> > has\n> > data that needs to be kept secret from the server?\n> \n> You sneaked \"arbitrary code execution\" into the discussion but I do\n> not know where it came from.  In any case, \"don't pull from or push\n> to untrustworthy place\" would be a common sense advice that would\n> make sense in any scenario ;-)\n\nA blanket statement like that without explanation is not very helpful\nto users who do find themselves needing to pull from or push to a\nserver they don't absolutely trust.  The only \"definitely safe\" option\nit leaves them is to run the entire thing in a sandbox.  A statement of\nthe nature of the risk is much more helpful: users can determine that\nthey don't care about the risk, or if it does, what the easiest\nworkaround is.\n\nThe new risk we discovered in this thread is of leakage of private data\nfrom the local repository.  To avoid that risk, it's sufficient for\nusers to move private data to a separate repository, so that's the\nadvice I propose to give.  Are you aware of issues with fetch/push with\npotential impact beyond leakage of private data, which would make my\nproposed text insufficient?  I was giving \"arbitrary code execution\" as\nan example of what the impact of such an issue could be.\n\n> Just for future reference, when you have ideas/issues that might\n> have possible security ramifications, I'd prefer to see it first\n> discussed on a private list we created for that exact purpose, until\n> we can assess the impact (if any).  Right now MaintNotes says this:\n> \n>     If you think you found a security-sensitive issue and want to\n> disclose\n>     it to us without announcing it to wider public, please contact us\n> at\n>     our security mailing list .  This\n> is\n>     a closed list that is limited to people who need to know early\n> about\n>     vulnerabilities, including:\n> \n>       - people triaging and fixing reported vulnerabilities\n>       - people operating major git hosting sites with many users\n>       - people packaging and distributing git to large numbers of\n> people\n> \n>     where these issues are discussed without risk of the information\n>     leaking out before we're ready to make public announcements.\n> \n> We may want to tweak the description from \"disclose it to us\" to\n> \"have a discussion on it with us\" (the former makes it sound as if\n> the topic has to be a definite problem, the latter can include an\n> idle speculation that may not be realistic attack vector).\n\nOK.  I'll admit that I didn't even look for a policy on reporting of\nsecurity issues because I believed the issue had low enough impact that\na report to a dedicated security contact point would be unwelcome.\n Maybe that was reckless.  The new text sounds good, if you put it in a\nplace where people like me would see it. :/\n\nMatt\n"},{"id":"305872","messageId":"1479005056.3471.40.camel@mattmccutchen.net","threadId":"44396","inReplyTo":"xmqqpomiuu8q.fsf@gitster.mtv.corp.google.com","subject":"Re: Fetch/push lets a malicious server steal the targets of \"have\" lines","fromName":"Matt McCutchen","fromEmail":"matt@mattmccutchen.net","sentAt":"2016-11-13T02:44:16Z","receivedAt":"2016-11-13T02:44:32Z","isPatch":false,"sender":{"key":"matt@mattmccutchen.net","avatar":"https://avatars.githubusercontent.com/u/8885753?v=4"},"body":"On Sun, 2016-10-30 at 01:16 -0700, Junio C Hamano wrote:\n> Jon Loeliger writes:\n> \n> > \n> > Is there an existing protocol provision, or an extension to\n> > the protocol that would allow a distrustful client to say to\n> > the server, \"Really, you have Y2?  Prove it.\"\n> \n> There is not, but I do not think it would be an effective solution.\n> \n> The issue is not the lack of protocol support, but how to determine\n> that the other side needs such a proof for Y2 but not for other\n> commits.  How does your side know what makes Y2 special and why does\n> yout side think they should not have Y2?\n> \n> Once you know how to determine Y2 is special, that knowledge can be\n> used to abort the \"push\" before even starting.  When you are pushing\n> back the 'master' and that 'master' reaches Y2, which must be kept\n> secret, you shouldn't be pushing that 'master' to them, whether they\n> claim to have Y2 or not.\n\nFWIW, I can imagine a protocol that would prove possession for all\nobjects, which would completely fix the problem.  Each object would\nhave a \"private\" hash computed recursively over the object graph, just\nlike the ordinary object hash, but with a different seed.  The object\ndatabase would be extended to cache the private hash of every object.\n Then, during a fetch or push, when the two sides identify a matching\nobject, the side that would otherwise have had to send the object sends\nthe private hash.  Support for storing multiple hashes per object might\nalso be useful in some way for the migration to a stronger hash\nfunction than SHA-1.\n\nThe next best solution, which doesn't require a protocol change but\nrequires a little user intervention, is to have a configuration option\nper remote for a set of refs whose reachable objects are known to be\nsafe to send to the server.  This set presumably includes the remote's\nown remote-tracking refs.  During fetch and push, the client looks for\nmatches only among these \"safe\" objects, effectively emulating a\nrepository containing only the safe objects.  A fetch may update the\nremote-tracking refs to point to unsafe objects that were already in\nthe local repository, effectively making them safe, but only after the\nserver sends the content of these objects (and the client validates the\nhashes!).\n\nUnfortunately, I'm not signing up to implement either solution. :(\n\nMatt\n"},{"id":"305880","messageId":"xmqq1syezs3g.fsf@gitster.mtv.corp.google.com","threadId":"44396","inReplyTo":"1479001205.3471.1.camel@mattmccutchen.net","subject":"Re: [PATCH] fetch/push: document that private data can be leaked","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-11-14T02:57:07Z","receivedAt":"2016-11-14T02:57:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matt McCutchen writes:\n\n> Documentation/fetch-push-security.txt | 9 +++++++++\n\nA new (consolidated) piece like this that can be included in\nmultiple places is a good idea. I wonder if the original\ndescription in \"namespaces\" thing can be moved here and then\n\"namespaces\" page can be made to also borrow from this?\n\n> Documentation/git-fetch.txt | 2 ++\n> Documentation/git-pull.txt | 2 ++\n> Documentation/git-push.txt | 2 ++\n> 4 files changed, 15 insertions(+)\n> create mode 100644 Documentation/fetch-push-security.txt\n>\n> diff --git a/Documentation/fetch-push-security.txt b/Documentation/fetch-push-security.txt\n> new file mode 100644\n> index 0000000..00944ed\n> --- /dev/null\n> +++ b/Documentation/fetch-push-security.txt\n> @@ -0,0 +1,9 @@\n> +SECURITY\n> +--------\n> +The fetch and push protocols are not designed to prevent a malicious\n> +server from stealing data from your repository that you did not intend to\n> +share. The possible attacks are similar to the ones described in the\n> +\"SECURITY\" section of linkgit:gitnamespaces[7]. If you have private data\n> +that you need to protect from the server, keep it in a separate\n> +repository.\n\nYup, and then \"do not push to untrustworthy place without checking\nwhat you are pushing\", too?\n\n> diff --git a/Documentation/git-fetch.txt b/Documentation/git-fetch.txt\n> diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt\n> diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt\n\nThese three look sensible.\n"},{"id":"305899","messageId":"1479148088.2406.27.camel@mattmccutchen.net","threadId":"44396","inReplyTo":"xmqq1syezs3g.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] fetch/push: document that private data can be leaked","fromName":"Matt McCutchen","fromEmail":"matt@mattmccutchen.net","sentAt":"2016-11-14T18:28:08Z","receivedAt":"2016-11-14T18:28:17Z","isPatch":true,"sender":{"key":"matt@mattmccutchen.net","avatar":"https://avatars.githubusercontent.com/u/8885753?v=4"},"body":"On Sun, 2016-11-13 at 18:57 -0800, Junio C Hamano wrote:\n> Matt McCutchen writes:\n> \n> > \n> >  Documentation/fetch-push-security.txt | 9 +++++++++\n> \n> A new (consolidated) piece like this that can be included in\n> multiple places is a good idea.  I wonder if the original\n> description in \"namespaces\" thing can be moved here and then\n> \"namespaces\" page can be made to also borrow from this?\n\nI gave this a try.  New patch coming.\n\n> > --- /dev/null\n> > +++ b/Documentation/fetch-push-security.txt\n> > @@ -0,0 +1,9 @@\n> > +SECURITY\n> > +--------\n> > +The fetch and push protocols are not designed to prevent a\n> > malicious\n> > +server from stealing data from your repository that you did not\n> > intend to\n> > +share. The possible attacks are similar to the ones described in\n> > the\n> > +\"SECURITY\" section of linkgit:gitnamespaces[7]. If you have\n> > private data\n> > +that you need to protect from the server, keep it in a separate\n> > +repository.\n> \n> Yup, and then \"do not push to untrustworthy place without checking\n> what you are pushing\", too?\n\nIf there is no private data in the repository, then there is no need\nfor the user to check what they are pushing.  As I've indicated before,\nIMO manually checking each push would not be a workable security\nmeasure in the long term anyway.\n\nMatt\n"},{"id":"305900","messageId":"1479148255.2406.30.camel@mattmccutchen.net","threadId":"44396","inReplyTo":"1479148088.2406.27.camel@mattmccutchen.net","subject":"[PATCH] doc: mention transfer data leaks in more places","fromName":"Matt McCutchen","fromEmail":"matt@mattmccutchen.net","sentAt":"2016-11-14T18:20:24Z","receivedAt":"2016-11-14T18:31:24Z","isPatch":true,"sender":{"key":"matt@mattmccutchen.net","avatar":"https://avatars.githubusercontent.com/u/8885753?v=4"},"body":"The \"SECURITY\" section of the gitnamespaces(7) man page described two\nways for a client to steal data from a server that wasn't intended to be\nshared. Similar attacks can be performed by a server on a client, so\nadapt the section to cover both directions and add it to the\ngit-fetch(1), git-pull(1), and git-push(1) man pages. Also add\nreferences to this section from the documentation of server\nconfiguration options that attempt to control data leakage but may not\nbe fully effective.\n\nSigned-off-by: Matt McCutchen \n---\n Documentation/config.txt | 17 ++++++++++++++---\n Documentation/git-fetch.txt | 2 ++\n Documentation/git-pull.txt | 2 ++\n Documentation/git-push.txt | 2 ++\n Documentation/gitnamespaces.txt | 20 +-------------------\n Documentation/transfer-data-leaks.txt | 30 ++++++++++++++++++++++++++++++\n 6 files changed, 51 insertions(+), 22 deletions(-)\n create mode 100644 Documentation/transfer-data-leaks.txt\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 21fdddf..fc2cf83 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -2898,6 +2898,11 @@ is omitted from the advertisements but `refs/heads/master` and\n `refs/namespaces/bar/refs/heads/master` are still advertised as so-called\n \"have\" lines. In order to match refs before stripping, add a `^` in front of\n the ref name. If you combine `!` and `^`, `!` must be specified first.\n++\n+Even if you hide refs, a client may still be able to steal the target\n+objects via the techniques described in the \"SECURITY\" section of the\n+linkgit:gitnamespaces[7] man page; it's best to keep private data in a\n+separate repository.\n \n transfer.unpackLimit::\n \tWhen `fetch.unpackLimit` or `receive.unpackLimit` are\n@@ -2907,7 +2912,7 @@ transfer.unpackLimit::\n uploadarchive.allowUnreachable::\n \tIf true, allow clients to use `git archive --remote` to request\n \tany tree, whether reachable from the ref tips or not. See the\n-\tdiscussion in the `SECURITY` section of\n+\tdiscussion in the \"SECURITY\" section of\n \tlinkgit:git-upload-archive[1] for more details. Defaults to\n \t`false`.\n \n@@ -2921,13 +2926,19 @@ uploadpack.allowTipSHA1InWant::\n \tWhen `uploadpack.hideRefs` is in effect, allow `upload-pack`\n \tto accept a fetch request that asks for an object at the tip\n \tof a hidden ref (by default, such a request is rejected).\n-\tsee also `uploadpack.hideRefs`.\n+\tSee also `uploadpack.hideRefs`. Even if this is false, a client\n+\tmay be able to steal objects via the techniques described in the\n+\t\"SECURITY\" section of the linkgit:gitnamespaces[7] man page; it's\n+\tbest to keep private data in a separate repository.\n \n uploadpack.allowReachableSHA1InWant::\n \tAllow `upload-pack` to accept a fetch request that asks for an\n \tobject that is reachable from any ref tip. However, note that\n \tcalculating object reachability is computationally expensive.\n-\tDefaults to `false`.\n+\tDefaults to `false`. Even if this is false, a client may be able\n+\tto steal objects via the techniques described in the \"SECURITY\"\n+\tsection of the linkgit:gitnamespaces[7] man page; it's best to\n+\tkeep private data in a separate repository.\n \n uploadpack.keepAlive::\n \tWhen `upload-pack` has started `pack-objects`, there may be a\ndiff --git a/Documentation/git-fetch.txt b/Documentation/git-fetch.txt\nindex 9e42169..b153aef 100644\n--- a/Documentation/git-fetch.txt\n+++ b/Documentation/git-fetch.txt\n@@ -192,6 +192,8 @@ The first command fetches the `maint` branch from the repository at\n objects will eventually be removed by git's built-in housekeeping (see\n linkgit:git-gc[1]).\n \n+include::transfer-data-leaks.txt[]\n+\n BUGS\n ----\n Using --recurse-submodules can only fetch new commits in already checked\ndiff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt\nindex d033b25..4470e4b 100644\n--- a/Documentation/git-pull.txt\n+++ b/Documentation/git-pull.txt\n@@ -237,6 +237,8 @@ If you tried a pull which resulted in complex conflicts and\n would want to start over, you can recover with 'git reset'.\n \n \n+include::transfer-data-leaks.txt[]\n+\n BUGS\n ----\n Using --recurse-submodules can only fetch new commits in already checked\ndiff --git a/Documentation/git-push.txt b/Documentation/git-push.txt\nindex 47b77e6..8eefabd 100644\n--- a/Documentation/git-push.txt\n+++ b/Documentation/git-push.txt\n@@ -559,6 +559,8 @@ Commits A and B would no longer belong to a branch with a symbolic name,\n and so would be unreachable. As such, these commits would be removed by\n a `git gc` command on the origin repository.\n \n+include::transfer-data-leaks.txt[]\n+\n GIT\n ---\n Part of the linkgit:git[1] suite\ndiff --git a/Documentation/gitnamespaces.txt b/Documentation/gitnamespaces.txt\nindex 7685e36..b614969 100644\n--- a/Documentation/gitnamespaces.txt\n+++ b/Documentation/gitnamespaces.txt\n@@ -61,22 +61,4 @@ For a simple local test, you can use linkgit:git-remote-ext[1]:\n git clone ext::'git --namespace=foo %s /tmp/prefixed.git'\n ----------\n \n-SECURITY\n---------\n-\n-Anyone with access to any namespace within a repository can potentially\n-access objects from any other namespace stored in the same repository.\n-You can't directly say \"give me object ABCD\" if you don't have a ref to\n-it, but you can do some other sneaky things like:\n-\n-. Claiming to push ABCD, at which point the server will optimize out the\n- need for you to actually send it. Now you have a ref to ABCD and can\n- fetch it (claiming not to have it, of course).\n-\n-. Requesting other refs, claiming that you have ABCD, at which point the\n- server may generate deltas against ABCD.\n-\n-None of this causes a problem if you only host public repositories, or\n-if everyone who may read one namespace may also read everything in every\n-other namespace (for instance, if everyone in an organization has read\n-permission to every repository).\n+include::transfer-data-leaks.txt[]\ndiff --git a/Documentation/transfer-data-leaks.txt b/Documentation/transfer-data-leaks.txt\nnew file mode 100644\nindex 0000000..914bacc\n--- /dev/null\n+++ b/Documentation/transfer-data-leaks.txt\n@@ -0,0 +1,30 @@\n+SECURITY\n+--------\n+The fetch and push protocols are not designed to prevent one side from\n+stealing data from the other repository that was not intended to be\n+shared. If you have private data that you need to protect from a malicious\n+peer, your best option is to store it in another repository. This applies\n+to both clients and servers. In particular, namespaces on a server are not\n+effective for read access control; you should only grant read access to a\n+namespace to clients that you would trust with read access to the entire\n+repository.\n+\n+The known attack vectors are as follows:\n+\n+. The victim sends \"have\" lines advertising the IDs of objects it has that\n+ are not explicitly intended to be shared but can be used to optimize the\n+ transfer if the peer also has them. The attacker chooses an object ID X\n+ to steal and sends a ref to X, but isn't required to send the content of\n+ X because the victim already has it. Now the victim believes that the\n+ attacker has X, and it sends the content of X back to the attacker\n+ later. (This attack is most straightforward for a client to perform on a\n+ server, by creating a ref to X in the namespace the client has access\n+ to and then fetching it. The most likely way for a server to perform it\n+ on a client is to \"merge\" X into a public branch and hope that the user\n+ does additional work on this branch and pushes it back to the server\n+ without noticing the merge.)\n+\n+. As in #1, the attacker chooses an object ID X to steal. The victim sends\n+ an object Y that the attacker already has, and the attacker falsely\n+ claims to have X and not Y, so the victim sends Y as a delta against X.\n+ The delta reveals regions of X that are similar to Y to the attacker.\n-- \n2.7.4\n\n\n"},{"id":"305903","messageId":"xmqqbmxhyjij.fsf@gitster.mtv.corp.google.com","threadId":"44396","inReplyTo":"1479148088.2406.27.camel@mattmccutchen.net","subject":"Re: [PATCH] fetch/push: document that private data can be leaked","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-11-14T19:00:04Z","receivedAt":"2016-11-14T19:00:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matt McCutchen writes:\n\n>> Yup, and then \"do not push to untrustworthy place without checking\n>> what you are pushing\", too?\n>\n> If there is no private data in the repository, then there is no need\n> for the user to check what they are pushing. As I've indicated before,\n> IMO manually checking each push would not be a workable security\n> measure in the long term anyway.\n\nThen what is? Don't answer; this is a rhetorical question.\n\nThe answer is \"do not push to untrustworthy place\", if you are\nunable to check what you are pushing.\n\n"},{"id":"305904","messageId":"20161114190725.fxjymvztc2eiomv6@sigill.intra.peff.net","threadId":"44396","inReplyTo":"xmqqbmxhyjij.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] fetch/push: document that private data can be leaked","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-11-14T19:07:26Z","receivedAt":"2016-11-14T19:07:32Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Nov 14, 2016 at 11:00:04AM -0800, Junio C Hamano wrote:\n\n> Matt McCutchen writes:\n> \n> >> Yup, and then \"do not push to untrustworthy place without checking\n> >> what you are pushing\", too?\n> >\n> > If there is no private data in the repository, then there is no need\n> > for the user to check what they are pushing. As I've indicated before,\n> > IMO manually checking each push would not be a workable security\n> > measure in the long term anyway.\n> \n> Then what is? Don't answer; this is a rhetorical question.\n> \n> The answer is \"do not push to untrustworthy place\", if you are\n> unable to check what you are pushing.\n\nI think \"check what you are pushing\" only covers one case (attacker lies\nto you during a fetch, and you accidentally push that back, thinking\nthey already have it).\n\nBut consider the other case mentioned: the attacker lies to you while\npushing and _says_ they have X, then deduces information from the delta\nyou generate. The only advice there is \"do not push to an untrusted\nplace from a repository containing private objects\".\n\nSo I think the in-between answer is \"it is OK to push to an\nuntrustworthy place, but do not do it from a repo that may contain\nsecret contents\".\n\n-Peff\n"},{"id":"305905","messageId":"1479150482.2406.35.camel@mattmccutchen.net","threadId":"44396","inReplyTo":"xmqqbmxhyjij.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] fetch/push: document that private data can be leaked","fromName":"Matt McCutchen","fromEmail":"matt@mattmccutchen.net","sentAt":"2016-11-14T19:08:02Z","receivedAt":"2016-11-14T19:08:10Z","isPatch":true,"sender":{"key":"matt@mattmccutchen.net","avatar":"https://avatars.githubusercontent.com/u/8885753?v=4"},"body":"On Mon, 2016-11-14 at 11:00 -0800, Junio C Hamano wrote:\n> Matt McCutchen writes:\n> \n> > \n> > > \n> > > Yup, and then \"do not push to untrustworthy place without\n> > > checking\n> > > what you are pushing\", too?\n> > \n> > If there is no private data in the repository, then there is no\n> > need\n> > for the user to check what they are pushing. As I've indicated\n> > before,\n> > IMO manually checking each push would not be a workable security\n> > measure in the long term anyway.\n> \n> Then what is?\n\nDon't put private data in the same repository, then the whole issue\nbecomes moot.  Am I missing something?\n\nMatt\n"},{"id":"305908","messageId":"xmqq7f85yiml.fsf@gitster.mtv.corp.google.com","threadId":"44396","inReplyTo":"1479148255.2406.30.camel@mattmccutchen.net","subject":"Re: [PATCH] doc: mention transfer data leaks in more places","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-11-14T19:19:14Z","receivedAt":"2016-11-14T19:19:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matt McCutchen writes:\n\n> The \"SECURITY\" section of the gitnamespaces(7) man page described two\n> ways for a client to steal data from a server that wasn't intended to be\n> shared. Similar attacks can be performed by a server on a client, so\n> adapt the section to cover both directions and add it to the\n> git-fetch(1), git-pull(1), and git-push(1) man pages. Also add\n> references to this section from the documentation of server\n> configuration options that attempt to control data leakage but may not\n> be fully effective.\n\nThis round looks OK. Will queue. Thanks.\n\n"},{"id":"305914","messageId":"xmqq37ityhc1.fsf@gitster.mtv.corp.google.com","threadId":"44396","inReplyTo":"20161114190725.fxjymvztc2eiomv6@sigill.intra.peff.net","subject":"Re: [PATCH] fetch/push: document that private data can be leaked","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-11-14T19:47:10Z","receivedAt":"2016-11-14T19:47:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> So I think the in-between answer is \"it is OK to push to an\n> untrustworthy place, but do not do it from a repo that may contain\n> secret contents\".\n\nYes, that sounds like a sensible piece of advice to give to the\nreaders.\n\n"}]}