Skip to content
This repository has been archived by the owner on Nov 30, 2021. It is now read-only.

git submodules hook/support ala Heroku #1094

Closed
azurewraith opened this issue Jun 2, 2014 · 14 comments
Closed

git submodules hook/support ala Heroku #1094

azurewraith opened this issue Jun 2, 2014 · 14 comments

Comments

@azurewraith
Copy link

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...

@abecode
Copy link

abecode commented Dec 30, 2014

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).

@abecode
Copy link

abecode commented Dec 30, 2014

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...

@ianblenke
Copy link

This is duplicated in #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!

@paralin
Copy link

paralin commented Mar 5, 2015

+1 just ran into this issue.

@bacongobbler
Copy link
Member

closed but relevant: #3375

@bacongobbler
Copy link
Member

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.

@bburky
Copy link

bburky commented Feb 10, 2016

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.

@nailgun
Copy link

nailgun commented Nov 18, 2016

Is there some workaround except not using submodules?

@bacongobbler
Copy link
Member

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

@nailgun
Copy link

nailgun commented Nov 18, 2016

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

@bacongobbler
Copy link
Member

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.

@nailgun
Copy link

nailgun commented Nov 18, 2016

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

@nailgun
Copy link

nailgun commented Nov 22, 2016

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

@deis-admin
Copy link

This issue was moved to deis/builder#468

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

10 participants