Consider removing `X_REWRITE_URL` due to potential security issues

[nyurik]

Apparently many systems used to support x-rewrite-url header, and they were dropped due to security concerns. Should we drop it too, or have a different workaround? I wonder if this is CORS related?

Relevant links:

• https://www.acunetix.com/vulnerabilities/web/url-rewrite-vulnerability/
• https://symfony.com/blog/cve-2018-14773-remove-support-for-legacy-and-risky-http-headers

CC for wider discussion @stepankuzmin @lseelenbinder @HarelM @birkskyum

[lseelenbinder]

It could affect CORS, but that's usually the Origin header + Access-Control-*headers. I haven't ever seen it used in practice in my experience, so I'm not quite sure the intended purpose (other than perhaps providing a base URL)?

Generally speaking, I think martin would be running behind some cache or reverse proxy, which allows users to remove/set the header before it hits martin code. Perhaps the best option is it should live behind a command-line flag?

[nyurik]

Note that Martin generates tilejson, which must have an absolute URL to the tiles per spec. Which means if Martin lives behind a proxy, it must still know the external endpoint, i.e. by looking at some proxy-provided headers...

[lseelenbinder]

by looking at some proxy-provided headers...

For sure. I've typically done this via x-forwarded-for, etc. headers (and then you only allow them set by internal systems).

[stepankuzmin]

The X-Rewrite-URL header is used to rewrite the tiles and TileJSON endpoints in the reverse proxy. We can drop support for this header, but we must reflect on how to do it with the X-Forwarded-For header and communicate it back to the community.

[nyurik]

@stepankuzmin turns out actix already handles forwarded and x-forwarded-prot/host headers - see https://github.com/actix/actix-web/blob/cfd40b4f1547629b2023430da55798fd942a0c42/actix-web/src/info.rs#L71

So safe to remove the whole hack outright.

[nyurik]

I kept thinking about this, and it seems we may need to keep it. For some silly reason, I though X-Rewrite-URL covers the same use case as Forwarded, which is totally not the case - instead, it is a URL rewrite, not host/port rewrite.

I think we may want to make this an opt-in feature rather than always on:

# in CLI
martin --allow-url-rewrite

# in config.yaml
allow_url_rewrite: true

[nyurik]

@maxammann and I spoke about this some more, and have the following use cases:

• proxy may pass forwarded / x-forwarded-proto/host / host headers which will always modify generated tilejson automatically, this is already being done by Actix - no changes needed.
• some proxies may rewrite URLs, e.g. external http://.../tiles/my_tiles is forwarded to Martin http://.../my_tiles. If proxy can remember the original URL /tiles/my_tiles as a x-rewrite-url header, Martin can use it.
  • x-rewrite-url is similar in concept to forwarded headers, but while host, port, and protocol forwarding are built into various systems, the URL forwarding is not, likely due to some uncertain security concerns - so it might be really good for us to move away from it.
• we may achieve the same feature with config: url_prefix: "/tiles" -- this would allow proxies to remove URL rewriting

[nyurik]

The #552 implements an opt-in support for x-rewrite-url - this should alleviate the issue for most of the security-concerns. Moving forward, we could offer some other means of rewrite, or disable it, etc

[nyurik]

Won't implement per discussion above