Amber: Remove x-powered-by header

Created on 14 Jan 2019 · 8Comments · Source: amberframework/amber

Description

Submitting this primarily as a discussion/RFC before spooling up a PR, but I think it would probably be a good idea to remove the x-powered-by header that is returned on every request. The primary line of thinking is that if there's ever a security bug found within amber/crystal then amber at least wouldn't advertise what it is, protecting it from automated vulnerability scanners.

Steps to Reproduce

curl -I localhost:3000

Versions

All/Current

Additional Information

Some prior-art and rationale:

Express JS docs: https://expressjs.com/en/advanced/best-practice-security.html#at-a-minimum-disable-x-powered-by-header
- Worth noting the sub-heading of the section highlighting express's severity; _At a minimum_, disable X-Powered-By header.
Troy Hunt article: https://www.troyhunt.com/shhh-dont-let-your-response-headers/

Now why express includes this header by default anyway, I'm not sure. It would also be possible to disable this only in production but I'm not sure of the additional value.

question

Source

jackturnbull

All 8 comments

Hi x-powered-by header is optional, you can disable it by commenting plug Amber::Pipe::PoweredByAmber.new on config/routes.cr.

Also used by other crystal frameworks like Kemal by @sdogruyol

https://github.com/kemalcr/kemal/blob/eed97877a7ad28eabbf6fd7fa352fe62ade0ddb6/src/kemal/init_handler.cr#L10

This optional header is used by tools like Wappanalyzer to detect pages built using amber

See: https://github.com/AliasIO/Wappalyzer/pull/1784

faustinoaq on 19 Jan 2019

It would also be possible to disable this only in production but I'm not sure of the additional value.

You can use Amber.env to avoid it only in production, by example:

Amber::Server.configure do
  pipeline :web do
    if Amber.env != "production"
      plug Amber::Pipe::PoweredByAmber.new
    end
    # other plugs ...
  end

  # Other pipelines and routes...
end

faustinoaq on 19 Jan 2019

Thanks for the response! I might not have explained my point well enough in that I realise that it can be removed on an individual level, it may not be responsible to have the middleware generared on a new Amber project (so it may want to be removed, either just the middleware or from the project entirely).

The if wrapper in your above example may have been roughly what I'd expect to see in a generated new project and feels more recommeneded from a security point of view so it may be an idea to add that to the generator?

jackturnbull on 22 Jan 2019

👍1

We could add a --skip-powered-by-amber-middleware or --skip-powered-by-amber-pipe flag to the generator, however at that point it's better to just remove it manually. I don't think it's an unreasonable default. Now if we were including Crystal or Amber version numbers in the headers that'd be another story. It's true if you know a site is using Amber you could only try Amber specific exploits however at that point you should focus on fixing vulnerabilities, not hiding that you are vulnerable.

irlcatgirl on 6 May 2019

@jackturnbull we understand the concern but I would have to agree that removing the plug Amber::Pipe::PoweredByAmber.new line from the project is very easy to do. Removing it from the generator will be more time consuming.

eliasjpr on 12 Sep 2019

@eliasjpr If we want to consider removing it from the generator https://github.com/amberframework/amber/pull/1134 is a good start.

irlcatgirl on 12 Sep 2019

@eliasjpr Agreed.

It's a minor issue to the topic of almost being bikeshedding - I probably shouldn't have raised it hah! I think that what to some might be a benefit (being identifiable as Amber), is to me a disadvantage and something undesirable.

So with that I'll say that your response is balanced enough. It's been there long enough where anyone security concious to this degree will probably spot it anyway.

Since it's mainly me that requested this change I'll close this with the solution;