From 406a79d9ebed129919748cba6a27e393fc1befbe Mon Sep 17 00:00:00 2001 From: Dan Milne Date: Thu, 11 Jun 2026 08:14:45 +1000 Subject: [PATCH] Block SSRF via backchannel_logout_uri MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit backchannel_logout_uri was validated only for scheme/HTTPS, so an admin (or a compromised admin account) could point it at internal infrastructure — cloud metadata (169.254.169.254), loopback, or RFC1918 hosts — and every user logout would fire a server-side POST there. Add PrivateAddressCheck (app/lib) and apply it as defense-in-depth: - Application validation rejects URIs whose host is, or is a literal, internal address (loopback / private / link-local / 0.0.0.0 / localhost / metadata hostnames). Fast, DNS-free, immediate admin feedback. - BackchannelLogoutJob re-checks at request time WITH DNS resolution and aborts (no retry) if the host resolves to a non-public address — covering URIs that predate the validation and public hostnames pointed at internal IPs. Tests cover the address classification, the model validation, and updates an existing test that used a localhost logout URI. Co-Authored-By: Claude Fable 5 --- app/jobs/backchannel_logout_job.rb | 8 ++++ app/lib/private_address_check.rb | 57 +++++++++++++++++++++++ app/models/application.rb | 14 ++++++ test/integration/session_security_test.rb | 2 +- test/lib/private_address_check_test.rb | 38 +++++++++++++++ test/models/application_test.rb | 25 ++++++++++ 6 files changed, 143 insertions(+), 1 deletion(-) create mode 100644 app/lib/private_address_check.rb create mode 100644 test/lib/private_address_check_test.rb diff --git a/app/jobs/backchannel_logout_job.rb b/app/jobs/backchannel_logout_job.rb index 9a97e61..989eb7f 100644 --- a/app/jobs/backchannel_logout_job.rb +++ b/app/jobs/backchannel_logout_job.rb @@ -28,6 +28,14 @@ class BackchannelLogoutJob < ApplicationJob # Send HTTP POST to the application's backchannel logout URI uri = URI.parse(application.backchannel_logout_uri) + # SSRF guard: re-check at request time (with DNS resolution) in case the URI + # predates the validation, or a public hostname now resolves to an internal + # address. Abort without retrying — retries would not change the outcome. + if PrivateAddressCheck.internal_host?(uri.host) || PrivateAddressCheck.resolves_to_internal?(uri.host) + Rails.logger.error "BackchannelLogout: Refusing to send logout to #{application.name} - #{uri.host} is or resolves to a non-public address (SSRF guard)" + return + end + begin response = Net::HTTP.start(uri.host, uri.port, use_ssl: uri.scheme == "https", open_timeout: 5, read_timeout: 5) do |http| request = Net::HTTP::Post.new(uri.path.presence || "/") diff --git a/app/lib/private_address_check.rb b/app/lib/private_address_check.rb new file mode 100644 index 0000000..8bfafa1 --- /dev/null +++ b/app/lib/private_address_check.rb @@ -0,0 +1,57 @@ +require "ipaddr" +require "resolv" + +# SSRF guard for outbound requests to admin-configured URLs (currently the OIDC +# backchannel logout endpoint). Blocks hosts that are, or resolve to, private, +# loopback, link-local (incl. the cloud metadata address 169.254.169.254) or +# otherwise non-public address space. +module PrivateAddressCheck + module_function + + # Hostnames that are internal by definition and must never be dialled. + BLOCKED_HOSTNAMES = %w[localhost metadata.google.internal].freeze + + # Fast, DNS-free check: catches IP literals and well-known internal hostnames. + # Suitable for model validation (deterministic, immediate admin feedback). + def internal_host?(host) + host = host.to_s.downcase + return true if host.blank? + return true if BLOCKED_HOSTNAMES.include?(host) + return true if host.end_with?(".localhost") + + ip = parse_ip(host) + ip ? internal_ip?(ip) : false + end + + # Authoritative check: resolves the hostname and blocks if ANY address is + # internal. Suitable for request time — also defeats a public hostname that + # has been pointed at an internal IP (DNS rebinding to internal space). + def resolves_to_internal?(host) + addresses(host).any? { |ip| internal_ip?(ip) } + end + + def addresses(host) + ip = parse_ip(host) + return [ip] if ip + + Resolv.getaddresses(host.to_s).filter_map { |a| parse_ip(a) } + rescue StandardError + # Resolution failure: surface no addresses. Callers treat "can't resolve" as + # not-provably-internal; the dial itself will then fail safely. + [] + end + + def internal_ip?(ip) + ip.loopback? || ip.private? || ip.link_local? || unspecified?(ip) + end + + def parse_ip(str) + IPAddr.new(str.to_s) + rescue IPAddr::Error + nil + end + + def unspecified?(ip) + ip == IPAddr.new("0.0.0.0") || ip == IPAddr.new("::") + end +end diff --git a/app/models/application.rb b/app/models/application.rb index 7c32717..3932d81 100644 --- a/app/models/application.rb +++ b/app/models/application.rb @@ -56,6 +56,7 @@ class Application < ApplicationRecord message: "must be a valid HTTP or HTTPS URL" } validate :backchannel_logout_uri_must_be_https_in_production, if: -> { backchannel_logout_uri.present? } + validate :backchannel_logout_uri_not_internal, if: -> { backchannel_logout_uri.present? } # Icon validation using ActiveStorage validators validate :icon_validation @@ -390,4 +391,17 @@ class Application < ApplicationRecord # Let the format validator handle invalid URIs end end + + # SSRF guard: the backchannel logout URI is dialled server-side on every user + # logout, so it must not target internal infrastructure (loopback, private + # ranges, or the link-local cloud metadata endpoint). This is the fast, + # config-time check; BackchannelLogoutJob re-checks with DNS resolution. + def backchannel_logout_uri_not_internal + uri = URI.parse(backchannel_logout_uri) + if uri.host.present? && PrivateAddressCheck.internal_host?(uri.host) + errors.add(:backchannel_logout_uri, "must not point to a private, loopback, or link-local address") + end + rescue URI::InvalidURIError + # Let the format validator handle invalid URIs + end end diff --git a/test/integration/session_security_test.rb b/test/integration/session_security_test.rb index d9412ec..fb7cea3 100644 --- a/test/integration/session_security_test.rb +++ b/test/integration/session_security_test.rb @@ -199,7 +199,7 @@ class SessionSecurityTest < ActionDispatch::IntegrationTest slug: "logout-test-app", app_type: "oidc", redirect_uris: ["http://localhost:4000/callback"].to_json, - backchannel_logout_uri: "http://localhost:4000/logout", + backchannel_logout_uri: "https://rp.example.com/backchannel-logout", active: true ) diff --git a/test/lib/private_address_check_test.rb b/test/lib/private_address_check_test.rb new file mode 100644 index 0000000..3730ceb --- /dev/null +++ b/test/lib/private_address_check_test.rb @@ -0,0 +1,38 @@ +require "test_helper" + +class PrivateAddressCheckTest < ActiveSupport::TestCase + # internal_host? — DNS-free checks on IP literals and known hostnames + test "flags loopback, private, and link-local IP literals as internal" do + %w[ + 127.0.0.1 + 10.0.0.1 + 172.16.5.5 + 192.168.1.1 + 169.254.169.254 + 0.0.0.0 + ::1 + ].each do |host| + assert PrivateAddressCheck.internal_host?(host), "expected #{host} to be internal" + end + end + + test "flags localhost-style hostnames as internal" do + assert PrivateAddressCheck.internal_host?("localhost") + assert PrivateAddressCheck.internal_host?("foo.localhost") + assert PrivateAddressCheck.internal_host?("metadata.google.internal") + assert PrivateAddressCheck.internal_host?("") + end + + test "does not flag public IP literals as internal" do + refute PrivateAddressCheck.internal_host?("8.8.8.8") + refute PrivateAddressCheck.internal_host?("1.1.1.1") + end + + # resolves_to_internal? on IP literals (no DNS needed) exercises the same + # address classification used after resolution. + test "resolves_to_internal? classifies IP literals" do + assert PrivateAddressCheck.resolves_to_internal?("169.254.169.254") + assert PrivateAddressCheck.resolves_to_internal?("127.0.0.1") + refute PrivateAddressCheck.resolves_to_internal?("8.8.8.8") + end +end diff --git a/test/models/application_test.rb b/test/models/application_test.rb index 052725b..4cda8f7 100644 --- a/test/models/application_test.rb +++ b/test/models/application_test.rb @@ -56,4 +56,29 @@ class ApplicationTest < ActiveSupport::TestCase tempfile&.close tempfile&.unlink end + + test "rejects backchannel_logout_uri pointing at internal addresses (SSRF guard)" do + app = applications(:kavita_app) + + internal_uris = [ + "http://127.0.0.1/logout", + "http://localhost/logout", + "https://169.254.169.254/latest/meta-data/", + "http://10.0.0.5/logout", + "http://192.168.1.10/logout" + ] + + internal_uris.each do |uri| + app.backchannel_logout_uri = uri + refute app.valid?, "expected #{uri} to be rejected" + assert_includes app.errors[:backchannel_logout_uri].join, "private, loopback, or link-local" + end + end + + test "allows backchannel_logout_uri pointing at a public host" do + app = applications(:kavita_app) + app.backchannel_logout_uri = "https://relying-party.example.com/backchannel-logout" + + assert app.valid?, app.errors.full_messages.to_sentence + end end