[Feature Proposal] List of paths to skip when checking host authorization

rubyonrails-core
1

Hi all,

I’m not quite sure where this belongs, and this is my first time posting here, so let me know if it belongs somewhere else (or if it’s already been discussed).

I’d like to have a list of paths to skip when checking the request host against the allow list. I have a Rails app with nginx running behind a load balancer on AWS, and the load balancer’s health check uses an endpoint in my application that just returns head :ok. I could respond to the health check with nginx, but I want it to tell me whether Rails is actually up and running. However, the host that Rails sees on that health check request is the load balancer’s IP address, which I can’t add to my config.hosts list because that would tie the code to the infrastructure. I basically just need a way to skip the host check for that one endpoint.

I monkey-patched the Rails code locally and it seems like this would be pretty easy to do. I added a @host_check_skip_paths array (open to feedback on the name) to Rails::Application::Configuration, and then I set

config.host_check_skip_paths << '/healthcheck'

in my application config. The default middleware stack then has this:

middleware.use ::ActionDispatch::HostAuthorization,
               config.hosts, config.host_check_skip_paths, config.action_dispatch.hosts_response_app

And then the HostAuthorization middleware has this:

    def initialize(app, hosts, host_check_skip_paths = [], response_app = nil)
      @app = app
      @host_check_skip_paths = host_skip_paths
    end

    private
      def authorized?(request)
        return true if @host_check_skip_paths.include?(request.path)
        
        # otherwise proceed as before
      end

Let me know what you think, or if there’s already a way to do this that I’m just missing. Haven’t had much luck searching for a solution. Thanks!

2

I think what you really want is the default behaviour of Rails anyway. The config.host parameter was added in Rails 6 to stop DNS rebinding attacks on development machines and the middleware that performs that check is disabled in production by default, since it doesn’t make sense to set that in production. See this blog post for more details: DNS rebinding attacks protection in Rails 6

What you really want is to not set that in production.rb at all so that the host check is not performed.

3

Ah. I guess maybe I misunderstood the purpose of the parameter. Isn’t it also useful (per this issue) to avoid host header injection, though? We’ve added custom middleware for that purpose in our Rails 5 app, and I was thinking we could ditch that in favor of the built-in version in Rails 6, which would mean we’d need it in production. Our custom middleware blindly allows the health check endpoint, which is what I was trying to recreate here, but if that’s not really the point of this feature, we can just keep using our own version.

4

Oh, right. That is an interesting use case for the config.host parameter I hadn’t considered.

Taking a look at the implementation of that feature in PR I can see that the parameter can be supplied a Proc for validating Host headers as well as Strings and Regexps (anything that responds to === basically). The only thing that is lacking there for your purposes though, is access to request in the context of that Proc. So I think a better way to add the feature you want, might be to special case how Procs are treated, so that they are explicitly called with the host value and to also start passing an optional request parameter, so that the proc can perform the filtering that you want. That way, you won’t have to add skip paths, etc with more conceptual overload.

5

Good point, thanks - that seems like a cleaner approach. I’ll try that, and hopefully I’ll get a little more feedback here.

6

That seems to work well enough. One catch is that, in order to provide that second request parameter without breaking things for people who already have this set up with lambdas, I think I’d need to do something like:

allowed === (allowed.is_a?(Proc) && allowed.arity > 1 ? [host, request] : host)

That feels like kind of a lot of proc-specific logic to me, but maybe that’s fine. I’m also not sure whether passing through the entire request (instead of just request.path, or maybe path and headers separately or something) is too much overhead, or whether it’s worth it for increased flexibility.

7

I opened a PR, since no one said it was a dumb idea (at least not yet…).