Skip to content

Commit

Permalink
Don't use underscores in HTTP headers
Browse files Browse the repository at this point in the history
Those are rejected by Gunicorn 22+, see [1] for details.
For older releases of Gunicorn we still unset the underscore notation.

[1] benoitc/gunicorn#2799
  • Loading branch information
evgeni authored and ekohl committed Sep 4, 2024
1 parent 935ddee commit 5aebaae
Show file tree
Hide file tree
Showing 5 changed files with 70 additions and 19 deletions.
5 changes: 4 additions & 1 deletion manifests/apache.pp
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,13 @@

# Pulp has a default for remote header. Here it's ensured that the end user
# can't send that header to spoof users.
$remote_user_environ_header = $pulpcore::remote_user_environ_name.regsubst(/^HTTP_/, '')
# The logic is only sufficient for headers with at most one underscore!
$remote_user_environ_header_underscore = $pulpcore::remote_user_environ_name.regsubst(/^HTTP_/, '')
$remote_user_environ_header = $remote_user_environ_header_underscore.regsubst('_', '-')

$api_default_request_headers = [
"unset ${remote_user_environ_header}",
"unset ${remote_user_environ_header_underscore}",
"set ${remote_user_environ_header} \"%{SSL_CLIENT_S_DN_CN}s\" env=SSL_CLIENT_S_DN_CN",
]

Expand Down
1 change: 1 addition & 0 deletions manifests/plugin/container.pp
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
# This is like pulpcore::apache's value, but slightly different
$api_default_request_headers = [
"unset ${pulpcore::apache::remote_user_environ_header}",
"unset ${pulpcore::apache::remote_user_environ_header_underscore}",
]

$context = {
Expand Down
60 changes: 46 additions & 14 deletions spec/classes/plugin_container_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,23 +4,25 @@
on_supported_os.each do |os, os_facts|
context "on #{os}" do
let(:facts) { os_facts }
let(:pre_condition) { 'include pulpcore' }
context "with default params" do
let(:pre_condition) { 'include pulpcore' }

it 'configures the plugin' do
is_expected.to compile.with_all_deps
is_expected.to contain_pulpcore__plugin('container')
.that_subscribes_to('Class[Pulpcore::Install]')
.that_notifies(['Class[Pulpcore::Database]', 'Class[Pulpcore::Service]'])
is_expected.to contain_package('pulpcore-plugin(container)')
is_expected.to contain_concat__fragment('plugin-container').with_content("\n# container plugin settings\nTOKEN_AUTH_DISABLED=True")
is_expected.to contain_pulpcore__apache__fragment('plugin-container')
is_expected.not_to contain_apache__vhost__fragment('pulpcore-http-plugin-container')
is_expected.to contain_apache__vhost__fragment('pulpcore-https-plugin-container')
.with_vhost('pulpcore-https')
.with_priority('10')
.with_content(<<APACHE_CONFIG)
it 'configures the plugin' do
is_expected.to compile.with_all_deps
is_expected.to contain_pulpcore__plugin('container')
.that_subscribes_to('Class[Pulpcore::Install]')
.that_notifies(['Class[Pulpcore::Database]', 'Class[Pulpcore::Service]'])
is_expected.to contain_package('pulpcore-plugin(container)')
is_expected.to contain_concat__fragment('plugin-container').with_content("\n# container plugin settings\nTOKEN_AUTH_DISABLED=True")
is_expected.to contain_pulpcore__apache__fragment('plugin-container')
is_expected.not_to contain_apache__vhost__fragment('pulpcore-http-plugin-container')
is_expected.to contain_apache__vhost__fragment('pulpcore-https-plugin-container')
.with_vhost('pulpcore-https')
.with_priority('10')
.with_content(<<APACHE_CONFIG)
<Location "/pulpcore_registry/v2/">
RequestHeader unset REMOTE-USER
RequestHeader unset REMOTE_USER
ProxyPass unix:///run/pulpcore-api.sock|http://pulpcore-api/v2/
ProxyPassReverse unix:///run/pulpcore-api.sock|http://pulpcore-api/v2/
Expand All @@ -29,6 +31,36 @@
ProxyPass /pulp/container/ unix:///run/pulpcore-content.sock|http://pulpcore-content/pulp/container/
ProxyPassReverse /pulp/container/ unix:///run/pulpcore-content.sock|http://pulpcore-content/pulp/container/
APACHE_CONFIG
end
end

context 'with API client auth common names' do
let(:pre_condition) do
<<~PUPPET
class {'pulpcore':
api_client_auth_cn_map => {'foreman.example.com' => 'admin'}
}
PUPPET
end
it 'configures the plugin' do
is_expected.to compile.with_all_deps
is_expected.to contain_apache__vhost__fragment('pulpcore-https-plugin-container')
.with_vhost('pulpcore-https')
.with_priority('10')
.with_content(<<APACHE_CONFIG)
<Location "/pulpcore_registry/v2/">
RequestHeader unset REMOTE-USER
RequestHeader unset REMOTE_USER
RequestHeader set REMOTE-USER "admin" "expr=%{SSL_CLIENT_S_DN_CN} == 'foreman.example.com'"
ProxyPass unix:///run/pulpcore-api.sock|http://pulpcore-api/v2/
ProxyPassReverse unix:///run/pulpcore-api.sock|http://pulpcore-api/v2/
</Location>
ProxyPass /pulp/container/ unix:///run/pulpcore-content.sock|http://pulpcore-content/pulp/container/
ProxyPassReverse /pulp/container/ unix:///run/pulpcore-content.sock|http://pulpcore-content/pulp/container/
APACHE_CONFIG
end
end
end
end
Expand Down
11 changes: 7 additions & 4 deletions spec/classes/pulpcore_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -131,8 +131,9 @@
'params' => {'timeout' => '600'},
}],
'request_headers' => [
'unset REMOTE-USER',
'unset REMOTE_USER',
'set REMOTE_USER "%{SSL_CLIENT_S_DN_CN}s" env=SSL_CLIENT_S_DN_CN',
'set REMOTE-USER "%{SSL_CLIENT_S_DN_CN}s" env=SSL_CLIENT_S_DN_CN',
],
}
])
Expand Down Expand Up @@ -356,8 +357,9 @@
</Location>
<Location "/pulp/api/v3">
RequestHeader unset REMOTE-USER
RequestHeader unset REMOTE_USER
RequestHeader set REMOTE_USER "%{SSL_CLIENT_S_DN_CN}s" env=SSL_CLIENT_S_DN_CN
RequestHeader set REMOTE-USER "%{SSL_CLIENT_S_DN_CN}s" env=SSL_CLIENT_S_DN_CN
ProxyPass unix:///run/pulpcore-api.sock|http://pulpcore-api/pulp/api/v3 timeout=600
ProxyPassReverse unix:///run/pulpcore-api.sock|http://pulpcore-api/pulp/api/v3
</Location>
Expand Down Expand Up @@ -520,9 +522,10 @@
'params' => {'timeout' => '600'},
}],
'request_headers' => [
'unset REMOTE-USER',
'unset REMOTE_USER',
'set REMOTE_USER "%{SSL_CLIENT_S_DN_CN}s" env=SSL_CLIENT_S_DN_CN',
'set REMOTE_USER "admin" "expr=%{SSL_CLIENT_S_DN_CN} == \'foreman.example.com\'"',
'set REMOTE-USER "%{SSL_CLIENT_S_DN_CN}s" env=SSL_CLIENT_S_DN_CN',
'set REMOTE-USER "admin" "expr=%{SSL_CLIENT_S_DN_CN} == \'foreman.example.com\'"',
],
}
])
Expand Down
12 changes: 12 additions & 0 deletions spec/support/acceptance/examples.rb
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,18 @@
its(:exit_status) { is_expected.to eq 0 }
end

describe curl_command("https://#{host_inventory['fqdn']}/pulp/api/v3/users/", cacert: "#{certdir}/ca-cert.pem", headers: {'remote-user': 'admin'}) do
its(:response_code) { is_expected.to eq(403) }
its(:body) { is_expected.to contain('Authentication credentials were not provided.') }
its(:exit_status) { is_expected.to eq 0 }
end

describe curl_command("https://#{host_inventory['fqdn']}/pulp/api/v3/users/", cacert: "#{certdir}/ca-cert.pem", headers: {'remote_user': 'admin'}) do
its(:response_code) { is_expected.to eq(403) }
its(:body) { is_expected.to contain('Authentication credentials were not provided.') }
its(:exit_status) { is_expected.to eq 0 }
end

describe curl_command("https://#{host_inventory['fqdn']}/pulp/api/v3/users/",
cacert: "#{certdir}/ca-cert.pem", key: "#{certdir}/client-key.pem", cert: "#{certdir}/client-cert.pem") do
its(:response_code) { is_expected.to eq(200) }
Expand Down

0 comments on commit 5aebaae

Please sign in to comment.