cactus / go-camo

A secure image proxy server
MIT License
255 stars 48 forks source link

Making content-type configurable #20

Closed j-manu closed 6 years ago

j-manu commented 6 years ago

Is it possible to make content type configurable so that we can use go-camo to proxy non image resources also? Right now it is hardcoded here - https://github.com/cactus/go-camo/blob/3a39055ae5e56c443cd7cbc688ac6734073745ee/src/go-camo/camo/proxy.go#L207

dropwhile commented 6 years ago

As you noted, currently only images types are proxied. The original goal of go-camo was to reimplement github's camo service (which as I recall also only proxies images) in Go, to provide better performance and scalability. Later some additional features (base64 encoding, http/2, some filtering support, etc) were added, to provide some additional functionality and/or robustness.

In regards to your specific question...In theory, different content types could be made to work similarly (audio/, video/, css, etc). However, there is some danger with supporting certain content types. For example, allowing proxying of JS places the context of execution under the proxy domain. This could be a severe security issue for many sites. There may be some remediations possible via content security policy, but that certainly increases complexity and is an additional burden on the user of go-camo to get things "just right".

Is there a specific content type you are looking to proxy, aside from images?

j-manu commented 6 years ago

We wanted to proxy CSS. What I ideally want is to allow overriding of what content-type is whitelisted. So go-camo can default to image/ but if a whitelist is supplied by the user, then it uses that instead.

Camo allows only images by default but it is possible to edit the mime-types.json and add the mime types you want - https://github.com/atmos/camo/blob/e59df56a01c023850962fac16905269d264fba50/server.coffee#L20

I don't see a reduction in security with this approach since it has to be done explicitly. Also since all URLs have to be signed that is an additional barrier any attacker.

MrSaints commented 6 years ago

@j-manu We had a similar interest, and we decided to fork this project to expand on the features (see: https://github.com/arachnys/go-camo, it is fully Dockerized too). There's quite a significant deviation with what we are doing which is why I've decided against merging my changes back upstream for now (unless there's interest in doing so of course! I'm more than happy to do so).

dropwhile commented 6 years ago

@j-manu as camo type servers are /usually/ intended to be used for user submitted content, I am leery of adding content types that expose the execution context (css, js, etc).

Other external content types generally aren't loaded into the frame (pdf, mp4, mp3), and are usually either uploaded or just linked, so generally shouldn't generate content warnings. I could maybe see an argument for fonts, but the likelihood of them being user supplied AND desired seems low.

If your use-case requires user submitted css as reference only, non-uploaded (eg. you are not letting users upload css to s3 which you can secure with ssl on its own) content, then maybe @MrSaints fork would be a better option for you to look at.

dropwhile commented 6 years ago

@MrSaints some of the features do look both useful and interesting -- termination signals and sentry support, for example.

I think I am still too leery of adding font and stylesheet proxying to add those features in at this time.

If you would like, I could link your fork in the go-camo readme, so people desiring that additional content type proxying functionality wouldn't have to hunt around for it.

MrSaints commented 6 years ago

@cactus I agree for the most part. And I'd probably maintain that direction too if it was my project (that is, only proxy images). In our case, we added it because of a very special / niche use case.

I'd be pleased if you included a link :) I'm also happy to port some changes over (e.g. Sentry).

j-manu commented 6 years ago

@cactus Yeah. I think I will use the @MrSaints fork.

dropwhile commented 6 years ago

@MrSaints I added a reference in the readme here.

If you have the time, a pull req for termination signals and/or sentry support would indeed be much appreciated. 👍


Closing this ticket.