Revision 9690f3ae
Added by Ewoud Kohl van Wijngaarden over 2 years ago
| app/controllers/concerns/foreman/controller/smart_proxy_auth.rb | ||
|---|---|---|
|
SmartProxy.unscoped.with_features(*features)
|
||
|
end
|
||
|
|
||
|
if !Setting[:restrict_registered_smart_proxies] || auth_smart_proxy(allowed_smart_proxies, Setting[:require_ssl_smart_proxies])
|
||
|
if !Setting[:restrict_registered_smart_proxies] || auth_smart_proxy(allowed_smart_proxies)
|
||
|
set_admin_user
|
||
|
return true
|
||
|
end
|
||
| ... | ... | |
|
end
|
||
|
|
||
|
# Filter requests to only permit from hosts with a registered smart proxy
|
||
|
# Uses rDNS of the request to match proxy hostnames
|
||
|
def auth_smart_proxy(proxies = SmartProxy.unscoped.all, require_cert = true)
|
||
|
def auth_smart_proxy(proxies = SmartProxy.unscoped.all)
|
||
|
request_hosts = nil
|
||
|
if request.ssl?
|
||
|
# If we have the client certficate in the request environment we can extract the dn and sans from there
|
||
| ... | ... | |
|
else
|
||
|
logger.warn "SSL cert has not been verified (#{client_certificate.verify}) - request from #{request.remote_ip}, #{client_certificate.subject}"
|
||
|
end
|
||
|
elsif require_cert
|
||
|
logger.warn "No SSL cert with CN supplied - request from #{request.remote_ip}"
|
||
|
else
|
||
|
logger.warn "No SSL cert supplied, falling back to reverse DNS for hostname lookup - request from #{request.remote_ip}"
|
||
|
request_hosts = Resolv.new.getnames(request.remote_ip)
|
||
|
logger.warn "No SSL cert with CN supplied - request from #{request.remote_ip}"
|
||
|
end
|
||
|
elsif SETTINGS[:require_ssl]
|
||
|
logger.warn "SSL is required - request from #{request.remote_ip}"
|
||
| app/registries/foreman/settings/auth.rb | ||
|---|---|---|
|
description: N_('Only known Smart Proxies may access features that use Smart Proxy authentication'),
|
||
|
default: true,
|
||
|
full_name: N_('Restrict registered smart proxies'))
|
||
|
setting('require_ssl_smart_proxies',
|
||
|
type: :boolean,
|
||
|
description: N_('Client SSL certificates are used to identify Smart Proxies (:require_ssl should also be enabled)'),
|
||
|
default: true,
|
||
|
full_name: N_('Require SSL for smart proxies'))
|
||
|
setting('trusted_hosts',
|
||
|
type: :array,
|
||
|
description: N_('List of hostnames, IPv4, IPv6 addresses or subnets to be trusted in addition to Smart Proxies for access to fact/report importers and ENC output'),
|
||
| db/migrate/20220111110149_drop_require_ssl_smart_proxies_setting.rb | ||
|---|---|---|
|
class DropRequireSslSmartProxiesSetting < ActiveRecord::Migration[6.0]
|
||
|
def up
|
||
|
Setting.where(name: 'require_ssl_smart_proxies').delete_all
|
||
|
end
|
||
|
end
|
||
| test/controllers/api/v2/config_reports_controller_test.rb | ||
|---|---|---|
|
assert_response :unprocessable_entity
|
||
|
end
|
||
|
|
||
|
test 'when ":restrict_registered_smart_proxies" is false, HTTP requests should be able to create a report' do
|
||
|
Setting[:restrict_registered_smart_proxies] = false
|
||
|
SETTINGS[:require_ssl] = false
|
||
|
|
||
|
Resolv.any_instance.stubs(:getnames).returns(['else.where'])
|
||
|
post :create, params: { :config_report => create_a_puppet_transaction_report }
|
||
|
assert_nil @controller.detected_proxy
|
||
|
assert_response :created
|
||
|
end
|
||
|
|
||
|
test 'hosts with a registered smart proxy on should create a report successfully' do
|
||
|
test 'hosts with a registered smart proxy and SSL cert should create a report successfully' do
|
||
|
Setting[:restrict_registered_smart_proxies] = true
|
||
|
Setting[:require_ssl_smart_proxies] = false
|
||
|
|
||
|
stub_smart_proxy_v2_features
|
||
|
proxy = smart_proxies(:puppetmaster)
|
||
|
as_admin { proxy.update_attribute(:url, 'http://configreports.foreman') }
|
||
|
host = URI.parse(proxy.url).host
|
||
|
Resolv.any_instance.stubs(:getnames).returns([host])
|
||
|
post :create, params: { :config_report => create_a_puppet_transaction_report }
|
||
|
assert_equal proxy, @controller.detected_proxy
|
||
|
assert_response :created
|
||
|
end
|
||
|
|
||
|
test 'hosts without a registered smart proxy on should not be able to create a report' do
|
||
|
Setting[:restrict_registered_smart_proxies] = true
|
||
|
Setting[:require_ssl_smart_proxies] = false
|
||
|
|
||
|
Resolv.any_instance.stubs(:getnames).returns(['another.host'])
|
||
|
post :create, params: { :config_report => create_a_puppet_transaction_report }
|
||
|
assert_response :forbidden
|
||
|
end
|
||
|
|
||
|
test 'hosts with a registered smart proxy and SSL cert should create a report successfully' do
|
||
|
Setting[:restrict_registered_smart_proxies] = true
|
||
|
Setting[:require_ssl_smart_proxies] = true
|
||
|
as_admin { proxy.update_attribute(:url, 'https://puppet.example.com') }
|
||
|
|
||
|
@request.env['HTTPS'] = 'on'
|
||
|
@request.env['SSL_CLIENT_S_DN'] = 'CN=else.where'
|
||
|
@request.env['SSL_CLIENT_S_DN'] = 'CN=puppet.example.com'
|
||
|
@request.env['SSL_CLIENT_VERIFY'] = 'SUCCESS'
|
||
|
post :create, params: { :config_report => create_a_puppet_transaction_report }
|
||
|
assert_equal proxy, @controller.detected_proxy
|
||
|
assert_response :created
|
||
|
end
|
||
|
|
||
|
test 'hosts without a registered smart proxy but with an SSL cert should not be able to create a report' do
|
||
|
Setting[:restrict_registered_smart_proxies] = true
|
||
|
Setting[:require_ssl_smart_proxies] = true
|
||
|
|
||
|
@request.env['HTTPS'] = 'on'
|
||
|
@request.env['SSL_CLIENT_S_DN'] = 'CN=another.host'
|
||
| ... | ... | |
|
|
||
|
test 'hosts with an unverified SSL cert should not be able to create a report' do
|
||
|
Setting[:restrict_registered_smart_proxies] = true
|
||
|
Setting[:require_ssl_smart_proxies] = true
|
||
|
|
||
|
@request.env['HTTPS'] = 'on'
|
||
|
@request.env['SSL_CLIENT_S_DN'] = 'CN=else.where'
|
||
| ... | ... | |
|
assert_response :forbidden
|
||
|
end
|
||
|
|
||
|
test 'when "require_ssl_smart_proxies" and "require_ssl" are true, HTTP requests should not be able to create a report' do
|
||
|
test 'when "require_ssl" is true, HTTP requests should not be able to create a report' do
|
||
|
Setting[:restrict_registered_smart_proxies] = true
|
||
|
Setting[:require_ssl_smart_proxies] = true
|
||
|
SETTINGS[:require_ssl] = true
|
||
|
|
||
|
Resolv.any_instance.stubs(:getnames).returns(['else.where'])
|
||
| ... | ... | |
|
assert_response :forbidden
|
||
|
end
|
||
|
|
||
|
test 'when "require_ssl_smart_proxies" is true and "require_ssl" is false, HTTP requests should be able to create reports' do
|
||
|
# since require_ssl_smart_proxies is only applicable to HTTPS connections, both should be set
|
||
|
test 'when "require_ssl" is false, hosts without a registered smart proxy on should not be able to create a report' do
|
||
|
Setting[:restrict_registered_smart_proxies] = true
|
||
|
SETTINGS[:require_ssl] = false
|
||
|
|
||
|
Resolv.any_instance.stubs(:getnames).returns(['another.host'])
|
||
|
post :create, params: { :config_report => create_a_puppet_transaction_report }
|
||
|
assert_response :forbidden
|
||
|
end
|
||
|
|
||
|
test 'when "require_ssl" is false, HTTP requests should be able to create reports' do
|
||
|
Setting[:restrict_registered_smart_proxies] = true
|
||
|
Setting[:require_ssl_smart_proxies] = true
|
||
|
SETTINGS[:require_ssl] = false
|
||
|
|
||
|
Resolv.any_instance.stubs(:getnames).returns(['else.where'])
|
||
| test/controllers/api/v2/hosts_controller_test.rb | ||
|---|---|---|
|
assert_response :success
|
||
|
end
|
||
|
|
||
|
test 'hosts with a registered smart proxy on should import facts successfully' do
|
||
|
stub_smart_proxy_v2_features
|
||
|
proxy = smart_proxies(:puppetmaster)
|
||
|
proxy.update_attribute(:url, 'https://factsimporter.foreman')
|
||
|
|
||
|
User.current = users(:one) # use an unprivileged user, not apiadmin
|
||
|
Setting[:restrict_registered_smart_proxies] = true
|
||
|
Setting[:require_ssl_smart_proxies] = false
|
||
|
|
||
|
host = URI.parse(proxy.url).host
|
||
|
Resolv.any_instance.stubs(:getnames).returns([host])
|
||
|
post :facts, params: { :name => hostname, :facts => facts }
|
||
|
assert_equal proxy, @controller.detected_proxy
|
||
|
assert_response :success
|
||
|
end
|
||
|
|
||
|
test 'hosts without a registered smart proxy on should not be able to import facts' do
|
||
|
User.current = users(:one) # use an unprivileged user, not apiadmin
|
||
|
Setting[:restrict_registered_smart_proxies] = true
|
||
|
Setting[:require_ssl_smart_proxies] = false
|
||
|
|
||
|
Resolv.any_instance.stubs(:getnames).returns(['another.host'])
|
||
|
post :facts, params: { :name => hostname, :facts => facts }
|
||
|
assert_response :forbidden
|
||
|
end
|
||
| ... | ... | |
|
test 'hosts with a registered smart proxy and SSL cert should import facts successfully' do
|
||
|
User.current = users(:one) # use an unprivileged user, not apiadmin
|
||
|
Setting[:restrict_registered_smart_proxies] = true
|
||
|
Setting[:require_ssl_smart_proxies] = true
|
||
|
|
||
|
@request.env['HTTPS'] = 'on'
|
||
|
@request.env['SSL_CLIENT_S_DN'] = 'CN=else.where'
|
||
| ... | ... | |
|
test 'hosts without a registered smart proxy but with an SSL cert should not be able to import facts' do
|
||
|
User.current = users(:one) # use an unprivileged user, not apiadmin
|
||
|
Setting[:restrict_registered_smart_proxies] = true
|
||
|
Setting[:require_ssl_smart_proxies] = true
|
||
|
|
||
|
@request.env['HTTPS'] = 'on'
|
||
|
@request.env['SSL_CLIENT_S_DN'] = 'CN=another.host'
|
||
| ... | ... | |
|
test 'hosts with an unverified SSL cert should not be able to import facts' do
|
||
|
User.current = users(:one) # use an unprivileged user, not apiadmin
|
||
|
Setting[:restrict_registered_smart_proxies] = true
|
||
|
Setting[:require_ssl_smart_proxies] = true
|
||
|
|
||
|
@request.env['HTTPS'] = 'on'
|
||
|
@request.env['SSL_CLIENT_S_DN'] = 'CN=secure.host'
|
||
| ... | ... | |
|
assert_response :forbidden
|
||
|
end
|
||
|
|
||
|
test 'when "require_ssl_smart_proxies" and "require_ssl" are true, HTTP requests should not be able to import facts' do
|
||
|
test 'when "require_ssl" is true, HTTP requests should not be able to import facts' do
|
||
|
User.current = users(:one) # use an unprivileged user, not apiadmin
|
||
|
Setting[:restrict_registered_smart_proxies] = true
|
||
|
Setting[:require_ssl_smart_proxies] = true
|
||
|
SETTINGS[:require_ssl] = true
|
||
|
|
||
|
Resolv.any_instance.stubs(:getnames).returns(['else.where'])
|
||
| ... | ... | |
|
assert_response :forbidden
|
||
|
end
|
||
|
|
||
|
test 'when "require_ssl_smart_proxies" is true and "require_ssl" is false, HTTP requests should be able to import facts' do
|
||
|
test 'when "require_ssl" is false, HTTP requests should be able to import facts' do
|
||
|
User.current = users(:one) # use an unprivileged user, not apiadmin
|
||
|
# since require_ssl_smart_proxies is only applicable to HTTPS connections, both should be set
|
||
|
Setting[:restrict_registered_smart_proxies] = true
|
||
|
Setting[:require_ssl_smart_proxies] = true
|
||
|
SETTINGS[:require_ssl] = false
|
||
|
|
||
|
Resolv.any_instance.stubs(:getnames).returns(['else.where'])
|
||
| test/fixtures/settings.yml | ||
|---|---|---|
|
category: Setting
|
||
|
default: "true"
|
||
|
description: "Only known Smart Proxies may access features that use Smart Proxy authentication"
|
||
|
attribute28:
|
||
|
name: require_ssl_smart_proxies
|
||
|
category: Setting
|
||
|
default: "true"
|
||
|
description: "Client SSL certificates are used to identify Smart Proxies (:require_ssl should also be enabled)"
|
||
|
attribute29:
|
||
|
name: ssl_client_dn_env
|
||
|
category: Setting
|
||
Also available in: Unified diff
Fixes #34236 - Drop require_ssl_smart_proxies setting
This defaults to true and setting it to false can create security
problems. Mandating client SSL certificates creates a more secure
environment.
Previously when require_ssl_smart_proxies was false, reverse DNS was
used. This code is dropped as it is insecure. Requests are now denied.