8 ms·

A Git Horror Story: Repository Integrity With Signed Commits

163 points by mikegerwitz 14y ago

jentulman 14y ago
Thank you for taking the time to write this up in such detail and for making it an enjoyable read too.
Roritharr 14y ago
This describes pretty exactly why security questions are hard. It's not as easy as using tool x and generate key y... its more than using software, if you want security, you have to think about the whole stack, down to the access rights that each part of your hardware has. There are good and best practices, but with IT Security, using the best practices in one place and just plain forgetting to unlock you PC when you go to the toilet at the office is more than enough... People are allowed to be afraid when asked security questions.
- nknight 14y ago
  > just plain forgetting to [lock] you PC when you go to the toilet You're not even trying. Physical access, game over, the lock is irrelevant. You fear what you don't understand, and you don't understand because you're afraid to learn. You have simply crafted a vicious cycle for yourself, and are making the world that much more unsafe by spreading your fear and acting as if everyone should share in your fear.
  - Roritharr 14y ago
    I'm not spreading fear, since i consider HN a place where people are educated enough understand the problems in it security. Yes, once someone has hardware access, its devilishly hard to secure an environment, but just stating that its impossible is as wrong as security systems that ask their users too many questions to which they most likely have no good answer/no doubtfree answer. There are systems which work pretty securely, i have worked with several educated security experts to create security infrastructures in companies. I'm not saying that i don't understand the problems or don't want to learn solutions to them, i'm trying to defend the position of the user that can't be asked to learn about all the caveats of it security, since it is one of the most complex problems in computer science. No, not every front-end developer that has to check-in his html und js files can be asked to understand all principles of it security.
mdehaan 14y ago
Warning -- This article posts a longwinded first person account and finally ends with "this scenario is purely hypothetical". So it sounds like it is talking about a security vulnerability with git that really happened (signed commits or tags being broken, perhaps?), and it's actually an article saying you should use signed commits if you can't control access to your repo.
eykanal 14y ago
Interesting solution, but is this really a problem? The only situations I can think this applies to are large-scale open-source projects with many maintainers, and even more people submitting patches. For closed-source projects without external patch submissions, this is really only avoids the security threat of someone sitting at your computer and committing malicious code to your repository, which (to me) doesn't sound like much of a threat. (At least, if that is a threat, rogue commits to your git repo are only one part of your problem, and door locks would probably be a better, more broadly beneficial solution than implementing this.)
- nknight 14y ago
  Corporate networks get broken into all the time. Any machine that a copy of the repository is stored on is a potential attack vector. Various strategies are used to specifically protect all kinds of sensitive systems far beyond simple network security. This article is a detailed exploration of a/some strategy/ies for protecting your git repository against malicious modification -- a scenario that is rare but far from unprecedented.
- wyck 14y ago
  Yes of course, creating a backdoor or sabotage via revision control is a really good attack vector (like really). Recently several large open source projects had shady forged commits, so it's safe that say that anything private is also being targeted.
  - MattJ100 14y ago
    Apart from Ruby, which?
    - Jebus 14y ago
      You mean Rails.
    - marshray 14y ago
      Wordpress http://wordpress.org/news/2011/06/passwords-reset/ http://wordpress.org/news/2011/06/passwords-reset/
roel_v 14y ago
I'm not sure what this 31 a4-pages story is saying beyond 'webs of trust are hard'. Nor am I sure what the first several long-winded pages add - they don't demonstrate a concrete problem, yet they are referenced to several times with the words 'as demonstrated by the story in the beginning, ...'. The summary at the bottom doesn't have a whole lot less information than all the content before it, and doesn't lead the reader to believe for the first 10 minutes that some ground-shaking vulnerability disclosure is imminent.
- nknight 14y ago
  It's a detailed exploration of git object signing. It's not trying to describe a "ground-shaking vulnerability". There is a current, known issue with display of git signature verifications, and he's including it in his article. That's not the point of the article. The point is an exploration of options for securing your repository history. That's it.
  - roel_v 14y ago
    Well then maybe (actually, not "maybe", but "certainly") the title should be "a detailed exploration of git object signing", and not link bait like "a git horror story". But of course the first title, while much more accurate and informative, would not draw nearly as many website hits, so the author deliberately set out to mislead people with the title, in order to get more hits/reads. This sort of misrepresentation of content is what makes the web much harder to explore and the continuous stream of content much harder to keep on top of for the information consumer, and therefore should be called out and chastised. But then again, I'm just an old grumpy man who is much faster to criticize and be negative than he should be, so who am I to cast judgement? Do I ameliorate or exacerbate the situation with posts like my previous one or this one? I don't know, maybe I'm part of the problem in that way. Either way, I much preferred (so are my motivations selfish? Is that good or bad? I think good, but how do I know?) the old HN when submissions like this one would be labeled for what they were.
    - mikegerwitz 14y ago
      I'm sorry that you feel that way. You have understandably misinterpreted my intent. The title was not intended as bait, nor was the story. Rather, I enjoy writing and was hoping to appeal to readers that have difficulty reading dry material (which is the material I tend to write), while at the same time providing something to relate to in order to put the article into context. The title itself was a play on a television series --- American Horror Story --- that my girlfriend once fell asleep to as I was hacking late one night. The story was intended to augment the actual useful material (code samples and explanations), not obscure it, which it appears to have done. Please notice that this article is segregated from the rest of my blog with no glaring/obvious links to it. This was intentional. I posted this article in the hope that it would be useful to others, not because I would gain any benefit from doing so. I will take your comment into consideration for future articles.
raverbashing 14y ago
So what does this means is if someone compromise their GPG key (more or less easy) and get his passphrase (not difficult) they can sign all kinds of commits and make it look like he did it. Great Security is only as good as the weakest link. Also, don't make it look like it's more secure when it may not be
rmc 14y ago
tl;dr : You can GPG sign git commits with -S option. This makes things more secure
- natep 14y ago
  But it's not a cure-all
  - rmc 14y ago
    Yes. "more secure" ≠ "100% secure"
    - natep 14y ago
      And sometimes, "more secure" == "less secure than before due to complacency"
asharp 14y ago
From what i've read, the problem is that an attacker can add commits with forced Author information into a central repository to frame somebody else. Wouldn't the signing of all commits as they are committed solve this problem? (ie. rather then trusting Author information from the commit, trust the signed-by information to give author information?)
- mikegerwitz 14y ago
  Note that Signed-off-by (as added by -s) is different than -S (GPG-sign). The -s option simply appends the "Signed-off-by" line to the commit message, so this can also be forged. The GPG signature cannot be forged (access to the private key is needed).
tytso 14y ago
The article puts forward a false dilemma by implying your only choices are (a) squashing all of the commits into a single patch, (b) examining the end point and then only signing the end point (with the risk that there might be some bad stuff hidden in some of the intermediate commits), and (c) examining and signing every single commit. In fact what we do in the kernel community is most patches are in fact reviewed via e-mail. This increases the likelihood more people will look at the patch and notice any potential problem, and more importantly, the e-mail archive introduces an additional external record of the proposed change. After each patch in a patch series is reviewed, in isolation, with each change small enough that it can be easily reviewed in a mail reader, and with especial attention made to maintaining bisectability, the maintainer who will ultimately by signing the git pull request will apply it into his or her tree. For larger changes and for larger subsystems where we have submaintainers, it is the maintainer's responsibility to make sure the submaintainer sends a signed pull request. At that point the submaintainer is assuming full responsibility for the set of patches which he or she is sending to the upper-level maintainer, just as the maintainer assumes responsibility for the patches he or she sends to Linus. In the case where the submaintainer does not send a signed pull, or the maintainer does not have full trust in the submaintainer (either for social or technical reasons), it is the responsibility for the maintainer to check each and every commit for correctness. If he or she can't do that, he shouldn't accept the pull request; instead, he might request that the patch series be sent to the mailing list so everyone can review the patch series. One of the things which follows from this is that for me as a kernel or e2fsprogs maintainer, github pull requests are not particularly useful. Most of the time I have no idea who the person is that initiated the pull request, because the primary community is on the mailing list. Worse, the pull request only gets sent to maintainer, which makes the maintainer the review bottleneck. If the patch series is sent to the mailing list, as individual commits, with no requirement to jump to a browser, it allows the review workload to be distributed to other developers on the mailing list. As far as I'm concerned, a github pull request is the same thing as a patch sent via private, personal e-mail. I'm going to review it very carefully, and only apply it if I know that it is clean. Typically the review involves extracting the commit as a diff, and as has been discussed in other places, github doesn't encourage properly formatted commit descriptions, so I end up needing to extract the diff, and then reapply it with an appropriate commit description. So for me github pull requests generally represent negative, not positive value, because of the quality control and quality assurance requirements of the code bases in which I work.
rmccue 14y ago
Wouldn't this be solved with a merge commit? If I can see where a commit merged in, I could prove that I didn't author it, as I don't have access to the other author's machine.
hdmi 14y ago
Very good and long read; should wake up a few people
bp2070 14y ago
An interesting article, but I would prefer the author presented the issues first in a VCS agnostic way (as they are not specific to git) and then introduce the hypothetical scenario using git. Actually these issues are not even specific to VCS, but security of most digital information.
dkarl 14y ago
Code gets reviewed for production at 6am by a guy freshly rousted out of bed who committed some of it four hours earlier, and who is doing this at the insistence of a "frantic" colleague, the fix involves "a bunch of commits" from more than one person, and his main worry is that he might be held responsible for a hidden back door maliciously inserted by his coworker? That's like driving home drunk with your headlights off and worrying that your car could get blown up by a drone because of eroding limits on the use of military force against civilians. I realize it's hypothetical, but man, fix your glaring problems first before you worry about unlikely scenarios.
- heretohelp 14y ago
  This does a great deal to explain why Facebook's release engineering ignores most attempts to push something out-of-band unless they can justify on the basis of real breakage for the users. (Cf. HotFix Bar)
crazydoggers 14y ago
`git tag -s` is all that's needed. The primary problem in your horror story is that you GPG signed a commit with a tag when you shouldn't have. When you tag with GPG, you are saying you do trust the history. You didn't review the commits you were signing with the tag and that's were you made your mistake. Linus himself is against GPG signing commits. http://git.661346.n2.nabble.com/GPG-signing-for-git-commit-td2582986.html#a2583316 http://git.661346.n2.nabble.com/GPG-signing-for-git-commit-t... Ultimately a human being has to be responsible for reviewing code. Once it's reviewed, you can tag and sign it. No GPG signature is going to make up for not reviewing your code.
kevinpet 14y ago
I can't read this. If there's a vulnerability in the sequence of events described at the beginning, it needs to be called out more explicitly. If I wanted a mystery, I'd pick up some Agatha Christie.
Natsu 14y ago
This part bothers me: git log --pretty="format:%H$t%aN$t%s$t%G?" "${chkafter:-HEAD}" --first-parent \ | grep -v "${t}G$" What of the case where some smartass decides to $ git commit -am 'Yet another foo G' So long as it doesn't have a bad signature, it will give an empty string, right? So how do I know where the G came from?
- mikegerwitz 14y ago
  I do appreciate the attention to detail. Fortunately, even if "G" were appended to the commit message, grep would fail to match (and remove) it: a tab is output directly before %G? (which, as you mentioned, could be an empty string). This ensures that there will always be characters after the subject. That said, it could be terribly confusing when viewing the output. For that reason, perhaps another character after the tab (before the %G?) would be a good addition.
  - Natsu 14y ago
    Is there no way to sneak a tab into the message?
    - mikegerwitz 14y ago
      Indeed there is, but there will always be a tab after that tab. Consider this commit message: Foo\tG This would result in the following output: ... Foo\tG\t
      - Natsu 14y ago
        Ok, that makes more sense. It's hard to see whitespace, so I didn't notice a tab lurking just before the newline.
        Natsu 14y ago
        Wait... if I can sneak a tab in there, what if someone hid a newline in there, too? Would you not end up with foo\tG \t And break the regex? Or is there absolutely no way to smuggle a \n into the message?

163 points by mikegerwitz 14y ago

Thank you for taking the time to write this up in such detail and for making it an enjoyable read too.

This describes pretty exactly why security questions are hard. It's not as easy as using tool x and generate key y... its more than using software, if you want security, you have to think about the whole stack, down to the access rights that each part of your hardware has. There are good and best practices, but with IT Security, using the best practices in one place and just plain forgetting to unlock you PC when you go to the toilet at the office is more than enough... People are allowed to be afraid when asked security questions.

nknight 14y ago
> just plain forgetting to [lock] you PC when you go to the toilet You're not even trying. Physical access, game over, the lock is irrelevant. You fear what you don't understand, and you don't understand because you're afraid to learn. You have simply crafted a vicious cycle for yourself, and are making the world that much more unsafe by spreading your fear and acting as if everyone should share in your fear.
- Roritharr 14y ago
  I'm not spreading fear, since i consider HN a place where people are educated enough understand the problems in it security. Yes, once someone has hardware access, its devilishly hard to secure an environment, but just stating that its impossible is as wrong as security systems that ask their users too many questions to which they most likely have no good answer/no doubtfree answer. There are systems which work pretty securely, i have worked with several educated security experts to create security infrastructures in companies. I'm not saying that i don't understand the problems or don't want to learn solutions to them, i'm trying to defend the position of the user that can't be asked to learn about all the caveats of it security, since it is one of the most complex problems in computer science. No, not every front-end developer that has to check-in his html und js files can be asked to understand all principles of it security.

mdehaan 14y ago

Warning -- This article posts a longwinded first person account and finally ends with "this scenario is purely hypothetical". So it sounds like it is talking about a security vulnerability with git that really happened (signed commits or tags being broken, perhaps?), and it's actually an article saying you should use signed commits if you can't control access to your repo.

eykanal 14y ago

Interesting solution, but is this really a problem? The only situations I can think this applies to are large-scale open-source projects with many maintainers, and even more people submitting patches. For closed-source projects without external patch submissions, this is really only avoids the security threat of someone sitting at your computer and committing malicious code to your repository, which (to me) doesn't sound like much of a threat. (At least, if that is a threat, rogue commits to your git repo are only one part of your problem, and door locks would probably be a better, more broadly beneficial solution than implementing this.)

nknight 14y ago
Corporate networks get broken into all the time. Any machine that a copy of the repository is stored on is a potential attack vector. Various strategies are used to specifically protect all kinds of sensitive systems far beyond simple network security. This article is a detailed exploration of a/some strategy/ies for protecting your git repository against malicious modification -- a scenario that is rare but far from unprecedented.
wyck 14y ago
Yes of course, creating a backdoor or sabotage via revision control is a really good attack vector (like really). Recently several large open source projects had shady forged commits, so it's safe that say that anything private is also being targeted.
- MattJ100 14y ago
  Apart from Ruby, which?
  - Jebus 14y ago
    You mean Rails.
  - marshray 14y ago
    Wordpress http://wordpress.org/news/2011/06/passwords-reset/ http://wordpress.org/news/2011/06/passwords-reset/

roel_v 14y ago

I'm not sure what this 31 a4-pages story is saying beyond 'webs of trust are hard'. Nor am I sure what the first several long-winded pages add - they don't demonstrate a concrete problem, yet they are referenced to several times with the words 'as demonstrated by the story in the beginning, ...'. The summary at the bottom doesn't have a whole lot less information than all the content before it, and doesn't lead the reader to believe for the first 10 minutes that some ground-shaking vulnerability disclosure is imminent.

nknight 14y ago
It's a detailed exploration of git object signing. It's not trying to describe a "ground-shaking vulnerability". There is a current, known issue with display of git signature verifications, and he's including it in his article. That's not the point of the article. The point is an exploration of options for securing your repository history. That's it.
- roel_v 14y ago
  Well then maybe (actually, not "maybe", but "certainly") the title should be "a detailed exploration of git object signing", and not link bait like "a git horror story". But of course the first title, while much more accurate and informative, would not draw nearly as many website hits, so the author deliberately set out to mislead people with the title, in order to get more hits/reads. This sort of misrepresentation of content is what makes the web much harder to explore and the continuous stream of content much harder to keep on top of for the information consumer, and therefore should be called out and chastised. But then again, I'm just an old grumpy man who is much faster to criticize and be negative than he should be, so who am I to cast judgement? Do I ameliorate or exacerbate the situation with posts like my previous one or this one? I don't know, maybe I'm part of the problem in that way. Either way, I much preferred (so are my motivations selfish? Is that good or bad? I think good, but how do I know?) the old HN when submissions like this one would be labeled for what they were.
  - mikegerwitz 14y ago
    I'm sorry that you feel that way. You have understandably misinterpreted my intent. The title was not intended as bait, nor was the story. Rather, I enjoy writing and was hoping to appeal to readers that have difficulty reading dry material (which is the material I tend to write), while at the same time providing something to relate to in order to put the article into context. The title itself was a play on a television series --- American Horror Story --- that my girlfriend once fell asleep to as I was hacking late one night. The story was intended to augment the actual useful material (code samples and explanations), not obscure it, which it appears to have done. Please notice that this article is segregated from the rest of my blog with no glaring/obvious links to it. This was intentional. I posted this article in the hope that it would be useful to others, not because I would gain any benefit from doing so. I will take your comment into consideration for future articles.

raverbashing 14y ago

So what does this means is if someone compromise their GPG key (more or less easy) and get his passphrase (not difficult) they can sign all kinds of commits and make it look like he did it. Great Security is only as good as the weakest link. Also, don't make it look like it's more secure when it may not be

rmc 14y ago

tl;dr : You can GPG sign git commits with -S option. This makes things more secure

natep 14y ago
But it's not a cure-all
- rmc 14y ago
  Yes. "more secure" ≠ "100% secure"
  - natep 14y ago
    And sometimes, "more secure" == "less secure than before due to complacency"

asharp 14y ago

From what i've read, the problem is that an attacker can add commits with forced Author information into a central repository to frame somebody else. Wouldn't the signing of all commits as they are committed solve this problem? (ie. rather then trusting Author information from the commit, trust the signed-by information to give author information?)

mikegerwitz 14y ago
Note that Signed-off-by (as added by -s) is different than -S (GPG-sign). The -s option simply appends the "Signed-off-by" line to the commit message, so this can also be forged. The GPG signature cannot be forged (access to the private key is needed).

tytso 14y ago

The article puts forward a false dilemma by implying your only choices are (a) squashing all of the commits into a single patch, (b) examining the end point and then only signing the end point (with the risk that there might be some bad stuff hidden in some of the intermediate commits), and (c) examining and signing every single commit. In fact what we do in the kernel community is most patches are in fact reviewed via e-mail. This increases the likelihood more people will look at the patch and notice any potential problem, and more importantly, the e-mail archive introduces an additional external record of the proposed change. After each patch in a patch series is reviewed, in isolation, with each change small enough that it can be easily reviewed in a mail reader, and with especial attention made to maintaining bisectability, the maintainer who will ultimately by signing the git pull request will apply it into his or her tree. For larger changes and for larger subsystems where we have submaintainers, it is the maintainer's responsibility to make sure the submaintainer sends a signed pull request. At that point the submaintainer is assuming full responsibility for the set of patches which he or she is sending to the upper-level maintainer, just as the maintainer assumes responsibility for the patches he or she sends to Linus. In the case where the submaintainer does not send a signed pull, or the maintainer does not have full trust in the submaintainer (either for social or technical reasons), it is the responsibility for the maintainer to check each and every commit for correctness. If he or she can't do that, he shouldn't accept the pull request; instead, he might request that the patch series be sent to the mailing list so everyone can review the patch series. One of the things which follows from this is that for me as a kernel or e2fsprogs maintainer, github pull requests are not particularly useful. Most of the time I have no idea who the person is that initiated the pull request, because the primary community is on the mailing list. Worse, the pull request only gets sent to maintainer, which makes the maintainer the review bottleneck. If the patch series is sent to the mailing list, as individual commits, with no requirement to jump to a browser, it allows the review workload to be distributed to other developers on the mailing list. As far as I'm concerned, a github pull request is the same thing as a patch sent via private, personal e-mail. I'm going to review it very carefully, and only apply it if I know that it is clean. Typically the review involves extracting the commit as a diff, and as has been discussed in other places, github doesn't encourage properly formatted commit descriptions, so I end up needing to extract the diff, and then reapply it with an appropriate commit description. So for me github pull requests generally represent negative, not positive value, because of the quality control and quality assurance requirements of the code bases in which I work.

rmccue 14y ago

Wouldn't this be solved with a merge commit? If I can see where a commit merged in, I could prove that I didn't author it, as I don't have access to the other author's machine.

hdmi 14y ago

Very good and long read; should wake up a few people

bp2070 14y ago

An interesting article, but I would prefer the author presented the issues first in a VCS agnostic way (as they are not specific to git) and then introduce the hypothetical scenario using git. Actually these issues are not even specific to VCS, but security of most digital information.

dkarl 14y ago

Code gets reviewed for production at 6am by a guy freshly rousted out of bed who committed some of it four hours earlier, and who is doing this at the insistence of a "frantic" colleague, the fix involves "a bunch of commits" from more than one person, and his main worry is that he might be held responsible for a hidden back door maliciously inserted by his coworker? That's like driving home drunk with your headlights off and worrying that your car could get blown up by a drone because of eroding limits on the use of military force against civilians. I realize it's hypothetical, but man, fix your glaring problems first before you worry about unlikely scenarios.

heretohelp 14y ago
This does a great deal to explain why Facebook's release engineering ignores most attempts to push something out-of-band unless they can justify on the basis of real breakage for the users. (Cf. HotFix Bar)

crazydoggers 14y ago

`git tag -s` is all that's needed. The primary problem in your horror story is that you GPG signed a commit with a tag when you shouldn't have. When you tag with GPG, you are saying you do trust the history. You didn't review the commits you were signing with the tag and that's were you made your mistake. Linus himself is against GPG signing commits. http://git.661346.n2.nabble.com/GPG-signing-for-git-commit-td2582986.html#a2583316 http://git.661346.n2.nabble.com/GPG-signing-for-git-commit-t... Ultimately a human being has to be responsible for reviewing code. Once it's reviewed, you can tag and sign it. No GPG signature is going to make up for not reviewing your code.

kevinpet 14y ago

I can't read this. If there's a vulnerability in the sequence of events described at the beginning, it needs to be called out more explicitly. If I wanted a mystery, I'd pick up some Agatha Christie.

Natsu 14y ago

This part bothers me: git log --pretty="format:%H$t%aN$t%s$t%G?" "${chkafter:-HEAD}" --first-parent \ | grep -v "${t}G$" What of the case where some smartass decides to $ git commit -am 'Yet another foo G' So long as it doesn't have a bad signature, it will give an empty string, right? So how do I know where the G came from?

mikegerwitz 14y ago
I do appreciate the attention to detail. Fortunately, even if "G" were appended to the commit message, grep would fail to match (and remove) it: a tab is output directly before %G? (which, as you mentioned, could be an empty string). This ensures that there will always be characters after the subject. That said, it could be terribly confusing when viewing the output. For that reason, perhaps another character after the tab (before the %G?) would be a good addition.
- Natsu 14y ago
  Is there no way to sneak a tab into the message?
  - mikegerwitz 14y ago
    Indeed there is, but there will always be a tab after that tab. Consider this commit message: Foo\tG This would result in the following output: ... Foo\tG\t
    - Natsu 14y ago
      Ok, that makes more sense. It's hard to see whitespace, so I didn't notice a tab lurking just before the newline.
      - Natsu 14y ago
        Wait... if I can sneak a tab in there, what if someone hid a newline in there, too? Would you not end up with foo\tG \t And break the regex? Or is there absolutely no way to smuggle a \n into the message?