From 70440ae6476cae534a4787e07c9dc61a2856d9fe Mon Sep 17 00:00:00 2001 From: Bar Weiss Date: Tue, 30 Aug 2022 15:26:13 +0300 Subject: [PATCH 01/14] Add support for `http.client_ip` tag for Rack-based frameworks --- lib/datadog/core/configuration/settings.rb | 21 ++ lib/datadog/tracing/client_ip.rb | 191 +++++++++++ lib/datadog/tracing/configuration/ext.rb | 6 + lib/datadog/tracing/contrib/rack/header.rb | 35 ++ .../tracing/contrib/rack/middlewares.rb | 27 +- lib/datadog/tracing/metadata/ext.rb | 1 + spec/datadog/tracing/client_ip_spec.rb | 321 ++++++++++++++++++ .../tracing/contrib/rack/headers_spec.rb | 36 ++ 8 files changed, 626 insertions(+), 12 deletions(-) create mode 100644 lib/datadog/tracing/client_ip.rb create mode 100644 lib/datadog/tracing/contrib/rack/header.rb create mode 100644 spec/datadog/tracing/client_ip_spec.rb create mode 100644 spec/datadog/tracing/contrib/rack/headers_spec.rb diff --git a/lib/datadog/core/configuration/settings.rb b/lib/datadog/core/configuration/settings.rb index 3cb412c04f0..bb4f127825a 100644 --- a/lib/datadog/core/configuration/settings.rb +++ b/lib/datadog/core/configuration/settings.rb @@ -618,6 +618,27 @@ def initialize(*_) # @default `{}` # @return [Hash,nil] option :writer_options, default: ->(_i) { {} }, lazy: true + + # Client IP configuration + # @public_api + settings :client_ip do + # Whether client IP collection is disabled. This disables client IPs from HTTP requests to be reported in traces. + # + # @default `DD_TRACE_CLIENT_IP_HEADER_DISABLED` environment variable, otherwise `false`. + # @return [Boolean] + option :disabled do |o| + o.default { env_to_bool(Tracing::Configuration::Ext::ClientIp::ENV_DISABLED, false) } + o.lazy + end + # An optional name of a custom header to resolve the client IP from. + # + # @default `DD_TRACE_CLIENT_IP_HEADER` environment variable, otherwise `nil`. + # @return [String,nil] + option :header_name do |o| + o.default { ENV.fetch(Tracing::Configuration::Ext::ClientIp::ENV_HEADER_NAME, nil) } + o.lazy + end + end end # The `version` tag in Datadog. Use it to enable [Deployment Tracking](https://docs.datadoghq.com/tracing/deployment_tracking/). diff --git a/lib/datadog/tracing/client_ip.rb b/lib/datadog/tracing/client_ip.rb new file mode 100644 index 00000000000..04580f56ca6 --- /dev/null +++ b/lib/datadog/tracing/client_ip.rb @@ -0,0 +1,191 @@ +# typed: true + +require 'ipaddr' + +require_relative '../core/configuration' +require_relative 'metadata/ext' +require_relative 'span' + +module Datadog + module Tracing + # Common functions for supporting the `http.client_ip` span attribute. + module ClientIp + DEFAULT_IP_HEADERS_NAMES = %w[ + x-forwarded-for + x-real-ip + x-client-ip + x-forwarded + x-cluster-client-ip + forwarded-for + forwarded + via + true-client-ip + ].freeze + + # A collection of headers. + class HeaderCollection + # Gets a single value of the header with the given name, case insensitive. + # + # @param [String] header_name Name of the header to get the value of. + # @returns [String, nil] A single value of the header, or nil if the header with + # the given name is missing from the collection. + def get(header_name) + nil + end + + def self.from_hash(hash) + HashHeaderCollection.new(hash) + end + end + + # Sets the `http.client_ip` tag on the given span. + # + # This function respects the user's settings: if they disable the client IP tagging, + # or provide a different IP header name. + # + # @param [Span] span The span that's associated with the request. + # @param [HeaderCollection, #get, nil] headers A collection with the request headers. + # @param [String, nil] remote_ip The remote IP the request associated with the span is sent to. + def self.set_client_ip_tag(span, headers, remote_ip) + return if configuration.disabled + + ip = client_address_from_request(headers, remote_ip) + if !configuration.header_name && ip.nil? + header_names = ip_headers(headers).keys + span.set_tag(TAG_MULTIPLE_IP_HEADERS, header_names.join(',')) unless header_names.empty? + return + end + + unless valid_ip?(ip) + ip = extract_ip_from_full_address(ip) + return unless valid_ip?(ip) + end + ip = strip_zone_specifier(ip) if valid_ipv6?(ip) + + span.set_tag(Tracing::Metadata::Ext::HTTP::TAG_CLIENT_IP, ip) + end + + TAG_MULTIPLE_IP_HEADERS = '_dd.multiple-ip-headers'.freeze + + # A header collection implementation that looks up headers in a Hash. + class HashHeaderCollection < HeaderCollection + def initialize(hash) + super() + @hash = hash.transform_keys(&:downcase) + end + + def get(header_name) + @hash[header_name.downcase] + end + end + + def self.ip_headers(headers) + return {} unless headers + + {}.tap do |result| + DEFAULT_IP_HEADERS_NAMES.each do |name| + value = headers.get(name) + next if value.nil? + + result[name] = value + end + end + end + + def self.client_address_from_request(headers, remote_ip) + return headers.get(configuration.header_name) if configuration.header_name && headers + + ip_values_from_headers = ip_headers(headers).values + case ip_values_from_headers.size + when 0 + remote_ip + when 1 + ip_values_from_headers.first + end + end + + def self.strip_zone_specifier(ipv6) + if /\A(.*?)%.*/ =~ ipv6 + return Regexp.last_match(1) + end + + ipv6 + end + + # Extracts the IP part from a full address (`ipv4:port` or `[ipv6]:port`). + # + # @param [String] address Full address to split + # @returns [String] The IP part of the full address. + def self.extract_ip_from_full_address(address) + if /\A\[(.*)\]:\d+\Z/ =~ address + return Regexp.last_match(1) + end + + if /\A(.*):\d+\Z/ =~ address + return Regexp.last_match(1) + end + + address + end + + def self.configuration + Datadog.configuration.tracing.client_ip + end + + # Determines whether the given IP is valid. + # + # @param [String] ip The IP to validate. + # @returns [Boolean] + def self.valid_ip?(ip) + valid_ipv4?(ip) || valid_ipv6?(ip) + end + + # --- Section vendored from the ipaddress gem --- # + + # rubocop:disable Layout/LineLength, Style/SpecialGlobalVars + + # + # Checks if the given string is a valid IPv4 address + # + # Example: + # + # IPAddress::valid_ipv4? "2002::1" + # #=> false + # + # IPAddress::valid_ipv4? "172.16.10.1" + # #=> true + # + # Vendored from `ipaddress` gem from file 'lib/ipaddress.rb', line 198. + def self.valid_ipv4?(addr) + if /^(0|[1-9]{1}\d{0,2})\.(0|[1-9]{1}\d{0,2})\.(0|[1-9]{1}\d{0,2})\.(0|[1-9]{1}\d{0,2})$/ =~ addr + return $~.captures.all? { |i| i.to_i < 256 } + end + + false + end + + # + # Checks if the given string is a valid IPv6 address + # + # Example: + # + # IPAddress::valid_ipv6? "2002::1" + # #=> true + # + # IPAddress::valid_ipv6? "2002::DEAD::BEEF" + # #=> false + # + # Vendored from `ipaddress` gem from file 'lib/ipaddress.rb', line 230. + def self.valid_ipv6?(addr) + # https://gist.github.com/cpetschnig/294476 + # http://forums.intermapper.com/viewtopic.php?t=452 + if /^\s*((([0-9A-Fa-f]{1,4}:){7}([0-9A-Fa-f]{1,4}|:))|(([0-9A-Fa-f]{1,4}:){6}(:[0-9A-Fa-f]{1,4}|((25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(\.(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3})|:))|(([0-9A-Fa-f]{1,4}:){5}(((:[0-9A-Fa-f]{1,4}){1,2})|:((25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(\.(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3})|:))|(([0-9A-Fa-f]{1,4}:){4}(((:[0-9A-Fa-f]{1,4}){1,3})|((:[0-9A-Fa-f]{1,4})?:((25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(\.(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3}))|:))|(([0-9A-Fa-f]{1,4}:){3}(((:[0-9A-Fa-f]{1,4}){1,4})|((:[0-9A-Fa-f]{1,4}){0,2}:((25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(\.(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3}))|:))|(([0-9A-Fa-f]{1,4}:){2}(((:[0-9A-Fa-f]{1,4}){1,5})|((:[0-9A-Fa-f]{1,4}){0,3}:((25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(\.(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3}))|:))|(([0-9A-Fa-f]{1,4}:){1}(((:[0-9A-Fa-f]{1,4}){1,6})|((:[0-9A-Fa-f]{1,4}){0,4}:((25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(\.(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3}))|:))|(:(((:[0-9A-Fa-f]{1,4}){1,7})|((:[0-9A-Fa-f]{1,4}){0,5}:((25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(\.(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3}))|:)))(%.+)?\s*$/ =~ addr + return true + end + + false + end + # rubocop:enable Layout/LineLength, Style/SpecialGlobalVars + end + end +end diff --git a/lib/datadog/tracing/configuration/ext.rb b/lib/datadog/tracing/configuration/ext.rb index 3682ee3f851..e5f40ae38b5 100644 --- a/lib/datadog/tracing/configuration/ext.rb +++ b/lib/datadog/tracing/configuration/ext.rb @@ -45,6 +45,12 @@ module Transport ENV_DEFAULT_PORT = 'DD_TRACE_AGENT_PORT'.freeze ENV_DEFAULT_URL = 'DD_TRACE_AGENT_URL'.freeze end + + # @public_api + module ClientIp + ENV_DISABLED = 'DD_TRACE_CLIENT_IP_HEADER_DISABLED'.freeze + ENV_HEADER_NAME = 'DD_TRACE_CLIENT_IP_HEADER'.freeze + end end end end diff --git a/lib/datadog/tracing/contrib/rack/header.rb b/lib/datadog/tracing/contrib/rack/header.rb new file mode 100644 index 00000000000..2367142593a --- /dev/null +++ b/lib/datadog/tracing/contrib/rack/header.rb @@ -0,0 +1,35 @@ +require_relative '../../client_ip' + +module Datadog + module Tracing + module Contrib + module Rack + # Classes and utilities for handling headers in Rack. + module Header + # An implementation of a header collection that looks up headers from a Rack environment. + class RequestHeaderCollection < Datadog::Tracing::ClientIp::HeaderCollection + # Creates a header collection from a rack environment. + def initialize(env) + super() + @env = env + end + + # Gets the value of the header with the given name. + def get(header_name) + @env[Header.header_to_rack_header(header_name)] + end + + # Tests whether a header with the given name exists in the environment. + def key?(header_name) + @env.key?(Header.header_to_rack_header(header_name)) + end + end + + def self.header_to_rack_header(name) + "HTTP_#{name.to_s.upcase.gsub(/[-\s]/, '_')}" + end + end + end + end + end +end diff --git a/lib/datadog/tracing/contrib/rack/middlewares.rb b/lib/datadog/tracing/contrib/rack/middlewares.rb index 80eb5248865..fae6589ae78 100644 --- a/lib/datadog/tracing/contrib/rack/middlewares.rb +++ b/lib/datadog/tracing/contrib/rack/middlewares.rb @@ -3,12 +3,14 @@ require 'date' require_relative '../../../core/environment/variable_helpers' +require_relative '../../client_ip' require_relative '../../metadata/ext' require_relative '../../propagation/http' require_relative '../analytics' +require_relative '../utils/quantization/http' require_relative 'ext' +require_relative 'header' require_relative 'request_queue' -require_relative '../utils/quantization/http' module Datadog module Tracing @@ -133,8 +135,9 @@ def set_request_tags!(trace, request_span, env, status, headers, response, origi # So when its not available, we want the original, unmutated PATH_INFO, which # is just the relative path without query strings. url = env['REQUEST_URI'] || original_env['PATH_INFO'] - request_headers = parse_request_headers(env) - response_headers = parse_response_headers(headers || {}) + request_header_collection = Header::RequestHeaderCollection.new(env) + request_headers_tags = parse_request_headers(request_header_collection) + response_headers_tags = parse_response_headers(headers || {}) # The priority # 1. User overrides span.resource @@ -177,6 +180,10 @@ def set_request_tags!(trace, request_span, env, status, headers, response, origi ) end + if request_span.get_tag(Tracing::Metadata::Ext::HTTP::TAG_CLIENT_IP).nil? + Tracing::ClientIp.set_client_ip_tag(request_span, request_header_collection, env['REMOTE_ADDR']) + end + if request_span.get_tag(Tracing::Metadata::Ext::HTTP::TAG_BASE_URL).nil? request_obj = ::Rack::Request.new(env) @@ -195,12 +202,12 @@ def set_request_tags!(trace, request_span, env, status, headers, response, origi end # Request headers - request_headers.each do |name, value| + request_headers_tags.each do |name, value| request_span.set_tag(name, value) if request_span.get_tag(name).nil? end # Response headers - response_headers.each do |name, value| + response_headers_tags.each do |name, value| request_span.set_tag(name, value) if request_span.get_tag(name).nil? end @@ -219,13 +226,13 @@ def configuration Datadog.configuration.tracing[:rack] end - def parse_request_headers(env) + def parse_request_headers(headers) {}.tap do |result| whitelist = configuration[:headers][:request] || [] whitelist.each do |header| rack_header = header_to_rack_header(header) - if env.key?(rack_header) - result[Tracing::Metadata::Ext::HTTP::RequestHeaders.to_tag(header)] = env[rack_header] + if headers.key?(rack_header) + result[Tracing::Metadata::Ext::HTTP::RequestHeaders.to_tag(header)] = headers[rack_header] end end end @@ -248,10 +255,6 @@ def parse_response_headers(headers) end end end - - def header_to_rack_header(name) - "HTTP_#{name.to_s.upcase.gsub(/[-\s]/, '_')}" - end end end end diff --git a/lib/datadog/tracing/metadata/ext.rb b/lib/datadog/tracing/metadata/ext.rb index 96b6569b724..cd1219d9d18 100644 --- a/lib/datadog/tracing/metadata/ext.rb +++ b/lib/datadog/tracing/metadata/ext.rb @@ -68,6 +68,7 @@ module HTTP TYPE_OUTBOUND = 'http' TYPE_PROXY = 'proxy' TYPE_TEMPLATE = 'template' + TAG_CLIENT_IP = 'http.client_ip' # General header functionality module Headers diff --git a/spec/datadog/tracing/client_ip_spec.rb b/spec/datadog/tracing/client_ip_spec.rb new file mode 100644 index 00000000000..7a456fdc6b2 --- /dev/null +++ b/spec/datadog/tracing/client_ip_spec.rb @@ -0,0 +1,321 @@ +require 'spec_helper' + +require 'datadog/tracing/client_ip' +require 'datadog/tracing/metadata/ext' + +RSpec.describe Datadog::Tracing::ClientIp do + subject(:client_ip) { described_class } + let(:tagging_enabled) { true } + let(:ip_header_name) { nil } + + before do + Datadog.configure do |c| + c.tracing.client_ip.disabled = !tagging_enabled + c.tracing.client_ip.header_name = ip_header_name + end + end + + after do + without_warnings { Datadog.configuration.reset! } + end + + describe '#valid_ip?' do + context 'when given valid ip addresses' do + subject do + [ + '10.0.0.0', + '10.0.0.1', + 'FEDC:BA98:7654:3210:FEDC:BA98:7654:3210', + '1080:0000:0000:0000:0008:0800:200C:417A', + '1080:0:0:0:8:800:200C:417A', + '1080:0::8:800:200C:417A', + '1080::8:800:200C:417A', + 'FF01:0:0:0:0:0:0:43', + 'FF01:0:0::0:0:43', + 'FF01::43', + '0:0:0:0:0:0:0:1', + '0:0:0::0:0:1', + '::1', + '0:0:0:0:0:0:0:0', + '0:0:0::0:0:0', + '::', + 'fe80::208:74ff:feda:625c', + 'ff80:03:02:01::' + ] + end + + it { is_expected.to all(satisfy { |address| client_ip.valid_ip?(address) }) } + end + + context 'when given invalid ip addresses' do + subject do + [ + '10.0.0.256', + '10.0.0.0.0', + '10.0.0', + '10.0', + '0.0.0.0/0', + '10.0.0.1/24', + '10.0.0.1/255.255.255.0', + ':1:2:3:4:5:6:7', + ':1:2:3:4:5:6:7', + '2002:516:2:200', + 'dd', + '2001:db8::8:800:200c:417a/64', + '02001:0000:1234:0000:0000:C1C0:ABCD:0876', + '2001:0000:1234:0000:00001:C1C0:ABCD:0876' + ] + end + + it { is_expected.to_not include(satisfy { |address| client_ip.valid_ip?(address) }) } + end + end + + describe '#set_client_ip_tag' do + let(:span) do + instance_double('Span') + end + + context 'when disabled' do + let(:tagging_enabled) { false } + + it 'does nothing' do + expect(span).to_not receive(:set_tag).with(any_args) + client_ip.set_client_ip_tag(span, nil, '1.1.1.1') + end + end + + context 'when configured with custom header name' do + let(:ip_header_name) { 'My-Custom-Header' } + let(:span) do + instance_double('Span') + end + + it 'ignores default header names' do + headers = client_ip::HeaderCollection.from_hash({ 'X-Forwarded-For' => '1.1.1.1' }) + + expect(span).to_not receive(:set_tag).with(any_args) + client_ip.set_client_ip_tag(span, headers, nil) + end + + it 'uses custom header value as client ip' do + headers = client_ip::HeaderCollection.from_hash({ 'My-Custom-Header' => '1.1.1.1' }) + + expect(span).to receive(:set_tag).with(Datadog::Tracing::Metadata::Ext::HTTP::TAG_CLIENT_IP, '1.1.1.1') + client_ip.set_client_ip_tag(span, headers, nil) + end + + it 'does nothing if custom header value is not a valid ip' do + headers = client_ip::HeaderCollection.from_hash({ 'My-Custom-Header' => '1.11.1' }) + + expect(span).to_not receive(:set_tag).with(any_args) + client_ip.set_client_ip_tag(span, headers, nil) + end + + it 'does not use other headers if custom header value is not a valid ip' do + headers = client_ip::HeaderCollection.from_hash( + { + 'My-Custom-Header' => '1.11.1', + 'X-Forwarded-For' => '1.11.1' + } + ) + + expect(span).to_not receive(:set_tag).with(any_args) + client_ip.set_client_ip_tag(span, headers, nil) + end + + it 'does not use remote ip if custom header value is not a vaild ip' do + headers = client_ip::HeaderCollection.from_hash( + { + 'My-Custom-Header' => '1.11.1', + } + ) + + expect(span).to_not receive(:set_tag).with(any_args) + client_ip.set_client_ip_tag(span, headers, '1.1.1.1') + end + + it 'prefers ip from custom header over remote ip' do + headers = client_ip::HeaderCollection.from_hash({ 'My-Custom-Header' => '1.1.1.1' }) + + expect(span).to receive(:set_tag).with(Datadog::Tracing::Metadata::Ext::HTTP::TAG_CLIENT_IP, '1.1.1.1') + client_ip.set_client_ip_tag(span, headers, '2.2.2.2') + end + end + + context 'when single ip header is present' do + it 'uses value from header as client ip' do + headers = client_ip::HeaderCollection.from_hash({ 'X-Forwarded-For' => '1.1.1.1' }) + + expect(span).to receive(:set_tag).with(Datadog::Tracing::Metadata::Ext::HTTP::TAG_CLIENT_IP, '1.1.1.1') + client_ip.set_client_ip_tag(span, headers, nil) + end + + it 'does nothing if header value is not a valid ip' do + headers = client_ip::HeaderCollection.from_hash({ 'X-Forwarded-For' => '1.11.1' }) + + expect(span).to_not receive(:set_tag).with(any_args) + client_ip.set_client_ip_tag(span, headers, nil) + end + + it 'does not use remote ip if header value is not a valid ip' do + headers = client_ip::HeaderCollection.from_hash({ 'X-Forwarded-For' => '1.11.1' }) + + expect(span).to_not receive(:set_tag).with(any_args) + client_ip.set_client_ip_tag(span, headers, '1.1.1.1') + end + + it 'prefers ip from header over remote ip' do + headers = client_ip::HeaderCollection.from_hash({ 'X-Forwarded-For' => '1.1.1.1' }) + + expect(span).to receive(:set_tag).with(Datadog::Tracing::Metadata::Ext::HTTP::TAG_CLIENT_IP, '1.1.1.1') + client_ip.set_client_ip_tag(span, headers, '2.2.2.2') + end + end + + context 'when multiple ip headers are present' do + it 'sets multiple ip headers tag only' do + headers = client_ip::HeaderCollection.from_hash( + { + 'X-Forwarded-For' => '1.1.1.1', + 'X-Client-Ip' => '2.2.2.2' + } + ) + + expect(span).to_not receive(:set_tag).with(Datadog::Tracing::Metadata::Ext::HTTP::TAG_CLIENT_IP, anything) + expect(span).to receive(:set_tag).with(client_ip::TAG_MULTIPLE_IP_HEADERS, 'x-forwarded-for,x-client-ip') + client_ip.set_client_ip_tag(span, headers, nil) + end + + it 'sets multiple ip headers tag only even if all ips are the same' do + headers = client_ip::HeaderCollection.from_hash( + { + 'X-Forwarded-For' => '1.1.1.1', + 'X-Client-Ip' => '1.1.1.1' + } + ) + + expect(span).to_not receive(:set_tag).with(Datadog::Tracing::Metadata::Ext::HTTP::TAG_CLIENT_IP, anything) + expect(span).to receive(:set_tag).with(client_ip::TAG_MULTIPLE_IP_HEADERS, 'x-forwarded-for,x-client-ip') + client_ip.set_client_ip_tag(span, headers, nil) + end + + it 'prefers multiple ip headers tag only over remote ip' do + headers = client_ip::HeaderCollection.from_hash( + { + 'X-Forwarded-For' => '1.1.1.1', + 'X-Client-Ip' => '2.2.2.2' + } + ) + + expect(span).to_not receive(:set_tag).with(Datadog::Tracing::Metadata::Ext::HTTP::TAG_CLIENT_IP, anything) + expect(span).to receive(:set_tag).with(client_ip::TAG_MULTIPLE_IP_HEADERS, 'x-forwarded-for,x-client-ip') + client_ip.set_client_ip_tag(span, headers, '3.3.3.3') + end + + it 'includes ip headers with invalid ips in multiple ip headers tag' do + headers = client_ip::HeaderCollection.from_hash( + { + 'X-Forwarded-For' => '1.1.1.1', + 'X-Client-Ip' => '2.2.2.2.3', + 'X-Real-Ip' => '3.3.3.3' + } + ) + + expect(span).to_not receive(:set_tag).with(Datadog::Tracing::Metadata::Ext::HTTP::TAG_CLIENT_IP, anything) + expect(span).to receive(:set_tag).with(client_ip::TAG_MULTIPLE_IP_HEADERS, 'x-forwarded-for,x-real-ip,x-client-ip') + client_ip.set_client_ip_tag(span, headers, nil) + end + + it 'includes ip headers with invalid ips in multiple ip headers tag even if exactly one ip is valid' do + headers = client_ip::HeaderCollection.from_hash( + { + 'X-Forwarded-For' => '1.1.1.1', + 'X-Client-Ip' => '2.22.2', + 'X-Real-Ip' => 'dd' + } + ) + + expect(span).to_not receive(:set_tag).with(Datadog::Tracing::Metadata::Ext::HTTP::TAG_CLIENT_IP, anything) + expect(span).to receive(:set_tag).with(client_ip::TAG_MULTIPLE_IP_HEADERS, 'x-forwarded-for,x-real-ip,x-client-ip') + client_ip.set_client_ip_tag(span, headers, nil) + end + end + + context 'when no ip headers are present' do + let(:headers) { client_ip::HeaderCollection.from_hash({}) } + + it 'uses remote ip as client ip as fallback' do + expect(span).to receive(:set_tag).with(Datadog::Tracing::Metadata::Ext::HTTP::TAG_CLIENT_IP, '1.1.1.1') + client_ip.set_client_ip_tag(span, headers, '1.1.1.1') + end + + it 'does nothing if remote ip is invalid' do + expect(span).to_not receive(:set_tag).with(any_args) + client_ip.set_client_ip_tag(span, headers, '1.11.1') + end + end + + context 'when non-ip headers are present' do + let(:headers) do + client_ip::HeaderCollection.from_hash( + { + 'Accept' => '*/*', + 'Authorization' => 'Bearer XXXXXX' + } + ) + end + + it 'uses remote ip as client ip as fallback' do + expect(span).to receive(:set_tag).with(Datadog::Tracing::Metadata::Ext::HTTP::TAG_CLIENT_IP, '1.1.1.1') + client_ip.set_client_ip_tag(span, headers, '1.1.1.1') + end + + it 'does nothing if remote ip is invalid' do + expect(span).to_not receive(:set_tag).with(any_args) + client_ip.set_client_ip_tag(span, headers, '1.11.1') + end + end + + context 'when ip' do + it 'is plain ipv4' do + expect(span).to receive(:set_tag).with(Datadog::Tracing::Metadata::Ext::HTTP::TAG_CLIENT_IP, '1.1.1.1') + client_ip.set_client_ip_tag(span, nil, '1.1.1.1') + end + + it 'is plain ipv6' do + expect(span).to receive(:set_tag).with( + Datadog::Tracing::Metadata::Ext::HTTP::TAG_CLIENT_IP, + '2001:db8::8a2e:370:7334' + ) + client_ip.set_client_ip_tag(span, nil, '2001:db8::8a2e:370:7334') + end + + it 'is ipv4 and port' do + expect(span).to receive(:set_tag).with(Datadog::Tracing::Metadata::Ext::HTTP::TAG_CLIENT_IP, '1.1.1.1') + client_ip.set_client_ip_tag(span, nil, '1.1.1.1:8080') + end + + it 'is ipv6 and port' do + expect(span).to receive(:set_tag).with( + Datadog::Tracing::Metadata::Ext::HTTP::TAG_CLIENT_IP, + '2001:db8::8a2e:370:7334' + ) + client_ip.set_client_ip_tag(span, nil, '[2001:db8::8a2e:370:7334]:8080') + end + + it 'is ipv6 with interface id' do + expect(span).to receive(:set_tag).with( + Datadog::Tracing::Metadata::Ext::HTTP::TAG_CLIENT_IP, + '2001:db8::8a2e:370:7334' + ) + client_ip.set_client_ip_tag(span, nil, '2001:db8::8a2e:370:7334%eth0') + end + + it 'is bracketed ipv6 without port' do + expect(span).to_not receive(:set_tag).with(any_args) + client_ip.set_client_ip_tag(span, nil, '[2001:db8::8a2e:370:7334]') + end + end + end +end diff --git a/spec/datadog/tracing/contrib/rack/headers_spec.rb b/spec/datadog/tracing/contrib/rack/headers_spec.rb new file mode 100644 index 00000000000..4180f58ea05 --- /dev/null +++ b/spec/datadog/tracing/contrib/rack/headers_spec.rb @@ -0,0 +1,36 @@ +require 'datadog/tracing/contrib/rack/headers' + +RSpec.describe Datadog::Tracing::Contrib::Rack::HeaderCollection do + subject(:collection) { described_class.new env } + let(:env) { {} } + + describe '#get' do + context 'when header exists in env' do + let(:env) do + { + 'HTTP_X_FORWARDED_FOR' => 'me' + } + end + + it 'returns header value' do + expect(collection.get('X-Forwarded-For')).to eq('me') + end + + it 'returns header value regardless of letter casing in the name' do + expect(collection.get('x-forwarded-for')).to eq('me'.freeze) + end + end + + context 'when header does not exists in env' do + let(:env) do + { + 'HTTP_USER_AGENT' => 'test' + } + end + + it 'returns nil' do + expect(collection.get('X-Forwarded-For')).to be_nil + end + end + end +end From cbfca125e87f9da65f2fdd2765985a93acd02691 Mon Sep 17 00:00:00 2001 From: Bar Weiss Date: Tue, 30 Aug 2022 23:24:10 +0300 Subject: [PATCH 02/14] Address @lloeki's great review comments --- lib/datadog/core/configuration/settings.rb | 1 + lib/datadog/core/header_collection.rb | 39 ++++ lib/datadog/tracing/client_ip.rb | 216 ++++++++---------- .../rack/{header.rb => header_collection.rb} | 10 +- .../tracing/contrib/rack/middlewares.rb | 17 +- spec/datadog/core/header_collection_spec.rb | 36 +++ spec/datadog/tracing/client_ip_spec.rb | 67 ++++-- ...ders_spec.rb => header_collection_spec.rb} | 4 +- 8 files changed, 234 insertions(+), 156 deletions(-) create mode 100644 lib/datadog/core/header_collection.rb rename lib/datadog/tracing/contrib/rack/{header.rb => header_collection.rb} (72%) create mode 100644 spec/datadog/core/header_collection_spec.rb rename spec/datadog/tracing/contrib/rack/{headers_spec.rb => header_collection_spec.rb} (83%) diff --git a/lib/datadog/core/configuration/settings.rb b/lib/datadog/core/configuration/settings.rb index bb4f127825a..02f3d3f812f 100644 --- a/lib/datadog/core/configuration/settings.rb +++ b/lib/datadog/core/configuration/settings.rb @@ -630,6 +630,7 @@ def initialize(*_) o.default { env_to_bool(Tracing::Configuration::Ext::ClientIp::ENV_DISABLED, false) } o.lazy end + # An optional name of a custom header to resolve the client IP from. # # @default `DD_TRACE_CLIENT_IP_HEADER` environment variable, otherwise `nil`. diff --git a/lib/datadog/core/header_collection.rb b/lib/datadog/core/header_collection.rb new file mode 100644 index 00000000000..55362001e2e --- /dev/null +++ b/lib/datadog/core/header_collection.rb @@ -0,0 +1,39 @@ +module Datadog + module Core + # A some-what abstract class representing a collection of headers. + # + # Use the `HeaderCollection.from_hash` function to create a header collection from a `Hash`. + # Another option is to use `HashHeaderCollection` directly. + class HeaderCollection + # Gets a single value of the header with the given name, case insensitive. + # + # @param [String] header_name Name of the header to get the value of. + # @returns [String, nil] A single value of the header, or nil if the header with + # the given name is missing from the collection. + def get(header_name) + nil + end + + # Create a header collection that retrieves headers from the given Hash. + # + # This can be useful for testing or other trivial use cases. + # + # @param [Hash] hash Hash with the headers. + def self.from_hash(hash) + HashHeaderCollection.new(hash) + end + end + + # A header collection implementation that looks up headers in a Hash. + class HashHeaderCollection < HeaderCollection + def initialize(hash) + super() + @hash = hash.transform_keys(&:downcase) + end + + def get(header_name) + @hash[header_name.downcase] + end + end + end +end diff --git a/lib/datadog/tracing/client_ip.rb b/lib/datadog/tracing/client_ip.rb index 04580f56ca6..8136a99484c 100644 --- a/lib/datadog/tracing/client_ip.rb +++ b/lib/datadog/tracing/client_ip.rb @@ -22,26 +22,16 @@ module ClientIp true-client-ip ].freeze - # A collection of headers. - class HeaderCollection - # Gets a single value of the header with the given name, case insensitive. - # - # @param [String] header_name Name of the header to get the value of. - # @returns [String, nil] A single value of the header, or nil if the header with - # the given name is missing from the collection. - def get(header_name) - nil - end - - def self.from_hash(hash) - HashHeaderCollection.new(hash) - end - end + TAG_MULTIPLE_IP_HEADERS = '_dd.multiple-ip-headers'.freeze # Sets the `http.client_ip` tag on the given span. # # This function respects the user's settings: if they disable the client IP tagging, - # or provide a different IP header name. + # or provide a different IP header name. + # + # If multiple IP headers are present in the request, this function will instead set + # the `_dd.multiple-ip-headers` tag with the names of the present headers, + # and **NOT** set the `http.client_ip` tag. # # @param [Span] span The span that's associated with the request. # @param [HeaderCollection, #get, nil] headers A collection with the request headers. @@ -49,143 +39,131 @@ def self.from_hash(hash) def self.set_client_ip_tag(span, headers, remote_ip) return if configuration.disabled - ip = client_address_from_request(headers, remote_ip) - if !configuration.header_name && ip.nil? - header_names = ip_headers(headers).keys - span.set_tag(TAG_MULTIPLE_IP_HEADERS, header_names.join(',')) unless header_names.empty? - return - end + begin + address = raw_ip_from_request(headers, remote_ip) + if address.nil? + # `address` can be `nil` if a custom header is configured but not present in the request. + # In that case, assume misconfiguration and avoid setting the tag. + return + end - unless valid_ip?(ip) - ip = extract_ip_from_full_address(ip) - return unless valid_ip?(ip) - end - ip = strip_zone_specifier(ip) if valid_ipv6?(ip) + ip = strip_decorations(address) + + validate_ip(ip) - span.set_tag(Tracing::Metadata::Ext::HTTP::TAG_CLIENT_IP, ip) + span.set_tag(Tracing::Metadata::Ext::HTTP::TAG_CLIENT_IP, ip) + rescue InvalidIpError + # Do nothing, assuming logs will spam here. + rescue MultipleIpHeadersError => e + span.set_tag(TAG_MULTIPLE_IP_HEADERS, e.header_names.join(',')) + end end - TAG_MULTIPLE_IP_HEADERS = '_dd.multiple-ip-headers'.freeze + # Returns the value of an IP-related header or the request's remote IP. + # + # The client IP is looked up by the following logic: + # * If the user has configured a header name, return that header's value. + # * If exactly one of the known IP headers is present, return that header's value. + # * If none of the known IP headers are present, return the remote IP from the request. + # + # Raises a [MultipleIpHeadersError] if multiple IP-related headers are present. + # + # @param [Datadog::Core::HeaderCollection, #get, nil] headers The request headers + # @param [String] remote_ip The remote IP of the request. + # @return [String] An unprocessed value retrieved from an + # IP header or the remote IP of the request. + def self.raw_ip_from_request(headers, remote_ip) + return headers && headers.get(configuration.header_name) if configuration.header_name - # A header collection implementation that looks up headers in a Hash. - class HashHeaderCollection < HeaderCollection - def initialize(hash) - super() - @hash = hash.transform_keys(&:downcase) - end + headers_present = ip_headers(headers) - def get(header_name) - @hash[header_name.downcase] + case headers_present.size + when 0 + remote_ip + when 1 + headers_present.values.first + else + raise MultipleIpHeadersError, headers_present.keys end end - def self.ip_headers(headers) - return {} unless headers + # Removes any port notations or zone specifiers from the IP address without + # verifying its validity. + def self.strip_decorations(address) + return strip_ipv4_port(address) if likely_ipv4?(address) - {}.tap do |result| - DEFAULT_IP_HEADERS_NAMES.each do |name| - value = headers.get(name) - next if value.nil? + address = strip_ipv6_port(address) - result[name] = value - end - end + strip_zone_specifier(address) end - def self.client_address_from_request(headers, remote_ip) - return headers.get(configuration.header_name) if configuration.header_name && headers + def self.strip_zone_specifier(ipv6) + ipv6.gsub(/%.*/, '') + end - ip_values_from_headers = ip_headers(headers).values - case ip_values_from_headers.size - when 0 - remote_ip - when 1 - ip_values_from_headers.first - end + def self.strip_ipv4_port(ip) + ip.gsub(/:\d+\z/, '') end - def self.strip_zone_specifier(ipv6) - if /\A(.*?)%.*/ =~ ipv6 - return Regexp.last_match(1) + def self.strip_ipv6_port(ip) + if /\[(.*)\](?::\d+)?/ =~ ip + Regexp.last_match(1) + else + ip end - - ipv6 end - # Extracts the IP part from a full address (`ipv4:port` or `[ipv6]:port`). + # Returns whether the given value is more likely to be an IPv4 than an IPv6 address. # - # @param [String] address Full address to split - # @returns [String] The IP part of the full address. - def self.extract_ip_from_full_address(address) - if /\A\[(.*)\]:\d+\Z/ =~ address - return Regexp.last_match(1) - end + # This is done by checking if a dot (`'.'`) character appears before a colon (`':'`) in the value. + # The rationale is that in valid IPv6 addresses, colons will always preced dots, + # and in valid IPv4 addresses dots will always preced colons. + def self.likely_ipv4?(value) + dot_index = value.index('.') || value.size + colon_index = value.index(':') || value.size + + dot_index < colon_index + end - if /\A(.*):\d+\Z/ =~ address - return Regexp.last_match(1) + def self.validate_ip(ip) + # IPs with netmasks are invalid. + raise InvalidIpError if ip.include?('/') + + begin + IPAddr.new(ip) + rescue IPAddr::Error + raise InvalidIpError end + end + + def self.ip_headers(headers) + return {} unless headers - address + DEFAULT_IP_HEADERS_NAMES.reduce({}) do |result, name| + value = headers.get(name) + result[name] = value unless value.nil? + + result + end end def self.configuration Datadog.configuration.tracing.client_ip end - # Determines whether the given IP is valid. - # - # @param [String] ip The IP to validate. - # @returns [Boolean] - def self.valid_ip?(ip) - valid_ipv4?(ip) || valid_ipv6?(ip) + class InvalidIpError < RuntimeError end - # --- Section vendored from the ipaddress gem --- # - - # rubocop:disable Layout/LineLength, Style/SpecialGlobalVars - - # - # Checks if the given string is a valid IPv4 address - # - # Example: - # - # IPAddress::valid_ipv4? "2002::1" - # #=> false - # - # IPAddress::valid_ipv4? "172.16.10.1" - # #=> true - # - # Vendored from `ipaddress` gem from file 'lib/ipaddress.rb', line 198. - def self.valid_ipv4?(addr) - if /^(0|[1-9]{1}\d{0,2})\.(0|[1-9]{1}\d{0,2})\.(0|[1-9]{1}\d{0,2})\.(0|[1-9]{1}\d{0,2})$/ =~ addr - return $~.captures.all? { |i| i.to_i < 256 } - end - - false - end + # An error that represents that multiple IP headers were present in a request, + # thus a singular IP value could not be determined. + class MultipleIpHeadersError < RuntimeError + attr_reader :header_names - # - # Checks if the given string is a valid IPv6 address - # - # Example: - # - # IPAddress::valid_ipv6? "2002::1" - # #=> true - # - # IPAddress::valid_ipv6? "2002::DEAD::BEEF" - # #=> false - # - # Vendored from `ipaddress` gem from file 'lib/ipaddress.rb', line 230. - def self.valid_ipv6?(addr) - # https://gist.github.com/cpetschnig/294476 - # http://forums.intermapper.com/viewtopic.php?t=452 - if /^\s*((([0-9A-Fa-f]{1,4}:){7}([0-9A-Fa-f]{1,4}|:))|(([0-9A-Fa-f]{1,4}:){6}(:[0-9A-Fa-f]{1,4}|((25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(\.(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3})|:))|(([0-9A-Fa-f]{1,4}:){5}(((:[0-9A-Fa-f]{1,4}){1,2})|:((25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(\.(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3})|:))|(([0-9A-Fa-f]{1,4}:){4}(((:[0-9A-Fa-f]{1,4}){1,3})|((:[0-9A-Fa-f]{1,4})?:((25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(\.(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3}))|:))|(([0-9A-Fa-f]{1,4}:){3}(((:[0-9A-Fa-f]{1,4}){1,4})|((:[0-9A-Fa-f]{1,4}){0,2}:((25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(\.(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3}))|:))|(([0-9A-Fa-f]{1,4}:){2}(((:[0-9A-Fa-f]{1,4}){1,5})|((:[0-9A-Fa-f]{1,4}){0,3}:((25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(\.(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3}))|:))|(([0-9A-Fa-f]{1,4}:){1}(((:[0-9A-Fa-f]{1,4}){1,6})|((:[0-9A-Fa-f]{1,4}){0,4}:((25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(\.(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3}))|:))|(:(((:[0-9A-Fa-f]{1,4}){1,7})|((:[0-9A-Fa-f]{1,4}){0,5}:((25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(\.(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3}))|:)))(%.+)?\s*$/ =~ addr - return true + def initialize(header_names) + super + @header_names = header_names end - - false end - # rubocop:enable Layout/LineLength, Style/SpecialGlobalVars end end end diff --git a/lib/datadog/tracing/contrib/rack/header.rb b/lib/datadog/tracing/contrib/rack/header_collection.rb similarity index 72% rename from lib/datadog/tracing/contrib/rack/header.rb rename to lib/datadog/tracing/contrib/rack/header_collection.rb index 2367142593a..61008760ae9 100644 --- a/lib/datadog/tracing/contrib/rack/header.rb +++ b/lib/datadog/tracing/contrib/rack/header_collection.rb @@ -1,4 +1,4 @@ -require_relative '../../client_ip' +require_relative '../../../core/header_collection' module Datadog module Tracing @@ -7,7 +7,7 @@ module Rack # Classes and utilities for handling headers in Rack. module Header # An implementation of a header collection that looks up headers from a Rack environment. - class RequestHeaderCollection < Datadog::Tracing::ClientIp::HeaderCollection + class RequestHeaderCollection < Datadog::Core::HeaderCollection # Creates a header collection from a rack environment. def initialize(env) super() @@ -16,16 +16,16 @@ def initialize(env) # Gets the value of the header with the given name. def get(header_name) - @env[Header.header_to_rack_header(header_name)] + @env[Header.to_rack_header(header_name)] end # Tests whether a header with the given name exists in the environment. def key?(header_name) - @env.key?(Header.header_to_rack_header(header_name)) + @env.key?(Header.to_rack_header(header_name)) end end - def self.header_to_rack_header(name) + def self.to_rack_header(name) "HTTP_#{name.to_s.upcase.gsub(/[-\s]/, '_')}" end end diff --git a/lib/datadog/tracing/contrib/rack/middlewares.rb b/lib/datadog/tracing/contrib/rack/middlewares.rb index fae6589ae78..440a92234c1 100644 --- a/lib/datadog/tracing/contrib/rack/middlewares.rb +++ b/lib/datadog/tracing/contrib/rack/middlewares.rb @@ -9,7 +9,7 @@ require_relative '../analytics' require_relative '../utils/quantization/http' require_relative 'ext' -require_relative 'header' +require_relative 'header_collection' require_relative 'request_queue' module Datadog @@ -227,14 +227,15 @@ def configuration end def parse_request_headers(headers) - {}.tap do |result| - whitelist = configuration[:headers][:request] || [] - whitelist.each do |header| - rack_header = header_to_rack_header(header) - if headers.key?(rack_header) - result[Tracing::Metadata::Ext::HTTP::RequestHeaders.to_tag(header)] = headers[rack_header] - end + whitelist = configuration[:headers][:request] || [] + whitelist.reduce({}) do |result, header| + header_value = headers.get(header) + unless header_value.nil? + header_tag = Tracing::Metadata::Ext::HTTP::RequestHeaders.to_tag(header) + result[header_tag] = header_value end + + result end end diff --git a/spec/datadog/core/header_collection_spec.rb b/spec/datadog/core/header_collection_spec.rb new file mode 100644 index 00000000000..c28e7d8e4ea --- /dev/null +++ b/spec/datadog/core/header_collection_spec.rb @@ -0,0 +1,36 @@ +require 'datadog/core/header_collection' + +RSpec.describe Datadog::Core::HeaderCollection do + subject(:collection) { described_class.from_hash hash } + let(:hash) { {} } + + describe '#get' do + context 'when header exists in env' do + let(:hash) do + { + 'X-Forwarded-For' => 'me' + } + end + + it 'returns header value' do + expect(collection.get('X-Forwarded-For')).to eq('me') + end + + it 'returns header value regardless of letter casing in the name' do + expect(collection.get('x-forwarded-for')).to eq('me') + end + end + + context 'when header does not exists in env' do + let(:env) do + { + 'User-Agent' => 'test' + } + end + + it 'returns nil' do + expect(collection.get('X-Forwarded-For')).to be_nil + end + end + end +end diff --git a/spec/datadog/tracing/client_ip_spec.rb b/spec/datadog/tracing/client_ip_spec.rb index 7a456fdc6b2..d3165cad20f 100644 --- a/spec/datadog/tracing/client_ip_spec.rb +++ b/spec/datadog/tracing/client_ip_spec.rb @@ -1,8 +1,22 @@ require 'spec_helper' +require 'datadog/core/header_collection' require 'datadog/tracing/client_ip' require 'datadog/tracing/metadata/ext' +RSpec::Matchers.define :be_valid_ip do + match do |actual| + normalised = Datadog::Tracing::ClientIp.strip_decorations(actual) + begin + Datadog::Tracing::ClientIp.validate_ip(normalised) + + true + rescue Datadog::Tracing::ClientIp::InvalidIpError + false + end + end +end + RSpec.describe Datadog::Tracing::ClientIp do subject(:client_ip) { described_class } let(:tagging_enabled) { true } @@ -19,12 +33,13 @@ without_warnings { Datadog.configuration.reset! } end - describe '#valid_ip?' do + describe 'ip validation' do context 'when given valid ip addresses' do subject do [ '10.0.0.0', '10.0.0.1', + '10.0.0.1:8080', 'FEDC:BA98:7654:3210:FEDC:BA98:7654:3210', '1080:0000:0000:0000:0008:0800:200C:417A', '1080:0:0:0:8:800:200C:417A', @@ -40,16 +55,21 @@ '0:0:0::0:0:0', '::', 'fe80::208:74ff:feda:625c', - 'ff80:03:02:01::' + 'fe80::208:74ff:feda:625c%eth0', + 'ff80:03:02:01::', + '[fe80::208:74ff:feda:625c]', + '[fe80::208:74ff:feda:625c]:8080', + '[fe80::208:74ff:feda:625c%eth0]:8080' ] end - it { is_expected.to all(satisfy { |address| client_ip.valid_ip?(address) }) } + it { is_expected.to all(be_valid_ip) } end context 'when given invalid ip addresses' do subject do [ + '', '10.0.0.256', '10.0.0.0.0', '10.0.0', @@ -67,7 +87,7 @@ ] end - it { is_expected.to_not include(satisfy { |address| client_ip.valid_ip?(address) }) } + it { is_expected.to_not include(be_valid_ip) } end end @@ -92,28 +112,28 @@ end it 'ignores default header names' do - headers = client_ip::HeaderCollection.from_hash({ 'X-Forwarded-For' => '1.1.1.1' }) + headers = Datadog::Core::HeaderCollection.from_hash({ 'X-Forwarded-For' => '1.1.1.1' }) expect(span).to_not receive(:set_tag).with(any_args) client_ip.set_client_ip_tag(span, headers, nil) end it 'uses custom header value as client ip' do - headers = client_ip::HeaderCollection.from_hash({ 'My-Custom-Header' => '1.1.1.1' }) + headers = Datadog::Core::HeaderCollection.from_hash({ 'My-Custom-Header' => '1.1.1.1' }) expect(span).to receive(:set_tag).with(Datadog::Tracing::Metadata::Ext::HTTP::TAG_CLIENT_IP, '1.1.1.1') client_ip.set_client_ip_tag(span, headers, nil) end it 'does nothing if custom header value is not a valid ip' do - headers = client_ip::HeaderCollection.from_hash({ 'My-Custom-Header' => '1.11.1' }) + headers = Datadog::Core::HeaderCollection.from_hash({ 'My-Custom-Header' => '1.11.1' }) expect(span).to_not receive(:set_tag).with(any_args) client_ip.set_client_ip_tag(span, headers, nil) end it 'does not use other headers if custom header value is not a valid ip' do - headers = client_ip::HeaderCollection.from_hash( + headers = Datadog::Core::HeaderCollection.from_hash( { 'My-Custom-Header' => '1.11.1', 'X-Forwarded-For' => '1.11.1' @@ -125,7 +145,7 @@ end it 'does not use remote ip if custom header value is not a vaild ip' do - headers = client_ip::HeaderCollection.from_hash( + headers = Datadog::Core::HeaderCollection.from_hash( { 'My-Custom-Header' => '1.11.1', } @@ -136,7 +156,7 @@ end it 'prefers ip from custom header over remote ip' do - headers = client_ip::HeaderCollection.from_hash({ 'My-Custom-Header' => '1.1.1.1' }) + headers = Datadog::Core::HeaderCollection.from_hash({ 'My-Custom-Header' => '1.1.1.1' }) expect(span).to receive(:set_tag).with(Datadog::Tracing::Metadata::Ext::HTTP::TAG_CLIENT_IP, '1.1.1.1') client_ip.set_client_ip_tag(span, headers, '2.2.2.2') @@ -145,28 +165,28 @@ context 'when single ip header is present' do it 'uses value from header as client ip' do - headers = client_ip::HeaderCollection.from_hash({ 'X-Forwarded-For' => '1.1.1.1' }) + headers = Datadog::Core::HeaderCollection.from_hash({ 'X-Forwarded-For' => '1.1.1.1' }) expect(span).to receive(:set_tag).with(Datadog::Tracing::Metadata::Ext::HTTP::TAG_CLIENT_IP, '1.1.1.1') client_ip.set_client_ip_tag(span, headers, nil) end it 'does nothing if header value is not a valid ip' do - headers = client_ip::HeaderCollection.from_hash({ 'X-Forwarded-For' => '1.11.1' }) + headers = Datadog::Core::HeaderCollection.from_hash({ 'X-Forwarded-For' => '1.11.1' }) expect(span).to_not receive(:set_tag).with(any_args) client_ip.set_client_ip_tag(span, headers, nil) end it 'does not use remote ip if header value is not a valid ip' do - headers = client_ip::HeaderCollection.from_hash({ 'X-Forwarded-For' => '1.11.1' }) + headers = Datadog::Core::HeaderCollection.from_hash({ 'X-Forwarded-For' => '1.11.1' }) expect(span).to_not receive(:set_tag).with(any_args) client_ip.set_client_ip_tag(span, headers, '1.1.1.1') end it 'prefers ip from header over remote ip' do - headers = client_ip::HeaderCollection.from_hash({ 'X-Forwarded-For' => '1.1.1.1' }) + headers = Datadog::Core::HeaderCollection.from_hash({ 'X-Forwarded-For' => '1.1.1.1' }) expect(span).to receive(:set_tag).with(Datadog::Tracing::Metadata::Ext::HTTP::TAG_CLIENT_IP, '1.1.1.1') client_ip.set_client_ip_tag(span, headers, '2.2.2.2') @@ -175,7 +195,7 @@ context 'when multiple ip headers are present' do it 'sets multiple ip headers tag only' do - headers = client_ip::HeaderCollection.from_hash( + headers = Datadog::Core::HeaderCollection.from_hash( { 'X-Forwarded-For' => '1.1.1.1', 'X-Client-Ip' => '2.2.2.2' @@ -188,7 +208,7 @@ end it 'sets multiple ip headers tag only even if all ips are the same' do - headers = client_ip::HeaderCollection.from_hash( + headers = Datadog::Core::HeaderCollection.from_hash( { 'X-Forwarded-For' => '1.1.1.1', 'X-Client-Ip' => '1.1.1.1' @@ -201,7 +221,7 @@ end it 'prefers multiple ip headers tag only over remote ip' do - headers = client_ip::HeaderCollection.from_hash( + headers = Datadog::Core::HeaderCollection.from_hash( { 'X-Forwarded-For' => '1.1.1.1', 'X-Client-Ip' => '2.2.2.2' @@ -214,7 +234,7 @@ end it 'includes ip headers with invalid ips in multiple ip headers tag' do - headers = client_ip::HeaderCollection.from_hash( + headers = Datadog::Core::HeaderCollection.from_hash( { 'X-Forwarded-For' => '1.1.1.1', 'X-Client-Ip' => '2.2.2.2.3', @@ -228,7 +248,7 @@ end it 'includes ip headers with invalid ips in multiple ip headers tag even if exactly one ip is valid' do - headers = client_ip::HeaderCollection.from_hash( + headers = Datadog::Core::HeaderCollection.from_hash( { 'X-Forwarded-For' => '1.1.1.1', 'X-Client-Ip' => '2.22.2', @@ -243,7 +263,7 @@ end context 'when no ip headers are present' do - let(:headers) { client_ip::HeaderCollection.from_hash({}) } + let(:headers) { Datadog::Core::HeaderCollection.from_hash({}) } it 'uses remote ip as client ip as fallback' do expect(span).to receive(:set_tag).with(Datadog::Tracing::Metadata::Ext::HTTP::TAG_CLIENT_IP, '1.1.1.1') @@ -258,7 +278,7 @@ context 'when non-ip headers are present' do let(:headers) do - client_ip::HeaderCollection.from_hash( + Datadog::Core::HeaderCollection.from_hash( { 'Accept' => '*/*', 'Authorization' => 'Bearer XXXXXX' @@ -313,7 +333,10 @@ end it 'is bracketed ipv6 without port' do - expect(span).to_not receive(:set_tag).with(any_args) + expect(span).to receive(:set_tag).with( + Datadog::Tracing::Metadata::Ext::HTTP::TAG_CLIENT_IP, + '2001:db8::8a2e:370:7334' + ) client_ip.set_client_ip_tag(span, nil, '[2001:db8::8a2e:370:7334]') end end diff --git a/spec/datadog/tracing/contrib/rack/headers_spec.rb b/spec/datadog/tracing/contrib/rack/header_collection_spec.rb similarity index 83% rename from spec/datadog/tracing/contrib/rack/headers_spec.rb rename to spec/datadog/tracing/contrib/rack/header_collection_spec.rb index 4180f58ea05..fedf4b22bd1 100644 --- a/spec/datadog/tracing/contrib/rack/headers_spec.rb +++ b/spec/datadog/tracing/contrib/rack/header_collection_spec.rb @@ -1,6 +1,6 @@ require 'datadog/tracing/contrib/rack/headers' -RSpec.describe Datadog::Tracing::Contrib::Rack::HeaderCollection do +RSpec.describe Datadog::Tracing::Contrib::Rack::Header::RequestHeaderCollection do subject(:collection) { described_class.new env } let(:env) { {} } @@ -17,7 +17,7 @@ end it 'returns header value regardless of letter casing in the name' do - expect(collection.get('x-forwarded-for')).to eq('me'.freeze) + expect(collection.get('x-forwarded-for')).to eq('me') end end From 010066ee17881dc8867c508e12052024a2159a64 Mon Sep 17 00:00:00 2001 From: Bar Weiss Date: Wed, 31 Aug 2022 10:41:37 +0300 Subject: [PATCH 03/14] Fix linter issues --- lib/datadog/tracing/client_ip.rb | 4 +--- lib/datadog/tracing/contrib/http/instrumentation.rb | 2 ++ lib/datadog/tracing/contrib/rack/middlewares.rb | 4 +--- 3 files changed, 4 insertions(+), 6 deletions(-) diff --git a/lib/datadog/tracing/client_ip.rb b/lib/datadog/tracing/client_ip.rb index 8136a99484c..3fbbde065b2 100644 --- a/lib/datadog/tracing/client_ip.rb +++ b/lib/datadog/tracing/client_ip.rb @@ -139,11 +139,9 @@ def self.validate_ip(ip) def self.ip_headers(headers) return {} unless headers - DEFAULT_IP_HEADERS_NAMES.reduce({}) do |result, name| + DEFAULT_IP_HEADERS_NAMES.each_with_object({}) do |name, result| value = headers.get(name) result[name] = value unless value.nil? - - result end end diff --git a/lib/datadog/tracing/contrib/http/instrumentation.rb b/lib/datadog/tracing/contrib/http/instrumentation.rb index 05b17b12688..fe735308962 100644 --- a/lib/datadog/tracing/contrib/http/instrumentation.rb +++ b/lib/datadog/tracing/contrib/http/instrumentation.rb @@ -5,6 +5,7 @@ require_relative '../../metadata/ext' require_relative '../analytics' require_relative '../http_annotation_helper' +require_relative '../../client_ip' module Datadog module Tracing @@ -86,6 +87,7 @@ def annotate_span_with_request!(span, request, request_options) # Set analytics sample rate set_analytics_sample_rate(span, request_options) + Tracing::ClientIp.set_client_ip_tag(span, request.headers, request.remote_addr) end def annotate_span_with_response!(span, response) diff --git a/lib/datadog/tracing/contrib/rack/middlewares.rb b/lib/datadog/tracing/contrib/rack/middlewares.rb index 440a92234c1..32f0de80898 100644 --- a/lib/datadog/tracing/contrib/rack/middlewares.rb +++ b/lib/datadog/tracing/contrib/rack/middlewares.rb @@ -228,14 +228,12 @@ def configuration def parse_request_headers(headers) whitelist = configuration[:headers][:request] || [] - whitelist.reduce({}) do |result, header| + whitelist.each_with_object({}) do |header, result| header_value = headers.get(header) unless header_value.nil? header_tag = Tracing::Metadata::Ext::HTTP::RequestHeaders.to_tag(header) result[header_tag] = header_value end - - result end end From 0c9eecbdb1c5ae4508e788f54ce10f357ee07834 Mon Sep 17 00:00:00 2001 From: Bar Weiss Date: Wed, 31 Aug 2022 10:49:45 +0300 Subject: [PATCH 04/14] Revert accidental changes to http instrumentation --- lib/datadog/tracing/contrib/http/instrumentation.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/datadog/tracing/contrib/http/instrumentation.rb b/lib/datadog/tracing/contrib/http/instrumentation.rb index fe735308962..05b17b12688 100644 --- a/lib/datadog/tracing/contrib/http/instrumentation.rb +++ b/lib/datadog/tracing/contrib/http/instrumentation.rb @@ -5,7 +5,6 @@ require_relative '../../metadata/ext' require_relative '../analytics' require_relative '../http_annotation_helper' -require_relative '../../client_ip' module Datadog module Tracing @@ -87,7 +86,6 @@ def annotate_span_with_request!(span, request, request_options) # Set analytics sample rate set_analytics_sample_rate(span, request_options) - Tracing::ClientIp.set_client_ip_tag(span, request.headers, request.remote_addr) end def annotate_span_with_response!(span, response) From ee61750cba82babfd2a0b15e8b36f130f3541c31 Mon Sep 17 00:00:00 2001 From: Bar Weiss Date: Wed, 31 Aug 2022 11:10:52 +0300 Subject: [PATCH 05/14] Fix require line --- spec/datadog/tracing/contrib/rack/header_collection_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/datadog/tracing/contrib/rack/header_collection_spec.rb b/spec/datadog/tracing/contrib/rack/header_collection_spec.rb index fedf4b22bd1..283673bfbd1 100644 --- a/spec/datadog/tracing/contrib/rack/header_collection_spec.rb +++ b/spec/datadog/tracing/contrib/rack/header_collection_spec.rb @@ -1,4 +1,4 @@ -require 'datadog/tracing/contrib/rack/headers' +require 'datadog/tracing/contrib/rack/header_collection' RSpec.describe Datadog::Tracing::Contrib::Rack::Header::RequestHeaderCollection do subject(:collection) { described_class.new env } From ca4c8997fdf1dabcd674cf26738837412156ba13 Mon Sep 17 00:00:00 2001 From: Bar Weiss Date: Wed, 31 Aug 2022 17:31:35 +0300 Subject: [PATCH 06/14] Fix comments from @TonyCTHsu's awesome review --- LICENSE-3rdparty.csv | 1 + lib/datadog/core/vendor/ipaddress/LICENSE.txt | 20 +++ .../core/vendor/ipaddress/ipaddress.rb | 76 ++++++++++++ lib/datadog/tracing/client_ip.rb | 63 +++------- spec/datadog/tracing/client_ip_spec.rb | 115 +++++++++--------- 5 files changed, 176 insertions(+), 99 deletions(-) create mode 100644 lib/datadog/core/vendor/ipaddress/LICENSE.txt create mode 100644 lib/datadog/core/vendor/ipaddress/ipaddress.rb diff --git a/LICENSE-3rdparty.csv b/LICENSE-3rdparty.csv index 4dfbe99c24e..ba84771ce98 100644 --- a/LICENSE-3rdparty.csv +++ b/LICENSE-3rdparty.csv @@ -1,5 +1,6 @@ Component,Origin,License,Copyright lib/datadog/core/vendor/multipart-post,https://github.com/socketry/multipart-post,MIT,"Copyright (c) 2007-2013 Nick Sieger." +lib/datadog/core/vendor/ipaddress,https://github.com/ipaddress-gem/ipaddress,MIT,"Copyright © 2009-2015 Marco Ceresa and Mike Mackintosh." lib/datadog/tracing/contrib/active_record/vendor,https://github.com/rails/rails/,MIT,"Copyright (c) 2005-2018 David Heinemeier Hansson" ext/ddtrace_profiling_native_extension/private_vm_api_access,https://github.com/ruby/ruby,BSD-2-Clause,"Copyright (C) 1993-2013 Yukihiro Matsumoto. All rights reserved." msgpack,https://rubygems.org/gems/msgpack,Apache-2.0,"Copyright (c) 2008-2015 Sadayuki Furuhashi" diff --git a/lib/datadog/core/vendor/ipaddress/LICENSE.txt b/lib/datadog/core/vendor/ipaddress/LICENSE.txt new file mode 100644 index 00000000000..943eee1df11 --- /dev/null +++ b/lib/datadog/core/vendor/ipaddress/LICENSE.txt @@ -0,0 +1,20 @@ +Copyright (c) 2009-2015 Marco Ceresa + +Permission is hereby granted, free of charge, to any person obtaining +a copy of this software and associated documentation files (the +"Software"), to deal in the Software without restriction, including +without limitation the rights to use, copy, modify, merge, publish, +distribute, sublicense, and/or sell copies of the Software, and to +permit persons to whom the Software is furnished to do so, subject to +the following conditions: + +The above copyright notice and this permission notice shall be +included in all copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, +EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF +MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND +NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE +LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION +OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION +WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. diff --git a/lib/datadog/core/vendor/ipaddress/ipaddress.rb b/lib/datadog/core/vendor/ipaddress/ipaddress.rb new file mode 100644 index 00000000000..b9ca439b0e7 --- /dev/null +++ b/lib/datadog/core/vendor/ipaddress/ipaddress.rb @@ -0,0 +1,76 @@ +# Note: This is a partial vendor. Only functions that are needed by `dd-trace-rb`` were vendored. + +# +# = IPAddress +# +# A ruby library to manipulate IPv4 and IPv6 addresses +# +# +# Package:: IPAddress +# Author:: Marco Ceresa +# License:: Ruby License +# +#-- +# +#++ + +module IPAddress + + NAME = "IPAddress" + GEM = "ipaddress" + AUTHORS = ["Marco Ceresa "] + + # + # Checks if the given string is a valid IP address, + # either IPv4 or IPv6 + # + # Example: + # + # IPAddress::valid_ip? "2002::1" + # #=> true + # + # IPAddress::valid_ip? "10.0.0.256" + # #=> false + # + def self.valid_ip?(addr) + valid_ipv4?(addr) || valid_ipv6?(addr) + end + + # + # Checks if the given string is a valid IPv4 address + # + # Example: + # + # IPAddress::valid_ipv4? "2002::1" + # #=> false + # + # IPAddress::valid_ipv4? "172.16.10.1" + # #=> true + # + def self.valid_ipv4?(addr) + if /^(0|[1-9]{1}\d{0,2})\.(0|[1-9]{1}\d{0,2})\.(0|[1-9]{1}\d{0,2})\.(0|[1-9]{1}\d{0,2})$/ =~ addr + return $~.captures.all? {|i| i.to_i < 256} + end + false + end + + # + # Checks if the given string is a valid IPv6 address + # + # Example: + # + # IPAddress::valid_ipv6? "2002::1" + # #=> true + # + # IPAddress::valid_ipv6? "2002::DEAD::BEEF" + # #=> false + # + def self.valid_ipv6?(addr) + # https://gist.github.com/cpetschnig/294476 + # http://forums.intermapper.com/viewtopic.php?t=452 + return true if /^\s*((([0-9A-Fa-f]{1,4}:){7}([0-9A-Fa-f]{1,4}|:))|(([0-9A-Fa-f]{1,4}:){6}(:[0-9A-Fa-f]{1,4}|((25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(\.(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3})|:))|(([0-9A-Fa-f]{1,4}:){5}(((:[0-9A-Fa-f]{1,4}){1,2})|:((25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(\.(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3})|:))|(([0-9A-Fa-f]{1,4}:){4}(((:[0-9A-Fa-f]{1,4}){1,3})|((:[0-9A-Fa-f]{1,4})?:((25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(\.(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3}))|:))|(([0-9A-Fa-f]{1,4}:){3}(((:[0-9A-Fa-f]{1,4}){1,4})|((:[0-9A-Fa-f]{1,4}){0,2}:((25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(\.(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3}))|:))|(([0-9A-Fa-f]{1,4}:){2}(((:[0-9A-Fa-f]{1,4}){1,5})|((:[0-9A-Fa-f]{1,4}){0,3}:((25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(\.(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3}))|:))|(([0-9A-Fa-f]{1,4}:){1}(((:[0-9A-Fa-f]{1,4}){1,6})|((:[0-9A-Fa-f]{1,4}){0,4}:((25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(\.(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3}))|:))|(:(((:[0-9A-Fa-f]{1,4}){1,7})|((:[0-9A-Fa-f]{1,4}){0,5}:((25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(\.(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3}))|:)))(%.+)?\s*$/ =~ addr + false + end + + +end diff --git a/lib/datadog/tracing/client_ip.rb b/lib/datadog/tracing/client_ip.rb index 3fbbde065b2..d333813ffc6 100644 --- a/lib/datadog/tracing/client_ip.rb +++ b/lib/datadog/tracing/client_ip.rb @@ -1,7 +1,6 @@ # typed: true -require 'ipaddr' - +require_relative '../core/vendor/ipaddress/ipaddress' require_relative '../core/configuration' require_relative 'metadata/ext' require_relative 'span' @@ -36,29 +35,23 @@ module ClientIp # @param [Span] span The span that's associated with the request. # @param [HeaderCollection, #get, nil] headers A collection with the request headers. # @param [String, nil] remote_ip The remote IP the request associated with the span is sent to. - def self.set_client_ip_tag(span, headers, remote_ip) + def self.set_client_ip_tag(span, headers: nil, remote_ip: nil) return if configuration.disabled - begin - address = raw_ip_from_request(headers, remote_ip) - if address.nil? - # `address` can be `nil` if a custom header is configured but not present in the request. - # In that case, assume misconfiguration and avoid setting the tag. - return - end - - ip = strip_decorations(address) + result = raw_ip_from_request(headers, remote_ip) - validate_ip(ip) + if result.raw_ip + ip = strip_decorations(result.raw_ip) + return unless valid_ip?(ip) span.set_tag(Tracing::Metadata::Ext::HTTP::TAG_CLIENT_IP, ip) - rescue InvalidIpError - # Do nothing, assuming logs will spam here. - rescue MultipleIpHeadersError => e - span.set_tag(TAG_MULTIPLE_IP_HEADERS, e.header_names.join(',')) + elsif result.multiple_ip_headers + span.set_tag(TAG_MULTIPLE_IP_HEADERS, result.multiple_ip_headers.keys.join(',')) end end + IpExtractionResult = Struct.new(:raw_ip, :multiple_ip_headers, keyword_init: true) + # Returns the value of an IP-related header or the request's remote IP. # # The client IP is looked up by the following logic: @@ -73,17 +66,19 @@ def self.set_client_ip_tag(span, headers, remote_ip) # @return [String] An unprocessed value retrieved from an # IP header or the remote IP of the request. def self.raw_ip_from_request(headers, remote_ip) - return headers && headers.get(configuration.header_name) if configuration.header_name + if configuration.header_name + return IpExtractionResult.new(raw_ip: headers && headers.get(configuration.header_name)) + end headers_present = ip_headers(headers) case headers_present.size when 0 - remote_ip + IpExtractionResult.new(raw_ip: remote_ip) when 1 - headers_present.values.first + IpExtractionResult.new(raw_ip: headers_present.values.first) else - raise MultipleIpHeadersError, headers_present.keys + IpExtractionResult.new(multiple_ip_headers: headers_present) end end @@ -125,15 +120,9 @@ def self.likely_ipv4?(value) dot_index < colon_index end - def self.validate_ip(ip) - # IPs with netmasks are invalid. - raise InvalidIpError if ip.include?('/') - - begin - IPAddr.new(ip) - rescue IPAddr::Error - raise InvalidIpError - end + # Determines whether the given string is a valid IPv4 or IPv6 address. + def self.valid_ip?(ip) + IPAddress.valid_ip?(ip) end def self.ip_headers(headers) @@ -148,20 +137,6 @@ def self.ip_headers(headers) def self.configuration Datadog.configuration.tracing.client_ip end - - class InvalidIpError < RuntimeError - end - - # An error that represents that multiple IP headers were present in a request, - # thus a singular IP value could not be determined. - class MultipleIpHeadersError < RuntimeError - attr_reader :header_names - - def initialize(header_names) - super - @header_names = header_names - end - end end end end diff --git a/spec/datadog/tracing/client_ip_spec.rb b/spec/datadog/tracing/client_ip_spec.rb index d3165cad20f..ad946162578 100644 --- a/spec/datadog/tracing/client_ip_spec.rb +++ b/spec/datadog/tracing/client_ip_spec.rb @@ -4,19 +4,6 @@ require 'datadog/tracing/client_ip' require 'datadog/tracing/metadata/ext' -RSpec::Matchers.define :be_valid_ip do - match do |actual| - normalised = Datadog::Tracing::ClientIp.strip_decorations(actual) - begin - Datadog::Tracing::ClientIp.validate_ip(normalised) - - true - rescue Datadog::Tracing::ClientIp::InvalidIpError - false - end - end -end - RSpec.describe Datadog::Tracing::ClientIp do subject(:client_ip) { described_class } let(:tagging_enabled) { true } @@ -33,9 +20,9 @@ without_warnings { Datadog.configuration.reset! } end - describe 'ip validation' do + describe '#valid_ip?' do context 'when given valid ip addresses' do - subject do + ips = [ '10.0.0.0', '10.0.0.1', @@ -61,13 +48,22 @@ '[fe80::208:74ff:feda:625c]:8080', '[fe80::208:74ff:feda:625c%eth0]:8080' ] - end - it { is_expected.to all(be_valid_ip) } + ips.each do |ip| + subject do + normalized = client_ip.strip_decorations(ip) + + client_ip.valid_ip?(normalized) + end + + it "returns true for #{ip}" do + is_expected.to be true + end + end end context 'when given invalid ip addresses' do - subject do + ips = [ '', '10.0.0.256', @@ -85,9 +81,18 @@ '02001:0000:1234:0000:0000:C1C0:ABCD:0876', '2001:0000:1234:0000:00001:C1C0:ABCD:0876' ] - end - it { is_expected.to_not include(be_valid_ip) } + ips.each do |ip| + subject do + normalized = client_ip.strip_decorations(ip) + + client_ip.valid_ip?(normalized) + end + + it "returns false for #{ip}" do + is_expected.to be false + end + end end end @@ -100,8 +105,8 @@ let(:tagging_enabled) { false } it 'does nothing' do - expect(span).to_not receive(:set_tag).with(any_args) - client_ip.set_client_ip_tag(span, nil, '1.1.1.1') + expect(span).to_not receive(:set_tag) + client_ip.set_client_ip_tag(span, remote_ip: '1.1.1.1') end end @@ -114,22 +119,22 @@ it 'ignores default header names' do headers = Datadog::Core::HeaderCollection.from_hash({ 'X-Forwarded-For' => '1.1.1.1' }) - expect(span).to_not receive(:set_tag).with(any_args) - client_ip.set_client_ip_tag(span, headers, nil) + expect(span).to_not receive(:set_tag) + client_ip.set_client_ip_tag(span, headers: headers) end it 'uses custom header value as client ip' do headers = Datadog::Core::HeaderCollection.from_hash({ 'My-Custom-Header' => '1.1.1.1' }) expect(span).to receive(:set_tag).with(Datadog::Tracing::Metadata::Ext::HTTP::TAG_CLIENT_IP, '1.1.1.1') - client_ip.set_client_ip_tag(span, headers, nil) + client_ip.set_client_ip_tag(span, headers: headers) end it 'does nothing if custom header value is not a valid ip' do headers = Datadog::Core::HeaderCollection.from_hash({ 'My-Custom-Header' => '1.11.1' }) - expect(span).to_not receive(:set_tag).with(any_args) - client_ip.set_client_ip_tag(span, headers, nil) + expect(span).to_not receive(:set_tag) + client_ip.set_client_ip_tag(span, headers: headers) end it 'does not use other headers if custom header value is not a valid ip' do @@ -140,8 +145,8 @@ } ) - expect(span).to_not receive(:set_tag).with(any_args) - client_ip.set_client_ip_tag(span, headers, nil) + expect(span).to_not receive(:set_tag) + client_ip.set_client_ip_tag(span, headers: headers) end it 'does not use remote ip if custom header value is not a vaild ip' do @@ -151,15 +156,15 @@ } ) - expect(span).to_not receive(:set_tag).with(any_args) - client_ip.set_client_ip_tag(span, headers, '1.1.1.1') + expect(span).to_not receive(:set_tag) + client_ip.set_client_ip_tag(span, headers: headers, remote_ip: '1.1.1.1') end it 'prefers ip from custom header over remote ip' do headers = Datadog::Core::HeaderCollection.from_hash({ 'My-Custom-Header' => '1.1.1.1' }) expect(span).to receive(:set_tag).with(Datadog::Tracing::Metadata::Ext::HTTP::TAG_CLIENT_IP, '1.1.1.1') - client_ip.set_client_ip_tag(span, headers, '2.2.2.2') + client_ip.set_client_ip_tag(span, headers: headers, remote_ip: '2.2.2.2') end end @@ -168,28 +173,28 @@ headers = Datadog::Core::HeaderCollection.from_hash({ 'X-Forwarded-For' => '1.1.1.1' }) expect(span).to receive(:set_tag).with(Datadog::Tracing::Metadata::Ext::HTTP::TAG_CLIENT_IP, '1.1.1.1') - client_ip.set_client_ip_tag(span, headers, nil) + client_ip.set_client_ip_tag(span, headers: headers) end it 'does nothing if header value is not a valid ip' do headers = Datadog::Core::HeaderCollection.from_hash({ 'X-Forwarded-For' => '1.11.1' }) - expect(span).to_not receive(:set_tag).with(any_args) - client_ip.set_client_ip_tag(span, headers, nil) + expect(span).to_not receive(:set_tag) + client_ip.set_client_ip_tag(span, headers: headers) end it 'does not use remote ip if header value is not a valid ip' do headers = Datadog::Core::HeaderCollection.from_hash({ 'X-Forwarded-For' => '1.11.1' }) - expect(span).to_not receive(:set_tag).with(any_args) - client_ip.set_client_ip_tag(span, headers, '1.1.1.1') + expect(span).to_not receive(:set_tag) + client_ip.set_client_ip_tag(span, headers: headers, remote_ip: '1.1.1.1') end it 'prefers ip from header over remote ip' do headers = Datadog::Core::HeaderCollection.from_hash({ 'X-Forwarded-For' => '1.1.1.1' }) expect(span).to receive(:set_tag).with(Datadog::Tracing::Metadata::Ext::HTTP::TAG_CLIENT_IP, '1.1.1.1') - client_ip.set_client_ip_tag(span, headers, '2.2.2.2') + client_ip.set_client_ip_tag(span, headers: headers, remote_ip: '2.2.2.2') end end @@ -204,7 +209,7 @@ expect(span).to_not receive(:set_tag).with(Datadog::Tracing::Metadata::Ext::HTTP::TAG_CLIENT_IP, anything) expect(span).to receive(:set_tag).with(client_ip::TAG_MULTIPLE_IP_HEADERS, 'x-forwarded-for,x-client-ip') - client_ip.set_client_ip_tag(span, headers, nil) + client_ip.set_client_ip_tag(span, headers: headers) end it 'sets multiple ip headers tag only even if all ips are the same' do @@ -217,7 +222,7 @@ expect(span).to_not receive(:set_tag).with(Datadog::Tracing::Metadata::Ext::HTTP::TAG_CLIENT_IP, anything) expect(span).to receive(:set_tag).with(client_ip::TAG_MULTIPLE_IP_HEADERS, 'x-forwarded-for,x-client-ip') - client_ip.set_client_ip_tag(span, headers, nil) + client_ip.set_client_ip_tag(span, headers: headers) end it 'prefers multiple ip headers tag only over remote ip' do @@ -230,7 +235,7 @@ expect(span).to_not receive(:set_tag).with(Datadog::Tracing::Metadata::Ext::HTTP::TAG_CLIENT_IP, anything) expect(span).to receive(:set_tag).with(client_ip::TAG_MULTIPLE_IP_HEADERS, 'x-forwarded-for,x-client-ip') - client_ip.set_client_ip_tag(span, headers, '3.3.3.3') + client_ip.set_client_ip_tag(span, headers: headers, remote_ip: '3.3.3.3') end it 'includes ip headers with invalid ips in multiple ip headers tag' do @@ -244,7 +249,7 @@ expect(span).to_not receive(:set_tag).with(Datadog::Tracing::Metadata::Ext::HTTP::TAG_CLIENT_IP, anything) expect(span).to receive(:set_tag).with(client_ip::TAG_MULTIPLE_IP_HEADERS, 'x-forwarded-for,x-real-ip,x-client-ip') - client_ip.set_client_ip_tag(span, headers, nil) + client_ip.set_client_ip_tag(span, headers: headers) end it 'includes ip headers with invalid ips in multiple ip headers tag even if exactly one ip is valid' do @@ -258,7 +263,7 @@ expect(span).to_not receive(:set_tag).with(Datadog::Tracing::Metadata::Ext::HTTP::TAG_CLIENT_IP, anything) expect(span).to receive(:set_tag).with(client_ip::TAG_MULTIPLE_IP_HEADERS, 'x-forwarded-for,x-real-ip,x-client-ip') - client_ip.set_client_ip_tag(span, headers, nil) + client_ip.set_client_ip_tag(span, headers: headers) end end @@ -267,12 +272,12 @@ it 'uses remote ip as client ip as fallback' do expect(span).to receive(:set_tag).with(Datadog::Tracing::Metadata::Ext::HTTP::TAG_CLIENT_IP, '1.1.1.1') - client_ip.set_client_ip_tag(span, headers, '1.1.1.1') + client_ip.set_client_ip_tag(span, headers: headers, remote_ip: '1.1.1.1') end it 'does nothing if remote ip is invalid' do - expect(span).to_not receive(:set_tag).with(any_args) - client_ip.set_client_ip_tag(span, headers, '1.11.1') + expect(span).to_not receive(:set_tag) + client_ip.set_client_ip_tag(span, headers: headers, remote_ip: '1.11.1') end end @@ -288,19 +293,19 @@ it 'uses remote ip as client ip as fallback' do expect(span).to receive(:set_tag).with(Datadog::Tracing::Metadata::Ext::HTTP::TAG_CLIENT_IP, '1.1.1.1') - client_ip.set_client_ip_tag(span, headers, '1.1.1.1') + client_ip.set_client_ip_tag(span, headers: headers, remote_ip: '1.1.1.1') end it 'does nothing if remote ip is invalid' do - expect(span).to_not receive(:set_tag).with(any_args) - client_ip.set_client_ip_tag(span, headers, '1.11.1') + expect(span).to_not receive(:set_tag) + client_ip.set_client_ip_tag(span, headers: headers, remote_ip: '1.11.1') end end context 'when ip' do it 'is plain ipv4' do expect(span).to receive(:set_tag).with(Datadog::Tracing::Metadata::Ext::HTTP::TAG_CLIENT_IP, '1.1.1.1') - client_ip.set_client_ip_tag(span, nil, '1.1.1.1') + client_ip.set_client_ip_tag(span, remote_ip: '1.1.1.1') end it 'is plain ipv6' do @@ -308,12 +313,12 @@ Datadog::Tracing::Metadata::Ext::HTTP::TAG_CLIENT_IP, '2001:db8::8a2e:370:7334' ) - client_ip.set_client_ip_tag(span, nil, '2001:db8::8a2e:370:7334') + client_ip.set_client_ip_tag(span, remote_ip: '2001:db8::8a2e:370:7334') end it 'is ipv4 and port' do expect(span).to receive(:set_tag).with(Datadog::Tracing::Metadata::Ext::HTTP::TAG_CLIENT_IP, '1.1.1.1') - client_ip.set_client_ip_tag(span, nil, '1.1.1.1:8080') + client_ip.set_client_ip_tag(span, remote_ip: '1.1.1.1:8080') end it 'is ipv6 and port' do @@ -321,7 +326,7 @@ Datadog::Tracing::Metadata::Ext::HTTP::TAG_CLIENT_IP, '2001:db8::8a2e:370:7334' ) - client_ip.set_client_ip_tag(span, nil, '[2001:db8::8a2e:370:7334]:8080') + client_ip.set_client_ip_tag(span, remote_ip: '[2001:db8::8a2e:370:7334]:8080') end it 'is ipv6 with interface id' do @@ -329,7 +334,7 @@ Datadog::Tracing::Metadata::Ext::HTTP::TAG_CLIENT_IP, '2001:db8::8a2e:370:7334' ) - client_ip.set_client_ip_tag(span, nil, '2001:db8::8a2e:370:7334%eth0') + client_ip.set_client_ip_tag(span, remote_ip: '2001:db8::8a2e:370:7334%eth0') end it 'is bracketed ipv6 without port' do @@ -337,7 +342,7 @@ Datadog::Tracing::Metadata::Ext::HTTP::TAG_CLIENT_IP, '2001:db8::8a2e:370:7334' ) - client_ip.set_client_ip_tag(span, nil, '[2001:db8::8a2e:370:7334]') + client_ip.set_client_ip_tag(span, remote_ip: '[2001:db8::8a2e:370:7334]') end end end From d9532caf1433460cbb5f6fb5988d3bc181653a4a Mon Sep 17 00:00:00 2001 From: Bar Weiss Date: Wed, 31 Aug 2022 17:36:21 +0300 Subject: [PATCH 07/14] Fix bad 'set_client_ip' call in Rack middleware --- lib/datadog/tracing/contrib/rack/middlewares.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/datadog/tracing/contrib/rack/middlewares.rb b/lib/datadog/tracing/contrib/rack/middlewares.rb index 32f0de80898..08840f517dc 100644 --- a/lib/datadog/tracing/contrib/rack/middlewares.rb +++ b/lib/datadog/tracing/contrib/rack/middlewares.rb @@ -181,7 +181,11 @@ def set_request_tags!(trace, request_span, env, status, headers, response, origi end if request_span.get_tag(Tracing::Metadata::Ext::HTTP::TAG_CLIENT_IP).nil? - Tracing::ClientIp.set_client_ip_tag(request_span, request_header_collection, env['REMOTE_ADDR']) + Tracing::ClientIp.set_client_ip_tag( + request_span, + headers: request_header_collection, + remote_ip: env['REMOTE_ADDR'] + ) end if request_span.get_tag(Tracing::Metadata::Ext::HTTP::TAG_BASE_URL).nil? From 5d221132c330945048c9da626daeda8429bee044 Mon Sep 17 00:00:00 2001 From: Bar Weiss Date: Wed, 31 Aug 2022 18:01:07 +0300 Subject: [PATCH 08/14] Fix Ruby 2.2 compatibilty issue --- lib/datadog/tracing/client_ip.rb | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/datadog/tracing/client_ip.rb b/lib/datadog/tracing/client_ip.rb index d333813ffc6..65663a38e14 100644 --- a/lib/datadog/tracing/client_ip.rb +++ b/lib/datadog/tracing/client_ip.rb @@ -50,7 +50,7 @@ def self.set_client_ip_tag(span, headers: nil, remote_ip: nil) end end - IpExtractionResult = Struct.new(:raw_ip, :multiple_ip_headers, keyword_init: true) + IpExtractionResult = Struct.new(:raw_ip, :multiple_ip_headers) # Returns the value of an IP-related header or the request's remote IP. # @@ -67,18 +67,18 @@ def self.set_client_ip_tag(span, headers: nil, remote_ip: nil) # IP header or the remote IP of the request. def self.raw_ip_from_request(headers, remote_ip) if configuration.header_name - return IpExtractionResult.new(raw_ip: headers && headers.get(configuration.header_name)) + return IpExtractionResult.new(headers && headers.get(configuration.header_name), nil) end headers_present = ip_headers(headers) case headers_present.size when 0 - IpExtractionResult.new(raw_ip: remote_ip) + IpExtractionResult.new(remote_ip, nil) when 1 - IpExtractionResult.new(raw_ip: headers_present.values.first) + IpExtractionResult.new(headers_present.values.first, nil) else - IpExtractionResult.new(multiple_ip_headers: headers_present) + IpExtractionResult.new(nil, headers_present) end end From ef6f427fffc5dbf5d1cc67eb605317364e8d4f05 Mon Sep 17 00:00:00 2001 From: Bar Weiss Date: Wed, 31 Aug 2022 18:12:35 +0300 Subject: [PATCH 09/14] Fix rubocop stuff that only appears in circleci --- lib/datadog/tracing/client_ip.rb | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/datadog/tracing/client_ip.rb b/lib/datadog/tracing/client_ip.rb index 65663a38e14..b68817a1abc 100644 --- a/lib/datadog/tracing/client_ip.rb +++ b/lib/datadog/tracing/client_ip.rb @@ -66,9 +66,7 @@ def self.set_client_ip_tag(span, headers: nil, remote_ip: nil) # @return [String] An unprocessed value retrieved from an # IP header or the remote IP of the request. def self.raw_ip_from_request(headers, remote_ip) - if configuration.header_name - return IpExtractionResult.new(headers && headers.get(configuration.header_name), nil) - end + return IpExtractionResult.new(headers && headers.get(configuration.header_name), nil) if configuration.header_name headers_present = ip_headers(headers) From 00f3c7d77a5044104b47f7038edf69fa54e2c21e Mon Sep 17 00:00:00 2001 From: Bar Weiss Date: Mon, 5 Sep 2022 19:09:24 +0300 Subject: [PATCH 10/14] Rename configuration option 'disabled' -> 'enabled', revert to using 'ipaddr' --- lib/datadog/core/configuration/settings.rb | 12 ++- lib/datadog/core/vendor/ipaddress/LICENSE.txt | 20 ----- .../core/vendor/ipaddress/ipaddress.rb | 76 ------------------- lib/datadog/tracing/client_ip.rb | 14 +++- spec/datadog/tracing/client_ip_spec.rb | 2 +- 5 files changed, 20 insertions(+), 104 deletions(-) delete mode 100644 lib/datadog/core/vendor/ipaddress/LICENSE.txt delete mode 100644 lib/datadog/core/vendor/ipaddress/ipaddress.rb diff --git a/lib/datadog/core/configuration/settings.rb b/lib/datadog/core/configuration/settings.rb index 02f3d3f812f..19a5d39e308 100644 --- a/lib/datadog/core/configuration/settings.rb +++ b/lib/datadog/core/configuration/settings.rb @@ -622,12 +622,16 @@ def initialize(*_) # Client IP configuration # @public_api settings :client_ip do - # Whether client IP collection is disabled. This disables client IPs from HTTP requests to be reported in traces. + # Whether client IP collection is enabled. When enabled client IPs from HTTP requests will + # be reported in traces. # - # @default `DD_TRACE_CLIENT_IP_HEADER_DISABLED` environment variable, otherwise `false`. + # @see https://docs.datadoghq.com/tracing/configure_data_security#configuring-a-client-ip-header + # + # @default The negated value of the `DD_TRACE_CLIENT_IP_HEADER_DISABLED` environment + # variable or `true` if it doesn't exist. # @return [Boolean] - option :disabled do |o| - o.default { env_to_bool(Tracing::Configuration::Ext::ClientIp::ENV_DISABLED, false) } + option :enabled do |o| + o.default { !env_to_bool(Tracing::Configuration::Ext::ClientIp::ENV_DISABLED, false) } o.lazy end diff --git a/lib/datadog/core/vendor/ipaddress/LICENSE.txt b/lib/datadog/core/vendor/ipaddress/LICENSE.txt deleted file mode 100644 index 943eee1df11..00000000000 --- a/lib/datadog/core/vendor/ipaddress/LICENSE.txt +++ /dev/null @@ -1,20 +0,0 @@ -Copyright (c) 2009-2015 Marco Ceresa - -Permission is hereby granted, free of charge, to any person obtaining -a copy of this software and associated documentation files (the -"Software"), to deal in the Software without restriction, including -without limitation the rights to use, copy, modify, merge, publish, -distribute, sublicense, and/or sell copies of the Software, and to -permit persons to whom the Software is furnished to do so, subject to -the following conditions: - -The above copyright notice and this permission notice shall be -included in all copies or substantial portions of the Software. - -THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, -EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF -MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND -NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE -LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION -OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION -WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. diff --git a/lib/datadog/core/vendor/ipaddress/ipaddress.rb b/lib/datadog/core/vendor/ipaddress/ipaddress.rb deleted file mode 100644 index b9ca439b0e7..00000000000 --- a/lib/datadog/core/vendor/ipaddress/ipaddress.rb +++ /dev/null @@ -1,76 +0,0 @@ -# Note: This is a partial vendor. Only functions that are needed by `dd-trace-rb`` were vendored. - -# -# = IPAddress -# -# A ruby library to manipulate IPv4 and IPv6 addresses -# -# -# Package:: IPAddress -# Author:: Marco Ceresa -# License:: Ruby License -# -#-- -# -#++ - -module IPAddress - - NAME = "IPAddress" - GEM = "ipaddress" - AUTHORS = ["Marco Ceresa "] - - # - # Checks if the given string is a valid IP address, - # either IPv4 or IPv6 - # - # Example: - # - # IPAddress::valid_ip? "2002::1" - # #=> true - # - # IPAddress::valid_ip? "10.0.0.256" - # #=> false - # - def self.valid_ip?(addr) - valid_ipv4?(addr) || valid_ipv6?(addr) - end - - # - # Checks if the given string is a valid IPv4 address - # - # Example: - # - # IPAddress::valid_ipv4? "2002::1" - # #=> false - # - # IPAddress::valid_ipv4? "172.16.10.1" - # #=> true - # - def self.valid_ipv4?(addr) - if /^(0|[1-9]{1}\d{0,2})\.(0|[1-9]{1}\d{0,2})\.(0|[1-9]{1}\d{0,2})\.(0|[1-9]{1}\d{0,2})$/ =~ addr - return $~.captures.all? {|i| i.to_i < 256} - end - false - end - - # - # Checks if the given string is a valid IPv6 address - # - # Example: - # - # IPAddress::valid_ipv6? "2002::1" - # #=> true - # - # IPAddress::valid_ipv6? "2002::DEAD::BEEF" - # #=> false - # - def self.valid_ipv6?(addr) - # https://gist.github.com/cpetschnig/294476 - # http://forums.intermapper.com/viewtopic.php?t=452 - return true if /^\s*((([0-9A-Fa-f]{1,4}:){7}([0-9A-Fa-f]{1,4}|:))|(([0-9A-Fa-f]{1,4}:){6}(:[0-9A-Fa-f]{1,4}|((25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(\.(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3})|:))|(([0-9A-Fa-f]{1,4}:){5}(((:[0-9A-Fa-f]{1,4}){1,2})|:((25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(\.(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3})|:))|(([0-9A-Fa-f]{1,4}:){4}(((:[0-9A-Fa-f]{1,4}){1,3})|((:[0-9A-Fa-f]{1,4})?:((25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(\.(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3}))|:))|(([0-9A-Fa-f]{1,4}:){3}(((:[0-9A-Fa-f]{1,4}){1,4})|((:[0-9A-Fa-f]{1,4}){0,2}:((25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(\.(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3}))|:))|(([0-9A-Fa-f]{1,4}:){2}(((:[0-9A-Fa-f]{1,4}){1,5})|((:[0-9A-Fa-f]{1,4}){0,3}:((25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(\.(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3}))|:))|(([0-9A-Fa-f]{1,4}:){1}(((:[0-9A-Fa-f]{1,4}){1,6})|((:[0-9A-Fa-f]{1,4}){0,4}:((25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(\.(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3}))|:))|(:(((:[0-9A-Fa-f]{1,4}){1,7})|((:[0-9A-Fa-f]{1,4}){0,5}:((25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(\.(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3}))|:)))(%.+)?\s*$/ =~ addr - false - end - - -end diff --git a/lib/datadog/tracing/client_ip.rb b/lib/datadog/tracing/client_ip.rb index b68817a1abc..440d110838f 100644 --- a/lib/datadog/tracing/client_ip.rb +++ b/lib/datadog/tracing/client_ip.rb @@ -1,6 +1,5 @@ # typed: true -require_relative '../core/vendor/ipaddress/ipaddress' require_relative '../core/configuration' require_relative 'metadata/ext' require_relative 'span' @@ -36,7 +35,7 @@ module ClientIp # @param [HeaderCollection, #get, nil] headers A collection with the request headers. # @param [String, nil] remote_ip The remote IP the request associated with the span is sent to. def self.set_client_ip_tag(span, headers: nil, remote_ip: nil) - return if configuration.disabled + return unless configuration.enabled result = raw_ip_from_request(headers, remote_ip) @@ -120,7 +119,16 @@ def self.likely_ipv4?(value) # Determines whether the given string is a valid IPv4 or IPv6 address. def self.valid_ip?(ip) - IPAddress.valid_ip?(ip) + # Client IPs should not have subnet masks even though IPAddr can parse them. + return false if ip.include?('/') + + begin + IPAddr.new(ip) + + true + rescue IPAddr::Error + false + end end def self.ip_headers(headers) diff --git a/spec/datadog/tracing/client_ip_spec.rb b/spec/datadog/tracing/client_ip_spec.rb index ad946162578..cb4faf58d6f 100644 --- a/spec/datadog/tracing/client_ip_spec.rb +++ b/spec/datadog/tracing/client_ip_spec.rb @@ -11,7 +11,7 @@ before do Datadog.configure do |c| - c.tracing.client_ip.disabled = !tagging_enabled + c.tracing.client_ip.enabled = tagging_enabled c.tracing.client_ip.header_name = ip_header_name end end From f49d03e3d96cd43375a3d4f9fcdbdb7cf815e8be Mon Sep 17 00:00:00 2001 From: Bar Weiss Date: Mon, 5 Sep 2022 19:12:26 +0300 Subject: [PATCH 11/14] Remove 'ipaddress' from LICENSE-3rdparty --- LICENSE-3rdparty.csv | 1 - 1 file changed, 1 deletion(-) diff --git a/LICENSE-3rdparty.csv b/LICENSE-3rdparty.csv index ba84771ce98..4dfbe99c24e 100644 --- a/LICENSE-3rdparty.csv +++ b/LICENSE-3rdparty.csv @@ -1,6 +1,5 @@ Component,Origin,License,Copyright lib/datadog/core/vendor/multipart-post,https://github.com/socketry/multipart-post,MIT,"Copyright (c) 2007-2013 Nick Sieger." -lib/datadog/core/vendor/ipaddress,https://github.com/ipaddress-gem/ipaddress,MIT,"Copyright © 2009-2015 Marco Ceresa and Mike Mackintosh." lib/datadog/tracing/contrib/active_record/vendor,https://github.com/rails/rails/,MIT,"Copyright (c) 2005-2018 David Heinemeier Hansson" ext/ddtrace_profiling_native_extension/private_vm_api_access,https://github.com/ruby/ruby,BSD-2-Clause,"Copyright (C) 1993-2013 Yukihiro Matsumoto. All rights reserved." msgpack,https://rubygems.org/gems/msgpack,Apache-2.0,"Copyright (c) 2008-2015 Sadayuki Furuhashi" From 2dae9e617f2fdf35a3ed786890a1ce18bea1fc54 Mon Sep 17 00:00:00 2001 From: Bar Weiss Date: Mon, 5 Sep 2022 19:23:52 +0300 Subject: [PATCH 12/14] Update some documentation --- lib/datadog/tracing/client_ip.rb | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/lib/datadog/tracing/client_ip.rb b/lib/datadog/tracing/client_ip.rb index 440d110838f..e33da386e9e 100644 --- a/lib/datadog/tracing/client_ip.rb +++ b/lib/datadog/tracing/client_ip.rb @@ -51,19 +51,22 @@ def self.set_client_ip_tag(span, headers: nil, remote_ip: nil) IpExtractionResult = Struct.new(:raw_ip, :multiple_ip_headers) - # Returns the value of an IP-related header or the request's remote IP. + # Returns a result struct that holds the raw client IP associated with the request if it was + # retrieved successfully. # # The client IP is looked up by the following logic: # * If the user has configured a header name, return that header's value. # * If exactly one of the known IP headers is present, return that header's value. # * If none of the known IP headers are present, return the remote IP from the request. # - # Raises a [MultipleIpHeadersError] if multiple IP-related headers are present. + # If more than one of the known IP headers is present, the result will have a `multiple_ip_headers` + # field with the name of the present IP headers. # # @param [Datadog::Core::HeaderCollection, #get, nil] headers The request headers # @param [String] remote_ip The remote IP of the request. - # @return [String] An unprocessed value retrieved from an - # IP header or the remote IP of the request. + # @return [IpExtractionResult] A struct that holds the unprocessed IP value, + # or `nil` if it wasn't found. Additionally, the `multiple_ip_headers` fields will hold the + # name of known IP headers present in the request if more than one of these were found. def self.raw_ip_from_request(headers, remote_ip) return IpExtractionResult.new(headers && headers.get(configuration.header_name), nil) if configuration.header_name From 76e6e0a92d2b07014cf51b7494c82c8c0713bdb3 Mon Sep 17 00:00:00 2001 From: Bar Weiss Date: Wed, 7 Sep 2022 13:12:25 +0300 Subject: [PATCH 13/14] Fix IPAddr usage for Ruby <= 2.3 --- lib/datadog/tracing/client_ip.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/datadog/tracing/client_ip.rb b/lib/datadog/tracing/client_ip.rb index e33da386e9e..3b3ce366e29 100644 --- a/lib/datadog/tracing/client_ip.rb +++ b/lib/datadog/tracing/client_ip.rb @@ -4,6 +4,8 @@ require_relative 'metadata/ext' require_relative 'span' +require 'ipaddr' + module Datadog module Tracing # Common functions for supporting the `http.client_ip` span attribute. From ea3b0647b0259f9628c466d709761f34de6240a5 Mon Sep 17 00:00:00 2001 From: Bar Weiss Date: Thu, 8 Sep 2022 14:07:54 +0300 Subject: [PATCH 14/14] Fix Ruby 2.1-2.4 compatibility issue --- lib/datadog/core/header_collection.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/datadog/core/header_collection.rb b/lib/datadog/core/header_collection.rb index 55362001e2e..ec837024079 100644 --- a/lib/datadog/core/header_collection.rb +++ b/lib/datadog/core/header_collection.rb @@ -28,7 +28,9 @@ def self.from_hash(hash) class HashHeaderCollection < HeaderCollection def initialize(hash) super() - @hash = hash.transform_keys(&:downcase) + @hash = {}.tap do |res| + hash.each_pair { |key, value| res[key.downcase] = value } + end end def get(header_name)