From: "Ævar Arnfjörð Bjarmason" <firstname.lastname@example.org> To: "brian m. carlson" <email@example.com> Cc: Albert Cui <firstname.lastname@example.org>, Derrick Stolee <email@example.com>, Albert Cui via GitGitGadget <firstname.lastname@example.org>, email@example.com Subject: Re: [PATCH v2] hooks: propose project configured hooks Date: Wed, 07 Apr 2021 09:53:33 +0200 [thread overview] Message-ID: <firstname.lastname@example.org> (raw) In-Reply-To: <YGzrfaSC4xd75j2U@camp.crustytoothpaste.net> On Wed, Apr 07 2021, brian m. carlson wrote: > On 2021-04-05 at 22:45:10, Albert Cui wrote: >> Right, this entire proposal is trying to get to a "Git-blessed" solution, >> and I should make the need clearer. A few reasons for standardizing >> this come to mind: >> >> 1. Many existing "standard" solutions. A multitude of existing solutions for >> this use case speaks to the fact that a basic config script is not sufficient. >> I mentioned Husky above, but here are a few more; basically each >> popular programming language environment has a solution for this. >> >> https://github.com/sds/overcommit - Ruby >> https://github.com/pre-commit/pre-commit - Python >> https://github.com/Arkweid/lefthook - Go >> https://github.com/shibapm/Komondor - Swift >> https://github.com/typicode/husky - Node >> >> These solutions all handle the installation and updating of hooks. A >> "configure-hooks.sh" script doesn't handle hook updates, unless you go through >> the trouble yourself of implementing and maintaining that. > > I think part of the problem is that an automated process to update hooks > is generally a security vulnerability, since it means that untrusted > remote code will automatically run on your computer. > > I want to be clear that I understand the desire for this feature, even > though it's not a feature I would personally use, and the fact that > there are many approaches means that clearly there are many people that > do want this functionality. I have in the past shared hooks with others > and we have mutually benefitted enormously from that fact. My concerns > here are solely about the security aspects of this feature. > >> 3. Improving security. As you mentioned, hooks are difficult to get >> right from a security >> perspective, and standardizing on a single implementation allows us to >> give developers >> a well-vetted solution with a better security model than what exists >> today. For example, >> we're proposing making it very clear to users whenever there's a hook >> update. This isn't >> something that existing solutions do. > > I don't think this materially improves security. All of these options > have the same security problems, and that's inherent in the solution. > What we're doing here is basically giving people a built-in feature that > is the equivalent of piping curl to bash and blessing it as secure when > it's not. > >> I'll also say in general, the Git project is much more likely to get >> security right than smaller >> projects, where oftentimes even popular projects end up unmaintained. > > I agree that Git tries to be careful about security. It is for these > reasons that I think Derrick and I have provided you the feedback we > have here. > >> Agreed. We already did a security review internally at Google. The main >> feedback was: >> >> * We need an explicit opt-in opposed to setting hooks up automatically, >> e.g. a command line flag like --accept-hooks at minimum. This is primarily >> to distinguish people who are just cloning a repository to browse the code >> from people who are developing. >> >> * The average user doesn't have the ability to review hooks in general >> (security is hard and obscuration is easy), and if the user has >> already opted into >> this feature because they are engaged in development, it's very likely >> that they're >> already running build scripts, so the additional attack vector here doesn't seem >> like a big issue. > > I think you've hit the nail on the head here, but drawn a mistaken > conclusion. The average user doesn't have the ability to review hooks > in general and therefore cannot make an informed decision about whether > to enable them, so the behavior we need to have is not to lead them to > doing things which are risky from a security perspective. > > If my goal is to just build a product and not to run its tests, which I > do with a decent number of projects, then I can audit a Go or Rust > project trivially and determine if it executes arbitrary code or not > during the build process and if so, inspect it and gain confidence in > it. In fact, there are many projects which don't execute build scripts > during the process, and therefore which are completely safe. This hook > design changes that calculus dramatically. > > I also want to point out that people clone repositories for a variety of > reasons. At GitHub, every team has its own repository with > documentation. Literally every employee at the company, regardless of > role, interacts with a Git repository, even people who do normally > nontechnical tasks such as our in-house lawyers and our event planners. > Many of these people are nontechnical, and almost none of these > repositories has any software development involvement. There are also > numerous people elsewhere who may work on projects such as books or > other non-software in repositories who are nontechnical. Under the > current model, the biggest problem these people face is accidentally > corrupting their local repository and losing data. With a design that > prompts them to install hooks, they face the possibility of arbitrary > code execution. > > The reason I proposed the FAQ we have in our documentation is because I > answer a decent number of questions on Stack Overflow, in addition to > questions that involve users that I get pulled into at work. > Overwhelmingly, the vast majority of users, even developers, are not > completely comfortable with Git and are unsure about how to use it > effectively (cf. https://ohshitgit.com/). If we propose to a user that > they should do something like enable hooks by adding a prompt, many > users will automatically say "yes" because (a) they don't understand and > they trust that Git is prompting them to do something beneficial and (b) > because they don't know or care and just want to get on with their > lives. As a result, we're exposing people to giant social engineering > attacks on behalf of potentially unscrupulous repository maintainers. > > This is made worse by the fact that we will prompt users even when > cloning a repo that they have no intention of performing development on > means that we will have users who are misled here where otherwise > nothing would happen. > > There is a huge problem with social engineering attacks and phishing on > the Internet today and I'm concerned that this is going in exactly the > wrong direction. > > I would want to see a comprehensive security analysis feature taking > into consideration social engineering attacks, the skill level and > comfort with Git of the majority of Git users, and the fact that people > clone repositories for many reasons other than software development. > It's easy to look at this from the perspective of the typical employee > at a major tech company and assume that users are generally security > conscious, comfortable with Git, and primarily engaged in software > development on the projects they clone, but I'm not sure any of those > cases are generally true, and anyway there are many counterexamples in > the real world whose use cases we need to take into account. > > I continue to have serious reservations about this series and approach, > and I'm not sure that any proposal we can adopt here will address the > security concerns. To be frank, I don't think this proposal should move > forward in its current state or otherwise, since I think the security > problems are inherent in this approach and fundamentally can't be fixed. > > This is, as should be obvious from my email address, my personal > opinion, despite my reference to my employer above. Unless otherwise > stated, I don't speak for my employer and they don't speak for me. I agree with pretty much every word you said, in particular the social engineering aspect of this. In past mails I've referred to elsewhere I've proposed some Emacs-like "ask" facility for git, but you've convinced me that that default would be a bad idea for the "user just clicks yes no matter what" reasons you noted. Still, I do think there is a way to make this work in a way that's probably acceptable for everyone: * We don't ever ask the user to install hooks, it's something that they'd have to know about and pro-actively set up in advance. * The security model is entirely focused on not approving changes as you "pull" them, but e.g. GPG-verifying the whole chain with some pre-setup key. The use-case (and I've had this use-case in the past) would be something like a BigCorp which automates its servers/laptops, but would prefer not to patch/build/ship something like git itself. So when you "git clone" your corporate repos you get relevant config/hooks, but not otherwise. We'd of course have a way to discover that you can set these up & do so after "clone", but it would be something more like check-mailmap, not something we'd prompt you to do. I'm personally much more interested in doing something like this for an in-repo .gitconfig, with us shipping a graduals whitelist of known config values at differeng safety levels. That sort of thing really *is* something we could imagine asking the user about, or even doing by default, e.g. applying a "diff -U<n>" setting for that repo, picking up non-executable aliases for non-data-changing git programs etc. But as you note hooks are really on the extreme other side of that security curve, which is why in some earlier thread discussing this I suggested that a much more productive way to start an effort like this would be the in-repo .gitconfig route. We could start with our N most safe config variables, and work from there...
next prev parent reply other threads:[~2021-04-07 7:53 UTC|newest] Thread overview: 39+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-03-18 22:03 [PATCH] hooks: propose repository owner " Albert Cui via GitGitGadget 2021-03-18 22:29 ` Junio C Hamano 2021-03-18 23:45 ` Albert Cui 2021-03-19 1:28 ` brian m. carlson 2021-03-19 10:27 ` Ævar Arnfjörð Bjarmason 2021-04-06 0:35 ` Albert Cui 2021-04-07 22:47 ` Ævar Arnfjörð Bjarmason 2021-06-21 19:36 ` Jonathan Tan 2021-06-21 20:35 ` Ævar Arnfjörð Bjarmason 2021-03-26 1:43 ` [PATCH v2] hooks: propose project " Albert Cui via GitGitGadget 2021-03-29 23:20 ` Emily Shaffer 2021-04-01 20:02 ` Albert Cui 2021-03-30 15:24 ` Derrick Stolee 2021-04-05 22:45 ` Albert Cui 2021-04-05 23:09 ` Junio C Hamano 2021-04-05 23:40 ` Albert Cui 2021-04-06 0:13 ` Junio C Hamano 2021-04-06 0:27 ` Albert Cui 2021-04-06 23:15 ` brian m. carlson 2021-04-07 7:53 ` Ævar Arnfjörð Bjarmason [this message] 2021-04-07 13:09 ` Derrick Stolee 2021-04-07 18:40 ` Albert Cui 2021-04-07 20:02 ` Junio C Hamano 2021-04-07 22:23 ` Ævar Arnfjörð Bjarmason 2021-04-15 16:52 ` Ed Maste 2021-04-15 19:41 ` Junio C Hamano 2021-04-15 20:37 ` Ed Maste 2021-04-15 20:50 ` Junio C Hamano 2021-04-15 22:28 ` brian m. carlson 2021-04-02 9:59 ` Ævar Arnfjörð Bjarmason 2021-04-05 23:42 ` Albert Cui 2021-04-02 10:30 ` Ævar Arnfjörð Bjarmason 2021-04-03 0:58 ` Albert Cui 2021-04-24 1:38 ` [PATCH v3] " Albert Cui via GitGitGadget 2021-04-28 2:48 ` Junio C Hamano 2021-05-05 19:11 ` [PATCH v4] " Albert Cui via GitGitGadget 2021-06-03 3:31 ` Jonathan Tan 2021-06-03 20:16 ` Albert Cui 2021-06-03 22:10 ` Jonathan Tan
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style List information: http://vger.kernel.org/majordomo-info.html * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --subject='Re: [PATCH v2] hooks: propose project configured hooks' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
Code repositories for project(s) associated with this inbox: https://80x24.org/mirrors/git.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).