deis / builder

Git server and application builder for Deis Workflow
https://deis.com
MIT License
40 stars 41 forks source link

git submodules hook/support ala Heroku #468

Open deis-admin opened 7 years ago

deis-admin commented 7 years ago

From @azurewraith on June 2, 2014 20:18

I attempted to reproduce some Heroku'esque workflow involving a vendored Redmine plugin and found this:

https://devcenter.heroku.com/articles/git-submodules

90% sure it probably wouldn't have worked for me because git submodules are a PITA but it seems like Heroku handles this in their post-receive hook before delegating to the buildpack...

Copied from original issue: deis/deis#1094

deis-admin commented 7 years ago

From @abecode on December 30, 2014 2:51

This was also an issue I had. Based on the deis chat on IRC, it seems that the functionality would most likely involve the file builder/image/slugbuildr/build.sh (https://github.com/deis/deis/blob/master/builder/image/slugbuilder/builder/build.sh).

deis-admin commented 7 years ago

From @abecode on December 30, 2014 22:0

I managed to build a slug using the deis/slugbuilder using the git-archive-all script, https://github.com/meitar/git-archive-all.sh/blob/master/git-archive-all.sh , but I was unable to get the slug to be deployed. I probably won't have time to get to it this week so I wanted to leave a breadcrumb...

deis-admin commented 7 years ago

From @ianblenke on February 28, 2015 0:44

This is duplicated in deis#3160 - closing that in favor of this on @bacongobbler's suggestion.

We have a couple of projects that use submodules. The precise SHA1 is important to us.

deis-builder uses this in `./builder/image/templates/builder' to generate the tarball that is sent to the docker slugbuilder:

git archive $GIT_SHA | tar -xmC $TMP_DIR

Unfortunately, git archive strips the .git directory, and we need that for our submodules.

@bacongobbler mentioned gh1094 as related to this, and rightfully explained that submodules are also an issue on Heroku.

To add .git, I've had to modify our deis/builder image to do this instead:

git ls-files --cached --full-name --no-empty-directory -z $GIT_SHA | tar cf - -T - --null |  tar -xmC $TMP_DIR

It would be nice to somehow flag an app to selectively include .git, as no doubt others will run into this limitation.

Thanks!

deis-admin commented 7 years ago

From @paralin on March 5, 2015 3:49

+1 just ran into this issue.

deis-admin commented 7 years ago

From @bacongobbler on May 4, 2015 20:32

closed but relevant: https://github.com/deis/deis/pull/3375

deis-admin commented 7 years ago

From @bacongobbler on February 10, 2016 0:18

Just an FYI by bburky on IRC:

16:05:00 <bburky> Hey, can I ask you someting about Deis?
16:09:05 <bacongobbler> sure
16:09:14 <bacongobbler> what's up? :)
16:10:08 <bburky> Something that I'm fairly certian isn't actually a security issue, but some stuff I wanted to make sure about
16:10:26 <bburky> For starters, deis is currently NOT multi tenant at all right?
16:10:52 <bacongobbler> correct. We explicitly say that in the docs that it's not intended to be deployed in a multi-tenant environment
16:11:12 <bburky> Cool
16:11:35 <bburky> And also how isolated/unprivledged is slugbuilder?
16:12:07 <bburky> I'm asking because I discovered CVE-2015-7545 awhile ago. Cloning git submodules could run arbitrary code
16:12:08 <bburky> https://github.com/deis/deis/blob/4caec61b033eaa06fa42e93fe8fac14db262ebb8/builder/rootfs/usr/local/src/slugbuilder/builder/build.sh#L115
16:12:49 <bacongobbler> unprivileged? it runs inside a docker container so it's isolated within its own environment. We also have no git submodule support so we're not affected
16:12:51 <bburky> Basically that `git submodule update --quiet --recursive` could run arbitrary code, but you do the clone within slugbuilder, and it's already untrusted/user code anyways
16:12:53 <bburky> Right?
16:13:14 <bburky> So this only applies if you're privledged as far as I know. So this is a non-issue
16:13:47 <bburky> The line I linked to looks like you do a submodule update on the buildpack?
16:13:52 <bacongobbler> bingo, but it would be a good idea to point out this CVE in https://github.com/deis/deis/issues/1094 :)
16:14:08 <bburky> Ah, didn't see that post
16:14:54 <bacongobbler> yep! however at that point it's inside the slugbuilder container, so the environment is completely isolated from the platform.
16:14:57 <bburky> Also, FYI I haven't actually tested this against Deis as I don't actually use it myself. Just trying to make sure that people who are building push-to-deploy things know that git isn't magically perfectly secure
16:15:01 <bburky> Yep
16:15:07 <bburky> I think you're fine.
16:15:14 <bacongobbler> essentially they're in a mini sandbox so the surface is good :)
16:15:20 <bacongobbler> attack surface*
16:15:24 <bburky> Yep
16:15:44 <bacongobbler> thanks for pointing this out!
16:15:46 <bburky> And if you have privledges to push (SSH? api key?) you have root anyway?
16:16:22 <bburky> (Or not, I really don't know how deis works)
16:16:28 <bacongobbler> if you have privileges to run `git push deis master`, you're logged in as the `git` user, so you're not a root user
16:16:46 <bburky> Ah, ok.
16:17:33 <bacongobbler> https://github.com/deis/deis/blob/29b6c0b2aab51739cd2a128f7fd5dbbb6b305809/client/pkg/git/git.go#L109
16:17:35 <bburky> But you can already get arbitrary code run within an app, so the ability to run code is irrelavent.
16:18:01 <bacongobbler> bingo :)
16:18:21 <bburky> Yay for being unaffected.

So we will need to keep this in mind if someone wants to implement git submodule support.

deis-admin commented 7 years ago

From @bburky on February 10, 2016 0:31

Because Deis is not multi-tenant, in practice Deis was unaffected. It might have been possible for code to be run while fetching a buildpack, but at that point the user is privileged and could just run arbitrary code within their app anyways. And slugbuilder is an isolated Docker container, so impact was minimal anyways.

But the real lesson here is that please treat git clone and fetching/checking out users' repos as untrusted. You probably never want to fetch repos for unprivileged users. CVE-2015-7545 affected recursive fetching of submodules and CVE-2014-9390 would have been terrible except everyone's servers run on Linux. So just please continue to do git clone within an isolated environment, and carefully consider git if you ever decide to make Deis multi-tenant.

deis-admin commented 7 years ago

From @nailgun on November 18, 2016 16:25

Is there some workaround except not using submodules?

deis-admin commented 7 years ago

From @bacongobbler on November 18, 2016 16:34

You could deploy your application using a Dockerfile or through deis pull. Is that what you mean?

deis-admin commented 7 years ago

From @nailgun on November 18, 2016 17:24

Thanks. Didn't know about deis pull. Does it support SSH_KEY config var?

deis-admin commented 7 years ago

From @bacongobbler on November 18, 2016 17:28

deis pull fetches a docker image from a registry so SSH_KEY does not apply here. It will go into the image when it runs, however.

deis-admin commented 7 years ago

From @nailgun on November 18, 2016 17:42

Ah. I thought it is some pull mode for builder, instead of push. Actually Dockerfile approach negates the most beautiful part of Deis...

deis-admin commented 7 years ago

From @nailgun on November 22, 2016 15:49

I've ended up with using git subtrees instead of submodules.

Cryptophobia commented 6 years ago

This issue was moved to teamhephy/builder#23