Set user/Event IP addresses more accurately.

.rubocop.yml

@@ -8,7 +8,7 @@ Metrics/ClassLength:
   CountComments: false
 
 Metrics/AbcSize:
-  Max: 50
+  Max: 53
 
 Metrics/CyclomaticComplexity:
   Max: 13
@@ -20,7 +20,7 @@ Metrics/LineLength:
   Max: 155
 
 Metrics/MethodLength:
-  Max: 34
+  Max: 37
 
 Style/SignalException:
   Enabled: false

lib/raven/base.rb

@@ -19,6 +19,7 @@
 require 'raven/interfaces/stack_trace'
 require 'raven/interfaces/http'
 require 'raven/utils/deep_merge'
+require 'raven/utils/real_ip'
 require 'raven/instance'
 
 require 'forwardable'

lib/raven/event.rb

@@ -58,6 +58,10 @@ def initialize(init = {})
         end
       end
 
+      if @context.rack_env
+        @context.user[:ip_address] = calculate_real_ip_from_rack
+      end
+
       init.each_pair { |key, val| instance_variable_set('@' + key.to_s, val) }
 
       @user = @context.user.merge(@user)
@@ -254,5 +258,18 @@ class << self
       alias capture_exception from_exception
       alias capture_message from_message
     end
+
+    private
+
+    # When behind a proxy (or if the user is using a proxy), we can't use
+    # REMOTE_ADDR to determine the Event IP, and must use other headers instead.
+    def calculate_real_ip_from_rack
+      Utils::RealIp.new(
+        :remote_addr => context.rack_env["REMOTE_ADDR"],
+        :client_ip => context.rack_env["HTTP_CLIENT_IP"],
+        :real_ip => context.rack_env["HTTP_X_REAL_IP"],
+        :forwarded_for => context.rack_env["HTTP_X_FORWARDED_FOR"]
+      ).calculate_ip
+    end
   end
 end

lib/raven/utils/real_ip.rb

@@ -0,0 +1,62 @@
+require 'ipaddr'
+
+# Based on ActionDispatch::RemoteIp. All security-related precautions from that
+# middleware have been removed, because the Event IP just needs to be accurate,
+# and spoofing an IP here only makes data inaccurate, not insecure. Don't re-use
+# this module if you have to *trust* the IP address.
+module Raven
+  module Utils
+    class RealIp
+      LOCAL_ADDRESSES = [
+        "127.0.0.1",      # localhost IPv4
+        "::1",            # localhost IPv6
+        "fc00::/7",       # private IPv6 range fc00::/7
+        "10.0.0.0/8",     # private IPv4 range 10.x.x.x
+        "172.16.0.0/12",  # private IPv4 range 172.16.0.0 .. 172.31.255.255
+        "192.168.0.0/16", # private IPv4 range 192.168.x.x
+      ].map { |proxy| IPAddr.new(proxy) }
+
+      attr_accessor :ip, :ip_addresses
+
+      def initialize(ip_addresses)
+        self.ip_addresses = ip_addresses
+      end
+
+      def calculate_ip
+        # CGI environment variable set by Rack
+        remote_addr = ips_from(ip_addresses[:remote_addr]).last
+
+        # Could be a CSV list and/or repeated headers that were concatenated.
+        client_ips    = ips_from(ip_addresses[:client_ip])
+        real_ips      = ips_from(ip_addresses[:real_ip])
+        forwarded_ips = ips_from(ip_addresses[:forwarded_for])
+
+        ips = [client_ips, real_ips, forwarded_ips, remote_addr].flatten.compact
+
+        # If every single IP option is in the trusted list, just return REMOTE_ADDR
+        self.ip = filter_local_addresses(ips).first || remote_addr
+      end
+
+      protected
+
+      def ips_from(header)
+        # Split the comma-separated list into an array of strings
+        ips = header ? header.strip.split(/[,\s]+/) : []
+        ips.select do |ip|
+          begin
+            # Only return IPs that are valid according to the IPAddr#new method
+            range = IPAddr.new(ip).to_range
+            # we want to make sure nobody is sneaking a netmask in
+            range.begin == range.end
+          rescue ArgumentError
+            nil
+          end
+        end
+      end
+
+      def filter_local_addresses(ips)
+        ips.reject { |ip| LOCAL_ADDRESSES.any? { |proxy| proxy === ip } }
+      end
+    end
+  end
+end

spec/raven/event_spec.rb

@@ -130,6 +130,8 @@
                          'HTTP_HOST' => 'localhost',
                          'SERVER_NAME' => 'localhost',
                          'SERVER_PORT' => '80',
+                         'HTTP_X_FORWARDED_FOR' => '1.1.1.1, 2.2.2.2',
+                         'REMOTE_ADDR' => '192.168.1.1',
                          'PATH_INFO' => '/lol',
                          'rack.url_scheme' => 'http',
                          'rack.input' => StringIO.new('foo=bar'))
@@ -147,13 +149,17 @@
 
     it "adds http data" do
       expect(hash[:request]).to eq(:data => { 'foo' => 'bar' },
-                                   :env => { 'SERVER_NAME' => 'localhost', 'SERVER_PORT' => '80' },
-                                   :headers => { 'Host' => 'localhost' },
+                                   :env => { 'SERVER_NAME' => 'localhost', 'SERVER_PORT' => '80', "REMOTE_ADDR" => "192.168.1.1" },
+                                   :headers => { 'Host' => 'localhost', "X-Forwarded-For" => "1.1.1.1, 2.2.2.2" },
                                    :method => 'POST',
                                    :query_string => 'biz=baz',
                                    :url => 'http://localhost/lol',
                                    :cookies => nil)
     end
+
+    it "sets user context ip address correctly" do
+      expect(hash[:user][:ip_address]).to eq("1.1.1.1")
+    end
   end
 
   context "rack context, long body" do

spec/raven/utils/real_ip_spec.rb

@@ -0,0 +1,78 @@
+require 'spec_helper'
+
+describe Raven::Utils::RealIp do
+  context "when no ip addresses are provided other than REMOTE_ADDR" do
+    subject { Raven::Utils::RealIp.new(:remote_addr => "1.1.1.1") }
+
+    it "should return the remote_addr" do
+      expect(subject.calculate_ip).to eq("1.1.1.1")
+    end
+  end
+
+  context "when a list of x-forwarded-for ips is provided" do
+    subject do
+      Raven::Utils::RealIp.new(
+        :forwarded_for => "192.168.0.2, 2.2.2.2, 3.3.3.3, 4.4.4.4",
+        :remote_addr => "192.168.0.1"
+      )
+    end
+
+    it "should return the oldest ancestor that is not a local IP" do
+      expect(subject.calculate_ip).to eq("2.2.2.2")
+    end
+  end
+
+  context "when client/real ips are provided" do
+    subject do
+      Raven::Utils::RealIp.new(
+        :forwarded_for => "2.2.2.2",
+        :real_ip => "4.4.4.4",
+        :client_ip => "3.3.3.3",
+        :remote_addr => "192.168.0.1"
+      )
+    end
+
+    it "should return the oldest ancestor, preferring client/real ips first" do
+      expect(subject.calculate_ip).to eq("3.3.3.3")
+    end
+  end
+
+  context "all provided ip addresses are actually local addresses" do
+    subject do
+      Raven::Utils::RealIp.new(
+        :forwarded_for => "127.0.0.1, ::1, 10.0.0.0",
+        :remote_addr => "192.168.0.1"
+      )
+    end
+
+    it "should return REMOTE_ADDR" do
+      expect(subject.calculate_ip).to eq("192.168.0.1")
+    end
+  end
+
+  context "when an invalid IP is provided" do
+    subject do
+      Raven::Utils::RealIp.new(
+        :forwarded_for => "4.4.4.4.4, 2.2.2.2",
+        :remote_addr => "192.168.0.1"
+      )
+    end
+
+    it "return the eldest valid IP" do
+      expect(subject.calculate_ip).to eq("2.2.2.2")
+    end
+  end
+
+  context "with IPv6 ips" do
+    subject do
+      Raven::Utils::RealIp.new(
+        :forwarded_for => "2001:db8:a0b:12f0::1",
+        :remote_addr => "192.168.0.1"
+      )
+    end
+
+    it "return the eldest valid IP" do
+      expect(subject.calculate_ip).to eq("2001:db8:a0b:12f0::1")
+    end
+  end
+end