{"thread":{"id":"33425","subject":"commit-message attack for extracting sensitive data from rewritten Git history","startedAt":"2013-04-07T23:17:51Z","lastAt":"2013-04-09T18:08:52Z","messageCount":6,"participants":["Roberto Tyley","Junio C Hamano","Jeff King","Johannes Sixt"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"213479","messageId":"CAFY1edbNPjs5JGOPRxzB+ie4w=SvR+rUeePhsEnpr0tWtZpeHg@mail.gmail.com","threadId":"33425","inReplyTo":null,"subject":"commit-message attack for extracting sensitive data from rewritten Git history","fromName":"Roberto Tyley","fromEmail":"roberto.tyley@gmail.com","sentAt":"2013-04-07T23:17:51Z","receivedAt":"2013-04-07T23:17:51Z","isPatch":false,"sender":{"key":"roberto.tyley@gmail.com","avatar":"https://avatars.githubusercontent.com/u/52038?v=4"},"body":"This is a demonstration of a mildly-interesting security concern\nrelating to Git & git-filter-branch - not a vulnerability in Git\nitself, just in the way it can be used. I thought it was interesting\nto demonstrate that there is sometimes an avenue of attack for\nrecovering sensitive data that's been removed from Git history using\ngit-filter-branch. I think it's a low-severity issue, you may wish to\nignore this, and indeed I've been very politely told already that it's\nclearly nonsense :)\n\nHere's an unmodified repo, in which the user unwisely committed a\ndatabase password:\n\nhttps://github.com/bfg-repo-cleaner-demos/gma-demo-repo-original/commit/8c9cfe3c\n\nThe unwise commit is reverted with a second commit using 'git revert',\nwhich obviously leaves the password in Git history, and - some time\nlater - it's decided to properly clean the repo history with\ngit-filter-branch & git gc, purging the password so the repo can be\nmore widely shared (open-sourced, or just externally hosted).\n\ngit-filter-branch works exactly as intended, purging the password, but\nthe one thing it does not- typically - do is update the commit\nmessage. So in the cleaned repo, the commit message for the revert\ncommit still looks like this:\n\nhttps://github.com/bfg-repo-cleaner-demos/gma-demo-repo-git-filter-branch-cleaned/commit/bf0637a5\n\nIt contains a commit id (8c9cfe3) which is no longer in the repo, but\ncan very easily be associated with an existing commit simply by\nexamining the subject line of the reverted commit (\"Carelessly\nchecking password into source control\"). It's also obvious, from\nexamining the repo, where the excised data was removed (ie at the\n\"db.password=\" line). At this point it's possible to do a brute-force\nattack where you generate possible passwords, insert them into the\navailable commit's tree, and compare them against the leaked commit\nid. When the the commit id matches, the sensitive data has been\nrecovered.\n\nA proof-of-concept implementation of this attack was indeed able to\nrecover the purged password:\n\n--\n$ java -jar gma-0.1.jar 8c9cfe3c attack-pinpoint\ngma-demo-repo-git-filter-branch-cleaned\n\nBrute-force search using these characters : 0123456789abcdefghijklmnopqrstuvwxyz\nAvailable commit, presumed cleaned : 8ebbf661\nFile path : src/main/resources/config.properties\nTemplate blob : dca1a2fb\nExhausted strings of length 1 or less\n...\nExhausted strings of length 4 or less\nMatch with '0g6rw'\n--\n\nSo all of this amounts to a fairly low severity issue - people should\nalways change credentials when they mistakenly commit them to a repo -\nbut I guess the point is that from a paranoia point of view, you want\nto remove all information - including old commit hashes buried in\ncommit messages - that relate to sensitive data when you clean a repo\nfor sharing. The git-filter-branch command has a --msg-filter option\nwhich could be used for this purpose, with the application of some\njudicious bash-scripting, grep&sed-ing. However, I must confess that I\nbelieve users would be better advised to use The BFG:\n\nhttp://rtyley.github.io/bfg-repo-cleaner/\n\nThe BFG already addresses this issue by replacing all old Git\nobject-ids found in commit/tag messages with the updated id. For\ninstance, here's that exact same commit message when cleaned with the\nBFG:\n\nhttps://github.com/bfg-repo-cleaner-demos/gma-demo-repo-bfg-cleaned/commit/35840201\n\nIn the case that the users specifies a filtering operation is not\nremoving 'private' data, the BFG replaces old ids with text of the\nform '\"newid [formerly oldid]\", but if the operation is in fact to\nstrip private data, the replacement value is simply the newid - and\nwithout the old commit id, the attack described above is not possible.\n\nI believe it's worth educating users to give them a more realistic\nunderstanding of their exposure, and would like to update the\ndocumentation of git-filter-branch to give them a better idea of their\noptions for removing private data - that would include noting the BFG\nas alternative.\n\n- Roberto Tyley\n\nhttps://github.com/rtyley/bfg-repo-cleaner/blob/v1.2.0/src/main/scala/com/madgag/git/bfg/cleaner/ObjectIdSubstitutor.scala#L33-L60\n"},{"id":"213525","messageId":"7vehelyqrv.fsf@alter.siamese.dyndns.org","threadId":"33425","inReplyTo":"CAFY1edbNPjs5JGOPRxzB+ie4w=SvR+rUeePhsEnpr0tWtZpeHg@mail.gmail.com","subject":"Re: commit-message attack for extracting sensitive data from rewritten Git history","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-04-08T15:40:36Z","receivedAt":"2013-04-08T15:40:36Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Roberto Tyley <roberto.tyley@gmail.com> writes:\n\n> Here's an unmodified repo, in which the user unwisely committed a\n> database password:\n>\n> https://github.com/bfg-repo-cleaner-demos/gma-demo-repo-original/commit/8c9cfe3c\n>\n> The unwise commit is reverted with a second commit using 'git revert',\n> which obviously leaves the password in Git history, and - some time\n> later - it's decided to properly clean the repo history with\n> git-filter-branch & git gc, purging the password so the repo can be\n> more widely shared (open-sourced, or just externally hosted).\n>\n> git-filter-branch works exactly as intended, purging the password, but\n> the one thing it does not- typically - do is update the commit\n> message....\n> .... The git-filter-branch command has a --msg-filter option\n> which could be used for this purpose, with the application of some\n> judicious bash-scripting, grep&sed-ing. However, I must confess that I\n> believe users would be better advised to use The BFG:\n>\n> http://rtyley.github.io/bfg-repo-cleaner/\n\nWith or without the security issue, leaving old object names that\nwill become irrelevant in the rewritten history will make the\nresulting history less useful, simply because people cannot look at\nthe objects these messages refer to. The same argument is behind the\nreason why \"cherry-pick -x\" was originally the default, found to be\na mistake and made optional.\n\nfilter-branch provides \"map\" helper function to help mapping old\nobject names to rewritten object names, but stops there; it leaves\nit up to the message filter script to identify what string in the\nmessage is an object name to be rewritten.\n\nIt can be taught to be more helpful to the message filter writers,\nand you seem to have done so in BFG, which is very good.\n"},{"id":"213617","messageId":"20130408215457.GB11227@sigill.intra.peff.net","threadId":"33425","inReplyTo":"7vehelyqrv.fsf@alter.siamese.dyndns.org","subject":"Re: commit-message attack for extracting sensitive data from rewritten Git history","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-04-08T21:54:57Z","receivedAt":"2013-04-08T21:54:57Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Apr 08, 2013 at 08:40:36AM -0700, Junio C Hamano wrote:\n\n> With or without the security issue, leaving old object names that\n> will become irrelevant in the rewritten history will make the\n> resulting history less useful, simply because people cannot look at\n> the objects these messages refer to. The same argument is behind the\n> reason why \"cherry-pick -x\" was originally the default, found to be\n> a mistake and made optional.\n> \n> filter-branch provides \"map\" helper function to help mapping old\n> object names to rewritten object names, but stops there; it leaves\n> it up to the message filter script to identify what string in the\n> message is an object name to be rewritten.\n> \n> It can be taught to be more helpful to the message filter writers,\n> and you seem to have done so in BFG, which is very good.\n\nYeah, it would make sense for filter-branch to have a \"--map-commit-ids\"\noption or similar that does the update. At first I thought it might take\ntwo passes, but I don't think it is necessary, as long as we traverse\nthe commits topologically (i.e., you cannot have mentioned X in a commit\nthat is an ancestor of X, so you do not have to worry about mapping it\nuntil after it has been processed).\n\n-Peff\n"},{"id":"213642","messageId":"5163AF2C.2020107@viscovery.net","threadId":"33425","inReplyTo":"20130408215457.GB11227@sigill.intra.peff.net","subject":"Re: commit-message attack for extracting sensitive data from rewritten Git history","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2013-04-09T06:03:24Z","receivedAt":"2013-04-09T06:03:24Z","isPatch":false,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 4/8/2013 23:54, schrieb Jeff King:\n> Yeah, it would make sense for filter-branch to have a \"--map-commit-ids\"\n> option or similar that does the update. At first I thought it might take\n> two passes, but I don't think it is necessary, as long as we traverse\n> the commits topologically (i.e., you cannot have mentioned X in a commit\n> that is an ancestor of X, so you do not have to worry about mapping it\n> until after it has been processed).\n\nTopological traversal is not sufficient. Consider this history:\n\n     o--A--o--\n    /     /\n --o--B--o\n\nIf A mentions B (think of cherry-pick -x), then you must ensure that the\nbranch containing B was traversed first.\n\n-- Hannes\n"},{"id":"213673","messageId":"20130409170159.GC21972@sigill.intra.peff.net","threadId":"33425","inReplyTo":"5163AF2C.2020107@viscovery.net","subject":"Re: commit-message attack for extracting sensitive data from rewritten Git history","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-04-09T17:01:59Z","receivedAt":"2013-04-09T17:01:59Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Apr 09, 2013 at 08:03:24AM +0200, Johannes Sixt wrote:\n\n> Am 4/8/2013 23:54, schrieb Jeff King:\n> > Yeah, it would make sense for filter-branch to have a \"--map-commit-ids\"\n> > option or similar that does the update. At first I thought it might take\n> > two passes, but I don't think it is necessary, as long as we traverse\n> > the commits topologically (i.e., you cannot have mentioned X in a commit\n> > that is an ancestor of X, so you do not have to worry about mapping it\n> > until after it has been processed).\n> \n> Topological traversal is not sufficient. Consider this history:\n> \n>      o--A--o--\n>     /     /\n>  --o--B--o\n> \n> If A mentions B (think of cherry-pick -x), then you must ensure that the\n> branch containing B was traversed first.\n\nYeah, you're right. Multiple passes are necessary to get it\ncompletely right. And because each pass may change more commit id's, you\nhave to recurse to pick up those changes, and keep going until you have\na pass with no changes.\n\nBut I haven't thought that hard about it. There might be a clever\noptimization where you can prune out parts of the history (e.g., if you\nknow that all changes to consider are in descendants of a commit, you do\nnot have to care about rewriting the commit or its ancestors).\n\n-Peff\n"},{"id":"213692","messageId":"CAFY1edbor4tE3bgiScBESX_XY3RY_WSKs7_Y3n++u6tru3nepQ@mail.gmail.com","threadId":"33425","inReplyTo":"20130409170159.GC21972@sigill.intra.peff.net","subject":"Re: commit-message attack for extracting sensitive data from rewritten Git history","fromName":"Roberto Tyley","fromEmail":"roberto.tyley@gmail.com","sentAt":"2013-04-09T18:08:52Z","receivedAt":"2013-04-09T18:08:52Z","isPatch":false,"sender":{"key":"roberto.tyley@gmail.com","avatar":"https://avatars.githubusercontent.com/u/52038?v=4"},"body":"On 9 April 2013 18:01, Jeff King <peff@peff.net> wrote:\n> On Tue, Apr 09, 2013 at 08:03:24AM +0200, Johannes Sixt wrote:\n>> If A mentions B (think of cherry-pick -x), then you must ensure that the\n>> branch containing B was traversed first.\n>\n> Yeah, you're right. Multiple passes are necessary to get it\n> completely right. And because each pass may change more commit id's, you\n> have to recurse to pick up those changes, and keep going until you have\n> a pass with no changes.\n\nJust to give some context on how the BFG handles this (without doing\nmultiple passes):\n\nThe BFG makes a design choice (based on it's intended use-case of\nannihilating unwanted data) that a specific tree or blob will always\nbe cleaned in exactly the same way - because when you're trying to get\nrid of large blobs or private data, you most likely /don't care/ where\nit is, what commit it belongs to, how old it is. The id for a cleaned\ntree or blob is always the same no matter where it came from, and so\nthe BFG maintains a in-memory mapping of 'dirty' to 'clean' object ids\nwhile cleaning a repo - whenever an object (commit, tag, tree, blob)\nis cleaned, these values are stored in the map:\n\n\n  dirty-id -> clean-id\n  clean-id -> clean-id\n\n(in terms of memory overhead, this amounts to only ~ 128MB for even\nquite a large repo like the linux kernel, so I don't spend much time\nworrying about it)\n\n\nThe map memoises the cleaning functions on all objects, so an object\n(particularly a tree) never gets cleaned more than once, which is one\nof the things that makes the BFG fast.\n\nHaving these memoised functions makes cleaning commit messages fairly\neasy - the message is grepped for hex strings more than a few\ncharacters in length, and if a matched string resolves uniquely to an\nobject id in the repo, the clean() method is called on it to get the\ncleaned id - which will either return immediately with a previously\ncalculated result, or if the id came from a different branch, trigger\na cascade of more cleaning, eventually returning the required cleaned\nid.\n\nIn the case of git-filter-branch, the user has a lot more freedom to\nchange the tree-structure of commits on a commit-by-commit basis, so\nmemoising tree-cleaning is out of the question, but I guess it might\nbe possible to do memoisation of just the commit ids to short-cut the\nmultiple-pass problem.\n\n- Roberto Tyley\n"}]}