mirror of
https://github.com/zammad/zammad
synced 2026-05-24 09:48:36 +00:00
Fixes #6056 - Allow wildcards in no_proxy configuration
This commit is contained in:
Mantas Masalskis
parent
bc94856d28
commit
aeab3788fe
4 changed files with 288 additions and 51 deletions
|
|
@ -230,6 +230,29 @@
|
|||
],
|
||||
"note": ""
|
||||
},
|
||||
{
|
||||
"warning_type": "SSL Verification Bypass",
|
||||
"warning_code": 71,
|
||||
"fingerprint": "539fc6f1e51adf3d01c8f72a88f7964878a9dd4f7bc059433557143d69a2461f",
|
||||
"check_name": "SSLVerify",
|
||||
"message": "SSL certificate verification was bypassed",
|
||||
"file": "lib/user_agent.rb",
|
||||
"line": 58,
|
||||
"link": "https://brakemanscanner.org/docs/warning_types/ssl_verification_bypass/",
|
||||
"code": "UserAgent::HttpClient.get_client(uri, options).new(uri.host, uri.port).verify_mode = OpenSSL::SSL::VERIFY_NONE",
|
||||
"render_path": null,
|
||||
"location": {
|
||||
"type": "method",
|
||||
"class": "UserAgent",
|
||||
"method": "self.get_http"
|
||||
},
|
||||
"user_input": null,
|
||||
"confidence": "High",
|
||||
"cwe_id": [
|
||||
295
|
||||
],
|
||||
"note": ""
|
||||
},
|
||||
{
|
||||
"warning_type": "SQL Injection",
|
||||
"warning_code": 0,
|
||||
|
|
@ -508,29 +531,6 @@
|
|||
],
|
||||
"note": ""
|
||||
},
|
||||
{
|
||||
"warning_type": "SSL Verification Bypass",
|
||||
"warning_code": 71,
|
||||
"fingerprint": "a777ed8060d76f50abcc211a4b998a0c7c5d4429b2ca2a275a6b577132e21c69",
|
||||
"check_name": "SSLVerify",
|
||||
"message": "SSL certificate verification was bypassed",
|
||||
"file": "lib/user_agent.rb",
|
||||
"line": 82,
|
||||
"link": "https://brakemanscanner.org/docs/warning_types/ssl_verification_bypass/",
|
||||
"code": "(Net::HTTP.Proxy($1, $2, ((options[\"proxy_username\"] or Setting.get(\"proxy_username\")) or nil), ((options[\"proxy_password\"] or Setting.get(\"proxy_password\")) or nil)).new(uri.host, uri.port) or Net::HTTP.new(uri.host, uri.port)).verify_mode = OpenSSL::SSL::VERIFY_NONE",
|
||||
"render_path": null,
|
||||
"location": {
|
||||
"type": "method",
|
||||
"class": "UserAgent",
|
||||
"method": "self.get_http"
|
||||
},
|
||||
"user_input": null,
|
||||
"confidence": "High",
|
||||
"cwe_id": [
|
||||
295
|
||||
],
|
||||
"note": "SSL Verification can already be requested from callers. The default value should be switched to true at some point."
|
||||
},
|
||||
{
|
||||
"warning_type": "Cross-Site Scripting",
|
||||
"warning_code": 2,
|
||||
|
|
|
|||
|
|
@ -41,34 +41,9 @@ class UserAgent
|
|||
end
|
||||
|
||||
def self.get_http(uri, options)
|
||||
|
||||
proxy = options['proxy'] || Setting.get('proxy')
|
||||
proxy_no = options['proxy_no'] || Setting.get('proxy_no') || ''
|
||||
proxy_no = proxy_no.split(',').map(&:strip) || []
|
||||
proxy_no.push('localhost', '127.0.0.1', '::1')
|
||||
if proxy.present? && proxy_no.exclude?(uri.host.downcase)
|
||||
if proxy =~ %r{^(.+?):(.+?)$}
|
||||
proxy_host = $1
|
||||
proxy_port = $2
|
||||
end
|
||||
|
||||
if proxy_host.blank? || proxy_port.blank?
|
||||
raise "Invalid proxy address: #{proxy} - expect e.g. proxy.example.com:3128"
|
||||
end
|
||||
|
||||
proxy_username = options['proxy_username'] || Setting.get('proxy_username')
|
||||
if proxy_username.blank?
|
||||
proxy_username = nil
|
||||
end
|
||||
proxy_password = options['proxy_password'] || Setting.get('proxy_password')
|
||||
if proxy_password.blank?
|
||||
proxy_password = nil
|
||||
end
|
||||
|
||||
http = Net::HTTP::Proxy(proxy_host, proxy_port, proxy_username, proxy_password).new(uri.host, uri.port)
|
||||
else
|
||||
http = Net::HTTP.new(uri.host, uri.port)
|
||||
end
|
||||
http = UserAgent::HttpClient
|
||||
.get_client(uri, options)
|
||||
.new(uri.host, uri.port)
|
||||
|
||||
# Defaults raised for slow links (e.g. OAuth to external IdPs); override globally via ENV, per-request via options. See https://github.com/zammad/zammad/issues/5991
|
||||
http.open_timeout = options[:open_timeout] || ENV.fetch('ZAMMAD_HTTP_OPEN_TIMEOUT', 30).to_i
|
||||
|
|
|
|||
80
lib/user_agent/http_client.rb
Normal file
80
lib/user_agent/http_client.rb
Normal file
|
|
@ -0,0 +1,80 @@
|
|||
# Copyright (C) 2012-2026 Zammad Foundation, https://zammad-foundation.org/
|
||||
|
||||
class UserAgent::HttpClient
|
||||
HOST_PORT_REGEXP = %r{^(?:\[(.+?)\]|(.+?)):(\d+)$}
|
||||
NO_PROXY_DEFAULT = %w[localhost 127.0.0.1 ::1].freeze
|
||||
|
||||
def initialize(uri, options)
|
||||
@uri = uri
|
||||
@options = options
|
||||
@proxy = fetch_proxy_configuration
|
||||
end
|
||||
|
||||
def client
|
||||
proxy? ? build_proxy : Net::HTTP
|
||||
end
|
||||
|
||||
def self.get_client(...)
|
||||
new(...).client
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
attr_reader :uri, :options, :proxy
|
||||
|
||||
def proxy?
|
||||
proxy && url_use_proxy?
|
||||
end
|
||||
|
||||
def url_use_proxy?
|
||||
host = uri.hostname.downcase
|
||||
|
||||
proxy_no.none? { url_use_proxy_single?(it, host) }
|
||||
end
|
||||
|
||||
def url_use_proxy_single?(no_proxy_entry, host)
|
||||
return true if no_proxy_entry.start_with?('*.') && host.end_with?(no_proxy_entry[1..])
|
||||
|
||||
host == no_proxy_entry
|
||||
end
|
||||
|
||||
def build_proxy
|
||||
Net::HTTP::Proxy(proxy[:host], proxy[:port], proxy_username, proxy_password)
|
||||
end
|
||||
|
||||
def fetch_proxy_configuration
|
||||
value = proxy_address
|
||||
|
||||
return if value.blank?
|
||||
|
||||
host, port = value
|
||||
.match(HOST_PORT_REGEXP)
|
||||
&.then do
|
||||
[it[1] || it[2], it[3]]
|
||||
end
|
||||
|
||||
if host.blank? || port.blank?
|
||||
raise "Invalid proxy address: #{value} - expect e.g. proxy.example.com:3128"
|
||||
end
|
||||
|
||||
{ host:, port: }
|
||||
end
|
||||
|
||||
def proxy_address
|
||||
options['proxy'] || Setting.get('proxy')
|
||||
end
|
||||
|
||||
def proxy_username
|
||||
(options['proxy_username'] || Setting.get('proxy_username')).presence
|
||||
end
|
||||
|
||||
def proxy_password
|
||||
(options['proxy_password'] || Setting.get('proxy_password')).presence
|
||||
end
|
||||
|
||||
def proxy_no
|
||||
given_proxy_no = options['proxy_no'] || Setting.get('proxy_no') || ''
|
||||
|
||||
NO_PROXY_DEFAULT + given_proxy_no.split(',').map { it.strip.downcase }
|
||||
end
|
||||
end
|
||||
182
spec/lib/user_agent/http_client_spec.rb
Normal file
182
spec/lib/user_agent/http_client_spec.rb
Normal file
|
|
@ -0,0 +1,182 @@
|
|||
# Copyright (C) 2012-2026 Zammad Foundation, https://zammad-foundation.org/
|
||||
|
||||
require 'rails_helper'
|
||||
|
||||
RSpec.describe UserAgent::HttpClient do
|
||||
let(:instance) { described_class.new(uri, options) }
|
||||
let(:uri) { URI('https://example.com/test') }
|
||||
let(:options) { {} }
|
||||
|
||||
describe '#client' do
|
||||
shared_examples 'returns the direct client' do
|
||||
it 'returns the direct client' do
|
||||
expect(instance.client).not_to be_proxy_class
|
||||
end
|
||||
end
|
||||
|
||||
shared_examples 'returns the proxy client' do
|
||||
it 'returns the proxy client' do
|
||||
expect(instance.client).to be_proxy_class
|
||||
end
|
||||
end
|
||||
|
||||
context 'without proxy configuration' do
|
||||
include_examples 'returns the direct client'
|
||||
end
|
||||
|
||||
context 'with valid proxy configuration' do
|
||||
let(:options) { { 'proxy' => 'proxy.example.com:3128' } }
|
||||
|
||||
include_examples 'returns the proxy client'
|
||||
|
||||
context 'when the address is loopback' do
|
||||
let(:uri) { URI('https://localhost/test') }
|
||||
|
||||
include_examples 'returns the direct client'
|
||||
end
|
||||
|
||||
context 'when the address is in proxy_no list' do
|
||||
let(:options) { super().merge('proxy_no' => 'example.com') }
|
||||
|
||||
include_examples 'returns the direct client'
|
||||
end
|
||||
|
||||
context 'when the address is a subdomain of a no-proxy entry starting with a wildcard' do
|
||||
let(:uri) { URI('https://sub.example.com/test') }
|
||||
let(:options) { super().merge('proxy_no' => '*.example.com') }
|
||||
|
||||
include_examples 'returns the direct client'
|
||||
end
|
||||
|
||||
context 'when the address is a subdomain of a no-proxy entry starting with a dot' do
|
||||
let(:uri) { URI('https://sub.example.com/test') }
|
||||
let(:options) { super().merge('proxy_no' => '.example.com') }
|
||||
|
||||
include_examples 'returns the proxy client'
|
||||
end
|
||||
|
||||
context 'when the address looks like a multi-level wildcard no-proxy entry' do
|
||||
let(:uri) { URI('https://sub.other.example.com/test') }
|
||||
let(:options) { super().merge('proxy_no' => 'sub.*.example.com') }
|
||||
|
||||
include_examples 'returns the proxy client'
|
||||
end
|
||||
|
||||
context 'when proxy is IPv6' do
|
||||
let(:options) { { 'proxy' => '[2001:db8::1]:3128' } }
|
||||
|
||||
include_examples 'returns the proxy client'
|
||||
end
|
||||
end
|
||||
|
||||
context 'with invalid proxy configuration' do
|
||||
let(:options) { { 'proxy' => 'invalid_proxy_format' } }
|
||||
|
||||
it 'raises an error' do
|
||||
expect { instance.client }.to raise_error(%r{Invalid proxy address})
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe '#proxy' do
|
||||
context 'when given via options' do
|
||||
let(:options) { { 'proxy' => 'proxy.example.com:3128' } }
|
||||
|
||||
it 'returns the given value' do
|
||||
expect(instance.client).to have_attributes(proxy_address: 'proxy.example.com', proxy_port: '3128')
|
||||
end
|
||||
end
|
||||
|
||||
context 'when given via settings' do
|
||||
before { Setting.set('proxy', 'example.com:3128') }
|
||||
|
||||
it 'returns the given value' do
|
||||
expect(instance.client).to have_attributes(proxy_address: 'example.com', proxy_port: '3128')
|
||||
end
|
||||
end
|
||||
|
||||
context 'when given via both options and settings' do
|
||||
let(:options) { { 'proxy' => 'proxy.example.com:3128' } }
|
||||
|
||||
before { Setting.set('proxy', 'example.com:3128') }
|
||||
|
||||
it 'returns the value from options' do
|
||||
expect(instance.client).to have_attributes(proxy_address: 'proxy.example.com', proxy_port: '3128')
|
||||
end
|
||||
end
|
||||
|
||||
context 'when proxy is IPv6' do
|
||||
let(:options) { { 'proxy' => '[2001:db8::1]:3128' } }
|
||||
|
||||
it 'returns the given value' do
|
||||
expect(instance.client).to have_attributes(proxy_address: '2001:db8::1', proxy_port: '3128')
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe '#proxy_username' do
|
||||
let(:options) { { 'proxy' => 'proxy.example.com:8080' } }
|
||||
|
||||
context 'when given via options' do
|
||||
let(:options) { super().merge('proxy_username' => 'user') }
|
||||
|
||||
it 'returns the given value' do
|
||||
expect(instance.client).to have_attributes(proxy_user: 'user')
|
||||
end
|
||||
end
|
||||
|
||||
context 'when given via settings' do
|
||||
before { Setting.set('proxy_username', 'user_setting') }
|
||||
|
||||
it 'returns the given value' do
|
||||
expect(instance.client).to have_attributes(proxy_user: 'user_setting')
|
||||
end
|
||||
end
|
||||
|
||||
context 'when given via both options and settings' do
|
||||
let(:options) { super().merge('proxy_username' => 'user') }
|
||||
|
||||
before { Setting.set('proxy_username', 'user_setting') }
|
||||
|
||||
it 'returns the value from options' do
|
||||
expect(instance.client).to have_attributes(proxy_user: 'user')
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe '#proxy_password' do
|
||||
let(:options) { { 'proxy' => 'proxy.example.com:8080' } }
|
||||
|
||||
context 'when given via options' do
|
||||
let(:options) { super().merge('proxy_password' => 'pass') }
|
||||
|
||||
it 'returns the given value' do
|
||||
expect(instance.client).to have_attributes(proxy_pass: 'pass')
|
||||
end
|
||||
end
|
||||
|
||||
context 'when given via settings' do
|
||||
before { Setting.set('proxy_password', 'pass_setting') }
|
||||
|
||||
it 'returns the given value' do
|
||||
expect(instance.client).to have_attributes(proxy_pass: 'pass_setting')
|
||||
end
|
||||
end
|
||||
|
||||
context 'when given via both options and settings' do
|
||||
let(:options) { super().merge('proxy_password' => 'pass') }
|
||||
|
||||
before { Setting.set('proxy_password', 'pass_setting') }
|
||||
|
||||
it 'returns the value from options' do
|
||||
expect(instance.client).to have_attributes(proxy_pass: 'pass')
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe '.get_client' do
|
||||
it 'returns the Net::HTTP instance' do
|
||||
expect(described_class.get_client(uri, options).new('example.com')).to be_a(Net::HTTP)
|
||||
end
|
||||
end
|
||||
end
|
||||
Loading…
Reference in a new issue