From 1431ab2c9fbebac416466a155dba130be1febfb5 Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Sun, 30 Sep 2018 23:40:58 +0200 Subject: [PATCH 1/6] Propery handle ${client_body,proxy}_temp_path The template already allowed setting it to false. This completes it. --- manifests/config.pp | 17 ++++++++++------- manifests/init.pp | 4 ++-- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/manifests/config.pp b/manifests/config.pp index 1ede16a78..0fb0523a1 100644 --- a/manifests/config.pp +++ b/manifests/config.pp @@ -182,15 +182,18 @@ group => $log_group, } - file {$client_body_temp_path: - ensure => directory, - owner => $daemon_user, + if $client_body_temp_path { + file {$client_body_temp_path: + ensure => directory, + owner => $daemon_user, + } } - - file {$proxy_temp_path: - ensure => directory, - owner => $daemon_user, + if $proxy_temp_path { + file {$proxy_temp_path: + ensure => directory, + owner => $daemon_user, + } } unless $confd_only { diff --git a/manifests/init.pp b/manifests/init.pp index c9de9a4c8..c8e7a28c2 100644 --- a/manifests/init.pp +++ b/manifests/init.pp @@ -26,7 +26,7 @@ # } class nginx ( ### START Nginx Configuration ### - $client_body_temp_path = $nginx::params::client_body_temp_path, + Variant[Stdlib::Absolutepath, Boolean] $client_body_temp_path = $nginx::params::client_body_temp_path, Boolean $confd_only = false, Boolean $confd_purge = false, $conf_dir = $nginx::params::conf_dir, @@ -45,7 +45,7 @@ Variant[String, Array[String]] $nginx_error_log = "${log_dir}/${nginx::params::nginx_error_log_file}", Nginx::ErrorLogSeverity $nginx_error_log_severity = 'error', $pid = $nginx::params::pid, - $proxy_temp_path = $nginx::params::proxy_temp_path, + Variant[Stdlib::Absolutepath, Boolean] $proxy_temp_path = $nginx::params::proxy_temp_path, $root_group = $nginx::params::root_group, $run_dir = $nginx::params::run_dir, $sites_available_owner = $nginx::params::sites_available_owner, From 1aedeca388e7a4623aff0fbd8f06fc6dd881ea3f Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Sun, 30 Sep 2018 23:47:18 +0200 Subject: [PATCH 2/6] Use more Puppet 4 types --- manifests/init.pp | 10 +++++----- manifests/resource/server.pp | 4 ++-- spec/classes/nginx_spec.rb | 2 +- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/manifests/init.pp b/manifests/init.pp index c8e7a28c2..ced1f1973 100644 --- a/manifests/init.pp +++ b/manifests/init.pp @@ -126,10 +126,10 @@ Array $proxy_hide_header = [], Array $proxy_pass_header = [], Array $proxy_ignore_header = [], - $sendfile = 'on', - String $server_tokens = 'on', - $spdy = 'off', - $http2 = 'off', + Enum['on', 'off'] $sendfile = 'on', + Enum['on', 'off'] $server_tokens = 'on', + Enum['on', 'off'] $spdy = 'off', + Enum['on', 'off'] $http2 = 'off', $ssl_stapling = 'off', $types_hash_bucket_size = '512', $types_hash_max_size = '1024', @@ -146,7 +146,7 @@ $package_name = $nginx::params::package_name, $package_source = 'nginx', $package_flavor = undef, - $manage_repo = $nginx::params::manage_repo, + Boolean $manage_repo = $nginx::params::manage_repo, Hash[String[1], String[1]] $mime_types = $nginx::params::mime_types, Optional[String] $repo_release = undef, $passenger_package_ensure = 'present', diff --git a/manifests/resource/server.pp b/manifests/resource/server.pp index 1ae123aab..9103bb501 100644 --- a/manifests/resource/server.pp +++ b/manifests/resource/server.pp @@ -156,7 +156,7 @@ Variant[Array, String] $ipv6_listen_ip = '::', Integer $ipv6_listen_port = 80, String $ipv6_listen_options = 'default ipv6only=on', - Optional[Hash] $add_header = undef, + Hash $add_header = {}, Boolean $ssl = false, Boolean $ssl_listen_option = true, Optional[Variant[String, Boolean]] $ssl_cert = undef, @@ -184,7 +184,7 @@ Optional[String] $ssl_trusted_cert = undef, Optional[Integer] $ssl_verify_depth = undef, String $spdy = $nginx::spdy, - $http2 = $nginx::http2, + Enum['on', 'off'] $http2 = $nginx::http2, Optional[String] $proxy = undef, Optional[String]$proxy_redirect = undef, String $proxy_read_timeout = $nginx::proxy_read_timeout, diff --git a/spec/classes/nginx_spec.rb b/spec/classes/nginx_spec.rb index 24e0e3838..7b017ea80 100644 --- a/spec/classes/nginx_spec.rb +++ b/spec/classes/nginx_spec.rb @@ -570,7 +570,7 @@ { title: 'should not set sendfile', attr: 'sendfile', - value: false, + value: 'off', notmatch: %r{sendfile} }, { From 64801475e166dd5f68da5001a3a702736886153c Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Sun, 30 Sep 2018 23:47:45 +0200 Subject: [PATCH 3/6] Strip line endings in mime.types This ensures there are no empty lines in the file. --- templates/conf.d/mime.types.epp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/templates/conf.d/mime.types.epp b/templates/conf.d/mime.types.epp index ea3cf60eb..f71022ecb 100644 --- a/templates/conf.d/mime.types.epp +++ b/templates/conf.d/mime.types.epp @@ -1,6 +1,6 @@ # MANAGED BY PUPPET types { -<% $nginx::mime_types.each |$key, $value| { %> +<% $nginx::mime_types.each |$key, $value| { -%> <%= $key %> <%= $value %>; -<% } %> +<% } -%> } From 369705eb8d556c5b1ee38d310a047c304e3c7728 Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Sun, 30 Sep 2018 23:48:16 +0200 Subject: [PATCH 4/6] Move error_log to the http context This aligns error_log and access_log --- spec/classes/nginx_spec.rb | 8 ++++---- templates/conf.d/nginx.conf.erb | 14 +++++++------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/spec/classes/nginx_spec.rb b/spec/classes/nginx_spec.rb index 7b017ea80..99f074e2c 100644 --- a/spec/classes/nginx_spec.rb +++ b/spec/classes/nginx_spec.rb @@ -433,22 +433,22 @@ title: 'should set error_log', attr: 'nginx_error_log', value: '/path/to/error.log', - match: 'error_log /path/to/error.log error;' + match: ' error_log /path/to/error.log error;' }, { title: 'should set multiple error_logs', attr: 'nginx_error_log', value: ['/path/to/error.log', 'syslog:server=localhost'], match: [ - 'error_log /path/to/error.log error;', - 'error_log syslog:server=localhost error;' + ' error_log /path/to/error.log error;', + ' error_log syslog:server=localhost error;' ] }, { title: 'should set error_log severity level', attr: 'nginx_error_log_severity', value: 'warn', - match: 'error_log /var/log/nginx/error.log warn;' + match: ' error_log /var/log/nginx/error.log warn;' }, { title: 'should set pid', diff --git a/templates/conf.d/nginx.conf.erb b/templates/conf.d/nginx.conf.erb index a1c35e211..49e8a9344 100644 --- a/templates/conf.d/nginx.conf.erb +++ b/templates/conf.d/nginx.conf.erb @@ -20,13 +20,6 @@ worker_rlimit_nofile <%= @worker_rlimit_nofile %>; <% if @pid -%> pid <%= @pid %>; <% end -%> -<% if @nginx_error_log.is_a?(Array) -%> - <%- @nginx_error_log.each do |log_item| -%> -error_log <%= log_item %> <%= @nginx_error_log_severity %>; - <%- end -%> -<% else -%> -error_log <%= @nginx_error_log %> <%= @nginx_error_log_severity %>; -<% end -%> <% if @nginx_cfg_prepend -%> <%- field_width = @nginx_cfg_prepend.inject(0) { |l,(k,v)| k.size > l ? k.size : l } -%> @@ -84,6 +77,13 @@ http { <% else -%> access_log <%= @http_access_log %><% if @http_format_log %> <%= @http_format_log%><% end %>; <% end -%> +<% if @nginx_error_log.is_a?(Array) -%> + <%- @nginx_error_log.each do |log_item| -%> + error_log <%= log_item %> <%= @nginx_error_log_severity %>; + <%- end -%> +<% else -%> + error_log <%= @nginx_error_log %> <%= @nginx_error_log_severity %>; +<% end -%> <% if @sendfile == 'on' -%> sendfile on; From 2d062ce784aaa146eace5bf1f5ca0f0d903f72da Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Sun, 30 Sep 2018 23:49:40 +0200 Subject: [PATCH 5/6] Remove double spacing This double spacing doesn't align to anything so it doesn't make sense. --- spec/classes/nginx_spec.rb | 20 ++++++++++---------- templates/conf.d/nginx.conf.erb | 10 +++++----- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/spec/classes/nginx_spec.rb b/spec/classes/nginx_spec.rb index 99f074e2c..567db3151 100644 --- a/spec/classes/nginx_spec.rb +++ b/spec/classes/nginx_spec.rb @@ -433,22 +433,22 @@ title: 'should set error_log', attr: 'nginx_error_log', value: '/path/to/error.log', - match: ' error_log /path/to/error.log error;' + match: ' error_log /path/to/error.log error;' }, { title: 'should set multiple error_logs', attr: 'nginx_error_log', value: ['/path/to/error.log', 'syslog:server=localhost'], match: [ - ' error_log /path/to/error.log error;', - ' error_log syslog:server=localhost error;' + ' error_log /path/to/error.log error;', + ' error_log syslog:server=localhost error;' ] }, { title: 'should set error_log severity level', attr: 'nginx_error_log_severity', value: 'warn', - match: ' error_log /var/log/nginx/error.log warn;' + match: ' error_log /var/log/nginx/error.log warn;' }, { title: 'should set pid', @@ -544,22 +544,22 @@ title: 'should set access_log', attr: 'http_access_log', value: '/path/to/access.log', - match: ' access_log /path/to/access.log;' + match: ' access_log /path/to/access.log;' }, { title: 'should set multiple access_logs', attr: 'http_access_log', value: ['/path/to/access.log', 'syslog:server=localhost'], match: [ - ' access_log /path/to/access.log;', - ' access_log syslog:server=localhost;' + ' access_log /path/to/access.log;', + ' access_log syslog:server=localhost;' ] }, { title: 'should set custom log format', attr: 'http_format_log', value: 'mycustomformat', - match: ' access_log /var/log/nginx/access.log mycustomformat;' + match: ' access_log /var/log/nginx/access.log mycustomformat;' }, { title: 'should set sendfile', @@ -1183,12 +1183,12 @@ it { is_expected.to contain_file('/foo/bar').with(ensure: 'directory') } it do is_expected.to contain_file('/etc/nginx/nginx.conf').with_content( - %r{access_log /foo/bar/access.log;} + %r{access_log /foo/bar/access.log;} ) end it do is_expected.to contain_file('/etc/nginx/nginx.conf').with_content( - %r{error_log /foo/bar/error.log error;} + %r{error_log /foo/bar/error.log error;} ) end end diff --git a/templates/conf.d/nginx.conf.erb b/templates/conf.d/nginx.conf.erb index 49e8a9344..c2428c04f 100644 --- a/templates/conf.d/nginx.conf.erb +++ b/templates/conf.d/nginx.conf.erb @@ -72,21 +72,21 @@ http { <% end -%> <% if @http_access_log.is_a?(Array) -%> <%- @http_access_log.each do |log_item| -%> - access_log <%= log_item %><% if @http_format_log %> <%= @http_format_log%><% end %>; + access_log <%= log_item %><% if @http_format_log %> <%= @http_format_log%><% end %>; <%- end -%> <% else -%> - access_log <%= @http_access_log %><% if @http_format_log %> <%= @http_format_log%><% end %>; + access_log <%= @http_access_log %><% if @http_format_log %> <%= @http_format_log%><% end %>; <% end -%> <% if @nginx_error_log.is_a?(Array) -%> <%- @nginx_error_log.each do |log_item| -%> - error_log <%= log_item %> <%= @nginx_error_log_severity %>; + error_log <%= log_item %> <%= @nginx_error_log_severity %>; <%- end -%> <% else -%> - error_log <%= @nginx_error_log %> <%= @nginx_error_log_severity %>; + error_log <%= @nginx_error_log %> <%= @nginx_error_log_severity %>; <% end -%> <% if @sendfile == 'on' -%> - sendfile on; + sendfile on; <%- if @http_tcp_nopush == 'on' -%> tcp_nopush on; <%- end -%> From 8c8fae010ec8d198a18eaa520f492d24d28d1733 Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Mon, 1 Oct 2018 00:05:06 +0200 Subject: [PATCH 6/6] Fix logging setup on Debian This mitigates DSA-3701 / CVE-2016-1247 and matches the default setup on Debian. https://www.debian.org/security/2016/dsa-3701 https://security-tracker.debian.org/tracker/CVE-2016-1247 --- manifests/config.pp | 3 ++- manifests/init.pp | 7 ++++--- manifests/params.pp | 22 +++++++++++++++------- 3 files changed, 21 insertions(+), 11 deletions(-) diff --git a/manifests/config.pp b/manifests/config.pp index 0fb0523a1..2d5d5e94d 100644 --- a/manifests/config.pp +++ b/manifests/config.pp @@ -29,6 +29,7 @@ $global_group = $nginx::global_group $global_mode = $nginx::global_mode $log_dir = $nginx::log_dir + $log_user = $nginx::log_user $log_group = $nginx::log_group $log_mode = $nginx::log_mode $http_access_log = $nginx::http_access_log @@ -178,7 +179,7 @@ file { $log_dir: ensure => directory, mode => $log_mode, - owner => $daemon_user, + owner => $log_user, group => $log_group, } diff --git a/manifests/init.pp b/manifests/init.pp index ced1f1973..22c4c09b4 100644 --- a/manifests/init.pp +++ b/manifests/init.pp @@ -37,9 +37,10 @@ $global_owner = $nginx::params::global_owner, $global_group = $nginx::params::global_group, $global_mode = $nginx::params::global_mode, - $log_dir = $nginx::params::log_dir, - $log_group = $nginx::params::log_group, - $log_mode = '0750', + Stdlib::Absolutepath $log_dir = $nginx::params::log_dir, + String $log_user = $nginx::params::log_user, + String $log_group = $nginx::params::log_group, + Stdlib::Filemode $log_mode = $nginx::params::log_mode, Variant[String, Array[String]] $http_access_log = "${log_dir}/${nginx::params::http_access_log_file}", $http_format_log = undef, Variant[String, Array[String]] $nginx_error_log = "${log_dir}/${nginx::params::nginx_error_log_file}", diff --git a/manifests/params.pp b/manifests/params.pp index d69b36b9a..cf6f3d403 100644 --- a/manifests/params.pp +++ b/manifests/params.pp @@ -7,13 +7,15 @@ ### Operating System Configuration ## This is my hacky... no hiera system. Oh well. :) $_module_defaults = { - 'conf_dir' => '/etc/nginx', - 'daemon_user' => 'nginx', - 'pid' => '/var/run/nginx.pid', - 'root_group' => 'root', - 'log_dir' => '/var/log/nginx', - 'log_group' => 'root', - 'run_dir' => '/var/nginx', + 'conf_dir' => '/etc/nginx', + 'daemon_user' => 'nginx', + 'pid' => '/var/run/nginx.pid', + 'root_group' => 'root', + 'log_dir' => '/var/log/nginx', + 'log_user' => 'nginx', + 'log_group' => 'root', + 'log_mode' => '0755', + 'run_dir' => '/var/nginx', 'package_name' => 'nginx', 'manage_repo' => false, 'mime_types' => { @@ -109,12 +111,16 @@ $_module_os_overrides = { 'manage_repo' => true, 'daemon_user' => 'www-data', + 'log_user' => 'root', 'log_group' => 'adm', + 'log_mode' => '0755', } } else { $_module_os_overrides = { 'daemon_user' => 'www-data', + 'log_user' => 'root', 'log_group' => 'adm', + 'log_mode' => '0755', } } } @@ -178,7 +184,9 @@ ### Referenced Variables $conf_dir = $_module_parameters['conf_dir'] $log_dir = $_module_parameters['log_dir'] + $log_user = $_module_parameters['log_user'] $log_group = $_module_parameters['log_group'] + $log_mode = $_module_parameters['log_mode'] $run_dir = $_module_parameters['run_dir'] $temp_dir = '/tmp' $pid = $_module_parameters['pid']