Skip to content

Commit

Permalink
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Don't patch ssl
Browse files Browse the repository at this point in the history
arkadiyt committed Nov 3, 2024
1 parent 651a711 commit 1062abc
Showing 10 changed files with 65 additions and 211 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/build-test.yml
Original file line number Diff line number Diff line change
@@ -9,7 +9,7 @@ jobs:
build-test:
strategy:
matrix:
ruby-version: [2.6.0, 2.7.0, 3.0.0, 3.1.0, 3.2.0, head]
ruby-version: [2.7.0, 3.0.0, 3.1.0, 3.2.0, 3.3.0, head]
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
3 changes: 2 additions & 1 deletion .rspec
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
--color
--order random
--warning
--order random
--require spec_helper
--format documentation
10 changes: 5 additions & 5 deletions .rubocop.yml
Original file line number Diff line number Diff line change
@@ -7,8 +7,8 @@ require:
AllCops:
NewCops: enable

Gemspec/RequiredRubyVersion:
Enabled: false
Gemspec/DevelopmentDependencies:
EnforcedStyle: gemspec

Metrics/AbcSize:
Enabled: false
@@ -52,15 +52,15 @@ Layout/SpaceInsideHashLiteralBraces:
RSpec/BeforeAfterAll:
Enabled: false

RSpec/IndexedLet:
Enabled: false

RSpec/MultipleExpectations:
Enabled: false

RSpec/ExampleLength:
Max: 40

RSpec/FilePath:
SpecSuffixOnly: true

RSpec/MessageSpies:
EnforcedStyle: receive

3 changes: 1 addition & 2 deletions Dockerfile
Original file line number Diff line number Diff line change
@@ -5,6 +5,5 @@ RUN apt update && apt-get install -y vim tmux tig
WORKDIR app
COPY Gemfile ssrf_filter.gemspec .
COPY lib/ssrf_filter/version.rb lib/ssrf_filter/version.rb
RUN bundle update
RUN bundle install
ENV CI=1
COPY . .
1 change: 0 additions & 1 deletion lib/ssrf_filter.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
# frozen_string_literal: true

require 'ssrf_filter/patch/ssl_socket'
require 'ssrf_filter/ssrf_filter'
require 'ssrf_filter/version'
66 changes: 0 additions & 66 deletions lib/ssrf_filter/patch/ssl_socket.rb

This file was deleted.

59 changes: 22 additions & 37 deletions lib/ssrf_filter/ssrf_filter.rb
Original file line number Diff line number Diff line change
@@ -10,7 +10,7 @@ def self.prefixlen_from_ipaddr(ipaddr)
mask_addr = ipaddr.instance_variable_get('@mask_addr')
raise ArgumentError, 'Invalid mask' if mask_addr.zero?

while (mask_addr & 0x1).zero?
while mask_addr.nobits?(0x1)
mask_addr >>= 1
end

@@ -84,8 +84,6 @@ def self.prefixlen_from_ipaddr(ipaddr)
patch: ::Net::HTTP::Patch
}.freeze

FIBER_HOSTNAME_KEY = :__ssrf_filter_hostname

class Error < ::StandardError
end

@@ -106,8 +104,6 @@ class CRLFInjection < Error

%i[get put post delete head patch].each do |method|
define_singleton_method(method) do |url, options = {}, &block|
::SsrfFilter::Patch::SSLSocket.apply!

original_url = url
scheme_whitelist = options.fetch(:scheme_whitelist, DEFAULT_SCHEME_WHITELIST)
resolver = options.fetch(:resolver, DEFAULT_RESOLVER)
@@ -156,16 +152,16 @@ def self.ipaddr_has_mask?(ipaddr)
end
private_class_method :ipaddr_has_mask?

def self.host_header(hostname, uri)
def self.normalized_hostname(uri)
# Attach port for non-default as per RFC2616
if (uri.port == 80 && uri.scheme == 'http') ||
(uri.port == 443 && uri.scheme == 'https')
hostname
uri.hostname
else
"#{hostname}:#{uri.port}"
"#{uri.hostname}:#{uri.port}"
end
end
private_class_method :host_header
private_class_method :normalized_hostname

def self.fetch_once(uri, ip, verb, options, &block)
if options[:params]
@@ -174,11 +170,8 @@ def self.fetch_once(uri, ip, verb, options, &block)
uri.query = ::URI.encode_www_form(params)
end

hostname = uri.hostname
uri.hostname = ip

request = VERB_MAP[verb].new(uri)
request['host'] = host_header(hostname, uri)
request['host'] = normalized_hostname(uri)

Array(options[:headers]).each do |header, value|
request[header] = value
@@ -189,24 +182,24 @@ def self.fetch_once(uri, ip, verb, options, &block)
options[:request_proc].call(request) if options[:request_proc].respond_to?(:call)
validate_request(request)

http_options = options[:http_options] || {}
http_options[:use_ssl] = (uri.scheme == 'https')
http_options = (options[:http_options] || {}).merge(
use_ssl: uri.scheme == 'https',
ipaddr: ip
)

with_forced_hostname(hostname) do
::Net::HTTP.start(uri.hostname, uri.port, **http_options) do |http|
response = http.request(request) do |res|
block&.call(res)
end
case response
when ::Net::HTTPRedirection
url = response['location']
# Handle relative redirects
url = "#{uri.scheme}://#{hostname}:#{uri.port}#{url}" if url.start_with?('/')
else
url = nil
end
return response, url
::Net::HTTP.start(uri.hostname, uri.port, **http_options) do |http|
response = http.request(request) do |res|
block&.call(res)
end
case response
when ::Net::HTTPRedirection
url = response['location']
# Handle relative redirects
url = "#{uri.scheme}://#{normalized_hostname(uri)}#{url}" if url.start_with?('/')
else
url = nil
end
return response, url
end
end
private_class_method :fetch_once
@@ -223,12 +216,4 @@ def self.validate_request(request)
end
end
private_class_method :validate_request

def self.with_forced_hostname(hostname, &_block)
::Thread.current[FIBER_HOSTNAME_KEY] = hostname
yield
ensure
::Thread.current[FIBER_HOSTNAME_KEY] = nil
end
private_class_method :with_forced_hostname
end
15 changes: 0 additions & 15 deletions spec/lib/patch/ssl_socket_spec.rb

This file was deleted.

104 changes: 27 additions & 77 deletions spec/lib/ssrf_filter_spec.rb
Original file line number Diff line number Diff line change
@@ -47,8 +47,7 @@
end

it 'returns true for unknown ip families' do
allow(public_ipv4).to receive(:ipv4?).and_return(false)
allow(public_ipv4).to receive(:ipv6?).and_return(false)
allow(public_ipv4).to receive_messages(ipv4?: false, ipv6?: false)
expect(described_class.unsafe_ip_address?(public_ipv4)).to be(true)
end
end
@@ -79,7 +78,7 @@

describe 'fetch_once' do
it 'sets the host header' do
stub_request(:post, "https://#{public_ipv4}").with(headers: {host: 'www.example.com'})
stub_request(:post, 'https://www.example.com').with(headers: {host: 'www.example.com'})
.to_return(status: 200, body: 'response body')
response, url = described_class.fetch_once(URI('https://www.example.com'), public_ipv4.to_s, :post, {})
expect(response.code).to eq('200')
@@ -88,7 +87,7 @@
end

it 'does not send the port in the host header for default ports (http)' do
stub_request(:post, "http://#{public_ipv4}").with(headers: {host: 'www.example.com'})
stub_request(:post, 'http://www.example.com').with(headers: {host: 'www.example.com'})
.to_return(status: 200, body: 'response body')
response, url = described_class.fetch_once(URI('http://www.example.com'), public_ipv4.to_s, :post, {})
expect(response.code).to eq('200')
@@ -97,16 +96,15 @@
end

it 'sends the port in the host header for non-default ports' do
stub_request(:post, "https://#{public_ipv4}:80").with(headers: {host: 'www.example.com:80'})
.to_return(status: 200, body: 'response body')
stub_request(:post, 'https://www.example.com:80').to_return(status: 200, body: 'response body')
response, url = described_class.fetch_once(URI('https://www.example.com:80'), public_ipv4.to_s, :post, {})
expect(response.code).to eq('200')
expect(response.body).to eq('response body')
expect(url).to be_nil
end

it 'passes headers, params, and blocks' do
stub_request(:get, "https://#{public_ipv4}/?key=value").with(headers:
stub_request(:get, 'https://www.example.com/?key=value').with(headers:
{host: 'www.example.com', header: 'value', header2: 'value2'}).to_return(status: 200, body: 'response body')
options = {
headers: {'header' => 'value'},
@@ -123,47 +121,26 @@
end

it 'merges params' do
stub_request(:get, "https://#{public_ipv4}/?key=value&key2=value2")
.with(headers: {host: 'www.example.com'}).to_return(status: 200, body: 'response body')
stub_request(:get, 'https://www.example.com/?key=value&key2=value2')
.to_return(status: 200, body: 'response body')
uri = URI('https://www.example.com/?key=value')
response, url = described_class.fetch_once(uri, public_ipv4.to_s, :get, params: {'key2' => 'value2'})
expect(response.code).to eq('200')
expect(response.body).to eq('response body')
expect(url).to be_nil
end

it 'does not use tls for http urls', only: true do
expect(::Net::HTTP).to receive(:start).with(public_ipv4.to_s, 80, use_ssl: false)
it 'does not use tls for http urls' do
expect(Net::HTTP).to receive(:start).with('www.example.com', 80, hash_including(use_ssl: false))
described_class.fetch_once(URI('http://www.example.com'), public_ipv4.to_s, :get, {})
end

it 'uses tls for https urls' do
expect(::Net::HTTP).to receive(:start).with(public_ipv4.to_s, 443, use_ssl: true)
expect(Net::HTTP).to receive(:start).with('www.example.com', 443, hash_including(use_ssl: true))
described_class.fetch_once(URI('https://www.example.com'), public_ipv4.to_s, :get, {})
end
end

describe 'with_forced_hostname' do
it 'sets the value for the block and clear it afterwards' do
expect(Thread.current[described_class::FIBER_HOSTNAME_KEY]).to be_nil
described_class.with_forced_hostname('test') do
expect(Thread.current[described_class::FIBER_HOSTNAME_KEY]).to eq('test')
end
expect(Thread.current[described_class::FIBER_HOSTNAME_KEY]).to be_nil
end

it 'clears the value even if an exception is raised' do
expect(Thread.current[described_class::FIBER_HOSTNAME_KEY]).to be_nil
expect do
described_class.with_forced_hostname('test') do
expect(Thread.current[described_class::FIBER_HOSTNAME_KEY]).to eq('test')
raise StandardError
end
end.to raise_error(StandardError)
expect(Thread.current[described_class::FIBER_HOSTNAME_KEY]).to be_nil
end
end

describe 'validate_request' do
it 'disallows header names with newlines and carriage returns' do
expect do
@@ -191,8 +168,8 @@
end

describe 'integration tests' do
# To test the SSLSocket patching logic (and hit 100% code coverage), we need to make a real connection to a
# TLS-enabled server. To do this we create a private key and certificate, spin up a web server in
# To hit 100% code coverage, we need to make a real connection to a TLS-enabled server.
# To do this we create a private key and certificate, spin up a web server in
# a thread (serving traffic on localhost), and make a request to the server. This requires several things:
# 1) creating a custom trust store with our certificate and using that for validation
# 2) allowing (non-mocked) network connections
@@ -244,7 +221,7 @@ def inject_custom_trust_store(*certificates)
store.add_cert(certificate)
end

expect(::Net::HTTP).to receive(:start).exactly(certificates.length).times
expect(Net::HTTP).to receive(:start).exactly(certificates.length).times
.and_wrap_original do |orig, *args, &block|
args.last[:cert_store] = store # Inject our custom trust store
orig.call(*args, &block)
@@ -452,22 +429,17 @@ def inject_custom_trust_store(*certificates)
end

it 'fails if there are too many redirects' do
stub_request(:get, "https://#{public_ipv4}").with(headers: {host: 'www.example.com'})
.to_return(status: 301, headers: {location: private_ipv4})
resolver = proc { [public_ipv4] }
stub_request(:get, 'https://www.example.com').to_return(status: 301, headers: {location: 'https://example2.com'})
expect do
described_class.get('https://www.example.com', resolver: resolver, max_redirects: 0)
described_class.get('https://www.example.com', max_redirects: 0)
end.to raise_error(described_class::TooManyRedirects)
end

it 'returns the last response if there are too many redirects and unfollowed redirects are allowed' do
stub_request(:get, "https://#{public_ipv4}").with(headers: {host: 'www.example.com'})
.to_return(status: 301, headers: {location: 'https://www.example2.com'})
resolver = proc { [public_ipv4] }
stub_request(:get, 'https://www.example.com').to_return(status: 301, headers: {location: 'https://www.example2.com'})
response =
described_class.get(
'https://www.example.com',
resolver: resolver,
allow_unfollowed_redirects: true,
max_redirects: 0
)
@@ -476,23 +448,15 @@ def inject_custom_trust_store(*certificates)
end

it 'fails if the redirected url is not in the scheme whitelist' do
stub_request(:put, "https://#{public_ipv4}").with(headers: {host: 'www.example.com'})
.to_return(status: 301, headers: {location: 'ftp://www.example.com'})
resolver = proc { [public_ipv4] }
stub_request(:put, 'https://www.example.com').to_return(status: 301, headers: {location: 'ftp://www.example.com'})
expect do
described_class.put('https://www.example.com', resolver: resolver)
described_class.put('https://www.example.com')
end.to raise_error(described_class::InvalidUriScheme)
end

it 'fails if the redirected url has no public ip address' do
stub_request(:delete, "https://#{public_ipv4}").with(headers: {host: 'www.example.com'})
.to_return(status: 301, headers: {location: 'https://www.example2.com'})
resolver = proc do |hostname|
[{
'www.example.com' => public_ipv4,
'www.example2.com' => private_ipv6
}[hostname]]
end
stub_request(:delete, 'https://www.example.com').to_return(status: 301, headers: {location: 'https://www.example2.com'})
resolver = proc { [private_ipv6] }
expect do
described_class.delete('https://www.example.com', resolver: resolver)
end.to raise_error(described_class::PrivateIPAddress)
@@ -512,32 +476,18 @@ def inject_custom_trust_store(*certificates)
end

it 'follows redirects and succeed on a public hostname' do
stub_request(:post, "https://#{public_ipv4}/path?key=value").with(headers: {host: 'www.example.com'})
.to_return(status: 301, headers: {location: 'https://www.example2.com/path2?key2=value2'})
stub_request(:post, "https://[#{public_ipv6}]/path2?key2=value2")
.with(headers: {host: 'www.example2.com'}).to_return(status: 200, body: 'response body')
resolver = proc do |hostname|
[{
'www.example.com' => public_ipv4,
'www.example2.com' => public_ipv6
}[hostname]]
end
response = described_class.post('https://www.example.com/path?key=value', resolver: resolver)
stub_request(:post, 'https://www.example.com/path?key=value').to_return(status: 301, headers: {location: 'https://www.example2.com/path2?key2=value2'})
stub_request(:post, 'https://www.example2.com/path2?key2=value2').to_return(status: 200, body: 'response body')
response = described_class.post('https://www.example.com/path?key=value')
expect(response.code).to eq('200')
expect(response.body).to eq('response body')
end

it 'follows relative redirects and succeed' do
stub_request(:post, "https://#{public_ipv4}/path?key=value").with(headers: {host: 'www.example.com'})
.to_return(status: 301, headers: {location: '/path2?key2=value2'})
stub_request(:post, "https://#{public_ipv4}/path2?key2=value2")
.with(headers: {host: 'www.example.com'}).to_return(status: 200, body: 'response body')
resolver = proc do |hostname|
[{
'www.example.com' => public_ipv4
}[hostname]]
end
response = described_class.post('https://www.example.com/path?key=value', resolver: resolver)
stub_request(:post, 'https://www.example.com/path?key=value').to_return(status: 301,
headers: {location: '/path2?key2=value2'})
stub_request(:post, 'https://www.example.com/path2?key2=value2').to_return(status: 200, body: 'response body')
response = described_class.post('https://www.example.com/path?key=value')
expect(response.code).to eq('200')
expect(response.body).to eq('response body')
end
13 changes: 7 additions & 6 deletions ssrf_filter.gemspec
Original file line number Diff line number Diff line change
@@ -8,21 +8,22 @@ Gem::Specification.new do |gem|
gem.platform = Gem::Platform::RUBY
gem.version = SsrfFilter::VERSION
gem.authors = ['Arkadiy Tetelman']
gem.required_ruby_version = '>= 2.6.0'
gem.required_ruby_version = '>= 2.7.0'
gem.summary = 'A gem that makes it easy to prevent server side request forgery (SSRF) attacks'
gem.description = gem.summary
gem.homepage = 'https://github.com/arkadiyt/ssrf_filter'
gem.license = 'MIT'
gem.files = Dir['lib/**/*.rb']
gem.metadata = {'rubygems_mfa_required' => 'true'}

gem.add_development_dependency('bundler-audit', '~> 0.9.1')
gem.add_development_dependency('base64', '~> 0.2.0') # For ruby >= 3.4
gem.add_development_dependency('bundler-audit', '~> 0.9.2')
gem.add_development_dependency('pry-byebug')
gem.add_development_dependency('rspec', '~> 3.12.0')
gem.add_development_dependency('rubocop', '~> 1.35.0')
gem.add_development_dependency('rubocop-rspec', '~> 2.12.1')
gem.add_development_dependency('rspec', '~> 3.13.0')
gem.add_development_dependency('rubocop', '~> 1.68.0')
gem.add_development_dependency('rubocop-rspec', '~> 3.2.0')
gem.add_development_dependency('simplecov', '~> 0.22.0')
gem.add_development_dependency('simplecov-lcov', '~> 0.8.0')
gem.add_development_dependency('webmock', '>= 3.18.0')
gem.add_development_dependency('webmock', '>= 3.24.0')
gem.add_development_dependency('webrick')
end

0 comments on commit 1062abc

Please sign in to comment.