diff --git a/app/models/push/subscription.rb b/app/models/push/subscription.rb index 0b273b563..afd417367 100644 --- a/app/models/push/subscription.rb +++ b/app/models/push/subscription.rb @@ -1,14 +1,54 @@ class Push::Subscription < ApplicationRecord + PERMITTED_ENDPOINT_HOSTS = %w[ + fcm.googleapis.com + updates.push.services.mozilla.com + web.push.apple.com + notify.windows.com + ].freeze + belongs_to :account, default: -> { user.account } belongs_to :user + validates :endpoint, presence: true + validate :validate_endpoint_url + def notification(**params) WebPush::Notification.new( **params, badge: user.notifications.unread.count, endpoint: endpoint, + endpoint_ip: resolved_endpoint_ip, p256dh_key: p256dh_key, auth_key: auth_key ) end + + def resolved_endpoint_ip + return @resolved_endpoint_ip if defined?(@resolved_endpoint_ip) + @resolved_endpoint_ip = SsrfProtection.resolve_public_ip(endpoint_uri&.host) + end + + private + def endpoint_uri + @endpoint_uri ||= URI.parse(endpoint) if endpoint.present? + rescue URI::InvalidURIError + nil + end + + def validate_endpoint_url + if endpoint_uri.nil? + errors.add(:endpoint, "is not a valid URL") + elsif endpoint_uri.scheme != "https" + errors.add(:endpoint, "must use HTTPS") + elsif !permitted_endpoint_host? + errors.add(:endpoint, "is not a permitted push service") + elsif resolved_endpoint_ip.nil? + errors.add(:endpoint, "resolves to a private or invalid IP address") + end + end + + def permitted_endpoint_host? + host = endpoint_uri&.host&.downcase + PERMITTED_ENDPOINT_HOSTS.any? { |permitted| host&.end_with?(permitted) } + end end diff --git a/app/models/ssrf_protection.rb b/app/models/ssrf_protection.rb index f3df2511b..580d15713 100644 --- a/app/models/ssrf_protection.rb +++ b/app/models/ssrf_protection.rb @@ -4,7 +4,9 @@ module SsrfProtection DNS_RESOLUTION_TIMEOUT = 2 DISALLOWED_IP_RANGES = [ - IPAddr.new("0.0.0.0/8") # Broadcasts + IPAddr.new("0.0.0.0/8"), # "This" network (RFC1700) + IPAddr.new("100.64.0.0/10"), # Carrier-grade NAT (RFC6598) + IPAddr.new("198.18.0.0/15") # Benchmark testing (RFC2544) ].freeze def resolve_public_ip(hostname) diff --git a/config/initializers/web_push.rb b/config/initializers/web_push.rb index ac960468e..6914dae22 100644 --- a/config/initializers/web_push.rb +++ b/config/initializers/web_push.rb @@ -17,7 +17,15 @@ end module WebPush::PersistentRequest def perform - if @options[:connection] + endpoint_ip = @options[:endpoint_ip] + + if endpoint_ip + http = Net::HTTP.new(uri.host, uri.port, ipaddr: endpoint_ip) + http.use_ssl = true + http.ssl_timeout = @options[:ssl_timeout] unless @options[:ssl_timeout].nil? + http.open_timeout = @options[:open_timeout] unless @options[:open_timeout].nil? + http.read_timeout = @options[:read_timeout] unless @options[:read_timeout].nil? + elsif @options[:connection] http = @options[:connection] else http = Net::HTTP.new(uri.host, uri.port, *proxy_options) diff --git a/lib/web_push/notification.rb b/lib/web_push/notification.rb index d2865ff46..4c873fb48 100644 --- a/lib/web_push/notification.rb +++ b/lib/web_push/notification.rb @@ -1,13 +1,13 @@ class WebPush::Notification - def initialize(title:, body:, path:, badge:, endpoint:, p256dh_key:, auth_key:) + def initialize(title:, body:, path:, badge:, endpoint:, endpoint_ip:, p256dh_key:, auth_key:) @title, @body, @path, @badge = title, body, path, badge - @endpoint, @p256dh_key, @auth_key = endpoint, p256dh_key, auth_key + @endpoint, @endpoint_ip, @p256dh_key, @auth_key = endpoint, endpoint_ip, p256dh_key, auth_key end def deliver(connection: nil) WebPush.payload_send \ message: encoded_message, - endpoint: @endpoint, p256dh: @p256dh_key, auth: @auth_key, + endpoint: @endpoint, endpoint_ip: @endpoint_ip, p256dh: @p256dh_key, auth: @auth_key, vapid: vapid_identification, connection: connection, urgency: "high" diff --git a/test/controllers/users/push_subscriptions_controller_test.rb b/test/controllers/users/push_subscriptions_controller_test.rb index 9c77dec9d..c41d8313e 100644 --- a/test/controllers/users/push_subscriptions_controller_test.rb +++ b/test/controllers/users/push_subscriptions_controller_test.rb @@ -1,12 +1,15 @@ require "test_helper" class Users::PushSubscriptionsControllerTest < ActionDispatch::IntegrationTest + PUBLIC_TEST_IP = "142.250.185.206" + setup do sign_in_as :david + stub_dns_resolution(PUBLIC_TEST_IP) end test "create new push subscription" do - subscription_params = { "endpoint" => "https://apple", "p256dh_key" => "123", "auth_key" => "456" } + subscription_params = { "endpoint" => "https://fcm.googleapis.com/fcm/send/abc123", "p256dh_key" => "123", "auth_key" => "456" } post user_push_subscriptions_path(users(:david)), params: { push_subscription: subscription_params }, headers: { "HTTP_USER_AGENT" => "Mozilla/5.0" } @@ -19,7 +22,7 @@ class Users::PushSubscriptionsControllerTest < ActionDispatch::IntegrationTest test "touch existing subscription" do existing_subscription = users(:david).push_subscriptions.create!( - endpoint: "https://apple", + endpoint: "https://fcm.googleapis.com/fcm/send/abc123", p256dh_key: "123", auth_key: "456" ) @@ -37,7 +40,7 @@ class Users::PushSubscriptionsControllerTest < ActionDispatch::IntegrationTest test "destroy a push subscription" do subscription = users(:david).push_subscriptions.create!( - endpoint: "https://apple", + endpoint: "https://fcm.googleapis.com/fcm/send/abc123", p256dh_key: "123", auth_key: "456" ) @@ -47,4 +50,35 @@ class Users::PushSubscriptionsControllerTest < ActionDispatch::IntegrationTest assert_redirected_to user_push_subscriptions_path(users(:david)) end end + + test "rejects subscription with non-permitted endpoint" do + subscription_params = { "endpoint" => "https://attacker.example.com/steal", "p256dh_key" => "123", "auth_key" => "456" } + + assert_no_difference -> { Push::Subscription.count } do + post user_push_subscriptions_path(users(:david)), + params: { push_subscription: subscription_params } + end + + assert_response :unprocessable_entity + end + + test "rejects subscription with endpoint resolving to private IP" do + stub_dns_resolution("192.168.1.1") + + subscription_params = { "endpoint" => "https://fcm.googleapis.com/fcm/send/abc123", "p256dh_key" => "123", "auth_key" => "456" } + + assert_no_difference -> { Push::Subscription.count } do + post user_push_subscriptions_path(users(:david)), + params: { push_subscription: subscription_params } + end + + assert_response :unprocessable_entity + end + + private + def stub_dns_resolution(*ips) + dns_mock = mock("dns") + dns_mock.stubs(:each_address).multiple_yields(*ips) + Resolv::DNS.stubs(:open).yields(dns_mock) + end end diff --git a/test/models/push/subscription_test.rb b/test/models/push/subscription_test.rb new file mode 100644 index 000000000..9f4650614 --- /dev/null +++ b/test/models/push/subscription_test.rb @@ -0,0 +1,124 @@ +require "test_helper" + +class Push::SubscriptionTest < ActiveSupport::TestCase + PUBLIC_TEST_IP = "142.250.185.206" # google.com IP + + setup do + stub_dns_resolution(PUBLIC_TEST_IP) + end + + test "valid subscription with permitted endpoint" do + subscription = Push::Subscription.new( + user: users(:david), + endpoint: "https://fcm.googleapis.com/fcm/send/abc123", + p256dh_key: "test_key", + auth_key: "test_auth" + ) + + assert subscription.valid? + end + + test "rejects endpoint with non-https scheme" do + subscription = Push::Subscription.new( + user: users(:david), + endpoint: "http://fcm.googleapis.com/fcm/send/abc123", + p256dh_key: "test_key", + auth_key: "test_auth" + ) + + assert_not subscription.valid? + assert_includes subscription.errors[:endpoint], "must use HTTPS" + end + + test "rejects endpoint with non-permitted host" do + subscription = Push::Subscription.new( + user: users(:david), + endpoint: "https://attacker.example.com/webhook", + p256dh_key: "test_key", + auth_key: "test_auth" + ) + + assert_not subscription.valid? + assert_includes subscription.errors[:endpoint], "is not a permitted push service" + end + + test "rejects endpoint that resolves to private IP" do + stub_dns_resolution("192.168.1.1") + + subscription = Push::Subscription.new( + user: users(:david), + endpoint: "https://fcm.googleapis.com/fcm/send/abc123", + p256dh_key: "test_key", + auth_key: "test_auth" + ) + + assert_not subscription.valid? + assert_includes subscription.errors[:endpoint], "resolves to a private or invalid IP address" + end + + test "rejects endpoint that resolves to loopback IP" do + stub_dns_resolution("127.0.0.1") + + subscription = Push::Subscription.new( + user: users(:david), + endpoint: "https://fcm.googleapis.com/fcm/send/abc123", + p256dh_key: "test_key", + auth_key: "test_auth" + ) + + assert_not subscription.valid? + assert_includes subscription.errors[:endpoint], "resolves to a private or invalid IP address" + end + + test "rejects endpoint that resolves to link-local IP (AWS IMDS)" do + stub_dns_resolution("169.254.169.254") + + subscription = Push::Subscription.new( + user: users(:david), + endpoint: "https://fcm.googleapis.com/fcm/send/abc123", + p256dh_key: "test_key", + auth_key: "test_auth" + ) + + assert_not subscription.valid? + assert_includes subscription.errors[:endpoint], "resolves to a private or invalid IP address" + end + + test "resolved_endpoint_ip returns pinned public IP" do + subscription = Push::Subscription.new( + user: users(:david), + endpoint: "https://fcm.googleapis.com/fcm/send/abc123", + p256dh_key: "test_key", + auth_key: "test_auth" + ) + + assert_equal PUBLIC_TEST_IP, subscription.resolved_endpoint_ip + end + + test "accepts all permitted push service domains" do + permitted_endpoints = [ + "https://fcm.googleapis.com/fcm/send/token123", + "https://updates.push.services.mozilla.com/wpush/v2/token123", + "https://web.push.apple.com/QaBC123", + "https://wns2-db5p.notify.windows.com/w/?token=abc123" + ] + + permitted_endpoints.each do |endpoint| + subscription = Push::Subscription.new( + user: users(:david), + endpoint: endpoint, + p256dh_key: "test_key", + auth_key: "test_auth" + ) + + assert subscription.valid?, "Expected #{endpoint} to be valid, got errors: #{subscription.errors.full_messages}" + end + end + + private + def stub_dns_resolution(*ips) + dns_mock = mock("dns") + dns_mock.stubs(:each_address).multiple_yields(*ips) + Resolv::DNS.stubs(:open).yields(dns_mock) + end +end diff --git a/test/models/ssrf_protection_test.rb b/test/models/ssrf_protection_test.rb new file mode 100644 index 000000000..d56f7390a --- /dev/null +++ b/test/models/ssrf_protection_test.rb @@ -0,0 +1,60 @@ +require "test_helper" + +class SsrfProtectionTest < ActiveSupport::TestCase + test "blocks loopback addresses" do + stub_dns_resolution("127.0.0.1") + assert_nil SsrfProtection.resolve_public_ip("localhost") + end + + test "blocks private 10.x.x.x addresses" do + stub_dns_resolution("10.0.0.1") + assert_nil SsrfProtection.resolve_public_ip("internal.example.com") + end + + test "blocks private 172.16.x.x addresses" do + stub_dns_resolution("172.16.0.1") + assert_nil SsrfProtection.resolve_public_ip("internal.example.com") + end + + test "blocks private 192.168.x.x addresses" do + stub_dns_resolution("192.168.1.1") + assert_nil SsrfProtection.resolve_public_ip("internal.example.com") + end + + test "blocks link-local addresses (AWS metadata endpoint)" do + stub_dns_resolution("169.254.169.254") + assert_nil SsrfProtection.resolve_public_ip("metadata.example.com") + end + + test "blocks carrier-grade NAT addresses" do + stub_dns_resolution("100.64.0.1") + assert_nil SsrfProtection.resolve_public_ip("cgnat.example.com") + end + + test "blocks benchmark testing addresses" do + stub_dns_resolution("198.18.0.1") + assert_nil SsrfProtection.resolve_public_ip("benchmark.example.com") + end + + test "blocks broadcast addresses" do + stub_dns_resolution("0.0.0.1") + assert_nil SsrfProtection.resolve_public_ip("broadcast.example.com") + end + + test "allows public addresses" do + stub_dns_resolution("93.184.216.34") + assert_equal "93.184.216.34", SsrfProtection.resolve_public_ip("example.com") + end + + test "returns first public IP when multiple addresses resolve" do + stub_dns_resolution("10.0.0.1", "93.184.216.34", "192.168.1.1") + assert_equal "93.184.216.34", SsrfProtection.resolve_public_ip("multi.example.com") + end + + private + def stub_dns_resolution(*ips) + dns_mock = mock("dns") + dns_mock.stubs(:each_address).multiple_yields(*ips) + Resolv::DNS.stubs(:open).yields(dns_mock) + end +end