request.local? fails with Dockerized application about active-elastic-job HOT 9 CLOSED

@RyanNaughton @tawan instead of making a local check optional, which is a security hole, how about allowing a configuration variable be set that holds the allowed/trusted network. For example (not proposed code, just to give an example):

# Assuming the configuration pattern is implemented.
ActiveElasticJob.configure do |c|
  c.allowed_network = "172.17.0.0/16"
end

def in_allowed_network?(ip)
  IPAddr.new(ActiveElasticJob.config.allowed_network).include?(ip)
end

# In the wild.
unless request.local? || in_allowed_network?(request.ip)
  m = "Accepts only requests from localhost for job processing".freeze
  return ['403', {CONTENT_TYPE_HEADER_NAME => 'text/plain' }, [ m ]]
end

Sorry for the messy code, just letting the idea flow and typing on my phone.

from active-elastic-job.

@RyanNaughton Thanks for the submission. Sorry for my late reply, I was on the road the recent days.

I'm thinking of introducing an optional environment variable, that overrides the check for a local IP. In your case, would the remote_ip always resolve to 172.17.0.1 or does it change between deployments?

from active-elastic-job.

@RyanNaughton 📟 ping. Any thoughts on PR #30?

from active-elastic-job.

@RyanNaughton @tawan I am under the impression we could handle this gracefully. There is no need to check for a custom ip address. The problem is actually forbiding a non-private ip address to deliver requests to the worker. It is just a matter of following RFC1918.

Follow this link: https://github.com/ipaddress-gem/ipaddress/blob/master/lib/ipaddress/ipv4.rb
Go to line 618, there you find an example implementation that would filter the correct ip addresses and avoid forcing the user to configure yet another thing.

from active-elastic-job.

@zaaroth 👍

from active-elastic-job.

@zaaroth I did have a concern with this idea initially, since all EC2 instances have a private IP assigned. I was thinking that an unwanted message could get through from an unauthorized instance 🔓 using this method. However, if I'm not mistaken, Elastic Beanstalk creates a 🔒 VPC for the instances to run in correct? If so, this should't be a concern. 😌

Other than that, I do like this idea. It's more elegant and like you said no more steps needed to allow a local network -- it's taken care of.

@tawan what do you think? If you like it, I can amend PR #30.

from active-elastic-job.

@zaaroth updated PR #30

from active-elastic-job.

@mdoza @zaaroth Thanks for your input and @mdoza thank you for your PR.
I took a step back and thought again about why this issue was actually created. The problem only exists if the application is deployed in a Docker environment. I think we got carried away a little be too much when we were thinking about allowing a user configured network. We actually only have to deal with the Docker case. The docs of Docker make it clear that if not configured otherwise, than the host will have the IP 172.17.0.1. So if we check if the request comes from this particular IP and if the application runs inside a Docker container, then this should be sufficient. See my commit 68711d9.

What do you think?

from active-elastic-job.

@tawan sehr gut. Stays within the scope of fixing the original issue. Simple and elegant. 👍

from active-elastic-job.