Skip to content

Commit 33802c9

Browse files
LukasAudLukasAud
committed
(maint) Hardening manifests and tasks
This PR aims to implement some changes that ensure no malformed commands are passed through to the system. Certain commands were left undivided as the commands did not get correctly interpreted. Primarily, the commands targeted were the ones related to Open3 and exec.
1 parent 1798668 commit 33802c9

31 files changed

+148
-116
lines changed

‎manifests/compose.pp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@
7171
}
7272

7373
if $facts['os']['family'] == 'windows' {
74-
$docker_download_command = "if (Invoke-WebRequest ${docker_compose_url} ${proxy_opt} -UseBasicParsing -OutFile \"${docker_compose_location_versioned}\") { exit 0 } else { exit 1}" # lint:ignore:140chars
74+
$docker_download_command = ['if', '(Invoke-WebRequest', $docker_compose_url, $proxy_opt, '-UseBasicParsing', '-OutFile', "\"${docker_compose_location_versioned}\") { exit 0 } else { exit 1}"] # lint:ignore:140chars
7575

7676
exec { "Install Docker Compose ${version}":
7777
command => template('docker/windows/download_docker_compose.ps1.erb'),
@@ -92,7 +92,7 @@
9292
exec { "Install Docker Compose ${version}":
9393
path => '/usr/bin/',
9494
cwd => '/tmp',
95-
command => "curl -s -S -L ${proxy_opt} ${docker_compose_url} -o ${docker_compose_location_versioned}",
95+
command => ['curl', '-s', '-S', '-L', $proxy_opt, $docker_compose_url, '-o', $docker_compose_location_versioned],
9696
creates => $docker_compose_location_versioned,
9797
require => Package['curl'],
9898
}

‎manifests/install.pp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,8 +113,9 @@
113113
}
114114
}
115115

116+
$fail_restart_command = ['SC.exe', 'failure', 'Docker', 'reset= 432000', 'actions= restart/30000/restart/60000/restart/60000']
116117
exec { 'service-restart-on-failure':
117-
command => 'SC.exe failure Docker reset= 432000 actions= restart/30000/restart/60000/restart/60000',
118+
command => $fail_restart_command,
118119
refreshonly => true,
119120
logoutput => true,
120121
provider => powershell,

‎manifests/machine.pp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@
5656
}
5757

5858
if $facts['os']['family'] == 'windows' {
59-
$docker_download_command = "if (Invoke-WebRequest ${docker_machine_url} ${proxy_opt} -UseBasicParsing -OutFile \"${docker_machine_location_versioned}\") { exit 0 } else { exit 1}" # lint:ignore:140chars
59+
$docker_download_command = ['if', '(Invoke-WebRequest', $docker_machine_url, $proxy_opt, '-UseBasicParsing', '-OutFile', "\"${docker_machine_location_versioned}\") { exit 0 } else { exit 1}"] # lint:ignore:140chars
6060

6161
exec { "Install Docker Machine ${version}":
6262
command => template('docker/windows/download_docker_machine.ps1.erb'),
@@ -74,10 +74,11 @@
7474
ensure_packages(['curl'])
7575
}
7676

77+
$install_command = ['curl', '-s', '-S', '-L', $proxy_opt, $docker_machine_url, '-o', $docker_machine_location_versioned] # lint:ignore:140chars
7778
exec { "Install Docker Machine ${version}":
7879
path => '/usr/bin/',
7980
cwd => '/tmp',
80-
command => "curl -s -S -L ${proxy_opt} ${docker_machine_url} -o ${docker_machine_location_versioned}",
81+
command => $install_command,
8182
creates => $docker_machine_location_versioned,
8283
require => Package['curl'],
8384
}

‎manifests/plugin.pp

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -68,8 +68,8 @@
6868
}
6969
)
7070

71-
$exec_install = "${docker_command} install ${docker_plugin_install_flags}"
72-
$unless_install = "${docker_command} ls --format='{{.PluginReference}}' | grep -w ${plugin_name}"
71+
$exec_install = [$docker_command, 'install', $docker_plugin_install_flags]
72+
$unless_install = [$docker_command, 'ls', "--format='{{.PluginReference}}' | grep -w ${plugin_name}"]
7373

7474
exec { "plugin install ${plugin_name}":
7575
command => $exec_install,
@@ -85,8 +85,8 @@
8585
}
8686
)
8787

88-
$exec_rm = "${docker_command} rm ${docker_plugin_remove_flags}"
89-
$onlyif_rm = "${docker_command} ls --format='{{.PluginReference}}' | grep -w ${plugin_name}"
88+
$exec_rm = [$docker_command, 'rm', $docker_plugin_remove_flags]
89+
$onlyif_rm = [$docker_command, 'ls', "--format='{{.PluginReference}}' | grep -w ${plugin_name}"]
9090

9191
exec { "plugin remove ${plugin_name}":
9292
command => $exec_rm,
@@ -105,8 +105,8 @@
105105
}
106106
)
107107

108-
$exec_enable = "${docker_command} enable ${docker_plugin_enable_flags}"
109-
$onlyif_enable = "${docker_command} ls -f enabled=false --format='{{.PluginReference}}' | grep -w ${plugin_name}"
108+
$exec_enable = [$docker_command, 'enable', $docker_plugin_enable_flags]
109+
$onlyif_enable = [$docker_command, 'ls', '-f', "enabled=false --format='{{.PluginReference}}' | grep -w ${plugin_name}"]
110110

111111
exec { "plugin enable ${plugin_name}":
112112
command => $exec_enable,
@@ -116,12 +116,15 @@
116116
onlyif => $onlyif_enable,
117117
}
118118
} elsif $enabled == false {
119+
$else_command = [$docker_command, 'disable', $plugin_name]
120+
$else_unless = [$docker_command, 'ls', '-f', "enabled=false --format='{{.PluginReference}}' | grep -w ${plugin_name}"]
121+
119122
exec { "disable ${plugin_name}":
120-
command => "${docker_command} disable ${plugin_name}",
123+
command => $else_command,
121124
environment => 'HOME=/root',
122125
path => ['/bin', '/usr/bin',],
123126
timeout => 0,
124-
unless => "${docker_command} ls -f enabled=false --format='{{.PluginReference}}' | grep -w ${plugin_name}",
127+
unless => $else_unless,
125128
}
126129
}
127130
}

‎manifests/registry.pp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@
125125
# server may be an URI, which can contain /
126126
$server_strip = regsubst($server, '[/:]', '_', 'G')
127127
$passfile = "${::docker_user_temp_path}/registry-auth-puppet_receipt_${server_strip}_${local_user}"
128-
$_auth_command = "if (-not (${auth_cmd})) { Remove-Item -Path ${passfile} -Force -Recurse -EA SilentlyContinue; exit 1 } else { exit 0 }" # lint:ignore:140chars
128+
$_auth_command = ["if (-not (${auth_cmd}))", "{ Remove-Item -Path ${passfile}", '-Force', '-Recurse', '-EA', 'SilentlyContinue; exit 1 } else { exit 0 }'] # lint:ignore:140chars
129129

130130
if $ensure == 'absent' {
131131
file { $passfile:

‎manifests/run.pp

Lines changed: 27 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -373,8 +373,10 @@
373373
}
374374

375375
if $restart_on_unhealthy {
376+
$unhealthy_command = [$docker_command, 'restart', $sanitised_title]
377+
376378
exec { "Restart unhealthy container ${title} with docker":
377-
command => "${docker_command} restart ${sanitised_title}",
379+
command => $unhealthy_command,
378380
onlyif => $restart_check,
379381
environment => $exec_environment,
380382
path => $exec_path,
@@ -385,18 +387,24 @@
385387

386388
if $restart {
387389
if $ensure == 'absent' {
390+
$restart_stop_command = [$docker_command, 'stop', "--time=${stop_wait_time}", $sanitised_title]
391+
$restart_stop_onlyif = [$docker_command, 'inspect', $sanitised_title]
392+
388393
exec { "stop ${title} with docker":
389-
command => "${docker_command} stop --time=${stop_wait_time} ${sanitised_title}",
390-
onlyif => "${docker_command} inspect ${sanitised_title}",
394+
command => $restart_stop_command,
395+
onlyif => $restart_stop_onlyif,
391396
environment => $exec_environment,
392397
path => $exec_path,
393398
provider => $exec_provider,
394399
timeout => $exec_timeout,
395400
}
396401

402+
$restart_remove_command = [$docker_command, 'rm', '-v', $sanitised_title]
403+
$restart_remove_onlyif = [$docker_command, 'inspect', $sanitised_title]
404+
397405
exec { "remove ${title} with docker":
398-
command => "${docker_command} rm -v ${sanitised_title}",
399-
onlyif => "${docker_command} inspect ${sanitised_title}",
406+
command => $restart_remove_command,
407+
onlyif => $restart_remove_onlyif,
400408
environment => $exec_environment,
401409
path => $exec_path,
402410
provider => $exec_provider,
@@ -432,17 +440,19 @@
432440
}
433441

434442
if $running == false {
443+
$running_stop_command = [$docker_command, 'stop', "--time=${stop_wait_time}", $sanitised_title]
435444
exec { "stop ${title} with docker":
436-
command => "${docker_command} stop --time=${stop_wait_time} ${sanitised_title}",
445+
command => $running_stop_command,
437446
onlyif => $container_running_check,
438447
environment => $exec_environment,
439448
path => $exec_path,
440449
provider => $exec_provider,
441450
timeout => $exec_timeout,
442451
}
443452
} else {
453+
$running_start_command = [$docker_command, 'start', $sanitised_title]
444454
exec { "start ${title} with docker":
445-
command => "${docker_command} start ${sanitised_title}",
455+
command => $running_start_command,
446456
unless => $container_running_check,
447457
environment => $exec_environment,
448458
path => $exec_path,
@@ -517,9 +527,12 @@
517527

518528
if $ensure == 'absent' {
519529
if $facts['os']['family'] == 'windows' {
530+
$absent_stop_command = [$docker_command, 'stop', "--time=${stop_wait_time}", $sanitised_title]
531+
$absent_stop_onlyif = [$docker_command, 'inspect', $sanitised_title]
532+
520533
exec { "stop container ${service_prefix}${sanitised_title}":
521-
command => "${docker_command} stop --time=${stop_wait_time} ${sanitised_title}",
522-
onlyif => "${docker_command} inspect ${sanitised_title}",
534+
command => $absent_stop_command,
535+
onlyif => $absent_stop_onlyif,
523536
environment => $exec_environment,
524537
path => $exec_path,
525538
provider => $exec_provider,
@@ -536,9 +549,12 @@
536549
notify => Exec["remove container ${service_prefix}${sanitised_title}"],
537550
}
538551
}
552+
$absent_remove_command = [$docker_command, 'rm', '-v', $sanitised_title]
553+
$absent_remove_onlyif = [$docker_command, 'inspect', $sanitised_title]
554+
539555
exec { "remove container ${service_prefix}${sanitised_title}":
540-
command => "${docker_command} rm -v ${sanitised_title}",
541-
onlyif => "${docker_command} inspect ${sanitised_title}",
556+
command => $absent_remove_command,
557+
onlyif => $absent_remove_onlyif,
542558
environment => $exec_environment,
543559
path => $exec_path,
544560
refreshonly => true,

‎manifests/secrets.pp

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,8 @@
2727
}
2828
)
2929

30-
$exec_secret = "${docker_command} ${docker_secrets_flags}"
31-
$unless_secret = "${docker_command} inspect ${secret_name}"
30+
$exec_secret = [$docker_command, $docker_secrets_flags]
31+
$unless_secret = [$docker_command, 'inspect', $secret_name]
3232

3333
exec { "${title} docker secret create":
3434
command => $exec_secret,
@@ -38,9 +38,12 @@
3838
}
3939

4040
if $ensure == 'absent' {
41+
$absent_secret_command = [$docker_command, 'rm', $secret_name]
42+
$absent_secret_onlyif = [$docker_command, 'inspect', $secret_name]
43+
4144
exec { "${title} docker secret rm":
42-
command => "${docker_command} rm ${secret_name}",
43-
onlyif => "${docker_command} inspect ${secret_name}",
45+
command => $absent_secret_command,
46+
onlyif => $absent_secret_onlyif,
4447
path => ['/bin', '/usr/bin',],
4548
}
4649
}

‎manifests/services.pp

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -132,8 +132,8 @@
132132
}
133133
)
134134

135-
$exec_create = "${docker_command} create --name ${docker_service_create_flags}"
136-
$unless_create = "docker service ps ${service_name}"
135+
$exec_create = [$docker_command, 'create', '--name', $docker_service_create_flags]
136+
$unless_create = ['docker', 'service', 'ps', "${service_name}"]
137137

138138
exec { "${title} docker service create":
139139
command => $exec_create,
@@ -163,7 +163,7 @@
163163
}
164164
)
165165

166-
$exec_update = "${docker_command} update ${docker_service_flags}"
166+
$exec_update = [$docker_command, 'update', $docker_service_flags]
167167

168168
exec { "${title} docker service update":
169169
command => $exec_update,
@@ -182,7 +182,7 @@
182182
}
183183
)
184184

185-
$exec_scale = "${docker_command} scale ${service_name}=${replicas}"
185+
$exec_scale = [$docker_command, 'scale', "${service_name}=${replicas}"]
186186

187187
exec { "${title} docker service scale":
188188
command => $exec_scale,
@@ -194,9 +194,12 @@
194194
}
195195

196196
if $ensure == 'absent' {
197+
$service_command = ['docker', 'service', 'rm', $service_name]
198+
$service_onlyif = ['docker', 'service', 'ps', $service_name]
199+
197200
exec { "${title} docker service remove":
198-
command => "docker service rm ${service_name}",
199-
onlyif => "docker service ps ${service_name}",
201+
command => $service_command,
202+
onlyif => $service_onlyif,
200203
path => $exec_path,
201204
provider => $exec_provider,
202205
timeout => $exec_timeout,

‎manifests/stack.pp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@
5959
}
6060
)
6161

62-
$exec_stack = "${docker_command} deploy ${docker_stack_flags} ${stack_name}"
62+
$exec_stack = [$docker_command, 'deploy', $docker_stack_flags, $stack_name]
6363

6464
exec { "docker stack create ${stack_name}":
6565
command => $exec_stack,
@@ -70,8 +70,10 @@
7070
}
7171

7272
if $ensure == 'absent' {
73+
$destroy_command = [$docker_command, 'rm', $stack_name]
74+
7375
exec { "docker stack destroy ${stack_name}":
74-
command => "${docker_command} rm ${stack_name}",
76+
command => $destroy_command,
7577
onlyif => $check_stack,
7678
path => $exec_path,
7779
provider => $provider,

‎manifests/swarm.pp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@
117117
}
118118
)
119119

120-
$exec_init = "${docker_command} ${docker_swarm_init_flags}"
120+
$exec_init = [$docker_command, $docker_swarm_init_flags]
121121

122122
exec { 'Swarm init':
123123
command => $exec_init,
@@ -138,7 +138,7 @@
138138
}
139139
)
140140

141-
$exec_join = "${docker_command} ${docker_swarm_join_flags} ${manager_ip}"
141+
$exec_join = [$docker_command, $docker_swarm_join_flags, $manager_ip]
142142

143143
exec { 'Swarm join':
144144
command => $exec_join,

‎spec/shared_examples/compose.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@
6262
is_expected.to contain_exec("Install Docker Compose #{version}").with(
6363
'path' => '/usr/bin/',
6464
'cwd' => '/tmp',
65-
'command' => "curl -s -S -L #{proxy_opt} #{docker_compose_url} -o #{docker_compose_location_versioned}",
65+
'command' => ['curl', '-s', '-S', '-L', proxy_opt, docker_compose_url, '-o', docker_compose_location_versioned],
6666
'creates' => docker_compose_location_versioned,
6767
).that_requires(
6868
'Package[curl]',

‎spec/shared_examples/install.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,7 @@
141141

142142
it {
143143
is_expected.to contain_exec('service-restart-on-failure').with(
144-
'command' => 'SC.exe failure Docker reset= 432000 actions= restart/30000/restart/60000/restart/60000',
144+
'command' => ['SC.exe', 'failure', 'Docker', 'reset= 432000', 'actions= restart/30000/restart/60000/restart/60000'],
145145
'refreshonly' => true,
146146
'logoutput' => true,
147147
'provider' => 'powershell',

‎spec/shared_examples/machine.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@
6262
is_expected.to contain_exec("Install Docker Machine #{version}").with(
6363
'path' => '/usr/bin/',
6464
'cwd' => '/tmp',
65-
'command' => "curl -s -S -L #{proxy_opt} #{docker_machine_url} -o #{docker_machine_location_versioned}",
65+
'command' => ['curl', '-s', '-S', '-L', proxy_opt, docker_machine_url, '-o', docker_machine_location_versioned],
6666
'creates' => docker_machine_location_versioned,
6767
).that_requires(
6868
'Package[curl]',

‎spec/shared_examples/plugin.rb

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,8 @@
2424
'settings' => settings,
2525
)
2626

27-
exec_install = "#{docker_command} install #{docker_plugin_install_flags}"
28-
unless_install = "#{docker_command} ls --format='{{.PluginReference}}' | grep -w #{plugin_name}"
27+
exec_install = [docker_command, 'install', docker_plugin_install_flags]
28+
unless_install = [docker_command, 'ls', "--format='{{.PluginReference}}' | grep -w #{plugin_name}"]
2929

3030
it {
3131
is_expected.to contain_exec("plugin install #{plugin_name}").with(
@@ -43,8 +43,8 @@
4343
'force_remove' => force_remove,
4444
)
4545

46-
exec_rm = "#{docker_command} rm #{docker_plugin_remove_flags}"
47-
onlyif_rm = "#{docker_command} ls --format='{{.PluginReference}}' | grep -w #{plugin_name}"
46+
exec_rm = [docker_command, 'rm', docker_plugin_remove_flags]
47+
onlyif_rm = [docker_command, 'ls', "--format='{{.PluginReference}}' | grep -w #{plugin_name}"]
4848

4949
it {
5050
is_expected.to contain_exec("plugin remove #{plugin_name}").with(
@@ -64,8 +64,8 @@
6464
'timeout' => timeout,
6565
)
6666

67-
exec_enable = "#{docker_command} enable #{docker_plugin_enable_flags}"
68-
onlyif_enable = "#{docker_command} ls -f enabled=false --format='{{.PluginReference}}' | grep -w #{plugin_name}"
67+
exec_enable = [docker_command, 'enable', docker_plugin_enable_flags]
68+
onlyif_enable = [docker_command, 'ls', '-f', "enabled=false --format='{{.PluginReference}}' | grep -w #{plugin_name}"]
6969

7070
it {
7171
is_expected.to contain_exec("plugin enable #{plugin_name}").with(
@@ -77,13 +77,16 @@
7777
)
7878
}
7979
else
80+
else_command = [docker_command, 'disable', plugin_name]
81+
else_unless = [docker_command, 'ls', '-f', "enabled=false --format='{{.PluginReference}}' | grep -w #{plugin_name}"]
82+
8083
it {
8184
is_expected.to contain_exec("disable #{plugin_name}").with(
82-
'command' => "#{docker_command} disable #{plugin_name}",
85+
'command' => else_command,
8386
'environment' => 'HOME=/root',
8487
'path' => ['/bin', '/usr/bin'],
8588
'timeout' => 0,
86-
'unless' => "#{docker_command} ls -f enabled=false --format='{{.PluginReference}}' | grep -w #{plugin_name}",
89+
'unless' => else_unless,
8790
)
8891
}
8992
end

0 commit comments

Comments
 (0)