Skip to content

Commit

Permalink
Fixes #36969 - return the actual exit status of registration command
Browse files Browse the repository at this point in the history
Refactor registration command to enhance security
by saving the downloaded script to a temporary file before execution.
  • Loading branch information
nofaralfasi committed Dec 25, 2023
1 parent a1142ab commit b1fbd30
Show file tree
Hide file tree
Showing 4 changed files with 13 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ module Foreman::Controller::RegistrationCommands

def command
args_query = "?#{registration_args.to_query}"
"curl -sS #{insecure} '#{registration_url(@smart_proxy)}#{args_query if args_query != '?'}' #{command_headers} | bash"
"tmp=$(mktemp) && curl -o $tmp -sS #{insecure} '#{registration_url(@smart_proxy)}#{args_query if args_query != '?'}' #{command_headers} && bash $tmp; CODE=$?; rm -f $tmp; (exit $CODE)
"
end

def registration_args
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,11 @@ cat << EOF > $SSL_CA_CERT
<%= foreman_server_ca_cert %>
EOF

REG_TMP=$(mktemp)

cleanup_and_exit() {
rm -f $SSL_CA_CERT
rm -f $REG_TMP
exit $1
}

Expand Down Expand Up @@ -98,7 +101,7 @@ fi
<% end -%>

register_host() {
curl --silent --show-error --cacert $SSL_CA_CERT --request POST <%= @registration_url %> \
curl -o $REG_TMP --silent --show-error --cacert $SSL_CA_CERT --request POST <%= @registration_url %> \
<%= headers.join(' ') %> \
--data "host[name]=$(hostname --fqdn)" \
--data "host[build]=false" \
Expand All @@ -123,7 +126,7 @@ echo "#"
<% if plugin_present?('katello') && activation_keys.present? -%>
register_katello_host(){
UUID=$(subscription-manager identity | grep --max-count 1 --only-matching '\([[:xdigit:]]\{8\}-[[:xdigit:]]\{4\}-[[:xdigit:]]\{4\}-[[:xdigit:]]\{4\}-[[:xdigit:]]\{12\}\)')
curl --silent --show-error --cacert $KATELLO_SERVER_CA_CERT --request POST "<%= @registration_url %>" \
curl -o $REG_TMP --silent --show-error --cacert $KATELLO_SERVER_CA_CERT --request POST "<%= @registration_url %>" \
<%= headers.join(' ') %> \
--data "uuid=$UUID" \
--data "host[build]=false" \
Expand All @@ -147,9 +150,9 @@ subscription-manager register <%= '--force' if truthy?(@force) %> \
--org='<%= @organization.label if @organization %>' \
--activationkey='<%= activation_keys %>' || <%= truthy?(@ignore_subman_errors) ? 'true' : 'cleanup_and_exit 1' %>

register_katello_host | bash
register_katello_host && bash $REG_TMP
<% else -%>
register_host | bash
register_host && bash $REG_TMP
<% end -%>

cleanup_and_exit
cleanup_and_exit $?
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ class Api::V2::RegistrationCommandsControllerTest < ActionController::TestCase
assert_response :success
response = ActiveSupport::JSON.decode(@response.body)['registration_command']

assert_includes response, "curl -sS 'http://test.host/register'"
assert_includes response, "curl -o $tmp -sS 'http://test.host/register'"
assert_includes response, "-H 'Authorization: Bearer"
end

Expand Down Expand Up @@ -50,7 +50,7 @@ class Api::V2::RegistrationCommandsControllerTest < ActionController::TestCase
assert_response :success

response = ActiveSupport::JSON.decode(@response.body)['registration_command']
assert_includes response, "curl -sS --insecure '#{proxy.url}/register'"
assert_includes response, "curl -o $tmp -sS --insecure '#{proxy.url}/register'"
end

test 'os without host_init_config template' do
Expand Down
2 changes: 1 addition & 1 deletion test/controllers/registration_commands_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ class RegistrationCommandsControllerTest < ActionController::TestCase
post :create, params: params, session: set_session_user
command = JSON.parse(@response.body)['command']

assert_includes command, "curl -sS --insecure '#{proxy.url}/register"
assert_includes command, "curl -o $tmp -sS --insecure '#{proxy.url}/register"
refute command.include?('smart_proxy_id')
refute command.include?('insecure=true')
refute command.include?('jwt_expiration')
Expand Down

0 comments on commit b1fbd30

Please sign in to comment.